Re: [PATCH] watchdog: introduce the ARM64 SBSA watchdog driver

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

On Fri, May 01, 2015 at 12:28:48PM -0500, Timur Tabi wrote:
> On 05/01/2015 11:16 AM, Timur Tabi wrote:
> >>>+    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >>>+    if (!res || !res->start) {
> >>>+        dev_err(&pdev->dev, "could not get control address\n");
> >>>+        return -ENOMEM;
> >>>+    }
> >>>+
> >>devm_ioremap_resource already prints an error message if res is NULL.
> >>And res->start can not be 0 unless there is a severe infrastructure
> >>problem.
> >
> >Will fix.
> >
> >>>+    data->control = devm_ioremap_resource(&pdev->dev, res);
> >>>+    if (!data->control)
> >>>+        return -ENOMEM;
> >>>+
> >>>+    res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> >>>+    if (!res || !res->start) {
> >>>+        dev_err(&pdev->dev, "could not get refresh address\n");
> >>>+        return -ENOMEM;
> >>>+    }
> >>Same here.
> 
> So I must be missing something here.  I'm only printing an error message if
> platform_get_resource() fails.  I'm not printing a message if
> devm_ioremap_resource() fails.  Are you saying that I should not print an
> error message if platform_get_resource() fails?  What's wrong with that?
> 
devm_ioremap_resource already prints a message. For this reason, elsewhere in the
kernel the check for !res before calling devm_ioremap_resource is being removed,
leaving the error handling to devm_ioremap_resource. I would suggest to do the
same here.

Guenter