Re: [PATCH RFC v2 0/3] pinctrl: add ACPI support to pin controller

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Tue, Nov 15, 2022 at 07:15:25PM +0000, Niyas Sait wrote:
> >>> 1) why do you need specific _DSD() for the pin mappings?
> >>
> >> _DSD is required to pass the GPIO number to Pin mapping. Is there a
> better
> >> way to do it ?
> >
> > Don't you have your pins sequential? Doesn't driver know the pin list?
> > Where are the device tree bindings for those properties? Are they already
> > present there?
> 
> They are not sequential. There are multiple GPIO controllers and GPIO pins
> are mapped to different pin ranges.
> 
> Please checkout arch/arm64/boot/dts/freescale/imx8mp.dtsi for device tree
> binding.
> 
> Yes I think it can be done in the driver.
> 
> >>> 2) wgy you need vendor data for some of Pin*() resources?
> >>
> >> Vendor data contains platform specific mux, config and input selection
> >> register for the pin group.
> >
> > Why can't this be in the driver?
> 
> Yes I think it can be done in driver. But I think we still have to pass the
> vendor data from the ACPI resources to the drivers in case if any
> implementation wants to pass vendor specific data for the driver.

I'm afraid that opening this to the drivers will allow to a lot of unknown data
to be encoded in ACPI, so I would really like to see the vendor data to be
absent (ideally).

> On 15/11/2022 18:47, Andy Shevchenko wrote:
> > On Tue, Nov 15, 2022 at 06:29:28PM +0000, Niyas Sait wrote:
> > > > Cover letter doesn't point to the ASL code you are using.
> > > 
> > > I've built this patch with a prototype ACPI implementation for NXP I.MX8MP
> > > platform.
> > > 
> > > You can see the code here: https://gitlab.com/nsait-linaro/acpi-patches/-/blob/master/0001-add-acpi-pinctrl-support-for-i2c-controllers.patch.
> > 
> > Yes, my point that you have to create a cover letter
> > 
> > > > 1) why do you need specific _DSD() for the pin mappings?
> > > 
> > > _DSD is required to pass the GPIO number to Pin mapping. Is there a better
> > > way to do it ?
> > 
> > Don't you have your pins sequential? Doesn't driver know the pin list?
> > Where are the device tree bindings for those properties? Are they already
> > present there?
> > 
> > > > 2) wgy you need vendor data for some of Pin*() resources?
> > > 
> > > Vendor data contains platform specific mux, config and input selection
> > > register for the pin group.
> > 
> > Why can't this be in the driver?
> > 
> > ...
> > 
> > On top of that, how other hardware can utilize what you are doing without
> > adding redundancy to the ACPI?

-- 
With Best Regards,
Andy Shevchenko