Re: [PATCH v1 6/7] mfd: lpc_ich: Add support for pinctrl in non-ACPI system

Andy Shevchenko
2021-04-12T16:57:04Z

On Mon, Apr 12, 2021 at 06:01:06PM +0200, Henning Schild wrote:
> Am Mon, 8 Mar 2021 14:20:19 +0200
> schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
>
> > From: Tan Jui Nee <jui.nee.tan@intel.com>
> >
> > Add support for non-ACPI systems, such as system that uses
> > Advanced Boot Loader (ABL) whereby a platform device has to be created
> > in order to bind with pin control and GPIO.
> >
> > At the moment, Intel Apollo Lake In-Vehicle Infotainment (IVI) system
> > requires a driver to hide and unhide P2SB to lookup P2SB BAR and pass
> > the PCI BAR address to GPIO.

...

> > +/* Offset data for Apollo Lake GPIO controllers */
> > +#define APL_GPIO_SOUTHWEST_OFFSET 0xc00000
> > +#define APL_GPIO_SOUTHWEST_SIZE 0x654
> > +#define APL_GPIO_NORTHWEST_OFFSET 0xc40000
> > +#define APL_GPIO_NORTHWEST_SIZE 0x764
> > +#define APL_GPIO_NORTH_OFFSET 0xc50000
> > +#define APL_GPIO_NORTH_SIZE 0x76c
>
> drivers/pinctrl/intel/pinctrl-broxton.c:653
> BXT_COMMUNITY(0, 77),
>
> > +#define APL_GPIO_WEST_OFFSET 0xc70000
> > +#define APL_GPIO_WEST_SIZE 0x674
>
> All these sizes correlate with 4 magic numbers from pinctrl-broxton.
>
> SIZE - 0x500 (pad_base?) - 4 (no clue) / 8
>
> It might be worth basing both numbers on a define and giving the magic
> numbers some names.

I didn't get this, sorry. The numbers above just precise sizes of the
resources. Actually they all one page anyway, so, I can drop magic of SIZEs and
leave only offsets.

> But all this seems like duplication of pinctrl-broxton, maybe the
> pinctrl driver should unhide the p2sb ...

Definitely should not. It's not a business of the pin control driver to know
how it has to be instantiated (or from what data). These offsets belong to the
platform description and since firmware hides the device without given an
appropriate ACPI device node, we have only one choice (assuming firmware is
carved in stone) -- board files.

P2SB on the other hand is a slice of many (independent) devices. There is no
"proper" place to unhide it except some core part of x86 / PCI.

--
With Best Regards,
Andy Shevchenko