* [PATCH v2 00/20] Make some spi device drivers return zero in .remove()
@ 2021-10-12 15:39 Uwe Kleine-König
2021-10-12 15:39 ` [PATCH v2 13/20] mtd: dataflash: Warn about failure to unregister mtd device Uwe Kleine-König
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2021-10-12 15:39 UTC (permalink / raw)
To: Alexandre Torgue, Arnd Bergmann, Bartosz Golaszewski,
Dmitry Torokhov, Eric Piel, Greg Kroah-Hartman, Guenter Roeck,
Hans de Goede, Jarkko Sakkinen, Jason Gunthorpe, Jean Delvare,
Jiri Slaby, Lee Jones, Linus Walleij, Mark Gross,
Mauro Carvalho Chehab, Maxime Coquelin, Michael Hennerich,
Miquel Raynal, Peter Huewe, Richard Weinberger, Thierry Reding,
Vignesh Raghavendra, Yasunari Takiguchi
Cc: Andy Shevchenko, Colin Ian King, Dan Carpenter, Fabio Estevam,
Heiko Schocher, Len Baker, Mark Brown, Phil Reid, Sam Ravnborg,
Tudor Ambarus, dri-devel, kernel, linux-fbdev, linux-gpio,
linux-hwmon, linux-input, linux-integrity, linux-kernel,
linux-media, linux-mtd, linux-serial, linux-spi, linux-staging,
linux-stm32, platform-driver-x86
Hello,
this is v2 of my quest to make spi remove callbacks return void. Today
they return an int, but the only result of returning a non-zero value is
a warning message. So it's a bad idea to return an error code in the
expectation that not freeing some resources is ok then. The same holds
true for i2c and platform devices which benefit en passant for a few
drivers.
The patches in this series address some of the spi drivers that might
return non-zero and adapt them accordingly to return zero instead. For
most drivers it's just about not hiding the fact that they already
return zero.
Given that there are quite some more patches of this type to create
before I can change the spi remove callback, I suggest the respective
subsystem maintainers pick up these patches. There are no
interdependencies in this series.
Compared to (implicit) v1
- I fixed a few compiler issues (this series it build tested with an
allmoddefconfig on arm64, m68k, powerpc, riscv, s390, sparc64 and
x86_64).
- A few new patches (2x gpio, 2x misc, 4x mtd)
- One patch already landed in next, this one I dropped. The drm/panel
patch as claimed to applied, too, but not yet in next. It's included
here, but I assume I was just too impatient and this one should be
ignored.
Full range-diff below.
Best regards
Uwe
Uwe Kleine-König (20):
drm/panel: s6e63m0: Make s6e63m0_remove() return void
gpio: max730x: Make __max730x_remove() return void
gpio: mc33880: Drop if with an always false condition
hwmon: max31722: Warn about failure to put device in stand-by in
.remove()
input: adxl34xx: Make adxl34x_remove() return void
input: touchscreen: tsc200x: Make tsc200x_remove() return void
media: cxd2880: Eliminate dead code
mfd: mc13xxx: Make mc13xxx_common_exit() return void
mfd: stmpe: Make stmpe_remove() return void
mfd: tps65912: Make tps65912_device_exit() return void
misc: ad525x_dpot: Make ad_dpot_remove() return void
misc: lis3lv02d: Make lis3lv02d_remove_fs() return void
mtd: dataflash: Warn about failure to unregister mtd device
mtd: mchp23k256: Warn about failure to unregister mtd device
mtd: mchp48l640: Warn about failure to unregister mtd device
mtd: sst25l: Warn about failure to unregister mtd device
serial: max310x: Make max310x_remove() return void
serial: sc16is7xx: Make sc16is7xx_remove() return void
staging: fbtft: Make fbtft_remove_common() return void
tpm: st33zp24: Make st33zp24_remove() return void
drivers/char/tpm/st33zp24/i2c.c | 5 +----
drivers/char/tpm/st33zp24/spi.c | 5 +----
drivers/char/tpm/st33zp24/st33zp24.c | 3 +--
drivers/char/tpm/st33zp24/st33zp24.h | 2 +-
drivers/gpio/gpio-max7300.c | 4 +++-
drivers/gpio/gpio-max7301.c | 4 +++-
drivers/gpio/gpio-max730x.c | 6 +-----
drivers/gpio/gpio-mc33880.c | 2 --
drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c | 3 ++-
drivers/gpu/drm/panel/panel-samsung-s6e63m0-spi.c | 3 ++-
drivers/gpu/drm/panel/panel-samsung-s6e63m0.c | 4 +---
drivers/gpu/drm/panel/panel-samsung-s6e63m0.h | 2 +-
drivers/hwmon/max31722.c | 8 +++++++-
drivers/input/misc/adxl34x-i2c.c | 4 +++-
drivers/input/misc/adxl34x-spi.c | 4 +++-
drivers/input/misc/adxl34x.c | 4 +---
drivers/input/misc/adxl34x.h | 2 +-
drivers/input/touchscreen/tsc2004.c | 4 +++-
drivers/input/touchscreen/tsc2005.c | 4 +++-
drivers/input/touchscreen/tsc200x-core.c | 4 +---
drivers/input/touchscreen/tsc200x-core.h | 2 +-
drivers/media/spi/cxd2880-spi.c | 13 +------------
drivers/mfd/mc13xxx-core.c | 4 +---
drivers/mfd/mc13xxx-i2c.c | 3 ++-
drivers/mfd/mc13xxx-spi.c | 3 ++-
drivers/mfd/mc13xxx.h | 2 +-
drivers/mfd/stmpe-i2c.c | 4 +++-
drivers/mfd/stmpe-spi.c | 4 +++-
drivers/mfd/stmpe.c | 4 +---
drivers/mfd/stmpe.h | 2 +-
drivers/mfd/tps65912-core.c | 4 +---
drivers/mfd/tps65912-i2c.c | 4 +++-
drivers/mfd/tps65912-spi.c | 4 +++-
drivers/misc/ad525x_dpot-i2c.c | 3 ++-
drivers/misc/ad525x_dpot-spi.c | 3 ++-
drivers/misc/ad525x_dpot.c | 4 +---
drivers/misc/ad525x_dpot.h | 2 +-
drivers/misc/lis3lv02d/lis3lv02d.c | 3 +--
drivers/misc/lis3lv02d/lis3lv02d.h | 2 +-
drivers/misc/lis3lv02d/lis3lv02d_spi.c | 4 +++-
drivers/mtd/devices/mchp23k256.c | 9 ++++++++-
drivers/mtd/devices/mchp48l640.c | 8 +++++++-
drivers/mtd/devices/mtd_dataflash.c | 5 ++++-
drivers/mtd/devices/sst25l.c | 8 +++++++-
drivers/platform/x86/hp_accel.c | 3 ++-
drivers/staging/fbtft/fbtft-core.c | 8 +-------
drivers/staging/fbtft/fbtft.h | 8 +++++---
drivers/tty/serial/max310x.c | 7 +++----
drivers/tty/serial/sc16is7xx.c | 12 +++++++-----
include/linux/mfd/tps65912.h | 2 +-
include/linux/spi/max7301.h | 2 +-
51 files changed, 119 insertions(+), 104 deletions(-)
Range-diff against v1:
1: 73a1a54d9ea0 = 1: 87fd7940fbfd drm/panel: s6e63m0: Make s6e63m0_remove() return void
2: 3bcc8e8bd1a3 < -: ------------ hwmon: adt7x10: Make adt7x10_remove() return void
-: ------------ > 2: 305311d63bbb gpio: max730x: Make __max730x_remove() return void
-: ------------ > 3: 0cafc31ea5c5 gpio: mc33880: Drop if with an always false condition
3: 07f067732aa9 ! 4: f39467b50f06 hwmon: max31722: Warn about failure to put device in stand-by in .remove()
@@ Commit message
nothing happens apart from emitting a generic error message. Make this
error message more device specific and return zero instead.
- Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by: Michael Hennerich <michael.hennerich@analog.com>
+ Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
## drivers/hwmon/max31722.c ##
@@ drivers/hwmon/max31722.c: static int max31722_probe(struct spi_device *spi)
4: 0b0a5497d105 = 5: de3a78214008 input: adxl34xx: Make adxl34x_remove() return void
5: 0d4f14bc2dd6 ! 6: 9629ac3f9e13 input: touchscreen: tsc200x: Make tsc200x_remove() return void
@@ drivers/input/touchscreen/tsc2005.c: static int tsc2005_probe(struct spi_device
- return tsc200x_remove(&spi->dev);
+ tsc200x_remove(&spi->dev);
+
-+ return 0
++ return 0;
}
#ifdef CONFIG_OF
6: a68bbd23223b = 7: 1aab41df9262 media: cxd2880: Eliminate dead code
7: 3801b37ac18f ! 8: 745d1a5f840e mfd: mc13xxx: Make mc13xxx_common_exit() return void
@@ drivers/mfd/mc13xxx-spi.c: static int mc13xxx_spi_probe(struct spi_device *spi)
{
- return mc13xxx_common_exit(&spi->dev);
+ mc13xxx_common_exit(&spi->dev);
-+ return 0
++ return 0;
}
static struct spi_driver mc13xxx_spi_driver = {
8: 22159093ce71 = 9: 7ee04277db66 mfd: stmpe: Make stmpe_remove() return void
9: f91da216c752 = 10: 4a21c90a57f8 mfd: tps65912: Make tps65912_device_exit() return void
-: ------------ > 11: f92aa824fd1c misc: ad525x_dpot: Make ad_dpot_remove() return void
-: ------------ > 12: 5b2fccd09a24 misc: lis3lv02d: Make lis3lv02d_remove_fs() return void
-: ------------ > 13: 609ab18323fc mtd: dataflash: Warn about failure to unregister mtd device
-: ------------ > 14: 3b220d5fa547 mtd: mchp23k256: Warn about failure to unregister mtd device
-: ------------ > 15: baf6f4b3a8c7 mtd: mchp48l640: Warn about failure to unregister mtd device
-: ------------ > 16: edf3788a30b0 mtd: sst25l: Warn about failure to unregister mtd device
10: f2def77b74d1 ! 17: 614f7c001377 serial: max310x: Make max310x_remove() return void
@@ drivers/tty/serial/max310x.c: static int max310x_spi_probe(struct spi_device *sp
{
- return max310x_remove(&spi->dev);
+ max310x_remove(&spi->dev);
-+ return 0
++ return 0;
}
static const struct spi_device_id max310x_id_table[] = {
11: 283e4bbeff38 ! 18: 35d1f5b36de5 serial: sc16is7xx: Make sc16is7xx_remove() return void
@@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_probe(struct device *dev,
{
struct sc16is7xx_port *s = dev_get_drvdata(dev);
int i;
+@@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_remove(struct device *dev)
+ kthread_stop(s->kworker_task);
+
+ clk_disable_unprepare(s->clk);
+-
+- return 0;
+ }
+
+ static const struct of_device_id __maybe_unused sc16is7xx_dt_ids[] = {
@@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_spi_probe(struct spi_device *spi)
static int sc16is7xx_spi_remove(struct spi_device *spi)
12: 5093fbdceee5 ! 19: d9ec9a96fbb8 staging: fbtft: Make fbtft_remove_common() return void
@@ drivers/staging/fbtft/fbtft-core.c: EXPORT_SYMBOL(fbtft_probe_common);
## drivers/staging/fbtft/fbtft.h ##
+@@ drivers/staging/fbtft/fbtft.h: void fbtft_unregister_backlight(struct fbtft_par *par);
+ int fbtft_init_display(struct fbtft_par *par);
+ int fbtft_probe_common(struct fbtft_display *display, struct spi_device *sdev,
+ struct platform_device *pdev);
+-int fbtft_remove_common(struct device *dev, struct fb_info *info);
++void fbtft_remove_common(struct device *dev, struct fb_info *info);
+
+ /* fbtft-io.c */
+ int fbtft_write_spi(struct fbtft_par *par, void *buf, size_t len);
@@ drivers/staging/fbtft/fbtft.h: static int fbtft_driver_remove_spi(struct spi_device *spi) \
{ \
struct fb_info *info = spi_get_drvdata(spi); \
13: 9156e6380a5e = 20: 89d0b85968a9 tpm: st33zp24: Make st33zp24_remove() return void
base-commit: 9e1ff307c779ce1f0f810c7ecce3d95bbae40896
--
2.30.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v2 13/20] mtd: dataflash: Warn about failure to unregister mtd device
2021-10-12 15:39 [PATCH v2 00/20] Make some spi device drivers return zero in .remove() Uwe Kleine-König
@ 2021-10-12 15:39 ` Uwe Kleine-König
2021-10-13 12:44 ` Miquel Raynal
2021-10-12 15:39 ` [PATCH v2 14/20] mtd: mchp23k256: " Uwe Kleine-König
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2021-10-12 15:39 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
Cc: Mark Brown, Tudor Ambarus, kernel, linux-mtd, linux-spi
When an spi driver's remove function returns a non-zero error code
nothing happens apart from emitting a generic error message. Make this
error message more device specific and return zero instead.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/mtd/devices/mtd_dataflash.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/devices/mtd_dataflash.c b/drivers/mtd/devices/mtd_dataflash.c
index 9802e265fca8..2691b6b79df8 100644
--- a/drivers/mtd/devices/mtd_dataflash.c
+++ b/drivers/mtd/devices/mtd_dataflash.c
@@ -919,7 +919,10 @@ static int dataflash_remove(struct spi_device *spi)
status = mtd_device_unregister(&flash->mtd);
if (status == 0)
kfree(flash);
- return status;
+ else
+ dev_warn(&spi->dev, "Failed to unregister mtd device (%pe)\n",
+ ERR_PTR(status));
+ return 0;
}
static struct spi_driver dataflash_driver = {
--
2.30.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 13/20] mtd: dataflash: Warn about failure to unregister mtd device
2021-10-12 15:39 ` [PATCH v2 13/20] mtd: dataflash: Warn about failure to unregister mtd device Uwe Kleine-König
@ 2021-10-13 12:44 ` Miquel Raynal
2021-10-13 14:08 ` Uwe Kleine-König
0 siblings, 1 reply; 9+ messages in thread
From: Miquel Raynal @ 2021-10-13 12:44 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Richard Weinberger, Vignesh Raghavendra, Mark Brown,
Tudor Ambarus, kernel, linux-mtd, linux-spi
Hi Uwe,
u.kleine-koenig@pengutronix.de wrote on Tue, 12 Oct 2021 17:39:38 +0200:
> When an spi driver's remove function returns a non-zero error code
Should we s/an spi/a SPI/?
> nothing happens apart from emitting a generic error message. Make this
> error message more device specific and return zero instead.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> drivers/mtd/devices/mtd_dataflash.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/devices/mtd_dataflash.c b/drivers/mtd/devices/mtd_dataflash.c
> index 9802e265fca8..2691b6b79df8 100644
> --- a/drivers/mtd/devices/mtd_dataflash.c
> +++ b/drivers/mtd/devices/mtd_dataflash.c
> @@ -919,7 +919,10 @@ static int dataflash_remove(struct spi_device *spi)
> status = mtd_device_unregister(&flash->mtd);
> if (status == 0)
> kfree(flash);
> - return status;
> + else
> + dev_warn(&spi->dev, "Failed to unregister mtd device (%pe)\n",
> + ERR_PTR(status));
> + return 0;
As part of a recent NAND cleanup series we ended up adding WARN_ON() [1]
to make it very clear that if this happens, it's not expected at all (it
was Boris' advice).
I don't think there is only one good solution but perhaps its best to
keep it sync'ed with the other drivers in MTD?
Thanks,
Miquèl
[1]
d6e4fd522461 mtd: rawnand: nandsim: Stop using nand_release()
9fdd78f7bcda mtd: rawnand: xway: Stop using nand_release()
d9f2a1af817d mtd: rawnand: vf610: Stop using nand_release()
f6fc75978d88 mtd: rawnand: txx9ndfmc: Stop using nand_release()
f3e169f44bdb mtd: rawnand: tmio: Stop using nand_release()
ab135c51bb81 mtd: rawnand: tango: Stop using nand_release()
068d86ecd9d9 mtd: rawnand: sunxi: Stop using nand_release()
24acc3fa8b36 mtd: rawnand: stm32_fmc2: Stop using nand_release()
c121cb980c09 mtd: rawnand: socrates: Stop using nand_release()
35a37f9198e5 mtd: rawnand: sharpsl: Stop using nand_release()
50abacbb621f mtd: rawnand: sh_flctl: Stop using nand_release()
9748110bd22c mtd: rawnand: s3c2410: Stop using nand_release()
10b87750ae17 mtd: rawnand: r852: Stop using nand_release()
0a2bc9919cf7 mtd: rawnand: qcom: Stop using nand_release()
d1aae005a00e mtd: rawnand: plat_nand: Stop using nand_release()
23cf34615010 mtd: rawnand: pasemi: Stop using nand_release()
2d9cf6f129f8 mtd: rawnand: oxnas: Stop using nand_release()
f342df67b19a mtd: rawnand: orion: Stop using nand_release()
b4533679c958 mtd: rawnand: omap2: Stop using nand_release()
a9384f95fe77 mtd: rawnand: ndfc: Stop using nand_release()
8fd507bb4210 mtd: rawnand: mxic: Stop using nand_release()
c6dc082793d2 mtd: rawnand: mxc: Stop using nand_release()
1fec333aadc2 mtd: rawnand: mtk: Stop using nand_release()
1a36a7f78898 mtd: rawnand: mpc5121: Stop using nand_release()
5ecbba617446 mtd: rawnand: marvell: Stop using nand_release()
21b758277724 mtd: rawnand: lpc32xx_slc: Stop using nand_release()
5f3bce3a5275 mtd: rawnand: lpc32xx_mlc: Stop using nand_release()
28dcc4e8a831 mtd: rawnand: ingenic: Stop using nand_release()
71a4917b4d4b mtd: rawnand: hisi504: Stop using nand_release()
194f6c48cdd8 mtd: rawnand: gpmi: Stop using nand_release()
dbe0241570ed mtd: rawnand: gpio: Stop using nand_release()
9cc02f4c0a87 mtd: rawnand: fsmc: Stop using nand_release()
f6c4e661491a mtd: rawnand: fsl_upm: Stop using nand_release()
e9f2f5a80754 mtd: rawnand: fsl_ifc: Stop using nand_release()
128bbbf0ac4d mtd: rawnand: fsl_elbc: Stop using nand_release()
63a1460768a1 mtd: rawnand: diskonchip: Stop using nand_release()
009e2e1d8318 mtd: rawnand: denali: Stop using nand_release()
a9575c48e520 mtd: rawnand: davinci: Stop using nand_release()
970024f031ae mtd: rawnand: cs553x: Stop using nand_release()
544bac8999a6 mtd: rawnand: cafe: Stop using nand_release()
8b88f4e0a88b mtd: rawnand: cadence: Stop using nand_release()
937d039dfdcf mtd: rawnand: brcmnand: Stop using nand_release()
936904305928 mtd: rawnand: bcm47xx: Stop using nand_release()
4a3d21bc25c1 mtd: rawnand: au1550nd: Stop using nand_release()
08f25cd767e1 mtd: rawnand: ams-delta: Stop using nand_release()
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 13/20] mtd: dataflash: Warn about failure to unregister mtd device
2021-10-13 12:44 ` Miquel Raynal
@ 2021-10-13 14:08 ` Uwe Kleine-König
2021-10-13 14:33 ` Miquel Raynal
0 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2021-10-13 14:08 UTC (permalink / raw)
To: Miquel Raynal
Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger, linux-spi,
Mark Brown, linux-mtd, kernel
[-- Attachment #1.1: Type: text/plain, Size: 2177 bytes --]
On Wed, Oct 13, 2021 at 02:44:29PM +0200, Miquel Raynal wrote:
> Hi Uwe,
>
> u.kleine-koenig@pengutronix.de wrote on Tue, 12 Oct 2021 17:39:38 +0200:
>
> > When an spi driver's remove function returns a non-zero error code
>
> Should we s/an spi/a SPI/?
My (German) knowledge about the English Grammar claims that independent
of how you spell SPI, it must be "an" because when I say it, it's
[ɛspi:aɪ] (unless you call it [spaɪ]?)
In my eyes "spi" is right, because SPI is the protocol and "spi" is the
kernel framework. But I don't feel strong here and you're already the
second who suggests something similar.
> > nothing happens apart from emitting a generic error message. Make this
> > error message more device specific and return zero instead.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > drivers/mtd/devices/mtd_dataflash.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/devices/mtd_dataflash.c b/drivers/mtd/devices/mtd_dataflash.c
> > index 9802e265fca8..2691b6b79df8 100644
> > --- a/drivers/mtd/devices/mtd_dataflash.c
> > +++ b/drivers/mtd/devices/mtd_dataflash.c
> > @@ -919,7 +919,10 @@ static int dataflash_remove(struct spi_device *spi)
> > status = mtd_device_unregister(&flash->mtd);
> > if (status == 0)
> > kfree(flash);
> > - return status;
> > + else
> > + dev_warn(&spi->dev, "Failed to unregister mtd device (%pe)\n",
> > + ERR_PTR(status));
> > + return 0;
>
> As part of a recent NAND cleanup series we ended up adding WARN_ON() [1]
> to make it very clear that if this happens, it's not expected at all (it
> was Boris' advice).
>
> I don't think there is only one good solution but perhaps its best to
> keep it sync'ed with the other drivers in MTD?
Well, if WARN_ON or dev_warn is the right thing is subjective. Your
subjective counts more as you're an MTD maintainer. Can rework
accordingly for v3.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 144 bytes --]
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 13/20] mtd: dataflash: Warn about failure to unregister mtd device
2021-10-13 14:08 ` Uwe Kleine-König
@ 2021-10-13 14:33 ` Miquel Raynal
2021-10-13 15:13 ` Mark Brown
0 siblings, 1 reply; 9+ messages in thread
From: Miquel Raynal @ 2021-10-13 14:33 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger, linux-spi,
Mark Brown, linux-mtd, kernel
Hi Uwe,
u.kleine-koenig@pengutronix.de wrote on Wed, 13 Oct 2021 16:08:35 +0200:
> On Wed, Oct 13, 2021 at 02:44:29PM +0200, Miquel Raynal wrote:
> > Hi Uwe,
> >
> > u.kleine-koenig@pengutronix.de wrote on Tue, 12 Oct 2021 17:39:38 +0200:
> >
> > > When an spi driver's remove function returns a non-zero error code
> >
> > Should we s/an spi/a SPI/?
>
> My (German) knowledge about the English Grammar claims that independent
> of how you spell SPI, it must be "an" because when I say it, it's
> [ɛspi:aɪ] (unless you call it [spaɪ]?)
I (personally) pronounce it [spaɪ] with my French background and it
looks wrong to my eyes to use "an" before SPI because of that, but this
is biased and possibly wrong as well so please keep it your way, it's
fine.
> In my eyes "spi" is right, because SPI is the protocol and "spi" is
> the kernel framework. But I don't feel strong here and you're already
> the second who suggests something similar.
I get it. Indeed I always use uppercase letters when I use acronyms
(such as SPI or MTD) and it's the first time I hear that the lowercase
letter refers to the framework more than the protocol, but TBH the
explanation kind of seduces me :)
> > > nothing happens apart from emitting a generic error message. Make
> > > this error message more device specific and return zero instead.
> > >
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > > drivers/mtd/devices/mtd_dataflash.c | 5 ++++-
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mtd/devices/mtd_dataflash.c
> > > b/drivers/mtd/devices/mtd_dataflash.c index
> > > 9802e265fca8..2691b6b79df8 100644 ---
> > > a/drivers/mtd/devices/mtd_dataflash.c +++
> > > b/drivers/mtd/devices/mtd_dataflash.c @@ -919,7 +919,10 @@ static
> > > int dataflash_remove(struct spi_device *spi) status =
> > > mtd_device_unregister(&flash->mtd); if (status == 0)
> > > kfree(flash);
> > > - return status;
> > > + else
> > > + dev_warn(&spi->dev, "Failed to unregister mtd
> > > device (%pe)\n",
> > > + ERR_PTR(status));
> > > + return 0;
> >
> > As part of a recent NAND cleanup series we ended up adding
> > WARN_ON() [1] to make it very clear that if this happens, it's not
> > expected at all (it was Boris' advice).
> >
> > I don't think there is only one good solution but perhaps its best
> > to keep it sync'ed with the other drivers in MTD?
>
> Well, if WARN_ON or dev_warn is the right thing is subjective. Your
> subjective counts more as you're an MTD maintainer. Can rework
> accordingly for v3.
Then let's all use WARN_ON() for now.
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 13/20] mtd: dataflash: Warn about failure to unregister mtd device
2021-10-13 14:33 ` Miquel Raynal
@ 2021-10-13 15:13 ` Mark Brown
0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2021-10-13 15:13 UTC (permalink / raw)
To: Miquel Raynal
Cc: Uwe Kleine-König, Vignesh Raghavendra, Tudor Ambarus,
Richard Weinberger, linux-spi, linux-mtd, kernel
[-- Attachment #1.1: Type: text/plain, Size: 632 bytes --]
On Wed, Oct 13, 2021 at 04:33:57PM +0200, Miquel Raynal wrote:
> > On Wed, Oct 13, 2021 at 02:44:29PM +0200, Miquel Raynal wrote:
> > My (German) knowledge about the English Grammar claims that independent
> > of how you spell SPI, it must be "an" because when I say it, it's
> > [ɛspi:aɪ] (unless you call it [spaɪ]?)
> I (personally) pronounce it [spaɪ] with my French background and it
> looks wrong to my eyes to use "an" before SPI because of that, but this
> is biased and possibly wrong as well so please keep it your way, it's
> fine.
I'd say a here too but really don't care too much, I hadn't noticed.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 144 bytes --]
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 14/20] mtd: mchp23k256: Warn about failure to unregister mtd device
2021-10-12 15:39 [PATCH v2 00/20] Make some spi device drivers return zero in .remove() Uwe Kleine-König
2021-10-12 15:39 ` [PATCH v2 13/20] mtd: dataflash: Warn about failure to unregister mtd device Uwe Kleine-König
@ 2021-10-12 15:39 ` Uwe Kleine-König
2021-10-12 15:39 ` [PATCH v2 15/20] mtd: mchp48l640: " Uwe Kleine-König
2021-10-12 15:39 ` [PATCH v2 16/20] mtd: sst25l: " Uwe Kleine-König
3 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2021-10-12 15:39 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
Cc: Mark Brown, kernel, linux-mtd, linux-spi
When an spi driver's remove function returns a non-zero error code
nothing happens apart from emitting a generic error message. Make this
error message more device specific and return zero instead.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/mtd/devices/mchp23k256.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/devices/mchp23k256.c b/drivers/mtd/devices/mchp23k256.c
index 77c872fd3d83..23213009ebf4 100644
--- a/drivers/mtd/devices/mchp23k256.c
+++ b/drivers/mtd/devices/mchp23k256.c
@@ -212,8 +212,15 @@ static int mchp23k256_probe(struct spi_device *spi)
static int mchp23k256_remove(struct spi_device *spi)
{
struct mchp23k256_flash *flash = spi_get_drvdata(spi);
+ int ret;
- return mtd_device_unregister(&flash->mtd);
+ ret = mtd_device_unregister(&flash->mtd);
+ if (ret)
+ /* There is nothing we can do about this ... */
+ dev_warn(&spi->dev, "Failed to unregister mtd device (%pe)\n",
+ ERR_PTR(ret));
+
+ return 0;
}
static const struct of_device_id mchp23k256_of_table[] = {
--
2.30.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 15/20] mtd: mchp48l640: Warn about failure to unregister mtd device
2021-10-12 15:39 [PATCH v2 00/20] Make some spi device drivers return zero in .remove() Uwe Kleine-König
2021-10-12 15:39 ` [PATCH v2 13/20] mtd: dataflash: Warn about failure to unregister mtd device Uwe Kleine-König
2021-10-12 15:39 ` [PATCH v2 14/20] mtd: mchp23k256: " Uwe Kleine-König
@ 2021-10-12 15:39 ` Uwe Kleine-König
2021-10-12 15:39 ` [PATCH v2 16/20] mtd: sst25l: " Uwe Kleine-König
3 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2021-10-12 15:39 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
Cc: Colin Ian King, Dan Carpenter, Fabio Estevam, Heiko Schocher,
Mark Brown, kernel, linux-mtd, linux-spi
When an spi driver's remove function returns a non-zero error code
nothing happens apart from emitting a generic error message. Make this
error message more device specific and return zero instead.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/mtd/devices/mchp48l640.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/devices/mchp48l640.c b/drivers/mtd/devices/mchp48l640.c
index 99400d0fb8c1..b87f7eca3e64 100644
--- a/drivers/mtd/devices/mchp48l640.c
+++ b/drivers/mtd/devices/mchp48l640.c
@@ -344,8 +344,14 @@ static int mchp48l640_probe(struct spi_device *spi)
static int mchp48l640_remove(struct spi_device *spi)
{
struct mchp48l640_flash *flash = spi_get_drvdata(spi);
+ int ret;
+
+ ret = mtd_device_unregister(&flash->mtd);
+ if (ret)
+ dev_warn(&spi->dev, "Failed to unregister mtd device (%pe)\n",
+ ERR_PTR(ret));
- return mtd_device_unregister(&flash->mtd);
+ return 0;
}
static const struct of_device_id mchp48l640_of_table[] = {
--
2.30.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 16/20] mtd: sst25l: Warn about failure to unregister mtd device
2021-10-12 15:39 [PATCH v2 00/20] Make some spi device drivers return zero in .remove() Uwe Kleine-König
` (2 preceding siblings ...)
2021-10-12 15:39 ` [PATCH v2 15/20] mtd: mchp48l640: " Uwe Kleine-König
@ 2021-10-12 15:39 ` Uwe Kleine-König
3 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2021-10-12 15:39 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
Cc: Mark Brown, kernel, linux-mtd, linux-spi
When an spi driver's remove function returns a non-zero error code
nothing happens apart from emitting a generic error message. Make this
error message more device specific and return zero instead.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/mtd/devices/sst25l.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/devices/sst25l.c b/drivers/mtd/devices/sst25l.c
index b81c3f0b85f9..04062a1f54c1 100644
--- a/drivers/mtd/devices/sst25l.c
+++ b/drivers/mtd/devices/sst25l.c
@@ -401,8 +401,14 @@ static int sst25l_probe(struct spi_device *spi)
static int sst25l_remove(struct spi_device *spi)
{
struct sst25l_flash *flash = spi_get_drvdata(spi);
+ int ret;
+
+ ret = mtd_device_unregister(&flash->mtd);
+ if (ret)
+ dev_warn(&spi->dev, "Failed to unregister mtd device (%pe)\n",
+ ERR_PTR(ret));
- return mtd_device_unregister(&flash->mtd);
+ return 0;
}
static struct spi_driver sst25l_driver = {
--
2.30.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-10-13 15:14 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-12 15:39 [PATCH v2 00/20] Make some spi device drivers return zero in .remove() Uwe Kleine-König
2021-10-12 15:39 ` [PATCH v2 13/20] mtd: dataflash: Warn about failure to unregister mtd device Uwe Kleine-König
2021-10-13 12:44 ` Miquel Raynal
2021-10-13 14:08 ` Uwe Kleine-König
2021-10-13 14:33 ` Miquel Raynal
2021-10-13 15:13 ` Mark Brown
2021-10-12 15:39 ` [PATCH v2 14/20] mtd: mchp23k256: " Uwe Kleine-König
2021-10-12 15:39 ` [PATCH v2 15/20] mtd: mchp48l640: " Uwe Kleine-König
2021-10-12 15:39 ` [PATCH v2 16/20] mtd: sst25l: " Uwe Kleine-König
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox