Linux-i2c Archive on lore.kernel.org
 help / color / Atom feed
* [RESEND] i2c: mediatek: Get device clock-stretch time via dts
@ 2021-03-13  8:04 qii.wang
  2021-03-18 11:23 ` Wolfram Sang
  2021-04-06 19:48 ` Wolfram Sang
  0 siblings, 2 replies; 8+ messages in thread
From: qii.wang @ 2021-03-13  8:04 UTC (permalink / raw)
  To: wsa
  Cc: matthias.bgg, linux-i2c, linux-arm-kernel, linux-kernel,
	linux-mediatek, srv_heupstream, leilk.liu, 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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RESEND] i2c: mediatek: Get device clock-stretch time via dts
  2021-03-13  8:04 [RESEND] i2c: mediatek: Get device clock-stretch time via dts qii.wang
@ 2021-03-18 11:23 ` Wolfram Sang
  2021-04-06 19:48 ` Wolfram Sang
  1 sibling, 0 replies; 8+ messages in thread
From: Wolfram Sang @ 2021-03-18 11:23 UTC (permalink / raw)
  To: qii.wang
  Cc: matthias.bgg, linux-i2c, linux-arm-kernel, linux-kernel,
	linux-mediatek, srv_heupstream, leilk.liu


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RESEND] i2c: mediatek: Get device clock-stretch time via dts
  2021-03-13  8:04 [RESEND] i2c: mediatek: Get device clock-stretch time via dts qii.wang
  2021-03-18 11:23 ` Wolfram Sang
@ 2021-04-06 19:48 ` Wolfram Sang
  2021-04-07 12:15   ` Qii Wang
  1 sibling, 1 reply; 8+ messages in thread
From: Wolfram Sang @ 2021-04-06 19:48 UTC (permalink / raw)
  To: qii.wang
  Cc: matthias.bgg, linux-i2c, linux-arm-kernel, linux-kernel,
	linux-mediatek, srv_heupstream, leilk.liu


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RESEND] i2c: mediatek: Get device clock-stretch time via dts
  2021-04-06 19:48 ` Wolfram Sang
@ 2021-04-07 12:15   ` Qii Wang
  2021-04-07 18:19     ` Wolfram Sang
  0 siblings, 1 reply; 8+ messages in thread
From: Qii Wang @ 2021-04-07 12:15 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: matthias.bgg, linux-i2c, linux-arm-kernel, linux-kernel,
	linux-mediatek, srv_heupstream, leilk.liu

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.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RESEND] i2c: mediatek: Get device clock-stretch time via dts
  2021-04-07 12:15   ` Qii Wang
@ 2021-04-07 18:19     ` Wolfram Sang
  2021-04-12 12:03       ` Qii Wang
  0 siblings, 1 reply; 8+ messages in thread
From: Wolfram Sang @ 2021-04-07 18:19 UTC (permalink / raw)
  To: Qii Wang
  Cc: matthias.bgg, linux-i2c, linux-arm-kernel, linux-kernel,
	linux-mediatek, srv_heupstream, leilk.liu


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RESEND] i2c: mediatek: Get device clock-stretch time via dts
  2021-04-07 18:19     ` Wolfram Sang
@ 2021-04-12 12:03       ` Qii Wang
  2021-04-13 20:17         ` Wolfram Sang
  0 siblings, 1 reply; 8+ messages in thread
From: Qii Wang @ 2021-04-12 12:03 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: matthias.bgg, linux-i2c, linux-arm-kernel, linux-kernel,
	linux-mediatek, srv_heupstream, leilk.liu

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.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RESEND] i2c: mediatek: Get device clock-stretch time via dts
  2021-04-12 12:03       ` Qii Wang
@ 2021-04-13 20:17         ` Wolfram Sang
  2021-04-14  1:37           ` Qii Wang
  0 siblings, 1 reply; 8+ messages in thread
From: Wolfram Sang @ 2021-04-13 20:17 UTC (permalink / raw)
  To: Qii Wang
  Cc: matthias.bgg, linux-i2c, linux-arm-kernel, linux-kernel,
	linux-mediatek, srv_heupstream, leilk.liu


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RESEND] i2c: mediatek: Get device clock-stretch time via dts
  2021-04-13 20:17         ` Wolfram Sang
@ 2021-04-14  1:37           ` Qii Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Qii Wang @ 2021-04-14  1:37 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: matthias.bgg, linux-i2c, linux-arm-kernel, linux-kernel,
	linux-mediatek, srv_heupstream, leilk.liu

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-13  8:04 [RESEND] i2c: mediatek: Get device clock-stretch time via dts qii.wang
2021-03-18 11:23 ` Wolfram Sang
2021-04-06 19:48 ` Wolfram Sang
2021-04-07 12:15   ` Qii Wang
2021-04-07 18:19     ` Wolfram Sang
2021-04-12 12:03       ` Qii Wang
2021-04-13 20:17         ` Wolfram Sang
2021-04-14  1:37           ` Qii Wang

Linux-i2c Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-i2c/0 linux-i2c/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-i2c linux-i2c/ https://lore.kernel.org/linux-i2c \
		linux-i2c@vger.kernel.org
	public-inbox-index linux-i2c

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-i2c


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git