Hi Uwe,
On 05/25/2018 04:41 PM, Uwe Kleine-König wrote:
Hello,
On Wed, May 16, 2018 at 09:46:20PM +0200, Jacek Anaszewski wrote:
On 05/15/2018 10:14 PM, Uwe Kleine-König wrote:
I looked into this driver and it does forbidden stuff. You must not call
device_create_file() for an already bound device. (@gregkh: Would it be
a good idea to issue a warning when device_create_file is called too
late, maybe only if CONFIG_DEBUG_KERNEL=y or CONFIG_DEBUG_SOMETHING?)
Many LED triggers create their specific sysfs files this way.
It is still not ok, even if others do it this way.
Nobody seems to have had any problem with this aspect of LED triggers
implementation so far. At least no complaint has been filed for
last 3 years for sure.
A sane approach to such trigger parameters would include putting a
const struct attribute_group **groups member into the struct
led_trigger and let led_trigger_register do the right thing with it.
Not sure what "the right thing" is here. Just adding the group
certainly is not. We'd somehow need a new kobject for this. @gregkh:
Maybe you can comment and point out the way to go here?
kobject_uevent_env() is called on led_cdev->dev->kobj from
led_trigger_set().
I'm not 100% confident, but I think this doesn't justify adding
attributes after registration.
It seems that the main rationale standing behind the rule "You must not
call device_create_file() for an already bound device" is lack of
automatic userspace notification in this case.
This is corroborated by the fact that this restriction does not apply
to the addition of a whole group, which entails creation of a directory,
and, in turn, a new kobject.
In case of led-trigger.c, this shortcoming is obviated by explicitly
sending KOBJ_CHANGE upon trigger addition/removal. It allows to notify
the userspace about respective changes in the state of existence of the
files related to the specific trigger.
This is how I see it, but I'm happy to see more accurate justification
of the discussed rule.
--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
