On Tue, 31 Jan 2017 10:39:36 -0800
Dmitry Torokhov [off-list ref] wrote:

On Tue, Jan 31, 2017 at 10:24:24AM +0100, Boris Brezillon wrote:

quoted

On Tue, 31 Jan 2017 01:11:55 -0800
Dmitry Torokhov [off-list ref] wrote:
  

quoted

On Tue, Jan 31, 2017 at 10:07:21AM +0100, Boris Brezillon wrote:  

quoted

On Tue, 31 Jan 2017 00:44:47 -0800
Dmitry Torokhov [off-list ref] wrote:
    

quoted

On Tue, Jan 31, 2017 at 09:04:32AM +0100, Boris Brezillon wrote:    

quoted

On Mon, 30 Jan 2017 17:06:07 -0800
Dmitry Torokhov [off-list ref] wrote:
      

quoted

On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:      

quoted

Rename devm_get_gpiod_from_child() into
devm_fwnode_get_gpiod_from_child() to reflect the fact that this
function is operating on a fwnode object.        

I believe this is completely pointless rename. Are you planning on
adding devm_of_get_gpiod_from_child()? Or
devm_acpt_get_gpiod_from_child()? (I sure hope not).      

Of course not.
      

quoted

Also, on what object? Does it take fwnode as first argument? Or maybe we
should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
know types of all arguments?      

Linus suggested to rename this function [1]. I personally don't care
much about the name, though I agree with Linus that names should be
consistent and descriptive. Moreover, he's the maintainer, and I tend
to follow maintainers suggestion when I contribute to a specific
subsystem.      

OK, I did not know that that was Linus' request, my objection still
stands.
    

quoted

IIUC, you're concerned about the length of this function name. If I had
to drop something it would be the _from_child() suffix, because the
function is not even checking that the child parameter is actually a
direct child (or a descendant) of device->fwnode.      

OK, that sounds better. Actually, we already have
fwnode_get_named_gpiod(), unfortunately it does not do suffixes
permutations. There are also no users, except
devm_get_gpiod_from_child(). So I would:

- rename fwnode_get_named_gpiod() -> static __fwnode_get_named_gpiod()
- made new fwnode_get_named_gpiod() that did suffix permutation and
  called __fwnode_get_named_gpiod() (or pulled its implementation
  inline)    

Sorry but I don't follow you. Why do you need
__fwnode_get_named_gpiod(),    

You do not need it, it will just reduce size of the patch if you use
it. I'd be perfectly fine not with having it and have everything in
fwnode_get_named_gpiod().  

Okay.
  

quoted

quoted

and what is the suffix permutation you're
mentioning here?    

devm_get_gpiod_from_child() tries to apply "-gpio" and "-gpios" suffixes
to the supplied GPIO ID while current fwnode_get_named_gpiod() takes
property name literally.  

fwnode_get_named_gpiod() just mimics what of_get_named_gpiod_flags(),
acpi_node_get_gpiod(), of_find_gpio() and acpi_find_gpio() do. It would
be weird/inconsistent to have the con_id suffixing logic moved in the
fwnode_get_named_gpiod() (if that's what you're suggesting, but I'm not
sure it is).  

Hmm, yeah, I agree, that would be weird. Then let's leave
devm_get_gpiod_from_child() as is ;)

Changing the internal implementation has never been the goal of this
patch. As explained in the commit log, I'm just renaming the function
to make it consistent with other fwnode functions (as suggested by
Linus).
What's happening here is exactly the kind of discussion I wanted to
avoid, and the reason I decided to not change the
devm_get_gpiod_from_child() prototype/name in the first place.

Linus, is this something you really care about? If that's the case, can
you step in?