Re: [PATCH v4 1/2] watchdog: ImgTec PDC Watchdog Timer Driver

Re: [PATCH v4 1/2] watchdog: ImgTec PDC Watchdog Timer Driver

[Andrew Bresticker]

Jude, Naidu,

On Thu, Nov 20, 2014 at 5:02 AM,  <jude.abraham-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:
> From: Jude Abraham <Jude.Abraham-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>
> This commit adds support for ImgTec PowerDown Controller Watchdog Timer.
>
> Signed-off-by: Jude Abraham <Jude.Abraham-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Naidu Tellapati <Naidu.Tellapati-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>

Reviewed-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Two small comments you may want to fix if you need to re-spin the series though:

> +static struct watchdog_info pdc_wdt_info = {
> +       .options                = WDIOF_SETTIMEOUT |
> +                                 WDIOF_KEEPALIVEPING |
> +                                 WDIOF_MAGICCLOSE,
> +       .identity = "IMG PDC Watchdog",
> +};

Inconsistent spacing.

> +       ret = watchdog_init_timeout(&pdc_wdt->wdt_dev, timeout, &pdev->dev);
> +       if (ret < 0)
> +               pdc_wdt->wdt_dev.timeout = pdc_wdt->wdt_dev.max_timeout;

Maybe print an error here?
--
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 v4 1/2] watchdog: ImgTec PDC Watchdog Timer Driver

[Naidu Tellapati]

Hi Andrew,

Many thanks for the review.

> On Thu, Nov 20, 2014 at 5:02 AM,  <jude.abraham-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:
>> From: Jude Abraham <Jude.Abraham-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>>
>> This commit adds support for ImgTec PowerDown Controller Watchdog Timer.
>>
>> Signed-off-by: Jude Abraham <Jude.Abraham-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Naidu Tellapati <Naidu.Tellapati-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>

> Reviewed-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

> Two small comments you may want to fix if you need to re-spin the series though:

Will address following two review comments and submit new Patch set today.

>> +static struct watchdog_info pdc_wdt_info = {
>> +       .options                = WDIOF_SETTIMEOUT |
>> +                                 WDIOF_KEEPALIVEPING |
>> +                                 WDIOF_MAGICCLOSE,
>> +       .identity = "IMG PDC Watchdog",
>> +};

> Inconsistent spacing.

>> +       ret = watchdog_init_timeout(&pdc_wdt->wdt_dev, timeout, &pdev->dev);
>> +       if (ret < 0)
>> +               pdc_wdt->wdt_dev.timeout = pdc_wdt->wdt_dev.max_timeout;

> Maybe print an error here?

Regards,
Naidu.
--
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