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
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 <[email protected]>
---
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
Add Reset Controller bindings to clock bindings for Actions Semi Owl
SoCs, S700 and S900.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
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
Add device tree binding constants for Actions Semi S700 SoC Reset
Management Unit (RMU).
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
.../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
Add device tree binding constants for Actions Semi S900 SoC Reset
Management Unit (RMU).
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
.../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
Add reset controller property and bindings header for the
Actions Semi S900 SoC DTS.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
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
Add reset controller property and bindings header for the
Actions Semi S700 SoC DTS.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
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
Add Reset Management Unit (RMU) support for Actions Semi S700 SoC.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
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
Add Reset Management Unit (RMU) support for Actions Semi S900 SoC.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
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
Add Reset Management Unit (RMU) support for Actions Semi Owl SoCs.
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
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 <[email protected]>
+
+#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, ®);
+ 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, ®);
+ 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, ®);
+
+ /*
+ * 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 <[email protected]>
+
+#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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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 <[email protected]>
> +
> +#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, ®);
> + 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, ®);
> + 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, ®);
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 <[email protected]>
> +
> +#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
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)
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 <[email protected]>
> ---
> 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
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)
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
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
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 <[email protected]>
> > ---
> > 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 <[email protected]>
> > +
> > +#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, ®);
> > + 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, ®);
> > + 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, ®);
>
> 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 <[email protected]>
> > +
> > +#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
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 <[email protected]>
> ---
> .../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 <[email protected]>
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 <[email protected]>
> ---
> Documentation/devicetree/bindings/clock/actions,owl-cmu.txt | 2 ++
> 1 file changed, 2 insertions(+)
This one is:
Reviewed-by: Rob Herring <[email protected]>
v2 is not.
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
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 <[email protected]>
> ---
> .../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 <[email protected]>
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
On Wed, Aug 8, 2018 at 11:30 AM Manivannan Sadhasivam
<[email protected]> 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