[PATCH] reset: ti-rstctrl: use the reset-simple driver

[PATCH] reset: ti-rstctrl: use the reset-simple driver

[Tony Lindgren]

We can support the RSTCTRL reset registers on many TI SoCs with
reset-simple.

Cc: Dave Gerlach <d-gerlach@ti.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Nishant Menon <nm@ti.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Suman Anna <s-anna@ti.com>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---

That's all there is to it :) Naturally this can wait for v4.17
if necessary.

---
 .../devicetree/bindings/reset/ti-rstctrl.txt       | 29 ++++++++++++++++++++++
 drivers/reset/Kconfig                              |  2 +-
 drivers/reset/reset-simple.c                       |  1 +
 3 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/reset/ti-rstctrl.txt

diff --git a/Documentation/devicetree/bindings/reset/ti-rstctrl.txt b/Documentation/devicetree/bindings/reset/ti-rstctrl.txt
new file mode 100644
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/ti-rstctrl.txt
@@ -0,0 +1,29 @@
+TI RSTCTRL Reset Controller
+
+Required properties:
+- compatible : "ti,rstctrl"
+- reg : Should contain 1 register ranges(address and length)
+- #reset-cells: 1
+
+Example:
+
+	prcm: prcm@200000 {
+		compatible = "ti,am3-prcm", "simple-bus";
+		reg = <0x200000 0x4000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0x200000 0x4000>;
+
+		prm_gfx: prm@1100 {
+			compatible = "simple-bus";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0 0x1100 0x100>;
+
+			gfx_rstctrl: rstctrl@4 {
+				reg = <0x4 0x4>;
+				#reset-cells = <1>;
+				compatible = "ti,rstctrl";
+			};
+		};
+	};
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -83,7 +83,7 @@ config RESET_PISTACHIO
 
 config RESET_SIMPLE
 	bool "Simple Reset Controller Driver" if COMPILE_TEST
-	default ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX
+	default ARCH_OMAP2PLUS || ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX
 	help
 	  This enables a simple reset controller driver for reset lines that
 	  that can be asserted and deasserted by toggling bits in a contiguous,
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -123,6 +123,7 @@ static const struct of_device_id reset_simple_dt_ids[] = {
 	{ .compatible = "st,stm32-rcc", },
 	{ .compatible = "allwinner,sun6i-a31-clock-reset",
 		.data = &reset_simple_active_low },
+	{ .compatible = "ti,rstctrl", },
 	{ .compatible = "zte,zx296718-reset",
 		.data = &reset_simple_active_low },
 	{ /* sentinel */ },
-- 
2.15.0

Re: [PATCH] reset: ti-rstctrl: use the reset-simple driver

[Tero Kristo]

On 16/01/18 03:11, Tony Lindgren wrote:
> We can support the RSTCTRL reset registers on many TI SoCs with
> reset-simple.
> 
> Cc: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Nishant Menon <nm-l0cyMroinI0@public.gmane.org>
> Cc: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
> Cc: Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> ---
> 
> That's all there is to it :) Naturally this can wait for v4.17
> if necessary.

Thats pretty neat. :)

-Tero

> 
> ---
>   .../devicetree/bindings/reset/ti-rstctrl.txt       | 29 ++++++++++++++++++++++
>   drivers/reset/Kconfig                              |  2 +-
>   drivers/reset/reset-simple.c                       |  1 +
>   3 files changed, 31 insertions(+), 1 deletion(-)
>   create mode 100644 Documentation/devicetree/bindings/reset/ti-rstctrl.txt
> 
> diff --git a/Documentation/devicetree/bindings/reset/ti-rstctrl.txt b/Documentation/devicetree/bindings/reset/ti-rstctrl.txt
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/ti-rstctrl.txt
> @@ -0,0 +1,29 @@
> +TI RSTCTRL Reset Controller
> +
> +Required properties:
> +- compatible : "ti,rstctrl"
> +- reg : Should contain 1 register ranges(address and length)
> +- #reset-cells: 1
> +
> +Example:
> +
> +	prcm: prcm@200000 {
> +		compatible = "ti,am3-prcm", "simple-bus";
> +		reg = <0x200000 0x4000>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0 0x200000 0x4000>;
> +
> +		prm_gfx: prm@1100 {
> +			compatible = "simple-bus";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges = <0 0x1100 0x100>;
> +
> +			gfx_rstctrl: rstctrl@4 {
> +				reg = <0x4 0x4>;
> +				#reset-cells = <1>;
> +				compatible = "ti,rstctrl";
> +			};
> +		};
> +	};
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -83,7 +83,7 @@ config RESET_PISTACHIO
>   
>   config RESET_SIMPLE
>   	bool "Simple Reset Controller Driver" if COMPILE_TEST
> -	default ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX
> +	default ARCH_OMAP2PLUS || ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX
>   	help
>   	  This enables a simple reset controller driver for reset lines that
>   	  that can be asserted and deasserted by toggling bits in a contiguous,
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -123,6 +123,7 @@ static const struct of_device_id reset_simple_dt_ids[] = {
>   	{ .compatible = "st,stm32-rcc", },
>   	{ .compatible = "allwinner,sun6i-a31-clock-reset",
>   		.data = &reset_simple_active_low },
> +	{ .compatible = "ti,rstctrl", },
>   	{ .compatible = "zte,zx296718-reset",
>   		.data = &reset_simple_active_low },
>   	{ /* sentinel */ },
> 

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] reset: ti-rstctrl: use the reset-simple driver

[Philipp Zabel]

Hi Tony,

On Mon, 2018-01-15 at 17:11 -0800, Tony Lindgren wrote:
> We can support the RSTCTRL reset registers on many TI SoCs with
> reset-simple.
> 
> Cc: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Nishant Menon <nm-l0cyMroinI0@public.gmane.org>
> Cc: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
> Cc: Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> ---
> 
> That's all there is to it :) Naturally this can wait for v4.17
> if necessary.
> 
> ---
>  .../devicetree/bindings/reset/ti-rstctrl.txt       | 29 ++++++++++++++++++++++
>  drivers/reset/Kconfig                              |  2 +-
>  drivers/reset/reset-simple.c                       |  1 +
>  3 files changed, 31 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/reset/ti-rstctrl.txt
> 
> diff --git a/Documentation/devicetree/bindings/reset/ti-rstctrl.txt b/Documentation/devicetree/bindings/reset/ti-rstctrl.txt
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/ti-rstctrl.txt
> @@ -0,0 +1,29 @@
> +TI RSTCTRL Reset Controller
> +
> +Required properties:
> +- compatible : "ti,rstctrl"
> +- reg : Should contain 1 register ranges(address and length)
> +- #reset-cells: 1
> +
> +Example:
> +
> +	prcm: prcm@200000 {
> +		compatible = "ti,am3-prcm", "simple-bus";
> +		reg = <0x200000 0x4000>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0 0x200000 0x4000>;
> +
> +		prm_gfx: prm@1100 {
> +			compatible = "simple-bus";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges = <0 0x1100 0x100>;
> +
> +			gfx_rstctrl: rstctrl@4 {
   			,->
> +			|	reg = <0x4 0x4>;
> +			|	#reset-cells = <1>;
> +			`--	compatible = "ti,rstctrl";

Looks good to me. Can I move the compatible property when applying?

> +			};
> +		};
> +	};
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -83,7 +83,7 @@ config RESET_PISTACHIO
>  
>  config RESET_SIMPLE
>  	bool "Simple Reset Controller Driver" if COMPILE_TEST
> -	default ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX
> +	default ARCH_OMAP2PLUS || ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX
>  	help
>  	  This enables a simple reset controller driver for reset lines that
>  	  that can be asserted and deasserted by toggling bits in a contiguous,
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -123,6 +123,7 @@ static const struct of_device_id reset_simple_dt_ids[] = {
>  	{ .compatible = "st,stm32-rcc", },
>  	{ .compatible = "allwinner,sun6i-a31-clock-reset",
>  		.data = &reset_simple_active_low },
> +	{ .compatible = "ti,rstctrl", },
>  	{ .compatible = "zte,zx296718-reset",
>  		.data = &reset_simple_active_low },
>  	{ /* sentinel */ },

regards
Philipp
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] reset: ti-rstctrl: use the reset-simple driver

[Tony Lindgren]

Hi,

* Philipp Zabel <p.zabel@pengutronix.de> [180116 09:52]:
> On Mon, 2018-01-15 at 17:11 -0800, Tony Lindgren wrote:
> > +Example:
> > +
> > +	prcm: prcm@200000 {
> > +		compatible = "ti,am3-prcm", "simple-bus";
> > +		reg = <0x200000 0x4000>;
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		ranges = <0 0x200000 0x4000>;
> > +
> > +		prm_gfx: prm@1100 {
> > +			compatible = "simple-bus";
> > +			#address-cells = <1>;
> > +			#size-cells = <1>;
> > +			ranges = <0 0x1100 0x100>;
> > +
> > +			gfx_rstctrl: rstctrl@4 {
>    			,->
> > +			|	reg = <0x4 0x4>;
> > +			|	#reset-cells = <1>;
> > +			`--	compatible = "ti,rstctrl";
> 
> Looks good to me. Can I move the compatible property when applying?

Oops, here's a better version. I also left out the "prcm" part as
at some point that should have just ranges instead of both reg and
ranges that it currently has.

Regards,

Tony

8< -----------------------
>From tony Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Mon, 15 Jan 2018 15:25:54 -0800
Subject: [PATCHv2] reset: ti-rstctrl: use the reset-simple driver

We can support the RSTCTRL reset registers on many TI SoCs with
reset-simple.

Cc: Dave Gerlach <d-gerlach@ti.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Nishant Menon <nm@ti.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Suman Anna <s-anna@ti.com>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 .../devicetree/bindings/reset/ti-rstctrl.txt         | 20 ++++++++++++++++++++
 drivers/reset/Kconfig                                |  2 +-
 drivers/reset/reset-simple.c                         |  1 +
 3 files changed, 22 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/reset/ti-rstctrl.txt

diff --git a/Documentation/devicetree/bindings/reset/ti-rstctrl.txt b/Documentation/devicetree/bindings/reset/ti-rstctrl.txt
new file mode 100644
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/ti-rstctrl.txt
@@ -0,0 +1,20 @@
+TI RSTCTRL Reset Controller
+
+Required properties:
+- compatible : "ti,rstctrl"
+- reg : Should contain 1 register ranges(address and length)
+- #reset-cells: 1
+
+Example:
+	prm_gfx: prm@1100 {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0x1100 0x100>;
+
+		gfx_rstctrl: rstctrl@4 {
+			compatible = "ti,rstctrl";
+			reg = <0x4 0x4>;
+			#reset-cells = <1>;
+		};
+	};
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -83,7 +83,7 @@ config RESET_PISTACHIO
 
 config RESET_SIMPLE
 	bool "Simple Reset Controller Driver" if COMPILE_TEST
-	default ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX
+	default ARCH_OMAP2PLUS || ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX
 	help
 	  This enables a simple reset controller driver for reset lines that
 	  that can be asserted and deasserted by toggling bits in a contiguous,
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -123,6 +123,7 @@ static const struct of_device_id reset_simple_dt_ids[] = {
 	{ .compatible = "st,stm32-rcc", },
 	{ .compatible = "allwinner,sun6i-a31-clock-reset",
 		.data = &reset_simple_active_low },
+	{ .compatible = "ti,rstctrl", },
 	{ .compatible = "zte,zx296718-reset",
 		.data = &reset_simple_active_low },
 	{ /* sentinel */ },
-- 
2.15.0

Re: [PATCH] reset: ti-rstctrl: use the reset-simple driver

[Suman Anna]

Hi Tony,

On 01/16/2018 09:03 AM, Tony Lindgren wrote:
> Hi,
> 
> * Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> [180116 09:52]:
>> On Mon, 2018-01-15 at 17:11 -0800, Tony Lindgren wrote:
>>> +Example:
>>> +
>>> +	prcm: prcm@200000 {
>>> +		compatible = "ti,am3-prcm", "simple-bus";
>>> +		reg = <0x200000 0x4000>;
>>> +		#address-cells = <1>;
>>> +		#size-cells = <1>;
>>> +		ranges = <0 0x200000 0x4000>;
>>> +
>>> +		prm_gfx: prm@1100 {
>>> +			compatible = "simple-bus";
>>> +			#address-cells = <1>;
>>> +			#size-cells = <1>;
>>> +			ranges = <0 0x1100 0x100>;
>>> +
>>> +			gfx_rstctrl: rstctrl@4 {
>>    			,->
>>> +			|	reg = <0x4 0x4>;
>>> +			|	#reset-cells = <1>;
>>> +			`--	compatible = "ti,rstctrl";
>>
>> Looks good to me. Can I move the compatible property when applying?
> 
> Oops, here's a better version. I also left out the "prcm" part as
> at some point that should have just ranges instead of both reg and
> ranges that it currently has.

While this adaptation is very simple for replacing the RSTCTRL registers
from the hwmod data into an existing reset driver, I am afraid that it
doesn't fit well when you want to use the reset API from client drivers.
The RSTST is not accounted for (which is what we rely on for saying that
a deassert is successful), and this is currently only replacing part of
the omap4_prminst_{assert/deassert}_hardreset functionality, which in
itself is only a small portion of what the current drivers use
(omap_hwmod_{assert/deassert}_hardreset() functions.

regards
Suman

> 
> Regards,
> 
> Tony
> 
> 8< -----------------------
> From tony Mon Sep 17 00:00:00 2001
> From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> Date: Mon, 15 Jan 2018 15:25:54 -0800
> Subject: [PATCHv2] reset: ti-rstctrl: use the reset-simple driver
> 
> We can support the RSTCTRL reset registers on many TI SoCs with
> reset-simple.
> 
> Cc: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Nishant Menon <nm-l0cyMroinI0@public.gmane.org>
> Cc: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
> Cc: Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> ---
>  .../devicetree/bindings/reset/ti-rstctrl.txt         | 20 ++++++++++++++++++++
>  drivers/reset/Kconfig                                |  2 +-
>  drivers/reset/reset-simple.c                         |  1 +
>  3 files changed, 22 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/reset/ti-rstctrl.txt
> 
> diff --git a/Documentation/devicetree/bindings/reset/ti-rstctrl.txt b/Documentation/devicetree/bindings/reset/ti-rstctrl.txt
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/ti-rstctrl.txt
> @@ -0,0 +1,20 @@
> +TI RSTCTRL Reset Controller
> +
> +Required properties:
> +- compatible : "ti,rstctrl"
> +- reg : Should contain 1 register ranges(address and length)
> +- #reset-cells: 1
> +
> +Example:
> +	prm_gfx: prm@1100 {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0 0x1100 0x100>;
> +
> +		gfx_rstctrl: rstctrl@4 {
> +			compatible = "ti,rstctrl";
> +			reg = <0x4 0x4>;
> +			#reset-cells = <1>;
> +		};
> +	};
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -83,7 +83,7 @@ config RESET_PISTACHIO
>  
>  config RESET_SIMPLE
>  	bool "Simple Reset Controller Driver" if COMPILE_TEST
> -	default ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX
> +	default ARCH_OMAP2PLUS || ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX
>  	help
>  	  This enables a simple reset controller driver for reset lines that
>  	  that can be asserted and deasserted by toggling bits in a contiguous,
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -123,6 +123,7 @@ static const struct of_device_id reset_simple_dt_ids[] = {
>  	{ .compatible = "st,stm32-rcc", },
>  	{ .compatible = "allwinner,sun6i-a31-clock-reset",
>  		.data = &reset_simple_active_low },
> +	{ .compatible = "ti,rstctrl", },
>  	{ .compatible = "zte,zx296718-reset",
>  		.data = &reset_simple_active_low },
>  	{ /* sentinel */ },
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] reset: ti-rstctrl: use the reset-simple driver

[Tony Lindgren]

Hi,

* Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [180116 21:23]:
> While this adaptation is very simple for replacing the RSTCTRL registers
> from the hwmod data into an existing reset driver, I am afraid that it
> doesn't fit well when you want to use the reset API from client drivers.

Well the reset controller framework is the Linux API for client
drivers though so it already works well for the client drivers :)

> The RSTST is not accounted for (which is what we rely on for saying that
> a deassert is successful), and this is currently only replacing part of
> the omap4_prminst_{assert/deassert}_hardreset functionality, which in
> itself is only a small portion of what the current drivers use
> (omap_hwmod_{assert/deassert}_hardreset() functions.

Seems like that's a different set of patches as the RSTST
registers require some API changes to the reset controller
framework or runtime PM.

The RSTST registers mostly tell the device internal reset reason
like watchdog reset for an accelerator. I'm not sure how the
API for those would look like, do you have some ideas?

Hmm, aren't you currently just reading the RSTST registers
directly for remoteproc etc?

And then we also have the CONTEXT register that tells if module
context was lost during idle :) The API for that could simply be
bool device_context_lost(struct device *dev) to describe that
kind of reset. Or it could maybe also use reset_control_status()
that returns -ECONNRESET? But then what resets the context lost
state as we don't need to reset the device?

It almost seems like the RSTST and CONTEXT should be client device
PM state related registers instead of reset controller registers?
But if we figure out the API them, then accessing them could
be added for "ti,rstctrl" compatible with:

reg = <0x4 0x4>,
       0x14 0x4>,
       0x24 0x4>;
reg-names = "rstctrl", "rststs", "context";

Anyways, for managing various rstctrl bits and OCP softresets,
here's what I was thinking we can do:

1. We keep ocp softreset internal to the interconnect target module
   driver and if child device drivers really need to access that we
   can implement reset driver in the interconnect target module

2. For rstctrl bits that are for a whole interconnect target module
   we access them in the interconnect target module and the child
   device driver can then do device_reset(dev->parent) if needed

3. For child module specific rstctrl bits we can have the child
   devices map them directly in dts if the interconnect target
   module does not need to worry about them

I briefly tested this yesterday with am33xx sgx module just to
read some registers. Here are the dts nodes that I used for
reference, I'll be posting the related ti-sysc driver changes
after -rc1.

prm_gfx: prm@1100 {
	compatible = "simple-bus";
	#address-cells = <1>;
	#size-cells = <1>;
	ranges = <0 0x1100 0x100>;

	gfx_rstctrl: rstctrl@4 {
		compatible = "ti,rstctrl";
		reg = <0x4 0x4>;
		#reset-cells = <1>;
	};
};

target-module@56000000 {
	compatible = "ti,sysc-omap4", "ti,sysc";
	ti,hwmods = "gfx";
	reg = <0x5600fe00 0x4>,
	      <0x5600fe10 0x4>;
	reg-names = "rev", "sysc";
	ti,sysc-midle = <SYSC_IDLE_FORCE>,
			<SYSC_IDLE_NO>,
			<SYSC_IDLE_SMART>;
	ti,sysc-sidle = <SYSC_IDLE_FORCE>,
			<SYSC_IDLE_NO>,
			<SYSC_IDLE_SMART>;
	clocks = <&gfx_l3_clkctrl AM3_GFX_CLKCTRL 0>;
	clock-names = "fck";
	resets = <&gfx_rstctrl 0>;
	reset-names = "rstctrl";
	#address-cells = <1>;
	#size-cells = <1>;
	ranges = <0 0x56000000 0x1000000>;

	/*
	 * Closed source PowerVR driver, no child device
	 * binding or driver in mainline
	 */
};

So if we had a child device driver there and the gfx_rstctrl
had other child device specific reset bits, they could be
properties of the child device with resets = <&gfx_rstctrl 1>
and so on. So hopefully that clarifies how the client drivers
would use the resets?

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] reset: ti-rstctrl: use the reset-simple driver

[Suman Anna]

Hi Tony,

On 01/16/2018 05:22 PM, Tony Lindgren wrote:
> Hi,
> 
> * Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [180116 21:23]:
>> While this adaptation is very simple for replacing the RSTCTRL registers
>> from the hwmod data into an existing reset driver, I am afraid that it
>> doesn't fit well when you want to use the reset API from client drivers.
> 
> Well the reset controller framework is the Linux API for client
> drivers though so it already works well for the client drivers :)
> 
>> The RSTST is not accounted for (which is what we rely on for saying that
>> a deassert is successful), and this is currently only replacing part of
>> the omap4_prminst_{assert/deassert}_hardreset functionality, which in
>> itself is only a small portion of what the current drivers use
>> (omap_hwmod_{assert/deassert}_hardreset() functions.
> 
> Seems like that's a different set of patches as the RSTST
> registers require some API changes to the reset controller
> framework or runtime PM.
> 
> The RSTST registers mostly tell the device internal reset reason
> like watchdog reset for an accelerator. I'm not sure how the
> API for those would look like, do you have some ideas?

There are multiple RSTST registers, each of which with different bits. .
The PRM_RSTST is the one where you can catch the reset reasons (along
with few others), but otherwise most of the other IP-specific RSTST
registers capture the local reset status. The RSTST behavior is somewhat
similar to how the softreset status is tracked on each IP's SYSC/SYSS
registers. It does tell the reset status for some software triggered
resets in RSTCTRL, and can have additional bits as well for some PRCM
triggered h/w resets (like RETention reset).

> 
> Hmm, aren't you currently just reading the RSTST registers
> directly for remoteproc etc?

Nope, all the PRCM related registers are hidden away underneath the
hwmod layer, so the only code that I use is pm_runtime_{get/put}_sync()
directly and omap_device_{assert/deassert}_reset() through pdata-quirks.
Take a look in drivers/iommu/omap-iommu.c and
arch/arm/mach-omap2/pdata-quirks.c (functional in mainline for OMAP4 and
OMAP5 DSP/IPU MMUs).

You can look through the reset sequences for any of DSP/IPU/IVA in any
of the TRMs, and you will notice that the various steps involve CLKCTRL,
CLKSTCTRL and RSTCTRL registers. There is a small delay between the
reset being released to having the module actually be out of reset, and
this is the status that is reflected in RSTST, so we do have to wait for
this. Today, this is automatically handled in hwmod code (See
omap4_prminst_deassert_hardreset()). The other register sequences are in
_deassert_hardreset() in omap_hwmod.c.

This design is just implementing the omap4_prminst_rmw_inst_reg_bits
line in omap4_prminst_deassert_hardreset(), and as such client drivers
cannot really use it as is without access to all the other registers
which have so far been hidden away underneath the hwmod layer.

> 
> And then we also have the CONTEXT register that tells if module
> context was lost during idle :) The API for that could simply be
> bool device_context_lost(struct device *dev) to describe that
> kind of reset. Or it could maybe also use reset_control_status()
> that returns -ECONNRESET? But then what resets the context lost
> state as we don't need to reset the device?

I am not sure why the CONTEXT register belongs to the same discussion as
the reset registers. In any case, I don't think any of the current code
relies on this (other than incrementing the debug counters).

> 
> It almost seems like the RSTST and CONTEXT should be client device
> PM state related registers instead of reset controller registers?
> But if we figure out the API them, then accessing them could
> be added for "ti,rstctrl" compatible with:
> 
> reg = <0x4 0x4>,
>        0x14 0x4>,
>        0x24 0x4>;
> reg-names = "rstctrl", "rststs", "context";

For K2 SoCs, we have done a ti-syscon-reset driver with these various
rstctrl and rststs registers and their bit offsets and masks (low or
high) incorporated. The reset integration is fairly straight-forward
there though as it is based on PSC and not PRCM, so there are not many
different registers involved.

> 
> Anyways, for managing various rstctrl bits and OCP softresets,
> here's what I was thinking we can do:
> 
> 1. We keep ocp softreset internal to the interconnect target module
>    driver and if child device drivers really need to access that we
>    can implement reset driver in the interconnect target module

Note that for IPs with hard-reset, you cannot program this register
unless the PRCM reset line is taken care of first. This is the case with
DSP/IPU MMUs for example (there's both a PRCM RSTCTRL as well as the
OCP_SOFTRESET within the MMU_SYSCONFIG, which only resets the MMU
sub-mdule registers). Also, the MODULEMODE and CLKCTRL registers apply
for the whole subsystem (DSP), while the OCP softreset bits apply only
to a child device (MMU) with the other child device being the DSP core
with your design.

> 
> 2. For rstctrl bits that are for a whole interconnect target module
>    we access them in the interconnect target module and the child
>    device driver can then do device_reset(dev->parent) if needed.
> 
> 3. For child module specific rstctrl bits we can have the child
>    devices map them directly in dts if the interconnect target
>    module does not need to worry about them
> 
> I briefly tested this yesterday with am33xx sgx module just to
> read some registers. Here are the dts nodes that I used for
> reference, I'll be posting the related ti-sysc driver changes
> after -rc1.

The OMAP IOMMU would be a better test given that there's no in-kernel
sgx module and we are not sure if the out-of-tree driver is functional
with these changes.

I have some unit-test code at
https://github.com/sumananna/omap-test-iommu.git

You would need to add a test node to DTS with reference to the MMU you
want to test.

> 
> prm_gfx: prm@1100 {
> 	compatible = "simple-bus";
> 	#address-cells = <1>;
> 	#size-cells = <1>;
> 	ranges = <0 0x1100 0x100>;
> 
> 	gfx_rstctrl: rstctrl@4 {
> 		compatible = "ti,rstctrl";
> 		reg = <0x4 0x4>;
> 		#reset-cells = <1>;
> 	};
> };
> 
> target-module@56000000 {
> 	compatible = "ti,sysc-omap4", "ti,sysc";
> 	ti,hwmods = "gfx";
> 	reg = <0x5600fe00 0x4>,
> 	      <0x5600fe10 0x4>;
> 	reg-names = "rev", "sysc";
> 	ti,sysc-midle = <SYSC_IDLE_FORCE>,
> 			<SYSC_IDLE_NO>,
> 			<SYSC_IDLE_SMART>;
> 	ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> 			<SYSC_IDLE_NO>,
> 			<SYSC_IDLE_SMART>;
> 	clocks = <&gfx_l3_clkctrl AM3_GFX_CLKCTRL 0>;
> 	clock-names = "fck";
> 	resets = <&gfx_rstctrl 0>;
> 	reset-names = "rstctrl";
> 	#address-cells = <1>;
> 	#size-cells = <1>;
> 	ranges = <0 0x56000000 0x1000000>;
> 
> 	/*
> 	 * Closed source PowerVR driver, no child device
> 	 * binding or driver in mainline
> 	 */
> };
> 
> So if we had a child device driver there and the gfx_rstctrl
> had other child device specific reset bits, they could be
> properties of the child device with resets = <&gfx_rstctrl 1>
> and so on. So hopefully that clarifies how the client drivers
> would use the resets?

If we are exposing all the other registers directly to client drivers,
then the client drivers can use the reset API directly. But if other
registers are hidden away underneath various frameworks, then we have
some sequencing issues.

Tero had done a series sometime back using notifiers to take care of
these sequencing steps, but that was deferred pending the other hwmod
cleanup. Last reference that touches it is here,
https://marc.info/?l=linux-omap&m=147279610902646&w=2

regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] reset: ti-rstctrl: use the reset-simple driver

[Tony Lindgren]

* Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [180119 20:23]:
> On 01/16/2018 05:22 PM, Tony Lindgren wrote:
> > The RSTST registers mostly tell the device internal reset reason
> > like watchdog reset for an accelerator. I'm not sure how the
> > API for those would look like, do you have some ideas?
> 
> There are multiple RSTST registers, each of which with different bits. .
> The PRM_RSTST is the one where you can catch the reset reasons (along
> with few others), but otherwise most of the other IP-specific RSTST
> registers capture the local reset status. The RSTST behavior is somewhat
> similar to how the softreset status is tracked on each IP's SYSC/SYSS
> registers. It does tell the reset status for some software triggered
> resets in RSTCTRL, and can have additional bits as well for some PRCM
> triggered h/w resets (like RETention reset).

Hmm so are you using these "additional" software status bits?

If the other bits don't matter, and the RSTCTRL reset bit has
a matching RSTST bit, then we could just read that bit back
in reset_simple_status and have a separate compatible value
for those like "ti,rstctrl-rstst".

> > Hmm, aren't you currently just reading the RSTST registers
> > directly for remoteproc etc?
> 
> Nope, all the PRCM related registers are hidden away underneath the
> hwmod layer, so the only code that I use is pm_runtime_{get/put}_sync()
> directly and omap_device_{assert/deassert}_reset() through pdata-quirks.
> Take a look in drivers/iommu/omap-iommu.c and
> arch/arm/mach-omap2/pdata-quirks.c (functional in mainline for OMAP4 and
> OMAP5 DSP/IPU MMUs).

OK, sorry I was thinking of the CLKCTRL IDLEST that are directly
accessed :)

> > And then we also have the CONTEXT register that tells if module
> > context was lost during idle :) The API for that could simply be
> > bool device_context_lost(struct device *dev) to describe that
> > kind of reset. Or it could maybe also use reset_control_status()
> > that returns -ECONNRESET? But then what resets the context lost
> > state as we don't need to reset the device?
> 
> I am not sure why the CONTEXT register belongs to the same discussion as
> the reset registers. In any case, I don't think any of the current code
> relies on this (other than incrementing the debug counters).

Well they are in the same PRM instance, and behave the same way
as RSTST :) They are like interrupt status registers where you can
read the state and write to clear it. I wonder if we do really have
a proper PRM interrupt for these too, Tero?

> The OMAP IOMMU would be a better test given that there's no in-kernel
> sgx module and we are not sure if the out-of-tree driver is functional
> with these changes.
> 
> I have some unit-test code at
> https://github.com/sumananna/omap-test-iommu.git
> 
> You would need to add a test node to DTS with reference to the MMU you
> want to test.

OK let's do some tests on that, I'll take a look at doing a dts
file over next few weeks.

> If we are exposing all the other registers directly to client drivers,
> then the client drivers can use the reset API directly. But if other
> registers are hidden away underneath various frameworks, then we have
> some sequencing issues.

Right that needs to be checked. For ti-sysc driver we only need to
care about the rests that are needed to read and configure the
interconnect target module. The rest can be handled directly by
the child device drivers hopefully.

> Tero had done a series sometime back using notifiers to take care of
> these sequencing steps, but that was deferred pending the other hwmod
> cleanup. Last reference that touches it is here,
> https://marc.info/?l=linux-omap&m=147279610902646&w=2

Yeah the resets can all be device driver(s) now that we have the
clkctrl clocks usable and should have ti-sysc usable for v4.17.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] reset: ti-rstctrl: use the reset-simple driver

[Suman Anna]

Hi Tony,

On 01/19/2018 03:33 PM, Tony Lindgren wrote:
> * Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [180119 20:23]:
>> On 01/16/2018 05:22 PM, Tony Lindgren wrote:
>>> The RSTST registers mostly tell the device internal reset reason
>>> like watchdog reset for an accelerator. I'm not sure how the
>>> API for those would look like, do you have some ideas?
>>
>> There are multiple RSTST registers, each of which with different bits. .
>> The PRM_RSTST is the one where you can catch the reset reasons (along
>> with few others), but otherwise most of the other IP-specific RSTST
>> registers capture the local reset status. The RSTST behavior is somewhat
>> similar to how the softreset status is tracked on each IP's SYSC/SYSS
>> registers. It does tell the reset status for some software triggered
>> resets in RSTCTRL, and can have additional bits as well for some PRCM
>> triggered h/w resets (like RETention reset).
> 
> Hmm so are you using these "additional" software status bits?

I am only using the ones associated with the respective software control
bits. For example, DRA7 DSP RSTST has two additional bits associated
with Emulation, but there are no corresponding bits in RSTCTRL.

Similarly, PRM_RSTST has lot more bits other than the global warm and
cold reset trigger sources from s/w. Note that the current reboot code
is directly using the low-level PRM api omap_prm_reset_system(), which
actually acts on the PRM_RSTCTRL. But I see these are usually handled in
drivers/power/reset.

> 
> If the other bits don't matter, and the RSTCTRL reset bit has
> a matching RSTST bit, then we could just read that bit back
> in reset_simple_status and have a separate compatible value
> for those like "ti,rstctrl-rstst".

We will have to cross-check all the RSTST and RSTCTRL instances across
all the AMxx and OMAPxx SoCs to see if the bit positions are exactly
matched up, I believe there are couple of exceptions (either bit
positions or register offsets). This is one of the reasons we used
separate fields in the reset-ti-syscon driver :)

> 
>>> Hmm, aren't you currently just reading the RSTST registers
>>> directly for remoteproc etc?
>>
>> Nope, all the PRCM related registers are hidden away underneath the
>> hwmod layer, so the only code that I use is pm_runtime_{get/put}_sync()
>> directly and omap_device_{assert/deassert}_reset() through pdata-quirks.
>> Take a look in drivers/iommu/omap-iommu.c and
>> arch/arm/mach-omap2/pdata-quirks.c (functional in mainline for OMAP4 and
>> OMAP5 DSP/IPU MMUs).
> 
> OK, sorry I was thinking of the CLKCTRL IDLEST that are directly
> accessed :)

I am not sure if everyone is able to access these normally atleast
before with hwmod code. I did use these in the TI downstream kernel
directly by having an explicit register reference to it in DT, which I
consider a workaround!!

> 
>>> And then we also have the CONTEXT register that tells if module
>>> context was lost during idle :) The API for that could simply be
>>> bool device_context_lost(struct device *dev) to describe that
>>> kind of reset. Or it could maybe also use reset_control_status()
>>> that returns -ECONNRESET? But then what resets the context lost
>>> state as we don't need to reset the device?
>>
>> I am not sure why the CONTEXT register belongs to the same discussion as
>> the reset registers. In any case, I don't think any of the current code
>> relies on this (other than incrementing the debug counters).
> 
> Well they are in the same PRM instance, and behave the same way
> as RSTST :) They are like interrupt status registers where you can
> read the state and write to clear it. I wonder if we do really have
> a proper PRM interrupt for these too, Tero?

They are just status registers, I don't see any interrupts associated
with them. Where are the PWRSTCTRL and PWRSTST registers being managed -
they are also part of the same PRM instances.

> 
>> The OMAP IOMMU would be a better test given that there's no in-kernel
>> sgx module and we are not sure if the out-of-tree driver is functional
>> with these changes.
>>
>> I have some unit-test code at
>> https://github.com/sumananna/omap-test-iommu.git
>>
>> You would need to add a test node to DTS with reference to the MMU you
>> want to test.
> 
> OK let's do some tests on that, I'll take a look at doing a dts
> file over next few weeks.

You can look up the patches folder in the above repo, there are some
example nodes there already, they are rather straight-forward.

> 
>> If we are exposing all the other registers directly to client drivers,
>> then the client drivers can use the reset API directly. But if other
>> registers are hidden away underneath various frameworks, then we have
>> some sequencing issues.
> 
> Right that needs to be checked. For ti-sysc driver we only need to
> care about the rests that are needed to read and configure the
> interconnect target module. The rest can be handled directly by
> the child device drivers hopefully.
> 
>> Tero had done a series sometime back using notifiers to take care of
>> these sequencing steps, but that was deferred pending the other hwmod
>> cleanup. Last reference that touches it is here,
>> https://marc.info/?l=linux-omap&m=147279610902646&w=2
> 
> Yeah the resets can all be device driver(s) now that we have the
> clkctrl clocks usable and should have ti-sysc usable for v4.17.

Yeah ok, hopefully Tero can look into this more now that the clkctrl is
mostly wrapped up.

regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] reset: ti-rstctrl: use the reset-simple driver

[Tony Lindgren]

* Suman Anna <s-anna@ti.com> [180119 23:30]:
> On 01/19/2018 03:33 PM, Tony Lindgren wrote:
> > OK let's do some tests on that, I'll take a look at doing a dts
> > file over next few weeks.
> 
> You can look up the patches folder in the above repo, there are some
> example nodes there already, they are rather straight-forward.

Just for reference, here's what I played with but keep getting
-EPROBE_DEFER somewhere during init.

Regards,

Tony

	prm: prm@6000 {
		...
		ranges = <0 0x6000 0x3000>;

		prm_dsp: prm@400 {
			compatible = "simple-bus";
			#address-cells = <1>;
			#size-cells = <1>;
			ranges = <0 0x400 0x100>;

			dsp_rstctrl: rstctrl@10 {
				compatible = "ti,rstctrl";
				reg = <0x10 0x4>;
				#reset-cells = <1>;
			};
		};
		...
	};

	target-module@4a066000 {
		compatible = "ti,sysc-omap2", "ti,sysc";
		ti,hwmods = "mmu_dsp";
		reg = <0x4a066000 0x4>,
		      <0x4a066010 0x4>,
		      <0x4a066014 0x4>;
		reg-names = "rev", "sysc", "syss";
		ti,sysc-mask = <(SYSC_OMAP2_CLOCKACTIVITY |
				 SYSC_OMAP2_SOFTRESET |
				 SYSC_OMAP2_AUTOIDLE)>;
		ti,sysc-sidle = <SYSC_IDLE_FORCE>,
				<SYSC_IDLE_NO>,
				<SYSC_IDLE_SMART>;
		clocks = <&tesla_clkctrl OMAP4_DSP_CLKCTRL 0>;
		clock-names = "fck";
		resets = <&dsp_rstctrl 1>,
			 <&dsp_rstctrl 0>;
		reset-names = "rst2", "rst1";
		#address-cells = <1>;
		#size-cells = <1>;
		ranges = <0 0x4a066000 0x1000>;

		mmu_dsp: mmu@0 {
			compatible = "ti,omap4-iommu";
			reg = <0 0x100>;
			interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
			#iommu-cells = <0>;
		};
	};

Re: [PATCH] reset: ti-rstctrl: use the reset-simple driver

[Suman Anna]

On 01/19/2018 05:49 PM, Tony Lindgren wrote:
> * Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [180119 23:30]:
>> On 01/19/2018 03:33 PM, Tony Lindgren wrote:
>>> OK let's do some tests on that, I'll take a look at doing a dts
>>> file over next few weeks.
>>
>> You can look up the patches folder in the above repo, there are some
>> example nodes there already, they are rather straight-forward.
> 
> Just for reference, here's what I played with but keep getting
> -EPROBE_DEFER somewhere during init.

Hmm, What's the baseline branch you are using - mainline, linux-next or
your for-next?

> 
> Regards,
> 
> Tony
> 
> 	prm: prm@6000 {
> 		...
> 		ranges = <0 0x6000 0x3000>;
> 
> 		prm_dsp: prm@400 {
> 			compatible = "simple-bus";
> 			#address-cells = <1>;
> 			#size-cells = <1>;
> 			ranges = <0 0x400 0x100>;
> 
> 			dsp_rstctrl: rstctrl@10 {
> 				compatible = "ti,rstctrl";
> 				reg = <0x10 0x4>;
> 				#reset-cells = <1>;
> 			};
> 		};
> 		...
> 	};
> 
> 	target-module@4a066000 {
> 		compatible = "ti,sysc-omap2", "ti,sysc";
> 		ti,hwmods = "mmu_dsp";
> 		reg = <0x4a066000 0x4>,
> 		      <0x4a066010 0x4>,
> 		      <0x4a066014 0x4>;
> 		reg-names = "rev", "sysc", "syss";
> 		ti,sysc-mask = <(SYSC_OMAP2_CLOCKACTIVITY |
> 				 SYSC_OMAP2_SOFTRESET |
> 				 SYSC_OMAP2_AUTOIDLE)>;
> 		ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> 				<SYSC_IDLE_NO>,
> 				<SYSC_IDLE_SMART>;
> 		clocks = <&tesla_clkctrl OMAP4_DSP_CLKCTRL 0>;
> 		clock-names = "fck";
> 		resets = <&dsp_rstctrl 1>,
> 			 <&dsp_rstctrl 0>;
> 		reset-names = "rst2", "rst1";

We definitely do not want the two resets here for sure, as the rst2
belongs to the dsp core (I believe it would be a sibling node to mmu_dsp
here), and cannot be released from reset without programming the MMU and
loading the code.

regards
Suman

> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 		ranges = <0 0x4a066000 0x1000>;
> 
> 		mmu_dsp: mmu@0 {
> 			compatible = "ti,omap4-iommu";
> 			reg = <0 0x100>;
> 			interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
> 			#iommu-cells = <0>;
> 		};
> 	};
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] reset: ti-rstctrl: use the reset-simple driver

[Tony Lindgren]

* Suman Anna <s-anna@ti.com> [180120 00:43]:
> On 01/19/2018 05:49 PM, Tony Lindgren wrote:
> > * Suman Anna <s-anna@ti.com> [180119 23:30]:
> >> On 01/19/2018 03:33 PM, Tony Lindgren wrote:
> >>> OK let's do some tests on that, I'll take a look at doing a dts
> >>> file over next few weeks.
> >>
> >> You can look up the patches folder in the above repo, there are some
> >> example nodes there already, they are rather straight-forward.
> > 
> > Just for reference, here's what I played with but keep getting
> > -EPROBE_DEFER somewhere during init.
> 
> Hmm, What's the baseline branch you are using - mainline, linux-next or
> your for-next?

I just quickly tested with Linux next + my yet to be posted patches..
I'll debug it further.

> > 	prm: prm@6000 {
> > 		...
> > 		ranges = <0 0x6000 0x3000>;
> > 
> > 		prm_dsp: prm@400 {
> > 			compatible = "simple-bus";
> > 			#address-cells = <1>;
> > 			#size-cells = <1>;
> > 			ranges = <0 0x400 0x100>;
> > 
> > 			dsp_rstctrl: rstctrl@10 {
> > 				compatible = "ti,rstctrl";
> > 				reg = <0x10 0x4>;
> > 				#reset-cells = <1>;
> > 			};
> > 		};
> > 		...
> > 	};
> > 
> > 	target-module@4a066000 {
> > 		compatible = "ti,sysc-omap2", "ti,sysc";
> > 		ti,hwmods = "mmu_dsp";
> > 		reg = <0x4a066000 0x4>,
> > 		      <0x4a066010 0x4>,
> > 		      <0x4a066014 0x4>;
> > 		reg-names = "rev", "sysc", "syss";
> > 		ti,sysc-mask = <(SYSC_OMAP2_CLOCKACTIVITY |
> > 				 SYSC_OMAP2_SOFTRESET |
> > 				 SYSC_OMAP2_AUTOIDLE)>;
> > 		ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> > 				<SYSC_IDLE_NO>,
> > 				<SYSC_IDLE_SMART>;
> > 		clocks = <&tesla_clkctrl OMAP4_DSP_CLKCTRL 0>;
> > 		clock-names = "fck";
> > 		resets = <&dsp_rstctrl 1>,
> > 			 <&dsp_rstctrl 0>;
> > 		reset-names = "rst2", "rst1";
> 
> We definitely do not want the two resets here for sure, as the rst2
> belongs to the dsp core (I believe it would be a sibling node to mmu_dsp
> here), and cannot be released from reset without programming the MMU and
> loading the code.

OK thanks, I'll give that a try next week at some point.

Regards,

Tony

Re: [PATCH] reset: ti-rstctrl: use the reset-simple driver

[Tony Lindgren]

* Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [180120 00:56]:
> * Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [180120 00:43]:
> > We definitely do not want the two resets here for sure, as the rst2
> > belongs to the dsp core (I believe it would be a sibling node to mmu_dsp
> > here), and cannot be released from reset without programming the MMU and
> > loading the code.
> 
> OK thanks, I'll give that a try next week at some point.

Actually there is only the DSP MMU in the interconnect
target module at 0x4a066000. It's size is just the usual 4K,
and there is no sibling to DSP MMU in that interconnect target
module between 0x4a0660100 and 0x4a066ffff.

So it seems the DSP MMU dts should become as below, and I've
confirmed it works for me for taking DSP MMU out of reset,
dumping out the revision register while not taking the DSP
out of reset. The rst2 is for MMU and rst1 is for DSP at
least on omap4. Taking rst1 out of reset will cause tons of
L3 interrupts as the DSP starts booting at some uncofigured
address :)

The following patch still needs my yet to be posted patches
to do anything, so just for reference for now. The
-EPROBE_DEFER I was getting earlier was the missing
"simple-bus" for probing children at the prm level.

And it seems that the interconnect target module driver ti-sysc
can claim rst2 and the ocp softreset, and if the iommu driver
ever needs to do a reset on the whole module for some reason,
it can call device_reset(dev->parent, 0);

Regards,

Tony

8< -------------------
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -249,13 +249,26 @@
 				};
 
 				prm: prm@6000 {
-					compatible = "ti,omap4-prm";
+					compatible = "ti,omap4-prm", "simple-bus";
 					reg = <0x6000 0x3000>;
 					interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
 					#address-cells = <1>;
 					#size-cells = <1>;
 					ranges = <0 0x6000 0x3000>;
 
+					prm_dsp: prm@400 {
+						compatible = "simple-bus";
+						#address-cells = <1>;
+						#size-cells = <1>;
+						ranges = <0 0x400 0x100>;
+
+						dsp_rstctrl: rstctrl@10 {
+							compatible = "ti,rstctrl";
+							reg = <0x10 0x4>;
+							#reset-cells = <1>;
+						};
+					};
+
 					prm_clocks: clocks {
 						#address-cells = <1>;
 						#size-cells = <0>;
@@ -892,12 +905,36 @@
 			};
 		};
 
-		mmu_dsp: mmu@4a066000 {
-			compatible = "ti,omap4-iommu";
-			reg = <0x4a066000 0x100>;
-			interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
+		target-module@4a066000 {
+			compatible = "ti,sysc-omap2", "ti,sysc";
 			ti,hwmods = "mmu_dsp";
-			#iommu-cells = <0>;
+			reg = <0x4a066000 0x4>,
+			      <0x4a066010 0x4>,
+			      <0x4a066014 0x4>;
+			reg-names = "rev", "sysc", "syss";
+			ti,sysc-mask = <(SYSC_OMAP2_CLOCKACTIVITY |
+					 SYSC_OMAP2_SOFTRESET |
+					 SYSC_OMAP2_AUTOIDLE)>;
+			ti,sysc-sidle = <SYSC_IDLE_FORCE>,
+					<SYSC_IDLE_NO>,
+					<SYSC_IDLE_SMART>;
+			clocks = <&tesla_clkctrl OMAP4_DSP_CLKCTRL 0>;
+			clock-names = "fck";
+			/* rst2 is for mmu, rst1 is for dsp */
+			resets = <&dsp_rstctrl 1>;
+			reset-names = "rst2";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0 0x4a066000 0x1000>;
+
+			mmu_dsp: mmu@0 {
+				compatible = "ti,omap4-iommu";
+				reg = <0 0x100>;
+				interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
+				resets = <&dsp_rstctrl 0>;
+				reset-names = "rst1";
+				#iommu-cells = <0>;
+			};
 		};
 
 		target-module@52000000 {
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] reset: ti-rstctrl: use the reset-simple driver

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [180116 07:03]:
> * Philipp Zabel <p.zabel@pengutronix.de> [180116 09:52]:
> > Looks good to me. Can I move the compatible property when applying?
> 
> Oops, here's a better version. I also left out the "prcm" part as
> at some point that should have just ranges instead of both reg and
> ranges that it currently has.

Philipp, do you need me to resend v2 of this patch against v4.16-rc?

The changes we discussed with Suman can then be done as
separate patches as needed :)

Regards,

Tony

Re: [PATCH] reset: ti-rstctrl: use the reset-simple driver

[Philipp Zabel]

Hi Tony,

On Thu, 2018-03-01 at 10:14 -0800, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [180116 07:03]:
> > * Philipp Zabel <p.zabel@pengutronix.de> [180116 09:52]:
> > > Looks good to me. Can I move the compatible property when applying?
> > 
> > Oops, here's a better version. I also left out the "prcm" part as
> > at some point that should have just ranges instead of both reg and
> > ranges that it currently has.
> 
> Philipp, do you need me to resend v2 of this patch against v4.16-rc?

Rebasing onto linux-next or
  git://git.pengutronix.de/pza/linux.git
reset/next
would help me, but is not required.

> The changes we discussed with Suman can then be done as
> separate patches as needed :)

The discussion just went a bit over my head. If you have reached an
agreement, could I get Suman's Acked-by for this patch?

regards
Philipp