From: Wolfram Sang <wsa@kernel.org>
To: linux-i2c@vger.kernel.org, devicetree-spec@vger.kernel.org
Cc: Andi Shyti <andi.shyti@kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Chris Packham <chris.packham@alliedtelesis.co.nz>
Subject: dtschema: i2c: messy situation about timeouts
Date: Mon, 26 Feb 2024 11:08:27 +0100 [thread overview]
Message-ID: <ZdxjGwvGXlDGkYs0@shikoro> (raw)
[-- Attachment #1: Type: text/plain, Size: 1973 bytes --]
Hey guys,
we have quite a messy situation regarding I2C timeouts in the dtschema.
Partly because I was too busy to pay detailed attention, partly because
reviewing dtschema changes happen on Github which I totally missed. No
complaining, though, here are my observations and suggestions to get it
straight. Comments are more than welcome.
- "i2c-transfer-timeout-us"
Description says "Number of microseconds to wait before considering an
I2C transfer has failed."
To me, this binding is very descriptive and makes sense. We should keep
it. Sadly, it is the newest one and we already have others.
- "i2c-scl-has-clk-low-timeout"
AFAIU this binding tells that the controller can do clock stretching.
But what for? I don't see why this is important for clients. If
anything, then it would be interesting if the *client* can do clock
stretching and if the controller can actually handle that. But no need
to describe it in DT, we have this as an adapter quirk already
'I2C_AQ_NO_CLK_STRETCH'. Two controllers use it, but no client checks
for it so far. Coming back to this binding, it is also unused in the
kernel.
Suggestion: let's remove it
- "i2c-scl-clk-low-timeout-us"
The description says "Number of microseconds the clock line needs to be
pulled down in order to force a waiting state." What does "forcing a
waiting state" mean here? I don't understand this description.
It is used in the i2c-mpc driver. The use case is simply to put it into
the 'struct i2c_adapter.timeout' member. That timeout is used to
determine if a transfer failed. So, to me, "i2c-transfer-timeout-us"
makes a lot more sense to use here.
Suggestion: let's remove this binding and conver i2c-mpc to
"i2c-transfer-timeout-us". Yes, not nice to have two deprecated
bindings, but things happened.
So, these are my thoughts about the current situation. I might have
missed something, so if you have anything to add, I am all ears.
Comments really welcome!
Happy hacking,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next reply other threads:[~2024-02-26 10:08 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-26 10:08 Wolfram Sang [this message]
2024-02-26 14:45 ` dtschema: i2c: messy situation about timeouts Rob Herring
2024-02-26 21:16 ` Wolfram Sang
2024-02-26 20:20 ` Chris Packham
2024-02-26 21:24 ` Wolfram Sang
2024-02-27 0:03 ` Andi Shyti
2024-02-27 7:12 ` Wolfram Sang
2024-02-27 12:57 ` 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=ZdxjGwvGXlDGkYs0@shikoro \
--to=wsa@kernel.org \
--cc=andi.shyti@kernel.org \
--cc=chris.packham@alliedtelesis.co.nz \
--cc=devicetree-spec@vger.kernel.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-i2c@vger.kernel.org \
--cc=robh+dt@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 read-only IMAP folder(s) and NNTP newsgroup(s).