Skip to contentSkip to conversation

i2c-s3c2410: Enable i2c clock only on xfer

PATCH New Vasily Khoruzhick 2 participants 3 messages Apr 17, 2010 – Apr 20, 2010

lore
Share & tools
lore

Vasily Khoruzhick

author
#0 lore raw

PATCH 1/1 i2c-s3c2410: Enable i2c clock only on xfer

I don't see any reason to keep i2c clock enabled all the time when i2c bus driver is loaded, and even more: it prevents me to disable uda1380 codec power
- uda1380 doesn't respond on i2c xfers if its power was disabled during i2c clock is enabled.

WinMobile enables i2c clock only on xfer (same for i2s clock)

Regards
Vasily
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-i2c-s3c2410-Enable-i2c-clock-only-on-xfer.patch
Type: text/x-patch
Size: 1086 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100417/6e5080f2/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100417/6e5080f2/attachment.sig>

#1 lore raw

PATCH 1/1 i2c-s3c2410: Enable i2c clock only on xfer

Quoted 4 lines · Vasily Khoruzhick

I don't see any reason to keep i2c clock enabled all the time when i2c bus driver is loaded, and even more: it prevents me to disable uda1380 codec power
- uda1380 doesn't respond on i2c xfers if its power was disabled during i2c clock is enabled.

Weird. The SCL line should only be clocked when the block is generating transfers.

Quoted 20 lines

WinMobile enables i2c clock only on xfer (same for i2s clock)

Regards
Vasily

From 398bcaeb1bde17d4e33052d99dea914dad8378e0 Mon Sep 17 00:00:00 2001
From: Vasily Khoruzhick <[email protected]>
Date: Sat, 17 Apr 2010 23:32:52 +0300
Subject: [PATCH] i2c-s3c2410: Enable i2c clock only on xfer

Signed-off-by Vasily Khoruzhick <[email protected]>
---
 drivers/i2c/busses/i2c-s3c2410.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-s3c2410.c +0 -0
index 1d8c986..662f6e4 100644

drivers/i2c/busses/i2c-s3c2410.c static int s3c24xx_i2c_xfer(struct i2c_adapter *adap, · line 546

Quoted 6 lines
drivers/i2c/busses/i2c-s3c2410.c +2 -0
@@ -541,7 +541,9 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
 
 	for (retry = 0; retry < adap->retries; retry++) {
 
+		clk_enable(i2c->clk);
 		ret = s3c24xx_i2c_doxfer(i2c, msgs, num);
+		clk_disable(i2c->clk);

I'll try and find out if there is any setup requirement between clock enable and the block being used.

It is probably worth moving the clk_enable() outside the loop to avoid calling it each time we try an transfer.

drivers/i2c/busses/i2c-s3c2410.c static int s3c24xx_i2c_probe(struct platform_device *pdev) · line 816

Quoted 8 lines
drivers/i2c/busses/i2c-s3c2410.c +0 -2
 		if (ret != -EAGAIN)
 			return ret;

@@ -812,8 +814,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 
 	dev_dbg(&pdev->dev, "clock source %p\n", i2c->clk);
 
-	clk_enable(i2c->clk);
-

I'm not sure, but I think the clock should also be enabled when reading or writing registers. I'd be happier to see a patch where the setup functions also ensure that the clock is enabled.

Quoted 14 lines
drivers/i2c/busses/i2c-s3c2410.c +0 -0
 	/* map the registers */
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
Signature · 10 lines
1.7.0.4





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 5 lines
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.
author
#2 lore raw

PATCH 1/1 i2c-s3c2410: Enable i2c clock only on xfer

? ????????? ?? 20 ?????? 2010 12:39:36 ????? Ben Dooks ???????:

Quoted 8 lines · Vasily Khoruzhick
Quoted 4 lines

I don't see any reason to keep i2c clock enabled all the time when i2c bus driver is loaded, and even more: it prevents me to disable uda1380 codec power - uda1380 doesn't respond on i2c xfers if its power was disabled during i2c clock is enabled.

Weird. The SCL line should only be clocked when the block is generating transfers.

Yeah, it should be so, but there's a fact - codec doesn't respond on i2c after its power off/power on sequence without this patch, and it responds with patch.
I suppose I can monitor SDA/SCL state via reading GPE15/GPE14 pin status, I'll investigate on it a bit more...

Quoted 2 lines

It is probably worth moving the clk_enable() outside the loop to avoid calling it each time we try an transfer.

Ok.

Quoted 3 lines

I'm not sure, but I think the clock should also be enabled when reading or writing registers. I'd be happier to see a patch where the setup functions also ensure that the clock is enabled.

Ok

Regards,
Vasily
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100420/b0067cbd/attachment.sig>