2021-07-06 15:53:29

by Uwe Kleine-König

Subject: [PATCH v2 3/4] s390/scm: Make struct scm_driver::remove return void

The driver core ignores the return value of scmdev_remove()
(because there is only little it can do when a device disappears).

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]>
---
arch/s390/include/asm/eadm.h | 2 +-
drivers/s390/block/scm_drv.c | 4 +---
drivers/s390/cio/scm.c | 5 ++++-
3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/s390/include/asm/eadm.h b/arch/s390/include/asm/eadm.h
index bb63b2afdf6f..445fe4c8184a 100644
--- a/arch/s390/include/asm/eadm.h
+++ b/arch/s390/include/asm/eadm.h
@@ -105,7 +105,7 @@ enum scm_event {SCM_CHANGE, SCM_AVAIL};
struct scm_driver {
struct device_driver drv;
int (*probe) (struct scm_device *scmdev);
- int (*remove) (struct scm_device *scmdev);
+ void (*remove) (struct scm_device *scmdev);
void (*notify) (struct scm_device *scmdev, enum scm_event event);
void (*handler) (struct scm_device *scmdev, void *data,
blk_status_t error);
diff --git a/drivers/s390/block/scm_drv.c b/drivers/s390/block/scm_drv.c
index 3134fd6e058e..69a845eb8b1f 100644
--- a/drivers/s390/block/scm_drv.c
+++ b/drivers/s390/block/scm_drv.c
@@ -60,15 +60,13 @@ static int scm_probe(struct scm_device *scmdev)
return ret;
}

-static int scm_remove(struct scm_device *scmdev)
+static void scm_remove(struct scm_device *scmdev)
{
struct scm_blk_dev *bdev = dev_get_drvdata(&scmdev->dev);

scm_blk_dev_cleanup(bdev);
dev_set_drvdata(&scmdev->dev, NULL);
kfree(bdev);
-
- return 0;
}

static struct scm_driver scm_drv = {
diff --git a/drivers/s390/cio/scm.c b/drivers/s390/cio/scm.c
index 9f26d4310bb3..b31711307e5a 100644
--- a/drivers/s390/cio/scm.c
+++ b/drivers/s390/cio/scm.c
@@ -33,7 +33,10 @@ static int scmdev_remove(struct device *dev)
struct scm_device *scmdev = to_scm_dev(dev);
struct scm_driver *scmdrv = to_scm_drv(dev->driver);

- return scmdrv->remove ? scmdrv->remove(scmdev) : -ENODEV;
+ if (scmdrv->remove)
+ scmdrv->remove(scmdev);
+
+ return 0;
}

static int scmdev_uevent(struct device *dev, struct kobj_uevent_env *env)
--
2.30.2

2021-07-06 16:07:02

by Cornelia Huck

Subject: Re: [PATCH v2 3/4] s390/scm: Make struct scm_driver::remove return void

On Tue, Jul 06 2021, Uwe Kleine-König <[email protected]> wrote:

> The driver core ignores the return value of scmdev_remove()
> (because there is only little it can do when a device disappears).
>
> 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]>
> ---
> arch/s390/include/asm/eadm.h | 2 +-
> drivers/s390/block/scm_drv.c | 4 +---
> drivers/s390/cio/scm.c | 5 ++++-
> 3 files changed, 6 insertions(+), 5 deletions(-)

Reviewed-by: Cornelia Huck <[email protected]>

2021-07-07 11:33:02

by Vineeth Vijayan

Subject: Re: [PATCH v2 3/4] s390/scm: Make struct scm_driver::remove return void

Looks sane to me.
Acked-by: Vineeth Vijayan <[email protected]>

Vasily/Heiko will pick this up for the next-s390-tree.


On 7/6/21 5:48 PM, Uwe Kleine-König wrote:
> The driver core ignores the return value of scmdev_remove()
> (because there is only little it can do when a device disappears).
>
> 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]>
> ---
> arch/s390/include/asm/eadm.h | 2 +-
> drivers/s390/block/scm_drv.c | 4 +---
> drivers/s390/cio/scm.c | 5 ++++-
> 3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/arch/s390/include/asm/eadm.h b/arch/s390/include/asm/eadm.h
> index bb63b2afdf6f..445fe4c8184a 100644
> --- a/arch/s390/include/asm/eadm.h
> +++ b/arch/s390/include/asm/eadm.h
> @@ -105,7 +105,7 @@ enum scm_event {SCM_CHANGE, SCM_AVAIL};
> struct scm_driver {
> struct device_driver drv;
> int (*probe) (struct scm_device *scmdev);
> - int (*remove) (struct scm_device *scmdev);
> + void (*remove) (struct scm_device *scmdev);
> void (*notify) (struct scm_device *scmdev, enum scm_event event);
> void (*handler) (struct scm_device *scmdev, void *data,
> blk_status_t error);
> diff --git a/drivers/s390/block/scm_drv.c b/drivers/s390/block/scm_drv.c
> index 3134fd6e058e..69a845eb8b1f 100644
> --- a/drivers/s390/block/scm_drv.c
> +++ b/drivers/s390/block/scm_drv.c
> @@ -60,15 +60,13 @@ static int scm_probe(struct scm_device *scmdev)
> return ret;
> }
>
> -static int scm_remove(struct scm_device *scmdev)
> +static void scm_remove(struct scm_device *scmdev)
> {
> struct scm_blk_dev *bdev = dev_get_drvdata(&scmdev->dev);
>
> scm_blk_dev_cleanup(bdev);
> dev_set_drvdata(&scmdev->dev, NULL);
> kfree(bdev);
> -
> - return 0;
> }
>
> static struct scm_driver scm_drv = {
> diff --git a/drivers/s390/cio/scm.c b/drivers/s390/cio/scm.c
> index 9f26d4310bb3..b31711307e5a 100644
> --- a/drivers/s390/cio/scm.c
> +++ b/drivers/s390/cio/scm.c
> @@ -33,7 +33,10 @@ static int scmdev_remove(struct device *dev)
> struct scm_device *scmdev = to_scm_dev(dev);
> struct scm_driver *scmdrv = to_scm_drv(dev->driver);
>
> - return scmdrv->remove ? scmdrv->remove(scmdev) : -ENODEV;
> + if (scmdrv->remove)
> + scmdrv->remove(scmdev);
> +
> + return 0;
> }
>
> static int scmdev_uevent(struct device *dev, struct kobj_uevent_env *env)