Re: [PATCH v2 00/12] arm64: Kconfig: Update ARCH_EXYNOS select configs

[Saravana Kannan <saravanak@google.com>]

On Thu, Sep 30, 2021 at 9:52 PM Olof Johansson <olof@lixom.net> wrote:
>
> On Wed, Sep 29, 2021 at 12:48 PM Will McVicker <willmcvicker@google.com> wrote:
> >
> > On Wed, Sep 29, 2021 at 6:02 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@canonical.com> wrote:
> > >
> > > On 29/09/2021 01:56, Will McVicker wrote:
> > > > This is v2 of the series of patches that modularizes a number of core
> > > > ARCH_EXYNOS drivers. Based off of the feedback from the v1 series, I have
> > > > modularized all of the drivers that are removed from the ARCH_EXYNOS
> > > > series of "select XXX". This includes setting the following configs as
> > > > tristate:
> > > >
> > > >  * COMMON_CLK_SAMSUNG
> > > >  * EXYNOS_ARM64_COMMON_CLK
> > > >  * PINCTRL_SAMSUNG
> > > >  * PINCTRL_EXYNOS
> > > >  * EXYNOS_PMU_ARM64
> > > >  * EXYNOS_PM_DOMAINS
> > > >
> > > > Additionally, it introduces the config EXYNOS_PMU_ARM64 and EXYNOS_PMU_ARM
> > > > which was previously EXYNOS_PMU and EXYNOS_PMU_ARM_DRIVERS respectively.
> > > > The reason for these new configs is because we are not able to easily
> > > > modularize the ARMv7 PMU driver due to built-in arch dependencies on
> > > > pmu_base_addr under arch/arm/mach-exynos/*. So the new configs split up
> > > > the ARM and ARM64 portions into two separate configs.
> > > >
> > > > Overall, these drivers didn't require much refactoring and converted to
> > > > modules relatively easily. However, due to my lack of exynos hardware, I
> > > > was not able to boot test these changes. I'm mostly concerned about the
> > > > CLK_OF_DECLARE() changes having dependencies on early timers. So I'm
> > > > requesting help for testing these changes on the respective hardware.
> > > >
> > >
> > > These are all not tested at all? In such case, since these are not
> > > trivial changes, please mark the series as RFT.
> > >
> > > I will not be able to test these for some days, so it must wait.
> > >
> > >
> > > Best regards,
> > > Krzysztof
> >
> > +Cc Arnd and Olof,
> >
> > Hi Krzysztof,
> >
> > To avoid the scrambled conversation from the first patchset, I'm going
> > to address all your general questions here in the cover letter thread
> > so that it's easier for everyone to follow and reference in the
> > future.
>
> This patchset shouldn't go in.
>
> GKI is a fantastic effort, since it finally seems like Google has the
> backbone to put pressure on the vendors to upstream all their stuff.
>
> This patcheset dilutes and undermines all of that by opening up a
> truck-size loophole, reducing the impact of GKI, and overall removes
> leverage to get vendors to do the right thing.
>
> It's against our interest as a community to have this happen, since
> there's no other reasonably justifiable reason to do this.

Oolf, Geert, Krzysztof, Arnd,

I skimmed through the emails and you all make a lot of good points. It
looks like you all at least like the idea of being able to have a
minimal generic kernel where everything that can be a module is a
module. Please correct me if I'm wrong on that.

I was thinking about this patch series and I was wondering if it'd be
good to come at it from the other end. Instead of taking the mostly
builtin generic kernel and trying to rip out drivers as modules (and
not having enough hardware to test them all) and hitting all these
issues, we could come at it from the other end.

A config like ARM64_MINIMAL_GENERIC_KERNEL that's off by default. But
if it's selected, all the "selects" done by the various ARCH_XXX are
not done any more. Something like:

ARCH_XXX
    select XXX_CLK1 if !ARM64_MINIMAL_GENERIC_KERNEL
    select XXX_PINCTRL1 if !ARM64_MINIMAL_GENERIC_KERNEL

ARCH_YYY
    select YYY_CLK1 if !ARM64_MINIMAL_GENERIC_KERNEL
    select YYY_PINCTRL1 if !ARM64_MINIMAL_GENERIC_KERNEL

And ARM64_MINIMAL_GENERIC_KERNEL itself would select the absolutely
mandatory stuff that can never be made into a module like the GIC,
architectured timer (as Geert mentioned) and UART early console
driver. I'm not sure if ARM32 has an equivalent to the standardized
GIC and arch timer. Basically the minimal kernel would need a timer
for the scheduler tick and IRQ controller to get the timer IRQ and the
fixed clock driver if the archtimer uses one to get its frequency and
the early UART console is pointless as a module (so build it in to
allow debugging/development).

And then all new drivers, we should make sure are implemented as
tristate drivers. And we can go back and slowly work on converting
existing drivers to modules (community effort -- not one person or
entity) -- at least the ones where the author has hardware or ones
where the change is very likely to be correct and someone else is
willing to test it. We'll never be able to support some/all ARM32 (do
they even have a GIC/arch timer standard?), but at least for ARM64,
this seems like a viable goal.

This way, we'll keep the existing model working, while working on a
fully modular kernel from the other end.

Thoughts?

-Saravana