Re: [PATCH i2c-tools v2] i2cdetect: add messages for errors during bus listing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux