The driver core ignores the return value of css_remove()
(because there is only little it can do when a device disappears) and
there are no pci_epf_drivers with a remove callback.
So make it impossible for future drivers to return an unused error code
by changing the remove prototype to return void.
The real motivation for this change is the quest to make struct
bus_type::remove return void, too.
Signed-off-by: Uwe Kleine-König <[email protected]>
---
drivers/s390/cio/chsc_sch.c | 3 +--
drivers/s390/cio/css.c | 7 ++++---
drivers/s390/cio/css.h | 2 +-
drivers/s390/cio/device.c | 5 ++---
drivers/s390/cio/eadm_sch.c | 4 +---
drivers/s390/cio/vfio_ccw_drv.c | 3 +--
6 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/s390/cio/chsc_sch.c b/drivers/s390/cio/chsc_sch.c
index c42405c620b5..684348d82f08 100644
--- a/drivers/s390/cio/chsc_sch.c
+++ b/drivers/s390/cio/chsc_sch.c
@@ -100,7 +100,7 @@ static int chsc_subchannel_probe(struct subchannel *sch)
return ret;
}
-static int chsc_subchannel_remove(struct subchannel *sch)
+static void chsc_subchannel_remove(struct subchannel *sch)
{
struct chsc_private *private;
@@ -112,7 +112,6 @@ static int chsc_subchannel_remove(struct subchannel *sch)
put_device(&sch->dev);
}
kfree(private);
- return 0;
}
static void chsc_subchannel_shutdown(struct subchannel *sch)
diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c
index a974943c27da..092fd1ea5799 100644
--- a/drivers/s390/cio/css.c
+++ b/drivers/s390/cio/css.c
@@ -1374,12 +1374,13 @@ static int css_probe(struct device *dev)
static int 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);
sch->driver = NULL;
- return ret;
+
+ return 0;
}
static void css_shutdown(struct device *dev)
diff --git a/drivers/s390/cio/css.h b/drivers/s390/cio/css.h
index 2eddfc47f687..c98522cbe276 100644
--- a/drivers/s390/cio/css.h
+++ b/drivers/s390/cio/css.h
@@ -81,7 +81,7 @@ struct css_driver {
int (*chp_event)(struct subchannel *, struct chp_link *, int);
int (*sch_event)(struct subchannel *, int);
int (*probe)(struct subchannel *);
- int (*remove)(struct subchannel *);
+ void (*remove)(struct subchannel *);
void (*shutdown)(struct subchannel *);
int (*settle)(void);
};
diff --git a/drivers/s390/cio/device.c b/drivers/s390/cio/device.c
index 84f659cafe76..cd5d2d4d8e46 100644
--- a/drivers/s390/cio/device.c
+++ b/drivers/s390/cio/device.c
@@ -137,7 +137,7 @@ static int ccw_uevent(struct device *dev, struct kobj_uevent_env *env)
static void io_subchannel_irq(struct subchannel *);
static int io_subchannel_probe(struct subchannel *);
-static int io_subchannel_remove(struct subchannel *);
+static void io_subchannel_remove(struct subchannel *);
static void io_subchannel_shutdown(struct subchannel *);
static int io_subchannel_sch_event(struct subchannel *, int);
static int io_subchannel_chp_event(struct subchannel *, struct chp_link *,
@@ -1101,7 +1101,7 @@ static int io_subchannel_probe(struct subchannel *sch)
return 0;
}
-static int io_subchannel_remove(struct subchannel *sch)
+static void io_subchannel_remove(struct subchannel *sch)
{
struct io_subchannel_private *io_priv = to_io_private(sch);
struct ccw_device *cdev;
@@ -1120,7 +1120,6 @@ static int io_subchannel_remove(struct subchannel *sch)
io_priv->dma_area, io_priv->dma_area_dma);
kfree(io_priv);
sysfs_remove_group(&sch->dev.kobj, &io_subchannel_attr_group);
- return 0;
}
static void io_subchannel_verify(struct subchannel *sch)
diff --git a/drivers/s390/cio/eadm_sch.c b/drivers/s390/cio/eadm_sch.c
index c8964e0a23e7..15bdae5981ca 100644
--- a/drivers/s390/cio/eadm_sch.c
+++ b/drivers/s390/cio/eadm_sch.c
@@ -282,7 +282,7 @@ static void eadm_quiesce(struct subchannel *sch)
spin_unlock_irq(sch->lock);
}
-static int eadm_subchannel_remove(struct subchannel *sch)
+static void eadm_subchannel_remove(struct subchannel *sch)
{
struct eadm_private *private = get_eadm_private(sch);
@@ -297,8 +297,6 @@ static int eadm_subchannel_remove(struct subchannel *sch)
spin_unlock_irq(sch->lock);
kfree(private);
-
- return 0;
}
static void eadm_subchannel_shutdown(struct subchannel *sch)
diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
index 9b61e9b131ad..76099bcb765b 100644
--- a/drivers/s390/cio/vfio_ccw_drv.c
+++ b/drivers/s390/cio/vfio_ccw_drv.c
@@ -234,7 +234,7 @@ static int vfio_ccw_sch_probe(struct subchannel *sch)
return ret;
}
-static int vfio_ccw_sch_remove(struct subchannel *sch)
+static void vfio_ccw_sch_remove(struct subchannel *sch)
{
struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev);
struct vfio_ccw_crw *crw, *temp;
@@ -257,7 +257,6 @@ static int vfio_ccw_sch_remove(struct subchannel *sch)
VFIO_CCW_MSG_EVENT(4, "unbound from subchannel %x.%x.%04x\n",
sch->schid.cssid, sch->schid.ssid,
sch->schid.sch_no);
- return 0;
}
static void vfio_ccw_sch_shutdown(struct subchannel *sch)
--
2.30.2
On Tue, Jul 06 2021, Uwe Kleine-König <[email protected]> wrote:
> The driver core ignores the return value of css_remove()
> (because there is only little it can do when a device disappears) and
> there are no pci_epf_drivers with a remove callback.
s/pci_epf/css/
>
> So make it impossible for future drivers to return an unused error code
> by changing the remove prototype to return void.
>
> The real motivation for this change is the quest to make struct
> bus_type::remove return void, too.
>
> Signed-off-by: Uwe Kleine-König <[email protected]>
> ---
> drivers/s390/cio/chsc_sch.c | 3 +--
> drivers/s390/cio/css.c | 7 ++++---
> drivers/s390/cio/css.h | 2 +-
> drivers/s390/cio/device.c | 5 ++---
> drivers/s390/cio/eadm_sch.c | 4 +---
> drivers/s390/cio/vfio_ccw_drv.c | 3 +--
> 6 files changed, 10 insertions(+), 14 deletions(-)
Reviewed-by: Cornelia Huck <[email protected]>
On Tue, Jul 06, 2021 at 05:58:25PM +0200, Cornelia Huck wrote:
> On Tue, Jul 06 2021, Uwe Kleine-K?nig <[email protected]> wrote:
>
> > The driver core ignores the return value of css_remove()
> > (because there is only little it can do when a device disappears) and
> > there are no pci_epf_drivers with a remove callback.
>
> s/pci_epf/css/
Argh, too much copy&paste. I make this:
The driver core ignores the return value of css_remove()
(because there is only little it can do when a device
disappears) and all callbacks return 0 anyhow.
to make this actually correct.
> Reviewed-by: Cornelia Huck <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Thank you. I will use the modified description. This will be picked up
by Vasily/Heiko to the s390-tree.
Also Acked-by: Vineeth Vijayan <[email protected]>
One question, is this patchset supposed to have 4 patches ? Are we
missing one ?
On 7/6/21 6:05 PM, Uwe Kleine-K?nig wrote:
> Argh, too much copy&paste. I make this:
>
> The driver core ignores the return value of css_remove()
> (because there is only little it can do when a device
> disappears) and all callbacks return 0 anyhow.
>
> to make this actually correct.
>
>> Reviewed-by: Cornelia Huck<[email protected]>
Hello Vineeth,
On Wed, Jul 07, 2021 at 01:28:11PM +0200, Vineeth Vijayan wrote:
> Thank you. I will use the modified description. This will be picked up by
> Vasily/Heiko to the s390-tree.
>
> Also Acked-by: Vineeth Vijayan <[email protected]>
>
> One question, is this patchset supposed to have 4 patches ? Are we missing
> one ?
Yes, the fourth patch[1] has the following shortstat:
80 files changed, 83 insertions(+), 219 deletions(-)
and the affected files are distributed over the whole source tree.
Given that this fourth patch is the actual motivation for the first
three, and I'd like to get this in during the next merge window, I would
prefer if these patches were taken together. (Well unless the first
three make it into 5.14-rc1 of course.)
Best regards
Uwe
[1] https://lore.kernel.org/lkml/[email protected]/
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Wed, Jul 07, 2021 at 04:34:31PM +0200, Uwe Kleine-K?nig wrote:
> Hello Vineeth,
>
> On Wed, Jul 07, 2021 at 01:28:11PM +0200, Vineeth Vijayan wrote:
> > Thank you. I will use the modified description. This will be picked up by
> > Vasily/Heiko to the s390-tree.
> >
> > Also Acked-by: Vineeth Vijayan <[email protected]>
> >
> > One question, is this patchset supposed to have 4 patches ? Are we missing
> > one ?
>
> Yes, the fourth patch[1] has the following shortstat:
>
> 80 files changed, 83 insertions(+), 219 deletions(-)
>
> and the affected files are distributed over the whole source tree.
>
> Given that this fourth patch is the actual motivation for the first
> three, and I'd like to get this in during the next merge window, I would
> prefer if these patches were taken together. (Well unless the first
> three make it into 5.14-rc1 of course.)
>
> Best regards
> Uwe
>
> [1] https://lore.kernel.org/lkml/[email protected]/
In this case I think Greg should pick up all four patches.
FWIW: it's usually also not very helpful to cc people only on parts of
a patch series and let them figure out the bigger picture.