Hi John, On Thu, Jan 23, 2025 at 6:05 PM John Madieu <john.madieu.xa@xxxxxxxxxxxxxx> wrote: > As per the other SoC variant of the same family, the system controller > provides SoC ID in its own registers. > > Signed-off-by: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx> Thanks for your patch! > --- a/drivers/soc/renesas/Kconfig > +++ b/drivers/soc/renesas/Kconfig > @@ -355,6 +355,7 @@ config ARCH_R9A09G047 > config ARCH_R9A09G057 > bool "ARM64 Platform support for RZ/V2H(P)" > select RENESAS_RZV2H_ICU > + select SYS_R9A09G057 > help > This enables support for the Renesas RZ/V2H(P) SoC variants. > > @@ -395,4 +396,8 @@ config SYSC_R9A08G045 > config SYS_R9A09G047 > bool "Renesas RZ/G3E System controller support" if COMPILE_TEST > select SYSC_RZ > + > +config SYS_R9A09G057 > + bool "Renesas RZ/V2H System controller support" if COMPILE_TEST > + select SYSC_RZ Please add a blank line here. > endif # SOC_RENESAS > --- a/drivers/soc/renesas/r9a09g047-sys.c > +++ b/drivers/soc/renesas/r9a09g047-sys.c > @@ -11,25 +11,11 @@ > #include <linux/io.h> > > #include "rz-sysc.h" > +#include "rzg3e-sys.h" > > -/* Register Offsets */ > -#define SYS_LSI_MODE 0x300 > -/* > - * BOOTPLLCA[1:0] > - * [0,0] => 1.1GHZ > - * [0,1] => 1.5GHZ > - * [1,0] => 1.6GHZ > - * [1,1] => 1.7GHZ > - */ > -#define SYS_LSI_MODE_STAT_BOOTPLLCA55 GENMASK(12, 11) > -#define SYS_LSI_MODE_CA55_1_7GHZ 0x3 > -#define SYS_LSI_DEVID 0x304 > -#define SYS_LSI_DEVID_REV GENMASK(31, 28) > -#define SYS_LSI_DEVID_SPECIFIC GENMASK(27, 0) > -#define SYS_LSI_PRR 0x308 > -#define SYS_LSI_PRR_CA55_DIS BIT(8) > -#define SYS_LSI_PRR_NPU_DIS BIT(1) > - > +/* RZ/G3E-specific feature bits */ > +#define SYS_LSI_PRR_CA55_DIS BIT(8) > +#define SYS_LSI_PRR_NPU_DIS BIT(1) > > static void rzg3e_sys_print_id(struct device *dev, > void __iomem *sysc_base, > diff --git a/drivers/soc/renesas/r9a09g057-sys.c b/drivers/soc/renesas/r9a09g057-sys.c > new file mode 100644 > index 000000000000..dc7885b340c4 > --- /dev/null > +++ b/drivers/soc/renesas/r9a09g057-sys.c > @@ -0,0 +1,26 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * RZ/V2H System controller (SYS) driver > + * > + * Copyright (C) 2025 Renesas Electronics Corp. > + */ > + > +#include <linux/bits.h> > +#include <linux/device.h> > +#include <linux/init.h> > +#include <linux/io.h> > + > +#include "rz-sysc.h" > +#include "rzg3e-sys.h" Using definitions for RZ/G3E for RZ/V2H feels wrong to me, as they are really SoC-specific. So I think you better keep them in drivers/soc/renesas/r9a09g047-sys.c and drivers/soc/renesas/r9a09g057-sys.c, even if that means duplication. RZ/G3S also has them in drivers/soc/renesas/r9a08g045-sys.c > + > +static const struct rz_sysc_soc_id_init_data rzv2h_sys_soc_id_init_data __initconst = { > + .family = "RZ/V2H", > + .id = 0x847a447, > + .offset = SYS_LSI_DEVID, > + .revision_mask = SYS_LSI_DEVID_REV, > + .specific_id_mask = SYS_LSI_DEVID_SPECIFIC, I wouldn't mind just putting the hex constants here, and getting rid of the SYS_LSI_DEVID* definitions, as the definitions are only used for populating these structures. > +}; > + > +const struct rz_sysc_init_data rzv2h_sys_init_data = { > + .soc_id_init_data = &rzv2h_sys_soc_id_init_data, > +}; > --- /dev/null > +++ b/drivers/soc/renesas/rzg3e-sys.h > @@ -0,0 +1,28 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Renesas RZ/G3E (SYS) System Controller > + * > + * Copyright (C) 2025 Renesas Electronics Corp. > + */ > + > +#ifndef __RZG3E_SYS_H__ > +#define __RZG3E_SYS_H__ > + > +/* SYS Common Register Offsets */ > + > +#define SYS_LSI_MODE 0x300 > +/* > + * BOOTPLLCA[1:0] > + * [0,0] => 1.1GHZ > + * [0,1] => 1.5GHZ > + * [1,0] => 1.6GHZ > + * [1,1] => 1.7GHZ > + */ > +#define SYS_LSI_MODE_STAT_BOOTPLLCA55 GENMASK(12, 11) > +#define SYS_LSI_MODE_CA55_1_7GHZ 0x3 > +#define SYS_LSI_DEVID 0x304 > +#define SYS_LSI_DEVID_REV GENMASK(31, 28) > +#define SYS_LSI_DEVID_SPECIFIC GENMASK(27, 0) > +#define SYS_LSI_PRR 0x308 > + > +#endif /* __RZG3E_SYSC_H__ */ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds