* [PATCH] leds: triggers: send uevent when changing triggers @ 2012-07-25 0:32 Colin Cross 2012-07-25 6:11 ` Bryan Wu 0 siblings, 1 reply; 15+ messages in thread From: Colin Cross @ 2012-07-25 0:32 UTC (permalink / raw) To: linux-kernel Cc: Greg KH, Henrique de Moraes Holschuh, Colin Cross, Bryan Wu, Richard Purdie, linux-leds Some triggers create sysfs files when they are enabled. Send a uevent "change" notification whenever the trigger is changed to allow userspace processes such as udev to modify permissions on the new files. Signed-off-by: Colin Cross <ccross@android.com> --- drivers/leds/led-triggers.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c index 46b4c76..a85ce09 100644 --- a/drivers/leds/led-triggers.c +++ b/drivers/leds/led-triggers.c @@ -102,6 +102,12 @@ ssize_t led_trigger_show(struct device *dev, struct device_attribute *attr, void led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trigger) { unsigned long flags; + char *event = NULL; + char *envp[2]; + const char *name; + + name = trigger ? trigger->name : "none"; + event = kasprintf(GFP_KERNEL, "TRIGGER=%s", name); /* Remove any existing trigger */ if (led_cdev->trigger) { @@ -122,6 +128,13 @@ void led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trigger) if (trigger->activate) trigger->activate(led_cdev); } + + if (event) { + envp[0] = event; + envp[1] = NULL; + kobject_uevent_env(&led_cdev->dev->kobj, KOBJ_CHANGE, envp); + kfree(event); + } } EXPORT_SYMBOL_GPL(led_trigger_set); -- 1.7.7.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] leds: triggers: send uevent when changing triggers 2012-07-25 0:32 [PATCH] leds: triggers: send uevent when changing triggers Colin Cross @ 2012-07-25 6:11 ` Bryan Wu 2012-07-25 14:36 ` Greg KH 2012-07-25 18:54 ` Colin Cross 0 siblings, 2 replies; 15+ messages in thread From: Bryan Wu @ 2012-07-25 6:11 UTC (permalink / raw) To: Colin Cross Cc: linux-kernel, Greg KH, Henrique de Moraes Holschuh, Richard Purdie, linux-leds On Wed, Jul 25, 2012 at 8:32 AM, Colin Cross <ccross@android.com> wrote: > Some triggers create sysfs files when they are enabled. Send a uevent > "change" notification whenever the trigger is changed to allow userspace > processes such as udev to modify permissions on the new files. > This looks like an workaround only for led trigger, can we fix this in sysfs level? Thanks, -Bryan > Signed-off-by: Colin Cross <ccross@android.com> > --- > drivers/leds/led-triggers.c | 13 +++++++++++++ > 1 files changed, 13 insertions(+), 0 deletions(-) > > diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c > index 46b4c76..a85ce09 100644 > --- a/drivers/leds/led-triggers.c > +++ b/drivers/leds/led-triggers.c > @@ -102,6 +102,12 @@ ssize_t led_trigger_show(struct device *dev, struct device_attribute *attr, > void led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trigger) > { > unsigned long flags; > + char *event = NULL; > + char *envp[2]; > + const char *name; > + > + name = trigger ? trigger->name : "none"; > + event = kasprintf(GFP_KERNEL, "TRIGGER=%s", name); > > /* Remove any existing trigger */ > if (led_cdev->trigger) { > @@ -122,6 +128,13 @@ void led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trigger) > if (trigger->activate) > trigger->activate(led_cdev); > } > + > + if (event) { > + envp[0] = event; > + envp[1] = NULL; > + kobject_uevent_env(&led_cdev->dev->kobj, KOBJ_CHANGE, envp); > + kfree(event); > + } > } > EXPORT_SYMBOL_GPL(led_trigger_set); > > -- > 1.7.7.3 > -- Bryan Wu <bryan.wu@canonical.com> Kernel Developer +86.186-168-78255 Mobile Canonical Ltd. www.canonical.com Ubuntu - Linux for human beings | www.ubuntu.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] leds: triggers: send uevent when changing triggers 2012-07-25 6:11 ` Bryan Wu @ 2012-07-25 14:36 ` Greg KH 2012-07-25 18:54 ` Colin Cross 1 sibling, 0 replies; 15+ messages in thread From: Greg KH @ 2012-07-25 14:36 UTC (permalink / raw) To: Bryan Wu Cc: Colin Cross, linux-kernel, Henrique de Moraes Holschuh, Richard Purdie, linux-leds On Wed, Jul 25, 2012 at 02:11:30PM +0800, Bryan Wu wrote: > On Wed, Jul 25, 2012 at 8:32 AM, Colin Cross <ccross@android.com> wrote: > > Some triggers create sysfs files when they are enabled. Send a uevent > > "change" notification whenever the trigger is changed to allow userspace > > processes such as udev to modify permissions on the new files. > > > > This looks like an workaround only for led trigger, can we fix this in > sysfs level? How do you propose doing that? greg k-h ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] leds: triggers: send uevent when changing triggers 2012-07-25 6:11 ` Bryan Wu 2012-07-25 14:36 ` Greg KH @ 2012-07-25 18:54 ` Colin Cross 2012-07-26 3:29 ` Bryan Wu 1 sibling, 1 reply; 15+ messages in thread From: Colin Cross @ 2012-07-25 18:54 UTC (permalink / raw) To: Bryan Wu Cc: linux-kernel, Greg KH, Henrique de Moraes Holschuh, Richard Purdie, linux-leds On Tue, Jul 24, 2012 at 11:11 PM, Bryan Wu <bryan.wu@canonical.com> wrote: > On Wed, Jul 25, 2012 at 8:32 AM, Colin Cross <ccross@android.com> wrote: >> Some triggers create sysfs files when they are enabled. Send a uevent >> "change" notification whenever the trigger is changed to allow userspace >> processes such as udev to modify permissions on the new files. >> > > This looks like an workaround only for led trigger, can we fix this in > sysfs level? See the previous discussion here: https://lkml.org/lkml/2012/7/20/458 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] leds: triggers: send uevent when changing triggers 2012-07-25 18:54 ` Colin Cross @ 2012-07-26 3:29 ` Bryan Wu 2012-07-26 3:59 ` Greg KH 0 siblings, 1 reply; 15+ messages in thread From: Bryan Wu @ 2012-07-26 3:29 UTC (permalink / raw) To: Colin Cross Cc: linux-kernel, Greg KH, Henrique de Moraes Holschuh, Richard Purdie, linux-leds On Thu, Jul 26, 2012 at 2:54 AM, Colin Cross <ccross@android.com> wrote: > On Tue, Jul 24, 2012 at 11:11 PM, Bryan Wu <bryan.wu@canonical.com> wrote: >> On Wed, Jul 25, 2012 at 8:32 AM, Colin Cross <ccross@android.com> wrote: >>> Some triggers create sysfs files when they are enabled. Send a uevent >>> "change" notification whenever the trigger is changed to allow userspace >>> processes such as udev to modify permissions on the new files. >>> >> >> This looks like an workaround only for led trigger, can we fix this in >> sysfs level? > > See the previous discussion here: https://lkml.org/lkml/2012/7/20/458 Thanks, I went through this thread here. Actually it was archived in my email account, so I missed that during a trip. Basically, I think this issue is a kind of general issue related to sysfs, not just only for led trigger system. And adding this uevent notification to a upper level LED driver is not good to me, if we got similar issue in other subsystem, we should add similar fix there. Why not we add this in sysfs when we call device_create_file(). And this will be benefit for other drivers. Please point out me why we can't do that in sysfs level. Thanks. -- Bryan Wu <bryan.wu@canonical.com> Kernel Developer +86.186-168-78255 Mobile Canonical Ltd. www.canonical.com Ubuntu - Linux for human beings | www.ubuntu.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] leds: triggers: send uevent when changing triggers 2012-07-26 3:29 ` Bryan Wu @ 2012-07-26 3:59 ` Greg KH 2012-07-26 5:03 ` Bryan Wu 0 siblings, 1 reply; 15+ messages in thread From: Greg KH @ 2012-07-26 3:59 UTC (permalink / raw) To: Bryan Wu Cc: Colin Cross, linux-kernel, Henrique de Moraes Holschuh, Richard Purdie, linux-leds On Thu, Jul 26, 2012 at 11:29:48AM +0800, Bryan Wu wrote: > On Thu, Jul 26, 2012 at 2:54 AM, Colin Cross <ccross@android.com> wrote: > > On Tue, Jul 24, 2012 at 11:11 PM, Bryan Wu <bryan.wu@canonical.com> wrote: > >> On Wed, Jul 25, 2012 at 8:32 AM, Colin Cross <ccross@android.com> wrote: > >>> Some triggers create sysfs files when they are enabled. Send a uevent > >>> "change" notification whenever the trigger is changed to allow userspace > >>> processes such as udev to modify permissions on the new files. > >>> > >> > >> This looks like an workaround only for led trigger, can we fix this in > >> sysfs level? > > > > See the previous discussion here: https://lkml.org/lkml/2012/7/20/458 > > Thanks, I went through this thread here. Actually it was archived in > my email account, so I missed that during a trip. > > Basically, I think this issue is a kind of general issue related to > sysfs, not just only for led trigger system. And adding this uevent > notification to a upper level LED driver is not good to me, if we got > similar issue in other subsystem, we should add similar fix there. Why > not we add this in sysfs when we call device_create_file(). And this > will be benefit for other drivers. > > Please point out me why we can't do that in sysfs level. Thanks. Please point out to me how you _can_ do this at a sysfs level :) greg k-h ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] leds: triggers: send uevent when changing triggers 2012-07-26 3:59 ` Greg KH @ 2012-07-26 5:03 ` Bryan Wu 2012-07-26 16:51 ` Greg KH 0 siblings, 1 reply; 15+ messages in thread From: Bryan Wu @ 2012-07-26 5:03 UTC (permalink / raw) To: Greg KH Cc: Colin Cross, linux-kernel, Henrique de Moraes Holschuh, Richard Purdie, linux-leds On Thu, Jul 26, 2012 at 11:59 AM, Greg KH <gregkh@linuxfoundation.org> wrote: > On Thu, Jul 26, 2012 at 11:29:48AM +0800, Bryan Wu wrote: >> On Thu, Jul 26, 2012 at 2:54 AM, Colin Cross <ccross@android.com> wrote: >> > On Tue, Jul 24, 2012 at 11:11 PM, Bryan Wu <bryan.wu@canonical.com> wrote: >> >> On Wed, Jul 25, 2012 at 8:32 AM, Colin Cross <ccross@android.com> wrote: >> >>> Some triggers create sysfs files when they are enabled. Send a uevent >> >>> "change" notification whenever the trigger is changed to allow userspace >> >>> processes such as udev to modify permissions on the new files. >> >>> >> >> >> >> This looks like an workaround only for led trigger, can we fix this in >> >> sysfs level? >> > >> > See the previous discussion here: https://lkml.org/lkml/2012/7/20/458 >> >> Thanks, I went through this thread here. Actually it was archived in >> my email account, so I missed that during a trip. >> >> Basically, I think this issue is a kind of general issue related to >> sysfs, not just only for led trigger system. And adding this uevent >> notification to a upper level LED driver is not good to me, if we got >> similar issue in other subsystem, we should add similar fix there. Why >> not we add this in sysfs when we call device_create_file(). And this >> will be benefit for other drivers. >> >> Please point out me why we can't do that in sysfs level. Thanks. > > Please point out to me how you _can_ do this at a sysfs level :) > > greg k-h 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); + return err; } int sysfs_create_files(struct kobject *kobj, const struct attribute **ptr) -- -- Bryan Wu <bryan.wu@canonical.com> Kernel Developer +86.186-168-78255 Mobile Canonical Ltd. www.canonical.com Ubuntu - Linux for human beings | www.ubuntu.com ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] leds: triggers: send uevent when changing triggers 2012-07-26 5:03 ` Bryan Wu @ 2012-07-26 16:51 ` Greg KH 2012-07-27 4:04 ` Bryan Wu 0 siblings, 1 reply; 15+ messages in thread From: Greg KH @ 2012-07-26 16:51 UTC (permalink / raw) To: Bryan Wu Cc: Colin Cross, linux-kernel, Henrique de Moraes Holschuh, Richard Purdie, linux-leds 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? greg k-h ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] leds: triggers: send uevent when changing triggers 2012-07-26 16:51 ` Greg KH @ 2012-07-27 4:04 ` Bryan Wu 2012-07-31 18:28 ` Colin Cross 0 siblings, 1 reply; 15+ messages in thread From: Bryan Wu @ 2012-07-27 4:04 UTC (permalink / raw) To: Greg KH Cc: Colin Cross, linux-kernel, Henrique de Moraes Holschuh, Richard Purdie, linux-leds On Fri, Jul 27, 2012 at 12:51 AM, Greg KH <gregkh@linuxfoundation.org> 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] leds: triggers: send uevent when changing triggers 2012-07-27 4:04 ` Bryan Wu @ 2012-07-31 18:28 ` Colin Cross 2012-08-07 2:57 ` Bryan Wu 0 siblings, 1 reply; 15+ messages in thread From: Colin Cross @ 2012-07-31 18:28 UTC (permalink / raw) To: Bryan Wu Cc: Greg KH, linux-kernel, Henrique de Moraes Holschuh, Richard Purdie, linux-leds On Thu, Jul 26, 2012 at 9:04 PM, Bryan Wu <bryan.wu@canonical.com> wrote: > On Fri, Jul 27, 2012 at 12:51 AM, Greg KH <gregkh@linuxfoundation.org> 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? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] leds: triggers: send uevent when changing triggers 2012-07-31 18:28 ` Colin Cross @ 2012-08-07 2:57 ` Bryan Wu 2012-08-07 3:34 ` Greg KH 2012-08-07 14:36 ` Henrique de Moraes Holschuh 0 siblings, 2 replies; 15+ messages in thread From: Bryan Wu @ 2012-08-07 2:57 UTC (permalink / raw) To: Colin Cross Cc: Greg KH, linux-kernel, Henrique de Moraes Holschuh, Richard Purdie, linux-leds On Wed, Aug 1, 2012 at 2:28 AM, Colin Cross <ccross@android.com> wrote: > On Thu, Jul 26, 2012 at 9:04 PM, Bryan Wu <bryan.wu@canonical.com> wrote: >> On Fri, Jul 27, 2012 at 12:51 AM, Greg KH <gregkh@linuxfoundation.org> 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 <bryan.wu@canonical.com> Kernel Developer +86.186-168-78255 Mobile Canonical Ltd. www.canonical.com Ubuntu - Linux for human beings | www.ubuntu.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] leds: triggers: send uevent when changing triggers 2012-08-07 2:57 ` Bryan Wu @ 2012-08-07 3:34 ` Greg KH [not found] ` <CAK5ve-KLdwEqW6MLbusMRkaHBQkxTqOGLoVWzSmiuo2qxwtwmA@mail.gmail.com> 2012-08-07 14:36 ` Henrique de Moraes Holschuh 1 sibling, 1 reply; 15+ messages in thread From: Greg KH @ 2012-08-07 3:34 UTC (permalink / raw) To: Bryan Wu Cc: Colin Cross, linux-kernel, Henrique de Moraes Holschuh, Richard Purdie, linux-leds On Tue, Aug 07, 2012 at 10:57:29AM +0800, Bryan Wu wrote: > On Wed, Aug 1, 2012 at 2:28 AM, Colin Cross <ccross@android.com> wrote: > > On Thu, Jul 26, 2012 at 9:04 PM, Bryan Wu <bryan.wu@canonical.com> wrote: > >> On Fri, Jul 27, 2012 at 12:51 AM, Greg KH <gregkh@linuxfoundation.org> 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <CAK5ve-KLdwEqW6MLbusMRkaHBQkxTqOGLoVWzSmiuo2qxwtwmA@mail.gmail.com>]
* Re: [PATCH] leds: triggers: send uevent when changing triggers [not found] ` <CAK5ve-KLdwEqW6MLbusMRkaHBQkxTqOGLoVWzSmiuo2qxwtwmA@mail.gmail.com> @ 2012-08-07 3:43 ` Greg KH 0 siblings, 0 replies; 15+ messages in thread From: Greg KH @ 2012-08-07 3:43 UTC (permalink / raw) To: Bryan Wu Cc: Linux LED Subsystem, Henrique de Moraes Holschuh, linux-kernel, Richard Purdie, Colin Cross On Tue, Aug 07, 2012 at 11:38:10AM +0800, Bryan Wu wrote: > > 在 2012-8-7 上午11:34，"Greg KH" <gregkh@linuxfoundation.org>写道： > > > > On Tue, Aug 07, 2012 at 10:57:29AM +0800, Bryan Wu wrote: > > > On Wed, Aug 1, 2012 at 2:28 AM, Colin Cross <ccross@android.com> wrote: > > > > On Thu, Jul 26, 2012 at 9:04 PM, Bryan Wu <bryan.wu@canonical.com> wrote: > > > >> On Fri, Jul 27, 2012 at 12:51 AM, Greg KH <gregkh@linuxfoundation.org> > 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? > > > > For Colin's patch in the first email of this loop. I have no idea what that patch contained anymore, that was 1000+ emails ago... ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] leds: triggers: send uevent when changing triggers 2012-08-07 2:57 ` Bryan Wu 2012-08-07 3:34 ` Greg KH @ 2012-08-07 14:36 ` Henrique de Moraes Holschuh 2012-08-07 19:44 ` Colin Cross 1 sibling, 1 reply; 15+ messages in thread From: Henrique de Moraes Holschuh @ 2012-08-07 14:36 UTC (permalink / raw) To: Bryan Wu; +Cc: Colin Cross, Greg KH, linux-kernel, Richard Purdie, linux-leds On Tue, 07 Aug 2012, Bryan Wu wrote: > On Wed, Aug 1, 2012 at 2:28 AM, Colin Cross <ccross@android.com> wrote: > > On Thu, Jul 26, 2012 at 9:04 PM, Bryan Wu <bryan.wu@canonical.com> wrote: > >> On Fri, Jul 27, 2012 at 12:51 AM, Greg KH <gregkh@linuxfoundation.org> 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] leds: triggers: send uevent when changing triggers 2012-08-07 14:36 ` Henrique de Moraes Holschuh @ 2012-08-07 19:44 ` Colin Cross 0 siblings, 0 replies; 15+ messages in thread From: Colin Cross @ 2012-08-07 19:44 UTC (permalink / raw) To: Henrique de Moraes Holschuh Cc: Bryan Wu, Greg KH, linux-kernel, Richard Purdie, linux-leds On Tue, Aug 7, 2012 at 7:36 AM, Henrique de Moraes Holschuh <hmh@hmh.eng.br> 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2012-08-07 19:44 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-25 0:32 [PATCH] leds: triggers: send uevent when changing triggers Colin Cross
2012-07-25 6:11 ` Bryan Wu
2012-07-25 14:36 ` Greg KH
2012-07-25 18:54 ` Colin Cross
2012-07-26 3:29 ` Bryan Wu
2012-07-26 3:59 ` Greg KH
2012-07-26 5:03 ` Bryan Wu
2012-07-26 16:51 ` Greg KH
2012-07-27 4:04 ` Bryan Wu
2012-07-31 18:28 ` Colin Cross
2012-08-07 2:57 ` Bryan Wu
2012-08-07 3:34 ` Greg KH
[not found] ` <CAK5ve-KLdwEqW6MLbusMRkaHBQkxTqOGLoVWzSmiuo2qxwtwmA@mail.gmail.com>
2012-08-07 3:43 ` Greg KH
2012-08-07 14:36 ` Henrique de Moraes Holschuh
2012-08-07 19:44 ` Colin Cross
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.