Hi Claudiu,
On Tue, Nov 26, 2024 at 10:21 AM Claudiu <claudiu.beznea@xxxxxxxxx> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> The RZ SYSC controller has registers that keep the SoC ID data. Add
> driver support to retrieve the SoC ID and register a SoC driver.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> ---
>
> Changes in v2:
> - this was patch 05/16 in v1
> - changed patch title and description
> - added SoC initialization code in its own function
> - addressed the review comments
> - introduced struct rz_sysc_soc_id_init_data and adjusted the code
> accordingly
> - dropped the RZ/G3S SoC detection code (it will be introduced in
> a separate patch)
Thanks for your patch!
> --- a/drivers/soc/renesas/rz-sysc.c
> +++ b/drivers/soc/renesas/rz-sysc.c
> @@ -211,6 +214,59 @@ static int rz_sysc_signals_init(struct rz_sysc *sysc,
> return 0;
> }
>
> +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};
> + u8 size;
unsigned int (or size_t?)
> +
> + if (!soc_data || !soc_data->family || !soc_data->offset ||
> + !soc_data->revision_mask)
> + return -EINVAL;
Cannot happen?
> +
> + soc_id_start = strchr(match->compatible, ',') + 1;
> + soc_id_end = strchr(match->compatible, '-');
> + size = soc_id_end - soc_id_start;
> + if (size > 32)
> + size = 32;
sizeof(soc_id) instead of hardcoded 32
> + strscpy(soc_id, soc_id_start, size);
Doesn't this wipe the last character, as strscpy() always
NUL-terminates the destination buffer?
> +
> + soc_dev_attr = devm_kzalloc(sysc->dev, sizeof(*soc_dev_attr), GFP_KERNEL);
> + if (!soc_dev_attr)
> + return -ENOMEM;
> +
> + soc_dev_attr->family = soc_data->family;
Shouldn't you duplicate family? It's in __initconst, hence freed later.
> + 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;
> +}
> +
> static struct regmap_config rz_sysc_regmap = {
> .name = "rz_sysc_regs",
> .reg_bits = 32,
> @@ -235,14 +291,15 @@ MODULE_DEVICE_TABLE(of, rz_sysc_match);
> static int rz_sysc_probe(struct platform_device *pdev)
> {
> const struct rz_sysc_init_data *data;
> + const struct of_device_id *match;
> struct device *dev = &pdev->dev;
> - struct rz_sysc *sysc;
> struct regmap *regmap;
> + struct rz_sysc *sysc;
> int ret;
>
> - data = device_get_match_data(dev);
> - if (!data || !data->max_register_offset)
> - return -EINVAL;
> + match = of_match_node(rz_sysc_match, dev->of_node);
> + if (!match || !match->data)
!match->data cannot happen
> + return -ENODEV;
>
> sysc = devm_kzalloc(dev, sizeof(*sysc), GFP_KERNEL);
> if (!sysc)
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
