Re: [PATCH v4 1/9] soc: renesas: Add SYSC driver for Renesas RZ family

[Geert Uytterhoeven]

Hi John, Claudiu,

On Thu, Jan 23, 2025 at 6:05 PM John Madieu
<john.madieu.xa@xxxxxxxxxxxxxx> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> The RZ/G3S system controller (SYSC) has various registers that control
> different functionalities. One of the exposed register offsers information
> about the SoC identification.
>
> Add a driver that identifies the SoC. Later the driver will be extended
> with other functionalities.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> Signed-off-by: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> ---
> v4: No changes
> v3: New patch

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/soc/renesas/rz-sysc.c

> +static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *match)
> +{
> +       const struct rz_sysc_init_data *sysc_data = match->data;
> +       const struct rz_sysc_soc_id_init_data *soc_data = sysc_data->soc_id_init_data;
> +       struct soc_device_attribute *soc_dev_attr;
> +       const char *soc_id_start, *soc_id_end;
> +       u32 val, revision, specific_id;
> +       struct soc_device *soc_dev;
> +       char soc_id[32] = {0};
> +       size_t size;
> +
> +       soc_id_start = strchr(match->compatible, ',') + 1;
> +       soc_id_end = strchr(match->compatible, '-');
> +       size = soc_id_end - soc_id_start + 1;
> +       if (size > 32)
> +               size = sizeof(soc_id);
> +       strscpy(soc_id, soc_id_start, size);
> +
> +       soc_dev_attr = devm_kzalloc(sysc->dev, sizeof(*soc_dev_attr), GFP_KERNEL);
> +       if (!soc_dev_attr)
> +               return -ENOMEM;
> +
> +       soc_dev_attr->family = devm_kstrdup(sysc->dev, soc_data->family, GFP_KERNEL);

Missing NULL-check

> +       soc_dev_attr->soc_id = devm_kstrdup(sysc->dev, soc_id, GFP_KERNEL);
> +       if (!soc_dev_attr->soc_id)
> +               return -ENOMEM;
> +
> +       val = readl(sysc->base + soc_data->offset);
> +       revision = field_get(soc_data->revision_mask, val);
> +       specific_id = field_get(soc_data->specific_id_mask, val);
> +       soc_dev_attr->revision = devm_kasprintf(sysc->dev, GFP_KERNEL, "%u", revision);
> +       if (!soc_dev_attr->revision)
> +               return -ENOMEM;
> +
> +       if (soc_data->id && specific_id != soc_data->id) {
> +               dev_warn(sysc->dev, "SoC mismatch (product = 0x%x)\n", specific_id);
> +               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);
> +
> +       soc_dev = soc_device_register(soc_dev_attr);
> +       if (IS_ERR(soc_dev))
> +               return PTR_ERR(soc_dev);
> +
> +       return 0;
> +}

> --- /dev/null
> +++ b/drivers/soc/renesas/rz-sysc.h
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Renesas RZ System Controller
> + *
> + * Copyright (C) 2024 Renesas Electronics Corp.
> + */
> +
> +#ifndef __SOC_RENESAS_RZ_SYSC_H__
> +#define __SOC_RENESAS_RZ_SYSC_H__
> +
> +#include <linux/types.h>
> +
> +/**
> + * struct rz_syc_soc_id_init_data - RZ SYSC SoC identification initialization data
> + * @family: RZ SoC family
> + * @id: RZ SoC expected ID
> + * @offset: SYSC SoC ID register offset
> + * @revision_mask: SYSC SoC ID revision mask
> + * @specific_id_mask: SYSC SoC ID specific ID mask
> + */
> +struct rz_sysc_soc_id_init_data {
> +       const char * const family;
> +       u32 id;
> +       u32 offset;

.devid_offset?

> +       u32 revision_mask;
> +       u32 specific_id_mask;
> +};

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