Add driver for EHCI in Sunplus SP7021
Subsystems (3)
v4 cover Add driver for EHCI in Sunplus SP7021
This is a patch series for EHCI driver for Sunplus SP7021 SoC.
Sunplus SP7021 is an ARM Cortex A7 (4 cores) based SoC. It integrates many peripherals (ex: UART, I2C, SPI, SDIO, eMMC, USB, SD Card and etc.) into a single chip. It is designed for industrial control.
Refer to:
https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
https://tibbo.com/store/plus1.html
Vincent Shih (2):
usb: host: ehci-sunplus: Add driver for ehci in Sunplus SP7021
dt-bindings: usb: Add bindings doc for Sunplus EHCI driver
.../bindings/usb/sunplus,sp7021-usb-ehci.yaml | 79 ++++++ MAINTAINERS | 7 + drivers/usb/host/Kconfig | 11 + drivers/usb/host/Makefile | 1 + drivers/usb/host/ehci-sunplus.c | 289 +++++++++++++++++++++ 5 files changed, 387 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/sunplus,sp7021-usb-ehci.yaml create mode 100644 drivers/usb/host/ehci-sunplus.c
Signature · 1 line
2.7.4
PATCH v4 1/2 usb: host: ehci-sunplus: Add driver for ehci in Sunplus SP7021
Add driver for ehci in Sunplus SP7021
---
Changes in v4:
- Implement power_on, power_off and power_suspend functions.
MAINTAINERS | 6 + drivers/usb/host/Kconfig | 11 ++ drivers/usb/host/Makefile | 1 + drivers/usb/host/ehci-sunplus.c | 289 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 307 insertions(+) create mode 100644 drivers/usb/host/ehci-sunplus.c
MAINTAINERS +6 -0
@@ -18914,6 +18914,12 @@ S: Maintained F: Documentation/devicetree/bindings/serial/sunplus,sp7021-uart.yaml F: drivers/tty/serial/sunplus-uart.c +SUNPLUS USB EHCI DRIVER +M: Vincent Shih <[email protected]> +L: [email protected] +S: Maintained +F: drivers/usb/host/ehci-sunplus.c + SUPERH M: Yoshinori Sato <[email protected]> M: Rich Felker <[email protected]>
drivers/usb/host/Kconfig +11 -0
@@ -340,6 +340,17 @@ config USB_OCTEON_EHCI USB 2.0 device support. All CN6XXX based chips with USB are supported. +config USB_EHCI_SUNPLUS + tristate "EHCI support for Sunplus SP7021" + depends on SOC_SP7021 + select PHY_SUNPLUS_USB + help + Enables support for the on-chip EHCI controller in Sunplus + SoCs. It supports 32-bit AHB/AXI config bus and 64-bit AXI + data bus. + This driver can also be built as a module. If so, the module + will be called ehci-sunplus. + endif # USB_EHCI_HCD config USB_OXU210HP_HCD
drivers/usb/host/Makefile +1 -0
@@ -46,6 +46,7 @@ obj-$(CONFIG_USB_EHCI_HCD_STI) += ehci-st.o obj-$(CONFIG_USB_EHCI_EXYNOS) += ehci-exynos.o obj-$(CONFIG_USB_EHCI_HCD_AT91) += ehci-atmel.o obj-$(CONFIG_USB_EHCI_BRCMSTB) += ehci-brcm.o +obj-$(CONFIG_USB_EHCI_SUNPLUS) += ehci-sunplus.o obj-$(CONFIG_USB_OXU210HP_HCD) += oxu210hp-hcd.o obj-$(CONFIG_USB_ISP116X_HCD) += isp116x-hcd.o
drivers/usb/host/ehci-sunplus.c new file +289 -0
289 lines not on this page · raw message
Signature · 1 line
2.7.4
Re: [PATCH v4 1/2] usb: host: ehci-sunplus: Add driver for ehci in Sunplus SP7021
Hi Vincent,
On Fr, 2022-04-29 at 16:27 +0800, Vincent Shih wrote:
[...]
drivers/usb/host/ehci-sunplus.c line 39
Quoted 9 lines
drivers/usb/host/ehci-sunplus.c new file +9
-0+static int sp_ehci_platform_power_on (struct platform_device *pdev) +{ +struct usb_hcd *hcd =platform_get_drvdata (pdev); +struct sp_ehci_priv *sp_priv =hcd_to_sp_ehci_priv (hcd); +int ret; + + ret =clk_prepare_enable (sp_priv->ehci_clk); +if (ret) + goto err_ehci_clk;
This should be:
return ret;
drivers/usb/host/ehci-sunplus.c line 43
Quoted 4 lines
drivers/usb/host/ehci-sunplus.c new file +4
-0+ + ret =reset_control_deassert (sp_priv->ehci_rstc); +if (ret) + goto err_ehci_reset;
And this should be:
goto err_ehci_clk;
Quoted 32 lines
drivers/usb/host/ehci-sunplus.c new file +32
-0+ + ret =phy_init (sp_priv->phy); +if (ret) + goto err_ehci_reset; + + ret =phy_power_on (sp_priv->phy); +if (ret) + goto err_phy_exit; + +return 0 ; + +err_phy_exit: +phy_exit (sp_priv->phy); +err_ehci_reset: +reset_control_assert (sp_priv->ehci_rstc); +err_ehci_clk: +clk_disable_unprepare (sp_priv->ehci_clk); + +return ret; +} + +static void sp_ehci_platform_power_off (struct platform_device *pdev) +{ +struct usb_hcd *hcd =platform_get_drvdata (pdev); +struct sp_ehci_priv *sp_priv =hcd_to_sp_ehci_priv (hcd); + +phy_power_off (sp_priv->phy); +phy_exit (sp_priv->phy); + +reset_control_assert (sp_priv->ehci_rstc); +clk_disable_unprepare (sp_priv->ehci_clk); +}
drivers/usb/host/ehci-sunplus.c line 85
Quoted 10 lines
drivers/usb/host/ehci-sunplus.c new file +10
-0+ +static struct usb_ehci_pdata usb_ehci_pdata = { + .has_tt =1 , + .has_synopsys_hc_bug =1 , + .big_endian_desc =1 , + .big_endian_mmio =1 , + .power_on = sp_ehci_platform_power_on, + .power_suspend = sp_ehci_platform_power_off, + .power_off = sp_ehci_platform_power_off, +
Superfluous whitespace.
regards
Philipp
Alan Stern
maintainerRe: [PATCH v4 1/2] usb: host: ehci-sunplus: Add driver for ehci in Sunplus SP7021
In reply to Vincent ShihQuoted 6 lines · Vincent Shih
Add driver for ehci in Sunplus SP7021
Signed-off-by Vincent Shih <[email protected]>---
Changes in v4:
- Implementpower_on,power_offandpower_suspendfunctions.
Did you test these? Did you try to rmmod the ehci-sunplus module after adding these functions?
Quoted 6 lines
drivers/usb/host/ehci-sunplus.c new file +0
-0@@ -0,0 +1,289 @@
...
drivers/usb/host/ehci-sunplus.c line 71
Quoted 7 lines
drivers/usb/host/ehci-sunplus.c new file +7
-0+static void sp_ehci_platform_power_off (struct platform_device *pdev) +{ +struct usb_hcd *hcd =platform_get_drvdata (pdev); +struct sp_ehci_priv *sp_priv =hcd_to_sp_ehci_priv (hcd); + +phy_power_off (sp_priv->phy); +phy_exit (sp_priv->phy);
Notice that this dereferences a field contained in the sp_ehci_priv extension to the usb_hcd structure.
...
drivers/usb/host/ehci-sunplus.c line 200
Quoted 7 lines
drivers/usb/host/ehci-sunplus.c new file +7
-0+static int ehci_sunplus_remove (struct platform_device *pdev) +{ +struct usb_hcd *hcd =platform_get_drvdata (pdev); +struct usb_ehci_pdata *pdata = pdev->dev.platform_data; + +usb_remove_hcd (hcd); +usb_put_hcd (hcd);
This call deallocates the usb_hcd structure.
drivers/usb/host/ehci-sunplus.c line 203
Quoted 3 lines
drivers/usb/host/ehci-sunplus.c new file +3
-0+ +if (pdata->power_off) + pdata->power_off (pdev);
But here you dereference a field that it contains. This power-off operation must occur _before_ the usb_put_hcd() call.
Alan Stern
Alan Stern
maintainerRe: [PATCH v4 1/2] usb: host: ehci-sunplus: Add driver for ehci in Sunplus SP7021
In reply to Vincent ShihQuoted 23 lines · Vincent Shih
Add driver for ehci in Sunplus SP7021
Signed-off-by Vincent Shih <[email protected]>---
drivers/usb/host/ehci-sunplus.c new file +12
-0+static struct usb_ehci_pdata usb_ehci_pdata = { + .has_tt =1 , + .has_synopsys_hc_bug =1 , + .big_endian_desc =1 , + .big_endian_mmio =1 , + .power_on = sp_ehci_platform_power_on, + .power_suspend = sp_ehci_platform_power_off, + .power_off = sp_ehci_platform_power_off, + +}; + +static int ehci_sunplus_reset (struct usb_hcd *hcd)
drivers/usb/host/ehci-sunplus.c line 98
Quoted 10 lines
drivers/usb/host/ehci-sunplus.c new file +10
-0+{ +struct platform_device *pdev =to_platform_device (hcd->self.controller); +struct usb_ehci_pdata *pdata = pdev->dev.platform_data; +struct ehci_hcd *ehci =hcd_to_ehci (hcd); +int retval; + + hcd->has_tt = pdata->has_tt; + ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug; + ehci->big_endian_desc = pdata->big_endian_desc; + ehci->big_endian_mmio = pdata->big_endian_mmio;
By the way, you don't need to add all this pdata stuff. You can just set hcd->has_tt, ehci->has_synopsys_hc_bug, ehci_big_endian_desc, and ehci->big_endian_mmio directly, since you already know what their values should be.
It looks like you simply copied the code from the ehci-platform driver. But that driver has to handle many different kinds of platform devices, so it needs to be told about their individual differences, whereas your driver only has to handle one kind.
In fact, there's no obvious reason why you didn't just use the ehci-platform driver instead of writing your own driver. That's the sort of thing you need to explain the patch description.
Alan Stern
PATCH v4 2/2 dt-bindings: usb: Add bindings doc for Sunplus EHCI driver
In reply to Vincent ShihAdd bindings doc for Sunplus EHCI driver
---
Changes in v2:
- Address the comments from Mr. Krzysztof Kozlowski and Mr. Rob Herring
.../bindings/usb/sunplus,sp7021-usb-ehci.yaml | 79 ++++++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 80 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/sunplus,sp7021-usb-ehci.yaml
Documentation/devicetree/bindings/usb/sunplus,sp7021-usb-ehci.yaml new file +79 -0
@@ -0,0 +1,79 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +# Copyright (C) Sunplus Co., Ltd. 2021 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/usb/sunplus,sp7021-usb-ehci.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sunplus SP7021 EHCI Controller Device Tree Bindings + +allOf: + - $ref: usb-hcd.yaml# + +maintainers: + - Vincent Shih <[email protected]> + +description: + Sunplus SP7021 USB HOST IP is a USB2.0 Host Controller. It supports both + Enhanced Host Controller Interface (EHCI) and Open Host Controller Interface + (OHCI). + + It supports 32-bits address bus and 64bit data bus interface, compliant + to AMBA AXI interface for data transfer. + + It supports 32-bits address and data bus interface, compliant to AMBA + AHB interface for register configurations. + + It supports 32-bits address and data bus interface, compliant to AMBA + AXI interface for register alternative configurations. + + The UTM Interface block generates PHY control signals, compliant to + USB2.0 Transceiver Macrocell Interface Specification Revision 1.0. + +properties: + compatible: + const: sunplus,sp7021-usb-ehci + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + + interrupts: + maxItems: 1 + + phys: + maxItems: 1 + + phy-names: + const: uphy + +required: + - compatible + - reg + - clocks + - resets + - interrupts + - phys + - phy-names + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + sp_ehci0: usb@9c102100 { + compatible = "sunplus,sp7021-usb-ehci"; + reg = <0x9c102100 0x68>; + clocks = <&clkc 0x3a>; + resets = <&rstc 0x2a>; + interrupt-parent = <&intc>; + interrupts = <14 IRQ_TYPE_LEVEL_HIGH>; + phys = <&sp_uphy0>; + phy-names = "uphy"; + };
MAINTAINERS +1 -0
@@ -18918,6 +18918,7 @@ SUNPLUS USB EHCI DRIVER M: Vincent Shih <[email protected]> L: [email protected] S: Maintained +F: Documentation/devicetree/bindings/usb/sunplus,sp7021-usb-ehci.yaml F: drivers/usb/host/ehci-sunplus.c SUPERH
Signature · 1 line
2.7.4
Re: [PATCH v4 2/2] dt-bindings: usb: Add bindings doc for Sunplus EHCI driver
Quoted 6 lines · Vincent Shih
Add bindings doc for Sunplus EHCI driver
Signed-off-by Vincent Shih <[email protected]>---
Changes in v2:
- Address the comments from Mr. Krzysztof Kozlowski and Mr. Rob Herring
Improved, but still I don't think you applied all of my comments. Please go through them one more time.
https://lore.kernel.org/all/[email protected]/
Best regards,
Krzysztof