The driver calls gpiod_get_optional() in the probe function but
never calls gpiod_put() in the remove function or in the probe
error path. This leads to a GPIO descriptor resource leak.
The lpc32xx_mlc.c driver in the same directory handles this
correctly by calling gpiod_put() on both paths.

Add gpiod_put() in the remove function and in the probe error path
to fix the resource leak.

Fixes: 6b923db2867c ("mtd: rawnand: lpc32xx_slc: switch to using gpiod API")
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
---
 drivers/mtd/nand/raw/lpc32xx_slc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/nand/raw/lpc32xx_slc.c b/drivers/mtd/nand/raw/lpc32xx_slc.c
index b54d76547ffb..fea3705a2138 100644
--- a/drivers/mtd/nand/raw/lpc32xx_slc.c
+++ b/drivers/mtd/nand/raw/lpc32xx_slc.c
@@ -937,6 +937,7 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	dma_release_channel(host->dma_chan);
 enable_wp:
 	lpc32xx_wp_enable(host);
+	gpiod_put(host->wp_gpio);
 
 	return res;
 }
@@ -962,6 +963,7 @@ static void lpc32xx_nand_remove(struct platform_device *pdev)
 	writel(tmp, SLC_CTRL(host->io_base));
 
 	lpc32xx_wp_enable(host);
+	gpiod_put(host->wp_gpio);
 }
 
 static int lpc32xx_nand_resume(struct platform_device *pdev)
-- 
2.50.1.windows.1

On 10/28/25 11:47, Haotian Zhang wrote:
> The driver calls gpiod_get_optional() in the probe function but
> never calls gpiod_put() in the remove function or in the probe
> error path. This leads to a GPIO descriptor resource leak.
> The lpc32xx_mlc.c driver in the same directory handles this
> correctly by calling gpiod_put() on both paths.
> 
> Add gpiod_put() in the remove function and in the probe error path
> to fix the resource leak.
> 
> Fixes: 6b923db2867c ("mtd: rawnand: lpc32xx_slc: switch to using gpiod API")
> Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>

The problem is well identified, thank you for the fix.

> ---
>   drivers/mtd/nand/raw/lpc32xx_slc.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/lpc32xx_slc.c b/drivers/mtd/nand/raw/lpc32xx_slc.c
> index b54d76547ffb..fea3705a2138 100644
> --- a/drivers/mtd/nand/raw/lpc32xx_slc.c
> +++ b/drivers/mtd/nand/raw/lpc32xx_slc.c
> @@ -937,6 +937,7 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
>   	dma_release_channel(host->dma_chan);
>   enable_wp:
>   	lpc32xx_wp_enable(host);
> +	gpiod_put(host->wp_gpio);
>   
>   	return res;
>   }
> @@ -962,6 +963,7 @@ static void lpc32xx_nand_remove(struct platform_device *pdev)
>   	writel(tmp, SLC_CTRL(host->io_base));
>   
>   	lpc32xx_wp_enable(host);
> +	gpiod_put(host->wp_gpio);
>   }
>   
>   static int lpc32xx_nand_resume(struct platform_device *pdev)

The fix can be greatly improved though, I kindly ask you to send v2 by switching
from gpiod_get_optional() to resource managed devm_gpiod_get_optional().

Thank you in advance.

-- 
Best wishes,
Vladimir

Hello,

On 03/11/2025 at 18:21:47 +02, Vladimir Zapolskiy <vz@mleia.com> wrote:

> On 10/28/25 11:47, Haotian Zhang wrote:
>> The driver calls gpiod_get_optional() in the probe function but
>> never calls gpiod_put() in the remove function or in the probe
>> error path. This leads to a GPIO descriptor resource leak.
>> The lpc32xx_mlc.c driver in the same directory handles this
>> correctly by calling gpiod_put() on both paths.
>> Add gpiod_put() in the remove function and in the probe error path
>> to fix the resource leak.
>> Fixes: 6b923db2867c ("mtd: rawnand: lpc32xx_slc: switch to using gpiod
>> API")
>> Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
>
> The problem is well identified, thank you for the fix.
>
>> ---
>>   drivers/mtd/nand/raw/lpc32xx_slc.c | 2 ++
>>   1 file changed, 2 insertions(+)
>> diff --git a/drivers/mtd/nand/raw/lpc32xx_slc.c
>> b/drivers/mtd/nand/raw/lpc32xx_slc.c
>> index b54d76547ffb..fea3705a2138 100644
>> --- a/drivers/mtd/nand/raw/lpc32xx_slc.c
>> +++ b/drivers/mtd/nand/raw/lpc32xx_slc.c
>> @@ -937,6 +937,7 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
>>   	dma_release_channel(host->dma_chan);
>>   enable_wp:
>>   	lpc32xx_wp_enable(host);
>> +	gpiod_put(host->wp_gpio);
>>     	return res;
>>   }
>> @@ -962,6 +963,7 @@ static void lpc32xx_nand_remove(struct platform_device *pdev)
>>   	writel(tmp, SLC_CTRL(host->io_base));
>>     	lpc32xx_wp_enable(host);
>> +	gpiod_put(host->wp_gpio);
>>   }
>>     static int lpc32xx_nand_resume(struct platform_device *pdev)
>
> The fix can be greatly improved though, I kindly ask you to send v2 by switching
> from gpiod_get_optional() to resource managed
> devm_gpiod_get_optional().

I've already applied this fix, however I will welcome a follow-up patch.

Thanks,
Miquèl

On Tue, 28 Oct 2025 17:47:47 +0800, Haotian Zhang wrote:
> The driver calls gpiod_get_optional() in the probe function but
> never calls gpiod_put() in the remove function or in the probe
> error path. This leads to a GPIO descriptor resource leak.
> The lpc32xx_mlc.c driver in the same directory handles this
> correctly by calling gpiod_put() on both paths.
> 
> Add gpiod_put() in the remove function and in the probe error path
> to fix the resource leak.
> 
> [...]

Applied to nand/next, thanks!

[1/1] mtd: rawnand: lpc32xx_slc: fix GPIO descriptor leak on probe error and remove
      commit: 3ae23f77b6a0cd53b557ac69c98312f7be22f081

Patche(s) should be available on mtd/linux.git and will be
part of the next PR (provided that no robot complains by then).

Kind regards,
Miquèl

The driver calls gpiod_get_optional() in the probe function but
never calls gpiod_put() in the remove function or in the probe
error path. This leads to a GPIO descriptor resource leak.
The lpc32xx_mlc.c driver in the same directory handles this
correctly by calling gpiod_put() on both paths.

Switch to devm_gpiod_get_optional() to automatically manage
GPIO descriptor resource.

Fixes: 6b923db2867c ("mtd: rawnand: lpc32xx_slc: switch to using gpiod API")
Suggested by: Vladimir Zapolskiy <vz@mleia.com>
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>

---
Changes in v2:
 -Switch to devm_gpiod_get_optional()
---
 drivers/mtd/nand/raw/lpc32xx_slc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/lpc32xx_slc.c b/drivers/mtd/nand/raw/lpc32xx_slc.c
index b54d76547ffb..3ca30e7dce33 100644
--- a/drivers/mtd/nand/raw/lpc32xx_slc.c
+++ b/drivers/mtd/nand/raw/lpc32xx_slc.c
@@ -854,7 +854,7 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
 	}
 
 	/* Start with WP disabled, if available */
-	host->wp_gpio = gpiod_get_optional(&pdev->dev, NULL, GPIOD_OUT_LOW);
+	host->wp_gpio = devm_gpiod_get_optional(&pdev->dev, NULL, GPIOD_OUT_LOW);
 	res = PTR_ERR_OR_ZERO(host->wp_gpio);
 	if (res) {
 		if (res != -EPROBE_DEFER)
-- 
2.50.1.windows.1

> The driver calls gpiod_get_optional() in the probe function but
> never calls gpiod_put() in the remove function or in the probe
…

You may occasionally put more than 63 characters into text lines
of such a change description.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc4#n658

Regards,
Markus

Hi,

> diff --git a/drivers/mtd/nand/raw/lpc32xx_slc.c b/drivers/mtd/nand/raw/lpc32xx_slc.c
> index b54d76547ffb..3ca30e7dce33 100644
> --- a/drivers/mtd/nand/raw/lpc32xx_slc.c
> +++ b/drivers/mtd/nand/raw/lpc32xx_slc.c
> @@ -854,7 +854,7 @@ static int lpc32xx_nand_probe(struct platform_device *pdev)
>  	}
>  
>  	/* Start with WP disabled, if available */
> -	host->wp_gpio = gpiod_get_optional(&pdev->dev, NULL, GPIOD_OUT_LOW);
> +	host->wp_gpio = devm_gpiod_get_optional(&pdev->dev, NULL,
>  	GPIOD_OUT_LOW);

Please send an update on top the previous patch which I already applied.

Thanks,
Miquèl