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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191022020748.GA26973@b29397-desktop>
Date:   Tue, 22 Oct 2019 02:11:19 +0000
From:   Peter Chen <peter.chen@....com>
To:     Igor Opaniuk <igor.opaniuk@...il.com>
CC:     "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        Marcel Ziswiler <marcel.ziswiler@...adex.com>,
        Philippe Schenker <philippe.schenker@...adex.com>,
        Stefan Agner <stefan.agner@...adex.com>,
        Max Krummenacher <max.krummenacher@...adex.com>,
        Oleksandr Suvorov <oleksandr.suvorov@...adex.com>,
        Sanchayan Maity <maitysanchayan@...il.com>,
        Igor Opaniuk <igor.opaniuk@...adex.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 1/2] usb: chipidea: use of extcon framework to work for
 non OTG case

On 19-10-21 19:16:53, Igor Opaniuk wrote:
> From: Stefan Agner <stefan.agner@...adex.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