Re: [PATCH 05/16] soc: renesas: sysc: Move RZ/G3S SoC detection on SYSC driver

[Geert Uytterhoeven]

Hi Claudiu,

On Thu, Aug 22, 2024 at 5:28 PM Claudiu <claudiu.beznea@xxxxxxxxx> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxx>

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

-- 
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