Re: [PATCH 08/14] mtd: rawnand: atmel: Warn about failure to unregister mtd device

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Tudor Ambarus <tudor.ambarus@microchip.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Claudiu Beznea <claudiu.beznea@microchip.com>,
	kernel@pengutronix.de, linux-mtd@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 08/14] mtd: rawnand: atmel: Warn about failure to unregister mtd device
Date: Mon, 6 Jun 2022 15:16:20 +0200	[thread overview]
Message-ID: <20220606151620.0282cece@xps-13> (raw)
In-Reply-To: <20220603210758.148493-9-u.kleine-koenig@pengutronix.de>

Hi Uwe,

u.kleine-koenig@pengutronix.de wrote on Fri,  3 Jun 2022 23:07:52 +0200:

> The Linux device core doesn't intend remove callbacks to fail. If an
> error code is returned the device is removed anyhow. So wail loudly if
> the atmel specific remove callback fails and return 0 anyhow to suppress
> the generic (and little helpful) error message by the device core.
> 
> Also check the remove callback to actually exist before calling it. That
> might happen if nc->caps->ops points to atmel_nand_controller_ops.

I believe you got mislead by grepping the code because there is:

* struct nand_controller_ops atmel_nand_controller_ops
  -> this is a NAND-wide controller ops structure

* struct atmel_nand_controller_ops atmel_<smtg>_nc_ops
  -> this is a driver specific structure to provide different
     registration helpers.

The latter always provide a probe and a remove implementation, so I
believe the addition if the "if (nc->caps->ops->remove)" check is not
relevant, unless I missed something.

> This is a preparation for making platform remove callbacks return void.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/mtd/nand/raw/atmel/nand-controller.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index 6ef14442c71a..bc6ee694f4e2 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -2629,7 +2629,10 @@ static int atmel_nand_controller_remove(struct platform_device *pdev)
>  {
>  	struct atmel_nand_controller *nc = platform_get_drvdata(pdev);
>  
> -	return nc->caps->ops->remove(nc);
> +	if (nc->caps->ops->remove)
> +		WARN_ON(nc->caps->ops->remove(nc));
> +
> +	return 0;
>  }
>  
>  static __maybe_unused int atmel_nand_controller_resume(struct device *dev)

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/