From fa4d30556883f2eaab425b88ba9904865a4d00f3 Mon Sep 17 00:00:00 2001 From: Christian Eggers Date: Wed, 7 Oct 2020 10:45:22 +0200 Subject: i2c: imx: Fix reset of I2SR_IAL flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit According to the "VFxxx Controller Reference Manual" (and the comment block starting at line 97), Vybrid requires writing a one for clearing an interrupt flag. Syncing the method for clearing I2SR_IIF in i2c_imx_isr(). Signed-off-by: Christian Eggers Fixes: 4b775022f6fd ("i2c: imx: add struct to hold more configurable quirks") Reviewed-by: Uwe Kleine-König Cc: stable@vger.kernel.org Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-imx.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index 0ab5381aa012..cbdcab73a055 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -412,6 +412,19 @@ static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx) dma->chan_using = NULL; } +static void i2c_imx_clear_irq(struct imx_i2c_struct *i2c_imx, unsigned int bits) +{ + unsigned int temp; + + /* + * i2sr_clr_opcode is the value to clear all interrupts. Here we want to + * clear only , so we write ~i2sr_clr_opcode with just + * toggled. This is required because i.MX needs W1C and Vybrid uses W0C. + */ + temp = ~i2c_imx->hwdata->i2sr_clr_opcode ^ bits; + imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2SR); +} + static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx, int for_busy, bool atomic) { unsigned long orig_jiffies = jiffies; @@ -424,8 +437,7 @@ static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx, int for_busy, bool a /* check for arbitration lost */ if (temp & I2SR_IAL) { - temp &= ~I2SR_IAL; - imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2SR); + i2c_imx_clear_irq(i2c_imx, I2SR_IAL); return -EAGAIN; } @@ -623,9 +635,7 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id) if (temp & I2SR_IIF) { /* save status register */ i2c_imx->i2csr = temp; - temp &= ~I2SR_IIF; - temp |= (i2c_imx->hwdata->i2sr_clr_opcode & I2SR_IIF); - imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2SR); + i2c_imx_clear_irq(i2c_imx, I2SR_IIF); wake_up(&i2c_imx->queue); return IRQ_HANDLED; } -- cgit From 28683e847e2f20eed22cdd24f185d7783db396d3 Mon Sep 17 00:00:00 2001 From: Jerome Brunet Date: Wed, 7 Oct 2020 10:07:49 +0200 Subject: i2c: meson: fix clock setting overwrite When the slave address is written in do_start(), SLAVE_ADDR is written completely. This may overwrite some setting related to the clock rate or signal filtering. Fix this by writing only the bits related to slave address. To avoid causing unexpected changed, explicitly disable filtering or high/low clock mode which may have been left over by the bootloader. Fixes: 30021e3707a7 ("i2c: add support for Amlogic Meson I2C controller") Signed-off-by: Jerome Brunet Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-meson.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c index c5dec572fc48..dac0d2a00cec 100644 --- a/drivers/i2c/busses/i2c-meson.c +++ b/drivers/i2c/busses/i2c-meson.c @@ -5,6 +5,7 @@ * Copyright (C) 2014 Beniamino Galvani */ +#include #include #include #include @@ -38,6 +39,12 @@ #define REG_CTRL_CLKDIVEXT_SHIFT 28 #define REG_CTRL_CLKDIVEXT_MASK GENMASK(29, 28) +#define REG_SLV_ADDR GENMASK(7, 0) +#define REG_SLV_SDA_FILTER GENMASK(10, 8) +#define REG_SLV_SCL_FILTER GENMASK(13, 11) +#define REG_SLV_SCL_LOW GENMASK(27, 16) +#define REG_SLV_SCL_LOW_EN BIT(28) + #define I2C_TIMEOUT_MS 500 enum { @@ -147,6 +154,9 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c, unsigned int freq) meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIVEXT_MASK, (div >> 10) << REG_CTRL_CLKDIVEXT_SHIFT); + /* Disable HIGH/LOW mode */ + meson_i2c_set_mask(i2c, REG_SLAVE_ADDR, REG_SLV_SCL_LOW_EN, 0); + dev_dbg(i2c->dev, "%s: clk %lu, freq %u, div %u\n", __func__, clk_rate, freq, div); } @@ -280,7 +290,10 @@ static void meson_i2c_do_start(struct meson_i2c *i2c, struct i2c_msg *msg) token = (msg->flags & I2C_M_RD) ? TOKEN_SLAVE_ADDR_READ : TOKEN_SLAVE_ADDR_WRITE; - writel(msg->addr << 1, i2c->regs + REG_SLAVE_ADDR); + + meson_i2c_set_mask(i2c, REG_SLAVE_ADDR, REG_SLV_ADDR, + FIELD_PREP(REG_SLV_ADDR, msg->addr << 1)); + meson_i2c_add_token(i2c, TOKEN_START); meson_i2c_add_token(i2c, token); } @@ -461,6 +474,10 @@ static int meson_i2c_probe(struct platform_device *pdev) return ret; } + /* Disable filtering */ + meson_i2c_set_mask(i2c, REG_SLAVE_ADDR, + REG_SLV_SDA_FILTER | REG_SLV_SCL_FILTER, 0); + meson_i2c_set_clk_div(i2c, timings.bus_freq_hz); return 0; -- cgit From 79e137b1540165f788394658442284d55a858984 Mon Sep 17 00:00:00 2001 From: Jerome Brunet Date: Wed, 7 Oct 2020 10:07:50 +0200 Subject: i2c: meson: keep peripheral clock enabled SCL rate appears to be different than what is expected. For example, We get 164kHz on i2c3 of the vim3 when 400kHz is expected. This is partially due to the peripheral clock being disabled when the clock is set. Let's keep the peripheral clock on after probe to fix the problem. This does not affect the SCL output which is still gated when i2c is idle. Fixes: 09af1c2fa490 ("i2c: meson: set clock divider in probe instead of setting it for each transfer") Signed-off-by: Jerome Brunet Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-meson.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c index dac0d2a00cec..e7ec2ab2a220 100644 --- a/drivers/i2c/busses/i2c-meson.c +++ b/drivers/i2c/busses/i2c-meson.c @@ -370,16 +370,12 @@ static int meson_i2c_xfer_messages(struct i2c_adapter *adap, struct meson_i2c *i2c = adap->algo_data; int i, ret = 0; - clk_enable(i2c->clk); - for (i = 0; i < num; i++) { ret = meson_i2c_xfer_msg(i2c, msgs + i, i == num - 1, atomic); if (ret) break; } - clk_disable(i2c->clk); - return ret ?: i; } @@ -448,7 +444,7 @@ static int meson_i2c_probe(struct platform_device *pdev) return ret; } - ret = clk_prepare(i2c->clk); + ret = clk_prepare_enable(i2c->clk); if (ret < 0) { dev_err(&pdev->dev, "can't prepare clock\n"); return ret; @@ -470,7 +466,7 @@ static int meson_i2c_probe(struct platform_device *pdev) ret = i2c_add_adapter(&i2c->adap); if (ret < 0) { - clk_unprepare(i2c->clk); + clk_disable_unprepare(i2c->clk); return ret; } @@ -488,7 +484,7 @@ static int meson_i2c_remove(struct platform_device *pdev) struct meson_i2c *i2c = platform_get_drvdata(pdev); i2c_del_adapter(&i2c->adap); - clk_unprepare(i2c->clk); + clk_disable_unprepare(i2c->clk); return 0; } -- cgit From 1334d3b4e49e35d8912a7c37ffca4c5afb9a0516 Mon Sep 17 00:00:00 2001 From: Nicolas Belin Date: Wed, 7 Oct 2020 10:07:51 +0200 Subject: i2c: meson: fixup rate calculation with filter delay Apparently, 15 cycles of the peripheral clock are used by the controller for sampling and filtering. Because this was not known before, the rate calculation is slightly off. Clean up and fix the calculation taking this filtering delay into account. Fixes: 30021e3707a7 ("i2c: add support for Amlogic Meson I2C controller") Signed-off-by: Nicolas Belin Signed-off-by: Jerome Brunet Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-meson.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c index e7ec2ab2a220..ef73a42577cc 100644 --- a/drivers/i2c/busses/i2c-meson.c +++ b/drivers/i2c/busses/i2c-meson.c @@ -34,10 +34,8 @@ #define REG_CTRL_ACK_IGNORE BIT(1) #define REG_CTRL_STATUS BIT(2) #define REG_CTRL_ERROR BIT(3) -#define REG_CTRL_CLKDIV_SHIFT 12 -#define REG_CTRL_CLKDIV_MASK GENMASK(21, 12) -#define REG_CTRL_CLKDIVEXT_SHIFT 28 -#define REG_CTRL_CLKDIVEXT_MASK GENMASK(29, 28) +#define REG_CTRL_CLKDIV GENMASK(21, 12) +#define REG_CTRL_CLKDIVEXT GENMASK(29, 28) #define REG_SLV_ADDR GENMASK(7, 0) #define REG_SLV_SDA_FILTER GENMASK(10, 8) @@ -46,6 +44,7 @@ #define REG_SLV_SCL_LOW_EN BIT(28) #define I2C_TIMEOUT_MS 500 +#define FILTER_DELAY 15 enum { TOKEN_END = 0, @@ -140,19 +139,21 @@ static void meson_i2c_set_clk_div(struct meson_i2c *i2c, unsigned int freq) unsigned long clk_rate = clk_get_rate(i2c->clk); unsigned int div; - div = DIV_ROUND_UP(clk_rate, freq * i2c->data->div_factor); + div = DIV_ROUND_UP(clk_rate, freq); + div -= FILTER_DELAY; + div = DIV_ROUND_UP(div, i2c->data->div_factor); /* clock divider has 12 bits */ - if (div >= (1 << 12)) { + if (div > GENMASK(11, 0)) { dev_err(i2c->dev, "requested bus frequency too low\n"); - div = (1 << 12) - 1; + div = GENMASK(11, 0); } - meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIV_MASK, - (div & GENMASK(9, 0)) << REG_CTRL_CLKDIV_SHIFT); + meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIV, + FIELD_PREP(REG_CTRL_CLKDIV, div & GENMASK(9, 0))); - meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIVEXT_MASK, - (div >> 10) << REG_CTRL_CLKDIVEXT_SHIFT); + meson_i2c_set_mask(i2c, REG_CTRL, REG_CTRL_CLKDIVEXT, + FIELD_PREP(REG_CTRL_CLKDIVEXT, div >> 10)); /* Disable HIGH/LOW mode */ meson_i2c_set_mask(i2c, REG_SLAVE_ADDR, REG_SLV_SCL_LOW_EN, 0); -- cgit From 5a02e7c429cb5e082e5d7be6e5b768828014ba70 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Sat, 10 Oct 2020 13:03:54 +0200 Subject: Revert "i2c: imx: Fix reset of I2SR_IAL flag" This reverts commit fa4d30556883f2eaab425b88ba9904865a4d00f3. An updated version was sent. So, revert this version and give the new version more time for testing. Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-imx.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index cbdcab73a055..0ab5381aa012 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -412,19 +412,6 @@ static void i2c_imx_dma_free(struct imx_i2c_struct *i2c_imx) dma->chan_using = NULL; } -static void i2c_imx_clear_irq(struct imx_i2c_struct *i2c_imx, unsigned int bits) -{ - unsigned int temp; - - /* - * i2sr_clr_opcode is the value to clear all interrupts. Here we want to - * clear only , so we write ~i2sr_clr_opcode with just - * toggled. This is required because i.MX needs W1C and Vybrid uses W0C. - */ - temp = ~i2c_imx->hwdata->i2sr_clr_opcode ^ bits; - imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2SR); -} - static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx, int for_busy, bool atomic) { unsigned long orig_jiffies = jiffies; @@ -437,7 +424,8 @@ static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx, int for_busy, bool a /* check for arbitration lost */ if (temp & I2SR_IAL) { - i2c_imx_clear_irq(i2c_imx, I2SR_IAL); + temp &= ~I2SR_IAL; + imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2SR); return -EAGAIN; } @@ -635,7 +623,9 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id) if (temp & I2SR_IIF) { /* save status register */ i2c_imx->i2csr = temp; - i2c_imx_clear_irq(i2c_imx, I2SR_IIF); + temp &= ~I2SR_IIF; + temp |= (i2c_imx->hwdata->i2sr_clr_opcode & I2SR_IIF); + imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2SR); wake_up(&i2c_imx->queue); return IRQ_HANDLED; } -- cgit From f5b3f433641c543ebe5171285a42aa6adcdb2d22 Mon Sep 17 00:00:00 2001 From: Cristian Ciocaltea Date: Fri, 9 Oct 2020 00:44:39 +0300 Subject: i2c: owl: Clear NACK and BUS error bits When the NACK and BUS error bits are set by the hardware, the driver is responsible for clearing them by writing "1" into the corresponding status registers. Hence perform the necessary operations in owl_i2c_interrupt(). Fixes: d211e62af466 ("i2c: Add Actions Semiconductor Owl family S900 I2C driver") Reported-by: Manivannan Sadhasivam Signed-off-by: Cristian Ciocaltea Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-owl.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/i2c/busses/i2c-owl.c b/drivers/i2c/busses/i2c-owl.c index 672f1f239bd6..a163b8f308c1 100644 --- a/drivers/i2c/busses/i2c-owl.c +++ b/drivers/i2c/busses/i2c-owl.c @@ -176,6 +176,9 @@ static irqreturn_t owl_i2c_interrupt(int irq, void *_dev) fifostat = readl(i2c_dev->base + OWL_I2C_REG_FIFOSTAT); if (fifostat & OWL_I2C_FIFOSTAT_RNB) { i2c_dev->err = -ENXIO; + /* Clear NACK error bit by writing "1" */ + owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_FIFOSTAT, + OWL_I2C_FIFOSTAT_RNB, true); goto stop; } @@ -183,6 +186,9 @@ static irqreturn_t owl_i2c_interrupt(int irq, void *_dev) stat = readl(i2c_dev->base + OWL_I2C_REG_STAT); if (stat & OWL_I2C_STAT_BEB) { i2c_dev->err = -EIO; + /* Clear BUS error bit by writing "1" */ + owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_STAT, + OWL_I2C_STAT_BEB, true); goto stop; } -- cgit