linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Wolfram Sang <wsa@kernel.org>
To: Daniel Stodden <daniel.stodden@gmail.com>
Cc: Jean Delvare <jdelvare@suse.de>, linux-i2c@vger.kernel.org
Subject: Re: [RFC PATCH] i2c: Support Smbus 3.0 block sizes up to 255 bytes.
Date: Wed, 29 Jul 2020 12:51:57 +0200	[thread overview]
Message-ID: <20200729105156.GA1015@ninjato> (raw)
In-Reply-To: <5E3EA2AC-83B1-4B7B-87DB-DBC4FAB2B7D0@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2882 bytes --]

Hi Daniel, hi Jean,

some more bits from me.

> Cool. Meaning we may then leave it at a single i2c_smbus_data.block[257]
> declaration in the headers, and i2c_smbus_data can be 255 bytes.

I think we all agree on this one.

> That doesn’t mean we shut the door on 32-byte buffers. After all, backward
> compat requires we maintain support for those. Just that we need not
> bother declaring or promoting dedicated small vs large transfer types
> while noone wants one.

That's how I see it as well.

> This would also mean we’re likely to add an I2C_FUNC_ to indicated smbus3
> support at runtime. It’s probably what you already have in mind.

I suggest I2C_FUNC_SMBUS3_BLOCKSIZE. I have seen the datasheet of one
client device where you could set a bit to select how many bytes should
be sent by the device in a block read (32 or 255). So, you could set
this bit according to the presence of the above FUNC. Also, I suggest to
do it finegrained, because SMBus3 also specifies 32- and 64-bit
transfers. If those are ever implemented, they should also get seperate
FUNC bits. All of them can then be ORed into I2C_FUNC_SMBUS3 if needed.

> We’d probably still prefer to move all kernel/driver buffers to 255 bytes
> unconditionally. However, I2C_FUNC_ presence normally indicates lower level
> driver + hardware support, right? Not just a kernel upgrade. Which makes 
> sense here, too.

Yes. I am all for using 257 byte buffers in-kernel only. a) defensive
programming b) for most I2C controllers only emulating SMBus, it will be
super easy to support the new block size. I expect it to become it kinda
default soon (for bus master drivers, that is).

> I’m just pointing it out because if we want an I2C_FUNC_SMBUS3, and at
> the driver’s discretion, not just indicating kernel + compatibility
> support presence, and if i2c_utils uses that to pick safe size values,
> that again means we’re yet again less likely to actually deprecate the
> old transfer numbers. Some drivers will never be SMBUS3.

Yes, they will stay around. When I said "deprecated", I didn't mean it
in the way of "will be removed" but more in "please use the new ones".

> I2C_FUNC_SMBUS3 is nice to have. I’m not against it, I’m for it.
> For i2c-tools and all clients, it can be valuable to confirm conflicts.
> 
> Still, one can keep it indicative only, and not synthesize faults 
> because of a perceived client/adapter/device feature mismatch. 

That's always been the case for I2C_FUNC_*. It helps people not to shoot
in their feet, but when ignored things happen.

> I’m not even sure if my new -EMSGSIZE condition in i2c-dev.c is such
> a good idea.

I'd need to see this in a new patch to comment on it. For me, it is
really easier to talk over code. But we are not right there yet, or?

Kind regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2020-07-29 10:52 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-28  0:47 [RFC PATCH] i2c: Support Smbus 3.0 block sizes up to 255 bytes daniel.stodden
2020-07-28  9:40 ` Wolfram Sang
2020-07-28 10:18   ` Daniel Stodden
2020-07-28 10:36     ` Wolfram Sang
2020-07-28 11:27       ` Daniel Stodden
2020-07-28 17:04   ` Jean Delvare
2020-07-28 21:16     ` Daniel Stodden
2020-07-29 10:51       ` Wolfram Sang [this message]
2020-07-28 11:16 ` Wolfram Sang
2020-07-28 11:40   ` Daniel Stodden
2020-07-28 12:46     ` Daniel Stodden

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=20200729105156.GA1015@ninjato \
    --to=wsa@kernel.org \
    --cc=daniel.stodden@gmail.com \
    --cc=jdelvare@suse.de \
    --cc=linux-i2c@vger.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).