| Message ID | 20190911075215.78047-3-dmitry.torokhov@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for software nodes to gpiolib | expand |
On Wed, Sep 11, 2019 at 12:52:06AM -0700, Dmitry Torokhov wrote: > devm_fwnode_get_index_gpiod_from_child() is too long, besides the fwnode > in question does not have to be a child of device node. Let's rename it > to devm_fwnode_gpiod_get_index() and keep the old name for compatibility > for now. > > Also let's add a devm_fwnode_gpiod_get() wrapper as majority of the > callers need a single GPIO. > + return devm_fwnode_gpiod_get_index(dev, fwnode, con_id, 0, > + flags, label); At least one parameter can fit previous line, but taking into consideration that moving second one makes it 81 character long, I would do it completely on one line. I don't remember Linus' preferences. > +} > + return devm_fwnode_gpiod_get_index(dev, child, con_id, index, > + flags, label); Ditto.
On Wed, Sep 11, 2019 at 6:01 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Wed, Sep 11, 2019 at 12:52:06AM -0700, Dmitry Torokhov wrote: > > devm_fwnode_get_index_gpiod_from_child() is too long, besides the fwnode > > in question does not have to be a child of device node. Let's rename it > > to devm_fwnode_gpiod_get_index() and keep the old name for compatibility > > for now. > > > > Also let's add a devm_fwnode_gpiod_get() wrapper as majority of the > > callers need a single GPIO. > > > + return devm_fwnode_gpiod_get_index(dev, fwnode, con_id, 0, > > + flags, label); > > At least one parameter can fit previous line, but taking into consideration > that moving second one makes it 81 character long, I would do it completely on > one line. I don't remember Linus' preferences. I don't really have one, I don't mind 80+ chars, I don't mind breaking lines to avoid it. Yours, Linus Walleij
On Wed, Sep 11, 2019 at 8:52 AM Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > devm_fwnode_get_index_gpiod_from_child() is too long, besides the fwnode > in question does not have to be a child of device node. Let's rename it > to devm_fwnode_gpiod_get_index() and keep the old name for compatibility > for now. > > Also let's add a devm_fwnode_gpiod_get() wrapper as majority of the > callers need a single GPIO. > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> The patch is good because this is in line with Rusty Russells API manifesto: 7. The obvious use is (probably) the correct one. 6. The name tells you how to use it. It doesn't apply to my "devel" branch as of now: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=devel If you rebase this and the next patch and send them separately I am willing to apply them already for v5.4 to easy your refactoring work during the v5.5 cycle here, provided we try to fix up the old users ASAP and delete the compatibility fallbacks in the near future. Yours, Linus Walleij
On Thu, Sep 12, 2019 at 10:48:13AM +0100, Linus Walleij wrote: > On Wed, Sep 11, 2019 at 8:52 AM Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: > > > devm_fwnode_get_index_gpiod_from_child() is too long, besides the fwnode > > in question does not have to be a child of device node. Let's rename it > > to devm_fwnode_gpiod_get_index() and keep the old name for compatibility > > for now. > > > > Also let's add a devm_fwnode_gpiod_get() wrapper as majority of the > > callers need a single GPIO. > > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > The patch is good because this is in line with Rusty Russells API > manifesto: > > 7. The obvious use is (probably) the correct one. > 6. The name tells you how to use it. > > It doesn't apply to my "devel" branch as of now: > https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=devel > > If you rebase this and the next patch and send them separately I > am willing to apply them already for v5.4 to easy your refactoring > work during the v5.5 cycle here, provided we try to fix up the old users > ASAP and delete the compatibility fallbacks in the near future. Done. Thanks.
diff --git a/drivers/gpio/gpiolib-devres.c b/drivers/gpio/gpiolib-devres.c index 0acc2cc6e868..190ce7c43b4e 100644 --- a/drivers/gpio/gpiolib-devres.c +++ b/drivers/gpio/gpiolib-devres.c @@ -185,12 +185,11 @@ struct gpio_desc *devm_gpiod_get_from_of_node(struct device *dev, EXPORT_SYMBOL(devm_gpiod_get_from_of_node); /** - * devm_fwnode_get_index_gpiod_from_child - get a GPIO descriptor from a - * device's child node + * devm_fwnode_gpiod_get_index - get a GPIO descriptor from a given node * @dev: GPIO consumer + * @fwnode: firmware node containing GPIO reference * @con_id: function within the GPIO consumer * @index: index of the GPIO to obtain in the consumer - * @child: firmware node (child of @dev) * @flags: GPIO initialization flags * @label: label to attach to the requested GPIO * @@ -200,11 +199,11 @@ EXPORT_SYMBOL(devm_gpiod_get_from_of_node); * On successful request the GPIO pin is configured in accordance with * provided @flags. */ -struct gpio_desc *devm_fwnode_get_index_gpiod_from_child(struct device *dev, - const char *con_id, int index, - struct fwnode_handle *child, - enum gpiod_flags flags, - const char *label) +struct gpio_desc *devm_fwnode_gpiod_get_index(struct device *dev, + struct fwnode_handle *fwnode, + const char *con_id, int index, + enum gpiod_flags flags, + const char *label) { char prop_name[32]; /* 32 is max size of property name */ struct gpio_desc **dr; @@ -224,7 +223,7 @@ struct gpio_desc *devm_fwnode_get_index_gpiod_from_child(struct device *dev, snprintf(prop_name, sizeof(prop_name), "%s", gpio_suffixes[i]); - desc = fwnode_get_named_gpiod(child, prop_name, index, flags, + desc = fwnode_get_named_gpiod(fwnode, prop_name, index, flags, label); if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT)) break; @@ -239,7 +238,7 @@ struct gpio_desc *devm_fwnode_get_index_gpiod_from_child(struct device *dev, return desc; } -EXPORT_SYMBOL(devm_fwnode_get_index_gpiod_from_child); +EXPORT_SYMBOL(devm_fwnode_gpiod_get_index); /** * devm_gpiod_get_index_optional - Resource-managed gpiod_get_index_optional() diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h index b70af921c614..dc0ddcd30515 100644 --- a/include/linux/gpio/consumer.h +++ b/include/linux/gpio/consumer.h @@ -176,11 +176,11 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode, const char *propname, int index, enum gpiod_flags dflags, const char *label); -struct gpio_desc *devm_fwnode_get_index_gpiod_from_child(struct device *dev, - const char *con_id, int index, - struct fwnode_handle *child, - enum gpiod_flags flags, - const char *label); +struct gpio_desc *devm_fwnode_gpiod_get_index(struct device *dev, + struct fwnode_handle *child, + const char *con_id, int index, + enum gpiod_flags flags, + const char *label); #else /* CONFIG_GPIOLIB */ @@ -531,6 +531,29 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode, return ERR_PTR(-ENOSYS); } +static inline +struct gpio_desc *devm_fwnode_gpiod_get_index(struct device *dev, + struct fwnode_handle *fwnode, + const char *con_id, int index, + enum gpiod_flags flags, + const char *label) +{ + return ERR_PTR(-ENOSYS); +} + +#endif /* CONFIG_GPIOLIB */ + +static inline +struct gpio_desc *devm_fwnode_gpiod_get(struct device *dev, + struct fwnode_handle *fwnode, + const char *con_id, + enum gpiod_flags flags, + const char *label) +{ + return devm_fwnode_gpiod_get_index(dev, fwnode, con_id, 0, + flags, label); +} + static inline struct gpio_desc *devm_fwnode_get_index_gpiod_from_child(struct device *dev, const char *con_id, int index, @@ -538,11 +561,10 @@ struct gpio_desc *devm_fwnode_get_index_gpiod_from_child(struct device *dev, enum gpiod_flags flags, const char *label) { - return ERR_PTR(-ENOSYS); + return devm_fwnode_gpiod_get_index(dev, child, con_id, index, + flags, label); } -#endif /* CONFIG_GPIOLIB */ - static inline struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev, const char *con_id, @@ -550,8 +572,7 @@ struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev, enum gpiod_flags flags, const char *label) { - return devm_fwnode_get_index_gpiod_from_child(dev, con_id, 0, child, - flags, label); + return devm_fwnode_gpiod_get_index(dev, child, con_id, 0, flags, label); } #if IS_ENABLED(CONFIG_GPIOLIB) && IS_ENABLED(CONFIG_OF_GPIO)
devm_fwnode_get_index_gpiod_from_child() is too long, besides the fwnode in question does not have to be a child of device node. Let's rename it to devm_fwnode_gpiod_get_index() and keep the old name for compatibility for now. Also let's add a devm_fwnode_gpiod_get() wrapper as majority of the callers need a single GPIO. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- drivers/gpio/gpiolib-devres.c | 19 ++++++++-------- include/linux/gpio/consumer.h | 41 ++++++++++++++++++++++++++--------- 2 files changed, 40 insertions(+), 20 deletions(-)