| Message ID | 20190627130634.28166-14-jjhiblot@ti.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | Improvement for the DWC3 USB generic driver and fixes for the K2 platforms | expand |
Hi Jean-Jacques On 6/27/19 3:06 PM, Jean-Jacques Hiblot wrote: > The mode selection for the DWC3 is kind of obsolete. The driver does not > have to be host only or gadget only. This choice is confusing. > All the remaining callers of dwc3_uboot_init() explicitly set dr_mode > before calling the function, so none rely on a default behavior. > > Remove the choice menu and keep only the USB_DWC3_GADGET option. Enable it > by default if USB_GADGET and USB_DWC3 are enabled. > It must be disabled for the evb-rk3328 as it uses DWC2 for the gadget and > DWC3 for the host. > > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> > > --- > > Changes in v3: None > Changes in v2: > Select USB_GADGET_DUALSPEED if USB_DWC3_GADGET is selected > > configs/evb-rk3328_defconfig | 1 + > drivers/usb/dwc3/Kconfig | 18 ++---------------- > drivers/usb/dwc3/core.c | 4 ++-- > 3 files changed, 5 insertions(+), 18 deletions(-) > > diff --git a/configs/evb-rk3328_defconfig b/configs/evb-rk3328_defconfig > index aff9c32362..98929f220a 100644 > --- a/configs/evb-rk3328_defconfig > +++ b/configs/evb-rk3328_defconfig > @@ -58,6 +58,7 @@ CONFIG_USB_OHCI_HCD=y > CONFIG_USB_OHCI_GENERIC=y > CONFIG_USB_DWC2=y > CONFIG_USB_DWC3=y > +# CONFIG_USB_DWC3_GADGET is not set > CONFIG_USB_GADGET=y > CONFIG_USB_GADGET_MANUFACTURER="Rockchip" > CONFIG_USB_GADGET_VENDOR_NUM=0x2207 > diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig > index 25e1a38aee..c302486291 100644 > --- a/drivers/usb/dwc3/Kconfig > +++ b/drivers/usb/dwc3/Kconfig > @@ -7,25 +7,11 @@ config USB_DWC3 > > if USB_DWC3 > > -choice > - bool "DWC3 Mode Selection" > - > -config USB_DWC3_HOST > - bool "Host only mode" > - depends on USB > - help > - Select this when you want to use DWC3 in host mode only, > - thereby the gadget feature will be regressed. > - > config USB_DWC3_GADGET > - bool "Gadget only mode" > + bool "USB Gadget support for DWC3" > + default y > depends on USB_GADGET > select USB_GADGET_DUALSPEED > - help > - Select this when you want to use DWC3 in gadget mode only, > - thereby the host feature will be regressed. > - > -endchoice > > comment "Platform Glue Driver Support" > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 1baad39796..9f7f053265 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -707,9 +707,9 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev) > return -ENOMEM; > } > > - if (IS_ENABLED(CONFIG_USB_DWC3_HOST)) > + if (!IS_ENABLED(USB_DWC3_GADGET)) Testing your series on 96board STiH410-B2260, this patch is breaking fastboot feature. Should be + if (!IS_ENABLED(CONFIG_USB_DWC3_GADGET)) Thanks Patrice > dwc->dr_mode = USB_DR_MODE_HOST; > - else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET)) > + else if (!IS_ENABLED(CONFIG_USB_HOST)) > dwc->dr_mode = USB_DR_MODE_PERIPHERAL; > > if (dwc->dr_mode == USB_DR_MODE_UNKNOWN)
On 27/06/2019 16:33, Patrice CHOTARD wrote: > Hi Jean-Jacques > > On 6/27/19 3:06 PM, Jean-Jacques Hiblot wrote: >> The mode selection for the DWC3 is kind of obsolete. The driver does not >> have to be host only or gadget only. This choice is confusing. >> All the remaining callers of dwc3_uboot_init() explicitly set dr_mode >> before calling the function, so none rely on a default behavior. >> >> Remove the choice menu and keep only the USB_DWC3_GADGET option. Enable it >> by default if USB_GADGET and USB_DWC3 are enabled. >> It must be disabled for the evb-rk3328 as it uses DWC2 for the gadget and >> DWC3 for the host. >> >> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> >> >> --- >> >> Changes in v3: None >> Changes in v2: >> Select USB_GADGET_DUALSPEED if USB_DWC3_GADGET is selected >> >> configs/evb-rk3328_defconfig | 1 + >> drivers/usb/dwc3/Kconfig | 18 ++---------------- >> drivers/usb/dwc3/core.c | 4 ++-- >> 3 files changed, 5 insertions(+), 18 deletions(-) >> >> diff --git a/configs/evb-rk3328_defconfig b/configs/evb-rk3328_defconfig >> index aff9c32362..98929f220a 100644 >> --- a/configs/evb-rk3328_defconfig >> +++ b/configs/evb-rk3328_defconfig >> @@ -58,6 +58,7 @@ CONFIG_USB_OHCI_HCD=y >> CONFIG_USB_OHCI_GENERIC=y >> CONFIG_USB_DWC2=y >> CONFIG_USB_DWC3=y >> +# CONFIG_USB_DWC3_GADGET is not set >> CONFIG_USB_GADGET=y >> CONFIG_USB_GADGET_MANUFACTURER="Rockchip" >> CONFIG_USB_GADGET_VENDOR_NUM=0x2207 >> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig >> index 25e1a38aee..c302486291 100644 >> --- a/drivers/usb/dwc3/Kconfig >> +++ b/drivers/usb/dwc3/Kconfig >> @@ -7,25 +7,11 @@ config USB_DWC3 >> >> if USB_DWC3 >> >> -choice >> - bool "DWC3 Mode Selection" >> - >> -config USB_DWC3_HOST >> - bool "Host only mode" >> - depends on USB >> - help >> - Select this when you want to use DWC3 in host mode only, >> - thereby the gadget feature will be regressed. >> - >> config USB_DWC3_GADGET >> - bool "Gadget only mode" >> + bool "USB Gadget support for DWC3" >> + default y >> depends on USB_GADGET >> select USB_GADGET_DUALSPEED >> - help >> - Select this when you want to use DWC3 in gadget mode only, >> - thereby the host feature will be regressed. >> - >> -endchoice >> >> comment "Platform Glue Driver Support" >> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >> index 1baad39796..9f7f053265 100644 >> --- a/drivers/usb/dwc3/core.c >> +++ b/drivers/usb/dwc3/core.c >> @@ -707,9 +707,9 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev) >> return -ENOMEM; >> } >> >> - if (IS_ENABLED(CONFIG_USB_DWC3_HOST)) >> + if (!IS_ENABLED(USB_DWC3_GADGET)) > > Testing your series on 96board STiH410-B2260, this patch is breaking fastboot feature. > > Should be > > + if (!IS_ENABLED(CONFIG_USB_DWC3_GADGET)) Thanks for testing and catching this. It'll be fixed in the next version of the series JJ > > Thanks > > Patrice > >> dwc->dr_mode = USB_DR_MODE_HOST; >> - else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET)) >> + else if (!IS_ENABLED(CONFIG_USB_HOST)) >> dwc->dr_mode = USB_DR_MODE_PERIPHERAL; >> >> if (dwc->dr_mode == USB_DR_MODE_UNKNOWN)
On 6/27/19 4:49 PM, Jean-Jacques Hiblot wrote: > > On 27/06/2019 16:33, Patrice CHOTARD wrote: >> Hi Jean-Jacques >> >> On 6/27/19 3:06 PM, Jean-Jacques Hiblot wrote: >>> The mode selection for the DWC3 is kind of obsolete. The driver does not >>> have to be host only or gadget only. This choice is confusing. >>> All the remaining callers of dwc3_uboot_init() explicitly set dr_mode >>> before calling the function, so none rely on a default behavior. >>> >>> Remove the choice menu and keep only the USB_DWC3_GADGET option. >>> Enable it >>> by default if USB_GADGET and USB_DWC3 are enabled. >>> It must be disabled for the evb-rk3328 as it uses DWC2 for the gadget >>> and >>> DWC3 for the host. >>> >>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> >>> >>> --- >>> >>> Changes in v3: None >>> Changes in v2: >>> Select USB_GADGET_DUALSPEED if USB_DWC3_GADGET is selected >>> >>> configs/evb-rk3328_defconfig | 1 + >>> drivers/usb/dwc3/Kconfig | 18 ++---------------- >>> drivers/usb/dwc3/core.c | 4 ++-- >>> 3 files changed, 5 insertions(+), 18 deletions(-) >>> >>> diff --git a/configs/evb-rk3328_defconfig b/configs/evb-rk3328_defconfig >>> index aff9c32362..98929f220a 100644 >>> --- a/configs/evb-rk3328_defconfig >>> +++ b/configs/evb-rk3328_defconfig >>> @@ -58,6 +58,7 @@ CONFIG_USB_OHCI_HCD=y >>> CONFIG_USB_OHCI_GENERIC=y >>> CONFIG_USB_DWC2=y >>> CONFIG_USB_DWC3=y >>> +# CONFIG_USB_DWC3_GADGET is not set >>> CONFIG_USB_GADGET=y >>> CONFIG_USB_GADGET_MANUFACTURER="Rockchip" >>> CONFIG_USB_GADGET_VENDOR_NUM=0x2207 >>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig >>> index 25e1a38aee..c302486291 100644 >>> --- a/drivers/usb/dwc3/Kconfig >>> +++ b/drivers/usb/dwc3/Kconfig >>> @@ -7,25 +7,11 @@ config USB_DWC3 >>> if USB_DWC3 >>> -choice >>> - bool "DWC3 Mode Selection" >>> - >>> -config USB_DWC3_HOST >>> - bool "Host only mode" >>> - depends on USB >>> - help >>> - Select this when you want to use DWC3 in host mode only, >>> - thereby the gadget feature will be regressed. >>> - >>> config USB_DWC3_GADGET >>> - bool "Gadget only mode" >>> + bool "USB Gadget support for DWC3" >>> + default y >>> depends on USB_GADGET >>> select USB_GADGET_DUALSPEED >>> - help >>> - Select this when you want to use DWC3 in gadget mode only, >>> - thereby the host feature will be regressed. >>> - >>> -endchoice >>> comment "Platform Glue Driver Support" >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>> index 1baad39796..9f7f053265 100644 >>> --- a/drivers/usb/dwc3/core.c >>> +++ b/drivers/usb/dwc3/core.c >>> @@ -707,9 +707,9 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev) >>> return -ENOMEM; >>> } >>> - if (IS_ENABLED(CONFIG_USB_DWC3_HOST)) >>> + if (!IS_ENABLED(USB_DWC3_GADGET)) >> >> Testing your series on 96board STiH410-B2260, this patch is breaking >> fastboot feature. >> >> Should be >> >> + if (!IS_ENABLED(CONFIG_USB_DWC3_GADGET)) > > Thanks for testing and catching this. > > It'll be fixed in the next version of the series Just send me an incremental patch on top of usb/next.
On 06/27/2019 09:06 PM, Jean-Jacques Hiblot wrote: > The mode selection for the DWC3 is kind of obsolete. The driver does not > have to be host only or gadget only. This choice is confusing. > All the remaining callers of dwc3_uboot_init() explicitly set dr_mode > before calling the function, so none rely on a default behavior. > > Remove the choice menu and keep only the USB_DWC3_GADGET option. Enable it > by default if USB_GADGET and USB_DWC3 are enabled. > It must be disabled for the evb-rk3328 as it uses DWC2 for the gadget and > DWC3 for the host. > > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> Reviewed-by: Kever Yang <kever.yang@rock-chips.com> Thanks, - Kever > --- > > Changes in v3: None > Changes in v2: > Select USB_GADGET_DUALSPEED if USB_DWC3_GADGET is selected > > configs/evb-rk3328_defconfig | 1 + > drivers/usb/dwc3/Kconfig | 18 ++---------------- > drivers/usb/dwc3/core.c | 4 ++-- > 3 files changed, 5 insertions(+), 18 deletions(-) > > diff --git a/configs/evb-rk3328_defconfig b/configs/evb-rk3328_defconfig > index aff9c32362..98929f220a 100644 > --- a/configs/evb-rk3328_defconfig > +++ b/configs/evb-rk3328_defconfig > @@ -58,6 +58,7 @@ CONFIG_USB_OHCI_HCD=y > CONFIG_USB_OHCI_GENERIC=y > CONFIG_USB_DWC2=y > CONFIG_USB_DWC3=y > +# CONFIG_USB_DWC3_GADGET is not set > CONFIG_USB_GADGET=y > CONFIG_USB_GADGET_MANUFACTURER="Rockchip" > CONFIG_USB_GADGET_VENDOR_NUM=0x2207 > diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig > index 25e1a38aee..c302486291 100644 > --- a/drivers/usb/dwc3/Kconfig > +++ b/drivers/usb/dwc3/Kconfig > @@ -7,25 +7,11 @@ config USB_DWC3 > > if USB_DWC3 > > -choice > - bool "DWC3 Mode Selection" > - > -config USB_DWC3_HOST > - bool "Host only mode" > - depends on USB > - help > - Select this when you want to use DWC3 in host mode only, > - thereby the gadget feature will be regressed. > - > config USB_DWC3_GADGET > - bool "Gadget only mode" > + bool "USB Gadget support for DWC3" > + default y > depends on USB_GADGET > select USB_GADGET_DUALSPEED > - help > - Select this when you want to use DWC3 in gadget mode only, > - thereby the host feature will be regressed. > - > -endchoice > > comment "Platform Glue Driver Support" > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 1baad39796..9f7f053265 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -707,9 +707,9 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev) > return -ENOMEM; > } > > - if (IS_ENABLED(CONFIG_USB_DWC3_HOST)) > + if (!IS_ENABLED(USB_DWC3_GADGET)) > dwc->dr_mode = USB_DR_MODE_HOST; > - else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET)) > + else if (!IS_ENABLED(CONFIG_USB_HOST)) > dwc->dr_mode = USB_DR_MODE_PERIPHERAL; > > if (dwc->dr_mode == USB_DR_MODE_UNKNOWN)
On 7/2/19 4:11 AM, Kever Yang wrote: > > > On 06/27/2019 09:06 PM, Jean-Jacques Hiblot wrote: >> The mode selection for the DWC3 is kind of obsolete. The driver does not >> have to be host only or gadget only. This choice is confusing. >> All the remaining callers of dwc3_uboot_init() explicitly set dr_mode >> before calling the function, so none rely on a default behavior. >> >> Remove the choice menu and keep only the USB_DWC3_GADGET option. Enable it >> by default if USB_GADGET and USB_DWC3 are enabled. >> It must be disabled for the evb-rk3328 as it uses DWC2 for the gadget and >> DWC3 for the host. >> >> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> > > Reviewed-by: Kever Yang <kever.yang@rock-chips.com> So is this a fix for current release or feature for next ? I take it it's the later.
On 02/07/2019 13:38, Marek Vasut wrote: > On 7/2/19 4:11 AM, Kever Yang wrote: >> >> On 06/27/2019 09:06 PM, Jean-Jacques Hiblot wrote: >>> The mode selection for the DWC3 is kind of obsolete. The driver does not >>> have to be host only or gadget only. This choice is confusing. >>> All the remaining callers of dwc3_uboot_init() explicitly set dr_mode >>> before calling the function, so none rely on a default behavior. >>> >>> Remove the choice menu and keep only the USB_DWC3_GADGET option. Enable it >>> by default if USB_GADGET and USB_DWC3 are enabled. >>> It must be disabled for the evb-rk3328 as it uses DWC2 for the gadget and >>> DWC3 for the host. >>> >>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> >> Reviewed-by: Kever Yang <kever.yang@rock-chips.com> > So is this a fix for current release or feature for next ? I take it > it's the later. > I think this is more the former. I should have changed the title of the series to "Fix the DWC3 generic driver" In the current release all the platforms relying on the dwc3-generic driver are broken because of the usage of MISC uclass. This series among other things fixes it.
On 7/2/19 2:06 PM, Jean-Jacques Hiblot wrote: > > On 02/07/2019 13:38, Marek Vasut wrote: >> On 7/2/19 4:11 AM, Kever Yang wrote: >>> >>> On 06/27/2019 09:06 PM, Jean-Jacques Hiblot wrote: >>>> The mode selection for the DWC3 is kind of obsolete. The driver does >>>> not >>>> have to be host only or gadget only. This choice is confusing. >>>> All the remaining callers of dwc3_uboot_init() explicitly set dr_mode >>>> before calling the function, so none rely on a default behavior. >>>> >>>> Remove the choice menu and keep only the USB_DWC3_GADGET option. >>>> Enable it >>>> by default if USB_GADGET and USB_DWC3 are enabled. >>>> It must be disabled for the evb-rk3328 as it uses DWC2 for the >>>> gadget and >>>> DWC3 for the host. >>>> >>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> >>> Reviewed-by: Kever Yang <kever.yang@rock-chips.com> >> So is this a fix for current release or feature for next ? I take it >> it's the later. >> > I think this is more the former. I should have changed the title of the > series to "Fix the DWC3 generic driver" > > In the current release all the platforms relying on the dwc3-generic > driver are broken because of the usage of MISC uclass. > > This series among other things fixes it. Are you able to somehow reduce this to a smaller fix for current release ? I see I have this series applied for -next now.
On 02/07/2019 14:10, Marek Vasut wrote: > On 7/2/19 2:06 PM, Jean-Jacques Hiblot wrote: >> On 02/07/2019 13:38, Marek Vasut wrote: >>> On 7/2/19 4:11 AM, Kever Yang wrote: >>>> On 06/27/2019 09:06 PM, Jean-Jacques Hiblot wrote: >>>>> The mode selection for the DWC3 is kind of obsolete. The driver does >>>>> not >>>>> have to be host only or gadget only. This choice is confusing. >>>>> All the remaining callers of dwc3_uboot_init() explicitly set dr_mode >>>>> before calling the function, so none rely on a default behavior. >>>>> >>>>> Remove the choice menu and keep only the USB_DWC3_GADGET option. >>>>> Enable it >>>>> by default if USB_GADGET and USB_DWC3 are enabled. >>>>> It must be disabled for the evb-rk3328 as it uses DWC2 for the >>>>> gadget and >>>>> DWC3 for the host. >>>>> >>>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> >>>> Reviewed-by: Kever Yang <kever.yang@rock-chips.com> >>> So is this a fix for current release or feature for next ? I take it >>> it's the later. >>> >> I think this is more the former. I should have changed the title of the >> series to "Fix the DWC3 generic driver" >> >> In the current release all the platforms relying on the dwc3-generic >> driver are broken because of the usage of MISC uclass. >> >> This series among other things fixes it. > Are you able to somehow reduce this to a smaller fix for current release ? For the uclass fix, only the following patches are required: dm: Add a No-op uclass usb: dwc3: Use UCLASS_NOP instead of UCLASS_MISC for the DWC3 generic glue I've just tested with only those 2 patches on a DRA7-evm The rest are improvements and fixes specific to the K2 platforms JJ > > I see I have this series applied for -next now. >
Hi Marek, On 02/07/2019 14:32, Jean-Jacques Hiblot wrote: > > On 02/07/2019 14:10, Marek Vasut wrote: >> On 7/2/19 2:06 PM, Jean-Jacques Hiblot wrote: >>> On 02/07/2019 13:38, Marek Vasut wrote: >>>> On 7/2/19 4:11 AM, Kever Yang wrote: >>>>> On 06/27/2019 09:06 PM, Jean-Jacques Hiblot wrote: >>>>>> The mode selection for the DWC3 is kind of obsolete. The driver does >>>>>> not >>>>>> have to be host only or gadget only. This choice is confusing. >>>>>> All the remaining callers of dwc3_uboot_init() explicitly set >>>>>> dr_mode >>>>>> before calling the function, so none rely on a default behavior. >>>>>> >>>>>> Remove the choice menu and keep only the USB_DWC3_GADGET option. >>>>>> Enable it >>>>>> by default if USB_GADGET and USB_DWC3 are enabled. >>>>>> It must be disabled for the evb-rk3328 as it uses DWC2 for the >>>>>> gadget and >>>>>> DWC3 for the host. >>>>>> >>>>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> >>>>> Reviewed-by: Kever Yang <kever.yang@rock-chips.com> >>>> So is this a fix for current release or feature for next ? I take it >>>> it's the later. >>>> >>> I think this is more the former. I should have changed the title of the >>> series to "Fix the DWC3 generic driver" >>> >>> In the current release all the platforms relying on the dwc3-generic >>> driver are broken because of the usage of MISC uclass. >>> >>> This series among other things fixes it. >> Are you able to somehow reduce this to a smaller fix for current >> release ? > > For the uclass fix, only the following patches are required: > > dm: Add a No-op uclass > > usb: dwc3: Use UCLASS_NOP instead of UCLASS_MISC for the DWC3 generic > glue > > I've just tested with only those 2 patches on a DRA7-evm > > > The rest are improvements and fixes specific to the K2 platforms Do you want that I send a separate series for those 2 patches ? > > JJ > > > >> >> I see I have this series applied for -next now. >> > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
On 7/4/19 9:37 AM, Jean-Jacques Hiblot wrote: > Hi Marek, > > On 02/07/2019 14:32, Jean-Jacques Hiblot wrote: >> >> On 02/07/2019 14:10, Marek Vasut wrote: >>> On 7/2/19 2:06 PM, Jean-Jacques Hiblot wrote: >>>> On 02/07/2019 13:38, Marek Vasut wrote: >>>>> On 7/2/19 4:11 AM, Kever Yang wrote: >>>>>> On 06/27/2019 09:06 PM, Jean-Jacques Hiblot wrote: >>>>>>> The mode selection for the DWC3 is kind of obsolete. The driver does >>>>>>> not >>>>>>> have to be host only or gadget only. This choice is confusing. >>>>>>> All the remaining callers of dwc3_uboot_init() explicitly set >>>>>>> dr_mode >>>>>>> before calling the function, so none rely on a default behavior. >>>>>>> >>>>>>> Remove the choice menu and keep only the USB_DWC3_GADGET option. >>>>>>> Enable it >>>>>>> by default if USB_GADGET and USB_DWC3 are enabled. >>>>>>> It must be disabled for the evb-rk3328 as it uses DWC2 for the >>>>>>> gadget and >>>>>>> DWC3 for the host. >>>>>>> >>>>>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> >>>>>> Reviewed-by: Kever Yang <kever.yang@rock-chips.com> >>>>> So is this a fix for current release or feature for next ? I take it >>>>> it's the later. >>>>> >>>> I think this is more the former. I should have changed the title of the >>>> series to "Fix the DWC3 generic driver" >>>> >>>> In the current release all the platforms relying on the dwc3-generic >>>> driver are broken because of the usage of MISC uclass. >>>> >>>> This series among other things fixes it. >>> Are you able to somehow reduce this to a smaller fix for current >>> release ? >> >> For the uclass fix, only the following patches are required: >> >> dm: Add a No-op uclass >> >> usb: dwc3: Use UCLASS_NOP instead of UCLASS_MISC for the DWC3 generic >> glue >> >> I've just tested with only those 2 patches on a DRA7-evm >> >> >> The rest are improvements and fixes specific to the K2 platforms > > Do you want that I send a separate series for those 2 patches ? Well, if you have some fixes (and only fixes, not 18-patch large series which reworks stuff) for current release, please send them. Otherwise, rebase on usb/next and send that too. However, please mark the patches somehow, so I know where they are supposed to go (next or master).
Marek, On 04/07/2019 15:38, Marek Vasut wrote: > On 7/4/19 9:37 AM, Jean-Jacques Hiblot wrote: >> Hi Marek, >> >> On 02/07/2019 14:32, Jean-Jacques Hiblot wrote: >>> On 02/07/2019 14:10, Marek Vasut wrote: >>>> On 7/2/19 2:06 PM, Jean-Jacques Hiblot wrote: >>>>> On 02/07/2019 13:38, Marek Vasut wrote: >>>>>> On 7/2/19 4:11 AM, Kever Yang wrote: >>>>>>> On 06/27/2019 09:06 PM, Jean-Jacques Hiblot wrote: >>>>>>>> The mode selection for the DWC3 is kind of obsolete. The driver does >>>>>>>> not >>>>>>>> have to be host only or gadget only. This choice is confusing. >>>>>>>> All the remaining callers of dwc3_uboot_init() explicitly set >>>>>>>> dr_mode >>>>>>>> before calling the function, so none rely on a default behavior. >>>>>>>> >>>>>>>> Remove the choice menu and keep only the USB_DWC3_GADGET option. >>>>>>>> Enable it >>>>>>>> by default if USB_GADGET and USB_DWC3 are enabled. >>>>>>>> It must be disabled for the evb-rk3328 as it uses DWC2 for the >>>>>>>> gadget and >>>>>>>> DWC3 for the host. >>>>>>>> >>>>>>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> >>>>>>> Reviewed-by: Kever Yang <kever.yang@rock-chips.com> >>>>>> So is this a fix for current release or feature for next ? I take it >>>>>> it's the later. >>>>>> >>>>> I think this is more the former. I should have changed the title of the >>>>> series to "Fix the DWC3 generic driver" >>>>> >>>>> In the current release all the platforms relying on the dwc3-generic >>>>> driver are broken because of the usage of MISC uclass. >>>>> >>>>> This series among other things fixes it. >>>> Are you able to somehow reduce this to a smaller fix for current >>>> release ? >>> For the uclass fix, only the following patches are required: >>> >>> dm: Add a No-op uclass >>> >>> usb: dwc3: Use UCLASS_NOP instead of UCLASS_MISC for the DWC3 generic >>> glue >>> >>> I've just tested with only those 2 patches on a DRA7-evm >>> >>> >>> The rest are improvements and fixes specific to the K2 platforms >> Do you want that I send a separate series for those 2 patches ? > Well, if you have some fixes (and only fixes, not 18-patch large series > which reworks stuff) for current release, please send them. Otherwise, > rebase on usb/next and send that too. However, please mark the patches > somehow, so I know where they are supposed to go (next or master). Done. Thanks JJ
On 7/5/19 9:34 AM, Jean-Jacques Hiblot wrote: > Marek, > > On 04/07/2019 15:38, Marek Vasut wrote: >> On 7/4/19 9:37 AM, Jean-Jacques Hiblot wrote: >>> Hi Marek, >>> >>> On 02/07/2019 14:32, Jean-Jacques Hiblot wrote: >>>> On 02/07/2019 14:10, Marek Vasut wrote: >>>>> On 7/2/19 2:06 PM, Jean-Jacques Hiblot wrote: >>>>>> On 02/07/2019 13:38, Marek Vasut wrote: >>>>>>> On 7/2/19 4:11 AM, Kever Yang wrote: >>>>>>>> On 06/27/2019 09:06 PM, Jean-Jacques Hiblot wrote: >>>>>>>>> The mode selection for the DWC3 is kind of obsolete. The driver >>>>>>>>> does >>>>>>>>> not >>>>>>>>> have to be host only or gadget only. This choice is confusing. >>>>>>>>> All the remaining callers of dwc3_uboot_init() explicitly set >>>>>>>>> dr_mode >>>>>>>>> before calling the function, so none rely on a default behavior. >>>>>>>>> >>>>>>>>> Remove the choice menu and keep only the USB_DWC3_GADGET option. >>>>>>>>> Enable it >>>>>>>>> by default if USB_GADGET and USB_DWC3 are enabled. >>>>>>>>> It must be disabled for the evb-rk3328 as it uses DWC2 for the >>>>>>>>> gadget and >>>>>>>>> DWC3 for the host. >>>>>>>>> >>>>>>>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> >>>>>>>> Reviewed-by: Kever Yang <kever.yang@rock-chips.com> >>>>>>> So is this a fix for current release or feature for next ? I take it >>>>>>> it's the later. >>>>>>> >>>>>> I think this is more the former. I should have changed the title >>>>>> of the >>>>>> series to "Fix the DWC3 generic driver" >>>>>> >>>>>> In the current release all the platforms relying on the dwc3-generic >>>>>> driver are broken because of the usage of MISC uclass. >>>>>> >>>>>> This series among other things fixes it. >>>>> Are you able to somehow reduce this to a smaller fix for current >>>>> release ? >>>> For the uclass fix, only the following patches are required: >>>> >>>> dm: Add a No-op uclass >>>> >>>> usb: dwc3: Use UCLASS_NOP instead of UCLASS_MISC for the DWC3 generic >>>> glue >>>> >>>> I've just tested with only those 2 patches on a DRA7-evm >>>> >>>> >>>> The rest are improvements and fixes specific to the K2 platforms >>> Do you want that I send a separate series for those 2 patches ? >> Well, if you have some fixes (and only fixes, not 18-patch large series >> which reworks stuff) for current release, please send them. Otherwise, >> rebase on usb/next and send that too. However, please mark the patches >> somehow, so I know where they are supposed to go (next or master). > > Done. > > Thanks Great, thanks!
diff --git a/configs/evb-rk3328_defconfig b/configs/evb-rk3328_defconfig index aff9c32362..98929f220a 100644 --- a/configs/evb-rk3328_defconfig +++ b/configs/evb-rk3328_defconfig @@ -58,6 +58,7 @@ CONFIG_USB_OHCI_HCD=y CONFIG_USB_OHCI_GENERIC=y CONFIG_USB_DWC2=y CONFIG_USB_DWC3=y +# CONFIG_USB_DWC3_GADGET is not set CONFIG_USB_GADGET=y CONFIG_USB_GADGET_MANUFACTURER="Rockchip" CONFIG_USB_GADGET_VENDOR_NUM=0x2207 diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig index 25e1a38aee..c302486291 100644 --- a/drivers/usb/dwc3/Kconfig +++ b/drivers/usb/dwc3/Kconfig @@ -7,25 +7,11 @@ config USB_DWC3 if USB_DWC3 -choice - bool "DWC3 Mode Selection" - -config USB_DWC3_HOST - bool "Host only mode" - depends on USB - help - Select this when you want to use DWC3 in host mode only, - thereby the gadget feature will be regressed. - config USB_DWC3_GADGET - bool "Gadget only mode" + bool "USB Gadget support for DWC3" + default y depends on USB_GADGET select USB_GADGET_DUALSPEED - help - Select this when you want to use DWC3 in gadget mode only, - thereby the host feature will be regressed. - -endchoice comment "Platform Glue Driver Support" diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 1baad39796..9f7f053265 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -707,9 +707,9 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev) return -ENOMEM; } - if (IS_ENABLED(CONFIG_USB_DWC3_HOST)) + if (!IS_ENABLED(USB_DWC3_GADGET)) dwc->dr_mode = USB_DR_MODE_HOST; - else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET)) + else if (!IS_ENABLED(CONFIG_USB_HOST)) dwc->dr_mode = USB_DR_MODE_PERIPHERAL; if (dwc->dr_mode == USB_DR_MODE_UNKNOWN)
The mode selection for the DWC3 is kind of obsolete. The driver does not have to be host only or gadget only. This choice is confusing. All the remaining callers of dwc3_uboot_init() explicitly set dr_mode before calling the function, so none rely on a default behavior. Remove the choice menu and keep only the USB_DWC3_GADGET option. Enable it by default if USB_GADGET and USB_DWC3 are enabled. It must be disabled for the evb-rk3328 as it uses DWC2 for the gadget and DWC3 for the host. Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> --- Changes in v3: None Changes in v2: Select USB_GADGET_DUALSPEED if USB_DWC3_GADGET is selected configs/evb-rk3328_defconfig | 1 + drivers/usb/dwc3/Kconfig | 18 ++---------------- drivers/usb/dwc3/core.c | 4 ++-- 3 files changed, 5 insertions(+), 18 deletions(-)