Re: [PATCH v2 6/7] Watchdog: introduce ARM SBSA watchdog driver

[Fu Wei]

Hi Guenter,

On 25 May 2015 at 00:23, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> On 05/24/2015 08:50 AM, Fu Wei wrote:
> [ ...]
>
>> Actually, I have added my thought at the head of sbsa_gwdt.c as a comment
>> :
>>
>>    *
>>   * Note: This SBSA Generic watchdog driver is compatible with
>>   *       the pretimeout concept of Linux kernel.
>>   *       The timeout and pretimeout are set by the different REGs.
>>   *       The first watch period is set by writing WCV directly,
>>   *       that can support more than 10s timeout at the maximum
>>   *       system counter frequency.
>>   *       The second watch period is set by WOR(32bit) which will be
>> loaded
>>   *       automatically by hardware, when WS0 is triggered.
>>   *       This gives a maximum watch period of around 10s at the maximum
>>   *       system counter frequency.
>>   *       The System Counter shall run at maximum of 400MHz.
>>   *       More details: DEN0029B - Server Base System Architecture (SBSA)
>>   *
>>   * Kernel/API:                         P---------| pretimeout
>>   *               |-------------------------------T timeout
>>   * SBSA GWDT:                          P--WOR---WS1 pretimeout
>>   *               |-------WCV----------WS0~~~~~~~~T timeout
>>   */
>>
>
> Yes, but do we actually _know_ that it works that way, ie that WCV
> drives WS0 and that WOR drives WS1 ? Unless I am missing something,
> the specification doesn't say that, and it would have been a really
> easy statement to make if that was the intent.

yes, Suravee has tested it on Seattle B0 Soc, that works.
But hope Suravee can provide more info about test, I will ping him later.

According to SBSA,  that WCV  and WOR can both drive WS1 and WS0

the timeout and pretimeout in my patchset have been tested on Seattle
B0 and  Foundation model.

>
> My concern here is that the above behavior is not spelled out in
> the document, meaning it is up to interpretation by the hardware
> engineer implementing it, to the point where it appears that not
> even two software engineers can agree how it is supposed to work.
> Which is a really bad starting point :-(.

Is that a real hardware teat can prove it works?

or actually, SBSA say that it should work:
-----------------
Note: the watchdog offset register is 32 bits wide. This gives a
maximum watch period of around 10s at a system
counter frequency of 400MHz. If a larger watch period is required then
the compare value can be programmed
directly into the compare value register.
-----------------
offset register == WOR
compare value register == WCV

>
> Thanks,
> Guenter
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
Ph: +86 21 61221326(direct)
Ph: +86 186 2020 4684 (mobile)
Room 1512, Regus One Corporate Avenue,Level 15,
One Corporate Avenue,222 Hubin Road,Huangpu District,
Shanghai,China 200021
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/