Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Franck Lenormand <franck.lenormand@nxp.com>
To:     Anson Huang <anson.huang@nxp.com>,
        "catalin.marinas@arm.com" <catalin.marinas@arm.com>,
        "will@kernel.org" <will@kernel.org>,
        "shawnguo@kernel.org" <shawnguo@kernel.org>,
        "s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
        "kernel@pengutronix.de" <kernel@pengutronix.de>,
        "festevam@gmail.com" <festevam@gmail.com>,
        "bjorn.andersson@linaro.org" <bjorn.andersson@linaro.org>,
        Leo Li <leoyang.li@nxp.com>,
        "vkoul@kernel.org" <vkoul@kernel.org>,
        "geert+renesas@glider.be" <geert+renesas@glider.be>,
        "olof@lixom.net" <olof@lixom.net>,
        Leonard Crestez <leonard.crestez@nxp.com>,
        Aisheng Dong <aisheng.dong@nxp.com>,
        Peng Fan <peng.fan@nxp.com>, "arnd@arndb.de" <arnd@arndb.de>,
        "krzk@kernel.org" <krzk@kernel.org>,
        Daniel Baluta <daniel.baluta@nxp.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC:     dl-linux-imx <linux-imx@nxp.com>
Subject: RE: [PATCH] firmware: imx: Move i.MX SCU soc driver into SCU firmware
 driver
Thread-Topic: [PATCH] firmware: imx: Move i.MX SCU soc driver into SCU
 firmware driver
Thread-Index: AQHWSiI9sFJUktKfZ0ueTRO9gk8o56jnsbFA
Date:   Wed, 24 Jun 2020 12:33:44 +0000
Message-ID: <AM4PR0401MB24016D7F1E97F70AEC23BEB392950@AM4PR0401MB2401.eurprd04.prod.outlook.com>
References: <1593000687-12213-1-git-send-email-Anson.Huang@nxp.com>
In-Reply-To: <1593000687-12213-1-git-send-email-Anson.Huang@nxp.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 6786d14d-e8ee-4f3a-db40-08d8183ad5e2
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Jun 2020 12:33:44.7759
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: anNtTJdjF6mpvU2ai0xPxSY09xihRC5BvYkNgwlLKE58BgC8nuxtJVG1koVMyanZiX5lMQJj4gItQbuKnWVk8koD7amc4tmYRhAbTSnG1JU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR04MB4593
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hello,

The move of folder is a good idea however I do not feel adding the code of =
soc-imx-scu.c  to imx-scu.c is the best.
Until now imx-scu.c was focused on exchanging the RPC and init the IRQ.
Why not have soc-imx-scu.c  copied to imx-scu-soc.c (change filename) to ha=
ve a pattern similar to imx-scu-irq? And keep the  imx_scu_soc_init into im=
x-scu.c.

Regards,

Franck LENORMAND, STEC Engineer

-----Original Message-----
From: Anson Huang <Anson.Huang@nxp.com>=20
Sent: Wednesday, June 24, 2020 2:11 PM
To: catalin.marinas@arm.com; will@kernel.org; shawnguo@kernel.org; s.hauer@=
pengutronix.de; kernel@pengutronix.de; festevam@gmail.com; bjorn.andersson@=
linaro.org; Leo Li <leoyang.li@nxp.com>; vkoul@kernel.org; geert+renesas@gl=
ider.be; olof@lixom.net; Leonard Crestez <leonard.crestez@nxp.com>; Aisheng=
 Dong <aisheng.dong@nxp.com>; Peng Fan <peng.fan@nxp.com>; Franck Lenormand=
 <franck.lenormand@nxp.com>; arnd@arndb.de; krzk@kernel.org; Daniel Baluta =
<daniel.baluta@nxp.com>; linux-arm-kernel@lists.infradead.org; linux-kernel=
@vger.kernel.org
Cc: dl-linux-imx <linux-imx@nxp.com>
Subject: [PATCH] firmware: imx: Move i.MX SCU soc driver into SCU firmware =
driver

The i.MX SCU soc driver depends on SCU firmware driver, so it has to use pl=
atform driver model for proper defer probe operation, since it has no devic=
e binding in DT file, a simple platform device is created together inside t=
he platform driver. To make it more clean, we can just move the entire SCU =
soc driver into SCU firmware driver.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm64/configs/defconfig   |   1 -
 drivers/firmware/imx/imx-scu.c | 127 ++++++++++++++++++++++++++++++
 drivers/soc/imx/Kconfig        |   9 ---
 drivers/soc/imx/Makefile       |   1 -
 drivers/soc/imx/soc-imx-scu.c  | 172 -------------------------------------=
----
 5 files changed, 127 insertions(+), 183 deletions(-)  delete mode 100644 d=
rivers/soc/imx/soc-imx-scu.c

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig in=
dex ae76fae..35f037f 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -854,7 +854,6 @@ CONFIG_OWL_PM_DOMAINS=3Dy  CONFIG_RASPBERRYPI_POWER=3Dy=
  CONFIG_FSL_DPAA=3Dy  CONFIG_FSL_MC_DPIO=3Dy -CONFIG_IMX_SCU_SOC=3Dy  CONF=
IG_QCOM_AOSS_QMP=3Dy  CONFIG_QCOM_GENI_SE=3Dy  CONFIG_QCOM_RMTFS_MEM=3Dm di=
ff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c =
index 2ab0482..2719487 100644
--- a/drivers/firmware/imx/imx-scu.c
+++ b/drivers/firmware/imx/imx-scu.c
@@ -7,6 +7,7 @@
  *
  */
=20
+#include <dt-bindings/firmware/imx/rsrc.h>
 #include <linux/err.h>
 #include <linux/firmware/imx/ipc.h>
 #include <linux/firmware/imx/sci.h>
@@ -18,6 +19,7 @@
 #include <linux/mutex.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/sys_soc.h>
=20
 #define SCU_MU_CHAN_NUM		8
 #define MAX_RX_TIMEOUT		(msecs_to_jiffies(30))
@@ -45,6 +47,25 @@ struct imx_sc_ipc {
 	u8 count;
 };
=20
+struct imx_sc_msg_misc_get_soc_id {
+	struct imx_sc_rpc_msg hdr;
+	union {
+		struct {
+			u32 control;
+			u16 resource;
+		} __packed req;
+		struct {
+			u32 id;
+		} resp;
+	} data;
+} __packed __aligned(4);
+
+struct imx_sc_msg_misc_get_soc_uid {
+	struct imx_sc_rpc_msg hdr;
+	u32 uid_low;
+	u32 uid_high;
+} __packed;
+
 /*
  * This type is used to indicate error response for most functions.
  */
@@ -257,6 +278,108 @@ int imx_scu_call_rpc(struct imx_sc_ipc *sc_ipc, void =
*msg, bool have_resp)  }  EXPORT_SYMBOL(imx_scu_call_rpc);
=20
+static int imx_scu_soc_uid(u64 *soc_uid) {
+	struct imx_sc_msg_misc_get_soc_uid msg;
+	struct imx_sc_rpc_msg *hdr =3D &msg.hdr;
+	int ret;
+
+	hdr->ver =3D IMX_SC_RPC_VERSION;
+	hdr->svc =3D IMX_SC_RPC_SVC_MISC;
+	hdr->func =3D IMX_SC_MISC_FUNC_UNIQUE_ID;
+	hdr->size =3D 1;
+
+	ret =3D imx_scu_call_rpc(imx_sc_ipc_handle, &msg, true);
+	if (ret) {
+		pr_err("%s: get soc uid failed, ret %d\n", __func__, ret);
+		return ret;
+	}
+
+	*soc_uid =3D msg.uid_high;
+	*soc_uid <<=3D 32;
+	*soc_uid |=3D msg.uid_low;
+
+	return 0;
+}
+
+static int imx_scu_soc_id(void)
+{
+	struct imx_sc_msg_misc_get_soc_id msg;
+	struct imx_sc_rpc_msg *hdr =3D &msg.hdr;
+	int ret;
+
+	hdr->ver =3D IMX_SC_RPC_VERSION;
+	hdr->svc =3D IMX_SC_RPC_SVC_MISC;
+	hdr->func =3D IMX_SC_MISC_FUNC_GET_CONTROL;
+	hdr->size =3D 3;
+
+	msg.data.req.control =3D IMX_SC_C_ID;
+	msg.data.req.resource =3D IMX_SC_R_SYSTEM;
+
+	ret =3D imx_scu_call_rpc(imx_sc_ipc_handle, &msg, true);
+	if (ret) {
+		pr_err("%s: get soc info failed, ret %d\n", __func__, ret);
+		return ret;
+	}
+
+	return msg.data.resp.id;
+}
+
+static int imx_scu_soc_init(struct device *dev) {
+	struct soc_device_attribute *soc_dev_attr;
+	struct soc_device *soc_dev;
+	int id, ret;
+	u64 uid =3D 0;
+	u32 val;
+
+	soc_dev_attr =3D devm_kzalloc(dev, sizeof(*soc_dev_attr),
+				    GFP_KERNEL);
+	if (!soc_dev_attr)
+		return -ENOMEM;
+
+	soc_dev_attr->family =3D "Freescale i.MX";
+
+	ret =3D of_property_read_string(of_root,
+				      "model",
+				      &soc_dev_attr->machine);
+	if (ret)
+		return ret;
+
+	id =3D imx_scu_soc_id();
+	if (id < 0)
+		return -EINVAL;
+
+	ret =3D imx_scu_soc_uid(&uid);
+	if (ret < 0)
+		return -EINVAL;
+
+	/* format soc_id value passed from SCU firmware */
+	val =3D id & 0x1f;
+	soc_dev_attr->soc_id =3D devm_kasprintf(dev, GFP_KERNEL, "0x%x", val);
+	if (!soc_dev_attr->soc_id)
+		return -ENOMEM;
+
+	/* format revision value passed from SCU firmware */
+	val =3D (id >> 5) & 0xf;
+	val =3D (((val >> 2) + 1) << 4) | (val & 0x3);
+	soc_dev_attr->revision =3D devm_kasprintf(dev, GFP_KERNEL, "%d.%d",
+						(val >> 4) & 0xf, val & 0xf);
+	if (!soc_dev_attr->revision)
+		return -ENOMEM;
+
+	soc_dev_attr->serial_number =3D devm_kasprintf(dev, GFP_KERNEL,
+						     "%016llX", uid);
+	if (!soc_dev_attr->serial_number)
+		return -ENOMEM;
+
+	soc_dev =3D soc_device_register(soc_dev_attr);
+	if (IS_ERR(soc_dev))
+		return PTR_ERR(soc_dev);
+
+	return 0;
+}
+
 static int imx_scu_probe(struct platform_device *pdev)  {
 	struct device *dev =3D &pdev->dev;
@@ -328,6 +451,10 @@ static int imx_scu_probe(struct platform_device *pdev)
=20
 	imx_sc_ipc_handle =3D sc_ipc;
=20
+	ret =3D imx_scu_soc_init(dev);
+	if (ret)
+		dev_warn(dev, "failed to initialize SoC info: %d\n", ret);
+
 	ret =3D imx_scu_enable_general_irq_channel(dev);
 	if (ret)
 		dev_warn(dev,
diff --git a/drivers/soc/imx/Kconfig b/drivers/soc/imx/Kconfig index d515d2=
c..d49fa63 100644
--- a/drivers/soc/imx/Kconfig
+++ b/drivers/soc/imx/Kconfig
@@ -8,15 +8,6 @@ config IMX_GPCV2_PM_DOMAINS
 	select PM_GENERIC_DOMAINS
 	default y if SOC_IMX7D
=20
-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.
-
 config SOC_IMX8M
 	bool "i.MX8M SoC family support"
 	depends on ARCH_MXC || COMPILE_TEST
diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile index 4461=
432..078dc91 100644
--- a/drivers/soc/imx/Makefile
+++ b/drivers/soc/imx/Makefile
@@ -5,4 +5,3 @@ endif
 obj-$(CONFIG_HAVE_IMX_GPC) +=3D gpc.o
 obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) +=3D gpcv2.o
 obj-$(CONFIG_SOC_IMX8M) +=3D soc-imx8m.o
-obj-$(CONFIG_IMX_SCU_SOC) +=3D soc-imx-scu.o diff --git a/drivers/soc/imx/=
soc-imx-scu.c b/drivers/soc/imx/soc-imx-scu.c deleted file mode 100644 inde=
x 92448ca..0000000
--- a/drivers/soc/imx/soc-imx-scu.c
+++ /dev/null
@@ -1,172 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright 2019 NXP.
- */
-
-#include <dt-bindings/firmware/imx/rsrc.h> -#include <linux/firmware/imx/s=
ci.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;
-		} __packed req;
-		struct {
-			u32 id;
-		} resp;
-	} data;
-} __packed __aligned(4);
-
-struct imx_sc_msg_misc_get_soc_uid {
-	struct imx_sc_rpc_msg hdr;
-	u32 uid_low;
-	u32 uid_high;
-} __packed;
-
-static int imx_scu_soc_uid(u64 *soc_uid) -{
-	struct imx_sc_msg_misc_get_soc_uid msg;
-	struct imx_sc_rpc_msg *hdr =3D &msg.hdr;
-	int ret;
-
-	hdr->ver =3D IMX_SC_RPC_VERSION;
-	hdr->svc =3D IMX_SC_RPC_SVC_MISC;
-	hdr->func =3D IMX_SC_MISC_FUNC_UNIQUE_ID;
-	hdr->size =3D 1;
-
-	ret =3D imx_scu_call_rpc(soc_ipc_handle, &msg, true);
-	if (ret) {
-		pr_err("%s: get soc uid failed, ret %d\n", __func__, ret);
-		return ret;
-	}
-
-	*soc_uid =3D msg.uid_high;
-	*soc_uid <<=3D 32;
-	*soc_uid |=3D msg.uid_low;
-
-	return 0;
-}
-
-static int imx_scu_soc_id(void)
-{
-	struct imx_sc_msg_misc_get_soc_id msg;
-	struct imx_sc_rpc_msg *hdr =3D &msg.hdr;
-	int ret;
-
-	hdr->ver =3D IMX_SC_RPC_VERSION;
-	hdr->svc =3D IMX_SC_RPC_SVC_MISC;
-	hdr->func =3D IMX_SC_MISC_FUNC_GET_CONTROL;
-	hdr->size =3D 3;
-
-	msg.data.req.control =3D IMX_SC_C_ID;
-	msg.data.req.resource =3D IMX_SC_R_SYSTEM;
-
-	ret =3D imx_scu_call_rpc(soc_ipc_handle, &msg, true);
-	if (ret) {
-		pr_err("%s: get soc info failed, ret %d\n", __func__, ret);
-		return ret;
-	}
-
-	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;
-	int id, ret;
-	u64 uid =3D 0;
-	u32 val;
-
-	ret =3D imx_scu_get_handle(&soc_ipc_handle);
-	if (ret)
-		return ret;
-
-	soc_dev_attr =3D devm_kzalloc(&pdev->dev, sizeof(*soc_dev_attr),
-				    GFP_KERNEL);
-	if (!soc_dev_attr)
-		return -ENOMEM;
-
-	soc_dev_attr->family =3D "Freescale i.MX";
-
-	ret =3D of_property_read_string(of_root,
-				      "model",
-				      &soc_dev_attr->machine);
-	if (ret)
-		return ret;
-
-	id =3D imx_scu_soc_id();
-	if (id < 0)
-		return -EINVAL;
-
-	ret =3D imx_scu_soc_uid(&uid);
-	if (ret < 0)
-		return -EINVAL;
-
-	/* format soc_id value passed from SCU firmware */
-	val =3D id & 0x1f;
-	soc_dev_attr->soc_id =3D devm_kasprintf(&pdev->dev, GFP_KERNEL, "0x%x", v=
al);
-	if (!soc_dev_attr->soc_id)
-		return -ENOMEM;
-
-	/* format revision value passed from SCU firmware */
-	val =3D (id >> 5) & 0xf;
-	val =3D (((val >> 2) + 1) << 4) | (val & 0x3);
-	soc_dev_attr->revision =3D devm_kasprintf(&pdev->dev, GFP_KERNEL, "%d.%d"=
,
-						(val >> 4) & 0xf, val & 0xf);
-	if (!soc_dev_attr->revision)
-		return -ENOMEM;
-
-	soc_dev_attr->serial_number =3D devm_kasprintf(&pdev->dev, GFP_KERNEL,
-						     "%016llX", uid);
-	if (!soc_dev_attr->serial_number)
-		return -ENOMEM;
-
-	soc_dev =3D soc_device_register(soc_dev_attr);
-	if (IS_ERR(soc_dev))
-		return PTR_ERR(soc_dev);
-
-	return 0;
-}
-
-static struct platform_driver imx_scu_soc_driver =3D {
-	.driver =3D {
-		.name =3D IMX_SCU_SOC_DRIVER_NAME,
-	},
-	.probe =3D imx_scu_soc_probe,
-};
-
-static int __init imx_scu_soc_init(void) -{
-	struct platform_device *pdev;
-	struct device_node *np;
-	int ret;
-
-	np =3D of_find_compatible_node(NULL, NULL, "fsl,imx-scu");
-	if (!np)
-		return -ENODEV;
-
-	of_node_put(np);
-
-	ret =3D platform_driver_register(&imx_scu_soc_driver);
-	if (ret)
-		return ret;
-
-	pdev =3D platform_device_register_simple(IMX_SCU_SOC_DRIVER_NAME,
-					       -1, NULL, 0);
-	if (IS_ERR(pdev))
-		platform_driver_unregister(&imx_scu_soc_driver);
-
-	return PTR_ERR_OR_ZERO(pdev);
-}
-device_initcall(imx_scu_soc_init);
--
2.7.4