[PATCH 00/12] i2c: designware_ic2: Improvements to timing

This series updates the Designware I2C driver to support reading its timing from the device tree and handling it in units of nanoseconds instead of clock cycles.
A new function converts from nanoseconds to the units used by the I2C controller and makes sure that the requested bus speed is not exceeded. This is more accurate than the existing method.
The series includes a few smaller clean-ups in the same driver.
There is currently an existing configuration method used just for a few x86 boards (Baytrail). This method is retained but it should be removed in favour of using the device tree. I have not done this in this series since I am not sure of the timings to use.
Simon Glass (12): i2c: designware_i2c: Add more registers i2c: designware_i2c: Don't allow changing IC_CLK i2c: designware_i2c: Include clk.h in the header file i2c: designware_i2c: Rename 'max' speed to 'high' speed i2c: designware_i2c: Use an enum for selected speed mode i2c: designware_i2c: Use an accurate bus clock instead of MHz i2c: designware_i2c: Bring in the binding file i2c: designware_i2c: Read device-tree properties i2c: designware_i2c: Drop scl_sda_cfg parameter i2c: designware_i2c: Put hold config in a struct i2c: designware_i2c: Rewrite timing calculation i2c: designware_i2c: Add spike supression
.../i2c/i2c-designware.txt | 73 +++++ drivers/i2c/designware_i2c.c | 263 ++++++++++++++---- drivers/i2c/designware_i2c.h | 53 +++- drivers/i2c/designware_i2c_pci.c | 4 +- 4 files changed, 332 insertions(+), 61 deletions(-) create mode 100644 doc/device-tree-bindings/i2c/i2c-designware.txt

Some versions of this peripherals previde more control of the bus behaviour. Add definitions for these registers.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/i2c/designware_i2c.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/designware_i2c.h b/drivers/i2c/designware_i2c.h index 48766d0806..d359c8c3f8 100644 --- a/drivers/i2c/designware_i2c.h +++ b/drivers/i2c/designware_i2c.h @@ -43,8 +43,19 @@ struct i2c_regs { u32 ic_rxflr; /* 0x78 */ u32 ic_sda_hold; /* 0x7c */ u32 ic_tx_abrt_source; /* 0x80 */ - u8 res1[0x18]; /* 0x84 */ + u32 slv_data_nak_only; + u32 dma_cr; + u32 dma_tdlr; + u32 dma_rdlr; + u32 sda_setup; + u32 ack_general_call; u32 ic_enable_status; /* 0x9c */ + u32 fs_spklen; + u32 hs_spklen; + u32 clr_restart_det; + u32 comp_param1; + u32 comp_version; + u32 comp_type; };
#if !defined(IC_CLK)

-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Sunday, December 22, 2019 12:15 AM To: U-Boot Mailing List u-boot@lists.denx.de Cc: Jun Chen ptchentw@gmail.com; Heiko Schocher hs@denx.de; Tan, Ley Foon ley.foon.tan@intel.com; Simon Glass sjg@chromium.org Subject: [PATCH 01/12] i2c: designware_i2c: Add more registers
Some versions of this peripherals previde more control of the bus behaviour. Add definitions for these registers.
Signed-off-by: Simon Glass sjg@chromium.org
Typo for 'previde' in commit message.
Reviewed-by: Ley Foon Tan ley.foon.tan@intel.com
drivers/i2c/designware_i2c.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/designware_i2c.h b/drivers/i2c/designware_i2c.h index 48766d0806..d359c8c3f8 100644 --- a/drivers/i2c/designware_i2c.h +++ b/drivers/i2c/designware_i2c.h @@ -43,8 +43,19 @@ struct i2c_regs { u32 ic_rxflr; /* 0x78 */ u32 ic_sda_hold; /* 0x7c */ u32 ic_tx_abrt_source; /* 0x80 */
- u8 res1[0x18]; /* 0x84 */
- u32 slv_data_nak_only;
- u32 dma_cr;
- u32 dma_tdlr;
- u32 dma_rdlr;
- u32 sda_setup;
- u32 ack_general_call; u32 ic_enable_status; /* 0x9c */
- u32 fs_spklen;
- u32 hs_spklen;
- u32 clr_restart_det;
- u32 comp_param1;
- u32 comp_version;
- u32 comp_type;
};
#if !defined(IC_CLK)
2.24.1.735.g03f4e72817-goog

-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Sunday, December 22, 2019 12:15 AM To: U-Boot Mailing List u-boot@lists.denx.de Cc: Jun Chen ptchentw@gmail.com; Heiko Schocher hs@denx.de; Tan, Ley Foon ley.foon.tan@intel.com; Simon Glass sjg@chromium.org Subject: [PATCH 01/12] i2c: designware_i2c: Add more registers
Some versions of this peripherals previde more control of the bus
behaviour.
Add definitions for these registers.
Signed-off-by: Simon Glass sjg@chromium.org
Typo for 'previde' in commit message.
Reviewed-by: Ley Foon Tan ley.foon.tan@intel.com
Reviewed-by: Jun Chen ptchentw@gmail.com
drivers/i2c/designware_i2c.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/designware_i2c.h b/drivers/i2c/designware_i2c.h index 48766d0806..d359c8c3f8 100644 --- a/drivers/i2c/designware_i2c.h +++ b/drivers/i2c/designware_i2c.h @@ -43,8 +43,19 @@ struct i2c_regs { u32 ic_rxflr; /* 0x78 */ u32 ic_sda_hold; /* 0x7c */ u32 ic_tx_abrt_source; /* 0x80 */
u8 res1[0x18]; /* 0x84 */
u32 slv_data_nak_only;
u32 dma_cr;
u32 dma_tdlr;
u32 dma_rdlr;
u32 sda_setup;
u32 ack_general_call; u32 ic_enable_status; /* 0x9c */
u32 fs_spklen;
u32 hs_spklen;
u32 clr_restart_det;
u32 comp_param1;
u32 comp_version;
u32 comp_type;
};
#if !defined(IC_CLK)
2.24.1.735.g03f4e72817-goog

If a different input clock is required then the correct way to do this is with a clock driver. Don't allow boards to override IC_CLK.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/i2c/designware_i2c.h | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/i2c/designware_i2c.h b/drivers/i2c/designware_i2c.h index d359c8c3f8..0eb28191d2 100644 --- a/drivers/i2c/designware_i2c.h +++ b/drivers/i2c/designware_i2c.h @@ -58,9 +58,7 @@ struct i2c_regs { u32 comp_type; };
-#if !defined(IC_CLK) #define IC_CLK 166 -#endif #define NANO_TO_MICRO 1000
/* High and low times in different speed modes (in ns) */

We use struct clk here so really should include this header file to avoid build errors. Also switch the order of clk.h in the C file to match the required code style.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/i2c/designware_i2c.c | 2 +- drivers/i2c/designware_i2c.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index b8cdd1c661..138fc72561 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -4,8 +4,8 @@ * Vipin Kumar, ST Micoelectronics, vipin.kumar@st.com. */
-#include <clk.h> #include <common.h> +#include <clk.h> #include <dm.h> #include <i2c.h> #include <pci.h> diff --git a/drivers/i2c/designware_i2c.h b/drivers/i2c/designware_i2c.h index 0eb28191d2..39e62bcf51 100644 --- a/drivers/i2c/designware_i2c.h +++ b/drivers/i2c/designware_i2c.h @@ -7,6 +7,7 @@ #ifndef __DW_I2C_H_ #define __DW_I2C_H_
+#include <clk.h> #include <reset.h>
struct i2c_regs {

-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Sunday, December 22, 2019 12:15 AM To: U-Boot Mailing List u-boot@lists.denx.de Cc: Jun Chen ptchentw@gmail.com; Heiko Schocher hs@denx.de; Tan, Ley Foon ley.foon.tan@intel.com; Simon Glass sjg@chromium.org Subject: [PATCH 03/12] i2c: designware_i2c: Include clk.h in the header file
We use struct clk here so really should include this header file to avoid build errors. Also switch the order of clk.h in the C file to match the required code style.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Ley Foon Tan ley.foon.tan@intel.com
drivers/i2c/designware_i2c.c | 2 +- drivers/i2c/designware_i2c.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index b8cdd1c661..138fc72561 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -4,8 +4,8 @@
- Vipin Kumar, ST Micoelectronics, vipin.kumar@st.com.
*/
-#include <clk.h> #include <common.h> +#include <clk.h> #include <dm.h> #include <i2c.h> #include <pci.h> diff --git a/drivers/i2c/designware_i2c.h b/drivers/i2c/designware_i2c.h index 0eb28191d2..39e62bcf51 100644 --- a/drivers/i2c/designware_i2c.h +++ b/drivers/i2c/designware_i2c.h @@ -7,6 +7,7 @@ #ifndef __DW_I2C_H_ #define __DW_I2C_H_
+#include <clk.h> #include <reset.h>
struct i2c_regs {
2.24.1.735.g03f4e72817-goog

Some SoCs support a higher speed than what is currently called 'max' in this driver. Rename it to 'high' speed, which is the official name of the 3.4MHz speed.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/i2c/designware_i2c.c | 10 +++++----- drivers/i2c/designware_i2c.h | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index 138fc72561..dd1cc0b823 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -62,10 +62,10 @@ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, unsigned int ena; int i2c_spd;
- /* Allow max speed if there is no config, or the config allows it */ - if (speed >= I2C_MAX_SPEED && - (!scl_sda_cfg || scl_sda_cfg->has_max_speed)) - i2c_spd = IC_SPEED_MODE_MAX; + /* Allow high speed if there is no config, or the config allows it */ + if (speed >= I2C_HIGH_SPEED && + (!scl_sda_cfg || scl_sda_cfg->has_high_speed)) + i2c_spd = IC_SPEED_MODE_HIGH; else if (speed >= I2C_FAST_SPEED) i2c_spd = IC_SPEED_MODE_FAST; else @@ -80,7 +80,7 @@ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, cntl = (readl(&i2c_base->ic_con) & (~IC_CON_SPD_MSK));
switch (i2c_spd) { - case IC_SPEED_MODE_MAX: + case IC_SPEED_MODE_HIGH: cntl |= IC_CON_SPD_SS; if (scl_sda_cfg) { hcnt = scl_sda_cfg->fs_hcnt; diff --git a/drivers/i2c/designware_i2c.h b/drivers/i2c/designware_i2c.h index 39e62bcf51..fcb7dde83a 100644 --- a/drivers/i2c/designware_i2c.h +++ b/drivers/i2c/designware_i2c.h @@ -137,16 +137,16 @@ struct i2c_regs { /* Speed Selection */ #define IC_SPEED_MODE_STANDARD 1 #define IC_SPEED_MODE_FAST 2 -#define IC_SPEED_MODE_MAX 3 +#define IC_SPEED_MODE_HIGH 3
-#define I2C_MAX_SPEED 3400000 +#define I2C_HIGH_SPEED 3400000 #define I2C_FAST_SPEED 400000 #define I2C_STANDARD_SPEED 100000
/** * struct dw_scl_sda_cfg - I2C timing configuration * - * @has_max_speed: Support maximum speed (1Mbps) + * @has_high_speed: Support high speed (3.4Mbps) * @ss_hcnt: Standard speed high time in ns * @fs_hcnt: Fast speed high time in ns * @ss_lcnt: Standard speed low time in ns @@ -154,7 +154,7 @@ struct i2c_regs { * @sda_hold: SDA hold time */ struct dw_scl_sda_cfg { - bool has_max_speed; + bool has_high_speed; u32 ss_hcnt; u32 fs_hcnt; u32 ss_lcnt;

Group these #defines into an enum to make it easier to understand the relationship between them.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/i2c/designware_i2c.c | 2 +- drivers/i2c/designware_i2c.h | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index dd1cc0b823..2416ef32f9 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -57,10 +57,10 @@ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, unsigned int speed, unsigned int bus_mhz) { + enum i2c_speed_mode i2c_spd; unsigned int cntl; unsigned int hcnt, lcnt; unsigned int ena; - int i2c_spd;
/* Allow high speed if there is no config, or the config allows it */ if (speed >= I2C_HIGH_SPEED && diff --git a/drivers/i2c/designware_i2c.h b/drivers/i2c/designware_i2c.h index fcb7dde83a..06d794ca64 100644 --- a/drivers/i2c/designware_i2c.h +++ b/drivers/i2c/designware_i2c.h @@ -135,9 +135,13 @@ struct i2c_regs { #define IC_STATUS_ACT 0x0001
/* Speed Selection */ -#define IC_SPEED_MODE_STANDARD 1 -#define IC_SPEED_MODE_FAST 2 -#define IC_SPEED_MODE_HIGH 3 +enum i2c_speed_mode { + IC_SPEED_MODE_STANDARD, + IC_SPEED_MODE_FAST, + IC_SPEED_MODE_HIGH, + + IC_SPEED_MODE_COUNT, +};
#define I2C_HIGH_SPEED 3400000 #define I2C_FAST_SPEED 400000

At present the driver uses an approximation for the bus clock, e.g. 166MHz instead of 166 2/3 MHz.
This can result in small errors in the resulting I2C speed, perhaps 0.5% or so.
Adjust the existing code to start from the accurate figure, even if later rounding reduces this accuracy.
Update the bus speed code to work in KHz instead of MHz, which removes most of the error.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/i2c/designware_i2c.c | 18 ++++++++---------- drivers/i2c/designware_i2c.h | 4 ++-- 2 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index 2416ef32f9..0a1df8015f 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -55,8 +55,9 @@ static int dw_i2c_enable(struct i2c_regs *i2c_base, bool enable) static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, struct dw_scl_sda_cfg *scl_sda_cfg, unsigned int speed, - unsigned int bus_mhz) + unsigned int bus_clk) { + ulong bus_khz = bus_clk / 1000; enum i2c_speed_mode i2c_spd; unsigned int cntl; unsigned int hcnt, lcnt; @@ -86,8 +87,8 @@ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, hcnt = scl_sda_cfg->fs_hcnt; lcnt = scl_sda_cfg->fs_lcnt; } else { - hcnt = (bus_mhz * MIN_HS_SCL_HIGHTIME) / NANO_TO_MICRO; - lcnt = (bus_mhz * MIN_HS_SCL_LOWTIME) / NANO_TO_MICRO; + hcnt = (bus_khz * MIN_HS_SCL_HIGHTIME) / NANO_TO_KILO; + lcnt = (bus_khz * MIN_HS_SCL_LOWTIME) / NANO_TO_KILO; } writel(hcnt, &i2c_base->ic_hs_scl_hcnt); writel(lcnt, &i2c_base->ic_hs_scl_lcnt); @@ -99,8 +100,8 @@ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, hcnt = scl_sda_cfg->ss_hcnt; lcnt = scl_sda_cfg->ss_lcnt; } else { - hcnt = (bus_mhz * MIN_SS_SCL_HIGHTIME) / NANO_TO_MICRO; - lcnt = (bus_mhz * MIN_SS_SCL_LOWTIME) / NANO_TO_MICRO; + hcnt = (bus_khz * MIN_SS_SCL_HIGHTIME) / NANO_TO_KILO; + lcnt = (bus_khz * MIN_SS_SCL_LOWTIME) / NANO_TO_KILO; } writel(hcnt, &i2c_base->ic_ss_scl_hcnt); writel(lcnt, &i2c_base->ic_ss_scl_lcnt); @@ -113,8 +114,8 @@ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, hcnt = scl_sda_cfg->fs_hcnt; lcnt = scl_sda_cfg->fs_lcnt; } else { - hcnt = (bus_mhz * MIN_FS_SCL_HIGHTIME) / NANO_TO_MICRO; - lcnt = (bus_mhz * MIN_FS_SCL_LOWTIME) / NANO_TO_MICRO; + hcnt = (bus_khz * MIN_FS_SCL_HIGHTIME) / NANO_TO_KILO; + lcnt = (bus_khz * MIN_FS_SCL_LOWTIME) / NANO_TO_KILO; } writel(hcnt, &i2c_base->ic_fs_scl_hcnt); writel(lcnt, &i2c_base->ic_fs_scl_lcnt); @@ -511,9 +512,6 @@ static int designware_i2c_set_bus_speed(struct udevice *bus, unsigned int speed) rate = clk_get_rate(&i2c->clk); if (IS_ERR_VALUE(rate)) return -EINVAL; - - /* Convert to MHz */ - rate /= 1000000; #else rate = IC_CLK; #endif diff --git a/drivers/i2c/designware_i2c.h b/drivers/i2c/designware_i2c.h index 06d794ca64..1f9940c2ba 100644 --- a/drivers/i2c/designware_i2c.h +++ b/drivers/i2c/designware_i2c.h @@ -59,8 +59,8 @@ struct i2c_regs { u32 comp_type; };
-#define IC_CLK 166 -#define NANO_TO_MICRO 1000 +#define IC_CLK 166666666 +#define NANO_TO_KILO 1000000
/* High and low times in different speed modes (in ns) */ #define MIN_SS_SCL_HIGHTIME 4000

Bring in this file from Linux v5.4.
Signed-off-by: Simon Glass sjg@chromium.org ---
.../i2c/i2c-designware.txt | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 doc/device-tree-bindings/i2c/i2c-designware.txt
diff --git a/doc/device-tree-bindings/i2c/i2c-designware.txt b/doc/device-tree-bindings/i2c/i2c-designware.txt new file mode 100644 index 0000000000..be766be812 --- /dev/null +++ b/doc/device-tree-bindings/i2c/i2c-designware.txt @@ -0,0 +1,73 @@ +* Synopsys DesignWare I2C + +Required properties : + + - compatible : should be "snps,designware-i2c" + or "mscc,ocelot-i2c" with "snps,designware-i2c" for fallback + - reg : Offset and length of the register set for the device + - interrupts : <IRQ> where IRQ is the interrupt number. + - clocks : phandles for the clocks, see the description of clock-names below. + The phandle for the "ic_clk" clock is required. The phandle for the "pclk" + clock is optional. If a single clock is specified but no clock-name, it is + the "ic_clk" clock. If both clocks are listed, the "ic_clk" must be first. + +Recommended properties : + + - clock-frequency : desired I2C bus clock frequency in Hz. + +Optional properties : + + - clock-names : Contains the names of the clocks: + "ic_clk", for the core clock used to generate the external I2C clock. + "pclk", the interface clock, required for register access. + + - reg : for "mscc,ocelot-i2c", a second register set to configure the SDA hold + time, named ICPU_CFG:TWI_DELAY in the datasheet. + + - i2c-sda-hold-time-ns : should contain the SDA hold time in nanoseconds. + This option is only supported in hardware blocks version 1.11a or newer and + on Microsemi SoCs ("mscc,ocelot-i2c" compatible). + + - i2c-scl-falling-time-ns : should contain the SCL falling time in nanoseconds. + This value which is by default 300ns is used to compute the tLOW period. + + - i2c-sda-falling-time-ns : should contain the SDA falling time in nanoseconds. + This value which is by default 300ns is used to compute the tHIGH period. + +Examples : + + i2c@f0000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "snps,designware-i2c"; + reg = <0xf0000 0x1000>; + interrupts = <11>; + clock-frequency = <400000>; + }; + + i2c@1120000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "snps,designware-i2c"; + reg = <0x1120000 0x1000>; + interrupt-parent = <&ictl>; + interrupts = <12 1>; + clock-frequency = <400000>; + i2c-sda-hold-time-ns = <300>; + i2c-sda-falling-time-ns = <300>; + i2c-scl-falling-time-ns = <300>; + };x + + i2c@1120000 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x2000 0x100>; + clock-frequency = <400000>; + clocks = <&i2cclk>; + interrupts = <0>; + + eeprom@64 { + compatible = "linux,slave-24c02"; + reg = <0x40000064>; + }; + };

The i2c controller defines a few timing properties. Read these in and store them for use by the driver.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/i2c/designware_i2c.c | 8 ++++++-- drivers/i2c/designware_i2c.h | 15 +++++++++++++++ drivers/i2c/designware_i2c_pci.c | 2 +- 3 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index 0a1df8015f..34b6816545 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -535,11 +535,15 @@ static int designware_i2c_probe_chip(struct udevice *bus, uint chip_addr, return ret; }
-static int designware_i2c_ofdata_to_platdata(struct udevice *bus) +int designware_i2c_ofdata_to_platdata(struct udevice *bus) { struct dw_i2c *priv = dev_get_priv(bus);
- priv->regs = (struct i2c_regs *)devfdt_get_addr_ptr(bus); + if (!priv->regs) + priv->regs = (struct i2c_regs *)devfdt_get_addr_ptr(bus); + dev_read_u32(bus, "i2c-scl-rising-time-ns", &priv->scl_rise_time_ns); + dev_read_u32(bus, "i2c-scl-falling-time-ns", &priv->scl_fall_time_ns); + dev_read_u32(bus, "i2c-sda-hold-time-ns", &priv->sda_hold_time_ns);
return 0; } diff --git a/drivers/i2c/designware_i2c.h b/drivers/i2c/designware_i2c.h index 1f9940c2ba..55e440f0c0 100644 --- a/drivers/i2c/designware_i2c.h +++ b/drivers/i2c/designware_i2c.h @@ -166,10 +166,24 @@ struct dw_scl_sda_cfg { u32 sda_hold; };
+/** + * struct dw_i2c - private information for the bus + * + * @regs: Registers pointer + * @scl_sda_cfg: Deprecated information for x86 (should move to device tree) + * @resets: Resets for the I2C controller + * @scl_rise_time_ns: Configured SCL rise time in nanoseconds + * @scl_fall_time_ns: Configured SCL fall time in nanoseconds + * @sda_hold_time_ns: Configured SDA hold time in nanoseconds + * @clk: Clock input to the I2C controller + */ struct dw_i2c { struct i2c_regs *regs; struct dw_scl_sda_cfg *scl_sda_cfg; struct reset_ctl_bulk resets; + u32 scl_rise_time_ns; + u32 scl_fall_time_ns; + u32 sda_hold_time_ns; #if CONFIG_IS_ENABLED(CLK) struct clk clk; #endif @@ -179,5 +193,6 @@ extern const struct dm_i2c_ops designware_i2c_ops;
int designware_i2c_probe(struct udevice *bus); int designware_i2c_remove(struct udevice *dev); +int designware_i2c_ofdata_to_platdata(struct udevice *bus);
#endif /* __DW_I2C_H_ */ diff --git a/drivers/i2c/designware_i2c_pci.c b/drivers/i2c/designware_i2c_pci.c index 7f0625df66..2b974a07c3 100644 --- a/drivers/i2c/designware_i2c_pci.c +++ b/drivers/i2c/designware_i2c_pci.c @@ -63,7 +63,7 @@ static int designware_i2c_pci_ofdata_to_platdata(struct udevice *dev) /* Use BayTrail specific timing values */ priv->scl_sda_cfg = &byt_config;
- return 0; + return designware_i2c_ofdata_to_platdata(dev); }
static int designware_i2c_pci_probe(struct udevice *dev)

Instead of passing this parameter into __dw_i2c_set_bus_speed(), pass in the driver's private data, from which the function can obtain that information. This allows the function to have access to the full state of the driver.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/i2c/designware_i2c.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index 34b6816545..6e5545cd0c 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -52,17 +52,20 @@ static int dw_i2c_enable(struct i2c_regs *i2c_base, bool enable) * * Set the i2c speed. */ -static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base, - struct dw_scl_sda_cfg *scl_sda_cfg, +static unsigned int __dw_i2c_set_bus_speed(struct dw_i2c *priv, + struct i2c_regs *i2c_base, unsigned int speed, unsigned int bus_clk) { + const struct dw_scl_sda_cfg *scl_sda_cfg = NULL; ulong bus_khz = bus_clk / 1000; enum i2c_speed_mode i2c_spd; unsigned int cntl; unsigned int hcnt, lcnt; unsigned int ena;
+ if (priv) + scl_sda_cfg = priv->scl_sda_cfg; /* Allow high speed if there is no config, or the config allows it */ if (speed >= I2C_HIGH_SPEED && (!scl_sda_cfg || scl_sda_cfg->has_high_speed)) @@ -371,7 +374,7 @@ static int __dw_i2c_init(struct i2c_regs *i2c_base, int speed, int slaveaddr) writel(IC_TX_TL, &i2c_base->ic_tx_tl); writel(IC_STOP_DET, &i2c_base->ic_intr_mask); #ifndef CONFIG_DM_I2C - __dw_i2c_set_bus_speed(i2c_base, NULL, speed, IC_CLK); + __dw_i2c_set_bus_speed(NULL, i2c_base, speed, IC_CLK); writel(slaveaddr, &i2c_base->ic_sar); #endif
@@ -416,7 +419,7 @@ static unsigned int dw_i2c_set_bus_speed(struct i2c_adapter *adap, unsigned int speed) { adap->speed = speed; - return __dw_i2c_set_bus_speed(i2c_get_base(adap), NULL, speed, IC_CLK); + return __dw_i2c_set_bus_speed(NULL, i2c_get_base(adap), speed, IC_CLK); }
static void dw_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr) @@ -515,8 +518,7 @@ static int designware_i2c_set_bus_speed(struct udevice *bus, unsigned int speed) #else rate = IC_CLK; #endif - return __dw_i2c_set_bus_speed(i2c->regs, i2c->scl_sda_cfg, speed, - rate); + return __dw_i2c_set_bus_speed(i2c, i2c->regs, speed, rate); }
static int designware_i2c_probe_chip(struct udevice *bus, uint chip_addr,

Create a struct to hold the three timing parameters. This will make it easier to move these calculations into a separate function in a later patch.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/i2c/designware_i2c.c | 82 ++++++++++++++++++++++++------------ 1 file changed, 55 insertions(+), 27 deletions(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index 6e5545cd0c..e50987a717 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -13,6 +13,23 @@ #include <asm/io.h> #include "designware_i2c.h"
+/** + * struct dw_i2c_speed_config - timings to use for a particular speed + * + * This holds calculated values to be written to the I2C controller. Each value + * is represented as a number of IC clock cycles. + * + * @scl_lcnt: Low count value for SCL + * @scl_hcnt: High count value for SCL + * @sda_hold: Data hold count + */ +struct dw_i2c_speed_config { + /* SCL high and low period count */ + uint scl_lcnt; + uint scl_hcnt; + uint sda_hold; +}; + #ifdef CONFIG_SYS_I2C_DW_ENABLE_STATUS_UNSUPPORTED static int dw_i2c_enable(struct i2c_regs *i2c_base, bool enable) { @@ -58,10 +75,10 @@ static unsigned int __dw_i2c_set_bus_speed(struct dw_i2c *priv, unsigned int bus_clk) { const struct dw_scl_sda_cfg *scl_sda_cfg = NULL; + struct dw_i2c_speed_config config; ulong bus_khz = bus_clk / 1000; enum i2c_speed_mode i2c_spd; unsigned int cntl; - unsigned int hcnt, lcnt; unsigned int ena;
if (priv) @@ -83,53 +100,64 @@ static unsigned int __dw_i2c_set_bus_speed(struct dw_i2c *priv,
cntl = (readl(&i2c_base->ic_con) & (~IC_CON_SPD_MSK));
+ config.scl_hcnt = 0; + config.scl_lcnt = 0; + config.sda_hold = 0; + if (scl_sda_cfg) { + config.sda_hold = scl_sda_cfg->sda_hold; + if (i2c_spd == IC_SPEED_MODE_STANDARD) { + config.scl_hcnt = scl_sda_cfg->ss_hcnt; + config.scl_lcnt = scl_sda_cfg->ss_lcnt; + } else { + config.scl_hcnt = scl_sda_cfg->fs_hcnt; + config.scl_lcnt = scl_sda_cfg->fs_lcnt; + } + } + switch (i2c_spd) { case IC_SPEED_MODE_HIGH: cntl |= IC_CON_SPD_SS; - if (scl_sda_cfg) { - hcnt = scl_sda_cfg->fs_hcnt; - lcnt = scl_sda_cfg->fs_lcnt; - } else { - hcnt = (bus_khz * MIN_HS_SCL_HIGHTIME) / NANO_TO_KILO; - lcnt = (bus_khz * MIN_HS_SCL_LOWTIME) / NANO_TO_KILO; + if (!scl_sda_cfg) { + config.scl_hcnt = (bus_khz * MIN_HS_SCL_HIGHTIME) / + NANO_TO_KILO; + config.scl_lcnt = (bus_khz * MIN_HS_SCL_LOWTIME) / + NANO_TO_KILO; } - writel(hcnt, &i2c_base->ic_hs_scl_hcnt); - writel(lcnt, &i2c_base->ic_hs_scl_lcnt); + writel(config.scl_hcnt, &i2c_base->ic_hs_scl_hcnt); + writel(config.scl_lcnt, &i2c_base->ic_hs_scl_lcnt); break;
case IC_SPEED_MODE_STANDARD: cntl |= IC_CON_SPD_SS; - if (scl_sda_cfg) { - hcnt = scl_sda_cfg->ss_hcnt; - lcnt = scl_sda_cfg->ss_lcnt; - } else { - hcnt = (bus_khz * MIN_SS_SCL_HIGHTIME) / NANO_TO_KILO; - lcnt = (bus_khz * MIN_SS_SCL_LOWTIME) / NANO_TO_KILO; + if (!scl_sda_cfg) { + config.scl_hcnt = (bus_khz * MIN_SS_SCL_HIGHTIME) / + NANO_TO_KILO; + config.scl_lcnt = (bus_khz * MIN_SS_SCL_LOWTIME) / + NANO_TO_KILO; } - writel(hcnt, &i2c_base->ic_ss_scl_hcnt); - writel(lcnt, &i2c_base->ic_ss_scl_lcnt); + writel(config.scl_hcnt, &i2c_base->ic_ss_scl_hcnt); + writel(config.scl_lcnt, &i2c_base->ic_ss_scl_lcnt); break;
case IC_SPEED_MODE_FAST: default: cntl |= IC_CON_SPD_FS; - if (scl_sda_cfg) { - hcnt = scl_sda_cfg->fs_hcnt; - lcnt = scl_sda_cfg->fs_lcnt; - } else { - hcnt = (bus_khz * MIN_FS_SCL_HIGHTIME) / NANO_TO_KILO; - lcnt = (bus_khz * MIN_FS_SCL_LOWTIME) / NANO_TO_KILO; + if (!scl_sda_cfg) { + config.scl_hcnt = (bus_khz * MIN_FS_SCL_HIGHTIME) / + NANO_TO_KILO; + config.scl_lcnt = (bus_khz * MIN_FS_SCL_LOWTIME) / + NANO_TO_KILO; } - writel(hcnt, &i2c_base->ic_fs_scl_hcnt); - writel(lcnt, &i2c_base->ic_fs_scl_lcnt); + writel(config.scl_hcnt, &i2c_base->ic_fs_scl_hcnt); + writel(config.scl_lcnt, &i2c_base->ic_fs_scl_lcnt); break; }
writel(cntl, &i2c_base->ic_con);
/* Configure SDA Hold Time if required */ - if (scl_sda_cfg) - writel(scl_sda_cfg->sda_hold, &i2c_base->ic_sda_hold); + if (config.sda_hold) + writel(config.sda_hold, &i2c_base->ic_sda_hold);
/* Restore back i2c now speed set */ if (ena == IC_ENABLE_0B)

At present the driver can end up with timing parameters which are slightly faster than those expected. It is possible to optimise the parameters to get the best possible result.
Create a new function to handle the timing calculation. This uses a table of defaults for each speed mode rather than writing it in code.
The function works by calculating the 'period' of each bit on the bus in terms of the input clock to the controller (IC_CLK). It makes sure that the constraints are met and that the different components of that period add up correctly.
This code was taken from coreboot which has ended up with this same driver, but now in a much-different form.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/i2c/designware_i2c.c | 169 ++++++++++++++++++++++++++++++----- 1 file changed, 147 insertions(+), 22 deletions(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index e50987a717..0069602103 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -63,6 +63,147 @@ static int dw_i2c_enable(struct i2c_regs *i2c_base, bool enable) } #endif
+/* High and low times in different speed modes (in ns) */ +enum { + /* SDA Hold Time */ + DEFAULT_SDA_HOLD_TIME = 300, +}; + +/** + * calc_counts() - Convert a period to a number of IC clk cycles + * + * @ic_clk: Input clock in Hz + * @period_ns: Period to represent, in ns + * @return calculated count + */ +static uint calc_counts(uint ic_clk, uint period_ns) +{ + return DIV_ROUND_UP(ic_clk / 1000 * period_ns, NANO_TO_KILO); +} + +/** + * struct i2c_mode_info - Information about an I2C speed mode + * + * Each speed mode has its own characteristics. This struct holds these to aid + * calculations in dw_i2c_calc_timing(). + * + * @speed: Speed in Hz + * @min_scl_lowtime_ns: Minimum value for SCL low period in ns + * @min_scl_hightime_ns: Minimum value for SCL high period in ns + * @def_rise_time_ns: Default rise time in ns + * @def_fall_time_ns: Default fall time in ns + */ +struct i2c_mode_info { + int speed; + int min_scl_hightime_ns; + int min_scl_lowtime_ns; + int def_rise_time_ns; + int def_fall_time_ns; +}; + +static const struct i2c_mode_info info_for_mode[] = { + [IC_SPEED_MODE_STANDARD] = { + I2C_STANDARD_SPEED, + MIN_SS_SCL_HIGHTIME, + MIN_SS_SCL_LOWTIME, + 1000, + 300, + }, + [IC_SPEED_MODE_FAST] = { + I2C_FAST_SPEED, + MIN_FS_SCL_HIGHTIME, + MIN_FS_SCL_LOWTIME, + 300, + 300, + }, + [IC_SPEED_MODE_HIGH] = { + I2C_HIGH_SPEED, + MIN_HS_SCL_HIGHTIME, + MIN_HS_SCL_LOWTIME, + 120, + 120, + }, +}; + +/** + * dw_i2c_calc_timing() - Calculate the timings to use for a bus + * + * @priv: Bus private information (NULL if not using driver model) + * @mode: Speed mode to use + * @ic_clk: IC clock speed in Hz + * @spk_cnt: Spike-suppression count + * @config: Returns value to use + * @return 0 if OK, -EINVAL if the calculation failed due to invalid data + */ +static int dw_i2c_calc_timing(struct dw_i2c *priv, enum i2c_speed_mode mode, + int ic_clk, int spk_cnt, + struct dw_i2c_speed_config *config) +{ + int fall_cnt, rise_cnt, min_tlow_cnt, min_thigh_cnt; + int hcnt, lcnt, period_cnt, diff, tot; + int sda_hold_time_ns, scl_rise_time_ns, scl_fall_time_ns; + const struct i2c_mode_info *info; + + /* + * Find the period, rise, fall, min tlow, and min thigh in terms of + * counts of the IC clock + */ + info = &info_for_mode[mode]; + period_cnt = ic_clk / info->speed; + scl_rise_time_ns = priv && priv->scl_rise_time_ns ? + priv->scl_rise_time_ns : info->def_rise_time_ns; + scl_fall_time_ns = priv && priv->scl_fall_time_ns ? + priv->scl_fall_time_ns : info->def_fall_time_ns; + rise_cnt = calc_counts(ic_clk, scl_rise_time_ns); + fall_cnt = calc_counts(ic_clk, scl_fall_time_ns); + min_tlow_cnt = calc_counts(ic_clk, info->min_scl_lowtime_ns); + min_thigh_cnt = calc_counts(ic_clk, info->min_scl_hightime_ns); + + debug("dw_i2c: period %d rise %d fall %d tlow %d thigh %d spk %d\n", + period_cnt, rise_cnt, fall_cnt, min_tlow_cnt, min_thigh_cnt, + spk_cnt); + + /* + * Back-solve for hcnt and lcnt according to the following equations: + * SCL_High_time = [(HCNT + IC_*_SPKLEN + 7) * ic_clk] + SCL_Fall_time + * SCL_Low_time = [(LCNT + 1) * ic_clk] - SCL_Fall_time + SCL_Rise_time + */ + hcnt = min_thigh_cnt - fall_cnt - 7 - spk_cnt; + lcnt = min_tlow_cnt - rise_cnt + fall_cnt - 1; + + if (hcnt < 0 || lcnt < 0) { + debug("dw_i2c: bad counts. hcnt = %d lcnt = %d\n", hcnt, lcnt); + return -EINVAL; + } + + /* + * Now add things back up to ensure the period is hit. If it is off, + * split the difference and bias to lcnt for remainder + */ + tot = hcnt + lcnt + 7 + spk_cnt + rise_cnt + 1; + + if (tot < period_cnt) { + diff = (period_cnt - tot) / 2; + hcnt += diff; + lcnt += diff; + tot = hcnt + lcnt + 7 + spk_cnt + rise_cnt + 1; + lcnt += period_cnt - tot; + } + + config->scl_lcnt = lcnt; + config->scl_hcnt = hcnt; + + /* Use internal default unless other value is specified */ + sda_hold_time_ns = priv && priv->sda_hold_time_ns ? + priv->sda_hold_time_ns : DEFAULT_SDA_HOLD_TIME; + config->sda_hold = calc_counts(ic_clk, sda_hold_time_ns); + + debug("dw_i2c: hcnt = %d lcnt = %d sda hold = %d\n", hcnt, lcnt, + config->sda_hold); + + return 0; +} + /* * i2c_set_bus_speed - Set the i2c speed * @speed: required i2c speed @@ -76,10 +217,10 @@ static unsigned int __dw_i2c_set_bus_speed(struct dw_i2c *priv, { const struct dw_scl_sda_cfg *scl_sda_cfg = NULL; struct dw_i2c_speed_config config; - ulong bus_khz = bus_clk / 1000; enum i2c_speed_mode i2c_spd; unsigned int cntl; unsigned int ena; + int ret;
if (priv) scl_sda_cfg = priv->scl_sda_cfg; @@ -100,9 +241,6 @@ static unsigned int __dw_i2c_set_bus_speed(struct dw_i2c *priv,
cntl = (readl(&i2c_base->ic_con) & (~IC_CON_SPD_MSK));
- config.scl_hcnt = 0; - config.scl_lcnt = 0; - config.sda_hold = 0; if (scl_sda_cfg) { config.sda_hold = scl_sda_cfg->sda_hold; if (i2c_spd == IC_SPEED_MODE_STANDARD) { @@ -112,29 +250,22 @@ static unsigned int __dw_i2c_set_bus_speed(struct dw_i2c *priv, config.scl_hcnt = scl_sda_cfg->fs_hcnt; config.scl_lcnt = scl_sda_cfg->fs_lcnt; } + } else { + ret = dw_i2c_calc_timing(priv, i2c_spd, bus_clk, 0, + &config); + if (ret) + return log_msg_ret("gen_confg", ret); }
switch (i2c_spd) { case IC_SPEED_MODE_HIGH: cntl |= IC_CON_SPD_SS; - if (!scl_sda_cfg) { - config.scl_hcnt = (bus_khz * MIN_HS_SCL_HIGHTIME) / - NANO_TO_KILO; - config.scl_lcnt = (bus_khz * MIN_HS_SCL_LOWTIME) / - NANO_TO_KILO; - } writel(config.scl_hcnt, &i2c_base->ic_hs_scl_hcnt); writel(config.scl_lcnt, &i2c_base->ic_hs_scl_lcnt); break;
case IC_SPEED_MODE_STANDARD: cntl |= IC_CON_SPD_SS; - if (!scl_sda_cfg) { - config.scl_hcnt = (bus_khz * MIN_SS_SCL_HIGHTIME) / - NANO_TO_KILO; - config.scl_lcnt = (bus_khz * MIN_SS_SCL_LOWTIME) / - NANO_TO_KILO; - } writel(config.scl_hcnt, &i2c_base->ic_ss_scl_hcnt); writel(config.scl_lcnt, &i2c_base->ic_ss_scl_lcnt); break; @@ -142,12 +273,6 @@ static unsigned int __dw_i2c_set_bus_speed(struct dw_i2c *priv, case IC_SPEED_MODE_FAST: default: cntl |= IC_CON_SPD_FS; - if (!scl_sda_cfg) { - config.scl_hcnt = (bus_khz * MIN_FS_SCL_HIGHTIME) / - NANO_TO_KILO; - config.scl_lcnt = (bus_khz * MIN_FS_SCL_LOWTIME) / - NANO_TO_KILO; - } writel(config.scl_hcnt, &i2c_base->ic_fs_scl_hcnt); writel(config.scl_lcnt, &i2c_base->ic_fs_scl_lcnt); break;

Some versions of this peripheral include a spike-suppression phase of the bus. Add support for this.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/i2c/designware_i2c.c | 10 +++++++++- drivers/i2c/designware_i2c.h | 2 ++ drivers/i2c/designware_i2c_pci.c | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index 0069602103..4aee25c543 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -220,6 +220,7 @@ static unsigned int __dw_i2c_set_bus_speed(struct dw_i2c *priv, enum i2c_speed_mode i2c_spd; unsigned int cntl; unsigned int ena; + int spk_cnt; int ret;
if (priv) @@ -241,6 +242,13 @@ static unsigned int __dw_i2c_set_bus_speed(struct dw_i2c *priv,
cntl = (readl(&i2c_base->ic_con) & (~IC_CON_SPD_MSK));
+ /* Get the proper spike-suppression count based on target speed */ + if (!priv || !priv->has_spk_cnt) + spk_cnt = 0; + else if (i2c_spd >= IC_SPEED_MODE_HIGH) + spk_cnt = readl(&i2c_base->hs_spklen); + else + spk_cnt = readl(&i2c_base->fs_spklen); if (scl_sda_cfg) { config.sda_hold = scl_sda_cfg->sda_hold; if (i2c_spd == IC_SPEED_MODE_STANDARD) { @@ -251,7 +259,7 @@ static unsigned int __dw_i2c_set_bus_speed(struct dw_i2c *priv, config.scl_lcnt = scl_sda_cfg->fs_lcnt; } } else { - ret = dw_i2c_calc_timing(priv, i2c_spd, bus_clk, 0, + ret = dw_i2c_calc_timing(priv, i2c_spd, bus_clk, spk_cnt, &config); if (ret) return log_msg_ret("gen_confg", ret); diff --git a/drivers/i2c/designware_i2c.h b/drivers/i2c/designware_i2c.h index 55e440f0c0..2c572a9e1f 100644 --- a/drivers/i2c/designware_i2c.h +++ b/drivers/i2c/designware_i2c.h @@ -175,6 +175,7 @@ struct dw_scl_sda_cfg { * @scl_rise_time_ns: Configured SCL rise time in nanoseconds * @scl_fall_time_ns: Configured SCL fall time in nanoseconds * @sda_hold_time_ns: Configured SDA hold time in nanoseconds + * @has_spk_cnt: true if the spike-count register is present * @clk: Clock input to the I2C controller */ struct dw_i2c { @@ -184,6 +185,7 @@ struct dw_i2c { u32 scl_rise_time_ns; u32 scl_fall_time_ns; u32 sda_hold_time_ns; + bool has_spk_cnt; #if CONFIG_IS_ENABLED(CLK) struct clk clk; #endif diff --git a/drivers/i2c/designware_i2c_pci.c b/drivers/i2c/designware_i2c_pci.c index 2b974a07c3..50f03e3d90 100644 --- a/drivers/i2c/designware_i2c_pci.c +++ b/drivers/i2c/designware_i2c_pci.c @@ -62,6 +62,8 @@ static int designware_i2c_pci_ofdata_to_platdata(struct udevice *dev) if (IS_ENABLED(CONFIG_INTEL_BAYTRAIL)) /* Use BayTrail specific timing values */ priv->scl_sda_cfg = &byt_config; + if (dev_get_driver_data(dev) == INTEL_APL) + priv->has_spk_cnt = true;
return designware_i2c_ofdata_to_platdata(dev); }
participants (3)
-
pt
-
Simon Glass
-
Tan, Ley Foon