* [PATCH v3 0/3] Add the clock stretching i2c property
@ 2023-03-12 23:36 Andi Shyti
2023-03-12 23:36 ` [PATCH v3 1/3] dt-bindings: i2c: mpc: Mark "fsl,timeout" as deprecated Andi Shyti
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Andi Shyti @ 2023-03-12 23:36 UTC (permalink / raw)
To: linux-i2c, devicetree, linux-kernel
Cc: Wolfram Sang, Rob Herring, Krzysztof Kozlowski, Chris Packham,
Ryan Chen, Andi Shyti
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].
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
Changelog
=========
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.
- Added Chris r-b in patch 3.
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(-)
--
2.39.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 1/3] dt-bindings: i2c: mpc: Mark "fsl,timeout" as deprecated
2023-03-12 23:36 [PATCH v3 0/3] Add the clock stretching i2c property Andi Shyti
@ 2023-03-12 23:36 ` Andi Shyti
2023-03-14 14:10 ` Rob Herring
2023-03-14 14:21 ` Krzysztof Kozlowski
2023-03-12 23:36 ` [PATCH v3 2/3] i2c: mpc: Use of_property_read_u32 instead of of_get_property Andi Shyti
2023-03-12 23:36 ` [PATCH v3 3/3] i2c: mpc: Use i2c-scl-clk-low-timeout-ms i2c property Andi Shyti
2 siblings, 2 replies; 16+ messages in thread
From: Andi Shyti @ 2023-03-12 23:36 UTC (permalink / raw)
To: linux-i2c, devicetree, linux-kernel
Cc: Wolfram Sang, Rob Herring, Krzysztof Kozlowski, Chris Packham,
Ryan Chen, Andi Shyti
Now we have the i2c-scl-clk-low-timeout-ms property defined in
the i2c schema.
Mark "fsl,timeout" as deprecated and update the example.
Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
---
Documentation/devicetree/bindings/i2c/i2c-mpc.yaml | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mpc.yaml b/Documentation/devicetree/bindings/i2c/i2c-mpc.yaml
index 018e1b944424..bd2dd82471c1 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mpc.yaml
+++ b/Documentation/devicetree/bindings/i2c/i2c-mpc.yaml
@@ -43,6 +43,7 @@ properties:
fsl,timeout:
$ref: /schemas/types.yaml#/definitions/uint32
+ deprecated: true
description: |
I2C bus timeout in microseconds
@@ -95,6 +96,6 @@ examples:
interrupts = <43 2>;
interrupt-parent = <&mpic>;
clock-frequency = <400000>;
- fsl,timeout = <10000>;
+ i2c-scl-clk-low-timeout-ms = <10000>;
};
...
--
2.39.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 2/3] i2c: mpc: Use of_property_read_u32 instead of of_get_property
2023-03-12 23:36 [PATCH v3 0/3] Add the clock stretching i2c property Andi Shyti
2023-03-12 23:36 ` [PATCH v3 1/3] dt-bindings: i2c: mpc: Mark "fsl,timeout" as deprecated Andi Shyti
@ 2023-03-12 23:36 ` Andi Shyti
2023-03-13 1:53 ` Chris Packham
2023-03-12 23:36 ` [PATCH v3 3/3] i2c: mpc: Use i2c-scl-clk-low-timeout-ms i2c property Andi Shyti
2 siblings, 1 reply; 16+ messages in thread
From: Andi Shyti @ 2023-03-12 23:36 UTC (permalink / raw)
To: linux-i2c, devicetree, linux-kernel
Cc: Wolfram Sang, Rob Herring, Krzysztof Kozlowski, Chris Packham,
Ryan Chen, Andi Shyti
"of_property_read_u32()" is preferred to "of_get_property()" for
retreiving u32 from the device tree. Replace it.
Suggested-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
---
drivers/i2c/busses/i2c-mpc.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 81ac92bb4f6f..87e5c1725750 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -770,7 +770,6 @@ static const struct i2c_algorithm mpc_algo = {
static struct i2c_adapter mpc_ops = {
.owner = THIS_MODULE,
.algo = &mpc_algo,
- .timeout = HZ,
};
static struct i2c_bus_recovery_info fsl_i2c_recovery_info = {
@@ -781,11 +780,9 @@ static int fsl_i2c_probe(struct platform_device *op)
{
const struct mpc_i2c_data *data;
struct mpc_i2c *i2c;
- const u32 *prop;
- u32 clock = MPC_I2C_CLOCK_LEGACY;
- int result = 0;
- int plen;
struct clk *clk;
+ int result;
+ u32 clock;
int err;
i2c = devm_kzalloc(&op->dev, sizeof(*i2c), GFP_KERNEL);
@@ -831,10 +828,10 @@ static int fsl_i2c_probe(struct platform_device *op)
if (of_property_read_bool(op->dev.of_node, "fsl,preserve-clocking")) {
clock = MPC_I2C_CLOCK_PRESERVE;
} else {
- prop = of_get_property(op->dev.of_node, "clock-frequency",
- &plen);
- if (prop && plen == sizeof(u32))
- clock = *prop;
+ result = of_property_read_u32(op->dev.of_node,
+ "clock-frequency", &clock);
+ if (result)
+ clock = MPC_I2C_CLOCK_LEGACY;
}
data = device_get_match_data(&op->dev);
@@ -846,12 +843,16 @@ static int fsl_i2c_probe(struct platform_device *op)
mpc_i2c_setup_8xxx(op->dev.of_node, i2c, clock);
}
- prop = of_get_property(op->dev.of_node, "fsl,timeout", &plen);
- if (prop && plen == sizeof(u32)) {
- mpc_ops.timeout = *prop * HZ / 1000000;
+ 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)
mpc_ops.timeout = 5;
+ } else {
+ mpc_ops.timeout = HZ;
}
+
dev_info(i2c->dev, "timeout %u us\n", mpc_ops.timeout * 1000000 / HZ);
if (of_property_read_bool(op->dev.of_node, "fsl,i2c-erratum-a004447"))
--
2.39.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 3/3] i2c: mpc: Use i2c-scl-clk-low-timeout-ms i2c property
2023-03-12 23:36 [PATCH v3 0/3] Add the clock stretching i2c property Andi Shyti
2023-03-12 23:36 ` [PATCH v3 1/3] dt-bindings: i2c: mpc: Mark "fsl,timeout" as deprecated Andi Shyti
2023-03-12 23:36 ` [PATCH v3 2/3] i2c: mpc: Use of_property_read_u32 instead of of_get_property Andi Shyti
@ 2023-03-12 23:36 ` Andi Shyti
2023-03-13 1:53 ` Chris Packham
2023-03-14 14:22 ` Krzysztof Kozlowski
2 siblings, 2 replies; 16+ messages in thread
From: Andi Shyti @ 2023-03-12 23:36 UTC (permalink / raw)
To: linux-i2c, devicetree, linux-kernel
Cc: Wolfram Sang, Rob Herring, Krzysztof Kozlowski, Chris Packham,
Ryan Chen, Andi Shyti
"fsl,timeout" is marked as deprecated and replaced by the
"i2c-scl-clk-low-timeout-ms" 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>
---
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 87e5c1725750..28f11e30ac50 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-ms",
+ &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)
--
2.39.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/3] i2c: mpc: Use of_property_read_u32 instead of of_get_property
2023-03-12 23:36 ` [PATCH v3 2/3] i2c: mpc: Use of_property_read_u32 instead of of_get_property Andi Shyti
@ 2023-03-13 1:53 ` Chris Packham
0 siblings, 0 replies; 16+ messages in thread
From: Chris Packham @ 2023-03-13 1:53 UTC (permalink / raw)
To: Andi Shyti, linux-i2c, devicetree, linux-kernel
Cc: Wolfram Sang, Rob Herring, Krzysztof Kozlowski, Ryan Chen
On 13/03/23 12:36, Andi Shyti wrote:
> "of_property_read_u32()" is preferred to "of_get_property()" for
> retreiving u32 from the device tree. Replace it.
>
> Suggested-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Gave the patches a spin on a P2041RDB so
Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> drivers/i2c/busses/i2c-mpc.c | 25 +++++++++++++------------
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> index 81ac92bb4f6f..87e5c1725750 100644
> --- a/drivers/i2c/busses/i2c-mpc.c
> +++ b/drivers/i2c/busses/i2c-mpc.c
> @@ -770,7 +770,6 @@ static const struct i2c_algorithm mpc_algo = {
> static struct i2c_adapter mpc_ops = {
> .owner = THIS_MODULE,
> .algo = &mpc_algo,
> - .timeout = HZ,
> };
>
> static struct i2c_bus_recovery_info fsl_i2c_recovery_info = {
> @@ -781,11 +780,9 @@ static int fsl_i2c_probe(struct platform_device *op)
> {
> const struct mpc_i2c_data *data;
> struct mpc_i2c *i2c;
> - const u32 *prop;
> - u32 clock = MPC_I2C_CLOCK_LEGACY;
> - int result = 0;
> - int plen;
> struct clk *clk;
> + int result;
> + u32 clock;
> int err;
>
> i2c = devm_kzalloc(&op->dev, sizeof(*i2c), GFP_KERNEL);
> @@ -831,10 +828,10 @@ static int fsl_i2c_probe(struct platform_device *op)
> if (of_property_read_bool(op->dev.of_node, "fsl,preserve-clocking")) {
> clock = MPC_I2C_CLOCK_PRESERVE;
> } else {
> - prop = of_get_property(op->dev.of_node, "clock-frequency",
> - &plen);
> - if (prop && plen == sizeof(u32))
> - clock = *prop;
> + result = of_property_read_u32(op->dev.of_node,
> + "clock-frequency", &clock);
> + if (result)
> + clock = MPC_I2C_CLOCK_LEGACY;
> }
>
> data = device_get_match_data(&op->dev);
> @@ -846,12 +843,16 @@ static int fsl_i2c_probe(struct platform_device *op)
> mpc_i2c_setup_8xxx(op->dev.of_node, i2c, clock);
> }
>
> - prop = of_get_property(op->dev.of_node, "fsl,timeout", &plen);
> - if (prop && plen == sizeof(u32)) {
> - mpc_ops.timeout = *prop * HZ / 1000000;
> + 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)
> mpc_ops.timeout = 5;
> + } else {
> + mpc_ops.timeout = HZ;
> }
> +
> dev_info(i2c->dev, "timeout %u us\n", mpc_ops.timeout * 1000000 / HZ);
>
> if (of_property_read_bool(op->dev.of_node, "fsl,i2c-erratum-a004447"))
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/3] i2c: mpc: Use i2c-scl-clk-low-timeout-ms i2c property
2023-03-12 23:36 ` [PATCH v3 3/3] i2c: mpc: Use i2c-scl-clk-low-timeout-ms i2c property Andi Shyti
@ 2023-03-13 1:53 ` Chris Packham
2023-03-13 8:58 ` Andi Shyti
2023-03-14 14:22 ` Krzysztof Kozlowski
1 sibling, 1 reply; 16+ messages in thread
From: Chris Packham @ 2023-03-13 1:53 UTC (permalink / raw)
To: Andi Shyti, linux-i2c, devicetree, linux-kernel
Cc: Wolfram Sang, Rob Herring, Krzysztof Kozlowski, Ryan Chen
On 13/03/23 12:36, Andi Shyti wrote:
> "fsl,timeout" is marked as deprecated and replaced by the
> "i2c-scl-clk-low-timeout-ms" 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>
Gave the patches a spin on a P2041RDB so
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 87e5c1725750..28f11e30ac50 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-ms",
> + &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)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/3] i2c: mpc: Use i2c-scl-clk-low-timeout-ms i2c property
2023-03-13 1:53 ` Chris Packham
@ 2023-03-13 8:58 ` Andi Shyti
0 siblings, 0 replies; 16+ messages in thread
From: Andi Shyti @ 2023-03-13 8:58 UTC (permalink / raw)
To: Chris Packham
Cc: Andi Shyti, linux-i2c, devicetree, linux-kernel, Wolfram Sang,
Rob Herring, Krzysztof Kozlowski, Ryan Chen
> On 13/03/23 12:36, Andi Shyti wrote:
> > "fsl,timeout" is marked as deprecated and replaced by the
> > "i2c-scl-clk-low-timeout-ms" 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>
> Gave the patches a spin on a P2041RDB so
>
> Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Thanks, Chris!
Andi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: i2c: mpc: Mark "fsl,timeout" as deprecated
2023-03-12 23:36 ` [PATCH v3 1/3] dt-bindings: i2c: mpc: Mark "fsl,timeout" as deprecated Andi Shyti
@ 2023-03-14 14:10 ` Rob Herring
2023-03-14 14:19 ` Krzysztof Kozlowski
2023-03-14 14:21 ` Krzysztof Kozlowski
1 sibling, 1 reply; 16+ messages in thread
From: Rob Herring @ 2023-03-14 14:10 UTC (permalink / raw)
To: Andi Shyti
Cc: linux-i2c, Ryan Chen, Wolfram Sang, Krzysztof Kozlowski,
Chris Packham, linux-kernel, devicetree, Rob Herring
On Mon, 13 Mar 2023 00:36:11 +0100, Andi Shyti wrote:
> Now we have the i2c-scl-clk-low-timeout-ms property defined in
> the i2c schema.
>
> Mark "fsl,timeout" as deprecated and update the example.
>
> Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
> ---
> Documentation/devicetree/bindings/i2c/i2c-mpc.yaml | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/i2c-mpc.example.dtb: i2c@3100: Unevaluated properties are not allowed ('i2c-scl-clk-low-timeout-ms' was unexpected)
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/i2c-mpc.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230312233613.303408-2-andi.shyti@kernel.org
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: i2c: mpc: Mark "fsl,timeout" as deprecated
2023-03-14 14:10 ` Rob Herring
@ 2023-03-14 14:19 ` Krzysztof Kozlowski
0 siblings, 0 replies; 16+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-14 14:19 UTC (permalink / raw)
To: Rob Herring, Andi Shyti
Cc: linux-i2c, Ryan Chen, Wolfram Sang, Krzysztof Kozlowski,
Chris Packham, linux-kernel, devicetree, Rob Herring
On 14/03/2023 15:10, Rob Herring wrote:
>
> On Mon, 13 Mar 2023 00:36:11 +0100, Andi Shyti wrote:
>> Now we have the i2c-scl-clk-low-timeout-ms property defined in
>> the i2c schema.
>>
>> Mark "fsl,timeout" as deprecated and update the example.
>>
>> Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
>> ---
>> Documentation/devicetree/bindings/i2c/i2c-mpc.yaml | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/i2c-mpc.example.dtb: i2c@3100: Unevaluated properties are not allowed ('i2c-scl-clk-low-timeout-ms' was unexpected)
That's expected as it depends on:
https://github.com/devicetree-org/dt-schema/pull/102
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: i2c: mpc: Mark "fsl,timeout" as deprecated
2023-03-12 23:36 ` [PATCH v3 1/3] dt-bindings: i2c: mpc: Mark "fsl,timeout" as deprecated Andi Shyti
2023-03-14 14:10 ` Rob Herring
@ 2023-03-14 14:21 ` Krzysztof Kozlowski
2023-03-14 14:22 ` Krzysztof Kozlowski
1 sibling, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-14 14:21 UTC (permalink / raw)
To: Andi Shyti, linux-i2c, devicetree, linux-kernel
Cc: Wolfram Sang, Rob Herring, Krzysztof Kozlowski, Chris Packham, Ryan Chen
On 13/03/2023 00:36, Andi Shyti wrote:
> Now we have the i2c-scl-clk-low-timeout-ms property defined in
> the i2c schema.
>
> Mark "fsl,timeout" as deprecated and update the example.
>
> Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
> ---
> Documentation/devicetree/bindings/i2c/i2c-mpc.yaml | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: i2c: mpc: Mark "fsl,timeout" as deprecated
2023-03-14 14:21 ` Krzysztof Kozlowski
@ 2023-03-14 14:22 ` Krzysztof Kozlowski
2023-03-14 14:36 ` Andi Shyti
0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-14 14:22 UTC (permalink / raw)
To: Andi Shyti, linux-i2c, devicetree, linux-kernel
Cc: Wolfram Sang, Rob Herring, Krzysztof Kozlowski, Chris Packham, Ryan Chen
On 14/03/2023 15:21, Krzysztof Kozlowski wrote:
> On 13/03/2023 00:36, Andi Shyti wrote:
>> Now we have the i2c-scl-clk-low-timeout-ms property defined in
>> the i2c schema.
>>
>> Mark "fsl,timeout" as deprecated and update the example.
>>
>> Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
>> ---
>> Documentation/devicetree/bindings/i2c/i2c-mpc.yaml | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
...
and unreviewed. Are you sure these are using same units? Old code used us.
Your example:
i2c-scl-clk-low-timeout-ms = <10000>;
10s timeout?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/3] i2c: mpc: Use i2c-scl-clk-low-timeout-ms i2c property
2023-03-12 23:36 ` [PATCH v3 3/3] i2c: mpc: Use i2c-scl-clk-low-timeout-ms i2c property Andi Shyti
2023-03-13 1:53 ` Chris Packham
@ 2023-03-14 14:22 ` Krzysztof Kozlowski
2023-03-14 15:16 ` Andi Shyti
1 sibling, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-14 14:22 UTC (permalink / raw)
To: Andi Shyti, linux-i2c, devicetree, linux-kernel
Cc: Wolfram Sang, Rob Herring, Krzysztof Kozlowski, Chris Packham, Ryan Chen
On 13/03/2023 00:36, Andi Shyti wrote:
> "fsl,timeout" is marked as deprecated and replaced by the
> "i2c-scl-clk-low-timeout-ms" 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>
> ---
> 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 87e5c1725750..28f11e30ac50 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-ms",
> + &mpc_ops.timeout);
> + if (result == -EINVAL)
> + result = of_property_read_u32(op->dev.of_node,
> + "fsl,timeout", &mpc_ops.timeout);
Wasn't old property in us and new one is in ms?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: i2c: mpc: Mark "fsl,timeout" as deprecated
2023-03-14 14:22 ` Krzysztof Kozlowski
@ 2023-03-14 14:36 ` Andi Shyti
0 siblings, 0 replies; 16+ messages in thread
From: Andi Shyti @ 2023-03-14 14:36 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andi Shyti, linux-i2c, devicetree, linux-kernel, Wolfram Sang,
Rob Herring, Krzysztof Kozlowski, Chris Packham, Ryan Chen
On Tue, Mar 14, 2023 at 03:22:19PM +0100, Krzysztof Kozlowski wrote:
> On 14/03/2023 15:21, Krzysztof Kozlowski wrote:
> > On 13/03/2023 00:36, Andi Shyti wrote:
> >> Now we have the i2c-scl-clk-low-timeout-ms property defined in
> >> the i2c schema.
> >>
> >> Mark "fsl,timeout" as deprecated and update the example.
> >>
> >> Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
> >> ---
> >> Documentation/devicetree/bindings/i2c/i2c-mpc.yaml | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> ...
>
> and unreviewed. Are you sure these are using same units? Old code used us.
>
> Your example:
> i2c-scl-clk-low-timeout-ms = <10000>;
>
> 10s timeout?
ops!
I will keep it ms, if anyone doesn't have anything against. The
clock stretching goes in order of milliseconds and in any case
doesn't need to be precisely that, as long as it's longer.
I don't see any need for it to be us... if it 12500us, let it be
13ms.
I will update the example.
Can I keep your r-b with:
i2c-scl-clk-low-timeout-ms = <10>;
?
Thanks for noticing it, it would have been a wacky mistake,
Andi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/3] i2c: mpc: Use i2c-scl-clk-low-timeout-ms i2c property
2023-03-14 14:22 ` Krzysztof Kozlowski
@ 2023-03-14 15:16 ` Andi Shyti
2023-03-14 19:47 ` Chris Packham
0 siblings, 1 reply; 16+ messages in thread
From: Andi Shyti @ 2023-03-14 15:16 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andi Shyti, linux-i2c, devicetree, linux-kernel, Wolfram Sang,
Rob Herring, Krzysztof Kozlowski, Chris Packham, Ryan Chen
Hi,
On Tue, Mar 14, 2023 at 03:22:52PM +0100, Krzysztof Kozlowski wrote:
> On 13/03/2023 00:36, Andi Shyti wrote:
> > "fsl,timeout" is marked as deprecated and replaced by the
> > "i2c-scl-clk-low-timeout-ms" 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>
> > ---
> > 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 87e5c1725750..28f11e30ac50 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-ms",
> > + &mpc_ops.timeout);
> > + if (result == -EINVAL)
> > + result = of_property_read_u32(op->dev.of_node,
> > + "fsl,timeout", &mpc_ops.timeout);
>
> Wasn't old property in us and new one is in ms?
Thanks, Krzysztof! Good catch!
Chris, you are the only user of this property, as of now. Is it
OK if we keep it ms? I will send a proper patch to do the
conversion.
To me it doesn't make much sense to have the timeout defined in
us as that's of the same order of the raising and falling time
of the clock. Any opinion?
Andi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/3] i2c: mpc: Use i2c-scl-clk-low-timeout-ms i2c property
2023-03-14 15:16 ` Andi Shyti
@ 2023-03-14 19:47 ` Chris Packham
2023-03-14 20:48 ` Andi Shyti
0 siblings, 1 reply; 16+ messages in thread
From: Chris Packham @ 2023-03-14 19:47 UTC (permalink / raw)
To: Andi Shyti, Krzysztof Kozlowski
Cc: linux-i2c, devicetree, linux-kernel, Wolfram Sang, Rob Herring,
Krzysztof Kozlowski, Ryan Chen
Hi Andi,
On 15/03/23 04:16, Andi Shyti wrote:
> Hi,
>
> On Tue, Mar 14, 2023 at 03:22:52PM +0100, Krzysztof Kozlowski wrote:
>> On 13/03/2023 00:36, Andi Shyti wrote:
>>> "fsl,timeout" is marked as deprecated and replaced by the
>>> "i2c-scl-clk-low-timeout-ms" 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>
>>> ---
>>> 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 87e5c1725750..28f11e30ac50 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-ms",
>>> + &mpc_ops.timeout);
>>> + if (result == -EINVAL)
>>> + result = of_property_read_u32(op->dev.of_node,
>>> + "fsl,timeout", &mpc_ops.timeout);
>> Wasn't old property in us and new one is in ms?
> Thanks, Krzysztof! Good catch!
>
> Chris, you are the only user of this property, as of now. Is it
> OK if we keep it ms? I will send a proper patch to do the
> conversion.
>
> To me it doesn't make much sense to have the timeout defined in
> us as that's of the same order of the raising and falling time
> of the clock. Any opinion?
I think it'd be easier to stick to us as then the same code can be used
to probe both the old property and the new one. However I won't object
if you adjust for the us to ms conversion between handling the new
property vs the old one.
>
> Andi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/3] i2c: mpc: Use i2c-scl-clk-low-timeout-ms i2c property
2023-03-14 19:47 ` Chris Packham
@ 2023-03-14 20:48 ` Andi Shyti
0 siblings, 0 replies; 16+ messages in thread
From: Andi Shyti @ 2023-03-14 20:48 UTC (permalink / raw)
To: Chris Packham
Cc: Andi Shyti, Krzysztof Kozlowski, linux-i2c, devicetree,
linux-kernel, Wolfram Sang, Rob Herring, Krzysztof Kozlowski,
Ryan Chen
Hi Chris,
> >>> "fsl,timeout" is marked as deprecated and replaced by the
> >>> "i2c-scl-clk-low-timeout-ms" 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>
> >>> ---
> >>> 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 87e5c1725750..28f11e30ac50 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-ms",
> >>> + &mpc_ops.timeout);
> >>> + if (result == -EINVAL)
> >>> + result = of_property_read_u32(op->dev.of_node,
> >>> + "fsl,timeout", &mpc_ops.timeout);
> >> Wasn't old property in us and new one is in ms?
> > Thanks, Krzysztof! Good catch!
> >
> > Chris, you are the only user of this property, as of now. Is it
> > OK if we keep it ms? I will send a proper patch to do the
> > conversion.
> >
> > To me it doesn't make much sense to have the timeout defined in
> > us as that's of the same order of the raising and falling time
> > of the clock. Any opinion?
> I think it'd be easier to stick to us as then the same code can be used
> to probe both the old property and the new one. However I won't object
> if you adjust for the us to ms conversion between handling the new
> property vs the old one.
yeah... indeed it became quite ugly.
OK, I will just rename it from "i2c-scl-clk-low-timeout-ms" to
"i2c-scl-clk-low-timeout-us", respin the schema pull request and
leave everything as it is.
Thanks for your input, Chris!
Andi
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-03-14 20:48 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-12 23:36 [PATCH v3 0/3] Add the clock stretching i2c property Andi Shyti
2023-03-12 23:36 ` [PATCH v3 1/3] dt-bindings: i2c: mpc: Mark "fsl,timeout" as deprecated Andi Shyti
2023-03-14 14:10 ` Rob Herring
2023-03-14 14:19 ` Krzysztof Kozlowski
2023-03-14 14:21 ` Krzysztof Kozlowski
2023-03-14 14:22 ` Krzysztof Kozlowski
2023-03-14 14:36 ` Andi Shyti
2023-03-12 23:36 ` [PATCH v3 2/3] i2c: mpc: Use of_property_read_u32 instead of of_get_property Andi Shyti
2023-03-13 1:53 ` Chris Packham
2023-03-12 23:36 ` [PATCH v3 3/3] i2c: mpc: Use i2c-scl-clk-low-timeout-ms i2c property Andi Shyti
2023-03-13 1:53 ` Chris Packham
2023-03-13 8:58 ` Andi Shyti
2023-03-14 14:22 ` Krzysztof Kozlowski
2023-03-14 15:16 ` Andi Shyti
2023-03-14 19:47 ` Chris Packham
2023-03-14 20:48 ` Andi Shyti
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).