* [PATCH] i2c-s3c2410: Enable i2c clock only on xfer @ 2010-04-17 20:42 Vasily Khoruzhick 2010-04-20 9:39 ` Ben Dooks 0 siblings, 1 reply; 3+ messages in thread From: Vasily Khoruzhick @ 2010-04-17 20:42 UTC (permalink / raw) To: linux-arm-kernel 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> ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] i2c-s3c2410: Enable i2c clock only on xfer 2010-04-17 20:42 [PATCH] i2c-s3c2410: Enable i2c clock only on xfer Vasily Khoruzhick @ 2010-04-20 9:39 ` Ben Dooks 2010-04-20 11:35 ` Vasily Khoruzhick 0 siblings, 1 reply; 3+ messages in thread From: Ben Dooks @ 2010-04-20 9:39 UTC (permalink / raw) To: linux-arm-kernel On Sat, Apr 17, 2010 at 11:42:32PM +0300, Vasily Khoruzhick wrote: > 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. > 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 <anarsoul@gmail.com> > Date: Sat, 17 Apr 2010 23:32:52 +0300 > Subject: [PATCH] i2c-s3c2410: Enable i2c clock only on xfer > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > --- > drivers/i2c/busses/i2c-s3c2410.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c > index 1d8c986..662f6e4 100644 > --- a/drivers/i2c/busses/i2c-s3c2410.c > +++ b/drivers/i2c/busses/i2c-s3c2410.c > @@ -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. > 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. > /* map the registers */ > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > -- > 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 -- -- Ben Q: What's a light-year? A: One-third less calories than a regular year. ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] i2c-s3c2410: Enable i2c clock only on xfer 2010-04-20 9:39 ` Ben Dooks @ 2010-04-20 11:35 ` Vasily Khoruzhick 0 siblings, 0 replies; 3+ messages in thread From: Vasily Khoruzhick @ 2010-04-20 11:35 UTC (permalink / raw) To: linux-arm-kernel ? ????????? ?? 20 ?????? 2010 12:39:36 ????? Ben Dooks ???????: > On Sat, Apr 17, 2010 at 11:42:32PM +0300, Vasily Khoruzhick wrote: > > 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... > It is probably worth moving the clk_enable() outside the loop to avoid > calling it each time we try an transfer. Ok. > 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> ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-04-20 11:35 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-04-17 20:42 [PATCH] i2c-s3c2410: Enable i2c clock only on xfer Vasily Khoruzhick 2010-04-20 9:39 ` Ben Dooks 2010-04-20 11:35 ` Vasily Khoruzhick
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).