| Message ID | 1536213014-1325-4-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: renesas_usbhs: add reset_control and multiple clocks management | expand |
Hi Shimoda-san > R-Car Gen3 needs to enable clocks of both host and peripheral. > Since [eo]hci-platform disables the reset(s) when the drivers are > removed, renesas_usbhs driver doesn't work correctly. To fix this > issue, this patch adds multiple clocks management on this > renesas_usbhs driver. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- (snip) > + /* enable clks if exist */ > + if (priv->num_clks && > + clk_bulk_prepare_enable(priv->num_clks, priv->clks)) > + return; (snip) > + /* disable clks if exist */ > + if (priv->num_clks) > + clk_bulk_disable_unprepare(priv->num_clks, priv->clks); I think clk_bulk_xxx() will do nothing if priv->num_clks was 0. priv->num_clks check is not neede, I think. Best regards --- Kuninori Morimoto
Hi Shimoda-san, On Thu, Sep 6, 2018 at 7:52 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > R-Car Gen3 needs to enable clocks of both host and peripheral. > Since [eo]hci-platform disables the reset(s) when the drivers are > removed, renesas_usbhs driver doesn't work correctly. To fix this > issue, this patch adds multiple clocks management on this > renesas_usbhs driver. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- a/drivers/usb/renesas_usbhs/common.c > +++ b/drivers/usb/renesas_usbhs/common.c > @@ -667,6 +684,12 @@ static int usbhs_probe(struct platform_device *pdev) > if (ret) > goto probe_fail_rst; > > + if (priv->num_clks) { > + ret = clk_bulk_get(&pdev->dev, priv->num_clks, priv->clks); (inspired by Morimoto-san) clk_bulk_get() is a no-op if num_clks is zero. > + if (ret == -EPROBE_DEFER) Why this special handling for -EPROBE_DEFER only? Shouldn't all errors be considered fatal? Perhaps this is needed for backwards compatibility with old DT? In that case, you should do more thorough checking (first clock should exist, second should return -ENOENT). > + goto probe_fail_clks; > + } > + > /* > * deviece reset here because > * USB device might be used in boot loader. Gr{oetje,eeting}s, Geert
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Thursday, September 6, 2018 4:28 PM > > Hi Shimoda-san, > > On Thu, Sep 6, 2018 at 7:52 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > R-Car Gen3 needs to enable clocks of both host and peripheral. > > Since [eo]hci-platform disables the reset(s) when the drivers are > > removed, renesas_usbhs driver doesn't work correctly. To fix this > > issue, this patch adds multiple clocks management on this > > renesas_usbhs driver. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > --- a/drivers/usb/renesas_usbhs/common.c > > +++ b/drivers/usb/renesas_usbhs/common.c > > > @@ -667,6 +684,12 @@ static int usbhs_probe(struct platform_device *pdev) > > if (ret) > > goto probe_fail_rst; > > > > + if (priv->num_clks) { > > + ret = clk_bulk_get(&pdev->dev, priv->num_clks, priv->clks); > > (inspired by Morimoto-san) clk_bulk_get() is a no-op if num_clks is zero. That's right. Thank you for your comment. > > + if (ret == -EPROBE_DEFER) > > Why this special handling for -EPROBE_DEFER only? > Shouldn't all errors be considered fatal? > > Perhaps this is needed for backwards compatibility with old DT? Yes, this is needed for backward s compatibility with old DT. > In that case, you should do more thorough checking (first clock should > exist, second should return -ENOENT). I understood it. (For backwards compatibility with old DT, if second clock returns -ENOENT, the driver should do a special handling.) I'll fix this patch. Best regards, Yoshihiro Shimoda > > + goto probe_fail_clks; > > + } > > + > > /* > > * deviece reset here because > > * USB device might be used in boot loader. > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c index 1d355d5..d1e37cc 100644 --- a/drivers/usb/renesas_usbhs/common.c +++ b/drivers/usb/renesas_usbhs/common.c @@ -5,6 +5,7 @@ * Copyright (C) 2011 Renesas Solutions Corp. * Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> */ +#include <linux/clk.h> #include <linux/err.h> #include <linux/gpio.h> #include <linux/io.h> @@ -341,6 +342,11 @@ static void usbhsc_power_ctrl(struct usbhs_priv *priv, int enable) /* enable PM */ pm_runtime_get_sync(dev); + /* enable clks if exist */ + if (priv->num_clks && + clk_bulk_prepare_enable(priv->num_clks, priv->clks)) + return; + /* enable platform power */ usbhs_platform_call(priv, power_ctrl, pdev, priv->base, enable); @@ -353,6 +359,10 @@ static void usbhsc_power_ctrl(struct usbhs_priv *priv, int enable) /* disable platform power */ usbhs_platform_call(priv, power_ctrl, pdev, priv->base, enable); + /* disable clks if exist */ + if (priv->num_clks) + clk_bulk_disable_unprepare(priv->num_clks, priv->clks); + /* disable PM */ pm_runtime_put_sync(dev); } @@ -620,6 +630,13 @@ static int usbhs_probe(struct platform_device *pdev) break; } + if (priv->dparam.type == USBHS_TYPE_RCAR_GEN3 || + priv->dparam.type == USBHS_TYPE_RCAR_GEN3_WITH_PLL) { + priv->clks[0].id = "hsusb"; + priv->clks[1].id = "ehci/ohci"; + priv->num_clks = ARRAY_SIZE(priv->clks); + }; + /* set driver callback functions for platform */ dfunc = &info->driver_callback; dfunc->notify_hotplug = usbhsc_drvcllbck_notify_hotplug; @@ -667,6 +684,12 @@ static int usbhs_probe(struct platform_device *pdev) if (ret) goto probe_fail_rst; + if (priv->num_clks) { + ret = clk_bulk_get(&pdev->dev, priv->num_clks, priv->clks); + if (ret == -EPROBE_DEFER) + goto probe_fail_clks; + } + /* * deviece reset here because * USB device might be used in boot loader. @@ -720,6 +743,8 @@ static int usbhs_probe(struct platform_device *pdev) return ret; probe_end_mod_exit: + clk_bulk_put(priv->num_clks, priv->clks); +probe_fail_clks: reset_control_assert(priv->rsts); probe_fail_rst: usbhs_mod_remove(priv); diff --git a/drivers/usb/renesas_usbhs/common.h b/drivers/usb/renesas_usbhs/common.h index bce7d35..6e7c5f2 100644 --- a/drivers/usb/renesas_usbhs/common.h +++ b/drivers/usb/renesas_usbhs/common.h @@ -8,6 +8,7 @@ #ifndef RENESAS_USB_DRIVER_H #define RENESAS_USB_DRIVER_H +#include <linux/clk.h> #include <linux/extcon.h> #include <linux/platform_device.h> #include <linux/reset.h> @@ -279,6 +280,8 @@ struct usbhs_priv { struct phy *phy; struct reset_control *rsts; + struct clk_bulk_data clks[2]; + int num_clks; }; /*
R-Car Gen3 needs to enable clocks of both host and peripheral. Since [eo]hci-platform disables the reset(s) when the drivers are removed, renesas_usbhs driver doesn't work correctly. To fix this issue, this patch adds multiple clocks management on this renesas_usbhs driver. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/usb/renesas_usbhs/common.c | 25 +++++++++++++++++++++++++ drivers/usb/renesas_usbhs/common.h | 3 +++ 2 files changed, 28 insertions(+)