| Message ID | 1554123232-58942-5-git-send-email-biju.das@bp.renesas.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | None | expand |
On Mon, Apr 01, 2019 at 01:53:44PM +0100, Biju Das wrote: > This patch adds the glue code required to ensure the on-chip EHCI > controller works on RZ/G1C(a.k.a R8A77470) SoC's from Renesas. > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > --- > V1-->V2 > * New patch > --- > drivers/usb/host/Kconfig | 9 ++ > drivers/usb/host/Makefile | 1 + > drivers/usb/host/ehci-r8a77470.c | 327 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 337 insertions(+) > create mode 100644 drivers/usb/host/ehci-r8a77470.c > > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig > index d809671..a474aa9 100644 > --- a/drivers/usb/host/Kconfig > +++ b/drivers/usb/host/Kconfig > @@ -237,6 +237,15 @@ config USB_EHCI_HCD_STI > Enable support for the on-chip EHCI controller found on > STMicroelectronics consumer electronics SoC's. > > +config USB_EHCI_HCD_R8A77470 > + tristate "Support for Renesas R8A77470 on-chip EHCI USB controller" > + depends on ARCH_R8A77470 && OF COMPILE_TEST so that others can also test-build this to ensure that we do not break anything over time? thanks, greg k-h
On Mon, 1 Apr 2019, Biju Das wrote: > This patch adds the glue code required to ensure the on-chip EHCI > controller works on RZ/G1C(a.k.a R8A77470) SoC's from Renesas. You should explain what this new glue code does. And why is it needed? Can't you use the existing ehci-platform driver instead? Alan Stern
Hi Alan, Thanks for the feedback. > Subject: Re: [PATCH 04/12] usb: host: ehci-r8a77470: Add EHCI support for > Renesas RZ/G1C devices > > On Mon, 1 Apr 2019, Biju Das wrote: > > > This patch adds the glue code required to ensure the on-chip EHCI > > controller works on RZ/G1C(a.k.a R8A77470) SoC's from Renesas. > > You should explain what this new glue code does. And why is it needed? Glue code initializes following registers on the host 1) initializing interrupt enable 2) OVC detection timer 3) suspend/ resume timer register Without glue code, interrupt generation is disabled. > Can't you use the existing ehci-platform driver instead? > This driver is almost similar to generic ehci-platform except that it needs to do some platform initialization as mentioned above. If I am correct, we cannot do any platform initialization in generic code. The reason for the new driver is because of the below discussion https://patchwork.kernel.org/patch/10655853/ regards, Biju
On Mon, 1 Apr 2019, Biju Das wrote: > Hi Alan, > > Thanks for the feedback. > > > Subject: Re: [PATCH 04/12] usb: host: ehci-r8a77470: Add EHCI support for > > Renesas RZ/G1C devices > > > > On Mon, 1 Apr 2019, Biju Das wrote: > > > > > This patch adds the glue code required to ensure the on-chip EHCI > > > controller works on RZ/G1C(a.k.a R8A77470) SoC's from Renesas. > > > > You should explain what this new glue code does. And why is it needed? > > Glue code initializes following registers on the host > > 1) initializing interrupt enable > 2) OVC detection timer > 3) suspend/ resume timer register > > Without glue code, interrupt generation is disabled. Good. This information should be added to the patch description. > > Can't you use the existing ehci-platform driver instead? > > > This driver is almost similar to generic ehci-platform except that it needs to do > some platform initialization as mentioned above. If I am correct, we cannot do > any platform initialization in generic code. Are you sure you can't? Also, have you considered modifying ehci-platform.c, in a general-purpose way, such that you could use it and still get this platform-specific work done? For example, you could add some callbacks to the usb_ehci_pdata structure. Or maybe you can use the callbacks that are already present, with no modifications needed. > The reason for the new driver is because of the below discussion > https://patchwork.kernel.org/patch/10655853/ I didn't notice anything in that discussion which would prevent you from using ehci-platform.c (possibly with a few modifications). Alan Stern
Hi Greg, Thanks for the feedback. > Subject: Re: [PATCH 04/12] usb: host: ehci-r8a77470: Add EHCI support for > Renesas RZ/G1C devices > > On Mon, Apr 01, 2019 at 01:53:44PM +0100, Biju Das wrote: > > This patch adds the glue code required to ensure the on-chip EHCI > > controller works on RZ/G1C(a.k.a R8A77470) SoC's from Renesas. > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > --- > > V1-->V2 > > * New patch > > --- > > drivers/usb/host/Kconfig | 9 ++ > > drivers/usb/host/Makefile | 1 + > > drivers/usb/host/ehci-r8a77470.c | 327 > > +++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 337 insertions(+) > > create mode 100644 drivers/usb/host/ehci-r8a77470.c > > > > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index > > d809671..a474aa9 100644 > > --- a/drivers/usb/host/Kconfig > > +++ b/drivers/usb/host/Kconfig > > @@ -237,6 +237,15 @@ config USB_EHCI_HCD_STI > > Enable support for the on-chip EHCI controller found on > > STMicroelectronics consumer electronics SoC's. > > > > +config USB_EHCI_HCD_R8A77470 > > + tristate "Support for Renesas R8A77470 on-chip EHCI USB controller" > > + depends on ARCH_R8A77470 && OF > > COMPILE_TEST so that others can also test-build this to ensure that we do > not break anything over time? OK. Will send V3 with below changes depends on (ARCH_R8A77470 && OF) || COMPILE_TEST regards, Biju
Hi Biju-san, Thank you for the patch! > From: Biju Das, Sent: Monday, April 1, 2019 9:54 PM <snip> > +/******* USB2.0 Host registers (original offset is +0x100) *******/ > +#define USB2_INT_ENABLE 0x100 > +#define USB2_SPD_RSM_TIMSET 0x20c > +#define USB2_OC_TIMSET 0x210 > + > +#define USB2_INT_ENABLE_USBH_INTB_EN BIT(2) > +#define USB2_INT_ENABLE_USBH_INTA_EN BIT(1) > +#define USB2_INT_ENABLE_INIT (USB2_INT_ENABLE_USBH_INTB_EN | \ > + USB2_INT_ENABLE_USBH_INTA_EN) > + > +#define USB2_SPD_RSM_TIMSET_INIT 0x014e029b > +#define USB2_OC_TIMSET_INIT 0x000209ab I should have notice this codes at the previous patch though, I'm thinking we can reuse the R-Car Gen3 codes because these values are similar with the phy-rcar-gen3-usb2.c like below: ---------------------------------------------------------------------- /* INT_ENABLE */ #define USB2_INT_ENABLE_UCOM_INTEN BIT(3) #define USB2_INT_ENABLE_USBH_INTB_EN BIT(2) #define USB2_INT_ENABLE_USBH_INTA_EN BIT(1) #define USB2_INT_ENABLE_INIT (USB2_INT_ENABLE_UCOM_INTEN | \ USB2_INT_ENABLE_USBH_INTB_EN | \ USB2_INT_ENABLE_USBH_INTA_EN) <snip> /* SPD_RSM_TIMSET */ #define USB2_SPD_RSM_TIMSET_INIT 0x014e029b /* OC_TIMSET */ #define USB2_OC_TIMSET_INIT 0x000209ab ---------------------------------------------------------------------- If so, we can use ehci-platform.c and ohci-platform.c as-is. In this case, the "phy: renesas: phy-rcar-gen2: Add support for r8a77470" driver should be initialized at first to release PLLRESET. And then, the phy-rcar-gen3-usb2 driver would be initialized as second step. I think we can set these resisters at phy_init timing because the following patch did so: https://patchwork.kernel.org/patch/10655855/ What do you think? Best regards, Yoshihiro Shimoda
Hi Shimoda-San, Thanks for the feedback. > Subject: RE: [PATCH 04/12] usb: host: ehci-r8a77470: Add EHCI support for > Renesas RZ/G1C devices > > Hi Biju-san, > > Thank you for the patch! > > > From: Biju Das, Sent: Monday, April 1, 2019 9:54 PM > <snip> > > +/******* USB2.0 Host registers (original offset is +0x100) *******/ > > +#define USB2_INT_ENABLE 0x100 > > +#define USB2_SPD_RSM_TIMSET 0x20c > > +#define USB2_OC_TIMSET 0x210 > > + > > +#define USB2_INT_ENABLE_USBH_INTB_EN BIT(2) > > +#define USB2_INT_ENABLE_USBH_INTA_EN BIT(1) > > +#define USB2_INT_ENABLE_INIT > (USB2_INT_ENABLE_USBH_INTB_EN | \ > > + USB2_INT_ENABLE_USBH_INTA_EN) > > + > > +#define USB2_SPD_RSM_TIMSET_INIT 0x014e029b > > +#define USB2_OC_TIMSET_INIT 0x000209ab > > I should have notice this codes at the previous patch though, I'm thinking we > can reuse the R-Car Gen3 codes because these values are similar with the > phy-rcar-gen3-usb2.c like below: > ---------------------------------------------------------------------- > /* INT_ENABLE */ > #define USB2_INT_ENABLE_UCOM_INTEN BIT(3) > #define USB2_INT_ENABLE_USBH_INTB_EN BIT(2) > #define USB2_INT_ENABLE_USBH_INTA_EN BIT(1) > #define USB2_INT_ENABLE_INIT > (USB2_INT_ENABLE_UCOM_INTEN | \ > USB2_INT_ENABLE_USBH_INTB_EN > | \ > USB2_INT_ENABLE_USBH_INTA_EN) > <snip> > /* SPD_RSM_TIMSET */ > #define USB2_SPD_RSM_TIMSET_INIT 0x014e029b > > /* OC_TIMSET */ > #define USB2_OC_TIMSET_INIT 0x000209ab > ---------------------------------------------------------------------- > > If so, we can use ehci-platform.c and ohci-platform.c as-is. In this case, the > "phy: renesas: phy-rcar-gen2: Add support for r8a77470" driver should be > initialized at first to release PLLRESET. And then, the phy-rcar-gen3-usb2 > driver would be initialized as second step. I think we can set these resisters at > phy_init timing because the following patch did so: > > https://patchwork.kernel.org/patch/10655855/ > > What do you think? Good catch. This solution will avoid overlapping regions as mentioned by Rob in the below patch and also will avoid modifying ehci-platform.c as suggested by Alen. https://patchwork.kernel.org/patch/10655853/ I will send V3 based on this solution. Please let me know, if you think otherwise. Regards, Biju
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index d809671..a474aa9 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -237,6 +237,15 @@ config USB_EHCI_HCD_STI Enable support for the on-chip EHCI controller found on STMicroelectronics consumer electronics SoC's. +config USB_EHCI_HCD_R8A77470 + tristate "Support for Renesas R8A77470 on-chip EHCI USB controller" + depends on ARCH_R8A77470 && OF + select GENERIC_PHY + select USB_EHCI_HCD_PLATFORM + help + Enable support for the on-chip EHCI controller found on + Renesas RZ/G1C(R8A77470) SoC's. + config USB_EHCI_HCD_AT91 tristate "Support for Atmel on-chip EHCI USB controller" depends on USB_EHCI_HCD && ARCH_AT91 diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile index 84514f7..785a1d1 100644 --- a/drivers/usb/host/Makefile +++ b/drivers/usb/host/Makefile @@ -48,6 +48,7 @@ obj-$(CONFIG_USB_EHCI_HCD_OMAP) += ehci-omap.o obj-$(CONFIG_USB_EHCI_HCD_ORION) += ehci-orion.o obj-$(CONFIG_USB_EHCI_HCD_SPEAR) += ehci-spear.o obj-$(CONFIG_USB_EHCI_HCD_STI) += ehci-st.o +obj-$(CONFIG_USB_EHCI_HCD_R8A77470) += ehci-r8a77470.o obj-$(CONFIG_USB_EHCI_EXYNOS) += ehci-exynos.o obj-$(CONFIG_USB_EHCI_HCD_AT91) += ehci-atmel.o obj-$(CONFIG_USB_EHCI_TEGRA) += ehci-tegra.o diff --git a/drivers/usb/host/ehci-r8a77470.c b/drivers/usb/host/ehci-r8a77470.c new file mode 100644 index 0000000..d57f6f1 --- /dev/null +++ b/drivers/usb/host/ehci-r8a77470.c @@ -0,0 +1,327 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Renesas R8A77470 ehci driver + * + * Copyright (C) 2019 Renesas Electronics Corp. + * + * Derived from ehci-platform.c + */ +#include <linux/clk.h> +#include <linux/dma-mapping.h> +#include <linux/err.h> +#include <linux/kernel.h> +#include <linux/hrtimer.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/reset.h> +#include <linux/usb.h> +#include <linux/usb/hcd.h> +#include <linux/usb/ehci_pdriver.h> +#include <linux/usb/of.h> + +#include "ehci.h" + +/******* USB2.0 Host registers (original offset is +0x100) *******/ +#define USB2_INT_ENABLE 0x100 +#define USB2_SPD_RSM_TIMSET 0x20c +#define USB2_OC_TIMSET 0x210 + +#define USB2_INT_ENABLE_USBH_INTB_EN BIT(2) +#define USB2_INT_ENABLE_USBH_INTA_EN BIT(1) +#define USB2_INT_ENABLE_INIT (USB2_INT_ENABLE_USBH_INTB_EN | \ + USB2_INT_ENABLE_USBH_INTA_EN) + +#define USB2_SPD_RSM_TIMSET_INIT 0x014e029b +#define USB2_OC_TIMSET_INIT 0x000209ab + +#define DRIVER_DESC "EHCI renesas r8a77470 driver" +#define EHCI_MAX_CLKS 4 +#define hcd_to_ehci_priv(h) ((struct ehci_platform_priv *)hcd_to_ehci(h)->priv) + +struct ehci_platform_priv { + struct clk *clks[EHCI_MAX_CLKS]; + struct reset_control *rsts; + bool reset_on_resume; +}; + +static const char hcd_name[] = "ehci-r8a77470"; + +static int r8a77470_ehci_platform_reset(struct usb_hcd *hcd) +{ + struct platform_device *pdev = to_platform_device(hcd->self.controller); + struct usb_ehci_pdata *pdata = dev_get_platdata(&pdev->dev); + struct ehci_hcd *ehci = hcd_to_ehci(hcd); + + ehci->caps = hcd->regs + pdata->caps_offset; + return ehci_setup(hcd); +} + +static int r8a77470_ehci_platform_power_on(struct platform_device *dev) +{ + struct usb_hcd *hcd = platform_get_drvdata(dev); + struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd); + int clk, ret; + + for (clk = 0; clk < EHCI_MAX_CLKS && priv->clks[clk]; clk++) { + ret = clk_prepare_enable(priv->clks[clk]); + if (ret) + goto err_disable_clks; + } + + writel(USB2_INT_ENABLE_INIT, hcd->regs + USB2_INT_ENABLE); + writel(USB2_SPD_RSM_TIMSET_INIT, hcd->regs + USB2_SPD_RSM_TIMSET); + writel(USB2_OC_TIMSET_INIT, hcd->regs + USB2_OC_TIMSET); + return 0; + +err_disable_clks: + while (--clk >= 0) + clk_disable_unprepare(priv->clks[clk]); + + return ret; +} + +static void r8a77470_ehci_platform_power_off(struct platform_device *dev) +{ + struct usb_hcd *hcd = platform_get_drvdata(dev); + struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd); + int clk; + + writel(0, hcd->regs + USB2_INT_ENABLE); + for (clk = EHCI_MAX_CLKS - 1; clk >= 0; clk--) + if (priv->clks[clk]) + clk_disable_unprepare(priv->clks[clk]); +} + +static struct hc_driver __read_mostly ehci_platform_hc_driver; + +static const struct ehci_driver_overrides platform_overrides __initconst = { + .reset = r8a77470_ehci_platform_reset, + .extra_priv_size = sizeof(struct ehci_platform_priv), +}; + +static struct usb_ehci_pdata ehci_platform_defaults = { + .power_on = r8a77470_ehci_platform_power_on, + .power_suspend = r8a77470_ehci_platform_power_off, + .power_off = r8a77470_ehci_platform_power_off, +}; + +static int r8a77470_ehci_platform_probe(struct platform_device *dev) +{ + struct usb_hcd *hcd; + struct resource *res_mem; + struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev); + struct ehci_platform_priv *priv; + struct ehci_hcd *ehci; + int err, irq, clk = 0; + + if (usb_disabled()) + return -ENODEV; + + /* + * Use reasonable defaults so platforms don't have to provide these + * with DT probing on ARM. + */ + if (!pdata) + pdata = &ehci_platform_defaults; + + err = dma_coerce_mask_and_coherent(&dev->dev, + pdata->dma_mask_64 ? DMA_BIT_MASK(64) : DMA_BIT_MASK(32)); + if (err) { + dev_err(&dev->dev, "Error: DMA mask configuration failed\n"); + return err; + } + + irq = platform_get_irq(dev, 0); + if (irq < 0) { + dev_err(&dev->dev, "no irq provided"); + return irq; + } + + hcd = usb_create_hcd(&ehci_platform_hc_driver, &dev->dev, + dev_name(&dev->dev)); + if (!hcd) + return -ENOMEM; + + platform_set_drvdata(dev, hcd); + dev->dev.platform_data = pdata; + priv = hcd_to_ehci_priv(hcd); + ehci = hcd_to_ehci(hcd); + + if (pdata == &ehci_platform_defaults && dev->dev.of_node) { + for (clk = 0; clk < EHCI_MAX_CLKS; clk++) { + priv->clks[clk] = of_clk_get(dev->dev.of_node, clk); + if (IS_ERR(priv->clks[clk])) { + err = PTR_ERR(priv->clks[clk]); + if (err == -EPROBE_DEFER) + goto err_put_clks; + priv->clks[clk] = NULL; + break; + } + } + } + + priv->rsts = devm_reset_control_array_get_optional_shared(&dev->dev); + if (IS_ERR(priv->rsts)) { + err = PTR_ERR(priv->rsts); + goto err_put_clks; + } + + err = reset_control_deassert(priv->rsts); + if (err) + goto err_put_clks; + + res_mem = platform_get_resource(dev, IORESOURCE_MEM, 0); + hcd->regs = devm_ioremap_resource(&dev->dev, res_mem); + if (IS_ERR(hcd->regs)) { + err = PTR_ERR(hcd->regs); + goto err_reset; + } + hcd->rsrc_start = res_mem->start; + hcd->rsrc_len = resource_size(res_mem); + + if (pdata->power_on) { + err = pdata->power_on(dev); + if (err < 0) + goto err_reset; + } + + err = usb_add_hcd(hcd, irq, IRQF_SHARED); + if (err) + goto err_power; + + device_wakeup_enable(hcd->self.controller); + device_enable_async_suspend(hcd->self.controller); + platform_set_drvdata(dev, hcd); + + return err; + +err_power: + if (pdata->power_off) + pdata->power_off(dev); +err_reset: + reset_control_assert(priv->rsts); +err_put_clks: + while (--clk >= 0) + clk_put(priv->clks[clk]); + + if (pdata == &ehci_platform_defaults) + dev->dev.platform_data = NULL; + + usb_put_hcd(hcd); + + return err; +} + +static int r8a77470_ehci_platform_remove(struct platform_device *dev) +{ + struct usb_hcd *hcd = platform_get_drvdata(dev); + struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev); + struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd); + int clk; + + usb_remove_hcd(hcd); + + if (pdata->power_off) + pdata->power_off(dev); + + reset_control_assert(priv->rsts); + + for (clk = 0; clk < EHCI_MAX_CLKS && priv->clks[clk]; clk++) + clk_put(priv->clks[clk]); + + usb_put_hcd(hcd); + + if (pdata == &ehci_platform_defaults) + dev->dev.platform_data = NULL; + + return 0; +} + +#ifdef CONFIG_PM_SLEEP +static int ehci_platform_suspend(struct device *dev) +{ + struct usb_hcd *hcd = dev_get_drvdata(dev); + struct usb_ehci_pdata *pdata = dev_get_platdata(dev); + struct platform_device *pdev = to_platform_device(dev); + bool do_wakeup = device_may_wakeup(dev); + int ret; + + ret = ehci_suspend(hcd, do_wakeup); + if (ret) + return ret; + + if (pdata->power_suspend) + pdata->power_suspend(pdev); + + return ret; +} + +static int ehci_platform_resume(struct device *dev) +{ + struct usb_hcd *hcd = dev_get_drvdata(dev); + struct usb_ehci_pdata *pdata = dev_get_platdata(dev); + struct platform_device *pdev = to_platform_device(dev); + struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd); + struct device *companion_dev; + int err; + + if (pdata->power_on) { + err = pdata->power_on(pdev); + if (err < 0) + return err; + } + + companion_dev = usb_of_get_companion_dev(hcd->self.controller); + if (companion_dev) { + device_pm_wait_for_dev(hcd->self.controller, companion_dev); + put_device(companion_dev); + } + + ehci_resume(hcd, priv->reset_on_resume); + return 0; +} +#endif /* CONFIG_PM_SLEEP */ + +static const struct of_device_id r8a77470_ehci_ids[] = { + { .compatible = "renesas,ehci-r8a77470", }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, r8a77470_ehci_ids); + +static SIMPLE_DEV_PM_OPS(ehci_platform_pm_ops, ehci_platform_suspend, + ehci_platform_resume); + +static struct platform_driver ehci_platform_driver = { + .probe = r8a77470_ehci_platform_probe, + .remove = r8a77470_ehci_platform_remove, + .shutdown = usb_hcd_platform_shutdown, + .driver = { + .name = "r8a77470-ehci", + .pm = &ehci_platform_pm_ops, + .of_match_table = r8a77470_ehci_ids, + } +}; + +static int __init ehci_platform_init(void) +{ + if (usb_disabled()) + return -ENODEV; + + pr_info("%s: " DRIVER_DESC "\n", hcd_name); + + ehci_init_driver(&ehci_platform_hc_driver, &platform_overrides); + return platform_driver_register(&ehci_platform_driver); +} +module_init(ehci_platform_init); + +static void __exit ehci_platform_cleanup(void) +{ + platform_driver_unregister(&ehci_platform_driver); +} +module_exit(ehci_platform_cleanup); + +MODULE_DESCRIPTION(DRIVER_DESC); +MODULE_AUTHOR("Biju Das <biju.das@bp.renesas.com>"); +MODULE_LICENSE("GPL");
This patch adds the glue code required to ensure the on-chip EHCI controller works on RZ/G1C(a.k.a R8A77470) SoC's from Renesas. Signed-off-by: Biju Das <biju.das@bp.renesas.com> --- V1-->V2 * New patch --- drivers/usb/host/Kconfig | 9 ++ drivers/usb/host/Makefile | 1 + drivers/usb/host/ehci-r8a77470.c | 327 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 337 insertions(+) create mode 100644 drivers/usb/host/ehci-r8a77470.c