[PATCH 0/9] Add Reset Controller support for Actions Semi Owl SoCs

[PATCH 0/9] Add Reset Controller support for Actions Semi Owl SoCs

[Manivannan Sadhasivam]

This patchset adds Reset Controller (RMU) support for Actions Semi
Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into
the clock subsystem in hardware. Hence, in software we integrate RMU
support into common clock driver inorder to maintain compatibility.

This patch series depends on the recently posted S700 clk series:
"[PATCH v7 0/5] Add clock driver for Actions S700 SoC". For the S700 clk
series, driver and bindings patches are applied through the clk tree.
But the DTS patches are not yet picked up by the platform maintainer,
Andreas.

Hence, Andreas is expected to pick the DTS patches in this series once
reviewed by the maintainers along with S700 clk DTS patches.

Because of the absence of the S500 SoC clk support, the reset controller
registration code is added to both S700 and S900 SoC clk drivers for now.
But once S500 clk support is added, the reset controller registration part
will be moved to Owl SoCs common clk code.

Thanks,
Mani

Manivannan Sadhasivam (9):
  clk: actions: Cache regmap info in private clock descriptor
  dt-bindings: clock: Add reset controller bindings for Actions Semi Owl
    SoCs
  dt-bindings: reset: Add binding constants for Actions Semi S700 RMU
  dt-bindings: reset: Add binding constants for Actions Semi S900 RMU
  arm64: dts: actions: Add Reset Controller support for S700 SoC
  arm64: dts: actions: Add Reset Controller support for S900 SoC
  clk: actions: Add Actions Semi Owl SoCs Reset Management Unit support
  clk: actions: Add Actions Semi S700 SoC Reset Management Unit support
  clk: actions: Add Actions Semi S900 SoC Reset Management Unit support

 .../bindings/clock/actions,owl-cmu.txt        |  2 +
 arch/arm64/boot/dts/actions/s700.dtsi         |  2 +
 arch/arm64/boot/dts/actions/s900.dtsi         |  2 +
 drivers/clk/actions/Kconfig                   |  1 +
 drivers/clk/actions/Makefile                  |  1 +
 drivers/clk/actions/owl-common.c              |  3 +-
 drivers/clk/actions/owl-common.h              |  5 +-
 drivers/clk/actions/owl-reset.c               | 72 ++++++++++++++++
 drivers/clk/actions/owl-reset.h               | 32 +++++++
 drivers/clk/actions/owl-s700.c                | 55 +++++++++++-
 drivers/clk/actions/owl-s900.c                | 86 ++++++++++++++++++-
 .../dt-bindings/reset/actions,s700-reset.h    | 34 ++++++++
 .../dt-bindings/reset/actions,s900-reset.h    | 65 ++++++++++++++
 13 files changed, 354 insertions(+), 6 deletions(-)
 create mode 100644 drivers/clk/actions/owl-reset.c
 create mode 100644 drivers/clk/actions/owl-reset.h
 create mode 100644 include/dt-bindings/reset/actions,s700-reset.h
 create mode 100644 include/dt-bindings/reset/actions,s900-reset.h

-- 
2.17.1

[PATCH 1/9] clk: actions: Cache regmap info in private clock descriptor

[Manivannan Sadhasivam]

In order to support the reset controller, regmap info needs to
be cached in the private clock descriptor, owl_clk_desc. Hence,
save that and also make the clock descriptor struct non const.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/clk/actions/owl-common.c | 3 ++-
 drivers/clk/actions/owl-common.h | 3 ++-
 drivers/clk/actions/owl-s700.c   | 4 ++--
 drivers/clk/actions/owl-s900.c   | 4 ++--
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/actions/owl-common.c b/drivers/clk/actions/owl-common.c
index 61c1071b5180..32dd29e0a37e 100644
--- a/drivers/clk/actions/owl-common.c
+++ b/drivers/clk/actions/owl-common.c
@@ -39,7 +39,7 @@ static void owl_clk_set_regmap(const struct owl_clk_desc *desc,
 }
 
 int owl_clk_regmap_init(struct platform_device *pdev,
-			 const struct owl_clk_desc *desc)
+			struct owl_clk_desc *desc)
 {
 	void __iomem *base;
 	struct regmap *regmap;
@@ -57,6 +57,7 @@ int owl_clk_regmap_init(struct platform_device *pdev,
 	}
 
 	owl_clk_set_regmap(desc, regmap);
+	desc->regmap = regmap;
 
 	return 0;
 }
diff --git a/drivers/clk/actions/owl-common.h b/drivers/clk/actions/owl-common.h
index 4fd726ec54a6..56f01f7774aa 100644
--- a/drivers/clk/actions/owl-common.h
+++ b/drivers/clk/actions/owl-common.h
@@ -26,6 +26,7 @@ struct owl_clk_desc {
 	struct owl_clk_common		**clks;
 	unsigned long			num_clks;
 	struct clk_hw_onecell_data	*hw_clks;
+	struct regmap			*regmap;
 };
 
 static inline struct owl_clk_common *
@@ -35,7 +36,7 @@ static inline struct owl_clk_common *
 }
 
 int owl_clk_regmap_init(struct platform_device *pdev,
-			 const struct owl_clk_desc *desc);
+			struct owl_clk_desc *desc);
 int owl_clk_probe(struct device *dev, struct clk_hw_onecell_data *hw_clks);
 
 #endif /* _OWL_COMMON_H_ */
diff --git a/drivers/clk/actions/owl-s700.c b/drivers/clk/actions/owl-s700.c
index 5e9531392ee5..e7cacd677275 100644
--- a/drivers/clk/actions/owl-s700.c
+++ b/drivers/clk/actions/owl-s700.c
@@ -569,7 +569,7 @@ static struct clk_hw_onecell_data s700_hw_clks = {
 		.num    = CLK_NR_CLKS,
 };
 
-static const struct owl_clk_desc s700_clk_desc = {
+static struct owl_clk_desc s700_clk_desc = {
 	.clks       = s700_clks,
 	.num_clks   = ARRAY_SIZE(s700_clks),
 
@@ -578,7 +578,7 @@ static const struct owl_clk_desc s700_clk_desc = {
 
 static int s700_clk_probe(struct platform_device *pdev)
 {
-	const struct owl_clk_desc *desc;
+	struct owl_clk_desc *desc;
 
 	desc = &s700_clk_desc;
 	owl_clk_regmap_init(pdev, desc);
diff --git a/drivers/clk/actions/owl-s900.c b/drivers/clk/actions/owl-s900.c
index 7f60ed6afe63..bb7ee872d316 100644
--- a/drivers/clk/actions/owl-s900.c
+++ b/drivers/clk/actions/owl-s900.c
@@ -684,7 +684,7 @@ static struct clk_hw_onecell_data s900_hw_clks = {
 	.num	= CLK_NR_CLKS,
 };
 
-static const struct owl_clk_desc s900_clk_desc = {
+static struct owl_clk_desc s900_clk_desc = {
 	.clks	    = s900_clks,
 	.num_clks   = ARRAY_SIZE(s900_clks),
 
@@ -693,7 +693,7 @@ static const struct owl_clk_desc s900_clk_desc = {
 
 static int s900_clk_probe(struct platform_device *pdev)
 {
-	const struct owl_clk_desc *desc;
+	struct owl_clk_desc *desc;
 
 	desc = &s900_clk_desc;
 	owl_clk_regmap_init(pdev, desc);
-- 
2.17.1

[PATCH 2/9] dt-bindings: clock: Add reset controller bindings for Actions Semi Owl SoCs

[Manivannan Sadhasivam]

Add Reset Controller bindings to clock bindings for Actions Semi Owl
SoCs, S700 and S900.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 Documentation/devicetree/bindings/clock/actions,owl-cmu.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt b/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt
index d1e60d297387..2ef86ae96df8 100644
--- a/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt
+++ b/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt
@@ -13,6 +13,7 @@ Required Properties:
   region.
 - clocks: Reference to the parent clocks ("hosc", "losc")
 - #clock-cells: should be 1.
+- #reset-cells: should be 1.
 
 Each clock is assigned an identifier, and client nodes can use this identifier
 to specify the clock which they consume.
@@ -36,6 +37,7 @@ Example: Clock Management Unit node:
                 reg = <0x0 0xe0160000 0x0 0x1000>;
                 clocks = <&hosc>, <&losc>;
                 #clock-cells = <1>;
+                #reset-cells = <1>;
         };
 
 Example: UART controller node that consumes clock generated by the clock
-- 
2.17.1

[PATCH 3/9] dt-bindings: reset: Add binding constants for Actions Semi S700 RMU

[Manivannan Sadhasivam]

Add device tree binding constants for Actions Semi S700 SoC Reset
Management Unit (RMU).

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 .../dt-bindings/reset/actions,s700-reset.h    | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 include/dt-bindings/reset/actions,s700-reset.h

diff --git a/include/dt-bindings/reset/actions,s700-reset.h b/include/dt-bindings/reset/actions,s700-reset.h
new file mode 100644
index 000000000000..5e3b16b8ef53
--- /dev/null
+++ b/include/dt-bindings/reset/actions,s700-reset.h
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: (GPL-2.0-or-later OR MIT)
+//
+// Device Tree binding constants for Actions Semi S700 Reset Management Unit
+//
+// Copyright (c) 2018 Linaro Ltd.
+
+#ifndef __DT_BINDINGS_ACTIONS_S700_RESET_H
+#define __DT_BINDINGS_ACTIONS_S700_RESET_H
+
+#define RESET_AUDIO				0
+#define RESET_CSI				1
+#define RESET_DE				2
+#define RESET_DSI				3
+#define RESET_GPIO				4
+#define RESET_I2C0				5
+#define RESET_I2C1				6
+#define RESET_I2C2				7
+#define RESET_I2C3				8
+#define RESET_KEY				9
+#define RESET_LCD0				10
+#define RESET_SI				11
+#define RESET_SPI0				12
+#define RESET_SPI1				13
+#define RESET_SPI2				14
+#define RESET_SPI3				15
+#define RESET_UART0				16
+#define RESET_UART1				17
+#define RESET_UART2				18
+#define RESET_UART3				19
+#define RESET_UART4				20
+#define RESET_UART5				21
+#define RESET_UART6				22
+
+#endif /* __DT_BINDINGS_ACTIONS_S700_RESET_H */
-- 
2.17.1

[PATCH 4/9] dt-bindings: reset: Add binding constants for Actions Semi S900 RMU

[Manivannan Sadhasivam]

Add device tree binding constants for Actions Semi S900 SoC Reset
Management Unit (RMU).

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 .../dt-bindings/reset/actions,s900-reset.h    | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 include/dt-bindings/reset/actions,s900-reset.h

diff --git a/include/dt-bindings/reset/actions,s900-reset.h b/include/dt-bindings/reset/actions,s900-reset.h
new file mode 100644
index 000000000000..42c19d02e43b
--- /dev/null
+++ b/include/dt-bindings/reset/actions,s900-reset.h
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: (GPL-2.0-or-later OR MIT)
+//
+// Device Tree binding constants for Actions Semi S900 Reset Management Unit
+//
+// Copyright (c) 2018 Linaro Ltd.
+
+#ifndef __DT_BINDINGS_ACTIONS_S900_RESET_H
+#define __DT_BINDINGS_ACTIONS_S900_RESET_H
+
+#define RESET_CHIPID				0
+#define RESET_CPU_SCNT				1
+#define RESET_SRAMI				2
+#define RESET_DDR_CTL_PHY			3
+#define RESET_DMAC				4
+#define RESET_GPIO				5
+#define RESET_BISP_AXI				6
+#define RESET_CSI0				7
+#define RESET_CSI1				8
+#define RESET_DE				9
+#define RESET_DSI				10
+#define RESET_GPU3D_PA				11
+#define RESET_GPU3D_PB				12
+#define RESET_HDE				13
+#define RESET_I2C0				14
+#define RESET_I2C1				15
+#define RESET_I2C2				16
+#define RESET_I2C3				17
+#define RESET_I2C4				18
+#define RESET_I2C5				19
+#define RESET_IMX				20
+#define RESET_NANDC0				21
+#define RESET_NANDC1				22
+#define RESET_SD0				23
+#define RESET_SD1				24
+#define RESET_SD2				25
+#define RESET_SD3				26
+#define RESET_SPI0				27
+#define RESET_SPI1				28
+#define RESET_SPI2				29
+#define RESET_SPI3				30
+#define RESET_UART0				31
+#define RESET_UART1				32
+#define RESET_UART2				33
+#define RESET_UART3				34
+#define RESET_UART4				35
+#define RESET_UART5				36
+#define RESET_UART6				37
+#define RESET_HDMI				38
+#define RESET_LVDS				39
+#define RESET_EDP				40
+#define RESET_USB2HUB				41
+#define RESET_USB2HSIC				42
+#define RESET_USB3				43
+#define RESET_PCM1				44
+#define RESET_AUDIO				45
+#define RESET_PCM0				46
+#define RESET_SE				47
+#define RESET_GIC				48
+#define RESET_DDR_CTL_PHY_AXI			49
+#define RESET_CMU_DDR				50
+#define RESET_DMM				51
+#define RESET_HDCP2TX				52
+#define RESET_ETHERNET				53
+
+#endif /* __DT_BINDINGS_ACTIONS_S900_RESET_H */
-- 
2.17.1

[PATCH 5/9] arm64: dts: actions: Add Reset Controller support for S700 SoC

[Manivannan Sadhasivam]

Add reset controller property and bindings header for the
Actions Semi S700 SoC DTS.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/actions/s700.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/actions/s700.dtsi b/arch/arm64/boot/dts/actions/s700.dtsi
index 59d29e4ca404..db4c544d5311 100644
--- a/arch/arm64/boot/dts/actions/s700.dtsi
+++ b/arch/arm64/boot/dts/actions/s700.dtsi
@@ -5,6 +5,7 @@
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/actions,s700-cmu.h>
+#include <dt-bindings/reset/actions,s900-reset.h>
 
 / {
 	compatible = "actions,s700";
@@ -165,6 +166,7 @@
 			reg = <0x0 0xe0168000 0x0 0x1000>;
 			clocks = <&hosc>, <&losc>;
 			#clock-cells = <1>;
+			#reset-cells = <1>;
 		};
 
 		sps: power-controller@e01b0100 {
-- 
2.17.1

[PATCH 6/9] arm64: dts: actions: Add Reset Controller support for S900 SoC

[Manivannan Sadhasivam]

Add reset controller property and bindings header for the
Actions Semi S900 SoC DTS.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/actions/s900.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/actions/s900.dtsi b/arch/arm64/boot/dts/actions/s900.dtsi
index aa3a49b0d646..4fbb39fd7971 100644
--- a/arch/arm64/boot/dts/actions/s900.dtsi
+++ b/arch/arm64/boot/dts/actions/s900.dtsi
@@ -6,6 +6,7 @@
 
 #include <dt-bindings/clock/actions,s900-cmu.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/reset/actions,s900-reset.h>
 
 / {
 	compatible = "actions,s900";
@@ -172,6 +173,7 @@
 			reg = <0x0 0xe0160000 0x0 0x1000>;
 			clocks = <&hosc>, <&losc>;
 			#clock-cells = <1>;
+			#reset-cells = <1>;
 		};
 
 		pinctrl: pinctrl@e01b0000 {
-- 
2.17.1

[PATCH 7/9] clk: actions: Add Actions Semi Owl SoCs Reset Management Unit support

[Manivannan Sadhasivam]

Add Reset Management Unit (RMU) support for Actions Semi Owl SoCs.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/clk/actions/Kconfig      |  1 +
 drivers/clk/actions/Makefile     |  1 +
 drivers/clk/actions/owl-common.h |  2 +
 drivers/clk/actions/owl-reset.c  | 72 ++++++++++++++++++++++++++++++++
 drivers/clk/actions/owl-reset.h  | 32 ++++++++++++++
 5 files changed, 108 insertions(+)
 create mode 100644 drivers/clk/actions/owl-reset.c
 create mode 100644 drivers/clk/actions/owl-reset.h

diff --git a/drivers/clk/actions/Kconfig b/drivers/clk/actions/Kconfig
index dc38c85a4833..04f0a6355726 100644
--- a/drivers/clk/actions/Kconfig
+++ b/drivers/clk/actions/Kconfig
@@ -2,6 +2,7 @@ config CLK_ACTIONS
 	bool "Clock driver for Actions Semi SoCs"
 	depends on ARCH_ACTIONS || COMPILE_TEST
 	select REGMAP_MMIO
+	select RESET_CONTROLLER
 	default ARCH_ACTIONS
 
 if CLK_ACTIONS
diff --git a/drivers/clk/actions/Makefile b/drivers/clk/actions/Makefile
index 78c17d56f991..ccfdf9781cef 100644
--- a/drivers/clk/actions/Makefile
+++ b/drivers/clk/actions/Makefile
@@ -7,6 +7,7 @@ clk-owl-y			+= owl-divider.o
 clk-owl-y			+= owl-factor.o
 clk-owl-y			+= owl-composite.o
 clk-owl-y			+= owl-pll.o
+clk-owl-y			+= owl-reset.o
 
 # SoC support
 obj-$(CONFIG_CLK_OWL_S700)	+= owl-s700.o
diff --git a/drivers/clk/actions/owl-common.h b/drivers/clk/actions/owl-common.h
index 56f01f7774aa..4dc7f286831f 100644
--- a/drivers/clk/actions/owl-common.h
+++ b/drivers/clk/actions/owl-common.h
@@ -26,6 +26,8 @@ struct owl_clk_desc {
 	struct owl_clk_common		**clks;
 	unsigned long			num_clks;
 	struct clk_hw_onecell_data	*hw_clks;
+	struct owl_reset_map		*resets;
+	unsigned long			num_resets;
 	struct regmap			*regmap;
 };
 
diff --git a/drivers/clk/actions/owl-reset.c b/drivers/clk/actions/owl-reset.c
new file mode 100644
index 000000000000..91b161cc68de
--- /dev/null
+++ b/drivers/clk/actions/owl-reset.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+//
+// Actions Semi Owl SoCs Reset Management Unit driver
+//
+// Copyright (c) 2018 Linaro Ltd.
+// Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+
+#include "owl-reset.h"
+
+static int owl_reset_assert(struct reset_controller_dev *rcdev,
+			    unsigned long id)
+{
+	struct owl_reset *reset = to_owl_reset(rcdev);
+	const struct owl_reset_map *map = &reset->reset_map[id];
+	u32 reg;
+
+	regmap_read(reset->regmap, map->reg, &reg);
+	regmap_write(reset->regmap, map->reg, reg & ~map->bit);
+
+	return 0;
+}
+
+static int owl_reset_deassert(struct reset_controller_dev *rcdev,
+			      unsigned long id)
+{
+	struct owl_reset *reset = to_owl_reset(rcdev);
+	const struct owl_reset_map *map = &reset->reset_map[id];
+	u32 reg;
+
+	regmap_read(reset->regmap, map->reg, &reg);
+	regmap_write(reset->regmap, map->reg, reg | map->bit);
+
+	return 0;
+}
+
+static int owl_reset_reset(struct reset_controller_dev *rcdev,
+			   unsigned long id)
+{
+	owl_reset_assert(rcdev, id);
+	udelay(1);
+	owl_reset_deassert(rcdev, id);
+
+	return 0;
+}
+
+static int owl_reset_status(struct reset_controller_dev *rcdev,
+			    unsigned long id)
+{
+	struct owl_reset *reset = to_owl_reset(rcdev);
+	const struct owl_reset_map *map = &reset->reset_map[id];
+	u32 reg;
+
+	regmap_read(reset->regmap, map->reg, &reg);
+
+	/*
+	 * The reset control API expects 0 if reset is not asserted,
+	 * which is the opposite of what our hardware uses.
+	 */
+	return !(map->bit & reg);
+}
+
+const struct reset_control_ops owl_reset_ops = {
+	.assert		= owl_reset_assert,
+	.deassert	= owl_reset_deassert,
+	.reset		= owl_reset_reset,
+	.status		= owl_reset_status,
+};
diff --git a/drivers/clk/actions/owl-reset.h b/drivers/clk/actions/owl-reset.h
new file mode 100644
index 000000000000..1a5e987ba99b
--- /dev/null
+++ b/drivers/clk/actions/owl-reset.h
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+//
+// Actions Semi Owl SoCs Reset Management Unit driver
+//
+// Copyright (c) 2018 Linaro Ltd.
+// Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
+
+#ifndef _OWL_RESET_H_
+#define _OWL_RESET_H_
+
+#include <linux/reset-controller.h>
+#include <linux/spinlock.h>
+
+struct owl_reset_map {
+	u16	reg;
+	u32	bit;
+};
+
+struct owl_reset {
+	struct reset_controller_dev	rcdev;
+	struct owl_reset_map		*reset_map;
+	struct regmap			*regmap;
+};
+
+static inline struct owl_reset *to_owl_reset(struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct owl_reset, rcdev);
+}
+
+extern const struct reset_control_ops owl_reset_ops;
+
+#endif /* _OWL_RESET_H_ */
-- 
2.17.1

[PATCH 8/9] clk: actions: Add Actions Semi S700 SoC Reset Management Unit support

[Manivannan Sadhasivam]

Add Reset Management Unit (RMU) support for Actions Semi S700 SoC.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/clk/actions/owl-s700.c | 51 ++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/clk/actions/owl-s700.c b/drivers/clk/actions/owl-s700.c
index e7cacd677275..d9c0c7870135 100644
--- a/drivers/clk/actions/owl-s700.c
+++ b/drivers/clk/actions/owl-s700.c
@@ -20,8 +20,10 @@
 #include "owl-gate.h"
 #include "owl-mux.h"
 #include "owl-pll.h"
+#include "owl-reset.h"
 
 #include <dt-bindings/clock/actions,s700-cmu.h>
+#include <dt-bindings/reset/actions,s700-reset.h>
 
 #define CMU_COREPLL		(0x0000)
 #define CMU_DEVPLL		(0x0004)
@@ -569,20 +571,69 @@ static struct clk_hw_onecell_data s700_hw_clks = {
 		.num    = CLK_NR_CLKS,
 };
 
+static struct owl_reset_map s700_resets[] = {
+	[RESET_DE]	= { CMU_DEVRST0, BIT(0) },
+	[RESET_LCD0]	= { CMU_DEVRST0, BIT(1) },
+	[RESET_DSI]	= { CMU_DEVRST0, BIT(2) },
+	[RESET_CSI]	= { CMU_DEVRST0, BIT(13) },
+	[RESET_SI]	= { CMU_DEVRST0, BIT(14) },
+	[RESET_I2C0]	= { CMU_DEVRST1, BIT(0) },
+	[RESET_I2C1]	= { CMU_DEVRST1, BIT(1) },
+	[RESET_I2C2]	= { CMU_DEVRST1, BIT(2) },
+	[RESET_I2C3]	= { CMU_DEVRST1, BIT(3) },
+	[RESET_SPI0]	= { CMU_DEVRST1, BIT(4) },
+	[RESET_SPI1]	= { CMU_DEVRST1, BIT(5) },
+	[RESET_SPI2]	= { CMU_DEVRST1, BIT(6) },
+	[RESET_SPI3]	= { CMU_DEVRST1, BIT(7) },
+	[RESET_UART0]	= { CMU_DEVRST1, BIT(8) },
+	[RESET_UART1]	= { CMU_DEVRST1, BIT(9) },
+	[RESET_UART2]	= { CMU_DEVRST1, BIT(10) },
+	[RESET_UART3]	= { CMU_DEVRST1, BIT(11) },
+	[RESET_UART4]	= { CMU_DEVRST1, BIT(12) },
+	[RESET_UART5]	= { CMU_DEVRST1, BIT(13) },
+	[RESET_UART6]	= { CMU_DEVRST1, BIT(14) },
+	[RESET_KEY]	= { CMU_DEVRST1, BIT(24) },
+	[RESET_GPIO]	= { CMU_DEVRST1, BIT(25) },
+	[RESET_AUDIO]	= { CMU_DEVRST1, BIT(29) },
+};
+
 static struct owl_clk_desc s700_clk_desc = {
 	.clks       = s700_clks,
 	.num_clks   = ARRAY_SIZE(s700_clks),
 
 	.hw_clks    = &s700_hw_clks,
+
+	.resets     = s700_resets,
+	.num_resets = ARRAY_SIZE(s700_resets),
 };
 
 static int s700_clk_probe(struct platform_device *pdev)
 {
 	struct owl_clk_desc *desc;
+	struct owl_reset *reset;
+	int ret;
 
 	desc = &s700_clk_desc;
 	owl_clk_regmap_init(pdev, desc);
 
+	/*
+	 * FIXME: Reset controller registration should be moved to
+	 * common code, once all SoCs of Owl family supports it.
+	 */
+	reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
+	if (!reset)
+		return -ENOMEM;
+
+	reset->rcdev.of_node = pdev->dev.of_node;
+	reset->rcdev.ops = &owl_reset_ops;
+	reset->rcdev.nr_resets = desc->num_resets;
+	reset->reset_map = desc->resets;
+	reset->regmap = desc->regmap;
+
+	ret = devm_reset_controller_register(&pdev->dev, &reset->rcdev);
+	if (ret)
+		dev_err(&pdev->dev, "Failed to register reset controller\n");
+
 	return owl_clk_probe(&pdev->dev, desc->hw_clks);
 }
 
-- 
2.17.1

[PATCH 9/9] clk: actions: Add Actions Semi S900 SoC Reset Management Unit support

[Manivannan Sadhasivam]

Add Reset Management Unit (RMU) support for Actions Semi S900 SoC.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/clk/actions/owl-s900.c | 82 ++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/drivers/clk/actions/owl-s900.c b/drivers/clk/actions/owl-s900.c
index bb7ee872d316..4d38b1265cc3 100644
--- a/drivers/clk/actions/owl-s900.c
+++ b/drivers/clk/actions/owl-s900.c
@@ -19,8 +19,10 @@
 #include "owl-gate.h"
 #include "owl-mux.h"
 #include "owl-pll.h"
+#include "owl-reset.h"
 
 #include <dt-bindings/clock/actions,s900-cmu.h>
+#include <dt-bindings/reset/actions,s900-reset.h>
 
 #define CMU_COREPLL		(0x0000)
 #define CMU_DEVPLL		(0x0004)
@@ -684,20 +686,100 @@ static struct clk_hw_onecell_data s900_hw_clks = {
 	.num	= CLK_NR_CLKS,
 };
 
+static struct owl_reset_map s900_resets[] = {
+	[RESET_DMAC]		= { CMU_DEVRST0, BIT(0) },
+	[RESET_SRAMI]		= { CMU_DEVRST0, BIT(1) },
+	[RESET_DDR_CTL_PHY]	= { CMU_DEVRST0, BIT(2) },
+	[RESET_NANDC0]		= { CMU_DEVRST0, BIT(3) },
+	[RESET_SD0]		= { CMU_DEVRST0, BIT(4) },
+	[RESET_SD1]		= { CMU_DEVRST0, BIT(5) },
+	[RESET_PCM1]		= { CMU_DEVRST0, BIT(6) },
+	[RESET_DE]		= { CMU_DEVRST0, BIT(7) },
+	[RESET_LVDS]		= { CMU_DEVRST0, BIT(8) },
+	[RESET_SD2]		= { CMU_DEVRST0, BIT(9) },
+	[RESET_DSI]		= { CMU_DEVRST0, BIT(10) },
+	[RESET_CSI0]		= { CMU_DEVRST0, BIT(11) },
+	[RESET_BISP_AXI]	= { CMU_DEVRST0, BIT(12) },
+	[RESET_CSI1]		= { CMU_DEVRST0, BIT(13) },
+	[RESET_GPIO]		= { CMU_DEVRST0, BIT(15) },
+	[RESET_EDP]		= { CMU_DEVRST0, BIT(16) },
+	[RESET_AUDIO]		= { CMU_DEVRST0, BIT(17) },
+	[RESET_PCM0]		= { CMU_DEVRST0, BIT(18) },
+	[RESET_HDE]		= { CMU_DEVRST0, BIT(21) },
+	[RESET_GPU3D_PA]	= { CMU_DEVRST0, BIT(22) },
+	[RESET_IMX]		= { CMU_DEVRST0, BIT(23) },
+	[RESET_SE]		= { CMU_DEVRST0, BIT(24) },
+	[RESET_NANDC1]		= { CMU_DEVRST0, BIT(25) },
+	[RESET_SD3]		= { CMU_DEVRST0, BIT(26) },
+	[RESET_GIC]		= { CMU_DEVRST0, BIT(27) },
+	[RESET_GPU3D_PB]	= { CMU_DEVRST0, BIT(28) },
+	[RESET_DDR_CTL_PHY_AXI]	= { CMU_DEVRST0, BIT(29) },
+	[RESET_CMU_DDR]		= { CMU_DEVRST0, BIT(30) },
+	[RESET_DMM]		= { CMU_DEVRST0, BIT(31) },
+	[RESET_USB2HUB]		= { CMU_DEVRST1, BIT(0) },
+	[RESET_USB2HSIC]	= { CMU_DEVRST1, BIT(1) },
+	[RESET_HDMI]		= { CMU_DEVRST1, BIT(2) },
+	[RESET_HDCP2TX]		= { CMU_DEVRST1, BIT(3) },
+	[RESET_UART6]		= { CMU_DEVRST1, BIT(4) },
+	[RESET_UART0]		= { CMU_DEVRST1, BIT(5) },
+	[RESET_UART1]		= { CMU_DEVRST1, BIT(6) },
+	[RESET_UART2]		= { CMU_DEVRST1, BIT(7) },
+	[RESET_SPI0]		= { CMU_DEVRST1, BIT(8) },
+	[RESET_SPI1]		= { CMU_DEVRST1, BIT(9) },
+	[RESET_SPI2]		= { CMU_DEVRST1, BIT(10) },
+	[RESET_SPI3]		= { CMU_DEVRST1, BIT(11) },
+	[RESET_I2C0]		= { CMU_DEVRST1, BIT(12) },
+	[RESET_I2C1]		= { CMU_DEVRST1, BIT(13) },
+	[RESET_USB3]		= { CMU_DEVRST1, BIT(14) },
+	[RESET_UART3]		= { CMU_DEVRST1, BIT(15) },
+	[RESET_UART4]		= { CMU_DEVRST1, BIT(16) },
+	[RESET_UART5]		= { CMU_DEVRST1, BIT(17) },
+	[RESET_I2C2]		= { CMU_DEVRST1, BIT(18) },
+	[RESET_I2C3]		= { CMU_DEVRST1, BIT(19) },
+	[RESET_ETHERNET]	= { CMU_DEVRST1, BIT(20) },
+	[RESET_CHIPID]		= { CMU_DEVRST1, BIT(21) },
+	[RESET_I2C4]		= { CMU_DEVRST1, BIT(22) },
+	[RESET_I2C5]		= { CMU_DEVRST1, BIT(23) },
+	[RESET_CPU_SCNT]	= { CMU_DEVRST1, BIT(30) }
+};
+
 static struct owl_clk_desc s900_clk_desc = {
 	.clks	    = s900_clks,
 	.num_clks   = ARRAY_SIZE(s900_clks),
 
 	.hw_clks    = &s900_hw_clks,
+
+	.resets     = s900_resets,
+	.num_resets = ARRAY_SIZE(s900_resets),
 };
 
 static int s900_clk_probe(struct platform_device *pdev)
 {
 	struct owl_clk_desc *desc;
+	struct owl_reset *reset;
+	int ret;
 
 	desc = &s900_clk_desc;
 	owl_clk_regmap_init(pdev, desc);
 
+	/*
+	 * FIXME: Reset controller registration should be moved to
+	 * common code, once all SoCs of Owl family supports it.
+	 */
+	reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
+	if (!reset)
+		return -ENOMEM;
+
+	reset->rcdev.of_node = pdev->dev.of_node;
+	reset->rcdev.ops = &owl_reset_ops;
+	reset->rcdev.nr_resets = desc->num_resets;
+	reset->reset_map = desc->resets;
+	reset->regmap = desc->regmap;
+
+	ret = devm_reset_controller_register(&pdev->dev, &reset->rcdev);
+	if (ret)
+		dev_err(&pdev->dev, "Failed to register reset controller\n");
+
 	return owl_clk_probe(&pdev->dev, desc->hw_clks);
 }
 
-- 
2.17.1

Re: [PATCH 5/9] arm64: dts: actions: Add Reset Controller support for S700 SoC

[Parthiban Nallathambi]

Hi Mani,

On 07/27/2018 08:45 PM, Manivannan Sadhasivam wrote:
> Add reset controller property and bindings header for the
> Actions Semi S700 SoC DTS.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  arch/arm64/boot/dts/actions/s700.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/actions/s700.dtsi b/arch/arm64/boot/dts/actions/s700.dtsi
> index 59d29e4ca404..db4c544d5311 100644
> --- a/arch/arm64/boot/dts/actions/s700.dtsi
> +++ b/arch/arm64/boot/dts/actions/s700.dtsi
> @@ -5,6 +5,7 @@
>  
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/clock/actions,s700-cmu.h>
> +#include <dt-bindings/reset/actions,s900-reset.h>

Typo here, this should be s700-reset.h

>  
>  / {
>  	compatible = "actions,s700";
> @@ -165,6 +166,7 @@
>  			reg = <0x0 0xe0168000 0x0 0x1000>;
>  			clocks = <&hosc>, <&losc>;
>  			#clock-cells = <1>;
> +			#reset-cells = <1>;
>  		};
>  
>  		sps: power-controller@e01b0100 {
> 

-- 
Thanks,
Parthiban N

Re: [PATCH 7/9] clk: actions: Add Actions Semi Owl SoCs Reset Management Unit support

[Philipp Zabel]

Hi Manivannan,

Thank you for the patch, a few small comments below:

On Sat, 2018-07-28 at 00:15 +0530, Manivannan Sadhasivam wrote:
> Add Reset Management Unit (RMU) support for Actions Semi Owl SoCs.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/clk/actions/Kconfig      |  1 +
>  drivers/clk/actions/Makefile     |  1 +
>  drivers/clk/actions/owl-common.h |  2 +
>  drivers/clk/actions/owl-reset.c  | 72 ++++++++++++++++++++++++++++++++
>  drivers/clk/actions/owl-reset.h  | 32 ++++++++++++++
>  5 files changed, 108 insertions(+)
>  create mode 100644 drivers/clk/actions/owl-reset.c
>  create mode 100644 drivers/clk/actions/owl-reset.h
> 
> diff --git a/drivers/clk/actions/Kconfig b/drivers/clk/actions/Kconfig
> index dc38c85a4833..04f0a6355726 100644
> --- a/drivers/clk/actions/Kconfig
> +++ b/drivers/clk/actions/Kconfig
> @@ -2,6 +2,7 @@ config CLK_ACTIONS
>  	bool "Clock driver for Actions Semi SoCs"
>  	depends on ARCH_ACTIONS || COMPILE_TEST
>  	select REGMAP_MMIO
> +	select RESET_CONTROLLER
>  	default ARCH_ACTIONS
>  
>  if CLK_ACTIONS
> diff --git a/drivers/clk/actions/Makefile b/drivers/clk/actions/Makefile
> index 78c17d56f991..ccfdf9781cef 100644
> --- a/drivers/clk/actions/Makefile
> +++ b/drivers/clk/actions/Makefile
> @@ -7,6 +7,7 @@ clk-owl-y			+= owl-divider.o
>  clk-owl-y			+= owl-factor.o
>  clk-owl-y			+= owl-composite.o
>  clk-owl-y			+= owl-pll.o
> +clk-owl-y			+= owl-reset.o
>  
>  # SoC support
>  obj-$(CONFIG_CLK_OWL_S700)	+= owl-s700.o
> diff --git a/drivers/clk/actions/owl-common.h b/drivers/clk/actions/owl-common.h
> index 56f01f7774aa..4dc7f286831f 100644
> --- a/drivers/clk/actions/owl-common.h
> +++ b/drivers/clk/actions/owl-common.h
> @@ -26,6 +26,8 @@ struct owl_clk_desc {
>  	struct owl_clk_common		**clks;
>  	unsigned long			num_clks;
>  	struct clk_hw_onecell_data	*hw_clks;
> +	struct owl_reset_map		*resets;

Could this be:
	const struct owl_reset_map	*resets;
?

> +	unsigned long			num_resets;
>  	struct regmap			*regmap;
>  };
>  
> diff --git a/drivers/clk/actions/owl-reset.c b/drivers/clk/actions/owl-reset.c
> new file mode 100644
> index 000000000000..91b161cc68de
> --- /dev/null
> +++ b/drivers/clk/actions/owl-reset.c
> @@ -0,0 +1,72 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +//
> +// Actions Semi Owl SoCs Reset Management Unit driver
> +//
> +// Copyright (c) 2018 Linaro Ltd.
> +// Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>

This seems unnecessary, since register access is done via regmap.

> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>
> +
> +#include "owl-reset.h"
> +
> +static int owl_reset_assert(struct reset_controller_dev *rcdev,
> +			    unsigned long id)
> +{
> +	struct owl_reset *reset = to_owl_reset(rcdev);
> +	const struct owl_reset_map *map = &reset->reset_map[id];
> +	u32 reg;
> +
> +	regmap_read(reset->regmap, map->reg, &reg);
> +	regmap_write(reset->regmap, map->reg, reg & ~map->bit);

This read-modify-write sequence needs locking against concurrent
register access. Better use regmap_update_bits():

	return regmap_update_bits(reset->regmap, map->reg, map->bit, 0);

> +
> +	return 0;
> +}
> +
> +static int owl_reset_deassert(struct reset_controller_dev *rcdev,
> +			      unsigned long id)
> +{
> +	struct owl_reset *reset = to_owl_reset(rcdev);
> +	const struct owl_reset_map *map = &reset->reset_map[id];
> +	u32 reg;
> +
> +	regmap_read(reset->regmap, map->reg, &reg);
> +	regmap_write(reset->regmap, map->reg, reg | map->bit);

Better:

	return regmap_update_bits(reset->regmap, map->reg, map->bit, map->bit);

> +
> +	return 0;
> +}
> +
> +static int owl_reset_reset(struct reset_controller_dev *rcdev,
> +			   unsigned long id)
> +{
> +	owl_reset_assert(rcdev, id);
> +	udelay(1);

Is the delay valid for all IP cores on all SoCs variants?

> +	owl_reset_deassert(rcdev, id);
> +
> +	return 0;
> +}
> +
> +static int owl_reset_status(struct reset_controller_dev *rcdev,
> +			    unsigned long id)
> +{
> +	struct owl_reset *reset = to_owl_reset(rcdev);
> +	const struct owl_reset_map *map = &reset->reset_map[id];
> +	u32 reg;
> +
> +	regmap_read(reset->regmap, map->reg, &reg);

If this could return an error, better report it.

> +
> +	/*
> +	 * The reset control API expects 0 if reset is not asserted,
> +	 * which is the opposite of what our hardware uses.
> +	 */
> +	return !(map->bit & reg);
> +}
> +
> +const struct reset_control_ops owl_reset_ops = {
> +	.assert		= owl_reset_assert,
> +	.deassert	= owl_reset_deassert,
> +	.reset		= owl_reset_reset,
> +	.status		= owl_reset_status,
> +};
> diff --git a/drivers/clk/actions/owl-reset.h b/drivers/clk/actions/owl-reset.h
> new file mode 100644
> index 000000000000..1a5e987ba99b
> --- /dev/null
> +++ b/drivers/clk/actions/owl-reset.h
> @@ -0,0 +1,32 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +//
> +// Actions Semi Owl SoCs Reset Management Unit driver
> +//
> +// Copyright (c) 2018 Linaro Ltd.
> +// Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> +
> +#ifndef _OWL_RESET_H_
> +#define _OWL_RESET_H_
> +
> +#include <linux/reset-controller.h>
> +#include <linux/spinlock.h>

spinlock?

> +
> +struct owl_reset_map {
> +	u16	reg;

Note that this will be aligned to 32-bits. If the intention was to save
space, consider storing an u8 bit index instead of the mask.

> +	u32	bit;
> +};
> +
> +struct owl_reset {
> +	struct reset_controller_dev	rcdev;
> +	struct owl_reset_map		*reset_map;

Could this be
	const struct owl_reset_map	*reset_map;
?

> +	struct regmap			*regmap;
> +};
> +
> +static inline struct owl_reset *to_owl_reset(struct reset_controller_dev *rcdev)
> +{
> +	return container_of(rcdev, struct owl_reset, rcdev);
> +}
> +
> +extern const struct reset_control_ops owl_reset_ops;
> +
> +#endif /* _OWL_RESET_H_ */

regards
Philipp

Re: [PATCH 0/9] Add Reset Controller support for Actions Semi Owl SoCs

[Andreas Färber]

Hi Mani,

Am 27.07.2018 um 20:45 schrieb Manivannan Sadhasivam:
> This patchset adds Reset Controller (RMU) support for Actions Semi
> Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into
> the clock subsystem in hardware. Hence, in software we integrate RMU
> support into common clock driver inorder to maintain compatibility.

Can this not be placed into drivers/reset/ by using mfd-simple with a
sub-node in DT?

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 8/9] clk: actions: Add Actions Semi S700 SoC Reset Management Unit support

[Philipp Zabel]

On Sat, 2018-07-28 at 00:15 +0530, Manivannan Sadhasivam wrote:
> Add Reset Management Unit (RMU) support for Actions Semi S700 SoC.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/clk/actions/owl-s700.c | 51 ++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/clk/actions/owl-s700.c b/drivers/clk/actions/owl-s700.c
> index e7cacd677275..d9c0c7870135 100644
> --- a/drivers/clk/actions/owl-s700.c
> +++ b/drivers/clk/actions/owl-s700.c
> @@ -20,8 +20,10 @@
>  #include "owl-gate.h"
>  #include "owl-mux.h"
>  #include "owl-pll.h"
> +#include "owl-reset.h"
>  
>  #include <dt-bindings/clock/actions,s700-cmu.h>
> +#include <dt-bindings/reset/actions,s700-reset.h>
>  
>  #define CMU_COREPLL		(0x0000)
>  #define CMU_DEVPLL		(0x0004)
> @@ -569,20 +571,69 @@ static struct clk_hw_onecell_data s700_hw_clks = {
>  		.num    = CLK_NR_CLKS,
>  };
>  
> +static struct owl_reset_map s700_resets[] = {

This could be
static const struct owl_reset_map s700_resets[] = {

> +	[RESET_DE]	= { CMU_DEVRST0, BIT(0) },
> +	[RESET_LCD0]	= { CMU_DEVRST0, BIT(1) },
> +	[RESET_DSI]	= { CMU_DEVRST0, BIT(2) },
> +	[RESET_CSI]	= { CMU_DEVRST0, BIT(13) },
> +	[RESET_SI]	= { CMU_DEVRST0, BIT(14) },
> +	[RESET_I2C0]	= { CMU_DEVRST1, BIT(0) },
> +	[RESET_I2C1]	= { CMU_DEVRST1, BIT(1) },
> +	[RESET_I2C2]	= { CMU_DEVRST1, BIT(2) },
> +	[RESET_I2C3]	= { CMU_DEVRST1, BIT(3) },
> +	[RESET_SPI0]	= { CMU_DEVRST1, BIT(4) },
> +	[RESET_SPI1]	= { CMU_DEVRST1, BIT(5) },
> +	[RESET_SPI2]	= { CMU_DEVRST1, BIT(6) },
> +	[RESET_SPI3]	= { CMU_DEVRST1, BIT(7) },
> +	[RESET_UART0]	= { CMU_DEVRST1, BIT(8) },
> +	[RESET_UART1]	= { CMU_DEVRST1, BIT(9) },
> +	[RESET_UART2]	= { CMU_DEVRST1, BIT(10) },
> +	[RESET_UART3]	= { CMU_DEVRST1, BIT(11) },
> +	[RESET_UART4]	= { CMU_DEVRST1, BIT(12) },
> +	[RESET_UART5]	= { CMU_DEVRST1, BIT(13) },
> +	[RESET_UART6]	= { CMU_DEVRST1, BIT(14) },
> +	[RESET_KEY]	= { CMU_DEVRST1, BIT(24) },
> +	[RESET_GPIO]	= { CMU_DEVRST1, BIT(25) },
> +	[RESET_AUDIO]	= { CMU_DEVRST1, BIT(29) },
> +};
> +
>  static struct owl_clk_desc s700_clk_desc = {
>  	.clks       = s700_clks,
>  	.num_clks   = ARRAY_SIZE(s700_clks),
>  
>  	.hw_clks    = &s700_hw_clks,
> +
> +	.resets     = s700_resets,
> +	.num_resets = ARRAY_SIZE(s700_resets),
>  };
>  
>  static int s700_clk_probe(struct platform_device *pdev)
>  {
>  	struct owl_clk_desc *desc;
> +	struct owl_reset *reset;
> +	int ret;
>  
>  	desc = &s700_clk_desc;
>  	owl_clk_regmap_init(pdev, desc);
>  
> +	/*
> +	 * FIXME: Reset controller registration should be moved to
> +	 * common code, once all SoCs of Owl family supports it.
> +	 */
> +	reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
> +	if (!reset)
> +		return -ENOMEM;
> +
> +	reset->rcdev.of_node = pdev->dev.of_node;
> +	reset->rcdev.ops = &owl_reset_ops;
> +	reset->rcdev.nr_resets = desc->num_resets;
> +	reset->reset_map = desc->resets;
> +	reset->regmap = desc->regmap;
> +
> +	ret = devm_reset_controller_register(&pdev->dev, &reset->rcdev);
> +	if (ret)
> +		dev_err(&pdev->dev, "Failed to register reset controller\n");
> +
>  	return owl_clk_probe(&pdev->dev, desc->hw_clks);
>  }

regards
Philipp

Re: [PATCH 0/9] Add Reset Controller support for Actions Semi Owl SoCs

[Manivannan Sadhasivam]

Hi Andreas,

On Mon, Jul 30, 2018 at 12:26:07PM +0200, Andreas Färber wrote:
> Hi Mani,
> 
> Am 27.07.2018 um 20:45 schrieb Manivannan Sadhasivam:
> > This patchset adds Reset Controller (RMU) support for Actions Semi
> > Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into
> > the clock subsystem in hardware. Hence, in software we integrate RMU
> > support into common clock driver inorder to maintain compatibility.
> 
> Can this not be placed into drivers/reset/ by using mfd-simple with a
> sub-node in DT?
> 

Actually I was not sure where to place this reset controller driver. When I
looked into other similar ones such as sunxi, they just integrated into the
clk subsystem. So I just chose that path. But yeah, this is hacky!

But this RMU is not MFD by any means. Since the CMU (Clock) and RMU (Reset)
are two separate IPs inside SoC, we shouldn't describe it as a MFD driver. Since
RMU has only 2 registers, the HW designers decided to use up the CMU memory
map. So, maybe syscon would be best option I think. What is your opinion?

Even if we go for syscon, we should place the reset driver within clk
framework as I can see other SoCs like Mediatek are doing the same. But again
I'm not sure!

Thanks,
Mani

> Regards,
> Andreas
> 
> -- 
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)

Re: [PATCH 0/9] Add Reset Controller support for Actions Semi Owl SoCs

[Philipp Zabel]

On Mon, 2018-07-30 at 20:41 +0530, Manivannan Sadhasivam wrote:
> Hi Andreas,
> 
> On Mon, Jul 30, 2018 at 12:26:07PM +0200, Andreas Färber wrote:
> > Hi Mani,
> > 
> > Am 27.07.2018 um 20:45 schrieb Manivannan Sadhasivam:
> > > This patchset adds Reset Controller (RMU) support for Actions Semi
> > > Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into
> > > the clock subsystem in hardware. Hence, in software we integrate RMU
> > > support into common clock driver inorder to maintain compatibility.
> > 
> > Can this not be placed into drivers/reset/ by using mfd-simple with a
> > sub-node in DT?
> > 
> 
> Actually I was not sure where to place this reset controller driver. When I
> looked into other similar ones such as sunxi, they just integrated into the
> clk subsystem. So I just chose that path. But yeah, this is hacky!
> 
> But this RMU is not MFD by any means. Since the CMU (Clock) and RMU (Reset)
> are two separate IPs inside SoC, we shouldn't describe it as a MFD driver. Since
> RMU has only 2 registers, the HW designers decided to use up the CMU memory
> map. So, maybe syscon would be best option I think. What is your opinion?

Using syscon seems cleaner than stuffing the regmap into owl_clk_desc.

> Even if we go for syscon, we should place the reset driver within clk
> framework as I can see other SoCs like Mediatek are doing the same. But again
> I'm not sure!

Me neither. If the CMU and RMU are really separate and only share the
memory map, a syscon driver could live in drivers/reset without
problems.
It's only when there are interactions between clocks and resets that you
really want to have the reset driver integrated with clk.

regards
Philipp

Re: [PATCH 0/9] Add Reset Controller support for Actions Semi Owl SoCs

[Manivannan Sadhasivam]

Hi Philipp,

On Mon, Jul 30, 2018 at 05:38:31PM +0200, Philipp Zabel wrote:
> On Mon, 2018-07-30 at 20:41 +0530, Manivannan Sadhasivam wrote:
> > Hi Andreas,
> > 
> > On Mon, Jul 30, 2018 at 12:26:07PM +0200, Andreas Färber wrote:
> > > Hi Mani,
> > > 
> > > Am 27.07.2018 um 20:45 schrieb Manivannan Sadhasivam:
> > > > This patchset adds Reset Controller (RMU) support for Actions Semi
> > > > Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into
> > > > the clock subsystem in hardware. Hence, in software we integrate RMU
> > > > support into common clock driver inorder to maintain compatibility.
> > > 
> > > Can this not be placed into drivers/reset/ by using mfd-simple with a
> > > sub-node in DT?
> > > 
> > 
> > Actually I was not sure where to place this reset controller driver. When I
> > looked into other similar ones such as sunxi, they just integrated into the
> > clk subsystem. So I just chose that path. But yeah, this is hacky!
> > 
> > But this RMU is not MFD by any means. Since the CMU (Clock) and RMU (Reset)
> > are two separate IPs inside SoC, we shouldn't describe it as a MFD driver. Since
> > RMU has only 2 registers, the HW designers decided to use up the CMU memory
> > map. So, maybe syscon would be best option I think. What is your opinion?
> 
> Using syscon seems cleaner than stuffing the regmap into owl_clk_desc.
> 

Okay.

> > Even if we go for syscon, we should place the reset driver within clk
> > framework as I can see other SoCs like Mediatek are doing the same. But again
> > I'm not sure!
> 
> Me neither. If the CMU and RMU are really separate and only share the
> memory map, a syscon driver could live in drivers/reset without
> problems.
> It's only when there are interactions between clocks and resets that you
> really want to have the reset driver integrated with clk.
> 

Okay. Then I will add it as a syscon driver under drivers/reset.

I hope that Andreas would also be happy with this.

Thanks a lot for your response!

Regards,
Mani

> regards
> Philipp

Re: [PATCH 7/9] clk: actions: Add Actions Semi Owl SoCs Reset Management Unit support

[Manivannan Sadhasivam]

Hi Philipp,

On Mon, Jul 30, 2018 at 12:21:52PM +0200, Philipp Zabel wrote:
> Hi Manivannan,
> 
> Thank you for the patch, a few small comments below:
> 
> On Sat, 2018-07-28 at 00:15 +0530, Manivannan Sadhasivam wrote:
> > Add Reset Management Unit (RMU) support for Actions Semi Owl SoCs.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  drivers/clk/actions/Kconfig      |  1 +
> >  drivers/clk/actions/Makefile     |  1 +
> >  drivers/clk/actions/owl-common.h |  2 +
> >  drivers/clk/actions/owl-reset.c  | 72 ++++++++++++++++++++++++++++++++
> >  drivers/clk/actions/owl-reset.h  | 32 ++++++++++++++
> >  5 files changed, 108 insertions(+)
> >  create mode 100644 drivers/clk/actions/owl-reset.c
> >  create mode 100644 drivers/clk/actions/owl-reset.h
> > 
> > diff --git a/drivers/clk/actions/Kconfig b/drivers/clk/actions/Kconfig
> > index dc38c85a4833..04f0a6355726 100644
> > --- a/drivers/clk/actions/Kconfig
> > +++ b/drivers/clk/actions/Kconfig
> > @@ -2,6 +2,7 @@ config CLK_ACTIONS
> >  	bool "Clock driver for Actions Semi SoCs"
> >  	depends on ARCH_ACTIONS || COMPILE_TEST
> >  	select REGMAP_MMIO
> > +	select RESET_CONTROLLER
> >  	default ARCH_ACTIONS
> >  
> >  if CLK_ACTIONS
> > diff --git a/drivers/clk/actions/Makefile b/drivers/clk/actions/Makefile
> > index 78c17d56f991..ccfdf9781cef 100644
> > --- a/drivers/clk/actions/Makefile
> > +++ b/drivers/clk/actions/Makefile
> > @@ -7,6 +7,7 @@ clk-owl-y			+= owl-divider.o
> >  clk-owl-y			+= owl-factor.o
> >  clk-owl-y			+= owl-composite.o
> >  clk-owl-y			+= owl-pll.o
> > +clk-owl-y			+= owl-reset.o
> >  
> >  # SoC support
> >  obj-$(CONFIG_CLK_OWL_S700)	+= owl-s700.o
> > diff --git a/drivers/clk/actions/owl-common.h b/drivers/clk/actions/owl-common.h
> > index 56f01f7774aa..4dc7f286831f 100644
> > --- a/drivers/clk/actions/owl-common.h
> > +++ b/drivers/clk/actions/owl-common.h
> > @@ -26,6 +26,8 @@ struct owl_clk_desc {
> >  	struct owl_clk_common		**clks;
> >  	unsigned long			num_clks;
> >  	struct clk_hw_onecell_data	*hw_clks;
> > +	struct owl_reset_map		*resets;
> 
> Could this be:
> 	const struct owl_reset_map	*resets;
> ?
> 

Ack.

> > +	unsigned long			num_resets;
> >  	struct regmap			*regmap;
> >  };
> >  
> > diff --git a/drivers/clk/actions/owl-reset.c b/drivers/clk/actions/owl-reset.c
> > new file mode 100644
> > index 000000000000..91b161cc68de
> > --- /dev/null
> > +++ b/drivers/clk/actions/owl-reset.c
> > @@ -0,0 +1,72 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +//
> > +// Actions Semi Owl SoCs Reset Management Unit driver
> > +//
> > +// Copyright (c) 2018 Linaro Ltd.
> > +// Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > +
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> 
> This seems unnecessary, since register access is done via regmap.
> 

Will remove this header.

> > +#include <linux/regmap.h>
> > +#include <linux/reset-controller.h>
> > +
> > +#include "owl-reset.h"
> > +
> > +static int owl_reset_assert(struct reset_controller_dev *rcdev,
> > +			    unsigned long id)
> > +{
> > +	struct owl_reset *reset = to_owl_reset(rcdev);
> > +	const struct owl_reset_map *map = &reset->reset_map[id];
> > +	u32 reg;
> > +
> > +	regmap_read(reset->regmap, map->reg, &reg);
> > +	regmap_write(reset->regmap, map->reg, reg & ~map->bit);
> 
> This read-modify-write sequence needs locking against concurrent
> register access. Better use regmap_update_bits():
> 
> 	return regmap_update_bits(reset->regmap, map->reg, map->bit, 0);
> 

Ack.

> > +
> > +	return 0;
> > +}
> > +
> > +static int owl_reset_deassert(struct reset_controller_dev *rcdev,
> > +			      unsigned long id)
> > +{
> > +	struct owl_reset *reset = to_owl_reset(rcdev);
> > +	const struct owl_reset_map *map = &reset->reset_map[id];
> > +	u32 reg;
> > +
> > +	regmap_read(reset->regmap, map->reg, &reg);
> > +	regmap_write(reset->regmap, map->reg, reg | map->bit);
> 
> Better:
> 
> 	return regmap_update_bits(reset->regmap, map->reg, map->bit, map->bit);
> 

Ack.

> > +
> > +	return 0;
> > +}
> > +
> > +static int owl_reset_reset(struct reset_controller_dev *rcdev,
> > +			   unsigned long id)
> > +{
> > +	owl_reset_assert(rcdev, id);
> > +	udelay(1);
> 
> Is the delay valid for all IP cores on all SoCs variants?
> 

It is valid for S900 and S700 for now but I'm not sure about S500. Will
make sure while adding S500 support.

> > +	owl_reset_deassert(rcdev, id);
> > +
> > +	return 0;
> > +}
> > +
> > +static int owl_reset_status(struct reset_controller_dev *rcdev,
> > +			    unsigned long id)
> > +{
> > +	struct owl_reset *reset = to_owl_reset(rcdev);
> > +	const struct owl_reset_map *map = &reset->reset_map[id];
> > +	u32 reg;
> > +
> > +	regmap_read(reset->regmap, map->reg, &reg);
> 
> If this could return an error, better report it.
> 

Ack.

> > +
> > +	/*
> > +	 * The reset control API expects 0 if reset is not asserted,
> > +	 * which is the opposite of what our hardware uses.
> > +	 */
> > +	return !(map->bit & reg);
> > +}
> > +
> > +const struct reset_control_ops owl_reset_ops = {
> > +	.assert		= owl_reset_assert,
> > +	.deassert	= owl_reset_deassert,
> > +	.reset		= owl_reset_reset,
> > +	.status		= owl_reset_status,
> > +};
> > diff --git a/drivers/clk/actions/owl-reset.h b/drivers/clk/actions/owl-reset.h
> > new file mode 100644
> > index 000000000000..1a5e987ba99b
> > --- /dev/null
> > +++ b/drivers/clk/actions/owl-reset.h
> > @@ -0,0 +1,32 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +//
> > +// Actions Semi Owl SoCs Reset Management Unit driver
> > +//
> > +// Copyright (c) 2018 Linaro Ltd.
> > +// Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > +
> > +#ifndef _OWL_RESET_H_
> > +#define _OWL_RESET_H_
> > +
> > +#include <linux/reset-controller.h>
> > +#include <linux/spinlock.h>
> 
> spinlock?
> 

Oops. Not needed, will remove that.

> > +
> > +struct owl_reset_map {
> > +	u16	reg;
> 
> Note that this will be aligned to 32-bits. If the intention was to save
> space, consider storing an u8 bit index instead of the mask.
> 

Better to change it to u32, no need of memory saving here :)

> > +	u32	bit;
> > +};
> > +
> > +struct owl_reset {
> > +	struct reset_controller_dev	rcdev;
> > +	struct owl_reset_map		*reset_map;
> 
> Could this be
> 	const struct owl_reset_map	*reset_map;
> ?
> 

Ack.

Thanks,
Mani

> > +	struct regmap			*regmap;
> > +};
> > +
> > +static inline struct owl_reset *to_owl_reset(struct reset_controller_dev *rcdev)
> > +{
> > +	return container_of(rcdev, struct owl_reset, rcdev);
> > +}
> > +
> > +extern const struct reset_control_ops owl_reset_ops;
> > +
> > +#endif /* _OWL_RESET_H_ */
> 
> regards
> Philipp

Re: [PATCH 0/9] Add Reset Controller support for Actions Semi Owl SoCs

[Rob Herring]

On Mon, Jul 30, 2018 at 08:41:31PM +0530, Manivannan Sadhasivam wrote:
> Hi Andreas,
> 
> On Mon, Jul 30, 2018 at 12:26:07PM +0200, Andreas Färber wrote:
> > Hi Mani,
> > 
> > Am 27.07.2018 um 20:45 schrieb Manivannan Sadhasivam:
> > > This patchset adds Reset Controller (RMU) support for Actions Semi
> > > Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into
> > > the clock subsystem in hardware. Hence, in software we integrate RMU
> > > support into common clock driver inorder to maintain compatibility.
> > 
> > Can this not be placed into drivers/reset/ by using mfd-simple with a
> > sub-node in DT?

That is exactly what I tell folks not to do. Design the DT based on h/w 
blocks, not current desired driver split for some OS.

> Actually I was not sure where to place this reset controller driver. When I
> looked into other similar ones such as sunxi, they just integrated into the
> clk subsystem. So I just chose that path. But yeah, this is hacky!
> 
> But this RMU is not MFD by any means. Since the CMU (Clock) and RMU (Reset)
> are two separate IPs inside SoC, we shouldn't describe it as a MFD driver. Since
> RMU has only 2 registers, the HW designers decided to use up the CMU memory
> map. So, maybe syscon would be best option I think. What is your opinion?

If there's nothing shared then it is not a syscon. If you can create 
separate address ranges, then 2 nodes is probably okay. If the registers 
are all mixed up, then 1 node.

Rob

Re: [PATCH 2/9] dt-bindings: clock: Add reset controller bindings for Actions Semi Owl SoCs

[Rob Herring]

On Sat, Jul 28, 2018 at 12:15:20AM +0530, Manivannan Sadhasivam wrote:
> Add Reset Controller bindings to clock bindings for Actions Semi Owl
> SoCs, S700 and S900.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  Documentation/devicetree/bindings/clock/actions,owl-cmu.txt | 2 ++
>  1 file changed, 2 insertions(+)

This one is:

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

v2 is not.

Re: [PATCH 3/9] dt-bindings: reset: Add binding constants for Actions Semi S700 RMU

[Rob Herring]

On Sat, Jul 28, 2018 at 12:15:21AM +0530, Manivannan Sadhasivam wrote:
> Add device tree binding constants for Actions Semi S700 SoC Reset
> Management Unit (RMU).
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  .../dt-bindings/reset/actions,s700-reset.h    | 34 +++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 include/dt-bindings/reset/actions,s700-reset.h

This can go in the previous patch. Either way

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

Re: [PATCH 4/9] dt-bindings: reset: Add binding constants for Actions Semi S900 RMU

[Rob Herring]

On Sat, Jul 28, 2018 at 12:15:22AM +0530, Manivannan Sadhasivam wrote:
> Add device tree binding constants for Actions Semi S900 SoC Reset
> Management Unit (RMU).
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  .../dt-bindings/reset/actions,s900-reset.h    | 65 +++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 include/dt-bindings/reset/actions,s900-reset.h

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

Re: [PATCH 0/9] Add Reset Controller support for Actions Semi Owl SoCs

[Manivannan Sadhasivam]

Hi Rob,

On Tue, Aug 07, 2018 at 12:47:10PM -0600, Rob Herring wrote:
> On Mon, Jul 30, 2018 at 08:41:31PM +0530, Manivannan Sadhasivam wrote:
> > Hi Andreas,
> > 
> > On Mon, Jul 30, 2018 at 12:26:07PM +0200, Andreas Färber wrote:
> > > Hi Mani,
> > > 
> > > Am 27.07.2018 um 20:45 schrieb Manivannan Sadhasivam:
> > > > This patchset adds Reset Controller (RMU) support for Actions Semi
> > > > Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into
> > > > the clock subsystem in hardware. Hence, in software we integrate RMU
> > > > support into common clock driver inorder to maintain compatibility.
> > > 
> > > Can this not be placed into drivers/reset/ by using mfd-simple with a
> > > sub-node in DT?
> 
> That is exactly what I tell folks not to do. Design the DT based on h/w 
> blocks, not current desired driver split for some OS.
> 
> > Actually I was not sure where to place this reset controller driver. When I
> > looked into other similar ones such as sunxi, they just integrated into the
> > clk subsystem. So I just chose that path. But yeah, this is hacky!
> > 
> > But this RMU is not MFD by any means. Since the CMU (Clock) and RMU (Reset)
> > are two separate IPs inside SoC, we shouldn't describe it as a MFD driver. Since
> > RMU has only 2 registers, the HW designers decided to use up the CMU memory
> > map. So, maybe syscon would be best option I think. What is your opinion?
> 
> If there's nothing shared then it is not a syscon. If you can create 
> separate address ranges, then 2 nodes is probably okay. If the registers 
> are all mixed up, then 1 node.
> 

I don't quite understand the reason for not being syscon. The definition
of syscon says that, "System controller node represents a register region
containing a set of miscellaneous registers. The registers are not cohesive
enough to represent as any specific type of device." which exactly fits
this case. Only the registers of CMU & RMU are shared and not the HW block!

Can you please clarify?

Thanks,
Mani

> Rob

Re: [PATCH 0/9] Add Reset Controller support for Actions Semi Owl SoCs

[Rob Herring]

On Wed, Aug 8, 2018 at 11:30 AM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> Hi Rob,
>
> On Tue, Aug 07, 2018 at 12:47:10PM -0600, Rob Herring wrote:
> > On Mon, Jul 30, 2018 at 08:41:31PM +0530, Manivannan Sadhasivam wrote:
> > > Hi Andreas,
> > >
> > > On Mon, Jul 30, 2018 at 12:26:07PM +0200, Andreas Färber wrote:
> > > > Hi Mani,
> > > >
> > > > Am 27.07.2018 um 20:45 schrieb Manivannan Sadhasivam:
> > > > > This patchset adds Reset Controller (RMU) support for Actions Semi
> > > > > Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into
> > > > > the clock subsystem in hardware. Hence, in software we integrate RMU
> > > > > support into common clock driver inorder to maintain compatibility.
> > > >
> > > > Can this not be placed into drivers/reset/ by using mfd-simple with a
> > > > sub-node in DT?
> >
> > That is exactly what I tell folks not to do. Design the DT based on h/w
> > blocks, not current desired driver split for some OS.
> >
> > > Actually I was not sure where to place this reset controller driver. When I
> > > looked into other similar ones such as sunxi, they just integrated into the
> > > clk subsystem. So I just chose that path. But yeah, this is hacky!
> > >
> > > But this RMU is not MFD by any means. Since the CMU (Clock) and RMU (Reset)
> > > are two separate IPs inside SoC, we shouldn't describe it as a MFD driver. Since
> > > RMU has only 2 registers, the HW designers decided to use up the CMU memory
> > > map. So, maybe syscon would be best option I think. What is your opinion?
> >
> > If there's nothing shared then it is not a syscon. If you can create
> > separate address ranges, then 2 nodes is probably okay. If the registers
> > are all mixed up, then 1 node.
> >
>
> I don't quite understand the reason for not being syscon. The definition
> of syscon says that, "System controller node represents a register region
> containing a set of miscellaneous registers. The registers are not cohesive
> enough to represent as any specific type of device." which exactly fits
> this case. Only the registers of CMU & RMU are shared and not the HW block!
>
> Can you please clarify?

IIRC, the original intent of 'syscon' was really for things that had
no subsystem. Random bits all just dumped together. A block with clock
and reset doesn't sounds pretty well defined functions. Plus that
criteria doesn't work well because what does and doesn't have a
subsystem (and DT binding) evolves. IMO, we should probably get rid of
'syscon'.

Let me turn it around. Why do you want it do be a syscon? Simply to
create a regmap I suppose because that is all that 'syscon' compatible
is. A flag to create a regmap. Why do you need a regmap then? Do you
need to protect concurrent accesses (a single register has both clock
and reset bits). If so, you can't really call CMU and RMU 2 blocks. If
not, you don't really need regmap.

Rob