| Message ID | cff4a5d615ec54a8bc234f26f8c4ead04e1dccd4.1702174575.git.ehem+openwrt@m5p.com |
|---|---|
| State | New |
| Headers | show |
| Series | Misc kernel config cleanup and small adjustments | expand |
Hi, On Sun, 10 Dec 2023 at 18:51, Elliott Mitchell <ehem+openwrt@m5p.com> wrote: > > Date: Fri, 3 Nov 2023 22:57:43 -0700 > > Enabling an Intel chipset feature on a platform originally made by > National Semiconductor and later bought by AMD. Could we cut the Intel > enthusiasm? > > This reverts commit 4eda2fddf2995c8ade2b1e0faddc8ce1f1e0ec5f. commit 4eda2fddf2995c8ade2b1e0faddc8ce1f1e0ec5f says "This makes it possible to use the MCP23S08 i/o expander on geode platforms with linux 4.14." So we don't need to enable PINCTRL (via other symbols) anymore for this? Please add an explanation why this is fine now to the commit message. Best Regards, Jonas
On Tue, Dec 12, 2023 at 04:54:05PM +0100, Jonas Gorski wrote: > > On Sun, 10 Dec 2023 at 18:51, Elliott Mitchell <ehem+openwrt@m5p.com> wrote: > > > > Date: Fri, 3 Nov 2023 22:57:43 -0700 > > > > Enabling an Intel chipset feature on a platform originally made by > > National Semiconductor and later bought by AMD. Could we cut the Intel > > enthusiasm? > > > > This reverts commit 4eda2fddf2995c8ade2b1e0faddc8ce1f1e0ec5f. > > commit 4eda2fddf2995c8ade2b1e0faddc8ce1f1e0ec5f says "This makes it > possible to use the MCP23S08 i/o expander on geode platforms with > linux 4.14." Problem is this is nonsensical. A Geode processor CANNOT be paired with an Intel chipset (the original Geode GX1 came out of National Semiconductor/Cyrix and was later bought by AMD). > So we don't need to enable PINCTRL (via other symbols) anymore for this? 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.
On 2023-12-13 02:45, Elliott Mitchell wrote: > On Tue, Dec 12, 2023 at 04:54:05PM +0100, Jonas Gorski wrote: >> >> On Sun, 10 Dec 2023 at 18:51, Elliott Mitchell <ehem+openwrt@m5p.com> >> wrote: >> > >> > Date: Fri, 3 Nov 2023 22:57:43 -0700 >> > >> > Enabling an Intel chipset feature on a platform originally made by >> > National Semiconductor and later bought by AMD. Could we cut the Intel >> > enthusiasm? >> > >> > This reverts commit 4eda2fddf2995c8ade2b1e0faddc8ce1f1e0ec5f. >> >> commit 4eda2fddf2995c8ade2b1e0faddc8ce1f1e0ec5f says "This makes it >> possible to use the MCP23S08 i/o expander on geode platforms with >> linux 4.14." > > Problem is this is nonsensical. A Geode processor CANNOT be paired > with > an Intel chipset (the original Geode GX1 came out of National > Semiconductor/Cyrix and was later bought by AMD). > >> So we don't need to enable PINCTRL (via other symbols) anymore for >> this? > > 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. Hi. 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. 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. Regards, Martin
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.
On Wed, 13 Dec 2023 at 23:55, Elliott Mitchell <ehem+openwrt@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
On Thu, Dec 14, 2023 at 10:38:34AM +0100, Jonas Gorski wrote: > On Wed, 13 Dec 2023 at 23:55, Elliott Mitchell <ehem+openwrt@m5p.com> wrote: > > > > 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). I didn't specifically check these, but I was operating on believing the situation was roughly this. > 4. In a later Linux release (4.15), PINCTRL was changed to a > user-selectable symbol [3]. Kconfig isn't my enemy, but nor is it my friend. I can believe that was sufficient to have that effect. I was unaware it had actually changed since that delta is rather small to cause such a change. > 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. I remain doubtful of anyone having used CONFIG_PINCTRL on a Geode system, but I was never planning to do anything beyond reverting 4eda2fddf2. Notice how the patch does nothing more or less than reverting 4eda2fddf2? I have noticed rather a lot of Intel-only features sneaking into OpenWRT's kernels. As someone who relies on ECC for reliability, Intel is presently unacceptable so those are bloat to me. There was a problem of 4eda2fddf2 looking quite strange since it chose to enable an option ill-suited to the hardware.
diff --git a/target/linux/x86/geode/config-5.15 b/target/linux/x86/geode/config-5.15 index 0104b1e7b3..6463934b77 100644 --- a/target/linux/x86/geode/config-5.15 +++ b/target/linux/x86/geode/config-5.15 @@ -94,22 +94,6 @@ CONFIG_PC8736x_GPIO=y # CONFIG_PCENGINES_APU2 is not set CONFIG_PCI_MMCONFIG=y # CONFIG_PCWATCHDOG is not set -CONFIG_PINCTRL=y -# CONFIG_PINCTRL_ALDERLAKE is not set -# CONFIG_PINCTRL_BAYTRAIL is not set -# CONFIG_PINCTRL_BROXTON is not set -# CONFIG_PINCTRL_CANNONLAKE is not set -# CONFIG_PINCTRL_CHERRYVIEW is not set -# CONFIG_PINCTRL_DENVERTON is not set -# CONFIG_PINCTRL_ELKHARTLAKE is not set -# CONFIG_PINCTRL_EMMITSBURG is not set -# CONFIG_PINCTRL_GEMINILAKE is not set -# CONFIG_PINCTRL_JASPERLAKE is not set -# CONFIG_PINCTRL_LAKEFIELD is not set -# CONFIG_PINCTRL_LEWISBURG is not set -# CONFIG_PINCTRL_LYNXPOINT is not set -# CONFIG_PINCTRL_SUNRISEPOINT is not set -# CONFIG_PINCTRL_TIGERLAKE is not set # CONFIG_PMIC_OPREGION is not set CONFIG_PNP=y CONFIG_PNPACPI=y @@ -138,7 +122,7 @@ CONFIG_X86_ALIGNMENT_16=y # CONFIG_X86_AMD_PLATFORM_DEVICE is not set CONFIG_X86_CPUID=y # CONFIG_X86_E_POWERSAVER is not set -CONFIG_X86_INTEL_LPSS=y +# CONFIG_X86_INTEL_LPSS is not set # CONFIG_X86_LONGHAUL is not set # CONFIG_X86_MCE is not set CONFIG_X86_MINIMUM_CPU_FAMILY=5 diff --git a/target/linux/x86/geode/config-6.1 b/target/linux/x86/geode/config-6.1 index cf02d2b9b0..e19cf7ea10 100644 --- a/target/linux/x86/geode/config-6.1 +++ b/target/linux/x86/geode/config-6.1 @@ -104,24 +104,6 @@ CONFIG_PC8736x_GPIO=y CONFIG_PCI_MMCONFIG=y # CONFIG_PCWATCHDOG is not set # CONFIG_PEAQ_WMI is not set -CONFIG_PINCTRL=y -# CONFIG_PINCTRL_ALDERLAKE is not set -# CONFIG_PINCTRL_BAYTRAIL is not set -# CONFIG_PINCTRL_BROXTON is not set -# CONFIG_PINCTRL_CANNONLAKE is not set -# CONFIG_PINCTRL_CHERRYVIEW is not set -# CONFIG_PINCTRL_DENVERTON is not set -# CONFIG_PINCTRL_ELKHARTLAKE is not set -# CONFIG_PINCTRL_EMMITSBURG is not set -# CONFIG_PINCTRL_GEMINILAKE is not set -# CONFIG_PINCTRL_JASPERLAKE is not set -# CONFIG_PINCTRL_LAKEFIELD is not set -# CONFIG_PINCTRL_LEWISBURG is not set -# CONFIG_PINCTRL_LYNXPOINT is not set -# CONFIG_PINCTRL_METEORLAKE is not set -# CONFIG_PINCTRL_SUNRISEPOINT is not set -# CONFIG_PINCTRL_TIGERLAKE is not set -# CONFIG_PMIC_OPREGION is not set CONFIG_PNP=y CONFIG_PNPACPI=y # CONFIG_PNPBIOS is not set @@ -165,7 +147,7 @@ CONFIG_X86_ALIGNMENT_16=y # CONFIG_X86_AMD_PSTATE_UT is not set CONFIG_X86_CPUID=y # CONFIG_X86_E_POWERSAVER is not set -CONFIG_X86_INTEL_LPSS=y +# CONFIG_X86_INTEL_LPSS is not set # CONFIG_X86_LONGHAUL is not set # CONFIG_X86_MCE is not set CONFIG_X86_MINIMUM_CPU_FAMILY=5
Date: Fri, 3 Nov 2023 22:57:43 -0700 Enabling an Intel chipset feature on a platform originally made by National Semiconductor and later bought by AMD. Could we cut the Intel enthusiasm? This reverts commit 4eda2fddf2995c8ade2b1e0faddc8ce1f1e0ec5f. Signed-off-by: Elliott Mitchell <ehem+openwrt@m5p.com> --- target/linux/x86/geode/config-5.15 | 18 +----------------- target/linux/x86/geode/config-6.1 | 20 +------------------- 2 files changed, 2 insertions(+), 36 deletions(-)