[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 at gmail.com> wrote:
> Hi Mark,
>
> On Wed, Nov 18, 2015 at 7:21 PM, Bhupesh SHARMA <bhupesh.linux at gmail.com> wrote:
>> Hi Mark,
>>
>> Thanks for the review.
>>
>> On Mon, Nov 16, 2015 at 8:15 PM, Mark Rutland <mark.rutland at 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 at 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 at 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
>>>>
>>>>