linux-kernel - Re: [PATCH 1/1] usb: dwc3: Do not get extcon device when usb-role-switch is used

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: <20221108020208.y3hrbpaxmzke2lms@synopsys.com>
Date:   Tue, 8 Nov 2022 02:02:19 +0000
From:   Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To:     Janne Grunau <j@...nau.net>
CC:     "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        Sven Peter <sven@...npeter.de>,
        "stable@...nel.org" <stable@...nel.org>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andrey Smirnov <andrew.smirnov@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] usb: dwc3: Do not get extcon device when
 usb-role-switch is used

Hi,

On Sun, Nov 06, 2022, Janne Grunau wrote:
> The change breaks device tree based platforms with PHY device and use
> usb-role-switch instead of an extcon switch. extcon_find_edev_by_node()
> will return EPROBE_DEFER if it can not find a device so probing without
> an extcon device will be deferred indefinitely. Fix this by
> explicitly checking for usb-role-switch.
> At least the out-of-tree USB3 support on Apple silicon based platforms
> using dwc3 with tipd USB Type-C and PD controller is affected by this
> issue.
> 
> Fixes: d182c2e1bc92 ("usb: dwc3: Don't switch OTG -> peripheral if extcon is present")
> Cc: stable@...nel.org
> Signed-off-by: Janne Grunau <j@...nau.net>
> ---
>  drivers/usb/dwc3/core.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index c0e7c76dc5c8..1f348bc867c2 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1710,6 +1710,16 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
>  	if (device_property_read_string(dev, "linux,extcon-name", &name) == 0)
>  		return extcon_get_extcon_dev(name);
>  
> +	/*
> +	 * Check explicitly if "usb-role-switch" is used since
> +	 * extcon_find_edev_by_node() can not be used to check the absence of

can not -> can't or cannot

> +	 * an extcon device. In the absence of an device it will always return

"a" device,

> +	 * EPROBE_DEFER.
> +	 */
> +	if (IS_ENABLED(CONFIG_USB_ROLE_SWITCH) &&
> +	    device_property_read_bool(dev, "usb-role-switch"))
> +		return NULL;
> +
>  	/*
>  	 * Try to get an extcon device from the USB PHY controller's "port"
>  	 * node. Check if it has the "port" node first, to avoid printing the
> -- 
> 2.37.3
> 

Looks like the "port" node check from c824c73a5e08 ("usb: dwc3: drd:
Avoid error when extcon is missing") did not account for this platform
setup.

This looks fine to me. Should we cleanup the incomplete check and stale
comment right after this too (as a separate patch)?

Thanks,
Thinh