* Re: [PATCH 00/13] leds: fix attribute-creation races
From: Greg Kroah-Hartman @ 2014-06-26 23:25 UTC (permalink / raw)
To: Bryan Wu
Cc: Johan Hovold, Richard Purdie, Dmitry Torokhov, linux-input, lkml,
Linux LED Subsystem, Janne Kanniainen, Jiri Kosina,
Bjørn Mork
In-Reply-To: <CAK5ve-JVBoeW+a+1PKcfCwNOC+HOdKUD=1iFrTJAZ_tqh10yuQ@mail.gmail.com>
On Wed, Jun 25, 2014 at 03:46:19PM -0700, Bryan Wu wrote:
> On Wed, Jun 25, 2014 at 10:08 AM, Johan Hovold <johan@kernel.org> wrote:
> > LED-class device attributes should be created using the attribute groups
> > of struct device, rather than be added manually after the device has
> > been registered in order to avoid racing with userspace.
> >
> > The first patch adds an attribute groups field to struct led_classdev,
> > which is used during registration to add device attributes in a
> > race-free manner.
> >
> > The remaining patches convert the current racy LED-device attribute
> > creations to use the new facility. Hopefully my grep-patterns have
> > caught most instances. Note that this also tends to simplify probe error
> > paths.
> >
> > The lp55xx-common failed to remove the attribute group it is currently
> > creating, something which is fixed separately.
> >
> > Note that this series include one driver from drivers/input/keyboard.
> >
> > The individual led-driver patches have been compile tested only.
> >
>
> Thanks a lot for driving this. I will applied this patchset into my
> -devel branch. After I got the Ack from Input guys, I will apply it to
> my for-next branch then.
Series looks good to me, Johan, thanks for doing this work.
greg k-h
^ permalink raw reply
* Re: [PATCH] HID: roccat: Drop cast
From: Jiri Kosina @ 2014-06-26 22:35 UTC (permalink / raw)
To: Himangi Saraogi; +Cc: Stefan Achatz, linux-input, linux-kernel, julia.lawall
In-Reply-To: <20140626172852.GA2878@himangi-Dell>
On Thu, 26 Jun 2014, Himangi Saraogi wrote:
> This patch removes the cast on data of type void* as it is not needed.
> The following Coccinelle semantic patch was used for making the change:
>
> @r@
> expression x;
> void* e;
> type T;
> identifier f;
> @@
>
> (
> *((T *)e)
> |
> ((T *)x)[...]
> |
> ((T *)x)->f
> |
> - (T *)
> e
> )
>
> Signed-off-by: Himangi Saraogi <himangi774@gmail.com>
> Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> ---
> drivers/hid/hid-roccat-lua.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-roccat-lua.c b/drivers/hid/hid-roccat-lua.c
> index 6adc0fa..65e2e76 100644
> --- a/drivers/hid/hid-roccat-lua.c
> +++ b/drivers/hid/hid-roccat-lua.c
> @@ -61,7 +61,7 @@ static ssize_t lua_sysfs_write(struct file *fp, struct kobject *kobj,
> return -EINVAL;
>
> mutex_lock(&lua->lua_lock);
> - retval = roccat_common2_send(usb_dev, command, (void *)buf, real_size);
> + retval = roccat_common2_send(usb_dev, command, buf, real_size);
> mutex_unlock(&lua->lua_lock);
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: roccat: Drop cast
From: Joe Perches @ 2014-06-26 17:38 UTC (permalink / raw)
To: Himangi Saraogi
Cc: Stefan Achatz, Jiri Kosina, linux-input, linux-kernel,
julia.lawall
In-Reply-To: <20140626172852.GA2878@himangi-Dell>
On Thu, 2014-06-26 at 22:58 +0530, Himangi Saraogi wrote:
> This patch removes the cast on data of type void* as it is not needed.
Hi Himangi
The cast of a const void * to a void * was odd.
Maybe a mechanism to verify appropriateness of
loss of constness for any pointer might be useful.
^ permalink raw reply
* [PATCH] HID: roccat: Drop cast
From: Himangi Saraogi @ 2014-06-26 17:28 UTC (permalink / raw)
To: Stefan Achatz, Jiri Kosina, linux-input, linux-kernel; +Cc: julia.lawall
This patch removes the cast on data of type void* as it is not needed.
The following Coccinelle semantic patch was used for making the change:
@r@
expression x;
void* e;
type T;
identifier f;
@@
(
*((T *)e)
|
((T *)x)[...]
|
((T *)x)->f
|
- (T *)
e
)
Signed-off-by: Himangi Saraogi <himangi774@gmail.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
---
drivers/hid/hid-roccat-lua.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-roccat-lua.c b/drivers/hid/hid-roccat-lua.c
index 6adc0fa..65e2e76 100644
--- a/drivers/hid/hid-roccat-lua.c
+++ b/drivers/hid/hid-roccat-lua.c
@@ -61,7 +61,7 @@ static ssize_t lua_sysfs_write(struct file *fp, struct kobject *kobj,
return -EINVAL;
mutex_lock(&lua->lua_lock);
- retval = roccat_common2_send(usb_dev, command, (void *)buf, real_size);
+ retval = roccat_common2_send(usb_dev, command, buf, real_size);
mutex_unlock(&lua->lua_lock);
return retval ? retval : real_size;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 2/2 v2] HID: leds: Use attribute-groups in MSI GT683R driver
From: Bryan Wu @ 2014-06-25 22:55 UTC (permalink / raw)
To: Jiri Kosina
Cc: Johan Hovold, Janne Kanniainen, Greg Kroah-Hartman,
Bjørn Mork, lkml, Linux LED Subsystem, linux-usb,
linux-input
In-Reply-To: <alpine.LNX.2.00.1406252107070.15939@pobox.suse.cz>
On Wed, Jun 25, 2014 at 12:09 PM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Wed, 25 Jun 2014, Johan Hovold wrote:
>
>> Did you see the attribute-race series I posted? Not sure how best to
>> handle the dependency, as those patches should probably go in through
>> the LEDs tree, while the first patch in that series (adding the groups
>> field) is a dependency for this patch.
>>
>> Jiri, how would this best be solved?
>
> I think the best course of action here is to gather Acks from the
> respective maintainers, and take the whole lot trough a single tree
> (probably the leds tree in this case) to avoid unnecessary intra-tree
> dependencies in a rather straighforward situation like this.
>
I think the better place is HID/input tree, since this patch depends
on the initial one which is not in my tree.
I'm going to merge Johan's whole patchset and this patch probably
depends Johan's work too.
-Bryan
^ permalink raw reply
* Re: [PATCH 00/13] leds: fix attribute-creation races
From: Bryan Wu @ 2014-06-25 22:46 UTC (permalink / raw)
To: Johan Hovold
Cc: Richard Purdie, Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
lkml, Linux LED Subsystem, Janne Kanniainen, Jiri Kosina,
Bjørn Mork
In-Reply-To: <1403716136-32694-1-git-send-email-johan@kernel.org>
On Wed, Jun 25, 2014 at 10:08 AM, Johan Hovold <johan@kernel.org> wrote:
> LED-class device attributes should be created using the attribute groups
> of struct device, rather than be added manually after the device has
> been registered in order to avoid racing with userspace.
>
> The first patch adds an attribute groups field to struct led_classdev,
> which is used during registration to add device attributes in a
> race-free manner.
>
> The remaining patches convert the current racy LED-device attribute
> creations to use the new facility. Hopefully my grep-patterns have
> caught most instances. Note that this also tends to simplify probe error
> paths.
>
> The lp55xx-common failed to remove the attribute group it is currently
> creating, something which is fixed separately.
>
> Note that this series include one driver from drivers/input/keyboard.
>
> The individual led-driver patches have been compile tested only.
>
Thanks a lot for driving this. I will applied this patchset into my
-devel branch. After I got the Ack from Input guys, I will apply it to
my for-next branch then.
-Bryan
> Johan
>
>
> Johan Hovold (13):
> leds: add led-class attribute-group support
> leds: lm3550: fix attribute-creation race
> leds: lm3533: fix attribute-creation race
> leds: lm355x: fix attribute-creation race
> leds: lm3642: fix attribute-creation race
> leds: max8997: fix attribute-creation race
> leds: netxbig: fix attribute-creation race
> leds: ns2: fix attribute-creation race
> leds: ss4200: fix attribute-creation race
> leds: wm831x-status: fix attribute-creation race
> input: lm8323: fix attribute-creation race
> leds: lp55xx-common: fix sysfs entry leak
> leds: lp55xx-common: fix attribute-creation race
>
> drivers/input/keyboard/lm8323.c | 22 +++++++++-------------
> drivers/leds/led-class.c | 5 +++--
> drivers/leds/leds-lm3530.c | 20 +++++++-------------
> drivers/leds/leds-lm3533.c | 20 ++++++++------------
> drivers/leds/leds-lm355x.c | 21 +++++++++------------
> drivers/leds/leds-lm3642.c | 30 ++++++++++++++----------------
> drivers/leds/leds-lp55xx-common.c | 20 +++-----------------
> drivers/leds/leds-max8997.c | 16 +++++++---------
> drivers/leds/leds-netxbig.c | 26 ++++++++++++--------------
> drivers/leds/leds-ns2.c | 16 +++++++---------
> drivers/leds/leds-ss4200.c | 14 +++++++++-----
> drivers/leds/leds-wm831x-status.c | 23 +++++++++--------------
> include/linux/leds.h | 2 ++
> 13 files changed, 99 insertions(+), 136 deletions(-)
>
> --
> 1.8.5.5
>
^ permalink raw reply
* Re: [PATCH 11/13] input: lm8323: fix attribute-creation race
From: Bryan Wu @ 2014-06-25 22:45 UTC (permalink / raw)
To: Johan Hovold
Cc: Richard Purdie, Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
lkml, Linux LED Subsystem, Janne Kanniainen, Jiri Kosina,
Bjørn Mork
In-Reply-To: <1403716136-32694-12-git-send-email-johan@kernel.org>
On Wed, Jun 25, 2014 at 10:08 AM, Johan Hovold <johan@kernel.org> wrote:
> Use the attribute groups of the led-class to create the time attribute
> during probe in order to avoid racing with userspace.
>
I'm about to merge this, but need an Ack from input subsystem maintainers.
Dmitry or Jiri, are you OK with this patch?
-Bryan
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
> drivers/input/keyboard/lm8323.c | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/input/keyboard/lm8323.c b/drivers/input/keyboard/lm8323.c
> index 0b42118cbf8f..cb32e2b506b7 100644
> --- a/drivers/input/keyboard/lm8323.c
> +++ b/drivers/input/keyboard/lm8323.c
> @@ -558,6 +558,12 @@ static ssize_t lm8323_pwm_store_time(struct device *dev,
> }
> static DEVICE_ATTR(time, 0644, lm8323_pwm_show_time, lm8323_pwm_store_time);
>
> +static struct attribute *lm8323_pwm_attrs[] = {
> + &dev_attr_time.attr,
> + NULL
> +};
> +ATTRIBUTE_GROUPS(lm8323_pwm);
> +
> static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
> const char *name)
> {
> @@ -580,16 +586,11 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
> if (name) {
> pwm->cdev.name = name;
> pwm->cdev.brightness_set = lm8323_pwm_set_brightness;
> + pwm->cdev.groups = lm8323_pwm_groups;
> if (led_classdev_register(dev, &pwm->cdev) < 0) {
> dev_err(dev, "couldn't register PWM %d\n", id);
> return -1;
> }
> - if (device_create_file(pwm->cdev.dev,
> - &dev_attr_time) < 0) {
> - dev_err(dev, "couldn't register time attribute\n");
> - led_classdev_unregister(&pwm->cdev);
> - return -1;
> - }
> pwm->enabled = true;
> }
>
> @@ -753,11 +754,8 @@ fail3:
> device_remove_file(&client->dev, &dev_attr_disable_kp);
> fail2:
> while (--pwm >= 0)
> - if (lm->pwm[pwm].enabled) {
> - device_remove_file(lm->pwm[pwm].cdev.dev,
> - &dev_attr_time);
> + if (lm->pwm[pwm].enabled)
> led_classdev_unregister(&lm->pwm[pwm].cdev);
> - }
> fail1:
> input_free_device(idev);
> kfree(lm);
> @@ -777,10 +775,8 @@ static int lm8323_remove(struct i2c_client *client)
> device_remove_file(&lm->client->dev, &dev_attr_disable_kp);
>
> for (i = 0; i < 3; i++)
> - if (lm->pwm[i].enabled) {
> - device_remove_file(lm->pwm[i].cdev.dev, &dev_attr_time);
> + if (lm->pwm[i].enabled)
> led_classdev_unregister(&lm->pwm[i].cdev);
> - }
>
> kfree(lm);
>
> --
> 1.8.5.5
>
^ permalink raw reply
* Re: [PATCH 07/13] leds: netxbig: fix attribute-creation race
From: Bryan Wu @ 2014-06-25 22:30 UTC (permalink / raw)
To: Johan Hovold
Cc: Richard Purdie, Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
lkml, Linux LED Subsystem, Janne Kanniainen, Jiri Kosina,
Bjørn Mork
In-Reply-To: <1403716136-32694-8-git-send-email-johan@kernel.org>
On Wed, Jun 25, 2014 at 10:08 AM, Johan Hovold <johan@kernel.org> wrote:
> Use the attribute groups of the led-class to create the sata attribute
> during probe in order to avoid racing with userspace.
>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
> drivers/leds/leds-netxbig.c | 26 ++++++++++++--------------
> 1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/leds/leds-netxbig.c b/drivers/leds/leds-netxbig.c
> index e97f443a6e07..8ab931b8fcd3 100644
> --- a/drivers/leds/leds-netxbig.c
> +++ b/drivers/leds/leds-netxbig.c
> @@ -293,10 +293,14 @@ static ssize_t netxbig_led_sata_show(struct device *dev,
>
> static DEVICE_ATTR(sata, 0644, netxbig_led_sata_show, netxbig_led_sata_store);
>
> +static struct attribute *netxbig_led_attrs[] = {
> + &dev_attr_sata.attr,
> + NULL
> +};
> +ATTRIBUTE_GROUPS(netxbig_led);
> +
> static void delete_netxbig_led(struct netxbig_led_data *led_dat)
> {
> - if (led_dat->mode_val[NETXBIG_LED_SATA] != NETXBIG_LED_INVALID_MODE)
> - device_remove_file(led_dat->cdev.dev, &dev_attr_sata);
> led_classdev_unregister(&led_dat->cdev);
> }
>
> @@ -327,6 +331,12 @@ create_netxbig_led(struct platform_device *pdev,
> led_dat->sata = 0;
> led_dat->cdev.brightness = LED_OFF;
> led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
> + /*
> + * If available, expose the SATA activity blink capability through
> + * a "sata" sysfs attribute.
> + */
> + if (led_dat->mode_val[NETXBIG_LED_SATA] != NETXBIG_LED_INVALID_MODE)
> + led_dat->cdev.groups = netxbig_led_groups;
> led_dat->mode_addr = template->mode_addr;
> led_dat->mode_val = template->mode_val;
> led_dat->bright_addr = template->bright_addr;
> @@ -335,18 +345,6 @@ create_netxbig_led(struct platform_device *pdev,
> led_dat->num_timer = pdata->num_timer;
>
> ret = led_classdev_register(&pdev->dev, &led_dat->cdev);
Let's just "return led_classdev_register(&pdev->dev, &led_dat->cdev);"
and remove "ret"
I will handle this trivial change.
-Bryan
> - if (ret < 0)
> - return ret;
> -
> - /*
> - * If available, expose the SATA activity blink capability through
> - * a "sata" sysfs attribute.
> - */
> - if (led_dat->mode_val[NETXBIG_LED_SATA] != NETXBIG_LED_INVALID_MODE) {
> - ret = device_create_file(led_dat->cdev.dev, &dev_attr_sata);
> - if (ret)
> - led_classdev_unregister(&led_dat->cdev);
> - }
>
> return ret;
> }
> --
> 1.8.5.5
>
^ permalink raw reply
* Re: [PATCH 0/4] HID: sony: Some cleanups
From: Jiri Kosina @ 2014-06-25 19:23 UTC (permalink / raw)
To: Antonio Ospite; +Cc: linux-input, simon, Frank Praznik
In-Reply-To: <1403609323-27283-1-git-send-email-ao2@ao2.it>
On Tue, 24 Jun 2014, Antonio Ospite wrote:
> here are some cleanups for hid-sony.
Good stuff. I have queued it for 3.17. Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 2/2 v2] HID: leds: Use attribute-groups in MSI GT683R driver
From: Jiri Kosina @ 2014-06-25 19:09 UTC (permalink / raw)
To: Johan Hovold
Cc: Janne Kanniainen, greg, bjorn, cooloney, linux-kernel, linux-leds,
linux-usb, linux-input
In-Reply-To: <20140625174109.GE1409@localhost>
On Wed, 25 Jun 2014, Johan Hovold wrote:
> Did you see the attribute-race series I posted? Not sure how best to
> handle the dependency, as those patches should probably go in through
> the LEDs tree, while the first patch in that series (adding the groups
> field) is a dependency for this patch.
>
> Jiri, how would this best be solved?
I think the best course of action here is to gather Acks from the
respective maintainers, and take the whole lot trough a single tree
(probably the leds tree in this case) to avoid unnecessary intra-tree
dependencies in a rather straighforward situation like this.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH 2/2 v3] HID: leds: move led_mode attribute to led-class devices in MSI GT683R driver
From: Janne Kanniainen @ 2014-06-25 18:13 UTC (permalink / raw)
To: johan, jkosina
Cc: greg, bjorn, cooloney, linux-kernel, linux-leds, linux-usb,
linux-input, Janne Kanniainen
In-Reply-To: <20140625174109.GE1409@localhost>
Move led_mode attribute from HID device to led-class devices and rename it
msi_mode. This will also fix race condition by using attribute-groups.
Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
---
Changes in v3:
- Style fixes
- Rename sysfs-class-hid-driver-gt683r to sysfs-class-leds-driver-gt683r
.../ABI/testing/sysfs-class-hid-driver-gt683r | 14 -----------
.../ABI/testing/sysfs-class-leds-driver-gt683r | 16 +++++++++++++
drivers/hid/hid-gt683r.c | 28 ++++++++++++----------
3 files changed, 32 insertions(+), 26 deletions(-)
delete mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
create mode 100644 Documentation/ABI/testing/sysfs-class-leds-driver-gt683r
diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
deleted file mode 100644
index 317e9d5..0000000
--- a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
+++ /dev/null
@@ -1,14 +0,0 @@
-What: /sys/class/hidraw/<hidraw>/device/leds_mode
-Date: Jun 2014
-KernelVersion: 3.17
-Contact: Janne Kanniainen <janne.kanniainen@gmail.com>
-Description:
- Set the mode of LEDs
-
- 0 - normal
- 1 - audio
- 2 - breathing
-
- Normal: LEDs are fully on when enabled
- Audio: LEDs brightness depends on sound level
- Breathing: LEDs brightness varies at human breathing rate
\ No newline at end of file
diff --git a/Documentation/ABI/testing/sysfs-class-leds-driver-gt683r b/Documentation/ABI/testing/sysfs-class-leds-driver-gt683r
new file mode 100644
index 0000000..29769fb
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-leds-driver-gt683r
@@ -0,0 +1,16 @@
+What: /sys/class/leds/<led>/msi_mode
+Date: Jun 2014
+KernelVersion: 3.17
+Contact: Janne Kanniainen <janne.kanniainen@gmail.com>
+Description:
+ Set the mode of LEDs. You should notice that changing the mode
+ of one LED will update the mode of its two sibling devices as
+ well.
+
+ 0 - normal
+ 1 - audio
+ 2 - breathing
+
+ Normal: LEDs are fully on when enabled
+ Audio: LEDs brightness depends on sound level
+ Breathing: LEDs brightness varies at human breathing rate
\ No newline at end of file
diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
index 073bd80..ba27ee1 100644
--- a/drivers/hid/hid-gt683r.c
+++ b/drivers/hid/hid-gt683r.c
@@ -84,12 +84,13 @@ static void gt683r_brightness_set(struct led_classdev *led_cdev,
}
}
-static ssize_t leds_mode_show(struct device *dev,
+static ssize_t msi_mode_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
u8 sysfs_mode;
- struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+ struct hid_device *hdev = container_of(dev->parent,
+ struct hid_device, dev);
struct gt683r_led *led = hid_get_drvdata(hdev);
if (led->mode == GT683R_LED_NORMAL)
@@ -102,12 +103,13 @@ static ssize_t leds_mode_show(struct device *dev,
return scnprintf(buf, PAGE_SIZE, "%u\n", sysfs_mode);
}
-static ssize_t leds_mode_store(struct device *dev,
+static ssize_t msi_mode_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
u8 sysfs_mode;
- struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+ struct hid_device *hdev = container_of(dev->parent,
+ struct hid_device, dev);
struct gt683r_led *led = hid_get_drvdata(hdev);
@@ -212,7 +214,14 @@ fail:
mutex_unlock(&led->lock);
}
-static DEVICE_ATTR_RW(leds_mode);
+static DEVICE_ATTR_RW(msi_mode);
+
+static struct attribute *gt683r_led_attrs[] = {
+ &dev_attr_msi_mode.attr,
+ NULL
+};
+
+ATTRIBUTE_GROUPS(gt683r_led);
static int gt683r_led_probe(struct hid_device *hdev,
const struct hid_device_id *id)
@@ -261,6 +270,8 @@ static int gt683r_led_probe(struct hid_device *hdev,
led->led_devs[i].name = name;
led->led_devs[i].max_brightness = 1;
led->led_devs[i].brightness_set = gt683r_brightness_set;
+ led->led_devs[i].groups = gt683r_led_groups;
+
ret = led_classdev_register(&hdev->dev, &led->led_devs[i]);
if (ret) {
hid_err(hdev, "could not register led device\n");
@@ -268,12 +279,6 @@ static int gt683r_led_probe(struct hid_device *hdev,
}
}
- ret = device_create_file(&led->hdev->dev, &dev_attr_leds_mode);
- if (ret) {
- hid_err(hdev, "could not make mode attribute file\n");
- goto fail;
- }
-
return 0;
fail:
@@ -288,7 +293,6 @@ static void gt683r_led_remove(struct hid_device *hdev)
int i;
struct gt683r_led *led = hid_get_drvdata(hdev);
- device_remove_file(&hdev->dev, &dev_attr_leds_mode);
for (i = 0; i < GT683R_LED_COUNT; i++)
led_classdev_unregister(&led->led_devs[i]);
flush_work(&led->work);
--
1.9.2
^ permalink raw reply related
* Re: [PATCH 2/2 v2] HID: leds: Use attribute-groups in MSI GT683R driver
From: Johan Hovold @ 2014-06-25 17:41 UTC (permalink / raw)
To: Janne Kanniainen, Jiri Kosina
Cc: johan, greg, bjorn, cooloney, linux-kernel, linux-leds, linux-usb,
linux-input
In-Reply-To: <1403711966-13003-1-git-send-email-janne.kanniainen@gmail.com>
On Wed, Jun 25, 2014 at 06:59:26PM +0300, Janne Kanniainen wrote:
> Use attribute-groups to fix race condition.
The primary thing you're doing here is moving and renaming the attribute
to the led class devices, so you need to mention that (and update the
patch subject). But you can mention the race as well.
> Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
> ---
> .../ABI/testing/sysfs-class-hid-driver-gt683r | 6 +++--
> drivers/hid/hid-gt683r.c | 31 +++++++++++++---------
> 2 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> index 317e9d5..c97970a 100644
> --- a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
> +++ b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
I think you need to rename this file as well to
sysfs-class-leds-driver-gt683r (use git mv).
> @@ -1,9 +1,11 @@
> -What: /sys/class/hidraw/<hidraw>/device/leds_mode
> +What: /sys/class/leds/<led>/msi_mode
> Date: Jun 2014
> KernelVersion: 3.17
> Contact: Janne Kanniainen <janne.kanniainen@gmail.com>
> Description:
> - Set the mode of LEDs
> + Set the mode of LEDs. You should notice that changing the mode
> + of one LED will update the mode of its two sibling devices as
> + well
Missing period ('.').
>
> 0 - normal
> 1 - audio
> diff --git a/drivers/hid/hid-gt683r.c b/drivers/hid/hid-gt683r.c
> index 073bd80..aba6636 100644
> --- a/drivers/hid/hid-gt683r.c
> +++ b/drivers/hid/hid-gt683r.c
> @@ -84,12 +84,13 @@ static void gt683r_brightness_set(struct led_classdev *led_cdev,
> }
> }
>
> -static ssize_t leds_mode_show(struct device *dev,
> +static ssize_t msi_mode_show(struct device *led_dev,
No need to rename dev.
> struct device_attribute *attr,
> char *buf)
> {
> u8 sysfs_mode;
> - struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> + struct hid_device *hdev = container_of(led_dev->parent,
> + struct hid_device, dev);
> struct gt683r_led *led = hid_get_drvdata(hdev);
>
> if (led->mode == GT683R_LED_NORMAL)
> @@ -102,15 +103,15 @@ static ssize_t leds_mode_show(struct device *dev,
> return scnprintf(buf, PAGE_SIZE, "%u\n", sysfs_mode);
> }
>
> -static ssize_t leds_mode_store(struct device *dev,
> +static ssize_t msi_mode_store(struct device *led_dev,
Same here.
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> u8 sysfs_mode;
> - struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> + struct hid_device *hdev = container_of(led_dev->parent,
> + struct hid_device, dev);
> struct gt683r_led *led = hid_get_drvdata(hdev);
>
> -
You should not include random whitespace fixes in your patches (there
are two more below). If needed you can do that in a separate patch.
> if (kstrtou8(buf, 10, &sysfs_mode) || sysfs_mode > 2)
> return -EINVAL;
>
> @@ -212,7 +213,14 @@ fail:
> mutex_unlock(&led->lock);
> }
>
> -static DEVICE_ATTR_RW(leds_mode);
> +static DEVICE_ATTR_RW(msi_mode);
> +
> +static struct attribute *gt683r_led_attrs[] = {
> + &dev_attr_msi_mode.attr,
> + NULL
> +};
> +
> +ATTRIBUTE_GROUPS(gt683r_led);
>
> static int gt683r_led_probe(struct hid_device *hdev,
> const struct hid_device_id *id)
> @@ -261,6 +269,8 @@ static int gt683r_led_probe(struct hid_device *hdev,
> led->led_devs[i].name = name;
> led->led_devs[i].max_brightness = 1;
> led->led_devs[i].brightness_set = gt683r_brightness_set;
> + led->led_devs[i].groups = gt683r_led_groups;
> +
Great. This fixes the race with userspace.
Did you see the attribute-race series I posted? Not sure how best to
handle the dependency, as those patches should probably go in through
the LEDs tree, while the first patch in that series (adding the groups
field) is a dependency for this patch.
Jiri, how would this best be solved?
Thanks,
Johan
> ret = led_classdev_register(&hdev->dev, &led->led_devs[i]);
> if (ret) {
> hid_err(hdev, "could not register led device\n");
> @@ -268,17 +278,12 @@ static int gt683r_led_probe(struct hid_device *hdev,
> }
> }
>
> - ret = device_create_file(&led->hdev->dev, &dev_attr_leds_mode);
> - if (ret) {
> - hid_err(hdev, "could not make mode attribute file\n");
> - goto fail;
> - }
> -
> return 0;
>
> fail:
> for (i = i - 1; i >= 0; i--)
> led_classdev_unregister(&led->led_devs[i]);
> +
> hid_hw_stop(hdev);
> return ret;
> }
> @@ -288,9 +293,9 @@ static void gt683r_led_remove(struct hid_device *hdev)
> int i;
> struct gt683r_led *led = hid_get_drvdata(hdev);
>
> - device_remove_file(&hdev->dev, &dev_attr_leds_mode);
> for (i = 0; i < GT683R_LED_COUNT; i++)
> led_classdev_unregister(&led->led_devs[i]);
> +
> flush_work(&led->work);
> hid_hw_stop(hdev);
> }
^ permalink raw reply
* [PATCH 13/13] leds: lp55xx-common: fix attribute-creation race
From: Johan Hovold @ 2014-06-25 17:08 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie
Cc: Dmitry Torokhov, Greg Kroah-Hartman, linux-input, linux-kernel,
linux-leds, Janne Kanniainen, Jiri Kosina, Bjørn Mork,
Johan Hovold
In-Reply-To: <1403716136-32694-1-git-send-email-johan@kernel.org>
Use the attribute groups of the led-class to create the LED attributes
during probe in order to avoid racing with userspace.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/leds/leds-lp55xx-common.c | 22 +++-------------------
1 file changed, 3 insertions(+), 19 deletions(-)
diff --git a/drivers/leds/leds-lp55xx-common.c b/drivers/leds/leds-lp55xx-common.c
index 3fbfb31602c7..77c26bc32eed 100644
--- a/drivers/leds/leds-lp55xx-common.c
+++ b/drivers/leds/leds-lp55xx-common.c
@@ -127,15 +127,12 @@ static DEVICE_ATTR(led_current, S_IRUGO | S_IWUSR, lp55xx_show_current,
lp55xx_store_current);
static DEVICE_ATTR(max_current, S_IRUGO , lp55xx_show_max_current, NULL);
-static struct attribute *lp55xx_led_attributes[] = {
+static struct attribute *lp55xx_led_attrs[] = {
&dev_attr_led_current.attr,
&dev_attr_max_current.attr,
NULL,
};
-
-static struct attribute_group lp55xx_led_attr_group = {
- .attrs = lp55xx_led_attributes
-};
+ATTRIBUTE_GROUPS(lp55xx_led);
static void lp55xx_set_brightness(struct led_classdev *cdev,
enum led_brightness brightness)
@@ -176,6 +173,7 @@ static int lp55xx_init_led(struct lp55xx_led *led,
}
led->cdev.brightness_set = lp55xx_set_brightness;
+ led->cdev.groups = lp55xx_led_groups;
if (pdata->led_config[chan].name) {
led->cdev.name = pdata->led_config[chan].name;
@@ -185,24 +183,12 @@ static int lp55xx_init_led(struct lp55xx_led *led,
led->cdev.name = name;
}
- /*
- * register led class device for each channel and
- * add device attributes
- */
-
ret = led_classdev_register(dev, &led->cdev);
if (ret) {
dev_err(dev, "led register err: %d\n", ret);
return ret;
}
- ret = sysfs_create_group(&led->cdev.dev->kobj, &lp55xx_led_attr_group);
- if (ret) {
- dev_err(dev, "led sysfs err: %d\n", ret);
- led_classdev_unregister(&led->cdev);
- return ret;
- }
-
return 0;
}
@@ -520,8 +506,6 @@ void lp55xx_unregister_leds(struct lp55xx_led *led, struct lp55xx_chip *chip)
for (i = 0; i < chip->num_leds; i++) {
each = led + i;
- sysfs_remove_group(&each->cdev.dev->kobj,
- &lp55xx_led_attr_group);
led_classdev_unregister(&each->cdev);
flush_work(&each->brightness_work);
}
--
1.8.5.5
^ permalink raw reply related
* [PATCH 12/13] leds: lp55xx-common: fix sysfs entry leak
From: Johan Hovold @ 2014-06-25 17:08 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie
Cc: Dmitry Torokhov, Greg Kroah-Hartman, linux-input, linux-kernel,
linux-leds, Janne Kanniainen, Jiri Kosina, Bjørn Mork,
Johan Hovold
In-Reply-To: <1403716136-32694-1-git-send-email-johan@kernel.org>
Make sure the sysfs group is removed when the LEDs are unregistered.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/leds/leds-lp55xx-common.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/leds/leds-lp55xx-common.c b/drivers/leds/leds-lp55xx-common.c
index 88317b4f7bf3..3fbfb31602c7 100644
--- a/drivers/leds/leds-lp55xx-common.c
+++ b/drivers/leds/leds-lp55xx-common.c
@@ -520,6 +520,8 @@ void lp55xx_unregister_leds(struct lp55xx_led *led, struct lp55xx_chip *chip)
for (i = 0; i < chip->num_leds; i++) {
each = led + i;
+ sysfs_remove_group(&each->cdev.dev->kobj,
+ &lp55xx_led_attr_group);
led_classdev_unregister(&each->cdev);
flush_work(&each->brightness_work);
}
--
1.8.5.5
^ permalink raw reply related
* [PATCH 11/13] input: lm8323: fix attribute-creation race
From: Johan Hovold @ 2014-06-25 17:08 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie
Cc: Dmitry Torokhov, Greg Kroah-Hartman, linux-input, linux-kernel,
linux-leds, Janne Kanniainen, Jiri Kosina, Bjørn Mork,
Johan Hovold
In-Reply-To: <1403716136-32694-1-git-send-email-johan@kernel.org>
Use the attribute groups of the led-class to create the time attribute
during probe in order to avoid racing with userspace.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/input/keyboard/lm8323.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/input/keyboard/lm8323.c b/drivers/input/keyboard/lm8323.c
index 0b42118cbf8f..cb32e2b506b7 100644
--- a/drivers/input/keyboard/lm8323.c
+++ b/drivers/input/keyboard/lm8323.c
@@ -558,6 +558,12 @@ static ssize_t lm8323_pwm_store_time(struct device *dev,
}
static DEVICE_ATTR(time, 0644, lm8323_pwm_show_time, lm8323_pwm_store_time);
+static struct attribute *lm8323_pwm_attrs[] = {
+ &dev_attr_time.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(lm8323_pwm);
+
static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
const char *name)
{
@@ -580,16 +586,11 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
if (name) {
pwm->cdev.name = name;
pwm->cdev.brightness_set = lm8323_pwm_set_brightness;
+ pwm->cdev.groups = lm8323_pwm_groups;
if (led_classdev_register(dev, &pwm->cdev) < 0) {
dev_err(dev, "couldn't register PWM %d\n", id);
return -1;
}
- if (device_create_file(pwm->cdev.dev,
- &dev_attr_time) < 0) {
- dev_err(dev, "couldn't register time attribute\n");
- led_classdev_unregister(&pwm->cdev);
- return -1;
- }
pwm->enabled = true;
}
@@ -753,11 +754,8 @@ fail3:
device_remove_file(&client->dev, &dev_attr_disable_kp);
fail2:
while (--pwm >= 0)
- if (lm->pwm[pwm].enabled) {
- device_remove_file(lm->pwm[pwm].cdev.dev,
- &dev_attr_time);
+ if (lm->pwm[pwm].enabled)
led_classdev_unregister(&lm->pwm[pwm].cdev);
- }
fail1:
input_free_device(idev);
kfree(lm);
@@ -777,10 +775,8 @@ static int lm8323_remove(struct i2c_client *client)
device_remove_file(&lm->client->dev, &dev_attr_disable_kp);
for (i = 0; i < 3; i++)
- if (lm->pwm[i].enabled) {
- device_remove_file(lm->pwm[i].cdev.dev, &dev_attr_time);
+ if (lm->pwm[i].enabled)
led_classdev_unregister(&lm->pwm[i].cdev);
- }
kfree(lm);
--
1.8.5.5
^ permalink raw reply related
* [PATCH 10/13] leds: wm831x-status: fix attribute-creation race
From: Johan Hovold @ 2014-06-25 17:08 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie
Cc: Dmitry Torokhov, Greg Kroah-Hartman, linux-input, linux-kernel,
linux-leds, Janne Kanniainen, Jiri Kosina, Bjørn Mork,
Johan Hovold
In-Reply-To: <1403716136-32694-1-git-send-email-johan@kernel.org>
Use the attribute groups of the led-class to create the src attribute
during probe in order to avoid racing with userspace.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/leds/leds-wm831x-status.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/leds/leds-wm831x-status.c b/drivers/leds/leds-wm831x-status.c
index e72c974142d0..1b71e0701002 100644
--- a/drivers/leds/leds-wm831x-status.c
+++ b/drivers/leds/leds-wm831x-status.c
@@ -219,6 +219,12 @@ static ssize_t wm831x_status_src_store(struct device *dev,
static DEVICE_ATTR(src, 0644, wm831x_status_src_show, wm831x_status_src_store);
+static struct attribute *wm831x_status_attrs[] = {
+ &dev_attr_src.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(wm831x_status);
+
static int wm831x_status_probe(struct platform_device *pdev)
{
struct wm831x *wm831x = dev_get_drvdata(pdev->dev.parent);
@@ -232,8 +238,7 @@ static int wm831x_status_probe(struct platform_device *pdev)
res = platform_get_resource(pdev, IORESOURCE_REG, 0);
if (res == NULL) {
dev_err(&pdev->dev, "No register resource\n");
- ret = -EINVAL;
- goto err;
+ return -EINVAL;
}
drvdata = devm_kzalloc(&pdev->dev, sizeof(struct wm831x_status),
@@ -284,31 +289,21 @@ static int wm831x_status_probe(struct platform_device *pdev)
drvdata->cdev.default_trigger = pdata.default_trigger;
drvdata->cdev.brightness_set = wm831x_status_set;
drvdata->cdev.blink_set = wm831x_status_blink_set;
+ drvdata->cdev.groups = wm831x_status_groups;
ret = led_classdev_register(wm831x->dev, &drvdata->cdev);
if (ret < 0) {
dev_err(&pdev->dev, "Failed to register LED: %d\n", ret);
- goto err_led;
+ return ret;
}
- ret = device_create_file(drvdata->cdev.dev, &dev_attr_src);
- if (ret != 0)
- dev_err(&pdev->dev,
- "No source control for LED: %d\n", ret);
-
return 0;
-
-err_led:
- led_classdev_unregister(&drvdata->cdev);
-err:
- return ret;
}
static int wm831x_status_remove(struct platform_device *pdev)
{
struct wm831x_status *drvdata = platform_get_drvdata(pdev);
- device_remove_file(drvdata->cdev.dev, &dev_attr_src);
led_classdev_unregister(&drvdata->cdev);
return 0;
--
1.8.5.5
^ permalink raw reply related
* [PATCH 09/13] leds: ss4200: fix attribute-creation race
From: Johan Hovold @ 2014-06-25 17:08 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie
Cc: Dmitry Torokhov, Greg Kroah-Hartman, linux-input, linux-kernel,
linux-leds, Janne Kanniainen, Jiri Kosina, Bjørn Mork,
Johan Hovold
In-Reply-To: <1403716136-32694-1-git-send-email-johan@kernel.org>
Use the attribute groups of the led-class to create the blink attribute
during probe in order to avoid racing with userspace.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/leds/leds-ss4200.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/leds/leds-ss4200.c b/drivers/leds/leds-ss4200.c
index 2eb3ef62962b..046cb7008745 100644
--- a/drivers/leds/leds-ss4200.c
+++ b/drivers/leds/leds-ss4200.c
@@ -469,6 +469,12 @@ static ssize_t nas_led_blink_store(struct device *dev,
static DEVICE_ATTR(blink, 0644, nas_led_blink_show, nas_led_blink_store);
+static struct attribute *nasgpio_led_attrs[] = {
+ &dev_attr_blink.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(nasgpio_led);
+
static int register_nasgpio_led(int led_nr)
{
int ret;
@@ -481,20 +487,18 @@ static int register_nasgpio_led(int led_nr)
led->brightness = LED_FULL;
led->brightness_set = nasgpio_led_set_brightness;
led->blink_set = nasgpio_led_set_blink;
+ led->groups = nasgpio_led_groups;
ret = led_classdev_register(&nas_gpio_pci_dev->dev, led);
if (ret)
return ret;
- ret = device_create_file(led->dev, &dev_attr_blink);
- if (ret)
- led_classdev_unregister(led);
- return ret;
+
+ return 0;
}
static void unregister_nasgpio_led(int led_nr)
{
struct led_classdev *led = get_classdev_for_led_nr(led_nr);
led_classdev_unregister(led);
- device_remove_file(led->dev, &dev_attr_blink);
}
/*
* module load/initialization
--
1.8.5.5
^ permalink raw reply related
* [PATCH 08/13] leds: ns2: fix attribute-creation race
From: Johan Hovold @ 2014-06-25 17:08 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie
Cc: Dmitry Torokhov, Greg Kroah-Hartman, linux-input, linux-kernel,
linux-leds, Janne Kanniainen, Jiri Kosina, Bjørn Mork,
Johan Hovold
In-Reply-To: <1403716136-32694-1-git-send-email-johan@kernel.org>
Use the attribute groups of the led-class to create the sata attribute
during probe in order to avoid racing with userspace.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/leds/leds-ns2.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/leds/leds-ns2.c b/drivers/leds/leds-ns2.c
index efa625883c83..231993d1fe21 100644
--- a/drivers/leds/leds-ns2.c
+++ b/drivers/leds/leds-ns2.c
@@ -185,6 +185,12 @@ static ssize_t ns2_led_sata_show(struct device *dev,
static DEVICE_ATTR(sata, 0644, ns2_led_sata_show, ns2_led_sata_store);
+static struct attribute *ns2_led_attrs[] = {
+ &dev_attr_sata.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(ns2_led);
+
static int
create_ns2_led(struct platform_device *pdev, struct ns2_led_data *led_dat,
const struct ns2_led *template)
@@ -219,6 +225,7 @@ create_ns2_led(struct platform_device *pdev, struct ns2_led_data *led_dat,
led_dat->cdev.blink_set = NULL;
led_dat->cdev.brightness_set = ns2_led_set;
led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
+ led_dat->cdev.groups = ns2_led_groups;
led_dat->cmd = template->cmd;
led_dat->slow = template->slow;
@@ -235,20 +242,11 @@ create_ns2_led(struct platform_device *pdev, struct ns2_led_data *led_dat,
if (ret < 0)
return ret;
- ret = device_create_file(led_dat->cdev.dev, &dev_attr_sata);
- if (ret < 0)
- goto err_free_cdev;
-
return 0;
-
-err_free_cdev:
- led_classdev_unregister(&led_dat->cdev);
- return ret;
}
static void delete_ns2_led(struct ns2_led_data *led_dat)
{
- device_remove_file(led_dat->cdev.dev, &dev_attr_sata);
led_classdev_unregister(&led_dat->cdev);
}
--
1.8.5.5
^ permalink raw reply related
* [PATCH 07/13] leds: netxbig: fix attribute-creation race
From: Johan Hovold @ 2014-06-25 17:08 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie
Cc: Dmitry Torokhov, Greg Kroah-Hartman, linux-input, linux-kernel,
linux-leds, Janne Kanniainen, Jiri Kosina, Bjørn Mork,
Johan Hovold
In-Reply-To: <1403716136-32694-1-git-send-email-johan@kernel.org>
Use the attribute groups of the led-class to create the sata attribute
during probe in order to avoid racing with userspace.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/leds/leds-netxbig.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/leds/leds-netxbig.c b/drivers/leds/leds-netxbig.c
index e97f443a6e07..8ab931b8fcd3 100644
--- a/drivers/leds/leds-netxbig.c
+++ b/drivers/leds/leds-netxbig.c
@@ -293,10 +293,14 @@ static ssize_t netxbig_led_sata_show(struct device *dev,
static DEVICE_ATTR(sata, 0644, netxbig_led_sata_show, netxbig_led_sata_store);
+static struct attribute *netxbig_led_attrs[] = {
+ &dev_attr_sata.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(netxbig_led);
+
static void delete_netxbig_led(struct netxbig_led_data *led_dat)
{
- if (led_dat->mode_val[NETXBIG_LED_SATA] != NETXBIG_LED_INVALID_MODE)
- device_remove_file(led_dat->cdev.dev, &dev_attr_sata);
led_classdev_unregister(&led_dat->cdev);
}
@@ -327,6 +331,12 @@ create_netxbig_led(struct platform_device *pdev,
led_dat->sata = 0;
led_dat->cdev.brightness = LED_OFF;
led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
+ /*
+ * If available, expose the SATA activity blink capability through
+ * a "sata" sysfs attribute.
+ */
+ if (led_dat->mode_val[NETXBIG_LED_SATA] != NETXBIG_LED_INVALID_MODE)
+ led_dat->cdev.groups = netxbig_led_groups;
led_dat->mode_addr = template->mode_addr;
led_dat->mode_val = template->mode_val;
led_dat->bright_addr = template->bright_addr;
@@ -335,18 +345,6 @@ create_netxbig_led(struct platform_device *pdev,
led_dat->num_timer = pdata->num_timer;
ret = led_classdev_register(&pdev->dev, &led_dat->cdev);
- if (ret < 0)
- return ret;
-
- /*
- * If available, expose the SATA activity blink capability through
- * a "sata" sysfs attribute.
- */
- if (led_dat->mode_val[NETXBIG_LED_SATA] != NETXBIG_LED_INVALID_MODE) {
- ret = device_create_file(led_dat->cdev.dev, &dev_attr_sata);
- if (ret)
- led_classdev_unregister(&led_dat->cdev);
- }
return ret;
}
--
1.8.5.5
^ permalink raw reply related
* [PATCH 06/13] leds: max8997: fix attribute-creation race
From: Johan Hovold @ 2014-06-25 17:08 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie
Cc: Dmitry Torokhov, Greg Kroah-Hartman, linux-input, linux-kernel,
linux-leds, Janne Kanniainen, Jiri Kosina, Bjørn Mork,
Johan Hovold
In-Reply-To: <1403716136-32694-1-git-send-email-johan@kernel.org>
Use the attribute groups of the led-class to create the mode attribute
during probe in order to avoid racing with userspace.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/leds/leds-max8997.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/leds/leds-max8997.c b/drivers/leds/leds-max8997.c
index f449a8bdddc7..607bc2755aba 100644
--- a/drivers/leds/leds-max8997.c
+++ b/drivers/leds/leds-max8997.c
@@ -229,6 +229,12 @@ static ssize_t max8997_led_store_mode(struct device *dev,
static DEVICE_ATTR(mode, 0644, max8997_led_show_mode, max8997_led_store_mode);
+static struct attribute *max8997_attrs[] = {
+ &dev_attr_mode.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(max8997);
+
static int max8997_led_probe(struct platform_device *pdev)
{
struct max8997_dev *iodev = dev_get_drvdata(pdev->dev.parent);
@@ -253,6 +259,7 @@ static int max8997_led_probe(struct platform_device *pdev)
led->cdev.brightness_set = max8997_led_brightness_set;
led->cdev.flags |= LED_CORE_SUSPENDRESUME;
led->cdev.brightness = 0;
+ led->cdev.groups = max8997_groups;
led->iodev = iodev;
/* initialize mode and brightness according to platform_data */
@@ -281,14 +288,6 @@ static int max8997_led_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
- ret = device_create_file(led->cdev.dev, &dev_attr_mode);
- if (ret != 0) {
- dev_err(&pdev->dev,
- "failed to create file: %d\n", ret);
- led_classdev_unregister(&led->cdev);
- return ret;
- }
-
return 0;
}
@@ -296,7 +295,6 @@ static int max8997_led_remove(struct platform_device *pdev)
{
struct max8997_led *led = platform_get_drvdata(pdev);
- device_remove_file(led->cdev.dev, &dev_attr_mode);
led_classdev_unregister(&led->cdev);
return 0;
--
1.8.5.5
^ permalink raw reply related
* [PATCH 05/13] leds: lm3642: fix attribute-creation race
From: Johan Hovold @ 2014-06-25 17:08 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie
Cc: Dmitry Torokhov, Greg Kroah-Hartman, linux-input, linux-kernel,
linux-leds, Janne Kanniainen, Jiri Kosina, Bjørn Mork,
Johan Hovold
In-Reply-To: <1403716136-32694-1-git-send-email-johan@kernel.org>
Use the attribute groups of the led-class to create the flash and
torch-LED attributes during probe in order to avoid racing with
userspace.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/leds/leds-lm3642.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/leds/leds-lm3642.c b/drivers/leds/leds-lm3642.c
index ceb6b3cde6fe..d3dec0132769 100644
--- a/drivers/leds/leds-lm3642.c
+++ b/drivers/leds/leds-lm3642.c
@@ -313,6 +313,18 @@ static const struct regmap_config lm3642_regmap = {
.max_register = REG_MAX,
};
+static struct attribute *lm3642_flash_attrs[] = {
+ &dev_attr_strobe_pin.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(lm3642_flash);
+
+static struct attribute *lm3642_torch_attrs[] = {
+ &dev_attr_torch_pin.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(lm3642_torch);
+
static int lm3642_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -364,17 +376,13 @@ static int lm3642_probe(struct i2c_client *client,
chip->cdev_flash.max_brightness = 16;
chip->cdev_flash.brightness_set = lm3642_strobe_brightness_set;
chip->cdev_flash.default_trigger = "flash";
+ chip->cdev_flash.groups = lm3642_flash_groups,
err = led_classdev_register((struct device *)
&client->dev, &chip->cdev_flash);
if (err < 0) {
dev_err(chip->dev, "failed to register flash\n");
goto err_out;
}
- err = device_create_file(chip->cdev_flash.dev, &dev_attr_strobe_pin);
- if (err < 0) {
- dev_err(chip->dev, "failed to create strobe-pin file\n");
- goto err_create_flash_pin_file;
- }
/* torch */
INIT_WORK(&chip->work_torch, lm3642_deferred_torch_brightness_set);
@@ -382,17 +390,13 @@ static int lm3642_probe(struct i2c_client *client,
chip->cdev_torch.max_brightness = 8;
chip->cdev_torch.brightness_set = lm3642_torch_brightness_set;
chip->cdev_torch.default_trigger = "torch";
+ chip->cdev_torch.groups = lm3642_torch_groups,
err = led_classdev_register((struct device *)
&client->dev, &chip->cdev_torch);
if (err < 0) {
dev_err(chip->dev, "failed to register torch\n");
goto err_create_torch_file;
}
- err = device_create_file(chip->cdev_torch.dev, &dev_attr_torch_pin);
- if (err < 0) {
- dev_err(chip->dev, "failed to create torch-pin file\n");
- goto err_create_torch_pin_file;
- }
/* indicator */
INIT_WORK(&chip->work_indicator,
@@ -411,12 +415,8 @@ static int lm3642_probe(struct i2c_client *client,
return 0;
err_create_indicator_file:
- device_remove_file(chip->cdev_torch.dev, &dev_attr_torch_pin);
-err_create_torch_pin_file:
led_classdev_unregister(&chip->cdev_torch);
err_create_torch_file:
- device_remove_file(chip->cdev_flash.dev, &dev_attr_strobe_pin);
-err_create_flash_pin_file:
led_classdev_unregister(&chip->cdev_flash);
err_out:
return err;
@@ -428,10 +428,8 @@ static int lm3642_remove(struct i2c_client *client)
led_classdev_unregister(&chip->cdev_indicator);
flush_work(&chip->work_indicator);
- device_remove_file(chip->cdev_torch.dev, &dev_attr_torch_pin);
led_classdev_unregister(&chip->cdev_torch);
flush_work(&chip->work_torch);
- device_remove_file(chip->cdev_flash.dev, &dev_attr_strobe_pin);
led_classdev_unregister(&chip->cdev_flash);
flush_work(&chip->work_flash);
regmap_write(chip->regmap, REG_ENABLE, 0);
--
1.8.5.5
^ permalink raw reply related
* [PATCH 04/13] leds: lm355x: fix attribute-creation race
From: Johan Hovold @ 2014-06-25 17:08 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie
Cc: Dmitry Torokhov, Greg Kroah-Hartman, linux-input, linux-kernel,
linux-leds, Janne Kanniainen, Jiri Kosina, Bjørn Mork,
Johan Hovold
In-Reply-To: <1403716136-32694-1-git-send-email-johan@kernel.org>
Use the attribute groups of the led-class to create the indicator-LED
attributes during probe in order to avoid racing with userspace.
Note that the comment about the pattern attribute only being for LM3554
was incorrect and did not match the code (the original leds-lm3556
driver had the attribute before LM3554 support was added).
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/leds/leds-lm355x.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/leds/leds-lm355x.c b/drivers/leds/leds-lm355x.c
index 591eb5e58ae3..f5112cb2d991 100644
--- a/drivers/leds/leds-lm355x.c
+++ b/drivers/leds/leds-lm355x.c
@@ -413,6 +413,12 @@ out:
static DEVICE_ATTR(pattern, S_IWUSR, NULL, lm3556_indicator_pattern_store);
+static struct attribute *lm355x_indicator_attrs[] = {
+ &dev_attr_pattern.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(lm355x_indicator);
+
static const struct regmap_config lm355x_regmap = {
.reg_bits = 8,
.val_bits = 8,
@@ -501,25 +507,18 @@ static int lm355x_probe(struct i2c_client *client,
else
chip->cdev_indicator.max_brightness = 8;
chip->cdev_indicator.brightness_set = lm355x_indicator_brightness_set;
+ /* indicator pattern control only for LM3556 */
+ if (id->driver_data == CHIP_LM3556)
+ chip->cdev_indicator.groups = lm355x_indicator_groups;
err = led_classdev_register((struct device *)
&client->dev, &chip->cdev_indicator);
if (err < 0)
goto err_create_indicator_file;
- /* indicator pattern control only for LM3554 */
- if (id->driver_data == CHIP_LM3556) {
- err =
- device_create_file(chip->cdev_indicator.dev,
- &dev_attr_pattern);
- if (err < 0)
- goto err_create_pattern_file;
- }
dev_info(&client->dev, "%s is initialized\n",
lm355x_name[id->driver_data]);
return 0;
-err_create_pattern_file:
- led_classdev_unregister(&chip->cdev_indicator);
err_create_indicator_file:
led_classdev_unregister(&chip->cdev_torch);
err_create_torch_file:
@@ -534,8 +533,6 @@ static int lm355x_remove(struct i2c_client *client)
struct lm355x_reg_data *preg = chip->regs;
regmap_write(chip->regmap, preg[REG_OPMODE].regno, 0);
- if (chip->type == CHIP_LM3556)
- device_remove_file(chip->cdev_indicator.dev, &dev_attr_pattern);
led_classdev_unregister(&chip->cdev_indicator);
flush_work(&chip->work_indicator);
led_classdev_unregister(&chip->cdev_torch);
--
1.8.5.5
^ permalink raw reply related
* [PATCH 03/13] leds: lm3533: fix attribute-creation race
From: Johan Hovold @ 2014-06-25 17:08 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie
Cc: Dmitry Torokhov, Greg Kroah-Hartman, linux-input, linux-kernel,
linux-leds, Janne Kanniainen, Jiri Kosina, Bjørn Mork,
Johan Hovold
In-Reply-To: <1403716136-32694-1-git-send-email-johan@kernel.org>
Use the attribute groups of the led-class to create the attributes
during probe in order to avoid racing with userspace.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/leds/leds-lm3533.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
index e2c642c1169b..cbf61a40137d 100644
--- a/drivers/leds/leds-lm3533.c
+++ b/drivers/leds/leds-lm3533.c
@@ -645,6 +645,11 @@ static struct attribute_group lm3533_led_attribute_group = {
.attrs = lm3533_led_attributes
};
+static const struct attribute_group *lm3533_led_attribute_groups[] = {
+ &lm3533_led_attribute_group,
+ NULL
+};
+
static int lm3533_led_setup(struct lm3533_led *led,
struct lm3533_led_platform_data *pdata)
{
@@ -692,6 +697,7 @@ static int lm3533_led_probe(struct platform_device *pdev)
led->cdev.brightness_get = lm3533_led_get;
led->cdev.blink_set = lm3533_led_blink_set;
led->cdev.brightness = LED_OFF;
+ led->cdev.groups = lm3533_led_attribute_groups,
led->id = pdev->id;
mutex_init(&led->mutex);
@@ -715,25 +721,16 @@ static int lm3533_led_probe(struct platform_device *pdev)
led->cb.dev = led->cdev.dev;
- ret = sysfs_create_group(&led->cdev.dev->kobj,
- &lm3533_led_attribute_group);
- if (ret < 0) {
- dev_err(&pdev->dev, "failed to create sysfs attributes\n");
- goto err_unregister;
- }
-
ret = lm3533_led_setup(led, pdata);
if (ret)
- goto err_sysfs_remove;
+ goto err_unregister;
ret = lm3533_ctrlbank_enable(&led->cb);
if (ret)
- goto err_sysfs_remove;
+ goto err_unregister;
return 0;
-err_sysfs_remove:
- sysfs_remove_group(&led->cdev.dev->kobj, &lm3533_led_attribute_group);
err_unregister:
led_classdev_unregister(&led->cdev);
flush_work(&led->work);
@@ -748,7 +745,6 @@ static int lm3533_led_remove(struct platform_device *pdev)
dev_dbg(&pdev->dev, "%s\n", __func__);
lm3533_ctrlbank_disable(&led->cb);
- sysfs_remove_group(&led->cdev.dev->kobj, &lm3533_led_attribute_group);
led_classdev_unregister(&led->cdev);
flush_work(&led->work);
--
1.8.5.5
^ permalink raw reply related
* [PATCH 02/13] leds: lm3550: fix attribute-creation race
From: Johan Hovold @ 2014-06-25 17:08 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie
Cc: Dmitry Torokhov, Greg Kroah-Hartman, linux-input, linux-kernel,
linux-leds, Janne Kanniainen, Jiri Kosina, Bjørn Mork,
Johan Hovold
In-Reply-To: <1403716136-32694-1-git-send-email-johan@kernel.org>
Use the attribute groups of the led-class to create the mode attribute
during probe in order to avoid racing with userspace.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/leds/leds-lm3530.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/leds/leds-lm3530.c b/drivers/leds/leds-lm3530.c
index 652368c2ea9a..91325de3cd33 100644
--- a/drivers/leds/leds-lm3530.c
+++ b/drivers/leds/leds-lm3530.c
@@ -400,6 +400,12 @@ static ssize_t lm3530_mode_set(struct device *dev, struct device_attribute
}
static DEVICE_ATTR(mode, 0644, lm3530_mode_get, lm3530_mode_set);
+static struct attribute *lm3530_attrs[] = {
+ &dev_attr_mode.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(lm3530);
+
static int lm3530_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -436,6 +442,7 @@ static int lm3530_probe(struct i2c_client *client,
drvdata->led_dev.name = LM3530_LED_DEV;
drvdata->led_dev.brightness_set = lm3530_brightness_set;
drvdata->led_dev.max_brightness = MAX_BRIGHTNESS;
+ drvdata->led_dev.groups = lm3530_groups;
i2c_set_clientdata(client, drvdata);
@@ -461,26 +468,13 @@ static int lm3530_probe(struct i2c_client *client,
return err;
}
- err = device_create_file(drvdata->led_dev.dev, &dev_attr_mode);
- if (err < 0) {
- dev_err(&client->dev, "File device creation failed: %d\n", err);
- err = -ENODEV;
- goto err_create_file;
- }
-
return 0;
-
-err_create_file:
- led_classdev_unregister(&drvdata->led_dev);
- return err;
}
static int lm3530_remove(struct i2c_client *client)
{
struct lm3530_data *drvdata = i2c_get_clientdata(client);
- device_remove_file(drvdata->led_dev.dev, &dev_attr_mode);
-
lm3530_led_disable(drvdata);
led_classdev_unregister(&drvdata->led_dev);
return 0;
--
1.8.5.5
^ permalink raw reply related
* [PATCH 01/13] leds: add led-class attribute-group support
From: Johan Hovold @ 2014-06-25 17:08 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie
Cc: Dmitry Torokhov, Greg Kroah-Hartman, linux-input, linux-kernel,
linux-leds, Janne Kanniainen, Jiri Kosina, Bjørn Mork,
Johan Hovold
In-Reply-To: <1403716136-32694-1-git-send-email-johan@kernel.org>
Allow led-class devices to be created with optional attribute groups.
This is needed in order to allow led drivers to create custom device
attributes in a race-free manner.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/leds/led-class.c | 5 +++--
include/linux/leds.h | 2 ++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index f37d63cf726b..aa29198fca3e 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -210,8 +210,9 @@ static const struct dev_pm_ops leds_class_dev_pm_ops = {
*/
int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
{
- led_cdev->dev = device_create(leds_class, parent, 0, led_cdev,
- "%s", led_cdev->name);
+ led_cdev->dev = device_create_with_groups(leds_class, parent, 0,
+ led_cdev, led_cdev->groups,
+ "%s", led_cdev->name);
if (IS_ERR(led_cdev->dev))
return PTR_ERR(led_cdev->dev);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 0287ab296689..e43686472197 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -63,6 +63,8 @@ struct led_classdev {
unsigned long *delay_off);
struct device *dev;
+ const struct attribute_group **groups;
+
struct list_head node; /* LED Device list */
const char *default_trigger; /* Trigger to use */
--
1.8.5.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox