On Wed, Aug 1, 2012 at 2:28 AM, Colin Cross <[email protected]> wrote:
> On Thu, Jul 26, 2012 at 9:04 PM, Bryan Wu <[email protected]> wrote:
>> On Fri, Jul 27, 2012 at 12:51 AM, Greg KH <[email protected]> wrote:
>>> On Thu, Jul 26, 2012 at 01:03:11PM +0800, Bryan Wu wrote:
>>>> Just one quick patch for my idea: emitting a uevent in sysfs_create_file().
>>>>
>>>> --
>>>> diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
>>>> index 00012e3..04da869 100644
>>>> --- a/fs/sysfs/file.c
>>>> +++ b/fs/sysfs/file.c
>>>> @@ -570,10 +570,14 @@ int sysfs_add_file(struct sysfs_dirent *dir_sd,
>>>> const struct attribute *attr,
>>>>
>>>> int sysfs_create_file(struct kobject * kobj, const struct attribute * attr)
>>>> {
>>>> + int err = 0;
>>>> +
>>>> BUG_ON(!kobj || !kobj->sd || !attr);
>>>>
>>>> - return sysfs_add_file(kobj->sd, attr, SYSFS_KOBJ_ATTR);
>>>> + err = sysfs_add_file(kobj->sd, attr, SYSFS_KOBJ_ATTR);
>>>> + kobject_uevent(kobj, KOBJ_CHANGE);
>>>
>>> That's a veritable flood of change events when a new kobject is created,
>>> right? It also created uevents for a device that has not told userspace
>>> that it is even present, which could cause massive confusion, don't you
>>> think?
>>>
>>
>> Indeed, this is unacceptable. I reworked a new patchset and just sent
>> our for you review.
>>
>> Thanks,
>> -Bryan
>
> Given the rejection of the other solutions to this problem, and chance
> of getting this acked?
Greg, Richard and Henrique, can I take you guys' Ack here?
Thanks,
--
Bryan Wu <[email protected]>
Kernel Developer +86.186-168-78255 Mobile
Canonical Ltd. http://www.canonical.com
Ubuntu - Linux for human beings | http://www.ubuntu.com
On Tue, Aug 07, 2012 at 10:57:29AM +0800, Bryan Wu wrote:
> On Wed, Aug 1, 2012 at 2:28 AM, Colin Cross <[email protected]> wrote:
> > On Thu, Jul 26, 2012 at 9:04 PM, Bryan Wu <[email protected]> wrote:
> >> On Fri, Jul 27, 2012 at 12:51 AM, Greg KH <[email protected]> wrote:
> >>> On Thu, Jul 26, 2012 at 01:03:11PM +0800, Bryan Wu wrote:
> >>>> Just one quick patch for my idea: emitting a uevent in sysfs_create_file().
> >>>>
> >>>> --
> >>>> diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> >>>> index 00012e3..04da869 100644
> >>>> --- a/fs/sysfs/file.c
> >>>> +++ b/fs/sysfs/file.c
> >>>> @@ -570,10 +570,14 @@ int sysfs_add_file(struct sysfs_dirent *dir_sd,
> >>>> const struct attribute *attr,
> >>>>
> >>>> int sysfs_create_file(struct kobject * kobj, const struct attribute * attr)
> >>>> {
> >>>> + int err = 0;
> >>>> +
> >>>> BUG_ON(!kobj || !kobj->sd || !attr);
> >>>>
> >>>> - return sysfs_add_file(kobj->sd, attr, SYSFS_KOBJ_ATTR);
> >>>> + err = sysfs_add_file(kobj->sd, attr, SYSFS_KOBJ_ATTR);
> >>>> + kobject_uevent(kobj, KOBJ_CHANGE);
> >>>
> >>> That's a veritable flood of change events when a new kobject is created,
> >>> right? It also created uevents for a device that has not told userspace
> >>> that it is even present, which could cause massive confusion, don't you
> >>> think?
> >>>
> >>
> >> Indeed, this is unacceptable. I reworked a new patchset and just sent
> >> our for you review.
> >>
> >> Thanks,
> >> -Bryan
> >
> > Given the rejection of the other solutions to this problem, and chance
> > of getting this acked?
>
> Greg, Richard and Henrique, can I take you guys' Ack here?
Ack for what specific patch are you referring to?
greg k-h
On Tue, 07 Aug 2012, Bryan Wu wrote:
> On Wed, Aug 1, 2012 at 2:28 AM, Colin Cross <[email protected]> wrote:
> > On Thu, Jul 26, 2012 at 9:04 PM, Bryan Wu <[email protected]> wrote:
> >> On Fri, Jul 27, 2012 at 12:51 AM, Greg KH <[email protected]> wrote:
> >>> On Thu, Jul 26, 2012 at 01:03:11PM +0800, Bryan Wu wrote:
> >>>> Just one quick patch for my idea: emitting a uevent in sysfs_create_file().
> >>>>
> >>>> --
> >>>> diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> >>>> index 00012e3..04da869 100644
> >>>> --- a/fs/sysfs/file.c
> >>>> +++ b/fs/sysfs/file.c
> >>>> @@ -570,10 +570,14 @@ int sysfs_add_file(struct sysfs_dirent *dir_sd,
> >>>> const struct attribute *attr,
> >>>>
> >>>> int sysfs_create_file(struct kobject * kobj, const struct attribute * attr)
> >>>> {
> >>>> + int err = 0;
> >>>> +
> >>>> BUG_ON(!kobj || !kobj->sd || !attr);
> >>>>
> >>>> - return sysfs_add_file(kobj->sd, attr, SYSFS_KOBJ_ATTR);
> >>>> + err = sysfs_add_file(kobj->sd, attr, SYSFS_KOBJ_ATTR);
> >>>> + kobject_uevent(kobj, KOBJ_CHANGE);
> >>>
> >>> That's a veritable flood of change events when a new kobject is created,
> >>> right? It also created uevents for a device that has not told userspace
> >>> that it is even present, which could cause massive confusion, don't you
> >>> think?
> >>>
> >>
> >> Indeed, this is unacceptable. I reworked a new patchset and just sent
> >> our for you review.
> >>
> >> Thanks,
> >> -Bryan
> >
> > Given the rejection of the other solutions to this problem, and chance
> > of getting this acked?
>
> Greg, Richard and Henrique, can I take you guys' Ack here?
Yes, you have my Acked-by, provided that the uevent is NOT sent before
the led is fully registered (I cannot check right now if the patch does
this right or not. I apologise in advance if this was an unecessary
question).
I don't care whether the uevent gets sent right after registration, or
only when the trigger *changes* after registering. But someone might,
so it would be nice to document this.
Considering Greg's answer, maybe it would be best to resend the patch
with the point above clarified in the commit message or in the in-tree
documentation of the LED class?
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
On Tue, Aug 7, 2012 at 7:36 AM, Henrique de Moraes Holschuh
<[email protected]> wrote:
> On Tue, 07 Aug 2012, Bryan Wu wrote:
<snip>
>> Greg, Richard and Henrique, can I take you guys' Ack here?
>
> Yes, you have my Acked-by, provided that the uevent is NOT sent before
> the led is fully registered (I cannot check right now if the patch does
> this right or not. I apologise in advance if this was an unecessary
> question).
>
> I don't care whether the uevent gets sent right after registration, or
> only when the trigger *changes* after registering. But someone might,
> so it would be nice to document this.
>
> Considering Greg's answer, maybe it would be best to resend the patch
> with the point above clarified in the commit message or in the in-tree
> documentation of the LED class?
led_trigger_set_default is called last from led_classdev_register, so
it will send a uevent during registration but after it is fully
registered. I will resend the patch with the clarification.