diff mbox

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

Message ID 1447683883-6600-2-git-send-email-bhupesh.sharma@freescale.com (mailing list archive)
State New, archived
Headers show

Commit Message

bhupesh.sharma@freescale.com Nov. 16, 2015, 2:24 p.m. UTC
This patch adds a devicetree binding documentation for ARM's
SP805 WatchDog Timer.

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

Comments

Mark Rutland Nov. 16, 2015, 2:45 p.m. UTC | #1
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@freescale.com>
> ---
>  .../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
> 
>
Rob Herring Nov. 17, 2015, 12:14 a.m. UTC | #2
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@freescale.com>
> > ---
> >  .../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
Bhupesh Sharma Nov. 18, 2015, 1:51 p.m. UTC | #3
Hi Mark,

Thanks for the review.

On Mon, Nov 16, 2015 at 8:15 PM, Mark Rutland <mark.rutland@arm.com> 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@freescale.com>
>> ---
>>  .../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
>>
>>
Bhupesh Sharma Nov. 18, 2015, 2:03 p.m. UTC | #4
Hi Rob,

Thanks for the review.

On Tue, Nov 17, 2015 at 5:44 AM, Rob Herring <robh@kernel.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@freescale.com>
>> > ---
>> >  .../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
>
Rob Herring Nov. 18, 2015, 2:19 p.m. UTC | #5
On Wed, Nov 18, 2015 at 8:03 AM, Bhupesh SHARMA <bhupesh.linux@gmail.com> wrote:
> Hi Rob,
>
> Thanks for the review.
>
> On Tue, Nov 17, 2015 at 5:44 AM, Rob Herring <robh@kernel.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@freescale.com>
>>> > ---
>>> >  .../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
Bhupesh Sharma Nov. 28, 2015, 9:44 p.m. UTC | #6
Hi Mark,

On Wed, Nov 18, 2015 at 7:21 PM, Bhupesh SHARMA <bhupesh.linux@gmail.com> wrote:
> Hi Mark,
>
> Thanks for the review.
>
> On Mon, Nov 16, 2015 at 8:15 PM, Mark Rutland <mark.rutland@arm.com> 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@freescale.com>
>>> ---
>>>  .../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
>>>
>>>
Bhupesh Sharma Nov. 28, 2015, 9:47 p.m. UTC | #7
Hi Rob,

On Wed, Nov 18, 2015 at 7:49 PM, Rob Herring <robh@kernel.org> wrote:
> On Wed, Nov 18, 2015 at 8:03 AM, Bhupesh SHARMA <bhupesh.linux@gmail.com> wrote:
>> Hi Rob,
>>
>> Thanks for the review.
>>
>> On Tue, Nov 17, 2015 at 5:44 AM, Rob Herring <robh@kernel.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@freescale.com>
>>>> > ---
>>>> >  .../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
Bhupesh Sharma Dec. 4, 2015, 12:32 p.m. UTC | #8
Hi Mark,

Ping..

On Sun, Nov 29, 2015 at 3:14 AM, Bhupesh SHARMA <bhupesh.linux@gmail.com> wrote:
> Hi Mark,
>
> On Wed, Nov 18, 2015 at 7:21 PM, Bhupesh SHARMA <bhupesh.linux@gmail.com> wrote:
>> Hi Mark,
>>
>> Thanks for the review.
>>
>> On Mon, Nov 16, 2015 at 8:15 PM, Mark Rutland <mark.rutland@arm.com> 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@freescale.com>
>>>> ---
>>>>  .../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
>>>>
>>>>
diff mbox

Patch

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";
+		};
+