| Message ID | 20240822152801.602318-6-claudiu.beznea.uj@bp.renesas.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Add initial USB support for the Renesas RZ/G3S SoC | expand |
Hi Claudiu, On Thu, Aug 22, 2024 at 5:28 PM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Now that we have a driver for SYSC driver for RZ/G3S move the SoC detection > for RZ/G3S in SYSC driver. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Thanks for your patch! > --- a/drivers/soc/renesas/rzg3s-sysc.c > +++ b/drivers/soc/renesas/rzg3s-sysc.c > @@ -85,6 +97,39 @@ static int rzg3s_sysc_probe(struct platform_device *pdev) > sysc->dev = dev; > spin_lock_init(&sysc->lock); > > + compatible = of_get_property(dev->of_node, "compatible", NULL); > + if (!compatible) > + return -ENODEV; Please use of_match_device() and of_device_id.compatible instead. > + > + soc_id_start = strchr(compatible, ',') + 1; > + soc_id_end = strchr(compatible, '-'); > + size = soc_id_end - soc_id_start; > + if (size > 32) > + size = 32; > + strscpy(soc_id, soc_id_start, size); > + > + soc_dev_attr = devm_kzalloc(dev, sizeof(*soc_dev_attr), GFP_KERNEL); > + if (!soc_dev_attr) > + return -ENOMEM; > + > + soc_dev_attr->family = "RZ/G3S"; > + soc_dev_attr->soc_id = devm_kstrdup(dev, soc_id, GFP_KERNEL); > + if (!soc_dev_attr->soc_id) > + return -ENOMEM; > + > + devid = readl(sysc->base + RZG3S_SYS_LSI_DEVID); > + revision = FIELD_GET(RZG3S_SYS_LSI_DEVID_REV, devid); > + soc_dev_attr->revision = devm_kasprintf(dev, GFP_KERNEL, "%u", revision); > + if (!soc_dev_attr->revision) > + return -ENOMEM; > + > + dev_info(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)) > + return PTR_ERR(soc_dev); > + > return rzg3s_sysc_reset_probe(sysc, "reset", 0); > } My first thought was "oh no, now this is handled/duplicated in two places", but if you later migrate the chip identification support for the rest of RZ/G2L devices to here, it may start to look better ;-) One caveat is that soc_device_match() can be called quite early in the boot process, hence renesas_soc_init() is an early_initcall(). So registering the soc_device from a platform_driver might be too late, especially since fw_devlinks won't help you in this particular case. However, I think all real early calls to soc_device_match() are gone since the removal of the support for R-Car H3 ES1.x, and all remaining calls impact only R-Car and RZ/Gx (not G2L) SoCs. Gr{oetje,eeting}s, Geert
Hi, Geert, On 08.10.2024 16:23, Geert Uytterhoeven wrote: > Hi Claudiu, > > On Thu, Aug 22, 2024 at 5:28 PM Claudiu <claudiu.beznea@tuxon.dev> wrote: >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> Now that we have a driver for SYSC driver for RZ/G3S move the SoC detection >> for RZ/G3S in SYSC driver. >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Thanks for your patch! > >> --- a/drivers/soc/renesas/rzg3s-sysc.c >> +++ b/drivers/soc/renesas/rzg3s-sysc.c >> @@ -85,6 +97,39 @@ static int rzg3s_sysc_probe(struct platform_device *pdev) >> sysc->dev = dev; >> spin_lock_init(&sysc->lock); >> >> + compatible = of_get_property(dev->of_node, "compatible", NULL); >> + if (!compatible) >> + return -ENODEV; > > Please use of_match_device() and of_device_id.compatible instead. OK. > >> + >> + soc_id_start = strchr(compatible, ',') + 1; >> + soc_id_end = strchr(compatible, '-'); >> + size = soc_id_end - soc_id_start; >> + if (size > 32) >> + size = 32; >> + strscpy(soc_id, soc_id_start, size); >> + >> + soc_dev_attr = devm_kzalloc(dev, sizeof(*soc_dev_attr), GFP_KERNEL); >> + if (!soc_dev_attr) >> + return -ENOMEM; >> + >> + soc_dev_attr->family = "RZ/G3S"; >> + soc_dev_attr->soc_id = devm_kstrdup(dev, soc_id, GFP_KERNEL); >> + if (!soc_dev_attr->soc_id) >> + return -ENOMEM; >> + >> + devid = readl(sysc->base + RZG3S_SYS_LSI_DEVID); >> + revision = FIELD_GET(RZG3S_SYS_LSI_DEVID_REV, devid); >> + soc_dev_attr->revision = devm_kasprintf(dev, GFP_KERNEL, "%u", revision); >> + if (!soc_dev_attr->revision) >> + return -ENOMEM; >> + >> + dev_info(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)) >> + return PTR_ERR(soc_dev); >> + >> return rzg3s_sysc_reset_probe(sysc, "reset", 0); >> } > > My first thought was "oh no, now this is handled/duplicated in two > places", but if you later migrate the chip identification support for > the rest of RZ/G2L devices to here, it may start to look better ;-) Yes, this is how I see it going forward. > > One caveat is that soc_device_match() can be called quite early in > the boot process, hence renesas_soc_init() is an early_initcall(). > So registering the soc_device from a platform_driver might be too late, > especially since fw_devlinks won't help you in this particular case. > However, I think all real early calls to soc_device_match() are gone > since the removal of the support for R-Car H3 ES1.x, and all remaining > calls impact only R-Car and RZ/Gx (not G2L) SoCs. That is good to know. I get that we should be safe going forward with this approach. Thank you, Claudiu Beznea > > Gr{oetje,eeting}s, > > Geert >
diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c index 172d59e6fbcf..425d9037dcd0 100644 --- a/drivers/soc/renesas/renesas-soc.c +++ b/drivers/soc/renesas/renesas-soc.c @@ -71,10 +71,6 @@ static const struct renesas_family fam_rzg2ul __initconst __maybe_unused = { .name = "RZ/G2UL", }; -static const struct renesas_family fam_rzg3s __initconst __maybe_unused = { - .name = "RZ/G3S", -}; - static const struct renesas_family fam_rzv2h __initconst __maybe_unused = { .name = "RZ/V2H", }; @@ -176,11 +172,6 @@ static const struct renesas_soc soc_rz_g2ul __initconst __maybe_unused = { .id = 0x8450447, }; -static const struct renesas_soc soc_rz_g3s __initconst __maybe_unused = { - .family = &fam_rzg3s, - .id = 0x85e0447, -}; - static const struct renesas_soc soc_rz_v2h __initconst __maybe_unused = { .family = &fam_rzv2h, .id = 0x847a447, @@ -410,9 +401,6 @@ static const struct of_device_id renesas_socs[] __initconst __maybe_unused = { #ifdef CONFIG_ARCH_R9A07G054 { .compatible = "renesas,r9a07g054", .data = &soc_rz_v2l }, #endif -#ifdef CONFIG_ARCH_R9A08G045 - { .compatible = "renesas,r9a08g045", .data = &soc_rz_g3s }, -#endif #ifdef CONFIG_ARCH_R9A09G011 { .compatible = "renesas,r9a09g011", .data = &soc_rz_v2m }, #endif diff --git a/drivers/soc/renesas/rzg3s-sysc.c b/drivers/soc/renesas/rzg3s-sysc.c index e664d29ce5c3..1dd48c7255d1 100644 --- a/drivers/soc/renesas/rzg3s-sysc.c +++ b/drivers/soc/renesas/rzg3s-sysc.c @@ -6,10 +6,16 @@ */ #include <linux/auxiliary_bus.h> +#include <linux/io.h> +#include <linux/of.h> #include <linux/platform_device.h> +#include <linux/sys_soc.h> #include <linux/soc/renesas/rzg3s-sysc-reset.h> +#define RZG3S_SYS_LSI_DEVID 0xa04 +#define RZG3S_SYS_LSI_DEVID_REV GENMASK(31, 28) + /** * struct rzg3s_sysc - SYSC private data structure * @base: base address @@ -71,8 +77,14 @@ static int rzg3s_sysc_reset_probe(struct rzg3s_sysc *sysc, const char *adev_name static int rzg3s_sysc_probe(struct platform_device *pdev) { + const char *soc_id_start, *soc_id_end, *compatible; + struct soc_device_attribute *soc_dev_attr; struct device *dev = &pdev->dev; + struct soc_device *soc_dev; struct rzg3s_sysc *sysc; + char soc_id[32] = {0}; + u32 devid, revision; + u8 size; sysc = devm_kzalloc(dev, sizeof(*sysc), GFP_KERNEL); if (!sysc) @@ -85,6 +97,39 @@ static int rzg3s_sysc_probe(struct platform_device *pdev) sysc->dev = dev; spin_lock_init(&sysc->lock); + compatible = of_get_property(dev->of_node, "compatible", NULL); + if (!compatible) + return -ENODEV; + + soc_id_start = strchr(compatible, ',') + 1; + soc_id_end = strchr(compatible, '-'); + size = soc_id_end - soc_id_start; + if (size > 32) + size = 32; + strscpy(soc_id, soc_id_start, size); + + soc_dev_attr = devm_kzalloc(dev, sizeof(*soc_dev_attr), GFP_KERNEL); + if (!soc_dev_attr) + return -ENOMEM; + + soc_dev_attr->family = "RZ/G3S"; + soc_dev_attr->soc_id = devm_kstrdup(dev, soc_id, GFP_KERNEL); + if (!soc_dev_attr->soc_id) + return -ENOMEM; + + devid = readl(sysc->base + RZG3S_SYS_LSI_DEVID); + revision = FIELD_GET(RZG3S_SYS_LSI_DEVID_REV, devid); + soc_dev_attr->revision = devm_kasprintf(dev, GFP_KERNEL, "%u", revision); + if (!soc_dev_attr->revision) + return -ENOMEM; + + dev_info(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)) + return PTR_ERR(soc_dev); + return rzg3s_sysc_reset_probe(sysc, "reset", 0); }