[PATCH] i2c: exynos5: simplify timings calculation

[PATCH] i2c: exynos5: simplify timings calculation

[Andrzej Hajda]

Instead of using cryptic loop direct calculation of timings
can be used.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/i2c/busses/i2c-exynos5.c | 40 ++++++++++++++--------------------------
 1 file changed, 14 insertions(+), 26 deletions(-)

diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
index cbd93ce..04127b8 100644
--- a/drivers/i2c/busses/i2c-exynos5.c
+++ b/drivers/i2c/busses/i2c-exynos5.c
@@ -292,9 +292,9 @@ static int exynos5_i2c_set_timing(struct exynos5_i2c *i2c, int mode)
 	unsigned int t_sr_release;
 	unsigned int t_ftl_cycle;
 	unsigned int clkin = clk_get_rate(i2c->clk);
-	unsigned int div, utemp0 = 0, utemp1 = 0, clk_cycle;
 	unsigned int op_clk = (mode == HSI2C_HIGH_SPD) ?
 				i2c->hs_clock : i2c->fs_clock;
+	int div, clk_cycle, temp;
 
 	/*
 	 * In case of HSI2C controller in Exynos5 series
@@ -305,33 +305,21 @@ static int exynos5_i2c_set_timing(struct exynos5_i2c *i2c, int mode)
 	 * FPCLK / FI2C =
 	 * (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) + 8 + FLT_CYCLE
 	 *
-	 * utemp0 = (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2)
-	 * utemp1 = (TSCLK_L + TSCLK_H + 2)
+	 * clk_cycle := TSCLK_L + TSCLK_H
+	 * temp := (CLK_DIV + 1) * (clk_cycle + 2)
+	 *
+	 * Constraints: 4 <= temp, 0 <= CLK_DIV < 256, 2 <= clk_cycle <= 510
+	 *
 	 */
 	t_ftl_cycle = (readl(i2c->regs + HSI2C_CONF) >> 16) & 0x7;
-	utemp0 = (clkin / op_clk) - 8;
-
-	if (i2c->variant->hw == HSI2C_EXYNOS7)
-		utemp0 -= t_ftl_cycle;
-	else
-		utemp0 -= 2 * t_ftl_cycle;
-
-	/* CLK_DIV max is 256 */
-	for (div = 0; div < 256; div++) {
-		utemp1 = utemp0 / (div + 1);
-
-		/*
-		 * SCL_L and SCL_H each has max value of 255
-		 * Hence, For the clk_cycle to the have right value
-		 * utemp1 has to be less then 512 and more than 4.
-		 */
-		if ((utemp1 < 512) && (utemp1 > 4)) {
-			clk_cycle = utemp1 - 2;
-			break;
-		} else if (div == 255) {
-			dev_warn(i2c->dev, "Failed to calculate divisor");
-			return -EINVAL;
-		}
+	temp = clkin / op_clk - 8 - t_ftl_cycle;
+	if (i2c->variant->hw != HSI2C_EXYNOS7)
+		temp -= t_ftl_cycle;
+	div = temp / 512;
+	clk_cycle = temp / (div + 1) - 2;
+	if (temp < 4 || div >= 256 || clk_cycle < 2) {
+		dev_warn(i2c->dev, "Failed to calculate divisor");
+		return -EINVAL;
 	}
 
 	t_scl_l = clk_cycle / 2;
-- 
2.7.4

Re: [PATCH] i2c: exynos5: simplify timings calculation

[Andrzej Hajda]

Hi Wolfram,

Gently ping.

Regards
Andrzej

On 23.02.2017 17:47, Andrzej Hajda wrote:
> Instead of using cryptic loop direct calculation of timings
> can be used.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/i2c/busses/i2c-exynos5.c | 40 ++++++++++++++--------------------------
>  1 file changed, 14 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
> index cbd93ce..04127b8 100644
> --- a/drivers/i2c/busses/i2c-exynos5.c
> +++ b/drivers/i2c/busses/i2c-exynos5.c
> @@ -292,9 +292,9 @@ static int exynos5_i2c_set_timing(struct exynos5_i2c *i2c, int mode)
>  	unsigned int t_sr_release;
>  	unsigned int t_ftl_cycle;
>  	unsigned int clkin = clk_get_rate(i2c->clk);
> -	unsigned int div, utemp0 = 0, utemp1 = 0, clk_cycle;
>  	unsigned int op_clk = (mode == HSI2C_HIGH_SPD) ?
>  				i2c->hs_clock : i2c->fs_clock;
> +	int div, clk_cycle, temp;
>  
>  	/*
>  	 * In case of HSI2C controller in Exynos5 series
> @@ -305,33 +305,21 @@ static int exynos5_i2c_set_timing(struct exynos5_i2c *i2c, int mode)
>  	 * FPCLK / FI2C =
>  	 * (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) + 8 + FLT_CYCLE
>  	 *
> -	 * utemp0 = (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2)
> -	 * utemp1 = (TSCLK_L + TSCLK_H + 2)
> +	 * clk_cycle := TSCLK_L + TSCLK_H
> +	 * temp := (CLK_DIV + 1) * (clk_cycle + 2)
> +	 *
> +	 * Constraints: 4 <= temp, 0 <= CLK_DIV < 256, 2 <= clk_cycle <= 510
> +	 *
>  	 */
>  	t_ftl_cycle = (readl(i2c->regs + HSI2C_CONF) >> 16) & 0x7;
> -	utemp0 = (clkin / op_clk) - 8;
> -
> -	if (i2c->variant->hw == HSI2C_EXYNOS7)
> -		utemp0 -= t_ftl_cycle;
> -	else
> -		utemp0 -= 2 * t_ftl_cycle;
> -
> -	/* CLK_DIV max is 256 */
> -	for (div = 0; div < 256; div++) {
> -		utemp1 = utemp0 / (div + 1);
> -
> -		/*
> -		 * SCL_L and SCL_H each has max value of 255
> -		 * Hence, For the clk_cycle to the have right value
> -		 * utemp1 has to be less then 512 and more than 4.
> -		 */
> -		if ((utemp1 < 512) && (utemp1 > 4)) {
> -			clk_cycle = utemp1 - 2;
> -			break;
> -		} else if (div == 255) {
> -			dev_warn(i2c->dev, "Failed to calculate divisor");
> -			return -EINVAL;
> -		}
> +	temp = clkin / op_clk - 8 - t_ftl_cycle;
> +	if (i2c->variant->hw != HSI2C_EXYNOS7)
> +		temp -= t_ftl_cycle;
> +	div = temp / 512;
> +	clk_cycle = temp / (div + 1) - 2;
> +	if (temp < 4 || div >= 256 || clk_cycle < 2) {
> +		dev_warn(i2c->dev, "Failed to calculate divisor");
> +		return -EINVAL;
>  	}
>  
>  	t_scl_l = clk_cycle / 2;

Re: [PATCH] i2c: exynos5: simplify timings calculation

[Andrzej Hajda]

Hi Wolfram,

Ping, 7 weeks passed.

Regards
Andrzej


On 23.02.2017 17:47, Andrzej Hajda wrote:
> Instead of using cryptic loop direct calculation of timings
> can be used.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/i2c/busses/i2c-exynos5.c | 40 ++++++++++++++--------------------------
>  1 file changed, 14 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
> index cbd93ce..04127b8 100644
> --- a/drivers/i2c/busses/i2c-exynos5.c
> +++ b/drivers/i2c/busses/i2c-exynos5.c
> @@ -292,9 +292,9 @@ static int exynos5_i2c_set_timing(struct exynos5_i2c *i2c, int mode)
>  	unsigned int t_sr_release;
>  	unsigned int t_ftl_cycle;
>  	unsigned int clkin = clk_get_rate(i2c->clk);
> -	unsigned int div, utemp0 = 0, utemp1 = 0, clk_cycle;
>  	unsigned int op_clk = (mode == HSI2C_HIGH_SPD) ?
>  				i2c->hs_clock : i2c->fs_clock;
> +	int div, clk_cycle, temp;
>  
>  	/*
>  	 * In case of HSI2C controller in Exynos5 series
> @@ -305,33 +305,21 @@ static int exynos5_i2c_set_timing(struct exynos5_i2c *i2c, int mode)
>  	 * FPCLK / FI2C =
>  	 * (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2) + 8 + FLT_CYCLE
>  	 *
> -	 * utemp0 = (CLK_DIV + 1) * (TSCLK_L + TSCLK_H + 2)
> -	 * utemp1 = (TSCLK_L + TSCLK_H + 2)
> +	 * clk_cycle := TSCLK_L + TSCLK_H
> +	 * temp := (CLK_DIV + 1) * (clk_cycle + 2)
> +	 *
> +	 * Constraints: 4 <= temp, 0 <= CLK_DIV < 256, 2 <= clk_cycle <= 510
> +	 *
>  	 */
>  	t_ftl_cycle = (readl(i2c->regs + HSI2C_CONF) >> 16) & 0x7;
> -	utemp0 = (clkin / op_clk) - 8;
> -
> -	if (i2c->variant->hw == HSI2C_EXYNOS7)
> -		utemp0 -= t_ftl_cycle;
> -	else
> -		utemp0 -= 2 * t_ftl_cycle;
> -
> -	/* CLK_DIV max is 256 */
> -	for (div = 0; div < 256; div++) {
> -		utemp1 = utemp0 / (div + 1);
> -
> -		/*
> -		 * SCL_L and SCL_H each has max value of 255
> -		 * Hence, For the clk_cycle to the have right value
> -		 * utemp1 has to be less then 512 and more than 4.
> -		 */
> -		if ((utemp1 < 512) && (utemp1 > 4)) {
> -			clk_cycle = utemp1 - 2;
> -			break;
> -		} else if (div == 255) {
> -			dev_warn(i2c->dev, "Failed to calculate divisor");
> -			return -EINVAL;
> -		}
> +	temp = clkin / op_clk - 8 - t_ftl_cycle;
> +	if (i2c->variant->hw != HSI2C_EXYNOS7)
> +		temp -= t_ftl_cycle;
> +	div = temp / 512;
> +	clk_cycle = temp / (div + 1) - 2;
> +	if (temp < 4 || div >= 256 || clk_cycle < 2) {
> +		dev_warn(i2c->dev, "Failed to calculate divisor");
> +		return -EINVAL;
>  	}
>  
>  	t_scl_l = clk_cycle / 2;

Re: [PATCH] i2c: exynos5: simplify timings calculation

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 511 bytes --]

On Fri, Apr 14, 2017 at 11:24:16AM +0200, Andrzej Hajda wrote:
> Hi Wolfram,
> 
> Ping, 7 weeks passed.

Known issue, there are not enough reviewers for I2C for a timely
response. I was hoping for some tags from other users of this hardware,
but that didn't happen. And I don't know this hardware, at all. So, I'll
apply your patches early in the next cycle, and we will see what happens
once they are in -next. Best would be if you could find other people to
donate tags.

Thanks,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] i2c: exynos5: simplify timings calculation

[Javier Martinez Canillas]

Hello Wolfram,

On 04/19/2017 03:17 PM, Wolfram Sang wrote:
> On Fri, Apr 14, 2017 at 11:24:16AM +0200, Andrzej Hajda wrote:
>> Hi Wolfram,
>>
>> Ping, 7 weeks passed.
> 
> Known issue, there are not enough reviewers for I2C for a timely
> response. I was hoping for some tags from other users of this hardware,
> but that didn't happen. And I don't know this hardware, at all. So, I'll
> apply your patches early in the next cycle, and we will see what happens
> once they are in -next. Best would be if you could find other people to
> donate tags.
>

I got a try to this patch on an Exynos5800 Peach Pi Chromebook and all
the I2C devices are working properly, so:

Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>

> Thanks,
> 
>    Wolfram
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH] i2c: exynos5: simplify timings calculation

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]

On Wed, Apr 19, 2017 at 03:36:39PM -0400, Javier Martinez Canillas wrote:
> Hello Wolfram,
> 
> On 04/19/2017 03:17 PM, Wolfram Sang wrote:
> > On Fri, Apr 14, 2017 at 11:24:16AM +0200, Andrzej Hajda wrote:
> >> Hi Wolfram,
> >>
> >> Ping, 7 weeks passed.
> > 
> > Known issue, there are not enough reviewers for I2C for a timely
> > response. I was hoping for some tags from other users of this hardware,
> > but that didn't happen. And I don't know this hardware, at all. So, I'll
> > apply your patches early in the next cycle, and we will see what happens
> > once they are in -next. Best would be if you could find other people to
> > donate tags.
> >
> 
> I got a try to this patch on an Exynos5800 Peach Pi Chromebook and all
> the I2C devices are working properly, so:
> 
> Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>

Cool, can you test his other patches as well?

http://patchwork.ozlabs.org/project/linux-i2c/list/?submitter=47122


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] i2c: exynos5: simplify timings calculation

[Andi Shyti]

Hi Andrzej,

> > Ping, 7 weeks passed.
> 
> Known issue, there are not enough reviewers for I2C for a timely
> response. I was hoping for some tags from other users of this hardware,
> but that didn't happen. And I don't know this hardware, at all. So, I'll
> apply your patches early in the next cycle, and we will see what happens
> once they are in -next. Best would be if you could find other people to
> donate tags.

I'm sorry, I forgot to reply to this patch, I actually reviewed
and tested it among all your i2c patches (that's why I asked you
on IRC to add me in Cc to all your i2c patches).

So you can add also my

Reviewed-by: Andi Shyti <andi.shyti@samsung.com>

Andi

Re: [PATCH] i2c: exynos5: simplify timings calculation

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 304 bytes --]


> I'm sorry, I forgot to reply to this patch, I actually reviewed
> and tested it among all your i2c patches

You mean you reviewed all his i2c patches? If so, could you add the tag
per patch. This way, patchwork will automatically collect them for me
and display the tag count in the list of patches.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] i2c: exynos5: simplify timings calculation

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 235 bytes --]

On Thu, Feb 23, 2017 at 05:47:26PM +0100, Andrzej Hajda wrote:
> Instead of using cryptic loop direct calculation of timings
> can be used.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] i2c: exynos5: simplify timings calculation

[Andi Shyti]

Hi Wolfram,

> > I'm sorry, I forgot to reply to this patch, I actually reviewed
> > and tested it among all your i2c patches
> 
> You mean you reviewed all his i2c patches? If so, could you add the tag
> per patch. This way, patchwork will automatically collect them for me
> and display the tag count in the list of patches.

yes, I reviewed, tested and used all the patches that Andrzej
has recently sent to the i2c-exynos driver. For all of them I
replied with my Reviewed-by, apparently I forgot to send the
reply to this one.

Andi