Re: [PATCH] reset: Put back *_optional variants

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux