Re: [PATCH 2/2] watchdog: Add new arm_smc_wdt watchdog driver

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

On Thu, Feb 20, 2020 at 05:50:03PM +1100, Evan Benn wrote:
> > > +     if ((int)res->a0 == PSCI_RET_NOT_SUPPORTED)
> > > +             return -ENOTSUPP;
> >
> > -ENODEV would be better here.
> >
> > > +     if ((int)res->a0 == PSCI_RET_INVALID_PARAMS)
> > > +             return -EINVAL;
> > > +     if ((int)res->a0 < 0)
> > > +             return -EIO;
> 
> In fixing this I found drivers/firmware/psci/psci.c:145
> Which also translates psci codes to errno codes, but uses EOPNOTSUPP:
> 
>     switch (errno) {
>     case PSCI_RET_SUCCESS:
>         return 0;
>     case PSCI_RET_NOT_SUPPORTED:
>         return -EOPNOTSUPP;
>     case PSCI_RET_INVALID_PARAMS:
>     case PSCI_RET_INVALID_ADDRESS:
>         return -EINVAL;
>     case PSCI_RET_DENIED:
>         return -EPERM;
>     };
> 
>     return -EINVAL;
> 
> Are these more appropriate?
> 

It is customary for driver probe functions to return -ENODEV if the
device is not supported. I don't see a reason why this driver should
behave any different. "but this other driver does it" is never a good
argument.

Having said that, yet, with the exception of -EOPNOTSUPP the other
return values would be more appropriate.

Guenter