Linux-fbdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/2] ALSA: ppc: drop if block with always false condition
@ 2020-11-26 16:59 Uwe Kleine-König
  2020-11-26 16:59 ` [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void Uwe Kleine-König
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Uwe Kleine-König @ 2020-11-26 16:59 UTC (permalink / raw)
  To: Geoff Levand, Jaroslav Kysela, Takashi Iwai, Michael Ellerman,
	Jens Axboe, Jim Paris, Arnd Bergmann, Greg Kroah-Hartman,
	David S. Miller, Jakub Kicinski, James E.J. Bottomley,
	Martin K. Petersen, Alan Stern, Bartlomiej Zolnierkiewicz
  Cc: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev, alsa-devel,
	linux-block, netdev, linux-scsi, linux-usb, dri-devel,
	linux-fbdev

The remove callback is only called for devices that were probed
successfully before. As the matching probe function cannot complete
without error if dev->match_id != PS3_MATCH_ID_SOUND, we don't have to
check this here.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 sound/ppc/snd_ps3.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
index 58bb49fff184..6ab796a5d936 100644
--- a/sound/ppc/snd_ps3.c
+++ b/sound/ppc/snd_ps3.c
@@ -1053,8 +1053,6 @@ static int snd_ps3_driver_remove(struct ps3_system_bus_device *dev)
 {
 	int ret;
 	pr_info("%s:start id=%d\n", __func__,  dev->match_id);
-	if (dev->match_id != PS3_MATCH_ID_SOUND)
-		return -ENXIO;
 
 	/*
 	 * ctl and preallocate buffer will be freed in
-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void
  2020-11-26 16:59 [PATCH 1/2] ALSA: ppc: drop if block with always false condition Uwe Kleine-König
@ 2020-11-26 16:59 ` Uwe Kleine-König
  2020-11-27  8:40   ` Geert Uytterhoeven
                     ` (2 more replies)
  2020-11-27  8:35 ` [PATCH 1/2] ALSA: ppc: drop if block with always false condition Geert Uytterhoeven
                   ` (3 subsequent siblings)
  4 siblings, 3 replies; 13+ messages in thread
From: Uwe Kleine-König @ 2020-11-26 16:59 UTC (permalink / raw)
  To: Geoff Levand, Jaroslav Kysela, Takashi Iwai, Michael Ellerman,
	Jens Axboe, Jim Paris, Arnd Bergmann, Greg Kroah-Hartman,
	David S. Miller, Jakub Kicinski, James E.J. Bottomley,
	Martin K. Petersen, Alan Stern, Bartlomiej Zolnierkiewicz
  Cc: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev, alsa-devel,
	linux-block, netdev, linux-scsi, linux-usb, dri-devel,
	linux-fbdev

The driver core ignores the return value of struct device_driver::remove
because there is only little that can be done. For the shutdown callback
it's ps3_system_bus_shutdown() which ignores the return value.

To simplify the quest to make struct device_driver::remove return void,
let struct ps3_system_bus_driver::remove return void, too. All users
already unconditionally return 0, this commit makes it obvious that
returning an error code is a bad idea and ensures future users behave
accordingly.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 arch/powerpc/include/asm/ps3.h               |  4 ++--
 arch/powerpc/platforms/ps3/system-bus.c      |  5 ++---
 drivers/block/ps3disk.c                      |  3 +--
 drivers/block/ps3vram.c                      |  3 +--
 drivers/char/ps3flash.c                      |  3 +--
 drivers/net/ethernet/toshiba/ps3_gelic_net.c |  3 +--
 drivers/ps3/ps3-lpm.c                        |  3 +--
 drivers/ps3/ps3-vuart.c                      | 10 ++++------
 drivers/scsi/ps3rom.c                        |  3 +--
 drivers/usb/host/ehci-ps3.c                  |  4 +---
 drivers/usb/host/ohci-ps3.c                  |  4 +---
 drivers/video/fbdev/ps3fb.c                  |  4 +---
 sound/ppc/snd_ps3.c                          |  3 +--
 13 files changed, 18 insertions(+), 34 deletions(-)

diff --git a/arch/powerpc/include/asm/ps3.h b/arch/powerpc/include/asm/ps3.h
index cb89e4bf55ce..e646c7f218bc 100644
--- a/arch/powerpc/include/asm/ps3.h
+++ b/arch/powerpc/include/asm/ps3.h
@@ -378,8 +378,8 @@ struct ps3_system_bus_driver {
 	enum ps3_match_sub_id match_sub_id;
 	struct device_driver core;
 	int (*probe)(struct ps3_system_bus_device *);
-	int (*remove)(struct ps3_system_bus_device *);
-	int (*shutdown)(struct ps3_system_bus_device *);
+	void (*remove)(struct ps3_system_bus_device *);
+	void (*shutdown)(struct ps3_system_bus_device *);
 /*	int (*suspend)(struct ps3_system_bus_device *, pm_message_t); */
 /*	int (*resume)(struct ps3_system_bus_device *); */
 };
diff --git a/arch/powerpc/platforms/ps3/system-bus.c b/arch/powerpc/platforms/ps3/system-bus.c
index c62aaa29a9d5..b431f41c6cb5 100644
--- a/arch/powerpc/platforms/ps3/system-bus.c
+++ b/arch/powerpc/platforms/ps3/system-bus.c
@@ -382,7 +382,6 @@ static int ps3_system_bus_probe(struct device *_dev)
 
 static int ps3_system_bus_remove(struct device *_dev)
 {
-	int result = 0;
 	struct ps3_system_bus_device *dev = ps3_dev_to_system_bus_dev(_dev);
 	struct ps3_system_bus_driver *drv;
 
@@ -393,13 +392,13 @@ static int ps3_system_bus_remove(struct device *_dev)
 	BUG_ON(!drv);
 
 	if (drv->remove)
-		result = drv->remove(dev);
+		drv->remove(dev);
 	else
 		dev_dbg(&dev->core, "%s:%d %s: no remove method\n",
 			__func__, __LINE__, drv->core.name);
 
 	pr_debug(" <- %s:%d: %s\n", __func__, __LINE__, dev_name(&dev->core));
-	return result;
+	return 0;
 }
 
 static void ps3_system_bus_shutdown(struct device *_dev)
diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
index 7b55811c2a81..ba3ece56cbb3 100644
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -507,7 +507,7 @@ static int ps3disk_probe(struct ps3_system_bus_device *_dev)
 	return error;
 }
 
-static int ps3disk_remove(struct ps3_system_bus_device *_dev)
+static void ps3disk_remove(struct ps3_system_bus_device *_dev)
 {
 	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
 	struct ps3disk_private *priv = ps3_system_bus_get_drvdata(&dev->sbd);
@@ -526,7 +526,6 @@ static int ps3disk_remove(struct ps3_system_bus_device *_dev)
 	kfree(dev->bounce_buf);
 	kfree(priv);
 	ps3_system_bus_set_drvdata(_dev, NULL);
-	return 0;
 }
 
 static struct ps3_system_bus_driver ps3disk = {
diff --git a/drivers/block/ps3vram.c b/drivers/block/ps3vram.c
index 1088798c8dd0..b71d28372ef3 100644
--- a/drivers/block/ps3vram.c
+++ b/drivers/block/ps3vram.c
@@ -797,7 +797,7 @@ static int ps3vram_probe(struct ps3_system_bus_device *dev)
 	return error;
 }
 
-static int ps3vram_remove(struct ps3_system_bus_device *dev)
+static void ps3vram_remove(struct ps3_system_bus_device *dev)
 {
 	struct ps3vram_priv *priv = ps3_system_bus_get_drvdata(dev);
 
@@ -817,7 +817,6 @@ static int ps3vram_remove(struct ps3_system_bus_device *dev)
 	free_pages((unsigned long) priv->xdr_buf, get_order(XDR_BUF_SIZE));
 	kfree(priv);
 	ps3_system_bus_set_drvdata(dev, NULL);
-	return 0;
 }
 
 static struct ps3_system_bus_driver ps3vram = {
diff --git a/drivers/char/ps3flash.c b/drivers/char/ps3flash.c
index 1a07fee33f66..23871cde41fb 100644
--- a/drivers/char/ps3flash.c
+++ b/drivers/char/ps3flash.c
@@ -403,7 +403,7 @@ static int ps3flash_probe(struct ps3_system_bus_device *_dev)
 	return error;
 }
 
-static int ps3flash_remove(struct ps3_system_bus_device *_dev)
+static void ps3flash_remove(struct ps3_system_bus_device *_dev)
 {
 	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
 
@@ -413,7 +413,6 @@ static int ps3flash_remove(struct ps3_system_bus_device *_dev)
 	kfree(ps3_system_bus_get_drvdata(&dev->sbd));
 	ps3_system_bus_set_drvdata(&dev->sbd, NULL);
 	ps3flash_dev = NULL;
-	return 0;
 }
 
 
diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
index d9a5722f561b..3d1fc8d2ca66 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
@@ -1791,7 +1791,7 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *dev)
  * ps3_gelic_driver_remove - remove a device from the control of this driver
  */
 
-static int ps3_gelic_driver_remove(struct ps3_system_bus_device *dev)
+static void ps3_gelic_driver_remove(struct ps3_system_bus_device *dev)
 {
 	struct gelic_card *card = ps3_system_bus_get_drvdata(dev);
 	struct net_device *netdev0;
@@ -1840,7 +1840,6 @@ static int ps3_gelic_driver_remove(struct ps3_system_bus_device *dev)
 	ps3_close_hv_device(dev);
 
 	pr_debug("%s: done\n", __func__);
-	return 0;
 }
 
 static struct ps3_system_bus_driver ps3_gelic_driver = {
diff --git a/drivers/ps3/ps3-lpm.c b/drivers/ps3/ps3-lpm.c
index e54aa2d82f50..65512b6cc6fd 100644
--- a/drivers/ps3/ps3-lpm.c
+++ b/drivers/ps3/ps3-lpm.c
@@ -1196,7 +1196,7 @@ static int ps3_lpm_probe(struct ps3_system_bus_device *dev)
 	return 0;
 }
 
-static int ps3_lpm_remove(struct ps3_system_bus_device *dev)
+static void ps3_lpm_remove(struct ps3_system_bus_device *dev)
 {
 	dev_dbg(&dev->core, " -> %s:%u:\n", __func__, __LINE__);
 
@@ -1206,7 +1206,6 @@ static int ps3_lpm_remove(struct ps3_system_bus_device *dev)
 	lpm_priv = NULL;
 
 	dev_info(&dev->core, " <- %s:%u:\n", __func__, __LINE__);
-	return 0;
 }
 
 static struct ps3_system_bus_driver ps3_lpm_driver = {
diff --git a/drivers/ps3/ps3-vuart.c b/drivers/ps3/ps3-vuart.c
index 4ed131eaff51..e34ae6a442c7 100644
--- a/drivers/ps3/ps3-vuart.c
+++ b/drivers/ps3/ps3-vuart.c
@@ -1102,7 +1102,7 @@ static int ps3_vuart_cleanup(struct ps3_system_bus_device *dev)
  * device can no longer be used.
  */
 
-static int ps3_vuart_remove(struct ps3_system_bus_device *dev)
+static void ps3_vuart_remove(struct ps3_system_bus_device *dev)
 {
 	struct ps3_vuart_port_priv *priv = to_port_priv(dev);
 	struct ps3_vuart_port_driver *drv;
@@ -1118,7 +1118,7 @@ static int ps3_vuart_remove(struct ps3_system_bus_device *dev)
 		dev_dbg(&dev->core, "%s:%d: no driver bound\n", __func__,
 			__LINE__);
 		mutex_unlock(&vuart_bus_priv.probe_mutex);
-		return 0;
+		return;
 	}
 
 	drv = ps3_system_bus_dev_to_vuart_drv(dev);
@@ -1141,7 +1141,6 @@ static int ps3_vuart_remove(struct ps3_system_bus_device *dev)
 
 	dev_dbg(&dev->core, " <- %s:%d\n", __func__, __LINE__);
 	mutex_unlock(&vuart_bus_priv.probe_mutex);
-	return 0;
 }
 
 /**
@@ -1154,7 +1153,7 @@ static int ps3_vuart_remove(struct ps3_system_bus_device *dev)
  * sequence.
  */
 
-static int ps3_vuart_shutdown(struct ps3_system_bus_device *dev)
+static void ps3_vuart_shutdown(struct ps3_system_bus_device *dev)
 {
 	struct ps3_vuart_port_driver *drv;
 
@@ -1169,7 +1168,7 @@ static int ps3_vuart_shutdown(struct ps3_system_bus_device *dev)
 		dev_dbg(&dev->core, "%s:%d: no driver bound\n", __func__,
 			__LINE__);
 		mutex_unlock(&vuart_bus_priv.probe_mutex);
-		return 0;
+		return;
 	}
 
 	drv = ps3_system_bus_dev_to_vuart_drv(dev);
@@ -1193,7 +1192,6 @@ static int ps3_vuart_shutdown(struct ps3_system_bus_device *dev)
 	dev_dbg(&dev->core, " <- %s:%d\n", __func__, __LINE__);
 
 	mutex_unlock(&vuart_bus_priv.probe_mutex);
-	return 0;
 }
 
 static int __init ps3_vuart_bus_init(void)
diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c
index f75c0b5cd587..ccb5771f1cb7 100644
--- a/drivers/scsi/ps3rom.c
+++ b/drivers/scsi/ps3rom.c
@@ -402,7 +402,7 @@ static int ps3rom_probe(struct ps3_system_bus_device *_dev)
 	return error;
 }
 
-static int ps3rom_remove(struct ps3_system_bus_device *_dev)
+static void ps3rom_remove(struct ps3_system_bus_device *_dev)
 {
 	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
 	struct Scsi_Host *host = ps3_system_bus_get_drvdata(&dev->sbd);
@@ -412,7 +412,6 @@ static int ps3rom_remove(struct ps3_system_bus_device *_dev)
 	scsi_host_put(host);
 	ps3_system_bus_set_drvdata(&dev->sbd, NULL);
 	kfree(dev->bounce_buf);
-	return 0;
 }
 
 static struct ps3_system_bus_driver ps3rom = {
diff --git a/drivers/usb/host/ehci-ps3.c b/drivers/usb/host/ehci-ps3.c
index fb52133c3557..98568b046a1a 100644
--- a/drivers/usb/host/ehci-ps3.c
+++ b/drivers/usb/host/ehci-ps3.c
@@ -200,7 +200,7 @@ static int ps3_ehci_probe(struct ps3_system_bus_device *dev)
 	return result;
 }
 
-static int ps3_ehci_remove(struct ps3_system_bus_device *dev)
+static void ps3_ehci_remove(struct ps3_system_bus_device *dev)
 {
 	unsigned int tmp;
 	struct usb_hcd *hcd = ps3_system_bus_get_drvdata(dev);
@@ -227,8 +227,6 @@ static int ps3_ehci_remove(struct ps3_system_bus_device *dev)
 
 	ps3_dma_region_free(dev->d_region);
 	ps3_close_hv_device(dev);
-
-	return 0;
 }
 
 static int __init ps3_ehci_driver_register(struct ps3_system_bus_driver *drv)
diff --git a/drivers/usb/host/ohci-ps3.c b/drivers/usb/host/ohci-ps3.c
index f77cd6af0ccf..4f5af929c3e4 100644
--- a/drivers/usb/host/ohci-ps3.c
+++ b/drivers/usb/host/ohci-ps3.c
@@ -184,7 +184,7 @@ static int ps3_ohci_probe(struct ps3_system_bus_device *dev)
 	return result;
 }
 
-static int ps3_ohci_remove(struct ps3_system_bus_device *dev)
+static void ps3_ohci_remove(struct ps3_system_bus_device *dev)
 {
 	unsigned int tmp;
 	struct usb_hcd *hcd = ps3_system_bus_get_drvdata(dev);
@@ -212,8 +212,6 @@ static int ps3_ohci_remove(struct ps3_system_bus_device *dev)
 
 	ps3_dma_region_free(dev->d_region);
 	ps3_close_hv_device(dev);
-
-	return 0;
 }
 
 static int __init ps3_ohci_driver_register(struct ps3_system_bus_driver *drv)
diff --git a/drivers/video/fbdev/ps3fb.c b/drivers/video/fbdev/ps3fb.c
index 203c254f8f6c..2fe08b67eda7 100644
--- a/drivers/video/fbdev/ps3fb.c
+++ b/drivers/video/fbdev/ps3fb.c
@@ -1208,7 +1208,7 @@ static int ps3fb_probe(struct ps3_system_bus_device *dev)
 	return retval;
 }
 
-static int ps3fb_shutdown(struct ps3_system_bus_device *dev)
+static void ps3fb_shutdown(struct ps3_system_bus_device *dev)
 {
 	struct fb_info *info = ps3_system_bus_get_drvdata(dev);
 	u64 xdr_lpar = ps3_mm_phys_to_lpar(__pa(ps3fb_videomemory.address));
@@ -1241,8 +1241,6 @@ static int ps3fb_shutdown(struct ps3_system_bus_device *dev)
 	lv1_gpu_memory_free(ps3fb.memory_handle);
 	ps3_close_hv_device(dev);
 	dev_dbg(&dev->core, " <- %s:%d\n", __func__, __LINE__);
-
-	return 0;
 }
 
 static struct ps3_system_bus_driver ps3fb_driver = {
diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
index 6ab796a5d936..8e44fa5d4dc7 100644
--- a/sound/ppc/snd_ps3.c
+++ b/sound/ppc/snd_ps3.c
@@ -1049,7 +1049,7 @@ static int snd_ps3_driver_probe(struct ps3_system_bus_device *dev)
 }; /* snd_ps3_probe */
 
 /* called when module removal */
-static int snd_ps3_driver_remove(struct ps3_system_bus_device *dev)
+static void snd_ps3_driver_remove(struct ps3_system_bus_device *dev)
 {
 	int ret;
 	pr_info("%s:start id=%d\n", __func__,  dev->match_id);
@@ -1075,7 +1075,6 @@ static int snd_ps3_driver_remove(struct ps3_system_bus_device *dev)
 	lv1_gpu_device_unmap(2);
 	ps3_close_hv_device(dev);
 	pr_info("%s:end id=%d\n", __func__, dev->match_id);
-	return 0;
 } /* snd_ps3_remove */
 
 static struct ps3_system_bus_driver snd_ps3_bus_driver_info = {
-- 
2.29.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ALSA: ppc: drop if block with always false condition
  2020-11-26 16:59 [PATCH 1/2] ALSA: ppc: drop if block with always false condition Uwe Kleine-König
  2020-11-26 16:59 ` [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void Uwe Kleine-König
@ 2020-11-27  8:35 ` Geert Uytterhoeven
  2020-11-27  9:45   ` Uwe Kleine-König
  2020-11-27 19:37 ` Geoff Levand
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Geert Uytterhoeven @ 2020-11-27  8:35 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Geoff Levand, Jaroslav Kysela, Takashi Iwai, Michael Ellerman,
	Jens Axboe, Jim Paris, Arnd Bergmann, Greg Kroah-Hartman,
	David S. Miller, Jakub Kicinski, James E.J. Bottomley,
	Martin K. Petersen, Alan Stern, Bartlomiej Zolnierkiewicz,
	Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev,
	ALSA Development Mailing List, linux-block, netdev, scsi,
	USB list, DRI Development, Linux Fbdev development list

Hi Uwe,

On Thu, Nov 26, 2020 at 6:03 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> The remove callback is only called for devices that were probed
> successfully before. As the matching probe function cannot complete
> without error if dev->match_id != PS3_MATCH_ID_SOUND, we don't have to
> check this here.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

Note that there are similar checks in snd_ps3_driver_probe(), which
can be removed, too:

        if (WARN_ON(!firmware_has_feature(FW_FEATURE_PS3_LV1)))
                return -ENODEV;
        if (WARN_ON(dev->match_id != PS3_MATCH_ID_SOUND))
                return -ENODEV;

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void
  2020-11-26 16:59 ` [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void Uwe Kleine-König
@ 2020-11-27  8:40   ` Geert Uytterhoeven
  2020-11-27 19:39   ` Geoff Levand
  2020-11-28  8:48   ` Takashi Iwai
  2 siblings, 0 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2020-11-27  8:40 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Geoff Levand, Jaroslav Kysela, Takashi Iwai, Michael Ellerman,
	Jens Axboe, Jim Paris, Arnd Bergmann, Greg Kroah-Hartman,
	David S. Miller, Jakub Kicinski, James E.J. Bottomley,
	Martin K. Petersen, Alan Stern, Bartlomiej Zolnierkiewicz,
	Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev,
	ALSA Development Mailing List, linux-block, netdev, scsi,
	USB list, DRI Development, Linux Fbdev development list

Hi Uwe,

On Thu, Nov 26, 2020 at 6:03 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> The driver core ignores the return value of struct device_driver::remove
> because there is only little that can be done. For the shutdown callback
> it's ps3_system_bus_shutdown() which ignores the return value.
>
> To simplify the quest to make struct device_driver::remove return void,
> let struct ps3_system_bus_driver::remove return void, too. All users
> already unconditionally return 0, this commit makes it obvious that
> returning an error code is a bad idea and ensures future users behave
> accordingly.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

Note that the same can be done for ps3_vuart_port_driver.remove().

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ALSA: ppc: drop if block with always false condition
  2020-11-27  8:35 ` [PATCH 1/2] ALSA: ppc: drop if block with always false condition Geert Uytterhoeven
@ 2020-11-27  9:45   ` Uwe Kleine-König
  0 siblings, 0 replies; 13+ messages in thread
From: Uwe Kleine-König @ 2020-11-27  9:45 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Geoff Levand, Jaroslav Kysela, Takashi Iwai, Michael Ellerman,
	Jens Axboe, Jim Paris, Arnd Bergmann, Greg Kroah-Hartman,
	David S. Miller, Jakub Kicinski, James E.J. Bottomley,
	Martin K. Petersen, Alan Stern, Bartlomiej Zolnierkiewicz,
	Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev,
	ALSA Development Mailing List, linux-block, netdev, scsi,
	USB list, DRI Development, Linux Fbdev development list


[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]

On Fri, Nov 27, 2020 at 09:35:39AM +0100, Geert Uytterhoeven wrote:
> Hi Uwe,
> 
> On Thu, Nov 26, 2020 at 6:03 PM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> > The remove callback is only called for devices that were probed
> > successfully before. As the matching probe function cannot complete
> > without error if dev->match_id != PS3_MATCH_ID_SOUND, we don't have to
> > check this here.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> Thanks for your patch!
> 
> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
> 
> Note that there are similar checks in snd_ps3_driver_probe(), which
> can be removed, too:
> 
>         if (WARN_ON(!firmware_has_feature(FW_FEATURE_PS3_LV1)))
>                 return -ENODEV;
>         if (WARN_ON(dev->match_id != PS3_MATCH_ID_SOUND))
>                 return -ENODEV;

I had to invest some brain cycles here. For the first:

Assuming firmware_has_feature(FW_FEATURE_PS3_LV1) always returns the
same value, snd_ps3_driver_probe is only used after this check succeeds
because the driver is registered only after this check in
snd_ps3_init().

The second is superflous because ps3_system_bus_match() yields false if
this doesn't match the driver's match_id.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ALSA: ppc: drop if block with always false condition
  2020-11-26 16:59 [PATCH 1/2] ALSA: ppc: drop if block with always false condition Uwe Kleine-König
  2020-11-26 16:59 ` [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void Uwe Kleine-König
  2020-11-27  8:35 ` [PATCH 1/2] ALSA: ppc: drop if block with always false condition Geert Uytterhoeven
@ 2020-11-27 19:37 ` Geoff Levand
  2020-11-28  8:48 ` Takashi Iwai
  2020-12-10 11:30 ` Michael Ellerman
  4 siblings, 0 replies; 13+ messages in thread
From: Geoff Levand @ 2020-11-27 19:37 UTC (permalink / raw)
  To: Uwe Kleine-König, Jaroslav Kysela, Takashi Iwai,
	Michael Ellerman, Jens Axboe, Jim Paris, Arnd Bergmann,
	Greg Kroah-Hartman, David S. Miller, Jakub Kicinski,
	James E.J. Bottomley, Martin K. Petersen, Alan Stern,
	Bartlomiej Zolnierkiewicz, Leonard Goehrs
  Cc: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev, alsa-devel,
	linux-block, netdev, linux-scsi, linux-usb, dri-devel,
	linux-fbdev, Geert Uytterhoeven

Hi Uwe,

On 11/26/20 8:59 AM, Uwe Kleine-König wrote:
> The remove callback is only called for devices that were probed
> successfully before. As the matching probe function cannot complete
> without error if dev->match_id != PS3_MATCH_ID_SOUND, we don't have to
> check this here.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

I tested your two patches plus Leonard's patch 'ALSA: ppc: remove
redundant checks in PS3 driver probe' applied to v5.9 on the PS3,
and they seem to work fine.

Thanks for both your efforts.

Tested by: Geoff Levand <geoff@infradead.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void
  2020-11-26 16:59 ` [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void Uwe Kleine-König
  2020-11-27  8:40   ` Geert Uytterhoeven
@ 2020-11-27 19:39   ` Geoff Levand
  2020-11-28  8:48   ` Takashi Iwai
  2 siblings, 0 replies; 13+ messages in thread
From: Geoff Levand @ 2020-11-27 19:39 UTC (permalink / raw)
  To: Uwe Kleine-König, Jaroslav Kysela, Takashi Iwai,
	Michael Ellerman, Jens Axboe, Jim Paris, Arnd Bergmann,
	Greg Kroah-Hartman, David S. Miller, Jakub Kicinski,
	James E.J. Bottomley, Martin K. Petersen, Alan Stern,
	Bartlomiej Zolnierkiewicz
  Cc: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev, alsa-devel,
	linux-block, netdev, linux-scsi, linux-usb, dri-devel,
	linux-fbdev

On 11/26/20 8:59 AM, Uwe Kleine-König wrote:
> The driver core ignores the return value of struct device_driver::remove
> because there is only little that can be done. For the shutdown callback
> it's ps3_system_bus_shutdown() which ignores the return value.
> 
> To simplify the quest to make struct device_driver::remove return void,
> let struct ps3_system_bus_driver::remove return void, too. All users
> already unconditionally return 0, this commit makes it obvious that
> returning an error code is a bad idea and ensures future users behave
> accordingly.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Seems OK with v5.9 on PS3.

Tested by: Geoff Levand <geoff@infradead.org>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ALSA: ppc: drop if block with always false condition
  2020-11-26 16:59 [PATCH 1/2] ALSA: ppc: drop if block with always false condition Uwe Kleine-König
                   ` (2 preceding siblings ...)
  2020-11-27 19:37 ` Geoff Levand
@ 2020-11-28  8:48 ` Takashi Iwai
  2020-12-10 11:30 ` Michael Ellerman
  4 siblings, 0 replies; 13+ messages in thread
From: Takashi Iwai @ 2020-11-28  8:48 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Geoff Levand, Jaroslav Kysela, Takashi Iwai, Michael Ellerman,
	Jens Axboe, Jim Paris, Arnd Bergmann, Greg Kroah-Hartman,
	David S. Miller, Jakub Kicinski, James E.J. Bottomley,
	Martin K. Petersen, Alan Stern, Bartlomiej Zolnierkiewicz,
	Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev, alsa-devel,
	linux-block, netdev, linux-scsi, linux-usb, dri-devel,
	linux-fbdev

On Thu, 26 Nov 2020 17:59:49 +0100,
Uwe Kleine-König wrote:
> 
> The remove callback is only called for devices that were probed
> successfully before. As the matching probe function cannot complete
> without error if dev->match_id != PS3_MATCH_ID_SOUND, we don't have to
> check this here.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied this one now.  Thanks.


Takashi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void
  2020-11-26 16:59 ` [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void Uwe Kleine-König
  2020-11-27  8:40   ` Geert Uytterhoeven
  2020-11-27 19:39   ` Geoff Levand
@ 2020-11-28  8:48   ` Takashi Iwai
  2020-11-29 17:31     ` Uwe Kleine-König
  2 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2020-11-28  8:48 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Geoff Levand, Jaroslav Kysela, Takashi Iwai, Michael Ellerman,
	Jens Axboe, Jim Paris, Arnd Bergmann, Greg Kroah-Hartman,
	David S. Miller, Jakub Kicinski, James E.J. Bottomley,
	Martin K. Petersen, Alan Stern, Bartlomiej Zolnierkiewicz,
	Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev, alsa-devel,
	linux-block, netdev, linux-scsi, linux-usb, dri-devel,
	linux-fbdev

On Thu, 26 Nov 2020 17:59:50 +0100,
Uwe Kleine-König wrote:
> 
> The driver core ignores the return value of struct device_driver::remove
> because there is only little that can be done. For the shutdown callback
> it's ps3_system_bus_shutdown() which ignores the return value.
> 
> To simplify the quest to make struct device_driver::remove return void,
> let struct ps3_system_bus_driver::remove return void, too. All users
> already unconditionally return 0, this commit makes it obvious that
> returning an error code is a bad idea and ensures future users behave
> accordingly.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

For the sound bit:
Acked-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

> ---
>  arch/powerpc/include/asm/ps3.h               |  4 ++--
>  arch/powerpc/platforms/ps3/system-bus.c      |  5 ++---
>  drivers/block/ps3disk.c                      |  3 +--
>  drivers/block/ps3vram.c                      |  3 +--
>  drivers/char/ps3flash.c                      |  3 +--
>  drivers/net/ethernet/toshiba/ps3_gelic_net.c |  3 +--
>  drivers/ps3/ps3-lpm.c                        |  3 +--
>  drivers/ps3/ps3-vuart.c                      | 10 ++++------
>  drivers/scsi/ps3rom.c                        |  3 +--
>  drivers/usb/host/ehci-ps3.c                  |  4 +---
>  drivers/usb/host/ohci-ps3.c                  |  4 +---
>  drivers/video/fbdev/ps3fb.c                  |  4 +---
>  sound/ppc/snd_ps3.c                          |  3 +--
>  13 files changed, 18 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/ps3.h b/arch/powerpc/include/asm/ps3.h
> index cb89e4bf55ce..e646c7f218bc 100644
> --- a/arch/powerpc/include/asm/ps3.h
> +++ b/arch/powerpc/include/asm/ps3.h
> @@ -378,8 +378,8 @@ struct ps3_system_bus_driver {
>  	enum ps3_match_sub_id match_sub_id;
>  	struct device_driver core;
>  	int (*probe)(struct ps3_system_bus_device *);
> -	int (*remove)(struct ps3_system_bus_device *);
> -	int (*shutdown)(struct ps3_system_bus_device *);
> +	void (*remove)(struct ps3_system_bus_device *);
> +	void (*shutdown)(struct ps3_system_bus_device *);
>  /*	int (*suspend)(struct ps3_system_bus_device *, pm_message_t); */
>  /*	int (*resume)(struct ps3_system_bus_device *); */
>  };
> diff --git a/arch/powerpc/platforms/ps3/system-bus.c b/arch/powerpc/platforms/ps3/system-bus.c
> index c62aaa29a9d5..b431f41c6cb5 100644
> --- a/arch/powerpc/platforms/ps3/system-bus.c
> +++ b/arch/powerpc/platforms/ps3/system-bus.c
> @@ -382,7 +382,6 @@ static int ps3_system_bus_probe(struct device *_dev)
>  
>  static int ps3_system_bus_remove(struct device *_dev)
>  {
> -	int result = 0;
>  	struct ps3_system_bus_device *dev = ps3_dev_to_system_bus_dev(_dev);
>  	struct ps3_system_bus_driver *drv;
>  
> @@ -393,13 +392,13 @@ static int ps3_system_bus_remove(struct device *_dev)
>  	BUG_ON(!drv);
>  
>  	if (drv->remove)
> -		result = drv->remove(dev);
> +		drv->remove(dev);
>  	else
>  		dev_dbg(&dev->core, "%s:%d %s: no remove method\n",
>  			__func__, __LINE__, drv->core.name);
>  
>  	pr_debug(" <- %s:%d: %s\n", __func__, __LINE__, dev_name(&dev->core));
> -	return result;
> +	return 0;
>  }
>  
>  static void ps3_system_bus_shutdown(struct device *_dev)
> diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
> index 7b55811c2a81..ba3ece56cbb3 100644
> --- a/drivers/block/ps3disk.c
> +++ b/drivers/block/ps3disk.c
> @@ -507,7 +507,7 @@ static int ps3disk_probe(struct ps3_system_bus_device *_dev)
>  	return error;
>  }
>  
> -static int ps3disk_remove(struct ps3_system_bus_device *_dev)
> +static void ps3disk_remove(struct ps3_system_bus_device *_dev)
>  {
>  	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
>  	struct ps3disk_private *priv = ps3_system_bus_get_drvdata(&dev->sbd);
> @@ -526,7 +526,6 @@ static int ps3disk_remove(struct ps3_system_bus_device *_dev)
>  	kfree(dev->bounce_buf);
>  	kfree(priv);
>  	ps3_system_bus_set_drvdata(_dev, NULL);
> -	return 0;
>  }
>  
>  static struct ps3_system_bus_driver ps3disk = {
> diff --git a/drivers/block/ps3vram.c b/drivers/block/ps3vram.c
> index 1088798c8dd0..b71d28372ef3 100644
> --- a/drivers/block/ps3vram.c
> +++ b/drivers/block/ps3vram.c
> @@ -797,7 +797,7 @@ static int ps3vram_probe(struct ps3_system_bus_device *dev)
>  	return error;
>  }
>  
> -static int ps3vram_remove(struct ps3_system_bus_device *dev)
> +static void ps3vram_remove(struct ps3_system_bus_device *dev)
>  {
>  	struct ps3vram_priv *priv = ps3_system_bus_get_drvdata(dev);
>  
> @@ -817,7 +817,6 @@ static int ps3vram_remove(struct ps3_system_bus_device *dev)
>  	free_pages((unsigned long) priv->xdr_buf, get_order(XDR_BUF_SIZE));
>  	kfree(priv);
>  	ps3_system_bus_set_drvdata(dev, NULL);
> -	return 0;
>  }
>  
>  static struct ps3_system_bus_driver ps3vram = {
> diff --git a/drivers/char/ps3flash.c b/drivers/char/ps3flash.c
> index 1a07fee33f66..23871cde41fb 100644
> --- a/drivers/char/ps3flash.c
> +++ b/drivers/char/ps3flash.c
> @@ -403,7 +403,7 @@ static int ps3flash_probe(struct ps3_system_bus_device *_dev)
>  	return error;
>  }
>  
> -static int ps3flash_remove(struct ps3_system_bus_device *_dev)
> +static void ps3flash_remove(struct ps3_system_bus_device *_dev)
>  {
>  	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
>  
> @@ -413,7 +413,6 @@ static int ps3flash_remove(struct ps3_system_bus_device *_dev)
>  	kfree(ps3_system_bus_get_drvdata(&dev->sbd));
>  	ps3_system_bus_set_drvdata(&dev->sbd, NULL);
>  	ps3flash_dev = NULL;
> -	return 0;
>  }
>  
>  
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> index d9a5722f561b..3d1fc8d2ca66 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> @@ -1791,7 +1791,7 @@ static int ps3_gelic_driver_probe(struct ps3_system_bus_device *dev)
>   * ps3_gelic_driver_remove - remove a device from the control of this driver
>   */
>  
> -static int ps3_gelic_driver_remove(struct ps3_system_bus_device *dev)
> +static void ps3_gelic_driver_remove(struct ps3_system_bus_device *dev)
>  {
>  	struct gelic_card *card = ps3_system_bus_get_drvdata(dev);
>  	struct net_device *netdev0;
> @@ -1840,7 +1840,6 @@ static int ps3_gelic_driver_remove(struct ps3_system_bus_device *dev)
>  	ps3_close_hv_device(dev);
>  
>  	pr_debug("%s: done\n", __func__);
> -	return 0;
>  }
>  
>  static struct ps3_system_bus_driver ps3_gelic_driver = {
> diff --git a/drivers/ps3/ps3-lpm.c b/drivers/ps3/ps3-lpm.c
> index e54aa2d82f50..65512b6cc6fd 100644
> --- a/drivers/ps3/ps3-lpm.c
> +++ b/drivers/ps3/ps3-lpm.c
> @@ -1196,7 +1196,7 @@ static int ps3_lpm_probe(struct ps3_system_bus_device *dev)
>  	return 0;
>  }
>  
> -static int ps3_lpm_remove(struct ps3_system_bus_device *dev)
> +static void ps3_lpm_remove(struct ps3_system_bus_device *dev)
>  {
>  	dev_dbg(&dev->core, " -> %s:%u:\n", __func__, __LINE__);
>  
> @@ -1206,7 +1206,6 @@ static int ps3_lpm_remove(struct ps3_system_bus_device *dev)
>  	lpm_priv = NULL;
>  
>  	dev_info(&dev->core, " <- %s:%u:\n", __func__, __LINE__);
> -	return 0;
>  }
>  
>  static struct ps3_system_bus_driver ps3_lpm_driver = {
> diff --git a/drivers/ps3/ps3-vuart.c b/drivers/ps3/ps3-vuart.c
> index 4ed131eaff51..e34ae6a442c7 100644
> --- a/drivers/ps3/ps3-vuart.c
> +++ b/drivers/ps3/ps3-vuart.c
> @@ -1102,7 +1102,7 @@ static int ps3_vuart_cleanup(struct ps3_system_bus_device *dev)
>   * device can no longer be used.
>   */
>  
> -static int ps3_vuart_remove(struct ps3_system_bus_device *dev)
> +static void ps3_vuart_remove(struct ps3_system_bus_device *dev)
>  {
>  	struct ps3_vuart_port_priv *priv = to_port_priv(dev);
>  	struct ps3_vuart_port_driver *drv;
> @@ -1118,7 +1118,7 @@ static int ps3_vuart_remove(struct ps3_system_bus_device *dev)
>  		dev_dbg(&dev->core, "%s:%d: no driver bound\n", __func__,
>  			__LINE__);
>  		mutex_unlock(&vuart_bus_priv.probe_mutex);
> -		return 0;
> +		return;
>  	}
>  
>  	drv = ps3_system_bus_dev_to_vuart_drv(dev);
> @@ -1141,7 +1141,6 @@ static int ps3_vuart_remove(struct ps3_system_bus_device *dev)
>  
>  	dev_dbg(&dev->core, " <- %s:%d\n", __func__, __LINE__);
>  	mutex_unlock(&vuart_bus_priv.probe_mutex);
> -	return 0;
>  }
>  
>  /**
> @@ -1154,7 +1153,7 @@ static int ps3_vuart_remove(struct ps3_system_bus_device *dev)
>   * sequence.
>   */
>  
> -static int ps3_vuart_shutdown(struct ps3_system_bus_device *dev)
> +static void ps3_vuart_shutdown(struct ps3_system_bus_device *dev)
>  {
>  	struct ps3_vuart_port_driver *drv;
>  
> @@ -1169,7 +1168,7 @@ static int ps3_vuart_shutdown(struct ps3_system_bus_device *dev)
>  		dev_dbg(&dev->core, "%s:%d: no driver bound\n", __func__,
>  			__LINE__);
>  		mutex_unlock(&vuart_bus_priv.probe_mutex);
> -		return 0;
> +		return;
>  	}
>  
>  	drv = ps3_system_bus_dev_to_vuart_drv(dev);
> @@ -1193,7 +1192,6 @@ static int ps3_vuart_shutdown(struct ps3_system_bus_device *dev)
>  	dev_dbg(&dev->core, " <- %s:%d\n", __func__, __LINE__);
>  
>  	mutex_unlock(&vuart_bus_priv.probe_mutex);
> -	return 0;
>  }
>  
>  static int __init ps3_vuart_bus_init(void)
> diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c
> index f75c0b5cd587..ccb5771f1cb7 100644
> --- a/drivers/scsi/ps3rom.c
> +++ b/drivers/scsi/ps3rom.c
> @@ -402,7 +402,7 @@ static int ps3rom_probe(struct ps3_system_bus_device *_dev)
>  	return error;
>  }
>  
> -static int ps3rom_remove(struct ps3_system_bus_device *_dev)
> +static void ps3rom_remove(struct ps3_system_bus_device *_dev)
>  {
>  	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
>  	struct Scsi_Host *host = ps3_system_bus_get_drvdata(&dev->sbd);
> @@ -412,7 +412,6 @@ static int ps3rom_remove(struct ps3_system_bus_device *_dev)
>  	scsi_host_put(host);
>  	ps3_system_bus_set_drvdata(&dev->sbd, NULL);
>  	kfree(dev->bounce_buf);
> -	return 0;
>  }
>  
>  static struct ps3_system_bus_driver ps3rom = {
> diff --git a/drivers/usb/host/ehci-ps3.c b/drivers/usb/host/ehci-ps3.c
> index fb52133c3557..98568b046a1a 100644
> --- a/drivers/usb/host/ehci-ps3.c
> +++ b/drivers/usb/host/ehci-ps3.c
> @@ -200,7 +200,7 @@ static int ps3_ehci_probe(struct ps3_system_bus_device *dev)
>  	return result;
>  }
>  
> -static int ps3_ehci_remove(struct ps3_system_bus_device *dev)
> +static void ps3_ehci_remove(struct ps3_system_bus_device *dev)
>  {
>  	unsigned int tmp;
>  	struct usb_hcd *hcd = ps3_system_bus_get_drvdata(dev);
> @@ -227,8 +227,6 @@ static int ps3_ehci_remove(struct ps3_system_bus_device *dev)
>  
>  	ps3_dma_region_free(dev->d_region);
>  	ps3_close_hv_device(dev);
> -
> -	return 0;
>  }
>  
>  static int __init ps3_ehci_driver_register(struct ps3_system_bus_driver *drv)
> diff --git a/drivers/usb/host/ohci-ps3.c b/drivers/usb/host/ohci-ps3.c
> index f77cd6af0ccf..4f5af929c3e4 100644
> --- a/drivers/usb/host/ohci-ps3.c
> +++ b/drivers/usb/host/ohci-ps3.c
> @@ -184,7 +184,7 @@ static int ps3_ohci_probe(struct ps3_system_bus_device *dev)
>  	return result;
>  }
>  
> -static int ps3_ohci_remove(struct ps3_system_bus_device *dev)
> +static void ps3_ohci_remove(struct ps3_system_bus_device *dev)
>  {
>  	unsigned int tmp;
>  	struct usb_hcd *hcd = ps3_system_bus_get_drvdata(dev);
> @@ -212,8 +212,6 @@ static int ps3_ohci_remove(struct ps3_system_bus_device *dev)
>  
>  	ps3_dma_region_free(dev->d_region);
>  	ps3_close_hv_device(dev);
> -
> -	return 0;
>  }
>  
>  static int __init ps3_ohci_driver_register(struct ps3_system_bus_driver *drv)
> diff --git a/drivers/video/fbdev/ps3fb.c b/drivers/video/fbdev/ps3fb.c
> index 203c254f8f6c..2fe08b67eda7 100644
> --- a/drivers/video/fbdev/ps3fb.c
> +++ b/drivers/video/fbdev/ps3fb.c
> @@ -1208,7 +1208,7 @@ static int ps3fb_probe(struct ps3_system_bus_device *dev)
>  	return retval;
>  }
>  
> -static int ps3fb_shutdown(struct ps3_system_bus_device *dev)
> +static void ps3fb_shutdown(struct ps3_system_bus_device *dev)
>  {
>  	struct fb_info *info = ps3_system_bus_get_drvdata(dev);
>  	u64 xdr_lpar = ps3_mm_phys_to_lpar(__pa(ps3fb_videomemory.address));
> @@ -1241,8 +1241,6 @@ static int ps3fb_shutdown(struct ps3_system_bus_device *dev)
>  	lv1_gpu_memory_free(ps3fb.memory_handle);
>  	ps3_close_hv_device(dev);
>  	dev_dbg(&dev->core, " <- %s:%d\n", __func__, __LINE__);
> -
> -	return 0;
>  }
>  
>  static struct ps3_system_bus_driver ps3fb_driver = {
> diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
> index 6ab796a5d936..8e44fa5d4dc7 100644
> --- a/sound/ppc/snd_ps3.c
> +++ b/sound/ppc/snd_ps3.c
> @@ -1049,7 +1049,7 @@ static int snd_ps3_driver_probe(struct ps3_system_bus_device *dev)
>  }; /* snd_ps3_probe */
>  
>  /* called when module removal */
> -static int snd_ps3_driver_remove(struct ps3_system_bus_device *dev)
> +static void snd_ps3_driver_remove(struct ps3_system_bus_device *dev)
>  {
>  	int ret;
>  	pr_info("%s:start id=%d\n", __func__,  dev->match_id);
> @@ -1075,7 +1075,6 @@ static int snd_ps3_driver_remove(struct ps3_system_bus_device *dev)
>  	lv1_gpu_device_unmap(2);
>  	ps3_close_hv_device(dev);
>  	pr_info("%s:end id=%d\n", __func__, dev->match_id);
> -	return 0;
>  } /* snd_ps3_remove */
>  
>  static struct ps3_system_bus_driver snd_ps3_bus_driver_info = {
> -- 
> 2.29.2
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void
  2020-11-28  8:48   ` Takashi Iwai
@ 2020-11-29 17:31     ` Uwe Kleine-König
  2020-12-02 12:14       ` Michael Ellerman
  0 siblings, 1 reply; 13+ messages in thread
From: Uwe Kleine-König @ 2020-11-29 17:31 UTC (permalink / raw)
  To: Takashi Iwai, Michael Ellerman
  Cc: Geoff Levand, Jaroslav Kysela, Takashi Iwai, Jens Axboe,
	Jim Paris, Arnd Bergmann, Greg Kroah-Hartman, David S. Miller,
	Jakub Kicinski, James E.J. Bottomley, Martin K. Petersen,
	Alan Stern, Bartlomiej Zolnierkiewicz, Benjamin Herrenschmidt,
	Paul Mackerras, linuxppc-dev, alsa-devel, linux-block, netdev,
	linux-scsi, linux-usb, dri-devel, linux-fbdev


[-- Attachment #1: Type: text/plain, Size: 1334 bytes --]

Hello Michael,

On Sat, Nov 28, 2020 at 09:48:30AM +0100, Takashi Iwai wrote:
> On Thu, 26 Nov 2020 17:59:50 +0100,
> Uwe Kleine-König wrote:
> > 
> > The driver core ignores the return value of struct device_driver::remove
> > because there is only little that can be done. For the shutdown callback
> > it's ps3_system_bus_shutdown() which ignores the return value.
> > 
> > To simplify the quest to make struct device_driver::remove return void,
> > let struct ps3_system_bus_driver::remove return void, too. All users
> > already unconditionally return 0, this commit makes it obvious that
> > returning an error code is a bad idea and ensures future users behave
> > accordingly.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> For the sound bit:
> Acked-by: Takashi Iwai <tiwai@suse.de>

assuming that you are the one who will apply this patch: Note that it
depends on patch 1 that Takashi already applied to his tree. So you
either have to wait untils patch 1 appears in some tree that you merge
before applying, or you have to take patch 1, too. (With Takashi
optinally dropping it then.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void
  2020-11-29 17:31     ` Uwe Kleine-König
@ 2020-12-02 12:14       ` Michael Ellerman
  2020-12-02 12:22         ` Takashi Iwai
  0 siblings, 1 reply; 13+ messages in thread
From: Michael Ellerman @ 2020-12-02 12:14 UTC (permalink / raw)
  To: Uwe Kleine-König, Takashi Iwai
  Cc: Geoff Levand, Jaroslav Kysela, Takashi Iwai, Jens Axboe,
	Jim Paris, Arnd Bergmann, Greg Kroah-Hartman, David S. Miller,
	Jakub Kicinski, James E.J. Bottomley, Martin K. Petersen,
	Alan Stern, Bartlomiej Zolnierkiewicz, Benjamin Herrenschmidt,
	Paul Mackerras, linuxppc-dev, alsa-devel, linux-block, netdev,
	linux-scsi, linux-usb, dri-devel, linux-fbdev

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
> Hello Michael,
>
> On Sat, Nov 28, 2020 at 09:48:30AM +0100, Takashi Iwai wrote:
>> On Thu, 26 Nov 2020 17:59:50 +0100,
>> Uwe Kleine-König wrote:
>> > 
>> > The driver core ignores the return value of struct device_driver::remove
>> > because there is only little that can be done. For the shutdown callback
>> > it's ps3_system_bus_shutdown() which ignores the return value.
>> > 
>> > To simplify the quest to make struct device_driver::remove return void,
>> > let struct ps3_system_bus_driver::remove return void, too. All users
>> > already unconditionally return 0, this commit makes it obvious that
>> > returning an error code is a bad idea and ensures future users behave
>> > accordingly.
>> > 
>> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> 
>> For the sound bit:
>> Acked-by: Takashi Iwai <tiwai@suse.de>
>
> assuming that you are the one who will apply this patch: Note that it
> depends on patch 1 that Takashi already applied to his tree. So you
> either have to wait untils patch 1 appears in some tree that you merge
> before applying, or you have to take patch 1, too. (With Takashi
> optinally dropping it then.)

Thanks. I've picked up both patches.

If Takashi doesn't want to rebase his tree to drop patch 1 that's OK, it
will just arrive in mainline via two paths, but git should handle it.

cheers

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void
  2020-12-02 12:14       ` Michael Ellerman
@ 2020-12-02 12:22         ` Takashi Iwai
  0 siblings, 0 replies; 13+ messages in thread
From: Takashi Iwai @ 2020-12-02 12:22 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Uwe Kleine-König, Geoff Levand, Jaroslav Kysela,
	Takashi Iwai, Jens Axboe, Jim Paris, Arnd Bergmann,
	Greg Kroah-Hartman, David S. Miller, Jakub Kicinski,
	James E.J. Bottomley, Martin K. Petersen, Alan Stern,
	Bartlomiej Zolnierkiewicz, Benjamin Herrenschmidt,
	Paul Mackerras, linuxppc-dev, alsa-devel, linux-block, netdev,
	linux-scsi, linux-usb, dri-devel, linux-fbdev

On Wed, 02 Dec 2020 13:14:06 +0100,
Michael Ellerman wrote:
> 
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
> > Hello Michael,
> >
> > On Sat, Nov 28, 2020 at 09:48:30AM +0100, Takashi Iwai wrote:
> >> On Thu, 26 Nov 2020 17:59:50 +0100,
> >> Uwe Kleine-König wrote:
> >> > 
> >> > The driver core ignores the return value of struct device_driver::remove
> >> > because there is only little that can be done. For the shutdown callback
> >> > it's ps3_system_bus_shutdown() which ignores the return value.
> >> > 
> >> > To simplify the quest to make struct device_driver::remove return void,
> >> > let struct ps3_system_bus_driver::remove return void, too. All users
> >> > already unconditionally return 0, this commit makes it obvious that
> >> > returning an error code is a bad idea and ensures future users behave
> >> > accordingly.
> >> > 
> >> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >> 
> >> For the sound bit:
> >> Acked-by: Takashi Iwai <tiwai@suse.de>
> >
> > assuming that you are the one who will apply this patch: Note that it
> > depends on patch 1 that Takashi already applied to his tree. So you
> > either have to wait untils patch 1 appears in some tree that you merge
> > before applying, or you have to take patch 1, too. (With Takashi
> > optinally dropping it then.)
> 
> Thanks. I've picked up both patches.
> 
> If Takashi doesn't want to rebase his tree to drop patch 1 that's OK, it
> will just arrive in mainline via two paths, but git should handle it.

Yeah, I'd like to avoid rebasing, so let's get it merge from both
trees.  git can handle such a case gracefully.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ALSA: ppc: drop if block with always false condition
  2020-11-26 16:59 [PATCH 1/2] ALSA: ppc: drop if block with always false condition Uwe Kleine-König
                   ` (3 preceding siblings ...)
  2020-11-28  8:48 ` Takashi Iwai
@ 2020-12-10 11:30 ` Michael Ellerman
  4 siblings, 0 replies; 13+ messages in thread
From: Michael Ellerman @ 2020-12-10 11:30 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Arnd Bergmann, James E.J. Bottomley,
	Jaroslav Kysela, Michael Ellerman, Geoff Levand, Jakub Kicinski,
	Martin K. Petersen, Jens Axboe, Jim Paris, Takashi Iwai,
	Uwe  Kleine-König, David S. Miller, Alan Stern,
	Greg Kroah-Hartman
  Cc: linux-usb, linux-fbdev, Benjamin Herrenschmidt, dri-devel,
	linuxppc-dev, linux-scsi, Paul Mackerras, alsa-devel,
	linux-block, netdev


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #0: Type: text/plain, Size: 622 bytes --]

On Thu, 26 Nov 2020 17:59:49 +0100, Uwe Kleine-König wrote:
> The remove callback is only called for devices that were probed
> successfully before. As the matching probe function cannot complete
> without error if dev->match_id != PS3_MATCH_ID_SOUND, we don't have to
> check this here.

Applied to powerpc/next.

[1/2] ALSA: ppc: drop if block with always false condition
      https://git.kernel.org/powerpc/c/7ff94669e7d8e50756cd57947283381ae9665759
[2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void
      https://git.kernel.org/powerpc/c/6d247e4d264961aa3b871290f9b11a48d5a567f2

cheers

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, back to index

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-26 16:59 [PATCH 1/2] ALSA: ppc: drop if block with always false condition Uwe Kleine-König
2020-11-26 16:59 ` [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void Uwe Kleine-König
2020-11-27  8:40   ` Geert Uytterhoeven
2020-11-27 19:39   ` Geoff Levand
2020-11-28  8:48   ` Takashi Iwai
2020-11-29 17:31     ` Uwe Kleine-König
2020-12-02 12:14       ` Michael Ellerman
2020-12-02 12:22         ` Takashi Iwai
2020-11-27  8:35 ` [PATCH 1/2] ALSA: ppc: drop if block with always false condition Geert Uytterhoeven
2020-11-27  9:45   ` Uwe Kleine-König
2020-11-27 19:37 ` Geoff Levand
2020-11-28  8:48 ` Takashi Iwai
2020-12-10 11:30 ` Michael Ellerman

Linux-fbdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fbdev/0 linux-fbdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fbdev linux-fbdev/ https://lore.kernel.org/linux-fbdev \
		linux-fbdev@vger.kernel.org
	public-inbox-index linux-fbdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fbdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git