public inbox for linux-i2c@vger.kernel.org
 help / color / mirror / Atom feed
From: Jean Delvare <jdelvare@suse.de>
To: Wolfram Sang <wsa@kernel.org>
Cc: Paul Wise <pabs3@bonedaddy.net>, linux-i2c@vger.kernel.org
Subject: Re: [PATCH i2c-tools v2] i2cdetect: add messages for errors during bus listing
Date: Mon, 19 Jun 2023 16:37:03 +0200	[thread overview]
Message-ID: <20230619163703.209d3b18@endymion.delvare> (raw)
In-Reply-To: <ZIoxmbo9Cu+bPHsF@shikoro>

Hi Wolfram, Paul,

First of all, sorry for not replying earlier to the v1 of this patch.

On Wed, 14 Jun 2023 23:31:05 +0200, Wolfram Sang wrote:
> Hi Paul,
> 
> thank you for this patch!
> 
> > This makes it easier for new users to understand what is going on when
> > they have a problem listing i2c busses that they do not understand.  
> 
> I like this basically. I do have questions, though.

I don't like it.

I don't think this is the purpose of i2c tools, which are very
specific, to tell the user how to perform generic system administration
tasks. If /proc or /sys is not mounted then you are facing a much
bigger problem than i2cdetect not working. Basically your system is
close to unusable, and this is completely unrelated to i2c-tools. I
can't even think of a realistic situation where these new error
messages would show up. So this is dead code to me.

Plus, manually mounting sysfs or procfs may solve your immediate
problem, but it is most certainly not the proper way to fix your
system. So this isn't even good advice.

The other thing this patch is doing is telling about missing i2c-dev.
Now this is something that can actually happen, and will happen. I
actually have an item about that on my todo list for over a decade now.
So I'm fine with that part, even though I think the proposed
implementation is too complex (I wouldn't bother checking if there are
i2c buses on the system).

However maybe we could be even more user-friendly. Considering that you
must be root to run these tools, and these tools can't work without
i2c-dev, I think we should simply load the i2c-dev module automatically
as needed. Either with a simple system("modprobe i2c-dev") call, or
maybe using libkmod if it has a public API (not sure).

-- 
Jean Delvare
SUSE L3 Support

      parent reply	other threads:[~2023-06-19 14:37 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-26  0:24 [PATCH i2c-tools v2] i2cdetect: add messages for errors during bus listing Paul Wise
2023-06-14 21:31 ` Wolfram Sang
2023-06-15  1:51   ` Paul Wise
2023-06-19 14:37   ` Jean Delvare [this message]

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=20230619163703.209d3b18@endymion.delvare \
    --to=jdelvare@suse.de \
    --cc=linux-i2c@vger.kernel.org \
    --cc=pabs3@bonedaddy.net \
    --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