Messages in this thread | | | Subject | Re: [PATCH v2] i2c: tegra: Create i2c_writesl_vi() to use with VI I2C for filling TX FIFO | From | Sowjanya Komatineni <> | Date | Tue, 12 Jan 2021 08:57:16 -0800 |
| |
On 1/11/21 9:56 PM, Dmitry Osipenko wrote: > 12.01.2021 07:06, Sowjanya Komatineni пишет: >> VI I2C don't have DMA support and uses PIO mode all the time. >> >> Current driver uses writesl() to fill TX FIFO based on available >> empty slots and with this seeing strange silent hang during any I2C >> register access after filling TX FIFO with 8 words. >> >> Using writel() followed by i2c_readl() in a loop to write all words >> to TX FIFO instead of using writesl() helps for large transfers in >> PIO mode. >> >> So, this patch creates i2c_writesl_vi() API to use with VI I2C for >> filling TX FIFO. >> >> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> >> --- >> drivers/i2c/busses/i2c-tegra.c | 20 +++++++++++++++++++- >> 1 file changed, 19 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c >> index 6f08c0c..e2b7503 100644 >> --- a/drivers/i2c/busses/i2c-tegra.c >> +++ b/drivers/i2c/busses/i2c-tegra.c >> @@ -339,6 +339,21 @@ static void i2c_writesl(struct tegra_i2c_dev *i2c_dev, void *data, >> writesl(i2c_dev->base + tegra_i2c_reg_addr(i2c_dev, reg), data, len); >> } >> >> +static void i2c_writesl_vi(struct tegra_i2c_dev *i2c_dev, u32 *data, >> + unsigned int reg, unsigned int len) >> +{ >> + /* >> + * Using writesl() to fill VI I2C TX FIFO for transfers more than >> + * 6 words is causing a silent hang on any VI I2C register access >> + * after TX FIFO writes. >> + * So using writel() followed by i2c_readl(). >> + */ >> + while (len--) { >> + writel(*data++, i2c_dev->base + tegra_i2c_reg_addr(i2c_dev, reg)); >> + i2c_readl(i2c_dev, I2C_INT_STATUS); >> + } >> +} >> + >> static void i2c_readsl(struct tegra_i2c_dev *i2c_dev, void *data, >> unsigned int reg, unsigned int len) >> { >> @@ -811,7 +826,10 @@ static int tegra_i2c_fill_tx_fifo(struct tegra_i2c_dev *i2c_dev) >> i2c_dev->msg_buf_remaining = buf_remaining; >> i2c_dev->msg_buf = buf + words_to_transfer * BYTES_PER_FIFO_WORD; >> >> - i2c_writesl(i2c_dev, buf, I2C_TX_FIFO, words_to_transfer); >> + if (i2c_dev->is_vi) >> + i2c_writesl_vi(i2c_dev, (u32 *)buf, I2C_TX_FIFO, words_to_transfer); >> + else >> + i2c_writesl(i2c_dev, buf, I2C_TX_FIFO, words_to_transfer); >> >> buf += words_to_transfer * BYTES_PER_FIFO_WORD; >> } >> > Looks almost good, could we please use a relaxed writel and avoid the casting in the code? > > Like this: > > diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c > index 6f08c0c3238d..4f843b423d83 100644 > --- a/drivers/i2c/busses/i2c-tegra.c > +++ b/drivers/i2c/busses/i2c-tegra.c > @@ -326,6 +326,8 @@ static void i2c_writel(struct tegra_i2c_dev *i2c_dev, u32 val, unsigned int reg) > /* read back register to make sure that register writes completed */ > if (reg != I2C_TX_FIFO) > readl_relaxed(i2c_dev->base + tegra_i2c_reg_addr(i2c_dev, reg)); > + else > + readl_relaxed(i2c_dev->base + tegra_i2c_reg_addr(i2c_dev, I2C_INT_STATUS)); > } > > static u32 i2c_readl(struct tegra_i2c_dev *i2c_dev, unsigned int reg) > @@ -339,6 +341,21 @@ static void i2c_writesl(struct tegra_i2c_dev *i2c_dev, void *data, > writesl(i2c_dev->base + tegra_i2c_reg_addr(i2c_dev, reg), data, len); > } > > +static void i2c_writesl_vi(struct tegra_i2c_dev *i2c_dev, void *data, > + unsigned int reg, unsigned int len) > +{ > + u32 *data32 = data; > + > + /* > + * Using writesl() to fill VI I2C TX FIFO for transfers more than > + * 6 words is causing a silent hang on any VI I2C register access > + * after TX FIFO writes. Each write to FIFO should follow by a read > + * of any I2C register in order to work around the problem. > + */ > + while (len--) > + i2c_writel(i2c_dev, *data32++, reg); > +} > + > static void i2c_readsl(struct tegra_i2c_dev *i2c_dev, void *data, > unsigned int reg, unsigned int len) > { > @@ -811,7 +828,10 @@ static int tegra_i2c_fill_tx_fifo(struct tegra_i2c_dev *i2c_dev) > i2c_dev->msg_buf_remaining = buf_remaining; > i2c_dev->msg_buf = buf + words_to_transfer * BYTES_PER_FIFO_WORD; > > - i2c_writesl(i2c_dev, buf, I2C_TX_FIFO, words_to_transfer); > + if (i2c_dev->is_vi) > + i2c_writesl_vi(i2c_dev, buf, I2C_TX_FIFO, words_to_transfer); > + else > + i2c_writesl(i2c_dev, buf, I2C_TX_FIFO, words_to_transfer); > > buf += words_to_transfer * BYTES_PER_FIFO_WORD; > }
Will have v3.
| |