[PATCH v1 0/3] pinctrl: intel: Export intel_gpio_add_pin_ranges() and use it

[PATCH v1 0/3] pinctrl: intel: Export intel_gpio_add_pin_ranges() and use it

[Andy Shevchenko]

Deduplicate more code with help of being exported intel_gpio_add_pin_ranges().

Andy Shevchenko (3):
  pinctrl: intel: Refactor intel_gpio_add_pin_ranges() to make it
    shorter
  pinctrl: intel: Export intel_gpio_add_pin_ranges()
  pinctrl: cherryview: Convert to use intel_gpio_add_pin_ranges()

 drivers/pinctrl/intel/pinctrl-cherryview.c | 20 +-------------------
 drivers/pinctrl/intel/pinctrl-intel.c      | 10 +++++-----
 drivers/pinctrl/intel/pinctrl-intel.h      |  2 ++
 3 files changed, 8 insertions(+), 24 deletions(-)

-- 
2.50.1

[PATCH v1 1/3] pinctrl: intel: Refactor intel_gpio_add_pin_ranges() to make it shorter

[Andy Shevchenko]

Refactor intel_gpio_add_pin_ranges() to make it shorter in binary
and source formats.

Function                                     old     new   delta
intel_gpio_add_pin_ranges                    219     215      -4
Total: Before=15522, After=15518, chg -0.03%

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 8e067aaf3399..a8b80a24e81f 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1348,16 +1348,15 @@ static int intel_gpio_irq_init_hw(struct gpio_chip *gc)
 static int intel_gpio_add_pin_ranges(struct gpio_chip *gc)
 {
 	struct intel_pinctrl *pctrl = gpiochip_get_data(gc);
+	struct device *dev = pctrl->dev;
 	const struct intel_community *community;
 	const struct intel_padgroup *grp;
 	int ret;
 
 	for_each_intel_gpio_group(pctrl, community, grp) {
-		ret = gpiochip_add_pin_range(&pctrl->chip, dev_name(pctrl->dev),
-					     grp->gpio_base, grp->base,
-					     grp->size);
+		ret = gpiochip_add_pin_range(gc, dev_name(dev), grp->gpio_base, grp->base, grp->size);
 		if (ret)
-			return dev_err_probe(pctrl->dev, ret, "failed to add GPIO pin range\n");
+			return dev_err_probe(dev, ret, "failed to add GPIO pin range\n");
 	}
 
 	return 0;
-- 
2.50.1

Re: [PATCH v1 1/3] pinctrl: intel: Refactor intel_gpio_add_pin_ranges() to make it shorter

[Mika Westerberg]

On Mon, Nov 17, 2025 at 08:56:59AM +0100, Andy Shevchenko wrote:
> Refactor intel_gpio_add_pin_ranges() to make it shorter in binary
> and source formats.
> 
> Function                                     old     new   delta
> intel_gpio_add_pin_ranges                    219     215      -4
> Total: Before=15522, After=15518, chg -0.03%
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pinctrl/intel/pinctrl-intel.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
> index 8e067aaf3399..a8b80a24e81f 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -1348,16 +1348,15 @@ static int intel_gpio_irq_init_hw(struct gpio_chip *gc)
>  static int intel_gpio_add_pin_ranges(struct gpio_chip *gc)
>  {
>  	struct intel_pinctrl *pctrl = gpiochip_get_data(gc);
> +	struct device *dev = pctrl->dev;

I prefer this keeping the reverse christmas tree.

Also it can be const.

>  	const struct intel_community *community;
>  	const struct intel_padgroup *grp;
>  	int ret;
>  
>  	for_each_intel_gpio_group(pctrl, community, grp) {
> -		ret = gpiochip_add_pin_range(&pctrl->chip, dev_name(pctrl->dev),
> -					     grp->gpio_base, grp->base,
> -					     grp->size);
> +		ret = gpiochip_add_pin_range(gc, dev_name(dev), grp->gpio_base, grp->base, grp->size);
>  		if (ret)
> -			return dev_err_probe(pctrl->dev, ret, "failed to add GPIO pin range\n");
> +			return dev_err_probe(dev, ret, "failed to add GPIO pin range\n");
>  	}
>  
>  	return 0;
> -- 
> 2.50.1

Re: [PATCH v1 1/3] pinctrl: intel: Refactor intel_gpio_add_pin_ranges() to make it shorter

[Andy Shevchenko]

On Mon, Nov 17, 2025 at 1:27 PM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Mon, Nov 17, 2025 at 08:56:59AM +0100, Andy Shevchenko wrote:

...

> >       struct intel_pinctrl *pctrl = gpiochip_get_data(gc);
> > +     struct device *dev = pctrl->dev;
>
> I prefer this keeping the reverse christmas tree.

And I prefer the logical split, if possible. putting it in between the
intel_community and intel_paggroup lines seems worse to me than the
proposed case.

> Also it can be const.

True, and it makes things closer to what you want if I leave it on the
same line. Do you agree with my reasoning?

> >       const struct intel_community *community;
> >       const struct intel_padgroup *grp;
> >       int ret;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/3] pinctrl: intel: Refactor intel_gpio_add_pin_ranges() to make it shorter

[Mika Westerberg]

On Mon, Nov 17, 2025 at 06:06:16PM +0200, Andy Shevchenko wrote:
> On Mon, Nov 17, 2025 at 1:27 PM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > On Mon, Nov 17, 2025 at 08:56:59AM +0100, Andy Shevchenko wrote:
> 
> ...
> 
> > >       struct intel_pinctrl *pctrl = gpiochip_get_data(gc);
> > > +     struct device *dev = pctrl->dev;
> >
> > I prefer this keeping the reverse christmas tree.
> 
> And I prefer the logical split, if possible. putting it in between the
> intel_community and intel_paggroup lines seems worse to me than the
> proposed case.
> 
> > Also it can be const.
> 
> True, and it makes things closer to what you want if I leave it on the
> same line. Do you agree with my reasoning?

As long as it keeps the reverse chrismas tree after you add const.

> 
> > >       const struct intel_community *community;
> > >       const struct intel_padgroup *grp;
> > >       int ret;
> 
> -- 
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v1 1/3] pinctrl: intel: Refactor intel_gpio_add_pin_ranges() to make it shorter

[Andy Shevchenko]

On Tue, Nov 18, 2025 at 7:27 AM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Mon, Nov 17, 2025 at 06:06:16PM +0200, Andy Shevchenko wrote:
> > On Mon, Nov 17, 2025 at 1:27 PM Mika Westerberg
> > <mika.westerberg@linux.intel.com> wrote:
> > > On Mon, Nov 17, 2025 at 08:56:59AM +0100, Andy Shevchenko wrote:

...

> > > >       struct intel_pinctrl *pctrl = gpiochip_get_data(gc);
> > > > +     struct device *dev = pctrl->dev;
> > >
> > > I prefer this keeping the reverse christmas tree.
> >
> > And I prefer the logical split, if possible. putting it in between the
> > intel_community and intel_paggroup lines seems worse to me than the
> > proposed case.
> >
> > > Also it can be const.
> >
> > True, and it makes things closer to what you want if I leave it on the
> > same line. Do you agree with my reasoning?
>
> As long as it keeps the reverse chrismas tree after you add const.

So, it means "no" then?
Let me try again. The current looking is this:

> > > >       const struct intel_community *community;
> > > >       const struct intel_padgroup *grp;
> > > >       int ret;


After what you are so insisting it will be like

       const struct intel_community *community;
       const struct device *dev = pctrl->dev;
       const struct intel_padgroup *grp;
       int ret;

which disrupts the established grouping of the Intel pin control
related definitions. And in all other functions where two definitions
appear they are never interleaved with other definitions. And I would
like to keep it that way. So, with my proposal it will be like

       const struct device *dev = pctrl->dev;
       const struct intel_community *community;
       const struct intel_padgroup *grp;
       int ret;

or like

       const struct intel_community *community;
       const struct intel_padgroup *grp;
       const struct device *dev = pctrl->dev;
       int ret;

which is not strictly reversed order (but the upper one is close
enough, like a couple of characters shorter than "required").

If you are still insisting on the strict xmas tree reversed order,
assume I drop this change because I reasoned why I want the way I put
it, can you review the rest of the series, please?

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 2/3] pinctrl: intel: Export intel_gpio_add_pin_ranges()

[Andy Shevchenko]

Export intel_gpio_add_pin_ranges() for reuse in other drivers.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 3 ++-
 drivers/pinctrl/intel/pinctrl-intel.h | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index a8b80a24e81f..2424b1bcc322 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1345,7 +1345,7 @@ static int intel_gpio_irq_init_hw(struct gpio_chip *gc)
 	return 0;
 }
 
-static int intel_gpio_add_pin_ranges(struct gpio_chip *gc)
+int intel_gpio_add_pin_ranges(struct gpio_chip *gc)
 {
 	struct intel_pinctrl *pctrl = gpiochip_get_data(gc);
 	struct device *dev = pctrl->dev;
@@ -1361,6 +1361,7 @@ static int intel_gpio_add_pin_ranges(struct gpio_chip *gc)
 
 	return 0;
 }
+EXPORT_SYMBOL_NS_GPL(intel_gpio_add_pin_ranges, "PINCTRL_INTEL");
 
 static unsigned int intel_gpio_ngpio(const struct intel_pinctrl *pctrl)
 {
diff --git a/drivers/pinctrl/intel/pinctrl-intel.h b/drivers/pinctrl/intel/pinctrl-intel.h
index 654af5977603..545bed9fb96c 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.h
+++ b/drivers/pinctrl/intel/pinctrl-intel.h
@@ -273,6 +273,8 @@ int intel_pinctrl_probe_by_uid(struct platform_device *pdev);
 
 extern const struct dev_pm_ops intel_pinctrl_pm_ops;
 
+int intel_gpio_add_pin_ranges(struct gpio_chip *gc);
+
 const struct intel_community *intel_get_community(const struct intel_pinctrl *pctrl,
 						  unsigned int pin);
 
-- 
2.50.1

Re: [PATCH v1 2/3] pinctrl: intel: Export intel_gpio_add_pin_ranges()

[Mika Westerberg]

On Mon, Nov 17, 2025 at 08:57:00AM +0100, Andy Shevchenko wrote:
> Export intel_gpio_add_pin_ranges() for reuse in other drivers.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pinctrl/intel/pinctrl-intel.c | 3 ++-
>  drivers/pinctrl/intel/pinctrl-intel.h | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
> index a8b80a24e81f..2424b1bcc322 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -1345,7 +1345,7 @@ static int intel_gpio_irq_init_hw(struct gpio_chip *gc)
>  	return 0;
>  }
>  
> -static int intel_gpio_add_pin_ranges(struct gpio_chip *gc)

As this is now exported, can you add kernel-doc too?

> +int intel_gpio_add_pin_ranges(struct gpio_chip *gc)
>  {
>  	struct intel_pinctrl *pctrl = gpiochip_get_data(gc);
>  	struct device *dev = pctrl->dev;
> @@ -1361,6 +1361,7 @@ static int intel_gpio_add_pin_ranges(struct gpio_chip *gc)
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_NS_GPL(intel_gpio_add_pin_ranges, "PINCTRL_INTEL");
>  
>  static unsigned int intel_gpio_ngpio(const struct intel_pinctrl *pctrl)
>  {
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.h b/drivers/pinctrl/intel/pinctrl-intel.h
> index 654af5977603..545bed9fb96c 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.h
> +++ b/drivers/pinctrl/intel/pinctrl-intel.h
> @@ -273,6 +273,8 @@ int intel_pinctrl_probe_by_uid(struct platform_device *pdev);
>  
>  extern const struct dev_pm_ops intel_pinctrl_pm_ops;
>  
> +int intel_gpio_add_pin_ranges(struct gpio_chip *gc);
> +

Can you move this down a bit, above intel_get_groups_count()?

>  const struct intel_community *intel_get_community(const struct intel_pinctrl *pctrl,
>  						  unsigned int pin);
>  
> -- 
> 2.50.1

Re: [PATCH v1 2/3] pinctrl: intel: Export intel_gpio_add_pin_ranges()

[Andy Shevchenko]

On Tue, Nov 18, 2025 at 1:47 PM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Mon, Nov 17, 2025 at 08:57:00AM +0100, Andy Shevchenko wrote:
> > Export intel_gpio_add_pin_ranges() for reuse in other drivers.

...

> > -static int intel_gpio_add_pin_ranges(struct gpio_chip *gc)
>
> As this is now exported, can you add kernel-doc too?

OK.

...

> > +int intel_gpio_add_pin_ranges(struct gpio_chip *gc);
>
> Can you move this down a bit, above intel_get_groups_count()?

Sure.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 3/3] pinctrl: cherryview: Convert to use intel_gpio_add_pin_ranges()

[Andy Shevchenko]

Driver is ready to use intel_gpio_add_pin_ranges() directly instead of
custom approach. Convert it now.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index 9c353e1ebe4a..d72e50486370 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1511,24 +1511,6 @@ static int chv_gpio_irq_init_hw(struct gpio_chip *chip)
 	return 0;
 }
 
-static int chv_gpio_add_pin_ranges(struct gpio_chip *chip)
-{
-	struct intel_pinctrl *pctrl = gpiochip_get_data(chip);
-	struct device *dev = pctrl->dev;
-	const struct intel_community *community = &pctrl->communities[0];
-	const struct intel_padgroup *gpp;
-	int ret, i;
-
-	for (i = 0; i < community->ngpps; i++) {
-		gpp = &community->gpps[i];
-		ret = gpiochip_add_pin_range(chip, dev_name(dev), gpp->base, gpp->base, gpp->size);
-		if (ret)
-			return dev_err_probe(dev, ret, "failed to add GPIO pin range\n");
-	}
-
-	return 0;
-}
-
 static int chv_gpio_probe(struct intel_pinctrl *pctrl, int irq)
 {
 	const struct intel_community *community = &pctrl->communities[0];
@@ -1542,7 +1524,7 @@ static int chv_gpio_probe(struct intel_pinctrl *pctrl, int irq)
 
 	chip->ngpio = pctrl->soc->pins[pctrl->soc->npins - 1].number + 1;
 	chip->label = dev_name(dev);
-	chip->add_pin_ranges = chv_gpio_add_pin_ranges;
+	chip->add_pin_ranges = intel_gpio_add_pin_ranges;
 	chip->parent = dev;
 	chip->base = -1;
 
-- 
2.50.1

Re: [PATCH v1 3/3] pinctrl: cherryview: Convert to use intel_gpio_add_pin_ranges()

[Mika Westerberg]

On Mon, Nov 17, 2025 at 08:57:01AM +0100, Andy Shevchenko wrote:
> Driver is ready to use intel_gpio_add_pin_ranges() directly instead of
> custom approach. Convert it now.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>