RE: [PATCH V2 1/2] soc: imx: Add SCU SoC info driver support
Hi, Leonard > -Original Message- > From: Leonard Crestez > Sent: Wednesday, May 15, 2019 6:05 PM > To: Anson Huang > Cc: catalin.mari...@arm.com; will.dea...@arm.com; > shawn...@kernel.org; s.ha...@pengutronix.de; ker...@pengutronix.de; > feste...@gmail.com; agr...@kernel.org; maxime.rip...@bootlin.com; > o...@lixom.net; horms+rene...@verge.net.au; > ja...@amarulasolutions.com; bjorn.anders...@linaro.org; > marc.w.gonza...@free.fr; dingu...@kernel.org; > enric.balle...@collabora.com; l.st...@pengutronix.de; Abel Vesa > ; r...@kernel.org; linux-arm- > ker...@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-imx > ; Aisheng Dong > Subject: Re: [PATCH V2 1/2] soc: imx: Add SCU SoC info driver support > > On 15.05.2019 11:32, Anson Huang wrote: > > 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: > > > + id = of_match_node(imx_scu_soc_match, root); > > + if (!id) { > > + of_node_put(root); > > + return -ENODEV; > > + } > > Perhaps this check should be moved from imx_scu_soc_probe to > imx_scu_soc_init? As far as I can tell this "probe" function will be attempted > on all SOCs (even non-imx). Better to check if we're on a SCU-based soc early > and avoid temporary allocations. Make sense, I will move this check block into imx_scu_soc_init() function and ONLY register platform driver when check passed. > > > +module_init(imx_scu_soc_init); > > +module_exit(imx_scu_soc_exit); > > Please don't make this a module OK, will fix it in V3. Thanks, Anson. > > -- > Regards, > Leonard
RE: [PATCH V2 1/2] soc: imx: Add SCU SoC info driver support
> -Original Message- > From: Daniel Baluta [mailto:daniel.bal...@gmail.com] > Sent: Wednesday, May 15, 2019 7:47 PM > To: Anson Huang > Cc: catalin.mari...@arm.com; will.dea...@arm.com; > shawn...@kernel.org; s.ha...@pengutronix.de; ker...@pengutronix.de; > feste...@gmail.com; agr...@kernel.org; maxime.rip...@bootlin.com; > o...@lixom.net; horms+rene...@verge.net.au; > ja...@amarulasolutions.com; bjorn.anders...@linaro.org; Leonard Crestez > ; marc.w.gonza...@free.fr; > dingu...@kernel.org; enric.balle...@collabora.com; > l.st...@pengutronix.de; Abel Vesa ; r...@kernel.org; > linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux- > imx > Subject: Re: [PATCH V2 1/2] soc: imx: Add SCU SoC info driver support > > Hi Anson, > > Since you are going to send a new version for this please consider my > comment inline. > > > > > +static u32 imx8qxp_soc_revision(void) { > > + struct imx_sc_msg_misc_get_soc_id msg; > > + struct imx_sc_rpc_msg *hdr = > > + u32 rev = 0; > > No need to initialize this here. > > > + 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, , true); > > + if (ret) { > > + dev_err(_scu_soc_pdev->dev, > > + "get soc info failed, ret %d\n", ret); > > + /* return 0 means getting revision failed */ > > Just return 0 here. No need for rev. OK, thanks. Anson. > > + return rev; > > + } > > +
Re: [PATCH V2 1/2] soc: imx: Add SCU SoC info driver support
Hi Anson, Since you are going to send a new version for this please consider my comment inline. > +static u32 imx8qxp_soc_revision(void) > +{ > + struct imx_sc_msg_misc_get_soc_id msg; > + struct imx_sc_rpc_msg *hdr = > + u32 rev = 0; No need to initialize this here. > + 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, , true); > + if (ret) { > + dev_err(_scu_soc_pdev->dev, > + "get soc info failed, ret %d\n", ret); > + /* return 0 means getting revision failed */ Just return 0 here. No need for rev. > + return rev; > + } > +
Re: [PATCH V2 1/2] soc: imx: Add SCU SoC info driver support
On 15.05.2019 11:32, Anson Huang wrote: > 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: > + id = of_match_node(imx_scu_soc_match, root); > + if (!id) { > + of_node_put(root); > + return -ENODEV; > + } Perhaps this check should be moved from imx_scu_soc_probe to imx_scu_soc_init? As far as I can tell this "probe" function will be attempted on all SOCs (even non-imx). Better to check if we're on a SCU-based soc early and avoid temporary allocations. > +module_init(imx_scu_soc_init); > +module_exit(imx_scu_soc_exit); Please don't make this a module -- Regards, Leonard
[PATCH V2 1/2] soc: imx: Add SCU SoC info driver support
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 i.MX8QXP 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 --- Changes since V1: - return correct value in probe function instead of hardcode; - add comment to explain why return 0 when get soc info from SCU firmware failed. --- drivers/soc/imx/Kconfig | 9 +++ drivers/soc/imx/Makefile | 1 + drivers/soc/imx/soc-imx-scu.c | 183 ++ 3 files changed, 193 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 000..2331209 --- /dev/null +++ b/drivers/soc/imx/soc-imx-scu.c @@ -0,0 +1,183 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright 2019 NXP. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define IMX_SCU_SOC_DRIVER_NAME"imx-scu-soc" + +static struct imx_sc_ipc *soc_ipc_handle; +static struct platform_device *imx_scu_soc_pdev; + +struct imx_sc_msg_misc_get_soc_id { + struct imx_sc_rpc_msg hdr; + union { + struct { + u32 control; + u16 resource; + } send; + struct { + u32 id; + u16 reserved; + } resp; + } data; +} __packed; + +struct imx_scu_soc_data { + char *name; + u32 (*soc_revision)(void); +}; + +static u32 imx8qxp_soc_revision(void) +{ + struct imx_sc_msg_misc_get_soc_id msg; + struct imx_sc_rpc_msg *hdr = + u32 rev = 0; + 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, , true); + if (ret) { + dev_err(_scu_soc_pdev->dev, + "get soc info failed, ret %d\n", ret); + /* return 0 means getting revision failed */ + return rev; + } + + /* format revision value passed from SCU firmware */ + rev = (msg.data.resp.id >> 5) & 0xf; + rev = (((rev >> 2) + 1) << 4) | (rev & 0x3); + + return rev; +} + +static const struct imx_scu_soc_data imx8qxp_soc_data = { + .name = "i.MX8QXP", + .soc_revision = imx8qxp_soc_revision, +}; + +static const struct of_device_id imx_scu_soc_match[] = { + { .compatible = "fsl,imx8qxp", .data = _soc_data, }, + { } +}; + +#define imx_scu_revision(soc_rev) \ + soc_rev ? \ + kasprintf(GFP_KERNEL, "%d.%d", (soc_rev >> 4) & 0xf, soc_rev & 0xf) : \ + "unknown" + +static int imx_scu_soc_probe(struct platform_device *pdev) +{ + struct soc_device_attribute *soc_dev_attr; + const struct imx_scu_soc_data *data; + const struct of_device_id *id; + struct soc_device *soc_dev; + struct device_node *root; + u32 soc_rev = 0; + int ret; + + /* wait i.MX SCU driver ready */ + ret = imx_scu_get_handle(_ipc_handle); + if (ret) + return ret; + + soc_dev_attr = devm_kzalloc(>dev, sizeof(*soc_dev_attr), + GFP_KERNEL); + if (!soc_dev_attr) + return -ENOMEM; + + soc_dev_attr->family = "Freescale i.MX"; + + root = of_find_node_by_path("/"); + ret = of_property_read_string(root, "model",