* [PATCH] IIO:sysfs.h: Use static declaration
@ 2010-03-09 15:13 Hennerich, Michael
2010-03-09 15:29 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Hennerich, Michael @ 2010-03-09 15:13 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-iio@vger.kernel.org,
uclinux-dist-devel@blackfin.uclinux.org
Shouldn't those be better declared 'static'?
Index: drivers/staging/iio/sysfs.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- drivers/staging/iio/sysfs.h (revision 8430)
+++ drivers/staging/iio/sysfs.h (working copy)
@@ -95,16 +95,16 @@
.val2 =3D _val2 }
#define IIO_DEVICE_ATTR(_name, _mode, _show, _store, _addr) \
- struct iio_dev_attr iio_dev_attr_##_name \
+ static struct iio_dev_attr iio_dev_attr_##_name \
=3D IIO_ATTR(_name, _mode, _show, _store, _addr)
#define IIO_DEVICE_ATTR_2(_name, _mode, _show, _store, _addr, _val2) \
- struct iio_dev_attr iio_dev_attr_##_name \
+ static struct iio_dev_attr iio_dev_attr_##_name \
=3D IIO_ATTR_2(_name, _mode, _show, _store, _addr, _val2)
#define IIO_CONST_ATTR(_name, _string) \
- struct iio_const_attr iio_const_attr_##_name \
+ static struct iio_const_attr iio_const_attr_##_name =
\
=3D { .string =3D _string, =
\
.dev_attr =3D __ATTR(_name, S_IRUGO, iio_read_const_attr, NULL)=
}
------------------------------------------------------------------
********* Analog Devices GmbH Open Platform Solutions
** *****
** ** Wilhelm-Wagenfeld-Strasse 6
** ***** D-80807 Munich
********* Germany
Registergericht M=FCnchen HRB 40368, Gesch=E4ftsf=FChrer: Thomas Wessel, W=
illiam A. Martin, Margaret K. Seif
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] IIO:sysfs.h: Use static declaration
2010-03-09 15:13 [PATCH] IIO:sysfs.h: Use static declaration Hennerich, Michael
@ 2010-03-09 15:29 ` Jonathan Cameron
2010-03-09 15:39 ` Hennerich, Michael
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2010-03-09 15:29 UTC (permalink / raw)
To: Hennerich, Michael
Cc: linux-iio@vger.kernel.org,
uclinux-dist-devel@blackfin.uclinux.org
On 03/09/10 15:13, Hennerich, Michael wrote:
> Shouldn't those be better declared 'static'?
No. Convention is to do that when calling the macro as then=20
it is explicit in the relevant place. Now if any of the calls
are missing the static keyword, that would be an error.
I've cheated a bit on the event equivalent as they define a pair
of structures. Keep meaning to ask on how to do things like that
properly (don't suppose anyone knows?)
On another note, I'm getting increasingly tempted to drop the device
specific headers and move over to a more hwmon like fix naming at
review. With new api they are huge and sometimes actually add code
rather than saving it. Not that it effects these particularly macros.
>=20
>=20
> Index: drivers/staging/iio/sysfs.h
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- drivers/staging/iio/sysfs.h (revision 8430)
> +++ drivers/staging/iio/sysfs.h (working copy)
> @@ -95,16 +95,16 @@
> .val2 =3D _val2 }
>=20
> #define IIO_DEVICE_ATTR(_name, _mode, _show, _store, _addr) \
> - struct iio_dev_attr iio_dev_attr_##_name \
> + static struct iio_dev_attr iio_dev_attr_##_name \
> =3D IIO_ATTR(_name, _mode, _show, _store, _addr)
>=20
>=20
> #define IIO_DEVICE_ATTR_2(_name, _mode, _show, _store, _addr, _val2)=
\
> - struct iio_dev_attr iio_dev_attr_##_name =
\
> + static struct iio_dev_attr iio_dev_attr_##_name =
\
> =3D IIO_ATTR_2(_name, _mode, _show, _store, _addr, _val2)
>=20
> #define IIO_CONST_ATTR(_name, _string) =
\
> - struct iio_const_attr iio_const_attr_##_name =
\
> + static struct iio_const_attr iio_const_attr_##_name =
\
> =3D { .string =3D _string, =
\
> .dev_attr =3D __ATTR(_name, S_IRUGO, iio_read_const_attr,=
NULL)}
>=20
>=20
> ------------------------------------------------------------------
> ********* Analog Devices GmbH Open Platform Solutions
> ** *****
> ** ** Wilhelm-Wagenfeld-Strasse 6
> ** ***** D-80807 Munich
> ********* Germany
> Registergericht M=FCnchen HRB 40368, Gesch=E4ftsf=FChrer: Thomas Wes=
sel, William A. Martin, Margaret K. Seif
>=20
>=20
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] IIO:sysfs.h: Use static declaration
2010-03-09 15:29 ` Jonathan Cameron
@ 2010-03-09 15:39 ` Hennerich, Michael
2010-03-09 15:46 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Hennerich, Michael @ 2010-03-09 15:39 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-iio@vger.kernel.org,
uclinux-dist-devel@blackfin.uclinux.org
Hi Jonathan,
Jonathan Cameron wrote on 2010-03-09:
> On 03/09/10 15:13, Hennerich, Michael wrote:
>> Shouldn't those be better declared 'static'?
>
> No. Convention is to do that when calling the macro as then it is
> explicit in the relevant place. Now if any of the calls are missing
> the static keyword, that would be an error.
I don't mind - as long as they are declared static.
Because I'm seeing collisions with your driver - that doesn't declare it's =
common attributes static.
What about this patch?
Index: drivers/staging/iio/adc/max1363_core.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- drivers/staging/iio/adc/max1363_core.c (revision 8430)
+++ drivers/staging/iio/adc/max1363_core.c (working copy)
@@ -446,12 +446,12 @@
return ret;
}
-IIO_DEV_ATTR_AVAIL_SCAN_MODES(max1363_show_av_scan_modes);
-IIO_DEV_ATTR_SCAN_MODE(S_IRUGO | S_IWUSR,
+static IIO_DEV_ATTR_AVAIL_SCAN_MODES(max1363_show_av_scan_modes);
+static IIO_DEV_ATTR_SCAN_MODE(S_IRUGO | S_IWUSR,
max1363_show_scan_mode,
max1363_store_scan_mode);
-IIO_DEV_ATTR_SCAN(max1363_scan);
+static IIO_DEV_ATTR_SCAN(max1363_scan);
static ssize_t max1363_show_name(struct device *dev,
struct device_attribute *attr,
@@ -462,7 +462,7 @@
return sprintf(buf, "%s\n", st->chip_info->name);
}
-IIO_DEVICE_ATTR(name, S_IRUGO, max1363_show_name, NULL, 0);
+static IIO_DEVICE_ATTR(name, S_IRUGO, max1363_show_name, NULL, 0);
/*name export */
-Michael
>
> I've cheated a bit on the event equivalent as they define a pair of
> structures. Keep meaning to ask on how to do things like that
> properly (don't suppose anyone knows?)
>
> On another note, I'm getting increasingly tempted to drop the device
> specific headers and move over to a more hwmon like fix naming at
> review. With new api they are huge and sometimes actually add code
> rather than saving it. Not that it effects these particularly macros.
>
>
>
>>
>>
>> Index: drivers/staging/iio/sysfs.h
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- drivers/staging/iio/sysfs.h (revision 8430)
>> +++ drivers/staging/iio/sysfs.h (working copy)
>> @@ -95,16 +95,16 @@
>> .val2 =3D _val2 }
>> #define IIO_DEVICE_ATTR(_name, _mode, _show, _store, _addr) \
>> - struct iio_dev_attr iio_dev_attr_##_name \
>> + static struct iio_dev_attr iio_dev_attr_##_name \
>> =3D IIO_ATTR(_name, _mode, _show, _store, _addr)
>>
>> #define IIO_DEVICE_ATTR_2(_name, _mode, _show, _store, _addr,
>> _val2) \ - struct iio_dev_attr iio_dev_attr_##_name \ +
>> static struct iio_dev_attr iio_dev_attr_##_name
> \
>> =3D IIO_ATTR_2(_name, _mode, _show, _store, _addr, _val2)
>> #define IIO_CONST_ATTR(_name, _string)
> \
>> - struct iio_const_attr iio_const_attr_##_name \ + static
>> struct iio_const_attr iio_const_attr_##_name
> \
>> =3D { .string =3D _string,
> \
>> .dev_attr =3D __ATTR(_name, S_IRUGO, iio_read_const_attr,
>> NULL)}
>>
>>
>> ------------------------------------------------------------------
>> ********* Analog Devices GmbH Open Platform Solutions
>> ** *****
>> ** ** Wilhelm-Wagenfeld-Strasse 6
>> ** ***** D-80807 Munich
>> ********* Germany
>> Registergericht M=FCnchen HRB 40368, Gesch=E4ftsf=FChrer: Thomas Wessel=
,
>> William A. Martin, Margaret K. Seif
>>
>>
Greetings,
Michael
Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 4036 Geschaeft=
sfuehrer Thomas Wessel, William A. Martin, Margaret Seif
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] IIO:sysfs.h: Use static declaration
2010-03-09 15:39 ` Hennerich, Michael
@ 2010-03-09 15:46 ` Jonathan Cameron
0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2010-03-09 15:46 UTC (permalink / raw)
To: Hennerich, Michael
Cc: linux-iio@vger.kernel.org,
uclinux-dist-devel@blackfin.uclinux.org
On 03/09/10 15:39, Hennerich, Michael wrote:
> Hi Jonathan,
>=20
> Jonathan Cameron wrote on 2010-03-09:
>> On 03/09/10 15:13, Hennerich, Michael wrote:
>>> Shouldn't those be better declared 'static'?
>>
>> No. Convention is to do that when calling the macro as then it is
>> explicit in the relevant place. Now if any of the calls are missing
>> the static keyword, that would be an error.
>=20
> I don't mind - as long as they are declared static.
> Because I'm seeing collisions with your driver - that doesn't declare=
it's common attributes static.
>=20
> What about this patch?
That's where they should be.... Actually I have a vague recollection th=
at
there is already a patch doing these in Greg's tree. Also, not
worth pushing upstream as huge changes going into this driver shortly.
Of these, only name still exists.
Jonathan
>=20
> Index: drivers/staging/iio/adc/max1363_core.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- drivers/staging/iio/adc/max1363_core.c (revision 8430)
> +++ drivers/staging/iio/adc/max1363_core.c (working copy)
> @@ -446,12 +446,12 @@
> return ret;
> }
>=20
> -IIO_DEV_ATTR_AVAIL_SCAN_MODES(max1363_show_av_scan_modes);
> -IIO_DEV_ATTR_SCAN_MODE(S_IRUGO | S_IWUSR,
> +static IIO_DEV_ATTR_AVAIL_SCAN_MODES(max1363_show_av_scan_modes);
> +static IIO_DEV_ATTR_SCAN_MODE(S_IRUGO | S_IWUSR,
> max1363_show_scan_mode,
> max1363_store_scan_mode);
>=20
> -IIO_DEV_ATTR_SCAN(max1363_scan);
> +static IIO_DEV_ATTR_SCAN(max1363_scan);
>=20
> static ssize_t max1363_show_name(struct device *dev,
> struct device_attribute *attr,
> @@ -462,7 +462,7 @@
> return sprintf(buf, "%s\n", st->chip_info->name);
> }
>=20
> -IIO_DEVICE_ATTR(name, S_IRUGO, max1363_show_name, NULL, 0);
> +static IIO_DEVICE_ATTR(name, S_IRUGO, max1363_show_name, NULL, 0);
>=20
> /*name export */
>=20
> -Michael
>=20
>=20
>>
>> I've cheated a bit on the event equivalent as they define a pair of
>> structures. Keep meaning to ask on how to do things like that
>> properly (don't suppose anyone knows?)
>>
>> On another note, I'm getting increasingly tempted to drop the device
>> specific headers and move over to a more hwmon like fix naming at
>> review. With new api they are huge and sometimes actually add code
>> rather than saving it. Not that it effects these particularly macro=
s.
>>
>>
>>
>>>
>>>
>>> Index: drivers/staging/iio/sysfs.h
>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>> --- drivers/staging/iio/sysfs.h (revision 8430)
>>> +++ drivers/staging/iio/sysfs.h (working copy)
>>> @@ -95,16 +95,16 @@
>>> .val2 =3D _val2 }
>>> #define IIO_DEVICE_ATTR(_name, _mode, _show, _store, _addr) \
>>> - struct iio_dev_attr iio_dev_attr_##_name \
>>> + static struct iio_dev_attr iio_dev_attr_##_name \
>>> =3D IIO_ATTR(_name, _mode, _show, _store, _addr)
>>>
>>> #define IIO_DEVICE_ATTR_2(_name, _mode, _show, _store, _addr,
>>> _val2) \ - struct iio_dev_attr iio_dev_attr_##_name \ +
>>> static struct iio_dev_attr iio_dev_attr_##_name
>> \
>>> =3D IIO_ATTR_2(_name, _mode, _show, _store, _addr, _val2)
>>> #define IIO_CONST_ATTR(_name, _string)
>> \
>>> - struct iio_const_attr iio_const_attr_##_name \ + stat=
ic
>>> struct iio_const_attr iio_const_attr_##_name
>> \
>>> =3D { .string =3D _string,
>> \
>>> .dev_attr =3D __ATTR(_name, S_IRUGO, iio_read_const_att=
r,
>>> NULL)}
>>>
>>>
>>> ------------------------------------------------------------------
>>> ********* Analog Devices GmbH Open Platform Solutions
>>> ** *****
>>> ** ** Wilhelm-Wagenfeld-Strasse 6
>>> ** ***** D-80807 Munich
>>> ********* Germany
>>> Registergericht M=FCnchen HRB 40368, Gesch=E4ftsf=FChrer: Thomas W=
essel,
>>> William A. Martin, Margaret K. Seif
>>>
>>>
> Greetings,
> Michael
>=20
> Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
> Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 4036 Ges=
chaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif
>=20
>=20
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-03-09 15:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-09 15:13 [PATCH] IIO:sysfs.h: Use static declaration Hennerich, Michael
2010-03-09 15:29 ` Jonathan Cameron
2010-03-09 15:39 ` Hennerich, Michael
2010-03-09 15:46 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).