[PATCH] usb: xhci-dwc3: Bail out if "dr_mode" is "peripheral"

[PATCH] usb: xhci-dwc3: Bail out if "dr_mode" is "peripheral"

[Siddharth Vadapalli]

Since the compatible "snps,dwc3" can be specified in the device-tree for
the Designware USB Controller configured for not only Host and OTG modes
of operation, but also for Peripheral mode of operation, bail out if
"dr_mode" is "peripheral".

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---

Hello,

This patch is based on commit
448add55297 Merge branch 'master' of https://source.denx.de/u-boot/custodians/u-boot-pmic
of the master branch of U-Boot.

Regards,
Siddharth.

 drivers/usb/host/xhci-dwc3.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/xhci-dwc3.c b/drivers/usb/host/xhci-dwc3.c
index e3e0ceff43e..7b1311503fe 100644
--- a/drivers/usb/host/xhci-dwc3.c
+++ b/drivers/usb/host/xhci-dwc3.c
@@ -208,6 +208,9 @@ static int xhci_dwc3_probe(struct udevice *dev)
 	writel(reg, &dwc3_reg->g_usb2phycfg[0]);
 
 	dr_mode = usb_get_dr_mode(dev_ofnode(dev));
+	if (dr_mode == USB_DR_MODE_PERIPHERAL)
+		return -ENODEV;
+
 	if (dr_mode == USB_DR_MODE_OTG &&
 	    dev_read_bool(dev, "usb-role-switch")) {
 		dr_mode = usb_get_role_switch_default_mode(dev_ofnode(dev));
-- 
2.43.0

Re: [PATCH] usb: xhci-dwc3: Bail out if "dr_mode" is "peripheral"

[Roger Quadros]

Hi Siddharth,

On 06/12/2024 13:04, Siddharth Vadapalli wrote:
> Since the compatible "snps,dwc3" can be specified in the device-tree for
> the Designware USB Controller configured for not only Host and OTG modes
> of operation, but also for Peripheral mode of operation, bail out if
> "dr_mode" is "peripheral".
> 
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
> 
> Hello,
> 
> This patch is based on commit
> 448add55297 Merge branch 'master' of https://source.denx.de/u-boot/custodians/u-boot-pmic
> of the master branch of U-Boot.
> 
> Regards,
> Siddharth.
> 
>  drivers/usb/host/xhci-dwc3.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-dwc3.c b/drivers/usb/host/xhci-dwc3.c
> index e3e0ceff43e..7b1311503fe 100644
> --- a/drivers/usb/host/xhci-dwc3.c
> +++ b/drivers/usb/host/xhci-dwc3.c
> @@ -208,6 +208,9 @@ static int xhci_dwc3_probe(struct udevice *dev)
>  	writel(reg, &dwc3_reg->g_usb2phycfg[0]);
>  
>  	dr_mode = usb_get_dr_mode(dev_ofnode(dev));
> +	if (dr_mode == USB_DR_MODE_PERIPHERAL)
> +		return -ENODEV;
> +
>  	if (dr_mode == USB_DR_MODE_OTG &&
>  	    dev_read_bool(dev, "usb-role-switch")) {
>  		dr_mode = usb_get_role_switch_default_mode(dev_ofnode(dev));

Although this proposal was suggested in [1] and later clarified in
[2] I will summarize the discussion here.

This patch fix is not sufficient for all cases as the xhci-dwc3.c
driver will still be probed if dr_mode is OTG or HOST along with
other platform drivers if enabled in config.

The only current solution is not not enable CONFIG_USB_XHCI_DWC3
together with another platform driver config (e.g. CONFIG_USB_DWC3_GENERIC)
if snps,dwc3 should not be managed by xhci-dwc3.c.

[1] - https://lore.kernel.org/all/unf757pwgah5ylkcfdumtdo5zs2ny3nxct5u26t6up7aldqim2@mxhhtb53gw4x/
[2] - https://lore.kernel.org/all/a9c458f8-68e5-448b-9977-e7bf37b00d35@kernel.org/

-- 
cheers,
-roger

Re: [PATCH] usb: xhci-dwc3: Bail out if "dr_mode" is "peripheral"

[Mattijs Korpershoek]

Hi Siddharth,

On lun., déc. 09, 2024 at 15:20, Roger Quadros <rogerq@kernel.org> wrote:

> Hi Siddharth,
>
> On 06/12/2024 13:04, Siddharth Vadapalli wrote:
>> Since the compatible "snps,dwc3" can be specified in the device-tree for
>> the Designware USB Controller configured for not only Host and OTG modes
>> of operation, but also for Peripheral mode of operation, bail out if
>> "dr_mode" is "peripheral".
>> 
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> ---
>> 
>> Hello,
>> 
>> This patch is based on commit
>> 448add55297 Merge branch 'master' of https://source.denx.de/u-boot/custodians/u-boot-pmic
>> of the master branch of U-Boot.
>> 
>> Regards,
>> Siddharth.
>> 
>>  drivers/usb/host/xhci-dwc3.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/drivers/usb/host/xhci-dwc3.c b/drivers/usb/host/xhci-dwc3.c
>> index e3e0ceff43e..7b1311503fe 100644
>> --- a/drivers/usb/host/xhci-dwc3.c
>> +++ b/drivers/usb/host/xhci-dwc3.c
>> @@ -208,6 +208,9 @@ static int xhci_dwc3_probe(struct udevice *dev)
>>  	writel(reg, &dwc3_reg->g_usb2phycfg[0]);
>>  
>>  	dr_mode = usb_get_dr_mode(dev_ofnode(dev));
>> +	if (dr_mode == USB_DR_MODE_PERIPHERAL)
>> +		return -ENODEV;
>> +
>>  	if (dr_mode == USB_DR_MODE_OTG &&
>>  	    dev_read_bool(dev, "usb-role-switch")) {
>>  		dr_mode = usb_get_role_switch_default_mode(dev_ofnode(dev));
>
> Although this proposal was suggested in [1] and later clarified in
> [2] I will summarize the discussion here.
>
> This patch fix is not sufficient for all cases as the xhci-dwc3.c
> driver will still be probed if dr_mode is OTG or HOST along with
> other platform drivers if enabled in config.
>
> The only current solution is not not enable CONFIG_USB_XHCI_DWC3
> together with another platform driver config (e.g. CONFIG_USB_DWC3_GENERIC)
> if snps,dwc3 should not be managed by xhci-dwc3.c.

Given Roger's explanation, should we simply drop this, or is any other
action expected from the U-Boot maintainers?

Thanks,
Mattijs

>
> [1] - https://lore.kernel.org/all/unf757pwgah5ylkcfdumtdo5zs2ny3nxct5u26t6up7aldqim2@mxhhtb53gw4x/
> [2] - https://lore.kernel.org/all/a9c458f8-68e5-448b-9977-e7bf37b00d35@kernel.org/
>
> -- 
> cheers,
> -roger

Re: [PATCH] usb: xhci-dwc3: Bail out if "dr_mode" is "peripheral"

[Siddharth Vadapalli]

On Thu, Jan 16, 2025 at 10:36:45AM +0100, Mattijs Korpershoek wrote:
> Hi Siddharth,

Hello Mattijs,

> 
> On lun., déc. 09, 2024 at 15:20, Roger Quadros <rogerq@kernel.org> wrote:
> 
> > Hi Siddharth,
> >
> > On 06/12/2024 13:04, Siddharth Vadapalli wrote:

[...]

> >> diff --git a/drivers/usb/host/xhci-dwc3.c b/drivers/usb/host/xhci-dwc3.c
> >> index e3e0ceff43e..7b1311503fe 100644
> >> --- a/drivers/usb/host/xhci-dwc3.c
> >> +++ b/drivers/usb/host/xhci-dwc3.c
> >> @@ -208,6 +208,9 @@ static int xhci_dwc3_probe(struct udevice *dev)
> >>  	writel(reg, &dwc3_reg->g_usb2phycfg[0]);
> >>  
> >>  	dr_mode = usb_get_dr_mode(dev_ofnode(dev));
> >> +	if (dr_mode == USB_DR_MODE_PERIPHERAL)
> >> +		return -ENODEV;
> >> +
> >>  	if (dr_mode == USB_DR_MODE_OTG &&
> >>  	    dev_read_bool(dev, "usb-role-switch")) {
> >>  		dr_mode = usb_get_role_switch_default_mode(dev_ofnode(dev));
> >
> > Although this proposal was suggested in [1] and later clarified in
> > [2] I will summarize the discussion here.
> >
> > This patch fix is not sufficient for all cases as the xhci-dwc3.c
> > driver will still be probed if dr_mode is OTG or HOST along with
> > other platform drivers if enabled in config.
> >
> > The only current solution is not not enable CONFIG_USB_XHCI_DWC3
> > together with another platform driver config (e.g. CONFIG_USB_DWC3_GENERIC)
> > if snps,dwc3 should not be managed by xhci-dwc3.c.
> 
> Given Roger's explanation, should we simply drop this, or is any other
> action expected from the U-Boot maintainers?

This patch could be dropped. Roger's suggestion is to fix this issue by
updating the platform configs wherein CONFIG_USB_XHCI_DWC3 should be
disabled on platforms that have "dr_mode" set to "peripheral".

Regards,
Siddharth.

Re: [PATCH] usb: xhci-dwc3: Bail out if "dr_mode" is "peripheral"

[Mattijs Korpershoek]

On jeu., janv. 16, 2025 at 15:48, Siddharth Vadapalli <s-vadapalli@ti.com> wrote:

> On Thu, Jan 16, 2025 at 10:36:45AM +0100, Mattijs Korpershoek wrote:
>> Hi Siddharth,
>
> Hello Mattijs,
>
>> 
>> On lun., déc. 09, 2024 at 15:20, Roger Quadros <rogerq@kernel.org> wrote:
>> 
>> > Hi Siddharth,
>> >
>> > On 06/12/2024 13:04, Siddharth Vadapalli wrote:
>
> [...]
>
>> >> diff --git a/drivers/usb/host/xhci-dwc3.c b/drivers/usb/host/xhci-dwc3.c
>> >> index e3e0ceff43e..7b1311503fe 100644
>> >> --- a/drivers/usb/host/xhci-dwc3.c
>> >> +++ b/drivers/usb/host/xhci-dwc3.c
>> >> @@ -208,6 +208,9 @@ static int xhci_dwc3_probe(struct udevice *dev)
>> >>  	writel(reg, &dwc3_reg->g_usb2phycfg[0]);
>> >>  
>> >>  	dr_mode = usb_get_dr_mode(dev_ofnode(dev));
>> >> +	if (dr_mode == USB_DR_MODE_PERIPHERAL)
>> >> +		return -ENODEV;
>> >> +
>> >>  	if (dr_mode == USB_DR_MODE_OTG &&
>> >>  	    dev_read_bool(dev, "usb-role-switch")) {
>> >>  		dr_mode = usb_get_role_switch_default_mode(dev_ofnode(dev));
>> >
>> > Although this proposal was suggested in [1] and later clarified in
>> > [2] I will summarize the discussion here.
>> >
>> > This patch fix is not sufficient for all cases as the xhci-dwc3.c
>> > driver will still be probed if dr_mode is OTG or HOST along with
>> > other platform drivers if enabled in config.
>> >
>> > The only current solution is not not enable CONFIG_USB_XHCI_DWC3
>> > together with another platform driver config (e.g. CONFIG_USB_DWC3_GENERIC)
>> > if snps,dwc3 should not be managed by xhci-dwc3.c.
>> 
>> Given Roger's explanation, should we simply drop this, or is any other
>> action expected from the U-Boot maintainers?
>
> This patch could be dropped. Roger's suggestion is to fix this issue by
> updating the platform configs wherein CONFIG_USB_XHCI_DWC3 should be
> disabled on platforms that have "dr_mode" set to "peripheral".

Thank you for confirming.

I've marked as "Handled elsewhere" on patchwork:
https://patchwork.ozlabs.org/project/uboot/patch/20241206110424.897283-1-s-vadapalli@ti.com/

>
> Regards,
> Siddharth.