Re: [PATCH v2 4/8] RISC-V: Kconfig.socs: Add Renesas RZ/Five SoC kconfig option

From: Lad, Prabhakar
Date: Thu Aug 18 2022 - 14:19:51 EST


Hi Geert,

Thank you for the review.

On Thu, Aug 18, 2022 at 4:16 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Prabhakar,
>
> On Mon, Aug 15, 2022 at 5:16 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> wrote:
> > Introduce SOC_RENESAS_RZFIVE config option to enable Renesas RZ/Five
> > (R9A07G043) SoC, along side also add ARCH_RENESAS config option as most
> > of the Renesas drivers depend on this config option.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> Thanks for your patch!
>
> The technical part LGTM, so
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
>
> > --- a/arch/riscv/Kconfig.socs
> > +++ b/arch/riscv/Kconfig.socs
> > @@ -80,4 +80,18 @@ config SOC_CANAAN_K210_DTB_SOURCE
> >
> > endif # SOC_CANAAN
> >
> > +config ARCH_RENESAS
>
> We definitely want ARCH_RENESAS, as it serves as a gatekeeper for
> Kconfig options for IP cores found on Renesas ARM and RISC-V SoCs.
>
Agreed, or else we will end up touching too many Kconfig files.

> > + bool
> > + select GPIOLIB
> > + select PINCTRL
> > + select SOC_BUS
> > +
> > +config SOC_RENESAS_RZFIVE
>
> Do we need this symbol? You could as well make ARCH_RENESAS above
> visible, and defer the actual SoC selection to ARCH_R9A07G043 in
> drivers/soc/renesas/Kconfig[1].
>
I think we could drop it and just defer the actual SoC selection to
ARCH_R9A07G043 as you said.

> I don't know what is the policy on RISC-V. ARM64 has a "single-symbol
> in arch/arm64/Kconfig.platforms"-policy, so we handle SoC selection
> in drivers/soc/renesas/Kconfig, and that is fine, as it avoids merge
> conflicts.
>
Agreed.

@Conor - Does the above sound OK?

Cheers,
Prabhakar