[PATCHv2] mtd: rawnand: fsl_ifc: allocate shared ctrl with devm_kzalloc

[PATCHv2] mtd: rawnand: fsl_ifc: allocate shared ctrl with devm_kzalloc

[Rosen Penev]

Allocate the shared fsl_ifc_nand_ctrl structure against the controller
device, which outlives all NAND child devices, so it is freed
automatically.  This drops the manual kfree() and the broken chip
counter that was decremented in remove() but never incremented
anywhere, leaking the structure and leaving the freed pointer in
ctrl->nand on re-probe.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: rebase.
 drivers/mtd/nand/raw/fsl_ifc_nand.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c
index a88ac2cfaccd..4c2d95461c2e 100644
--- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
@@ -50,7 +50,6 @@ struct fsl_ifc_nand_ctrl {
 	unsigned int index;	/* Pointer to next byte to 'read'	*/
 	unsigned int oob;	/* Non zero if operating on OOB data	*/
 	unsigned int eccread;	/* Non zero for a full-page ECC read	*/
-	unsigned int counter;	/* counter for the initializations	*/
 	unsigned int max_bitflips;  /* Saved during READ0 cmd		*/
 };
 
@@ -1033,15 +1032,13 @@ static int fsl_ifc_nand_probe(struct platform_device *dev)
 
 	mutex_lock(&fsl_ifc_nand_mutex);
 	if (!fsl_ifc_ctrl_dev->nand) {
-		ifc_nand_ctrl = kzalloc_obj(*ifc_nand_ctrl);
+		ifc_nand_ctrl = devm_kzalloc(fsl_ifc_ctrl_dev->dev, sizeof(*ifc_nand_ctrl),
+					     GFP_KERNEL);
 		if (!ifc_nand_ctrl) {
 			mutex_unlock(&fsl_ifc_nand_mutex);
 			return -ENOMEM;
 		}
 
-		ifc_nand_ctrl->read_bytes = 0;
-		ifc_nand_ctrl->index = 0;
-		ifc_nand_ctrl->addr = NULL;
 		fsl_ifc_ctrl_dev->nand = ifc_nand_ctrl;
 
 		nand_controller_init(&ifc_nand_ctrl->controller);
@@ -1121,14 +1118,6 @@ static void fsl_ifc_nand_remove(struct platform_device *dev)
 	nand_cleanup(chip);
 
 	fsl_ifc_chip_remove(priv);
-
-	mutex_lock(&fsl_ifc_nand_mutex);
-	ifc_nand_ctrl->counter--;
-	if (!ifc_nand_ctrl->counter) {
-		fsl_ifc_ctrl_dev->nand = NULL;
-		kfree(ifc_nand_ctrl);
-	}
-	mutex_unlock(&fsl_ifc_nand_mutex);
 }
 
 static const struct of_device_id fsl_ifc_nand_match[] = {
-- 
2.55.0

Re: [PATCHv2] mtd: rawnand: fsl_ifc: allocate shared ctrl with devm_kzalloc

[Miquel Raynal]

On 07/09/2026 at 23:01:11 -07, Rosen Penev <rosenp@gmail.com> wrote:

> Allocate the shared fsl_ifc_nand_ctrl structure against the controller
> device, which outlives all NAND child devices, so it is freed
> automatically.  This drops the manual kfree() and the broken chip
> counter that was decremented in remove() but never incremented
> anywhere, leaking the structure and leaving the freed pointer in
> ctrl->nand on re-probe.
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  v2: rebase.
>  drivers/mtd/nand/raw/fsl_ifc_nand.c | 15 ++-------------
>  1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c
> index a88ac2cfaccd..4c2d95461c2e 100644
> --- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
> +++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
> @@ -50,7 +50,6 @@ struct fsl_ifc_nand_ctrl {
>  	unsigned int index;	/* Pointer to next byte to 'read'	*/
>  	unsigned int oob;	/* Non zero if operating on OOB data	*/
>  	unsigned int eccread;	/* Non zero for a full-page ECC read	*/
> -	unsigned int counter;	/* counter for the initializations	*/
>  	unsigned int max_bitflips;  /* Saved during READ0 cmd		*/
>  };
>  
> @@ -1033,15 +1032,13 @@ static int fsl_ifc_nand_probe(struct platform_device *dev)
>  
>  	mutex_lock(&fsl_ifc_nand_mutex);
>  	if (!fsl_ifc_ctrl_dev->nand) {
> -		ifc_nand_ctrl = kzalloc_obj(*ifc_nand_ctrl);
> +		ifc_nand_ctrl = devm_kzalloc(fsl_ifc_ctrl_dev->dev, sizeof(*ifc_nand_ctrl),
> +					     GFP_KERNEL);

This is a global object. I don't get why it's global. But it seems not
relevant to tie with ctrl_dev->dev (see Sashiko report). Can you please
go one step further in this cleanup?

Thanks,
Miquèl

Re: [PATCHv2] mtd: rawnand: fsl_ifc: allocate shared ctrl with devm_kzalloc

[Rosen Penev]

On Tue, Sep 8, 2026 at 2:37 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> On 07/09/2026 at 23:01:11 -07, Rosen Penev <rosenp@gmail.com> wrote:
>
> > Allocate the shared fsl_ifc_nand_ctrl structure against the controller
> > device, which outlives all NAND child devices, so it is freed
> > automatically.  This drops the manual kfree() and the broken chip
> > counter that was decremented in remove() but never incremented
> > anywhere, leaking the structure and leaving the freed pointer in
> > ctrl->nand on re-probe.
> >
> > Assisted-by: opencode:big-pickle
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  v2: rebase.
> >  drivers/mtd/nand/raw/fsl_ifc_nand.c | 15 ++-------------
> >  1 file changed, 2 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c
> > index a88ac2cfaccd..4c2d95461c2e 100644
> > --- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
> > +++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
> > @@ -50,7 +50,6 @@ struct fsl_ifc_nand_ctrl {
> >       unsigned int index;     /* Pointer to next byte to 'read'       */
> >       unsigned int oob;       /* Non zero if operating on OOB data    */
> >       unsigned int eccread;   /* Non zero for a full-page ECC read    */
> > -     unsigned int counter;   /* counter for the initializations      */
> >       unsigned int max_bitflips;  /* Saved during READ0 cmd           */
> >  };
> >
> > @@ -1033,15 +1032,13 @@ static int fsl_ifc_nand_probe(struct platform_device *dev)
> >
> >       mutex_lock(&fsl_ifc_nand_mutex);
> >       if (!fsl_ifc_ctrl_dev->nand) {
> > -             ifc_nand_ctrl = kzalloc_obj(*ifc_nand_ctrl);
> > +             ifc_nand_ctrl = devm_kzalloc(fsl_ifc_ctrl_dev->dev, sizeof(*ifc_nand_ctrl),
> > +                                          GFP_KERNEL);
>
> This is a global object. I don't get why it's global. But it seems not
> relevant to tie with ctrl_dev->dev (see Sashiko report). Can you please
> go one step further in this cleanup?
The problem is, that global comes from drivers/memory/fsl_ifc.c . Note
the forward declaration in fsl_ifc_nand.c

I haven't had luck getting anything merged there.
>
> Thanks,
> Miquèl