On 5/30/2016 4:32 AM, Hans de Goede wrote: > Hi, > > On 30-05-16 12:18, Philipp Zabel wrote: >> 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. > > Great. > > John Youn, can you submit a patch changing the return of the stubs > from -EINVAL to -ENOTSUPP please ? >
Sure. I'll send a patch tomorrow. Regards, John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html