| Message ID | 20220603210758.148493-9-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mtd: Fix platform remove callbacks to always return 0 | expand |
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
On Mon, Jun 06, 2022 at 03:16:20PM +0200, Miquel Raynal wrote: > 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. You're right. I assume it's easiest for you if I send a v2 with all 14 patches? If you consider this a waste of bytes, please advise. The patches are independant, so it would work if you pick up 1-7 + 9-14, too. Then I'd resend a fixed patch 8 individually. Best regards Uwe
Hi Uwe, u.kleine-koenig@pengutronix.de wrote on Mon, 6 Jun 2022 21:37:21 +0200: > On Mon, Jun 06, 2022 at 03:16:20PM +0200, Miquel Raynal wrote: > > 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. > > You're right. I assume it's easiest for you if I send a v2 with all 14 > patches? If you consider this a waste of bytes, please advise. The > patches are independant, so it would work if you pick up 1-7 + 9-14, > too. Then I'd resend a fixed patch 8 individually. Please just resend patch 8, I'll handle it. > > Best regards > Uwe > Thanks, Miquèl
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)
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. 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(-)