diff mbox

i2c: davinci: Avoid zero value of CLKH

Message ID 20180713152017.2207-1-alexander.sverdlin@nokia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alexander Sverdlin July 13, 2018, 3:20 p.m. UTC
If CLKH is set to 0 I2C clock is not generated at all, so avoid this value
and stretch the clock in this case.

Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
---
 drivers/i2c/busses/i2c-davinci.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Sekhar Nori July 17, 2018, 11:57 a.m. UTC | #1
On Friday 13 July 2018 08:50 PM, Alexander Sverdlin wrote:
> If CLKH is set to 0 I2C clock is not generated at all, so avoid this value
> and stretch the clock in this case.

This must be by observation since specification does not seem to state
this.

> 
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>

In any case, the patch is fine.

Acked-by: Sekhar Nori <nsekhar@ti.com>

Thanks,
Sekhar

> ---
>  drivers/i2c/busses/i2c-davinci.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
> index 75d6ab177055..7379043711df 100644
> --- a/drivers/i2c/busses/i2c-davinci.c
> +++ b/drivers/i2c/busses/i2c-davinci.c
> @@ -237,12 +237,16 @@ static void i2c_davinci_calc_clk_dividers(struct davinci_i2c_dev *dev)
>  	/*
>  	 * It's not always possible to have 1 to 2 ratio when d=7, so fall back
>  	 * to minimal possible clkh in this case.
> +	 *
> +	 * Note:
> +	 * CLKH is not allowed to be 0, in this case I2C clock is not generated
> +	 * at all
>  	 */
> -	if (clk >= clkl + d) {
> +	if (clk > clkl + d) {
>  		clkh = clk - clkl - d;
>  		clkl -= d;
>  	} else {
> -		clkh = 0;
> +		clkh = 1;
>  		clkl = clk - (d << 1);
>  	}
>  
> -- 
> 2.18.0
>
Grygorii Strashko July 20, 2018, 6:43 p.m. UTC | #2
On 07/17/2018 06:57 AM, Sekhar Nori wrote:
> On Friday 13 July 2018 08:50 PM, Alexander Sverdlin wrote:
>> If CLKH is set to 0 I2C clock is not generated at all, so avoid this value
>> and stretch the clock in this case.
> 
> This must be by observation since specification does not seem to state
> this.

It will be good to know (mention) on what hw was it observed?

> 
>>
>> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
> 
> In any case, the patch is fine.
> 
> Acked-by: Sekhar Nori <nsekhar@ti.com>
> 
[...]
Wolfram Sang July 20, 2018, 10:18 p.m. UTC | #3
On Fri, Jul 20, 2018 at 01:43:46PM -0500, Grygorii Strashko wrote:
> 
> 
> On 07/17/2018 06:57 AM, Sekhar Nori wrote:
> > On Friday 13 July 2018 08:50 PM, Alexander Sverdlin wrote:
> > > If CLKH is set to 0 I2C clock is not generated at all, so avoid this value
> > > and stretch the clock in this case.
> > 
> > This must be by observation since specification does not seem to state
> > this.
> 
> It will be good to know (mention) on what hw was it observed?

Some more info would also help me to decide if this is stable material
or not. Is it a bug fix or more theoretical (but still valid, of
course)?
Wolfram Sang July 23, 2018, 6:07 p.m. UTC | #4
On Fri, Jul 13, 2018 at 05:20:17PM +0200, Alexander Sverdlin wrote:
> If CLKH is set to 0 I2C clock is not generated at all, so avoid this value
> and stretch the clock in this case.
> 
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>

Applied to for-current, thanks!

I did not add stable because Alexander told me this is very likely not
to be observed on HW out there. But TI people are investigating more.
I suggest they resend this patch to stable if they see fit. D'accord
everyone?
Sekhar Nori July 24, 2018, 6:17 a.m. UTC | #5
On Monday 23 July 2018 11:37 PM, Wolfram Sang wrote:
> On Fri, Jul 13, 2018 at 05:20:17PM +0200, Alexander Sverdlin wrote:
>> If CLKH is set to 0 I2C clock is not generated at all, so avoid this value
>> and stretch the clock in this case.
>>
>> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
> 
> Applied to for-current, thanks!
> 
> I did not add stable because Alexander told me this is very likely not
> to be observed on HW out there. But TI people are investigating more.
> I suggest they resend this patch to stable if they see fit. D'accord
> everyone?

Sounds good to me.

Thanks,
Sekhar
diff mbox

Patch

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 75d6ab177055..7379043711df 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -237,12 +237,16 @@  static void i2c_davinci_calc_clk_dividers(struct davinci_i2c_dev *dev)
 	/*
 	 * It's not always possible to have 1 to 2 ratio when d=7, so fall back
 	 * to minimal possible clkh in this case.
+	 *
+	 * Note:
+	 * CLKH is not allowed to be 0, in this case I2C clock is not generated
+	 * at all
 	 */
-	if (clk >= clkl + d) {
+	if (clk > clkl + d) {
 		clkh = clk - clkl - d;
 		clkl -= d;
 	} else {
-		clkh = 0;
+		clkh = 1;
 		clkl = clk - (d << 1);
 	}