Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58053 )
Change subject: soc/mediatek: Fix I2C failures by adjusting AC timing and bus speed ......................................................................
soc/mediatek: Fix I2C failures by adjusting AC timing and bus speed
1. The original algorithm for I2C speed cannot always make the timing meet I2C specification so a new algorithm is introduced to calculate the timing parameters more correctly. 2. Some I2C buses should be initialized in a different speed while the original implementation was fixed at fast mode (400Khz). So the mtk_i2c_bus_init is now also taking an extra speed parameter.
There is an equivalent change in kernel side: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/dr...
BUG=b:189899864 TEST=Test on Tomato, boot pass and timing pass at 100/300/400/500/800/1000Khz.
Signed-off-by: Daolong Zhu jg_daolongzhu@mediatek.corp-partner.google.com Change-Id: Id25b7bb3a76908a7943b940eb5bee799e80626a0 Reviewed-on: https://review.coreboot.org/c/coreboot/+/58053 Reviewed-by: Rex-BC Chen rex-bc.chen@mediatek.corp-partner.google.com Reviewed-by: Yu-Ping Wu yupingso@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/cherry/bootblock.c M src/mainboard/google/cherry/mainboard.c M src/mainboard/google/cherry/romstage.c M src/soc/mediatek/common/i2c.c M src/soc/mediatek/common/include/soc/i2c_common.h M src/soc/mediatek/mt8195/i2c.c M src/soc/mediatek/mt8195/include/soc/i2c.h 7 files changed, 341 insertions(+), 74 deletions(-)
Approvals: build bot (Jenkins): Verified Yu-Ping Wu: Looks good to me, approved Rex-BC Chen: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/cherry/bootblock.c b/src/mainboard/google/cherry/bootblock.c index dca2f13..c506caf 100644 --- a/src/mainboard/google/cherry/bootblock.c +++ b/src/mainboard/google/cherry/bootblock.c @@ -43,7 +43,7 @@
void bootblock_mainboard_init(void) { - mtk_i2c_bus_init(CONFIG_DRIVER_TPM_I2C_BUS); + mtk_i2c_bus_init(CONFIG_DRIVER_TPM_I2C_BUS, I2C_SPEED_FAST); mtk_spi_init(CONFIG_EC_GOOGLE_CHROMEEC_SPI_BUS, SPI_PAD0_MASK, 3 * MHz, 0); nor_set_gpio_pinmux(); setup_chromeos_gpios(); diff --git a/src/mainboard/google/cherry/mainboard.c b/src/mainboard/google/cherry/mainboard.c index 7b9ba65..a097050 100644 --- a/src/mainboard/google/cherry/mainboard.c +++ b/src/mainboard/google/cherry/mainboard.c @@ -131,7 +131,7 @@ MSDC1_GPIO_MODE1_2, MSDC1_GPIO_MODE1_VALUE, MSDC1_GPIO_MODE1_3, MSDC1_GPIO_MODE1_VALUE);
- mtk_i2c_bus_init(I2C7); + mtk_i2c_bus_init(I2C7, I2C_SPEED_FAST);
if (CONFIG(BOARD_GOOGLE_CHERRY)) mt6360_init(I2C7); @@ -205,7 +205,7 @@
/* for audio usage */ if (CONFIG(CHERRY_USE_RT1011)) - mtk_i2c_bus_init(I2C2); + mtk_i2c_bus_init(I2C2, I2C_SPEED_FAST);
if (dpm_init()) printk(BIOS_ERR, "dpm init failed, DVFS may not work\n"); diff --git a/src/mainboard/google/cherry/romstage.c b/src/mainboard/google/cherry/romstage.c index 71291c2..f375867 100644 --- a/src/mainboard/google/cherry/romstage.c +++ b/src/mainboard/google/cherry/romstage.c @@ -28,7 +28,7 @@ mt6359p_init(); mt6315_init(); raise_little_cpu_freq(); - mtk_i2c_bus_init(I2C7); + mtk_i2c_bus_init(I2C7, I2C_SPEED_FAST); if (CONFIG(BOARD_GOOGLE_CHERRY)) mt6360_init(I2C7); clk_buf_init(); diff --git a/src/soc/mediatek/common/i2c.c b/src/soc/mediatek/common/i2c.c index 3c54b17..1e713db 100644 --- a/src/soc/mediatek/common/i2c.c +++ b/src/soc/mediatek/common/i2c.c @@ -10,6 +10,39 @@ #include <soc/i2c.h> #include <device/i2c_simple.h>
+const struct i2c_spec_values standard_mode_spec = { + .min_low_ns = 4700 + I2C_STANDARD_MODE_BUFFER, + .min_su_sta_ns = 4700 + I2C_STANDARD_MODE_BUFFER, + .max_hd_dat_ns = 3450 - I2C_STANDARD_MODE_BUFFER, + .min_su_dat_ns = 250 + I2C_STANDARD_MODE_BUFFER, +}; + +const struct i2c_spec_values fast_mode_spec = { + .min_low_ns = 1300 + I2C_FAST_MODE_BUFFER, + .min_su_sta_ns = 600 + I2C_FAST_MODE_BUFFER, + .max_hd_dat_ns = 900 - I2C_FAST_MODE_BUFFER, + .min_su_dat_ns = 100 + I2C_FAST_MODE_BUFFER, +}; + +const struct i2c_spec_values fast_mode_plus_spec = { + .min_low_ns = 500 + I2C_FAST_MODE_PLUS_BUFFER, + .min_su_sta_ns = 260 + I2C_FAST_MODE_PLUS_BUFFER, + .max_hd_dat_ns = 400 - I2C_FAST_MODE_PLUS_BUFFER, + .min_su_dat_ns = 50 + I2C_FAST_MODE_PLUS_BUFFER, +}; + +__weak void mtk_i2c_dump_more_info(struct mt_i2c_regs *regs) { /* do nothing */ } + +const struct i2c_spec_values *mtk_i2c_get_spec(uint32_t speed) +{ + if (speed <= I2C_SPEED_STANDARD) + return &standard_mode_spec; + else if (speed <= I2C_SPEED_FAST) + return &fast_mode_spec; + else + return &fast_mode_plus_spec; +} + static inline void i2c_hw_reset(uint8_t bus) { struct mt_i2c_regs *regs; @@ -42,24 +75,26 @@
static inline void mtk_i2c_dump_info(struct mt_i2c_regs *regs) { - printk(BIOS_ERR, "I2C register:\nSLAVE_ADDR %x\nINTR_MASK %x\n" + printk(BIOS_DEBUG, "I2C register:\nSLAVE_ADDR %x\nINTR_MASK %x\n" "INTR_STAT %x\nCONTROL %x\nTRANSFER_LEN %x\nTRANSAC_LEN %x\n" "DELAY_LEN %x\nTIMING %x\nSTART %x\nFIFO_STAT %x\nIO_CONFIG %x\n" "HS %x\nDEBUGSTAT %x\nEXT_CONF %x\n", - read32(®s->slave_addr), - read32(®s->intr_mask), - read32(®s->intr_stat), - read32(®s->control), - read32(®s->transfer_len), - read32(®s->transac_len), - read32(®s->delay_len), - read32(®s->timing), - read32(®s->start), - read32(®s->fifo_stat), - read32(®s->io_config), - read32(®s->hs), - read32(®s->debug_stat), - read32(®s->ext_conf)); + read32(®s->slave_addr), + read32(®s->intr_mask), + read32(®s->intr_stat), + read32(®s->control), + read32(®s->transfer_len), + read32(®s->transac_len), + read32(®s->delay_len), + read32(®s->timing), + read32(®s->start), + read32(®s->fifo_stat), + read32(®s->io_config), + read32(®s->hs), + read32(®s->debug_stat), + read32(®s->ext_conf)); + + mtk_i2c_dump_more_info(regs); }
static uint32_t mtk_i2c_transfer(uint8_t bus, struct i2c_msg *seg, diff --git a/src/soc/mediatek/common/include/soc/i2c_common.h b/src/soc/mediatek/common/include/soc/i2c_common.h index d2da27e..72ec46a 100644 --- a/src/soc/mediatek/common/include/soc/i2c_common.h +++ b/src/soc/mediatek/common/include/soc/i2c_common.h @@ -3,6 +3,8 @@ #ifndef MTK_COMMON_I2C_H #define MTK_COMMON_I2C_H
+#include <device/i2c.h> + /* I2C DMA Registers */ struct mt_i2c_dma_regs { uint32_t dma_int_flag; @@ -84,7 +86,6 @@ };
/* I2C Status Code */ - enum { I2C_OK = 0x0000, I2C_SET_SPEED_FAIL_OVER_SPEED = 0xA001, @@ -95,11 +96,51 @@ I2C_TRANSFER_INVALID_ARGUMENT = 0xA006 };
+struct mtk_i2c_ac_timing { + u16 htiming; + u16 ltiming; + u16 hs; + u16 ext; + u16 inter_clk_div; + u16 scl_hl_ratio; + u16 hs_scl_hl_ratio; + u16 sta_stop; + u16 hs_sta_stop; + u16 sda_timing; +}; + struct mtk_i2c { struct mt_i2c_regs *i2c_regs; struct mt_i2c_dma_regs *i2c_dma_regs; + struct mtk_i2c_ac_timing ac_timing; uint32_t mt_i2c_flag; };
+#define I2C_TIME_CLR_VALUE 0x0000 +#define MAX_SAMPLE_CNT_DIV 8 +#define MAX_STEP_CNT_DIV 64 +#define MAX_HS_STEP_CNT_DIV 8 +#define I2C_TIME_DEFAULT_VALUE 0x0083 +#define I2C_STANDARD_MODE_BUFFER (1000 / 3) +#define I2C_FAST_MODE_BUFFER (300 / 3) +#define I2C_FAST_MODE_PLUS_BUFFER (20 / 3) + +/* + * struct i2c_spec_values: + * @min_low_ns: min LOW period of the SCL clock + * @min_su_sta_ns: min set-up time for a repeated START condition + * @max_hd_dat_ns: max data hold time + * @min_su_dat_ns: min data set-up time + */ +struct i2c_spec_values { + uint32_t min_low_ns; + uint32_t min_su_sta_ns; + uint32_t max_hd_dat_ns; + uint32_t min_su_dat_ns; +}; + extern struct mtk_i2c mtk_i2c_bus_controller[]; +const struct i2c_spec_values *mtk_i2c_get_spec(uint32_t speed); +void mtk_i2c_dump_more_info(struct mt_i2c_regs *regs); + #endif diff --git a/src/soc/mediatek/mt8195/i2c.c b/src/soc/mediatek/mt8195/i2c.c index 648083e..960eb12 100644 --- a/src/soc/mediatek/mt8195/i2c.c +++ b/src/soc/mediatek/mt8195/i2c.c @@ -1,16 +1,15 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <assert.h> +#include <console/console.h> #include <device/mmio.h> +#include <device/i2c_simple.h> #include <soc/pll.h> #include <soc/i2c.h> #include <soc/gpio.h> +#include <timer.h>
#define I2C_CLK_HZ (UNIVPLL_HZ / 20) -#define I2C_FULL_DUTY 100 -#define I2C_HALF_DUTY 50 -#define I2C_ADJUSTED_DUTY 50 -#define I2C_FS_START_CON 0x0
struct mtk_i2c mtk_i2c_bus_controller[] = { [0] = { @@ -112,57 +111,247 @@ } }
-static void mtk_i2c_speed_init(uint8_t bus) +static int mtk_i2c_max_step_cnt(uint32_t target_speed) { - uint8_t step_div; - const uint8_t clock_div = 5; - const uint8_t sample_div = 1; - uint32_t i2c_freq; - uint32_t tar_speed = 400; - uint32_t tar_speed_high; - uint32_t tar_speed_low; - - assert(bus < I2C_BUS_NUMBER); - - /* Adjust ratio of high/low level */ - tar_speed_high = tar_speed * I2C_HALF_DUTY / I2C_ADJUSTED_DUTY; - - /* Calculate i2c frequency */ - step_div = DIV_ROUND_UP(I2C_CLK_HZ, - (tar_speed_high * KHz * sample_div * 2) * clock_div); - i2c_freq = I2C_CLK_HZ / (step_div * sample_div * 2 * clock_div); - assert(sample_div < 8 && step_div < 64 && - i2c_freq <= tar_speed_high * KHz && - i2c_freq >= (tar_speed_high - 20) * KHz); - - /* Init i2c bus timing register */ - write32(&mtk_i2c_bus_controller[bus].i2c_regs->timing, - (sample_div - 1) << 8 | (step_div - 1)); - - /* Adjust ratio of high/low level */ - tar_speed_low = tar_speed * I2C_HALF_DUTY / - (I2C_FULL_DUTY - I2C_ADJUSTED_DUTY); - - /* Calculate i2c frequency */ - step_div = DIV_ROUND_UP(I2C_CLK_HZ, - (tar_speed_low * KHz * sample_div * 2) * clock_div); - i2c_freq = I2C_CLK_HZ / (step_div * sample_div * 2 * clock_div); - assert(sample_div < 8 && step_div < 64 && - i2c_freq <= tar_speed_low * KHz && - i2c_freq >= (tar_speed_low - 20) * KHz); - write32(&mtk_i2c_bus_controller[bus].i2c_regs->ltiming, - (sample_div - 1) << 6 | (step_div - 1)); - - /* Init i2c bus clock_div register */ - write32(&mtk_i2c_bus_controller[bus].i2c_regs->clock_div, - clock_div - 1); - - /* Adjust tSU,STA/tHD,STA/tSU,STO */ - write32(&mtk_i2c_bus_controller[bus].i2c_regs->ext_conf, I2C_FS_START_CON); + if (target_speed > I2C_SPEED_FAST_PLUS) + return MAX_HS_STEP_CNT_DIV; + else + return MAX_STEP_CNT_DIV; }
-void mtk_i2c_bus_init(uint8_t bus) +/* + * Check and calculate i2c ac-timing. + * + * Hardware design: + * sample_ns = (1000000000 * (sample_cnt + 1)) / clk_src + * xxx_cnt_div = spec->min_xxx_ns / sample_ns + * + * The calculation of sample_ns is rounded down; + * otherwise xxx_cnt_div would be greater than the smallest spec. + * The sda_timing is chosen as the middle value between + * the largest and smallest. + */ +static int mtk_i2c_check_ac_timing(uint8_t bus, uint32_t clk_src, + uint32_t check_speed, + uint32_t step_cnt, + uint32_t sample_cnt) { - mtk_i2c_speed_init(bus); + const struct i2c_spec_values *spec; + uint32_t su_sta_cnt, low_cnt, high_cnt, max_step_cnt; + uint32_t sda_max, sda_min, clk_ns, max_sta_cnt = 0x100; + uint32_t sample_ns = ((uint64_t)NSECS_PER_SEC * (sample_cnt + 1)) / clk_src; + struct mtk_i2c_ac_timing *ac_timing; + + spec = mtk_i2c_get_spec(check_speed); + + clk_ns = NSECS_PER_SEC / clk_src; + + su_sta_cnt = DIV_ROUND_UP(spec->min_su_sta_ns, clk_ns); + if (su_sta_cnt > max_sta_cnt) + return -1; + + low_cnt = DIV_ROUND_UP(spec->min_low_ns, sample_ns); + max_step_cnt = mtk_i2c_max_step_cnt(check_speed); + if (2 * step_cnt > low_cnt && low_cnt < max_step_cnt) { + if (low_cnt > step_cnt) { + high_cnt = 2 * step_cnt - low_cnt; + } else { + high_cnt = step_cnt; + low_cnt = step_cnt; + } + } else { + return -2; + } + + sda_max = spec->max_hd_dat_ns / sample_ns; + if (sda_max > low_cnt) + sda_max = 0; + + sda_min = DIV_ROUND_UP(spec->min_su_dat_ns, sample_ns); + if (sda_min < low_cnt) + sda_min = 0; + + if (sda_min > sda_max) + return -3; + + ac_timing = &mtk_i2c_bus_controller[bus].ac_timing; + if (check_speed > I2C_SPEED_FAST_PLUS) { + ac_timing->hs = I2C_TIME_DEFAULT_VALUE | (sample_cnt << 12) | (high_cnt << 8); + ac_timing->ltiming &= ~GENMASK(15, 9); + ac_timing->ltiming |= (sample_cnt << 12) | (low_cnt << 9); + ac_timing->ext &= ~GENMASK(7, 1); + ac_timing->ext |= (su_sta_cnt << 1) | (1 << 0); + } else { + ac_timing->htiming = (sample_cnt << 8) | (high_cnt); + ac_timing->ltiming = (sample_cnt << 6) | (low_cnt); + ac_timing->ext = (su_sta_cnt << 8) | (1 << 0); + } + + return 0; +} + +/* + * Calculate i2c port speed. + * + * Hardware design: + * i2c_bus_freq = parent_clk / (clock_div * 2 * sample_cnt * step_cnt) + * clock_div: fixed in hardware, but may be various in different SoCs + * + * To calculate sample_cnt and step_cnt, we pick the highest bus frequency + * that is still no larger than i2c->speed_hz. + */ +static int mtk_i2c_calculate_speed(uint8_t bus, uint32_t clk_src, + uint32_t target_speed, + uint32_t *timing_step_cnt, + uint32_t *timing_sample_cnt) +{ + uint32_t step_cnt; + uint32_t sample_cnt; + uint32_t max_step_cnt; + uint32_t base_sample_cnt = MAX_SAMPLE_CNT_DIV; + uint32_t base_step_cnt; + uint32_t opt_div; + uint32_t best_mul; + uint32_t cnt_mul; + uint32_t clk_div = mtk_i2c_bus_controller[bus].ac_timing.inter_clk_div; + int32_t clock_div_constraint = 0; + int success = 0; + + if (target_speed > I2C_SPEED_HIGH) + target_speed = I2C_SPEED_HIGH; + + max_step_cnt = mtk_i2c_max_step_cnt(target_speed); + base_step_cnt = max_step_cnt; + + /* Find the best combination */ + opt_div = DIV_ROUND_UP(clk_src >> 1, target_speed); + best_mul = MAX_SAMPLE_CNT_DIV * max_step_cnt; + + /* Search for the best pair (sample_cnt, step_cnt) with + * 0 < sample_cnt < MAX_SAMPLE_CNT_DIV + * 0 < step_cnt < max_step_cnt + * sample_cnt * step_cnt >= opt_div + * optimizing for sample_cnt * step_cnt being minimal + */ + for (sample_cnt = 1; sample_cnt <= MAX_SAMPLE_CNT_DIV; sample_cnt++) { + if (sample_cnt == 1) { + if (clk_div != 0) + clock_div_constraint = 1; + else + clock_div_constraint = 0; + } else { + if (clk_div > 1) + clock_div_constraint = 1; + else if (clk_div == 0) + clock_div_constraint = -1; + else + clock_div_constraint = 0; + } + + step_cnt = DIV_ROUND_UP(opt_div + clock_div_constraint, sample_cnt); + if (step_cnt > max_step_cnt) + continue; + + cnt_mul = step_cnt * sample_cnt; + if (cnt_mul >= best_mul) + continue; + + if (mtk_i2c_check_ac_timing(bus, clk_src, + target_speed, step_cnt - 1, + sample_cnt - 1)) + continue; + + success = 1; + best_mul = cnt_mul; + base_sample_cnt = sample_cnt; + base_step_cnt = step_cnt; + if (best_mul == opt_div + clock_div_constraint) + break; + + } + + if (!success) + return -1; + + sample_cnt = base_sample_cnt; + step_cnt = base_step_cnt; + + if (clk_src / (2 * (sample_cnt * step_cnt - clock_div_constraint)) > + target_speed) + return -1; + + *timing_step_cnt = step_cnt - 1; + *timing_sample_cnt = sample_cnt - 1; + + return 0; +} + +static void mtk_i2c_speed_init(uint8_t bus, uint32_t speed) +{ + uint32_t max_clk_div = MAX_CLOCK_DIV; + uint32_t clk_src, clk_div, step_cnt, sample_cnt; + uint32_t l_step_cnt, l_sample_cnt; + uint32_t timing_reg_value, ltiming_reg_value; + struct mtk_i2c *bus_ctrl; + + if (bus >= I2C_BUS_NUMBER) { + printk(BIOS_ERR, "%s, error bus num:%d\n", __func__, bus); + return; + } + + bus_ctrl = &mtk_i2c_bus_controller[bus]; + + for (clk_div = 1; clk_div <= max_clk_div; clk_div++) { + clk_src = I2C_CLK_HZ / clk_div; + bus_ctrl->ac_timing.inter_clk_div = clk_div - 1; + + if (speed > I2C_SPEED_FAST_PLUS) { + /* Set master code speed register */ + if (mtk_i2c_calculate_speed(bus, clk_src, I2C_SPEED_FAST, + &l_step_cnt, &l_sample_cnt)) + continue; + + timing_reg_value = (l_sample_cnt << 8) | l_step_cnt; + + /* Set the high speed mode register */ + if (mtk_i2c_calculate_speed(bus, clk_src, speed, + &step_cnt, &sample_cnt)) + continue; + + ltiming_reg_value = (l_sample_cnt << 6) | l_step_cnt | + (sample_cnt << 12) | (step_cnt << 9); + bus_ctrl->ac_timing.inter_clk_div = (clk_div - 1) << 8 | (clk_div - 1); + } else { + if (mtk_i2c_calculate_speed(bus, clk_src, speed, + &l_step_cnt, &l_sample_cnt)) + continue; + + timing_reg_value = (l_sample_cnt << 8) | l_step_cnt; + + /* Disable the high speed transaction */ + bus_ctrl->ac_timing.hs = I2C_TIME_CLR_VALUE; + + ltiming_reg_value = (l_sample_cnt << 6) | l_step_cnt; + } + + break; + } + + if (clk_div > max_clk_div) { + printk(BIOS_ERR, "%s, cannot support %d hz on i2c-%d\n", __func__, speed, bus); + return; + } + + /* Init i2c bus timing register */ + write32(&bus_ctrl->i2c_regs->clock_div, bus_ctrl->ac_timing.inter_clk_div); + write32(&bus_ctrl->i2c_regs->timing, bus_ctrl->ac_timing.htiming); + write32(&bus_ctrl->i2c_regs->ltiming, bus_ctrl->ac_timing.ltiming); + write32(&bus_ctrl->i2c_regs->hs, bus_ctrl->ac_timing.hs); + write32(&bus_ctrl->i2c_regs->ext_conf, bus_ctrl->ac_timing.ext); +} + +void mtk_i2c_bus_init(uint8_t bus, uint32_t speed) +{ + mtk_i2c_speed_init(bus, speed); mtk_i2c_set_gpio_pinmux(bus); } diff --git a/src/soc/mediatek/mt8195/include/soc/i2c.h b/src/soc/mediatek/mt8195/include/soc/i2c.h index 977106e..743faa1 100644 --- a/src/soc/mediatek/mt8195/include/soc/i2c.h +++ b/src/soc/mediatek/mt8195/include/soc/i2c.h @@ -22,7 +22,8 @@ uint32_t hs; uint32_t io_config; uint32_t fifo_addr_clr; - uint32_t reserved0[2]; + uint32_t data_timing; + uint32_t reserved0; uint32_t transfer_aux_len; uint32_t clock_div; uint32_t time_out; @@ -51,8 +52,9 @@ I2C7, };
+#define MAX_CLOCK_DIV 32 check_member(mt_i2c_regs, multi_dma, 0xf8c);
-void mtk_i2c_bus_init(uint8_t bus); +void mtk_i2c_bus_init(uint8_t bus, uint32_t speed);
#endif /* SOC_MEDIATEK_MT8195_I2C_H */