* [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 ` (3 more replies) 0 siblings, 4 replies; 16+ 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: alsa-devel, linux-scsi, Benjamin Herrenschmidt, linux-usb, linux-fbdev, dri-devel, linux-block, Paul Mackerras, netdev, linuxppc-dev 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 related [flat|nested] 16+ 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 ` (2 subsequent siblings) 3 siblings, 3 replies; 16+ 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: alsa-devel, linux-scsi, Benjamin Herrenschmidt, linux-usb, linux-fbdev, dri-devel, linux-block, Paul Mackerras, netdev, linuxppc-dev 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 related [flat|nested] 16+ 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; 16+ messages in thread From: Geert Uytterhoeven @ 2020-11-27 8:40 UTC (permalink / raw) To: Uwe Kleine-König Cc: ALSA Development Mailing List, Benjamin Herrenschmidt, Linux Fbdev development list, DRI Development, Paul Mackerras, scsi, Michael Ellerman, Alan Stern, Jakub Kicinski, Arnd Bergmann, Bartlomiej Zolnierkiewicz, James E.J. Bottomley, linux-block, Jens Axboe, Martin K. Petersen, Geoff Levand, netdev, USB list, Takashi Iwai, Jim Paris, Greg Kroah-Hartman, linuxppc-dev, David S. Miller 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] 16+ 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; 16+ 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: alsa-devel, linux-scsi, Benjamin Herrenschmidt, linux-usb, linux-fbdev, dri-devel, linux-block, Paul Mackerras, netdev, linuxppc-dev 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] 16+ 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; 16+ messages in thread From: Takashi Iwai @ 2020-11-28 8:48 UTC (permalink / raw) To: Uwe Kleine-König Cc: alsa-devel, Benjamin Herrenschmidt, linux-fbdev, dri-devel, Paul Mackerras, linux-scsi, Michael Ellerman, Alan Stern, Jakub Kicinski, Arnd Bergmann, Bartlomiej Zolnierkiewicz, James E.J. Bottomley, linux-block, Jens Axboe, Martin K. Petersen, Geoff Levand, netdev, linux-usb, Takashi Iwai, Jim Paris, Greg Kroah-Hartman, linuxppc-dev, David S. Miller 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] 16+ 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; 16+ messages in thread From: Uwe Kleine-König @ 2020-11-29 17:31 UTC (permalink / raw) To: Takashi Iwai, Michael Ellerman Cc: alsa-devel, Benjamin Herrenschmidt, linux-fbdev, dri-devel, Paul Mackerras, linux-scsi, Alan Stern, Jakub Kicinski, Arnd Bergmann, Bartlomiej Zolnierkiewicz, James E.J. Bottomley, linux-block, Jens Axboe, Martin K. Petersen, Geoff Levand, Greg Kroah-Hartman, linux-usb, Takashi Iwai, Jim Paris, netdev, linuxppc-dev, David S. Miller [-- 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] 16+ 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; 16+ messages in thread From: Michael Ellerman @ 2020-12-02 12:14 UTC (permalink / raw) To: Uwe Kleine-König, Takashi Iwai Cc: alsa-devel, Benjamin Herrenschmidt, linux-fbdev, dri-devel, Paul Mackerras, linux-scsi, Alan Stern, Jakub Kicinski, Arnd Bergmann, Bartlomiej Zolnierkiewicz, James E.J. Bottomley, linux-block, Jens Axboe, Martin K. Petersen, Geoff Levand, Greg Kroah-Hartman, linux-usb, Takashi Iwai, Jim Paris, netdev, linuxppc-dev, David S. Miller 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] 16+ 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; 16+ messages in thread From: Takashi Iwai @ 2020-12-02 12:22 UTC (permalink / raw) To: Michael Ellerman Cc: alsa-devel, Benjamin Herrenschmidt, linux-fbdev, dri-devel, Paul Mackerras, linux-scsi, Alan Stern, Uwe Kleine-König, Jakub Kicinski, Arnd Bergmann, Bartlomiej Zolnierkiewicz, James E.J. Bottomley, linux-block, Jens Axboe, Martin K. Petersen, Geoff Levand, Greg Kroah-Hartman, linux-usb, Takashi Iwai, Jim Paris, netdev, linuxppc-dev, David S. Miller 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] 16+ 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 ` [PATCH 1/2] ALSA: ppc: drop if block with always false condition Geoff Levand 2020-11-28 8:48 ` Takashi Iwai 3 siblings, 1 reply; 16+ messages in thread From: Geert Uytterhoeven @ 2020-11-27 8:35 UTC (permalink / raw) To: Uwe Kleine-König Cc: ALSA Development Mailing List, Benjamin Herrenschmidt, Linux Fbdev development list, DRI Development, Paul Mackerras, scsi, Michael Ellerman, Alan Stern, Jakub Kicinski, Arnd Bergmann, Bartlomiej Zolnierkiewicz, James E.J. Bottomley, linux-block, Jens Axboe, Martin K. Petersen, Geoff Levand, netdev, USB list, Takashi Iwai, Jim Paris, Greg Kroah-Hartman, linuxppc-dev, David S. Miller 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] 16+ 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 2020-11-27 15:22 ` [PATCH] ALSA: ppc: remove redundant checks in PS3 driver probe Leonard Goehrs 0 siblings, 1 reply; 16+ messages in thread From: Uwe Kleine-König @ 2020-11-27 9:45 UTC (permalink / raw) To: Geert Uytterhoeven Cc: ALSA Development Mailing List, Benjamin Herrenschmidt, Linux Fbdev development list, DRI Development, Paul Mackerras, scsi, Michael Ellerman, Alan Stern, Jakub Kicinski, Arnd Bergmann, Bartlomiej Zolnierkiewicz, James E.J. Bottomley, linux-block, Jens Axboe, Martin K. Petersen, Geoff Levand, netdev, USB list, Takashi Iwai, Jim Paris, Greg Kroah-Hartman, linuxppc-dev, David S. Miller [-- 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] 16+ messages in thread
* [PATCH] ALSA: ppc: remove redundant checks in PS3 driver probe 2020-11-27 9:45 ` Uwe Kleine-König @ 2020-11-27 15:22 ` Leonard Goehrs 2020-11-27 17:34 ` Uwe Kleine-König ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Leonard Goehrs @ 2020-11-27 15:22 UTC (permalink / raw) To: u.kleine-koenig, geoff, perex, tiwai, mpe Cc: alsa-devel, benh, paulus, kernel, Geert Uytterhoeven, Leonard Goehrs, linuxppc-dev The check for the FW_FEATURE_PS3_LV1 firmware feature is already performed in ps3_system_bus_init() before registering the driver. So if the probe function is actually used, this feature is already known to be available. The check for the match id is also superfluous; the condition is always true because the bus' match function (ps3_system_bus_match()) only considers this driver for devices having: dev->match_id == snd_ps3_bus_driver_info.match_id. Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: Leonard Goehrs <l.goehrs@pengutronix.de> --- sound/ppc/snd_ps3.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c index 58bb49fff184..b6e4aa3df870 100644 --- a/sound/ppc/snd_ps3.c +++ b/sound/ppc/snd_ps3.c @@ -896,11 +896,6 @@ static int snd_ps3_driver_probe(struct ps3_system_bus_device *dev) u64 lpar_addr, lpar_size; static u64 dummy_mask; - if (WARN_ON(!firmware_has_feature(FW_FEATURE_PS3_LV1))) - return -ENODEV; - if (WARN_ON(dev->match_id != PS3_MATCH_ID_SOUND)) - return -ENODEV; - the_card.ps3_dev = dev; ret = ps3_open_hv_device(dev); -- 2.20.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] ALSA: ppc: remove redundant checks in PS3 driver probe 2020-11-27 15:22 ` [PATCH] ALSA: ppc: remove redundant checks in PS3 driver probe Leonard Goehrs @ 2020-11-27 17:34 ` Uwe Kleine-König 2020-11-27 19:39 ` Geoff Levand 2020-11-28 8:49 ` Takashi Iwai 2 siblings, 0 replies; 16+ messages in thread From: Uwe Kleine-König @ 2020-11-27 17:34 UTC (permalink / raw) To: Leonard Goehrs Cc: alsa-devel, geoff, mpe, tiwai, paulus, kernel, benh, Geert Uytterhoeven, linuxppc-dev [-- Attachment #1: Type: text/plain, Size: 999 bytes --] Hallo Leonard, On Fri, Nov 27, 2020 at 04:22:59PM +0100, Leonard Goehrs wrote: > The check for the FW_FEATURE_PS3_LV1 firmware feature is already performed > in ps3_system_bus_init() before registering the driver. So if the probe > function is actually used, this feature is already known to be available. > > The check for the match id is also superfluous; the condition is always > true because the bus' match function (ps3_system_bus_match()) only > considers this driver for devices having: > dev->match_id == snd_ps3_bus_driver_info.match_id. > > Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Leonard Goehrs <l.goehrs@pengutronix.de> Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Thanks for picking this up. Best regards and have a nice week-end, Uwe Kleine-König -- 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] 16+ messages in thread
* Re: [PATCH] ALSA: ppc: remove redundant checks in PS3 driver probe 2020-11-27 15:22 ` [PATCH] ALSA: ppc: remove redundant checks in PS3 driver probe Leonard Goehrs 2020-11-27 17:34 ` Uwe Kleine-König @ 2020-11-27 19:39 ` Geoff Levand 2020-11-28 8:49 ` Takashi Iwai 2 siblings, 0 replies; 16+ messages in thread From: Geoff Levand @ 2020-11-27 19:39 UTC (permalink / raw) To: Leonard Goehrs, u.kleine-koenig, perex, tiwai, mpe Cc: alsa-devel, benh, paulus, kernel, Geert Uytterhoeven, linuxppc-dev On 11/27/20 7:22 AM, Leonard Goehrs wrote: > The check for the FW_FEATURE_PS3_LV1 firmware feature is already performed > in ps3_system_bus_init() before registering the driver. So if the probe > function is actually used, this feature is already known to be available. > > The check for the match id is also superfluous; the condition is always > true because the bus' match function (ps3_system_bus_match()) only > considers this driver for devices having: > dev->match_id == snd_ps3_bus_driver_info.match_id. > > Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Leonard Goehrs <l.goehrs@pengutronix.de> Seems OK with v5.9 on PS3. Tested by: Geoff Levand <geoff@infradead.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] ALSA: ppc: remove redundant checks in PS3 driver probe 2020-11-27 15:22 ` [PATCH] ALSA: ppc: remove redundant checks in PS3 driver probe Leonard Goehrs 2020-11-27 17:34 ` Uwe Kleine-König 2020-11-27 19:39 ` Geoff Levand @ 2020-11-28 8:49 ` Takashi Iwai 2 siblings, 0 replies; 16+ messages in thread From: Takashi Iwai @ 2020-11-28 8:49 UTC (permalink / raw) To: Leonard Goehrs Cc: alsa-devel, geoff, mpe, u.kleine-koenig, tiwai, paulus, kernel, benh, Geert Uytterhoeven, linuxppc-dev On Fri, 27 Nov 2020 16:22:59 +0100, Leonard Goehrs wrote: > > The check for the FW_FEATURE_PS3_LV1 firmware feature is already performed > in ps3_system_bus_init() before registering the driver. So if the probe > function is actually used, this feature is already known to be available. > > The check for the match id is also superfluous; the condition is always > true because the bus' match function (ps3_system_bus_match()) only > considers this driver for devices having: > dev->match_id == snd_ps3_bus_driver_info.match_id. > > Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Leonard Goehrs <l.goehrs@pengutronix.de> Applied now. Thanks. Takashi ^ permalink raw reply [flat|nested] 16+ 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 3 siblings, 0 replies; 16+ 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: alsa-devel, linux-scsi, Benjamin Herrenschmidt, linux-usb, linux-fbdev, dri-devel, linux-block, Paul Mackerras, netdev, Geert Uytterhoeven, linuxppc-dev 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] 16+ 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 ` [PATCH 1/2] ALSA: ppc: drop if block with always false condition Geoff Levand @ 2020-11-28 8:48 ` Takashi Iwai 3 siblings, 0 replies; 16+ messages in thread From: Takashi Iwai @ 2020-11-28 8:48 UTC (permalink / raw) To: Uwe Kleine-König Cc: alsa-devel, Benjamin Herrenschmidt, linux-fbdev, dri-devel, Paul Mackerras, linux-scsi, Michael Ellerman, Alan Stern, Jakub Kicinski, Arnd Bergmann, Bartlomiej Zolnierkiewicz, James E.J. Bottomley, linux-block, Jens Axboe, Martin K. Petersen, Geoff Levand, netdev, linux-usb, Takashi Iwai, Jim Paris, Greg Kroah-Hartman, linuxppc-dev, David S. Miller 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] 16+ messages in thread
end of thread, other threads:[~2020-12-02 12:23 UTC | newest] Thread overview: 16+ 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 15:22 ` [PATCH] ALSA: ppc: remove redundant checks in PS3 driver probe Leonard Goehrs 2020-11-27 17:34 ` Uwe Kleine-König 2020-11-27 19:39 ` Geoff Levand 2020-11-28 8:49 ` Takashi Iwai 2020-11-27 19:37 ` [PATCH 1/2] ALSA: ppc: drop if block with always false condition Geoff Levand 2020-11-28 8:48 ` Takashi Iwai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).