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: Fri, 4 Jun 2021 15:57:08 +0200
Message-ID: <20210604155708.14159db0@endymion> (raw)
In-Reply-To: <YLew8cFWTRQKrBuk@kunai>

On Wed, 2 Jun 2021 18:25:21 +0200, Wolfram Sang wrote:
> Well, I thought mainly about this patch "tools: i2cbusses: Handle bus
> names like /dev/i2c-0" but you took care of it faster than I did.
> 
> There is only one patch left in patchwork from 2016:
> 
> http://patchwork.ozlabs.org/project/linux-i2c/patch/044b3af6a47dfa92e047f0ff57e39a5b61e00058.1463165295.git.leonard.crestez@intel.com/
> "[i2c-tools] i2cget: Add support for i2c block data"
> 
> That's all what's left in patchwork.
> 
> Dunno if you care about the old patch, but I think we are good to go for
> a release.

Yes I do care. Apparently I wasn't Cc'd on that thread so I wasn't even
aware that patch existed. It would be very nice if you could bump the
thread to me (unless there's a way to retrieve it from patchwork that I
didn't find?)

I read the thread, my initial answer would have matched Guenter's (as
always...) but the contributor has a point, the range option is not
supported by i2cdump in I2C block mode. Whether it's better to add
support for it there, or in i2cget, I'll see.

I think i2cget has my preference, because i2cdump pretty much assumes
that I2C block reads retrieve successive 8-bit register values, so you
can have block boundaries anywhere without changing the result. As a
matter of fact, while i2cdump attempts to make 32-byte I2C block reads,
it will transparently handle short reads by issuing additional block
reads at arbitrary offsets. This works nicely for EEPROMs but could
fail for other devices.

If we only want *one* I2C block read at a specified offset then i2cget
seems more appropriate indeed.

Looking at the code, I see that i2cdump already supports SMBus block
mode, in a way which is very similar to what the contributor asked for,
i.e. it doesn't really dump the chip's registers, but merely reads one
SMBus block at a specific offset. It's questionable why this should
belong to i2cdump in the first place, beyond the fact that it returns
several values when i2cget typically returns only one value.

So one option would be get rid of "s" mode in i2cdump, and add support
for both I2C and SMBus block read to i2cget.

Might be possible to add support for range limitation to I2C block
reads in i2cdump nevertheless, as it could be useful to read only
portions of EEPROMs.

-- 
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 [this message]
2021-06-04 19:21                   ` Wolfram Sang
2021-06-08 12:22                     ` Jean Delvare
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=20210604155708.14159db0@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