On Fri, Oct 22, 2021 at 12:40 PM Emil Renner Berthing <kernel@xxxxxxxx> wrote: > 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: ... > > > +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
