| Message ID | 20200306102301.16870-1-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 7e13d0a6b189169d9339a6ef96383cd6f0e00b2c |
| Headers | show |
| Series | Revert "tty: serial: samsung_tty: build it for any platform" | expand |
On Fri, Mar 06, 2020 at 11:23:01AM +0100, Geert Uytterhoeven wrote: > This reverts commit 175b558d0efb8b4f33aa7bd2c1b5389b912d3019. > > When the user configures a kernel without support for Samsung SoCs, it > makes no sense to ask the user about enabling "Samsung SoC serial > support", as Samsung serial ports can only be found on Samsung SoCs. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > drivers/tty/serial/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig > index 880b962015302dca..932ad51099deae7d 100644 > --- a/drivers/tty/serial/Kconfig > +++ b/drivers/tty/serial/Kconfig > @@ -237,6 +237,7 @@ config SERIAL_CLPS711X_CONSOLE > > config SERIAL_SAMSUNG > tristate "Samsung SoC serial support" > + depends on PLAT_SAMSUNG || ARCH_EXYNOS || COMPILE_TEST > select SERIAL_CORE > help > Support for the on-chip UARTs on the Samsung S3C24XX series CPUs, {sigh} No, I don't want this. My "goal" is to be able to get rid of all of the crazy "PLAT_*" symbols as they make it impossible to build a single kernel that supports multiple ARM64 systems. As an example of just such a system, see the 5.4 tree here: https://android.googlesource.com/kernel/common/+/refs/heads/android-5.4 it is now building and booting on multiple SoCs. But yes, it still does have to enable some PLAT_* config options, but the goal is to not have to do that eventually. There is no reason that we need vendor-specific config options just to lump random drivers into, like serial drivers. If the hardware is not present, the driver will just not bind to the hardware, and all is fine. Just like x86, we don't have this issue there, and ARM64 should also not have this. Sorry for delay in writing this back to the original thread where you objected to the original patch, it's still in my review queue along with a ton of other serial patches. thanks, greg k-h
On Fri, Mar 06, 2020 at 11:36:52AM +0100, Greg Kroah-Hartman wrote: > On Fri, Mar 06, 2020 at 11:23:01AM +0100, Geert Uytterhoeven wrote: > > This reverts commit 175b558d0efb8b4f33aa7bd2c1b5389b912d3019. > > > > When the user configures a kernel without support for Samsung SoCs, it > > makes no sense to ask the user about enabling "Samsung SoC serial > > support", as Samsung serial ports can only be found on Samsung SoCs. > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > --- > > drivers/tty/serial/Kconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig > > index 880b962015302dca..932ad51099deae7d 100644 > > --- a/drivers/tty/serial/Kconfig > > +++ b/drivers/tty/serial/Kconfig > > @@ -237,6 +237,7 @@ config SERIAL_CLPS711X_CONSOLE > > > > config SERIAL_SAMSUNG > > tristate "Samsung SoC serial support" > > + depends on PLAT_SAMSUNG || ARCH_EXYNOS || COMPILE_TEST > > select SERIAL_CORE > > help > > Support for the on-chip UARTs on the Samsung S3C24XX series CPUs, > > {sigh} > > No, I don't want this. My "goal" is to be able to get rid of all of the > crazy "PLAT_*" symbols as they make it impossible to build a single > kernel that supports multiple ARM64 systems. > > As an example of just such a system, see the 5.4 tree here: > https://android.googlesource.com/kernel/common/+/refs/heads/android-5.4 > it is now building and booting on multiple SoCs. > > But yes, it still does have to enable some PLAT_* config options, but > the goal is to not have to do that eventually. > > There is no reason that we need vendor-specific config options just to > lump random drivers into, like serial drivers. If the hardware is not > present, the driver will just not bind to the hardware, and all is fine. > > Just like x86, we don't have this issue there, and ARM64 should also not > have this. > > Sorry for delay in writing this back to the original thread where you > objected to the original patch, it's still in my review queue along with > a ton of other serial patches. Here's another good example of this happening, it's not just me working toward this goal: https://lore.kernel.org/lkml/20200305103228.9686-2-zhang.lyra@gmail.com/
Hi Greg, On Fri, Mar 6, 2020 at 1:29 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Fri, Mar 06, 2020 at 11:23:01AM +0100, Geert Uytterhoeven wrote: > > This reverts commit 175b558d0efb8b4f33aa7bd2c1b5389b912d3019. > > > > When the user configures a kernel without support for Samsung SoCs, it > > makes no sense to ask the user about enabling "Samsung SoC serial > > support", as Samsung serial ports can only be found on Samsung SoCs. > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > --- > > drivers/tty/serial/Kconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig > > index 880b962015302dca..932ad51099deae7d 100644 > > --- a/drivers/tty/serial/Kconfig > > +++ b/drivers/tty/serial/Kconfig > > @@ -237,6 +237,7 @@ config SERIAL_CLPS711X_CONSOLE > > > > config SERIAL_SAMSUNG > > tristate "Samsung SoC serial support" > > + depends on PLAT_SAMSUNG || ARCH_EXYNOS || COMPILE_TEST > > select SERIAL_CORE > > help > > Support for the on-chip UARTs on the Samsung S3C24XX series CPUs, > > {sigh} Exactly my feeling. > No, I don't want this. My "goal" is to be able to get rid of all of the > crazy "PLAT_*" symbols as they make it impossible to build a single > kernel that supports multiple ARM64 systems. This dependency does not make it impossible to build a single kernel that supports multiple ARM64 systems. Those "PLAT_*" symbols are not crazy. They are needed to configure a kernel for your specific hardware, leaving out support you don't need. Not everyone has the hardware resources to run an allyesconfig kernel. > As an example of just such a system, see the 5.4 tree here: > https://android.googlesource.com/kernel/common/+/refs/heads/android-5.4 > it is now building and booting on multiple SoCs. arm/multi_v7_defconfig and arm64/defconfig kernels are already booting on multiple SoCs in upstream, and have done so for years. > But yes, it still does have to enable some PLAT_* config options, but > the goal is to not have to do that eventually. Whether the dependency is present or not does not change this. > There is no reason that we need vendor-specific config options just to > lump random drivers into, like serial drivers. If the hardware is not > present, the driver will just not bind to the hardware, and all is fine. Not having the dependency means you will ask the user useless questions when configuring his kernel. My goal is to make kernel configuration easier, not more difficult. > Just like x86, we don't have this issue there, and ARM64 should also not > have this. No, because x86 is considered the golden standard ;-) Dropping those dependencies is similar to always having a simple PCI core without any host PCI bridges, dropping "depends on PCI" from all PCI drivers, and building an all*config kernel for your old i386 that predates PCI (you can replace PCI by ACPI to modernize the example). What am I missing?!? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Fri, Mar 06, 2020 at 01:53:01PM +0100, Geert Uytterhoeven wrote: > Hi Greg, > > On Fri, Mar 6, 2020 at 1:29 PM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > On Fri, Mar 06, 2020 at 11:23:01AM +0100, Geert Uytterhoeven wrote: > > > This reverts commit 175b558d0efb8b4f33aa7bd2c1b5389b912d3019. > > > > > > When the user configures a kernel without support for Samsung SoCs, it > > > makes no sense to ask the user about enabling "Samsung SoC serial > > > support", as Samsung serial ports can only be found on Samsung SoCs. > > > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > --- > > > drivers/tty/serial/Kconfig | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig > > > index 880b962015302dca..932ad51099deae7d 100644 > > > --- a/drivers/tty/serial/Kconfig > > > +++ b/drivers/tty/serial/Kconfig > > > @@ -237,6 +237,7 @@ config SERIAL_CLPS711X_CONSOLE > > > > > > config SERIAL_SAMSUNG > > > tristate "Samsung SoC serial support" > > > + depends on PLAT_SAMSUNG || ARCH_EXYNOS || COMPILE_TEST > > > select SERIAL_CORE > > > help > > > Support for the on-chip UARTs on the Samsung S3C24XX series CPUs, > > > > {sigh} > > Exactly my feeling. > > > No, I don't want this. My "goal" is to be able to get rid of all of the > > crazy "PLAT_*" symbols as they make it impossible to build a single > > kernel that supports multiple ARM64 systems. > > This dependency does not make it impossible to build a single > kernel that supports multiple ARM64 systems. > > Those "PLAT_*" symbols are not crazy. They are needed to configure a > kernel for your specific hardware, leaving out support you don't need. > Not everyone has the hardware resources to run an allyesconfig kernel. > > > As an example of just such a system, see the 5.4 tree here: > > https://android.googlesource.com/kernel/common/+/refs/heads/android-5.4 > > it is now building and booting on multiple SoCs. > > arm/multi_v7_defconfig and arm64/defconfig kernels are already booting > on multiple SoCs in upstream, and have done so for years. > > > But yes, it still does have to enable some PLAT_* config options, but > > the goal is to not have to do that eventually. > > Whether the dependency is present or not does not change this. > > > There is no reason that we need vendor-specific config options just to > > lump random drivers into, like serial drivers. If the hardware is not > > present, the driver will just not bind to the hardware, and all is fine. > > Not having the dependency means you will ask the user useless questions > when configuring his kernel. > My goal is to make kernel configuration easier, not more difficult. > > > Just like x86, we don't have this issue there, and ARM64 should also not > > have this. > > No, because x86 is considered the golden standard ;-) > > Dropping those dependencies is similar to always having a simple PCI > core without any host PCI bridges, dropping "depends on PCI" from all > PCI drivers, and building an all*config kernel for your old i386 that > predates PCI (you can replace PCI by ACPI to modernize the example). > > What am I missing?!? "depends on PCI" describes the hardware bus that a driver depends on. PLAT_FOO is just trying to somehow classify that this type of driver only shows up on this vendor's devices. It is not defining the hardware at all. We try to always describe functionality of hardware, not try to declare specific vendor's hardware choices, right? PLAT_FOO is interesting, but given that a specific driver is really not tied to that platform logically, only by virtue that no one else might not happen to have that hardware, it seems odd to have that. Yes, asking lots of questions is tough, but we passed that problem so long ago. Are we now trying to add PLAT_FOO entries to all hardware drivers in order to make this type of selection easier? I thought we were just doing that by providing defconfig files to make the initial selection saner. thanks, greg k-h
Hi Greg, On Fri, Mar 6, 2020 at 2:03 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Fri, Mar 06, 2020 at 01:53:01PM +0100, Geert Uytterhoeven wrote: > > On Fri, Mar 6, 2020 at 1:29 PM Greg Kroah-Hartman > > <gregkh@linuxfoundation.org> wrote: > > > On Fri, Mar 06, 2020 at 11:23:01AM +0100, Geert Uytterhoeven wrote: > > > > This reverts commit 175b558d0efb8b4f33aa7bd2c1b5389b912d3019. > > > > > > > > When the user configures a kernel without support for Samsung SoCs, it > > > > makes no sense to ask the user about enabling "Samsung SoC serial > > > > support", as Samsung serial ports can only be found on Samsung SoCs. > > > > > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > --- > > > > drivers/tty/serial/Kconfig | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig > > > > index 880b962015302dca..932ad51099deae7d 100644 > > > > --- a/drivers/tty/serial/Kconfig > > > > +++ b/drivers/tty/serial/Kconfig > > > > @@ -237,6 +237,7 @@ config SERIAL_CLPS711X_CONSOLE > > > > > > > > config SERIAL_SAMSUNG > > > > tristate "Samsung SoC serial support" > > > > + depends on PLAT_SAMSUNG || ARCH_EXYNOS || COMPILE_TEST > > > > select SERIAL_CORE > > > > help > > > > Support for the on-chip UARTs on the Samsung S3C24XX series CPUs, > > > > > > {sigh} > > > > Exactly my feeling. > > > > > No, I don't want this. My "goal" is to be able to get rid of all of the > > > crazy "PLAT_*" symbols as they make it impossible to build a single > > > kernel that supports multiple ARM64 systems. > > > > This dependency does not make it impossible to build a single > > kernel that supports multiple ARM64 systems. > > > > Those "PLAT_*" symbols are not crazy. They are needed to configure a > > kernel for your specific hardware, leaving out support you don't need. > > Not everyone has the hardware resources to run an allyesconfig kernel. > > > > > As an example of just such a system, see the 5.4 tree here: > > > https://android.googlesource.com/kernel/common/+/refs/heads/android-5.4 > > > it is now building and booting on multiple SoCs. > > > > arm/multi_v7_defconfig and arm64/defconfig kernels are already booting > > on multiple SoCs in upstream, and have done so for years. > > > > > But yes, it still does have to enable some PLAT_* config options, but > > > the goal is to not have to do that eventually. > > > > Whether the dependency is present or not does not change this. > > > > > There is no reason that we need vendor-specific config options just to > > > lump random drivers into, like serial drivers. If the hardware is not > > > present, the driver will just not bind to the hardware, and all is fine. > > > > Not having the dependency means you will ask the user useless questions > > when configuring his kernel. > > My goal is to make kernel configuration easier, not more difficult. > > > > > Just like x86, we don't have this issue there, and ARM64 should also not > > > have this. > > > > No, because x86 is considered the golden standard ;-) > > > > Dropping those dependencies is similar to always having a simple PCI > > core without any host PCI bridges, dropping "depends on PCI" from all > > PCI drivers, and building an all*config kernel for your old i386 that > > predates PCI (you can replace PCI by ACPI to modernize the example). > > > > What am I missing?!? > > "depends on PCI" describes the hardware bus that a driver depends on. Yes. > PLAT_FOO is just trying to somehow classify that this type of driver > only shows up on this vendor's devices. It is not defining the hardware > at all. We try to always describe functionality of hardware, not try to > declare specific vendor's hardware choices, right? DT-based drivers do not bind to a hardware-specific bus, and thus there is no config symbol for a hardware-specific bus they can depend on. Still, there are hardware classes, based on SoC vendors and SoC families. Hence PLAT_FOO describes the latter. > PLAT_FOO is interesting, but given that a specific driver is really not > tied to that platform logically, only by virtue that no one else might > not happen to have that hardware, it seems odd to have that. There exist IP cores that are present on either PCI and non-PCI hardware. With hardware-specific bus drivers, drivers for these need to implement both a pci_driver and <some_other>_driver. And they depend on PCI || <OTHER>. With DT and ACPI, and device properties, a single platform_driver needs to be written, just the matching is done differently. And there is no "hard" (no "else the driver doesn't link") need for a hardware-specific config-symbol dependency. But it's still good to have one. > Yes, asking lots of questions is tough, but we passed that problem so > long ago. Are we now trying to add PLAT_FOO entries to all hardware > drivers in order to make this type of selection easier? I thought we > were just doing that by providing defconfig files to make the initial > selection saner. Defconfigs were the previous step, from an evolutionary point of view. Arm64 has a single defconfig. All dependencies must be expressed in Kconfig. I can take arm64/defconfig, remove support for other SoC families, and I'll have a good kernel for my hardware, without bagage I don't need. Without these dependencies, I have to remove lots and lots of drivers I won't need. If you want to compile drivers for hardware that cannot be present, use COMPILE_TEST=y. Gr{oetje,eeting}s, Geert
On 3/6/20 2:03 PM, Greg Kroah-Hartman wrote: > On Fri, Mar 06, 2020 at 01:53:01PM +0100, Geert Uytterhoeven wrote: >> Hi Greg, >> >> On Fri, Mar 6, 2020 at 1:29 PM Greg Kroah-Hartman >> <gregkh@linuxfoundation.org> wrote: >>> On Fri, Mar 06, 2020 at 11:23:01AM +0100, Geert Uytterhoeven wrote: >>>> This reverts commit 175b558d0efb8b4f33aa7bd2c1b5389b912d3019. >>>> >>>> When the user configures a kernel without support for Samsung SoCs, it >>>> makes no sense to ask the user about enabling "Samsung SoC serial >>>> support", as Samsung serial ports can only be found on Samsung SoCs. >>>> >>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> The original commit has been merged so quickly after being posted that people had no time to review/NAK it properly: commit 175b558d0efb8b4f33aa7bd2c1b5389b912d3019 Author: Greg Kroah-Hartman <gregkh@linuxfoundation.org> AuthorDate: Thu Feb 20 11:26:27 2020 +0100 Commit: Greg Kroah-Hartman <gregkh@linuxfoundation.org> CommitDate: Thu Feb 20 13:46:19 2020 +0100 Also I have no idea why Krzysztof has posted his "Reviewed-by:" tag for the original commit. >>>> --- >>>> drivers/tty/serial/Kconfig | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig >>>> index 880b962015302dca..932ad51099deae7d 100644 >>>> --- a/drivers/tty/serial/Kconfig >>>> +++ b/drivers/tty/serial/Kconfig >>>> @@ -237,6 +237,7 @@ config SERIAL_CLPS711X_CONSOLE >>>> >>>> config SERIAL_SAMSUNG >>>> tristate "Samsung SoC serial support" >>>> + depends on PLAT_SAMSUNG || ARCH_EXYNOS || COMPILE_TEST >>>> select SERIAL_CORE >>>> help >>>> Support for the on-chip UARTs on the Samsung S3C24XX series CPUs, >>> >>> {sigh} >> >> Exactly my feeling. >> >>> No, I don't want this. My "goal" is to be able to get rid of all of the >>> crazy "PLAT_*" symbols as they make it impossible to build a single >>> kernel that supports multiple ARM64 systems. No, they don't. Geert has explained this to you twice (however for some reason you seem to keep on ignoring this). Also on ARM64 there is no PLAT_SAMSUNG defined at all! In case of PLAT_SAMSUNG usage in SERIAL_SAMSUNG dependencies it is just a shortcut for saying: depends on ARCH_S3C24XX || ARCH_S3C64XX || ARCH_S5PV210 || \ ARCH_EXYNOS || COMPILE_TEST and we can just use ARCH_* dependencies as well (PLAT_SAMSUNG is used only because it is shorter). ARCH_* dependencies on ARM platforms are used to describe SoC families and are in no conflict of supporting multi-platforms images (somer SoC families don't support such images but for other reasons). On ARM64 building the single kernel that supports multiple ARM64 systems is the default and all Samsung SoCs are included in such image (as only some Samsung Exynos SoCs are 64-bit capable only ARCH_EXYNOS is relevant on ARM64). Also ARM multi-platform support is similar to other archs (like mips or powerpc). >> This dependency does not make it impossible to build a single >> kernel that supports multiple ARM64 systems. >> >> Those "PLAT_*" symbols are not crazy. They are needed to configure a >> kernel for your specific hardware, leaving out support you don't need. >> Not everyone has the hardware resources to run an allyesconfig kernel. >> >>> As an example of just such a system, see the 5.4 tree here: >>> https://android.googlesource.com/kernel/common/+/refs/heads/android-5.4 >>> it is now building and booting on multiple SoCs. >> >> arm/multi_v7_defconfig and arm64/defconfig kernels are already booting >> on multiple SoCs in upstream, and have done so for years. >> >>> But yes, it still does have to enable some PLAT_* config options, but >>> the goal is to not have to do that eventually. >> >> Whether the dependency is present or not does not change this. >> >>> There is no reason that we need vendor-specific config options just to >>> lump random drivers into, like serial drivers. If the hardware is not >>> present, the driver will just not bind to the hardware, and all is fine. >> >> Not having the dependency means you will ask the user useless questions >> when configuring his kernel. >> My goal is to make kernel configuration easier, not more difficult. >> >>> Just like x86, we don't have this issue there, and ARM64 should also not >>> have this. On x86 we have BIOS, ACPI and other forms of hardware abstractions that we don't have on ARM64. Please also note that we have things like "depends on X86_32" or "depends on ACPI" also on x86. >> No, because x86 is considered the golden standard ;-) >> >> Dropping those dependencies is similar to always having a simple PCI >> core without any host PCI bridges, dropping "depends on PCI" from all >> PCI drivers, and building an all*config kernel for your old i386 that >> predates PCI (you can replace PCI by ACPI to modernize the example). >> >> What am I missing?!? > > "depends on PCI" describes the hardware bus that a driver depends on. > > PLAT_FOO is just trying to somehow classify that this type of driver > only shows up on this vendor's devices. It is not defining the hardware Like described above: PLAT_SAMSUNG is just a way to share the common architecture code between different Samsung SoCs families inside arch/arm/*: arch/arm/plat-samsung/Kconfig: ... config PLAT_SAMSUNG bool depends on PLAT_S3C24XX || ARCH_S3C64XX || ARCH_EXYNOS || ARCH_S5PV210 default y select GENERIC_IRQ_CHIP select NO_IOPORT_MAP help Base platform code for all Samsung SoC based systems ... while PLAT_S3C24XX comes from arch/arm/mach-s3c24xx/Kconfig: ... if ARCH_S3C24XX config PLAT_S3C24XX def_bool y select GPIOLIB select NO_IOPORT_MAP select S3C_DEV_NAND select IRQ_DOMAIN select COMMON_CLK help Base platform code for any Samsung S3C24XX device ... We can use ARCH_* symbols for device drivers dependencies directly, PLAT_SAMSUNG is used only because it is shorter. > at all. We try to always describe functionality of hardware, not try to > declare specific vendor's hardware choices, right? > > PLAT_FOO is interesting, but given that a specific driver is really not > tied to that platform logically, only by virtue that no one else might > not happen to have that hardware, it seems odd to have that. We don't expect Samsung Serial hardware to start appearing on non ARM/Samsung SoCs any day soon (if ever). Currently your changes make i.e. x86_64 configs to ask about its support, (people doing "make oldconfig" on linux-next are seeing it). > Yes, asking lots of questions is tough, but we passed that problem so > long ago. Are we now trying to add PLAT_FOO entries to all hardware Yes, we have passed that problem with the usage of COMPILE_TEST config option and your change defeats its whole purpose. I've asked you in the original commit mail thread whether you are planning to remove COMPILE_TEST and you seem to avoid answering my question. > drivers in order to make this type of selection easier? I thought we > were just doing that by providing defconfig files to make the initial > selection saner. > > thanks, > > greg k-h Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
On 3/6/20 2:03 PM, Greg Kroah-Hartman wrote: > On Fri, Mar 06, 2020 at 01:53:01PM +0100, Geert Uytterhoeven wrote: >> Hi Greg, >> >> On Fri, Mar 6, 2020 at 1:29 PM Greg Kroah-Hartman >> <gregkh@linuxfoundation.org> wrote: >>> On Fri, Mar 06, 2020 at 11:23:01AM +0100, Geert Uytterhoeven wrote: >>>> This reverts commit 175b558d0efb8b4f33aa7bd2c1b5389b912d3019. >>>> >>>> When the user configures a kernel without support for Samsung SoCs, it >>>> makes no sense to ask the user about enabling "Samsung SoC serial >>>> support", as Samsung serial ports can only be found on Samsung SoCs. >>>> >>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >>>> --- >>>> drivers/tty/serial/Kconfig | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig >>>> index 880b962015302dca..932ad51099deae7d 100644 >>>> --- a/drivers/tty/serial/Kconfig >>>> +++ b/drivers/tty/serial/Kconfig >>>> @@ -237,6 +237,7 @@ config SERIAL_CLPS711X_CONSOLE >>>> >>>> config SERIAL_SAMSUNG >>>> tristate "Samsung SoC serial support" >>>> + depends on PLAT_SAMSUNG || ARCH_EXYNOS || COMPILE_TEST >>>> select SERIAL_CORE >>>> help >>>> Support for the on-chip UARTs on the Samsung S3C24XX series CPUs, >>> >>> {sigh} [...] >> Dropping those dependencies is similar to always having a simple PCI >> core without any host PCI bridges, dropping "depends on PCI" from all >> PCI drivers, and building an all*config kernel for your old i386 that >> predates PCI (you can replace PCI by ACPI to modernize the example). >> >> What am I missing?!? > > "depends on PCI" describes the hardware bus that a driver depends on. > > PLAT_FOO is just trying to somehow classify that this type of driver > only shows up on this vendor's devices. It is not defining the hardware > at all. We try to always describe functionality of hardware, not try to > declare specific vendor's hardware choices, right? > > PLAT_FOO is interesting, but given that a specific driver is really not > tied to that platform logically, only by virtue that no one else might > not happen to have that hardware, it seems odd to have that. Your particular patch is not about removing PLAT_FOO dependency but about removing actual architecture/platform specific dependencies. Please look at your patch and note that in addition to removing PLAT_SAMSUNG dependency (even ignoring for a moment that it can be replaced by a few ARCH_* dependencies and PLAT_SAMSUNG was used only because it was shorter) has also removed ARCH_EXYNOS dependency. How do you explain this? > Yes, asking lots of questions is tough, but we passed that problem so > long ago. Are we now trying to add PLAT_FOO entries to all hardware > drivers in order to make this type of selection easier? I thought we We are not going to add anything because for the vast majority of the drivers the needed entries are already there: $ find drivers/ -name 'Kconfig*'|xargs cat|grep "depends on"|grep ARCH_|wc -l 1310 > were just doing that by providing defconfig files to make the initial > selection saner. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
.On Fri, 6 Mar 2020 at 11:23, Geert Uytterhoeven <geert+renesas@glider.be> wrote: > > This reverts commit 175b558d0efb8b4f33aa7bd2c1b5389b912d3019. > > When the user configures a kernel without support for Samsung SoCs, it > makes no sense to ask the user about enabling "Samsung SoC serial > support", as Samsung serial ports can only be found on Samsung SoCs. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > drivers/tty/serial/Kconfig | 1 + > 1 file changed, 1 insertion(+) > Discussion about removal and then re-adding of PLAT_SAMSUNG and ARCH_EXYNOS dependencies remind me [1]: "[RFC] Input: tm2-touchkey - add hardware dependency". In both cases the driver is clearly only for Samsung SoC or even for particular device, although one could argue that touchscreen could be reused while re-usage of serial IP of SoC is highly unlikely. My understanding, maybe not correct, of "depends on" syntax is a kernel source code, building or running dependency. I do not see it as a hardware dependency. Although Samsung S3C/Exynos serial driver will not exist outside of Samsung SoC, there is no kernel dependency. Unless I missed something... I understand and agree with concerns mentioned in [1] and in the thread here, that removal of this "depends on" makes life of distributions and generic users more difficult. To solve this problem I was thinking about adding weaker type of dependency. A hint about hardware dependency. Something like the "imply" is for "select". This did not happen, therefore I still stand on my understanding of "depends on" thus I gave positive feedback to Greg's patch. It is a pity though that Greg's patch was applied so fast while discussion was still on going... [1] https://patchwork.kernel.org/patch/9695769/ Best regards, Krzysztof
Hi Krzysztof, On Mon, Mar 9, 2020 at 7:09 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > .On Fri, 6 Mar 2020 at 11:23, Geert Uytterhoeven > <geert+renesas@glider.be> wrote: > > This reverts commit 175b558d0efb8b4f33aa7bd2c1b5389b912d3019. > > > > When the user configures a kernel without support for Samsung SoCs, it > > makes no sense to ask the user about enabling "Samsung SoC serial > > support", as Samsung serial ports can only be found on Samsung SoCs. > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > --- > > drivers/tty/serial/Kconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > Discussion about removal and then re-adding of PLAT_SAMSUNG and > ARCH_EXYNOS dependencies remind me [1]: "[RFC] Input: tm2-touchkey - > add hardware dependency". > > In both cases the driver is clearly only for Samsung SoC or even for > particular device, although one could argue that touchscreen could be > reused while re-usage of serial IP of SoC is highly unlikely. My > understanding, maybe not correct, of "depends on" syntax is a kernel > source code, building or running dependency. I do not see it as a > hardware dependency. Although Samsung S3C/Exynos serial driver will > not exist outside of Samsung SoC, there is no kernel dependency. > Unless I missed something... The touchscreen is something different: I can easily mount that type of touchscreen on my own board, while I cannot integrate a Samsung serial port on my board, unless I'm using a Samsung SoC. > I understand and agree with concerns mentioned in [1] and in the > thread here, that removal of this "depends on" makes life of > distributions and generic users more difficult. To solve this problem > I was thinking about adding weaker type of dependency. A hint about > hardware dependency. Something like the "imply" is for "select". This > did not happen, therefore I still stand on my understanding of > "depends on" thus I gave positive feedback to Greg's patch. A weak dependency can be expressed using "|| COMPILE_TESTING". <(not so) wild idea> During the past few days, I've been giving this more thought. And I realized we might as well get rid of pci_driver, and just have platform_drivers that match against "pci<VendorID>,<DeviceID>" (yes, this is what real Open Firmware had in the compatible property, cfr. http://users.telenet.be/geertu/Linux/PPC/pci/ethernetAT4/). That way there would be no longer a build dependency on CONFIG_PCI, and we can drop all "depends on PCI" from driver Kconfig symbols. But would dropping that dependency be welcomed? Perhaps, as "everybody" uses PCI. Now repeat the exercise for Zorro, TURBOchannel, NuBus, Sbus, PCMCIA, ..., and wait for the outcry from Linus suddenly seeing lots of questions about support for hardware he can't possibly have in his machine... </(not so) wild idea> Gr{oetje,eeting}s, Geert
On Fri, Mar 06, 2020 at 11:23:01AM +0100, Geert Uytterhoeven wrote: > This reverts commit 175b558d0efb8b4f33aa7bd2c1b5389b912d3019. > > When the user configures a kernel without support for Samsung SoCs, it > makes no sense to ask the user about enabling "Samsung SoC serial > support", as Samsung serial ports can only be found on Samsung SoCs. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > drivers/tty/serial/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig > index 880b962015302dca..932ad51099deae7d 100644 > --- a/drivers/tty/serial/Kconfig > +++ b/drivers/tty/serial/Kconfig > @@ -237,6 +237,7 @@ config SERIAL_CLPS711X_CONSOLE > > config SERIAL_SAMSUNG > tristate "Samsung SoC serial support" > + depends on PLAT_SAMSUNG || ARCH_EXYNOS || COMPILE_TEST > select SERIAL_CORE > help > Support for the on-chip UARTs on the Samsung S3C24XX series CPUs, > -- > 2.17.1 > Ok, I really don't like the PLAT_* stuff, but I'll go apply this now and push to remove that instead... thanks, greg k-h
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig index 880b962015302dca..932ad51099deae7d 100644 --- a/drivers/tty/serial/Kconfig +++ b/drivers/tty/serial/Kconfig @@ -237,6 +237,7 @@ config SERIAL_CLPS711X_CONSOLE config SERIAL_SAMSUNG tristate "Samsung SoC serial support" + depends on PLAT_SAMSUNG || ARCH_EXYNOS || COMPILE_TEST select SERIAL_CORE help Support for the on-chip UARTs on the Samsung S3C24XX series CPUs,
This reverts commit 175b558d0efb8b4f33aa7bd2c1b5389b912d3019. When the user configures a kernel without support for Samsung SoCs, it makes no sense to ask the user about enabling "Samsung SoC serial support", as Samsung serial ports can only be found on Samsung SoCs. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- drivers/tty/serial/Kconfig | 1 + 1 file changed, 1 insertion(+)