Linux-i2c Archive on lore.kernel.org
 help / color / Atom feed
From: Jean Delvare <jdelvare@suse.de>
To: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Peter Korsgaard <peter@korsgaard.com>,
	linux-i2c@vger.kernel.org, linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH i2c-tools] Revert "tools: i2ctransfer: add check for returned length from driver"
Date: Tue, 8 Jun 2021 14:22:11 +0200
Message-ID: <20210608142211.6aa9ad4f@endymion> (raw)
In-Reply-To: <YLp9Lc5Ondu3Gicg@kunai>

On Fri, 4 Jun 2021 21:21:17 +0200, Wolfram Sang wrote:
> > If we only want *one* I2C block read at a specified offset then i2cget
> > seems more appropriate indeed.  
> 
> I don't have a clear opinion here. On the one hand, if we want just one
> block read, this is more a "get" than a "dump". On the other hand,
> i2cdump already has a range-parameter. So, from a user perspective,
> keeping i2cget to single values and update the range parameter in
> i2cdump might be least confusing?

The feature doesn't clearly fit in either tool but could be made to fit
in both ;-) Which isn't a situation I like.

I think this is really a conceptual difference. For certain devices
(like EEPROMs) I2C Block read means reading from a range of register
addresses, which is i2cdump's realm. But for other devices, I2C Block
read means reading multiple register values from a single address
(which is typically what _SMBus_ Block reads are always about, but
nothing prevents devices from using I2C Block reads for the same
purpose). Which is more like i2cget's realm.

The problem is that, depending on which device our users are working
with, they will *expect* the feature to be in one tool or the other.
And most probably won't even have the idea of trying the other. Of
course we could add a redirection note in the manual page, but users
will have wasted time already. If they read the manual page at all. And
even then, the way the data is presented could be confusing if they
need to use the "other" tool.

As a matter of fact, I have at least two more records of people asking
for this very feature without realizing it was already (partially)
available in another tool (Aleksandr Amelkin in 2015 and Gina Ko in
2019). The fact that i2cset does support Block writes (since 2011!)
when i2cget does not support Block reads is definitely confusing.

So I'm actually tempted to add the feature to *both* tools. Crestez's
patch would be the base for the i2cget implementation, to which I would
happily add SMBus block read support. For i2cdump, it's about adding
support for register range to the "i" mode. I took a quick look already
and it seems fairly trivial to implement.

Stay tuned,
-- 
Jean Delvare
SUSE L3 Support

  reply index

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-09 11:05 Wolfram Sang
2021-02-26 16:43 ` Jean Delvare
2021-03-10 20:46   ` Wolfram Sang
2021-04-10  8:14     ` Peter Korsgaard
2021-04-13 12:54       ` Wolfram Sang
2021-04-21 20:01         ` Peter Korsgaard
2021-05-21 11:21         ` Jean Delvare
2021-05-25 20:36           ` Wolfram Sang
2021-06-02  9:03             ` Jean Delvare
2021-06-02 16:25               ` Wolfram Sang
2021-06-04 13:57                 ` Jean Delvare
2021-06-04 19:21                   ` Wolfram Sang
2021-06-08 12:22                     ` Jean Delvare [this message]
2021-06-08 12:26                       ` Wolfram Sang
2021-05-21 11:16       ` Jean Delvare
2021-03-07  5:42 ` Wolfram Sang

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=20210608142211.6aa9ad4f@endymion \
    --to=jdelvare@suse.de \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=peter@korsgaard.com \
    --cc=wsa+renesas@sang-engineering.com \
    /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

Linux-i2c Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-i2c/0 linux-i2c/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-i2c linux-i2c/ https://lore.kernel.org/linux-i2c \
		linux-i2c@vger.kernel.org
	public-inbox-index linux-i2c

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-i2c


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git