lkml.org 
[lkml]   [2014]   [Jun]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 2/2] HID: leds: move led_mode attribute to led-class devices in MSI GT683R driver
    On Tue, Jun 24, 2014 at 03:56:51PM -0400, Greg KH wrote:
    > On Tue, Jun 24, 2014 at 10:38:38PM +0300, Janne Kanniainen wrote:

    > > static int gt683r_led_probe(struct hid_device *hdev,
    > > const struct hid_device_id *id)
    > > @@ -261,6 +272,7 @@ 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;
    > > +
    > > ret = led_classdev_register(&hdev->dev, &led->led_devs[i]);
    > > if (ret) {
    > > hid_err(hdev, "could not register led device\n");
    > > @@ -268,17 +280,29 @@ 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;
    > > + for (i = 0; i < GT683R_LED_COUNT; i++) {
    > > + led->dev_attr_group[i] = &gt683r_group;
    > > +
    > > + ret = sysfs_create_group(&led->led_devs[i].dev->kobj,
    > > + led->dev_attr_group[i]);
    >
    > why not use sysfs_create_groups()?
    >
    > And why are you doing it this way, the led device should have the
    > attribute group and it should be created automatically by the driver
    > core, no driver should need to create it like you are doing so here.

    Turns out it was easier said than done when I asked Janne to move the
    attribute. And all led drivers with custom attributes currently suffer
    from this race.

    Janne, you'll need to apply patch below and then set the groups field of
    the led_devs before registering them. You can use the ATTRIBUTE_GROUPS
    macro in the following way:

    static struct attribute *gt683r_led_attrs[] = {
    &dev_attr_msi_mode.attr,
    NULL,
    };
    ATTRIBUTE_GROUPS(gt683r_led);

    and then set .groups to gt683r_led_groups.

    I have started fixing some of the other led drivers and I'll try to
    submit a series (including the below patch) later today.

    Thanks,
    Johan


    >From 226f1de5e094f2ec99f45d486652505ef915af73 Mon Sep 17 00:00:00 2001
    From: Johan Hovold <johan@kernel.org>
    Date: Wed, 25 Jun 2014 12:33:18 +0200
    Subject: [PATCH] leds: add led-class attribute-group support

    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


    \
     
     \ /
      Last update: 2014-06-25 14:41    [W:4.123 / U:0.272 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site