Re: [PATCH] bus: Make remove callback return void
From: Uwe Kleine-König <hidden>
Date: 2021-07-06 12:44:15
Also in:
alsa-devel, dmaengine, kvm, linux-acpi, linux-arm-msm, linux-cxl, linux-fpga, linux-hyperv, linux-i2c, linux-i3c, linux-input, linux-media, linux-mips, linux-mmc, linux-remoteproc, linux-scsi, linux-serial, linux-spi, linux-staging, linux-sunxi, linux-usb, linux-wireless, linuxppc-dev, lkml, netdev, nvdimm, platform-driver-x86, target-devel, xen-devel
On Tue, Jul 06, 2021 at 01:17:37PM +0200, Cornelia Huck wrote:
On Tue, Jul 06 2021, Cornelia Huck [off-list ref] wrote:quoted
On Tue, Jul 06 2021, Uwe Kleine-König [off-list ref] wrote:quoted
The driver core ignores the return value of this callback because there is only little it can do when a device disappears. This is the final bit of a long lasting cleanup quest where several buses were converted to also return void from their remove callback. Additionally some resource leaks were fixed that were caused by drivers returning an error code in the expectation that the driver won't go away. With struct bus_type::remove returning void it's prevented that newly implemented buses return an ignored error code and so don't anticipate wrong expectations for driver authors.Yay!quoted
Signed-off-by: Uwe Kleine-König <redacted> --- Hello, this patch depends on "PCI: endpoint: Make struct pci_epf_driver::remove return void" that is not yet applied, see https://lore.kernel.org/r/20210223090757.57604-1-u.kleine-koenig@pengutronix.de (local). I tested it using allmodconfig on amd64 and arm, but I wouldn't be surprised if I still missed to convert a driver. So it would be great to get this into next early after the merge window closes.I'm afraid you missed the s390-specific busses in drivers/s390/cio/ (css/ccw/ccwgroup).
:-\
quoted hunk ↗ jump to hunk
The change for vfio/mdev looks good. The following should do the trick for s390; not sure if other architectures have easy-to-miss busses as well.diff --git a/drivers/s390/cio/ccwgroup.c b/drivers/s390/cio/ccwgroup.c index 9748165e08e9..a66f416138ab 100644 --- a/drivers/s390/cio/ccwgroup.c +++ b/drivers/s390/cio/ccwgroup.c@@ -439,17 +439,15 @@ module_exit(cleanup_ccwgroup); /************************** driver stuff ******************************/ -static int ccwgroup_remove(struct device *dev) +static void ccwgroup_remove(struct device *dev) { struct ccwgroup_device *gdev = to_ccwgroupdev(dev); struct ccwgroup_driver *gdrv = to_ccwgroupdrv(dev->driver); if (!dev->driver) - return 0; + return;
This is fine to be squashed into my patch. In the long run: in a remove callback dev->driver cannot be NULL, so this if could go away.
quoted hunk ↗ jump to hunk
if (gdrv->remove) gdrv->remove(gdev); - - return 0; } static void ccwgroup_shutdown(struct device *dev)diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c index a974943c27da..ebc321edba51 100644 --- a/drivers/s390/cio/css.c +++ b/drivers/s390/cio/css.c@@ -1371,15 +1371,14 @@ static int css_probe(struct device *dev) return ret; } -static int css_remove(struct device *dev) +static void css_remove(struct device *dev) { struct subchannel *sch; - int ret; sch = to_subchannel(dev); - ret = sch->driver->remove ? sch->driver->remove(sch) : 0; + if (sch->driver->remove) + sch->driver->remove(sch);
Maybe the return type for this function pointer can be changed to void, too. I will add these changes to a v2 that I plan to send out after the dust settles some more. Thanks Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachments
- signature.asc [application/pgp-signature] 488 bytes