[PATCH 0/2] Staging: iio: constify attribute_group structures

[PATCH 0/2] Staging: iio: constify attribute_group structures

[Bhumika Goyal]

Constify attribute_group structures.

Bhumika Goyal (2):
  Staging: iio: adc: constify attribute_group structures
  Staging: iio: light: constify attribute_group structures

 drivers/staging/iio/adc/ad7280a.c   | 2 +-
 drivers/staging/iio/light/tsl2583.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
1.9.1

[PATCH 1/2] Staging: iio: adc: constify attribute_group structures

[Bhumika Goyal]

Check for attribute_group structures that are only stored in the
attrs filed of iio_info structure. As the attrs field of iio_info
structures is constant, so these attribute_group structures can also be
declared constant.
Done using coccinelle:

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct attribute_group i@p = {...};

@ok1@
identifier r1.i;
position p;
struct iio_info x;
@@
x.attrs=&i@p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct attribute_group i={...};

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct attribute_group i;

File size before: drivers/staging/iio/adc/ad7280a.o
   text	   data	    bss	    dec	    hex	filename
   6487	    584	    776	   7847	   1ea7
drivers/staging/iio/adc/ad7280a.o

File size after: drivers/staging/iio/adc/ad7280a.o
   text	   data	    bss	    dec	    hex	filename
   6551	    544	    776	   7871	   1ebf
drivers/staging/iio/adc/ad7280a.o

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 drivers/staging/iio/adc/ad7280a.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
index 253a7f1..977241e 100644
--- a/drivers/staging/iio/adc/ad7280a.c
+++ b/drivers/staging/iio/adc/ad7280a.c
@@ -478,7 +478,7 @@ static ssize_t ad7280_store_balance_timer(struct device *dev,
 static struct attribute *ad7280_attributes[AD7280A_MAX_CHAIN *
 					   AD7280A_CELLS_PER_DEV * 2 + 1];
 
-static struct attribute_group ad7280_attrs_group = {
+static const struct attribute_group ad7280_attrs_group = {
 	.attrs = ad7280_attributes,
 };
 
-- 
1.9.1

[PATCH 2/2] Staging: iio: light: constify attribute_group structures

[Bhumika Goyal]

Check for attribute_group structures that are only stored in the
attrs filed of iio_info structure. As the attrs field of iio_info
structures is constant, so these attribute_group structures can also be
declared constant.
Done using coccinelle

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct attribute_group i@p = {...};

@ok1@
identifier r1.i;
position p;
struct iio_info x;
@@
x.attrs=&i@p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct attribute_group i={...};

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct attribute_group i;

File size before: drivers/staging/iio/light/tsl2583.o
   text	   data	    bss	    dec	    hex	filename
   6529	   1052	      0	   7581	   1d9d
drivers/staging/iio/light/tsl2583.o

File size after: drivers/staging/iio/light/tsl2583.o
   text	   data	    bss	    dec	    hex	filename
   6593	    988	      0	   7581	   1d9d
drivers/staging/iio/light/tsl2583.o

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 drivers/staging/iio/light/tsl2583.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
index 05b4ad4..08f1583 100644
--- a/drivers/staging/iio/light/tsl2583.c
+++ b/drivers/staging/iio/light/tsl2583.c
@@ -803,7 +803,7 @@ static struct attribute *sysfs_attrs_ctrl[] = {
 	NULL
 };
 
-static struct attribute_group tsl2583_attribute_group = {
+static const struct attribute_group tsl2583_attribute_group = {
 	.attrs = sysfs_attrs_ctrl,
 };
 
-- 
1.9.1

Re: [PATCH 1/2] Staging: iio: adc: constify attribute_group structures

[Bhumika Goyal]

On Mon, Sep 26, 2016 at 2:54 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 09/26/2016 07:01 AM, Bhumika Goyal wrote:
>> Check for attribute_group structures that are only stored in the
>> attrs filed of iio_info structure. As the attrs field of iio_info
>> structures is constant, so these attribute_group structures can also be
>> declared constant.
>> Done using coccinelle:
>>
>> @r1 disable optional_qualifier @
>> identifier i;
>> position p;
>> @@
>> static struct attribute_group i@p = {...};
>>
>> @ok1@
>> identifier r1.i;
>> position p;
>> struct iio_info x;
>> @@
>> x.attrs=&i@p;
>>
>> @bad@
>> position p!={r1.p,ok1.p};
>> identifier r1.i;
>> @@
>> i@p
>>
>> @depends on !bad disable optional_qualifier@
>> identifier r1.i;
>> @@
>> static
>> +const
>> struct attribute_group i={...};
>>
>> @depends on !bad disable optional_qualifier@
>> identifier r1.i;
>> @@
>> +const
>> struct attribute_group i;
>>
>> File size before: drivers/staging/iio/adc/ad7280a.o
>>    text          data     bss     dec     hex filename
>>    6487           584     776    7847    1ea7
>> drivers/staging/iio/adc/ad7280a.o
>>
>> File size after: drivers/staging/iio/adc/ad7280a.o
>>    text          data     bss     dec     hex filename
>>    6551           544     776    7871    1ebf
>> drivers/staging/iio/adc/ad7280a.o
>>
>> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
>
> Looks good, thanks. Just as something to remember for future patches, if you
> send a patch for a single driver it is best to mention the driver in the
> subject. Usually you can also leave out the sub-subsystem. So in this case
> something like "staging:iio:ad7280a: ..."
>
OK, i will keep this is mind.
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
>
> This patch also raises another issue. In this driver we have two
> attribute_groups, one for the device attributes, one for the event
> attributes. The device attribute group has now been made const, but the
> event struct has not.
>
> For some reason the event_attrs field in the iio_info struct is not a const
> struct attribute_group. Even though there is no reason why it shouldn't be.
> Let me know if this is something you want to fix, once that is done there
> are a few more attribute_group structs in the drivers that can be made const.
>

Well, I thought of constifying the attribute_groups structures for
event attributes but then as you said the event_attrs filed of
iio_info is not constant so left it. But I would definitely like to
work on this further. So, firstly I need to find why the event_attrs
field can be made constant in the iio_info structure definition?

Thanks,
Bhumika
>> ---
>>  drivers/staging/iio/adc/ad7280a.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
>> index 253a7f1..977241e 100644
>> --- a/drivers/staging/iio/adc/ad7280a.c
>> +++ b/drivers/staging/iio/adc/ad7280a.c
>> @@ -478,7 +478,7 @@ static ssize_t ad7280_store_balance_timer(struct device *dev,
>>  static struct attribute *ad7280_attributes[AD7280A_MAX_CHAIN *
>>                                          AD7280A_CELLS_PER_DEV * 2 + 1];
>>
>> -static struct attribute_group ad7280_attrs_group = {
>> +static const struct attribute_group ad7280_attrs_group = {
>>       .attrs = ad7280_attributes,
>>  };
>>
>>
>

Re: [PATCH 1/2] Staging: iio: adc: constify attribute_group structures

[Lars-Peter Clausen]

On 09/26/2016 12:03 PM, Bhumika Goyal wrote:
>> This patch also raises another issue. In this driver we have two
>> > attribute_groups, one for the device attributes, one for the event
>> > attributes. The device attribute group has now been made const, but the
>> > event struct has not.
>> >
>> > For some reason the event_attrs field in the iio_info struct is not a const
>> > struct attribute_group. Even though there is no reason why it shouldn't be.
>> > Let me know if this is something you want to fix, once that is done there
>> > are a few more attribute_group structs in the drivers that can be made const.
>> >
> Well, I thought of constifying the attribute_groups structures for
> event attributes but then as you said the event_attrs filed of
> iio_info is not constant so left it. But I would definitely like to
> work on this further. So, firstly I need to find why the event_attrs
> field can be made constant in the iio_info structure definition?

As far as I can tell this is simply an oversight, there is no reason why it
should not be const, the IIO core is not going to modify the field, it is
handled in the same way as the device attributes. The best way to handle
this is send one patch to mark the field const in the iio_info struct (with
an explanation in the commit message why that can and should be done) and
then send the cleanups as follow up patches.

- Lars

Re: [PATCH 1/2] Staging: iio: adc: constify attribute_group structures

[Lars-Peter Clausen]

On 09/27/2016 09:22 PM, Jonathan Cameron wrote:
>>
>> Looks good, thanks. Just as something to remember for future patches, if you
>> send a patch for a single driver it is best to mention the driver in the
>> subject. Usually you can also leave out the sub-subsystem. So in this case
>> something like "staging:iio:ad7280a: ..."
>>
>> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
> Applied to the togreg branch of iio.git and pushed out as testing.

Greg fast-tracked this one, it's already in staging-testing.

Re: [Outreachy kernel] Re: [PATCH 1/2] Staging: iio: adc: constify attribute_group structures

[Greg KH]

On Tue, Sep 27, 2016 at 09:26:26PM +0200, Lars-Peter Clausen wrote:
> On 09/27/2016 09:22 PM, Jonathan Cameron wrote:
> >>
> >> Looks good, thanks. Just as something to remember for future patches, if you
> >> send a patch for a single driver it is best to mention the driver in the
> >> subject. Usually you can also leave out the sub-subsystem. So in this case
> >> something like "staging:iio:ad7280a: ..."
> >>
> >> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
> > Applied to the togreg branch of iio.git and pushed out as testing.
> 
> Greg fast-tracked this one, it's already in staging-testing.

That's fine, git handles merges wonderfully :)