[PATCH] reset: Put back *_optional variants

[Hans de Goede]

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 ?

Thanks & Regards,

Hans