Re: [PATCH v2 01/16] RISC-V: Add StarFive SoC Kconfig option

[Emil Renner Berthing]

On Fri, 22 Oct 2021 at 10:51, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
> On Thu, Oct 21, 2021 at 8:42 PM Emil Renner Berthing <kernel@xxxxxxxx> wrote:
> >
> > Add StarFive Kconfig option to select SoC specific and common drivers
> > required for these SoCs.
>
> ...
>
> > +config SOC_STARFIVE
> > +       bool "StarFive SoCs"
> > +       select PINCTRL
> > +       select RESET_CONTROLLER
>
> > +       select SIFIVE_PLIC
>
> If this is well understood and platform related the above two are too
> generic. Why have you selected them?

>From your last comments the criterion seemed to be to only add it here
if it would otherwise fail to boot. Well it does fail to boot without
the reset and pinctrl drivers. The clock driver too, but RISCV already
selects COMMON_CLK. Once PINCTRL and RESET_CONTROLLER is selected the
specific drivers defaults to SOC_STARFIVE.

Alternatively we'd select the drivers too, but I can't promise that
future StarFive chips will need the same JH7100 clock and reset
drivers. Doing it this way means that selecting SOC_STARFIVE by
default gives you a kernel that will boot on all StarFive SoCs, but
you can still customise it further to your particular chip. It seems
like SOC_SIFIVE is doing the same.

> > +       help
> > +         This enables support for StarFive SoC platform hardware.
>
> Not too much to read here. What is the point of this help?
> I would elaborate what kind of platform it may support, what kind of
> drivers it selects due to necessity of the accomplishing the boot
> process, etc.

This is exactly as the other descriptions in this file. I don't know
why SOC_STARFIVE should be special.

/Emil