Re: [PATCH 1/4] watchdog: renesas-wdt: add driver

[Guenter Roeck <linux@roeck-us.net>]

Hi Wolfram,

On 04/01/2016 04:36 AM, Wolfram Sang wrote:
> Hi Guenter,
>
>>> +static bool nowayout = WATCHDOG_NOWAYOUT;
>>> +module_param(nowayout, bool, S_IRUGO);
>>
>> Sure you want this parameter readable ? No problem with me, but it is unusual,
>> so I figure it is worth asking.
>
> No reason, will stick to the usual case.
>
>>> +static void rwdt_write(struct rwdt_priv *priv, u32 val, unsigned reg)
>>
>> Please use 'unsigned int' throughout.
>
> Can do. May I ask why?
>

There are people changing unsigned -> unsigned int with coccinelle scripts.
Besides that, checkpatch warns about it.

Sure, I know, checkpatch is just a script, I know that story, but I don't
want to have to deal with the inevitable follow-up patches.

>>> +	rwdt_write(priv, 65536 - wdev->timeout * priv->clks_per_sec, RWTCNT);
>>> +
>>
>> Just wondering, does reading RWTCNT return the remaining timeout ?
>> If so, you could easily implement WDIOC_GETTIMEOUT.
>
> I assume you mean GETTIMELEFT. Tried that, seems to work.
>
Yes, sorry.

Thanks,
Guenter

>> function. In other words, you can just drop rwdt_set_timeout() entirely.
>
> Cool news! Works fine.
>
> Thanks for the prompt review! Will send V2 in a minute.
>
>     Wolfram
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>