Skip to contentSkip to conversation

i2c: ti: Update SCLH and SCLL to be in sync with v4.9 Linux kernel

PATCH New Lukasz Majewski 2 participants 6 messages Mar 15, 2017 – Mar 27, 2017 I2C

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 i2c: ti: Update SCLH and SCLL to be in sync with v4.9 Linux kernel

v4.9 Linux release:
SHA1: 69973b830859bc6529a7a0468ba0d80ee5117826
in the ./drivers/i2c/busses/i2c-omap.c

recommends to use SCLH=5 and SCLL=7 values.
This patch sets them to default.

---
 drivers/i2c/omap24xx_i2c.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/i2c/omap24xx_i2c.h +5 -5
index 3dae295..8a4ae98 100644
@@ -121,17 +121,17 @@
  * scll_trim = 7
  * sclh_trim = 5
  *
- * The linux 2.6.30 kernel uses
- * scll_trim = 6
- * sclh_trim = 6
+ * The linux 4.9 kernel uses
+ * scll_trim = 7
+ * sclh_trim = 5
  *
  * These are the trim values for standard and fast speed
  */
 #ifndef I2C_FASTSPEED_SCLL_TRIM
-#define I2C_FASTSPEED_SCLL_TRIM		6
+#define I2C_FASTSPEED_SCLL_TRIM		7
 #endif
 #ifndef I2C_FASTSPEED_SCLH_TRIM
-#define I2C_FASTSPEED_SCLH_TRIM		6
+#define I2C_FASTSPEED_SCLH_TRIM		5
 #endif
 
 /* These are the trim values for high speed */
Signature · 1 line
2.1.4
#1 lore raw

PATCH 1/2 i2c: ti: Update SCLH and SCLL to be in sync with v4.9 Linux kernel

Dear All,

Quoted 6 lines

v4.9 Linux release:
SHA1: 69973b830859bc6529a7a0468ba0d80ee5117826
in the ./drivers/i2c/busses/i2c-omap.c

recommends to use SCLH=5 and SCLL=7 values.
This patch sets them to default.

Any comments on this?

Quoted 23 lines
---
 drivers/i2c/omap24xx_i2c.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/i2c/omap24xx_i2c.h +3 -3
index 3dae295..8a4ae98 100644
@@ -121,17 +121,17 @@
  * scll_trim = 7
  * sclh_trim = 5
  *
- * The linux 2.6.30 kernel uses
- * scll_trim = 6
- * sclh_trim = 6
+ * The linux 4.9 kernel uses
+ * scll_trim = 7
+ * sclh_trim = 5
  *
  * These are the trim values for standard and fast speed
  */

drivers/i2c/omap24xx_i2c.h line 137

Quoted 10 lines
drivers/i2c/omap24xx_i2c.h +2 -2
 #ifndef I2C_FASTSPEED_SCLL_TRIM
-#define I2C_FASTSPEED_SCLL_TRIM		6
+#define I2C_FASTSPEED_SCLL_TRIM		7
 #endif
 #ifndef I2C_FASTSPEED_SCLH_TRIM
-#define I2C_FASTSPEED_SCLH_TRIM		6
+#define I2C_FASTSPEED_SCLH_TRIM		5
 #endif
 
 /* These are the trim values for high speed */

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

#2 lore raw

PATCH 1/2 i2c: ti: Update SCLH and SCLL to be in sync with v4.9 Linux kernel

Hello Lukasz,

Am 27.03.2017 um 10:04 schrieb Lukasz Majewski:

Quoted 10 lines

Dear All,

Quoted 6 lines

v4.9 Linux release:
SHA1: 69973b830859bc6529a7a0468ba0d80ee5117826
in the ./drivers/i2c/busses/i2c-omap.c

recommends to use SCLH=5 and SCLL=7 values.
This patch sets them to default.

Any comments on this?

I am fine with it, it is in my queue for sending a pull request to Tom.

bye,
Heiko

Quoted 48 lines
Quoted 24 lines
---
  drivers/i2c/omap24xx_i2c.h | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)
drivers/i2c/omap24xx_i2c.h +3 -3
index 3dae295..8a4ae98 100644
@@ -121,17 +121,17 @@
   * scll_trim = 7
   * sclh_trim = 5
   *
- * The linux 2.6.30 kernel uses
- * scll_trim = 6
- * sclh_trim = 6
+ * The linux 4.9 kernel uses
+ * scll_trim = 7
+ * sclh_trim = 5
   *
   * These are the trim values for standard and fast speed
   */

drivers/i2c/omap24xx_i2c.h line 137

Quoted 10 lines
drivers/i2c/omap24xx_i2c.h +2 -2
  #ifndef I2C_FASTSPEED_SCLL_TRIM
-#define I2C_FASTSPEED_SCLL_TRIM		6
+#define I2C_FASTSPEED_SCLL_TRIM		7
  #endif
  #ifndef I2C_FASTSPEED_SCLH_TRIM
-#define I2C_FASTSPEED_SCLH_TRIM		6
+#define I2C_FASTSPEED_SCLH_TRIM		5
  #endif
 
  /* These are the trim values for high speed */

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

Signature · 2 lines
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#3 lore raw

PATCH 2/2 i2c: ti: Update method to calculate psc, sscl and ssch I2C parameters

In reply to Lukasz Majewski

This patch updates the way in which psc, sscl and ssch I2C parameters are calculated to be in sync with v4.9 Linux kernel
SHA1: 69973b830859bc6529a7a0468ba0d80ee5117826
in the ./drivers/i2c/busses/i2c-omap.c

The previous method was causing several issues:
- The internal I2C frequency (after prescaler) was far above recommended one (7 - 12 MHz [*]) - the current approach brings better noise suppression (as stated in Linux commit: SHA1: 84bf2c868f3ca996e5bb)

- The values calculated (psc, sscl and ssch) were far from optimal, which caused on the test platform (AM57xx) the I2C0 SCL signal low time (Fast Mode) of ~1.0us (the standard requires > 1.3 us).

[*] for AM57xx TRM SPRUHZ6G, Table 24,7
"HS I2C Register Values for Maximum I2C Bit Rates in I2C F/S, I2C HS Modes"

---
 drivers/i2c/omap24xx_i2c.c | 66 ++++++++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 25 deletions(-)
drivers/i2c/omap24xx_i2c.c +41 -25
index 0006343..26996e9 100644
@@ -64,36 +64,52 @@ struct omap_i2c {
 
 static int omap24_i2c_findpsc(u32 *pscl, u32 *psch, uint speed)
 {
-	unsigned int sampleclk, prescaler;
-	int fsscll, fssclh;
+	unsigned long internal_clk = 0, fclk;
+	unsigned int prescaler;
 
-	speed <<= 1;
-	prescaler = 0;
 	/*
-	 * some divisors may cause a precission loss, but shouldn't
-	 * be a big thing, because i2c_clk is then allready very slow.
+	 * This method is only called for Standard and Fast Mode speeds
+	 *
+	 * For some TI SoCs it is explicitly written in TRM (e,g, SPRUHZ6G,
+	 * page 5685, Table 24-7)
+	 * that the internal I2C clock (after prescaler) should be between
+	 * 7-12 MHz (at least for Fast Mode (FS)).
+	 *
+	 * Such approach is used in v4.9 Linux kernel in:
+	 * ./drivers/i2c/busses/i2c-omap.c (omap_i2c_init function).
 	 */
-	while (prescaler <= 0xFF) {
-		sampleclk = I2C_IP_CLK / (prescaler+1);
 
-		fsscll = sampleclk / speed;
-		fssclh = fsscll;
-		fsscll -= I2C_FASTSPEED_SCLL_TRIM;
-		fssclh -= I2C_FASTSPEED_SCLH_TRIM;
-
-		if (((fsscll > 0) && (fssclh > 0)) &&
-		    ((fsscll <= (255-I2C_FASTSPEED_SCLL_TRIM)) &&
-		    (fssclh <= (255-I2C_FASTSPEED_SCLH_TRIM)))) {
-			if (pscl)
-				*pscl = fsscll;
-			if (psch)
-				*psch = fssclh;
-
-			return prescaler;
-		}
-		prescaler++;
+	speed /= 1000; /* convert speed to kHz */
+
+	if (speed > 100)
+		internal_clk = 9600;
+	else
+		internal_clk = 4000;
+
+	fclk = I2C_IP_CLK / 1000;
+	prescaler = fclk / internal_clk;
+	prescaler = prescaler - 1;
+
+	if (speed > 100) {
+		unsigned long scl;
+
+		/* Fast mode */
+		scl = internal_clk / speed;
+		*pscl = scl - (scl / 3) - I2C_FASTSPEED_SCLL_TRIM;
+		*psch = (scl / 3) - I2C_FASTSPEED_SCLH_TRIM;
+	} else {
+		/* Standard mode */
+		*pscl = internal_clk / (speed * 2) - I2C_FASTSPEED_SCLL_TRIM;
+		*psch = internal_clk / (speed * 2) - I2C_FASTSPEED_SCLH_TRIM;
 	}
-	return -1;
+
+	debug("%s: speed [kHz]: %d psc: 0x%x sscl: 0x%x ssch: 0x%x\n",
+	      __func__, speed, prescaler, *pscl, *psch);
+
+	if (*pscl <= 0 || *psch <= 0 || prescaler <= 0)
+		return -EINVAL;
+
+	return prescaler;
 }
 
 /*
Signature · 1 line
2.1.4
#4 lore raw

PATCH 2/2 i2c: ti: Update method to calculate psc, sscl and ssch I2C parameters

Dear All,

Quoted 4 lines

This patch updates the way in which psc, sscl and ssch I2C parameters are calculated to be in sync with v4.9 Linux kernel
SHA1: 69973b830859bc6529a7a0468ba0d80ee5117826
in the ./drivers/i2c/busses/i2c-omap.c

Any comments on this?

Quoted 100 lines

The previous method was causing several issues:
- The internal I2C frequency (after prescaler) was far above recommended one (7 - 12 MHz [*]) - the current approach brings better noise suppression (as stated in Linux commit: SHA1:
84bf2c868f3ca996e5bb)

- The values calculated (psc, sscl and ssch) were far from optimal, which caused on the test platform (AM57xx) the I2C0 SCL signal low time (Fast Mode) of ~1.0us (the standard requires > 1.3 us).

[*] for AM57xx TRM SPRUHZ6G, Table 24,7
"HS I2C Register Values for Maximum I2C Bit Rates in I2C F/S, I2C HS Modes"

---
 drivers/i2c/omap24xx_i2c.c | 66

++++++++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 25 deletions(-)

drivers/i2c/omap24xx_i2c.c +2 -6
index 0006343..26996e9 100644
@@ -64,36 +64,52 @@ struct omap_i2c {
 
 static int omap24_i2c_findpsc(u32 *pscl, u32 *psch, uint speed)
 {
-	unsigned int sampleclk, prescaler;
-	int fsscll, fssclh;
+	unsigned long internal_clk = 0, fclk;
+	unsigned int prescaler;
 
-	speed <<= 1;
-	prescaler = 0;
 	/*
-	 * some divisors may cause a precission loss, but shouldn't
-	 * be a big thing, because i2c_clk is then allready very

slow.

drivers/i2c/omap24xx_i2c.c +1 -0
+	 * This method is only called for Standard and Fast Mode

speeds

drivers/i2c/omap24xx_i2c.c +2 -0
+	 *
+	 * For some TI SoCs it is explicitly written in TRM (e,g,

SPRUHZ6G,

drivers/i2c/omap24xx_i2c.c +2 -0
+	 * page 5685, Table 24-7)
+	 * that the internal I2C clock (after prescaler) should be

between

drivers/i2c/omap24xx_i2c.c +25 -18
+	 * 7-12 MHz (at least for Fast Mode (FS)).
+	 *
+	 * Such approach is used in v4.9 Linux kernel in:
+	 * ./drivers/i2c/busses/i2c-omap.c (omap_i2c_init function).
 	 */
-	while (prescaler <= 0xFF) {
-		sampleclk = I2C_IP_CLK / (prescaler+1);
 
-		fsscll = sampleclk / speed;
-		fssclh = fsscll;
-		fsscll -= I2C_FASTSPEED_SCLL_TRIM;
-		fssclh -= I2C_FASTSPEED_SCLH_TRIM;
-
-		if (((fsscll > 0) && (fssclh > 0)) &&
-		    ((fsscll <= (255-I2C_FASTSPEED_SCLL_TRIM)) &&
-		    (fssclh <= (255-I2C_FASTSPEED_SCLH_TRIM)))) {
-			if (pscl)
-				*pscl = fsscll;
-			if (psch)
-				*psch = fssclh;
-
-			return prescaler;
-		}
-		prescaler++;
+	speed /= 1000; /* convert speed to kHz */
+
+	if (speed > 100)
+		internal_clk = 9600;
+	else
+		internal_clk = 4000;
+
+	fclk = I2C_IP_CLK / 1000;
+	prescaler = fclk / internal_clk;
+	prescaler = prescaler - 1;
+
+	if (speed > 100) {
+		unsigned long scl;
+
+		/* Fast mode */
+		scl = internal_clk / speed;
+		*pscl = scl - (scl / 3) - I2C_FASTSPEED_SCLL_TRIM;
+		*psch = (scl / 3) - I2C_FASTSPEED_SCLH_TRIM;
+	} else {
+		/* Standard mode */
+		*pscl = internal_clk / (speed * 2) -

I2C_FASTSPEED_SCLL_TRIM;

drivers/i2c/omap24xx_i2c.c +1 -0
+		*psch = internal_clk / (speed * 2) -

I2C_FASTSPEED_SCLH_TRIM; }

drivers/i2c/omap24xx_i2c.c +2 -1
-	return -1;
+
+	debug("%s: speed [kHz]: %d psc: 0x%x sscl: 0x%x ssch:

0x%x\n",

drivers/i2c/omap24xx_i2c.c struct omap_i2c { · line 115

Quoted 9 lines
drivers/i2c/omap24xx_i2c.c +6 -0
+	      __func__, speed, prescaler, *pscl, *psch);
+
+	if (*pscl <= 0 || *psch <= 0 || prescaler <= 0)
+		return -EINVAL;
+
+	return prescaler;
 }
 
 /*

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

#5 lore raw

PATCH 2/2 i2c: ti: Update method to calculate psc, sscl and ssch I2C parameters

Hello Lukasz,

Am 27.03.2017 um 10:04 schrieb Lukasz Majewski:

Quoted 8 lines

Dear All,

Quoted 4 lines

This patch updates the way in which psc, sscl and ssch I2C parameters are calculated to be in sync with v4.9 Linux kernel
SHA1: 69973b830859bc6529a7a0468ba0d80ee5117826
in the ./drivers/i2c/busses/i2c-omap.c

Any comments on this?

I am fine with it, it is in my queue for sending a pull request to Tom.

bye,
Heiko

Quoted 124 lines
Quoted 101 lines

The previous method was causing several issues:
- The internal I2C frequency (after prescaler) was far above recommended one (7 - 12 MHz [*]) - the current approach brings better noise suppression (as stated in Linux commit: SHA1:
84bf2c868f3ca996e5bb)

- The values calculated (psc, sscl and ssch) were far from optimal, which caused on the test platform (AM57xx) the I2C0 SCL signal low time (Fast Mode) of ~1.0us (the standard requires > 1.3 us).

[*] for AM57xx TRM SPRUHZ6G, Table 24,7
"HS I2C Register Values for Maximum I2C Bit Rates in I2C F/S, I2C HS Modes"

---
  drivers/i2c/omap24xx_i2c.c | 66

++++++++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 25 deletions(-)

drivers/i2c/omap24xx_i2c.c +2 -6
index 0006343..26996e9 100644
@@ -64,36 +64,52 @@ struct omap_i2c {
 
  static int omap24_i2c_findpsc(u32 *pscl, u32 *psch, uint speed)
  {
-	unsigned int sampleclk, prescaler;
-	int fsscll, fssclh;
+	unsigned long internal_clk = 0, fclk;
+	unsigned int prescaler;
 
-	speed <<= 1;
-	prescaler = 0;
  	/*
-	 * some divisors may cause a precission loss, but shouldn't
-	 * be a big thing, because i2c_clk is then allready very

slow.

drivers/i2c/omap24xx_i2c.c +1 -0
+	 * This method is only called for Standard and Fast Mode

speeds

drivers/i2c/omap24xx_i2c.c +2 -0
+	 *
+	 * For some TI SoCs it is explicitly written in TRM (e,g,

SPRUHZ6G,

drivers/i2c/omap24xx_i2c.c +2 -0
+	 * page 5685, Table 24-7)
+	 * that the internal I2C clock (after prescaler) should be

between

drivers/i2c/omap24xx_i2c.c +25 -18
+	 * 7-12 MHz (at least for Fast Mode (FS)).
+	 *
+	 * Such approach is used in v4.9 Linux kernel in:
+	 * ./drivers/i2c/busses/i2c-omap.c (omap_i2c_init function).
  	 */
-	while (prescaler <= 0xFF) {
-		sampleclk = I2C_IP_CLK / (prescaler+1);
 
-		fsscll = sampleclk / speed;
-		fssclh = fsscll;
-		fsscll -= I2C_FASTSPEED_SCLL_TRIM;
-		fssclh -= I2C_FASTSPEED_SCLH_TRIM;
-
-		if (((fsscll > 0) && (fssclh > 0)) &&
-		    ((fsscll <= (255-I2C_FASTSPEED_SCLL_TRIM)) &&
-		    (fssclh <= (255-I2C_FASTSPEED_SCLH_TRIM)))) {
-			if (pscl)
-				*pscl = fsscll;
-			if (psch)
-				*psch = fssclh;
-
-			return prescaler;
-		}
-		prescaler++;
+	speed /= 1000; /* convert speed to kHz */
+
+	if (speed > 100)
+		internal_clk = 9600;
+	else
+		internal_clk = 4000;
+
+	fclk = I2C_IP_CLK / 1000;
+	prescaler = fclk / internal_clk;
+	prescaler = prescaler - 1;
+
+	if (speed > 100) {
+		unsigned long scl;
+
+		/* Fast mode */
+		scl = internal_clk / speed;
+		*pscl = scl - (scl / 3) - I2C_FASTSPEED_SCLL_TRIM;
+		*psch = (scl / 3) - I2C_FASTSPEED_SCLH_TRIM;
+	} else {
+		/* Standard mode */
+		*pscl = internal_clk / (speed * 2) -

I2C_FASTSPEED_SCLL_TRIM;

drivers/i2c/omap24xx_i2c.c +1 -0
+		*psch = internal_clk / (speed * 2) -

I2C_FASTSPEED_SCLH_TRIM; }

drivers/i2c/omap24xx_i2c.c +2 -1
-	return -1;
+
+	debug("%s: speed [kHz]: %d psc: 0x%x sscl: 0x%x ssch:

0x%x\n",

drivers/i2c/omap24xx_i2c.c struct omap_i2c { · line 115

Quoted 9 lines
drivers/i2c/omap24xx_i2c.c +6 -0
+	      __func__, speed, prescaler, *pscl, *psch);
+
+	if (*pscl <= 0 || *psch <= 0 || prescaler <= 0)
+		return -EINVAL;
+
+	return prescaler;
  }
 
  /*

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

Signature · 2 lines
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany