Re: [PATCH 1/3] pinctrl: mediatek: Add gpio-range record in pinctrl driver

["andriy.shevchenko@intel.com" <andriy.shevchenko@intel.com>]

On Fri, Mar 27, 2026 at 01:33:09PM +0000, Deep Pani wrote:
> Hi Andy,
> 
> You mean gpiochip_add_pin_range(), correct?
> 
> IIRC, that adds to gpiochip's range, not the range we are using for our
> pinctrl driver. 
> 
> The range we are utilizing inside our hardware is of the type struct
> pinctrl_gpio_range. There is no callback in gpiochip that handles this
> type of range

I see, thanks for elaboration.

> I also recall that gpiochip_add_data() doesn't initialize the hw but
> rather initializes the gpiochip from the hw data we will provide in
> mtk_build_gpiochip().

It does for IRQ if one specifies an IRQ chip.

> Thus  we need a function which will help
> initialize the pinctrl_gpio_range inside our pinctrl driver structure.
> This is why we make the mtk_pinctrl_gpio_range_init function here.

But this sounds like the solution from other end of a stick.
OTOH there are a few drivers that use this approach.

> For the second question, we are keeping it because before ACPI is
> invoked we still need some other pins to be configured, especially if
> different pins have different styles of pull configuration. The method
> we use is to define those configurations in the pinctrl-mt8901.c file
> which determines the gpio ranges and maps pinctrl device to acpi, one
> set of gpio ranges per configuration, for different type of pull
> configurations we have different gpio ranges, this callback helps add
> them into the pinctrl subsystem such that other device maintainers can
> easily leverage that subsystem to add their resources in their _CRS
> calls using the common interfaces. 
> 
> Thus we need to keep both the functions.

OK.

> On Thu, 2026-03-26 at 12:33 +0000, Fred-WY Chen (陳威宇) wrote:
> > On Wed, 2025-11-26 at 19:06 +0100, Andy Shevchenko wrote:
> > > 
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.

Please, get rid of this header, it's not compatible with OSS development
process.

> > > On Tue, Nov 25, 2025 at 10:36:34AM +0800, Lei Xue wrote:
> > > > Kernel GPIO subsystem mapping hardware pin number to a different
> > > > range of gpio number. Add gpio-range structure to hold
> > > > the mapped gpio range in pinctrl driver. That enables the kernel
> > > > to search a range of mapped gpio range against a pinctrl device.
> > > 
> > > ...
> > > 
> > > >  static int mtk_build_gpiochip(struct mtk_pinctrl *hw)
> > > >  {
> > > >       struct gpio_chip *chip = &hw->chip;
> > > 
> > > >       if (ret < 0)
> > > >               return ret;
> > > > 
> > > > +     mtk_pinctrl_gpio_range_init(hw, chip);
> > > > +
> > > >       return 0;
> > > 
> > > We have a callback for that in struct gpio_chip. Any reason not to
> > > use it?
> > > 
> > > >  }
> > > 
> > > ...
> > > 
> > > > +     pinctrl_add_gpio_range(hw->pctrl, &hw->range);
> > > 
> > > Not sure if this is needed.
> > 
> > Could you please check this and feedback?

-- 
With Best Regards,
Andy Shevchenko