| Message ID | 20230312233613.303408-4-andi.shyti@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add the clock stretching i2c property | expand |
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)
> 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
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
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
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
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
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)