From: Bartosz Golaszewski <[email protected]>
Use the improved variant of pinctrl_gpio_can_use_line() which takes a
pointer to the gpio_chip and a controller-relative offset.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/gpio/gpiolib-cdev.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 31fc71a612c2..54ee075410db 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2287,8 +2287,7 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
* FIXME: find a non-racy way to retrieve this information. Maybe a
* lock common to both frameworks?
*/
- ok_for_pinctrl =
- pinctrl_gpio_can_use_line(gc->base + info->offset);
+ ok_for_pinctrl = pinctrl_gpio_can_use_line_new(gc, info->offset);
spin_lock_irqsave(&gpio_lock, flags);
--
2.39.2
On Tue, Oct 03, 2023 at 04:50:42PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> Use the improved variant of pinctrl_gpio_can_use_line() which takes a
> pointer to the gpio_chip and a controller-relative offset.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
> ---
> drivers/gpio/gpiolib-cdev.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index 31fc71a612c2..54ee075410db 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -2287,8 +2287,7 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
> * FIXME: find a non-racy way to retrieve this information. Maybe a
> * lock common to both frameworks?
> */
> - ok_for_pinctrl =
> - pinctrl_gpio_can_use_line(gc->base + info->offset);
> + ok_for_pinctrl = pinctrl_gpio_can_use_line_new(gc, info->offset);
>
_new?? In what sense?
I agree with the change in principle, just not comfortable with the naming.
Cheers,
Kent.
On Tue, Oct 3, 2023 at 6:02 PM Kent Gibson <[email protected]> wrote:
> On Tue, Oct 03, 2023 at 04:50:42PM +0200, Bartosz Golaszewski wrote:
...
> I agree with the change in principle, just not comfortable with the naming.
+1 here. I proposed some names, have you seen my comment(s)?
--
With Best Regards,
Andy Shevchenko
On Tue, Oct 03, 2023 at 06:17:27PM +0300, Andy Shevchenko wrote:
> On Tue, Oct 3, 2023 at 6:02 PM Kent Gibson <[email protected]> wrote:
> > On Tue, Oct 03, 2023 at 04:50:42PM +0200, Bartosz Golaszewski wrote:
>
> ...
>
> > I agree with the change in principle, just not comfortable with the naming.
>
> +1 here. I proposed some names, have you seen my comment(s)?
>
I have now - any of those work for me.
Whichever is consistent with what we are using for gpiochip functions in
gpiolib would make most sense to me.
Cheers,
Kent.
On Tue, Oct 3, 2023 at 5:24 PM Kent Gibson <[email protected]> wrote:
>
> On Tue, Oct 03, 2023 at 06:17:27PM +0300, Andy Shevchenko wrote:
> > On Tue, Oct 3, 2023 at 6:02 PM Kent Gibson <[email protected]> wrote:
> > > On Tue, Oct 03, 2023 at 04:50:42PM +0200, Bartosz Golaszewski wrote:
> >
> > ...
> >
> > > I agree with the change in principle, just not comfortable with the naming.
> >
> > +1 here. I proposed some names, have you seen my comment(s)?
> >
>
> I have now - any of those work for me.
> Whichever is consistent with what we are using for gpiochip functions in
> gpiolib would make most sense to me.
>
Does it really matter? It's not here to stay, it's temporary and
exists only until the whole series is applied - which given that it's
limited to gpio and pinctrl, shouldn't take more than one release
cycle.
There are plenty of examples of this naming convention for temporary
symbols - there's even an ongoing effort to replace all .remove()
callbacks with .remove_new() which will then be changed back to
.remove() treewide.
Bart
On Tue, Oct 03, 2023 at 08:07:05PM +0200, Bartosz Golaszewski wrote:
> On Tue, Oct 3, 2023 at 5:24 PM Kent Gibson <[email protected]> wrote:
> >
> > On Tue, Oct 03, 2023 at 06:17:27PM +0300, Andy Shevchenko wrote:
> > > On Tue, Oct 3, 2023 at 6:02 PM Kent Gibson <[email protected]> wrote:
> > > > On Tue, Oct 03, 2023 at 04:50:42PM +0200, Bartosz Golaszewski wrote:
> > >
> > > ...
> > >
> > > > I agree with the change in principle, just not comfortable with the naming.
> > >
> > > +1 here. I proposed some names, have you seen my comment(s)?
> > >
> >
> > I have now - any of those work for me.
> > Whichever is consistent with what we are using for gpiochip functions in
> > gpiolib would make most sense to me.
> >
>
> Does it really matter? It's not here to stay, it's temporary and
> exists only until the whole series is applied - which given that it's
> limited to gpio and pinctrl, shouldn't take more than one release
> cycle.
>
> There are plenty of examples of this naming convention for temporary
> symbols - there's even an ongoing effort to replace all .remove()
> callbacks with .remove_new() which will then be changed back to
> .remove() treewide.
>
This was the only patch that I was included into, so I didn't realise
there was a treewide rename at the end.
Even so, using _new suffix for that purpose is poor (well
pinctrl_gpio_free_new() did draw a laugh, but other than that...).
Perhaps use something specific to the patch series so it is clear what
its purpose is?
Cheers,
Kent.
On Wed, Oct 4, 2023 at 6:16 AM Kent Gibson <[email protected]> wrote:
>
> On Tue, Oct 03, 2023 at 08:07:05PM +0200, Bartosz Golaszewski wrote:
> > On Tue, Oct 3, 2023 at 5:24 PM Kent Gibson <[email protected]> wrote:
> > >
> > > On Tue, Oct 03, 2023 at 06:17:27PM +0300, Andy Shevchenko wrote:
> > > > On Tue, Oct 3, 2023 at 6:02 PM Kent Gibson <[email protected]> wrote:
> > > > > On Tue, Oct 03, 2023 at 04:50:42PM +0200, Bartosz Golaszewski wrote:
> > > >
> > > > ...
> > > >
> > > > > I agree with the change in principle, just not comfortable with the naming.
> > > >
> > > > +1 here. I proposed some names, have you seen my comment(s)?
> > > >
> > >
> > > I have now - any of those work for me.
> > > Whichever is consistent with what we are using for gpiochip functions in
> > > gpiolib would make most sense to me.
> > >
> >
> > Does it really matter? It's not here to stay, it's temporary and
> > exists only until the whole series is applied - which given that it's
> > limited to gpio and pinctrl, shouldn't take more than one release
> > cycle.
> >
> > There are plenty of examples of this naming convention for temporary
> > symbols - there's even an ongoing effort to replace all .remove()
> > callbacks with .remove_new() which will then be changed back to
> > .remove() treewide.
> >
>
> This was the only patch that I was included into, so I didn't realise
> there was a treewide rename at the end.
I didn't want to spam 20+ maintainers with the entire series of 36
patches. Should have probably Cc'ed everyone on the cover letter
though.
> Even so, using _new suffix for that purpose is poor (well
> pinctrl_gpio_free_new() did draw a laugh, but other than that...).
> Perhaps use something specific to the patch series so it is clear what
> its purpose is?
>
I think Linus will end up applying the entire series to his tree in
one go in which case the name really doesn't matter. Do we really need
to bikeshed about the name which will exist for as long as it takes to
apply the series on his laptop? I much more care about preserving
bisectability across the series which it does.
Bart