[U-Boot] [PATCH] usb: dwc2: Rename CONFIG_DWC2_UTMI_PHY_WIDTH to CONFIG_DWC2_UTMI_WIDTH
For some reason from day one we used to have both CONFIG_DWC2_UTMI_WIDTH mentioned in dwc2.h and in scripts/config_whitelist.txt but never really used and CONFIG_DWC2_UTMI_PHY_WIDTH used in real code in dwc2.c (but never defined).
Moreover even though CONFIG_DWC2_UTMI_WIDTH might be either 8 or 16 depending on hardware (and the same is said in a comment for it in dwc2.h) but then 8 is hardcoded in the header leaving no ability to override this value in board's configuration.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Marek Vasut marex@denx.de --- drivers/usb/host/dwc2.c | 2 +- drivers/usb/host/dwc2.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index 1293e18f75e7..784fcbdbd94f 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -375,7 +375,7 @@ static void dwc_otg_core_init(struct dwc2_priv *priv) usbcfg &= ~DWC2_GUSBCFG_DDRSEL; #endif } else { /* UTMI+ interface */ -#if (CONFIG_DWC2_UTMI_PHY_WIDTH == 16) +#if (CONFIG_DWC2_UTMI_WIDTH == 16) usbcfg |= DWC2_GUSBCFG_PHYIF; #endif } diff --git a/drivers/usb/host/dwc2.h b/drivers/usb/host/dwc2.h index 4482dc621d69..574607a2acbb 100644 --- a/drivers/usb/host/dwc2.h +++ b/drivers/usb/host/dwc2.h @@ -775,7 +775,9 @@ struct dwc2_core_regs { #define DWC2_PHY_TYPE_UTMI 1 #define DWC2_PHY_TYPE_ULPI 2 #define CONFIG_DWC2_PHY_TYPE DWC2_PHY_TYPE_UTMI /* PHY type */ +#ifndef CONFIG_DWC2_UTMI_WIDTH #define CONFIG_DWC2_UTMI_WIDTH 8 /* UTMI bus width (8/16) */ +#endif
#undef CONFIG_DWC2_PHY_ULPI_DDR /* ULPI PHY uses DDR mode */ #define CONFIG_DWC2_PHY_ULPI_EXT_VBUS /* ULPI PHY controls VBUS */
On 01/31/2018 03:56 PM, Alexey Brodkin wrote:
For some reason from day one we used to have both CONFIG_DWC2_UTMI_WIDTH mentioned in dwc2.h and in scripts/config_whitelist.txt but never really used and CONFIG_DWC2_UTMI_PHY_WIDTH used in real code in dwc2.c (but never defined).
Moreover even though CONFIG_DWC2_UTMI_WIDTH might be either 8 or 16 depending on hardware (and the same is said in a comment for it in dwc2.h) but then 8 is hardcoded in the header leaving no ability to override this value in board's configuration.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Marek Vasut marex@denx.de
Applied, thanks.
btw How the heck did that code ever even compile ?
drivers/usb/host/dwc2.c | 2 +- drivers/usb/host/dwc2.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index 1293e18f75e7..784fcbdbd94f 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -375,7 +375,7 @@ static void dwc_otg_core_init(struct dwc2_priv *priv) usbcfg &= ~DWC2_GUSBCFG_DDRSEL; #endif } else { /* UTMI+ interface */ -#if (CONFIG_DWC2_UTMI_PHY_WIDTH == 16) +#if (CONFIG_DWC2_UTMI_WIDTH == 16) usbcfg |= DWC2_GUSBCFG_PHYIF; #endif } diff --git a/drivers/usb/host/dwc2.h b/drivers/usb/host/dwc2.h index 4482dc621d69..574607a2acbb 100644 --- a/drivers/usb/host/dwc2.h +++ b/drivers/usb/host/dwc2.h @@ -775,7 +775,9 @@ struct dwc2_core_regs { #define DWC2_PHY_TYPE_UTMI 1 #define DWC2_PHY_TYPE_ULPI 2 #define CONFIG_DWC2_PHY_TYPE DWC2_PHY_TYPE_UTMI /* PHY type */ +#ifndef CONFIG_DWC2_UTMI_WIDTH #define CONFIG_DWC2_UTMI_WIDTH 8 /* UTMI bus width (8/16) */ +#endif
#undef CONFIG_DWC2_PHY_ULPI_DDR /* ULPI PHY uses DDR mode */ #define CONFIG_DWC2_PHY_ULPI_EXT_VBUS /* ULPI PHY controls VBUS */
Hi Marek,
On Wed, 2018-01-31 at 16:13 +0100, Marek Vasut wrote:
On 01/31/2018 03:56 PM, Alexey Brodkin wrote:
For some reason from day one we used to have both CONFIG_DWC2_UTMI_WIDTH mentioned in dwc2.h and in scripts/config_whitelist.txt but never really used and CONFIG_DWC2_UTMI_PHY_WIDTH used in real code in dwc2.c (but never defined).
Moreover even though CONFIG_DWC2_UTMI_WIDTH might be either 8 or 16 depending on hardware (and the same is said in a comment for it in dwc2.h) but then 8 is hardcoded in the header leaving no ability to override this value in board's configuration.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Marek Vasut marex@denx.de
Applied, thanks.
btw How the heck did that code ever even compile ?
Well my change doesn't really fix anything in existing code except makes once check meaningful compared to dummy always negative as it was before :)
I'd say it means the driver was not very widely used on different hardwares. Still it does work [at least] to some extent which is really nice.
-Alexey
On 02/01/2018 03:53 PM, Alexey Brodkin wrote:
Hi Marek,
On Wed, 2018-01-31 at 16:13 +0100, Marek Vasut wrote:
On 01/31/2018 03:56 PM, Alexey Brodkin wrote:
For some reason from day one we used to have both CONFIG_DWC2_UTMI_WIDTH mentioned in dwc2.h and in scripts/config_whitelist.txt but never really used and CONFIG_DWC2_UTMI_PHY_WIDTH used in real code in dwc2.c (but never defined).
Moreover even though CONFIG_DWC2_UTMI_WIDTH might be either 8 or 16 depending on hardware (and the same is said in a comment for it in dwc2.h) but then 8 is hardcoded in the header leaving no ability to override this value in board's configuration.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Marek Vasut marex@denx.de
Applied, thanks.
btw How the heck did that code ever even compile ?
Well my change doesn't really fix anything in existing code except makes once check meaningful compared to dummy always negative as it was before :)
Ha, now I see it.
I'd say it means the driver was not very widely used on different hardwares. Still it does work [at least] to some extent which is really nice.
Probably. All this info should be pulled from DT anyway.
Hi Marek,
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Monday, February 5, 2018 4:18 PM To: Alexey Brodkin Alexey.Brodkin@synopsys.com Cc: u-boot@lists.denx.de Subject: Re: [PATCH] usb: dwc2: Rename CONFIG_DWC2_UTMI_PHY_WIDTH to CONFIG_DWC2_UTMI_WIDTH
On 02/01/2018 03:53 PM, Alexey Brodkin wrote:
Hi Marek,
On Wed, 2018-01-31 at 16:13 +0100, Marek Vasut wrote:
On 01/31/2018 03:56 PM, Alexey Brodkin wrote:
For some reason from day one we used to have both CONFIG_DWC2_UTMI_WIDTH mentioned in dwc2.h and in scripts/config_whitelist.txt but never really used and CONFIG_DWC2_UTMI_PHY_WIDTH used in real code in dwc2.c (but never defined).
Moreover even though CONFIG_DWC2_UTMI_WIDTH might be either 8 or 16 depending on hardware (and the same is said in a comment for it in dwc2.h) but then 8 is hardcoded in the header leaving no ability to override this value in board's configuration.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Marek Vasut marex@denx.de
Applied, thanks.
btw How the heck did that code ever even compile ?
Well my change doesn't really fix anything in existing code except makes once check meaningful compared to dummy always negative as it was before :)
Ha, now I see it.
I'd say it means the driver was not very widely used on different hardwares. Still it does work [at least] to some extent which is really nice.
Probably. All this info should be pulled from DT anyway.
That's so true... but now when there're real users of this driver moving stuff to Device tree is not that trivial: 1) We need to move there those options that are now "configured" via defines 2) We need to keep compatibility with Linux DT options for the same controller otherwise we'll lose ability to use the same .dtb-s in both Linux and U-Boot.
I'm not saying it's not doable but it is not a one-liner unfortunately.
-Alexey
On 02/05/2018 07:09 PM, Alexey Brodkin wrote:
Hi Marek,
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Monday, February 5, 2018 4:18 PM To: Alexey Brodkin Alexey.Brodkin@synopsys.com Cc: u-boot@lists.denx.de Subject: Re: [PATCH] usb: dwc2: Rename CONFIG_DWC2_UTMI_PHY_WIDTH to CONFIG_DWC2_UTMI_WIDTH
On 02/01/2018 03:53 PM, Alexey Brodkin wrote:
Hi Marek,
On Wed, 2018-01-31 at 16:13 +0100, Marek Vasut wrote:
On 01/31/2018 03:56 PM, Alexey Brodkin wrote:
For some reason from day one we used to have both CONFIG_DWC2_UTMI_WIDTH mentioned in dwc2.h and in scripts/config_whitelist.txt but never really used and CONFIG_DWC2_UTMI_PHY_WIDTH used in real code in dwc2.c (but never defined).
Moreover even though CONFIG_DWC2_UTMI_WIDTH might be either 8 or 16 depending on hardware (and the same is said in a comment for it in dwc2.h) but then 8 is hardcoded in the header leaving no ability to override this value in board's configuration.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Marek Vasut marex@denx.de
Applied, thanks.
btw How the heck did that code ever even compile ?
Well my change doesn't really fix anything in existing code except makes once check meaningful compared to dummy always negative as it was before :)
Ha, now I see it.
I'd say it means the driver was not very widely used on different hardwares. Still it does work [at least] to some extent which is really nice.
Probably. All this info should be pulled from DT anyway.
That's so true... but now when there're real users of this driver moving stuff to Device tree is not that trivial:
- We need to move there those options that are now "configured" via defines
- We need to keep compatibility with Linux DT options for the same controller otherwise we'll lose ability to use the same .dtb-s in both Linux and U-Boot.
I'm not saying it's not doable but it is not a one-liner unfortunately.
It should be done, soon. I'm not saying it's gonna be easy, but I don't see it being hard either.
btw same should be done for the Linux driver.
participants (2)
-
Alexey Brodkin -
Marek Vasut