On 3/6/20 2:03 PM, Greg Kroah-Hartman wrote:

On Fri, Mar 06, 2020 at 01:53:01PM +0100, Geert Uytterhoeven wrote:

quoted

Hi Greg,

On Fri, Mar 6, 2020 at 1:29 PM Greg Kroah-Hartman
[off-list ref] wrote:

quoted

On Fri, Mar 06, 2020 at 11:23:01AM +0100, Geert Uytterhoeven wrote:

quoted

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 <redacted>

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 [off-list ref]
AuthorDate: Thu Feb 20 11:26:27 2020 +0100
Commit:     Greg Kroah-Hartman [off-list ref]
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. 

quoted

quoted

quoted

---
 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.

quoted

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).

quoted

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.

quoted

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.

quoted

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.

quoted

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.

quoted

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.

quoted

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel