[00/36] pinctrl: don't use GPIOLIB global numberspace in helpers

  From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

We have a set of pinctrl helpers for GPIOLIB drivers that take a number
from the global GPIO numberspace as argument. We are trying to get rid
of this global numbering. Let's rework these helpers to use the
recommended gpio_chip + controller-relative offset instead.

This work is split into phases: first let's introduce the new variants
of the helpers. Next: let's convert all users one-by-one for easier
review. Finally let's remove the old helpers and rename the new variants
to take the place of the old ones.

Bartosz Golaszewski (36):
  pinctrl: remove unneeded extern specifiers from consumer.h
  pinctrl: provide new GPIO-to-pinctrl glue helpers
  gpiolib: generic: use new pinctrl GPIO helpers
  gpio: cdev: use pinctrl_gpio_can_use_line_new()
  gpio: rcar: use new pinctrl GPIO helpers
  gpio: tegra: use new pinctrl GPIO helpers
  gpio: em: use new pinctrl GPIO helpers
  gpio: aspeed: use new pinctrl GPIO helpers
  gpio: mvebu: use new pinctrl GPIO helpers
  gpio: pxa: use new pinctrl GPIO helpers
  gpio: rockchip: use new pinctrl GPIO helpers
  gpio: vf610: use new pinctrl GPIO helpers
  pinctrl: nuvoton: use new pinctrl GPIO helpers
  pinctrl: renesas: use new pinctrl GPIO helpers
  pinctrl: bcm: use new pinctrl GPIO helpers
  pinctrl: stm32: use new pinctrl GPIO helpers
  pinctrl: spear: use new pinctrl GPIO helpers
  pinctrl: starfive: use new pinctrl GPIO helpers
  pinctrl: ocelot: use new pinctrl GPIO helpers
  pinctrl: rk805: use new pinctrl GPIO helpers
  pinctrl: cirrus: use new pinctrl GPIO helpers
  pinctrl: mediatek: use new pinctrl GPIO helpers
  pinctrl: axp209: use new pinctrl GPIO helpers
  pinctrl: vt8500: use new pinctrl GPIO helpers
  pinctrl: cy8c95x0: use new pinctrl GPIO helpers
  pinctrl: as3722: use new pinctrl GPIO helpers
  pinctrl: ingenic: use new pinctrl GPIO helpers
  pinctrl: intel: use new pinctrl GPIO helpers
  pinctrl: st: use new pinctrl GPIO helpers
  pinctrl: remove old GPIO helpers
  treewide: rename pinctrl_gpio_can_use_line_new()
  treewide: rename pinctrl_gpio_request_new()
  treewide: rename pinctrl_gpio_free_new()
  treewide: rename pinctrl_gpio_direction_input_new()
  treewide: rename pinctrl_gpio_direction_output_new()
  treewide: rename pinctrl_gpio_set_config_new()

 drivers/gpio/gpio-aspeed.c                    |   6 +-
 drivers/gpio/gpio-em.c                        |   4 +-
 drivers/gpio/gpio-mvebu.c                     |   4 +-
 drivers/gpio/gpio-pxa.c                       |   4 +-
 drivers/gpio/gpio-rcar.c                      |   4 +-
 drivers/gpio/gpio-rockchip.c                  |   4 +-
 drivers/gpio/gpio-tegra.c                     |   8 +-
 drivers/gpio/gpio-vf610.c                     |   4 +-
 drivers/gpio/gpiolib-cdev.c                   |   3 +-
 drivers/gpio/gpiolib.c                        |   6 +-
 drivers/pinctrl/bcm/pinctrl-iproc-gpio.c      |   6 +-
 drivers/pinctrl/cirrus/pinctrl-cs42l43.c      |   4 +-
 drivers/pinctrl/cirrus/pinctrl-lochnagar.c    |   2 +-
 drivers/pinctrl/core.c                        | 182 +++++++++---------
 drivers/pinctrl/intel/pinctrl-cherryview.c    |   4 +-
 drivers/pinctrl/intel/pinctrl-intel.c         |   4 +-
 drivers/pinctrl/intel/pinctrl-lynxpoint.c     |   4 +-
 drivers/pinctrl/mediatek/pinctrl-moore.c      |   4 +-
 drivers/pinctrl/mediatek/pinctrl-mtk-common.c |   4 +-
 drivers/pinctrl/mediatek/pinctrl-paris.c      |   4 +-
 drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c     |   8 +-
 drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c     |   8 +-
 drivers/pinctrl/pinctrl-as3722.c              |   4 +-
 drivers/pinctrl/pinctrl-axp209.c              |   2 +-
 drivers/pinctrl/pinctrl-cy8c95x0.c            |   4 +-
 drivers/pinctrl/pinctrl-ingenic.c             |  11 +-
 drivers/pinctrl/pinctrl-ocelot.c              |   4 +-
 drivers/pinctrl/pinctrl-rk805.c               |   4 +-
 drivers/pinctrl/pinctrl-st.c                  |   4 +-
 drivers/pinctrl/renesas/gpio.c                |   8 +-
 drivers/pinctrl/renesas/pinctrl-rzg2l.c       |   4 +-
 drivers/pinctrl/renesas/pinctrl-rzv2m.c       |   4 +-
 drivers/pinctrl/spear/pinctrl-plgpio.c        |   8 +-
 .../starfive/pinctrl-starfive-jh7100.c        |   4 +-
 .../starfive/pinctrl-starfive-jh7110.c        |   4 +-
 drivers/pinctrl/stm32/pinctrl-stm32.c         |   8 +-
 drivers/pinctrl/vt8500/pinctrl-wmt.c          |   4 +-
 include/linux/pinctrl/consumer.h              |  57 +++---
 38 files changed, 210 insertions(+), 205 deletions(-)
  

On Tue, Oct 3, 2023 at 4:51 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> We have a set of pinctrl helpers for GPIOLIB drivers that take a number
> from the global GPIO numberspace as argument. We are trying to get rid
> of this global numbering. Let's rework these helpers to use the
> recommended gpio_chip + controller-relative offset instead.
>
> This work is split into phases: first let's introduce the new variants
> of the helpers. Next: let's convert all users one-by-one for easier
> review. Finally let's remove the old helpers and rename the new variants
> to take the place of the old ones.

Almost too good attention to process here, I hope you used some
tooling and didn't do all this by hand...

I reviewed it by applying the lot with b4 on a branch off
my devel branch, and

git diff devel..HEAD

which shows what the goal of the patches is and since the
kernel clearly looks better after than before the patches:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Or I can just merge this branch into my devel (for v6.7)
branch, and offer you the same as immutable.
Which is my plan.

Shall I just do it?

Yours,
Linus Walleij
  

On Tue, Oct 3, 2023 at 11:51 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Oct 3, 2023 at 4:51 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > We have a set of pinctrl helpers for GPIOLIB drivers that take a number
> > from the global GPIO numberspace as argument. We are trying to get rid
> > of this global numbering. Let's rework these helpers to use the
> > recommended gpio_chip + controller-relative offset instead.
> >
> > This work is split into phases: first let's introduce the new variants
> > of the helpers. Next: let's convert all users one-by-one for easier
> > review. Finally let's remove the old helpers and rename the new variants
> > to take the place of the old ones.
>
> Almost too good attention to process here, I hope you used some
> tooling and didn't do all this by hand...
>
> I reviewed it by applying the lot with b4 on a branch off
> my devel branch, and
>
> git diff devel..HEAD
>
> which shows what the goal of the patches is and since the
> kernel clearly looks better after than before the patches:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> Or I can just merge this branch into my devel (for v6.7)
> branch, and offer you the same as immutable.
> Which is my plan.
>

I'll need to send a v2 because there was an issue with one of the stub
declarations and I think we should let it rest on the list for a week
but eventually I think you should just pick up the entire series and
if anything new for the GPIO tree conflicts then we can deal with
immutable tags.

What is your view on Andy's and Kent's issues with the _new() name
suffix? My argument is that it's just temporary and will be gone once
you apply the entire series. Bikeshedding about a temp name is just
unnecessary churn and _new() is as good as anything else.

Bart

> Shall I just do it?
>
> Yours,
> Linus Walleij
  

On Wed, Oct 4, 2023 at 10:12 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> What is your view on Andy's and Kent's issues with the _new() name
> suffix?

We have done similar operations in the past, and it is similar to what
Uwe is doing for the moment with the .remove() callbacks.

Usually the strategy is employed when the work needs to be spread
out over a few merge windows so it is a bit of a marker that "this is
in transition".

There is the  horror story of this staying around forever and becoming
idiomatic: struct napi_struct (include/linux/netdevice.h) where
"napi" means "new API" - yeah that could have been handled better...

If there is more moaning about it I will simply squash all the patches
into one and call it a day - the end result will be the same and no
sign of any *_new suffix anywhere. It was still worth it for reviewing
the driver changes on a per-driver basis so then it becomes one of
those Schopenhauer ladders that you can toss away after climbing
it.

Yours,
Linus Walleij
  

On Wed, Oct 4, 2023 at 11:42 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Oct 4, 2023 at 10:12 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > What is your view on Andy's and Kent's issues with the _new() name
> > suffix?
>
> We have done similar operations in the past, and it is similar to what
> Uwe is doing for the moment with the .remove() callbacks.
>
> Usually the strategy is employed when the work needs to be spread
> out over a few merge windows so it is a bit of a marker that "this is
> in transition".
>
> There is the  horror story of this staying around forever and becoming
> idiomatic: struct napi_struct (include/linux/netdevice.h) where
> "napi" means "new API" - yeah that could have been handled better...
>
> If there is more moaning about it I will simply squash all the patches
> into one and call it a day - the end result will be the same and no
> sign of any *_new suffix anywhere. It was still worth it for reviewing
> the driver changes on a per-driver basis so then it becomes one of
> those Schopenhauer ladders that you can toss away after climbing
> it.

You can go with a compromise and name it better from the start, so at
least the patches that are taking care of renaming back won't be
needed.
Another way to have three or so patches with combined efforts, but still...
  

On Wed, Oct 4, 2023 at 11:36 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Oct 4, 2023 at 11:42 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Wed, Oct 4, 2023 at 10:12 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > What is your view on Andy's and Kent's issues with the _new() name
> > > suffix?
> >
> > We have done similar operations in the past, and it is similar to what
> > Uwe is doing for the moment with the .remove() callbacks.
> >
> > Usually the strategy is employed when the work needs to be spread
> > out over a few merge windows so it is a bit of a marker that "this is
> > in transition".
> >
> > There is the  horror story of this staying around forever and becoming
> > idiomatic: struct napi_struct (include/linux/netdevice.h) where
> > "napi" means "new API" - yeah that could have been handled better...
> >
> > If there is more moaning about it I will simply squash all the patches
> > into one and call it a day - the end result will be the same and no
> > sign of any *_new suffix anywhere. It was still worth it for reviewing
> > the driver changes on a per-driver basis so then it becomes one of
> > those Schopenhauer ladders that you can toss away after climbing
> > it.
>
> You can go with a compromise and name it better from the start, so at
> least the patches that are taking care of renaming back won't be
> needed.

What are you talking about?! The names are *FINE*. I DON'T want to
change them. I want to keep them. The temporary renaming is there to
make the review process easier but the end effect is that the names
stay the same with only the signature changed.

Bart

> Another way to have three or so patches with combined efforts, but still...
>
  

On Wed, Oct 04, 2023 at 11:40:39AM +0200, Bartosz Golaszewski wrote:
> On Wed, Oct 4, 2023 at 11:36 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Wed, Oct 4, 2023 at 11:42 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> > > On Wed, Oct 4, 2023 at 10:12 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > > What is your view on Andy's and Kent's issues with the _new() name
> > > > suffix?

...

> > You can go with a compromise and name it better from the start, so at
> > least the patches that are taking care of renaming back won't be
> > needed.
> 
> What are you talking about?! The names are *FINE*. I DON'T want to
> change them. I want to keep them. The temporary renaming is there to
> make the review process easier but the end effect is that the names
> stay the same with only the signature changed.

I just replied how to leave with renamings done only in a single place
(pinctrl core). Would it work for you?

> > Another way to have three or so patches with combined efforts, but still...