All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/5] i2c: i2c-sh_mobile: calculate clock parameters at driver probing time
Date: Wed, 24 Oct 2012 10:56:51 +0000	[thread overview]
Message-ID: <5087C973.4010305@renesas.com> (raw)
In-Reply-To: <5087C93F.6080601@renesas.com>

Currently SCL clock parameters (ICCH/ICCL) are calculated in
activate_ch(), which gets called every time sh_mobile_i2c_xfer() is
processed, while each I2C bus speed is system-defined and in general
those parameters do not have to be updated over I2C transactions.

The only reason I could see having it transaction-time is to adjust
ICCH/ICCL values according to the operating frequency of the I2C
hardware block, in the face of DFS (Dynamic Frequency Scaling).

However, this won't be necessary.

The operating frequency of the I2C hardware block can change _even_
in the middle of I2C transactions.  There is no way to prevent it
from happening, and I2C hardware block can work with such dynamic
frequency change, of course.

Another is that ICCH/ICCL clock parameters optimized for the faster
operating frequency, can also be applied to the slower operating
frequency, as long as slave devices work.  However, the converse is
not true.  It would violate SCL timing specs of the I2C standard.

What we can do now is to calculate the ICCH/ICCL clock parameters
according to the fastest operating clock of the I2C hardware block.
And if that's the case, that calculation should be done just once
at driver-module-init time.

This patch moves ICCH/ICCL calculating part from activate_ch() into
sh_mobile_i2c_init(), and call it from sh_mobile_i2c_probe().

Note that sh_mobile_i2c_init() just prepares clock parameters using
the clock rate and platform data provided, but does _not_ make any
hardware I/O accesses.  We don't have to care about run-time PM
maintenance here.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/i2c/busses/i2c-sh_mobile.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c
index 8110ca4..309d0d5 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -187,18 +187,15 @@ static void iic_set_clr(struct sh_mobile_i2c_data *pd, int offs,
 	iic_wr(pd, offs, (iic_rd(pd, offs) | set) & ~clr);
 }
 
-static void activate_ch(struct sh_mobile_i2c_data *pd)
+static void sh_mobile_i2c_init(struct sh_mobile_i2c_data *pd)
 {
 	unsigned long i2c_clk;
 	u_int32_t num;
 	u_int32_t denom;
 	u_int32_t tmp;
 
-	/* Wake up device and enable clock */
-	pm_runtime_get_sync(pd->dev);
-	clk_enable(pd->clk);
-
 	/* Get clock rate after clock is enabled */
+	clk_enable(pd->clk);
 	i2c_clk = clk_get_rate(pd->clk);
 
 	/* Calculate the value for iccl. From the data sheet:
@@ -239,6 +236,15 @@ static void activate_ch(struct sh_mobile_i2c_data *pd)
 			pd->icic &= ~ICIC_ICCHB8;
 	}
 
+	clk_disable(pd->clk);
+}
+
+static void activate_ch(struct sh_mobile_i2c_data *pd)
+{
+	/* Wake up device and enable clock */
+	pm_runtime_get_sync(pd->dev);
+	clk_enable(pd->clk);
+
 	/* Enable channel and configure rx ack */
 	iic_set_clr(pd, ICCR, ICCR_ICE, 0);
 
@@ -632,6 +638,8 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
 	if (size > 0x17)
 		pd->flags |= IIC_FLAG_HAS_ICIC67;
 
+	sh_mobile_i2c_init(pd);
+
 	/* Enable Runtime PM for this device.
 	 *
 	 * Also tell the Runtime PM core to ignore children
-- 
1.7.12.4


WARNING: multiple messages have this Message-ID (diff)
From: Shinya Kuribayashi <shinya.kuribayashi.px-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
To: w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
	ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org,
	magnus.damm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-sh-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: [PATCH 1/5] i2c: i2c-sh_mobile: calculate clock parameters at driver probing time
Date: Wed, 24 Oct 2012 19:56:51 +0900	[thread overview]
Message-ID: <5087C973.4010305@renesas.com> (raw)
In-Reply-To: <5087C93F.6080601-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

Currently SCL clock parameters (ICCH/ICCL) are calculated in
activate_ch(), which gets called every time sh_mobile_i2c_xfer() is
processed, while each I2C bus speed is system-defined and in general
those parameters do not have to be updated over I2C transactions.

The only reason I could see having it transaction-time is to adjust
ICCH/ICCL values according to the operating frequency of the I2C
hardware block, in the face of DFS (Dynamic Frequency Scaling).

However, this won't be necessary.

The operating frequency of the I2C hardware block can change _even_
in the middle of I2C transactions.  There is no way to prevent it
from happening, and I2C hardware block can work with such dynamic
frequency change, of course.

Another is that ICCH/ICCL clock parameters optimized for the faster
operating frequency, can also be applied to the slower operating
frequency, as long as slave devices work.  However, the converse is
not true.  It would violate SCL timing specs of the I2C standard.

What we can do now is to calculate the ICCH/ICCL clock parameters
according to the fastest operating clock of the I2C hardware block.
And if that's the case, that calculation should be done just once
at driver-module-init time.

This patch moves ICCH/ICCL calculating part from activate_ch() into
sh_mobile_i2c_init(), and call it from sh_mobile_i2c_probe().

Note that sh_mobile_i2c_init() just prepares clock parameters using
the clock rate and platform data provided, but does _not_ make any
hardware I/O accesses.  We don't have to care about run-time PM
maintenance here.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-sh_mobile.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c
index 8110ca4..309d0d5 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -187,18 +187,15 @@ static void iic_set_clr(struct sh_mobile_i2c_data *pd, int offs,
 	iic_wr(pd, offs, (iic_rd(pd, offs) | set) & ~clr);
 }
 
-static void activate_ch(struct sh_mobile_i2c_data *pd)
+static void sh_mobile_i2c_init(struct sh_mobile_i2c_data *pd)
 {
 	unsigned long i2c_clk;
 	u_int32_t num;
 	u_int32_t denom;
 	u_int32_t tmp;
 
-	/* Wake up device and enable clock */
-	pm_runtime_get_sync(pd->dev);
-	clk_enable(pd->clk);
-
 	/* Get clock rate after clock is enabled */
+	clk_enable(pd->clk);
 	i2c_clk = clk_get_rate(pd->clk);
 
 	/* Calculate the value for iccl. From the data sheet:
@@ -239,6 +236,15 @@ static void activate_ch(struct sh_mobile_i2c_data *pd)
 			pd->icic &= ~ICIC_ICCHB8;
 	}
 
+	clk_disable(pd->clk);
+}
+
+static void activate_ch(struct sh_mobile_i2c_data *pd)
+{
+	/* Wake up device and enable clock */
+	pm_runtime_get_sync(pd->dev);
+	clk_enable(pd->clk);
+
 	/* Enable channel and configure rx ack */
 	iic_set_clr(pd, ICCR, ICCR_ICE, 0);
 
@@ -632,6 +638,8 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
 	if (size > 0x17)
 		pd->flags |= IIC_FLAG_HAS_ICIC67;
 
+	sh_mobile_i2c_init(pd);
+
 	/* Enable Runtime PM for this device.
 	 *
 	 * Also tell the Runtime PM core to ignore children
-- 
1.7.12.4

WARNING: multiple messages have this Message-ID (diff)
From: shinya.kuribayashi.px@renesas.com (Shinya Kuribayashi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/5] i2c: i2c-sh_mobile: calculate clock parameters at driver probing time
Date: Wed, 24 Oct 2012 19:56:51 +0900	[thread overview]
Message-ID: <5087C973.4010305@renesas.com> (raw)
In-Reply-To: <5087C93F.6080601@renesas.com>

Currently SCL clock parameters (ICCH/ICCL) are calculated in
activate_ch(), which gets called every time sh_mobile_i2c_xfer() is
processed, while each I2C bus speed is system-defined and in general
those parameters do not have to be updated over I2C transactions.

The only reason I could see having it transaction-time is to adjust
ICCH/ICCL values according to the operating frequency of the I2C
hardware block, in the face of DFS (Dynamic Frequency Scaling).

However, this won't be necessary.

The operating frequency of the I2C hardware block can change _even_
in the middle of I2C transactions.  There is no way to prevent it
from happening, and I2C hardware block can work with such dynamic
frequency change, of course.

Another is that ICCH/ICCL clock parameters optimized for the faster
operating frequency, can also be applied to the slower operating
frequency, as long as slave devices work.  However, the converse is
not true.  It would violate SCL timing specs of the I2C standard.

What we can do now is to calculate the ICCH/ICCL clock parameters
according to the fastest operating clock of the I2C hardware block.
And if that's the case, that calculation should be done just once
at driver-module-init time.

This patch moves ICCH/ICCL calculating part from activate_ch() into
sh_mobile_i2c_init(), and call it from sh_mobile_i2c_probe().

Note that sh_mobile_i2c_init() just prepares clock parameters using
the clock rate and platform data provided, but does _not_ make any
hardware I/O accesses.  We don't have to care about run-time PM
maintenance here.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/i2c/busses/i2c-sh_mobile.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c
index 8110ca4..309d0d5 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -187,18 +187,15 @@ static void iic_set_clr(struct sh_mobile_i2c_data *pd, int offs,
 	iic_wr(pd, offs, (iic_rd(pd, offs) | set) & ~clr);
 }
 
-static void activate_ch(struct sh_mobile_i2c_data *pd)
+static void sh_mobile_i2c_init(struct sh_mobile_i2c_data *pd)
 {
 	unsigned long i2c_clk;
 	u_int32_t num;
 	u_int32_t denom;
 	u_int32_t tmp;
 
-	/* Wake up device and enable clock */
-	pm_runtime_get_sync(pd->dev);
-	clk_enable(pd->clk);
-
 	/* Get clock rate after clock is enabled */
+	clk_enable(pd->clk);
 	i2c_clk = clk_get_rate(pd->clk);
 
 	/* Calculate the value for iccl. From the data sheet:
@@ -239,6 +236,15 @@ static void activate_ch(struct sh_mobile_i2c_data *pd)
 			pd->icic &= ~ICIC_ICCHB8;
 	}
 
+	clk_disable(pd->clk);
+}
+
+static void activate_ch(struct sh_mobile_i2c_data *pd)
+{
+	/* Wake up device and enable clock */
+	pm_runtime_get_sync(pd->dev);
+	clk_enable(pd->clk);
+
 	/* Enable channel and configure rx ack */
 	iic_set_clr(pd, ICCR, ICCR_ICE, 0);
 
@@ -632,6 +638,8 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
 	if (size > 0x17)
 		pd->flags |= IIC_FLAG_HAS_ICIC67;
 
+	sh_mobile_i2c_init(pd);
+
 	/* Enable Runtime PM for this device.
 	 *
 	 * Also tell the Runtime PM core to ignore children
-- 
1.7.12.4

  reply	other threads:[~2012-10-24 10:56 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-24 10:55 [PATCH 0/5] i2c-sh_mobile non-urgent changes Shinya Kuribayashi
2012-10-24 10:55 ` Shinya Kuribayashi
2012-10-24 10:55 ` Shinya Kuribayashi
2012-10-24 10:56 ` Shinya Kuribayashi [this message]
2012-10-24 10:56   ` [PATCH 1/5] i2c: i2c-sh_mobile: calculate clock parameters at driver probing time Shinya Kuribayashi
2012-10-24 10:56   ` Shinya Kuribayashi
2012-10-24 10:57 ` [PATCH 2/5] i2c: i2c-sh_mobile: optimize ICCH/ICCL values according to I2C bus speed Shinya Kuribayashi
2012-10-24 10:57   ` Shinya Kuribayashi
2012-10-24 10:57   ` Shinya Kuribayashi
2012-10-24 10:57 ` [PATCH 3/5] i2c: i2c-sh_mobile: fix ICCH to avoid violation of the tHD;STA timing spec Shinya Kuribayashi
2012-10-24 10:57   ` Shinya Kuribayashi
2012-10-24 10:57   ` Shinya Kuribayashi
2012-10-24 10:58 ` [PATCH 4/5] i2c: i2c-sh_mobile: support I2C hardware block with a faster operating clock Shinya Kuribayashi
2012-10-24 10:58   ` Shinya Kuribayashi
2012-10-24 10:58   ` Shinya Kuribayashi
2012-10-24 10:58 ` [PATCH 5/5] i2c: i2c-sh_mobile: fix spurious transfer request timed out Shinya Kuribayashi
2012-10-24 10:58   ` Shinya Kuribayashi
2012-10-24 10:58   ` Shinya Kuribayashi
2012-11-16  8:07 ` [PATCH 0/5] i2c-sh_mobile non-urgent changes Wolfram Sang
2012-11-16  8:07   ` Wolfram Sang
2012-11-16  8:07   ` Wolfram Sang
2012-11-16  8:53   ` Shinya Kuribayashi
2012-11-16  8:53     ` Shinya Kuribayashi
2012-11-16  8:53     ` Shinya Kuribayashi
2012-11-17 23:27     ` Guennadi Liakhovetski
2012-11-17 23:27       ` Guennadi Liakhovetski
2012-11-17 23:27       ` Guennadi Liakhovetski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5087C973.4010305@renesas.com \
    --to=shinya.kuribayashi.px@renesas.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.