[PATCH v3 2/3] mtd: part: Add sysfs variable for offset of partition

[Daniel Ehrenberg]

On Fri, Mar 27, 2015 at 5:53 PM, Brian Norris
<computersforpeace at gmail.com> wrote:
> On Fri, Mar 27, 2015 at 12:24:16PM -0700, Dan Ehrenberg wrote:
>> This patch makes a sysfs variable called 'offset' on each partition
>> which contains the offset in bytes from the beginning of the master
>> device that the partition starts.
>>
>> Signed-off-by: Dan Ehrenberg <dehrenberg at chromium.org>
>> ---
>>  drivers/mtd/mtdpart.c | 29 +++++++++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>
>> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
>> index a19ec5a..b8ed202 100644
>> --- a/drivers/mtd/mtdpart.c
>> +++ b/drivers/mtd/mtdpart.c
>> @@ -554,6 +554,30 @@ out_register:
>>       return slave;
>>  }
>>
>> +static ssize_t mtd_partition_offset_show(struct device *dev,
>> +             struct device_attribute *attr, char *buf)
>> +{
>> +     struct mtd_info *mtd = dev_get_drvdata(dev);
>> +     struct mtd_part *part = PART(mtd);
>> +     return snprintf(buf, PAGE_SIZE, "%lld\n", part->offset);
>> +}
>> +
>> +static DEVICE_ATTR(offset, S_IRUGO, mtd_partition_offset_show, NULL);
>
> You'll want #include <linux/device.h>, just to be explicit.
>
> And if you're adding to the sysfs ABI, please update
> Documentation/ABI/testing/sysfs-class-mtd
>
>> +
>> +const static struct attribute *mtd_partition_attrs[] = {
>
> gcc complains:
>
>   drivers/mtd/mtdpart.c:568:1: warning: 'static' is not at beginning of declaration [-Wold-style-declaration]
>
>> +     &dev_attr_offset.attr,
>> +     NULL
>> +};
>> +
>> +int mtd_add_partition_attrs(struct mtd_part *new)
>
> gcc and sprase complain:
>
>   drivers/mtd/mtdpart.c:573:5: warning: no previous prototype for 'mtd_add_partition_attrs' [-Wmissing-prototypes]
>   drivers/mtd/mtdpart.c:573:5: warning: symbol 'mtd_add_partition_attrs' was not declared. Should it be static? [sparse]

Thanks, not sure why these warnings haven't been showing up for me
(while other warnings do, e.g., if I make an unused variable). I'll
fix this and the docs in a new patch.
>
>> +{
>> +     int ret = sysfs_create_files(&new->mtd.dev.kobj, mtd_partition_attrs);
>> +     if (ret)
>> +             printk(KERN_WARNING
>> +                    "mtd: failed to create partition attrs, err=%d\n", ret);
>> +     return ret;
>> +}
>> +
>>  int mtd_add_partition(struct mtd_info *master, const char *name,
>>                     long long offset, long long length)
>>  {
>> @@ -603,6 +627,8 @@ int mtd_add_partition(struct mtd_info *master, const char *name,
>>
>>       add_mtd_device(&new->mtd);
>>
>> +     mtd_add_partition_attrs(new);
>
> Hmm, so you're adding the partition attribute only to dynamically
> created partitions? I think we'll want it for all partitions. I know
> the naming isn't completely straightforward, but some partitions are
> added through add_mtd_device(), without calling mtd_add_partition().

It's also called in add_mtd_partitions, as you can see just below. I
tested this with static partitions and the attribute was visible.
>
>> +
>>       return ret;
>>  err_inv:
>>       mutex_unlock(&mtd_partitions_mutex);
>> @@ -620,6 +646,8 @@ int mtd_del_partition(struct mtd_info *master, int partno)
>>       list_for_each_entry_safe(slave, next, &mtd_partitions, list)
>>               if ((slave->master == master) &&
>>                   (slave->mtd.index == partno)) {
>> +                     sysfs_remove_files(&slave->mtd.dev.kobj,
>> +                                        mtd_partition_attrs);
>>                       ret = del_mtd_device(&slave->mtd);
>>                       if (ret < 0)
>>                               break;
>> @@ -663,6 +691,7 @@ int add_mtd_partitions(struct mtd_info *master,
>>               mutex_unlock(&mtd_partitions_mutex);
>>
>>               add_mtd_device(&slave->mtd);
>> +             mtd_add_partition_attrs(slave);
>>
>>               cur_offset = slave->offset + slave->mtd.size;
>>       }
>
> Brian