* [PATCH v2 0/4] Some cleanups after making bus_type::remove return void
@ 2021-07-30 19:10 Uwe Kleine-König
2021-07-30 19:10 ` [PATCH v2 1/4] nubus: Simplify check in remove callback Uwe Kleine-König
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Uwe Kleine-König @ 2021-07-30 19:10 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, kernel, Chen-Yu Tsai, Dmitry Torokhov, Finn Thain,
Geert Uytterhoeven, Pali Rohár, Rich Felker,
Samuel Iglesias Gonsálvez, linux-m68k, linux-sh
Hello,
compared to (implicit) v1 that can be found at
https://lore.kernel.org/lkml/20210727080840.3550927-1-u.kleine-koenig@pengutronix.de
I rebased on top of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git's
driver-core-next where Greg already applied one of the patches.
Patch #1 has an updated commit log, the other three patches are
unmodified.
There are no interdependencies between these patches apart from the two
zorro patches. So the patches can also be taken independently by their
respective maintainers.
Uwe Kleine-König (4):
nubus: Simplify check in remove callback
sh: superhyway: Simplify check in remove callback
zorro: Simplify remove callback
zorro: Drop useless (and hardly used) .driver member in struct
zorro_dev
drivers/nubus/bus.c | 2 +-
drivers/sh/superhyway/superhyway.c | 2 +-
drivers/zorro/zorro-driver.c | 13 ++++---------
include/linux/zorro.h | 1 -
4 files changed, 6 insertions(+), 12 deletions(-)
base-commit: b2c943e52705b211d1aa0633c9196150cf30be47
--
2.30.2
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2 1/4] nubus: Simplify check in remove callback
2021-07-30 19:10 [PATCH v2 0/4] Some cleanups after making bus_type::remove return void Uwe Kleine-König
@ 2021-07-30 19:10 ` Uwe Kleine-König
2021-08-01 5:04 ` Finn Thain
2021-07-30 19:10 ` [PATCH v2 2/4] sh: superhyway: " Uwe Kleine-König
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2021-07-30 19:10 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, kernel, Finn Thain, linux-m68k
The driver core only calls a remove callback when the device was
successfully bound (aka probed) before. So dev->driver is never NULL and
the respective check can just be dropped.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/nubus/bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nubus/bus.c b/drivers/nubus/bus.c
index d9d04f27f89b..17fad660032c 100644
--- a/drivers/nubus/bus.c
+++ b/drivers/nubus/bus.c
@@ -33,7 +33,7 @@ static void nubus_device_remove(struct device *dev)
{
struct nubus_driver *ndrv = to_nubus_driver(dev->driver);
- if (dev->driver && ndrv->remove)
+ if (ndrv->remove)
ndrv->remove(to_nubus_board(dev));
}
--
2.30.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 1/4] nubus: Simplify check in remove callback
2021-07-30 19:10 ` [PATCH v2 1/4] nubus: Simplify check in remove callback Uwe Kleine-König
@ 2021-08-01 5:04 ` Finn Thain
2021-08-02 6:52 ` Uwe Kleine-König
0 siblings, 1 reply; 10+ messages in thread
From: Finn Thain @ 2021-08-01 5:04 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Greg Kroah-Hartman, linux-kernel, kernel, linux-m68k
[-- Attachment #1: Type: text/plain, Size: 1398 bytes --]
On Fri, 30 Jul 2021, Uwe Kleine-König wrote:
> The driver core only calls a remove callback when the device was
> successfully bound (aka probed) before. So dev->driver is never NULL and
> the respective check can just be dropped.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by: Finn Thain <fthain@linux-m68k.org>
BTW, aside from nubus, zorro and superhyway you can find the same pattern
in many other busses. You may want to patch the following methods too.
acpi_device_remove
apr_device_remove
ccwgroup_remove
gio_device_remove
hid_device_remove
ibmebus_bus_device_remove
macio_device_remove
memstick_device_remove
ntb_remove
pci_device_remove
pnp_device_remove
ps3_system_bus_remove
rio_device_remove
slim_device_remove
soundbus_device_remove
ssb_device_remove
tifm_device_remove
vdpa_dev_remove
vmbus_remove
> ---
> drivers/nubus/bus.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/nubus/bus.c b/drivers/nubus/bus.c
> index d9d04f27f89b..17fad660032c 100644
> --- a/drivers/nubus/bus.c
> +++ b/drivers/nubus/bus.c
> @@ -33,7 +33,7 @@ static void nubus_device_remove(struct device *dev)
> {
> struct nubus_driver *ndrv = to_nubus_driver(dev->driver);
>
> - if (dev->driver && ndrv->remove)
> + if (ndrv->remove)
> ndrv->remove(to_nubus_board(dev));
> }
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 1/4] nubus: Simplify check in remove callback
2021-08-01 5:04 ` Finn Thain
@ 2021-08-02 6:52 ` Uwe Kleine-König
2021-08-03 2:14 ` Finn Thain
0 siblings, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2021-08-02 6:52 UTC (permalink / raw)
To: Finn Thain; +Cc: Greg Kroah-Hartman, linux-m68k, linux-kernel, kernel
[-- Attachment #1: Type: text/plain, Size: 1446 bytes --]
Hello Finn,
On Sun, Aug 01, 2021 at 03:04:03PM +1000, Finn Thain wrote:
> On Fri, 30 Jul 2021, Uwe Kleine-König wrote:
>
> > The driver core only calls a remove callback when the device was
> > successfully bound (aka probed) before. So dev->driver is never NULL and
> > the respective check can just be dropped.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>
> Acked-by: Finn Thain <fthain@linux-m68k.org>
>
> BTW, aside from nubus, zorro and superhyway you can find the same pattern
> in many other busses. You may want to patch the following methods too.
>
> acpi_device_remove
> apr_device_remove
> ccwgroup_remove
> gio_device_remove
> hid_device_remove
> ibmebus_bus_device_remove
> macio_device_remove
> memstick_device_remove
> ntb_remove
> pci_device_remove
> pnp_device_remove
> ps3_system_bus_remove
> rio_device_remove
> slim_device_remove
> soundbus_device_remove
> ssb_device_remove
> tifm_device_remove
> vdpa_dev_remove
> vmbus_remove
Did you find these by hand? Or using a coccinelle match?
Anyhow, thanks for the list, I'll add it to my todo list but if you're
motivated don't consider these cleanups as my property. (Please Cc: me
though to prevent duplicated effort.)
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] 10+ messages in thread
* Re: [PATCH v2 1/4] nubus: Simplify check in remove callback
2021-08-02 6:52 ` Uwe Kleine-König
@ 2021-08-03 2:14 ` Finn Thain
0 siblings, 0 replies; 10+ messages in thread
From: Finn Thain @ 2021-08-03 2:14 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Greg Kroah-Hartman, linux-m68k, linux-kernel, kernel
[-- Attachment #1: Type: text/plain, Size: 1753 bytes --]
On Mon, 2 Aug 2021, Uwe Kleine-König wrote:
> >
> > BTW, aside from nubus, zorro and superhyway you can find the same
> > pattern in many other busses. You may want to patch the following
> > methods too.
> >
> > acpi_device_remove
> > apr_device_remove
> > ccwgroup_remove
> > gio_device_remove
> > hid_device_remove
> > ibmebus_bus_device_remove
> > macio_device_remove
> > memstick_device_remove
> > ntb_remove
> > pci_device_remove
> > pnp_device_remove
> > ps3_system_bus_remove
> > rio_device_remove
> > slim_device_remove
> > soundbus_device_remove
> > ssb_device_remove
> > tifm_device_remove
> > vdpa_dev_remove
> > vmbus_remove
>
> Did you find these by hand? Or using a coccinelle match?
>
I used grep and visual inspection.
> Anyhow, thanks for the list, I'll add it to my todo list but if you're
> motivated don't consider these cleanups as my property. (Please Cc: me
> though to prevent duplicated effort.)
>
I went looking for the other examples of this pattern because I wanted to
understand it better. Then I realized that I might as well make a list
since I was searching anyway.
When skimming that code, I took the impression that the dev->driver ==
NULL test probably comes from old code written before the 'remove'
functions became bus methods. See also commit 594c8281f905 ("[PATCH] Add
bus_type probe, remove, shutdown methods.")
Back when I wrote drivers/nubus/bus.c, apparently I copied-and-pasted the
old pattern from drivers/pci/pci-driver.c, even though that pattern was
already obsolete.
So I do see the value in this cleanup but I'm afraid I'm too busy to help
further. If you don't want to finish it perhaps you can get the janitors
to do so.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/4] sh: superhyway: Simplify check in remove callback
2021-07-30 19:10 [PATCH v2 0/4] Some cleanups after making bus_type::remove return void Uwe Kleine-König
2021-07-30 19:10 ` [PATCH v2 1/4] nubus: Simplify check in remove callback Uwe Kleine-König
@ 2021-07-30 19:10 ` Uwe Kleine-König
2021-07-30 19:10 ` [PATCH v2 3/4] zorro: Simplify " Uwe Kleine-König
2021-07-30 19:10 ` [PATCH v2 4/4] zorro: Drop useless (and hardly used) .driver member in struct zorro_dev Uwe Kleine-König
3 siblings, 0 replies; 10+ messages in thread
From: Uwe Kleine-König @ 2021-07-30 19:10 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, kernel, Rich Felker, Samuel Iglesias Gonsálvez,
Dmitry Torokhov, Chen-Yu Tsai, Pali Rohár, linux-sh
The driver core only calls a remove callback when the device was
successfully bound (aka probed) before. So dev->driver is never NULL.
(And even if it was NULL, to_superhyway_driver(NULL) isn't ...)
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/sh/superhyway/superhyway.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/sh/superhyway/superhyway.c b/drivers/sh/superhyway/superhyway.c
index c0ab904c76ec..44324abe21da 100644
--- a/drivers/sh/superhyway/superhyway.c
+++ b/drivers/sh/superhyway/superhyway.c
@@ -155,7 +155,7 @@ static void superhyway_device_remove(struct device *dev)
struct superhyway_device *shyway_dev = to_superhyway_device(dev);
struct superhyway_driver *shyway_drv = to_superhyway_driver(dev->driver);
- if (shyway_drv && shyway_drv->remove)
+ if (shyway_drv->remove)
shyway_drv->remove(shyway_dev);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/4] zorro: Simplify remove callback
2021-07-30 19:10 [PATCH v2 0/4] Some cleanups after making bus_type::remove return void Uwe Kleine-König
2021-07-30 19:10 ` [PATCH v2 1/4] nubus: Simplify check in remove callback Uwe Kleine-König
2021-07-30 19:10 ` [PATCH v2 2/4] sh: superhyway: " Uwe Kleine-König
@ 2021-07-30 19:10 ` Uwe Kleine-König
2021-08-09 13:22 ` Geert Uytterhoeven
2021-07-30 19:10 ` [PATCH v2 4/4] zorro: Drop useless (and hardly used) .driver member in struct zorro_dev Uwe Kleine-König
3 siblings, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2021-07-30 19:10 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, kernel, Geert Uytterhoeven, linux-m68k
The driver core only calls a remove callback when the device was
successfully bound (aka probed) before. So dev->driver is never NULL.
(And even if it was NULL, to_zorro_driver(NULL) isn't ...)
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/zorro/zorro-driver.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/zorro/zorro-driver.c b/drivers/zorro/zorro-driver.c
index c18524bb8b2a..ab06c9ce2c78 100644
--- a/drivers/zorro/zorro-driver.c
+++ b/drivers/zorro/zorro-driver.c
@@ -67,11 +67,9 @@ static void zorro_device_remove(struct device *dev)
struct zorro_dev *z = to_zorro_dev(dev);
struct zorro_driver *drv = to_zorro_driver(dev->driver);
- if (drv) {
- if (drv->remove)
- drv->remove(z);
- z->driver = NULL;
- }
+ if (drv->remove)
+ drv->remove(z);
+ z->driver = NULL;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 3/4] zorro: Simplify remove callback
2021-07-30 19:10 ` [PATCH v2 3/4] zorro: Simplify " Uwe Kleine-König
@ 2021-08-09 13:22 ` Geert Uytterhoeven
0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2021-08-09 13:22 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Greg Kroah-Hartman, Linux Kernel Mailing List, Sascha Hauer,
linux-m68k
On Fri, Jul 30, 2021 at 9:10 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> The driver core only calls a remove callback when the device was
> successfully bound (aka probed) before. So dev->driver is never NULL.
>
> (And even if it was NULL, to_zorro_driver(NULL) isn't ...)
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
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] 10+ messages in thread
* [PATCH v2 4/4] zorro: Drop useless (and hardly used) .driver member in struct zorro_dev
2021-07-30 19:10 [PATCH v2 0/4] Some cleanups after making bus_type::remove return void Uwe Kleine-König
` (2 preceding siblings ...)
2021-07-30 19:10 ` [PATCH v2 3/4] zorro: Simplify " Uwe Kleine-König
@ 2021-07-30 19:10 ` Uwe Kleine-König
2021-08-09 13:25 ` Geert Uytterhoeven
3 siblings, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2021-07-30 19:10 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, kernel, Geert Uytterhoeven, linux-m68k
The only actual use is to check in zorro_device_probe() that the device
isn't already bound. The driver core already ensures this however so the
check can go away which allows to drop the then assigned-only member
from struct zorro_dev.
If the value was indeed needed somewhere it can always be calculated by
to_zorro_driver(z->dev.driver)
.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/zorro/zorro-driver.c | 7 ++-----
include/linux/zorro.h | 1 -
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/zorro/zorro-driver.c b/drivers/zorro/zorro-driver.c
index ab06c9ce2c78..96f068830549 100644
--- a/drivers/zorro/zorro-driver.c
+++ b/drivers/zorro/zorro-driver.c
@@ -47,16 +47,14 @@ static int zorro_device_probe(struct device *dev)
struct zorro_driver *drv = to_zorro_driver(dev->driver);
struct zorro_dev *z = to_zorro_dev(dev);
- if (!z->driver && drv->probe) {
+ if (drv->probe) {
const struct zorro_device_id *id;
id = zorro_match_device(drv->id_table, z);
if (id)
error = drv->probe(z, id);
- if (error >= 0) {
- z->driver = drv;
+ if (error >= 0)
error = 0;
- }
}
return error;
}
@@ -69,7 +67,6 @@ static void zorro_device_remove(struct device *dev)
if (drv->remove)
drv->remove(z);
- z->driver = NULL;
}
diff --git a/include/linux/zorro.h b/include/linux/zorro.h
index e2e4de188d84..db7416ed6057 100644
--- a/include/linux/zorro.h
+++ b/include/linux/zorro.h
@@ -29,7 +29,6 @@
struct zorro_dev {
struct ExpansionRom rom;
zorro_id id;
- struct zorro_driver *driver; /* which driver has allocated this device */
struct device dev; /* Generic device interface */
u16 slotaddr;
u16 slotsize;
--
2.30.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 4/4] zorro: Drop useless (and hardly used) .driver member in struct zorro_dev
2021-07-30 19:10 ` [PATCH v2 4/4] zorro: Drop useless (and hardly used) .driver member in struct zorro_dev Uwe Kleine-König
@ 2021-08-09 13:25 ` Geert Uytterhoeven
0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2021-08-09 13:25 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Greg Kroah-Hartman, Linux Kernel Mailing List, Sascha Hauer,
linux-m68k
Hi Uwe,
On Fri, Jul 30, 2021 at 9:10 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> The only actual use is to check in zorro_device_probe() that the device
> isn't already bound. The driver core already ensures this however so the
> check can go away which allows to drop the then assigned-only member
> from struct zorro_dev.
>
> If the value was indeed needed somewhere it can always be calculated by
>
> to_zorro_driver(z->dev.driver)
>
> .
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Thanks for your patch!
> --- a/drivers/zorro/zorro-driver.c
> +++ b/drivers/zorro/zorro-driver.c
> @@ -47,16 +47,14 @@ static int zorro_device_probe(struct device *dev)
> struct zorro_driver *drv = to_zorro_driver(dev->driver);
> struct zorro_dev *z = to_zorro_dev(dev);
>
> - if (!z->driver && drv->probe) {
> + if (drv->probe) {
> const struct zorro_device_id *id;
>
> id = zorro_match_device(drv->id_table, z);
> if (id)
> error = drv->probe(z, id);
> - if (error >= 0) {
> - z->driver = drv;
> + if (error >= 0)
I guess this test can become "> 0" now, but that doesn't matter much.
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> error = 0;
> - }
> }
> return error;
> }
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] 10+ messages in thread
end of thread, other threads:[~2021-08-09 13:26 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-30 19:10 [PATCH v2 0/4] Some cleanups after making bus_type::remove return void Uwe Kleine-König
2021-07-30 19:10 ` [PATCH v2 1/4] nubus: Simplify check in remove callback Uwe Kleine-König
2021-08-01 5:04 ` Finn Thain
2021-08-02 6:52 ` Uwe Kleine-König
2021-08-03 2:14 ` Finn Thain
2021-07-30 19:10 ` [PATCH v2 2/4] sh: superhyway: " Uwe Kleine-König
2021-07-30 19:10 ` [PATCH v2 3/4] zorro: Simplify " Uwe Kleine-König
2021-08-09 13:22 ` Geert Uytterhoeven
2021-07-30 19:10 ` [PATCH v2 4/4] zorro: Drop useless (and hardly used) .driver member in struct zorro_dev Uwe Kleine-König
2021-08-09 13:25 ` Geert Uytterhoeven
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.