All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] usb: renesas_usbhs: add reset_control and multiple clocks management
@ 2018-09-06  5:50 Yoshihiro Shimoda
  2018-09-06  5:50   ` [v3,1/3] " Yoshihiro Shimoda
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2018-09-06  5:50 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/

Changes from v2:
 - Use clk_bulk_enable_prepare() instead of two functions on patch 3/3.

Changes from v1:
 - Fix error path on patch 3/3.
 - Use clk_bulk_disable_unprepare() instead of two functions on patch 3/3.
 - Use staic array of struct clk_bulk_data instead of a pointer on patch 3/3.

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                 | 37 ++++++++++++++++++++++
 drivers/usb/renesas_usbhs/common.h                 |  5 +++
 3 files changed, 44 insertions(+)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 1/3] usb: renesas_usbhs: Add reset_control
@ 2018-09-06  5:50   ` Yoshihiro Shimoda
  0 siblings, 0 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2018-09-06  5:50 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	[flat|nested] 14+ messages in thread

* [v3,1/3] usb: renesas_usbhs: Add reset_control
@ 2018-09-06  5:50   ` Yoshihiro Shimoda
  0 siblings, 0 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2018-09-06  5:50 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	[flat|nested] 14+ messages in thread

* [PATCH v3 2/3] dt-bindings: usb: renesas_usbhs: add clock-names property
@ 2018-09-06  5:50   ` Yoshihiro Shimoda
  0 siblings, 0 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2018-09-06  5:50 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	[flat|nested] 14+ messages in thread

* [v3,2/3] dt-bindings: usb: renesas_usbhs: add clock-names property
@ 2018-09-06  5:50   ` Yoshihiro Shimoda
  0 siblings, 0 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2018-09-06  5:50 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	[flat|nested] 14+ messages in thread

* [PATCH v3 3/3] usb: renesas_usbhs: Add multiple clocks management
@ 2018-09-06  5:50   ` Yoshihiro Shimoda
  0 siblings, 0 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2018-09-06  5:50 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 | 25 +++++++++++++++++++++++++
 drivers/usb/renesas_usbhs/common.h |  3 +++
 2 files changed, 28 insertions(+)

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;
 };
 
 /*
-- 
1.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [v3,3/3] usb: renesas_usbhs: Add multiple clocks management
@ 2018-09-06  5:50   ` Yoshihiro Shimoda
  0 siblings, 0 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2018-09-06  5:50 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 | 25 +++++++++++++++++++++++++
 drivers/usb/renesas_usbhs/common.h |  3 +++
 2 files changed, 28 insertions(+)

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;
 };
 
 /*

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 3/3] usb: renesas_usbhs: Add multiple clocks management
  2018-09-06  5:50   ` [v3,3/3] " Yoshihiro Shimoda
  (?)
@ 2018-09-06  6:49     ` Kuninori Morimoto
  -1 siblings, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2018-09-06  6:49 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: balbi, robh+dt, mark.rutland, gregkh, linux-usb,
	linux-renesas-soc, devicetree


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 3/3] usb: renesas_usbhs: Add multiple clocks management
@ 2018-09-06  6:49     ` Kuninori Morimoto
  0 siblings, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2018-09-06  6:49 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: balbi, robh+dt, mark.rutland, gregkh, linux-usb,
	linux-renesas-soc, devicetree


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [v3,3/3] usb: renesas_usbhs: Add multiple clocks management
@ 2018-09-06  6:49     ` Kuninori Morimoto
  0 siblings, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2018-09-06  6:49 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: balbi, robh+dt, mark.rutland, gregkh, linux-usb,
	linux-renesas-soc, devicetree

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 3/3] usb: renesas_usbhs: Add multiple clocks management
@ 2018-09-06  7:28     ` Geert Uytterhoeven
  0 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2018-09-06  7:28 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 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

-- 
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] 14+ messages in thread

* [v3,3/3] usb: renesas_usbhs: Add multiple clocks management
@ 2018-09-06  7:28     ` Geert Uytterhoeven
  0 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2018-09-06  7:28 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 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH v3 3/3] usb: renesas_usbhs: Add multiple clocks management
@ 2018-09-06 12:09       ` Yoshihiro Shimoda
  0 siblings, 0 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2018-09-06 12:09 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: 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [v3,3/3] usb: renesas_usbhs: Add multiple clocks management
@ 2018-09-06 12:09       ` Yoshihiro Shimoda
  0 siblings, 0 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2018-09-06 12:09 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: 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-09-06 12:09 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-06  5:50 [PATCH v3 0/3] usb: renesas_usbhs: add reset_control and multiple clocks management Yoshihiro Shimoda
2018-09-06  5:50 ` [PATCH v3 1/3] usb: renesas_usbhs: Add reset_control Yoshihiro Shimoda
2018-09-06  5:50   ` [v3,1/3] " Yoshihiro Shimoda
2018-09-06  5:50 ` [PATCH v3 2/3] dt-bindings: usb: renesas_usbhs: add clock-names property Yoshihiro Shimoda
2018-09-06  5:50   ` [v3,2/3] " Yoshihiro Shimoda
2018-09-06  5:50 ` [PATCH v3 3/3] usb: renesas_usbhs: Add multiple clocks management Yoshihiro Shimoda
2018-09-06  5:50   ` [v3,3/3] " Yoshihiro Shimoda
2018-09-06  6:49   ` [PATCH v3 3/3] " Kuninori Morimoto
2018-09-06  6:49     ` [v3,3/3] " Kuninori Morimoto
2018-09-06  6:49     ` [PATCH v3 3/3] " Kuninori Morimoto
2018-09-06  7:28   ` Geert Uytterhoeven
2018-09-06  7:28     ` [v3,3/3] " Geert Uytterhoeven
2018-09-06 12:09     ` [PATCH v3 3/3] " Yoshihiro Shimoda
2018-09-06 12:09       ` [v3,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.