How to attach file operations to sysfs attribute in platform driver?

Question

I wrote a platform driver for a peripheral we developed and would like to expose some configuration options to the sysfs. I have managed to create the appropriate files using attribute structs (see below) and sysfs_create_file in the probe function, but I can't figure out how to attach the show/store functions to the structs in a platform driver.

Most resources I found online used a device_attribute struct or something similar to create their files, is that also appropriate here? Is there another way to do this for a platform driver?

My attribute struct looks like this:

struct attribute subkey_attr = {
    .name = "subkeys",
    .mode = S_IWUGO | S_IRUGO,
};

And I register the file using this call:

riddler_kobject = &pdev->dev.kobj;
ret_val = sysfs_create_file(riddler_kobject, &subkey_attr);

Solution 1

It boils down to next:

• reuse existing kobject from struct device (from your struct platform_device) for sysfs_create_group() (instead of creating your own kobject)
• use DEVICE_ATTR() to declare struct device_attribute instead of regular __ATTR(), which creates struct kobj_attribute.

Here is how I created sysfs attributes for my platform driver.

1. Create structure you'll be using as private data in show() / store() operations for your sysfs attribute (file). For example:

   struct mydrv {
       struct device *dev;
       long myparam;
   };
   

2. Allocate this structure in your driver's probe():

   static int mydrv_probe(struct platform_device *pdev)
   {
       struct mydrv *mydrv;
   
       mydrv = devm_kzalloc(&pdev->dev, sizeof(*mydrv), GFP_KERNEL);
       mydrv->dev = &pdev->dev;
       platform_set_drvdata(pdev, mydrv);
   
       ...
   }
   

3. Create show() / store() functions:

   static ssize_t mydrv_myparam_show(struct device *dev,
           struct device_attribute *attr, char *buf)
   {
       struct mydrv *mydrv = dev_get_drvdata(dev);
       int len;
   
       len = sprintf(buf, "%d\n", mydrv->myparam);
       if (len <= 0)
           dev_err(dev, "mydrv: Invalid sprintf len: %d\n", len);
   
       return len;
   }
   
   static ssize_t mydrv_myparam_store(struct device *dev,
           struct device_attribute *attr, const char *buf, size_t count)
   {
       struct mydrv *mydrv = dev_get_drvdata(dev);
   
       kstrtol(buf, 10, &mydrv->myparam);
       return count;
   }
   

4. Create device attribute for those functions (right after those functions):

   static DEVICE_ATTR(myparam, S_IRUGO | S_IWUSR, mydrv_myparam_show,
                      mydrv_myparam_store);
   

5. Declare attributes table (listing in fact sysfs files for you driver):

   static struct attribute *mydrv_attrs[] = {
       &dev_attr_myparam.attr,
       NULL
   };
   

6. Declare attribute group (specifying in fact sysfs directory for your driver):

   static struct attribute_group mydrv_group = {
       .name = "mydrv",
       .attrs = mydrv_attrs,
   };
   
   static struct attribute_group *mydrv_groups[] = {
       &mydrv_group,
       NULL
   }
   

   which can be actually replaced with one line:

   ATTRIBUTE_GROUPS(mydrv);
   

7. Create sysfs directory and files in your driver's probe() function:

   static int mydrv_probe(struct platform_device *pdev)
   {
       int ret;
   
       ...
   
       ret = sysfs_create_group(&pdev->dev.kobj, &mydrv_group);
       if (ret) {
           dev_err(&pdev->dev, "sysfs creation failed\n");
           return ret;
       }
   
       ...
   }
   

8. Remove your sysfs files in your driver's remove() function:

   static int mydrv_remove(struct platform_device *pdev)
   {
       sysfs_remove_group(&pdev->dev.kobj, &mydrv_group);
       ...
   }
   

Race condition note

As @FranzForstmayr correctly pointed out, there may be race condition when adding sysfs files with sysfs_create_group() in mydrv_probe(). That's because user-space can be already notified that those files exist before mydrv_probe() called (where those files are actually being created by sysfs_create_group() function). This issue covered in details in "How to Create a sysfs File Correctly" article by Greg Kroah-Hartman.

So in our case of platform_device, instead of calling sysfs_create_group() (and its counterpart sysfs_remove_group()), you can use default attribute group. To do so, you need to assign corresponding .groups field of your struct device to your attribute groups variable:

static int mydrv_probe(struct platform_device *pdev)
{
    ...

    pdev->dev.groups = mydrv_groups;

    ...
}

DISCLAIMER: I didn't test this code, though it should work, because of this code.

See [1,2,3] links for more insights on mentioned race condition.

For more examples, run next command in kernel source directory:

$ git grep -l --all-match -e platform_device -e attribute -e '\.groups =' -- drivers/

Also you can search by "default attribute" in commit messages:

$ git log --no-merges --oneline --grep="default attribute" -- drivers/

Some commits I found this way: [4,5,6,7].

References

[1] My attributes are way too racy, what should I do?

[2] PATCH: sysfs: add devm_sysfs_create_group() and friends

[3] [GIT PATCH] Driver core patches for 3.11-rc2

[4] commit 1

[5] commit 2

[6] commit 3

[7] commit 4

Solution 2

Not enough reputation to post a comment, but I just want to comment on the default attribute group note from the accepted answer. My understanding is that this should not be added in the probe function, as given in the example, but instead should be set in the device struct, (or device_driver, class, or bus depending on your driver) usually defined at the end of your file. For example:

static struct device iio_evgen_dev = {
    .bus = &iio_bus_type,
    .groups = iio_evgen_groups,
    .release = &iio_evgen_release,
};

from this example

Strangely, according to this it doesn't work correctly when using DEVICE_INT_ATTR to create the attribute, so not sure what that's all about.

Also, I'm not 100% sure, but I think that this is invoked when the driver is loaded, not when the device is probed.

Comments

[Franz Forstmayr]

Can you explain how to adapt this solution according to the post of Greg KH? kroah.com/log/blog/2013/06/26/…

[Sam Protsenko]

@FranzForstmayr That's a valid point. Frankly I didn't know about that. But I added Race condition note to my answer, explaining how to use insights from article you mentioned.