4

One of Linux kernel training courses task is to add sysfs support to one of previously written device drivers. I choose my ds1307 rtc driver and want to add device attributes, not to replace rtc device ones.

But I am not sure that my solution is correct.

At the time when the probe function is called i2c client device is already created, so that any manipulation on device attributes will cause race condition. Thus I decide to add my attributes to rtc device before device registration. I just count default rtc attribute groups, allocate new array with one extra position for my attribute group and replace rtc device groups pointer with allocated array.

static int ds1307x_probe(struct i2c_client *client,
    const struct i2c_device_id *id)
{
    struct device *dev;
    const struct attribute_group **grp, **ngrp;
    struct rtc_device *rtc;
    int ret, cnt;

    /* device presence check code skipped */

    dev = &client->dev;
    rtc = devm_rtc_allocate_device(dev);
    if (!rtc)
        return -ENOMEM;

    dev = &rtc->dev;
    grp = dev->groups;
    if (!grp) {
        /* no default groups, just use own groups array */
        dev->groups = ds1307x_groups;
    } else {
        /* copy rtc groups array, add own group at end */
        cnt = 0;
        while (grp[cnt])
            ++cnt;

        ngrp = devm_kmalloc(&rtc->dev, (cnt+2) * sizeof(*ngrp),
                            GFP_KERNEL);
        if (!ngrp)
            return -ENOMEM;

        memcpy(ngrp, grp, cnt * sizeof(*ngrp));
        ngrp[cnt] = &ds1307x_group;
        ngrp[cnt+1] = NULL;
        dev->groups = ngrp;
    }

    rtc->uie_unsupported = 1;
    rtc->ops = &ds1307x_ops;

    ret = rtc_register_device(rtc);

    /* remaining of code skipped */
}

Attribute group ds1307x_control with attributes clock_halt and out_ctrl appears in rtc0 device directory among standard rtc class attributes:

~ # ls /sys/class/rtc/rtc0/
date             ds1307x_control  name             subsystem
dev              hctosys          power            time
device           max_user_freq    since_epoch      uevent
~ # ls /sys/class/rtc/rtc0/ds1307x_control/
clock_halt  out_ctrl

This solution works for me and I hope it is good enough for training courses.

But what about real life? Are there any hidden problems?

ReAl
  • 1,231
  • 1
  • 8
  • 19
  • 2
    Yes, it's racy. Your attributes must be very careful about object lifetimes. Nowadays we have `.dev_groups` member for that purpose and also feq `sysfs_*()` APIs if you wish to do this dynamically at the probing stage. Read Greg KH's article on the topic. – 0andriy Jul 07 '21 at 20:22
  • 1
    Thanks! I saw it added in 5.0 kernel, [rtc_add_groups](https://elixir.bootlin.com/linux/v5.0/source/drivers/rtc/sysfs.c#L317) as for RTC and I made almost the same. I just used constant 1 as `add_cnt` and did not free old attr because it might be a static struct and will be freed by devm at rmmod if it is dynamically allocated. – ReAl Jul 08 '21 at 20:03

0 Answers0