* [PATCH 0/3] usb: renesas_usbhs: add reset_control and multiple clocks management
@ 2018-08-31 6:48 Yoshihiro Shimoda
2018-08-31 6:48 ` [1/3] " Yoshihiro Shimoda
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Yoshihiro Shimoda @ 2018-08-31 6:48 UTC (permalink / raw)
To: balbi, robh+dt, mark.rutland
Cc: gregkh, linux-usb, linux-renesas-soc, devicetree, Yoshihiro Shimoda
This patch set is based on Felipe's usb.git / testing/next branch
(the commit id is 5b394b2ddf0347bef56e50c69a58773c94343ff3) with
the following patch:
https://patchwork.kernel.org/patch/10574875/
Yoshihiro Shimoda (3):
usb: renesas_usbhs: Add reset_control
dt-bindings: usb: renesas_usbhs: add clock-names property
usb: renesas_usbhs: Add multiple clocks management
.../devicetree/bindings/usb/renesas_usbhs.txt | 2 +
drivers/usb/renesas_usbhs/common.c | 53 ++++++++++++++++++++++
drivers/usb/renesas_usbhs/common.h | 5 ++
3 files changed, 60 insertions(+)
--
1.9.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] usb: renesas_usbhs: Add reset_control
@ 2018-08-31 6:48 ` Yoshihiro Shimoda
0 siblings, 0 replies; 11+ messages in thread
From: Yoshihiro Shimoda @ 2018-08-31 6:48 UTC (permalink / raw)
To: balbi, robh+dt, mark.rutland
Cc: gregkh, linux-usb, linux-renesas-soc, devicetree, Yoshihiro Shimoda
R-Car Gen3 needs to deassert resets of both host and peripheral.
Since [eo]hci-platform is possible to assert the reset(s) when
the probing failed, renesas_usbhs driver doesn't work correctly
regardless of finished probing. To fix this issue, this patch adds
reset_control on this renesas_usbhs driver.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
drivers/usb/renesas_usbhs/common.c | 12 ++++++++++++
drivers/usb/renesas_usbhs/common.h | 2 ++
2 files changed, 14 insertions(+)
diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c
index 4310df4..1d355d5 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -12,6 +12,7 @@
#include <linux/of_device.h>
#include <linux/of_gpio.h>
#include <linux/pm_runtime.h>
+#include <linux/reset.h>
#include <linux/slab.h>
#include <linux/sysfs.h>
#include "common.h"
@@ -574,6 +575,10 @@ static int usbhs_probe(struct platform_device *pdev)
return PTR_ERR(priv->edev);
}
+ priv->rsts = devm_reset_control_array_get_optional_shared(&pdev->dev);
+ if (IS_ERR(priv->rsts))
+ return PTR_ERR(priv->rsts);
+
/*
* care platform info
*/
@@ -658,6 +663,10 @@ static int usbhs_probe(struct platform_device *pdev)
/* dev_set_drvdata should be called after usbhs_mod_init */
platform_set_drvdata(pdev, priv);
+ ret = reset_control_deassert(priv->rsts);
+ if (ret)
+ goto probe_fail_rst;
+
/*
* deviece reset here because
* USB device might be used in boot loader.
@@ -711,6 +720,8 @@ static int usbhs_probe(struct platform_device *pdev)
return ret;
probe_end_mod_exit:
+ reset_control_assert(priv->rsts);
+probe_fail_rst:
usbhs_mod_remove(priv);
probe_end_fifo_exit:
usbhs_fifo_remove(priv);
@@ -739,6 +750,7 @@ static int usbhs_remove(struct platform_device *pdev)
pm_runtime_disable(&pdev->dev);
usbhs_platform_call(priv, hardware_exit, pdev);
+ reset_control_assert(priv->rsts);
usbhs_mod_remove(priv);
usbhs_fifo_remove(priv);
usbhs_pipe_remove(priv);
diff --git a/drivers/usb/renesas_usbhs/common.h b/drivers/usb/renesas_usbhs/common.h
index 6137f79..bce7d35 100644
--- a/drivers/usb/renesas_usbhs/common.h
+++ b/drivers/usb/renesas_usbhs/common.h
@@ -10,6 +10,7 @@
#include <linux/extcon.h>
#include <linux/platform_device.h>
+#include <linux/reset.h>
#include <linux/usb/renesas_usbhs.h>
struct usbhs_priv;
@@ -277,6 +278,7 @@ struct usbhs_priv {
struct usbhs_fifo_info fifo_info;
struct phy *phy;
+ struct reset_control *rsts;
};
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [1/3] usb: renesas_usbhs: Add reset_control
@ 2018-08-31 6:48 ` Yoshihiro Shimoda
0 siblings, 0 replies; 11+ messages in thread
From: Yoshihiro Shimoda @ 2018-08-31 6:48 UTC (permalink / raw)
To: balbi, robh+dt, mark.rutland
Cc: gregkh, linux-usb, linux-renesas-soc, devicetree, Yoshihiro Shimoda
R-Car Gen3 needs to deassert resets of both host and peripheral.
Since [eo]hci-platform is possible to assert the reset(s) when
the probing failed, renesas_usbhs driver doesn't work correctly
regardless of finished probing. To fix this issue, this patch adds
reset_control on this renesas_usbhs driver.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
drivers/usb/renesas_usbhs/common.c | 12 ++++++++++++
drivers/usb/renesas_usbhs/common.h | 2 ++
2 files changed, 14 insertions(+)
diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c
index 4310df4..1d355d5 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -12,6 +12,7 @@
#include <linux/of_device.h>
#include <linux/of_gpio.h>
#include <linux/pm_runtime.h>
+#include <linux/reset.h>
#include <linux/slab.h>
#include <linux/sysfs.h>
#include "common.h"
@@ -574,6 +575,10 @@ static int usbhs_probe(struct platform_device *pdev)
return PTR_ERR(priv->edev);
}
+ priv->rsts = devm_reset_control_array_get_optional_shared(&pdev->dev);
+ if (IS_ERR(priv->rsts))
+ return PTR_ERR(priv->rsts);
+
/*
* care platform info
*/
@@ -658,6 +663,10 @@ static int usbhs_probe(struct platform_device *pdev)
/* dev_set_drvdata should be called after usbhs_mod_init */
platform_set_drvdata(pdev, priv);
+ ret = reset_control_deassert(priv->rsts);
+ if (ret)
+ goto probe_fail_rst;
+
/*
* deviece reset here because
* USB device might be used in boot loader.
@@ -711,6 +720,8 @@ static int usbhs_probe(struct platform_device *pdev)
return ret;
probe_end_mod_exit:
+ reset_control_assert(priv->rsts);
+probe_fail_rst:
usbhs_mod_remove(priv);
probe_end_fifo_exit:
usbhs_fifo_remove(priv);
@@ -739,6 +750,7 @@ static int usbhs_remove(struct platform_device *pdev)
pm_runtime_disable(&pdev->dev);
usbhs_platform_call(priv, hardware_exit, pdev);
+ reset_control_assert(priv->rsts);
usbhs_mod_remove(priv);
usbhs_fifo_remove(priv);
usbhs_pipe_remove(priv);
diff --git a/drivers/usb/renesas_usbhs/common.h b/drivers/usb/renesas_usbhs/common.h
index 6137f79..bce7d35 100644
--- a/drivers/usb/renesas_usbhs/common.h
+++ b/drivers/usb/renesas_usbhs/common.h
@@ -10,6 +10,7 @@
#include <linux/extcon.h>
#include <linux/platform_device.h>
+#include <linux/reset.h>
#include <linux/usb/renesas_usbhs.h>
struct usbhs_priv;
@@ -277,6 +278,7 @@ struct usbhs_priv {
struct usbhs_fifo_info fifo_info;
struct phy *phy;
+ struct reset_control *rsts;
};
/*
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] dt-bindings: usb: renesas_usbhs: add clock-names property
@ 2018-08-31 6:48 ` Yoshihiro Shimoda
0 siblings, 0 replies; 11+ messages in thread
From: Yoshihiro Shimoda @ 2018-08-31 6:48 UTC (permalink / raw)
To: balbi, robh+dt, mark.rutland
Cc: gregkh, linux-usb, linux-renesas-soc, devicetree, Yoshihiro Shimoda
R-Car Gen3 needs to enable clocks of both host and peripheral.
Otherwise, other side device cannot work correctly. So, this patch
adds a property of clock-names for R-Car Gen3 as an optional.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
Documentation/devicetree/bindings/usb/renesas_usbhs.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
index 087140a..a8fa3ec 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
@@ -37,6 +37,8 @@ Optional properties:
- dmas: Must contain a list of references to DMA specifiers.
- dma-names : named "ch%d", where %d is the channel number ranging from zero
to the number of channels (DnFIFOs) minus one.
+ - clock-names: For a "renesas,rcar-gen3-usbhs" compatible device,
+ "hsusb" and "ehci/ohci" can be set.
Example:
usbhs: usb@e6590000 {
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [2/3] dt-bindings: usb: renesas_usbhs: add clock-names property
@ 2018-08-31 6:48 ` Yoshihiro Shimoda
0 siblings, 0 replies; 11+ messages in thread
From: Yoshihiro Shimoda @ 2018-08-31 6:48 UTC (permalink / raw)
To: balbi, robh+dt, mark.rutland
Cc: gregkh, linux-usb, linux-renesas-soc, devicetree, Yoshihiro Shimoda
R-Car Gen3 needs to enable clocks of both host and peripheral.
Otherwise, other side device cannot work correctly. So, this patch
adds a property of clock-names for R-Car Gen3 as an optional.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
Documentation/devicetree/bindings/usb/renesas_usbhs.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
index 087140a..a8fa3ec 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
@@ -37,6 +37,8 @@ Optional properties:
- dmas: Must contain a list of references to DMA specifiers.
- dma-names : named "ch%d", where %d is the channel number ranging from zero
to the number of channels (DnFIFOs) minus one.
+ - clock-names: For a "renesas,rcar-gen3-usbhs" compatible device,
+ "hsusb" and "ehci/ohci" can be set.
Example:
usbhs: usb@e6590000 {
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] usb: renesas_usbhs: Add multiple clocks management
@ 2018-08-31 6:48 ` Yoshihiro Shimoda
0 siblings, 0 replies; 11+ messages in thread
From: Yoshihiro Shimoda @ 2018-08-31 6:48 UTC (permalink / raw)
To: balbi, robh+dt, mark.rutland
Cc: gregkh, linux-usb, linux-renesas-soc, devicetree, Yoshihiro Shimoda
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 | 41 ++++++++++++++++++++++++++++++++++++++
drivers/usb/renesas_usbhs/common.h | 3 +++
2 files changed, 44 insertions(+)
diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c
index 1d355d5..00d32d1 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>
@@ -336,11 +337,23 @@ static void usbhsc_power_ctrl(struct usbhs_priv *priv, int enable)
{
struct platform_device *pdev = usbhs_priv_to_pdev(priv);
struct device *dev = usbhs_priv_to_dev(priv);
+ int ret;
if (enable) {
/* enable PM */
pm_runtime_get_sync(dev);
+ /* enable clks if exist */
+ if (priv->clks) {
+ ret = clk_bulk_prepare(priv->num_clks, priv->clks);
+ if (!ret) {
+ ret = clk_bulk_enable(priv->num_clks,
+ priv->clks);
+ if (ret)
+ return;
+ }
+ }
+
/* enable platform power */
usbhs_platform_call(priv, power_ctrl, pdev, priv->base, enable);
@@ -353,6 +366,12 @@ 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->clks) {
+ clk_bulk_disable(priv->num_clks, priv->clks);
+ clk_bulk_unprepare(priv->num_clks, priv->clks);
+ }
+
/* disable PM */
pm_runtime_put_sync(dev);
}
@@ -534,6 +553,11 @@ static struct renesas_usbhs_platform_info *usbhs_parse_dt(struct device *dev)
return info;
}
+static const struct clk_bulk_data usbhs_rcar3_clks[] = {
+ { .id = "hsusb" },
+ { .id = "ehci/ohci" },
+};
+
static int usbhs_probe(struct platform_device *pdev)
{
struct renesas_usbhs_platform_info *info = renesas_usbhs_get_info(pdev);
@@ -620,6 +644,15 @@ 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 = devm_kmemdup(&pdev->dev, usbhs_rcar3_clks,
+ sizeof(usbhs_rcar3_clks), GFP_KERNEL);
+ if (!priv->clks)
+ return -ENOMEM;
+ priv->num_clks = ARRAY_SIZE(usbhs_rcar3_clks);
+ }
+
/* set driver callback functions for platform */
dfunc = &info->driver_callback;
dfunc->notify_hotplug = usbhsc_drvcllbck_notify_hotplug;
@@ -667,6 +700,12 @@ static int usbhs_probe(struct platform_device *pdev)
if (ret)
goto probe_fail_rst;
+ if (priv->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 +759,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..9105ee0 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;
+ int num_clks;
};
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [3/3] usb: renesas_usbhs: Add multiple clocks management
@ 2018-08-31 6:48 ` Yoshihiro Shimoda
0 siblings, 0 replies; 11+ messages in thread
From: Yoshihiro Shimoda @ 2018-08-31 6:48 UTC (permalink / raw)
To: balbi, robh+dt, mark.rutland
Cc: gregkh, linux-usb, linux-renesas-soc, devicetree, Yoshihiro Shimoda
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 | 41 ++++++++++++++++++++++++++++++++++++++
drivers/usb/renesas_usbhs/common.h | 3 +++
2 files changed, 44 insertions(+)
diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c
index 1d355d5..00d32d1 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>
@@ -336,11 +337,23 @@ static void usbhsc_power_ctrl(struct usbhs_priv *priv, int enable)
{
struct platform_device *pdev = usbhs_priv_to_pdev(priv);
struct device *dev = usbhs_priv_to_dev(priv);
+ int ret;
if (enable) {
/* enable PM */
pm_runtime_get_sync(dev);
+ /* enable clks if exist */
+ if (priv->clks) {
+ ret = clk_bulk_prepare(priv->num_clks, priv->clks);
+ if (!ret) {
+ ret = clk_bulk_enable(priv->num_clks,
+ priv->clks);
+ if (ret)
+ return;
+ }
+ }
+
/* enable platform power */
usbhs_platform_call(priv, power_ctrl, pdev, priv->base, enable);
@@ -353,6 +366,12 @@ 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->clks) {
+ clk_bulk_disable(priv->num_clks, priv->clks);
+ clk_bulk_unprepare(priv->num_clks, priv->clks);
+ }
+
/* disable PM */
pm_runtime_put_sync(dev);
}
@@ -534,6 +553,11 @@ static struct renesas_usbhs_platform_info *usbhs_parse_dt(struct device *dev)
return info;
}
+static const struct clk_bulk_data usbhs_rcar3_clks[] = {
+ { .id = "hsusb" },
+ { .id = "ehci/ohci" },
+};
+
static int usbhs_probe(struct platform_device *pdev)
{
struct renesas_usbhs_platform_info *info = renesas_usbhs_get_info(pdev);
@@ -620,6 +644,15 @@ 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 = devm_kmemdup(&pdev->dev, usbhs_rcar3_clks,
+ sizeof(usbhs_rcar3_clks), GFP_KERNEL);
+ if (!priv->clks)
+ return -ENOMEM;
+ priv->num_clks = ARRAY_SIZE(usbhs_rcar3_clks);
+ }
+
/* set driver callback functions for platform */
dfunc = &info->driver_callback;
dfunc->notify_hotplug = usbhsc_drvcllbck_notify_hotplug;
@@ -667,6 +700,12 @@ static int usbhs_probe(struct platform_device *pdev)
if (ret)
goto probe_fail_rst;
+ if (priv->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 +759,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..9105ee0 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;
+ int num_clks;
};
/*
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] usb: renesas_usbhs: Add multiple clocks management
@ 2018-08-31 7:22 ` Geert Uytterhoeven
0 siblings, 0 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-08-31 7:22 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Felipe Balbi, Rob Herring, Mark Rutland, Greg KH, USB list,
Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Hi Shimoda-san,
On Fri, Aug 31, 2018 at 8:50 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>
Thanks for your patch!
> --- 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>
> @@ -336,11 +337,23 @@ static void usbhsc_power_ctrl(struct usbhs_priv *priv, int enable)
> {
> struct platform_device *pdev = usbhs_priv_to_pdev(priv);
> struct device *dev = usbhs_priv_to_dev(priv);
> + int ret;
>
> if (enable) {
> /* enable PM */
> pm_runtime_get_sync(dev);
>
> + /* enable clks if exist */
> + if (priv->clks) {
> + ret = clk_bulk_prepare(priv->num_clks, priv->clks);
> + if (!ret) {
> + ret = clk_bulk_enable(priv->num_clks,
> + priv->clks);
> + if (ret)
> + return;
clk_bulk_prepare_enable(), which also takes care of unpreparing all
clocks if any clock fails to enabled.
> + }
> + }
> +
> /* enable platform power */
> usbhs_platform_call(priv, power_ctrl, pdev, priv->base, enable);
>
> @@ -353,6 +366,12 @@ 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->clks) {
> + clk_bulk_disable(priv->num_clks, priv->clks);
> + clk_bulk_unprepare(priv->num_clks, priv->clks);
clk_bulk_disable_unprepare()
> + }
> +
> /* disable PM */
> pm_runtime_put_sync(dev);
> }
> @@ -534,6 +553,11 @@ static struct renesas_usbhs_platform_info *usbhs_parse_dt(struct device *dev)
> return info;
> }
>
> +static const struct clk_bulk_data usbhs_rcar3_clks[] = {
> + { .id = "hsusb" },
> + { .id = "ehci/ohci" },
> +};
This structure contains space for the clock pointers, which are not really
needed here (the struct is copied).
Perhaps you can replace "struct clk_bulk_data *clks" in struct usbhs_priv
replace by "struct clk_bulk_data clks[2]", ...
> +
> static int usbhs_probe(struct platform_device *pdev)
> {
> struct renesas_usbhs_platform_info *info = renesas_usbhs_get_info(pdev);
> @@ -620,6 +644,15 @@ 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 = devm_kmemdup(&pdev->dev, usbhs_rcar3_clks,
> + sizeof(usbhs_rcar3_clks), GFP_KERNEL);
> + if (!priv->clks)
> + return -ENOMEM;
... and instead of allocating/copying, fill in the clock names here:
priv->clks[0].id = "hsusb";
priv->clks[1].id = "ehci/ohci";
As priv->clks will no longer be a pointer, all checks for it should be
replaced by checks for priv->num_clks.
What do you think?
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [3/3] usb: renesas_usbhs: Add multiple clocks management
@ 2018-08-31 7:22 ` Geert Uytterhoeven
0 siblings, 0 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-08-31 7:22 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Felipe Balbi, Rob Herring, Mark Rutland, Greg KH, USB list,
Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Hi Shimoda-san,
On Fri, Aug 31, 2018 at 8:50 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>
Thanks for your patch!
> --- 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>
> @@ -336,11 +337,23 @@ static void usbhsc_power_ctrl(struct usbhs_priv *priv, int enable)
> {
> struct platform_device *pdev = usbhs_priv_to_pdev(priv);
> struct device *dev = usbhs_priv_to_dev(priv);
> + int ret;
>
> if (enable) {
> /* enable PM */
> pm_runtime_get_sync(dev);
>
> + /* enable clks if exist */
> + if (priv->clks) {
> + ret = clk_bulk_prepare(priv->num_clks, priv->clks);
> + if (!ret) {
> + ret = clk_bulk_enable(priv->num_clks,
> + priv->clks);
> + if (ret)
> + return;
clk_bulk_prepare_enable(), which also takes care of unpreparing all
clocks if any clock fails to enabled.
> + }
> + }
> +
> /* enable platform power */
> usbhs_platform_call(priv, power_ctrl, pdev, priv->base, enable);
>
> @@ -353,6 +366,12 @@ 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->clks) {
> + clk_bulk_disable(priv->num_clks, priv->clks);
> + clk_bulk_unprepare(priv->num_clks, priv->clks);
clk_bulk_disable_unprepare()
> + }
> +
> /* disable PM */
> pm_runtime_put_sync(dev);
> }
> @@ -534,6 +553,11 @@ static struct renesas_usbhs_platform_info *usbhs_parse_dt(struct device *dev)
> return info;
> }
>
> +static const struct clk_bulk_data usbhs_rcar3_clks[] = {
> + { .id = "hsusb" },
> + { .id = "ehci/ohci" },
> +};
This structure contains space for the clock pointers, which are not really
needed here (the struct is copied).
Perhaps you can replace "struct clk_bulk_data *clks" in struct usbhs_priv
replace by "struct clk_bulk_data clks[2]", ...
> +
> static int usbhs_probe(struct platform_device *pdev)
> {
> struct renesas_usbhs_platform_info *info = renesas_usbhs_get_info(pdev);
> @@ -620,6 +644,15 @@ 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 = devm_kmemdup(&pdev->dev, usbhs_rcar3_clks,
> + sizeof(usbhs_rcar3_clks), GFP_KERNEL);
> + if (!priv->clks)
> + return -ENOMEM;
... and instead of allocating/copying, fill in the clock names here:
priv->clks[0].id = "hsusb";
priv->clks[1].id = "ehci/ohci";
As priv->clks will no longer be a pointer, all checks for it should be
replaced by checks for priv->num_clks.
What do you think?
Gr{oetje,eeting}s,
Geert
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 3/3] usb: renesas_usbhs: Add multiple clocks management
@ 2018-08-31 7:44 ` Yoshihiro Shimoda
0 siblings, 0 replies; 11+ messages in thread
From: Yoshihiro Shimoda @ 2018-08-31 7:44 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Felipe Balbi, Rob Herring, Mark Rutland, Greg KH, USB list,
Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Hi Geert-san,
> From: Geert Uytterhoeven, Sent: Friday, August 31, 2018 4:22 PM
>
> Hi Shimoda-san,
>
> On Fri, Aug 31, 2018 at 8:50 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>
>
> Thanks for your patch!
Thank you for your review!
> > --- 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>
> > @@ -336,11 +337,23 @@ static void usbhsc_power_ctrl(struct usbhs_priv *priv, int enable)
> > {
> > struct platform_device *pdev = usbhs_priv_to_pdev(priv);
> > struct device *dev = usbhs_priv_to_dev(priv);
> > + int ret;
> >
> > if (enable) {
> > /* enable PM */
> > pm_runtime_get_sync(dev);
> >
> > + /* enable clks if exist */
> > + if (priv->clks) {
> > + ret = clk_bulk_prepare(priv->num_clks, priv->clks);
> > + if (!ret) {
> > + ret = clk_bulk_enable(priv->num_clks,
> > + priv->clks);
> > + if (ret)
> > + return;
>
> clk_bulk_prepare_enable(), which also takes care of unpreparing all
> clocks if any clock fails to enabled.
Oops! I'll fix this in v2.
> > + }
> > + }
> > +
> > /* enable platform power */
> > usbhs_platform_call(priv, power_ctrl, pdev, priv->base, enable);
> >
> > @@ -353,6 +366,12 @@ 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->clks) {
> > + clk_bulk_disable(priv->num_clks, priv->clks);
> > + clk_bulk_unprepare(priv->num_clks, priv->clks);
>
> clk_bulk_disable_unprepare()
Thank you for the indicate. I'll fix this in v2.
> > + }
> > +
> > /* disable PM */
> > pm_runtime_put_sync(dev);
> > }
> > @@ -534,6 +553,11 @@ static struct renesas_usbhs_platform_info *usbhs_parse_dt(struct device *dev)
> > return info;
> > }
> >
> > +static const struct clk_bulk_data usbhs_rcar3_clks[] = {
> > + { .id = "hsusb" },
> > + { .id = "ehci/ohci" },
> > +};
>
> This structure contains space for the clock pointers, which are not really
> needed here (the struct is copied).
>
> Perhaps you can replace "struct clk_bulk_data *clks" in struct usbhs_priv
> replace by "struct clk_bulk_data clks[2]", ...
>
> > +
> > static int usbhs_probe(struct platform_device *pdev)
> > {
> > struct renesas_usbhs_platform_info *info = renesas_usbhs_get_info(pdev);
> > @@ -620,6 +644,15 @@ 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 = devm_kmemdup(&pdev->dev, usbhs_rcar3_clks,
> > + sizeof(usbhs_rcar3_clks), GFP_KERNEL);
> > + if (!priv->clks)
> > + return -ENOMEM;
>
> ... and instead of allocating/copying, fill in the clock names here:
>
> priv->clks[0].id = "hsusb";
> priv->clks[1].id = "ehci/ohci";
>
> As priv->clks will no longer be a pointer, all checks for it should be
> replaced by checks for priv->num_clks.
>
> What do you think?
I think your suggestion is better than this patch. So, I'll modify the code in v2.
Best regards,
Yoshihiro Shimoda
> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [3/3] usb: renesas_usbhs: Add multiple clocks management
@ 2018-08-31 7:44 ` Yoshihiro Shimoda
0 siblings, 0 replies; 11+ messages in thread
From: Yoshihiro Shimoda @ 2018-08-31 7:44 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Felipe Balbi, Rob Herring, Mark Rutland, Greg KH, USB list,
Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Hi Geert-san,
> From: Geert Uytterhoeven, Sent: Friday, August 31, 2018 4:22 PM
>
> Hi Shimoda-san,
>
> On Fri, Aug 31, 2018 at 8:50 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>
>
> Thanks for your patch!
Thank you for your review!
> > --- 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>
> > @@ -336,11 +337,23 @@ static void usbhsc_power_ctrl(struct usbhs_priv *priv, int enable)
> > {
> > struct platform_device *pdev = usbhs_priv_to_pdev(priv);
> > struct device *dev = usbhs_priv_to_dev(priv);
> > + int ret;
> >
> > if (enable) {
> > /* enable PM */
> > pm_runtime_get_sync(dev);
> >
> > + /* enable clks if exist */
> > + if (priv->clks) {
> > + ret = clk_bulk_prepare(priv->num_clks, priv->clks);
> > + if (!ret) {
> > + ret = clk_bulk_enable(priv->num_clks,
> > + priv->clks);
> > + if (ret)
> > + return;
>
> clk_bulk_prepare_enable(), which also takes care of unpreparing all
> clocks if any clock fails to enabled.
Oops! I'll fix this in v2.
> > + }
> > + }
> > +
> > /* enable platform power */
> > usbhs_platform_call(priv, power_ctrl, pdev, priv->base, enable);
> >
> > @@ -353,6 +366,12 @@ 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->clks) {
> > + clk_bulk_disable(priv->num_clks, priv->clks);
> > + clk_bulk_unprepare(priv->num_clks, priv->clks);
>
> clk_bulk_disable_unprepare()
Thank you for the indicate. I'll fix this in v2.
> > + }
> > +
> > /* disable PM */
> > pm_runtime_put_sync(dev);
> > }
> > @@ -534,6 +553,11 @@ static struct renesas_usbhs_platform_info *usbhs_parse_dt(struct device *dev)
> > return info;
> > }
> >
> > +static const struct clk_bulk_data usbhs_rcar3_clks[] = {
> > + { .id = "hsusb" },
> > + { .id = "ehci/ohci" },
> > +};
>
> This structure contains space for the clock pointers, which are not really
> needed here (the struct is copied).
>
> Perhaps you can replace "struct clk_bulk_data *clks" in struct usbhs_priv
> replace by "struct clk_bulk_data clks[2]", ...
>
> > +
> > static int usbhs_probe(struct platform_device *pdev)
> > {
> > struct renesas_usbhs_platform_info *info = renesas_usbhs_get_info(pdev);
> > @@ -620,6 +644,15 @@ 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 = devm_kmemdup(&pdev->dev, usbhs_rcar3_clks,
> > + sizeof(usbhs_rcar3_clks), GFP_KERNEL);
> > + if (!priv->clks)
> > + return -ENOMEM;
>
> ... and instead of allocating/copying, fill in the clock names here:
>
> priv->clks[0].id = "hsusb";
> priv->clks[1].id = "ehci/ohci";
>
> As priv->clks will no longer be a pointer, all checks for it should be
> replaced by checks for priv->num_clks.
>
> What do you think?
I think your suggestion is better than this patch. So, I'll modify the code in v2.
Best regards,
Yoshihiro Shimoda
> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-08-31 11:51 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-31 6:48 [PATCH 0/3] usb: renesas_usbhs: add reset_control and multiple clocks management Yoshihiro Shimoda
2018-08-31 6:48 ` [PATCH 1/3] usb: renesas_usbhs: Add reset_control Yoshihiro Shimoda
2018-08-31 6:48 ` [1/3] " Yoshihiro Shimoda
2018-08-31 6:48 ` [PATCH 2/3] dt-bindings: usb: renesas_usbhs: add clock-names property Yoshihiro Shimoda
2018-08-31 6:48 ` [2/3] " Yoshihiro Shimoda
2018-08-31 6:48 ` [PATCH 3/3] usb: renesas_usbhs: Add multiple clocks management Yoshihiro Shimoda
2018-08-31 6:48 ` [3/3] " Yoshihiro Shimoda
2018-08-31 7:22 ` [PATCH 3/3] " Geert Uytterhoeven
2018-08-31 7:22 ` [3/3] " Geert Uytterhoeven
2018-08-31 7:44 ` [PATCH 3/3] " Yoshihiro Shimoda
2018-08-31 7:44 ` [3/3] " Yoshihiro Shimoda
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.