LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] powerpc/vio: use simple dummy struct device as bus parent
@ 2019-09-27 13:04 Dan Streetman
  2019-09-27 18:18 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 7+ messages in thread
From: Dan Streetman @ 2019-09-27 13:04 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Rob Herring, Alexey Kardashevskiy, Greg Kroah-Hartman,
	linux-kernel, Dan Streetman, Thomas Gleixner, linuxppc-dev

The dummy vio_bus_device creates the /sys/devices/vio directory, which
contains real vio devices under it; since it represents itself as having
a bus = &vio_bus_type, its /sys/devices/vio/uevent does call the bus's
.uevent function, vio_hotplug(), and as that function won't find a real
device for the dummy vio_dev, it will return -ENODEV.

One of the main users of the uevent node is udevadm, e.g. when it is called
with 'udevadm trigger --devices'.  Up until recently, it would ignore any
errors returned when writing to devices' uevent file, but it was recently
changed to start returning error if it gets an error writing to any uevent
file:
https://github.com/systemd/systemd/commit/97afc0351a96e0daa83964df33937967c75c644f

since the /sys/devices/vio/uevent file has always returned ENODEV from
any write to it, this now causes the udevadm trigger command to return
an error.  This may be fixed in udevadm to ignore ENODEV errors, but the
vio driver should still be fixed.

This patch changes the arch/powerpc/platform/pseries/vio.c 'dummy'
parent device into a real dummy device with no .bus, so its uevent
file will stop returning ENODEV and simply do nothing and return 0.

Signed-off-by: Dan Streetman <ddstreet@canonical.com>
---
 arch/powerpc/platforms/pseries/vio.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
index 79e2287991db..63bc16631680 100644
--- a/arch/powerpc/platforms/pseries/vio.c
+++ b/arch/powerpc/platforms/pseries/vio.c
@@ -32,11 +32,8 @@
 #include <asm/page.h>
 #include <asm/hvcall.h>
 
-static struct vio_dev vio_bus_device  = { /* fake "parent" device */
-	.name = "vio",
-	.type = "",
-	.dev.init_name = "vio",
-	.dev.bus = &vio_bus_type,
+static struct device vio_bus = {
+	.init_name	= "vio",
 };
 
 #ifdef CONFIG_PPC_SMLPAR
@@ -1412,7 +1409,7 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
 	set_dev_node(&viodev->dev, of_node_to_nid(of_node));
 
 	/* init generic 'struct device' fields: */
-	viodev->dev.parent = &vio_bus_device.dev;
+	viodev->dev.parent = &vio_bus;
 	viodev->dev.bus = &vio_bus_type;
 	viodev->dev.release = vio_dev_release;
 
@@ -1499,7 +1496,7 @@ static int __init vio_bus_init(void)
 	 * The fake parent of all vio devices, just to give us
 	 * a nice directory
 	 */
-	err = device_register(&vio_bus_device.dev);
+	err = device_register(&vio_bus);
 	if (err) {
 		printk(KERN_WARNING "%s: device_register returned %i\n",
 				__func__, err);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] powerpc/vio: use simple dummy struct device as bus parent
  2019-09-27 13:04 [PATCH] powerpc/vio: use simple dummy struct device as bus parent Dan Streetman
@ 2019-09-27 18:18 ` Greg Kroah-Hartman
  2019-09-27 19:48   ` Dan Streetman
  2019-10-03 19:10   ` Dan Streetman
  0 siblings, 2 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2019-09-27 18:18 UTC (permalink / raw)
  To: Dan Streetman
  Cc: Rob Herring, Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
	Thomas Gleixner, linuxppc-dev

On Fri, Sep 27, 2019 at 09:04:02AM -0400, Dan Streetman wrote:
> The dummy vio_bus_device creates the /sys/devices/vio directory, which
> contains real vio devices under it; since it represents itself as having
> a bus = &vio_bus_type, its /sys/devices/vio/uevent does call the bus's
> .uevent function, vio_hotplug(), and as that function won't find a real
> device for the dummy vio_dev, it will return -ENODEV.
> 
> One of the main users of the uevent node is udevadm, e.g. when it is called
> with 'udevadm trigger --devices'.  Up until recently, it would ignore any
> errors returned when writing to devices' uevent file, but it was recently
> changed to start returning error if it gets an error writing to any uevent
> file:
> https://github.com/systemd/systemd/commit/97afc0351a96e0daa83964df33937967c75c644f
> 
> since the /sys/devices/vio/uevent file has always returned ENODEV from
> any write to it, this now causes the udevadm trigger command to return
> an error.  This may be fixed in udevadm to ignore ENODEV errors, but the
> vio driver should still be fixed.
> 
> This patch changes the arch/powerpc/platform/pseries/vio.c 'dummy'
> parent device into a real dummy device with no .bus, so its uevent
> file will stop returning ENODEV and simply do nothing and return 0.
> 
> Signed-off-by: Dan Streetman <ddstreet@canonical.com>
> ---
>  arch/powerpc/platforms/pseries/vio.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
> index 79e2287991db..63bc16631680 100644
> --- a/arch/powerpc/platforms/pseries/vio.c
> +++ b/arch/powerpc/platforms/pseries/vio.c
> @@ -32,11 +32,8 @@
>  #include <asm/page.h>
>  #include <asm/hvcall.h>
>  
> -static struct vio_dev vio_bus_device  = { /* fake "parent" device */
> -	.name = "vio",
> -	.type = "",
> -	.dev.init_name = "vio",
> -	.dev.bus = &vio_bus_type,
> +static struct device vio_bus = {
> +	.init_name	= "vio",

Eeek, no!  Why are you creating a static device that will then be
reference counted?  Not nice :(

What's wrong with a simple call to device_create() for your "fake"
device you want to make here?  That's what it is there for :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] powerpc/vio: use simple dummy struct device as bus parent
  2019-09-27 18:18 ` Greg Kroah-Hartman
@ 2019-09-27 19:48   ` Dan Streetman
  2019-09-28  7:41     ` Greg Kroah-Hartman
  2019-10-03 19:10   ` Dan Streetman
  1 sibling, 1 reply; 7+ messages in thread
From: Dan Streetman @ 2019-09-27 19:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rob Herring, Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
	Thomas Gleixner, linuxppc-dev

On Fri, Sep 27, 2019 at 2:19 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Sep 27, 2019 at 09:04:02AM -0400, Dan Streetman wrote:
> > The dummy vio_bus_device creates the /sys/devices/vio directory, which
> > contains real vio devices under it; since it represents itself as having
> > a bus = &vio_bus_type, its /sys/devices/vio/uevent does call the bus's
> > .uevent function, vio_hotplug(), and as that function won't find a real
> > device for the dummy vio_dev, it will return -ENODEV.
> >
> > One of the main users of the uevent node is udevadm, e.g. when it is called
> > with 'udevadm trigger --devices'.  Up until recently, it would ignore any
> > errors returned when writing to devices' uevent file, but it was recently
> > changed to start returning error if it gets an error writing to any uevent
> > file:
> > https://github.com/systemd/systemd/commit/97afc0351a96e0daa83964df33937967c75c644f
> >
> > since the /sys/devices/vio/uevent file has always returned ENODEV from
> > any write to it, this now causes the udevadm trigger command to return
> > an error.  This may be fixed in udevadm to ignore ENODEV errors, but the
> > vio driver should still be fixed.
> >
> > This patch changes the arch/powerpc/platform/pseries/vio.c 'dummy'
> > parent device into a real dummy device with no .bus, so its uevent
> > file will stop returning ENODEV and simply do nothing and return 0.
> >
> > Signed-off-by: Dan Streetman <ddstreet@canonical.com>
> > ---
> >  arch/powerpc/platforms/pseries/vio.c | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
> > index 79e2287991db..63bc16631680 100644
> > --- a/arch/powerpc/platforms/pseries/vio.c
> > +++ b/arch/powerpc/platforms/pseries/vio.c
> > @@ -32,11 +32,8 @@
> >  #include <asm/page.h>
> >  #include <asm/hvcall.h>
> >
> > -static struct vio_dev vio_bus_device  = { /* fake "parent" device */
> > -     .name = "vio",
> > -     .type = "",
> > -     .dev.init_name = "vio",
> > -     .dev.bus = &vio_bus_type,
> > +static struct device vio_bus = {
> > +     .init_name      = "vio",
>
> Eeek, no!  Why are you creating a static device that will then be
> reference counted?  Not nice :(

sorry!  I'll admit that I simply copied what drivers/base/platform.c
seemed to be doing.

>
> What's wrong with a simple call to device_create() for your "fake"
> device you want to make here?  That's what it is there for :)

ack, will send a new patch using that.  thanks!

>
> thanks,
>
> greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] powerpc/vio: use simple dummy struct device as bus parent
  2019-09-27 19:48   ` Dan Streetman
@ 2019-09-28  7:41     ` Greg Kroah-Hartman
  2019-09-28 13:28       ` Dan Streetman
  0 siblings, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2019-09-28  7:41 UTC (permalink / raw)
  To: Dan Streetman
  Cc: Rob Herring, Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
	Thomas Gleixner, linuxppc-dev

On Fri, Sep 27, 2019 at 03:48:49PM -0400, Dan Streetman wrote:
> On Fri, Sep 27, 2019 at 2:19 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Sep 27, 2019 at 09:04:02AM -0400, Dan Streetman wrote:
> > > The dummy vio_bus_device creates the /sys/devices/vio directory, which
> > > contains real vio devices under it; since it represents itself as having
> > > a bus = &vio_bus_type, its /sys/devices/vio/uevent does call the bus's
> > > .uevent function, vio_hotplug(), and as that function won't find a real
> > > device for the dummy vio_dev, it will return -ENODEV.
> > >
> > > One of the main users of the uevent node is udevadm, e.g. when it is called
> > > with 'udevadm trigger --devices'.  Up until recently, it would ignore any
> > > errors returned when writing to devices' uevent file, but it was recently
> > > changed to start returning error if it gets an error writing to any uevent
> > > file:
> > > https://github.com/systemd/systemd/commit/97afc0351a96e0daa83964df33937967c75c644f
> > >
> > > since the /sys/devices/vio/uevent file has always returned ENODEV from
> > > any write to it, this now causes the udevadm trigger command to return
> > > an error.  This may be fixed in udevadm to ignore ENODEV errors, but the
> > > vio driver should still be fixed.
> > >
> > > This patch changes the arch/powerpc/platform/pseries/vio.c 'dummy'
> > > parent device into a real dummy device with no .bus, so its uevent
> > > file will stop returning ENODEV and simply do nothing and return 0.
> > >
> > > Signed-off-by: Dan Streetman <ddstreet@canonical.com>
> > > ---
> > >  arch/powerpc/platforms/pseries/vio.c | 11 ++++-------
> > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
> > > index 79e2287991db..63bc16631680 100644
> > > --- a/arch/powerpc/platforms/pseries/vio.c
> > > +++ b/arch/powerpc/platforms/pseries/vio.c
> > > @@ -32,11 +32,8 @@
> > >  #include <asm/page.h>
> > >  #include <asm/hvcall.h>
> > >
> > > -static struct vio_dev vio_bus_device  = { /* fake "parent" device */
> > > -     .name = "vio",
> > > -     .type = "",
> > > -     .dev.init_name = "vio",
> > > -     .dev.bus = &vio_bus_type,
> > > +static struct device vio_bus = {
> > > +     .init_name      = "vio",
> >
> > Eeek, no!  Why are you creating a static device that will then be
> > reference counted?  Not nice :(
> 
> sorry!  I'll admit that I simply copied what drivers/base/platform.c
> seemed to be doing.

I don't see platform.c having a 'static struct device' anywhere in it,
am I missing it in my searching?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] powerpc/vio: use simple dummy struct device as bus parent
  2019-09-28  7:41     ` Greg Kroah-Hartman
@ 2019-09-28 13:28       ` Dan Streetman
  0 siblings, 0 replies; 7+ messages in thread
From: Dan Streetman @ 2019-09-28 13:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rob Herring, Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
	Thomas Gleixner, linuxppc-dev

On Sat, Sep 28, 2019 at 3:41 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Sep 27, 2019 at 03:48:49PM -0400, Dan Streetman wrote:
> > On Fri, Sep 27, 2019 at 2:19 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Fri, Sep 27, 2019 at 09:04:02AM -0400, Dan Streetman wrote:
> > > > The dummy vio_bus_device creates the /sys/devices/vio directory, which
> > > > contains real vio devices under it; since it represents itself as having
> > > > a bus = &vio_bus_type, its /sys/devices/vio/uevent does call the bus's
> > > > .uevent function, vio_hotplug(), and as that function won't find a real
> > > > device for the dummy vio_dev, it will return -ENODEV.
> > > >
> > > > One of the main users of the uevent node is udevadm, e.g. when it is called
> > > > with 'udevadm trigger --devices'.  Up until recently, it would ignore any
> > > > errors returned when writing to devices' uevent file, but it was recently
> > > > changed to start returning error if it gets an error writing to any uevent
> > > > file:
> > > > https://github.com/systemd/systemd/commit/97afc0351a96e0daa83964df33937967c75c644f
> > > >
> > > > since the /sys/devices/vio/uevent file has always returned ENODEV from
> > > > any write to it, this now causes the udevadm trigger command to return
> > > > an error.  This may be fixed in udevadm to ignore ENODEV errors, but the
> > > > vio driver should still be fixed.
> > > >
> > > > This patch changes the arch/powerpc/platform/pseries/vio.c 'dummy'
> > > > parent device into a real dummy device with no .bus, so its uevent
> > > > file will stop returning ENODEV and simply do nothing and return 0.
> > > >
> > > > Signed-off-by: Dan Streetman <ddstreet@canonical.com>
> > > > ---
> > > >  arch/powerpc/platforms/pseries/vio.c | 11 ++++-------
> > > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
> > > > index 79e2287991db..63bc16631680 100644
> > > > --- a/arch/powerpc/platforms/pseries/vio.c
> > > > +++ b/arch/powerpc/platforms/pseries/vio.c
> > > > @@ -32,11 +32,8 @@
> > > >  #include <asm/page.h>
> > > >  #include <asm/hvcall.h>
> > > >
> > > > -static struct vio_dev vio_bus_device  = { /* fake "parent" device */
> > > > -     .name = "vio",
> > > > -     .type = "",
> > > > -     .dev.init_name = "vio",
> > > > -     .dev.bus = &vio_bus_type,
> > > > +static struct device vio_bus = {
> > > > +     .init_name      = "vio",
> > >
> > > Eeek, no!  Why are you creating a static device that will then be
> > > reference counted?  Not nice :(
> >
> > sorry!  I'll admit that I simply copied what drivers/base/platform.c
> > seemed to be doing.
>
> I don't see platform.c having a 'static struct device' anywhere in it,
> am I missing it in my searching?

no, you are right, what I meant was:

struct device platform_bus = {
        .init_name      = "platform",
};


>
> thanks,
>
> greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] powerpc/vio: use simple dummy struct device as bus parent
  2019-09-27 18:18 ` Greg Kroah-Hartman
  2019-09-27 19:48   ` Dan Streetman
@ 2019-10-03 19:10   ` Dan Streetman
  2019-10-03 19:21     ` Greg Kroah-Hartman
  1 sibling, 1 reply; 7+ messages in thread
From: Dan Streetman @ 2019-10-03 19:10 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rob Herring, Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
	Thomas Gleixner, linuxppc-dev

On Fri, Sep 27, 2019 at 2:19 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Sep 27, 2019 at 09:04:02AM -0400, Dan Streetman wrote:
> > The dummy vio_bus_device creates the /sys/devices/vio directory, which
> > contains real vio devices under it; since it represents itself as having
> > a bus = &vio_bus_type, its /sys/devices/vio/uevent does call the bus's
> > .uevent function, vio_hotplug(), and as that function won't find a real
> > device for the dummy vio_dev, it will return -ENODEV.
> >
> > One of the main users of the uevent node is udevadm, e.g. when it is called
> > with 'udevadm trigger --devices'.  Up until recently, it would ignore any
> > errors returned when writing to devices' uevent file, but it was recently
> > changed to start returning error if it gets an error writing to any uevent
> > file:
> > https://github.com/systemd/systemd/commit/97afc0351a96e0daa83964df33937967c75c644f
> >
> > since the /sys/devices/vio/uevent file has always returned ENODEV from
> > any write to it, this now causes the udevadm trigger command to return
> > an error.  This may be fixed in udevadm to ignore ENODEV errors, but the
> > vio driver should still be fixed.
> >
> > This patch changes the arch/powerpc/platform/pseries/vio.c 'dummy'
> > parent device into a real dummy device with no .bus, so its uevent
> > file will stop returning ENODEV and simply do nothing and return 0.
> >
> > Signed-off-by: Dan Streetman <ddstreet@canonical.com>
> > ---
> >  arch/powerpc/platforms/pseries/vio.c | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
> > index 79e2287991db..63bc16631680 100644
> > --- a/arch/powerpc/platforms/pseries/vio.c
> > +++ b/arch/powerpc/platforms/pseries/vio.c
> > @@ -32,11 +32,8 @@
> >  #include <asm/page.h>
> >  #include <asm/hvcall.h>
> >
> > -static struct vio_dev vio_bus_device  = { /* fake "parent" device */
> > -     .name = "vio",
> > -     .type = "",
> > -     .dev.init_name = "vio",
> > -     .dev.bus = &vio_bus_type,
> > +static struct device vio_bus = {
> > +     .init_name      = "vio",
>
> Eeek, no!  Why are you creating a static device that will then be
> reference counted?  Not nice :(

so, I looked again and it seems quite a few places appear to do
exactly this, is it something that should be fixed?

$ git grep 'static struct device [^*{]*{'
arch/arm/kernel/dma-isa.c:static struct device isa_dma_dev = {
arch/arm/mach-rpc/dma.c:static struct device isa_dma_dev = {
arch/arm/mach-s3c24xx/s3c2410.c:static struct device s3c2410_dev = {
arch/arm/mach-s3c24xx/s3c2412.c:static struct device s3c2412_dev = {
arch/arm/mach-s3c24xx/s3c2416.c:static struct device s3c2416_dev = {
arch/arm/mach-s3c24xx/s3c2440.c:static struct device s3c2440_dev = {
arch/arm/mach-s3c24xx/s3c2442.c:static struct device s3c2442_dev = {
arch/arm/mach-s3c24xx/s3c2443.c:static struct device s3c2443_dev = {
arch/arm/mach-s3c64xx/common.c:static struct device s3c64xx_dev = {
arch/arm/mach-s3c64xx/s3c6400.c:static struct device s3c6400_dev = {
arch/arm/mach-s3c64xx/s3c6410.c:static struct device s3c6410_dev = {
arch/mips/sgi-ip22/ip22-gio.c:static struct device gio_bus = {
arch/parisc/kernel/drivers.c:static struct device root = {
arch/powerpc/platforms/ps3/system-bus.c:static struct device ps3_system_bus = {
arch/powerpc/platforms/pseries/ibmebus.c:static struct device
ibmebus_bus_device = { /* fake "parent" device */
arch/powerpc/platforms/pseries/vio.c:static struct device vio_bus = {
arch/um/drivers/virtio_uml.c:static struct device vu_cmdline_parent = {
drivers/base/isa.c:static struct device isa_bus = {
drivers/block/rbd.c:static struct device rbd_root_dev = {
drivers/gpu/drm/ttm/ttm_module.c:static struct device ttm_drm_class_device = {
drivers/iio/dummy/iio_dummy_evgen.c:static struct device iio_evgen_dev = {
drivers/iio/trigger/iio-trig-sysfs.c:static struct device iio_sysfs_trig_dev = {
drivers/misc/sgi-gru/grumain.c:static struct device gru_device = {
drivers/nubus/bus.c:static struct device nubus_parent = {
drivers/sh/maple/maple.c:static struct device maple_bus = {
drivers/sh/superhyway/superhyway.c:static struct device
superhyway_bus_device = {
drivers/soc/fsl/qe/qe_ic.c:static struct device device_qe_ic = {
drivers/virtio/virtio_mmio.c:static struct device vm_cmdline_parent = {
kernel/time/clockevents.c:static struct device tick_bc_dev = {
kernel/time/clocksource.c:static struct device device_clocksource = {


>
> What's wrong with a simple call to device_create() for your "fake"
> device you want to make here?  That's what it is there for :)
>
> thanks,
>
> greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] powerpc/vio: use simple dummy struct device as bus parent
  2019-10-03 19:10   ` Dan Streetman
@ 2019-10-03 19:21     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2019-10-03 19:21 UTC (permalink / raw)
  To: Dan Streetman
  Cc: Rob Herring, Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
	Thomas Gleixner, linuxppc-dev

On Thu, Oct 03, 2019 at 03:10:03PM -0400, Dan Streetman wrote:
> On Fri, Sep 27, 2019 at 2:19 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Sep 27, 2019 at 09:04:02AM -0400, Dan Streetman wrote:
> > > The dummy vio_bus_device creates the /sys/devices/vio directory, which
> > > contains real vio devices under it; since it represents itself as having
> > > a bus = &vio_bus_type, its /sys/devices/vio/uevent does call the bus's
> > > .uevent function, vio_hotplug(), and as that function won't find a real
> > > device for the dummy vio_dev, it will return -ENODEV.
> > >
> > > One of the main users of the uevent node is udevadm, e.g. when it is called
> > > with 'udevadm trigger --devices'.  Up until recently, it would ignore any
> > > errors returned when writing to devices' uevent file, but it was recently
> > > changed to start returning error if it gets an error writing to any uevent
> > > file:
> > > https://github.com/systemd/systemd/commit/97afc0351a96e0daa83964df33937967c75c644f
> > >
> > > since the /sys/devices/vio/uevent file has always returned ENODEV from
> > > any write to it, this now causes the udevadm trigger command to return
> > > an error.  This may be fixed in udevadm to ignore ENODEV errors, but the
> > > vio driver should still be fixed.
> > >
> > > This patch changes the arch/powerpc/platform/pseries/vio.c 'dummy'
> > > parent device into a real dummy device with no .bus, so its uevent
> > > file will stop returning ENODEV and simply do nothing and return 0.
> > >
> > > Signed-off-by: Dan Streetman <ddstreet@canonical.com>
> > > ---
> > >  arch/powerpc/platforms/pseries/vio.c | 11 ++++-------
> > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
> > > index 79e2287991db..63bc16631680 100644
> > > --- a/arch/powerpc/platforms/pseries/vio.c
> > > +++ b/arch/powerpc/platforms/pseries/vio.c
> > > @@ -32,11 +32,8 @@
> > >  #include <asm/page.h>
> > >  #include <asm/hvcall.h>
> > >
> > > -static struct vio_dev vio_bus_device  = { /* fake "parent" device */
> > > -     .name = "vio",
> > > -     .type = "",
> > > -     .dev.init_name = "vio",
> > > -     .dev.bus = &vio_bus_type,
> > > +static struct device vio_bus = {
> > > +     .init_name      = "vio",
> >
> > Eeek, no!  Why are you creating a static device that will then be
> > reference counted?  Not nice :(
> 
> so, I looked again and it seems quite a few places appear to do
> exactly this, is it something that should be fixed?

Yes.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-27 13:04 [PATCH] powerpc/vio: use simple dummy struct device as bus parent Dan Streetman
2019-09-27 18:18 ` Greg Kroah-Hartman
2019-09-27 19:48   ` Dan Streetman
2019-09-28  7:41     ` Greg Kroah-Hartman
2019-09-28 13:28       ` Dan Streetman
2019-10-03 19:10   ` Dan Streetman
2019-10-03 19:21     ` Greg Kroah-Hartman

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org linuxppc-dev@archiver.kernel.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox