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

Guenter Roeck
2015-05-01T17:32:13Z

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