* Re: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family
2024-12-06 21:25 ` [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family John Madieu
@ 2024-12-07 10:21 ` Claudiu Beznea
2024-12-09 11:30 ` John Madieu
2024-12-07 10:29 ` Biju Das
` (2 subsequent siblings)
3 siblings, 1 reply; 31+ messages in thread
From: Claudiu Beznea @ 2024-12-07 10:21 UTC (permalink / raw)
To: John Madieu, Geert Uytterhoeven, Magnus Damm, Rob Herring,
Biju Das, Krzysztof Kozlowski, Conor Dooley, Claudiu Beznea
Cc: john.madieu, linux-renesas-soc, linux-kernel, devicetree
Hi, John,
On 06.12.2024 23:25, John Madieu wrote:
> Add SoC detection support for RZ/G3E SoC. Also add support for detecting the
> number of cores and ETHOS-U55 NPU and also detect PLL mismatch for SW settings
> other than 1.7GHz.
>
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> ---
> drivers/soc/renesas/Kconfig | 6 +++
> drivers/soc/renesas/Makefile | 1 +
> drivers/soc/renesas/r9a09g047-sysc.c | 70 ++++++++++++++++++++++++++++
> drivers/soc/renesas/rz-sysc.c | 44 +++++++++++------
> drivers/soc/renesas/rz-sysc.h | 7 +++
> 5 files changed, 114 insertions(+), 14 deletions(-)
> create mode 100644 drivers/soc/renesas/r9a09g047-sysc.c
>
> diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
> index a792a3e915fe..9e46b0ee6e80 100644
> --- a/drivers/soc/renesas/Kconfig
> +++ b/drivers/soc/renesas/Kconfig
> @@ -348,6 +348,7 @@ config ARCH_R9A09G011
>
> config ARCH_R9A09G047
> bool "ARM64 Platform support for RZ/G3E"
> + select SYSC_R9A09G047
> help
> This enables support for the Renesas RZ/G3E SoC variants.
>
> @@ -386,9 +387,14 @@ config RST_RCAR
>
> config SYSC_RZ
> bool "System controller for RZ SoCs" if COMPILE_TEST
> + depends on MFD_SYSCON
>
> config SYSC_R9A08G045
> bool "Renesas RZ/G3S System controller support" if COMPILE_TEST
> select SYSC_RZ
>
> +config SYSC_R9A09G047
> + bool "Renesas RZ/G3E System controller support" if COMPILE_TEST
> + select SYSC_RZ
> +
> endif # SOC_RENESAS
> diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile
> index 8cd139b3dd0a..3256706112d9 100644
> --- a/drivers/soc/renesas/Makefile
> +++ b/drivers/soc/renesas/Makefile
> @@ -7,6 +7,7 @@ ifdef CONFIG_SMP
> obj-$(CONFIG_ARCH_R9A06G032) += r9a06g032-smp.o
> endif
> obj-$(CONFIG_SYSC_R9A08G045) += r9a08g045-sysc.o
> +obj-$(CONFIG_SYSC_R9A09G047) += r9a09g047-sysc.o
>
> # Family
> obj-$(CONFIG_PWC_RZV2M) += pwc-rzv2m.o
> diff --git a/drivers/soc/renesas/r9a09g047-sysc.c b/drivers/soc/renesas/r9a09g047-sysc.c
> new file mode 100644
> index 000000000000..32bdab9f1774
> --- /dev/null
> +++ b/drivers/soc/renesas/r9a09g047-sysc.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RZ/G3E System controller driver
> + *
> + * Copyright (C) 2024 Renesas Electronics Corp.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +
> +#include "rz-sysc.h"
> +
> +/* Register definitions */
> +#define SYS_LSI_DEVID 0x304
> +#define SYS_LSI_MODE 0x300
> +#define SYS_LSI_PRR 0x308
> +#define SYS_LSI_DEVID_REV GENMASK(31, 28)
> +#define SYS_LSI_DEVID_SPECIFIC GENMASK(27, 0)
> +#define SYS_LSI_PRR_CA55_DIS BIT(8)
> +#define SYS_LSI_PRR_NPU_DIS BIT(1)
> +/*
> + * 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
> +
> +static void rzg3e_extended_device_identification(struct device *dev,
> + void __iomem *sysc_base,
> + struct soc_device_attribute *soc_dev_attr)
Not strong preference here, I think it can still be aligned to (
> +{
> + u32 prr_val, mode_val;
> + bool is_quad_core, npu_enabled;
Reverse christmass tree order?
> +
> + prr_val = readl(sysc_base + SYS_LSI_PRR);
> + mode_val = readl(sysc_base + SYS_LSI_MODE);
> +
> + /* Check CPU and NPU configuration */
> + is_quad_core = !(prr_val & SYS_LSI_PRR_CA55_DIS);
> + npu_enabled = !(prr_val & SYS_LSI_PRR_NPU_DIS);
> +
> + dev_info(dev, "Detected Renesas %s Core %s %s Rev %s %s\n",
I think you have an extra space towards the end: "%s %s"
> + is_quad_core ? "Quad" : "Dual",
> + soc_dev_attr->family,
> + soc_dev_attr->soc_id,
> + soc_dev_attr->revision,
> + npu_enabled ? "with Ethos-U55" : "");
> +
> + /* Check CA55 PLL configuration */
> + if (FIELD_GET(SYS_LSI_MODE_STAT_BOOTPLLCA55, mode_val) != SYS_LSI_MODE_CA55_1_7GHz)
> + dev_warn(dev, "CA55 PLL is not set to 1.7GHz\n");
> +}
> +
> +static const struct rz_sysc_soc_id_init_data rzg3e_sysc_soc_id_init_data __initconst = {
> + .family = "RZ/G3E",
> + .id = 0x8679447,
> + .offset = SYS_LSI_DEVID,
> + .revision_mask = SYS_LSI_DEVID_REV,
> + .specific_id_mask = SYS_LSI_DEVID_SPECIFIC,
> + .extended_device_identification = rzg3e_extended_device_identification,
> +};
> +
> +const struct rz_sysc_init_data rzg3e_sysc_init_data = {
> + .soc_id_init_data = &rzg3e_sysc_soc_id_init_data,
> +};
> diff --git a/drivers/soc/renesas/rz-sysc.c b/drivers/soc/renesas/rz-sysc.c
> index d34d295831b8..515eca249b6e 100644
> --- a/drivers/soc/renesas/rz-sysc.c
> +++ b/drivers/soc/renesas/rz-sysc.c
> @@ -231,7 +231,7 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
>
> soc_id_start = strchr(match->compatible, ',') + 1;
> soc_id_end = strchr(match->compatible, '-');
> - size = soc_id_end - soc_id_start;
> + size = soc_id_end - soc_id_start + 1;
This may worth a different patch.
> if (size > 32)
> size = 32;
> strscpy(soc_id, soc_id_start, size);
> @@ -257,8 +257,16 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
> return -ENODEV;
> }
>
> - dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n", soc_dev_attr->family,
> - soc_dev_attr->soc_id, soc_dev_attr->revision);
> + /* Try to call SoC-specific device identification */
> + if (soc_data->extended_device_identification) {
> + soc_data->extended_device_identification(sysc->dev, sysc->base,
> + soc_dev_attr);
> + } else {
> + dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n",
> + soc_dev_attr->family,
> + soc_dev_attr->soc_id,
> + soc_dev_attr->revision);
> + }
>
> soc_dev = soc_device_register(soc_dev_attr);
> if (IS_ERR(soc_dev))
> @@ -283,6 +291,9 @@ static struct regmap_config rz_sysc_regmap = {
> static const struct of_device_id rz_sysc_match[] = {
> #ifdef CONFIG_SYSC_R9A08G045
> { .compatible = "renesas,r9a08g045-sysc", .data = &rzg3s_sysc_init_data },
> +#endif
> +#ifdef CONFIG_SYSC_R9A09G047
> + { .compatible = "renesas,r9a09g047-sys", .data = &rzg3e_sysc_init_data },
> #endif
> { }
> };
> @@ -315,20 +326,25 @@ static int rz_sysc_probe(struct platform_device *pdev)
> return ret;
>
> data = match->data;
> - if (!data->max_register_offset)
> - return -EINVAL;
The idea with this was to still have the syscon regmap registered no matter
the signals are available or not. This may be needed for other use cases.
> + if (data->signals_init_data) {
I'd prefer to have this check in rz_sysc_signals_init(). In this way you
have everything signal init specific in a single function.
> + if (!data->max_register_offset)
> + return -EINVAL;
>
> - ret = rz_sysc_signals_init(sysc, data->signals_init_data, data->num_signals);
> - if (ret)
> - return ret;
> + ret = rz_sysc_signals_init(sysc, data->signals_init_data, data->num_signals);
> + if (ret)
> + return ret;
> +
> + rz_sysc_regmap.max_register = data->max_register_offset;
> + dev_set_drvdata(dev, sysc);
Why changed the initial order?
Thank you,
Claudiu
>
> - dev_set_drvdata(dev, sysc);
> - rz_sysc_regmap.max_register = data->max_register_offset;
> - regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> - if (IS_ERR(regmap))
> - return PTR_ERR(regmap);
> + regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
>
> - return of_syscon_register_regmap(dev->of_node, regmap);
> + return of_syscon_register_regmap(dev->of_node, regmap);
> + }
> +
> + return 0;
> }
>
> static struct platform_driver rz_sysc_driver = {
> diff --git a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h
> index babca9c743c7..2b5ad41cef9e 100644
> --- a/drivers/soc/renesas/rz-sysc.h
> +++ b/drivers/soc/renesas/rz-sysc.h
> @@ -8,7 +8,9 @@
> #ifndef __SOC_RENESAS_RZ_SYSC_H__
> #define __SOC_RENESAS_RZ_SYSC_H__
>
> +#include <linux/device.h>
> #include <linux/refcount.h>
> +#include <linux/sys_soc.h>
> #include <linux/types.h>
>
> /**
> @@ -42,6 +44,7 @@ struct rz_sysc_signal {
> * @offset: SYSC SoC ID register offset
> * @revision_mask: SYSC SoC ID revision mask
> * @specific_id_mask: SYSC SoC ID specific ID mask
> + * @extended_device_identification: SoC-specific extended device identification
> */
> struct rz_sysc_soc_id_init_data {
> const char * const family;
> @@ -49,6 +52,9 @@ struct rz_sysc_soc_id_init_data {
> u32 offset;
> u32 revision_mask;
> u32 specific_id_mask;
> + void (*extended_device_identification)(struct device *dev,
> + void __iomem *sysc_base,
> + struct soc_device_attribute *soc_dev_attr);
> };
>
> /**
> @@ -65,6 +71,7 @@ struct rz_sysc_init_data {
> u32 max_register_offset;
> };
>
> +extern const struct rz_sysc_init_data rzg3e_sysc_init_data;
> extern const struct rz_sysc_init_data rzg3s_sysc_init_data;
>
> #endif /* __SOC_RENESAS_RZ_SYSC_H__ */
^ permalink raw reply [flat|nested] 31+ messages in thread* RE: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family
2024-12-07 10:21 ` Claudiu Beznea
@ 2024-12-09 11:30 ` John Madieu
0 siblings, 0 replies; 31+ messages in thread
From: John Madieu @ 2024-12-09 11:30 UTC (permalink / raw)
To: Claudiu.Beznea, Geert Uytterhoeven, Magnus Damm, Rob Herring,
Biju Das, Krzysztof Kozlowski, Conor Dooley, Claudiu Beznea
Cc: john.madieu@gmail.com, linux-renesas-soc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
> -----Original Message-----
> From: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> Sent: Saturday, December 7, 2024 11:22 AM
> To: John Madieu <john.madieu.xa@bp.renesas.com>; Geert Uytterhoeven
> <geert+renesas@glider.be>; Magnus Damm <magnus.damm@gmail.com>; Rob
> Herring <robh@kernel.org>; Biju Das <biju.das.jz@bp.renesas.com>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Cc: john.madieu@gmail.com; linux-renesas-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E
> family
>
> Hi, John,
Hello Claudiu,
>
> On 06.12.2024 23:25, John Madieu wrote:
> > Add SoC detection support for RZ/G3E SoC. Also add support for
> > detecting the number of cores and ETHOS-U55 NPU and also detect PLL
> > mismatch for SW settings other than 1.7GHz.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > ---
> > drivers/soc/renesas/Kconfig | 6 +++
> > drivers/soc/renesas/Makefile | 1 +
> > drivers/soc/renesas/r9a09g047-sysc.c | 70 ++++++++++++++++++++++++++++
> > drivers/soc/renesas/rz-sysc.c | 44 +++++++++++------
> > drivers/soc/renesas/rz-sysc.h | 7 +++
> > 5 files changed, 114 insertions(+), 14 deletions(-) create mode
> > 100644 drivers/soc/renesas/r9a09g047-sysc.c
> >
> > diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
> > index a792a3e915fe..9e46b0ee6e80 100644
> > --- a/drivers/soc/renesas/Kconfig
> > +++ b/drivers/soc/renesas/Kconfig
> > @@ -348,6 +348,7 @@ config ARCH_R9A09G011
> >
> > config ARCH_R9A09G047
> > bool "ARM64 Platform support for RZ/G3E"
> > + select SYSC_R9A09G047
> > help
> > This enables support for the Renesas RZ/G3E SoC variants.
> >
> > @@ -386,9 +387,14 @@ config RST_RCAR
> >
> > config SYSC_RZ
> > bool "System controller for RZ SoCs" if COMPILE_TEST
> > + depends on MFD_SYSCON
> >
> > config SYSC_R9A08G045
> > bool "Renesas RZ/G3S System controller support" if COMPILE_TEST
> > select SYSC_RZ
> >
> > +config SYSC_R9A09G047
> > + bool "Renesas RZ/G3E System controller support" if COMPILE_TEST
> > + select SYSC_RZ
> > +
> > endif # SOC_RENESAS
> > diff --git a/drivers/soc/renesas/Makefile
> > b/drivers/soc/renesas/Makefile index 8cd139b3dd0a..3256706112d9 100644
> > --- a/drivers/soc/renesas/Makefile
> > +++ b/drivers/soc/renesas/Makefile
> > @@ -7,6 +7,7 @@ ifdef CONFIG_SMP
> > obj-$(CONFIG_ARCH_R9A06G032) += r9a06g032-smp.o
> > endif
> > obj-$(CONFIG_SYSC_R9A08G045) += r9a08g045-sysc.o
> > +obj-$(CONFIG_SYSC_R9A09G047) += r9a09g047-sysc.o
> >
> > # Family
> > obj-$(CONFIG_PWC_RZV2M) += pwc-rzv2m.o
> > diff --git a/drivers/soc/renesas/r9a09g047-sysc.c
> > b/drivers/soc/renesas/r9a09g047-sysc.c
> > new file mode 100644
> > index 000000000000..32bdab9f1774
> > --- /dev/null
> > +++ b/drivers/soc/renesas/r9a09g047-sysc.c
> > @@ -0,0 +1,70 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * RZ/G3E System controller driver
> > + *
> > + * Copyright (C) 2024 Renesas Electronics Corp.
> > + */
> > +
> > +#include <linux/bits.h>
> > +#include <linux/device.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +
> > +#include "rz-sysc.h"
> > +
> > +/* Register definitions */
> > +#define SYS_LSI_DEVID 0x304
> > +#define SYS_LSI_MODE 0x300
> > +#define SYS_LSI_PRR 0x308
> > +#define SYS_LSI_DEVID_REV GENMASK(31, 28)
> > +#define SYS_LSI_DEVID_SPECIFIC GENMASK(27, 0)
> > +#define SYS_LSI_PRR_CA55_DIS BIT(8)
> > +#define SYS_LSI_PRR_NPU_DIS BIT(1)
> > +/*
> > + * 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
> > +
> > +static void rzg3e_extended_device_identification(struct device *dev,
> > + void __iomem *sysc_base,
> > + struct soc_device_attribute *soc_dev_attr)
>
> Not strong preference here, I think it can still be aligned to (
Noted. Thanks!
>
> > +{
> > + u32 prr_val, mode_val;
> > + bool is_quad_core, npu_enabled;
>
> Reverse christmass tree order?
Will swap it in v2.
>
> > +
> > + prr_val = readl(sysc_base + SYS_LSI_PRR);
> > + mode_val = readl(sysc_base + SYS_LSI_MODE);
> > +
> > + /* Check CPU and NPU configuration */
> > + is_quad_core = !(prr_val & SYS_LSI_PRR_CA55_DIS);
> > + npu_enabled = !(prr_val & SYS_LSI_PRR_NPU_DIS);
> > +
> > + dev_info(dev, "Detected Renesas %s Core %s %s Rev %s %s\n",
>
> I think you have an extra space towards the end: "%s %s"
Thanks for pointing it out.
>
> > + is_quad_core ? "Quad" : "Dual",
> > + soc_dev_attr->family,
> > + soc_dev_attr->soc_id,
> > + soc_dev_attr->revision,
> > + npu_enabled ? "with Ethos-U55" : "");
> > +
> > + /* Check CA55 PLL configuration */
> > + if (FIELD_GET(SYS_LSI_MODE_STAT_BOOTPLLCA55, mode_val) !=
> SYS_LSI_MODE_CA55_1_7GHz)
> > + dev_warn(dev, "CA55 PLL is not set to 1.7GHz\n"); }
> > +
> > +static const struct rz_sysc_soc_id_init_data
> rzg3e_sysc_soc_id_init_data __initconst = {
> > + .family = "RZ/G3E",
> > + .id = 0x8679447,
> > + .offset = SYS_LSI_DEVID,
> > + .revision_mask = SYS_LSI_DEVID_REV,
> > + .specific_id_mask = SYS_LSI_DEVID_SPECIFIC,
> > + .extended_device_identification =
> > +rzg3e_extended_device_identification,
> > +};
> > +
> > +const struct rz_sysc_init_data rzg3e_sysc_init_data = {
> > + .soc_id_init_data = &rzg3e_sysc_soc_id_init_data, };
> > diff --git a/drivers/soc/renesas/rz-sysc.c
> > b/drivers/soc/renesas/rz-sysc.c index d34d295831b8..515eca249b6e
> > 100644
> > --- a/drivers/soc/renesas/rz-sysc.c
> > +++ b/drivers/soc/renesas/rz-sysc.c
> > @@ -231,7 +231,7 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc,
> > const struct of_device_id *mat
> >
> > soc_id_start = strchr(match->compatible, ',') + 1;
> > soc_id_end = strchr(match->compatible, '-');
> > - size = soc_id_end - soc_id_start;
> > + size = soc_id_end - soc_id_start + 1;
>
> This may worth a different patch.
Got it.
>
> > if (size > 32)
> > size = 32;
> > strscpy(soc_id, soc_id_start, size); @@ -257,8 +257,16 @@ static int
> > rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
> > return -ENODEV;
> > }
> >
> > - dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n", soc_dev_attr-
> >family,
> > - soc_dev_attr->soc_id, soc_dev_attr->revision);
> > + /* Try to call SoC-specific device identification */
> > + if (soc_data->extended_device_identification) {
> > + soc_data->extended_device_identification(sysc->dev, sysc-
> >base,
> > + soc_dev_attr);
> > + } else {
> > + dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n",
> > + soc_dev_attr->family,
> > + soc_dev_attr->soc_id,
> > + soc_dev_attr->revision);
> > + }
> >
> > soc_dev = soc_device_register(soc_dev_attr);
> > if (IS_ERR(soc_dev))
> > @@ -283,6 +291,9 @@ static struct regmap_config rz_sysc_regmap = {
> > static const struct of_device_id rz_sysc_match[] = { #ifdef
> > CONFIG_SYSC_R9A08G045
> > { .compatible = "renesas,r9a08g045-sysc", .data =
> > &rzg3s_sysc_init_data },
> > +#endif
> > +#ifdef CONFIG_SYSC_R9A09G047
> > + { .compatible = "renesas,r9a09g047-sys", .data =
> > +&rzg3e_sysc_init_data },
> > #endif
> > { }
> > };
> > @@ -315,20 +326,25 @@ static int rz_sysc_probe(struct platform_device
> *pdev)
> > return ret;
> >
> > data = match->data;
> > - if (!data->max_register_offset)
> > - return -EINVAL;
>
> The idea with this was to still have the syscon regmap registered no
> matter the signals are available or not. This may be needed for other use
> cases.
>
> > + if (data->signals_init_data) {
>
> I'd prefer to have this check in rz_sysc_signals_init(). In this way you
> have everything signal init specific in a single function.
The idea here is to avoid calling rz_sysc_signals_init() for drivers that
don't need it instead of always calling it, and potentially dealing with
errors due to drivers not supporting it, instead of the function actually
failing.
>
> > + if (!data->max_register_offset)
> > + return -EINVAL;
> >
> > - ret = rz_sysc_signals_init(sysc, data->signals_init_data, data-
> >num_signals);
> > - if (ret)
> > - return ret;
> > + ret = rz_sysc_signals_init(sysc, data->signals_init_data,
> data->num_signals);
> > + if (ret)
> > + return ret;
> > +
> > + rz_sysc_regmap.max_register = data->max_register_offset;
> > + dev_set_drvdata(dev, sysc);
>
> Why changed the initial order?
No special reason. Will be reverted.
>
> Thank you,
> Claudiu
Thanks!
>
> >
> > - dev_set_drvdata(dev, sysc);
> > - rz_sysc_regmap.max_register = data->max_register_offset;
> > - regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> > - if (IS_ERR(regmap))
> > - return PTR_ERR(regmap);
> > + regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> > + if (IS_ERR(regmap))
> > + return PTR_ERR(regmap);
> >
> > - return of_syscon_register_regmap(dev->of_node, regmap);
> > + return of_syscon_register_regmap(dev->of_node, regmap);
> > + }
> > +
> > + return 0;
> > }
> >
> > static struct platform_driver rz_sysc_driver = { diff --git
> > a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h index
> > babca9c743c7..2b5ad41cef9e 100644
> > --- a/drivers/soc/renesas/rz-sysc.h
> > +++ b/drivers/soc/renesas/rz-sysc.h
> > @@ -8,7 +8,9 @@
> > #ifndef __SOC_RENESAS_RZ_SYSC_H__
> > #define __SOC_RENESAS_RZ_SYSC_H__
> >
> > +#include <linux/device.h>
> > #include <linux/refcount.h>
> > +#include <linux/sys_soc.h>
> > #include <linux/types.h>
> >
> > /**
> > @@ -42,6 +44,7 @@ struct rz_sysc_signal {
> > * @offset: SYSC SoC ID register offset
> > * @revision_mask: SYSC SoC ID revision mask
> > * @specific_id_mask: SYSC SoC ID specific ID mask
> > + * @extended_device_identification: SoC-specific extended device
> > + identification
> > */
> > struct rz_sysc_soc_id_init_data {
> > const char * const family;
> > @@ -49,6 +52,9 @@ struct rz_sysc_soc_id_init_data {
> > u32 offset;
> > u32 revision_mask;
> > u32 specific_id_mask;
> > + void (*extended_device_identification)(struct device *dev,
> > + void __iomem *sysc_base,
> > + struct soc_device_attribute *soc_dev_attr);
> > };
> >
> > /**
> > @@ -65,6 +71,7 @@ struct rz_sysc_init_data {
> > u32 max_register_offset;
> > };
> >
> > +extern const struct rz_sysc_init_data rzg3e_sysc_init_data;
> > extern const struct rz_sysc_init_data rzg3s_sysc_init_data;
> >
> > #endif /* __SOC_RENESAS_RZ_SYSC_H__ */
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family
2024-12-06 21:25 ` [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family John Madieu
2024-12-07 10:21 ` Claudiu Beznea
@ 2024-12-07 10:29 ` Biju Das
2024-12-09 10:44 ` John Madieu
2024-12-07 10:36 ` Biju Das
2024-12-13 16:24 ` Geert Uytterhoeven
3 siblings, 1 reply; 31+ messages in thread
From: Biju Das @ 2024-12-07 10:29 UTC (permalink / raw)
To: John Madieu, Geert Uytterhoeven, Magnus Damm, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Claudiu Beznea
Cc: john.madieu@gmail.com, linux-renesas-soc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
John Madieu
Hi John,
Thanks for the patch.
> -----Original Message-----
> From: John Madieu <john.madieu.xa@bp.renesas.com>
> Sent: 06 December 2024 21:26
> Subject: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family
>
> Add SoC detection support for RZ/G3E SoC. Also add support for detecting the number of cores and
> ETHOS-U55 NPU and also detect PLL mismatch for SW settings other than 1.7GHz.
>
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> ---
>
> data = match->data;
> - if (!data->max_register_offset)
> - return -EINVAL;
> + if (data->signals_init_data) {
> + if (!data->max_register_offset)
> + return -EINVAL;
>
> - ret = rz_sysc_signals_init(sysc, data->signals_init_data, data->num_signals);
> - if (ret)
> - return ret;
> + ret = rz_sysc_signals_init(sysc, data->signals_init_data, data->num_signals);
> + if (ret)
> + return ret;
> +
> + rz_sysc_regmap.max_register = data->max_register_offset;
> + dev_set_drvdata(dev, sysc);
>
> - dev_set_drvdata(dev, sysc);
> - rz_sysc_regmap.max_register = data->max_register_offset;
> - regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> - if (IS_ERR(regmap))
> - return PTR_ERR(regmap);
> + regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
>
> - return of_syscon_register_regmap(dev->of_node, regmap);
> + return of_syscon_register_regmap(dev->of_node, regmap);
Basically if I understand correctly, you are going to use normal
Syscon call for register access in PCIe and TSU drivers. Is it correct?
For example,
priv->syscon = syscon_regmap_lookup_by_phandle(np, "syscon");
regmap_read(priv->syscon,xxx)
Cheers,
Biju
> + }
> +
> + return 0;
> }
>
> static struct platform_driver rz_sysc_driver = { diff --git a/drivers/soc/renesas/rz-sysc.h
> b/drivers/soc/renesas/rz-sysc.h index babca9c743c7..2b5ad41cef9e 100644
> --- a/drivers/soc/renesas/rz-sysc.h
> +++ b/drivers/soc/renesas/rz-sysc.h
> @@ -8,7 +8,9 @@
> #ifndef __SOC_RENESAS_RZ_SYSC_H__
> #define __SOC_RENESAS_RZ_SYSC_H__
>
> +#include <linux/device.h>
> #include <linux/refcount.h>
> +#include <linux/sys_soc.h>
> #include <linux/types.h>
>
> /**
> @@ -42,6 +44,7 @@ struct rz_sysc_signal {
> * @offset: SYSC SoC ID register offset
> * @revision_mask: SYSC SoC ID revision mask
> * @specific_id_mask: SYSC SoC ID specific ID mask
> + * @extended_device_identification: SoC-specific extended device
> + identification
> */
> struct rz_sysc_soc_id_init_data {
> const char * const family;
> @@ -49,6 +52,9 @@ struct rz_sysc_soc_id_init_data {
> u32 offset;
> u32 revision_mask;
> u32 specific_id_mask;
> + void (*extended_device_identification)(struct device *dev,
> + void __iomem *sysc_base,
> + struct soc_device_attribute *soc_dev_attr);
> };
>
> /**
> @@ -65,6 +71,7 @@ struct rz_sysc_init_data {
> u32 max_register_offset;
> };
>
> +extern const struct rz_sysc_init_data rzg3e_sysc_init_data;
> extern const struct rz_sysc_init_data rzg3s_sysc_init_data;
>
> #endif /* __SOC_RENESAS_RZ_SYSC_H__ */
> --
> 2.25.1
^ permalink raw reply [flat|nested] 31+ messages in thread* RE: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family
2024-12-07 10:29 ` Biju Das
@ 2024-12-09 10:44 ` John Madieu
0 siblings, 0 replies; 31+ messages in thread
From: John Madieu @ 2024-12-09 10:44 UTC (permalink / raw)
To: Biju Das, Geert Uytterhoeven, Magnus Damm, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Claudiu Beznea
Cc: john.madieu@gmail.com, linux-renesas-soc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Saturday, December 7, 2024 11:30 AM
> To: John Madieu <john.madieu.xa@bp.renesas.com>; Geert Uytterhoeven
> <geert+renesas@glider.be>; Magnus Damm <magnus.damm@gmail.com>; Rob
> Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
> Dooley <conor+dt@kernel.org>; Claudiu Beznea
> <claudiu.beznea.uj@bp.renesas.com>
> Cc: john.madieu@gmail.com; linux-renesas-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; John Madieu
> <john.madieu.xa@bp.renesas.com>
> Subject: RE: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E
> family
>
> Hi John,
>
> Thanks for the patch.
>
> > -----Original Message-----
> > From: John Madieu <john.madieu.xa@bp.renesas.com>
> > Sent: 06 December 2024 21:26
> > Subject: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E
> > family
> >
> > Add SoC detection support for RZ/G3E SoC. Also add support for
> > detecting the number of cores and
> > ETHOS-U55 NPU and also detect PLL mismatch for SW settings other than
> 1.7GHz.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > ---
> >
> > data = match->data;
> > - if (!data->max_register_offset)
> > - return -EINVAL;
> > + if (data->signals_init_data) {
> > + if (!data->max_register_offset)
> > + return -EINVAL;
> >
> > - ret = rz_sysc_signals_init(sysc, data->signals_init_data, data-
> >num_signals);
> > - if (ret)
> > - return ret;
> > + ret = rz_sysc_signals_init(sysc, data->signals_init_data,
> data->num_signals);
> > + if (ret)
> > + return ret;
> > +
> > + rz_sysc_regmap.max_register = data->max_register_offset;
> > + dev_set_drvdata(dev, sysc);
> >
> > - dev_set_drvdata(dev, sysc);
> > - rz_sysc_regmap.max_register = data->max_register_offset;
> > - regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> > - if (IS_ERR(regmap))
> > - return PTR_ERR(regmap);
> > + regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> > + if (IS_ERR(regmap))
> > + return PTR_ERR(regmap);
> >
> > - return of_syscon_register_regmap(dev->of_node, regmap);
> > + return of_syscon_register_regmap(dev->of_node, regmap);
>
> Basically if I understand correctly, you are going to use normal Syscon
> call for register access in PCIe and TSU drivers. Is it correct?
>
> For example,
>
> priv->syscon = syscon_regmap_lookup_by_phandle(np, "syscon");
Yes, that's the idea.
>
>
> regmap_read(priv->syscon,xxx)
>
>
> Cheers,
> Biju
Cheers.
>
>
> > + }
> > +
> > + return 0;
> > }
> >
> > static struct platform_driver rz_sysc_driver = { diff --git
> > a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h index
> > babca9c743c7..2b5ad41cef9e 100644
> > --- a/drivers/soc/renesas/rz-sysc.h
> > +++ b/drivers/soc/renesas/rz-sysc.h
> > @@ -8,7 +8,9 @@
> > #ifndef __SOC_RENESAS_RZ_SYSC_H__
> > #define __SOC_RENESAS_RZ_SYSC_H__
> >
> > +#include <linux/device.h>
> > #include <linux/refcount.h>
> > +#include <linux/sys_soc.h>
> > #include <linux/types.h>
> >
> > /**
> > @@ -42,6 +44,7 @@ struct rz_sysc_signal {
> > * @offset: SYSC SoC ID register offset
> > * @revision_mask: SYSC SoC ID revision mask
> > * @specific_id_mask: SYSC SoC ID specific ID mask
> > + * @extended_device_identification: SoC-specific extended device
> > + identification
> > */
> > struct rz_sysc_soc_id_init_data {
> > const char * const family;
> > @@ -49,6 +52,9 @@ struct rz_sysc_soc_id_init_data {
> > u32 offset;
> > u32 revision_mask;
> > u32 specific_id_mask;
> > + void (*extended_device_identification)(struct device *dev,
> > + void __iomem *sysc_base,
> > + struct soc_device_attribute *soc_dev_attr);
> > };
> >
> > /**
> > @@ -65,6 +71,7 @@ struct rz_sysc_init_data {
> > u32 max_register_offset;
> > };
> >
> > +extern const struct rz_sysc_init_data rzg3e_sysc_init_data;
> > extern const struct rz_sysc_init_data rzg3s_sysc_init_data;
> >
> > #endif /* __SOC_RENESAS_RZ_SYSC_H__ */
> > --
> > 2.25.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family
2024-12-06 21:25 ` [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family John Madieu
2024-12-07 10:21 ` Claudiu Beznea
2024-12-07 10:29 ` Biju Das
@ 2024-12-07 10:36 ` Biju Das
2024-12-09 10:43 ` John Madieu
2024-12-13 16:24 ` Geert Uytterhoeven
3 siblings, 1 reply; 31+ messages in thread
From: Biju Das @ 2024-12-07 10:36 UTC (permalink / raw)
To: John Madieu, Geert Uytterhoeven, Magnus Damm, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Claudiu Beznea
Cc: john.madieu@gmail.com, linux-renesas-soc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
John Madieu
Hi John,
Thanks for the patch.
> -----Original Message-----
> From: John Madieu <john.madieu.xa@bp.renesas.com>
> Sent: 06 December 2024 21:26
> Subject: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family
>
> Add SoC detection support for RZ/G3E SoC. Also add support for detecting the number of cores and
> ETHOS-U55 NPU and also detect PLL mismatch for SW settings other than 1.7GHz.
>
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> ---
> drivers/soc/renesas/Kconfig | 6 +++
> drivers/soc/renesas/Makefile | 1 +
> drivers/soc/renesas/r9a09g047-sysc.c | 70 ++++++++++++++++++++++++++++
> drivers/soc/renesas/rz-sysc.c | 44 +++++++++++------
> drivers/soc/renesas/rz-sysc.h | 7 +++
> 5 files changed, 114 insertions(+), 14 deletions(-) create mode 100644
> drivers/soc/renesas/r9a09g047-sysc.c
>
> diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig index
> a792a3e915fe..9e46b0ee6e80 100644
> --- a/drivers/soc/renesas/Kconfig
> +++ b/drivers/soc/renesas/Kconfig
> @@ -348,6 +348,7 @@ config ARCH_R9A09G011
>
> config ARCH_R9A09G047
> bool "ARM64 Platform support for RZ/G3E"
> + select SYSC_R9A09G047
> help
> This enables support for the Renesas RZ/G3E SoC variants.
>
> @@ -386,9 +387,14 @@ config RST_RCAR
>
> config SYSC_RZ
> bool "System controller for RZ SoCs" if COMPILE_TEST
> + depends on MFD_SYSCON
>
> config SYSC_R9A08G045
> bool "Renesas RZ/G3S System controller support" if COMPILE_TEST
> select SYSC_RZ
>
> +config SYSC_R9A09G047
> + bool "Renesas RZ/G3E System controller support" if COMPILE_TEST
> + select SYSC_RZ
> +
> endif # SOC_RENESAS
> diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile index
> 8cd139b3dd0a..3256706112d9 100644
> --- a/drivers/soc/renesas/Makefile
> +++ b/drivers/soc/renesas/Makefile
> @@ -7,6 +7,7 @@ ifdef CONFIG_SMP
> obj-$(CONFIG_ARCH_R9A06G032) += r9a06g032-smp.o
> endif
> obj-$(CONFIG_SYSC_R9A08G045) += r9a08g045-sysc.o
> +obj-$(CONFIG_SYSC_R9A09G047) += r9a09g047-sysc.o
>
> # Family
> obj-$(CONFIG_PWC_RZV2M) += pwc-rzv2m.o
> diff --git a/drivers/soc/renesas/r9a09g047-sysc.c b/drivers/soc/renesas/r9a09g047-sysc.c
> new file mode 100644
> index 000000000000..32bdab9f1774
> --- /dev/null
> +++ b/drivers/soc/renesas/r9a09g047-sysc.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RZ/G3E System controller driver
> + *
> + * Copyright (C) 2024 Renesas Electronics Corp.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +
> +#include "rz-sysc.h"
> +
> +/* Register definitions */
> +#define SYS_LSI_DEVID 0x304
> +#define SYS_LSI_MODE 0x300
> +#define SYS_LSI_PRR 0x308
> +#define SYS_LSI_DEVID_REV GENMASK(31, 28)
> +#define SYS_LSI_DEVID_SPECIFIC GENMASK(27, 0)
> +#define SYS_LSI_PRR_CA55_DIS BIT(8)
> +#define SYS_LSI_PRR_NPU_DIS BIT(1)
> +/*
> + * 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
> +
> +static void rzg3e_extended_device_identification(struct device *dev,
> + void __iomem *sysc_base,
> + struct soc_device_attribute *soc_dev_attr) {
> + u32 prr_val, mode_val;
> + bool is_quad_core, npu_enabled;
> +
> + prr_val = readl(sysc_base + SYS_LSI_PRR);
> + mode_val = readl(sysc_base + SYS_LSI_MODE);
> +
> + /* Check CPU and NPU configuration */
> + is_quad_core = !(prr_val & SYS_LSI_PRR_CA55_DIS);
> + npu_enabled = !(prr_val & SYS_LSI_PRR_NPU_DIS);
> +
> + dev_info(dev, "Detected Renesas %s Core %s %s Rev %s %s\n",
> + is_quad_core ? "Quad" : "Dual",
> + soc_dev_attr->family,
> + soc_dev_attr->soc_id,
> + soc_dev_attr->revision,
> + npu_enabled ? "with Ethos-U55" : "");
> +
> + /* Check CA55 PLL configuration */
> + if (FIELD_GET(SYS_LSI_MODE_STAT_BOOTPLLCA55, mode_val) != SYS_LSI_MODE_CA55_1_7GHz)
> + dev_warn(dev, "CA55 PLL is not set to 1.7GHz\n"); }
> +
> +static const struct rz_sysc_soc_id_init_data rzg3e_sysc_soc_id_init_data __initconst = {
> + .family = "RZ/G3E",
> + .id = 0x8679447,
> + .offset = SYS_LSI_DEVID,
> + .revision_mask = SYS_LSI_DEVID_REV,
> + .specific_id_mask = SYS_LSI_DEVID_SPECIFIC,
> + .extended_device_identification =
> +rzg3e_extended_device_identification,
> +};
> +
> +const struct rz_sysc_init_data rzg3e_sysc_init_data = {
> + .soc_id_init_data = &rzg3e_sysc_soc_id_init_data, };
> diff --git a/drivers/soc/renesas/rz-sysc.c b/drivers/soc/renesas/rz-sysc.c index
> d34d295831b8..515eca249b6e 100644
> --- a/drivers/soc/renesas/rz-sysc.c
> +++ b/drivers/soc/renesas/rz-sysc.c
> @@ -231,7 +231,7 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
>
> soc_id_start = strchr(match->compatible, ',') + 1;
> soc_id_end = strchr(match->compatible, '-');
> - size = soc_id_end - soc_id_start;
> + size = soc_id_end - soc_id_start + 1;
> if (size > 32)
> size = 32;
> strscpy(soc_id, soc_id_start, size);
> @@ -257,8 +257,16 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
> return -ENODEV;
> }
>
> - dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n", soc_dev_attr->family,
> - soc_dev_attr->soc_id, soc_dev_attr->revision);
> + /* Try to call SoC-specific device identification */
> + if (soc_data->extended_device_identification) {
> + soc_data->extended_device_identification(sysc->dev, sysc->base,
> + soc_dev_attr);
> + } else {
> + dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n",
> + soc_dev_attr->family,
> + soc_dev_attr->soc_id,
> + soc_dev_attr->revision);
> + }
>
> soc_dev = soc_device_register(soc_dev_attr);
> if (IS_ERR(soc_dev))
> @@ -283,6 +291,9 @@ static struct regmap_config rz_sysc_regmap = { static const struct of_device_id
> rz_sysc_match[] = { #ifdef CONFIG_SYSC_R9A08G045
> { .compatible = "renesas,r9a08g045-sysc", .data = &rzg3s_sysc_init_data },
> +#endif
> +#ifdef CONFIG_SYSC_R9A09G047
> + { .compatible = "renesas,r9a09g047-sys", .data = &rzg3e_sysc_init_data
> +},
> #endif
> { }
> };
> @@ -315,20 +326,25 @@ static int rz_sysc_probe(struct platform_device *pdev)
> return ret;
>
> data = match->data;
> - if (!data->max_register_offset)
> - return -EINVAL;
> + if (data->signals_init_data) {
> + if (!data->max_register_offset)
> + return -EINVAL;
I assume you don't have any signal use case like RZ/G3S.
Can you please confirm is it correct for RZ/G3E?
Multiple drivers/devices accessing same register like USB PWRRDY register in RZ/G3S?
Cheers,
Biju
^ permalink raw reply [flat|nested] 31+ messages in thread* RE: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family
2024-12-07 10:36 ` Biju Das
@ 2024-12-09 10:43 ` John Madieu
0 siblings, 0 replies; 31+ messages in thread
From: John Madieu @ 2024-12-09 10:43 UTC (permalink / raw)
To: Biju Das, Geert Uytterhoeven, Magnus Damm, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Claudiu Beznea
Cc: john.madieu@gmail.com, linux-renesas-soc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Saturday, December 7, 2024 11:36 AM
> To: John Madieu <john.madieu.xa@bp.renesas.com>; Geert Uytterhoeven
> <geert+renesas@glider.be>; Magnus Damm <magnus.damm@gmail.com>; Rob
> Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
> Dooley <conor+dt@kernel.org>; Claudiu Beznea
> <claudiu.beznea.uj@bp.renesas.com>
> Cc: john.madieu@gmail.com; linux-renesas-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; John Madieu
> <john.madieu.xa@bp.renesas.com>
> Subject: RE: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E
> family
>
> Hi John,
Hi Biju,
>
> Thanks for the patch.
>
> > -----Original Message-----
> > From: John Madieu <john.madieu.xa@bp.renesas.com>
> > Sent: 06 December 2024 21:26
> > Subject: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E
> > family
> >
> > Add SoC detection support for RZ/G3E SoC. Also add support for
> > detecting the number of cores and
> > ETHOS-U55 NPU and also detect PLL mismatch for SW settings other than
> 1.7GHz.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > ---
> > drivers/soc/renesas/Kconfig | 6 +++
> > drivers/soc/renesas/Makefile | 1 +
> > drivers/soc/renesas/r9a09g047-sysc.c | 70 ++++++++++++++++++++++++++++
> > drivers/soc/renesas/rz-sysc.c | 44 +++++++++++------
> > drivers/soc/renesas/rz-sysc.h | 7 +++
> > 5 files changed, 114 insertions(+), 14 deletions(-) create mode
> > 100644 drivers/soc/renesas/r9a09g047-sysc.c
> >
> > diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
> > index
> > a792a3e915fe..9e46b0ee6e80 100644
> > --- a/drivers/soc/renesas/Kconfig
> > +++ b/drivers/soc/renesas/Kconfig
> > @@ -348,6 +348,7 @@ config ARCH_R9A09G011
> >
> > config ARCH_R9A09G047
> > bool "ARM64 Platform support for RZ/G3E"
> > + select SYSC_R9A09G047
> > help
> > This enables support for the Renesas RZ/G3E SoC variants.
> >
> > @@ -386,9 +387,14 @@ config RST_RCAR
> >
> > config SYSC_RZ
> > bool "System controller for RZ SoCs" if COMPILE_TEST
> > + depends on MFD_SYSCON
> >
> > config SYSC_R9A08G045
> > bool "Renesas RZ/G3S System controller support" if COMPILE_TEST
> > select SYSC_RZ
> >
> > +config SYSC_R9A09G047
> > + bool "Renesas RZ/G3E System controller support" if COMPILE_TEST
> > + select SYSC_RZ
> > +
> > endif # SOC_RENESAS
> > diff --git a/drivers/soc/renesas/Makefile
> > b/drivers/soc/renesas/Makefile index
> > 8cd139b3dd0a..3256706112d9 100644
> > --- a/drivers/soc/renesas/Makefile
> > +++ b/drivers/soc/renesas/Makefile
> > @@ -7,6 +7,7 @@ ifdef CONFIG_SMP
> > obj-$(CONFIG_ARCH_R9A06G032) += r9a06g032-smp.o
> > endif
> > obj-$(CONFIG_SYSC_R9A08G045) += r9a08g045-sysc.o
> > +obj-$(CONFIG_SYSC_R9A09G047) += r9a09g047-sysc.o
> >
> > # Family
> > obj-$(CONFIG_PWC_RZV2M) += pwc-rzv2m.o
> > diff --git a/drivers/soc/renesas/r9a09g047-sysc.c
> > b/drivers/soc/renesas/r9a09g047-sysc.c
> > new file mode 100644
> > index 000000000000..32bdab9f1774
> > --- /dev/null
> > +++ b/drivers/soc/renesas/r9a09g047-sysc.c
> > @@ -0,0 +1,70 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * RZ/G3E System controller driver
> > + *
> > + * Copyright (C) 2024 Renesas Electronics Corp.
> > + */
> > +
> > +#include <linux/bits.h>
> > +#include <linux/device.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +
> > +#include "rz-sysc.h"
> > +
> > +/* Register definitions */
> > +#define SYS_LSI_DEVID 0x304
> > +#define SYS_LSI_MODE 0x300
> > +#define SYS_LSI_PRR 0x308
> > +#define SYS_LSI_DEVID_REV GENMASK(31, 28)
> > +#define SYS_LSI_DEVID_SPECIFIC GENMASK(27, 0)
> > +#define SYS_LSI_PRR_CA55_DIS BIT(8)
> > +#define SYS_LSI_PRR_NPU_DIS BIT(1)
> > +/*
> > + * 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
> > +
> > +static void rzg3e_extended_device_identification(struct device *dev,
> > + void __iomem *sysc_base,
> > + struct soc_device_attribute *soc_dev_attr) {
> > + u32 prr_val, mode_val;
> > + bool is_quad_core, npu_enabled;
> > +
> > + prr_val = readl(sysc_base + SYS_LSI_PRR);
> > + mode_val = readl(sysc_base + SYS_LSI_MODE);
> > +
> > + /* Check CPU and NPU configuration */
> > + is_quad_core = !(prr_val & SYS_LSI_PRR_CA55_DIS);
> > + npu_enabled = !(prr_val & SYS_LSI_PRR_NPU_DIS);
> > +
> > + dev_info(dev, "Detected Renesas %s Core %s %s Rev %s %s\n",
> > + is_quad_core ? "Quad" : "Dual",
> > + soc_dev_attr->family,
> > + soc_dev_attr->soc_id,
> > + soc_dev_attr->revision,
> > + npu_enabled ? "with Ethos-U55" : "");
> > +
> > + /* Check CA55 PLL configuration */
> > + if (FIELD_GET(SYS_LSI_MODE_STAT_BOOTPLLCA55, mode_val) !=
> SYS_LSI_MODE_CA55_1_7GHz)
> > + dev_warn(dev, "CA55 PLL is not set to 1.7GHz\n"); }
> > +
> > +static const struct rz_sysc_soc_id_init_data
> rzg3e_sysc_soc_id_init_data __initconst = {
> > + .family = "RZ/G3E",
> > + .id = 0x8679447,
> > + .offset = SYS_LSI_DEVID,
> > + .revision_mask = SYS_LSI_DEVID_REV,
> > + .specific_id_mask = SYS_LSI_DEVID_SPECIFIC,
> > + .extended_device_identification =
> > +rzg3e_extended_device_identification,
> > +};
> > +
> > +const struct rz_sysc_init_data rzg3e_sysc_init_data = {
> > + .soc_id_init_data = &rzg3e_sysc_soc_id_init_data, };
> > diff --git a/drivers/soc/renesas/rz-sysc.c
> > b/drivers/soc/renesas/rz-sysc.c index d34d295831b8..515eca249b6e
> > 100644
> > --- a/drivers/soc/renesas/rz-sysc.c
> > +++ b/drivers/soc/renesas/rz-sysc.c
> > @@ -231,7 +231,7 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc,
> > const struct of_device_id *mat
> >
> > soc_id_start = strchr(match->compatible, ',') + 1;
> > soc_id_end = strchr(match->compatible, '-');
> > - size = soc_id_end - soc_id_start;
> > + size = soc_id_end - soc_id_start + 1;
> > if (size > 32)
> > size = 32;
> > strscpy(soc_id, soc_id_start, size); @@ -257,8 +257,16 @@ static int
> > rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
> > return -ENODEV;
> > }
> >
> > - dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n", soc_dev_attr-
> >family,
> > - soc_dev_attr->soc_id, soc_dev_attr->revision);
> > + /* Try to call SoC-specific device identification */
> > + if (soc_data->extended_device_identification) {
> > + soc_data->extended_device_identification(sysc->dev, sysc-
> >base,
> > + soc_dev_attr);
> > + } else {
> > + dev_info(sysc->dev, "Detected Renesas %s %s Rev %s\n",
> > + soc_dev_attr->family,
> > + soc_dev_attr->soc_id,
> > + soc_dev_attr->revision);
> > + }
> >
> > soc_dev = soc_device_register(soc_dev_attr);
> > if (IS_ERR(soc_dev))
> > @@ -283,6 +291,9 @@ static struct regmap_config rz_sysc_regmap = {
> > static const struct of_device_id rz_sysc_match[] = { #ifdef
> CONFIG_SYSC_R9A08G045
> > { .compatible = "renesas,r9a08g045-sysc", .data =
> > &rzg3s_sysc_init_data },
> > +#endif
> > +#ifdef CONFIG_SYSC_R9A09G047
> > + { .compatible = "renesas,r9a09g047-sys", .data =
> > +&rzg3e_sysc_init_data },
> > #endif
> > { }
> > };
> > @@ -315,20 +326,25 @@ static int rz_sysc_probe(struct platform_device
> *pdev)
> > return ret;
> >
> > data = match->data;
> > - if (!data->max_register_offset)
> > - return -EINVAL;
> > + if (data->signals_init_data) {
> > + if (!data->max_register_offset)
> > + return -EINVAL;
>
> I assume you don't have any signal use case like RZ/G3S.
> Can you please confirm is it correct for RZ/G3E?
There is no such concept of signal.
>
> Multiple drivers/devices accessing same register like USB PWRRDY register
> in RZ/G3S?
There is no register shared between drivers/devices. They each have
their own respective registers.
>
> Cheers,
> Biju
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family
2024-12-06 21:25 ` [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family John Madieu
` (2 preceding siblings ...)
2024-12-07 10:36 ` Biju Das
@ 2024-12-13 16:24 ` Geert Uytterhoeven
2024-12-14 4:36 ` John Madieu
3 siblings, 1 reply; 31+ messages in thread
From: Geert Uytterhoeven @ 2024-12-13 16:24 UTC (permalink / raw)
To: John Madieu
Cc: Magnus Damm, Rob Herring, Biju Das, Krzysztof Kozlowski,
Conor Dooley, Claudiu Beznea, john.madieu, linux-renesas-soc,
linux-kernel, devicetree
Hi John,
On Fri, Dec 6, 2024 at 10:26 PM John Madieu
<john.madieu.xa@bp.renesas.com> wrote:
> Add SoC detection support for RZ/G3E SoC. Also add support for detecting the
> number of cores and ETHOS-U55 NPU and also detect PLL mismatch for SW settings
> other than 1.7GHz.
>
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
Thanks for your patch!
> --- a/drivers/soc/renesas/Kconfig
> +++ b/drivers/soc/renesas/Kconfig
> @@ -348,6 +348,7 @@ config ARCH_R9A09G011
>
> config ARCH_R9A09G047
> bool "ARM64 Platform support for RZ/G3E"
> + select SYSC_R9A09G047
> help
> This enables support for the Renesas RZ/G3E SoC variants.
>
> @@ -386,9 +387,14 @@ config RST_RCAR
>
> config SYSC_RZ
> bool "System controller for RZ SoCs" if COMPILE_TEST
> + depends on MFD_SYSCON
WARNING: unmet direct dependencies detected for SYSC_RZ
Depends on [n]: SOC_RENESAS [=y] && MFD_SYSCON [=n]
Selected by [y]:
- SYSC_R9A08G045 [=y] && SOC_RENESAS [=y]
- SYSC_R9A09G047 [=y] && SOC_RENESAS [=y]
So please select MFD_SYSCON instead.
>
> config SYSC_R9A08G045
> bool "Renesas RZ/G3S System controller support" if COMPILE_TEST
> select SYSC_RZ
>
> +config SYSC_R9A09G047
> + bool "Renesas RZ/G3E System controller support" if COMPILE_TEST
> + select SYSC_RZ
> +
> endif # SOC_RENESAS
> --- /dev/null
> +++ b/drivers/soc/renesas/r9a09g047-sysc.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RZ/G3E System controller driver
> + *
> + * Copyright (C) 2024 Renesas Electronics Corp.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +
> +#include "rz-sysc.h"
> +
> +/* Register definitions */
> +#define SYS_LSI_DEVID 0x304
> +#define SYS_LSI_MODE 0x300
> +#define SYS_LSI_PRR 0x308
> +#define SYS_LSI_DEVID_REV GENMASK(31, 28)
> +#define SYS_LSI_DEVID_SPECIFIC GENMASK(27, 0)
> +#define SYS_LSI_PRR_CA55_DIS BIT(8)
> +#define SYS_LSI_PRR_NPU_DIS BIT(1)
> +/*
> + * 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
Please align the second column, and please group register offsets
and bits together.
> +
> +static void rzg3e_extended_device_identification(struct device *dev,
> + void __iomem *sysc_base,
> + struct soc_device_attribute *soc_dev_attr)
> +{
> + u32 prr_val, mode_val;
> + bool is_quad_core, npu_enabled;
> +
> + prr_val = readl(sysc_base + SYS_LSI_PRR);
> + mode_val = readl(sysc_base + SYS_LSI_MODE);
> +
> + /* Check CPU and NPU configuration */
> + is_quad_core = !(prr_val & SYS_LSI_PRR_CA55_DIS);
> + npu_enabled = !(prr_val & SYS_LSI_PRR_NPU_DIS);
> +
> + dev_info(dev, "Detected Renesas %s Core %s %s Rev %s %s\n",
There are two spaces before the last %s.
Please drop both spaces...
> + is_quad_core ? "Quad" : "Dual",
> + soc_dev_attr->family,
> + soc_dev_attr->soc_id,
> + soc_dev_attr->revision,
> + npu_enabled ? "with Ethos-U55" : "");
... and add one space here, just before "with".
> +
> + /* Check CA55 PLL configuration */
> + if (FIELD_GET(SYS_LSI_MODE_STAT_BOOTPLLCA55, mode_val) != SYS_LSI_MODE_CA55_1_7GHz)
> + dev_warn(dev, "CA55 PLL is not set to 1.7GHz\n");
Just wondering: is that a problem? Can't it be handled in the clock
driver?
> +}
> --- a/drivers/soc/renesas/rz-sysc.c
> +++ b/drivers/soc/renesas/rz-sysc.c
> @@ -231,7 +231,7 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
>
> soc_id_start = strchr(match->compatible, ',') + 1;
> soc_id_end = strchr(match->compatible, '-');
> - size = soc_id_end - soc_id_start;
> + size = soc_id_end - soc_id_start + 1;
Unrelated fix?
> if (size > 32)
> size = 32;
> strscpy(soc_id, soc_id_start, size);
> @@ -315,20 +326,25 @@ static int rz_sysc_probe(struct platform_device *pdev)
> return ret;
>
> data = match->data;
> - if (!data->max_register_offset)
> - return -EINVAL;
> + if (data->signals_init_data) {
if (!data->signals_init_data)
return 0;
> + if (!data->max_register_offset)
> + return -EINVAL;
>
> - ret = rz_sysc_signals_init(sysc, data->signals_init_data, data->num_signals);
> - if (ret)
> - return ret;
> + ret = rz_sysc_signals_init(sysc, data->signals_init_data, data->num_signals);
> + if (ret)
> + return ret;
> +
> + rz_sysc_regmap.max_register = data->max_register_offset;
> + dev_set_drvdata(dev, sysc);
>
> - dev_set_drvdata(dev, sysc);
> - rz_sysc_regmap.max_register = data->max_register_offset;
> - regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> - if (IS_ERR(regmap))
> - return PTR_ERR(regmap);
> + regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
>
> - return of_syscon_register_regmap(dev->of_node, regmap);
> + return of_syscon_register_regmap(dev->of_node, regmap);
> + }
> +
> + return 0;
> }
>
> static struct platform_driver rz_sysc_driver = {
> --- a/drivers/soc/renesas/rz-sysc.h
> +++ b/drivers/soc/renesas/rz-sysc.h
> @@ -42,6 +44,7 @@ struct rz_sysc_signal {
> * @offset: SYSC SoC ID register offset
> * @revision_mask: SYSC SoC ID revision mask
> * @specific_id_mask: SYSC SoC ID specific ID mask
> + * @extended_device_identification: SoC-specific extended device identification
> */
> struct rz_sysc_soc_id_init_data {
> const char * const family;
> @@ -49,6 +52,9 @@ struct rz_sysc_soc_id_init_data {
> u32 offset;
> u32 revision_mask;
> u32 specific_id_mask;
> + void (*extended_device_identification)(struct device *dev,
> + void __iomem *sysc_base,
> + struct soc_device_attribute *soc_dev_attr);
That's a rather long name...
> };
>
> /**
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
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
^ permalink raw reply [flat|nested] 31+ messages in thread* RE: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family
2024-12-13 16:24 ` Geert Uytterhoeven
@ 2024-12-14 4:36 ` John Madieu
2024-12-16 9:03 ` Geert Uytterhoeven
0 siblings, 1 reply; 31+ messages in thread
From: John Madieu @ 2024-12-14 4:36 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Magnus Damm, Rob Herring, Biju Das, Krzysztof Kozlowski,
Conor Dooley, Claudiu Beznea, john.madieu@gmail.com,
linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org
> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Friday, December 13, 2024 5:25 PM
> Subject: Re: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E
> family
>
> Hi John,
Hello Geert,
>
> On Fri, Dec 6, 2024 at 10:26 PM John Madieu
> <john.madieu.xa@bp.renesas.com> wrote:
> > Add SoC detection support for RZ/G3E SoC. Also add support for
> > detecting the number of cores and ETHOS-U55 NPU and also detect PLL
> > mismatch for SW settings other than 1.7GHz.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/soc/renesas/Kconfig
> > +++ b/drivers/soc/renesas/Kconfig
> > @@ -348,6 +348,7 @@ config ARCH_R9A09G011
> >
> > config ARCH_R9A09G047
> > bool "ARM64 Platform support for RZ/G3E"
> > + select SYSC_R9A09G047
> > help
> > This enables support for the Renesas RZ/G3E SoC variants.
> >
> > @@ -386,9 +387,14 @@ config RST_RCAR
> >
> > config SYSC_RZ
> > bool "System controller for RZ SoCs" if COMPILE_TEST
> > + depends on MFD_SYSCON
>
> WARNING: unmet direct dependencies detected for SYSC_RZ
> Depends on [n]: SOC_RENESAS [=y] && MFD_SYSCON [=n]
> Selected by [y]:
> - SYSC_R9A08G045 [=y] && SOC_RENESAS [=y]
> - SYSC_R9A09G047 [=y] && SOC_RENESAS [=y]
>
> So please select MFD_SYSCON instead.
Will be done in v2.
>
> >
> > config SYSC_R9A08G045
> > bool "Renesas RZ/G3S System controller support" if COMPILE_TEST
> > select SYSC_RZ
> >
> > +config SYSC_R9A09G047
> > + bool "Renesas RZ/G3E System controller support" if COMPILE_TEST
> > + select SYSC_RZ
> > +
> > endif # SOC_RENESAS
>
> > --- /dev/null
> > +++ b/drivers/soc/renesas/r9a09g047-sysc.c
> > @@ -0,0 +1,70 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * RZ/G3E System controller driver
> > + *
> > + * Copyright (C) 2024 Renesas Electronics Corp.
> > + */
> > +
> > +#include <linux/bits.h>
> > +#include <linux/device.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +
> > +#include "rz-sysc.h"
> > +
> > +/* Register definitions */
> > +#define SYS_LSI_DEVID 0x304
> > +#define SYS_LSI_MODE 0x300
> > +#define SYS_LSI_PRR 0x308
> > +#define SYS_LSI_DEVID_REV GENMASK(31, 28)
> > +#define SYS_LSI_DEVID_SPECIFIC GENMASK(27, 0)
> > +#define SYS_LSI_PRR_CA55_DIS BIT(8)
> > +#define SYS_LSI_PRR_NPU_DIS BIT(1)
> > +/*
> > + * 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
>
> Please align the second column, and please group register offsets and bits
> together.
Noted.
>
> > +
> > +static void rzg3e_extended_device_identification(struct device *dev,
> > + void __iomem *sysc_base,
> > + struct soc_device_attribute
> > +*soc_dev_attr) {
> > + u32 prr_val, mode_val;
> > + bool is_quad_core, npu_enabled;
> > +
> > + prr_val = readl(sysc_base + SYS_LSI_PRR);
> > + mode_val = readl(sysc_base + SYS_LSI_MODE);
> > +
> > + /* Check CPU and NPU configuration */
> > + is_quad_core = !(prr_val & SYS_LSI_PRR_CA55_DIS);
> > + npu_enabled = !(prr_val & SYS_LSI_PRR_NPU_DIS);
> > +
> > + dev_info(dev, "Detected Renesas %s Core %s %s Rev %s %s\n",
>
> There are two spaces before the last %s.
> Please drop both spaces...
>
> > + is_quad_core ? "Quad" : "Dual",
> > + soc_dev_attr->family,
> > + soc_dev_attr->soc_id,
> > + soc_dev_attr->revision,
> > + npu_enabled ? "with Ethos-U55" : "");
>
> ... and add one space here, just before "with".
Thanks for the hint.
>
> > +
> > + /* Check CA55 PLL configuration */
> > + if (FIELD_GET(SYS_LSI_MODE_STAT_BOOTPLLCA55, mode_val) !=
> SYS_LSI_MODE_CA55_1_7GHz)
> > + dev_warn(dev, "CA55 PLL is not set to 1.7GHz\n");
>
> Just wondering: is that a problem? Can't it be handled in the clock
> driver?
The current implementation supports only 1.7GHz, /2, /4, /8 frequencies,
hence, the warning.
>
> > +}
>
> > --- a/drivers/soc/renesas/rz-sysc.c
> > +++ b/drivers/soc/renesas/rz-sysc.c
> > @@ -231,7 +231,7 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc,
> > const struct of_device_id *mat
> >
> > soc_id_start = strchr(match->compatible, ',') + 1;
> > soc_id_end = strchr(match->compatible, '-');
> > - size = soc_id_end - soc_id_start;
> > + size = soc_id_end - soc_id_start + 1;
>
> Unrelated fix?
I guess so. Was thinking of moving it in a separate patch in the v2
as suggested by Claudiu.
>
> > if (size > 32)
> > size = 32;
> > strscpy(soc_id, soc_id_start, size);
>
> > @@ -315,20 +326,25 @@ static int rz_sysc_probe(struct platform_device
> *pdev)
> > return ret;
> >
> > data = match->data;
> > - if (!data->max_register_offset)
> > - return -EINVAL;
> > + if (data->signals_init_data) {
>
> if (!data->signals_init_data)
> return 0;
Thanks for the hint.
>
> > + if (!data->max_register_offset)
> > + return -EINVAL;
> >
> > - ret = rz_sysc_signals_init(sysc, data->signals_init_data, data-
> >num_signals);
> > - if (ret)
> > - return ret;
> > + ret = rz_sysc_signals_init(sysc, data-
> >signals_init_data, data->num_signals);
> > + if (ret)
> > + return ret;
> > +
> > + rz_sysc_regmap.max_register = data->max_register_offset;
> > + dev_set_drvdata(dev, sysc);
> >
> > - dev_set_drvdata(dev, sysc);
> > - rz_sysc_regmap.max_register = data->max_register_offset;
> > - regmap = devm_regmap_init(dev, NULL, sysc, &rz_sysc_regmap);
> > - if (IS_ERR(regmap))
> > - return PTR_ERR(regmap);
> > + regmap = devm_regmap_init(dev, NULL, sysc,
> &rz_sysc_regmap);
> > + if (IS_ERR(regmap))
> > + return PTR_ERR(regmap);
> >
> > - return of_syscon_register_regmap(dev->of_node, regmap);
> > + return of_syscon_register_regmap(dev->of_node, regmap);
> > + }
> > +
> > + return 0;
> > }
> >
> > static struct platform_driver rz_sysc_driver = {
>
> > --- a/drivers/soc/renesas/rz-sysc.h
> > +++ b/drivers/soc/renesas/rz-sysc.h
> > @@ -42,6 +44,7 @@ struct rz_sysc_signal {
> > * @offset: SYSC SoC ID register offset
> > * @revision_mask: SYSC SoC ID revision mask
> > * @specific_id_mask: SYSC SoC ID specific ID mask
> > + * @extended_device_identification: SoC-specific extended device
> > + identification
> > */
> > struct rz_sysc_soc_id_init_data {
> > const char * const family;
> > @@ -49,6 +52,9 @@ struct rz_sysc_soc_id_init_data {
> > u32 offset;
> > u32 revision_mask;
> > u32 specific_id_mask;
> > + void (*extended_device_identification)(struct device *dev,
> > + void __iomem *sysc_base,
> > + struct soc_device_attribute *soc_dev_attr);
>
> That's a rather long name...
Will be shortened in v2. I'm thinking of ext_dev_id().
>
> > };
> >
> > /**
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
>
> 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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 3/5] soc: renesas: rz-sysc: Add support for RZ/G3E family
2024-12-14 4:36 ` John Madieu
@ 2024-12-16 9:03 ` Geert Uytterhoeven
0 siblings, 0 replies; 31+ messages in thread
From: Geert Uytterhoeven @ 2024-12-16 9:03 UTC (permalink / raw)
To: John Madieu
Cc: Magnus Damm, Rob Herring, Biju Das, Krzysztof Kozlowski,
Conor Dooley, Claudiu Beznea, john.madieu@gmail.com,
linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org
Hi John,
On Sat, Dec 14, 2024 at 5:36 AM John Madieu
<john.madieu.xa@bp.renesas.com> wrote:
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > On Fri, Dec 6, 2024 at 10:26 PM John Madieu
> > <john.madieu.xa@bp.renesas.com> wrote:
> > > Add SoC detection support for RZ/G3E SoC. Also add support for
> > > detecting the number of cores and ETHOS-U55 NPU and also detect PLL
> > > mismatch for SW settings other than 1.7GHz.
> > >
> > > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > > --- a/drivers/soc/renesas/rz-sysc.h
> > > +++ b/drivers/soc/renesas/rz-sysc.h
> > > @@ -42,6 +44,7 @@ struct rz_sysc_signal {
> > > * @offset: SYSC SoC ID register offset
> > > * @revision_mask: SYSC SoC ID revision mask
> > > * @specific_id_mask: SYSC SoC ID specific ID mask
> > > + * @extended_device_identification: SoC-specific extended device
> > > + identification
> > > */
> > > struct rz_sysc_soc_id_init_data {
> > > const char * const family;
> > > @@ -49,6 +52,9 @@ struct rz_sysc_soc_id_init_data {
> > > u32 offset;
> > > u32 revision_mask;
> > > u32 specific_id_mask;
> > > + void (*extended_device_identification)(struct device *dev,
> > > + void __iomem *sysc_base,
> > > + struct soc_device_attribute *soc_dev_attr);
> >
> > That's a rather long name...
>
> Will be shortened in v2. I'm thinking of ext_dev_id().
What about print_id() or print_ext_id(), which is what the function really does?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
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
^ permalink raw reply [flat|nested] 31+ messages in thread