Message-ID: <CAMuHMdUaHPoCDnu3sD7WOnvyaMp5qNjuRGaFxEDQp1ZoFkcrvg@mail.gmail.com>
Date:   Thu, 23 Sep 2021 18:19:14 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        Will McVicker <willmcvicker@...gle.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        "Cc: Android Kernel" <kernel-team@...roid.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-samsung-soc <linux-samsung-soc@...r.kernel.org>,
        Greg KH <gregkh@...uxfoundation.org>
Subject: Re: [PATCH v1 2/4] soc: samsung: change SOC_SAMSUNG default config logic

Hi Lee,

On Thu, Sep 23, 2021 at 3:42 PM Lee Jones <lee.jones@...aro.org> wrote:
> On Thu, 23 Sep 2021, Krzysztof Kozlowski wrote:
> > On 23/09/2021 14:39, Lee Jones wrote:
> > > As I've explained before, the trigger for all of this was
> > > SERIAL_SAMSUNG which is required for early console on supported
> > > Samsung platforms i.e. this symbol *has* to be built-in.
> >
> > Actually SERIAL_SAMSUNG does not have to be built-in. It is necessary
> > for built-in only for debugging or development, not for real products.
>
> Right.  And in the early stages, GKI is used for early (non-released)
> H/W (this is also the part of the reason these differences can't be
> upstreamed early/now/yet) and sometimes changes break things requiring
> low-level debugging techniques to solve (inc. early console).
>
> > Unlike other drivers which have to be built-in, e.g. clocks or pinctrl,
> > or heavily tested whether setup from initrd works. Plus not breaking
> > distros who like to have everything as module (solution from Geert?)...
>
> We don't know which drivers *need* to be built-in yet.
>
> Clocks is probably not a good example even, since the power-on default
> is most likely all-on, which is fine.  Pinctrl remains to be seen.

Clocks is an excellent example: if a clock is missing, the driver
will fail to probe (unless the clock is considered optional by
the driver), regardless of the power-on or boot loader defaults.
With fw_devlinks=on (which is the default now, and developed by a
Google engineer (GKI or another division?)), the driver won't even
get to the probing point.

Pinctrl is different, as unless I'm mistaken, drivers will still
probe if the pin control driver is missing, so they will work if the
power-on or boot loader defaults of pin control are fine.

> > > In order for
> > > this to built-in ARCH_EXYNOS has to be enabled due to the listed
> > > dependencies in Kconfig.  And since ARCH_EXYNOS 'selects' all of these
> > > different extra symbols, it means they too will be built-in, meaning
> > > that a) the core binary will be unnecessarily bloated and b) vendors
> > > who wish to overwrite/replace this functionality with their
> > > non-shareable value-add, are not able to do so.
> >
> > I am sorry, but this is not reflecting status we want to have in
> > usptream. Everything selected by ARCH_EXYNOS *has to be selected* for
> > supported platforms. Since vendor does not contribute anything new
> > (except mentioned one work for UFS), we are not going to sacrifice
> > supported mainline platforms for a non-cooperative out-of-tree unknown
> > platforms.
>
> The is the part of the discussion that is the most contentious.
>
> Ideally we wouldn't have to enable any ARCH_* explicitly.  Greg has
> mentioned this publicly on a number of discussions.  However, removing
> the dependencies (from Kconfig in this case) is in contention with
> other user's use-cases.  No one wants to be asked seemingly irrelevant
> configuration questions during the config stages of a kernel build.
>
> So we are forced to enable ARCH_* to have our requirements built-in
> (ARCH_EXYNOS for SAMSUNG_SERIAL [early console] in this case).
> Unfortunately, this comes with additional cruft that we *might* not
> want (resulting in bloat) or that we wish to overwrite with more
> featureful driver modules.  We can't do that if these features are
> built-in.

The question is if Linux can actually boot on the affected platform
without this "additional cruft" builtin, for which we still haven't
received any confirmation yet...

So claiming to be "upstream first", and submitting patches, is great,
but only if the changes you're upstreaming actually work.
If they don't, and if you insist on keeping on upstreaming them,
without providing evidence that they don't break the affected platform
completely, perhaps this should be treated similar to the UMN patches?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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

Powered by blists - more mailing lists