Re: [PATCH v1 1/2] usb: chipidea: use of extcon framework to work for non OTG case

[Peter Chen <peter.chen@nxp.com>]

On 19-10-21 19:16:53, Igor Opaniuk wrote:
> From: Stefan Agner <stefan.agner@toradex.com>
> 
> The existing usage of extcon in chipidea driver freezes the kernel
> presumably due to OTGSC register access.
> 
> Prevent accessing any OTG registers for SoC with dual role devices
> but no true OTG support. Use the flag CI_HDRC_DUAL_ROLE_NOT_OTG for
> those devices and in case extcon is present, do the role switch
> using extcon only.

Hi Igor & Stefan,

I have several questions about the problem you met:
- Which vendor's controller you have used?
- What do you mean "no true OTG"? Does it mean no "OTGSC" register?

>  	if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
>  		ret = ci_hdrc_host_init(ci);
> @@ -1145,8 +1208,18 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  
>  	if (!ci_otg_is_fsm_mode(ci)) {
>  		/* only update vbus status for peripheral */
> -		if (ci->role == CI_ROLE_GADGET)
> -			ci_handle_vbus_change(ci);
> +		if (dr_mode == USB_DR_MODE_PERIPHERAL) {
> +			usb_gadget_vbus_connect(&ci->gadget);

We only use ci->role at runtime, since it has already considered the
dts setting, kernel configuration and hardware setting.

If your controller doesn't otgsc register, but do need to support
role switch, you may enhance the function ci_get_role

Peter

> +		} else if (ci->role == CI_ROLE_GADGET) {
> +			struct ci_hdrc_cable *vbus = &ci->platdata->vbus_extcon;
> +
> +			/* Use vbus state from extcon if provided */
> +			if (!IS_ERR(vbus->edev) &&
> +			    extcon_get_state(vbus->edev, EXTCON_USB))
> +				usb_gadget_vbus_connect(&ci->gadget);
> +			else
> +				ci_handle_vbus_change(ci);
> +		}
>  
>  		ret = ci_role_start(ci, ci->role);
>  		if (ret) {
> @@ -1161,10 +1234,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto stop;
>  
> -	ret = ci_extcon_register(ci);
> -	if (ret)
> -		goto stop;
> -
>  	if (ci->supports_runtime_pm) {
>  		pm_runtime_set_active(&pdev->dev);
>  		pm_runtime_enable(&pdev->dev);
> -- 
> 2.17.1
> 

-- 

Thanks,
Peter Chen