| Message ID | 1472010404-334-4-git-send-email-kever.yang@rock-chips.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
On 08/24/2016 05:46 AM, Kever Yang wrote: > The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY, > add one variable in dwc3/dwc3_device struct to support 16 bit > UTMI+ interface on some SoCs like Rockchip rk3399. > > Signed-off-by: Kever Yang <kever.yang@rock-chips.com> > --- > > Changes in v2: > - use a variable to identify utmi+ bus width instead of CONFIG MACRO > > drivers/usb/dwc3/core.c | 6 ++++++ > drivers/usb/dwc3/core.h | 12 ++++++++++++ > include/dwc3-uboot.h | 1 + > 3 files changed, 19 insertions(+) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 85cc96a..0613508 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc) > if (dwc->dis_u2_susphy_quirk) > reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; > > + if (dwc->usb2_phyif_utmi_width == 16) { > + reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK; > + reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT; > + reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT; > + } Didn't we agree to pull this info from OF ? > dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); > > mdelay(100); > @@ -676,6 +681,7 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev) > > dwc->lpm_nyet_threshold = lpm_nyet_threshold; > dwc->tx_de_emphasis = tx_de_emphasis; > + dwc->usb2_phyif_utmi_width = dwc3_dev->usb2_phyif_utmi_width; > > dwc->hird_threshold = hird_threshold > | (dwc->is_utmi_l1_suspend << 4); > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 72d2fcd..d5bdf97 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -74,6 +74,7 @@ > #define DWC3_GCTL 0xc110 > #define DWC3_GEVTEN 0xc114 > #define DWC3_GSTS 0xc118 > +#define DWC3_GUCTL1 0xc11c > #define DWC3_GSNPSID 0xc120 > #define DWC3_GGPIO 0xc124 > #define DWC3_GUID 0xc128 > @@ -162,7 +163,17 @@ > > /* Global USB2 PHY Configuration Register */ > #define DWC3_GUSB2PHYCFG_PHYSOFTRST (1 << 31) > +#define DWC3_GUSB2PHYCFG_ENBLSLPM (1 << 8) > #define DWC3_GUSB2PHYCFG_SUSPHY (1 << 6) > +#define DWC3_GUSB2PHYCFG_PHYIF_8BIT (0 << 3) > +#define DWC3_GUSB2PHYCFG_PHYIF_16BIT (1 << 3) > +#define DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT (10) > +#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASK (0xf << \ > + DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT) > +#define DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT (0x5 << \ > + DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT) > +#define DWC3_GUSB2PHYCFG_USBTRDTIM_8BIT (0x9 << \ > + DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT) > > /* Global USB3 PIPE Control Register */ > #define DWC3_GUSB3PIPECTL_PHYSOFTRST (1 << 31) > @@ -813,6 +824,7 @@ struct dwc3 { > > unsigned tx_de_emphasis_quirk:1; > unsigned tx_de_emphasis:2; > + unsigned usb2_phyif_utmi_width; > int index; > struct list_head list; > }; > diff --git a/include/dwc3-uboot.h b/include/dwc3-uboot.h > index 7af2ad1..cc9ffe8 100644 > --- a/include/dwc3-uboot.h > +++ b/include/dwc3-uboot.h > @@ -33,6 +33,7 @@ struct dwc3_device { > unsigned dis_u2_susphy_quirk; > unsigned tx_de_emphasis_quirk; > unsigned tx_de_emphasis; > + unsigned usb2_phyif_utmi_width; The utmi width is either 8 or 16, so you can continue the bitfield instead of wasting 32 bits. > int index; > }; > >
Hi Marek, On 08/24/2016 07:38 PM, Marek Vasut wrote: > On 08/24/2016 05:46 AM, Kever Yang wrote: >> The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY, >> add one variable in dwc3/dwc3_device struct to support 16 bit >> UTMI+ interface on some SoCs like Rockchip rk3399. >> >> Signed-off-by: Kever Yang <kever.yang@rock-chips.com> >> --- >> >> Changes in v2: >> - use a variable to identify utmi+ bus width instead of CONFIG MACRO >> >> drivers/usb/dwc3/core.c | 6 ++++++ >> drivers/usb/dwc3/core.h | 12 ++++++++++++ >> include/dwc3-uboot.h | 1 + >> 3 files changed, 19 insertions(+) >> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >> index 85cc96a..0613508 100644 >> --- a/drivers/usb/dwc3/core.c >> +++ b/drivers/usb/dwc3/core.c >> @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc) >> if (dwc->dis_u2_susphy_quirk) >> reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; >> >> + if (dwc->usb2_phyif_utmi_width == 16) { >> + reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK; >> + reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT; >> + reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT; >> + } > Didn't we agree to pull this info from OF ? Yes, the dwc->usb2_phyif_utmi_width is from OF, but I make the DT parse in board file instead of in the dwc3 driver, see my patch: [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget I think implement in this way won't affect other soc also using dwc3 controller, the DT parse would cost some time which may not necessary for other soc. > >> dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); >> >> mdelay(100); >> @@ -676,6 +681,7 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev) >> >> dwc->lpm_nyet_threshold = lpm_nyet_threshold; >> dwc->tx_de_emphasis = tx_de_emphasis; >> + dwc->usb2_phyif_utmi_width = dwc3_dev->usb2_phyif_utmi_width; >> >> dwc->hird_threshold = hird_threshold >> | (dwc->is_utmi_l1_suspend << 4); >> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h >> index 72d2fcd..d5bdf97 100644 >> --- a/drivers/usb/dwc3/core.h >> +++ b/drivers/usb/dwc3/core.h >> @@ -74,6 +74,7 @@ >> #define DWC3_GCTL 0xc110 >> #define DWC3_GEVTEN 0xc114 >> #define DWC3_GSTS 0xc118 >> +#define DWC3_GUCTL1 0xc11c >> #define DWC3_GSNPSID 0xc120 >> #define DWC3_GGPIO 0xc124 >> #define DWC3_GUID 0xc128 >> @@ -162,7 +163,17 @@ >> >> /* Global USB2 PHY Configuration Register */ >> #define DWC3_GUSB2PHYCFG_PHYSOFTRST (1 << 31) >> +#define DWC3_GUSB2PHYCFG_ENBLSLPM (1 << 8) >> #define DWC3_GUSB2PHYCFG_SUSPHY (1 << 6) >> +#define DWC3_GUSB2PHYCFG_PHYIF_8BIT (0 << 3) >> +#define DWC3_GUSB2PHYCFG_PHYIF_16BIT (1 << 3) >> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT (10) >> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASK (0xf << \ >> + DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT) >> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT (0x5 << \ >> + DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT) >> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_8BIT (0x9 << \ >> + DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT) >> >> /* Global USB3 PIPE Control Register */ >> #define DWC3_GUSB3PIPECTL_PHYSOFTRST (1 << 31) >> @@ -813,6 +824,7 @@ struct dwc3 { >> >> unsigned tx_de_emphasis_quirk:1; >> unsigned tx_de_emphasis:2; >> + unsigned usb2_phyif_utmi_width; >> int index; >> struct list_head list; >> }; >> diff --git a/include/dwc3-uboot.h b/include/dwc3-uboot.h >> index 7af2ad1..cc9ffe8 100644 >> --- a/include/dwc3-uboot.h >> +++ b/include/dwc3-uboot.h >> @@ -33,6 +33,7 @@ struct dwc3_device { >> unsigned dis_u2_susphy_quirk; >> unsigned tx_de_emphasis_quirk; >> unsigned tx_de_emphasis; >> + unsigned usb2_phyif_utmi_width; > The utmi width is either 8 or 16, so you can continue the bitfield > instead of wasting 32 bits. I think you mean the usb2_phyif_utmi_width in struct dwc3, but not struct dwc3_device, right? Thanks, -Kever > >> int index; >> }; >> >> >
On 08/25/2016 03:17 AM, Kever Yang wrote: > Hi Marek, > > On 08/24/2016 07:38 PM, Marek Vasut wrote: >> On 08/24/2016 05:46 AM, Kever Yang wrote: >>> The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY, >>> add one variable in dwc3/dwc3_device struct to support 16 bit >>> UTMI+ interface on some SoCs like Rockchip rk3399. >>> >>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com> >>> --- >>> >>> Changes in v2: >>> - use a variable to identify utmi+ bus width instead of CONFIG MACRO >>> >>> drivers/usb/dwc3/core.c | 6 ++++++ >>> drivers/usb/dwc3/core.h | 12 ++++++++++++ >>> include/dwc3-uboot.h | 1 + >>> 3 files changed, 19 insertions(+) >>> >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>> index 85cc96a..0613508 100644 >>> --- a/drivers/usb/dwc3/core.c >>> +++ b/drivers/usb/dwc3/core.c >>> @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc) >>> if (dwc->dis_u2_susphy_quirk) >>> reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; >>> + if (dwc->usb2_phyif_utmi_width == 16) { >>> + reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK; >>> + reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT; >>> + reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT; >>> + } >> Didn't we agree to pull this info from OF ? > > Yes, the dwc->usb2_phyif_utmi_width is from OF, but I make the DT parse > in board file instead of in the dwc3 driver, see my patch: > [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget So this is basically a passing of platform data ? > I think implement in this way won't affect other soc also using dwc3 > controller, > the DT parse would cost some time which may not necessary for other soc. The time needed to run the DT parsing is completely insignificant. This sounds like a premature optimization. >>> dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); >>> mdelay(100); >>> @@ -676,6 +681,7 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev) >>> dwc->lpm_nyet_threshold = lpm_nyet_threshold; >>> dwc->tx_de_emphasis = tx_de_emphasis; >>> + dwc->usb2_phyif_utmi_width = dwc3_dev->usb2_phyif_utmi_width; >>> dwc->hird_threshold = hird_threshold >>> | (dwc->is_utmi_l1_suspend << 4); >>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h >>> index 72d2fcd..d5bdf97 100644 >>> --- a/drivers/usb/dwc3/core.h >>> +++ b/drivers/usb/dwc3/core.h >>> @@ -74,6 +74,7 @@ >>> #define DWC3_GCTL 0xc110 >>> #define DWC3_GEVTEN 0xc114 >>> #define DWC3_GSTS 0xc118 >>> +#define DWC3_GUCTL1 0xc11c >>> #define DWC3_GSNPSID 0xc120 >>> #define DWC3_GGPIO 0xc124 >>> #define DWC3_GUID 0xc128 >>> @@ -162,7 +163,17 @@ >>> /* Global USB2 PHY Configuration Register */ >>> #define DWC3_GUSB2PHYCFG_PHYSOFTRST (1 << 31) >>> +#define DWC3_GUSB2PHYCFG_ENBLSLPM (1 << 8) >>> #define DWC3_GUSB2PHYCFG_SUSPHY (1 << 6) >>> +#define DWC3_GUSB2PHYCFG_PHYIF_8BIT (0 << 3) >>> +#define DWC3_GUSB2PHYCFG_PHYIF_16BIT (1 << 3) >>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT (10) >>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASK (0xf << \ >>> + DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT) >>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT (0x5 << \ >>> + DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT) >>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_8BIT (0x9 << \ >>> + DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT) >>> /* Global USB3 PIPE Control Register */ >>> #define DWC3_GUSB3PIPECTL_PHYSOFTRST (1 << 31) >>> @@ -813,6 +824,7 @@ struct dwc3 { >>> unsigned tx_de_emphasis_quirk:1; >>> unsigned tx_de_emphasis:2; >>> + unsigned usb2_phyif_utmi_width; >>> int index; >>> struct list_head list; >>> }; >>> diff --git a/include/dwc3-uboot.h b/include/dwc3-uboot.h >>> index 7af2ad1..cc9ffe8 100644 >>> --- a/include/dwc3-uboot.h >>> +++ b/include/dwc3-uboot.h >>> @@ -33,6 +33,7 @@ struct dwc3_device { >>> unsigned dis_u2_susphy_quirk; >>> unsigned tx_de_emphasis_quirk; >>> unsigned tx_de_emphasis; >>> + unsigned usb2_phyif_utmi_width; >> The utmi width is either 8 or 16, so you can continue the bitfield >> instead of wasting 32 bits. > > I think you mean the usb2_phyif_utmi_width in struct dwc3, but not > struct dwc3_device, right? Yes, sorry. > Thanks, > -Kever >> >>> int index; >>> }; >>> >> > >
Hi Marek, On 08/26/2016 05:09 PM, Marek Vasut wrote: > On 08/25/2016 03:17 AM, Kever Yang wrote: >> Hi Marek, >> >> On 08/24/2016 07:38 PM, Marek Vasut wrote: >>> On 08/24/2016 05:46 AM, Kever Yang wrote: >>>> The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY, >>>> add one variable in dwc3/dwc3_device struct to support 16 bit >>>> UTMI+ interface on some SoCs like Rockchip rk3399. >>>> >>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com> >>>> --- >>>> >>>> Changes in v2: >>>> - use a variable to identify utmi+ bus width instead of CONFIG MACRO >>>> >>>> drivers/usb/dwc3/core.c | 6 ++++++ >>>> drivers/usb/dwc3/core.h | 12 ++++++++++++ >>>> include/dwc3-uboot.h | 1 + >>>> 3 files changed, 19 insertions(+) >>>> >>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>> index 85cc96a..0613508 100644 >>>> --- a/drivers/usb/dwc3/core.c >>>> +++ b/drivers/usb/dwc3/core.c >>>> @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc) >>>> if (dwc->dis_u2_susphy_quirk) >>>> reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; >>>> + if (dwc->usb2_phyif_utmi_width == 16) { >>>> + reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK; >>>> + reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT; >>>> + reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT; >>>> + } >>> Didn't we agree to pull this info from OF ? >> Yes, the dwc->usb2_phyif_utmi_width is from OF, but I make the DT parse >> in board file instead of in the dwc3 driver, see my patch: >> [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget > So this is basically a passing of platform data ? Yes, this is what other platform do, I just extend one more setting. > >> I think implement in this way won't affect other soc also using dwc3 >> controller, >> the DT parse would cost some time which may not necessary for other soc. > The time needed to run the DT parsing is completely insignificant. > This sounds like a premature optimization. Which way of implement this DT parse do you prefer and more reasonable, in dwc3 driver or in board init and dwc3 driver use it as platform data? Thanks, - Kever > >>>> dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); >>>> mdelay(100); >>>> @@ -676,6 +681,7 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev) >>>> dwc->lpm_nyet_threshold = lpm_nyet_threshold; >>>> dwc->tx_de_emphasis = tx_de_emphasis; >>>> + dwc->usb2_phyif_utmi_width = dwc3_dev->usb2_phyif_utmi_width; >>>> dwc->hird_threshold = hird_threshold >>>> | (dwc->is_utmi_l1_suspend << 4); >>>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h >>>> index 72d2fcd..d5bdf97 100644 >>>> --- a/drivers/usb/dwc3/core.h >>>> +++ b/drivers/usb/dwc3/core.h >>>> @@ -74,6 +74,7 @@ >>>> #define DWC3_GCTL 0xc110 >>>> #define DWC3_GEVTEN 0xc114 >>>> #define DWC3_GSTS 0xc118 >>>> +#define DWC3_GUCTL1 0xc11c >>>> #define DWC3_GSNPSID 0xc120 >>>> #define DWC3_GGPIO 0xc124 >>>> #define DWC3_GUID 0xc128 >>>> @@ -162,7 +163,17 @@ >>>> /* Global USB2 PHY Configuration Register */ >>>> #define DWC3_GUSB2PHYCFG_PHYSOFTRST (1 << 31) >>>> +#define DWC3_GUSB2PHYCFG_ENBLSLPM (1 << 8) >>>> #define DWC3_GUSB2PHYCFG_SUSPHY (1 << 6) >>>> +#define DWC3_GUSB2PHYCFG_PHYIF_8BIT (0 << 3) >>>> +#define DWC3_GUSB2PHYCFG_PHYIF_16BIT (1 << 3) >>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT (10) >>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASK (0xf << \ >>>> + DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT) >>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT (0x5 << \ >>>> + DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT) >>>> +#define DWC3_GUSB2PHYCFG_USBTRDTIM_8BIT (0x9 << \ >>>> + DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT) >>>> /* Global USB3 PIPE Control Register */ >>>> #define DWC3_GUSB3PIPECTL_PHYSOFTRST (1 << 31) >>>> @@ -813,6 +824,7 @@ struct dwc3 { >>>> unsigned tx_de_emphasis_quirk:1; >>>> unsigned tx_de_emphasis:2; >>>> + unsigned usb2_phyif_utmi_width; >>>> int index; >>>> struct list_head list; >>>> }; >>>> diff --git a/include/dwc3-uboot.h b/include/dwc3-uboot.h >>>> index 7af2ad1..cc9ffe8 100644 >>>> --- a/include/dwc3-uboot.h >>>> +++ b/include/dwc3-uboot.h >>>> @@ -33,6 +33,7 @@ struct dwc3_device { >>>> unsigned dis_u2_susphy_quirk; >>>> unsigned tx_de_emphasis_quirk; >>>> unsigned tx_de_emphasis; >>>> + unsigned usb2_phyif_utmi_width; >>> The utmi width is either 8 or 16, so you can continue the bitfield >>> instead of wasting 32 bits. >> I think you mean the usb2_phyif_utmi_width in struct dwc3, but not >> struct dwc3_device, right? > Yes, sorry. > >> Thanks, >> -Kever >>>> int index; >>>> }; >>>> >> >
On 08/29/2016 02:55 AM, Kever Yang wrote: > Hi Marek, > > On 08/26/2016 05:09 PM, Marek Vasut wrote: >> On 08/25/2016 03:17 AM, Kever Yang wrote: >>> Hi Marek, >>> >>> On 08/24/2016 07:38 PM, Marek Vasut wrote: >>>> On 08/24/2016 05:46 AM, Kever Yang wrote: >>>>> The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY, >>>>> add one variable in dwc3/dwc3_device struct to support 16 bit >>>>> UTMI+ interface on some SoCs like Rockchip rk3399. >>>>> >>>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com> >>>>> --- >>>>> >>>>> Changes in v2: >>>>> - use a variable to identify utmi+ bus width instead of CONFIG MACRO >>>>> >>>>> drivers/usb/dwc3/core.c | 6 ++++++ >>>>> drivers/usb/dwc3/core.h | 12 ++++++++++++ >>>>> include/dwc3-uboot.h | 1 + >>>>> 3 files changed, 19 insertions(+) >>>>> >>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>>> index 85cc96a..0613508 100644 >>>>> --- a/drivers/usb/dwc3/core.c >>>>> +++ b/drivers/usb/dwc3/core.c >>>>> @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc) >>>>> if (dwc->dis_u2_susphy_quirk) >>>>> reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; >>>>> + if (dwc->usb2_phyif_utmi_width == 16) { >>>>> + reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK; >>>>> + reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT; >>>>> + reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT; >>>>> + } >>>> Didn't we agree to pull this info from OF ? >>> Yes, the dwc->usb2_phyif_utmi_width is from OF, but I make the DT parse >>> in board file instead of in the dwc3 driver, see my patch: >>> [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget >> So this is basically a passing of platform data ? > > Yes, this is what other platform do, I just extend one more setting. Platform data should go away in favor of the DT probing. >>> I think implement in this way won't affect other soc also using dwc3 >>> controller, >>> the DT parse would cost some time which may not necessary for other soc. >> The time needed to run the DT parsing is completely insignificant. >> This sounds like a premature optimization. > > Which way of implement this DT parse do you prefer and more reasonable, > in dwc3 driver or in board init and dwc3 driver use it as platform data? Driver should parse the DT. > Thanks, > - Kever [...]
Hi Marek, On 08/29/2016 09:01 AM, Marek Vasut wrote: > On 08/29/2016 02:55 AM, Kever Yang wrote: >> Hi Marek, >> >> On 08/26/2016 05:09 PM, Marek Vasut wrote: >>> On 08/25/2016 03:17 AM, Kever Yang wrote: >>>> Hi Marek, >>>> >>>> On 08/24/2016 07:38 PM, Marek Vasut wrote: >>>>> On 08/24/2016 05:46 AM, Kever Yang wrote: >>>>>> The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY, >>>>>> add one variable in dwc3/dwc3_device struct to support 16 bit >>>>>> UTMI+ interface on some SoCs like Rockchip rk3399. >>>>>> >>>>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com> >>>>>> --- >>>>>> >>>>>> Changes in v2: >>>>>> - use a variable to identify utmi+ bus width instead of CONFIG MACRO >>>>>> >>>>>> drivers/usb/dwc3/core.c | 6 ++++++ >>>>>> drivers/usb/dwc3/core.h | 12 ++++++++++++ >>>>>> include/dwc3-uboot.h | 1 + >>>>>> 3 files changed, 19 insertions(+) >>>>>> >>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>>>> index 85cc96a..0613508 100644 >>>>>> --- a/drivers/usb/dwc3/core.c >>>>>> +++ b/drivers/usb/dwc3/core.c >>>>>> @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc) >>>>>> if (dwc->dis_u2_susphy_quirk) >>>>>> reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; >>>>>> + if (dwc->usb2_phyif_utmi_width == 16) { >>>>>> + reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK; >>>>>> + reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT; >>>>>> + reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT; >>>>>> + } >>>>> Didn't we agree to pull this info from OF ? >>>> Yes, the dwc->usb2_phyif_utmi_width is from OF, but I make the DT parse >>>> in board file instead of in the dwc3 driver, see my patch: >>>> [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget >>> So this is basically a passing of platform data ? >> Yes, this is what other platform do, I just extend one more setting. > Platform data should go away in favor of the DT probing. > >>>> I think implement in this way won't affect other soc also using dwc3 >>>> controller, >>>> the DT parse would cost some time which may not necessary for other soc. >>> The time needed to run the DT parsing is completely insignificant. >>> This sounds like a premature optimization. >> Which way of implement this DT parse do you prefer and more reasonable, >> in dwc3 driver or in board init and dwc3 driver use it as platform data? > Driver should parse the DT. I'm going to parse the DT for this new 16bit UTMI+ interface, and keep other part "as is" because the dwc3 device node for different SoC are different now. Does this make sense to you? Thanks, - Kever > >> Thanks, >> - Kever > [...] >
On 08/30/2016 05:21 AM, Kever Yang wrote: > Hi Marek, > > On 08/29/2016 09:01 AM, Marek Vasut wrote: >> On 08/29/2016 02:55 AM, Kever Yang wrote: >>> Hi Marek, >>> >>> On 08/26/2016 05:09 PM, Marek Vasut wrote: >>>> On 08/25/2016 03:17 AM, Kever Yang wrote: >>>>> Hi Marek, >>>>> >>>>> On 08/24/2016 07:38 PM, Marek Vasut wrote: >>>>>> On 08/24/2016 05:46 AM, Kever Yang wrote: >>>>>>> The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY, >>>>>>> add one variable in dwc3/dwc3_device struct to support 16 bit >>>>>>> UTMI+ interface on some SoCs like Rockchip rk3399. >>>>>>> >>>>>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com> >>>>>>> --- >>>>>>> >>>>>>> Changes in v2: >>>>>>> - use a variable to identify utmi+ bus width instead of CONFIG MACRO >>>>>>> >>>>>>> drivers/usb/dwc3/core.c | 6 ++++++ >>>>>>> drivers/usb/dwc3/core.h | 12 ++++++++++++ >>>>>>> include/dwc3-uboot.h | 1 + >>>>>>> 3 files changed, 19 insertions(+) >>>>>>> >>>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>>>>> index 85cc96a..0613508 100644 >>>>>>> --- a/drivers/usb/dwc3/core.c >>>>>>> +++ b/drivers/usb/dwc3/core.c >>>>>>> @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc) >>>>>>> if (dwc->dis_u2_susphy_quirk) >>>>>>> reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; >>>>>>> + if (dwc->usb2_phyif_utmi_width == 16) { >>>>>>> + reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK; >>>>>>> + reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT; >>>>>>> + reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT; >>>>>>> + } >>>>>> Didn't we agree to pull this info from OF ? >>>>> Yes, the dwc->usb2_phyif_utmi_width is from OF, but I make the DT >>>>> parse >>>>> in board file instead of in the dwc3 driver, see my patch: >>>>> [PATCH v2 2/4] board: evb-rk3399: add api to support dwc3 gadget >>>> So this is basically a passing of platform data ? >>> Yes, this is what other platform do, I just extend one more setting. >> Platform data should go away in favor of the DT probing. >> >>>>> I think implement in this way won't affect other soc also using dwc3 >>>>> controller, >>>>> the DT parse would cost some time which may not necessary for other >>>>> soc. >>>> The time needed to run the DT parsing is completely insignificant. >>>> This sounds like a premature optimization. >>> Which way of implement this DT parse do you prefer and more reasonable, >>> in dwc3 driver or in board init and dwc3 driver use it as platform data? >> Driver should parse the DT. > > I'm going to parse the DT for this new 16bit UTMI+ interface, and keep > other part "as is" > because the dwc3 device node for different SoC are different now. > > Does this make sense to you? That's fine, I'll complain if the code looks crappy tho ;-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 85cc96a..0613508 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -388,6 +388,11 @@ static void dwc3_phy_setup(struct dwc3 *dwc) if (dwc->dis_u2_susphy_quirk) reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; + if (dwc->usb2_phyif_utmi_width == 16) { + reg &= ~DWC3_GUSB2PHYCFG_USBTRDTIM_MASK; + reg |= DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT; + reg |= DWC3_GUSB2PHYCFG_PHYIF_16BIT; + } dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); mdelay(100); @@ -676,6 +681,7 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev) dwc->lpm_nyet_threshold = lpm_nyet_threshold; dwc->tx_de_emphasis = tx_de_emphasis; + dwc->usb2_phyif_utmi_width = dwc3_dev->usb2_phyif_utmi_width; dwc->hird_threshold = hird_threshold | (dwc->is_utmi_l1_suspend << 4); diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 72d2fcd..d5bdf97 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -74,6 +74,7 @@ #define DWC3_GCTL 0xc110 #define DWC3_GEVTEN 0xc114 #define DWC3_GSTS 0xc118 +#define DWC3_GUCTL1 0xc11c #define DWC3_GSNPSID 0xc120 #define DWC3_GGPIO 0xc124 #define DWC3_GUID 0xc128 @@ -162,7 +163,17 @@ /* Global USB2 PHY Configuration Register */ #define DWC3_GUSB2PHYCFG_PHYSOFTRST (1 << 31) +#define DWC3_GUSB2PHYCFG_ENBLSLPM (1 << 8) #define DWC3_GUSB2PHYCFG_SUSPHY (1 << 6) +#define DWC3_GUSB2PHYCFG_PHYIF_8BIT (0 << 3) +#define DWC3_GUSB2PHYCFG_PHYIF_16BIT (1 << 3) +#define DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT (10) +#define DWC3_GUSB2PHYCFG_USBTRDTIM_MASK (0xf << \ + DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT) +#define DWC3_GUSB2PHYCFG_USBTRDTIM_16BIT (0x5 << \ + DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT) +#define DWC3_GUSB2PHYCFG_USBTRDTIM_8BIT (0x9 << \ + DWC3_GUSB2PHYCFG_USBTRDTIM_SHIFT) /* Global USB3 PIPE Control Register */ #define DWC3_GUSB3PIPECTL_PHYSOFTRST (1 << 31) @@ -813,6 +824,7 @@ struct dwc3 { unsigned tx_de_emphasis_quirk:1; unsigned tx_de_emphasis:2; + unsigned usb2_phyif_utmi_width; int index; struct list_head list; }; diff --git a/include/dwc3-uboot.h b/include/dwc3-uboot.h index 7af2ad1..cc9ffe8 100644 --- a/include/dwc3-uboot.h +++ b/include/dwc3-uboot.h @@ -33,6 +33,7 @@ struct dwc3_device { unsigned dis_u2_susphy_quirk; unsigned tx_de_emphasis_quirk; unsigned tx_de_emphasis; + unsigned usb2_phyif_utmi_width; int index; };
The dwc3 controller is using 8 bit UTMI+ interface for USB2.0 PHY, add one variable in dwc3/dwc3_device struct to support 16 bit UTMI+ interface on some SoCs like Rockchip rk3399. Signed-off-by: Kever Yang <kever.yang@rock-chips.com> --- Changes in v2: - use a variable to identify utmi+ bus width instead of CONFIG MACRO drivers/usb/dwc3/core.c | 6 ++++++ drivers/usb/dwc3/core.h | 12 ++++++++++++ include/dwc3-uboot.h | 1 + 3 files changed, 19 insertions(+)