mbox series

[v4,0/3] Add the clock stretching i2c property

Message ID 20230314215612.23741-1-andi.shyti@kernel.org
Headers show
Series Add the clock stretching i2c property | expand

Message

Andi Shyti March 14, 2023, 9:56 p.m. UTC
Hello,

fter a discussion between Krzysztof and Ryan[1], it has become
apparent that the i2c binding is lacking the definition of a
property that needs to be added at a more generic level. This
property is also used by the mpc i2c controller, which has been
updated in the second patch.

The DTS schema change has been sent as github pull reqest[2][3].

Thanks Krzysztof and Chris for the reviews.

Thank you,
Andi

[1] https://lore.kernel.org/all/c41ee6b5-ddb4-1253-de54-a295b3bab2cc@linaro.org/
[2] https://github.com/devicetree-org/dt-schema/pull/102
[3] https://github.com/devicetree-org/dt-schema/pull/103

Changelog
=========
v3 -> v4:
 - Replaced "i2c-scl-clk-low-timeout-ms" with
   "i2c-scl-clk-low-timeout-us". Use microseconds instead of
   milliseconds.
 - Add tags from Chris.

v2 -> v3:
 - Chris recommended to use of_property_read_u32() instead of
   of_get_property(). Because there were two use of it I added
   the suggested cleanup in a separate patch.

v1 -> v2:
 - Removed the binding patch and send through a different channel
 - To ensure back compatibility, which was broken in v1, the
   legacy "fsl,timeout" has not been removed and marked as
   deprecated. In the driver the that property is checked anyway
   as a fallback in case the main i2c-scl-clk-low-timeout-ms is
   missing.

Andi Shyti (3):
  dt-bindings: i2c: mpc: Mark "fsl,timeout" as deprecated
  i2c: mpc: Use of_property_read_u32 instead of of_get_property
  i2c: mpc: Use i2c-scl-clk-low-timeout-ms i2c property

 .../devicetree/bindings/i2c/i2c-mpc.yaml      |  3 +-
 drivers/i2c/busses/i2c-mpc.c                  | 35 ++++++++++++-------
 2 files changed, 25 insertions(+), 13 deletions(-)

Comments

Andi Shyti March 14, 2023, 10:07 p.m. UTC | #1
> Just to be a pain the subject line still says "ms"

I have changed hundreds of /ms/us/ in this last half an hour...
of course I forgot some :)

There is also one in the comment.

Thanks for spotting it, Chris!

Andi

> On 15/03/23 10:56, Andi Shyti wrote:
> > "fsl,timeout" is marked as deprecated and replaced by the
> > "i2c-scl-clk-low-timeout-us" i2c property.
> >
> > Use this latter and, in case it is missing, for back
> > compatibility, check whether we still have "fsl,timeout" defined.
> >
> > Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
> > Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> > Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> > ---
> >   drivers/i2c/busses/i2c-mpc.c | 12 +++++++++++-
> >   1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> > index 87e5c1725750f..e8798f9c23fcc 100644
> > --- a/drivers/i2c/busses/i2c-mpc.c
> > +++ b/drivers/i2c/busses/i2c-mpc.c
> > @@ -843,8 +843,18 @@ static int fsl_i2c_probe(struct platform_device *op)
> >   			mpc_i2c_setup_8xxx(op->dev.of_node, i2c, clock);
> >   	}
> >   
> > +	/*
> > +	 * "fsl,timeout" has been marked as deprecated and, to maintain
> > +	 * backward compatibility, we will only look for it if
> > +	 * "i2c-scl-clk-low-timeout-ms" is not present.
> > +	 */
> >   	result = of_property_read_u32(op->dev.of_node,
> > -				      "fsl,timeout", &mpc_ops.timeout);
> > +				      "i2c-scl-clk-low-timeout-us",
> > +				      &mpc_ops.timeout);
> > +	if (result == -EINVAL)
> > +		result = of_property_read_u32(op->dev.of_node,
> > +					      "fsl,timeout", &mpc_ops.timeout);
> > +
> >   	if (!result) {
> >   		mpc_ops.timeout *= HZ / 1000000;
> >   		if (mpc_ops.timeout < 5)