Message ID | 20241206110424.897283-1-s-vadapalli@ti.com |
---|---|
State | Handled Elsewhere, archived |
Delegated to: | Marek Vasut |
Headers | show |
Series | usb: xhci-dwc3: Bail out if "dr_mode" is "peripheral" | expand |
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/
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
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.
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.
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));
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(+)