On 1/27/21 1:50 PM, Uwe Kleine-König wrote: > The driver core ignores the return value of struct bus_type::remove() > because there is only little that can be done. To simplify the quest to > make this function return void, let struct vio_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 makes it > obvious for future driver authors that returning an error code isn't > intended. > > Note there are two nominally different implementations for a vio bus: > one in arch/sparc/kernel/vio.c and the other in > arch/powerpc/platforms/pseries/vio.c. I didn't care to check which > driver is using which of these busses (or if even some of them can be > used with both) and simply adapt all drivers and the two bus codes in > one go. > > Note that for the powerpc implementation there is a semantical change: > Before this patch for a device that was bound to a driver without a > remove callback vio_cmo_bus_remove(viodev) wasn't called. As the device > core still considers the device unbound after vio_bus_remove() returns > calling this unconditionally is the consistent behaviour which is > implemented here. > > Signed-off-by: Uwe Kleine-König <uwe@xxxxxxxxxxxxxxxxx> Reviewed-by: Tyrel Datwyler <tyreld@xxxxxxxxxxxxx> > --- > Hello, > > note that this change depends on > https://lore.kernel.org/r/20210121062005.53271-1-ljp@xxxxxxxxxxxxx which removes > an (ignored) return -EBUSY in drivers/net/ethernet/ibm/ibmvnic.c. > I don't know when/if this latter patch will be applied, so it might take > some time until my patch can go in. > > Best regards > Uwe > > arch/powerpc/include/asm/vio.h | 2 +- > arch/powerpc/platforms/pseries/vio.c | 7 +++---- > arch/sparc/include/asm/vio.h | 2 +- > arch/sparc/kernel/ds.c | 6 ------ > arch/sparc/kernel/vio.c | 4 ++-- > drivers/block/sunvdc.c | 3 +-- > drivers/char/hw_random/pseries-rng.c | 3 +-- > drivers/char/tpm/tpm_ibmvtpm.c | 4 +--- > drivers/crypto/nx/nx-842-pseries.c | 4 +--- > drivers/crypto/nx/nx.c | 4 +--- > drivers/misc/ibmvmc.c | 4 +--- > drivers/net/ethernet/ibm/ibmveth.c | 4 +--- > drivers/net/ethernet/ibm/ibmvnic.c | 4 +--- > drivers/net/ethernet/sun/ldmvsw.c | 4 +--- > drivers/net/ethernet/sun/sunvnet.c | 3 +-- > drivers/scsi/ibmvscsi/ibmvfc.c | 3 +-- > drivers/scsi/ibmvscsi/ibmvscsi.c | 4 +--- > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 4 +--- > drivers/tty/hvc/hvcs.c | 3 +-- > drivers/tty/vcc.c | 4 +--- > 20 files changed, 22 insertions(+), 54 deletions(-) > > diff --git a/arch/powerpc/include/asm/vio.h b/arch/powerpc/include/asm/vio.h > index 0cf52746531b..721c0d6715ac 100644 > --- a/arch/powerpc/include/asm/vio.h > +++ b/arch/powerpc/include/asm/vio.h > @@ -113,7 +113,7 @@ struct vio_driver { > const char *name; > const struct vio_device_id *id_table; > int (*probe)(struct vio_dev *dev, const struct vio_device_id *id); > - int (*remove)(struct vio_dev *dev); > + void (*remove)(struct vio_dev *dev); > /* A driver must have a get_desired_dma() function to > * be loaded in a CMO environment if it uses DMA. > */ > diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c > index b2797cfe4e2b..9cb4fc839fd5 100644 > --- a/arch/powerpc/platforms/pseries/vio.c > +++ b/arch/powerpc/platforms/pseries/vio.c > @@ -1261,7 +1261,6 @@ static int vio_bus_remove(struct device *dev) > struct vio_dev *viodev = to_vio_dev(dev); > struct vio_driver *viodrv = to_vio_driver(dev->driver); > struct device *devptr; > - int ret = 1; > > /* > * Hold a reference to the device after the remove function is called > @@ -1270,13 +1269,13 @@ static int vio_bus_remove(struct device *dev) > devptr = get_device(dev); > > if (viodrv->remove) > - ret = viodrv->remove(viodev); > + viodrv->remove(viodev); > > - if (!ret && firmware_has_feature(FW_FEATURE_CMO)) > + if (firmware_has_feature(FW_FEATURE_CMO)) > vio_cmo_bus_remove(viodev); > > put_device(devptr); > - return ret; > + return 0; > } > > /** > diff --git a/arch/sparc/include/asm/vio.h b/arch/sparc/include/asm/vio.h > index 059f0eb678e0..8a1a83bbb6d5 100644 > --- a/arch/sparc/include/asm/vio.h > +++ b/arch/sparc/include/asm/vio.h > @@ -362,7 +362,7 @@ struct vio_driver { > struct list_head node; > const struct vio_device_id *id_table; > int (*probe)(struct vio_dev *dev, const struct vio_device_id *id); > - int (*remove)(struct vio_dev *dev); > + void (*remove)(struct vio_dev *dev); > void (*shutdown)(struct vio_dev *dev); > unsigned long driver_data; > struct device_driver driver; > diff --git a/arch/sparc/kernel/ds.c b/arch/sparc/kernel/ds.c > index 522e5b51050c..4a5bdb0df779 100644 > --- a/arch/sparc/kernel/ds.c > +++ b/arch/sparc/kernel/ds.c > @@ -1236,11 +1236,6 @@ static int ds_probe(struct vio_dev *vdev, const struct vio_device_id *id) > return err; > } > > -static int ds_remove(struct vio_dev *vdev) > -{ > - return 0; > -} > - > static const struct vio_device_id ds_match[] = { > { > .type = "domain-services-port", > @@ -1251,7 +1246,6 @@ static const struct vio_device_id ds_match[] = { > static struct vio_driver ds_driver = { > .id_table = ds_match, > .probe = ds_probe, > - .remove = ds_remove, > .name = "ds", > }; > > diff --git a/arch/sparc/kernel/vio.c b/arch/sparc/kernel/vio.c > index 4f57056ed463..348a88691219 100644 > --- a/arch/sparc/kernel/vio.c > +++ b/arch/sparc/kernel/vio.c > @@ -105,10 +105,10 @@ static int vio_device_remove(struct device *dev) > * routines to do so at the moment. TBD > */ > > - return drv->remove(vdev); > + drv->remove(vdev); > } > > - return 1; > + return 0; > } > > static ssize_t devspec_show(struct device *dev, > diff --git a/drivers/block/sunvdc.c b/drivers/block/sunvdc.c > index 39aeebc6837d..1547d4345ad8 100644 > --- a/drivers/block/sunvdc.c > +++ b/drivers/block/sunvdc.c > @@ -1071,7 +1071,7 @@ static int vdc_port_probe(struct vio_dev *vdev, const struct vio_device_id *id) > return err; > } > > -static int vdc_port_remove(struct vio_dev *vdev) > +static void vdc_port_remove(struct vio_dev *vdev) > { > struct vdc_port *port = dev_get_drvdata(&vdev->dev); > > @@ -1094,7 +1094,6 @@ static int vdc_port_remove(struct vio_dev *vdev) > > kfree(port); > } > - return 0; > } > > static void vdc_requeue_inflight(struct vdc_port *port) > diff --git a/drivers/char/hw_random/pseries-rng.c b/drivers/char/hw_random/pseries-rng.c > index 8038a8a9fb58..f4949b689bd5 100644 > --- a/drivers/char/hw_random/pseries-rng.c > +++ b/drivers/char/hw_random/pseries-rng.c > @@ -54,10 +54,9 @@ static int pseries_rng_probe(struct vio_dev *dev, > return hwrng_register(&pseries_rng); > } > > -static int pseries_rng_remove(struct vio_dev *dev) > +static void pseries_rng_remove(struct vio_dev *dev) > { > hwrng_unregister(&pseries_rng); > - return 0; > } > > static const struct vio_device_id pseries_rng_driver_ids[] = { > diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c > index 994385bf37c0..903604769de9 100644 > --- a/drivers/char/tpm/tpm_ibmvtpm.c > +++ b/drivers/char/tpm/tpm_ibmvtpm.c > @@ -343,7 +343,7 @@ static int ibmvtpm_crq_send_init_complete(struct ibmvtpm_dev *ibmvtpm) > * > * Return: Always 0. > */ > -static int tpm_ibmvtpm_remove(struct vio_dev *vdev) > +static void tpm_ibmvtpm_remove(struct vio_dev *vdev) > { > struct tpm_chip *chip = dev_get_drvdata(&vdev->dev); > struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev); > @@ -372,8 +372,6 @@ static int tpm_ibmvtpm_remove(struct vio_dev *vdev) > kfree(ibmvtpm); > /* For tpm_ibmvtpm_get_desired_dma */ > dev_set_drvdata(&vdev->dev, NULL); > - > - return 0; > } > > /** > diff --git a/drivers/crypto/nx/nx-842-pseries.c b/drivers/crypto/nx/nx-842-pseries.c > index 2de5e3672e42..cc8dd3072b8b 100644 > --- a/drivers/crypto/nx/nx-842-pseries.c > +++ b/drivers/crypto/nx/nx-842-pseries.c > @@ -1042,7 +1042,7 @@ static int nx842_probe(struct vio_dev *viodev, > return ret; > } > > -static int nx842_remove(struct vio_dev *viodev) > +static void nx842_remove(struct vio_dev *viodev) > { > struct nx842_devdata *old_devdata; > unsigned long flags; > @@ -1063,8 +1063,6 @@ static int nx842_remove(struct vio_dev *viodev) > if (old_devdata) > kfree(old_devdata->counters); > kfree(old_devdata); > - > - return 0; > } > > static const struct vio_device_id nx842_vio_driver_ids[] = { > diff --git a/drivers/crypto/nx/nx.c b/drivers/crypto/nx/nx.c > index 0d2dc5be7f19..1d0e8a1ba160 100644 > --- a/drivers/crypto/nx/nx.c > +++ b/drivers/crypto/nx/nx.c > @@ -783,7 +783,7 @@ static int nx_probe(struct vio_dev *viodev, const struct vio_device_id *id) > return nx_register_algs(); > } > > -static int nx_remove(struct vio_dev *viodev) > +static void nx_remove(struct vio_dev *viodev) > { > dev_dbg(&viodev->dev, "entering nx_remove for UA 0x%x\n", > viodev->unit_address); > @@ -811,8 +811,6 @@ static int nx_remove(struct vio_dev *viodev) > nx_unregister_skcipher(&nx_ecb_aes_alg, NX_FC_AES, > NX_MODE_AES_ECB); > } > - > - return 0; > } > > > diff --git a/drivers/misc/ibmvmc.c b/drivers/misc/ibmvmc.c > index 2d778d0f011e..c0fe3295c330 100644 > --- a/drivers/misc/ibmvmc.c > +++ b/drivers/misc/ibmvmc.c > @@ -2288,15 +2288,13 @@ static int ibmvmc_probe(struct vio_dev *vdev, const struct vio_device_id *id) > return -EPERM; > } > > -static int ibmvmc_remove(struct vio_dev *vdev) > +static void ibmvmc_remove(struct vio_dev *vdev) > { > struct crq_server_adapter *adapter = dev_get_drvdata(&vdev->dev); > > dev_info(adapter->dev, "Entering remove for UA 0x%x\n", > vdev->unit_address); > ibmvmc_release_crq_queue(adapter); > - > - return 0; > } > > static struct vio_device_id ibmvmc_device_table[] = { > diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c > index c3ec9ceed833..7fea9ae60f13 100644 > --- a/drivers/net/ethernet/ibm/ibmveth.c > +++ b/drivers/net/ethernet/ibm/ibmveth.c > @@ -1758,7 +1758,7 @@ static int ibmveth_probe(struct vio_dev *dev, const struct vio_device_id *id) > return 0; > } > > -static int ibmveth_remove(struct vio_dev *dev) > +static void ibmveth_remove(struct vio_dev *dev) > { > struct net_device *netdev = dev_get_drvdata(&dev->dev); > struct ibmveth_adapter *adapter = netdev_priv(netdev); > @@ -1771,8 +1771,6 @@ static int ibmveth_remove(struct vio_dev *dev) > > free_netdev(netdev); > dev_set_drvdata(&dev->dev, NULL); > - > - return 0; > } > > static struct attribute veth_active_attr; > diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c > index a187d51bcf92..2eec0652760c 100644 > --- a/drivers/net/ethernet/ibm/ibmvnic.c > +++ b/drivers/net/ethernet/ibm/ibmvnic.c > @@ -5430,7 +5430,7 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id) > return rc; > } > > -static int ibmvnic_remove(struct vio_dev *dev) > +static void ibmvnic_remove(struct vio_dev *dev) > { > struct net_device *netdev = dev_get_drvdata(&dev->dev); > struct ibmvnic_adapter *adapter = netdev_priv(netdev); > @@ -5460,8 +5460,6 @@ static int ibmvnic_remove(struct vio_dev *dev) > device_remove_file(&dev->dev, &dev_attr_failover); > free_netdev(netdev); > dev_set_drvdata(&dev->dev, NULL); > - > - return 0; > } > > static ssize_t failover_store(struct device *dev, struct device_attribute *attr, > diff --git a/drivers/net/ethernet/sun/ldmvsw.c b/drivers/net/ethernet/sun/ldmvsw.c > index 01ea0d6f8819..50bd4e3b0af9 100644 > --- a/drivers/net/ethernet/sun/ldmvsw.c > +++ b/drivers/net/ethernet/sun/ldmvsw.c > @@ -404,7 +404,7 @@ static int vsw_port_probe(struct vio_dev *vdev, const struct vio_device_id *id) > return err; > } > > -static int vsw_port_remove(struct vio_dev *vdev) > +static void vsw_port_remove(struct vio_dev *vdev) > { > struct vnet_port *port = dev_get_drvdata(&vdev->dev); > unsigned long flags; > @@ -430,8 +430,6 @@ static int vsw_port_remove(struct vio_dev *vdev) > > free_netdev(port->dev); > } > - > - return 0; > } > > static void vsw_cleanup(void) > diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c > index 96b883f965f6..58ee89223951 100644 > --- a/drivers/net/ethernet/sun/sunvnet.c > +++ b/drivers/net/ethernet/sun/sunvnet.c > @@ -510,7 +510,7 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id) > return err; > } > > -static int vnet_port_remove(struct vio_dev *vdev) > +static void vnet_port_remove(struct vio_dev *vdev) > { > struct vnet_port *port = dev_get_drvdata(&vdev->dev); > > @@ -533,7 +533,6 @@ static int vnet_port_remove(struct vio_dev *vdev) > > kfree(port); > } > - return 0; > } > > static const struct vio_device_id vnet_port_match[] = { > diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c > index 42e4d35e0d35..0a472acaca5d 100644 > --- a/drivers/scsi/ibmvscsi/ibmvfc.c > +++ b/drivers/scsi/ibmvscsi/ibmvfc.c > @@ -5253,7 +5253,7 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id) > * Return value: > * 0 > **/ > -static int ibmvfc_remove(struct vio_dev *vdev) > +static void ibmvfc_remove(struct vio_dev *vdev) > { > struct ibmvfc_host *vhost = dev_get_drvdata(&vdev->dev); > unsigned long flags; > @@ -5282,7 +5282,6 @@ static int ibmvfc_remove(struct vio_dev *vdev) > spin_unlock(&ibmvfc_driver_lock); > scsi_host_put(vhost->host); > LEAVE; > - return 0; > } > > /** > diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c > index 29fcc44be2d5..77fafb1bc173 100644 > --- a/drivers/scsi/ibmvscsi/ibmvscsi.c > +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c > @@ -2335,7 +2335,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const struct vio_device_id *id) > return -1; > } > > -static int ibmvscsi_remove(struct vio_dev *vdev) > +static void ibmvscsi_remove(struct vio_dev *vdev) > { > struct ibmvscsi_host_data *hostdata = dev_get_drvdata(&vdev->dev); > > @@ -2356,8 +2356,6 @@ static int ibmvscsi_remove(struct vio_dev *vdev) > spin_unlock(&ibmvscsi_driver_lock); > > scsi_host_put(hostdata->host); > - > - return 0; > } > > /** > diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c > index cc3908c2d2f9..9abd9e253af6 100644 > --- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c > +++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c > @@ -3595,7 +3595,7 @@ static int ibmvscsis_probe(struct vio_dev *vdev, > return rc; > } > > -static int ibmvscsis_remove(struct vio_dev *vdev) > +static void ibmvscsis_remove(struct vio_dev *vdev) > { > struct scsi_info *vscsi = dev_get_drvdata(&vdev->dev); > > @@ -3622,8 +3622,6 @@ static int ibmvscsis_remove(struct vio_dev *vdev) > list_del(&vscsi->list); > spin_unlock_bh(&ibmvscsis_dev_lock); > kfree(vscsi); > - > - return 0; > } > > static ssize_t system_id_show(struct device *dev, > diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c > index 3e0461285c34..80874945ded8 100644 > --- a/drivers/tty/hvc/hvcs.c > +++ b/drivers/tty/hvc/hvcs.c > @@ -819,7 +819,7 @@ static int hvcs_probe( > return 0; > } > > -static int hvcs_remove(struct vio_dev *dev) > +static void hvcs_remove(struct vio_dev *dev) > { > struct hvcs_struct *hvcsd = dev_get_drvdata(&dev->dev); > unsigned long flags; > @@ -849,7 +849,6 @@ static int hvcs_remove(struct vio_dev *dev) > > printk(KERN_INFO "HVCS: vty-server@%X removed from the" > " vio bus.\n", dev->unit_address); > - return 0; > }; > > static struct vio_driver hvcs_vio_driver = { > diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c > index e2d6205f83ce..5f72ebf93821 100644 > --- a/drivers/tty/vcc.c > +++ b/drivers/tty/vcc.c > @@ -677,7 +677,7 @@ static int vcc_probe(struct vio_dev *vdev, const struct vio_device_id *id) > * > * Return: status of removal > */ > -static int vcc_remove(struct vio_dev *vdev) > +static void vcc_remove(struct vio_dev *vdev) > { > struct vcc_port *port = dev_get_drvdata(&vdev->dev); > > @@ -712,8 +712,6 @@ static int vcc_remove(struct vio_dev *vdev) > kfree(port->domain); > kfree(port); > } > - > - return 0; > } > > static const struct vio_device_id vcc_match[] = { >
