> From: Anson Huang > Sent: Wednesday, May 22, 2019 2:24 PM > > Add i.MX SCU SoC info driver to support i.MX8QXP SoC, introduce driver > dependency into Kconfig as CONFIG_IMX_SCU must be selected to support > i.MX SCU SoC driver, also need to use platform driver model to make sure > IMX_SCU driver is probed before i.MX SCU SoC driver. > > With this patch, SoC info can be read from sysfs: > > i.mx8qxp-mek# cat /sys/devices/soc0/family Freescale i.MX > > i.mx8qxp-mek# cat /sys/devices/soc0/soc_id > 0x2 > > i.mx8qxp-mek# cat /sys/devices/soc0/machine Freescale i.MX8QXP MEK > > i.mx8qxp-mek# cat /sys/devices/soc0/revision > 1.1 > > Signed-off-by: Anson Huang <[email protected]>
A few minor comments below, Otherwise: Dong Aisheng <[email protected]> > --- > Changes since V4: > - using extern struct of_root instead of searching it again from DT; > - add of_node_put() after "fsl,imx-scu" is found. > --- > drivers/soc/imx/Kconfig | 9 +++ > drivers/soc/imx/Makefile | 1 + > drivers/soc/imx/soc-imx-scu.c | 155 > ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 165 insertions(+) > create mode 100644 drivers/soc/imx/soc-imx-scu.c > > diff --git a/drivers/soc/imx/Kconfig b/drivers/soc/imx/Kconfig index > d80f899..cbc1a41 100644 > --- a/drivers/soc/imx/Kconfig > +++ b/drivers/soc/imx/Kconfig > @@ -7,4 +7,13 @@ config IMX_GPCV2_PM_DOMAINS > select PM_GENERIC_DOMAINS > default y if SOC_IMX7D > > +config IMX_SCU_SOC > + bool "i.MX System Controller Unit SoC info support" > + depends on IMX_SCU > + select SOC_BUS > + help > + If you say yes here you get support for the NXP i.MX System > + Controller Unit SoC info module, it will provide the SoC info > + like SoC family, ID and revision etc. > + > endmenu > diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile index > d6b529e0..ddf343d 100644 > --- a/drivers/soc/imx/Makefile > +++ b/drivers/soc/imx/Makefile > @@ -1,3 +1,4 @@ > obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o > obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o > obj-$(CONFIG_ARCH_MXC) += soc-imx8.o > +obj-$(CONFIG_IMX_SCU_SOC) += soc-imx-scu.o > diff --git a/drivers/soc/imx/soc-imx-scu.c b/drivers/soc/imx/soc-imx-scu.c new > file mode 100644 index 0000000..17deb4e > --- /dev/null > +++ b/drivers/soc/imx/soc-imx-scu.c > @@ -0,0 +1,155 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright 2019 NXP. > + */ > + > +#include <dt-bindings/firmware/imx/rsrc.h> #include > +<linux/firmware/imx/sci.h> #include <linux/slab.h> #include > +<linux/sys_soc.h> #include <linux/platform_device.h> #include > +<linux/of.h> > + > +#define IMX_SCU_SOC_DRIVER_NAME "imx-scu-soc" > + > +static struct imx_sc_ipc *soc_ipc_handle; > + > +struct imx_sc_msg_misc_get_soc_id { > + struct imx_sc_rpc_msg hdr; > + union { > + struct { > + u32 control; > + u16 resource; > + } send; Please follow the exist naming conversion to avoid a diverge if no specific reasons. s/send/req BTW, sub structures also needs packed and better add an explicit __packed As I'm not sure whether GCC will always generate no padding for ARM64 platforms For above structures. > + struct { > + u32 id; > + u16 reserved; What's reserved member for? > + } resp; > + } data; > +} __packed; > + > +static u32 imx_scu_soc_id(void) > +{ > + struct imx_sc_msg_misc_get_soc_id msg; > + struct imx_sc_rpc_msg *hdr = &msg.hdr; > + int ret; > + > + hdr->ver = IMX_SC_RPC_VERSION; > + hdr->svc = IMX_SC_RPC_SVC_MISC; > + hdr->func = IMX_SC_MISC_FUNC_GET_CONTROL; > + hdr->size = 3; > + > + msg.data.send.control = IMX_SC_C_ID; > + msg.data.send.resource = IMX_SC_R_SYSTEM; > + > + ret = imx_scu_call_rpc(soc_ipc_handle, &msg, true); > + if (ret) { > + pr_err("%s: get soc info failed, ret %d\n", __func__, ret); > + /* return 0 means getting revision failed */ > + return 0; > + } > + > + return msg.data.resp.id; > +} > + > +static int imx_scu_soc_probe(struct platform_device *pdev) { > + struct soc_device_attribute *soc_dev_attr; > + struct soc_device *soc_dev; > + u32 id, val; > + int ret; > + > + /* wait i.MX SCU driver ready */ Nitpick: remove this unnecessary line > + ret = imx_scu_get_handle(&soc_ipc_handle); > + if (ret) > + return ret; > + > + soc_dev_attr = devm_kzalloc(&pdev->dev, sizeof(*soc_dev_attr), > + GFP_KERNEL); > + if (!soc_dev_attr) > + return -ENOMEM; > + > + soc_dev_attr->family = "Freescale i.MX"; > + > + ret = of_property_read_string(of_root, > + "model", > + &soc_dev_attr->machine); > + if (ret) > + return ret; > + > + id = imx_scu_soc_id(); > + > + /* format soc_id value passed from SCU firmware */ > + val = id & 0x1f; > + soc_dev_attr->soc_id = val ? kasprintf(GFP_KERNEL, "0x%x", val) > + : "unknown"; > + if (!soc_dev_attr->soc_id) > + return -ENOMEM; > + > + /* format revision value passed from SCU firmware */ > + val = (id >> 5) & 0xf; > + val = (((val >> 2) + 1) << 4) | (val & 0x3); > + soc_dev_attr->revision = val ? kasprintf(GFP_KERNEL, > + "%d.%d", > + (val >> 4) & 0xf, > + val & 0xf) : "unknown"; > + if (!soc_dev_attr->revision) { > + ret = -ENOMEM; > + goto free_soc_id; > + } > + > + soc_dev = soc_device_register(soc_dev_attr); > + if (IS_ERR(soc_dev)) { > + ret = PTR_ERR(soc_dev); > + goto free_revision; > + } > + > + return 0; > + > +free_revision: > + kfree(soc_dev_attr->revision); > +free_soc_id: > + kfree(soc_dev_attr->soc_id); > + return ret; > +} > + > +static struct platform_driver imx_scu_soc_driver = { > + .driver = { > + .name = IMX_SCU_SOC_DRIVER_NAME, > + }, > + .probe = imx_scu_soc_probe, > +}; > + > +static int __init imx_scu_soc_init(void) { > + struct platform_device *imx_scu_soc_pdev; > + struct device_node *np; > + int ret; > + > + np = of_find_compatible_node(NULL, NULL, "fsl,imx-scu"); > + if (!np) > + return -ENODEV; > + > + of_node_put(np); > + > + ret = platform_driver_register(&imx_scu_soc_driver); > + if (ret) > + return ret; > + > + imx_scu_soc_pdev = > + platform_device_register_simple(IMX_SCU_SOC_DRIVER_NAME, > + -1, > + NULL, > + 0); [...] > + if (IS_ERR(imx_scu_soc_pdev)) { > + ret = PTR_ERR(imx_scu_soc_pdev); > + goto unreg_platform_driver; > + } > + > + return 0; > + > +unreg_platform_driver: > + platform_driver_unregister(&imx_scu_soc_driver); > + return ret; > +} This could be simplied with 3 lines: If (IS_ERR(pdev)) Platform_driver_unregsiter(xxx) Return PTR_ERR_OR_ZERO(pdev) Regards Dong Aisheng > +device_initcall(imx_scu_soc_init); > -- > 2.7.4
