[PATCH 0/2] ARM64: Enable SP805 WDT support for FSL LS2080A

[PATCH 0/2] ARM64: Enable SP805 WDT support for FSL LS2080A

[Bhupesh Sharma]

This patchset adds the support for SP805 WDT on FSL LS2080A and
also adds the missing documentation of SP805 WDT device-tree bindings.

Rebased against arm-soc/next/dt

Bhupesh Sharma (2):
  Documentation: DT: Add entry for ARM SP805-WDT
  dts/ls2080a: Update DTSI to add support of SP805 WDT

 .../devicetree/bindings/watchdog/sp805-wdt.txt     |   33 ++++++++++
 arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi     |   64 ++++++++++++++++++++
 2 files changed, 97 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt

-- 
1.7.9.5


--
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

[PATCH 1/2] Documentation: DT: Add entry for ARM SP805-WDT

[Bhupesh Sharma]

This patch adds a devicetree binding documentation for ARM's
SP805 WatchDog Timer.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 .../devicetree/bindings/watchdog/sp805-wdt.txt     |   33 ++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
new file mode 100644
index 0000000..ec70fe9
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
@@ -0,0 +1,33 @@
+* ARM SP805 Watchdog Timer (WDT) Controller
+
+SP805 WDT is a ARM Primecell Peripheral and has a standard-id register that
+can be used to identify the peripheral type, vendor, and revision.
+This value can be used for driver matching.
+
+Note that the current sp805_wdt driver relies on the 'drivers/amba/bus.c'
+framework to invoke the probe function of the sp805_wdt driver using the
+unique PRIMECELL identifiers of the sp805 wdt IP.
+
+As SP805 WDT is a primecell IP, it follows the base bindings specified in
+'arm/primecell.txt'
+
+Required properties:
+- compatible : Should be "arm,sp805-wdt", "arm,primecell"
+- reg : Base address and size of the watchdog timer registers.
+- interrupts : Should specify WDT interrupt number.
+
+Optional properties:
+- clocks : From common clock binding. First clock is phandle to clock for apb
+           pclk. Additional clocks are optional.
+- clock-names : From common clock binding. Shall be "apb_pclk" for first clock.
+
+Examples:
+
+		cluster1_core0_watchdog: wdt@c000000 {
+			compatible = "arm,sp805-wdt", "arm,primecell";
+			reg = <0x0 0xc000000 0x0 0x1000>;
+			interrupts = <1 12 0x8>; /* PPI, Level low type */
+			clocks = <&clockgen 4 3>;
+			clock-names = "apb_pclk";
+		};
+
-- 
1.7.9.5


--
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

[PATCH 2/2] dts/ls2080a: Update DTSI to add support of SP805 WDT

[Bhupesh Sharma]

This patch updates the LS2080a DTSI (DTS Include) file to add
support for eight SP805 Watchdog units which can be used to
reset the eight Cortex-A57 cores available on LS2080A.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi |   64 ++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
index e81cd48..15637ab 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
@@ -193,6 +193,70 @@
 			interrupts = <0 32 0x4>; /* Level high type */
 		};
 
+		cluster1_core0_watchdog: wdt@c000000 {
+			compatible = "arm,sp805-wdt", "arm,primecell";
+			reg = <0x0 0xc000000 0x0 0x1000>;
+			interrupts = <1 12 0x8>; /* PPI, Level low type */
+			clocks = <&clockgen 4 3>;
+			clock-names = "apb_pclk";
+		};
+
+		cluster1_core1_watchdog: wdt@c010000 {
+			compatible = "arm,sp805-wdt", "arm,primecell";
+			reg = <0x0 0xc010000 0x0 0x1000>;
+			interrupts = <1 12 0x8>; /* PPI, Level low type */
+			clocks = <&clockgen 4 3>;
+			clock-names = "apb_pclk";
+		};
+
+		cluster2_core0_watchdog: wdt@c100000 {
+			compatible = "arm,sp805-wdt", "arm,primecell";
+			reg = <0x0 0xc100000 0x0 0x1000>;
+			interrupts = <1 12 0x8>; /* PPI, Level low type */
+			clocks = <&clockgen 4 3>;
+			clock-names = "apb_pclk";
+		};
+
+		cluster2_core1_watchdog: wdt@c110000 {
+			compatible = "arm,sp805-wdt", "arm,primecell";
+			reg = <0x0 0xc110000 0x0 0x1000>;
+			interrupts = <1 12 0x8>; /* PPI, Level low type */
+			clocks = <&clockgen 4 3>;
+			clock-names = "apb_pclk";
+		};
+
+		cluster3_core0_watchdog: wdt@c200000 {
+			compatible = "arm,sp805-wdt", "arm,primecell";
+			reg = <0x0 0xc200000 0x0 0x1000>;
+			interrupts = <1 12 0x8>; /* PPI, Level low type */
+			clocks = <&clockgen 4 3>;
+			clock-names = "apb_pclk";
+		};
+
+		cluster3_core1_watchdog: wdt@c210000 {
+			compatible = "arm,sp805-wdt", "arm,primecell";
+			reg = <0x0 0xc210000 0x0 0x1000>;
+			interrupts = <1 12 0x8>; /* PPI, Level low type */
+			clocks = <&clockgen 4 3>;
+			clock-names = "apb_pclk";
+		};
+
+		cluster4_core0_watchdog: wdt@c300000 {
+			compatible = "arm,sp805-wdt", "arm,primecell";
+			reg = <0x0 0xc300000 0x0 0x1000>;
+			interrupts = <1 12 0x8>; /* PPI, Level low type */
+			clocks = <&clockgen 4 3>;
+			clock-names = "apb_pclk";
+		};
+
+		cluster4_core1_watchdog: wdt@c310000 {
+			compatible = "arm,sp805-wdt", "arm,primecell";
+			reg = <0x0 0xc310000 0x0 0x1000>;
+			interrupts = <1 12 0x8>; /* PPI, Level low type */
+			clocks = <&clockgen 4 3>;
+			clock-names = "apb_pclk";
+		};
+
 		fsl_mc: fsl-mc@80c000000 {
 			compatible = "fsl,qoriq-mc";
 			reg = <0x00000008 0x0c000000 0 0x40>,	 /* MC portal base */
-- 
1.7.9.5


--
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 2/2] dts/ls2080a: Update DTSI to add support of SP805 WDT

[Mark Rutland]

On Mon, Nov 16, 2015 at 07:54:43PM +0530, Bhupesh Sharma wrote:
> This patch updates the LS2080a DTSI (DTS Include) file to add
> support for eight SP805 Watchdog units which can be used to
> reset the eight Cortex-A57 cores available on LS2080A.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi |   64 ++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> index e81cd48..15637ab 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> @@ -193,6 +193,70 @@
>  			interrupts = <0 32 0x4>; /* Level high type */
>  		};
>  
> +		cluster1_core0_watchdog: wdt@c000000 {
> +			compatible = "arm,sp805-wdt", "arm,primecell";
> +			reg = <0x0 0xc000000 0x0 0x1000>;
> +			interrupts = <1 12 0x8>; /* PPI, Level low type */
> +			clocks = <&clockgen 4 3>;
> +			clock-names = "apb_pclk";
> +		};
> +
> +		cluster1_core1_watchdog: wdt@c010000 {
> +			compatible = "arm,sp805-wdt", "arm,primecell";
> +			reg = <0x0 0xc010000 0x0 0x1000>;
> +			interrupts = <1 12 0x8>; /* PPI, Level low type */
> +			clocks = <&clockgen 4 3>;
> +			clock-names = "apb_pclk";
> +		};

How does this pseudo-banking work?

For one thing as far as I can see, the SP805 driver can't handle PPIs in
mainline.

Additionally, the kernel has no idea which CPU each of these is
associated with, and we effectively need to deal with shared PPIs in
order to use this appropriately in the kernel (if each WDT's interrupt
is wired to a particular core).

Thanks,
Mark.
--
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 1/2] Documentation: DT: Add entry for ARM SP805-WDT

[Mark Rutland]

On Mon, Nov 16, 2015 at 07:54:42PM +0530, Bhupesh Sharma wrote:
> This patch adds a devicetree binding documentation for ARM's
> SP805 WatchDog Timer.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  .../devicetree/bindings/watchdog/sp805-wdt.txt     |   33 ++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> new file mode 100644
> index 0000000..ec70fe9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> @@ -0,0 +1,33 @@
> +* ARM SP805 Watchdog Timer (WDT) Controller
> +
> +SP805 WDT is a ARM Primecell Peripheral and has a standard-id register that
> +can be used to identify the peripheral type, vendor, and revision.
> +This value can be used for driver matching.
> +
> +Note that the current sp805_wdt driver relies on the 'drivers/amba/bus.c'
> +framework to invoke the probe function of the sp805_wdt driver using the
> +unique PRIMECELL identifiers of the sp805 wdt IP.

This paragraph can go. We shouldn't describe kernel internals.

> +As SP805 WDT is a primecell IP, it follows the base bindings specified in
> +'arm/primecell.txt'
> +
> +Required properties:
> +- compatible : Should be "arm,sp805-wdt", "arm,primecell"
> +- reg : Base address and size of the watchdog timer registers.
> +- interrupts : Should specify WDT interrupt number.
> +
> +Optional properties:
> +- clocks : From common clock binding. First clock is phandle to clock for apb
> +           pclk. Additional clocks are optional.
> +- clock-names : From common clock binding. Shall be "apb_pclk" for first clock.

The hardware has "WDOGCLK", which is what the driver appears to expect
first implicitly.

> +Examples:
> +
> +		cluster1_core0_watchdog: wdt@c000000 {
> +			compatible = "arm,sp805-wdt", "arm,primecell";
> +			reg = <0x0 0xc000000 0x0 0x1000>;
> +			interrupts = <1 12 0x8>; /* PPI, Level low type */

I don't see how you can use PPIs here. This is not banked per CPU.

Mark.

> +			clocks = <&clockgen 4 3>;
> +			clock-names = "apb_pclk";
> +		};
> +
> -- 
> 1.7.9.5
> 
> 
--
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 1/2] Documentation: DT: Add entry for ARM SP805-WDT

[Rob Herring]

On Mon, Nov 16, 2015 at 02:45:25PM +0000, Mark Rutland wrote:
> On Mon, Nov 16, 2015 at 07:54:42PM +0530, Bhupesh Sharma wrote:
> > This patch adds a devicetree binding documentation for ARM's
> > SP805 WatchDog Timer.
> > 
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > ---
> >  .../devicetree/bindings/watchdog/sp805-wdt.txt     |   33 ++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> > new file mode 100644
> > index 0000000..ec70fe9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> > @@ -0,0 +1,33 @@
> > +* ARM SP805 Watchdog Timer (WDT) Controller
> > +
> > +SP805 WDT is a ARM Primecell Peripheral and has a standard-id register that
> > +can be used to identify the peripheral type, vendor, and revision.
> > +This value can be used for driver matching.
> > +
> > +Note that the current sp805_wdt driver relies on the 'drivers/amba/bus.c'
> > +framework to invoke the probe function of the sp805_wdt driver using the
> > +unique PRIMECELL identifiers of the sp805 wdt IP.
> 
> This paragraph can go. We shouldn't describe kernel internals.
> 
> > +As SP805 WDT is a primecell IP, it follows the base bindings specified in
> > +'arm/primecell.txt'
> > +
> > +Required properties:
> > +- compatible : Should be "arm,sp805-wdt", "arm,primecell"
> > +- reg : Base address and size of the watchdog timer registers.
> > +- interrupts : Should specify WDT interrupt number.
> > +
> > +Optional properties:
> > +- clocks : From common clock binding. First clock is phandle to clock for apb
> > +           pclk. Additional clocks are optional.
> > +- clock-names : From common clock binding. Shall be "apb_pclk" for first clock.
> 
> The hardware has "WDOGCLK", which is what the driver appears to expect
> first implicitly.

The h/w has 2 clocks, PCLK and WDOGCLK, so both should be described and 
neither should be optional.

Rob

--
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 1/2] Documentation: DT: Add entry for ARM SP805-WDT

[Bhupesh SHARMA]

Hi Mark,

Thanks for the review.

On Mon, Nov 16, 2015 at 8:15 PM, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> On Mon, Nov 16, 2015 at 07:54:42PM +0530, Bhupesh Sharma wrote:
>> This patch adds a devicetree binding documentation for ARM's
>> SP805 WatchDog Timer.
>>
>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> ---
>>  .../devicetree/bindings/watchdog/sp805-wdt.txt     |   33 ++++++++++++++++++++
>>  1 file changed, 33 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>
>> diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>> new file mode 100644
>> index 0000000..ec70fe9
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>> @@ -0,0 +1,33 @@
>> +* ARM SP805 Watchdog Timer (WDT) Controller
>> +
>> +SP805 WDT is a ARM Primecell Peripheral and has a standard-id register that
>> +can be used to identify the peripheral type, vendor, and revision.
>> +This value can be used for driver matching.
>> +
>> +Note that the current sp805_wdt driver relies on the 'drivers/amba/bus.c'
>> +framework to invoke the probe function of the sp805_wdt driver using the
>> +unique PRIMECELL identifiers of the sp805 wdt IP.
>
> This paragraph can go. We shouldn't describe kernel internals.

Ok.

>> +As SP805 WDT is a primecell IP, it follows the base bindings specified in
>> +'arm/primecell.txt'
>> +
>> +Required properties:
>> +- compatible : Should be "arm,sp805-wdt", "arm,primecell"
>> +- reg : Base address and size of the watchdog timer registers.
>> +- interrupts : Should specify WDT interrupt number.
>> +
>> +Optional properties:
>> +- clocks : From common clock binding. First clock is phandle to clock for apb
>> +           pclk. Additional clocks are optional.
>> +- clock-names : From common clock binding. Shall be "apb_pclk" for first clock.
>
> The hardware has "WDOGCLK", which is what the driver appears to expect
> first implicitly.

Ok.

>> +Examples:
>> +
>> +             cluster1_core0_watchdog: wdt@c000000 {
>> +                     compatible = "arm,sp805-wdt", "arm,primecell";
>> +                     reg = <0x0 0xc000000 0x0 0x1000>;
>> +                     interrupts = <1 12 0x8>; /* PPI, Level low type */
>
> I don't see how you can use PPIs here. This is not banked per CPU.

I have raised this concern to my hardware team. This might be an issue
with the documentation.
I will change it in v2 as per their comments.

Regards,
Bhupesh

> Mark.
>
>> +                     clocks = <&clockgen 4 3>;
>> +                     clock-names = "apb_pclk";
>> +             };
>> +
>> --
>> 1.7.9.5
>>
>>
--
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 1/2] Documentation: DT: Add entry for ARM SP805-WDT

[Bhupesh SHARMA]

Hi Rob,

Thanks for the review.

On Tue, Nov 17, 2015 at 5:44 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Nov 16, 2015 at 02:45:25PM +0000, Mark Rutland wrote:
>> On Mon, Nov 16, 2015 at 07:54:42PM +0530, Bhupesh Sharma wrote:
>> > This patch adds a devicetree binding documentation for ARM's
>> > SP805 WatchDog Timer.
>> >
>> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> > ---
>> >  .../devicetree/bindings/watchdog/sp805-wdt.txt     |   33 ++++++++++++++++++++
>> >  1 file changed, 33 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>> > new file mode 100644
>> > index 0000000..ec70fe9
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>> > @@ -0,0 +1,33 @@
>> > +* ARM SP805 Watchdog Timer (WDT) Controller
>> > +
>> > +SP805 WDT is a ARM Primecell Peripheral and has a standard-id register that
>> > +can be used to identify the peripheral type, vendor, and revision.
>> > +This value can be used for driver matching.
>> > +

[snip..]

>> > +As SP805 WDT is a primecell IP, it follows the base bindings specified in
>> > +'arm/primecell.txt'
>> > +
>> > +Required properties:
>> > +- compatible : Should be "arm,sp805-wdt", "arm,primecell"
>> > +- reg : Base address and size of the watchdog timer registers.
>> > +- interrupts : Should specify WDT interrupt number.
>> > +
>> > +Optional properties:
>> > +- clocks : From common clock binding. First clock is phandle to clock for apb
>> > +           pclk. Additional clocks are optional.
>> > +- clock-names : From common clock binding. Shall be "apb_pclk" for first clock.
>>
>> The hardware has "WDOGCLK", which is what the driver appears to expect
>> first implicitly.
>
> The h/w has 2 clocks, PCLK and WDOGCLK, so both should be described and
> neither should be optional.

As per the SP805 WDT TRM I have with me (see [1], Figure 1-1), this
h/w has only only input
clock WDOGCLK.

[1] http://infocenter.arm.com/help/topic/com.arm.doc.ddi0270b/DDI0270.pdf

Regards,
Bhupesh

> Rob
>
--
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 1/2] Documentation: DT: Add entry for ARM SP805-WDT

[Rob Herring]

On Wed, Nov 18, 2015 at 8:03 AM, Bhupesh SHARMA <bhupesh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Rob,
>
> Thanks for the review.
>
> On Tue, Nov 17, 2015 at 5:44 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On Mon, Nov 16, 2015 at 02:45:25PM +0000, Mark Rutland wrote:
>>> On Mon, Nov 16, 2015 at 07:54:42PM +0530, Bhupesh Sharma wrote:
>>> > This patch adds a devicetree binding documentation for ARM's
>>> > SP805 WatchDog Timer.
>>> >
>>> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>>> > ---
>>> >  .../devicetree/bindings/watchdog/sp805-wdt.txt     |   33 ++++++++++++++++++++
>>> >  1 file changed, 33 insertions(+)
>>> >  create mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>> >
>>> > diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>> > new file mode 100644
>>> > index 0000000..ec70fe9
>>> > --- /dev/null
>>> > +++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>> > @@ -0,0 +1,33 @@
>>> > +* ARM SP805 Watchdog Timer (WDT) Controller
>>> > +
>>> > +SP805 WDT is a ARM Primecell Peripheral and has a standard-id register that
>>> > +can be used to identify the peripheral type, vendor, and revision.
>>> > +This value can be used for driver matching.
>>> > +
>
> [snip..]
>
>>> > +As SP805 WDT is a primecell IP, it follows the base bindings specified in
>>> > +'arm/primecell.txt'
>>> > +
>>> > +Required properties:
>>> > +- compatible : Should be "arm,sp805-wdt", "arm,primecell"
>>> > +- reg : Base address and size of the watchdog timer registers.
>>> > +- interrupts : Should specify WDT interrupt number.
>>> > +
>>> > +Optional properties:
>>> > +- clocks : From common clock binding. First clock is phandle to clock for apb
>>> > +           pclk. Additional clocks are optional.
>>> > +- clock-names : From common clock binding. Shall be "apb_pclk" for first clock.
>>>
>>> The hardware has "WDOGCLK", which is what the driver appears to expect
>>> first implicitly.
>>
>> The h/w has 2 clocks, PCLK and WDOGCLK, so both should be described and
>> neither should be optional.
>
> As per the SP805 WDT TRM I have with me (see [1], Figure 1-1), this
> h/w has only only input
> clock WDOGCLK.
>
> [1] http://infocenter.arm.com/help/topic/com.arm.doc.ddi0270b/DDI0270.pdf

Look closer, PCLK is in the AMBA bus signals. The version online has
some timing diagrams also which I didn't find here.

Rob
--
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 1/2] Documentation: DT: Add entry for ARM SP805-WDT

[Bhupesh SHARMA]

Hi Mark,

On Wed, Nov 18, 2015 at 7:21 PM, Bhupesh SHARMA <bhupesh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Mark,
>
> Thanks for the review.
>
> On Mon, Nov 16, 2015 at 8:15 PM, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
>> On Mon, Nov 16, 2015 at 07:54:42PM +0530, Bhupesh Sharma wrote:
>>> This patch adds a devicetree binding documentation for ARM's
>>> SP805 WatchDog Timer.
>>>
>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>>> ---
>>>  .../devicetree/bindings/watchdog/sp805-wdt.txt     |   33 ++++++++++++++++++++
>>>  1 file changed, 33 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>> new file mode 100644
>>> index 0000000..ec70fe9
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>> @@ -0,0 +1,33 @@
>>> +* ARM SP805 Watchdog Timer (WDT) Controller
>>> +
>>> +SP805 WDT is a ARM Primecell Peripheral and has a standard-id register that
>>> +can be used to identify the peripheral type, vendor, and revision.
>>> +This value can be used for driver matching.
>>> +
>>> +Note that the current sp805_wdt driver relies on the 'drivers/amba/bus.c'
>>> +framework to invoke the probe function of the sp805_wdt driver using the
>>> +unique PRIMECELL identifiers of the sp805 wdt IP.
>>
>> This paragraph can go. We shouldn't describe kernel internals.
>
> Ok.
>
>>> +As SP805 WDT is a primecell IP, it follows the base bindings specified in
>>> +'arm/primecell.txt'
>>> +
>>> +Required properties:
>>> +- compatible : Should be "arm,sp805-wdt", "arm,primecell"
>>> +- reg : Base address and size of the watchdog timer registers.
>>> +- interrupts : Should specify WDT interrupt number.
>>> +
>>> +Optional properties:
>>> +- clocks : From common clock binding. First clock is phandle to clock for apb
>>> +           pclk. Additional clocks are optional.
>>> +- clock-names : From common clock binding. Shall be "apb_pclk" for first clock.
>>
>> The hardware has "WDOGCLK", which is what the driver appears to expect
>> first implicitly.
>
> Ok.
>
>>> +Examples:
>>> +
>>> +             cluster1_core0_watchdog: wdt@c000000 {
>>> +                     compatible = "arm,sp805-wdt", "arm,primecell";
>>> +                     reg = <0x0 0xc000000 0x0 0x1000>;
>>> +                     interrupts = <1 12 0x8>; /* PPI, Level low type */
>>
>> I don't see how you can use PPIs here. This is not banked per CPU.
>
> I have raised this concern to my hardware team. This might be an issue
> with the documentation.
> I will change it in v2 as per their comments.

I just checked back with the hardware team. They confirm that the
WDOGINT interrupts lines
from the eight instances of the SP805 WDT are infact connected to PPI
input of the GICv3 controller.

Also this SP805 WDT IP supports 2 interrupt lines:
- WDOGINT
- WDOGRES

but the current sp805_wdt.c driver doesn't handle the WDOGINT
interrupt (irrespective of whether
it is a SPI or PPI). I could not trace a request_irq being called for
the same in the driver.

So, I would suggest the following:

I can spin a patch for sp805_wdt.c to add support to handle WDOGINT
interrupt when the WDT counter expires
as currently the cadence_wdt driver does (see [1] and [2] as reference):

[1] http://lxr.free-electrons.com/source/drivers/watchdog/cadence_wdt.c#L343
[2] http://lxr.free-electrons.com/source/drivers/watchdog/cadence_wdt.c#L254

Please suggest if this approach seems fine to you.

Regards,
Bhupesh


>
> Regards,
> Bhupesh
>
>> Mark.
>>
>>> +                     clocks = <&clockgen 4 3>;
>>> +                     clock-names = "apb_pclk";
>>> +             };
>>> +
>>> --
>>> 1.7.9.5
>>>
>>>
--
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 1/2] Documentation: DT: Add entry for ARM SP805-WDT

[Bhupesh SHARMA]

Hi Rob,

On Wed, Nov 18, 2015 at 7:49 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Nov 18, 2015 at 8:03 AM, Bhupesh SHARMA <bhupesh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Hi Rob,
>>
>> Thanks for the review.
>>
>> On Tue, Nov 17, 2015 at 5:44 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>> On Mon, Nov 16, 2015 at 02:45:25PM +0000, Mark Rutland wrote:
>>>> On Mon, Nov 16, 2015 at 07:54:42PM +0530, Bhupesh Sharma wrote:
>>>> > This patch adds a devicetree binding documentation for ARM's
>>>> > SP805 WatchDog Timer.
>>>> >
>>>> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>>>> > ---
>>>> >  .../devicetree/bindings/watchdog/sp805-wdt.txt     |   33 ++++++++++++++++++++
>>>> >  1 file changed, 33 insertions(+)
>>>> >  create mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>> >
>>>> > diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>> > new file mode 100644
>>>> > index 0000000..ec70fe9
>>>> > --- /dev/null
>>>> > +++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>> > @@ -0,0 +1,33 @@
>>>> > +* ARM SP805 Watchdog Timer (WDT) Controller
>>>> > +
>>>> > +SP805 WDT is a ARM Primecell Peripheral and has a standard-id register that
>>>> > +can be used to identify the peripheral type, vendor, and revision.
>>>> > +This value can be used for driver matching.
>>>> > +
>>
>> [snip..]
>>
>>>> > +As SP805 WDT is a primecell IP, it follows the base bindings specified in
>>>> > +'arm/primecell.txt'
>>>> > +
>>>> > +Required properties:
>>>> > +- compatible : Should be "arm,sp805-wdt", "arm,primecell"
>>>> > +- reg : Base address and size of the watchdog timer registers.
>>>> > +- interrupts : Should specify WDT interrupt number.
>>>> > +
>>>> > +Optional properties:
>>>> > +- clocks : From common clock binding. First clock is phandle to clock for apb
>>>> > +           pclk. Additional clocks are optional.
>>>> > +- clock-names : From common clock binding. Shall be "apb_pclk" for first clock.
>>>>
>>>> The hardware has "WDOGCLK", which is what the driver appears to expect
>>>> first implicitly.
>>>
>>> The h/w has 2 clocks, PCLK and WDOGCLK, so both should be described and
>>> neither should be optional.
>>
>> As per the SP805 WDT TRM I have with me (see [1], Figure 1-1), this
>> h/w has only only input
>> clock WDOGCLK.
>>
>> [1] http://infocenter.arm.com/help/topic/com.arm.doc.ddi0270b/DDI0270.pdf
>
> Look closer, PCLK is in the AMBA bus signals. The version online has
> some timing diagrams also which I didn't find here.

Correct. So will add both PCLK and WDOGCLK to the compatible node in
the v2 of this patch.

Regards,
Bhupesh
--
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 1/2] Documentation: DT: Add entry for ARM SP805-WDT

[Bhupesh SHARMA]

Hi Mark,

Ping..

On Sun, Nov 29, 2015 at 3:14 AM, Bhupesh SHARMA <bhupesh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Mark,
>
> On Wed, Nov 18, 2015 at 7:21 PM, Bhupesh SHARMA <bhupesh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Hi Mark,
>>
>> Thanks for the review.
>>
>> On Mon, Nov 16, 2015 at 8:15 PM, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
>>> On Mon, Nov 16, 2015 at 07:54:42PM +0530, Bhupesh Sharma wrote:
>>>> This patch adds a devicetree binding documentation for ARM's
>>>> SP805 WatchDog Timer.
>>>>
>>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>>>> ---
>>>>  .../devicetree/bindings/watchdog/sp805-wdt.txt     |   33 ++++++++++++++++++++
>>>>  1 file changed, 33 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>> new file mode 100644
>>>> index 0000000..ec70fe9
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>> @@ -0,0 +1,33 @@
>>>> +* ARM SP805 Watchdog Timer (WDT) Controller
>>>> +
>>>> +SP805 WDT is a ARM Primecell Peripheral and has a standard-id register that
>>>> +can be used to identify the peripheral type, vendor, and revision.
>>>> +This value can be used for driver matching.
>>>> +
>>>> +Note that the current sp805_wdt driver relies on the 'drivers/amba/bus.c'
>>>> +framework to invoke the probe function of the sp805_wdt driver using the
>>>> +unique PRIMECELL identifiers of the sp805 wdt IP.
>>>
>>> This paragraph can go. We shouldn't describe kernel internals.
>>
>> Ok.
>>
>>>> +As SP805 WDT is a primecell IP, it follows the base bindings specified in
>>>> +'arm/primecell.txt'
>>>> +
>>>> +Required properties:
>>>> +- compatible : Should be "arm,sp805-wdt", "arm,primecell"
>>>> +- reg : Base address and size of the watchdog timer registers.
>>>> +- interrupts : Should specify WDT interrupt number.
>>>> +
>>>> +Optional properties:
>>>> +- clocks : From common clock binding. First clock is phandle to clock for apb
>>>> +           pclk. Additional clocks are optional.
>>>> +- clock-names : From common clock binding. Shall be "apb_pclk" for first clock.
>>>
>>> The hardware has "WDOGCLK", which is what the driver appears to expect
>>> first implicitly.
>>
>> Ok.
>>
>>>> +Examples:
>>>> +
>>>> +             cluster1_core0_watchdog: wdt@c000000 {
>>>> +                     compatible = "arm,sp805-wdt", "arm,primecell";
>>>> +                     reg = <0x0 0xc000000 0x0 0x1000>;
>>>> +                     interrupts = <1 12 0x8>; /* PPI, Level low type */
>>>
>>> I don't see how you can use PPIs here. This is not banked per CPU.
>>
>> I have raised this concern to my hardware team. This might be an issue
>> with the documentation.
>> I will change it in v2 as per their comments.
>
> I just checked back with the hardware team. They confirm that the
> WDOGINT interrupts lines
> from the eight instances of the SP805 WDT are infact connected to PPI
> input of the GICv3 controller.
>
> Also this SP805 WDT IP supports 2 interrupt lines:
> - WDOGINT
> - WDOGRES
>
> but the current sp805_wdt.c driver doesn't handle the WDOGINT
> interrupt (irrespective of whether
> it is a SPI or PPI). I could not trace a request_irq being called for
> the same in the driver.
>
> So, I would suggest the following:
>
> I can spin a patch for sp805_wdt.c to add support to handle WDOGINT
> interrupt when the WDT counter expires
> as currently the cadence_wdt driver does (see [1] and [2] as reference):
>
> [1] http://lxr.free-electrons.com/source/drivers/watchdog/cadence_wdt.c#L343
> [2] http://lxr.free-electrons.com/source/drivers/watchdog/cadence_wdt.c#L254
>
> Please suggest if this approach seems fine to you.
>
> Regards,
> Bhupesh
>
>
>>
>> Regards,
>> Bhupesh
>>
>>> Mark.
>>>
>>>> +                     clocks = <&clockgen 4 3>;
>>>> +                     clock-names = "apb_pclk";
>>>> +             };
>>>> +
>>>> --
>>>> 1.7.9.5
>>>>
>>>>
--
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