* [PATCH v2] Create i2c_writesl_vi() to use with VI I2C
@ 2021-01-12 4:06 Sowjanya Komatineni
2021-01-12 4:06 ` [PATCH v2] i2c: tegra: Create i2c_writesl_vi() to use with VI I2C for filling TX FIFO Sowjanya Komatineni
0 siblings, 1 reply; 5+ messages in thread
From: Sowjanya Komatineni @ 2021-01-12 4:06 UTC (permalink / raw)
To: thierry.reding, jonathanh, digetx, wsa, skomatineni
Cc: linux-tegra, linux-kernel, linux-i2c
Patch in this series is to fix silent hang seen when using writesl()
for filling VI I2C TX FIFO.
Delta between patch versions:
[v2]: Creates i2c_writesl_vi() for vi i2c based on v1 feedback.
[v1]: Updates i2c_writesl() to use writel() followed by i2c_readl().
Sowjanya Komatineni (1):
i2c: tegra: Create i2c_writesl_vi() to use with VI I2C for filling TX
FIFO
drivers/i2c/busses/i2c-tegra.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] i2c: tegra: Create i2c_writesl_vi() to use with VI I2C for filling TX FIFO
2021-01-12 4:06 [PATCH v2] Create i2c_writesl_vi() to use with VI I2C Sowjanya Komatineni
@ 2021-01-12 4:06 ` Sowjanya Komatineni
2021-01-12 5:56 ` Dmitry Osipenko
0 siblings, 1 reply; 5+ messages in thread
From: Sowjanya Komatineni @ 2021-01-12 4:06 UTC (permalink / raw)
To: thierry.reding, jonathanh, digetx, wsa, skomatineni
Cc: linux-tegra, linux-kernel, linux-i2c
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;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] i2c: tegra: Create i2c_writesl_vi() to use with VI I2C for filling TX FIFO
2021-01-12 4:06 ` [PATCH v2] i2c: tegra: Create i2c_writesl_vi() to use with VI I2C for filling TX FIFO Sowjanya Komatineni
@ 2021-01-12 5:56 ` Dmitry Osipenko
2021-01-12 16:57 ` Sowjanya Komatineni
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Osipenko @ 2021-01-12 5:56 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, wsa
Cc: linux-tegra, linux-kernel, linux-i2c
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;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] i2c: tegra: Create i2c_writesl_vi() to use with VI I2C for filling TX FIFO
2021-01-12 5:56 ` Dmitry Osipenko
@ 2021-01-12 16:57 ` Sowjanya Komatineni
2021-01-12 17:34 ` Dmitry Osipenko
0 siblings, 1 reply; 5+ messages in thread
From: Sowjanya Komatineni @ 2021-01-12 16:57 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, wsa
Cc: linux-tegra, linux-kernel, linux-i2c
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] i2c: tegra: Create i2c_writesl_vi() to use with VI I2C for filling TX FIFO
2021-01-12 16:57 ` Sowjanya Komatineni
@ 2021-01-12 17:34 ` Dmitry Osipenko
0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Osipenko @ 2021-01-12 17:34 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, wsa
Cc: linux-tegra, linux-kernel, linux-i2c
12.01.2021 19:57, Sowjanya Komatineni пишет:
...
>> @@ -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));
>
Perhaps will be a bit better to replace this "else" with "else if
(i2c_dev->is_vi)".
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-12 17:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-12 4:06 [PATCH v2] Create i2c_writesl_vi() to use with VI I2C Sowjanya Komatineni
2021-01-12 4:06 ` [PATCH v2] i2c: tegra: Create i2c_writesl_vi() to use with VI I2C for filling TX FIFO Sowjanya Komatineni
2021-01-12 5:56 ` Dmitry Osipenko
2021-01-12 16:57 ` Sowjanya Komatineni
2021-01-12 17:34 ` Dmitry Osipenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).