[PATCH] reset: Put back *_optional variants

Philipp Zabel p.zabel at pengutronix.de
Mon May 30 03:18:40 PDT 2016


Hi,

Am Freitag, den 27.05.2016, 09:06 +0200 schrieb Hans de Goede:
[...]
> >> So IMHO the following change would be a better way to fix this:
> >>
> >> --- a/include/linux/reset.h
> >> +++ b/include/linux/reset.h
> >> @@ -65,14 +65,14 @@ static inline struct reset_control *__of_reset_control_get(
> >>                                          struct device_node *node,
> >>                                          const char *id, int index, int shared)
> >>   {
> >> -   return ERR_PTR(-EINVAL);
> >> + return ERR_PTR(-ENOTSUPP);
> >>   }
> >>
> >>   static inline struct reset_control *__devm_reset_control_get(
> >>                                          struct device *dev,
> >>                                          const char *id, int index, int shared)
> >>   {
> >> -   return ERR_PTR(-EINVAL);
> >> + return ERR_PTR(-ENOTSUPP);
> >>   }
> >>
> >>   #endif /* CONFIG_RESET_CONTROLLER */
> >
> >
> > I'm good with this. However, per Philipp on a previous thread, the
> > intended behavior is to return -EINVAL for the non-optional functions.
> >
> > http://marc.info/?l=linux-usb&m=146156945528848&w=2

Adding Dinh to Cc: because he wanted this changed from -EINVAL.
My point then was that WARN_ON + -EINVAL is indented in this case.

Given that the warning backtrace already helps to identify the issue
(CONFIG_RESET_CONTROLLER disabled), and that changing the return value
to -ENOTSUPP improves consistency with the rest of the API and reduces
the amount of inline wrappers, I concur.

> As Philipp rightfully points out, calling the non-optional functions
> without CONFIG_RESET_CONTROLLER is a no-no, so IMHO we should not have to
> care too much about the error code in that case, as long as we return
> an error.
> 
> Also note that even before my patch things were already inconsistent
> with the of_...get... functions already always returning
> -ENOTSUPP when CONFIG_RESET_CONTROLLER was not set, to me it seems
> best and also KISS to just return -ENOTSUPP from all get functions
> when CONFIG_RESET_CONTROLLER is not set.
> 
> Anyways this is Philipp's call, this is all just my 2 cents.

Let the stubs return -ENOTSUPP consistently. A quick grep doesn't show
any driver handling -EINVAL explicitly either.

regards
Philipp




More information about the linux-arm-kernel mailing list