[PATCH 0/8] power: reset: at91-reset: add support for sama7g5

[PATCH 0/8] power: reset: at91-reset: add support for sama7g5

[Claudiu Beznea]

Hi,

The series adds reset controller support for SAMA7G5 SoCs. Compared with
previous version the reset controller embedded on SAMA7G5 is able to
reset individual on SoC devices (e.g. USB PHY controllers).

Among with this I took the change and converted reset controller
bindings to YAML (patch 2/8) and adapt reset controller nodes in
device tree files to comply with DT specifications (patch 1/8).

Thank you,
Claudiu Beznea

Claudiu Beznea (8):
  ARM: dts: at91: use generic name for reset controller
  dt-bindings: reset: convert Atmel/Microchip reset controller to YAML
  dt-bindings: reset: atmel,at91sam9260-reset: add sama7g5 bindings
  dt-bindings: reset: add sama7g5 definitions
  power: reset: at91-reset: add at91_reset_data
  power: reset: at91-reset: add reset_controller_dev support
  power: reset: at91-reset: add support for SAMA7G5
  ARM: dts: at91: sama7g5: add reset-controller node

 .../devicetree/bindings/arm/atmel-sysregs.txt |  15 --
 .../reset/atmel,at91sam9260-reset.yaml        |  68 +++++++++
 arch/arm/boot/dts/at91sam9260.dtsi            |   2 +-
 arch/arm/boot/dts/at91sam9261.dtsi            |   2 +-
 arch/arm/boot/dts/at91sam9263.dtsi            |   2 +-
 arch/arm/boot/dts/at91sam9g45.dtsi            |   2 +-
 arch/arm/boot/dts/at91sam9n12.dtsi            |   2 +-
 arch/arm/boot/dts/at91sam9rl.dtsi             |   2 +-
 arch/arm/boot/dts/at91sam9x5.dtsi             |   2 +-
 arch/arm/boot/dts/sam9x60.dtsi                |   2 +-
 arch/arm/boot/dts/sama5d2.dtsi                |   2 +-
 arch/arm/boot/dts/sama5d3.dtsi                |   2 +-
 arch/arm/boot/dts/sama5d4.dtsi                |   2 +-
 arch/arm/boot/dts/sama7g5.dtsi                |   7 +
 drivers/power/reset/at91-reset.c              | 134 ++++++++++++++++--
 include/dt-bindings/reset/sama7g5-reset.h     |  10 ++
 16 files changed, 217 insertions(+), 39 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/reset/atmel,at91sam9260-reset.yaml
 create mode 100644 include/dt-bindings/reset/sama7g5-reset.h

-- 
2.32.0

[PATCH 1/8] ARM: dts: at91: use generic name for reset controller

[Claudiu Beznea]

Use generic name for reset controller of AT91 devices to comply with
DT specifications.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 arch/arm/boot/dts/at91sam9260.dtsi | 2 +-
 arch/arm/boot/dts/at91sam9261.dtsi | 2 +-
 arch/arm/boot/dts/at91sam9263.dtsi | 2 +-
 arch/arm/boot/dts/at91sam9g45.dtsi | 2 +-
 arch/arm/boot/dts/at91sam9n12.dtsi | 2 +-
 arch/arm/boot/dts/at91sam9rl.dtsi  | 2 +-
 arch/arm/boot/dts/at91sam9x5.dtsi  | 2 +-
 arch/arm/boot/dts/sam9x60.dtsi     | 2 +-
 arch/arm/boot/dts/sama5d2.dtsi     | 2 +-
 arch/arm/boot/dts/sama5d3.dtsi     | 2 +-
 arch/arm/boot/dts/sama5d4.dtsi     | 2 +-
 11 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/arm/boot/dts/at91sam9260.dtsi b/arch/arm/boot/dts/at91sam9260.dtsi
index 7368347c9357..9d9820db9482 100644
--- a/arch/arm/boot/dts/at91sam9260.dtsi
+++ b/arch/arm/boot/dts/at91sam9260.dtsi
@@ -123,7 +123,7 @@ pmc: pmc@fffffc00 {
 				clock-names = "slow_xtal", "main_xtal";
 			};
 
-			rstc@fffffd00 {
+			reset-controller@fffffd00 {
 				compatible = "atmel,at91sam9260-rstc";
 				reg = <0xfffffd00 0x10>;
 				clocks = <&pmc PMC_TYPE_CORE PMC_SLOW>;
diff --git a/arch/arm/boot/dts/at91sam9261.dtsi b/arch/arm/boot/dts/at91sam9261.dtsi
index 7adc36ca8a46..259aca565305 100644
--- a/arch/arm/boot/dts/at91sam9261.dtsi
+++ b/arch/arm/boot/dts/at91sam9261.dtsi
@@ -603,7 +603,7 @@ pmc: pmc@fffffc00 {
 				clock-names = "slow_xtal", "main_xtal";
 			};
 
-			rstc@fffffd00 {
+			reset-controller@fffffd00 {
 				compatible = "atmel,at91sam9260-rstc";
 				reg = <0xfffffd00 0x10>;
 				clocks = <&slow_xtal>;
diff --git a/arch/arm/boot/dts/at91sam9263.dtsi b/arch/arm/boot/dts/at91sam9263.dtsi
index fe45d96239c9..c080df8c2312 100644
--- a/arch/arm/boot/dts/at91sam9263.dtsi
+++ b/arch/arm/boot/dts/at91sam9263.dtsi
@@ -151,7 +151,7 @@ tcb0: timer@fff7c000 {
 				clock-names = "t0_clk", "slow_clk";
 			};
 
-			rstc@fffffd00 {
+			reset-controller@fffffd00 {
 				compatible = "atmel,at91sam9260-rstc";
 				reg = <0xfffffd00 0x10>;
 				clocks = <&slow_xtal>;
diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index 2ab730fd6472..09794561c7ce 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -137,7 +137,7 @@ pmc: pmc@fffffc00 {
 				clock-names = "slow_clk", "main_xtal";
 			};
 
-			rstc@fffffd00 {
+			reset-controller@fffffd00 {
 				compatible = "atmel,at91sam9g45-rstc";
 				reg = <0xfffffd00 0x10>;
 				clocks = <&clk32k>;
diff --git a/arch/arm/boot/dts/at91sam9n12.dtsi b/arch/arm/boot/dts/at91sam9n12.dtsi
index 0785389f5507..556f35ce49e3 100644
--- a/arch/arm/boot/dts/at91sam9n12.dtsi
+++ b/arch/arm/boot/dts/at91sam9n12.dtsi
@@ -126,7 +126,7 @@ pmc: pmc@fffffc00 {
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
 			};
 
-			rstc@fffffe00 {
+			reset-controller@fffffe00 {
 				compatible = "atmel,at91sam9g45-rstc";
 				reg = <0xfffffe00 0x10>;
 				clocks = <&clk32k>;
diff --git a/arch/arm/boot/dts/at91sam9rl.dtsi b/arch/arm/boot/dts/at91sam9rl.dtsi
index 730d1182c73e..12c634811820 100644
--- a/arch/arm/boot/dts/at91sam9rl.dtsi
+++ b/arch/arm/boot/dts/at91sam9rl.dtsi
@@ -766,7 +766,7 @@ pmc: pmc@fffffc00 {
 				clock-names = "slow_clk", "main_xtal";
 			};
 
-			rstc@fffffd00 {
+			reset-controller@fffffd00 {
 				compatible = "atmel,at91sam9260-rstc";
 				reg = <0xfffffd00 0x10>;
 				clocks = <&clk32k>;
diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
index 395e883644cd..ea3b11336c79 100644
--- a/arch/arm/boot/dts/at91sam9x5.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5.dtsi
@@ -134,7 +134,7 @@ pmc: pmc@fffffc00 {
 				clock-names = "slow_clk", "main_xtal";
 			};
 
-			reset_controller: rstc@fffffe00 {
+			reset_controller: reset-controller@fffffe00 {
 				compatible = "atmel,at91sam9g45-rstc";
 				reg = <0xfffffe00 0x10>;
 				clocks = <&clk32k>;
diff --git a/arch/arm/boot/dts/sam9x60.dtsi b/arch/arm/boot/dts/sam9x60.dtsi
index ec45ced3cde6..211e743e2597 100644
--- a/arch/arm/boot/dts/sam9x60.dtsi
+++ b/arch/arm/boot/dts/sam9x60.dtsi
@@ -671,7 +671,7 @@ pmc: pmc@fffffc00 {
 				clock-names = "td_slck", "md_slck", "main_xtal";
 			};
 
-			reset_controller: rstc@fffffe00 {
+			reset_controller: reset-controller@fffffe00 {
 				compatible = "microchip,sam9x60-rstc";
 				reg = <0xfffffe00 0x10>;
 				clocks = <&clk32k 0>;
diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
index 09c741e8ecb8..769befc06b57 100644
--- a/arch/arm/boot/dts/sama5d2.dtsi
+++ b/arch/arm/boot/dts/sama5d2.dtsi
@@ -662,7 +662,7 @@ securam: sram@f8044000 {
 				ranges = <0 0xf8044000 0x1420>;
 			};
 
-			reset_controller: rstc@f8048000 {
+			reset_controller: reset-controller@f8048000 {
 				compatible = "atmel,sama5d3-rstc";
 				reg = <0xf8048000 0x10>;
 				clocks = <&clk32k>;
diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index d1841bffe3c5..ab124c09f70e 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -1003,7 +1003,7 @@ pmc: pmc@fffffc00 {
 				clock-names = "slow_clk", "main_xtal";
 			};
 
-			reset_controller: rstc@fffffe00 {
+			reset_controller: reset-controller@fffffe00 {
 				compatible = "atmel,sama5d3-rstc", "atmel,at91sam9g45-rstc";
 				reg = <0xfffffe00 0x10>;
 				clocks = <&clk32k>;
diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index f6e3e6f57252..bc41d302dbf3 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -729,7 +729,7 @@ pmecc: ecc-engine@ffffc070 {
 				};
 			};
 
-			reset_controller: rstc@fc068600 {
+			reset_controller: reset-controller@fc068600 {
 				compatible = "atmel,sama5d3-rstc", "atmel,at91sam9g45-rstc";
 				reg = <0xfc068600 0x10>;
 				clocks = <&clk32k>;
-- 
2.32.0

[PATCH 2/8] dt-bindings: reset: convert Atmel/Microchip reset controller to YAML

[Claudiu Beznea]

Convert Atmel/Microchip reset controller to YAML.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 .../devicetree/bindings/arm/atmel-sysregs.txt | 15 ------
 .../reset/atmel,at91sam9260-reset.yaml        | 49 +++++++++++++++++++
 2 files changed, 49 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/reset/atmel,at91sam9260-reset.yaml

diff --git a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt
index 16eef600d599..ab1b352344ae 100644
--- a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt
+++ b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt
@@ -25,21 +25,6 @@ System Timer (ST) required properties:
 Its subnodes can be:
 - watchdog: compatible should be "atmel,at91rm9200-wdt"
 
-RSTC Reset Controller required properties:
-- compatible: Should be "atmel,<chip>-rstc".
-  <chip> can be "at91sam9260", "at91sam9g45", "sama5d3" or "samx7"
-  it also can be "microchip,sam9x60-rstc"
-- reg: Should contain registers location and length
-- clocks: phandle to input clock.
-
-Example:
-
-	rstc@fffffd00 {
-		compatible = "atmel,at91sam9260-rstc";
-		reg = <0xfffffd00 0x10>;
-		clocks = <&clk32k>;
-	};
-
 RAMC SDRAM/DDR Controller required properties:
 - compatible: Should be "atmel,at91rm9200-sdramc", "syscon"
 			"atmel,at91sam9260-sdramc",
diff --git a/Documentation/devicetree/bindings/reset/atmel,at91sam9260-reset.yaml b/Documentation/devicetree/bindings/reset/atmel,at91sam9260-reset.yaml
new file mode 100644
index 000000000000..92936c987c9a
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/atmel,at91sam9260-reset.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/atmel,at91sam9260-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel/Microchip System Reset Controller
+
+maintainers:
+  - Claudiu Beznea <claudiu.beznea@gmail.com>
+
+description: |
+  The system reset controller can be used to reset the CPU.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - atmel,at91sam9260-rstc
+              - atmel,at91sam9g45-rstc
+              - atmel,sama5d3-rstc
+              - microchip,sam9x60-rstc
+      - items:
+          - const: atmel,sama5d3-rstc
+          - const: atmel,at91sam9g45-rstc
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/at91.h>
+
+    reset-controller@fffffd00 {
+        compatible = "atmel,at91sam9260-rstc";
+        reg = <0xfffffd00 0x10>;
+        clocks = <&pmc PMC_TYPE_CORE PMC_SLOW>;
+    };
-- 
2.32.0

Re: [PATCH 2/8] dt-bindings: reset: convert Atmel/Microchip reset controller to YAML

[Rob Herring]

On Tue, 05 Apr 2022 14:27:18 +0300, Claudiu Beznea wrote:
> Convert Atmel/Microchip reset controller to YAML.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  .../devicetree/bindings/arm/atmel-sysregs.txt | 15 ------
>  .../reset/atmel,at91sam9260-reset.yaml        | 49 +++++++++++++++++++
>  2 files changed, 49 insertions(+), 15 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/reset/atmel,at91sam9260-reset.yaml
> 

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

[PATCH 3/8] dt-bindings: reset: atmel,at91sam9260-reset: add sama7g5 bindings

[Claudiu Beznea]

Add documentation for SAMA7G5 reset controller. Compared with previous
versions of reset controllers this one contains support for resetting
in SoC devices (e.g. USB PHYs).

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 .../reset/atmel,at91sam9260-reset.yaml        | 23 +++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/reset/atmel,at91sam9260-reset.yaml b/Documentation/devicetree/bindings/reset/atmel,at91sam9260-reset.yaml
index 92936c987c9a..a165c10ae474 100644
--- a/Documentation/devicetree/bindings/reset/atmel,at91sam9260-reset.yaml
+++ b/Documentation/devicetree/bindings/reset/atmel,at91sam9260-reset.yaml
@@ -10,7 +10,8 @@ maintainers:
   - Claudiu Beznea <claudiu.beznea@gmail.com>
 
 description: |
-  The system reset controller can be used to reset the CPU.
+  The system reset controller can be used to reset the CPU. In case of
+  SAMA7G5 it can also reset some devices (e.g. USB PHYs).
 
 properties:
   compatible:
@@ -21,21 +22,39 @@ properties:
               - atmel,at91sam9g45-rstc
               - atmel,sama5d3-rstc
               - microchip,sam9x60-rstc
+              - microchip,sama7g5-rstc
       - items:
           - const: atmel,sama5d3-rstc
           - const: atmel,at91sam9g45-rstc
 
   reg:
-    maxItems: 1
+    minItems: 1
+    items:
+      - description: base registers for system reset control
+      - description: registers for device specific reset control
 
   clocks:
     maxItems: 1
 
+  "#reset-cells":
+    const: 1
+
 required:
   - compatible
   - reg
   - clocks
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - microchip,sama7g5-rstc
+    then:
+      required:
+        - "#reset-cells"
+
 additionalProperties: false
 
 examples:
-- 
2.32.0

Re: [PATCH 3/8] dt-bindings: reset: atmel,at91sam9260-reset: add sama7g5 bindings

[Rob Herring]

On Tue, 05 Apr 2022 14:27:19 +0300, Claudiu Beznea wrote:
> Add documentation for SAMA7G5 reset controller. Compared with previous
> versions of reset controllers this one contains support for resetting
> in SoC devices (e.g. USB PHYs).
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  .../reset/atmel,at91sam9260-reset.yaml        | 23 +++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 

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

[PATCH 4/8] dt-bindings: reset: add sama7g5 definitions

[Claudiu Beznea]

Add reset bindings for SAMA7G5. At the moment only USB PHYs are
included.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 include/dt-bindings/reset/sama7g5-reset.h | 10 ++++++++++
 1 file changed, 10 insertions(+)
 create mode 100644 include/dt-bindings/reset/sama7g5-reset.h

diff --git a/include/dt-bindings/reset/sama7g5-reset.h b/include/dt-bindings/reset/sama7g5-reset.h
new file mode 100644
index 000000000000..670d8075f463
--- /dev/null
+++ b/include/dt-bindings/reset/sama7g5-reset.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef __DT_BINDINGS_RESET_SAMA7G5_H
+#define __DT_BINDINGS_RESET_SAMA7G5_H
+
+#define RESET_USB_PHY1		4
+#define RESET_USB_PHY2		5
+#define RESET_USB_PHY3		6
+
+#endif /* __DT_BINDINGS_RESET_SAMA7G5_H */
-- 
2.32.0

Re: [PATCH 4/8] dt-bindings: reset: add sama7g5 definitions

[Philipp Zabel]

On Di, 2022-04-05 at 14:27 +0300, Claudiu Beznea wrote:
> Add reset bindings for SAMA7G5. At the moment only USB PHYs are
> included.

What do you mean by "at the moment only USB PHYs are included"? Are
those the only reset controls on sama7g5 and later hardware might have
more?

regards
Philipp

Re: [PATCH 4/8] dt-bindings: reset: add sama7g5 definitions

[Claudiu.Beznea]

On 05.04.2022 18:09, Philipp Zabel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Di, 2022-04-05 at 14:27 +0300, Claudiu Beznea wrote:
>> Add reset bindings for SAMA7G5. At the moment only USB PHYs are
>> included.
> 
> What do you mean by "at the moment only USB PHYs are included"? Are
> those the only reset controls on sama7g5 and later hardware might have
> more?

There are 2 more resets one for DDR controller and one for DDR PHY
controller. Those 2 controllers have no drivers in Linux at the moment (and
probably will never have). These 2 extra resets are controlled in the RAM
initialization phase by bootloaders.

Thank you,
Claudiu Beznea

> 
> regards
> Philipp

Re: [PATCH 4/8] dt-bindings: reset: add sama7g5 definitions

[Rob Herring]

On Tue, Apr 05, 2022 at 02:27:20PM +0300, Claudiu Beznea wrote:
> Add reset bindings for SAMA7G5. At the moment only USB PHYs are
> included.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  include/dt-bindings/reset/sama7g5-reset.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>  create mode 100644 include/dt-bindings/reset/sama7g5-reset.h
> 
> diff --git a/include/dt-bindings/reset/sama7g5-reset.h b/include/dt-bindings/reset/sama7g5-reset.h
> new file mode 100644
> index 000000000000..670d8075f463
> --- /dev/null
> +++ b/include/dt-bindings/reset/sama7g5-reset.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */

Dual license and GPL-2.0-only is preferred.

> +
> +#ifndef __DT_BINDINGS_RESET_SAMA7G5_H
> +#define __DT_BINDINGS_RESET_SAMA7G5_H
> +
> +#define RESET_USB_PHY1		4
> +#define RESET_USB_PHY2		5
> +#define RESET_USB_PHY3		6
> +
> +#endif /* __DT_BINDINGS_RESET_SAMA7G5_H */
> -- 
> 2.32.0
> 
> 

[PATCH 5/8] power: reset: at91-reset: add at91_reset_data

[Claudiu Beznea]

Add struct at91_reset_data to keep per platform related information.
This is a prerequisite for adding reset_controller_dev support.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/power/reset/at91-reset.c | 33 +++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 64def79d557a..0d721e27f545 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -59,6 +59,11 @@ struct at91_reset {
 	u32 ramc_lpr;
 };
 
+struct at91_reset_data {
+	u32 reset_args;
+	u32 n_device_reset;
+};
+
 /*
 * unless the SDRAM is cleanly shutdown before we hit the
 * reset register it can be left driving the data bus and
@@ -153,29 +158,34 @@ static const struct of_device_id at91_ramc_of_match[] = {
 	{ /* sentinel */ }
 };
 
+static const struct at91_reset_data sam9260 = {
+	.reset_args = AT91_RSTC_KEY | AT91_RSTC_PERRST | AT91_RSTC_PROCRST,
+};
+
+static const struct at91_reset_data samx7 = {
+	.reset_args = AT91_RSTC_KEY | AT91_RSTC_PROCRST,
+};
+
 static const struct of_device_id at91_reset_of_match[] = {
 	{
 		.compatible = "atmel,at91sam9260-rstc",
-		.data = (void *)(AT91_RSTC_KEY | AT91_RSTC_PERRST |
-				 AT91_RSTC_PROCRST),
+		.data = &sam9260,
 	},
 	{
 		.compatible = "atmel,at91sam9g45-rstc",
-		.data = (void *)(AT91_RSTC_KEY | AT91_RSTC_PERRST |
-				 AT91_RSTC_PROCRST)
+		.data = &sam9260,
 	},
 	{
 		.compatible = "atmel,sama5d3-rstc",
-		.data = (void *)(AT91_RSTC_KEY | AT91_RSTC_PERRST |
-				 AT91_RSTC_PROCRST)
+		.data = &sam9260,
 	},
 	{
 		.compatible = "atmel,samx7-rstc",
-		.data = (void *)(AT91_RSTC_KEY | AT91_RSTC_PROCRST)
+		.data = &samx7,
 	},
 	{
 		.compatible = "microchip,sam9x60-rstc",
-		.data = (void *)(AT91_RSTC_KEY | AT91_RSTC_PROCRST)
+		.data = &samx7,
 	},
 	{ /* sentinel */ }
 };
@@ -184,6 +194,7 @@ MODULE_DEVICE_TABLE(of, at91_reset_of_match);
 static int __init at91_reset_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *match;
+	const struct at91_reset_data *data;
 	struct at91_reset *reset;
 	struct device_node *np;
 	int ret, idx = 0;
@@ -213,9 +224,13 @@ static int __init at91_reset_probe(struct platform_device *pdev)
 	}
 
 	match = of_match_node(at91_reset_of_match, pdev->dev.of_node);
+	if (!match || !match->data)
+		return -ENODEV;
+
+	data = match->data;
 	reset->nb.notifier_call = at91_reset;
 	reset->nb.priority = 192;
-	reset->args = (u32)match->data;
+	reset->args = data->reset_args;
 
 	reset->sclk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(reset->sclk))
-- 
2.32.0

[PATCH 6/8] power: reset: at91-reset: add reset_controller_dev support

[Claudiu Beznea]

SAMA7G5 reset controller has 5 extra lines that goes to different devices
(3 lines to USB PHYs, 1 line to DDR controller, one line DDR PHY
controller). These reset lines could be requested by different controller
drivers (e.g. USB PHY driver) and these controllers' drivers could
assert/deassert these lines when necessary. Thus add support for
reset_controller_dev which brings this functionality.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/power/reset/at91-reset.c | 92 ++++++++++++++++++++++++++++++--
 1 file changed, 88 insertions(+), 4 deletions(-)

diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 0d721e27f545..b04df54c15d2 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -17,6 +17,7 @@
 #include <linux/of_address.h>
 #include <linux/platform_device.h>
 #include <linux/reboot.h>
+#include <linux/reset-controller.h>
 
 #include <soc/at91/at91sam9_ddrsdr.h>
 #include <soc/at91/at91sam9_sdramc.h>
@@ -53,12 +54,16 @@ enum reset_type {
 struct at91_reset {
 	void __iomem *rstc_base;
 	void __iomem *ramc_base[2];
+	void __iomem *dev_base;
+	struct reset_controller_dev rcdev;
 	struct clk *sclk;
 	struct notifier_block nb;
 	u32 args;
 	u32 ramc_lpr;
 };
 
+#define to_at91_reset(r)	container_of(r, struct at91_reset, rcdev)
+
 struct at91_reset_data {
 	u32 reset_args;
 	u32 n_device_reset;
@@ -191,6 +196,79 @@ static const struct of_device_id at91_reset_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, at91_reset_of_match);
 
+static int at91_reset_update(struct reset_controller_dev *rcdev,
+			     unsigned long id, bool assert)
+{
+	struct at91_reset *reset = to_at91_reset(rcdev);
+	u32 val;
+
+	val = readl_relaxed(reset->dev_base);
+	if (assert)
+		val |= BIT(id);
+	else
+		val &= ~BIT(id);
+	writel_relaxed(val, reset->dev_base);
+
+	return 0;
+}
+
+static int at91_reset_assert(struct reset_controller_dev *rcdev,
+			     unsigned long id)
+{
+	return at91_reset_update(rcdev, id, true);
+}
+
+static int at91_reset_deassert(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	return at91_reset_update(rcdev, id, false);
+}
+
+static int at91_reset_dev_status(struct reset_controller_dev *rcdev,
+				 unsigned long id)
+{
+	struct at91_reset *reset = to_at91_reset(rcdev);
+	u32 val;
+
+	val = readl_relaxed(reset->dev_base);
+
+	return !!(val & BIT(id));
+}
+
+static const struct reset_control_ops at91_reset_ops = {
+	.assert = at91_reset_assert,
+	.deassert = at91_reset_deassert,
+	.status = at91_reset_dev_status,
+};
+
+static int at91_reset_of_xlate(struct reset_controller_dev *rcdev,
+			       const struct of_phandle_args *reset_spec)
+{
+	return reset_spec->args[0];
+}
+
+static int at91_rcdev_init(struct at91_reset *reset,
+			   const struct at91_reset_data *data,
+			   struct platform_device *pdev)
+{
+	if (!data->n_device_reset)
+		return 0;
+
+	reset->dev_base = devm_of_iomap(&pdev->dev, pdev->dev.of_node, 1,
+					NULL);
+	if (IS_ERR(reset->rstc_base))
+		return -ENODEV;
+
+	reset->rcdev.ops = &at91_reset_ops;
+	reset->rcdev.owner = THIS_MODULE;
+	reset->rcdev.of_node = pdev->dev.of_node;
+	reset->rcdev.nr_resets = data->n_device_reset;
+	reset->rcdev.of_reset_n_cells = 1;
+	reset->rcdev.of_xlate = at91_reset_of_xlate;
+
+	return devm_reset_controller_register(&pdev->dev, &reset->rcdev);
+}
+
 static int __init at91_reset_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *match;
@@ -244,6 +322,10 @@ static int __init at91_reset_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, reset);
 
+	ret = at91_rcdev_init(reset, data, pdev);
+	if (ret)
+		goto disable_clk;
+
 	if (of_device_is_compatible(pdev->dev.of_node, "microchip,sam9x60-rstc")) {
 		u32 val = readl(reset->rstc_base + AT91_RSTC_MR);
 
@@ -252,14 +334,16 @@ static int __init at91_reset_probe(struct platform_device *pdev)
 	}
 
 	ret = register_restart_handler(&reset->nb);
-	if (ret) {
-		clk_disable_unprepare(reset->sclk);
-		return ret;
-	}
+	if (ret)
+		goto disable_clk;
 
 	at91_reset_status(pdev, reset->rstc_base);
 
 	return 0;
+
+disable_clk:
+	clk_disable_unprepare(reset->sclk);
+	return ret;
 }
 
 static int __exit at91_reset_remove(struct platform_device *pdev)
-- 
2.32.0

Re: [PATCH 6/8] power: reset: at91-reset: add reset_controller_dev support

[Philipp Zabel]

Hi Claudiu,

On Di, 2022-04-05 at 14:27 +0300, Claudiu Beznea wrote:
> SAMA7G5 reset controller has 5 extra lines that goes to different
> devices
> (3 lines to USB PHYs, 1 line to DDR controller, one line DDR PHY
> controller). These reset lines could be requested by different
> controller
> drivers (e.g. USB PHY driver) and these controllers' drivers could
> assert/deassert these lines when necessary. Thus add support for
> reset_controller_dev which brings this functionality.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  drivers/power/reset/at91-reset.c | 92
> ++++++++++++++++++++++++++++++--
>  1 file changed, 88 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/power/reset/at91-reset.c
> b/drivers/power/reset/at91-reset.c
> index 0d721e27f545..b04df54c15d2 100644
> --- a/drivers/power/reset/at91-reset.c
> +++ b/drivers/power/reset/at91-reset.c
> @@ -17,6 +17,7 @@
>  #include <linux/of_address.h>
>  #include <linux/platform_device.h>
>  #include <linux/reboot.h>
> +#include <linux/reset-controller.h>
>  
>  #include <soc/at91/at91sam9_ddrsdr.h>
>  #include <soc/at91/at91sam9_sdramc.h>
> @@ -53,12 +54,16 @@ enum reset_type {
>  struct at91_reset {
>         void __iomem *rstc_base;
>         void __iomem *ramc_base[2];
> +       void __iomem *dev_base;
> +       struct reset_controller_dev rcdev;
>         struct clk *sclk;
>         struct notifier_block nb;
>         u32 args;
>         u32 ramc_lpr;
>  };
>  
> +#define to_at91_reset(r)       container_of(r, struct at91_reset, rcdev)
> +
>  struct at91_reset_data {
>         u32 reset_args;
>         u32 n_device_reset;
> @@ -191,6 +196,79 @@ static const struct of_device_id
> at91_reset_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, at91_reset_of_match);
>  
> +static int at91_reset_update(struct reset_controller_dev *rcdev,
> +                            unsigned long id, bool assert)
> +{
> +       struct at91_reset *reset = to_at91_reset(rcdev);
> +       u32 val;
> +
> +       val = readl_relaxed(reset->dev_base);
> +       if (assert)
> +               val |= BIT(id);
> +       else
> +               val &= ~BIT(id);
> +       writel_relaxed(val, reset->dev_base);

This read-modify-update should be protected by a spinlock.

> +
> +       return 0;
> +}
> +
> +static int at91_reset_assert(struct reset_controller_dev *rcdev,
> +                            unsigned long id)
> +{
> +       return at91_reset_update(rcdev, id, true);
> +}
> +
> +static int at91_reset_deassert(struct reset_controller_dev *rcdev,
> +                              unsigned long id)
> +{
> +       return at91_reset_update(rcdev, id, false);
> +}
> +
> +static int at91_reset_dev_status(struct reset_controller_dev *rcdev,
> +                                unsigned long id)
> +{
> +       struct at91_reset *reset = to_at91_reset(rcdev);
> +       u32 val;
> +
> +       val = readl_relaxed(reset->dev_base);
> +
> +       return !!(val & BIT(id));
> +}
> +
> +static const struct reset_control_ops at91_reset_ops = {
> +       .assert = at91_reset_assert,
> +       .deassert = at91_reset_deassert,
> +       .status = at91_reset_dev_status,
> +};
> +
> +static int at91_reset_of_xlate(struct reset_controller_dev *rcdev,
> +                              const struct of_phandle_args *reset_spec)
> +{
> +       return reset_spec->args[0];
> +}

For 1:1 mappings there is no need for a custom of_xlate handler. Just
leave of_xlate and of_reset_n_cells empty.

> +
> +static int at91_rcdev_init(struct at91_reset *reset,
> +                          const struct at91_reset_data *data,
> +                          struct platform_device *pdev)
> +{
> +       if (!data->n_device_reset)
> +               return 0;
> +
> +       reset->dev_base = devm_of_iomap(&pdev->dev, pdev->dev.of_node, 1,
> +                                       NULL);
> +       if (IS_ERR(reset->rstc_base))

Should check reset->dev_base here.

> +               return -ENODEV;
> +
> +       reset->rcdev.ops = &at91_reset_ops;
> +       reset->rcdev.owner = THIS_MODULE;
> +       reset->rcdev.of_node = pdev->dev.of_node;
> +       reset->rcdev.nr_resets = data->n_device_reset;
> +       reset->rcdev.of_reset_n_cells = 1;
> +       reset->rcdev.of_xlate = at91_reset_of_xlate;
> +
> +       return devm_reset_controller_register(&pdev->dev, &reset->rcdev);
> +}
> +
>  static int __init at91_reset_probe(struct platform_device *pdev)
>  {
>         const struct of_device_id *match;
> @@ -244,6 +322,10 @@ static int __init at91_reset_probe(struct
> platform_device *pdev)
>  
>         platform_set_drvdata(pdev, reset);
>  
> +       ret = at91_rcdev_init(reset, data, pdev);
> +       if (ret)
> +               goto disable_clk;
> +
>         if (of_device_is_compatible(pdev->dev.of_node,
> "microchip,sam9x60-rstc")) {
>                 u32 val = readl(reset->rstc_base + AT91_RSTC_MR);
>  
> @@ -252,14 +334,16 @@ static int __init at91_reset_probe(struct
> platform_device *pdev)
>         }
>  
>         ret = register_restart_handler(&reset->nb);
> -       if (ret) {
> -               clk_disable_unprepare(reset->sclk);
> -               return ret;
> -       }
> +       if (ret)
> +               goto disable_clk;
>  
>         at91_reset_status(pdev, reset->rstc_base);
>  
>         return 0;
> +
> +disable_clk:
> +       clk_disable_unprepare(reset->sclk);
> +       return ret;
>  }
>  
>  static int __exit at91_reset_remove(struct platform_device *pdev)

regards
Philipp

Re: [PATCH 6/8] power: reset: at91-reset: add reset_controller_dev support

[Claudiu.Beznea]

Hi, Philipp,

On 05.04.2022 14:47, Philipp Zabel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Claudiu,
> 
> On Di, 2022-04-05 at 14:27 +0300, Claudiu Beznea wrote:
>> SAMA7G5 reset controller has 5 extra lines that goes to different
>> devices
>> (3 lines to USB PHYs, 1 line to DDR controller, one line DDR PHY
>> controller). These reset lines could be requested by different
>> controller
>> drivers (e.g. USB PHY driver) and these controllers' drivers could
>> assert/deassert these lines when necessary. Thus add support for
>> reset_controller_dev which brings this functionality.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>  drivers/power/reset/at91-reset.c | 92
>> ++++++++++++++++++++++++++++++--
>>  1 file changed, 88 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/power/reset/at91-reset.c
>> b/drivers/power/reset/at91-reset.c
>> index 0d721e27f545..b04df54c15d2 100644
>> --- a/drivers/power/reset/at91-reset.c
>> +++ b/drivers/power/reset/at91-reset.c
>> @@ -17,6 +17,7 @@
>>  #include <linux/of_address.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/reboot.h>
>> +#include <linux/reset-controller.h>
>>
>>  #include <soc/at91/at91sam9_ddrsdr.h>
>>  #include <soc/at91/at91sam9_sdramc.h>
>> @@ -53,12 +54,16 @@ enum reset_type {
>>  struct at91_reset {
>>         void __iomem *rstc_base;
>>         void __iomem *ramc_base[2];
>> +       void __iomem *dev_base;
>> +       struct reset_controller_dev rcdev;
>>         struct clk *sclk;
>>         struct notifier_block nb;
>>         u32 args;
>>         u32 ramc_lpr;
>>  };
>>
>> +#define to_at91_reset(r)       container_of(r, struct at91_reset, rcdev)
>> +
>>  struct at91_reset_data {
>>         u32 reset_args;
>>         u32 n_device_reset;
>> @@ -191,6 +196,79 @@ static const struct of_device_id
>> at91_reset_of_match[] = {
>>  };
>>  MODULE_DEVICE_TABLE(of, at91_reset_of_match);
>>
>> +static int at91_reset_update(struct reset_controller_dev *rcdev,
>> +                            unsigned long id, bool assert)
>> +{
>> +       struct at91_reset *reset = to_at91_reset(rcdev);
>> +       u32 val;
>> +
>> +       val = readl_relaxed(reset->dev_base);
>> +       if (assert)
>> +               val |= BIT(id);
>> +       else
>> +               val &= ~BIT(id);
>> +       writel_relaxed(val, reset->dev_base);
> 
> This read-modify-update should be protected by a spinlock.

Right, I missed it.

> 
>> +
>> +       return 0;
>> +}
>> +
>> +static int at91_reset_assert(struct reset_controller_dev *rcdev,
>> +                            unsigned long id)
>> +{
>> +       return at91_reset_update(rcdev, id, true);
>> +}
>> +
>> +static int at91_reset_deassert(struct reset_controller_dev *rcdev,
>> +                              unsigned long id)
>> +{
>> +       return at91_reset_update(rcdev, id, false);
>> +}
>> +
>> +static int at91_reset_dev_status(struct reset_controller_dev *rcdev,
>> +                                unsigned long id)
>> +{
>> +       struct at91_reset *reset = to_at91_reset(rcdev);
>> +       u32 val;
>> +
>> +       val = readl_relaxed(reset->dev_base);
>> +
>> +       return !!(val & BIT(id));
>> +}
>> +
>> +static const struct reset_control_ops at91_reset_ops = {
>> +       .assert = at91_reset_assert,
>> +       .deassert = at91_reset_deassert,
>> +       .status = at91_reset_dev_status,
>> +};
>> +
>> +static int at91_reset_of_xlate(struct reset_controller_dev *rcdev,
>> +                              const struct of_phandle_args *reset_spec)
>> +{
>> +       return reset_spec->args[0];
>> +}
> 
> For 1:1 mappings there is no need for a custom of_xlate handler. Just
> leave of_xlate and of_reset_n_cells empty.

I'll have a look on it.

> 
>> +
>> +static int at91_rcdev_init(struct at91_reset *reset,
>> +                          const struct at91_reset_data *data,
>> +                          struct platform_device *pdev)
>> +{
>> +       if (!data->n_device_reset)
>> +               return 0;
>> +
>> +       reset->dev_base = devm_of_iomap(&pdev->dev, pdev->dev.of_node, 1,
>> +                                       NULL);
>> +       if (IS_ERR(reset->rstc_base))
> 
> Should check reset->dev_base here.

That's true.

Thank you for your review,
Claudiu Beznea

> 
>> +               return -ENODEV;
>> +
>> +       reset->rcdev.ops = &at91_reset_ops;
>> +       reset->rcdev.owner = THIS_MODULE;
>> +       reset->rcdev.of_node = pdev->dev.of_node;
>> +       reset->rcdev.nr_resets = data->n_device_reset;
>> +       reset->rcdev.of_reset_n_cells = 1;
>> +       reset->rcdev.of_xlate = at91_reset_of_xlate;
>> +
>> +       return devm_reset_controller_register(&pdev->dev, &reset->rcdev);
>> +}
>> +
>>  static int __init at91_reset_probe(struct platform_device *pdev)
>>  {
>>         const struct of_device_id *match;
>> @@ -244,6 +322,10 @@ static int __init at91_reset_probe(struct
>> platform_device *pdev)
>>
>>         platform_set_drvdata(pdev, reset);
>>
>> +       ret = at91_rcdev_init(reset, data, pdev);
>> +       if (ret)
>> +               goto disable_clk;
>> +
>>         if (of_device_is_compatible(pdev->dev.of_node,
>> "microchip,sam9x60-rstc")) {
>>                 u32 val = readl(reset->rstc_base + AT91_RSTC_MR);
>>
>> @@ -252,14 +334,16 @@ static int __init at91_reset_probe(struct
>> platform_device *pdev)
>>         }
>>
>>         ret = register_restart_handler(&reset->nb);
>> -       if (ret) {
>> -               clk_disable_unprepare(reset->sclk);
>> -               return ret;
>> -       }
>> +       if (ret)
>> +               goto disable_clk;
>>
>>         at91_reset_status(pdev, reset->rstc_base);
>>
>>         return 0;
>> +
>> +disable_clk:
>> +       clk_disable_unprepare(reset->sclk);
>> +       return ret;
>>  }
>>
>>  static int __exit at91_reset_remove(struct platform_device *pdev)
> 
> regards
> Philipp

Re: [PATCH 6/8] power: reset: at91-reset: add reset_controller_dev support

[Claudiu.Beznea]

Hi, Philipp,

On 05.04.2022 14:47, Philipp Zabel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Claudiu,
> 
> On Di, 2022-04-05 at 14:27 +0300, Claudiu Beznea wrote:
>> SAMA7G5 reset controller has 5 extra lines that goes to different
>> devices
>> (3 lines to USB PHYs, 1 line to DDR controller, one line DDR PHY
>> controller). These reset lines could be requested by different
>> controller
>> drivers (e.g. USB PHY driver) and these controllers' drivers could
>> assert/deassert these lines when necessary. Thus add support for
>> reset_controller_dev which brings this functionality.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>  drivers/power/reset/at91-reset.c | 92
>> ++++++++++++++++++++++++++++++--
>>  1 file changed, 88 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/power/reset/at91-reset.c
>> b/drivers/power/reset/at91-reset.c
>> index 0d721e27f545..b04df54c15d2 100644
>> --- a/drivers/power/reset/at91-reset.c
>> +++ b/drivers/power/reset/at91-reset.c
>> @@ -17,6 +17,7 @@
>>  #include <linux/of_address.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/reboot.h>
>> +#include <linux/reset-controller.h>
>>
>>  #include <soc/at91/at91sam9_ddrsdr.h>
>>  #include <soc/at91/at91sam9_sdramc.h>
>> @@ -53,12 +54,16 @@ enum reset_type {
>>  struct at91_reset {
>>         void __iomem *rstc_base;
>>         void __iomem *ramc_base[2];
>> +       void __iomem *dev_base;
>> +       struct reset_controller_dev rcdev;
>>         struct clk *sclk;
>>         struct notifier_block nb;
>>         u32 args;
>>         u32 ramc_lpr;
>>  };
>>
>> +#define to_at91_reset(r)       container_of(r, struct at91_reset, rcdev)
>> +
>>  struct at91_reset_data {
>>         u32 reset_args;
>>         u32 n_device_reset;
>> @@ -191,6 +196,79 @@ static const struct of_device_id
>> at91_reset_of_match[] = {
>>  };
>>  MODULE_DEVICE_TABLE(of, at91_reset_of_match);
>>
>> +static int at91_reset_update(struct reset_controller_dev *rcdev,
>> +                            unsigned long id, bool assert)
>> +{
>> +       struct at91_reset *reset = to_at91_reset(rcdev);
>> +       u32 val;
>> +
>> +       val = readl_relaxed(reset->dev_base);
>> +       if (assert)
>> +               val |= BIT(id);
>> +       else
>> +               val &= ~BIT(id);
>> +       writel_relaxed(val, reset->dev_base);
> 
> This read-modify-update should be protected by a spinlock.
> 
>> +
>> +       return 0;
>> +}
>> +
>> +static int at91_reset_assert(struct reset_controller_dev *rcdev,
>> +                            unsigned long id)
>> +{
>> +       return at91_reset_update(rcdev, id, true);
>> +}
>> +
>> +static int at91_reset_deassert(struct reset_controller_dev *rcdev,
>> +                              unsigned long id)
>> +{
>> +       return at91_reset_update(rcdev, id, false);
>> +}
>> +
>> +static int at91_reset_dev_status(struct reset_controller_dev *rcdev,
>> +                                unsigned long id)
>> +{
>> +       struct at91_reset *reset = to_at91_reset(rcdev);
>> +       u32 val;
>> +
>> +       val = readl_relaxed(reset->dev_base);
>> +
>> +       return !!(val & BIT(id));
>> +}
>> +
>> +static const struct reset_control_ops at91_reset_ops = {
>> +       .assert = at91_reset_assert,
>> +       .deassert = at91_reset_deassert,
>> +       .status = at91_reset_dev_status,
>> +};
>> +
>> +static int at91_reset_of_xlate(struct reset_controller_dev *rcdev,
>> +                              const struct of_phandle_args *reset_spec)
>> +{
>> +       return reset_spec->args[0];
>> +}
> 
> For 1:1 mappings there is no need for a custom of_xlate handler. Just
> leave of_xlate and of_reset_n_cells empty.

I've double checked that. This would work if reset ids are continuous from
zero to rcdev.nr_resets. This the of_reset_simple_xlate:

static int of_reset_simple_xlate(struct reset_controller_dev *rcdev,
                                 const struct of_phandle_args *reset_spec)
{
	if (reset_spec->args[0] >= rcdev->nr_resets)
		return -EINVAL;
	return reset_spec->args[0];
}

But in this driver's case we have 3 ids: 4, 5, 6. That is the reason I had
this simple xlate function.

Thank you,
Claudiu Beznea

> 
>> +
>> +static int at91_rcdev_init(struct at91_reset *reset,
>> +                          const struct at91_reset_data *data,
>> +                          struct platform_device *pdev)
>> +{
>> +       if (!data->n_device_reset)
>> +               return 0;
>> +
>> +       reset->dev_base = devm_of_iomap(&pdev->dev, pdev->dev.of_node, 1,
>> +                                       NULL);
>> +       if (IS_ERR(reset->rstc_base))
> 
> Should check reset->dev_base here.
> 
>> +               return -ENODEV;
>> +
>> +       reset->rcdev.ops = &at91_reset_ops;
>> +       reset->rcdev.owner = THIS_MODULE;
>> +       reset->rcdev.of_node = pdev->dev.of_node;
>> +       reset->rcdev.nr_resets = data->n_device_reset;
>> +       reset->rcdev.of_reset_n_cells = 1;
>> +       reset->rcdev.of_xlate = at91_reset_of_xlate;
>> +
>> +       return devm_reset_controller_register(&pdev->dev, &reset->rcdev);
>> +}
>> +
>>  static int __init at91_reset_probe(struct platform_device *pdev)
>>  {
>>         const struct of_device_id *match;
>> @@ -244,6 +322,10 @@ static int __init at91_reset_probe(struct
>> platform_device *pdev)
>>
>>         platform_set_drvdata(pdev, reset);
>>
>> +       ret = at91_rcdev_init(reset, data, pdev);
>> +       if (ret)
>> +               goto disable_clk;
>> +
>>         if (of_device_is_compatible(pdev->dev.of_node,
>> "microchip,sam9x60-rstc")) {
>>                 u32 val = readl(reset->rstc_base + AT91_RSTC_MR);
>>
>> @@ -252,14 +334,16 @@ static int __init at91_reset_probe(struct
>> platform_device *pdev)
>>         }
>>
>>         ret = register_restart_handler(&reset->nb);
>> -       if (ret) {
>> -               clk_disable_unprepare(reset->sclk);
>> -               return ret;
>> -       }
>> +       if (ret)
>> +               goto disable_clk;
>>
>>         at91_reset_status(pdev, reset->rstc_base);
>>
>>         return 0;
>> +
>> +disable_clk:
>> +       clk_disable_unprepare(reset->sclk);
>> +       return ret;
>>  }
>>
>>  static int __exit at91_reset_remove(struct platform_device *pdev)
> 
> regards
> Philipp

Re: [PATCH 6/8] power: reset: at91-reset: add reset_controller_dev support

[Philipp Zabel]

Hi Claudio,

On Di, 2022-04-05 at 14:47 +0000, Claudiu.Beznea@microchip.com wrote:
> Hi, Philipp,
> 
> On 05.04.2022 14:47, Philipp Zabel wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> > 
> > Hi Claudiu,
> > 
> > On Di, 2022-04-05 at 14:27 +0300, Claudiu Beznea wrote:
> > > SAMA7G5 reset controller has 5 extra lines that goes to different
> > > devices
> > > (3 lines to USB PHYs, 1 line to DDR controller, one line DDR PHY
> > > controller). These reset lines could be requested by different
> > > controller
> > > drivers (e.g. USB PHY driver) and these controllers' drivers
> > > could
> > > assert/deassert these lines when necessary. Thus add support for
> > > reset_controller_dev which brings this functionality.
> > > 
> > > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> > > ---
> > >  drivers/power/reset/at91-reset.c | 92
> > > ++++++++++++++++++++++++++++++--
> > >  1 file changed, 88 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/power/reset/at91-reset.c
> > > b/drivers/power/reset/at91-reset.c
> > > index 0d721e27f545..b04df54c15d2 100644
> > > --- a/drivers/power/reset/at91-reset.c
> > > +++ b/drivers/power/reset/at91-reset.c
> > > @@ -17,6 +17,7 @@
> > >  #include <linux/of_address.h>
> > >  #include <linux/platform_device.h>
> > >  #include <linux/reboot.h>
> > > +#include <linux/reset-controller.h>
> > > 
> > >  #include <soc/at91/at91sam9_ddrsdr.h>
> > >  #include <soc/at91/at91sam9_sdramc.h>
> > > @@ -53,12 +54,16 @@ enum reset_type {
> > >  struct at91_reset {
> > >         void __iomem *rstc_base;
> > >         void __iomem *ramc_base[2];
> > > +       void __iomem *dev_base;
> > > +       struct reset_controller_dev rcdev;
> > >         struct clk *sclk;
> > >         struct notifier_block nb;
> > >         u32 args;
> > >         u32 ramc_lpr;
> > >  };
> > > 
> > > +#define to_at91_reset(r)       container_of(r, struct
> > > at91_reset, rcdev)
> > > +
> > >  struct at91_reset_data {
> > >         u32 reset_args;
> > >         u32 n_device_reset;
> > > @@ -191,6 +196,79 @@ static const struct of_device_id
> > > at91_reset_of_match[] = {
> > >  };
> > >  MODULE_DEVICE_TABLE(of, at91_reset_of_match);
> > > 
> > > +static int at91_reset_update(struct reset_controller_dev *rcdev,
> > > +                            unsigned long id, bool assert)
> > > +{
> > > +       struct at91_reset *reset = to_at91_reset(rcdev);
> > > +       u32 val;
> > > +
> > > +       val = readl_relaxed(reset->dev_base);
> > > +       if (assert)
> > > +               val |= BIT(id);
> > > +       else
> > > +               val &= ~BIT(id);
> > > +       writel_relaxed(val, reset->dev_base);
> > 
> > This read-modify-update should be protected by a spinlock.
> > 
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int at91_reset_assert(struct reset_controller_dev *rcdev,
> > > +                            unsigned long id)
> > > +{
> > > +       return at91_reset_update(rcdev, id, true);
> > > +}
> > > +
> > > +static int at91_reset_deassert(struct reset_controller_dev
> > > *rcdev,
> > > +                              unsigned long id)
> > > +{
> > > +       return at91_reset_update(rcdev, id, false);
> > > +}
> > > +
> > > +static int at91_reset_dev_status(struct reset_controller_dev
> > > *rcdev,
> > > +                                unsigned long id)
> > > +{
> > > +       struct at91_reset *reset = to_at91_reset(rcdev);
> > > +       u32 val;
> > > +
> > > +       val = readl_relaxed(reset->dev_base);
> > > +
> > > +       return !!(val & BIT(id));
> > > +}
> > > +
> > > +static const struct reset_control_ops at91_reset_ops = {
> > > +       .assert = at91_reset_assert,
> > > +       .deassert = at91_reset_deassert,
> > > +       .status = at91_reset_dev_status,
> > > +};
> > > +
> > > +static int at91_reset_of_xlate(struct reset_controller_dev
> > > *rcdev,
> > > +                              const struct of_phandle_args
> > > *reset_spec)
> > > +{
> > > +       return reset_spec->args[0];
> > > +}
> > 
> > For 1:1 mappings there is no need for a custom of_xlate handler.
> > Just
> > leave of_xlate and of_reset_n_cells empty.
> 
> I've double checked that. This would work if reset ids are continuous
> from
> zero to rcdev.nr_resets. This the of_reset_simple_xlate:
> 
> static int of_reset_simple_xlate(struct reset_controller_dev *rcdev,
>                                  const struct of_phandle_args
> *reset_spec)
> {
>         if (reset_spec->args[0] >= rcdev->nr_resets)
>                 return -EINVAL;
>         return reset_spec->args[0];
> }
> 
> But in this driver's case we have 3 ids: 4, 5, 6. That is the reason
> I had this simple xlate function.

I see. In that case I'd say keep the custom of_xlate but let it return
-EINVAL if the args[0] value is not 4, 5, or 6.

Or you could set nr_resets to 7, but unless there are more resets at
the lower bits, that wouldn't necessarily be better.

regards
Philipp

Re: [PATCH 6/8] power: reset: at91-reset: add reset_controller_dev support

[Claudiu.Beznea]

On 05.04.2022 18:15, Philipp Zabel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Claudio,
> 
> On Di, 2022-04-05 at 14:47 +0000, Claudiu.Beznea@microchip.com wrote:
>> Hi, Philipp,
>>
>> On 05.04.2022 14:47, Philipp Zabel wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>> know the content is safe
>>>
>>> Hi Claudiu,
>>>
>>> On Di, 2022-04-05 at 14:27 +0300, Claudiu Beznea wrote:
>>>> SAMA7G5 reset controller has 5 extra lines that goes to different
>>>> devices
>>>> (3 lines to USB PHYs, 1 line to DDR controller, one line DDR PHY
>>>> controller). These reset lines could be requested by different
>>>> controller
>>>> drivers (e.g. USB PHY driver) and these controllers' drivers
>>>> could
>>>> assert/deassert these lines when necessary. Thus add support for
>>>> reset_controller_dev which brings this functionality.
>>>>
>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>>> ---
>>>>  drivers/power/reset/at91-reset.c | 92
>>>> ++++++++++++++++++++++++++++++--
>>>>  1 file changed, 88 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/power/reset/at91-reset.c
>>>> b/drivers/power/reset/at91-reset.c
>>>> index 0d721e27f545..b04df54c15d2 100644
>>>> --- a/drivers/power/reset/at91-reset.c
>>>> +++ b/drivers/power/reset/at91-reset.c
>>>> @@ -17,6 +17,7 @@
>>>>  #include <linux/of_address.h>
>>>>  #include <linux/platform_device.h>
>>>>  #include <linux/reboot.h>
>>>> +#include <linux/reset-controller.h>
>>>>
>>>>  #include <soc/at91/at91sam9_ddrsdr.h>
>>>>  #include <soc/at91/at91sam9_sdramc.h>
>>>> @@ -53,12 +54,16 @@ enum reset_type {
>>>>  struct at91_reset {
>>>>         void __iomem *rstc_base;
>>>>         void __iomem *ramc_base[2];
>>>> +       void __iomem *dev_base;
>>>> +       struct reset_controller_dev rcdev;
>>>>         struct clk *sclk;
>>>>         struct notifier_block nb;
>>>>         u32 args;
>>>>         u32 ramc_lpr;
>>>>  };
>>>>
>>>> +#define to_at91_reset(r)       container_of(r, struct
>>>> at91_reset, rcdev)
>>>> +
>>>>  struct at91_reset_data {
>>>>         u32 reset_args;
>>>>         u32 n_device_reset;
>>>> @@ -191,6 +196,79 @@ static const struct of_device_id
>>>> at91_reset_of_match[] = {
>>>>  };
>>>>  MODULE_DEVICE_TABLE(of, at91_reset_of_match);
>>>>
>>>> +static int at91_reset_update(struct reset_controller_dev *rcdev,
>>>> +                            unsigned long id, bool assert)
>>>> +{
>>>> +       struct at91_reset *reset = to_at91_reset(rcdev);
>>>> +       u32 val;
>>>> +
>>>> +       val = readl_relaxed(reset->dev_base);
>>>> +       if (assert)
>>>> +               val |= BIT(id);
>>>> +       else
>>>> +               val &= ~BIT(id);
>>>> +       writel_relaxed(val, reset->dev_base);
>>>
>>> This read-modify-update should be protected by a spinlock.
>>>
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static int at91_reset_assert(struct reset_controller_dev *rcdev,
>>>> +                            unsigned long id)
>>>> +{
>>>> +       return at91_reset_update(rcdev, id, true);
>>>> +}
>>>> +
>>>> +static int at91_reset_deassert(struct reset_controller_dev
>>>> *rcdev,
>>>> +                              unsigned long id)
>>>> +{
>>>> +       return at91_reset_update(rcdev, id, false);
>>>> +}
>>>> +
>>>> +static int at91_reset_dev_status(struct reset_controller_dev
>>>> *rcdev,
>>>> +                                unsigned long id)
>>>> +{
>>>> +       struct at91_reset *reset = to_at91_reset(rcdev);
>>>> +       u32 val;
>>>> +
>>>> +       val = readl_relaxed(reset->dev_base);
>>>> +
>>>> +       return !!(val & BIT(id));
>>>> +}
>>>> +
>>>> +static const struct reset_control_ops at91_reset_ops = {
>>>> +       .assert = at91_reset_assert,
>>>> +       .deassert = at91_reset_deassert,
>>>> +       .status = at91_reset_dev_status,
>>>> +};
>>>> +
>>>> +static int at91_reset_of_xlate(struct reset_controller_dev
>>>> *rcdev,
>>>> +                              const struct of_phandle_args
>>>> *reset_spec)
>>>> +{
>>>> +       return reset_spec->args[0];
>>>> +}
>>>
>>> For 1:1 mappings there is no need for a custom of_xlate handler.
>>> Just
>>> leave of_xlate and of_reset_n_cells empty.
>>
>> I've double checked that. This would work if reset ids are continuous
>> from
>> zero to rcdev.nr_resets. This the of_reset_simple_xlate:
>>
>> static int of_reset_simple_xlate(struct reset_controller_dev *rcdev,
>>                                  const struct of_phandle_args
>> *reset_spec)
>> {
>>         if (reset_spec->args[0] >= rcdev->nr_resets)
>>                 return -EINVAL;
>>         return reset_spec->args[0];
>> }
>>
>> But in this driver's case we have 3 ids: 4, 5, 6. That is the reason
>> I had this simple xlate function.
> 
> I see. In that case I'd say keep the custom of_xlate but let it return
> -EINVAL if the args[0] value is not 4, 5, or 6.

I will go for this approach (I though of it initially but let aside after)
to also protect the other 2 resets (DDR controller and DDR PHY controller)
which are at bits 0 and 2 in register at rstc->dev_base.

Thank you,
Claudiu Beznea

> 
> Or you could set nr_resets to 7, but unless there are more resets at
> the lower bits, that wouldn't necessarily be better.
> 
> regards
> Philipp

[PATCH 7/8] power: reset: at91-reset: add support for SAMA7G5

[Claudiu Beznea]

Add support for SAMA7G5 including reset_controller_dev support for 3 lines
(which are USB PHYs).

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/power/reset/at91-reset.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index b04df54c15d2..45313acf0c55 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -171,6 +171,11 @@ static const struct at91_reset_data samx7 = {
 	.reset_args = AT91_RSTC_KEY | AT91_RSTC_PROCRST,
 };
 
+static const struct at91_reset_data sama7g5 = {
+	.reset_args = AT91_RSTC_KEY | AT91_RSTC_PROCRST,
+	.n_device_reset = 3,
+};
+
 static const struct of_device_id at91_reset_of_match[] = {
 	{
 		.compatible = "atmel,at91sam9260-rstc",
@@ -192,6 +197,10 @@ static const struct of_device_id at91_reset_of_match[] = {
 		.compatible = "microchip,sam9x60-rstc",
 		.data = &samx7,
 	},
+	{
+		.compatible = "microchip,sama7g5-rstc",
+		.data = &sama7g5,
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, at91_reset_of_match);
-- 
2.32.0

[PATCH 8/8] ARM: dts: at91: sama7g5: add reset-controller node

[Claudiu Beznea]

Add reset controller node.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 arch/arm/boot/dts/sama7g5.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/sama7g5.dtsi b/arch/arm/boot/dts/sama7g5.dtsi
index eddcfbf4d223..aa0e72d4d2d5 100644
--- a/arch/arm/boot/dts/sama7g5.dtsi
+++ b/arch/arm/boot/dts/sama7g5.dtsi
@@ -122,6 +122,13 @@ pmc: pmc@e0018000 {
 			clock-names = "td_slck", "md_slck", "main_xtal";
 		};
 
+		reset_controller: reset-controller@e001d000 {
+			compatible = "microchip,sama7g5-rstc";
+			reg = <0xe001d000 0xc>, <0xe001d0e4 0x4>;
+			#reset-cells = <1>;
+			clocks = <&clk32k 0>;
+		};
+
 		shdwc: shdwc@e001d010 {
 			compatible = "microchip,sama7g5-shdwc", "syscon";
 			reg = <0xe001d010 0x10>;
-- 
2.32.0