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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 16 Jan 2018 15:22:54 -0600
From:   Suman Anna <s-anna@...com>
To:     Tony Lindgren <tony@...mide.com>,
        Philipp Zabel <p.zabel@...gutronix.de>
CC:     Philipp Zabel <philipp.zabel@...il.com>,
        Paul Parsons <lost.distance@...oo.com>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-omap@...r.kernel.org>, Dave Gerlach <d-gerlach@...com>,
        Mark Rutland <mark.rutland@....com>, Nishant Menon <nm@...com>,
        Rob Herring <robh+dt@...nel.org>, Tero Kristo <t-kristo@...com>
Subject: Re: [PATCH] reset: ti-rstctrl: use the reset-simple driver

Hi Tony,

On 01/16/2018 09:03 AM, Tony Lindgren wrote:
> Hi,
> 
> * Philipp Zabel <p.zabel@...gutronix.de> [180116 09:52]:
>> On Mon, 2018-01-15 at 17:11 -0800, Tony Lindgren wrote:
>>> +Example:
>>> +
>>> +	prcm: prcm@...000 {
>>> +		compatible = "ti,am3-prcm", "simple-bus";
>>> +		reg = <0x200000 0x4000>;
>>> +		#address-cells = <1>;
>>> +		#size-cells = <1>;
>>> +		ranges = <0 0x200000 0x4000>;
>>> +
>>> +		prm_gfx: prm@...0 {
>>> +			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@...mide.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@...com>
> Cc: Mark Rutland <mark.rutland@....com>
> Cc: Nishant Menon <nm@...com>
> Cc: Philipp Zabel <p.zabel@...gutronix.de>
> Cc: Rob Herring <robh+dt@...nel.org>
> Cc: Suman Anna <s-anna@...com>
> Cc: Tero Kristo <t-kristo@...com>
> Signed-off-by: Tony Lindgren <tony@...mide.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@...0 {
> +		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 */ },
>