Re: [PATCH] Revert "tty: serial: samsung_tty: build it for any platform"

[Geert Uytterhoeven <geert@linux-m68k.org>]

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

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