From d43d4c2c01c49caeaba2952ab04eb2d04201bbbc Mon Sep 17 00:00:00 2001 From: Krzysztof Adamski Date: Thu, 30 Mar 2023 08:20:29 +0200 Subject: [PATCH 1/6] i2c: bl808: msg_err should be int It contains the error code which can be negative (actually, it is never set to positive value). --- drivers/i2c/busses/i2c-bl808.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-bl808.c b/drivers/i2c/busses/i2c-bl808.c index 5f50ef1f16df69..5c708fcd54f56b 100644 --- a/drivers/i2c/busses/i2c-bl808.c +++ b/drivers/i2c/busses/i2c-bl808.c @@ -195,7 +195,7 @@ struct bl808_i2c_dev { struct i2c_msg *curr_msg; struct clk *bus_clk; int num_msgs; - u32 msg_err; + int msg_err; u8 *msg_buf; u16 msg_buf_remaining; }; From 100b3a3e25c4da1374ba53062b87f8a4d9bd8ec6 Mon Sep 17 00:00:00 2001 From: Krzysztof Adamski Date: Thu, 30 Mar 2023 08:22:10 +0200 Subject: [PATCH 2/6] i2c: bl808: reset msg_err before transfer It makes sense to set it to a known value so that we do not use the old value in case it wasn't set (like when there was a timeout). --- drivers/i2c/busses/i2c-bl808.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i2c/busses/i2c-bl808.c b/drivers/i2c/busses/i2c-bl808.c index 5c708fcd54f56b..0300f7b6e29ec9 100644 --- a/drivers/i2c/busses/i2c-bl808.c +++ b/drivers/i2c/busses/i2c-bl808.c @@ -703,6 +703,7 @@ static int bl808_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], i2c_dev->num_msgs = num; reinit_completion(&i2c_dev->completion); + i2c_dev->msg_err = 0; ret = bl808_i2c_start_transfer(i2c_dev); if (ret) { return i2c_dev->msg_err; From 4f571386cdbb0e1bc859fad6fa1f558440b91aef Mon Sep 17 00:00:00 2001 From: Krzysztof Adamski Date: Thu, 30 Mar 2023 08:26:35 +0200 Subject: [PATCH 3/6] i2c: bl808: fix return val from bl808_i2c_start_transfer bl808_i2c_start_transfer() never set's the i2c_dev->msg_err itself, it is only set by an interrupt but this will be valid only if we get a completion. In this place, we should just return ret. --- drivers/i2c/busses/i2c-bl808.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-bl808.c b/drivers/i2c/busses/i2c-bl808.c index 0300f7b6e29ec9..3084f6a2d2f8b7 100644 --- a/drivers/i2c/busses/i2c-bl808.c +++ b/drivers/i2c/busses/i2c-bl808.c @@ -706,7 +706,7 @@ static int bl808_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], i2c_dev->msg_err = 0; ret = bl808_i2c_start_transfer(i2c_dev); if (ret) { - return i2c_dev->msg_err; + return ret; } time_left = wait_for_completion_timeout(&i2c_dev->completion, From d81eaf78bf678de0cae9c971d8ed46f7452f7722 Mon Sep 17 00:00:00 2001 From: Krzysztof Adamski Date: Thu, 30 Mar 2023 08:29:55 +0200 Subject: [PATCH 4/6] i2c: bl808: show STS in case of multiple irqs --- drivers/i2c/busses/i2c-bl808.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-bl808.c b/drivers/i2c/busses/i2c-bl808.c index 3084f6a2d2f8b7..a40a8a84ec0dc5 100644 --- a/drivers/i2c/busses/i2c-bl808.c +++ b/drivers/i2c/busses/i2c-bl808.c @@ -588,8 +588,8 @@ static irqreturn_t bl808_i2c_isr(int this_isq, void *data) { val = bl808_i2c_readl(i2c_dev, BL808_I2C_STS); - if(hweight32(val & 0x3f) != 0) { - dev_err(i2c_dev->dev, "Multiple interrupts %u\n", val & 0x3f); + if(hweight32(val & 0x3f) > 1) { + dev_err(i2c_dev->dev, "Multiple interrupts %u, sts=0x%x\n", val & 0x3f, val); } if (!i2c_dev->curr_msg) { From 221045274100f9b20648c3b1c3ba81da8f5a5eb2 Mon Sep 17 00:00:00 2001 From: Krzysztof Adamski Date: Thu, 30 Mar 2023 08:44:53 +0200 Subject: [PATCH 5/6] i2c: bl808: use devm_ variant of request_irq By using resource managed request_irq we don't have to worry about freeign it manualy. --- drivers/i2c/busses/i2c-bl808.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/i2c/busses/i2c-bl808.c b/drivers/i2c/busses/i2c-bl808.c index a40a8a84ec0dc5..9caf022c5b7949 100644 --- a/drivers/i2c/busses/i2c-bl808.c +++ b/drivers/i2c/busses/i2c-bl808.c @@ -800,7 +800,7 @@ static int bl808_i2c_probe(struct platform_device *pdev){ goto err_disable_unprepare_clk; } - ret = request_irq(i2c_dev->irq, bl808_i2c_isr, IRQF_SHARED, + ret = devm_request_irq(&pdev->dev, i2c_dev->irq, bl808_i2c_isr, IRQF_SHARED, dev_name(&pdev->dev), i2c_dev); if (ret) { dev_err(&pdev->dev, "Could not request IRQ\n"); @@ -820,14 +820,12 @@ static int bl808_i2c_probe(struct platform_device *pdev){ ret = i2c_add_adapter(adap); if (ret) - goto err_free_irq; + goto err_disable_unprepare_clk; bl808_i2c_init(i2c_dev); return 0; -err_free_irq: - free_irq(i2c_dev->irq, i2c_dev); err_disable_unprepare_clk: clk_disable_unprepare(i2c_dev->bus_clk); err_put_exclusive_rate: @@ -843,7 +841,6 @@ static int bl808_i2c_remove(struct platform_device *pdev) clk_rate_exclusive_put(i2c_dev->bus_clk); clk_disable_unprepare(i2c_dev->bus_clk); - free_irq(i2c_dev->irq, i2c_dev); i2c_del_adapter(&i2c_dev->adapter); return 0; From c0e267901a2b05ab1a7b600147d66538bab520c3 Mon Sep 17 00:00:00 2001 From: Krzysztof Adamski Date: Thu, 30 Mar 2023 09:07:10 +0200 Subject: [PATCH 6/6] i2c: bl808: add more debug info to FIFO ERR Show the TX/RX fifo counter when writing FIFO error log message so that we can better understand when exactly is this interrupt generated. --- drivers/i2c/busses/i2c-bl808.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/drivers/i2c/busses/i2c-bl808.c b/drivers/i2c/busses/i2c-bl808.c index 9caf022c5b7949..592eb1f486e140 100644 --- a/drivers/i2c/busses/i2c-bl808.c +++ b/drivers/i2c/busses/i2c-bl808.c @@ -606,15 +606,20 @@ static irqreturn_t bl808_i2c_isr(int this_isq, void *data) { i2c_dev->msg_err = -ENXIO; goto complete; } else if (val & BL808_I2C_STS_FER_INT) { - val = bl808_i2c_readl(i2c_dev, BL808_I2C_FIFO_CONFIG_0); - if (val & BL808_I2C_FIFO_CONFIG_0_RX_FIFO_OVFLW) { - dev_err(i2c_dev->dev, "RX FIFO Overflow\n"); - } else if (val & BL808_I2C_FIFO_CONFIG_0_RX_FIFO_UDFLW) { - dev_err(i2c_dev->dev, "RX FIFO Underflow\n"); - } else if (val & BL808_I2C_FIFO_CONFIG_0_TX_FIFO_OVFLW) { - dev_err(i2c_dev->dev, "TX FIFO Overflow\n"); - } else if (val & BL808_I2C_FIFO_CONFIG_0_TX_FIFO_UDFLW) { - dev_err(i2c_dev->dev, "TX FIFO Underflow\n"); + u32 config_0, config_1, tx_cnt, rx_cnt; + config_0 = bl808_i2c_readl(i2c_dev, BL808_I2C_FIFO_CONFIG_0); + config_1 = bl808_i2c_readl(i2c_dev, BL808_I2C_FIFO_CONFIG_1); + tx_cnt = (config_1 & BL808_I2C_FIFO_CONFIG_1_TX_FIFO_CNT_MASK) >> BL808_I2C_FIFO_CONFIG_1_TX_FIFO_CNT_SHIFT; + rx_cnt = (config_1 & BL808_I2C_FIFO_CONFIG_1_RX_FIFO_CNT_MASK) >> BL808_I2C_FIFO_CONFIG_1_RX_FIFO_CNT_SHIFT; + + if (config_0 & BL808_I2C_FIFO_CONFIG_0_RX_FIFO_OVFLW) { + dev_err(i2c_dev->dev, "RX FIFO Overflow, cnt=%d\n", rx_cnt); + } else if (config_0 & BL808_I2C_FIFO_CONFIG_0_RX_FIFO_UDFLW) { + dev_err(i2c_dev->dev, "RX FIFO Underflow, cnt=%d\n", rx_cnt); + } else if (config_0 & BL808_I2C_FIFO_CONFIG_0_TX_FIFO_OVFLW) { + dev_err(i2c_dev->dev, "TX FIFO Overflow, cnt=%d\n", tx_cnt); + } else if (config_0 & BL808_I2C_FIFO_CONFIG_0_TX_FIFO_UDFLW) { + dev_err(i2c_dev->dev, "TX FIFO Underflow, cnt=%d\n", tx_cnt); } i2c_dev->msg_err = -EIO;