Hi Michael, After checking the definition of modalias in modalias_show(), I think it's better to keep the same logic in vio_hotplug(), that's removing the else part in my original patch shown below. + if (dn && (cp = of_get_property(dn, "compatible", NULL)) + add_uevent_var(env, "MODALIAS=vio:T%sS%s", vio_dev->type, cp); + else + add_uevent_var(env, "MODALIAS=vio:T%s", vio_dev->type); I think we can avoid some possible regression then. I'll make the change in my v2 patch.
-- Regards, Lidong Zhong On Wed, Apr 10, 2024 at 9:25 AM Lidong Zhong <lidong.zh...@suse.com> wrote: > Hi Michael, > > Thanks for your reply. > > On Tue, Apr 9, 2024 at 4:46 PM Michael Ellerman <m...@ellerman.id.au> > wrote: > >> Hi Lidong, >> >> Thanks for the patch. >> >> I'm not an expert on udev etc. so apologies if any of these questions >> are stupid. >> >> Lidong Zhong <lidong.zh...@suse.com> writes: >> > We have noticed the following nuisance messages during boot >> > >> > [ 7.120610][ T1060] vio vio: uevent: failed to send synthetic uevent >> > [ 7.122281][ T1060] vio 4000: uevent: failed to send synthetic uevent >> > [ 7.122304][ T1060] vio 4001: uevent: failed to send synthetic uevent >> > [ 7.122324][ T1060] vio 4002: uevent: failed to send synthetic uevent >> > [ 7.122345][ T1060] vio 4004: uevent: failed to send synthetic uevent >> > >> > It's caused by either vio_register_device_node() failed to set >> dev->of_node or >> > the missing "compatible" property. Try return as much information as >> possible >> > instead of a failure. >> >> Does udev etc. cope with that? Can we just change the content of the >> MODALIAS value like that? >> >> With this patch we'll start emitting uevents for devices we previously >> didn't. I guess that's OK because nothing is expecting them? >> >> Can you include a log of udev showing the event firing and that nothing >> breaks. >> >> On my system here I see nothing matches the devices except for libvpd, >> which seems to match lots of things. >> > > It's an issue reported by our customer. I am sorry I can't provide more > information because I don't have the environment > to reproduce this issue. The only related log I got is shown below: > > Feb 07 14:08:03 rb3i0060 udevadm[623]: vio: Failed to write 'add' to > '/sys/devices/vio/uevent', ignoring: No such device > > Feb 07 14:08:03 rb3i0060 kernel: synth uevent: /devices/vio: failed to > send uevent > > Feb 07 14:08:03 rb3i0060 kernel: vio vio: uevent: failed to send synthetic > uevent > > Feb 07 14:08:03 rb3i0060 kernel: synth uevent: /devices/vio/4000: failed > to send uevent > > Feb 07 14:08:03 rb3i0060 kernel: vio 4000: uevent: failed to send > synthetic uevent > > Feb 07 14:08:03 rb3i0060 kernel: synth uevent: /devices/vio/4001: failed > to send uevent > > Feb 07 14:08:03 rb3i0060 kernel: vio 4001: uevent: failed to send > synthetic uevent > > Feb 07 14:08:03 rb3i0060 kernel: synth uevent: /devices/vio/4002: failed > to send uevent > > Feb 07 14:08:03 rb3i0060 kernel: vio 4002: uevent: failed to send > synthetic uevent > > Feb 07 14:08:03 rb3i0060 kernel: synth uevent: /devices/vio/4004: failed > to send uevent > > Feb 07 14:08:03 rb3i0060 kernel: vio 4004: uevent: failed to send > synthetic uevent > > Feb 07 14:08:03 rb3i0060 udevadm[623]: 4000: Failed to write 'add' to > '/sys/devices/vio/4000/uevent', ignoring: No such device > > Feb 07 14:08:03 rb3i0060 udevadm[623]: 4001: Failed to write 'add' to > '/sys/devices/vio/4001/uevent', ignoring: No such device > > Feb 07 14:08:03 rb3i0060 udevadm[623]: 4002: Failed to write 'add' to > '/sys/devices/vio/4002/uevent', ignoring: No such device > > Feb 07 14:08:03 rb3i0060 udevadm[623]: 4004: Failed to write 'add' to > '/sys/devices/vio/4004/uevent', ignoring: No such device > > systemd-udev-trigger service calls 'udevadm trigger --type=devices > --action=add' and kernel returns -ENODEV because either > dev->of_node is NULL or 'compatible' property is not present. Similar > cases were already reported after some search, for example > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1827162 > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1845319 > I don't think it causes real problems but confusion to users. > > >> > diff --git a/arch/powerpc/platforms/pseries/vio.c >> b/arch/powerpc/platforms/pseries/vio.c >> > index 90ff85c879bf..62961715ca24 100644 >> > --- a/arch/powerpc/platforms/pseries/vio.c >> > +++ b/arch/powerpc/platforms/pseries/vio.c >> > @@ -1593,12 +1593,13 @@ static int vio_hotplug(const struct device >> *dev, struct kobj_uevent_env *env) >> > >> > dn = dev->of_node; >> > if (!dn) >> > - return -ENODEV; >> > + goto out; >> > cp = of_get_property(dn, "compatible", NULL); >> > if (!cp) >> > - return -ENODEV; >> > - >> > - add_uevent_var(env, "MODALIAS=vio:T%sS%s", vio_dev->type, cp); >> > + add_uevent_var(env, "MODALIAS=vio:T%s", vio_dev->type); >> >> If it's OK to skip the compatible property then we don't need the >> of_node at all, and we could always emit this, even when of_node is not >> available. >> > > You mean something like this? > @@ -1592,13 +1592,10 @@ static int vio_hotplug(const struct device *dev, > struct kobj_uevent_env *env) > const char *cp; > > dn = dev->of_node; > - if (!dn) > - return -ENODEV; > - cp = of_get_property(dn, "compatible", NULL); > - if (!cp) > - return -ENODEV; > - > - add_uevent_var(env, "MODALIAS=vio:T%sS%s", vio_dev->type, cp); > + if (dn && (cp = of_get_property(dn, "compatible", NULL)) > + add_uevent_var(env, "MODALIAS=vio:T%sS%s", vio_dev->type, > cp); > + else > + add_uevent_var(env, "MODALIAS=vio:T%s", vio_dev->type); > return 0; > > >> >> > + else >> > + add_uevent_var(env, "MODALIAS=vio:T%sS%s", vio_dev->type, >> cp); >> > +out: >> > return 0; >> > } >> >> I think we also should update the vio modalias_show() to follow the same >> logic, otherwise the uevent MODALIAS value and the modalias file won't >> match which is confusing. >> >> Preferably vio_hotplug() and modalias_show() would just call a common >> helper. >> >> cheers >> > > Thanks for the suggestion. I'll send a v2 patch. > > > -- > Regards, > Lidong Zhong >
