1. In order to facilitate the review, the two series of patches submitted
before have been integrated together.
2. Resubmit a patch to fixing the incorrect register value.
3. Add modify bus speed calculation formula patch
Kewei Xu (8):
dt-bindings: i2c: update bindings for MT8195 SoC
i2c: mediatek: Dump i2c/dma register when a timeout occurs
i2c: mediatek: fixing the incorrect register offset
i2c: mediatek: Reset the handshake signal between i2c and dma
i2c: mediatek: Add OFFSET_EXT_CONF setting back
dt-bindings: i2c: add attribute default-timing-adjust
i2c: mediatek: Isolate speed setting via dts for special devices
i2c: mediatek: modify bus speed calculation formula
.../devicetree/bindings/i2c/i2c-mt65xx.txt | 3 +
drivers/i2c/busses/i2c-mt65xx.c | 229 +++++++++++++++++++--
2 files changed, 217 insertions(+), 15 deletions(-)
--
1.9.1
Add a DT binding documentation for the MT8195 soc.
Signed-off-by: Kewei Xu <[email protected]>
---
Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt b/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt
index 7f0194f..7c4915bc 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt
@@ -15,6 +15,7 @@ Required properties:
"mediatek,mt8173-i2c": for MediaTek MT8173
"mediatek,mt8183-i2c": for MediaTek MT8183
"mediatek,mt8192-i2c": for MediaTek MT8192
+ "mediatek,mt8195-i2c", "mediatek,mt8192-i2c": for MediaTek MT8195
"mediatek,mt8516-i2c", "mediatek,mt2712-i2c": for MediaTek MT8516
- reg: physical base address of the controller and dma base, length of memory
mapped region.
--
1.9.1
The reason for the modification here is that the previous
offset information is incorrect, OFFSET_DEBUGSTAT = 0xE4 is
the correct value.
Signed-off-by: Kewei Xu <[email protected]>
---
drivers/i2c/busses/i2c-mt65xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
index 64acd96..e65a41e 100644
--- a/drivers/i2c/busses/i2c-mt65xx.c
+++ b/drivers/i2c/busses/i2c-mt65xx.c
@@ -195,7 +195,7 @@ enum I2C_REGS_OFFSET {
[OFFSET_SOFTRESET] = 0x50,
[OFFSET_MULTI_DMA] = 0x84,
[OFFSET_SCL_MIS_COMP_POINT] = 0x90,
- [OFFSET_DEBUGSTAT] = 0xe0,
+ [OFFSET_DEBUGSTAT] = 0xe4,
[OFFSET_DEBUGCTRL] = 0xe8,
[OFFSET_FIFO_STAT] = 0xf4,
[OFFSET_FIFO_THRESH] = 0xf8,
--
1.9.1
In the commit be5ce0e97cc7 ("i2c: mediatek: Add i2c ac-timing adjust
support"), we miss setting OFFSET_EXT_CONF register if
i2c->dev_comp->timing_adjust is false, now add it back.
Fixes: be5ce0e97cc7 ("i2c: mediatek: Add i2c ac-timing adjust support")
Signed-off-by: Kewei Xu <[email protected]>
---
drivers/i2c/busses/i2c-mt65xx.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
index ded94f9..fe3cea7 100644
--- a/drivers/i2c/busses/i2c-mt65xx.c
+++ b/drivers/i2c/busses/i2c-mt65xx.c
@@ -41,6 +41,8 @@
#define I2C_HANDSHAKE_RST 0x0020
#define I2C_FIFO_ADDR_CLR 0x0001
#define I2C_DELAY_LEN 0x0002
+#define I2C_ST_START_CON 0x8001
+#define I2C_FS_START_CON 0x1800
#define I2C_TIME_CLR_VALUE 0x0000
#define I2C_TIME_DEFAULT_VALUE 0x0003
#define I2C_WRRD_TRANAC_VALUE 0x0002
@@ -484,6 +486,7 @@ static void mtk_i2c_clock_disable(struct mtk_i2c *i2c)
static void mtk_i2c_init_hw(struct mtk_i2c *i2c)
{
u16 control_reg;
+ u16 ext_conf_val;
if (i2c->dev_comp->apdma_sync) {
writel(I2C_DMA_WARM_RST, i2c->pdmabase + OFFSET_RST);
@@ -518,8 +521,13 @@ static void mtk_i2c_init_hw(struct mtk_i2c *i2c)
if (i2c->dev_comp->ltiming_adjust)
mtk_i2c_writew(i2c, i2c->ltiming_reg, OFFSET_LTIMING);
+ if (i2c->speed_hz <= I2C_MAX_STANDARD_MODE_FREQ)
+ ext_conf_val = I2C_ST_START_CON;
+ else
+ ext_conf_val = I2C_FS_START_CON;
+
if (i2c->dev_comp->timing_adjust) {
- mtk_i2c_writew(i2c, i2c->ac_timing.ext, OFFSET_EXT_CONF);
+ ext_conf_val = i2c->ac_timing.ext;
mtk_i2c_writew(i2c, i2c->ac_timing.inter_clk_div,
OFFSET_CLOCK_DIV);
mtk_i2c_writew(i2c, I2C_SCL_MIS_COMP_VALUE,
@@ -544,6 +552,7 @@ static void mtk_i2c_init_hw(struct mtk_i2c *i2c)
OFFSET_HS_STA_STO_AC_TIMING);
}
}
+ mtk_i2c_writew(i2c, ext_conf_val, OFFSET_EXT_CONF);
/* If use i2c pin from PMIC mt6397 side, need set PATH_DIR first */
if (i2c->have_pmic)
--
1.9.1
Add attribute default-timing-adjust for DT-binding document.
Fixes: be5ce0e97cc7 ("i2c: mediatek: Add i2c ac-timing adjust support")
Signed-off-by: Kewei Xu <[email protected]>
---
Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt b/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt
index 7c4915bc..7b80a11 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt
@@ -33,6 +33,8 @@ Optional properties:
- mediatek,have-pmic: platform can control i2c form special pmic side.
Only mt6589 and mt8135 support this feature.
- mediatek,use-push-pull: IO config use push-pull mode.
+ - mediatek,default-timing-adjust: use default timing calculation, no timing
+ adjustment.
Example:
--
1.9.1
When clock-div is 0 or greater than 1, the bus speed
calculated by the old speed calculation formula will be
larger than the target speed. So we update the formula.
Signed-off-by: Kewei Xu <[email protected]>
---
drivers/i2c/busses/i2c-mt65xx.c | 32 ++++++++++++++++++++++++--------
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
index 486076f..4e43a79 100644
--- a/drivers/i2c/busses/i2c-mt65xx.c
+++ b/drivers/i2c/busses/i2c-mt65xx.c
@@ -68,11 +68,12 @@
#define I2C_DEFAULT_CLK_DIV 5
#define MAX_SAMPLE_CNT_DIV 8
#define MAX_STEP_CNT_DIV 64
-#define MAX_CLOCK_DIV 256
+#define MAX_CLOCK_DIV_8BITS 256
+#define MAX_CLOCK_DIV_5BITS 32
#define MAX_HS_STEP_CNT_DIV 8
-#define I2C_STANDARD_MODE_BUFFER (1000 / 2)
-#define I2C_FAST_MODE_BUFFER (300 / 2)
-#define I2C_FAST_MODE_PLUS_BUFFER (20 / 2)
+#define I2C_STANDARD_MODE_BUFFER (1000 / 3)
+#define I2C_FAST_MODE_BUFFER (300 / 3)
+#define I2C_FAST_MODE_PLUS_BUFFER (20 / 3)
#define I2C_CONTROL_RS (0x1 << 1)
#define I2C_CONTROL_DMA_EN (0x1 << 2)
@@ -719,14 +720,25 @@ static int mtk_i2c_calculate_speed(struct mtk_i2c *i2c, unsigned int clk_src,
unsigned int best_mul;
unsigned int cnt_mul;
int ret = -EINVAL;
+ int clock_div_constraint = 0;
if (target_speed > I2C_MAX_HIGH_SPEED_MODE_FREQ)
target_speed = I2C_MAX_HIGH_SPEED_MODE_FREQ;
+ if (i2c->default_timing_adjust) {
+ clock_div_constraint = 0;
+ } else if (i2c->dev_comp->ltiming_adjust &&
+ i2c->ac_timing.inter_clk_div > 1) {
+ clock_div_constraint = 1;
+ } else if (i2c->dev_comp->ltiming_adjust &&
+ i2c->ac_timing.inter_clk_div == 0) {
+ clock_div_constraint = -1;
+ }
+
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);
+ opt_div = DIV_ROUND_UP(clk_src >> 1, target_speed) + clock_div_constraint;
best_mul = MAX_SAMPLE_CNT_DIV * max_step_cnt;
/* Search for the best pair (sample_cnt, step_cnt) with
@@ -761,7 +773,8 @@ static int mtk_i2c_calculate_speed(struct mtk_i2c *i2c, unsigned int clk_src,
sample_cnt = base_sample_cnt;
step_cnt = base_step_cnt;
- if ((clk_src / (2 * sample_cnt * step_cnt)) > target_speed) {
+ if ((clk_src / (2 * (sample_cnt * step_cnt - clock_div_constraint))) >
+ target_speed) {
/* In this case, hardware can't support such
* low i2c_bus_freq
*/
@@ -846,13 +859,16 @@ static int mtk_i2c_set_speed_adjust_timing(struct mtk_i2c *i2c,
target_speed = i2c->speed_hz;
parent_clk /= i2c->clk_src_div;
- if (i2c->dev_comp->timing_adjust)
- max_clk_div = MAX_CLOCK_DIV;
+ if (i2c->dev_comp->timing_adjust && i2c->dev_comp->ltiming_adjust)
+ max_clk_div = MAX_CLOCK_DIV_5BITS;
+ else if (i2c->dev_comp->timing_adjust)
+ max_clk_div = MAX_CLOCK_DIV_8BITS;
else
max_clk_div = 1;
for (clk_div = 1; clk_div <= max_clk_div; clk_div++) {
clk_src = parent_clk / clk_div;
+ i2c->ac_timing.inter_clk_div = clk_div - 1;
if (target_speed > I2C_MAX_FAST_MODE_PLUS_FREQ) {
/* Set master code speed register */
--
1.9.1
When a timeout error occurs in i2c transter, it is usually related
to the i2c/dma IP hardware configuration. Therefore, the purpose of
this patch is to dump the key register values of i2c/dma when a
timeout occurs in i2c for debugging.
Signed-off-by: Kewei Xu <[email protected]>
---
drivers/i2c/busses/i2c-mt65xx.c | 95 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
index 5ddfa4e..64acd96 100644
--- a/drivers/i2c/busses/i2c-mt65xx.c
+++ b/drivers/i2c/busses/i2c-mt65xx.c
@@ -125,6 +125,7 @@ enum I2C_REGS_OFFSET {
OFFSET_HS,
OFFSET_SOFTRESET,
OFFSET_DCM_EN,
+ OFFSET_MULTI_DMA,
OFFSET_PATH_DIR,
OFFSET_DEBUGSTAT,
OFFSET_DEBUGCTRL,
@@ -192,6 +193,7 @@ enum I2C_REGS_OFFSET {
[OFFSET_TRANSFER_LEN_AUX] = 0x44,
[OFFSET_CLOCK_DIV] = 0x48,
[OFFSET_SOFTRESET] = 0x50,
+ [OFFSET_MULTI_DMA] = 0x84,
[OFFSET_SCL_MIS_COMP_POINT] = 0x90,
[OFFSET_DEBUGSTAT] = 0xe0,
[OFFSET_DEBUGCTRL] = 0xe8,
@@ -828,6 +830,96 @@ static int mtk_i2c_set_speed(struct mtk_i2c *i2c, unsigned int parent_clk)
return 0;
}
+static void i2c_dump_register(struct mtk_i2c *i2c)
+{
+ dev_err(i2c->dev, "SLAVE_ADDR[0x%x]: 0x%x, INTR_MASK[0x%x]: 0x%x\n",
+ OFFSET_SLAVE_ADDR,
+ (mtk_i2c_readw(i2c, OFFSET_SLAVE_ADDR)),
+ OFFSET_INTR_MASK,
+ (mtk_i2c_readw(i2c, OFFSET_INTR_MASK)));
+ dev_err(i2c->dev, "INTR_STAT[0x%x]: 0x%x, CONTROL[0x%x]: 0x%x\n",
+ OFFSET_INTR_STAT,
+ (mtk_i2c_readw(i2c, OFFSET_INTR_STAT)),
+ OFFSET_CONTROL,
+ (mtk_i2c_readw(i2c, OFFSET_CONTROL)));
+ dev_err(i2c->dev, "TRANSFER_LEN[0x%x]: 0x%x, TRANSAC_LEN[0x%x]: 0x%x\n",
+ OFFSET_TRANSFER_LEN,
+ (mtk_i2c_readw(i2c, OFFSET_TRANSFER_LEN)),
+ OFFSET_TRANSAC_LEN,
+ (mtk_i2c_readw(i2c, OFFSET_TRANSAC_LEN)));
+ dev_err(i2c->dev, "DELAY_LEN[0x%x]: 0x%x, HTIMING[0x%x]: 0x%x\n",
+ OFFSET_DELAY_LEN,
+ (mtk_i2c_readw(i2c, OFFSET_DELAY_LEN)),
+ OFFSET_TIMING,
+ (mtk_i2c_readw(i2c, OFFSET_TIMING)));
+ dev_err(i2c->dev, "OFFSET_START[0x%x]: 0x%x\n",
+ OFFSET_START,
+ mtk_i2c_readw(i2c, OFFSET_START));
+ dev_err(i2c->dev, "OFFSET_EXT_CONF[0x%x]: 0x%x\n",
+ OFFSET_EXT_CONF,
+ mtk_i2c_readw(i2c, OFFSET_EXT_CONF));
+ dev_err(i2c->dev, "OFFSET_HS[0x%x]: 0x%x\n",
+ OFFSET_HS,
+ mtk_i2c_readw(i2c, OFFSET_HS));
+ dev_err(i2c->dev, "OFFSET_IO_CONFIG[0x%x]: 0x%x\n",
+ OFFSET_IO_CONFIG,
+ mtk_i2c_readw(i2c, OFFSET_IO_CONFIG));
+ dev_err(i2c->dev, "OFFSET_FIFO_ADDR_CLR[0x%x]: 0x%x\n",
+ OFFSET_FIFO_ADDR_CLR,
+ mtk_i2c_readw(i2c, OFFSET_FIFO_ADDR_CLR));
+ dev_err(i2c->dev, "TRANSFER_LEN_AUX[0x%x]: 0x%x\n",
+ OFFSET_TRANSFER_LEN_AUX,
+ mtk_i2c_readw(i2c, OFFSET_TRANSFER_LEN_AUX));
+ dev_err(i2c->dev, "CLOCK_DIV[0x%x]: 0x%x\n",
+ OFFSET_CLOCK_DIV,
+ mtk_i2c_readw(i2c, OFFSET_CLOCK_DIV));
+ dev_err(i2c->dev, "FIFO_STAT[0x%x]: 0x%x, FIFO_THRESH[0x%x]: 0x%x\n",
+ OFFSET_FIFO_STAT,
+ mtk_i2c_readw(i2c, OFFSET_FIFO_STAT),
+ OFFSET_FIFO_THRESH,
+ mtk_i2c_readw(i2c, OFFSET_FIFO_THRESH));
+ dev_err(i2c->dev, "DCM_EN[0x%x] 0x%x\n",
+ OFFSET_DCM_EN,
+ mtk_i2c_readw(i2c, OFFSET_DCM_EN));
+ dev_err(i2c->dev, "DEBUGSTAT[0x%x]: 0x%x, DEBUGCTRL[0x%x]: 0x%x\n",
+ OFFSET_DEBUGSTAT,
+ (mtk_i2c_readw(i2c, OFFSET_DEBUGSTAT)),
+ OFFSET_DEBUGCTRL,
+ (mtk_i2c_readw(i2c, OFFSET_DEBUGCTRL)));
+
+ if (i2c->dev_comp->regs == mt_i2c_regs_v2) {
+ dev_err(i2c->dev, "OFFSET_LTIMING[0x%x]: 0x%x\n",
+ OFFSET_LTIMING,
+ mtk_i2c_readw(i2c, OFFSET_LTIMING));
+ dev_err(i2c->dev, "MULTI_DMA[0x%x]: 0x%x\n",
+ OFFSET_MULTI_DMA,
+ (mtk_i2c_readw(i2c, OFFSET_MULTI_DMA)));
+ }
+
+ dev_err(i2c->dev, "OFFSET_INT_FLAG = 0x%x\n",
+ readl(i2c->pdmabase + OFFSET_INT_FLAG));
+ dev_err(i2c->dev, "OFFSET_INT_EN = 0x%x\n",
+ readl(i2c->pdmabase + OFFSET_INT_EN));
+ dev_err(i2c->dev, "OFFSET_EN = 0x%x\n",
+ readl(i2c->pdmabase + OFFSET_EN));
+ dev_err(i2c->dev, "OFFSET_RST = 0x%x\n",
+ readl(i2c->pdmabase + OFFSET_RST));
+ dev_err(i2c->dev, "OFFSET_CON = 0x%x\n",
+ readl(i2c->pdmabase + OFFSET_CON));
+ dev_err(i2c->dev, "OFFSET_TX_MEM_ADDR = 0x%x\n",
+ readl(i2c->pdmabase + OFFSET_TX_MEM_ADDR));
+ dev_err(i2c->dev, "OFFSET_RX_MEM_ADDR = 0x%x\n",
+ readl(i2c->pdmabase + OFFSET_RX_MEM_ADDR));
+ dev_err(i2c->dev, "OFFSET_TX_LEN = 0x%x\n",
+ readl(i2c->pdmabase + OFFSET_TX_LEN));
+ dev_err(i2c->dev, "OFFSET_RX_LEN = 0x%x\n",
+ readl(i2c->pdmabase + OFFSET_RX_LEN));
+ dev_err(i2c->dev, "OFFSET_TX_4G_MODE = 0x%x\n",
+ readl(i2c->pdmabase + OFFSET_TX_4G_MODE));
+ dev_err(i2c->dev, "OFFSET_RX_4G_MODE = 0x%x\n",
+ readl(i2c->pdmabase + OFFSET_RX_4G_MODE));
+}
+
static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
int num, int left_num)
{
@@ -1034,7 +1126,8 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
}
if (ret == 0) {
- dev_dbg(i2c->dev, "addr: %x, transfer timeout\n", msgs->addr);
+ dev_err(i2c->dev, "addr: %x, transfer timeout\n", msgs->addr);
+ i2c_dump_register(i2c);
mtk_i2c_init_hw(i2c);
return -ETIMEDOUT;
}
--
1.9.1
In the commit be5ce0e97cc7 ("i2c: mediatek: Add i2c ac-timing adjust
support"), the I2C timing calculation has been revised to support
ac-timing adjustment, however that will break on some I2C components.
As a result we want to introduce a new setting "default-adjust-timing"
so those components can choose to use the old (default) timing algorithm.
Fixes: be5ce0e97cc7 ("i2c: mediatek: Add i2c ac-timing adjust support")
Signed-off-by: Kewei Xu <[email protected]>
---
drivers/i2c/busses/i2c-mt65xx.c | 75 ++++++++++++++++++++++++++++++++++++++---
1 file changed, 71 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
index fe3cea7..486076f 100644
--- a/drivers/i2c/busses/i2c-mt65xx.c
+++ b/drivers/i2c/busses/i2c-mt65xx.c
@@ -65,6 +65,7 @@
#define I2C_DMA_HARD_RST 0x0002
#define I2C_DMA_HANDSHAKE_RST 0x0004
+#define I2C_DEFAULT_CLK_DIV 5
#define MAX_SAMPLE_CNT_DIV 8
#define MAX_STEP_CNT_DIV 64
#define MAX_CLOCK_DIV 256
@@ -249,6 +250,7 @@ struct mtk_i2c {
struct clk *clk_arb; /* Arbitrator clock for i2c */
bool have_pmic; /* can use i2c pins from PMIC */
bool use_push_pull; /* IO config push-pull mode */
+ bool default_timing_adjust; /* no timing adjust mode */
u16 irq_stat; /* interrupt status */
unsigned int clk_src_div;
@@ -526,7 +528,11 @@ static void mtk_i2c_init_hw(struct mtk_i2c *i2c)
else
ext_conf_val = I2C_FS_START_CON;
- if (i2c->dev_comp->timing_adjust) {
+ if (i2c->default_timing_adjust) {
+ if (i2c->dev_comp->timing_adjust)
+ mtk_i2c_writew(i2c, I2C_DEFAULT_CLK_DIV - 1,
+ OFFSET_CLOCK_DIV);
+ } else if (i2c->dev_comp->timing_adjust) {
ext_conf_val = i2c->ac_timing.ext;
mtk_i2c_writew(i2c, i2c->ac_timing.inter_clk_div,
OFFSET_CLOCK_DIV);
@@ -609,7 +615,7 @@ static int mtk_i2c_check_ac_timing(struct mtk_i2c *i2c,
unsigned int sample_ns = div_u64(1000000000ULL * (sample_cnt + 1),
clk_src);
- if (!i2c->dev_comp->timing_adjust)
+ if (i2c->default_timing_adjust || !i2c->dev_comp->timing_adjust)
return 0;
if (i2c->dev_comp->ltiming_adjust)
@@ -769,7 +775,63 @@ static int mtk_i2c_calculate_speed(struct mtk_i2c *i2c, unsigned int clk_src,
return 0;
}
-static int mtk_i2c_set_speed(struct mtk_i2c *i2c, unsigned int parent_clk)
+static int mtk_i2c_set_speed_default_timing(struct mtk_i2c *i2c, unsigned int parent_clk)
+{
+ unsigned int clk_src;
+ unsigned int step_cnt;
+ unsigned int sample_cnt;
+ unsigned int l_step_cnt;
+ unsigned int l_sample_cnt;
+ unsigned int target_speed;
+ int ret;
+
+ if (i2c->dev_comp->timing_adjust)
+ i2c->clk_src_div *= I2C_DEFAULT_CLK_DIV;
+
+ clk_src = parent_clk / i2c->clk_src_div;
+ target_speed = i2c->speed_hz;
+
+ if (target_speed > I2C_MAX_FAST_MODE_PLUS_FREQ) {
+ /* Set master code speed register */
+ ret = mtk_i2c_calculate_speed(i2c, clk_src, I2C_MAX_FAST_MODE_FREQ,
+ &l_step_cnt, &l_sample_cnt);
+ if (ret < 0)
+ return ret;
+
+ i2c->timing_reg = (l_sample_cnt << 8) | l_step_cnt;
+
+ /* Set the high speed mode register */
+ ret = mtk_i2c_calculate_speed(i2c, clk_src, target_speed,
+ &step_cnt, &sample_cnt);
+ if (ret < 0)
+ return ret;
+
+ i2c->high_speed_reg = I2C_TIME_DEFAULT_VALUE |
+ (sample_cnt << 12) | (step_cnt << 8);
+
+ if (i2c->dev_comp->ltiming_adjust)
+ i2c->ltiming_reg = (l_sample_cnt << 6) | l_step_cnt |
+ (sample_cnt << 12) | (step_cnt << 9);
+ } else {
+ ret = mtk_i2c_calculate_speed(i2c, clk_src, target_speed,
+ &step_cnt, &sample_cnt);
+ if (ret < 0)
+ return ret;
+
+ i2c->timing_reg = (sample_cnt << 8) | step_cnt;
+
+ /* Disable the high speed transaction */
+ i2c->high_speed_reg = I2C_TIME_CLR_VALUE;
+
+ if (i2c->dev_comp->ltiming_adjust)
+ i2c->ltiming_reg = (sample_cnt << 6) | step_cnt;
+ }
+
+ return 0;
+}
+
+static int mtk_i2c_set_speed_adjust_timing(struct mtk_i2c *i2c,
+ unsigned int parent_clk)
{
unsigned int clk_src;
unsigned int step_cnt;
@@ -1293,6 +1355,8 @@ static int mtk_i2c_parse_dt(struct device_node *np, struct mtk_i2c *i2c)
i2c->have_pmic = of_property_read_bool(np, "mediatek,have-pmic");
i2c->use_push_pull =
of_property_read_bool(np, "mediatek,use-push-pull");
+ i2c->default_timing_adjust =
+ of_property_read_bool(np, "mediatek,default-timing-adjust");
i2c_parse_fw_timings(i2c->dev, &i2c->timing_info, true);
@@ -1372,7 +1436,10 @@ static int mtk_i2c_probe(struct platform_device *pdev)
strlcpy(i2c->adap.name, I2C_DRV_NAME, sizeof(i2c->adap.name));
- ret = mtk_i2c_set_speed(i2c, clk_get_rate(clk));
+ if (i2c->default_timing_adjust)
+ ret = mtk_i2c_set_speed_default_timing(i2c, clk_get_rate(clk));
+ else
+ ret = mtk_i2c_set_speed_adjust_timing(i2c, clk_get_rate(clk));
if (ret) {
dev_err(&pdev->dev, "Failed to set the speed.\n");
return -EINVAL;
--
1.9.1
Hi,
On Thu, Jul 15, 2021 at 10:37 AM Kewei Xu <[email protected]> wrote:
>
> 1. In order to facilitate the review, the two series of patches submitted
> before have been integrated together.
I'm not sure that really helps. We'll see.
> 2. Resubmit a patch to fixing the incorrect register value.
Fixes can be submitted separately, or at the very least, put at the very
front of the series.
> 3. Add modify bus speed calculation formula patch
When resubmitting patch series, please add appropriate versioning and
changelogs. This will help maintainers understand that this is a new
version of the series, and what has changed. This includes times when
the original patches weren't changed, but additional patches were added.
Please also keep the original series subject, which IIRC was about adding
support for MT8195 I2C. The subject you now used should be part of the
cover letter.
If you combine different patch series, you should use the highest version
number + 1.
Regards
ChenYu
>
> Kewei Xu (8):
> dt-bindings: i2c: update bindings for MT8195 SoC
> i2c: mediatek: Dump i2c/dma register when a timeout occurs
> i2c: mediatek: fixing the incorrect register offset
> i2c: mediatek: Reset the handshake signal between i2c and dma
> i2c: mediatek: Add OFFSET_EXT_CONF setting back
> dt-bindings: i2c: add attribute default-timing-adjust
> i2c: mediatek: Isolate speed setting via dts for special devices
> i2c: mediatek: modify bus speed calculation formula
>
> .../devicetree/bindings/i2c/i2c-mt65xx.txt | 3 +
> drivers/i2c/busses/i2c-mt65xx.c | 229 +++++++++++++++++++--
> 2 files changed, 217 insertions(+), 15 deletions(-)
>
> --
> 1.9.1
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Due to changes in the hardware design of the handshaking signal
between i2c and dma, it is necessary to reset the handshaking
signal before each transfer to ensure that the multi-msgs can
be transferred correctly.
Signed-off-by: Kewei Xu <[email protected]>
---
drivers/i2c/busses/i2c-mt65xx.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
index e65a41e..ded94f9 100644
--- a/drivers/i2c/busses/i2c-mt65xx.c
+++ b/drivers/i2c/busses/i2c-mt65xx.c
@@ -47,6 +47,9 @@
#define I2C_RD_TRANAC_VALUE 0x0001
#define I2C_SCL_MIS_COMP_VALUE 0x0000
#define I2C_CHN_CLR_FLAG 0x0000
+#define I2C_CLR_DEBUGCTR 0x0000
+#define I2C_RELIABILITY 0x0010
+#define I2C_DMAACK_ENABLE 0x0008
#define I2C_DMA_CON_TX 0x0000
#define I2C_DMA_CON_RX 0x0001
@@ -942,6 +945,17 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
reinit_completion(&i2c->msg_complete);
+ if (i2c->dev_comp->apdma_sync) {
+ mtk_i2c_writew(i2c, I2C_CLR_DEBUGCTR, OFFSET_DEBUGCTRL);
+ writel(I2C_DMA_HANDSHAKE_RST | I2C_DMA_WARM_RST,
+ i2c->pdmabase + OFFSET_RST);
+ writel(I2C_DMA_CLR_FLAG, i2c->pdmabase + OFFSET_RST);
+ mtk_i2c_writew(i2c, I2C_HANDSHAKE_RST, OFFSET_SOFTRESET);
+ mtk_i2c_writew(i2c, I2C_CHN_CLR_FLAG, OFFSET_SOFTRESET);
+ mtk_i2c_writew(i2c, I2C_RELIABILITY | I2C_DMAACK_ENABLE,
+ OFFSET_DEBUGCTRL);
+ }
+
control_reg = mtk_i2c_readw(i2c, OFFSET_CONTROL) &
~(I2C_CONTROL_DIR_CHANGE | I2C_CONTROL_RS);
if ((i2c->speed_hz > I2C_MAX_FAST_MODE_PLUS_FREQ) || (left_num >= 1))
--
1.9.1
On Thu, Jul 15, 2021 at 10:31 AM Kewei Xu <[email protected]> wrote:
>
> Add a DT binding documentation for the MT8195 soc.
>
> Signed-off-by: Kewei Xu <[email protected]>
Please remember to pick up tags from other reviewers.
ChenYu
Hi,
On Thu, Jul 15, 2021 at 10:31 AM Kewei Xu <[email protected]> wrote:
>
> The reason for the modification here is that the previous
> offset information is incorrect, OFFSET_DEBUGSTAT = 0xE4 is
> the correct value.
>
> Signed-off-by: Kewei Xu <[email protected]>
This needs a fixes tag:
Fixes: 25708278f810 ("i2c: mediatek: Add i2c support for MediaTek MT8183")
Otherwise,
Reviewed-by: Chen-Yu Tsai <[email protected]>
Hi,
On Thu, Jul 15, 2021 at 10:31 AM Kewei Xu <[email protected]> wrote:
>
> When a timeout error occurs in i2c transter, it is usually related
> to the i2c/dma IP hardware configuration. Therefore, the purpose of
> this patch is to dump the key register values of i2c/dma when a
> timeout occurs in i2c for debugging.
>
> Signed-off-by: Kewei Xu <[email protected]>
> ---
> drivers/i2c/busses/i2c-mt65xx.c | 95 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 94 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
> index 5ddfa4e..64acd96 100644
> --- a/drivers/i2c/busses/i2c-mt65xx.c
> +++ b/drivers/i2c/busses/i2c-mt65xx.c
> @@ -125,6 +125,7 @@ enum I2C_REGS_OFFSET {
> OFFSET_HS,
> OFFSET_SOFTRESET,
> OFFSET_DCM_EN,
> + OFFSET_MULTI_DMA,
> OFFSET_PATH_DIR,
> OFFSET_DEBUGSTAT,
> OFFSET_DEBUGCTRL,
> @@ -192,6 +193,7 @@ enum I2C_REGS_OFFSET {
> [OFFSET_TRANSFER_LEN_AUX] = 0x44,
> [OFFSET_CLOCK_DIV] = 0x48,
> [OFFSET_SOFTRESET] = 0x50,
> + [OFFSET_MULTI_DMA] = 0x84,
On the datasheets I have, MULTI_DMA is 0x8c, while 0x84 is CHANNEL_SEC on
MT8192, and not defined on MT8183 nor MT8195.
> [OFFSET_SCL_MIS_COMP_POINT] = 0x90,
> [OFFSET_DEBUGSTAT] = 0xe0,
> [OFFSET_DEBUGCTRL] = 0xe8,
> @@ -828,6 +830,96 @@ static int mtk_i2c_set_speed(struct mtk_i2c *i2c, unsigned int parent_clk)
> return 0;
> }
>
> +static void i2c_dump_register(struct mtk_i2c *i2c)
> +{
> + dev_err(i2c->dev, "SLAVE_ADDR[0x%x]: 0x%x, INTR_MASK[0x%x]: 0x%x\n",
> + OFFSET_SLAVE_ADDR,
> + (mtk_i2c_readw(i2c, OFFSET_SLAVE_ADDR)),
Drop the extra outer parentheses. Same goes for all the other invocations.
> + OFFSET_INTR_MASK,
> + (mtk_i2c_readw(i2c, OFFSET_INTR_MASK)));
> + dev_err(i2c->dev, "INTR_STAT[0x%x]: 0x%x, CONTROL[0x%x]: 0x%x\n",
> + OFFSET_INTR_STAT,
> + (mtk_i2c_readw(i2c, OFFSET_INTR_STAT)),
> + OFFSET_CONTROL,
> + (mtk_i2c_readw(i2c, OFFSET_CONTROL)));
> + dev_err(i2c->dev, "TRANSFER_LEN[0x%x]: 0x%x, TRANSAC_LEN[0x%x]: 0x%x\n",
> + OFFSET_TRANSFER_LEN,
> + (mtk_i2c_readw(i2c, OFFSET_TRANSFER_LEN)),
> + OFFSET_TRANSAC_LEN,
> + (mtk_i2c_readw(i2c, OFFSET_TRANSAC_LEN)));
> + dev_err(i2c->dev, "DELAY_LEN[0x%x]: 0x%x, HTIMING[0x%x]: 0x%x\n",
> + OFFSET_DELAY_LEN,
> + (mtk_i2c_readw(i2c, OFFSET_DELAY_LEN)),
> + OFFSET_TIMING,
> + (mtk_i2c_readw(i2c, OFFSET_TIMING)));
> + dev_err(i2c->dev, "OFFSET_START[0x%x]: 0x%x\n",
> + OFFSET_START,
> + mtk_i2c_readw(i2c, OFFSET_START));
> + dev_err(i2c->dev, "OFFSET_EXT_CONF[0x%x]: 0x%x\n",
> + OFFSET_EXT_CONF,
> + mtk_i2c_readw(i2c, OFFSET_EXT_CONF));
> + dev_err(i2c->dev, "OFFSET_HS[0x%x]: 0x%x\n",
> + OFFSET_HS,
> + mtk_i2c_readw(i2c, OFFSET_HS));
> + dev_err(i2c->dev, "OFFSET_IO_CONFIG[0x%x]: 0x%x\n",
> + OFFSET_IO_CONFIG,
> + mtk_i2c_readw(i2c, OFFSET_IO_CONFIG));
> + dev_err(i2c->dev, "OFFSET_FIFO_ADDR_CLR[0x%x]: 0x%x\n",
> + OFFSET_FIFO_ADDR_CLR,
> + mtk_i2c_readw(i2c, OFFSET_FIFO_ADDR_CLR));
> + dev_err(i2c->dev, "TRANSFER_LEN_AUX[0x%x]: 0x%x\n",
> + OFFSET_TRANSFER_LEN_AUX,
> + mtk_i2c_readw(i2c, OFFSET_TRANSFER_LEN_AUX));
> + dev_err(i2c->dev, "CLOCK_DIV[0x%x]: 0x%x\n",
> + OFFSET_CLOCK_DIV,
> + mtk_i2c_readw(i2c, OFFSET_CLOCK_DIV));
> + dev_err(i2c->dev, "FIFO_STAT[0x%x]: 0x%x, FIFO_THRESH[0x%x]: 0x%x\n",
> + OFFSET_FIFO_STAT,
> + mtk_i2c_readw(i2c, OFFSET_FIFO_STAT),
> + OFFSET_FIFO_THRESH,
> + mtk_i2c_readw(i2c, OFFSET_FIFO_THRESH));
> + dev_err(i2c->dev, "DCM_EN[0x%x] 0x%x\n",
> + OFFSET_DCM_EN,
> + mtk_i2c_readw(i2c, OFFSET_DCM_EN));
> + dev_err(i2c->dev, "DEBUGSTAT[0x%x]: 0x%x, DEBUGCTRL[0x%x]: 0x%x\n",
Nit: Why do some have two registers per line, and some only have one?
ChenYu
> + OFFSET_DEBUGSTAT,
> + (mtk_i2c_readw(i2c, OFFSET_DEBUGSTAT)),
> + OFFSET_DEBUGCTRL,
> + (mtk_i2c_readw(i2c, OFFSET_DEBUGCTRL)));
> +
> + if (i2c->dev_comp->regs == mt_i2c_regs_v2) {
> + dev_err(i2c->dev, "OFFSET_LTIMING[0x%x]: 0x%x\n",
> + OFFSET_LTIMING,
> + mtk_i2c_readw(i2c, OFFSET_LTIMING));
> + dev_err(i2c->dev, "MULTI_DMA[0x%x]: 0x%x\n",
> + OFFSET_MULTI_DMA,
> + (mtk_i2c_readw(i2c, OFFSET_MULTI_DMA)));
> + }
> +
> + dev_err(i2c->dev, "OFFSET_INT_FLAG = 0x%x\n",
> + readl(i2c->pdmabase + OFFSET_INT_FLAG));
> + dev_err(i2c->dev, "OFFSET_INT_EN = 0x%x\n",
> + readl(i2c->pdmabase + OFFSET_INT_EN));
> + dev_err(i2c->dev, "OFFSET_EN = 0x%x\n",
> + readl(i2c->pdmabase + OFFSET_EN));
> + dev_err(i2c->dev, "OFFSET_RST = 0x%x\n",
> + readl(i2c->pdmabase + OFFSET_RST));
> + dev_err(i2c->dev, "OFFSET_CON = 0x%x\n",
> + readl(i2c->pdmabase + OFFSET_CON));
> + dev_err(i2c->dev, "OFFSET_TX_MEM_ADDR = 0x%x\n",
> + readl(i2c->pdmabase + OFFSET_TX_MEM_ADDR));
> + dev_err(i2c->dev, "OFFSET_RX_MEM_ADDR = 0x%x\n",
> + readl(i2c->pdmabase + OFFSET_RX_MEM_ADDR));
> + dev_err(i2c->dev, "OFFSET_TX_LEN = 0x%x\n",
> + readl(i2c->pdmabase + OFFSET_TX_LEN));
> + dev_err(i2c->dev, "OFFSET_RX_LEN = 0x%x\n",
> + readl(i2c->pdmabase + OFFSET_RX_LEN));
> + dev_err(i2c->dev, "OFFSET_TX_4G_MODE = 0x%x\n",
> + readl(i2c->pdmabase + OFFSET_TX_4G_MODE));
> + dev_err(i2c->dev, "OFFSET_RX_4G_MODE = 0x%x\n",
> + readl(i2c->pdmabase + OFFSET_RX_4G_MODE));
> +}
> +
> static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
> int num, int left_num)
> {
> @@ -1034,7 +1126,8 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
> }
>
> if (ret == 0) {
> - dev_dbg(i2c->dev, "addr: %x, transfer timeout\n", msgs->addr);
> + dev_err(i2c->dev, "addr: %x, transfer timeout\n", msgs->addr);
> + i2c_dump_register(i2c);
> mtk_i2c_init_hw(i2c);
> return -ETIMEDOUT;
> }
> --
> 1.9.1
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Thu, Jul 15, 2021 at 10:32 AM Kewei Xu <[email protected]> wrote:
> When clock-div is 0 or greater than 1, the bus speed
> calculated by the old speed calculation formula will be
> larger than the target speed. So we update the formula.
The patch sounds like a fix up. Need a "Fixes" tag.
> for (clk_div = 1; clk_div <= max_clk_div; clk_div++) {
> clk_src = parent_clk / clk_div;
> + i2c->ac_timing.inter_clk_div = clk_div - 1;
Using the way to pass the parameter "inter_clk_div" to
mtk_i2c_calculate_speed() looks like a hack. inter_clk_div is set
again[1] next to the for loop.
[1]: https://elixir.bootlin.com/linux/v5.14-rc1/source/drivers/i2c/busses/i2c-mt65xx.c#L831
I have no domain knowledge of what/how the patch fixes. But if this
is a standalone fixup patch, suggest separating to an independent
patch.
On Thu, 2021-07-15 at 13:23 +0800, Chen-Yu Tsai wrote:
> Hi,
>
> On Thu, Jul 15, 2021 at 10:31 AM Kewei Xu <[email protected]> wrote:
> >
> > The reason for the modification here is that the previous
> > offset information is incorrect, OFFSET_DEBUGSTAT = 0xE4 is
> > the correct value.
> >
> > Signed-off-by: Kewei Xu <[email protected]>
>
> This needs a fixes tag:
>
> Fixes: 25708278f810 ("i2c: mediatek: Add i2c support for MediaTek MT8183")
>
> Otherwise,
>
> Reviewed-by: Chen-Yu Tsai <[email protected]>
Hi Chen-Yu,
OK, I will resubmit a patch to add a fixes tag.
thanks
Kewei
On Thu, 2021-07-15 at 15:09 +0800, Tzung-Bi Shih wrote:
> On Thu, Jul 15, 2021 at 10:32 AM Kewei Xu <[email protected]> wrote:
> > When clock-div is 0 or greater than 1, the bus speed
> > calculated by the old speed calculation formula will be
> > larger than the target speed. So we update the formula.
> The patch sounds like a fix up. Need a "Fixes" tag.
>
> > for (clk_div = 1; clk_div <= max_clk_div; clk_div++) {
> > clk_src = parent_clk / clk_div;
> > + i2c->ac_timing.inter_clk_div = clk_div - 1;
> Using the way to pass the parameter "inter_clk_div" to
> mtk_i2c_calculate_speed() looks like a hack. inter_clk_div is set
> again[1] next to the for loop.
>
> [1]: https://elixir.bootlin.com/linux/v5.14-rc1/source/drivers/i2c/busses/i2c-mt65xx.c#L831
>
>
>
> I have no domain knowledge of what/how the patch fixes. But if this
> is a standalone fixup patch, suggest separating to an independent
> patch.
Hi Tzung-Bi,
1. This Patch is not for fixing previous commit,it is just for the bad
speed formula.
2. I will fix this problem according to your suggestion in the next
patch.
Thanks
Kewei
On Thu, 2021-07-15 at 11:03 +0800, Chen-Yu Tsai wrote:
> Hi,
>
> On Thu, Jul 15, 2021 at 10:37 AM Kewei Xu <[email protected]> wrote:
> >
> > 1. In order to facilitate the review, the two series of patches submitted
> > before have been integrated together.
>
> I'm not sure that really helps. We'll see.
>
> > 2. Resubmit a patch to fixing the incorrect register value.
>
> Fixes can be submitted separately, or at the very least, put at the very
> front of the series.
>
> > 3. Add modify bus speed calculation formula patch
>
> When resubmitting patch series, please add appropriate versioning and
> changelogs. This will help maintainers understand that this is a new
> version of the series, and what has changed. This includes times when
> the original patches weren't changed, but additional patches were added.
>
> Please also keep the original series subject, which IIRC was about adding
> support for MT8195 I2C. The subject you now used should be part of the
> cover letter.
>
> If you combine different patch series, you should use the highest version
> number + 1.
>
>
> Regards
> ChenYu
>
> >
> > Kewei Xu (8):
> > dt-bindings: i2c: update bindings for MT8195 SoC
> > i2c: mediatek: Dump i2c/dma register when a timeout occurs
> > i2c: mediatek: fixing the incorrect register offset
> > i2c: mediatek: Reset the handshake signal between i2c and dma
> > i2c: mediatek: Add OFFSET_EXT_CONF setting back
> > dt-bindings: i2c: add attribute default-timing-adjust
> > i2c: mediatek: Isolate speed setting via dts for special devices
> > i2c: mediatek: modify bus speed calculation formula
> >
> > .../devicetree/bindings/i2c/i2c-mt65xx.txt | 3 +
> > drivers/i2c/busses/i2c-mt65xx.c | 229 +++++++++++++++++++--
> > 2 files changed, 217 insertions(+), 15 deletions(-)
> >
> > --
> > 1.9.1
> > _______________________________________________
> > Linux-mediatek mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek
Hi ChenYu,
2. OK, I will place the fixes at the very least in the next patch.
3. I will take your opinion combine different patch series, you should
use the highest version number +1.
I have submitted two different series of patches before. The topics are
"Add i2c support for mt8195" (highest version number: V1) and "Introduce
an attribute to select timing settings" (highest version number:V3).
Later I will combine the above two series of patches and submit V4 with
"Introducing an attribute to select the time setting" as the subject
title.
Thanks
Regards
Kewei