[RESEND] i2c: mediatek: Get device clock-stretch time via dts

[RESEND] i2c: mediatek: Get device clock-stretch time via dts

[qii.wang]

From: Qii Wang <qii.wang@mediatek.com>

tSU,STA/tHD,STA/tSU,STOP maybe out of spec due to device
clock-stretching or circuit loss, we could get device
clock-stretch time from dts to adjust these parameters
to meet the spec via EXT_CONF register.

Signed-off-by: Qii Wang <qii.wang@mediatek.com>
---
 drivers/i2c/busses/i2c-mt65xx.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
index 2ffd2f3..47c7255 100644
--- a/drivers/i2c/busses/i2c-mt65xx.c
+++ b/drivers/i2c/busses/i2c-mt65xx.c
@@ -245,6 +245,7 @@ struct mtk_i2c {
 	u16 irq_stat;			/* interrupt status */
 	unsigned int clk_src_div;
 	unsigned int speed_hz;		/* The speed in transfer */
+	unsigned int clock_stretch_ns;
 	enum mtk_trans_op op;
 	u16 timing_reg;
 	u16 high_speed_reg;
@@ -607,7 +608,8 @@ static int mtk_i2c_check_ac_timing(struct mtk_i2c *i2c,
 	else
 		clk_ns = sample_ns / 2;
 
-	su_sta_cnt = DIV_ROUND_UP(spec->min_su_sta_ns, clk_ns);
+	su_sta_cnt = DIV_ROUND_UP(spec->min_su_sta_ns + i2c->clock_stretch_ns,
+				  clk_ns);
 	if (su_sta_cnt > max_sta_cnt)
 		return -1;
 
@@ -1171,6 +1173,8 @@ static int mtk_i2c_parse_dt(struct device_node *np, struct mtk_i2c *i2c)
 	if (i2c->clk_src_div == 0)
 		return -EINVAL;
 
+	of_property_read_u32(np, "clock-stretch-ns", &i2c->clock_stretch_ns);
+
 	i2c->have_pmic = of_property_read_bool(np, "mediatek,have-pmic");
 	i2c->use_push_pull =
 		of_property_read_bool(np, "mediatek,use-push-pull");
-- 
1.9.1

Re: [RESEND] i2c: mediatek: Get device clock-stretch time via dts

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 377 bytes --]


> tSU,STA/tHD,STA/tSU,STOP maybe out of spec due to device
> clock-stretching or circuit loss, we could get device
> clock-stretch time from dts to adjust these parameters
> to meet the spec via EXT_CONF register.
> 
> Signed-off-by: Qii Wang <qii.wang@mediatek.com>

I will look at this next and think about it. New bindings are always a
bit more time consuming.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RESEND] i2c: mediatek: Get device clock-stretch time via dts

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 626 bytes --]

On Sat, Mar 13, 2021 at 04:04:24PM +0800, qii.wang@mediatek.com wrote:
> From: Qii Wang <qii.wang@mediatek.com>
> 
> tSU,STA/tHD,STA/tSU,STOP maybe out of spec due to device
> clock-stretching or circuit loss, we could get device
> clock-stretch time from dts to adjust these parameters
> to meet the spec via EXT_CONF register.
> 
> Signed-off-by: Qii Wang <qii.wang@mediatek.com>

I tried to understand from the code what the new binding expresses, but
I don't fully understand it. Is it the maximum clock stretch time?
Because I cannot recall a device which always uses the same delay for
clock stretching.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RESEND] i2c: mediatek: Get device clock-stretch time via dts

[Qii Wang]

On Tue, 2021-04-06 at 21:48 +0200, Wolfram Sang wrote:
> On Sat, Mar 13, 2021 at 04:04:24PM +0800, qii.wang@mediatek.com wrote:
> > From: Qii Wang <qii.wang@mediatek.com>
> > 
> > tSU,STA/tHD,STA/tSU,STOP maybe out of spec due to device
> > clock-stretching or circuit loss, we could get device
> > clock-stretch time from dts to adjust these parameters
> > to meet the spec via EXT_CONF register.
> > 
> > Signed-off-by: Qii Wang <qii.wang@mediatek.com>
> 
> I tried to understand from the code what the new binding expresses, but
> I don't fully understand it. Is it the maximum clock stretch time?
> Because I cannot recall a device which always uses the same delay for
> clock stretching.
> 

Due to clock stretch, our HW IP cannot meet the ac-timing
spec(tSU;STA,tSU;STO). 
There isn't a same delay for clock stretching, so we need pass a
parameter which can be found through measurement to meet most
conditions.

Re: [RESEND] i2c: mediatek: Get device clock-stretch time via dts

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 390 bytes --]


> Due to clock stretch, our HW IP cannot meet the ac-timing
> spec(tSU;STA,tSU;STO). 
> There isn't a same delay for clock stretching, so we need pass a
> parameter which can be found through measurement to meet most
> conditions.

What about using this existing binding?

- i2c-scl-internal-delay-ns
        Number of nanoseconds the IP core additionally needs to setup SCL.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RESEND] i2c: mediatek: Get device clock-stretch time via dts

[Qii Wang]

On Wed, 2021-04-07 at 20:19 +0200, Wolfram Sang wrote:
> > Due to clock stretch, our HW IP cannot meet the ac-timing
> > spec(tSU;STA,tSU;STO). 
> > There isn't a same delay for clock stretching, so we need pass a
> > parameter which can be found through measurement to meet most
> > conditions.
> 
> What about using this existing binding?
> 
> - i2c-scl-internal-delay-ns
>         Number of nanoseconds the IP core additionally needs to setup SCL.
> 

I can't see the relationship between "i2c-scl-falling-time-ns" and clock
stretching, is there a parameter related to clock stretching?
If you think both of them will affect the ac-timing of SCL, at this
point, "i2c-scl-falling-time-ns" maybe a good choice.

Re: [RESEND] i2c: mediatek: Get device clock-stretch time via dts

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1289 bytes --]

On Mon, Apr 12, 2021 at 08:03:14PM +0800, Qii Wang wrote:
> On Wed, 2021-04-07 at 20:19 +0200, Wolfram Sang wrote:
> > > Due to clock stretch, our HW IP cannot meet the ac-timing
> > > spec(tSU;STA,tSU;STO). 
> > > There isn't a same delay for clock stretching, so we need pass a
> > > parameter which can be found through measurement to meet most
> > > conditions.
> > 
> > What about using this existing binding?
> > 
> > - i2c-scl-internal-delay-ns
> >         Number of nanoseconds the IP core additionally needs to setup SCL.
> > 
> 
> I can't see the relationship between "i2c-scl-falling-time-ns" and clock
> stretching, is there a parameter related to clock stretching?

( you wrote "i2c-scl-falling-time-ns" above, didn't you mean
"i2c-scl-internal-delay-ns" instead? )

Not yet, and I wonder if there can be one. In I2C (not SMBus), devices
are allowed to stretch the clock as long as they want, so what should be
specified here?

I suggesteed "internal-delay" because AFAIU your hardware needs this
delay to be able to cope with clock stretching.

> If you think both of them will affect the ac-timing of SCL, at this
> point, "i2c-scl-falling-time-ns" maybe a good choice.

Do you mean "i2c-scl-falling-time-ns" or "i2c-scl-internal-delay-ns"?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RESEND] i2c: mediatek: Get device clock-stretch time via dts

[Qii Wang]

On Tue, 2021-04-13 at 22:17 +0200, Wolfram Sang wrote:
> On Mon, Apr 12, 2021 at 08:03:14PM +0800, Qii Wang wrote:
> > I can't see the relationship between "i2c-scl-falling-time-ns" and clock
> > stretching, is there a parameter related to clock stretching?
> 
> ( you wrote "i2c-scl-falling-time-ns" above, didn't you mean
> "i2c-scl-internal-delay-ns" instead? )
> 

I am sorry, I have confused your comment with lkjoon's comment in the
last mail. what I actually want to say is "i2c-scl-internal-delay-ns".

> Not yet, and I wonder if there can be one. In I2C (not SMBus), devices
> are allowed to stretch the clock as long as they want, so what should be
> specified here?
> 
> I suggesteed "internal-delay" because AFAIU your hardware needs this
> delay to be able to cope with clock stretching.
> 

If there is not a maximum value for clock stretching,
"i2c-scl-internal-delay-ns" should be a good choice for our hardware,
although it maybe not for clock stretching.

> > If you think both of them will affect the ac-timing of SCL, at this
> > point, "i2c-scl-falling-time-ns" maybe a good choice.
> 
> Do you mean "i2c-scl-falling-time-ns" or "i2c-scl-internal-delay-ns"?
> 

"i2c-scl-internal-delay-ns" is better.

Thanks for your review.
Qii