Re: dtschema: i2c: messy situation about timeouts

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

 



On Mon, Feb 26, 2024 at 4:08 AM Wolfram Sang <wsa@xxxxxxxxxx> wrote:
>
> 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

Seems like it can be implied by compatible... I'll defer to Andi.

>
>
> - "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.

Does the commit msg or PR help?:
https://github.com/devicetree-org/dt-schema/pull/103

>
> 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.

Maybe the core code should read it instead?

I think we should mark as deprecated rather than remove unless we can
just remove the properties from the kernel. The reason being that
every property the kernel accesses should be documented. I might start
actually checking that. That's already done for compatible strings,
but those are mostly defined by a common pattern we can extract while
properties are less so.

Rob





[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