[PATCH] staging: iio: ring_generic: provide IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT

[PATCH] staging: iio: ring_generic: provide IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT

[michael.hennerich]

From: Michael Hennerich <michael.hennerich@analog.com>


Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
---
 drivers/staging/iio/ring_generic.h |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/ring_generic.h b/drivers/staging/iio/ring_generic.h
index abd52ea..3b46236 100644
--- a/drivers/staging/iio/ring_generic.h
+++ b/drivers/staging/iio/ring_generic.h
@@ -292,6 +292,33 @@ ssize_t iio_scan_el_ts_show(struct device *dev, struct device_attribute *attr,
  **/
 #define IIO_CONST_ATTR_SCAN_EL_TYPE(_name, _sign, _bits, _storagebits) \
 	IIO_CONST_ATTR(_name##_type, #_sign#_bits"/"#_storagebits);
+
+/**
+ * IIO_CONST_ATTR_SCAN_EL_TYPE - attr to specify the data format of a scan el
+ * @name: the scan el name (may be more general and cover a set of scan elements
+ * @_sign: either s or u for signed or unsigned
+ * @_bits: number of actual bits occuplied by the value
+ * @_storagebits: number of bits _bits is padded to when read out of buffer
+ **/
+#define IIO_CONST_ATTR_SCAN_EL_TYPE(_name, _sign, _bits, _storagebits) \
+	IIO_CONST_ATTR(_name##_type, #_sign#_bits"/"#_storagebits);
+
+/**
+ * IIO_CONST_ATTR_SCAN_EL_TYPE - attr to specify the data format of a scan el
+ * @name: the scan el name (may be more general and cover a set of scan elements
+ * @_sign: either s or u for signed or unsigned
+ * @_bits: number of actual bits occuplied by the value
+ * @_storagebits: number of bits _bits is padded to when read out of buffer
+ * @_shiftbits: number of bits _shiftbits the result must be shifted
+ **/
+#define IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT(_name, _sign, _bits, \
+					       _storagebits, _shiftbits) \
+	IIO_CONST_ATTR(_name##_type, #_sign#_bits"/"#_storagebits \
+		       ">>"#_shiftbits);
+
+#define IIO_SCAN_EL_TYPE_SIGNED         's'
+#define IIO_SCAN_EL_TYPE_UNSIGNED       'u'
+
 /*
  * These are mainly provided to allow for a change of implementation if a device
  * has a large number of scan elements
-- 
1.6.0.2

[PATCH] staging: iio: ring_generic: provide IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT

[Michael Hennerich]

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
---
 drivers/staging/iio/ring_generic.h |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/ring_generic.h b/drivers/staging/iio/ring_generic.h
index abd52ea..728aa41 100644
--- a/drivers/staging/iio/ring_generic.h
+++ b/drivers/staging/iio/ring_generic.h
@@ -292,6 +292,23 @@ ssize_t iio_scan_el_ts_show(struct device *dev, struct device_attribute *attr,
  **/
 #define IIO_CONST_ATTR_SCAN_EL_TYPE(_name, _sign, _bits, _storagebits) \
 	IIO_CONST_ATTR(_name##_type, #_sign#_bits"/"#_storagebits);
+
+/**
+ * IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT - attr to specify the data format of a scan el
+ * @name: the scan el name (may be more general and cover a set of scan elements
+ * @_sign: either s or u for signed or unsigned
+ * @_bits: number of actual bits occuplied by the value
+ * @_storagebits: number of bits _bits is padded to when read out of buffer
+ * @_shiftbits: number of bits _shiftbits the result must be shifted
+ **/
+#define IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT(_name, _sign, _bits, \
+					       _storagebits, _shiftbits) \
+	IIO_CONST_ATTR(_name##_type, #_sign#_bits"/"#_storagebits \
+		       ">>"#_shiftbits);
+
+#define IIO_SCAN_EL_TYPE_SIGNED         's'
+#define IIO_SCAN_EL_TYPE_UNSIGNED       'u'
+
 /*
  * These are mainly provided to allow for a change of implementation if a device
  * has a large number of scan elements
-- 
1.6.0.2

Re: [PATCH] staging: iio: ring_generic: provide IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT

[Jonathan Cameron]

On 10/05/10 14:00, Michael Hennerich wrote:
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>

Send it to Greg with the first user... Also need to update the
documentation in sysfs-bus-iio to cover this, but that can be
a separate patch.

Jonathan
> ---
>  drivers/staging/iio/ring_generic.h |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/staging/iio/ring_generic.h b/drivers/staging/iio/ring_generic.h
> index abd52ea..728aa41 100644
> --- a/drivers/staging/iio/ring_generic.h
> +++ b/drivers/staging/iio/ring_generic.h
> @@ -292,6 +292,23 @@ ssize_t iio_scan_el_ts_show(struct device *dev, struct device_attribute *attr,
>   **/
>  #define IIO_CONST_ATTR_SCAN_EL_TYPE(_name, _sign, _bits, _storagebits) \
>  	IIO_CONST_ATTR(_name##_type, #_sign#_bits"/"#_storagebits);
> +
> +/**
> + * IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT - attr to specify the data format of a scan el
> + * @name: the scan el name (may be more general and cover a set of scan elements
> + * @_sign: either s or u for signed or unsigned
> + * @_bits: number of actual bits occuplied by the value
> + * @_storagebits: number of bits _bits is padded to when read out of buffer
> + * @_shiftbits: number of bits _shiftbits the result must be shifted
> + **/
> +#define IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT(_name, _sign, _bits, \
> +					       _storagebits, _shiftbits) \
> +	IIO_CONST_ATTR(_name##_type, #_sign#_bits"/"#_storagebits \
> +		       ">>"#_shiftbits);
> +
> +#define IIO_SCAN_EL_TYPE_SIGNED         's'
> +#define IIO_SCAN_EL_TYPE_UNSIGNED       'u'
> +
>  /*
>   * These are mainly provided to allow for a change of implementation if a device
>   * has a large number of scan elements

RE: [PATCH] staging: iio: ring_generic: provide IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT

[Hennerich, Michael]

Jonathan Cameron wrote on 2010-10-05:
> On 10/05/10 14:00, Michael Hennerich wrote:
>>
>> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
>
> Send it to Greg with the first user... Also need to update the
> documentation in sysfs-bus-iio to cover this, but that can be a
> separate patch.
>
> Jonathan

Hi Jonathan,

I wonder why the documentation is specific to accel?
I think that can be many things...

What:           /sys/.../device[n]/buffer/scan_elements/accel[_x0]_type
KernelVersion:  2.6.37
Contact:        linux-iio@vger.kernel.org
Description:
                Description of the scan element data storage within the buf=
fer
                and hence the form in which it is read from userspace.
                Form is [s|u]bits/storagebits.  s or u specifies if signed
                (2's complement) or unsigned. bits is the number of bits of
                data and storagebits is the space (after padding) that it
                occupies in the buffer.  Note that some devices will have
                additional information in the unused bits so to get a clean
                value, the bits value must be used to mask the buffer outpu=
t
                value appropriately.  The storagebits value also specifies =
the
                data alignment.  So s48/64 will be a signed 48 bit integer
                stored in a 64 bit location aligned to a a64 bit boundary.
                For other storage combinations this attribute will be exten=
ded
                appropriately.





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

Re: [PATCH] staging: iio: ring_generic: provide IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT

[Jonathan Cameron]

On 10/05/10 15:11, Hennerich, Michael wrote:
> Jonathan Cameron wrote on 2010-10-05:
>> On 10/05/10 14:00, Michael Hennerich wrote:
>>>
>>> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
>> Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
>>
>> Send it to Greg with the first user... Also need to update the
>> documentation in sysfs-bus-iio to cover this, but that can be a
>> separate patch.
>>
>> Jonathan
> 
> Hi Jonathan,
> 
> I wonder why the documentation is specific to accel?
> I think that can be many things...
Simply because the format of those file doesn't (I think) allow for
a variable in the name and as you say there are lots of sensor types.

Hence if I did it as a [accel|magn| etc] I'd end up with a very very long
line.

It appears that those file were created by Greg KH (according to git blame
on the readme) 

So Greg, if you are the person to ask (if not please suggest who is!)...

In the sysfs abi docs, we have been rather fast and loose with the formatting
and Michael's question in this email motivated me to ask for some guidance.

We have an awful lot of possible sysfs attributes to document.
So far we have been using [] to indicate optional elements in naming (explained
in the description). (e.g. accel[_x]_type )

A lot of the file sysfs-bus-iio uses only accel as an example whereas the
attributes equally exist for all the other types (e.g. magn, gyro, illuminance etc).

In discussions on list we have been using <type> to indicate a variable which can
take any of the channel types, but I don't know if it is valid to use this in
the abi files.  That is, can we define some variable that takes a set of values
just once for the file and then use <type> in the naming to indicate replacement
with one of the set?

Another option would be to have multiple what lines. e.g.

What:  accel_[x|y|z]_type
What:  gyro_[x|y|z]_type
What:  magn_[x|y|z]_type

etc.

The approach of just doing it for accel has caused a number of people to miss
certain definitions so far so we need to work out some want of making this clearer
preferably without obliging people to have sections like the following

What: gyro_[x|y|z]_type
...
Description:
	Equivalent of accel_[x|y|z]_type but for gyroscope channels.

Thanks,

Jonathan


> 
> What:           /sys/.../device[n]/buffer/scan_elements/accel[_x0]_type
> KernelVersion:  2.6.37
> Contact:        linux-iio@vger.kernel.org
> Description:
>                 Description of the scan element data storage within the buffer
>                 and hence the form in which it is read from userspace.
>                 Form is [s|u]bits/storagebits.  s or u specifies if signed
>                 (2's complement) or unsigned. bits is the number of bits of
>                 data and storagebits is the space (after padding) that it
>                 occupies in the buffer.  Note that some devices will have
>                 additional information in the unused bits so to get a clean
>                 value, the bits value must be used to mask the buffer output
>                 value appropriately.  The storagebits value also specifies the
>                 data alignment.  So s48/64 will be a signed 48 bit integer
>                 stored in a 64 bit location aligned to a a64 bit boundary.
>                 For other storage combinations this attribute will be extended
>                 appropriately.
> 
> 
> 
> 
> 
> Greetings,
> Michael
> 
> Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
> Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 4036 Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif
> 
> 
> 

[PATCH] staging: iio: ring_generic: provide IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT

[michael.hennerich]

From: Michael Hennerich <michael.hennerich@analog.com>


Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
---
 drivers/staging/iio/ring_generic.h |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/ring_generic.h b/drivers/staging/iio/ring_generic.h
index abd52ea..728aa41 100644
--- a/drivers/staging/iio/ring_generic.h
+++ b/drivers/staging/iio/ring_generic.h
@@ -292,6 +292,23 @@ ssize_t iio_scan_el_ts_show(struct device *dev, struct device_attribute *attr,
  **/
 #define IIO_CONST_ATTR_SCAN_EL_TYPE(_name, _sign, _bits, _storagebits) \
 	IIO_CONST_ATTR(_name##_type, #_sign#_bits"/"#_storagebits);
+
+/**
+ * IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT - attr to specify the data format of a scan el
+ * @name: the scan el name (may be more general and cover a set of scan elements
+ * @_sign: either s or u for signed or unsigned
+ * @_bits: number of actual bits occuplied by the value
+ * @_storagebits: number of bits _bits is padded to when read out of buffer
+ * @_shiftbits: number of bits _shiftbits the result must be shifted
+ **/
+#define IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT(_name, _sign, _bits, \
+					       _storagebits, _shiftbits) \
+	IIO_CONST_ATTR(_name##_type, #_sign#_bits"/"#_storagebits \
+		       ">>"#_shiftbits);
+
+#define IIO_SCAN_EL_TYPE_SIGNED         's'
+#define IIO_SCAN_EL_TYPE_UNSIGNED       'u'
+
 /*
  * These are mainly provided to allow for a change of implementation if a device
  * has a large number of scan elements
-- 
1.6.0.2

Re: [PATCH] staging: iio: ring_generic: provide IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT

[Jonathan Cameron]

On 10/07/10 13:24, michael.hennerich@analog.com wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
I'm guessing a user of this is following soon?

Ideally the patch title would mention the SIGNED UNSIGNED
defines as well, but they are so simple it doesn't really
matter.
> 
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
> ---
>  drivers/staging/iio/ring_generic.h |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/staging/iio/ring_generic.h b/drivers/staging/iio/ring_generic.h
> index abd52ea..728aa41 100644
> --- a/drivers/staging/iio/ring_generic.h
> +++ b/drivers/staging/iio/ring_generic.h
> @@ -292,6 +292,23 @@ ssize_t iio_scan_el_ts_show(struct device *dev, struct device_attribute *attr,
>   **/
>  #define IIO_CONST_ATTR_SCAN_EL_TYPE(_name, _sign, _bits, _storagebits) \
>  	IIO_CONST_ATTR(_name##_type, #_sign#_bits"/"#_storagebits);
> +
> +/**
> + * IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT - attr to specify the data format of a scan el
> + * @name: the scan el name (may be more general and cover a set of scan elements
> + * @_sign: either s or u for signed or unsigned
> + * @_bits: number of actual bits occuplied by the value
> + * @_storagebits: number of bits _bits is padded to when read out of buffer
> + * @_shiftbits: number of bits _shiftbits the result must be shifted
> + **/
> +#define IIO_CONST_ATTR_SCAN_EL_TYPE_WITH_SHIFT(_name, _sign, _bits, \
> +					       _storagebits, _shiftbits) \
> +	IIO_CONST_ATTR(_name##_type, #_sign#_bits"/"#_storagebits \
> +		       ">>"#_shiftbits);
> +
> +#define IIO_SCAN_EL_TYPE_SIGNED         's'
> +#define IIO_SCAN_EL_TYPE_UNSIGNED       'u'
> +
>  /*
>   * These are mainly provided to allow for a change of implementation if a device
>   * has a large number of scan elements