All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ardelean, Alexandru" <alexandru.Ardelean@analog.com>
To: "jic23@kernel.org" <jic23@kernel.org>,
	"lars@metafoo.de" <lars@metafoo.de>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>
Subject: Re: [RFC PATCH 00/14] iio: buffer: add support for multiple buffers
Date: Mon, 11 May 2020 10:33:27 +0000	[thread overview]
Message-ID: <8c5d9ef5ed4ea9037c5459daa2044d1cd7c5db7a.camel@analog.com> (raw)
In-Reply-To: <20200510110958.29046a18@archlinux>

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@metafoo.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.

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.

I tried using devtmpfs_create_node() directly, and it worked, but again, doing
'git grep devtmpfs_create_node'

drivers/base/base.h:int devtmpfs_create_node(struct device *dev);
drivers/base/base.h:static inline int devtmpfs_create_node(struct device *dev) {
return 0; }
drivers/base/core.c:            devtmpfs_create_node(dev);
drivers/base/devtmpfs.c:int devtmpfs_create_node(struct device *dev)
drivers/s390/char/hmcdrv_dev.c: * See: devtmpfs.c, function
devtmpfs_create_node()

Most calls are in core, and the one in hmcdrv driver isn't convincing enough to
do it.
In fact, some may consider it dangerous as it allows drivers/frameworks to use
it as precedent to do more name manipulation.
Again, if you guys say that this would be acceptable, I can use
device_get_devnode() and other stuff from core.


> 
> Do we want to make the naming a bit more self describing, something like
> iio/device0:buffer0?  Given the legacy interface will be outside
> the directory anyway, could even do
> 
> iio/device0/buffer0 with link to iio:device0
> iio/device0/buffer1 with no legacy link.
> 
> Ah, the bikeshedding fun we have ahead of us!

This may also depend on how much code it takes to implement these many levels of
folder hierarchy.

> 
> I think this set is going to take too much thinking for a Sunday
> so may take me a little while to do a proper review...
> + I have a few other side projects I want to hammer on today :)
> 
> Jonathan
> 

  reply	other threads:[~2020-05-11 10:33 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-08 13:53 [RFC PATCH 00/14] iio: buffer: add support for multiple buffers Alexandru Ardelean
2020-05-08 13:53 ` [RFC PATCH 01/14] iio: Move scan mask management to the core Alexandru Ardelean
2020-05-08 13:53 ` [RFC PATCH 02/14] iio: hw_consumer: use new scanmask functions Alexandru Ardelean
2020-05-08 13:53 ` [RFC PATCH 03/14] iio: buffer: add back-ref from iio_buffer to iio_dev Alexandru Ardelean
2020-05-08 13:53 ` [RFC PATCH 04/14] iio: core,buffer: wrap iio_buffer_put() call into iio_buffers_put() Alexandru Ardelean
2020-05-08 13:53 ` [RFC PATCH 05/14] iio: core: register chardev only if needed Alexandru Ardelean
2020-05-24 16:40   ` Jonathan Cameron
2020-05-08 13:53 ` [RFC PATCH 06/14] iio: buffer,event: duplicate chardev creation for buffers & events Alexandru Ardelean
2020-05-08 13:53 ` [RFC PATCH 07/14] iio: core: add simple centralized mechanism for ioctl() handlers Alexandru Ardelean
2020-05-24 16:45   ` Jonathan Cameron
2020-05-25  7:24     ` Ardelean, Alexandru
2020-05-08 13:53 ` [RFC PATCH 08/14] iio: core: use new common ioctl() mechanism Alexandru Ardelean
2020-05-24 16:47   ` Jonathan Cameron
2020-05-25  7:27     ` Ardelean, Alexandru
2020-05-31 15:20       ` Jonathan Cameron
2020-05-08 13:53 ` [RFC PATCH 09/14] iio: buffer: split buffer sysfs creation to take buffer as primary arg Alexandru Ardelean
2020-05-24 16:49   ` Jonathan Cameron
2020-05-25  7:28     ` Ardelean, Alexandru
2020-05-31 15:21       ` Jonathan Cameron
2020-05-08 13:53 ` [RFC PATCH 10/14] iio: buffer: remove attrcount_orig var from sysfs creation Alexandru Ardelean
2020-05-08 13:53 ` [RFC PATCH 11/14] iio: buffer: add underlying device object and convert buffers to devices Alexandru Ardelean
2020-05-08 13:53 ` [RFC PATCH 12/14] iio: buffer: symlink the scan_elements dir back into IIO device's dir Alexandru Ardelean
2020-05-08 13:53 ` [RFC PATCH 13/14] iio: unpack all iio buffer attributes correctly Alexandru Ardelean
2020-05-24 17:28   ` Jonathan Cameron
2020-05-08 13:53 ` [RFC PATCH 14/14] iio: buffer: convert single buffer to list of buffers Alexandru Ardelean
2020-05-09  8:52 ` [RFC PATCH 00/14] iio: buffer: add support for multiple buffers Lars-Peter Clausen
2020-05-10 10:09   ` Jonathan Cameron
2020-05-11 10:33     ` Ardelean, Alexandru [this message]
2020-05-11 10:37       ` Lars-Peter Clausen
2020-05-11 13:03         ` Ardelean, Alexandru
2020-05-11 13:24           ` Ardelean, Alexandru
2020-05-11 13:58             ` Lars-Peter Clausen
2020-05-11 14:56               ` Ardelean, Alexandru
2020-05-11 19:56                 ` Lars-Peter Clausen
2020-05-12  6:26                   ` Ardelean, Alexandru
2020-05-16 13:08                     ` Ardelean, Alexandru
2020-05-16 16:24                       ` Jonathan Cameron
2020-05-17  6:26                         ` Ardelean, Alexandru
2020-05-17 13:40                           ` Jonathan Cameron

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=8c5d9ef5ed4ea9037c5459daa2044d1cd7c5db7a.camel@analog.com \
    --to=alexandru.ardelean@analog.com \
    --cc=jic23@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.