netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: wan: framer: pef2256: Fix missing mfd_remove_devices() call
@ 2025-11-03 11:18 Haotian Zhang
  2025-11-03 11:43 ` Herve Codina
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Haotian Zhang @ 2025-11-03 11:18 UTC (permalink / raw)
  To: Herve Codina, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: netdev, linux-kernel, Haotian Zhang

The driver calls mfd_add_devices() but fails to call mfd_remove_devices()
in error paths after successful MFD device registration and in the remove
function. This leads to resource leaks where MFD child devices are not
properly unregistered.

Add mfd_remove_devices() call in the error path after mfd_add_devices()
succeeds, and add the missing mfd_remove_devices() call in pef2256_remove()
to properly clean up MFD devices.

Fixes: c96e976d9a05 ("net: wan: framer: Add support for the Lantiq PEF2256 framer")
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
---
 drivers/net/wan/framer/pef2256/pef2256.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wan/framer/pef2256/pef2256.c b/drivers/net/wan/framer/pef2256/pef2256.c
index 1e4c8e85d598..d43fbf9bb27d 100644
--- a/drivers/net/wan/framer/pef2256/pef2256.c
+++ b/drivers/net/wan/framer/pef2256/pef2256.c
@@ -821,27 +821,34 @@ static int pef2256_probe(struct platform_device *pdev)
 
 	ret = pef2256_setup_e1(pef2256);
 	if (ret)
-		return ret;
+		goto err_mfd_remove;
 
 	framer_provider = devm_framer_provider_of_register(pef2256->dev,
 							   framer_provider_simple_of_xlate);
-	if (IS_ERR(framer_provider))
-		return PTR_ERR(framer_provider);
+	if (IS_ERR(framer_provider)) {
+		ret = PTR_ERR(framer_provider);
+		goto err_mfd_remove;
+	}
 
 	/* Add audio devices */
 	ret = pef2256_add_audio_devices(pef2256);
 	if (ret < 0) {
 		dev_err(pef2256->dev, "add audio devices failed (%d)\n", ret);
-		return ret;
+		goto err_mfd_remove;
 	}
 
 	return 0;
+
+err_mfd_remove:
+	mfd_remove_devices(pef2256->dev);
+	return ret;
 }
 
 static void pef2256_remove(struct platform_device *pdev)
 {
 	struct pef2256 *pef2256 = platform_get_drvdata(pdev);
 
+	mfd_remove_devices(pef2256->dev);
 	/* Disable interrupts */
 	pef2256_write8(pef2256, PEF2256_IMR0, 0xff);
 	pef2256_write8(pef2256, PEF2256_IMR1, 0xff);
-- 
2.50.1.windows.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] net: wan: framer: pef2256: Fix missing mfd_remove_devices() call
  2025-11-03 11:18 [PATCH] net: wan: framer: pef2256: Fix missing mfd_remove_devices() call Haotian Zhang
@ 2025-11-03 11:43 ` Herve Codina
  2025-11-03 12:37 ` [PATCH v2] net: wan: framer: pef2256: Switch to devm_mfd_add_devices() Haotian Zhang
  2025-11-05  3:47 ` [PATCH v3] " Haotian Zhang
  2 siblings, 0 replies; 8+ messages in thread
From: Herve Codina @ 2025-11-03 11:43 UTC (permalink / raw)
  To: Haotian Zhang
  Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-kernel

Hi Haotian,

On Mon,  3 Nov 2025 19:18:44 +0800
Haotian Zhang <vulab@iscas.ac.cn> wrote:

> The driver calls mfd_add_devices() but fails to call mfd_remove_devices()
> in error paths after successful MFD device registration and in the remove
> function. This leads to resource leaks where MFD child devices are not
> properly unregistered.
> 
> Add mfd_remove_devices() call in the error path after mfd_add_devices()
> succeeds, and add the missing mfd_remove_devices() call in pef2256_remove()
> to properly clean up MFD devices.
> 
> Fixes: c96e976d9a05 ("net: wan: framer: Add support for the Lantiq PEF2256 framer")
> Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
> ---
>  drivers/net/wan/framer/pef2256/pef2256.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wan/framer/pef2256/pef2256.c b/drivers/net/wan/framer/pef2256/pef2256.c
> index 1e4c8e85d598..d43fbf9bb27d 100644
> --- a/drivers/net/wan/framer/pef2256/pef2256.c
> +++ b/drivers/net/wan/framer/pef2256/pef2256.c
> @@ -821,27 +821,34 @@ static int pef2256_probe(struct platform_device *pdev)
>  
>  	ret = pef2256_setup_e1(pef2256);
>  	if (ret)
> -		return ret;
> +		goto err_mfd_remove;
>  
>  	framer_provider = devm_framer_provider_of_register(pef2256->dev,
>  							   framer_provider_simple_of_xlate);
> -	if (IS_ERR(framer_provider))
> -		return PTR_ERR(framer_provider);
> +	if (IS_ERR(framer_provider)) {
> +		ret = PTR_ERR(framer_provider);
> +		goto err_mfd_remove;
> +	}
>  
>  	/* Add audio devices */
>  	ret = pef2256_add_audio_devices(pef2256);
>  	if (ret < 0) {
>  		dev_err(pef2256->dev, "add audio devices failed (%d)\n", ret);
> -		return ret;
> +		goto err_mfd_remove;
>  	}
>  
>  	return 0;
> +
> +err_mfd_remove:
> +	mfd_remove_devices(pef2256->dev);
> +	return ret;
>  }
>  
>  static void pef2256_remove(struct platform_device *pdev)
>  {
>  	struct pef2256 *pef2256 = platform_get_drvdata(pdev);
>  
> +	mfd_remove_devices(pef2256->dev);
>  	/* Disable interrupts */
>  	pef2256_write8(pef2256, PEF2256_IMR0, 0xff);
>  	pef2256_write8(pef2256, PEF2256_IMR1, 0xff);

Thanks for the patch.

Instead of calling mfd_remove_devices() in the error path and the remove()
function, can you replace the mfd_add_devices() call by the device managed
variant devm_mfd_add_devices()?

The device managed variant will handle the removal.

Best regards,
Hervé

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] net: wan: framer: pef2256: Switch to devm_mfd_add_devices()
  2025-11-03 11:18 [PATCH] net: wan: framer: pef2256: Fix missing mfd_remove_devices() call Haotian Zhang
  2025-11-03 11:43 ` Herve Codina
@ 2025-11-03 12:37 ` Haotian Zhang
  2025-11-03 12:39   ` Herve Codina
  2025-11-05  1:49   ` Jakub Kicinski
  2025-11-05  3:47 ` [PATCH v3] " Haotian Zhang
  2 siblings, 2 replies; 8+ messages in thread
From: Haotian Zhang @ 2025-11-03 12:37 UTC (permalink / raw)
  To: herve.codina, davem, edumazet, kuba, pabeni, thomas.petazzoni
  Cc: netdev, linux-kernel, Haotian Zhang

The driver calls mfd_add_devices() but fails to call mfd_remove_devices()
in error paths after successful MFD device registration and in the remove
function. This leads to resource leaks where MFD child devices are not
properly unregistered.

Replace mfd_add_devices with devm_mfd_add_devices to automatically
manage the device resources.

Fixes: c96e976d9a05 ("net: wan: framer: Add support for the Lantiq PEF2256 framer")
Suggested-by: Herve Codina<herve.codina@bootlin.com>
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
---
Changes in v2:
  - Use devm_mfd_add_devices() instead of manual cleanup
---
 drivers/net/wan/framer/pef2256/pef2256.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wan/framer/pef2256/pef2256.c b/drivers/net/wan/framer/pef2256/pef2256.c
index 1e4c8e85d598..4f4433560964 100644
--- a/drivers/net/wan/framer/pef2256/pef2256.c
+++ b/drivers/net/wan/framer/pef2256/pef2256.c
@@ -812,7 +812,7 @@ static int pef2256_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, pef2256);
 
-	ret = mfd_add_devices(pef2256->dev, 0, pef2256_devs,
+	ret = devm_mfd_add_devices(pef2256->dev, 0, pef2256_devs,
 			      ARRAY_SIZE(pef2256_devs), NULL, 0, NULL);
 	if (ret) {
 		dev_err(pef2256->dev, "add devices failed (%d)\n", ret);
-- 
2.50.1.windows.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] net: wan: framer: pef2256: Switch to devm_mfd_add_devices()
  2025-11-03 12:37 ` [PATCH v2] net: wan: framer: pef2256: Switch to devm_mfd_add_devices() Haotian Zhang
@ 2025-11-03 12:39   ` Herve Codina
  2025-11-05  1:49   ` Jakub Kicinski
  1 sibling, 0 replies; 8+ messages in thread
From: Herve Codina @ 2025-11-03 12:39 UTC (permalink / raw)
  To: Haotian Zhang
  Cc: davem, edumazet, kuba, pabeni, thomas.petazzoni, netdev,
	linux-kernel

Hi Haotian,

On Mon,  3 Nov 2025 20:37:41 +0800
Haotian Zhang <vulab@iscas.ac.cn> wrote:

> The driver calls mfd_add_devices() but fails to call mfd_remove_devices()
> in error paths after successful MFD device registration and in the remove
> function. This leads to resource leaks where MFD child devices are not
> properly unregistered.
> 
> Replace mfd_add_devices with devm_mfd_add_devices to automatically
> manage the device resources.
> 
> Fixes: c96e976d9a05 ("net: wan: framer: Add support for the Lantiq PEF2256 framer")
> Suggested-by: Herve Codina<herve.codina@bootlin.com>
> Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
> ---
> Changes in v2:
>   - Use devm_mfd_add_devices() instead of manual cleanup
> ---
>  drivers/net/wan/framer/pef2256/pef2256.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wan/framer/pef2256/pef2256.c b/drivers/net/wan/framer/pef2256/pef2256.c
> index 1e4c8e85d598..4f4433560964 100644
> --- a/drivers/net/wan/framer/pef2256/pef2256.c
> +++ b/drivers/net/wan/framer/pef2256/pef2256.c
> @@ -812,7 +812,7 @@ static int pef2256_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, pef2256);
>  
> -	ret = mfd_add_devices(pef2256->dev, 0, pef2256_devs,
> +	ret = devm_mfd_add_devices(pef2256->dev, 0, pef2256_devs,
>  			      ARRAY_SIZE(pef2256_devs), NULL, 0, NULL);
>  	if (ret) {
>  		dev_err(pef2256->dev, "add devices failed (%d)\n", ret);

Thanks for your update.

Acked-by: Herve Codina <herve.codina@bootlin.com>

Best regards,
Hervé

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] net: wan: framer: pef2256: Switch to devm_mfd_add_devices()
  2025-11-03 12:37 ` [PATCH v2] net: wan: framer: pef2256: Switch to devm_mfd_add_devices() Haotian Zhang
  2025-11-03 12:39   ` Herve Codina
@ 2025-11-05  1:49   ` Jakub Kicinski
  1 sibling, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2025-11-05  1:49 UTC (permalink / raw)
  To: Haotian Zhang
  Cc: herve.codina, davem, edumazet, pabeni, thomas.petazzoni, netdev,
	linux-kernel

On Mon,  3 Nov 2025 20:37:41 +0800 Haotian Zhang wrote:
> The driver calls mfd_add_devices() but fails to call mfd_remove_devices()
> in error paths after successful MFD device registration and in the remove
> function. This leads to resource leaks where MFD child devices are not
> properly unregistered.
> 
> Replace mfd_add_devices with devm_mfd_add_devices to automatically
> manage the device resources.
> 
> Fixes: c96e976d9a05 ("net: wan: framer: Add support for the Lantiq PEF2256 framer")

This commit has another call to mfd_add_devices()
please add an explanation of why that one is fine as is.

> Suggested-by: Herve Codina<herve.codina@bootlin.com>

missing space between name and address

> Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
> ---
> Changes in v2:
>   - Use devm_mfd_add_devices() instead of manual cleanup
> ---
>  drivers/net/wan/framer/pef2256/pef2256.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wan/framer/pef2256/pef2256.c b/drivers/net/wan/framer/pef2256/pef2256.c
> index 1e4c8e85d598..4f4433560964 100644
> --- a/drivers/net/wan/framer/pef2256/pef2256.c
> +++ b/drivers/net/wan/framer/pef2256/pef2256.c
> @@ -812,7 +812,7 @@ static int pef2256_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, pef2256);
>  
> -	ret = mfd_add_devices(pef2256->dev, 0, pef2256_devs,
> +	ret = devm_mfd_add_devices(pef2256->dev, 0, pef2256_devs,
>  			      ARRAY_SIZE(pef2256_devs), NULL, 0, NULL);

please adjust the continuation line so that it aligns with the opening
bracket
-- 
pw-bot: cr

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3] net: wan: framer: pef2256: Switch to devm_mfd_add_devices()
  2025-11-03 11:18 [PATCH] net: wan: framer: pef2256: Fix missing mfd_remove_devices() call Haotian Zhang
  2025-11-03 11:43 ` Herve Codina
  2025-11-03 12:37 ` [PATCH v2] net: wan: framer: pef2256: Switch to devm_mfd_add_devices() Haotian Zhang
@ 2025-11-05  3:47 ` Haotian Zhang
  2025-11-05  7:34   ` Herve Codina
  2025-11-06  2:10   ` patchwork-bot+netdevbpf
  2 siblings, 2 replies; 8+ messages in thread
From: Haotian Zhang @ 2025-11-05  3:47 UTC (permalink / raw)
  To: Herve Codina, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: netdev, linux-kernel, Haotian Zhang

The driver calls mfd_add_devices() but fails to call mfd_remove_devices()
in error paths after successful MFD device registration and in the remove
function. This leads to resource leaks where MFD child devices are not
properly unregistered.

Replace mfd_add_devices with devm_mfd_add_devices to automatically
manage the device resources.

Fixes: c96e976d9a05 ("net: wan: framer: Add support for the Lantiq PEF2256 framer")
Suggested-by: Herve Codina <herve.codina@bootlin.com>
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
---
Changes in v3:
  - Also replace mfd_add_devices() with devm_mfd_add_devices()
    in pef2256_add_audio_devices(), which was overlooked by
    previous patch.
Changes in v2:
  - Use devm_mfd_add_devices() instead of manual cleanup.
---
---
 drivers/net/wan/framer/pef2256/pef2256.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wan/framer/pef2256/pef2256.c b/drivers/net/wan/framer/pef2256/pef2256.c
index 1e4c8e85d598..180edde177c6 100644
--- a/drivers/net/wan/framer/pef2256/pef2256.c
+++ b/drivers/net/wan/framer/pef2256/pef2256.c
@@ -637,7 +637,7 @@ static int pef2256_add_audio_devices(struct pef2256 *pef2256)
 		audio_devs[i].id = i;
 	}
 
-	ret = mfd_add_devices(pef2256->dev, 0, audio_devs, count, NULL, 0, NULL);
+	ret = devm_mfd_add_devices(pef2256->dev, 0, audio_devs, count, NULL, 0, NULL);
 	kfree(audio_devs);
 	return ret;
 }
@@ -812,8 +812,8 @@ static int pef2256_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, pef2256);
 
-	ret = mfd_add_devices(pef2256->dev, 0, pef2256_devs,
-			      ARRAY_SIZE(pef2256_devs), NULL, 0, NULL);
+	ret = devm_mfd_add_devices(pef2256->dev, 0, pef2256_devs,
+				   ARRAY_SIZE(pef2256_devs), NULL, 0, NULL);
 	if (ret) {
 		dev_err(pef2256->dev, "add devices failed (%d)\n", ret);
 		return ret;
-- 
2.50.1.windows.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] net: wan: framer: pef2256: Switch to devm_mfd_add_devices()
  2025-11-05  3:47 ` [PATCH v3] " Haotian Zhang
@ 2025-11-05  7:34   ` Herve Codina
  2025-11-06  2:10   ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 8+ messages in thread
From: Herve Codina @ 2025-11-05  7:34 UTC (permalink / raw)
  To: Haotian Zhang
  Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-kernel

Hi Haotian,

On Wed,  5 Nov 2025 11:47:16 +0800
Haotian Zhang <vulab@iscas.ac.cn> wrote:

> The driver calls mfd_add_devices() but fails to call mfd_remove_devices()
> in error paths after successful MFD device registration and in the remove
> function. This leads to resource leaks where MFD child devices are not
> properly unregistered.
> 
> Replace mfd_add_devices with devm_mfd_add_devices to automatically
> manage the device resources.
> 
> Fixes: c96e976d9a05 ("net: wan: framer: Add support for the Lantiq PEF2256 framer")
> Suggested-by: Herve Codina <herve.codina@bootlin.com>
> Suggested-by: Jakub Kicinski <kuba@kernel.org>
> Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>

Not sure that 'Suggested-by: Jakub Kicinski' was needed according to his
comments on v2. Also, you forgot my 'Acked-by' I sent on v2.

Anyway, the v3 is ok on my side

Acked-by: Herve Codina <herve.codina@bootlin.com>

Best regards,
Hervé

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] net: wan: framer: pef2256: Switch to devm_mfd_add_devices()
  2025-11-05  3:47 ` [PATCH v3] " Haotian Zhang
  2025-11-05  7:34   ` Herve Codina
@ 2025-11-06  2:10   ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-11-06  2:10 UTC (permalink / raw)
  To: Haotian Zhang
  Cc: herve.codina, andrew+netdev, davem, edumazet, kuba, pabeni,
	netdev, linux-kernel

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed,  5 Nov 2025 11:47:16 +0800 you wrote:
> The driver calls mfd_add_devices() but fails to call mfd_remove_devices()
> in error paths after successful MFD device registration and in the remove
> function. This leads to resource leaks where MFD child devices are not
> properly unregistered.
> 
> Replace mfd_add_devices with devm_mfd_add_devices to automatically
> manage the device resources.
> 
> [...]

Here is the summary with links:
  - [v3] net: wan: framer: pef2256: Switch to devm_mfd_add_devices()
    https://git.kernel.org/netdev/net/c/4d6ec3a7932c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-11-06  2:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-03 11:18 [PATCH] net: wan: framer: pef2256: Fix missing mfd_remove_devices() call Haotian Zhang
2025-11-03 11:43 ` Herve Codina
2025-11-03 12:37 ` [PATCH v2] net: wan: framer: pef2256: Switch to devm_mfd_add_devices() Haotian Zhang
2025-11-03 12:39   ` Herve Codina
2025-11-05  1:49   ` Jakub Kicinski
2025-11-05  3:47 ` [PATCH v3] " Haotian Zhang
2025-11-05  7:34   ` Herve Codina
2025-11-06  2:10   ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).