[PATCH V3] soc: imx8m: Unregister cpufreq and soc dev in cleanup path

Peng Fan (OSS) posted 1 patch 12 months ago
There is a newer version of this series
drivers/soc/imx/soc-imx8m.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
[PATCH V3] soc: imx8m: Unregister cpufreq and soc dev in cleanup path
Posted by Peng Fan (OSS) 12 months ago
From: Peng Fan <peng.fan@nxp.com>

Unregister the cpufreq device and soc device when resource unwinding,
otherwise there will be warning when do removing test:
sysfs: cannot create duplicate filename '/devices/platform/imx-cpufreq-dt'
CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.13.0-rc1-next-20241204
Hardware name: NXP i.MX8MPlus EVK board (DT)

Fixes: 9cc832d37799 ("soc: imx8m: Probe the SoC driver as platform driver")
Cc: Marco Felsch <m.felsch@pengutronix.de>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---

V3:
 Per Marco, drop remove function, use devm_add_action and update the patch
 title accordingly.

V2:
 Add err check when create the cpufreq platform device
 https://lore.kernel.org/all/20241217015826.1374497-1-peng.fan@oss.nxp.com/

 drivers/soc/imx/soc-imx8m.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
index 8ac7658e3d52..585631b7ae44 100644
--- a/drivers/soc/imx/soc-imx8m.c
+++ b/drivers/soc/imx/soc-imx8m.c
@@ -192,9 +192,20 @@ static __maybe_unused const struct of_device_id imx8_soc_match[] = {
 	devm_kasprintf((dev), GFP_KERNEL, "%d.%d", ((soc_rev) >> 4) & 0xf, (soc_rev) & 0xf) : \
 	"unknown"
 
+static void imx8m_unregister_soc(void *data)
+{
+	soc_device_unregister(data);
+}
+
+static void imx8m_unregister_cpufreq(void *data)
+{
+	platform_device_unregister(data);
+}
+
 static int imx8m_soc_probe(struct platform_device *pdev)
 {
 	struct soc_device_attribute *soc_dev_attr;
+	struct platform_device *cpufreq_dev;
 	const struct imx8_soc_data *data;
 	struct device *dev = &pdev->dev;
 	const struct of_device_id *id;
@@ -239,11 +250,19 @@ static int imx8m_soc_probe(struct platform_device *pdev)
 	if (IS_ERR(soc_dev))
 		return PTR_ERR(soc_dev);
 
+	ret = devm_add_action(dev, imx8m_unregister_soc, soc_dev);
+	if (ret)
+		return ret;
+
 	pr_info("SoC: %s revision %s\n", soc_dev_attr->soc_id,
 		soc_dev_attr->revision);
 
-	if (IS_ENABLED(CONFIG_ARM_IMX_CPUFREQ_DT))
-		platform_device_register_simple("imx-cpufreq-dt", -1, NULL, 0);
+	if (IS_ENABLED(CONFIG_ARM_IMX_CPUFREQ_DT)) {
+		cpufreq_dev = platform_device_register_simple("imx-cpufreq-dt", -1, NULL, 0);
+		ret = devm_add_action(dev, imx8m_unregister_cpufreq, cpufreq_dev);
+		if (ret)
+			return ret;
+	}
 
 	return 0;
 }
-- 
2.37.1
Re: [PATCH V3] soc: imx8m: Unregister cpufreq and soc dev in cleanup path
Posted by Peng Fan 10 months, 1 week ago
Hi Shawn,

On Thu, Dec 19, 2024 at 10:50:29PM +0800, Peng Fan (OSS) wrote:
>From: Peng Fan <peng.fan@nxp.com>
>
>Unregister the cpufreq device and soc device when resource unwinding,
>otherwise there will be warning when do removing test:
>sysfs: cannot create duplicate filename '/devices/platform/imx-cpufreq-dt'
>CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.13.0-rc1-next-20241204
>Hardware name: NXP i.MX8MPlus EVK board (DT)
>
>Fixes: 9cc832d37799 ("soc: imx8m: Probe the SoC driver as platform driver")
>Cc: Marco Felsch <m.felsch@pengutronix.de>
>Signed-off-by: Peng Fan <peng.fan@nxp.com>
>---

Any comments?

Thanks,
Peng

>
>V3:
> Per Marco, drop remove function, use devm_add_action and update the patch
> title accordingly.
>
>V2:
> Add err check when create the cpufreq platform device
> https://lore.kernel.org/all/20241217015826.1374497-1-peng.fan@oss.nxp.com/
>
> drivers/soc/imx/soc-imx8m.c | 23 +++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
>index 8ac7658e3d52..585631b7ae44 100644
>--- a/drivers/soc/imx/soc-imx8m.c
>+++ b/drivers/soc/imx/soc-imx8m.c
>@@ -192,9 +192,20 @@ static __maybe_unused const struct of_device_id imx8_soc_match[] = {
> 	devm_kasprintf((dev), GFP_KERNEL, "%d.%d", ((soc_rev) >> 4) & 0xf, (soc_rev) & 0xf) : \
> 	"unknown"
> 
>+static void imx8m_unregister_soc(void *data)
>+{
>+	soc_device_unregister(data);
>+}
>+
>+static void imx8m_unregister_cpufreq(void *data)
>+{
>+	platform_device_unregister(data);
>+}
>+
> static int imx8m_soc_probe(struct platform_device *pdev)
> {
> 	struct soc_device_attribute *soc_dev_attr;
>+	struct platform_device *cpufreq_dev;
> 	const struct imx8_soc_data *data;
> 	struct device *dev = &pdev->dev;
> 	const struct of_device_id *id;
>@@ -239,11 +250,19 @@ static int imx8m_soc_probe(struct platform_device *pdev)
> 	if (IS_ERR(soc_dev))
> 		return PTR_ERR(soc_dev);
> 
>+	ret = devm_add_action(dev, imx8m_unregister_soc, soc_dev);
>+	if (ret)
>+		return ret;
>+
> 	pr_info("SoC: %s revision %s\n", soc_dev_attr->soc_id,
> 		soc_dev_attr->revision);
> 
>-	if (IS_ENABLED(CONFIG_ARM_IMX_CPUFREQ_DT))
>-		platform_device_register_simple("imx-cpufreq-dt", -1, NULL, 0);
>+	if (IS_ENABLED(CONFIG_ARM_IMX_CPUFREQ_DT)) {
>+		cpufreq_dev = platform_device_register_simple("imx-cpufreq-dt", -1, NULL, 0);
>+		ret = devm_add_action(dev, imx8m_unregister_cpufreq, cpufreq_dev);
>+		if (ret)
>+			return ret;
>+	}
> 
> 	return 0;
> }
>-- 
>2.37.1
>
>
Re: [PATCH V3] soc: imx8m: Unregister cpufreq and soc dev in cleanup path
Posted by Marco Felsch 10 months, 1 week ago
Hi Peng,

On 25-02-14, Peng Fan wrote:
> Hi Shawn,
> 
> On Thu, Dec 19, 2024 at 10:50:29PM +0800, Peng Fan (OSS) wrote:
> >From: Peng Fan <peng.fan@nxp.com>
> >
> >Unregister the cpufreq device and soc device when resource unwinding,
> >otherwise there will be warning when do removing test:
> >sysfs: cannot create duplicate filename '/devices/platform/imx-cpufreq-dt'
> >CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.13.0-rc1-next-20241204
> >Hardware name: NXP i.MX8MPlus EVK board (DT)
> >
> >Fixes: 9cc832d37799 ("soc: imx8m: Probe the SoC driver as platform driver")
> >Cc: Marco Felsch <m.felsch@pengutronix.de>
> >Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >---
> 
> Any comments?
> 
> Thanks,
> Peng
> 
> >
> >V3:
> > Per Marco, drop remove function, use devm_add_action and update the patch
> > title accordingly.
> >
> >V2:
> > Add err check when create the cpufreq platform device
> > https://lore.kernel.org/all/20241217015826.1374497-1-peng.fan@oss.nxp.com/

This is got lost. Albeit platform_device_unregister() can handle
IS_ERROR() cases I don't like the fact that we do not return the error
if there is any during probe. Please see below.

> >
> > drivers/soc/imx/soc-imx8m.c | 23 +++++++++++++++++++++--
> > 1 file changed, 21 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
> >index 8ac7658e3d52..585631b7ae44 100644
> >--- a/drivers/soc/imx/soc-imx8m.c
> >+++ b/drivers/soc/imx/soc-imx8m.c
> >@@ -192,9 +192,20 @@ static __maybe_unused const struct of_device_id imx8_soc_match[] = {
> > 	devm_kasprintf((dev), GFP_KERNEL, "%d.%d", ((soc_rev) >> 4) & 0xf, (soc_rev) & 0xf) : \
> > 	"unknown"
> > 
> >+static void imx8m_unregister_soc(void *data)
> >+{
> >+	soc_device_unregister(data);
> >+}
> >+
> >+static void imx8m_unregister_cpufreq(void *data)
> >+{
> >+	platform_device_unregister(data);
> >+}
> >+
> > static int imx8m_soc_probe(struct platform_device *pdev)
> > {
> > 	struct soc_device_attribute *soc_dev_attr;
> >+	struct platform_device *cpufreq_dev;
> > 	const struct imx8_soc_data *data;
> > 	struct device *dev = &pdev->dev;
> > 	const struct of_device_id *id;
> >@@ -239,11 +250,19 @@ static int imx8m_soc_probe(struct platform_device *pdev)
> > 	if (IS_ERR(soc_dev))
> > 		return PTR_ERR(soc_dev);
> > 
> >+	ret = devm_add_action(dev, imx8m_unregister_soc, soc_dev);
> >+	if (ret)
> >+		return ret;
> >+
> > 	pr_info("SoC: %s revision %s\n", soc_dev_attr->soc_id,
> > 		soc_dev_attr->revision);
> > 
> >-	if (IS_ENABLED(CONFIG_ARM_IMX_CPUFREQ_DT))
> >-		platform_device_register_simple("imx-cpufreq-dt", -1, NULL, 0);
> >+	if (IS_ENABLED(CONFIG_ARM_IMX_CPUFREQ_DT)) {
> >+		cpufreq_dev = platform_device_register_simple("imx-cpufreq-dt", -1, NULL, 0);
		if (IS_ERROR(cpufreq_dev))
			return dev_err_probe(dev, "Failed to imx-cpufreq-dev device\n",
					     PTR_ERR(cpufreq_dev));

Regards,
  Marco

> >+		ret = devm_add_action(dev, imx8m_unregister_cpufreq, cpufreq_dev);
> >+		if (ret)
> >+			return ret;
> >+	}
> > 
> > 	return 0;
> > }
> >-- 
> >2.37.1
> >
> >
>