mbox series

[0/3] spi: fsl-qspi: Fix double cleanup in probe error path

Message ID 20250410-spi-v1-0-56e867cc19cf@gmail.com
Headers show
Series spi: fsl-qspi: Fix double cleanup in probe error path | expand

Message

Kevin Hao April 10, 2025, 6:56 a.m. UTC
This patch series fixes double cleanup issues in the fsl-qspi probe
error path and also simplifies the probe error handling using managed APIs.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
Kevin Hao (3):
      spi: fsl-qspi: Fix double cleanup in probe error path
      spi: fsl-spi: Remove redundant probe error message
      spi: fsl-qspi: Simplify probe error handling using managed API

 drivers/spi/spi-fsl-qspi.c | 77 +++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 46 deletions(-)
---
base-commit: 29e7bf01ed8033c9a14ed0dc990dfe2736dbcd18
change-id: 20250410-spi-10adc098d566

Best regards,

Comments

Frank Li April 10, 2025, 3:52 p.m. UTC | #1
On Thu, Apr 10, 2025 at 11:45:13AM -0400, Frank Li wrote:
> On Thu, Apr 10, 2025 at 02:56:09PM +0800, Kevin Hao wrote:
> > Commit 40369bfe717e ("spi: fsl-qspi: use devm function instead of driver
> > remove") introduced managed cleanup via fsl_qspi_cleanup(), but
> > incorrectly retain manual cleanup in two scenarios:
> >
> > - On devm_add_action_or_reset() failure, the function automatically call
> >   fsl_qspi_cleanup(). However, the current code still jumps to
> >   err_destroy_mutex, repeating cleanup.
> >
> > - After the fsl_qspi_cleanup() action is added successfully, there is no
> >   need to manually perform the cleanup in the subsequent error path.
> >   However, the current code still jumps to err_destroy_mutex on spi
> >   controller failure, repeating cleanup.
> >
> > Skip redundant manual cleanup calls to fix these issues.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 40369bfe717e ("spi: fsl-qspi: use devm function instead of driver remove")
> > Signed-off-by: Kevin Hao <haokexin@gmail.com>
> > ---
> >  drivers/spi/spi-fsl-qspi.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
> > index 5c59fddb32c1b9cc030e7abb49484662ec7b382c..2f54dc09d11b1c56cfe57ceec8452fbb29322d3f 100644
> > --- a/drivers/spi/spi-fsl-qspi.c
> > +++ b/drivers/spi/spi-fsl-qspi.c
> > @@ -949,17 +949,14 @@ static int fsl_qspi_probe(struct platform_device *pdev)
> >
> >  	ret = devm_add_action_or_reset(dev, fsl_qspi_cleanup, q);
> >  	if (ret)
> > -		goto err_destroy_mutex;
> > +		goto err_put_ctrl;
>
> fsl_qspi_cleanup() already included mutex_destroy() and
> fsl_qspi_clk_disable_unprep()
>
> simple
> 		return ret;
> >
> >  	ret = devm_spi_register_controller(dev, ctlr);
> >  	if (ret)
> > -		goto err_destroy_mutex;
> > +		goto err_put_ctrl;
>
> return ret;
> >
> >  	return 0;
> >
> > -err_destroy_mutex:
> > -	mutex_destroy(&q->lock);
> > -
> >  err_disable_clk:
> >  	fsl_qspi_clk_disable_unprep(q);
>
> remove these two labels

Sorry, I missed your patch3 and Mark already applied. Please discard
my comment. it should be fine.

Frank
>
> Frank
> >
> >
> > --
> > 2.49.0
> >