Message-ID: <c88b17c3-a9d6-e755-04e8-bc9f225e2a3b@metafoo.de>
Date:   Mon, 11 May 2020 12:37:56 +0200
From:   Lars-Peter Clausen <lars@...afoo.de>
To:     "Ardelean, Alexandru" <alexandru.Ardelean@...log.com>,
        "jic23@...nel.org" <jic23@...nel.org>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>
Subject: Re: [RFC PATCH 00/14] iio: buffer: add support for multiple buffers

On 5/11/20 12:33 PM, Ardelean, Alexandru wrote:
> On Sun, 2020-05-10 at 11:09 +0100, Jonathan Cameron wrote:
>> [External]
>>
>> On Sat, 9 May 2020 10:52:14 +0200
>> Lars-Peter Clausen <lars@...afoo.de> wrote:
>>
>>> On 5/8/20 3:53 PM, Alexandru Ardelean wrote:
>>>> [...]
>>>> What I don't like, is that iio:device3 has iio:buffer3:0 (to 3).
>>>> This is because the 'buffer->dev.parent = &indio_dev->dev'.
>>>> But I do feel this is correct.
>>>> So, now I don't know whether to leave it like that or symlink to shorter
>>>> versions like 'iio:buffer3:Y' -> 'iio:device3/bufferY'.
>>>> The reason for naming the IIO buffer devices to 'iio:bufferX:Y' is
>>>> mostly to make the names unique. It would have looked weird to do
>>>> '/dev/buffer1' if I would have named the buffer devices 'bufferX'.
>>>>
>>>> So, now I'm thinking of whether all this is acceptable.
>>>> Or what is acceptable?
>>>> Should I symlink 'iio:device3/iio:buffer3:0' -> 'iio:device3/buffer0'?
>>>> What else should I consider moving forward?
>>>> What means forward?
>>>> Where did I leave my beer?
>>> Looking at how the /dev/ devices are named I think we can provide a name
>>> that is different from the dev_name() of the device. Have a look at
>>> device_get_devnode() in drivers/base/core.c. We should be able to
>>> provide the name for the chardev through the devnode() callback.
>>>
>>> While we are at this, do we want to move the new devices into an iio
>>> subfolder? So iio/buffer0:0 instead of iio:buffer0:0?
>> Possibly on the folder.  I can't for the life of me remember why I decided
>> not to do that the first time around - I'll leave it at the
>> mysterious "it may turn out to be harder than you'd think..."
>> Hopefully not ;)
> I was also thinking about the /dev/iio subfolder while doing this.
> I can copy that from /dev/input
> They seem to do it already.
> I don't know how difficult it would be. But it looks like a good precedent.

All you have to do is return "iio/..." from the devnode() callback.

>
> My concern regarding going to use stuff from core [like device_get_devnode()] is
> that it seems to bypass some layers of kernel.
> If I do 'git grep device_get_devnode', I get:
>
> drivers/base/core.c:            name = device_get_devnode(dev, &mode, &uid,
> &gid, &tmp);
> drivers/base/core.c: * device_get_devnode - path of device node file
> drivers/base/core.c:const char *device_get_devnode(struct device *dev,
> drivers/base/devtmpfs.c:        req.name = device_get_devnode(dev, &req.mode,
> &req.uid, &req.gid, &tmp);
> drivers/base/devtmpfs.c:        req.name = device_get_devnode(dev, NULL, NULL,
> NULL, &tmp);
> include/linux/device.h:extern const char *device_get_devnode(struct device *dev,
> (END)
>
> So, basically, most uses of device_get_devnode() are in core code, and I feel
> that this may be sanctioned somewhere by some core people, if I do it.
> I could be wrong, but if you disagree, I'll take your word for it.
You are not supposed to use the function itself, you should implement 
the devnode() callback for the IIO bus, which is then used by the core 
device_get_devnode() function.

Powered by blists - more mailing lists