Messages in this thread | | | Date | Mon, 18 Sep 2023 22:16:03 +0200 | Subject | Re: [PATCH v2 2/3] soc: mediatek: mtk-socinfo: Add driver for getting chip information | From | Christophe JAILLET <> |
| |
Le 15/09/2023 à 17:26, William-tw Lin a écrit : > Add driver for socinfo retrieval. This patch includes the following: > 1. mtk-socinfo driver for chip info retrieval > 2. Related changes to Makefile and Kconfig > > Signed-off-by: William-tw Lin <william-tw.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> > ---
...
> +static int mtk_socinfo_create_socinfo_node(struct mtk_socinfo *mtk_socinfop) > +{ > + struct soc_device_attribute *attrs; > + static char machine[30] = {0}; > + > + attrs = devm_kzalloc(mtk_socinfop->dev, sizeof(*attrs), GFP_KERNEL); > + if (!attrs) > + return -ENOMEM; > + > + snprintf(machine, 30, "%s (%s)", mtk_socinfop->socinfo_data->marketing_name,
sizeof(machine) instead of 30?
> + mtk_socinfop->socinfo_data->soc_name); > + attrs->family = soc_manufacturer; > + attrs->machine = machine; > + > + soc_dev = soc_device_register(attrs); > + if (IS_ERR(soc_dev)) > + return PTR_ERR(soc_dev); > + > + dev_info(mtk_socinfop->dev, "%s SoC detected.\n", attrs->machine); > + return 0; > +} > + > +static int mtk_socinfo_get_socinfo_data(struct mtk_socinfo *mtk_socinfop) > +{ > + unsigned int i = 0, j = 0;
No need to init.
> + unsigned int num_cell_data = 0; > + u32 *cell_datap[MAX_CELLS] = { NULL }; > + size_t efuse_bytes; > + struct nvmem_cell *cell; > + bool match_socinfo = true;
No need to init.
> + int match_socinfo_index = -1; > + > + for (i = 0; i < MAX_CELLS; i++) { > + cell = nvmem_cell_get(mtk_socinfop->dev, cell_names[i]); > + if (IS_ERR_OR_NULL(cell))
I don't think that testing for NULL is needed.
> + break; > + cell_datap[i] = (u32 *)nvmem_cell_read(cell, &efuse_bytes); > + nvmem_cell_put(cell); > + num_cell_data++; > + } > + > + if (!num_cell_data) > + return -ENOENT; > + > + for (i = 0; i < ARRAY_SIZE(socinfo_data_table); i++) { > + match_socinfo = true; > + for (j = 0; j < num_cell_data; j++) { > + if (*(cell_datap[j]) != socinfo_data_table[i].cell_data[j]) > + match_socinfo = false;
I think that a "break" could be added.
> + } > + if (num_cell_data > 0 && match_socinfo) {
This test can be simplified, becasue 'num_cell_data' can't be <= 0. It is an unsigned int, and we return -ENOENT if it is zero.
> + mtk_socinfop->socinfo_data = &(socinfo_data_table[i]); > + match_socinfo_index = i; > + break; > + } > + } > + > + return match_socinfo_index >= 0 ? match_socinfo_index : -ENOENT; > +} > + > +static const struct of_device_id mtk_socinfo_id_table[] = { > + { .compatible = "mediatek,socinfo" }, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, mtk_socinfo_id_table); > + > +static int mtk_socinfo_probe(struct platform_device *pdev) > +{ > + struct mtk_socinfo *mtk_socinfop; > + int ret = 0;
No need to init.
> + > + mtk_socinfop = devm_kzalloc(&pdev->dev, sizeof(*mtk_socinfop), GFP_KERNEL); > + if (!mtk_socinfop) > + return -ENOMEM; > + > + mtk_socinfop->dev = &pdev->dev; > + > + ret = mtk_socinfo_get_socinfo_data(mtk_socinfop); > + if (ret < 0) > + return dev_err_probe(mtk_socinfop->dev, ret, "Failed to get socinfo data\n"); > + > + ret = mtk_socinfo_create_socinfo_node(mtk_socinfop); > + if (ret != 0) > + return dev_err_probe(mtk_socinfop->dev, -EINVAL, "Failed to create socinfo node\n");
Why not propagating 'ret'?
CJ
> + > + return 0; > +}
...
| |