Re: [PATCH v8 2/5] i2c: mediatek: Dump i2c/dma register when a timeout occurs

Re: [PATCH v8 2/5] i2c: mediatek: Dump i2c/dma register when a timeout occurs

[Boris Lysov]

On Sun, 10 Oct 2021 15:05:13 +0800
Kewei Xu <kewei.xu@mediatek.com> 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 <kewei.xu@mediatek.com>
> Reviewed-by: Qii Wang <qii.wang@mediatek.com>

I would like to test this patchset on one of supported platforms, an
mt6577-powered device. This driver requires a DMA base...
> 	i2c->pdmabase = devm_ioremap_resource(&pdev->dev, res);
>	if (IS_ERR(i2c->pdmabase))
>		return PTR_ERR(i2c->pdmabase);
... however I am not sure what address should I specify in the .dts file. While
other i2c busses supported by this driver such as mt6589 [1][2] and mt6797 [3]
have designated DMA for I2C, mt6577 seems to lack dedicated DMA engine for I2C
[4][5].

Do I need to specify the Generic DMA base address [4] instead?

Worth mentioning I brought up this issue in the past [5][6].

[1] ALPS.KK1.MP5.V1.3_EASTAEON89_WET_KK source code (downstream)
[2] MT6589 HSPA+ Smartphone Application Processor Datasheet v1.0, page 869
[3] MT6797 LTE-A Smartphone Application Processor Register Table (Part 1) v1.1,
page 1796
[4] MT6577 HSPA Smartphone Application Processor Datasheet v0.94, page 547
[5] ALPS.JB.MP.V1.19_MBK77_TB_JB source code (downstream)
[6] https://marc.info/?l=devicetree&m=159949247901831&w=2
[7] https://marc.info/?l=linux-i2c&m=159939730714187&w=2

Re: [PATCH v8 2/5] i2c: mediatek: Dump i2c/dma register when a timeout occurs

[Kewei Xu]

On Mon, 2021-10-11 at 18:38 +0300, Boris Lysov wrote:
> On Sun, 10 Oct 2021 15:05:13 +0800
> Kewei Xu <kewei.xu@mediatek.com> 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 <kewei.xu@mediatek.com>
> > Reviewed-by: Qii Wang <qii.wang@mediatek.com>
> 
> I would like to test this patchset on one of supported platforms, an
> mt6577-powered device. This driver requires a DMA base...
> > 	i2c->pdmabase = devm_ioremap_resource(&pdev->dev, res);
> > 	if (IS_ERR(i2c->pdmabase))
> > 		return PTR_ERR(i2c->pdmabase);
> 
> ... however I am not sure what address should I specify in the .dts
> file. While
> other i2c busses supported by this driver such as mt6589 [1][2] and
> mt6797 [3]
> have designated DMA for I2C, mt6577 seems to lack dedicated DMA
> engine for I2C
> [4][5].
> 
> Do I need to specify the Generic DMA base address [4] instead?
> 
> Worth mentioning I brought up this issue in the past [5][6].
> 
> [1] ALPS.KK1.MP5.V1.3_EASTAEON89_WET_KK source code (downstream)
> [2] MT6589 HSPA+ Smartphone Application Processor Datasheet v1.0,
> page 869
> [3] MT6797 LTE-A Smartphone Application Processor Register Table
> (Part 1) v1.1,
> page 1796
> [4] MT6577 HSPA Smartphone Application Processor Datasheet v0.94,
> page 547
> [5] ALPS.JB.MP.V1.19_MBK77_TB_JB source code (downstream)
> [6] https://marc.info/?l=devicetree&m=159949247901831&w=2
> [7] https://marc.info/?l=linux-i2c&m=159939730714187&w=2
Hi, This patch only applies to APDMA(application processor DMA) and
does not use GDMA(Generic DMA).In addition, the file i2c-mt65xx.c only
supports APDMA mode by default. If APDMA address is not configured in
dts, i2c transmission will be fail.thx~

[PATCH v8 0/5] Introducing an attribute to select the time setting

[Kewei Xu]

v7:
Resubmit the patch based on version 5.15-rc1

v6:
1. Add the judgment condition, clear the handshake signal between dma and
i2c when multiple msgs are transmitted.

v5:
1. Replace the previous variable name "default_timing_adjust" with "use-default-timing"
2. Added waiting for dma reset mechanism
3. Remove received patch(dt-bindings: i2c: update bindings for MT8195 SOC)

v4:
1. Remove the repeated assignment of the inter_clk_div parameter
2. Modify the wrong assignment of OFFSET_MULTI_DMA
3. Unify the log print format of the i2c_dump_register() and drop the extra outer parentheses
4. Place the fixes at the very least
5. Add fixed tags 25708278f810 ("i2c: mediatek: Add i2c support for MediaTek MT8183")
6. Add "i2c: mediatek: modify bus speed calculation formula"
7. Fix single line characters exceeding 80 characters
8. Combine two different series of patches.

v3:
1. Fix code errors caused by v2 modification

v2:
1. Add "dt-bindings: i2c: add attribute default-timing-adjust"
2. Split the fix into sepatate patch.

Kewei Xu (5):
  i2c: mediatek: Reset the handshake signal between i2c and dma
  i2c: mediatek: Dump i2c/dma register when a timeout occurs
  dt-bindings: i2c: add attribute use-default-timing
  i2c: mediatek: Isolate speed setting via dts for special devices
  i2c: mediatek: modify bus speed calculation formula

 .../devicetree/bindings/i2c/i2c-mt65xx.txt    |   2 +
 drivers/i2c/busses/i2c-mt65xx.c               | 192 ++++++++++++++++--
 2 files changed, 180 insertions(+), 14 deletions(-)

--
2.18.0

[PATCH v8 2/5] i2c: mediatek: Dump i2c/dma register when a timeout occurs

[Kewei Xu]

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 <kewei.xu@mediatek.com>
Reviewed-by: Qii Wang <qii.wang@mediatek.com>
---
 drivers/i2c/busses/i2c-mt65xx.c | 54 +++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
index dad3a85cd499..a1a2af066704 100644
--- a/drivers/i2c/busses/i2c-mt65xx.c
+++ b/drivers/i2c/busses/i2c-mt65xx.c
@@ -130,6 +130,7 @@ enum I2C_REGS_OFFSET {
 	OFFSET_HS,
 	OFFSET_SOFTRESET,
 	OFFSET_DCM_EN,
+	OFFSET_MULTI_DMA,
 	OFFSET_PATH_DIR,
 	OFFSET_DEBUGSTAT,
 	OFFSET_DEBUGCTRL,
@@ -197,6 +198,7 @@ static const u16 mt_i2c_regs_v2[] = {
 	[OFFSET_TRANSFER_LEN_AUX] = 0x44,
 	[OFFSET_CLOCK_DIV] = 0x48,
 	[OFFSET_SOFTRESET] = 0x50,
+	[OFFSET_MULTI_DMA] = 0x8c,
 	[OFFSET_SCL_MIS_COMP_POINT] = 0x90,
 	[OFFSET_DEBUGSTAT] = 0xe4,
 	[OFFSET_DEBUGCTRL] = 0xe8,
@@ -845,6 +847,57 @@ 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_dbg(i2c->dev, "SLAVE_ADDR: 0x%x, INTR_MASK: 0x%x\n",
+		mtk_i2c_readw(i2c, OFFSET_SLAVE_ADDR),
+		mtk_i2c_readw(i2c, OFFSET_INTR_MASK));
+	dev_dbg(i2c->dev, "INTR_STAT: 0x%x, CONTROL: 0x%x\n",
+		mtk_i2c_readw(i2c, OFFSET_INTR_STAT),
+		mtk_i2c_readw(i2c, OFFSET_CONTROL));
+	dev_dbg(i2c->dev, "TRANSFER_LEN: 0x%x, TRANSAC_LEN: 0x%x\n",
+		mtk_i2c_readw(i2c, OFFSET_TRANSFER_LEN),
+		mtk_i2c_readw(i2c, OFFSET_TRANSAC_LEN));
+	dev_dbg(i2c->dev, "DELAY_LEN: 0x%x, HTIMING: 0x%x\n",
+		mtk_i2c_readw(i2c, OFFSET_DELAY_LEN),
+		mtk_i2c_readw(i2c, OFFSET_TIMING));
+	dev_dbg(i2c->dev, "START: 0x%x, EXT_CONF: 0x%x\n",
+		mtk_i2c_readw(i2c, OFFSET_START),
+		mtk_i2c_readw(i2c, OFFSET_EXT_CONF));
+	dev_dbg(i2c->dev, "HS: 0x%x, IO_CONFIG: 0x%x\n",
+		mtk_i2c_readw(i2c, OFFSET_HS),
+		mtk_i2c_readw(i2c, OFFSET_IO_CONFIG));
+	dev_dbg(i2c->dev, "DCM_EN: 0x%x, TRANSFER_LEN_AUX: 0x%x\n",
+		mtk_i2c_readw(i2c, OFFSET_DCM_EN),
+		mtk_i2c_readw(i2c, OFFSET_TRANSFER_LEN_AUX));
+	dev_dbg(i2c->dev, "CLOCK_DIV: 0x%x, FIFO_STAT: 0x%x\n",
+		mtk_i2c_readw(i2c, OFFSET_CLOCK_DIV),
+		mtk_i2c_readw(i2c, OFFSET_FIFO_STAT));
+	dev_dbg(i2c->dev, "DEBUGCTRL : 0x%x, DEBUGSTAT: 0x%x\n",
+		mtk_i2c_readw(i2c, OFFSET_DEBUGCTRL),
+		mtk_i2c_readw(i2c, OFFSET_DEBUGSTAT));
+	if (i2c->dev_comp->regs == mt_i2c_regs_v2) {
+		dev_dbg(i2c->dev, "LTIMING: 0x%x, MULTI_DMA: 0x%x\n",
+			mtk_i2c_readw(i2c, OFFSET_LTIMING),
+			mtk_i2c_readw(i2c, OFFSET_MULTI_DMA));
+	}
+	dev_dbg(i2c->dev, "\nDMA_INT_FLAG: 0x%x, DMA_INT_EN: 0x%x\n",
+		readl(i2c->pdmabase + OFFSET_INT_FLAG),
+		readl(i2c->pdmabase + OFFSET_INT_EN));
+	dev_dbg(i2c->dev, "DMA_EN: 0x%x, DMA_CON: 0x%x\n",
+		readl(i2c->pdmabase + OFFSET_EN),
+		readl(i2c->pdmabase + OFFSET_CON));
+	dev_dbg(i2c->dev, "DMA_TX_MEM_ADDR: 0x%x, DMA_RX_MEM_ADDR: 0x%x\n",
+		readl(i2c->pdmabase + OFFSET_TX_MEM_ADDR),
+		readl(i2c->pdmabase + OFFSET_RX_MEM_ADDR));
+	dev_dbg(i2c->dev, "DMA_TX_LEN: 0x%x, DMA_RX_LEN: 0x%x\n",
+		readl(i2c->pdmabase + OFFSET_TX_LEN),
+		readl(i2c->pdmabase + OFFSET_RX_LEN));
+	dev_dbg(i2c->dev, "DMA_TX_4G_MODE: 0x%x, DMA_RX_4G_MODE: 0x%x",
+		readl(i2c->pdmabase + OFFSET_TX_4G_MODE),
+		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)
 {
@@ -1075,6 +1128,7 @@ 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);
+		i2c_dump_register(i2c);
 		mtk_i2c_init_hw(i2c);
 		return -ETIMEDOUT;
 	}
-- 
2.25.1

Re: [PATCH v8 2/5] i2c: mediatek: Dump i2c/dma register when a timeout occurs

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 445 bytes --]

On Sun, Oct 10, 2021 at 03:05:13PM +0800, Kewei Xu 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 <kewei.xu@mediatek.com>
> Reviewed-by: Qii Wang <qii.wang@mediatek.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]