Re: [PATCH] soc: imx8m: Add remove function
From: Marco Felsch
Date: Fri Dec 06 2024 - 06:41:42 EST
On 24-12-06, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@xxxxxxx>
>
> Unregister the cpufreq device and soc device in remove path, 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")
> Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> ---
> drivers/soc/imx/soc-imx8m.c | 32 +++++++++++++++++++++++++++-----
> 1 file changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
> index 8ac7658e3d52..8c368947d1e5 100644
> --- a/drivers/soc/imx/soc-imx8m.c
> +++ b/drivers/soc/imx/soc-imx8m.c
> @@ -33,6 +33,11 @@ struct imx8_soc_data {
> int (*soc_revision)(u32 *socrev, u64 *socuid);
> };
>
> +struct imx8m_soc_priv {
> + struct soc_device *soc_dev;
> + struct platform_device *cpufreq_dev;
> +};
> +
> #ifdef CONFIG_HAVE_ARM_SMCCC
> static u32 imx8mq_soc_revision_from_atf(void)
> {
> @@ -198,7 +203,7 @@ static int imx8m_soc_probe(struct platform_device *pdev)
> const struct imx8_soc_data *data;
> struct device *dev = &pdev->dev;
> const struct of_device_id *id;
> - struct soc_device *soc_dev;
> + struct imx8m_soc_priv *priv;
> u32 soc_rev = 0;
> u64 soc_uid = 0;
> int ret;
> @@ -207,6 +212,10 @@ static int imx8m_soc_probe(struct platform_device *pdev)
> if (!soc_dev_attr)
> return -ENOMEM;
>
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> soc_dev_attr->family = "Freescale i.MX";
>
> ret = of_property_read_string(of_root, "model", &soc_dev_attr->machine);
> @@ -235,21 +244,34 @@ static int imx8m_soc_probe(struct platform_device *pdev)
> if (!soc_dev_attr->serial_number)
> return -ENOMEM;
>
> - soc_dev = soc_device_register(soc_dev_attr);
> - if (IS_ERR(soc_dev))
> - return PTR_ERR(soc_dev);
> + priv->soc_dev = soc_device_register(soc_dev_attr);
> + if (IS_ERR(priv->soc_dev))
> + return PTR_ERR(priv->soc_dev);
>
> 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);
> + priv->cpufreq_dev = platform_device_register_simple("imx-cpufreq-dt", -1, NULL, 0);
If CONFIG_ARM_IMX_CPUFREQ_DT is enabled, I asusme that
platform_device_register_simple() shouldn't fail else it will be an
error, right? Therefore I would like to add the 'if(!IS_ERR())' check
here instead of the remove function.
> +
> + platform_set_drvdata(pdev, priv);
>
> return 0;
> }
>
> +static void imx8m_soc_remove(struct platform_device *pdev)
> +{
> + struct imx8m_soc_priv *priv = platform_get_drvdata(pdev);
> +
> + if (!IS_ERR(priv->cpufreq_dev))
With the above shifted, we only need to:
if (priv->cpufreq_dev)
Regards,
Marco
> + platform_device_unregister(priv->cpufreq_dev);
> +
> + soc_device_unregister(priv->soc_dev);
> +}
> +
> static struct platform_driver imx8m_soc_driver = {
> .probe = imx8m_soc_probe,
> + .remove = imx8m_soc_remove,
> .driver = {
> .name = "imx8m-soc",
> },
> --
> 2.37.1
>
>
>