From: Lars-Peter Clausen <lars@metafoo.de>
To: Jonathan Cameron <jic23@kernel.org>,
Crestez Dan Leonard <cdleonard@gmail.com>,
Yong Li <sdliyong@gmail.com>
Cc: knaack.h@gmx.de, pmeerw@pmeerw.net, k.kozlowski@samsung.com,
mranostay@gmail.com, linux-iio@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iio: tmp006: Set correct iio name
Date: Tue, 26 Apr 2016 12:57:52 +0200 [thread overview]
Message-ID: <571F49B0.4060407@metafoo.de> (raw)
In-Reply-To: <7621197a-95ed-239e-8baa-83014bc27a26@kernel.org>
On 04/25/2016 11:11 PM, Jonathan Cameron wrote:
> On 25/04/16 21:59, Crestez Dan Leonard wrote:
>> On 04/25/2016 10:33 PM, Jonathan Cameron wrote:
>>> On 22/04/16 04:43, Yong Li wrote:
>>>> When load the driver using the below command:
>>>> echo tmp006 0x40 > /sys/bus/i2c/devices/i2c-0/new_device
>>>>
>>>> In sysfs, the i2c name is tmp006, however the iio name is 0-0040,
>>>> they are inconsistent. With this patch,
>>>> the iio name will be the same as the i2c device name
>>>>
>>>> Signed-off-by: Yong Li <sdliyong@gmail.com>
>>> Peter, this looks right to me, but could you take a quick look as I guess
>>> there might be a reason you did this in an unusual way originally?
>>>
>> Is there a "correct" or "usual" way to set indio_dev->name? Some quick grepping shows no clear standard:
>>
>> $ git grep -h 'indio_dev->name =' drivers/iio/ | wc -l
>> 148
>> $ git grep -h 'indio_dev->name =' drivers/iio/ | grep id | wc -l
>> 52
>> $ git grep -h 'indio_dev->name =' drivers/iio/ | grep dev_name | wc -l
>> 20
>> $ git grep -h 'indio_dev->name =' drivers/iio/ | grep -i drv | wc -l
>> 19
>> $ git grep -h 'indio_dev->name =' drivers/iio/ | grep -i driver | wc -l
>> 15
>>
>> It seems that many devices use dev_name(&i2c_client->dev) or
>> otherwise some sort of "ABC123_DRIVER_NAME" constant.
>>
>> It's also not clear what this "name" field is for. Is it more than
>> just a cosmetic sysfs attribute? It seems to me that names don't have
>> to be unique so it would be wrong to use them for identification.
>>
> It's a convenience field really as there is no clear standard for where else
> to find out what a part actually is (i.e. if it is an i2c part you can look
> in the name attribute i2c supplies - but otherwise you are on your own).
I'd like to argue that it is supposed to be the device type allowing the
application to identify which kind of device they are talking to. In which
case the dev_name() initialization is wrong. Unfortunately there seem quite
a few drivers which use it, especially the the SoC ADC drivers. But we can't
really change this for existing drivers since there might be applications
relying on the name.
We should pay more attention to this for new driver submissions and make
sure we don't get any more of this.
As a side note if you want to know the dev_name() of the parent device you
can do a readlink() on the iio device in /sys/bus/iio/devices/. The second
last entry in the path name is the name of the parent device.
next prev parent reply other threads:[~2016-04-26 10:58 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-22 3:43 [PATCH] iio: tmp006: Set correct iio name Yong Li
2016-04-25 19:33 ` Jonathan Cameron
2016-04-25 20:59 ` Crestez Dan Leonard
2016-04-25 21:11 ` Jonathan Cameron
2016-04-26 10:57 ` Lars-Peter Clausen [this message]
2016-04-26 11:47 ` Yong Li
2016-04-26 12:01 ` Lars-Peter Clausen
2016-04-26 13:14 ` Yong Li
2016-04-26 15:21 ` Daniel Baluta
2016-04-27 3:42 ` Yong Li
2016-04-27 16:58 ` Crestez Dan Leonard
2016-04-28 8:25 ` Lars-Peter Clausen
2016-04-28 13:24 ` One Thousand Gnomes
2016-04-28 13:30 ` Peter Meerwald-Stadler
2016-04-28 14:19 ` Lars-Peter Clausen
2016-05-01 19:34 ` Jonathan Cameron
2016-05-03 9:43 ` [was iio: tmp006: Set correct iio name] how to support multiple instances of same device type Gregor Boirie
2016-05-03 12:00 ` Crestez Dan Leonard
2016-04-28 14:17 ` [PATCH] iio: tmp006: Set correct iio name Lars-Peter Clausen
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=571F49B0.4060407@metafoo.de \
--to=lars@metafoo.de \
--cc=cdleonard@gmail.com \
--cc=jic23@kernel.org \
--cc=k.kozlowski@samsung.com \
--cc=knaack.h@gmx.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mranostay@gmail.com \
--cc=pmeerw@pmeerw.net \
--cc=sdliyong@gmail.com \
/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).