Cleanup Octeon DWC3 glue code
Subsystems (3)
v2 cover Cleanup Octeon DWC3 glue code
Hi,
primary motivation was to fix issue Id 29206 as described in OCTEON III CN70XX/CN71XX Known Issues, version: 1.9. Said document is marked as Marvell Proprietary and Confidential, therefore I'm not sure if I can cite from it.
This probably does not matter too much as the root of the information listed there is a workaround being implemented in OCTEON SDK 3.1.2 patch 7 and later in u-boot/drivers/usb/host/xhci-octeon.c:dwc3_uphy_pll_reset()
My coleague ported that patch to linux-4.9 and I will later modify it to work on top of current glue driver.
The glue code currently lives in arch/mips/cavium-octeon/octeon-usb.c and loops for each "cavium,octeon-7130-usb-uctl" compatible. However there is no bond with dwc3 core code, so if anything goes wrong in glue code, the loop breaks, leaving dwc3 in reset.
Later on when dwc3 core tries to read any device register, bus error is emited, leading to kernel panic.
Therefore move it to drivers/usb/dwc3 while making it glue driver.
This is a second attempt, see changelog appended to patches.
Ladislav Michl (3):
usb: dwc3: dwc3-octeon: Convert to glue driver
usb: dwc3: dwc3-octeon: Move node parsing into driver probe
usb: dwc3: Add SPDX header and copyright
arch/mips/cavium-octeon/Makefile | 1 - arch/mips/cavium-octeon/octeon-platform.c | 1 - drivers/usb/dwc3/Kconfig | 10 + drivers/usb/dwc3/Makefile | 1 + .../usb/dwc3/dwc3-octeon.c | 262 +++++++++--------- drivers/usb/dwc3/dwc3-of-simple.c | 1 - 6 files changed, 142 insertions(+), 134 deletions(-) rename arch/mips/cavium-octeon/octeon-usb.c => drivers/usb/dwc3/dwc3-octeon.c (80%)
Signature · 1 line
2.39.2
PATCH v2 1/3 usb: dwc3: dwc3-octeon: Convert to glue driver
From: Ladislav Michl <[email protected]>
Move Octeon DWC3 glue code from arch/mips and use it instead of dwc3-of-simple.
---
CHANGES:
- v2: squashed move and glue conversion patch, fixed sparse warning and formatting issue. Set private data at the end of probe. Clear drvdata on remove. Added host mode only notice. Collected ack for move from arch/mips.
arch/mips/cavium-octeon/Makefile | 1 - arch/mips/cavium-octeon/octeon-platform.c | 1 - drivers/usb/dwc3/Kconfig | 10 ++ drivers/usb/dwc3/Makefile | 1 + .../usb/dwc3/dwc3-octeon.c | 108 ++++++++++-------- drivers/usb/dwc3/dwc3-of-simple.c | 1 - 6 files changed, 69 insertions(+), 53 deletions(-) rename arch/mips/cavium-octeon/octeon-usb.c => drivers/usb/dwc3/dwc3-octeon.c (91%)
arch/mips/cavium-octeon/Makefile +0 -1
@@ -18,4 +18,3 @@ obj-y += crypto/ obj-$(CONFIG_MTD) += flash_setup.o obj-$(CONFIG_SMP) += smp.o obj-$(CONFIG_OCTEON_ILM) += oct_ilm.o -obj-$(CONFIG_USB) += octeon-usb.o
arch/mips/cavium-octeon/octeon-platform.c +0 -1
@@ -450,7 +450,6 @@ static const struct of_device_id octeon_ids[] __initconst = { { .compatible = "cavium,octeon-3860-bootbus", }, { .compatible = "cavium,mdio-mux", }, { .compatible = "gpio-leds", }, - { .compatible = "cavium,octeon-7130-usb-uctl", }, {}, };
drivers/usb/dwc3/Kconfig +10 -0
@@ -168,4 +168,14 @@ config USB_DWC3_AM62 The Designware Core USB3 IP is programmed to operate in in USB 2.0 mode only. Say 'Y' or 'M' here if you have one such device + +config USB_DWC3_OCTEON + tristate "Cavium Octeon Platforms" + depends on CAVIUM_OCTEON_SOC || COMPILE_TEST + default USB_DWC3 + help + Support Cavium Octeon platforms with DesignWare Core USB3 IP. + Only the host mode is currently supported. + Say 'Y' or 'M' here if you have one such device. + endif
drivers/usb/dwc3/Makefile +1 -0
@@ -54,3 +54,4 @@ obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o obj-$(CONFIG_USB_DWC3_IMX8MP) += dwc3-imx8mp.o obj-$(CONFIG_USB_DWC3_XILINX) += dwc3-xilinx.o +obj-$(CONFIG_USB_DWC3_OCTEON) += dwc3-octeon.o
drivers/usb/dwc3/dwc3-octeon.c renamed from arch/mips/cavium-octeon/octeon-usb.c +58 -50
143 lines not on this page · raw message
drivers/usb/dwc3/dwc3-of-simple.c +0 -1
@@ -172,7 +172,6 @@ static const struct dev_pm_ops dwc3_of_simple_dev_pm_ops = {static const struct of_device_id of_dwc3_simple_match[] = { { .compatible ="rockchip,rk3399-dwc3" }, - { .compatible ="cavium,octeon-7130-usb-uctl" }, { .compatible ="sprd,sc9860-dwc3" }, { .compatible ="allwinner,sun50i-h6-dwc3" }, { .compatible ="hisilicon,hi3670-dwc3" },
Signature · 1 line
2.39.2
Thinh Nguyen
maintainerRe: [PATCH v2 1/3] usb: dwc3: dwc3-octeon: Convert to glue driver
Quoted 4 lines · Ladislav Michl
From: Ladislav Michl <[email protected]>
Move Octeon DWC3 glue code from
arch/mipsand use it instead of dwc3-of-simple.
Please provide more context and the reason to why the move here.
Quoted 100 lines
Signed-off-by Ladislav Michl <[email protected]>---
CHANGES:
- v2: squashed move and glue conversion patch, fixed sparse warning and formatting issue. Set private data at the end of probe. Clear drvdata on remove. Added host mode only notice. Collected ack for move fromarch/mips.arch/mips/cavium-octeon/Makefile | 1 - arch/mips/cavium-octeon/octeon-platform.c | 1 - drivers/usb/dwc3/Kconfig | 10 ++ drivers/usb/dwc3/Makefile | 1 + .../usb/dwc3/dwc3-octeon.c | 108 ++++++++++-------- drivers/usb/dwc3/dwc3-of-simple.c | 1 - 6 files changed, 69 insertions(+), 53 deletions(-) rename arch/mips/cavium-octeon/octeon-usb.c => drivers/usb/dwc3/dwc3-octeon.c (91%)arch/mips/cavium-octeon/Makefile +0
-1@@ -18,4 +18,3 @@ obj-y += crypto/ obj-$(CONFIG_MTD) += flash_setup.o obj-$(CONFIG_SMP) += smp.o obj-$(CONFIG_OCTEON_ILM) += oct_ilm.o -obj-$(CONFIG_USB) += octeon-usb.oarch/mips/cavium-octeon/octeon-platform.c +0
-1@@ -450,7 +450,6 @@ static const struct of_device_id octeon_ids[] __initconst = { { .compatible = "cavium,octeon-3860-bootbus", }, { .compatible = "cavium,mdio-mux", }, { .compatible = "gpio-leds", }, - { .compatible = "cavium,octeon-7130-usb-uctl", }, {}, };drivers/usb/dwc3/Kconfig +10
-0@@ -168,4 +168,14 @@ config USB_DWC3_AM62 The Designware Core USB3 IP is programmed to operate in in USB 2.0 mode only. Say 'Y' or 'M' here if you have one such device + +config USB_DWC3_OCTEON + tristate "Cavium Octeon Platforms" + depends on CAVIUM_OCTEON_SOC || COMPILE_TEST + default USB_DWC3 + help + Support Cavium Octeon platforms with DesignWare Core USB3 IP. + Only the host mode is currently supported. + Say 'Y' or 'M' here if you have one such device. + endifdrivers/usb/dwc3/Makefile +1
-0@@ -54,3 +54,4 @@ obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o obj-$(CONFIG_USB_DWC3_IMX8MP) += dwc3-imx8mp.o obj-$(CONFIG_USB_DWC3_XILINX) += dwc3-xilinx.o +obj-$(CONFIG_USB_DWC3_OCTEON) += dwc3-octeon.odrivers/usb/dwc3/dwc3-octeon.c renamed from arch/mips/cavium-octeon/octeon-usb.c +9
-1@@ -187,7 +187,10 @@#define USBDRD_UCTL_ECC 0xf0#define USBDRD_UCTL_SPARE1 0xf8 -static DEFINE_MUTEX (dwc3_octeon_clocks_mutex); +struct dwc3_data { +struct device *dev; +void __iomem *base; +};#ifdef CONFIG_CAVIUM_OCTEON_SOC #include <asm/octeon/octeon.h> @@ -233,6 +236,11 @@ static inline uint64_t dwc3_octeon_readq(void __iomem *addr)static inline void dwc3_octeon_writeq (void __iomem *base,uint64_t val) { }static inline void dwc3_octeon_config_gpio (int index,int gpio) { } + +static uint64_t octeon_get_io_clock_rate (void ) +{ +return 150000000 ; +}#endif static int dwc3_octeon_get_divider (void )
drivers/usb/dwc3/dwc3-octeon.c static int dwc3_octeon_config_power(struct device *dev, void __iomem *base) · line 282
Quoted 5 lines
drivers/usb/dwc3/dwc3-octeon.c renamed from arch/mips/cavium-octeon/octeon-usb.c +1
-1@@ -271,7 +279,7 @@ static int dwc3_octeon_config_power(struct device *dev, void __iomem *base)dev_err (dev,"invalid power configuration\n" );return -EINVAL ; } -dwc3_octeon_config_gpio (((u64 )base >>24 ) &1 , gpio); +dwc3_octeon_config_gpio (((__force u64 )base >>24 ) &1 , gpio);
You're doing more than just moving the code here. Please separate the change to a different patch if there are additional functional change and provide the reason for it.
drivers/usb/dwc3/dwc3-octeon.c static int dwc3_octeon_clocks_start(struct device *dev, void __iomem *base) · line 394
Quoted 9 lines
drivers/usb/dwc3/dwc3-octeon.c renamed from arch/mips/cavium-octeon/octeon-usb.c +1
-1/* Enable XHCI power control and set if active high or low. */ val =dwc3_octeon_readq (uctl_host_cfg_reg);@@ -383,7 +391,7 @@ static int dwc3_octeon_clocks_start(struct device *dev, void __iomem *base)if ((div !=FIELD_GET (USBDRD_UCTL_CTL_H_CLKDIV_SEL , val)) || (!(FIELD_GET (USBDRD_UCTL_CTL_H_CLK_EN , val)))) {dev_err (dev,"dwc3 controller clock init failure.\n" ); -return -EINVAL ; +return -EINVAL ;
Avoid mixing cleanup change with functional change.
Quoted 124 lines
drivers/usb/dwc3/dwc3-octeon.c renamed from arch/mips/cavium-octeon/octeon-usb.c +47
-47}/* Step 4c: Deassert the controller clock divider reset. */ @@ -494,58 +502,58 @@ static void __init dwc3_octeon_phy_reset(void __iomem *base)dwc3_octeon_writeq (uctl_ctl_reg, val); } -static int __init dwc3_octeon_device_init (void ) +static int dwc3_octeon_probe (struct platform_device *pdev) { -const char compat_node_name[] ="cavium,octeon-7130-usb-uctl" ; -struct platform_device *pdev; -struct device_node *node; -struct resource *res; -void __iomem *base; +struct device *dev = &pdev->dev; +struct dwc3_data *data; +int err; -/* -* There should only be three universal controllers, "uctl" -* in the device tree. Two USB and a SATA, which we ignore. -*/ - node =NULL ; -do { - node =of_find_node_by_name (node,"uctl" ); -if (!node) -return -ENODEV ; - -if (of_device_is_compatible (node, compat_node_name)) { - pdev =of_find_device_by_node (node); -if (!pdev) -return -ENODEV ; - -/* -* The code below maps in the registers necessary for -* setting up the clocks and reseting PHYs. We must -* release the resources so the dwc3 subsystem doesn't -* know the difference. -*/ - base =devm_platform_get_and_ioremap_resource (pdev,0 , &res); -if (IS_ERR (base)) { -put_device (&pdev->dev); -return PTR_ERR (base); - } + data =devm_kzalloc (dev,sizeof (*data),GFP_KERNEL ); +if (!data) +return -ENOMEM ; -mutex_lock (&dwc3_octeon_clocks_mutex); -if (dwc3_octeon_clocks_start (&pdev->dev, base) ==0 ) -dev_info (&pdev->dev,"clocks initialized.\n" ); -dwc3_octeon_set_endian_mode (base); -dwc3_octeon_phy_reset (base); -mutex_unlock (&dwc3_octeon_clocks_mutex); -devm_iounmap (&pdev->dev, base); -devm_release_mem_region (&pdev->dev, res->start, -resource_size (res)); -put_device (&pdev->dev); - } - }while (node !=NULL ); + data->base =devm_platform_ioremap_resource (pdev,0 ); +if (IS_ERR (data->base)) +return PTR_ERR (data->base); -return 0 ; + err =dwc3_octeon_clocks_start (dev, data->base); +if (err) +return err; + +dwc3_octeon_set_endian_mode (data->base); +dwc3_octeon_phy_reset (data->base); + + data->dev = dev; +platform_set_drvdata (pdev, data); + +return of_platform_populate (node,NULL ,NULL , dev); +} + +static void dwc3_octeon_remove (struct platform_device *pdev) +{ +struct dwc3_data *data =platform_get_drvdata (pdev); + +of_platform_depopulate (data->dev); +platform_set_drvdata (pdev,NULL ); } -device_initcall (dwc3_octeon_device_init); +static const struct of_device_id dwc3_octeon_of_match[] = { + { .compatible ="cavium,octeon-7130-usb-uctl" }, + { }, +}; +MODULE_DEVICE_TABLE (of, dwc3_octeon_of_match); + +static struct platform_driver dwc3_octeon_driver = { + .probe = dwc3_octeon_probe, + .remove_new = dwc3_octeon_remove, + .driver = { + .name ="dwc3-octeon" , + .of_match_table = dwc3_octeon_of_match, + }, +}; +module_platform_driver (dwc3_octeon_driver); + +MODULE_ALIAS ("platform:dwc3-octeon" );MODULE_AUTHOR ("David Daney <[email protected]>" );MODULE_LICENSE ("GPL" ); -MODULE_DESCRIPTION ("USB driver for OCTEON III SoC" ); +MODULE_DESCRIPTION ("DesignWare USB3 OCTEON III Glue Layer" );drivers/usb/dwc3/dwc3-of-simple.c +0
-1@@ -172,7 +172,6 @@ static const struct dev_pm_ops dwc3_of_simple_dev_pm_ops = {static const struct of_device_id of_dwc3_simple_match[] = { { .compatible ="rockchip,rk3399-dwc3" }, - { .compatible ="cavium,octeon-7130-usb-uctl" }, { .compatible ="sprd,sc9860-dwc3" }, { .compatible ="allwinner,sun50i-h6-dwc3" }, { .compatible ="hisilicon,hi3670-dwc3" },Signature · 1 line
2.39.2
Thanks,
Thinh
Re: [PATCH v2 1/3] usb: dwc3: dwc3-octeon: Convert to glue driver
Quoted 106 lines · Thinh Nguyen
Quoted 4 lines · Ladislav Michl
From: Ladislav Michl <[email protected]>
Move Octeon DWC3 glue code from
arch/mipsand use it instead of dwc3-of-simple.Please provide more context and the reason to why the move here.
Quoted 98 lines
Signed-off-by Ladislav Michl <[email protected]>---
CHANGES:
- v2: squashed move and glue conversion patch, fixed sparse warning and formatting issue. Set private data at the end of probe. Clear drvdata on remove. Added host mode only notice. Collected ack for move fromarch/mips.arch/mips/cavium-octeon/Makefile | 1 - arch/mips/cavium-octeon/octeon-platform.c | 1 - drivers/usb/dwc3/Kconfig | 10 ++ drivers/usb/dwc3/Makefile | 1 + .../usb/dwc3/dwc3-octeon.c | 108 ++++++++++-------- drivers/usb/dwc3/dwc3-of-simple.c | 1 - 6 files changed, 69 insertions(+), 53 deletions(-) rename arch/mips/cavium-octeon/octeon-usb.c => drivers/usb/dwc3/dwc3-octeon.c (91%)arch/mips/cavium-octeon/Makefile +0
-1@@ -18,4 +18,3 @@ obj-y += crypto/ obj-$(CONFIG_MTD) += flash_setup.o obj-$(CONFIG_SMP) += smp.o obj-$(CONFIG_OCTEON_ILM) += oct_ilm.o -obj-$(CONFIG_USB) += octeon-usb.oarch/mips/cavium-octeon/octeon-platform.c +0
-1@@ -450,7 +450,6 @@ static const struct of_device_id octeon_ids[] __initconst = { { .compatible = "cavium,octeon-3860-bootbus", }, { .compatible = "cavium,mdio-mux", }, { .compatible = "gpio-leds", }, - { .compatible = "cavium,octeon-7130-usb-uctl", }, {}, };drivers/usb/dwc3/Kconfig +10
-0@@ -168,4 +168,14 @@ config USB_DWC3_AM62 The Designware Core USB3 IP is programmed to operate in in USB 2.0 mode only. Say 'Y' or 'M' here if you have one such device + +config USB_DWC3_OCTEON + tristate "Cavium Octeon Platforms" + depends on CAVIUM_OCTEON_SOC || COMPILE_TEST + default USB_DWC3 + help + Support Cavium Octeon platforms with DesignWare Core USB3 IP. + Only the host mode is currently supported. + Say 'Y' or 'M' here if you have one such device. + endifdrivers/usb/dwc3/Makefile +1
-0@@ -54,3 +54,4 @@ obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o obj-$(CONFIG_USB_DWC3_IMX8MP) += dwc3-imx8mp.o obj-$(CONFIG_USB_DWC3_XILINX) += dwc3-xilinx.o +obj-$(CONFIG_USB_DWC3_OCTEON) += dwc3-octeon.odrivers/usb/dwc3/dwc3-octeon.c renamed from arch/mips/cavium-octeon/octeon-usb.c +9
-1@@ -187,7 +187,10 @@#define USBDRD_UCTL_ECC 0xf0#define USBDRD_UCTL_SPARE1 0xf8 -static DEFINE_MUTEX (dwc3_octeon_clocks_mutex); +struct dwc3_data { +struct device *dev; +void __iomem *base; +};#ifdef CONFIG_CAVIUM_OCTEON_SOC #include <asm/octeon/octeon.h> @@ -233,6 +236,11 @@ static inline uint64_t dwc3_octeon_readq(void __iomem *addr)static inline void dwc3_octeon_writeq (void __iomem *base,uint64_t val) { }static inline void dwc3_octeon_config_gpio (int index,int gpio) { } + +static uint64_t octeon_get_io_clock_rate (void ) +{ +return 150000000 ; +}#endif
Quoted 11 lines
drivers/usb/dwc3/dwc3-octeon.c static int dwc3_octeon_config_power(struct device *dev, void __iomem *base) · line 282
Quoted 7 lines
drivers/usb/dwc3/dwc3-octeon.c renamed from arch/mips/cavium-octeon/octeon-usb.c +1
-1static int dwc3_octeon_get_divider (void )@@ -271,7 +279,7 @@ static int dwc3_octeon_config_power(struct device *dev, void __iomem *base)dev_err (dev,"invalid power configuration\n" );return -EINVAL ; } -dwc3_octeon_config_gpio (((u64 )base >>24 ) &1 , gpio); +dwc3_octeon_config_gpio (((__force u64 )base >>24 ) &1 , gpio);You're doing more than just moving the code here. Please separate the change to a different patch if there are additional functional change and provide the reason for it.
Is it okay to move file with sparse warning or am I supposed to fix it in the arch code first?
Quoted 11 lines
drivers/usb/dwc3/dwc3-octeon.c static int dwc3_octeon_clocks_start(struct device *dev, void __iomem *base) · line 394
Quoted 9 lines
drivers/usb/dwc3/dwc3-octeon.c renamed from arch/mips/cavium-octeon/octeon-usb.c +1
-1/* Enable XHCI power control and set if active high or low. */ val =dwc3_octeon_readq (uctl_host_cfg_reg);@@ -383,7 +391,7 @@ static int dwc3_octeon_clocks_start(struct device *dev, void __iomem *base)if ((div !=FIELD_GET (USBDRD_UCTL_CTL_H_CLKDIV_SEL , val)) || (!(FIELD_GET (USBDRD_UCTL_CTL_H_CLK_EN , val)))) {dev_err (dev,"dwc3 controller clock init failure.\n" ); -return -EINVAL ; +return -EINVAL ;Avoid mixing cleanup change with functional change.
I'll just drop it, not worth separate patch.
Quoted 127 lines
Quoted 124 lines
drivers/usb/dwc3/dwc3-octeon.c renamed from arch/mips/cavium-octeon/octeon-usb.c +47
-47}/* Step 4c: Deassert the controller clock divider reset. */ @@ -494,58 +502,58 @@ static void __init dwc3_octeon_phy_reset(void __iomem *base)dwc3_octeon_writeq (uctl_ctl_reg, val); } -static int __init dwc3_octeon_device_init (void ) +static int dwc3_octeon_probe (struct platform_device *pdev) { -const char compat_node_name[] ="cavium,octeon-7130-usb-uctl" ; -struct platform_device *pdev; -struct device_node *node; -struct resource *res; -void __iomem *base; +struct device *dev = &pdev->dev; +struct dwc3_data *data; +int err; -/* -* There should only be three universal controllers, "uctl" -* in the device tree. Two USB and a SATA, which we ignore. -*/ - node =NULL ; -do { - node =of_find_node_by_name (node,"uctl" ); -if (!node) -return -ENODEV ; - -if (of_device_is_compatible (node, compat_node_name)) { - pdev =of_find_device_by_node (node); -if (!pdev) -return -ENODEV ; - -/* -* The code below maps in the registers necessary for -* setting up the clocks and reseting PHYs. We must -* release the resources so the dwc3 subsystem doesn't -* know the difference. -*/ - base =devm_platform_get_and_ioremap_resource (pdev,0 , &res); -if (IS_ERR (base)) { -put_device (&pdev->dev); -return PTR_ERR (base); - } + data =devm_kzalloc (dev,sizeof (*data),GFP_KERNEL ); +if (!data) +return -ENOMEM ; -mutex_lock (&dwc3_octeon_clocks_mutex); -if (dwc3_octeon_clocks_start (&pdev->dev, base) ==0 ) -dev_info (&pdev->dev,"clocks initialized.\n" ); -dwc3_octeon_set_endian_mode (base); -dwc3_octeon_phy_reset (base); -mutex_unlock (&dwc3_octeon_clocks_mutex); -devm_iounmap (&pdev->dev, base); -devm_release_mem_region (&pdev->dev, res->start, -resource_size (res)); -put_device (&pdev->dev); - } - }while (node !=NULL ); + data->base =devm_platform_ioremap_resource (pdev,0 ); +if (IS_ERR (data->base)) +return PTR_ERR (data->base); -return 0 ; + err =dwc3_octeon_clocks_start (dev, data->base); +if (err) +return err; + +dwc3_octeon_set_endian_mode (data->base); +dwc3_octeon_phy_reset (data->base); + + data->dev = dev; +platform_set_drvdata (pdev, data); + +return of_platform_populate (node,NULL ,NULL , dev); +} + +static void dwc3_octeon_remove (struct platform_device *pdev) +{ +struct dwc3_data *data =platform_get_drvdata (pdev); + +of_platform_depopulate (data->dev); +platform_set_drvdata (pdev,NULL ); } -device_initcall (dwc3_octeon_device_init); +static const struct of_device_id dwc3_octeon_of_match[] = { + { .compatible ="cavium,octeon-7130-usb-uctl" }, + { }, +}; +MODULE_DEVICE_TABLE (of, dwc3_octeon_of_match); + +static struct platform_driver dwc3_octeon_driver = { + .probe = dwc3_octeon_probe, + .remove_new = dwc3_octeon_remove, + .driver = { + .name ="dwc3-octeon" , + .of_match_table = dwc3_octeon_of_match, + }, +}; +module_platform_driver (dwc3_octeon_driver); + +MODULE_ALIAS ("platform:dwc3-octeon" );MODULE_AUTHOR ("David Daney <[email protected]>" );MODULE_LICENSE ("GPL" ); -MODULE_DESCRIPTION ("USB driver for OCTEON III SoC" ); +MODULE_DESCRIPTION ("DesignWare USB3 OCTEON III Glue Layer" );drivers/usb/dwc3/dwc3-of-simple.c +0
-1@@ -172,7 +172,6 @@ static const struct dev_pm_ops dwc3_of_simple_dev_pm_ops = {static const struct of_device_id of_dwc3_simple_match[] = { { .compatible ="rockchip,rk3399-dwc3" }, - { .compatible ="cavium,octeon-7130-usb-uctl" }, { .compatible ="sprd,sc9860-dwc3" }, { .compatible ="allwinner,sun50i-h6-dwc3" }, { .compatible ="hisilicon,hi3670-dwc3" },Signature · 1 line
2.39.2Thanks,
Thinh
Thinh Nguyen
maintainerRe: [PATCH v2 1/3] usb: dwc3: dwc3-octeon: Convert to glue driver
Quoted 119 lines · Ladislav Michl
Quoted 106 lines · Thinh Nguyen
Quoted 4 lines · Ladislav Michl
From: Ladislav Michl <[email protected]>
Move Octeon DWC3 glue code from
arch/mipsand use it instead of dwc3-of-simple.Please provide more context and the reason to why the move here.
Quoted 98 lines
Signed-off-by Ladislav Michl <[email protected]>---
CHANGES:
- v2: squashed move and glue conversion patch, fixed sparse warning and formatting issue. Set private data at the end of probe. Clear drvdata on remove. Added host mode only notice. Collected ack for move fromarch/mips.arch/mips/cavium-octeon/Makefile | 1 - arch/mips/cavium-octeon/octeon-platform.c | 1 - drivers/usb/dwc3/Kconfig | 10 ++ drivers/usb/dwc3/Makefile | 1 + .../usb/dwc3/dwc3-octeon.c | 108 ++++++++++-------- drivers/usb/dwc3/dwc3-of-simple.c | 1 - 6 files changed, 69 insertions(+), 53 deletions(-) rename arch/mips/cavium-octeon/octeon-usb.c => drivers/usb/dwc3/dwc3-octeon.c (91%)arch/mips/cavium-octeon/Makefile +0
-1@@ -18,4 +18,3 @@ obj-y += crypto/ obj-$(CONFIG_MTD) += flash_setup.o obj-$(CONFIG_SMP) += smp.o obj-$(CONFIG_OCTEON_ILM) += oct_ilm.o -obj-$(CONFIG_USB) += octeon-usb.oarch/mips/cavium-octeon/octeon-platform.c +0
-1@@ -450,7 +450,6 @@ static const struct of_device_id octeon_ids[] __initconst = { { .compatible = "cavium,octeon-3860-bootbus", }, { .compatible = "cavium,mdio-mux", }, { .compatible = "gpio-leds", }, - { .compatible = "cavium,octeon-7130-usb-uctl", }, {}, };drivers/usb/dwc3/Kconfig +10
-0@@ -168,4 +168,14 @@ config USB_DWC3_AM62 The Designware Core USB3 IP is programmed to operate in in USB 2.0 mode only. Say 'Y' or 'M' here if you have one such device + +config USB_DWC3_OCTEON + tristate "Cavium Octeon Platforms" + depends on CAVIUM_OCTEON_SOC || COMPILE_TEST + default USB_DWC3 + help + Support Cavium Octeon platforms with DesignWare Core USB3 IP. + Only the host mode is currently supported. + Say 'Y' or 'M' here if you have one such device. + endifdrivers/usb/dwc3/Makefile +1
-0@@ -54,3 +54,4 @@ obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o obj-$(CONFIG_USB_DWC3_IMX8MP) += dwc3-imx8mp.o obj-$(CONFIG_USB_DWC3_XILINX) += dwc3-xilinx.o +obj-$(CONFIG_USB_DWC3_OCTEON) += dwc3-octeon.odrivers/usb/dwc3/dwc3-octeon.c renamed from arch/mips/cavium-octeon/octeon-usb.c +9
-1@@ -187,7 +187,10 @@#define USBDRD_UCTL_ECC 0xf0#define USBDRD_UCTL_SPARE1 0xf8 -static DEFINE_MUTEX (dwc3_octeon_clocks_mutex); +struct dwc3_data { +struct device *dev; +void __iomem *base; +};#ifdef CONFIG_CAVIUM_OCTEON_SOC #include <asm/octeon/octeon.h> @@ -233,6 +236,11 @@ static inline uint64_t dwc3_octeon_readq(void __iomem *addr)static inline void dwc3_octeon_writeq (void __iomem *base,uint64_t val) { }static inline void dwc3_octeon_config_gpio (int index,int gpio) { } + +static uint64_t octeon_get_io_clock_rate (void ) +{ +return 150000000 ; +}#endif Quoted 11 lines
drivers/usb/dwc3/dwc3-octeon.c static int dwc3_octeon_config_power(struct device *dev, void __iomem *base) · line 282
Quoted 7 lines
drivers/usb/dwc3/dwc3-octeon.c renamed from arch/mips/cavium-octeon/octeon-usb.c +1
-1static int dwc3_octeon_get_divider (void )@@ -271,7 +279,7 @@ static int dwc3_octeon_config_power(struct device *dev, void __iomem *base)dev_err (dev,"invalid power configuration\n" );return -EINVAL ; } -dwc3_octeon_config_gpio (((u64 )base >>24 ) &1 , gpio); +dwc3_octeon_config_gpio (((__force u64 )base >>24 ) &1 , gpio);You're doing more than just moving the code here. Please separate the change to a different patch if there are additional functional change and provide the reason for it.
Quoted 3 lines
Is it okay to move file with sparse warning or am I supposed to fix it in the arch code first?
Yes. Keep the change to only what is described in the change log. Any other change should be in a separate commit.
Thanks,
Thinh
PATCH v2 2/3 usb: dwc3: dwc3-octeon: Move node parsing into driver probe
In reply to Ladislav MichlFrom: Ladislav Michl <[email protected]>
Make dwc3_octeon_clocks_start just start the clocks.
---
CHANGES:
-v2: if else block bracket according CodingStyle
drivers/usb/dwc3/dwc3-octeon.c | 148 ++++++++++++++++----------------- 1 file changed, 71 insertions(+), 77 deletions(-)
drivers/usb/dwc3/dwc3-octeon.c +71 -77
@@ -300,67 +300,14 @@ static int dwc3_octeon_config_power(struct device *dev, void __iomem *base)return 0 ; } -static int dwc3_octeon_clocks_start (struct device *dev,void __iomem *base) +static int dwc3_octeon_clocks_start (struct device *dev,void __iomem *base, +int ref_clk_sel,int ref_clk_fsel, +int mpll_mul) { -int i, div, mpll_mul, ref_clk_fsel, ref_clk_sel =2 ; -u32 clock_rate; +int div;u64 val;void __iomem *uctl_ctl_reg = base +USBDRD_UCTL_CTL ; -if (dev->of_node) { -const char *ss_clock_type; -const char *hs_clock_type; - - i =of_property_read_u32 (dev->of_node, -"refclk-frequency" , &clock_rate); -if (i) { -dev_err (dev,"No UCTL \"refclk-frequency\"\n" ); -return -EINVAL ; - } - i =of_property_read_string (dev->of_node, -"refclk-type-ss" , &ss_clock_type); -if (i) { -dev_err (dev,"No UCTL \"refclk-type-ss\"\n" ); -return -EINVAL ; - } - i =of_property_read_string (dev->of_node, -"refclk-type-hs" , &hs_clock_type); -if (i) { -dev_err (dev,"No UCTL \"refclk-type-hs\"\n" ); -return -EINVAL ; - } -if (strcmp ("dlmc_ref_clk0" , ss_clock_type) ==0 ) { -if (strcmp (hs_clock_type,"dlmc_ref_clk0" ) ==0 ) - ref_clk_sel =0 ; -else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) - ref_clk_sel =2 ; -else -dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , - hs_clock_type); - }else if (strcmp (ss_clock_type,"dlmc_ref_clk1" ) ==0 ) { -if (strcmp (hs_clock_type,"dlmc_ref_clk1" ) ==0 ) - ref_clk_sel =1 ; -else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) - ref_clk_sel =3 ; -else { -dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , - hs_clock_type); - ref_clk_sel =3 ; - } - }else -dev_warn (dev,"Invalid SS clock type %s, using dlmc_ref_clk0 instead\n" , - ss_clock_type); - -if ((ref_clk_sel ==0 || ref_clk_sel ==1 ) && - (clock_rate !=100000000 )) -dev_warn (dev,"Invalid UCTL clock rate of %u, using 100000000 instead\n" , - clock_rate); - - }else { -dev_err (dev,"No USB UCTL device node\n" ); -return -EINVAL ; - } -/* * Step 1: Wait for all voltages to be stable...that surely * happened before starting the kernel. SKIP
@@ -404,24 +351,6 @@ static int dwc3_octeon_clocks_start(struct device *dev, void __iomem *base) val &= ~USBDRD_UCTL_CTL_REF_CLK_SEL ; val |=FIELD_PREP (USBDRD_UCTL_CTL_REF_CLK_SEL , ref_clk_sel); - ref_clk_fsel =0x07 ; -switch (clock_rate) { -default : -dev_warn (dev,"Invalid ref_clk %u, using 100000000 instead\n" , - clock_rate); - fallthrough; -case 100000000 : - mpll_mul =0x19 ; -if (ref_clk_sel <2 ) - ref_clk_fsel =0x27 ; -break ; -case 50000000 : - mpll_mul =0x32 ; -break ; -case 125000000 : - mpll_mul =0x28 ; -break ; - } val &= ~USBDRD_UCTL_CTL_REF_CLK_FSEL ; val |=FIELD_PREP (USBDRD_UCTL_CTL_REF_CLK_FSEL , ref_clk_fsel);
@@ -505,8 +434,72 @@ static void __init dwc3_octeon_phy_reset(void __iomem *base)static int dwc3_octeon_probe (struct platform_device *pdev) {struct device *dev = &pdev->dev; +struct device_node *node = dev->of_node;struct dwc3_data *data; -int err; +int err, ref_clk_sel, ref_clk_fsel, mpll_mul; +uint32_t clock_rate; +const char *hs_clock_type, *ss_clock_type; + +if (!node) { +dev_err (dev,"No USB UCTL device node\n" ); +return -EINVAL ; + } + +if (of_property_read_u32 (node,"refclk-frequency" , &clock_rate)) { +dev_err (dev,"No UCTL \"refclk-frequency\"\n" ); +return -EINVAL ; + } +if (of_property_read_string (node,"refclk-type-ss" , &ss_clock_type)) { +dev_err (dev,"No UCTL \"refclk-type-ss\"\n" ); +return -EINVAL ; + } +if (of_property_read_string (node,"refclk-type-hs" , &hs_clock_type)) { +dev_err (dev,"No UCTL \"refclk-type-hs\"\n" ); +return -EINVAL ; + } + + ref_clk_sel =2 ; +if (strcmp ("dlmc_ref_clk0" , ss_clock_type) ==0 ) { +if (strcmp (hs_clock_type,"dlmc_ref_clk0" ) ==0 ) + ref_clk_sel =0 ; +else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) + ref_clk_sel =2 ; +else +dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , + hs_clock_type); + }else if (strcmp (ss_clock_type,"dlmc_ref_clk1" ) ==0 ) { +if (strcmp (hs_clock_type,"dlmc_ref_clk1" ) ==0 ) + ref_clk_sel =1 ; +else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) { + ref_clk_sel =3 ; + }else { +dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , + hs_clock_type); + ref_clk_sel =3 ; + } + }else { +dev_warn (dev,"Invalid SS clock type %s, using dlmc_ref_clk0 instead\n" , + ss_clock_type); + } + + ref_clk_fsel =0x07 ; +switch (clock_rate) { +default : +dev_warn (dev,"Invalid ref_clk %u, using 100000000 instead\n" , + clock_rate); + fallthrough; +case 100000000 : + mpll_mul =0x19 ; +if (ref_clk_sel <2 ) + ref_clk_fsel =0x27 ; +break ; +case 50000000 : + mpll_mul =0x32 ; +break ; +case 125000000 : + mpll_mul =0x28 ; +break ; + } data =devm_kzalloc (dev,sizeof (*data),GFP_KERNEL );if (!data)
@@ -516,7 +509,8 @@ static int dwc3_octeon_probe(struct platform_device *pdev)if (IS_ERR (data->base))return PTR_ERR (data->base); - err =dwc3_octeon_clocks_start (dev, data->base); + err =dwc3_octeon_clocks_start (dev, data->base, + ref_clk_sel, ref_clk_fsel, mpll_mul);if (err)return err;
Signature · 1 line
2.39.2
Thinh Nguyen
maintainerRe: [PATCH v2 2/3] usb: dwc3: dwc3-octeon: Move node parsing into driver probe
Quoted 3 lines · Ladislav Michl
From: Ladislav Michl <[email protected]>
Make
dwc3_octeon_clocks_startjust start the clocks.
This commit message is different than the subject. Also, please explain why we need to move this logic to probe in this commit message body.
Quoted 111 lines
Signed-off-by Ladislav Michl <[email protected]>---
CHANGES:
-v2: if else block bracket according CodingStyledrivers/usb/dwc3/dwc3-octeon.c | 148 ++++++++++++++++----------------- 1 file changed, 71 insertions(+), 77 deletions(-)drivers/usb/dwc3/dwc3-octeon.c +4
-75@@ -300,67 +300,14 @@ static int dwc3_octeon_config_power(struct device *dev, void __iomem *base)return 0 ; } -static int dwc3_octeon_clocks_start (struct device *dev,void __iomem *base) +static int dwc3_octeon_clocks_start (struct device *dev,void __iomem *base, +int ref_clk_sel,int ref_clk_fsel, +int mpll_mul) { -int i, div, mpll_mul, ref_clk_fsel, ref_clk_sel =2 ; -u32 clock_rate; +int div;u64 val;void __iomem *uctl_ctl_reg = base +USBDRD_UCTL_CTL ; -if (dev->of_node) { -const char *ss_clock_type; -const char *hs_clock_type; - - i =of_property_read_u32 (dev->of_node, -"refclk-frequency" , &clock_rate); -if (i) { -dev_err (dev,"No UCTL \"refclk-frequency\"\n" ); -return -EINVAL ; - } - i =of_property_read_string (dev->of_node, -"refclk-type-ss" , &ss_clock_type); -if (i) { -dev_err (dev,"No UCTL \"refclk-type-ss\"\n" ); -return -EINVAL ; - } - i =of_property_read_string (dev->of_node, -"refclk-type-hs" , &hs_clock_type); -if (i) { -dev_err (dev,"No UCTL \"refclk-type-hs\"\n" ); -return -EINVAL ; - } -if (strcmp ("dlmc_ref_clk0" , ss_clock_type) ==0 ) { -if (strcmp (hs_clock_type,"dlmc_ref_clk0" ) ==0 ) - ref_clk_sel =0 ; -else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) - ref_clk_sel =2 ; -else -dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , - hs_clock_type); - }else if (strcmp (ss_clock_type,"dlmc_ref_clk1" ) ==0 ) { -if (strcmp (hs_clock_type,"dlmc_ref_clk1" ) ==0 ) - ref_clk_sel =1 ; -else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) - ref_clk_sel =3 ; -else { -dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , - hs_clock_type); - ref_clk_sel =3 ; - } - }else -dev_warn (dev,"Invalid SS clock type %s, using dlmc_ref_clk0 instead\n" , - ss_clock_type); - -if ((ref_clk_sel ==0 || ref_clk_sel ==1 ) && - (clock_rate !=100000000 )) -dev_warn (dev,"Invalid UCTL clock rate of %u, using 100000000 instead\n" , - clock_rate); - - }else { -dev_err (dev,"No USB UCTL device node\n" ); -return -EINVAL ; - } -/* * Step 1: Wait for all voltages to be stable...that surely * happened before starting the kernel. SKIP @@ -404,24 +351,6 @@ static int dwc3_octeon_clocks_start(struct device *dev, void __iomem *base) val &= ~USBDRD_UCTL_CTL_REF_CLK_SEL ; val |=FIELD_PREP (USBDRD_UCTL_CTL_REF_CLK_SEL , ref_clk_sel); - ref_clk_fsel =0x07 ; -switch (clock_rate) { -default : -dev_warn (dev,"Invalid ref_clk %u, using 100000000 instead\n" , - clock_rate); - fallthrough; -case 100000000 : - mpll_mul =0x19 ; -if (ref_clk_sel <2 ) - ref_clk_fsel =0x27 ; -break ; -case 50000000 : - mpll_mul =0x32 ; -break ; -case 125000000 : - mpll_mul =0x28 ; -break ; - } val &= ~USBDRD_UCTL_CTL_REF_CLK_FSEL ; val |=FIELD_PREP (USBDRD_UCTL_CTL_REF_CLK_FSEL , ref_clk_fsel);
drivers/usb/dwc3/dwc3-octeon.c static void __init dwc3_octeon_phy_reset(void __iomem *base) · line 440
Quoted 8 lines
drivers/usb/dwc3/dwc3-octeon.c +3
-1@@ -505,8 +434,72 @@ static void __init dwc3_octeon_phy_reset(void __iomem *base)static int dwc3_octeon_probe (struct platform_device *pdev) {struct device *dev = &pdev->dev; +struct device_node *node = dev->of_node;struct dwc3_data *data; -int err; +int err, ref_clk_sel, ref_clk_fsel, mpll_mul; +uint32_t clock_rate;
Use u32?
Quoted 20 lines
drivers/usb/dwc3/dwc3-octeon.c +20
-0+const char *hs_clock_type, *ss_clock_type; + +if (!node) { +dev_err (dev,"No USB UCTL device node\n" ); +return -EINVAL ; + } + +if (of_property_read_u32 (node,"refclk-frequency" , &clock_rate)) { +dev_err (dev,"No UCTL \"refclk-frequency\"\n" ); +return -EINVAL ; + } +if (of_property_read_string (node,"refclk-type-ss" , &ss_clock_type)) { +dev_err (dev,"No UCTL \"refclk-type-ss\"\n" ); +return -EINVAL ; + } +if (of_property_read_string (node,"refclk-type-hs" , &hs_clock_type)) { +dev_err (dev,"No UCTL \"refclk-type-hs\"\n" ); +return -EINVAL ; + } +
drivers/usb/dwc3/dwc3-octeon.c static void __init dwc3_octeon_phy_reset(void __iomem *base) · line 470
Quoted 10 lines
drivers/usb/dwc3/dwc3-octeon.c +10
-0+ ref_clk_sel =2 ; +if (strcmp ("dlmc_ref_clk0" , ss_clock_type) ==0 ) { +if (strcmp (hs_clock_type,"dlmc_ref_clk0" ) ==0 ) + ref_clk_sel =0 ; +else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) + ref_clk_sel =2 ; +else +dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , + hs_clock_type); + }else if (strcmp (ss_clock_type,"dlmc_ref_clk1" ) ==0 ) {
Did you run checkpatch? I still see some minor formatting issues.
Quoted 48 lines
drivers/usb/dwc3/dwc3-octeon.c +34
-1+if (strcmp (hs_clock_type,"dlmc_ref_clk1" ) ==0 ) + ref_clk_sel =1 ; +else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) { + ref_clk_sel =3 ; + }else { +dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , + hs_clock_type); + ref_clk_sel =3 ; + } + }else { +dev_warn (dev,"Invalid SS clock type %s, using dlmc_ref_clk0 instead\n" , + ss_clock_type); + } + + ref_clk_fsel =0x07 ; +switch (clock_rate) { +default : +dev_warn (dev,"Invalid ref_clk %u, using 100000000 instead\n" , + clock_rate); + fallthrough; +case 100000000 : + mpll_mul =0x19 ; +if (ref_clk_sel <2 ) + ref_clk_fsel =0x27 ; +break ; +case 50000000 : + mpll_mul =0x32 ; +break ; +case 125000000 : + mpll_mul =0x28 ; +break ; + } data =devm_kzalloc (dev,sizeof (*data),GFP_KERNEL );if (!data)@@ -516,7 +509,8 @@ static int dwc3_octeon_probe(struct platform_device *pdev)if (IS_ERR (data->base))return PTR_ERR (data->base); - err =dwc3_octeon_clocks_start (dev, data->base); + err =dwc3_octeon_clocks_start (dev, data->base, + ref_clk_sel, ref_clk_fsel, mpll_mul);if (err)return err;Signature · 1 line
2.39.2
BR,
Thinh
Re: [PATCH v2 2/3] usb: dwc3: dwc3-octeon: Move node parsing into driver probe
Quoted 118 lines · Thinh Nguyen
Quoted 3 lines · Ladislav Michl
From: Ladislav Michl <[email protected]>
Make
dwc3_octeon_clocks_startjust start the clocks.This commit message is different than the subject. Also, please explain why we need to move this logic to probe in this commit message body.
Quoted 110 lines
Signed-off-by Ladislav Michl <[email protected]>---
CHANGES:
-v2: if else block bracket according CodingStyledrivers/usb/dwc3/dwc3-octeon.c | 148 ++++++++++++++++----------------- 1 file changed, 71 insertions(+), 77 deletions(-)drivers/usb/dwc3/dwc3-octeon.c +4
-75@@ -300,67 +300,14 @@ static int dwc3_octeon_config_power(struct device *dev, void __iomem *base)return 0 ; } -static int dwc3_octeon_clocks_start (struct device *dev,void __iomem *base) +static int dwc3_octeon_clocks_start (struct device *dev,void __iomem *base, +int ref_clk_sel,int ref_clk_fsel, +int mpll_mul) { -int i, div, mpll_mul, ref_clk_fsel, ref_clk_sel =2 ; -u32 clock_rate; +int div;u64 val;void __iomem *uctl_ctl_reg = base +USBDRD_UCTL_CTL ; -if (dev->of_node) { -const char *ss_clock_type; -const char *hs_clock_type; - - i =of_property_read_u32 (dev->of_node, -"refclk-frequency" , &clock_rate); -if (i) { -dev_err (dev,"No UCTL \"refclk-frequency\"\n" ); -return -EINVAL ; - } - i =of_property_read_string (dev->of_node, -"refclk-type-ss" , &ss_clock_type); -if (i) { -dev_err (dev,"No UCTL \"refclk-type-ss\"\n" ); -return -EINVAL ; - } - i =of_property_read_string (dev->of_node, -"refclk-type-hs" , &hs_clock_type); -if (i) { -dev_err (dev,"No UCTL \"refclk-type-hs\"\n" ); -return -EINVAL ; - } -if (strcmp ("dlmc_ref_clk0" , ss_clock_type) ==0 ) { -if (strcmp (hs_clock_type,"dlmc_ref_clk0" ) ==0 ) - ref_clk_sel =0 ; -else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) - ref_clk_sel =2 ; -else -dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , - hs_clock_type); - }else if (strcmp (ss_clock_type,"dlmc_ref_clk1" ) ==0 ) { -if (strcmp (hs_clock_type,"dlmc_ref_clk1" ) ==0 ) - ref_clk_sel =1 ; -else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) - ref_clk_sel =3 ; -else { -dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , - hs_clock_type); - ref_clk_sel =3 ; - } - }else -dev_warn (dev,"Invalid SS clock type %s, using dlmc_ref_clk0 instead\n" , - ss_clock_type); - -if ((ref_clk_sel ==0 || ref_clk_sel ==1 ) && - (clock_rate !=100000000 )) -dev_warn (dev,"Invalid UCTL clock rate of %u, using 100000000 instead\n" , - clock_rate); - - }else { -dev_err (dev,"No USB UCTL device node\n" ); -return -EINVAL ; - } -/* * Step 1: Wait for all voltages to be stable...that surely * happened before starting the kernel. SKIP @@ -404,24 +351,6 @@ static int dwc3_octeon_clocks_start(struct device *dev, void __iomem *base) val &= ~USBDRD_UCTL_CTL_REF_CLK_SEL ; val |=FIELD_PREP (USBDRD_UCTL_CTL_REF_CLK_SEL , ref_clk_sel); - ref_clk_fsel =0x07 ; -switch (clock_rate) { -default : -dev_warn (dev,"Invalid ref_clk %u, using 100000000 instead\n" , - clock_rate); - fallthrough; -case 100000000 : - mpll_mul =0x19 ; -if (ref_clk_sel <2 ) - ref_clk_fsel =0x27 ; -break ; -case 50000000 : - mpll_mul =0x32 ; -break ; -case 125000000 : - mpll_mul =0x28 ; -break ; - } val &= ~USBDRD_UCTL_CTL_REF_CLK_FSEL ; val |=FIELD_PREP (USBDRD_UCTL_CTL_REF_CLK_FSEL , ref_clk_fsel);
Quoted 11 lines
drivers/usb/dwc3/dwc3-octeon.c static void __init dwc3_octeon_phy_reset(void __iomem *base) · line 440
Quoted 9 lines
drivers/usb/dwc3/dwc3-octeon.c +3
-1@@ -505,8 +434,72 @@ static void __init dwc3_octeon_phy_reset(void __iomem *base)static int dwc3_octeon_probe (struct platform_device *pdev) {struct device *dev = &pdev->dev; +struct device_node *node = dev->of_node;struct dwc3_data *data; -int err; +int err, ref_clk_sel, ref_clk_fsel, mpll_mul; +uint32_t clock_rate;Use u32?
I do not like homebrew types when we have standard ones. But it seems almost all usb code is using u32, so I can live with that.
Quoted 24 lines
Quoted 24 lines
drivers/usb/dwc3/dwc3-octeon.c +24
-0+const char *hs_clock_type, *ss_clock_type; + +if (!node) { +dev_err (dev,"No USB UCTL device node\n" ); +return -EINVAL ; + } + +if (of_property_read_u32 (node,"refclk-frequency" , &clock_rate)) { +dev_err (dev,"No UCTL \"refclk-frequency\"\n" ); +return -EINVAL ; + } +if (of_property_read_string (node,"refclk-type-ss" , &ss_clock_type)) { +dev_err (dev,"No UCTL \"refclk-type-ss\"\n" ); +return -EINVAL ; + } +if (of_property_read_string (node,"refclk-type-hs" , &hs_clock_type)) { +dev_err (dev,"No UCTL \"refclk-type-hs\"\n" ); +return -EINVAL ; + } + + ref_clk_sel =2 ; +if (strcmp ("dlmc_ref_clk0" , ss_clock_type) ==0 ) { +if (strcmp (hs_clock_type,"dlmc_ref_clk0" ) ==0 ) + ref_clk_sel =0 ;
Quoted 8 lines
drivers/usb/dwc3/dwc3-octeon.c static void __init dwc3_octeon_phy_reset(void __iomem *base) · line 470
Quoted 6 lines
drivers/usb/dwc3/dwc3-octeon.c +6
-0+else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) + ref_clk_sel =2 ; +else +dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , + hs_clock_type); + }else if (strcmp (ss_clock_type,"dlmc_ref_clk1" ) ==0 ) {Did you run checkpatch? I still see some minor formatting issues.
Yes I did. What complain do you see from checkpatch?
Quoted 51 lines
Quoted 48 lines
drivers/usb/dwc3/dwc3-octeon.c +34
-1+if (strcmp (hs_clock_type,"dlmc_ref_clk1" ) ==0 ) + ref_clk_sel =1 ; +else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) { + ref_clk_sel =3 ; + }else { +dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , + hs_clock_type); + ref_clk_sel =3 ; + } + }else { +dev_warn (dev,"Invalid SS clock type %s, using dlmc_ref_clk0 instead\n" , + ss_clock_type); + } + + ref_clk_fsel =0x07 ; +switch (clock_rate) { +default : +dev_warn (dev,"Invalid ref_clk %u, using 100000000 instead\n" , + clock_rate); + fallthrough; +case 100000000 : + mpll_mul =0x19 ; +if (ref_clk_sel <2 ) + ref_clk_fsel =0x27 ; +break ; +case 50000000 : + mpll_mul =0x32 ; +break ; +case 125000000 : + mpll_mul =0x28 ; +break ; + } data =devm_kzalloc (dev,sizeof (*data),GFP_KERNEL );if (!data)@@ -516,7 +509,8 @@ static int dwc3_octeon_probe(struct platform_device *pdev)if (IS_ERR (data->base))return PTR_ERR (data->base); - err =dwc3_octeon_clocks_start (dev, data->base); + err =dwc3_octeon_clocks_start (dev, data->base, + ref_clk_sel, ref_clk_fsel, mpll_mul);if (err)return err;Signature · 1 line
2.39.2BR,
Thinh
Thinh Nguyen
maintainerRe: [PATCH v2 2/3] usb: dwc3: dwc3-octeon: Move node parsing into driver probe
Quoted 167 lines · Ladislav Michl
Quoted 118 lines · Thinh Nguyen
Quoted 3 lines · Ladislav Michl
From: Ladislav Michl <[email protected]>
Make
dwc3_octeon_clocks_startjust start the clocks.This commit message is different than the subject. Also, please explain why we need to move this logic to probe in this commit message body.
Quoted 110 lines
Signed-off-by Ladislav Michl <[email protected]>---
CHANGES:
-v2: if else block bracket according CodingStyledrivers/usb/dwc3/dwc3-octeon.c | 148 ++++++++++++++++----------------- 1 file changed, 71 insertions(+), 77 deletions(-)drivers/usb/dwc3/dwc3-octeon.c +4
-75@@ -300,67 +300,14 @@ static int dwc3_octeon_config_power(struct device *dev, void __iomem *base)return 0 ; } -static int dwc3_octeon_clocks_start (struct device *dev,void __iomem *base) +static int dwc3_octeon_clocks_start (struct device *dev,void __iomem *base, +int ref_clk_sel,int ref_clk_fsel, +int mpll_mul) { -int i, div, mpll_mul, ref_clk_fsel, ref_clk_sel =2 ; -u32 clock_rate; +int div;u64 val;void __iomem *uctl_ctl_reg = base +USBDRD_UCTL_CTL ; -if (dev->of_node) { -const char *ss_clock_type; -const char *hs_clock_type; - - i =of_property_read_u32 (dev->of_node, -"refclk-frequency" , &clock_rate); -if (i) { -dev_err (dev,"No UCTL \"refclk-frequency\"\n" ); -return -EINVAL ; - } - i =of_property_read_string (dev->of_node, -"refclk-type-ss" , &ss_clock_type); -if (i) { -dev_err (dev,"No UCTL \"refclk-type-ss\"\n" ); -return -EINVAL ; - } - i =of_property_read_string (dev->of_node, -"refclk-type-hs" , &hs_clock_type); -if (i) { -dev_err (dev,"No UCTL \"refclk-type-hs\"\n" ); -return -EINVAL ; - } -if (strcmp ("dlmc_ref_clk0" , ss_clock_type) ==0 ) { -if (strcmp (hs_clock_type,"dlmc_ref_clk0" ) ==0 ) - ref_clk_sel =0 ; -else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) - ref_clk_sel =2 ; -else -dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , - hs_clock_type); - }else if (strcmp (ss_clock_type,"dlmc_ref_clk1" ) ==0 ) { -if (strcmp (hs_clock_type,"dlmc_ref_clk1" ) ==0 ) - ref_clk_sel =1 ; -else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) - ref_clk_sel =3 ; -else { -dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , - hs_clock_type); - ref_clk_sel =3 ; - } - }else -dev_warn (dev,"Invalid SS clock type %s, using dlmc_ref_clk0 instead\n" , - ss_clock_type); - -if ((ref_clk_sel ==0 || ref_clk_sel ==1 ) && - (clock_rate !=100000000 )) -dev_warn (dev,"Invalid UCTL clock rate of %u, using 100000000 instead\n" , - clock_rate); - - }else { -dev_err (dev,"No USB UCTL device node\n" ); -return -EINVAL ; - } -/* * Step 1: Wait for all voltages to be stable...that surely * happened before starting the kernel. SKIP @@ -404,24 +351,6 @@ static int dwc3_octeon_clocks_start(struct device *dev, void __iomem *base) val &= ~USBDRD_UCTL_CTL_REF_CLK_SEL ; val |=FIELD_PREP (USBDRD_UCTL_CTL_REF_CLK_SEL , ref_clk_sel); - ref_clk_fsel =0x07 ; -switch (clock_rate) { -default : -dev_warn (dev,"Invalid ref_clk %u, using 100000000 instead\n" , - clock_rate); - fallthrough; -case 100000000 : - mpll_mul =0x19 ; -if (ref_clk_sel <2 ) - ref_clk_fsel =0x27 ; -break ; -case 50000000 : - mpll_mul =0x32 ; -break ; -case 125000000 : - mpll_mul =0x28 ; -break ; - } val &= ~USBDRD_UCTL_CTL_REF_CLK_FSEL ; val |=FIELD_PREP (USBDRD_UCTL_CTL_REF_CLK_FSEL , ref_clk_fsel);Quoted 11 lines
drivers/usb/dwc3/dwc3-octeon.c static void __init dwc3_octeon_phy_reset(void __iomem *base) · line 440
Quoted 9 lines
drivers/usb/dwc3/dwc3-octeon.c +3
-1@@ -505,8 +434,72 @@ static void __init dwc3_octeon_phy_reset(void __iomem *base)static int dwc3_octeon_probe (struct platform_device *pdev) {struct device *dev = &pdev->dev; +struct device_node *node = dev->of_node;struct dwc3_data *data; -int err; +int err, ref_clk_sel, ref_clk_fsel, mpll_mul; +uint32_t clock_rate;Use u32?
I do not like homebrew types when we have standard ones. But it seems almost all usb code is using u32, so I can live with that.
Quoted 24 lines
Quoted 24 lines
drivers/usb/dwc3/dwc3-octeon.c +24
-0+const char *hs_clock_type, *ss_clock_type; + +if (!node) { +dev_err (dev,"No USB UCTL device node\n" ); +return -EINVAL ; + } + +if (of_property_read_u32 (node,"refclk-frequency" , &clock_rate)) { +dev_err (dev,"No UCTL \"refclk-frequency\"\n" ); +return -EINVAL ; + } +if (of_property_read_string (node,"refclk-type-ss" , &ss_clock_type)) { +dev_err (dev,"No UCTL \"refclk-type-ss\"\n" ); +return -EINVAL ; + } +if (of_property_read_string (node,"refclk-type-hs" , &hs_clock_type)) { +dev_err (dev,"No UCTL \"refclk-type-hs\"\n" ); +return -EINVAL ; + } + + ref_clk_sel =2 ; +if (strcmp ("dlmc_ref_clk0" , ss_clock_type) ==0 ) { +if (strcmp (hs_clock_type,"dlmc_ref_clk0" ) ==0 ) + ref_clk_sel =0 ;Quoted 8 lines
drivers/usb/dwc3/dwc3-octeon.c static void __init dwc3_octeon_phy_reset(void __iomem *base) · line 470
Quoted 6 lines
drivers/usb/dwc3/dwc3-octeon.c +6
-0+else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) + ref_clk_sel =2 ; +else +dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , + hs_clock_type); + }else if (strcmp (ss_clock_type,"dlmc_ref_clk1" ) ==0 ) {Did you run checkpatch? I still see some minor formatting issues.
Quoted 1 line
Yes I did. What complain do you see from checkpatch?
checkpatch.pl --strict
CHECK: Prefer kernel type 'u32' over 'uint32_t'
#134: FILE: drivers/usb/dwc3/dwc3-octeon.c:440:
+ uint32_t clock_rate;
CHECK: braces {} should be used on all arms of this statement #165: FILE: drivers/usb/dwc3/dwc3-octeon.c:471:
+ if (strcmp(hs_clock_type, "dlmc_ref_clk1") == 0)
[...]
+ else if (strcmp(hs_clock_type, "pll_ref_clk") == 0) {
[...]
+ } else {
[...]
It should be the same missing bracket fix that you did for the second if, but for the first if statement.
Quoted 14 lines
Quoted 14 lines
Quoted 4 lines
drivers/usb/dwc3/dwc3-octeon.c +3
-0+if (strcmp (hs_clock_type,"dlmc_ref_clk1" ) ==0 ) + ref_clk_sel =1 ; +else if (strcmp (hs_clock_type,"pll_ref_clk" ) ==0 ) {drivers/usb/dwc3/dwc3-octeon.c static void __init dwc3_octeon_phy_reset(void __iomem *base) · line 483
Quoted 10 lines
drivers/usb/dwc3/dwc3-octeon.c +10
-0+ ref_clk_sel =3 ; + }else { +dev_warn (dev,"Invalid HS clock type %s, using pll_ref_clk instead\n" , + hs_clock_type); + ref_clk_sel =3 ; + } + }else { +dev_warn (dev,"Invalid SS clock type %s, using dlmc_ref_clk0 instead\n" , + ss_clock_type); + }
BR,
Thinh
PATCH v2 3/3 usb: dwc3: Add SPDX header and copyright
In reply to Ladislav MichlFrom: Ladislav Michl <[email protected]>
As driver is rewritten and David no longer works for Marvell (Cavium), I'm to blame for breakage.
---
CHANGES:
-v2: None
drivers/usb/dwc3/dwc3-octeon.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/usb/dwc3/dwc3-octeon.c +4 -6
@@ -1,11 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 /* -* XHCI HCD glue for Cavium Octeon III SOCs. +* DWC3 glue for Cavium Octeon III SOCs. * * Copyright (C) 2010-2017 Cavium Networks -* -* This file is subject to the terms and conditions of the GNU General Public -* License. See the file "COPYING" in the main directory of this archive -* for more details. +* Copyright (C) 2023 Ladislav Michl <[email protected]> */ #include <linux/bitfield.h>
@@ -548,6 +546,6 @@ static struct platform_driver dwc3_octeon_driver = {module_platform_driver (dwc3_octeon_driver);MODULE_ALIAS ("platform:dwc3-octeon" ); -MODULE_AUTHOR ("David Daney <[email protected]>" ); +MODULE_AUTHOR ("Ladislav Michl <[email protected]>" );MODULE_LICENSE ("GPL" );MODULE_DESCRIPTION ("DesignWare USB3 OCTEON III Glue Layer" );
Signature · 1 line
2.39.2