2021-03-13 08:15:45

by Qii Wang (王琪)

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

From: Qii Wang <[email protected]>

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 <[email protected]>
---
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


2021-03-18 11:25:08

by Wolfram Sang

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


> 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 <[email protected]>

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


Attachments:
(No filename) (377.00 B)
signature.asc (849.00 B)
Download all attachments

2021-04-07 12:21:18

by Wolfram Sang

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

On Sat, Mar 13, 2021 at 04:04:24PM +0800, [email protected] wrote:
> From: Qii Wang <[email protected]>
>
> 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 <[email protected]>

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.


Attachments:
(No filename) (626.00 B)
signature.asc (849.00 B)
Download all attachments

2021-04-07 20:56:33

by Qii Wang (王琪)

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

On Tue, 2021-04-06 at 21:48 +0200, Wolfram Sang wrote:
> On Sat, Mar 13, 2021 at 04:04:24PM +0800, [email protected] wrote:
> > From: Qii Wang <[email protected]>
> >
> > 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 <[email protected]>
>
> 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.

2021-04-07 21:51:09

by Wolfram Sang

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


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


Attachments:
(No filename) (390.00 B)
signature.asc (849.00 B)
Download all attachments

2021-04-12 12:04:41

by Qii Wang (王琪)

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

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.

2021-04-14 00:12:34

by Wolfram Sang

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

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


Attachments:
(No filename) (1.26 kB)
signature.asc (849.00 B)
Download all attachments

2021-04-14 10:02:02

by Qii Wang (王琪)

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

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