From: Jean Delvare <jdelvare@suse.de>
To: Chris Packham <Chris.Packham@alliedtelesis.co.nz>
Cc: wsa@kernel.org, linux-i2c@vger.kernel.org
Subject: Re: [i2c-tools PATCH] tools: i2cbusses: Handle bus names like /dev/i2c-0
Date: Wed, 2 Jun 2021 09:25:04 +0200 [thread overview]
Message-ID: <20210602092504.462bc28e@endymion> (raw)
In-Reply-To: <a9bce37a-085b-f863-e1b0-5f5faa91f063@alliedtelesis.co.nz>
Hi Chris,
On Wed, 26 May 2021 21:23:07 +0000, Chris Packham wrote:
> On 26/05/21 7:39 pm, Jean Delvare wrote:
> > I can't really see the value of this change, sorry. You want to use a
> > longer parameter so you can tab-complete it. The original parameter was
> > a 1- or 2-digit number, which is faster to type than /d<tab>i2<tab>.
> > Plus if you have multiple i2c buses, tab completion can't guess which
> > one you want anyway, so you'll have to type the bus number eventually.
> >
> > So, what do we actually win here?
>
> My main motivation was to replace an in-house tool that is provides
> similar functionality but it currently takes the bus as a path. At first
> I even thought there was a bug because I thought "or an I2C bus name"
> meant the path, it wasn't until I looked at the code that I realised
> this was the name used in the kernel.
OK, that's a better explanation. But I'm still not convinced by the
benefit. I'm sure you guys can learn quickly to pass just the i2c bus
number as the first parameter. Plus I don't like your implementation
for various technical reasons anyway (like allocating extra memory for
every bus when you may never actually need it, and hard-coding the
/dev/i2c-<N> pattern when there's at least one alternative supported by
i2c-tools at the moment - although I'm unsure if anyone still uses it).
So I'm not going to apply your patch, sorry.
> One advantage I can see is that the /d<tab>/i2<tab> implicitly validates
> that the bus actually exists (assuming /dev is managed by devtmpfs
> and/or udev).
That's not an advantage. Running the command on the wrong I2C bus could
have bad consequences. The only safe way to use the tool without
checking the list of available i2c buses first is to select the I2C bus
by name.
--
Jean Delvare
SUSE L3 Support
next prev parent reply other threads:[~2021-06-02 7:25 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-25 9:06 [i2c-tools PATCH] tools: i2cbusses: Handle bus names like /dev/i2c-0 Chris Packham
2021-05-26 7:39 ` Jean Delvare
2021-05-26 21:23 ` Chris Packham
2021-06-02 7:25 ` Jean Delvare [this message]
2021-06-03 22:57 ` Chris Packham
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=20210602092504.462bc28e@endymion \
--to=jdelvare@suse.de \
--cc=Chris.Packham@alliedtelesis.co.nz \
--cc=linux-i2c@vger.kernel.org \
--cc=wsa@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 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).