Messages in this thread | | | From | Andy Shevchenko <> | Date | Fri, 22 Oct 2021 15:40:24 +0300 | Subject | Re: [PATCH v2 01/16] RISC-V: Add StarFive SoC Kconfig option |
| |
On Fri, Oct 22, 2021 at 12:40 PM Emil Renner Berthing <kernel@esmil.dk> wrote: > On Fri, 22 Oct 2021 at 10:51, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Thu, Oct 21, 2021 at 8:42 PM Emil Renner Berthing <kernel@esmil.dk> wrote:
...
> > > +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.
Okay, please add this justification to the commit message in the next version.
...
> > > + 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.
OK.
-- With Best Regards, Andy Shevchenko
| |