[PATCH RESEND 06/11] Revert "x86/geode: enable X86_INTEL_LPSS to select PINCTRL"

[Jonas Gorski]

On Wed, 13 Dec 2023 at 23:55, Elliott Mitchell <ehem+openwrt at m5p.com> wrote:
>
> On Wed, Dec 13, 2023 at 01:59:07PM +0100, Martin Schiller wrote:
> > On 2023-12-13 02:45, Elliott Mitchell wrote:
> > >
> > > No idea, I wasn't able to find very much information when I looked at
> > > this.
> > >
> > > I did find:
> > > https://lists.openwrt.org/pipermail/openwrt-devel/2018-August/019479.html
> > >
> > > This doesn't tell me what platform Martin Schiller was trying for.
> > > 17f30bfcf7 makes me suspect Martin Schiller was simply trying to do
> > > this
> > > to all x86 platforms and didn't realize geode was a specialized target.
> > >
> > > Alternatively Martin Schiller may have been trying to use a MCP23S08 on
> > > a
> > > Geode processor.  Unfortunately using CONFIG_X86_INTEL_LPSS is a
> > > bizzare
> > > choice since CONFIG_X86_AMD_PLATFORM_DEVICE has fewer side-effects and
> > > then current Geodes were AMD processors.
> > >
> > > With sparse information the former is my present belief.  Is anyone
> > > reading this list using a Geode processor with a MCP23S08?  Otherwise
> > > my
> > > present belief is only people with Intel x86 processors are interested
> > > in
> > > the MCP23S08.
>
> > The problem was and is that the PINCTRL subsystem can only be used on
> > x86
> > platforms if either X86_INTEL_LPSS or X86_AMD_PLATFORM_DEVICE is
> > activated.
> > I no longer know why I chose the former at the time.
>
> Which leaves me suspecting the reason was you had a computer with a
> processor from Intel.
>
> > X86_AMD_PLATFORM_DEVICE is now activated for x86/generic and x86/64.
> >
> >  From my point of view, we can deactivate X86_INTEL_LPSS if no one else
> > need it.
>
> Seeing how Xiaopo Zhang submitted patches to enable them, I assume at
> least 1 other person used them on x86/64 at some point.
>
> What situation/hardware were YOU using CONFIG_PINCTRL for?
>
> Were you using CONFIG_PINCTRL on a desktop which had an Intel processor?
>
> Were you using CONFIG_PINCTRL on a system which had a Geode processor?
>
> If someone out there is actively using CONFIG_PINCTRL on a system with a
> Geode processor, I would disable CONFIG_X86_INTEL_LPSS and enable
> CONFIG_X86_AMD_PLATFORM_DEVICE.  The reason is both options select
> CONFIG_COMMON_CLK and CONFIG_PINCTRL, but CONFIG_X86_INTEL_LPSS
> additionally selects CONFIG_IOSF_MBI (less bloat).
>
> If my belief no one is using CONFIG_PINCTRL on a Geode platform is
> correct, then the original patch is correct.  From examination of the
> Linux kernel source, I believe none of Geode's normal peripherals go
> through the PINCTRL subsystem.
>
> Problem is too few people have systems with Geode processors in use, so
> support is difficult.

Here, I'll do some research work for you:

1. To select the MCP23S08 driver you need to have PINCTRL enabled
since 4.13 (see also [1]).
2. At time of Linux 4.14, PINCTRL was a non user-selectable symbol [2].
3. Therefore, a driver selecting this was needed in the kernel config
(it didn't matter which one).
4. In a later Linux release (4.15), PINCTRL was changed to a
user-selectable symbol [3].
5. Therefore, the intel driver is not needed anymore, but PINCTRL
needs to stay enabled.

And since we build the MCP23S08 driver as a module/kmod package, it
really doesn't matter if this driver is used or not; having it
available makes sure it can be installed if needed.

Micromanaging which drivers/modules should be available to which
(sub-)targets really doesn't provide much benefit compared to the
effort for it, unless the driver/module is one specific to the
hardware targeted by the (sub)target. And as you said, too few people
have systems with Geode processors, so getting a definite answer there
is difficult.

Best Regards,
Jonas

[1] https://github.com/openwrt/openwrt/commit/a904003b9b5fe2744ee5d5d8718c54d001f1c93e
[2] https://elixir.bootlin.com/linux/v4.14.333/source/drivers/pinctrl/Kconfig#L5
[3] https://github.com/torvalds/linux/commit/d219b924611a5cceb17cc6b9a8dd103ab9668c94