[PATCH 0/4] reset: uniphier: Rename from USB3 reset to glue reset and add AHCI reset support

[PATCH 0/4] reset: uniphier: Rename from USB3 reset to glue reset and add AHCI reset support

[Kunihiko Hayashi]

This series renames the reset control of core reset included in USB3 glue
layer with in the glue layer for generic peripherals to allow other devices
to use it.

And this series adds support for the core reset included in AHCI glue layer.

Kunihiko Hayashi (4):
  dt-bindings: reset: uniphier: Replace the expression of USB3 with
    generic peripherals
  reset: uniphier-usb3: Rename to reset-uniphier-glue
  dt-bindings: reset: uniphier: Add AHCI core reset description
  reset: uniphier-glue: Add AHCI reset control support in glue layer

 .../devicetree/bindings/reset/uniphier-reset.txt   |  25 +--
 drivers/reset/Kconfig                              |  10 +-
 drivers/reset/Makefile                             |   2 +-
 drivers/reset/reset-uniphier-glue.c                | 183 +++++++++++++++++++++
 drivers/reset/reset-uniphier-usb3.c                | 171 -------------------
 5 files changed, 203 insertions(+), 188 deletions(-)
 create mode 100644 drivers/reset/reset-uniphier-glue.c
 delete mode 100644 drivers/reset/reset-uniphier-usb3.c

-- 
2.7.4

[PATCH 1/4] dt-bindings: reset: uniphier: Replace the expression of USB3 with generic peripherals

[Kunihiko Hayashi]

Replace the expression of "USB3 glue layer" with the glue layer of the
generic peripherals to allow other devices to use it. The reset control
belongs to this glue layer.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 .../devicetree/bindings/reset/uniphier-reset.txt   | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/reset/uniphier-reset.txt b/Documentation/devicetree/bindings/reset/uniphier-reset.txt
index 101743d..f63c511 100644
--- a/Documentation/devicetree/bindings/reset/uniphier-reset.txt
+++ b/Documentation/devicetree/bindings/reset/uniphier-reset.txt
@@ -120,27 +120,27 @@ Example:
 	};
 
 
-USB3 core reset
----------------
+Peripheral core reset in glue layer
+-----------------------------------
 
-USB3 core reset belongs to USB3 glue layer. Before using the core reset,
-it is necessary to control the clocks and resets to enable this layer.
-These clocks and resets should be described in each property.
+Some peripheral core reset belongs to its own glue layer. Before using
+this core reset, it is necessary to control the clocks and resets to enable
+this layer. These clocks and resets should be described in each property.
 
 Required properties:
 - compatible: Should be
-    "socionext,uniphier-pro4-usb3-reset" - for Pro4 SoC
-    "socionext,uniphier-pxs2-usb3-reset" - for PXs2 SoC
-    "socionext,uniphier-ld20-usb3-reset" - for LD20 SoC
-    "socionext,uniphier-pxs3-usb3-reset" - for PXs3 SoC
+    "socionext,uniphier-pro4-usb3-reset" - for Pro4 SoC USB3
+    "socionext,uniphier-pxs2-usb3-reset" - for PXs2 SoC USB3
+    "socionext,uniphier-ld20-usb3-reset" - for LD20 SoC USB3
+    "socionext,uniphier-pxs3-usb3-reset" - for PXs3 SoC USB3
 - #reset-cells: Should be 1.
 - reg: Specifies offset and length of the register set for the device.
-- clocks: A list of phandles to the clock gate for USB3 glue layer.
+- clocks: A list of phandles to the clock gate for the glue layer.
 	According to the clock-names, appropriate clocks are required.
 - clock-names: Should contain
     "gio", "link" - for Pro4 SoC
     "link"        - for others
-- resets: A list of phandles to the reset control for USB3 glue layer.
+- resets: A list of phandles to the reset control for the glue layer.
 	According to the reset-names, appropriate resets are required.
 - reset-names: Should contain
     "gio", "link" - for Pro4 SoC
-- 
2.7.4

Re: [PATCH 1/4] dt-bindings: reset: uniphier: Replace the expression of USB3 with generic peripherals

[Rob Herring]

On Fri,  9 Nov 2018 10:42:04 +0900, Kunihiko Hayashi wrote:
> Replace the expression of "USB3 glue layer" with the glue layer of the
> generic peripherals to allow other devices to use it. The reset control
> belongs to this glue layer.
> 
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> ---
>  .../devicetree/bindings/reset/uniphier-reset.txt   | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH 2/4] reset: uniphier-usb3: Rename to reset-uniphier-glue

[Kunihiko Hayashi]

This driver works for controlling the reset lines including USB3
glue layer, however, this can be applied to other glue layers.
Now this patch renames the driver from "reset-uniphier-usb3" to
"reset-uniphier-glue".

At the same time, this changes CONFIG_RESET_UNIPHIER_USB3 to
CONFIG_RESET_UNIPHIER_GLUE.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/reset/Kconfig               |  10 +--
 drivers/reset/Makefile              |   2 +-
 drivers/reset/reset-uniphier-glue.c | 171 ++++++++++++++++++++++++++++++++++++
 drivers/reset/reset-uniphier-usb3.c | 171 ------------------------------------
 4 files changed, 177 insertions(+), 177 deletions(-)
 create mode 100644 drivers/reset/reset-uniphier-glue.c
 delete mode 100644 drivers/reset/reset-uniphier-usb3.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index c21da9f..ef7f468 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -163,15 +163,15 @@ config RESET_UNIPHIER
 	  Say Y if you want to control reset signals provided by System Control
 	  block, Media I/O block, Peripheral Block.
 
-config RESET_UNIPHIER_USB3
-	tristate "USB3 reset driver for UniPhier SoCs"
+config RESET_UNIPHIER_GLUE
+	tristate "Reset driver in glue layer for UniPhier SoCs"
 	depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF
 	default ARCH_UNIPHIER
 	select RESET_SIMPLE
 	help
-	  Support for the USB3 core reset on UniPhier SoCs.
-	  Say Y if you want to control reset signals provided by
-	  USB3 glue layer.
+	  Support for peripheral core reset included in its own glue layer
+	  on UniPhier SoCs. Say Y if you want to control reset signals
+	  provided by the glue layer.
 
 config RESET_ZYNQ
 	bool "ZYNQ Reset Driver" if COMPILE_TEST
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index d08e8b9..4570ecf 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -23,6 +23,6 @@ obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
 obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
 obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
-obj-$(CONFIG_RESET_UNIPHIER_USB3) += reset-uniphier-usb3.o
+obj-$(CONFIG_RESET_UNIPHIER_GLUE) += reset-uniphier-glue.o
 obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
 
diff --git a/drivers/reset/reset-uniphier-glue.c b/drivers/reset/reset-uniphier-glue.c
new file mode 100644
index 0000000..1b8ea03
--- /dev/null
+++ b/drivers/reset/reset-uniphier-glue.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// reset-uniphier-glue.c - Glue layer reset driver for UniPhier
+// Copyright 2018 Socionext Inc.
+// Author: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#include "reset-simple.h"
+
+#define MAX_CLKS	2
+#define MAX_RSTS	2
+
+struct uniphier_glue_reset_soc_data {
+	int nclks;
+	const char * const *clock_names;
+	int nrsts;
+	const char * const *reset_names;
+};
+
+struct uniphier_glue_reset_priv {
+	struct clk_bulk_data clk[MAX_CLKS];
+	struct reset_control *rst[MAX_RSTS];
+	struct reset_simple_data rdata;
+	const struct uniphier_glue_reset_soc_data *data;
+};
+
+static int uniphier_glue_reset_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct uniphier_glue_reset_priv *priv;
+	struct resource *res;
+	resource_size_t size;
+	const char *name;
+	int i, ret, nr;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->data = of_device_get_match_data(dev);
+	if (WARN_ON(!priv->data || priv->data->nclks > MAX_CLKS ||
+		    priv->data->nrsts > MAX_RSTS))
+		return -EINVAL;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	size = resource_size(res);
+	priv->rdata.membase = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->rdata.membase))
+		return PTR_ERR(priv->rdata.membase);
+
+	for (i = 0; i < priv->data->nclks; i++)
+		priv->clk[i].id = priv->data->clock_names[i];
+	ret = devm_clk_bulk_get(dev, priv->data->nclks, priv->clk);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < priv->data->nrsts; i++) {
+		name = priv->data->reset_names[i];
+		priv->rst[i] = devm_reset_control_get_shared(dev, name);
+		if (IS_ERR(priv->rst[i]))
+			return PTR_ERR(priv->rst[i]);
+	}
+
+	ret = clk_bulk_prepare_enable(priv->data->nclks, priv->clk);
+	if (ret)
+		return ret;
+
+	for (nr = 0; nr < priv->data->nrsts; nr++) {
+		ret = reset_control_deassert(priv->rst[nr]);
+		if (ret)
+			goto out_rst_assert;
+	}
+
+	spin_lock_init(&priv->rdata.lock);
+	priv->rdata.rcdev.owner = THIS_MODULE;
+	priv->rdata.rcdev.nr_resets = size * BITS_PER_BYTE;
+	priv->rdata.rcdev.ops = &reset_simple_ops;
+	priv->rdata.rcdev.of_node = dev->of_node;
+	priv->rdata.active_low = true;
+
+	platform_set_drvdata(pdev, priv);
+
+	ret = devm_reset_controller_register(dev, &priv->rdata.rcdev);
+	if (ret)
+		goto out_rst_assert;
+
+	return 0;
+
+out_rst_assert:
+	while (nr--)
+		reset_control_assert(priv->rst[nr]);
+
+	clk_bulk_disable_unprepare(priv->data->nclks, priv->clk);
+
+	return ret;
+}
+
+static int uniphier_glue_reset_remove(struct platform_device *pdev)
+{
+	struct uniphier_glue_reset_priv *priv = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < priv->data->nrsts; i++)
+		reset_control_assert(priv->rst[i]);
+
+	clk_bulk_disable_unprepare(priv->data->nclks, priv->clk);
+
+	return 0;
+}
+
+static const char * const uniphier_pro4_clock_reset_names[] = {
+	"gio", "link",
+};
+
+static const struct uniphier_glue_reset_soc_data uniphier_pro4_data = {
+	.nclks = ARRAY_SIZE(uniphier_pro4_clock_reset_names),
+	.clock_names = uniphier_pro4_clock_reset_names,
+	.nrsts = ARRAY_SIZE(uniphier_pro4_clock_reset_names),
+	.reset_names = uniphier_pro4_clock_reset_names,
+};
+
+static const char * const uniphier_pxs2_clock_reset_names[] = {
+	"link",
+};
+
+static const struct uniphier_glue_reset_soc_data uniphier_pxs2_data = {
+	.nclks = ARRAY_SIZE(uniphier_pxs2_clock_reset_names),
+	.clock_names = uniphier_pxs2_clock_reset_names,
+	.nrsts = ARRAY_SIZE(uniphier_pxs2_clock_reset_names),
+	.reset_names = uniphier_pxs2_clock_reset_names,
+};
+
+static const struct of_device_id uniphier_glue_reset_match[] = {
+	{
+		.compatible = "socionext,uniphier-pro4-usb3-reset",
+		.data = &uniphier_pro4_data,
+	},
+	{
+		.compatible = "socionext,uniphier-pxs2-usb3-reset",
+		.data = &uniphier_pxs2_data,
+	},
+	{
+		.compatible = "socionext,uniphier-ld20-usb3-reset",
+		.data = &uniphier_pxs2_data,
+	},
+	{
+		.compatible = "socionext,uniphier-pxs3-usb3-reset",
+		.data = &uniphier_pxs2_data,
+	},
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, uniphier_glue_reset_match);
+
+static struct platform_driver uniphier_glue_reset_driver = {
+	.probe = uniphier_glue_reset_probe,
+	.remove = uniphier_glue_reset_remove,
+	.driver = {
+		.name = "uniphier-glue-reset",
+		.of_match_table = uniphier_glue_reset_match,
+	},
+};
+module_platform_driver(uniphier_glue_reset_driver);
+
+MODULE_AUTHOR("Kunihiko Hayashi <hayashi.kunihiko@socionext.com>");
+MODULE_DESCRIPTION("UniPhier Glue layer reset driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/reset/reset-uniphier-usb3.c b/drivers/reset/reset-uniphier-usb3.c
deleted file mode 100644
index ffa1b19..0000000
--- a/drivers/reset/reset-uniphier-usb3.c
+++ /dev/null
@@ -1,171 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-//
-// reset-uniphier-usb3.c - USB3 reset driver for UniPhier
-// Copyright 2018 Socionext Inc.
-// Author: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
-
-#include <linux/clk.h>
-#include <linux/module.h>
-#include <linux/of_device.h>
-#include <linux/platform_device.h>
-#include <linux/reset.h>
-
-#include "reset-simple.h"
-
-#define MAX_CLKS	2
-#define MAX_RSTS	2
-
-struct uniphier_usb3_reset_soc_data {
-	int nclks;
-	const char * const *clock_names;
-	int nrsts;
-	const char * const *reset_names;
-};
-
-struct uniphier_usb3_reset_priv {
-	struct clk_bulk_data clk[MAX_CLKS];
-	struct reset_control *rst[MAX_RSTS];
-	struct reset_simple_data rdata;
-	const struct uniphier_usb3_reset_soc_data *data;
-};
-
-static int uniphier_usb3_reset_probe(struct platform_device *pdev)
-{
-	struct device *dev = &pdev->dev;
-	struct uniphier_usb3_reset_priv *priv;
-	struct resource *res;
-	resource_size_t size;
-	const char *name;
-	int i, ret, nr;
-
-	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
-
-	priv->data = of_device_get_match_data(dev);
-	if (WARN_ON(!priv->data || priv->data->nclks > MAX_CLKS ||
-		    priv->data->nrsts > MAX_RSTS))
-		return -EINVAL;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	size = resource_size(res);
-	priv->rdata.membase = devm_ioremap_resource(dev, res);
-	if (IS_ERR(priv->rdata.membase))
-		return PTR_ERR(priv->rdata.membase);
-
-	for (i = 0; i < priv->data->nclks; i++)
-		priv->clk[i].id = priv->data->clock_names[i];
-	ret = devm_clk_bulk_get(dev, priv->data->nclks, priv->clk);
-	if (ret)
-		return ret;
-
-	for (i = 0; i < priv->data->nrsts; i++) {
-		name = priv->data->reset_names[i];
-		priv->rst[i] = devm_reset_control_get_shared(dev, name);
-		if (IS_ERR(priv->rst[i]))
-			return PTR_ERR(priv->rst[i]);
-	}
-
-	ret = clk_bulk_prepare_enable(priv->data->nclks, priv->clk);
-	if (ret)
-		return ret;
-
-	for (nr = 0; nr < priv->data->nrsts; nr++) {
-		ret = reset_control_deassert(priv->rst[nr]);
-		if (ret)
-			goto out_rst_assert;
-	}
-
-	spin_lock_init(&priv->rdata.lock);
-	priv->rdata.rcdev.owner = THIS_MODULE;
-	priv->rdata.rcdev.nr_resets = size * BITS_PER_BYTE;
-	priv->rdata.rcdev.ops = &reset_simple_ops;
-	priv->rdata.rcdev.of_node = dev->of_node;
-	priv->rdata.active_low = true;
-
-	platform_set_drvdata(pdev, priv);
-
-	ret = devm_reset_controller_register(dev, &priv->rdata.rcdev);
-	if (ret)
-		goto out_rst_assert;
-
-	return 0;
-
-out_rst_assert:
-	while (nr--)
-		reset_control_assert(priv->rst[nr]);
-
-	clk_bulk_disable_unprepare(priv->data->nclks, priv->clk);
-
-	return ret;
-}
-
-static int uniphier_usb3_reset_remove(struct platform_device *pdev)
-{
-	struct uniphier_usb3_reset_priv *priv = platform_get_drvdata(pdev);
-	int i;
-
-	for (i = 0; i < priv->data->nrsts; i++)
-		reset_control_assert(priv->rst[i]);
-
-	clk_bulk_disable_unprepare(priv->data->nclks, priv->clk);
-
-	return 0;
-}
-
-static const char * const uniphier_pro4_clock_reset_names[] = {
-	"gio", "link",
-};
-
-static const struct uniphier_usb3_reset_soc_data uniphier_pro4_data = {
-	.nclks = ARRAY_SIZE(uniphier_pro4_clock_reset_names),
-	.clock_names = uniphier_pro4_clock_reset_names,
-	.nrsts = ARRAY_SIZE(uniphier_pro4_clock_reset_names),
-	.reset_names = uniphier_pro4_clock_reset_names,
-};
-
-static const char * const uniphier_pxs2_clock_reset_names[] = {
-	"link",
-};
-
-static const struct uniphier_usb3_reset_soc_data uniphier_pxs2_data = {
-	.nclks = ARRAY_SIZE(uniphier_pxs2_clock_reset_names),
-	.clock_names = uniphier_pxs2_clock_reset_names,
-	.nrsts = ARRAY_SIZE(uniphier_pxs2_clock_reset_names),
-	.reset_names = uniphier_pxs2_clock_reset_names,
-};
-
-static const struct of_device_id uniphier_usb3_reset_match[] = {
-	{
-		.compatible = "socionext,uniphier-pro4-usb3-reset",
-		.data = &uniphier_pro4_data,
-	},
-	{
-		.compatible = "socionext,uniphier-pxs2-usb3-reset",
-		.data = &uniphier_pxs2_data,
-	},
-	{
-		.compatible = "socionext,uniphier-ld20-usb3-reset",
-		.data = &uniphier_pxs2_data,
-	},
-	{
-		.compatible = "socionext,uniphier-pxs3-usb3-reset",
-		.data = &uniphier_pxs2_data,
-	},
-	{ /* Sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, uniphier_usb3_reset_match);
-
-static struct platform_driver uniphier_usb3_reset_driver = {
-	.probe = uniphier_usb3_reset_probe,
-	.remove = uniphier_usb3_reset_remove,
-	.driver = {
-		.name = "uniphier-usb3-reset",
-		.of_match_table = uniphier_usb3_reset_match,
-	},
-};
-module_platform_driver(uniphier_usb3_reset_driver);
-
-MODULE_AUTHOR("Kunihiko Hayashi <hayashi.kunihiko@socionext.com>");
-MODULE_DESCRIPTION("UniPhier USB3 Reset Driver");
-MODULE_LICENSE("GPL");
-- 
2.7.4

[PATCH 3/4] dt-bindings: reset: uniphier: Add AHCI core reset description

[Kunihiko Hayashi]

Add compatible strings for reset control of AHCI core implemented in
UniPhier SoCs. The reset control belongs to AHCI glue layer.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 Documentation/devicetree/bindings/reset/uniphier-reset.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/reset/uniphier-reset.txt b/Documentation/devicetree/bindings/reset/uniphier-reset.txt
index f63c511..ea00517 100644
--- a/Documentation/devicetree/bindings/reset/uniphier-reset.txt
+++ b/Documentation/devicetree/bindings/reset/uniphier-reset.txt
@@ -133,6 +133,9 @@ Required properties:
     "socionext,uniphier-pxs2-usb3-reset" - for PXs2 SoC USB3
     "socionext,uniphier-ld20-usb3-reset" - for LD20 SoC USB3
     "socionext,uniphier-pxs3-usb3-reset" - for PXs3 SoC USB3
+    "socionext,uniphier-pro4-ahci-reset" - for Pro4 SoC AHCI
+    "socionext,uniphier-pxs2-ahci-reset" - for PXs2 SoC AHCI
+    "socionext,uniphier-pxs3-ahci-reset" - for PXs3 SoC AHCI
 - #reset-cells: Should be 1.
 - reg: Specifies offset and length of the register set for the device.
 - clocks: A list of phandles to the clock gate for the glue layer.
-- 
2.7.4

Re: [PATCH 3/4] dt-bindings: reset: uniphier: Add AHCI core reset description

[Philipp Zabel]

Hi Kunihiko,

On Fri, 2018-11-09 at 10:42 +0900, Kunihiko Hayashi wrote:
> Add compatible strings for reset control of AHCI core implemented in
> UniPhier SoCs. The reset control belongs to AHCI glue layer.
> 
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> ---
>  Documentation/devicetree/bindings/reset/uniphier-reset.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/reset/uniphier-reset.txt b/Documentation/devicetree/bindings/reset/uniphier-reset.txt
> index f63c511..ea00517 100644
> --- a/Documentation/devicetree/bindings/reset/uniphier-reset.txt
> +++ b/Documentation/devicetree/bindings/reset/uniphier-reset.txt
> @@ -133,6 +133,9 @@ Required properties:
>      "socionext,uniphier-pxs2-usb3-reset" - for PXs2 SoC USB3
>      "socionext,uniphier-ld20-usb3-reset" - for LD20 SoC USB3
>      "socionext,uniphier-pxs3-usb3-reset" - for PXs3 SoC USB3
> +    "socionext,uniphier-pro4-ahci-reset" - for Pro4 SoC AHCI
> +    "socionext,uniphier-pxs2-ahci-reset" - for PXs2 SoC AHCI
> +    "socionext,uniphier-pxs3-ahci-reset" - for PXs3 SoC AHCI

Since the driver behaves identically for "socionext,uniphier-pro4-usb3-
reset" and "socionext,uniphier-pro4-ahci-reset", would it make sense to
add a common compatible?
Something like:
"socionext,uniphier-pro4-usb3-reset", "socionext,uniphier-pro4-glue-reset" - for USB3 SoC AHCI
"socionext,uniphier-pro4-ahci-reset", "socionext,uniphier-pro4-glue-reset" - for Pro4 SoC AHCI

That way if more places turn up where the glue layer reset is used,
you can add them without patching the driver every time.

regards
Philipp

Re: [PATCH 3/4] dt-bindings: reset: uniphier: Add AHCI core reset description

[Masahiro Yamada]

On Sat, Nov 10, 2018 at 12:02 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> Hi Kunihiko,
>
> On Fri, 2018-11-09 at 10:42 +0900, Kunihiko Hayashi wrote:
> > Add compatible strings for reset control of AHCI core implemented in
> > UniPhier SoCs. The reset control belongs to AHCI glue layer.
> >
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> > ---
> >  Documentation/devicetree/bindings/reset/uniphier-reset.txt | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/reset/uniphier-reset.txt b/Documentation/devicetree/bindings/reset/uniphier-reset.txt
> > index f63c511..ea00517 100644
> > --- a/Documentation/devicetree/bindings/reset/uniphier-reset.txt
> > +++ b/Documentation/devicetree/bindings/reset/uniphier-reset.txt
> > @@ -133,6 +133,9 @@ Required properties:
> >      "socionext,uniphier-pxs2-usb3-reset" - for PXs2 SoC USB3
> >      "socionext,uniphier-ld20-usb3-reset" - for LD20 SoC USB3
> >      "socionext,uniphier-pxs3-usb3-reset" - for PXs3 SoC USB3
> > +    "socionext,uniphier-pro4-ahci-reset" - for Pro4 SoC AHCI
> > +    "socionext,uniphier-pxs2-ahci-reset" - for PXs2 SoC AHCI
> > +    "socionext,uniphier-pxs3-ahci-reset" - for PXs3 SoC AHCI
>
> Since the driver behaves identically for "socionext,uniphier-pro4-usb3-
> reset" and "socionext,uniphier-pro4-ahci-reset", would it make sense to
> add a common compatible?


As far as I could guess, he just happened to find the same driver code
could be reused for other hardware.

Theoretically, this can happen anywhere since
a reset controller is just a set of registers
each bit of which is connected to a reset line.

If you added a super-generic compatible like "simple-reset",
I would agree with
"socionext,uniphier-pro4-usb3-reset", "simple-reset"
since this is a pattern.


However,
"socionext,uniphier-pro4-glue-reset" is kind of a halfway house
where it is SoC-specific, but still ambiguous.


> Something like:
> "socionext,uniphier-pro4-usb3-reset", "socionext,uniphier-pro4-glue-reset" - for USB3 SoC AHCI
> "socionext,uniphier-pro4-ahci-reset", "socionext,uniphier-pro4-glue-reset" - for Pro4 SoC AHCI
>
> That way if more places turn up where the glue layer reset is used,
> you can add them without patching the driver every time.


This is a trade-off between "patch the driver"
and "potential change of the binding".

There is no real hardware like pro4-glue-reset.



I am guessing this is a part of syscon or something,
but I cannot find any explanation in a bigger picture.

So, I cannot judge this further more.




> regards
> Philipp



--
Best Regards
Masahiro Yamada

Re: [PATCH 3/4] dt-bindings: reset: uniphier: Add AHCI core reset description

[Kunihiko Hayashi]

Hi,

Thank you for some comments and pointing out.

On Sat, 10 Nov 2018 01:14:06 +0900 <yamada.masahiro@socionext.com> wrote:

> On Sat, Nov 10, 2018 at 12:02 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> >
> > Hi Kunihiko,
> >
> > On Fri, 2018-11-09 at 10:42 +0900, Kunihiko Hayashi wrote:
> > > Add compatible strings for reset control of AHCI core implemented in
> > > UniPhier SoCs. The reset control belongs to AHCI glue layer.
> > >
> > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> > > ---
> > >  Documentation/devicetree/bindings/reset/uniphier-reset.txt | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/reset/uniphier-reset.txt b/Documentation/devicetree/bindings/reset/uniphier-reset.txt
> > > index f63c511..ea00517 100644
> > > --- a/Documentation/devicetree/bindings/reset/uniphier-reset.txt
> > > +++ b/Documentation/devicetree/bindings/reset/uniphier-reset.txt
> > > @@ -133,6 +133,9 @@ Required properties:
> > >      "socionext,uniphier-pxs2-usb3-reset" - for PXs2 SoC USB3
> > >      "socionext,uniphier-ld20-usb3-reset" - for LD20 SoC USB3
> > >      "socionext,uniphier-pxs3-usb3-reset" - for PXs3 SoC USB3
> > > +    "socionext,uniphier-pro4-ahci-reset" - for Pro4 SoC AHCI
> > > +    "socionext,uniphier-pxs2-ahci-reset" - for PXs2 SoC AHCI
> > > +    "socionext,uniphier-pxs3-ahci-reset" - for PXs3 SoC AHCI
> >
> > Since the driver behaves identically for "socionext,uniphier-pro4-usb3-
> > reset" and "socionext,uniphier-pro4-ahci-reset", would it make sense to
> > add a common compatible?
> 
> 
> As far as I could guess, he just happened to find the same driver code
> could be reused for other hardware.
> 
> Theoretically, this can happen anywhere since
> a reset controller is just a set of registers
> each bit of which is connected to a reset line.
> 
> If you added a super-generic compatible like "simple-reset",
> I would agree with
> "socionext,uniphier-pro4-usb3-reset", "simple-reset"
> since this is a pattern.

I think it's more generic to define simple-reset with parent clock/reset
control without both SoC and device names.

However, such parent clocks/resets strongly depends on SoC, 
so I think it's difficut to lead generic definition in this case.

If we add generic compatible string, I also add "simple-reset".


> However,
> "socionext,uniphier-pro4-glue-reset" is kind of a halfway house
> where it is SoC-specific, but still ambiguous.

Surely, it might be hard to understand that pro4-glue-reset is SoC-specific
but for generic-device.


> > Something like:
> > "socionext,uniphier-pro4-usb3-reset", "socionext,uniphier-pro4-glue-reset" - for USB3 SoC AHCI
> > "socionext,uniphier-pro4-ahci-reset", "socionext,uniphier-pro4-glue-reset" - for Pro4 SoC AHCI
> >
> > That way if more places turn up where the glue layer reset is used,
> > you can add them without patching the driver every time.
> 
> 
> This is a trade-off between "patch the driver"
> and "potential change of the binding".

Adding "glue-reset" is usable for devices having same parent clocks/resets as
usb3/ahci, and we can add the devices without patches.

However, if the device needs other parent clocks/resets for the same SoC,
we can't add "glue-reset" and we might add patches for the device as a result.
In this case, the "glue-reset" will become difficult to understand.


> There is no real hardware like pro4-glue-reset.
>
> I am guessing this is a part of syscon or something,
> but I cannot find any explanation in a bigger picture.
> 
> So, I cannot judge this further more.

Since it's hard to lead the best result,
currently I'd like to suggest the current compatibles, with "simple-reset"
if necessary.

Thank you,

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH 3/4] dt-bindings: reset: uniphier: Add AHCI core reset description

[Philipp Zabel]

Hi,

On Mon, 2018-11-12 at 21:02 +0900, Kunihiko Hayashi wrote:
> Hi,
> 
> Thank you for some comments and pointing out.
> 
> On Sat, 10 Nov 2018 01:14:06 +0900 <yamada.masahiro@socionext.com> wrote:
> 
> > On Sat, Nov 10, 2018 at 12:02 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > > 
> > > Hi Kunihiko,
> > > 
> > > On Fri, 2018-11-09 at 10:42 +0900, Kunihiko Hayashi wrote:
> > > > Add compatible strings for reset control of AHCI core implemented in
> > > > UniPhier SoCs. The reset control belongs to AHCI glue layer.
> > > > 
> > > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/reset/uniphier-reset.txt | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/reset/uniphier-reset.txt b/Documentation/devicetree/bindings/reset/uniphier-reset.txt
> > > > index f63c511..ea00517 100644
> > > > --- a/Documentation/devicetree/bindings/reset/uniphier-reset.txt
> > > > +++ b/Documentation/devicetree/bindings/reset/uniphier-reset.txt
> > > > @@ -133,6 +133,9 @@ Required properties:
> > > >      "socionext,uniphier-pxs2-usb3-reset" - for PXs2 SoC USB3
> > > >      "socionext,uniphier-ld20-usb3-reset" - for LD20 SoC USB3
> > > >      "socionext,uniphier-pxs3-usb3-reset" - for PXs3 SoC USB3
> > > > +    "socionext,uniphier-pro4-ahci-reset" - for Pro4 SoC AHCI
> > > > +    "socionext,uniphier-pxs2-ahci-reset" - for PXs2 SoC AHCI
> > > > +    "socionext,uniphier-pxs3-ahci-reset" - for PXs3 SoC AHCI
> > > 
> > > Since the driver behaves identically for "socionext,uniphier-pro4-usb3-
> > > reset" and "socionext,uniphier-pro4-ahci-reset", would it make sense to
> > > add a common compatible?
> > 
> > As far as I could guess, he just happened to find the same driver code
> > could be reused for other hardware.

Ok, in that case never mind. I just assumed that this could be a case of
glue layer building blocks being reused by the hardware engineers, since
the driver reuses the same clock names for USB3 and AHCI both on Pro4
and on PXs2.

> > Theoretically, this can happen anywhere since
> > a reset controller is just a set of registers
> > each bit of which is connected to a reset line.
> > 
> > If you added a super-generic compatible like "simple-reset",
> > I would agree with
> > "socionext,uniphier-pro4-usb3-reset", "simple-reset"
> > since this is a pattern.
> 
> I think it's more generic to define simple-reset with parent clock/reset
> control without both SoC and device names.
> 
> However, such parent clocks/resets strongly depends on SoC, 
> so I think it's difficut to lead generic definition in this case.
> 
> If we add generic compatible string, I also add "simple-reset".

There is no "simple-reset" binding definition. As soon as there are SoC
specific clocks, it's not really generic anymore.

> > However,
> > "socionext,uniphier-pro4-glue-reset" is kind of a halfway house
> > where it is SoC-specific, but still ambiguous.
> 
> Surely, it might be hard to understand that pro4-glue-reset is SoC-specific
> but for generic-device.

I agree.

> > > Something like:
> > > "socionext,uniphier-pro4-usb3-reset", "socionext,uniphier-pro4-glue-reset" - for USB3 SoC AHCI
> > > "socionext,uniphier-pro4-ahci-reset", "socionext,uniphier-pro4-glue-reset" - for Pro4 SoC AHCI
> > > 
> > > That way if more places turn up where the glue layer reset is used,
> > > you can add them without patching the driver every time.
> > 
> > 
> > This is a trade-off between "patch the driver"
> > and "potential change of the binding".
> 
> Adding "glue-reset" is usable for devices having same parent clocks/resets as
> usb3/ahci, and we can add the devices without patches.
> 
> However, if the device needs other parent clocks/resets for the same SoC,
> we can't add "glue-reset" and we might add patches for the device as a result.
> In this case, the "glue-reset" will become difficult to understand.

Then let's not bother with it.
I made the suggestion without knowing the full picture.

> > There is no real hardware like pro4-glue-reset.
> > 
> > I am guessing this is a part of syscon or something,
> > but I cannot find any explanation in a bigger picture.
> > 
> > So, I cannot judge this further more.
> 
> Since it's hard to lead the best result,
> currently I'd like to suggest the current compatibles,

That is fine with me.

> with "simple-reset" if necessary.

Not necessary.

best regards
Philipp

Re: [PATCH 3/4] dt-bindings: reset: uniphier: Add AHCI core reset description

[Kunihiko Hayashi]

Hi Philipp,

On Mon, 12 Nov 2018 15:21:46 +0100 <p.zabel@pengutronix.de> wrote:

> Hi,
> 
> On Mon, 2018-11-12 at 21:02 +0900, Kunihiko Hayashi wrote:
> > Hi,
> > 
> > Thank you for some comments and pointing out.
> > 
> > On Sat, 10 Nov 2018 01:14:06 +0900 <yamada.masahiro@socionext.com> wrote:
> > 
> > > On Sat, Nov 10, 2018 at 12:02 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > > > 
> > > > Hi Kunihiko,
> > > > 
> > > > On Fri, 2018-11-09 at 10:42 +0900, Kunihiko Hayashi wrote:
> > > > > Add compatible strings for reset control of AHCI core implemented in
> > > > > UniPhier SoCs. The reset control belongs to AHCI glue layer.
> > > > > 
> > > > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> > > > > ---
> > > > >  Documentation/devicetree/bindings/reset/uniphier-reset.txt | 3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/reset/uniphier-reset.txt b/Documentation/devicetree/bindings/reset/uniphier-reset.txt
> > > > > index f63c511..ea00517 100644
> > > > > --- a/Documentation/devicetree/bindings/reset/uniphier-reset.txt
> > > > > +++ b/Documentation/devicetree/bindings/reset/uniphier-reset.txt
> > > > > @@ -133,6 +133,9 @@ Required properties:
> > > > >      "socionext,uniphier-pxs2-usb3-reset" - for PXs2 SoC USB3
> > > > >      "socionext,uniphier-ld20-usb3-reset" - for LD20 SoC USB3
> > > > >      "socionext,uniphier-pxs3-usb3-reset" - for PXs3 SoC USB3
> > > > > +    "socionext,uniphier-pro4-ahci-reset" - for Pro4 SoC AHCI
> > > > > +    "socionext,uniphier-pxs2-ahci-reset" - for PXs2 SoC AHCI
> > > > > +    "socionext,uniphier-pxs3-ahci-reset" - for PXs3 SoC AHCI
> > > > 
> > > > Since the driver behaves identically for "socionext,uniphier-pro4-usb3-
> > > > reset" and "socionext,uniphier-pro4-ahci-reset", would it make sense to
> > > > add a common compatible?
> > > 
> > > As far as I could guess, he just happened to find the same driver code
> > > could be reused for other hardware.
> 
> Ok, in that case never mind. I just assumed that this could be a case of
> glue layer building blocks being reused by the hardware engineers, since
> the driver reuses the same clock names for USB3 and AHCI both on Pro4
> and on PXs2.
> 
> > > Theoretically, this can happen anywhere since
> > > a reset controller is just a set of registers
> > > each bit of which is connected to a reset line.
> > > 
> > > If you added a super-generic compatible like "simple-reset",
> > > I would agree with
> > > "socionext,uniphier-pro4-usb3-reset", "simple-reset"
> > > since this is a pattern.
> > 
> > I think it's more generic to define simple-reset with parent clock/reset
> > control without both SoC and device names.
> > 
> > However, such parent clocks/resets strongly depends on SoC, 
> > so I think it's difficut to lead generic definition in this case.
> > 
> > If we add generic compatible string, I also add "simple-reset".
> 
> There is no "simple-reset" binding definition. As soon as there are SoC
> specific clocks, it's not really generic anymore.

I see. I understand "simple-reset" doesn't need.

> > > However,
> > > "socionext,uniphier-pro4-glue-reset" is kind of a halfway house
> > > where it is SoC-specific, but still ambiguous.
> > 
> > Surely, it might be hard to understand that pro4-glue-reset is SoC-specific
> > but for generic-device.
> 
> I agree.
> 
> > > > Something like:
> > > > "socionext,uniphier-pro4-usb3-reset", "socionext,uniphier-pro4-glue-reset" - for USB3 SoC AHCI
> > > > "socionext,uniphier-pro4-ahci-reset", "socionext,uniphier-pro4-glue-reset" - for Pro4 SoC AHCI
> > > > 
> > > > That way if more places turn up where the glue layer reset is used,
> > > > you can add them without patching the driver every time.
> > > 
> > > 
> > > This is a trade-off between "patch the driver"
> > > and "potential change of the binding".
> > 
> > Adding "glue-reset" is usable for devices having same parent clocks/resets as
> > usb3/ahci, and we can add the devices without patches.
> > 
> > However, if the device needs other parent clocks/resets for the same SoC,
> > we can't add "glue-reset" and we might add patches for the device as a result.
> > In this case, the "glue-reset" will become difficult to understand.
> 
> Then let's not bother with it.
> I made the suggestion without knowing the full picture.

This might confuse you without bigger picture.

> > > There is no real hardware like pro4-glue-reset.
> > > 
> > > I am guessing this is a part of syscon or something,
> > > but I cannot find any explanation in a bigger picture.
> > > 
> > > So, I cannot judge this further more.
> > 
> > Since it's hard to lead the best result,
> > currently I'd like to suggest the current compatibles,
> 
> That is fine with me.
> 
> > with "simple-reset" if necessary.
> 
> Not necessary.

Okay, I don't add it.

Thank you,

---
Best Regards,
Kunihiko Hayashi

Re: [PATCH 3/4] dt-bindings: reset: uniphier: Add AHCI core reset description

[Rob Herring]

On Fri,  9 Nov 2018 10:42:06 +0900, Kunihiko Hayashi wrote:
> Add compatible strings for reset control of AHCI core implemented in
> UniPhier SoCs. The reset control belongs to AHCI glue layer.
> 
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> ---
>  Documentation/devicetree/bindings/reset/uniphier-reset.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH 4/4] reset: uniphier-glue: Add AHCI reset control support in glue layer

[Kunihiko Hayashi]

Add a reset line included in AHCI glue layer to enable AHCI core
implemented in UniPhier SoCs.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/reset/reset-uniphier-glue.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/reset/reset-uniphier-glue.c b/drivers/reset/reset-uniphier-glue.c
index 1b8ea03..a45923f 100644
--- a/drivers/reset/reset-uniphier-glue.c
+++ b/drivers/reset/reset-uniphier-glue.c
@@ -152,6 +152,18 @@ static const struct of_device_id uniphier_glue_reset_match[] = {
 		.compatible = "socionext,uniphier-pxs3-usb3-reset",
 		.data = &uniphier_pxs2_data,
 	},
+	{
+		.compatible = "socionext,uniphier-pro4-ahci-reset",
+		.data = &uniphier_pro4_data,
+	},
+	{
+		.compatible = "socionext,uniphier-pxs2-ahci-reset",
+		.data = &uniphier_pxs2_data,
+	},
+	{
+		.compatible = "socionext,uniphier-pxs3-ahci-reset",
+		.data = &uniphier_pxs2_data,
+	},
 	{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, uniphier_glue_reset_match);
-- 
2.7.4

Re: [PATCH 0/4] reset: uniphier: Rename from USB3 reset to glue reset and add AHCI reset support

[Philipp Zabel]

On Fri, 2018-11-09 at 10:42 +0900, Kunihiko Hayashi wrote:
> This series renames the reset control of core reset included in USB3 glue
> layer with in the glue layer for generic peripherals to allow other devices
> to use it.
> 
> And this series adds support for the core reset included in AHCI glue layer.
> 
> Kunihiko Hayashi (4):
>   dt-bindings: reset: uniphier: Replace the expression of USB3 with
>     generic peripherals
>   reset: uniphier-usb3: Rename to reset-uniphier-glue
>   dt-bindings: reset: uniphier: Add AHCI core reset description
>   reset: uniphier-glue: Add AHCI reset control support in glue layer

Thank you, applied to reset/next.

regards
Philipp