From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
Alexandre Courbot <gnurou@gmail.com>,
linux-gpio@vger.kernel.org, linux-input@vger.kernel.org,
Bryan Wu <cooloney@gmail.com>, Richard Purdie <rpurdie@rpsys.net>,
Jacek Anaszewski <j.anaszewski@samsung.com>,
linux-leds@vger.kernel.org,
Tomi Valkeinen <tomi.valkeinen@ti.com>,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Russell King <linux@armlinux.org.uk>
Subject: Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()
Date: Tue, 31 Jan 2017 08:04:32 +0000 [thread overview]
Message-ID: <20170131090432.72a1b1b8@bbrezillon> (raw)
In-Reply-To: <20170131010607.GC35974@dtor-ws>
On Mon, 30 Jan 2017 17:06:07 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:
> > 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.
>
> 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.
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. Also, if we want to
be consistent with the rest of the GPIO API, we could rename it
devm_gpiod_get_from_fwnode() (with the function in added in patch 2
renamed into devm_gpiod_get_from_fwnode()).
Linus, what do you think?
One last thing, I don't want to start a discussion where we're
bikeshedding on a function name instead of focusing on the
functionality, so if it turns into this kind of discussion I'll
probably implement devm_fwnode_get_gpiod_from_child() directly in the
atmel NAND driver and wait for an agreement before switching to the
official version.
Regards,
Boris
[1]https://www.spinics.net/lists/arm-kernel/msg558986.html
WARNING: multiple messages have this Message-ID (diff)
From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
Alexandre Courbot <gnurou@gmail.com>,
linux-gpio@vger.kernel.org, linux-input@vger.kernel.org,
Bryan Wu <cooloney@gmail.com>, Richard Purdie <rpurdie@rpsys.net>,
Jacek Anaszewski <j.anaszewski@samsung.com>,
linux-leds@vger.kernel.org,
Tomi Valkeinen <tomi.valkeinen@ti.com>,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Russell King <linux@armlinux.org.uk>
Subject: Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()
Date: Tue, 31 Jan 2017 09:04:32 +0100 [thread overview]
Message-ID: <20170131090432.72a1b1b8@bbrezillon> (raw)
In-Reply-To: <20170131010607.GC35974@dtor-ws>
On Mon, 30 Jan 2017 17:06:07 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:
> > 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.
>
> 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.
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. Also, if we want to
be consistent with the rest of the GPIO API, we could rename it
devm_gpiod_get_from_fwnode() (with the function in added in patch 2
renamed into devm_gpiod_get_from_fwnode()).
Linus, what do you think?
One last thing, I don't want to start a discussion where we're
bikeshedding on a function name instead of focusing on the
functionality, so if it turns into this kind of discussion I'll
probably implement devm_fwnode_get_gpiod_from_child() directly in the
atmel NAND driver and wait for an agreement before switching to the
official version.
Regards,
Boris
[1]https://www.spinics.net/lists/arm-kernel/msg558986.html
next prev parent reply other threads:[~2017-01-31 8:04 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-30 15:41 [PATCH 0/2] gpio: Add the devm_fwnode_get_index_gpiod_from_child() helper Boris Brezillon
2017-01-30 15:41 ` Boris Brezillon
2017-01-30 15:41 ` [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child() Boris Brezillon
2017-01-30 15:41 ` Boris Brezillon
2017-01-30 19:57 ` Jacek Anaszewski
2017-01-30 19:57 ` Jacek Anaszewski
2017-01-31 1:06 ` Dmitry Torokhov
2017-01-31 1:06 ` Dmitry Torokhov
2017-01-31 8:04 ` Boris Brezillon [this message]
2017-01-31 8:04 ` Boris Brezillon
2017-01-31 8:44 ` Dmitry Torokhov
2017-01-31 8:44 ` Dmitry Torokhov
2017-01-31 9:07 ` Boris Brezillon
2017-01-31 9:07 ` Boris Brezillon
2017-01-31 9:11 ` Dmitry Torokhov
2017-01-31 9:11 ` Dmitry Torokhov
2017-01-31 9:24 ` Boris Brezillon
2017-01-31 9:24 ` Boris Brezillon
2017-01-31 18:39 ` Dmitry Torokhov
2017-01-31 18:39 ` Dmitry Torokhov
2017-01-31 19:42 ` Boris Brezillon
2017-01-31 19:42 ` Boris Brezillon
2017-02-01 13:05 ` Linus Walleij
2017-02-01 13:05 ` Linus Walleij
2017-02-01 13:22 ` Boris Brezillon
2017-02-01 13:22 ` Boris Brezillon
2017-02-01 14:51 ` Linus Walleij
2017-02-01 14:51 ` Linus Walleij
2017-02-01 17:18 ` Dmitry Torokhov
2017-02-01 17:18 ` Dmitry Torokhov
2017-02-02 10:07 ` Mika Westerberg
2017-02-02 10:07 ` Mika Westerberg
2017-02-01 17:17 ` Dmitry Torokhov
2017-02-01 17:17 ` Dmitry Torokhov
2017-02-02 10:53 ` Linus Walleij
2017-02-02 10:53 ` Linus Walleij
2017-02-02 11:53 ` Boris Brezillon
2017-02-02 11:53 ` Boris Brezillon
2017-01-30 15:41 ` [PATCH 2/2] gpio: Add the devm_fwnode_get_index_gpiod_from_child() helper Boris Brezillon
2017-01-30 15:41 ` Boris Brezillon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170131090432.72a1b1b8@bbrezillon \
--to=boris.brezillon@free-electrons.com \
--cc=cooloney@gmail.com \
--cc=dmitry.torokhov@gmail.com \
--cc=gnurou@gmail.com \
--cc=j.anaszewski@samsung.com \
--cc=linus.walleij@linaro.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=rpurdie@rpsys.net \
--cc=tomi.valkeinen@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.