Re: [PATCH 02/11] gpiolib: introduce devm_fwnode_gpiod_get_index()
On Thu, Sep 12, 2019 at 10:48:13AM +0100, Linus Walleij wrote: > On Wed, Sep 11, 2019 at 8:52 AM 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. > > > > Signed-off-by: Dmitry Torokhov > > 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. -- Dmitry
Re: [PATCH 02/11] gpiolib: introduce devm_fwnode_gpiod_get_index()
On Wed, Sep 11, 2019 at 8:52 AM 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. > > Signed-off-by: Dmitry Torokhov 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
Re: [PATCH 02/11] gpiolib: introduce devm_fwnode_gpiod_get_index()
On Wed, Sep 11, 2019 at 6:01 PM Andy Shevchenko 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
Re: [PATCH 02/11] gpiolib: introduce devm_fwnode_gpiod_get_index()
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. -- With Best Regards, Andy Shevchenko
