[PATCH] iio: core: Fix mapping of iio channels to entry numbers

[PATCH] iio: core: Fix mapping of iio channels to entry numbers

[Gaurav Gupta]

When adding maps to the list, they were added using list_add, which adds
them in LIFO order. When parsing using iio_channel_get_all(), these
elements are hence returned in reverse order. As a result, the iio_hwmon
mapping maps the first entry to the last channel and so on.

Signed-off-by: Gaurav Gupta <gauragup@cisco.com>
---
 drivers/iio/inkern.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 7a13535..9b0715c 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -44,7 +44,7 @@ int iio_map_array_register(struct iio_dev *indio_dev, struct iio_map *maps)
 		}
 		mapi->map = &maps[i];
 		mapi->indio_dev = indio_dev;
-		list_add(&mapi->l, &iio_map_list);
+		list_add_tail(&mapi->l, &iio_map_list);
 		i++;
 	}
 error_ret:
-- 
2.10.2.dirty

Re: [PATCH] iio: core: Fix mapping of iio channels to entry numbers

[Jonathan Cameron]

On Tue, 27 Jun 2017 09:46:01 -0700
Gaurav Gupta <gauragup@cisco.com> wrote:

> When adding maps to the list, they were added using list_add, which adds
> them in LIFO order. When parsing using iio_channel_get_all(), these
> elements are hence returned in reverse order. As a result, the iio_hwmon
> mapping maps the first entry to the last channel and so on.
> 
> Signed-off-by: Gaurav Gupta <gauragup@cisco.com>
Gah.  This is one of those nasty cases where I clearly messed up a
long time ago, but unfortunately it's become user space ABI.

So looking for general opinions on whether we can fix this one
or not.  I can't immediately think of a clean way to tidy this up.

So do we think people will notice if we fix this?

Jonathan
> ---
>  drivers/iio/inkern.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 7a13535..9b0715c 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -44,7 +44,7 @@ int iio_map_array_register(struct iio_dev *indio_dev, struct iio_map *maps)
>  		}
>  		mapi->map = &maps[i];
>  		mapi->indio_dev = indio_dev;
> -		list_add(&mapi->l, &iio_map_list);
> +		list_add_tail(&mapi->l, &iio_map_list);
>  		i++;
>  	}
>  error_ret:

Re: [PATCH] iio: core: Fix mapping of iio channels to entry numbers

[Guenter Roeck]

On 07/01/2017 10:40 AM, Jonathan Cameron wrote:
> On Tue, 27 Jun 2017 09:46:01 -0700
> Gaurav Gupta <gauragup@cisco.com> wrote:
> 
>> When adding maps to the list, they were added using list_add, which adds
>> them in LIFO order. When parsing using iio_channel_get_all(), these
>> elements are hence returned in reverse order. As a result, the iio_hwmon
>> mapping maps the first entry to the last channel and so on.
>>
>> Signed-off-by: Gaurav Gupta <gauragup@cisco.com>
> Gah.  This is one of those nasty cases where I clearly messed up a
> long time ago, but unfortunately it's become user space ABI.
> 
> So looking for general opinions on whether we can fix this one
> or not.  I can't immediately think of a clean way to tidy this up.
> 
> So do we think people will notice if we fix this?
> 

I would just go for it.

Guenter

> Jonathan
>> ---
>>   drivers/iio/inkern.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
>> index 7a13535..9b0715c 100644
>> --- a/drivers/iio/inkern.c
>> +++ b/drivers/iio/inkern.c
>> @@ -44,7 +44,7 @@ int iio_map_array_register(struct iio_dev *indio_dev, struct iio_map *maps)
>>   		}
>>   		mapi->map = &maps[i];
>>   		mapi->indio_dev = indio_dev;
>> -		list_add(&mapi->l, &iio_map_list);
>> +		list_add_tail(&mapi->l, &iio_map_list);
>>   		i++;
>>   	}
>>   error_ret:
> 
> 

Re: [PATCH] iio: core: Fix mapping of iio channels to entry numbers

[Gaurav Gupta (gauragup)]

SGkgSm9uYXRoYW4sDQoNCkp1c3Qgd29uZGVyaW5nIGlmIHlvdSBmb3Jlc2VlIGFueSBpc3N1ZXMg
d2l0aCB0aGlzIGNoYW5nZSBvciBhbnkgZnVydGhlciBjaGFuZ2VzIHRoYXQgbmVlZCB0byBoYXBw
ZW4gYXMgYSBzaWRlLWVmZmVjdCBvZiB0aGlzPw0KDQpUaGFua3MsDQpHYXVyYXYNCg0KT24gNy8x
LzE3LCAxMTo0NiBBTSwgImxpbnV4LWlpby1vd25lckB2Z2VyLmtlcm5lbC5vcmcgb24gYmVoYWxm
IG9mIEd1ZW50ZXIgUm9lY2siIDxsaW51eC1paW8tb3duZXJAdmdlci5rZXJuZWwub3JnIG9uIGJl
aGFsZiBvZiBsaW51eEByb2Vjay11cy5uZXQ+IHdyb3RlOg0KDQogICAgT24gMDcvMDEvMjAxNyAx
MDo0MCBBTSwgSm9uYXRoYW4gQ2FtZXJvbiB3cm90ZToNCiAgICA+IE9uIFR1ZSwgMjcgSnVuIDIw
MTcgMDk6NDY6MDEgLTA3MDANCiAgICA+IEdhdXJhdiBHdXB0YSA8Z2F1cmFndXBAY2lzY28uY29t
PiB3cm90ZToNCiAgICA+IA0KICAgID4+IFdoZW4gYWRkaW5nIG1hcHMgdG8gdGhlIGxpc3QsIHRo
ZXkgd2VyZSBhZGRlZCB1c2luZyBsaXN0X2FkZCwgd2hpY2ggYWRkcw0KICAgID4+IHRoZW0gaW4g
TElGTyBvcmRlci4gV2hlbiBwYXJzaW5nIHVzaW5nIGlpb19jaGFubmVsX2dldF9hbGwoKSwgdGhl
c2UNCiAgICA+PiBlbGVtZW50cyBhcmUgaGVuY2UgcmV0dXJuZWQgaW4gcmV2ZXJzZSBvcmRlci4g
QXMgYSByZXN1bHQsIHRoZSBpaW9faHdtb24NCiAgICA+PiBtYXBwaW5nIG1hcHMgdGhlIGZpcnN0
IGVudHJ5IHRvIHRoZSBsYXN0IGNoYW5uZWwgYW5kIHNvIG9uLg0KICAgID4+DQogICAgPj4gU2ln
bmVkLW9mZi1ieTogR2F1cmF2IEd1cHRhIDxnYXVyYWd1cEBjaXNjby5jb20+DQogICAgPiBHYWgu
ICBUaGlzIGlzIG9uZSBvZiB0aG9zZSBuYXN0eSBjYXNlcyB3aGVyZSBJIGNsZWFybHkgbWVzc2Vk
IHVwIGENCiAgICA+IGxvbmcgdGltZSBhZ28sIGJ1dCB1bmZvcnR1bmF0ZWx5IGl0J3MgYmVjb21l
IHVzZXIgc3BhY2UgQUJJLg0KICAgID4gDQogICAgPiBTbyBsb29raW5nIGZvciBnZW5lcmFsIG9w
aW5pb25zIG9uIHdoZXRoZXIgd2UgY2FuIGZpeCB0aGlzIG9uZQ0KICAgID4gb3Igbm90LiAgSSBj
YW4ndCBpbW1lZGlhdGVseSB0aGluayBvZiBhIGNsZWFuIHdheSB0byB0aWR5IHRoaXMgdXAuDQog
ICAgPiANCiAgICA+IFNvIGRvIHdlIHRoaW5rIHBlb3BsZSB3aWxsIG5vdGljZSBpZiB3ZSBmaXgg
dGhpcz8NCiAgICA+IA0KICAgIA0KICAgIEkgd291bGQganVzdCBnbyBmb3IgaXQuDQogICAgDQog
ICAgR3VlbnRlcg0KICAgIA0KICAgID4gSm9uYXRoYW4NCiAgICA+PiAtLS0NCiAgICA+PiAgIGRy
aXZlcnMvaWlvL2lua2Vybi5jIHwgMiArLQ0KICAgID4+ICAgMSBmaWxlIGNoYW5nZWQsIDEgaW5z
ZXJ0aW9uKCspLCAxIGRlbGV0aW9uKC0pDQogICAgPj4NCiAgICA+PiBkaWZmIC0tZ2l0IGEvZHJp
dmVycy9paW8vaW5rZXJuLmMgYi9kcml2ZXJzL2lpby9pbmtlcm4uYw0KICAgID4+IGluZGV4IDdh
MTM1MzUuLjliMDcxNWMgMTAwNjQ0DQogICAgPj4gLS0tIGEvZHJpdmVycy9paW8vaW5rZXJuLmMN
CiAgICA+PiArKysgYi9kcml2ZXJzL2lpby9pbmtlcm4uYw0KICAgID4+IEBAIC00NCw3ICs0NCw3
IEBAIGludCBpaW9fbWFwX2FycmF5X3JlZ2lzdGVyKHN0cnVjdCBpaW9fZGV2ICppbmRpb19kZXYs
IHN0cnVjdCBpaW9fbWFwICptYXBzKQ0KICAgID4+ICAgCQl9DQogICAgPj4gICAJCW1hcGktPm1h
cCA9ICZtYXBzW2ldOw0KICAgID4+ICAgCQltYXBpLT5pbmRpb19kZXYgPSBpbmRpb19kZXY7DQog
ICAgPj4gLQkJbGlzdF9hZGQoJm1hcGktPmwsICZpaW9fbWFwX2xpc3QpOw0KICAgID4+ICsJCWxp
c3RfYWRkX3RhaWwoJm1hcGktPmwsICZpaW9fbWFwX2xpc3QpOw0KICAgID4+ICAgCQlpKys7DQog
ICAgPj4gICAJfQ0KICAgID4+ICAgZXJyb3JfcmV0Og0KICAgID4gDQogICAgPiANCiAgICANCiAg
ICAtLQ0KICAgIFRvIHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1
bnN1YnNjcmliZSBsaW51eC1paW8iIGluDQogICAgdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1h
am9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcNCiAgICBNb3JlIG1ham9yZG9tbyBpbmZvIGF0ICBodHRw
Oi8vdmdlci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0bWwNCiAgICANCg0K

Re: [PATCH] iio: core: Fix mapping of iio channels to entry numbers

[Jonathan Cameron]

On Fri, 14 Jul 2017 16:51:12 +0000
"Gaurav Gupta (gauragup)" <gauragup@cisco.com> wrote:

> Hi Jonathan,
> 
> Just wondering if you foresee any issues with this change or any further changes that need to happen as a side-effect of this?
This still makes me nervous so I'm going to take the cautious route of pushing it
out via the slow path of the next merge window.  If no one shouts for a month
or so after that, we can request a backport to stable.

So applied to the togreg branch of iio.git.

Jonathan
> 
> Thanks,
> Gaurav
> 
> On 7/1/17, 11:46 AM, "linux-iio-owner@vger.kernel.org on behalf of Guenter Roeck" <linux-iio-owner@vger.kernel.org on behalf of linux@roeck-us.net> wrote:
> 
>     On 07/01/2017 10:40 AM, Jonathan Cameron wrote:
>     > On Tue, 27 Jun 2017 09:46:01 -0700
>     > Gaurav Gupta <gauragup@cisco.com> wrote:
>     >   
>     >> When adding maps to the list, they were added using list_add, which adds
>     >> them in LIFO order. When parsing using iio_channel_get_all(), these
>     >> elements are hence returned in reverse order. As a result, the iio_hwmon
>     >> mapping maps the first entry to the last channel and so on.
>     >>
>     >> Signed-off-by: Gaurav Gupta <gauragup@cisco.com>  
>     > Gah.  This is one of those nasty cases where I clearly messed up a
>     > long time ago, but unfortunately it's become user space ABI.
>     > 
>     > So looking for general opinions on whether we can fix this one
>     > or not.  I can't immediately think of a clean way to tidy this up.
>     > 
>     > So do we think people will notice if we fix this?
>     >   
>     
>     I would just go for it.
>     
>     Guenter
>     
>     > Jonathan  
>     >> ---
>     >>   drivers/iio/inkern.c | 2 +-
>     >>   1 file changed, 1 insertion(+), 1 deletion(-)
>     >>
>     >> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
>     >> index 7a13535..9b0715c 100644
>     >> --- a/drivers/iio/inkern.c
>     >> +++ b/drivers/iio/inkern.c
>     >> @@ -44,7 +44,7 @@ int iio_map_array_register(struct iio_dev *indio_dev, struct iio_map *maps)
>     >>   		}
>     >>   		mapi->map = &maps[i];
>     >>   		mapi->indio_dev = indio_dev;
>     >> -		list_add(&mapi->l, &iio_map_list);
>     >> +		list_add_tail(&mapi->l, &iio_map_list);
>     >>   		i++;
>     >>   	}
>     >>   error_ret:  
>     > 
>     >   
>     
>     --
>     To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>     the body of a message to majordomo@vger.kernel.org
>     More majordomo info at  http://vger.kernel.org/majordomo-info.html
>     
>