Hi Greg,
On 6/4/19 12:03 PM, Greg KH wrote:
On Tue, Jun 04, 2019 at 11:13:02AM -0500, Richard Gong wrote:
Hi Greg,
On 6/4/19 9:28 AM, Greg KH wrote:
On Tue, Jun 04, 2019 at 12:33:03PM +0200, Romain Izard wrote:
On Mon, Jun 03, 2019 at 08:02:55PM +0200, Greg KH wrote:
@@ -394,7 +432,7 @@ static struct platform_driver stratix10_rsu_driver = {
.remove = stratix10_rsu_remove,
.driver = {
.name = "stratix10-rsu",
- .groups = rsu_groups,
+// .groups = rsu_groups,
Are you sure this is the correct pointer? I think that might be
pointing to the driver's attributes, not the device's attributes.
If platform drivers do not have a way to register groups properly, then
that really needs to be fixed, as trying to register it by yourself as
you are doing, is ripe for racing with userspace.
This is a very common issue with platform drivers, and it seems to me that
it is not possible to add device attributes when binding a device to a
driver without entering the race condition.
My understanding is the following one:
The root cause is that the device has already been created and reported
to the userspace with a KOBJ_ADD uevent before the device and the driver
are bound together. On receiving this event, userspace will react, and
it will try to read the device's attributes. In parallel the kernel will
try to find a matching driver. If a driver is found, the kernel will
call the probe function from the driver with the device as a parameter,
and if successful a KOBJ_BIND uevent will be sent to userspace, but this
is a recent addition.
Unfortunately, not all created devices will be bound to a driver, and the
existing udev code relies on KOBJ_ADD uevents rather than KOBJ_BIND uevents.
If new per-device attributes have been added to the device during the
binding stage userspace may or may not see them, depending on when userspace
tries to read the device's attributes.
I have this possible workaround, but I do not know if it is a good solution:
When binding the device and the driver together, create a new device as a
child to the current device, and fill its "groups" member to point to the
per-device attributes' group. As the device will be created with all the
attributes, it will not be affected by the race issues. The functions
handling the attributes will need to be modified to use the parents of their
"device" parameter, instead of the device itself. Additionnaly, the sysfs
location of the attributes will be different, as the child device will show
up in the sysfs path. But for a newly introduced device this will not be
a problem.
Is this a good compromise ?
Not really. You just want the attributes on the platform device itself.
Given the horrible hack that platform devices are today, what's one more
hack!
Here's a patch below of what should probably be done here. Richard, can
you change your code to use the new dev_groups pointer in the struct
platform_driver and this patch and let me know if that works or not?
Note, I've only compiled this code, not tested it...
Your patch works.
Many thanks for your help!
Nice!
I guess I need to turn it into a real patch now. Let me do that tonight
and see if I can convert some existing drivers to use it as well...
Sorry for asking.
I haven't seen your patch, did you release that?
Regards,
Richard
thanks,
greg k-h
