linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@cam.ac.uk>
To: "Hennerich, Michael" <Michael.Hennerich@analog.com>
Cc: "linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	"uclinux-dist-devel@blackfin.uclinux.org"
	<uclinux-dist-devel@blackfin.uclinux.org>
Subject: Re: [PATCH] IIO:sysfs.h: Use static declaration
Date: Tue, 09 Mar 2010 15:46:45 +0000	[thread overview]
Message-ID: <4B966D65.8050100@cam.ac.uk> (raw)
In-Reply-To: <544AC56F16B56944AEC3BD4E3D5917712D6B1D97E9@LIMKCMBX1.ad.analog.com>

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


      reply	other threads:[~2010-03-09 15:44 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B966D65.8050100@cam.ac.uk \
    --to=jic23@cam.ac.uk \
    --cc=Michael.Hennerich@analog.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=uclinux-dist-devel@blackfin.uclinux.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).