From: Mikko Perttunen <cyndis@kapsi.fi>
To: Dmitry Osipenko <digetx@gmail.com>,
Mikko Perttunen <mperttunen@nvidia.com>,
ldewangan@nvidia.com, thierry.reding@gmail.com,
jonathanh@nvidia.com, wsa@kernel.org
Cc: linux-i2c@vger.kernel.org, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] i2c: tegra: Wait for config load atomically while in ISR
Date: Mon, 11 Jan 2021 17:52:19 +0200 [thread overview]
Message-ID: <a180bd42-2cd9-87b6-1848-61058f198d63@kapsi.fi> (raw)
In-Reply-To: <b7f69fb9-2960-f994-51d8-afd86af26bba@gmail.com>
On 1/11/21 5:14 PM, Dmitry Osipenko wrote:
> 11.01.2021 16:55, Mikko Perttunen пишет:
>> Upon a communication error, the interrupt handler can call
>> tegra_i2c_disable_packet_mode. This causes a sleeping poll to happen
>> unless the current transaction was marked atomic. Since
>> tegra_i2c_disable_packet_mode is only called from the interrupt path,
>> make it use atomic waiting always.
>>
>> Fixes: ede2299f7101 ("i2c: tegra: Support atomic transfers")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>> ---
>> This fixes constant spew for me while HDMI is connected to a
>> powered off television.
>>
>> drivers/i2c/busses/i2c-tegra.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>> index 6f08c0c3238d..4a7ff1840565 100644
>> --- a/drivers/i2c/busses/i2c-tegra.c
>> +++ b/drivers/i2c/busses/i2c-tegra.c
>> @@ -528,12 +528,12 @@ static void tegra_i2c_vi_init(struct tegra_i2c_dev *i2c_dev)
>>
>> static int tegra_i2c_poll_register(struct tegra_i2c_dev *i2c_dev,
>> u32 reg, u32 mask, u32 delay_us,
>> - u32 timeout_us)
>> + u32 timeout_us, bool force_atomic)
>> {
>> void __iomem *addr = i2c_dev->base + tegra_i2c_reg_addr(i2c_dev, reg);
>> u32 val;
>>
>> - if (!i2c_dev->atomic_mode)
>> + if (!i2c_dev->atomic_mode && !force_atomic)
>> return readl_relaxed_poll_timeout(addr, val, !(val & mask),
>> delay_us, timeout_us);
>>
>> @@ -560,7 +560,7 @@ static int tegra_i2c_flush_fifos(struct tegra_i2c_dev *i2c_dev)
>> val |= mask;
>> i2c_writel(i2c_dev, val, offset);
>>
>> - err = tegra_i2c_poll_register(i2c_dev, offset, mask, 1000, 1000000);
>> + err = tegra_i2c_poll_register(i2c_dev, offset, mask, 1000, 1000000, false);
>> if (err) {
>> dev_err(i2c_dev->dev, "failed to flush FIFO\n");
>> return err;
>> @@ -569,7 +569,7 @@ static int tegra_i2c_flush_fifos(struct tegra_i2c_dev *i2c_dev)
>> return 0;
>> }
>>
>> -static int tegra_i2c_wait_for_config_load(struct tegra_i2c_dev *i2c_dev)
>> +static int tegra_i2c_wait_for_config_load(struct tegra_i2c_dev *i2c_dev, bool force_atomic)
>> {
>> int err;
>>
>> @@ -579,7 +579,7 @@ static int tegra_i2c_wait_for_config_load(struct tegra_i2c_dev *i2c_dev)
>> i2c_writel(i2c_dev, I2C_MSTR_CONFIG_LOAD, I2C_CONFIG_LOAD);
>>
>> err = tegra_i2c_poll_register(i2c_dev, I2C_CONFIG_LOAD, 0xffffffff,
>> - 1000, I2C_CONFIG_LOAD_TIMEOUT);
>> + 1000, I2C_CONFIG_LOAD_TIMEOUT, force_atomic);
>> if (err) {
>> dev_err(i2c_dev->dev, "failed to load config\n");
>> return err;
>> @@ -684,7 +684,7 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
>> if (i2c_dev->multimaster_mode && i2c_dev->hw->has_slcg_override_reg)
>> i2c_writel(i2c_dev, I2C_MST_CORE_CLKEN_OVR, I2C_CLKEN_OVERRIDE);
>>
>> - err = tegra_i2c_wait_for_config_load(i2c_dev);
>> + err = tegra_i2c_wait_for_config_load(i2c_dev, false);
>> if (err)
>> return err;
>>
>> @@ -707,7 +707,7 @@ static int tegra_i2c_disable_packet_mode(struct tegra_i2c_dev *i2c_dev)
>> if (cnfg & I2C_CNFG_PACKET_MODE_EN)
>> i2c_writel(i2c_dev, cnfg & ~I2C_CNFG_PACKET_MODE_EN, I2C_CNFG);
>>
>> - return tegra_i2c_wait_for_config_load(i2c_dev);
>> + return tegra_i2c_wait_for_config_load(i2c_dev, true);
>> }
>>
>> static int tegra_i2c_empty_rx_fifo(struct tegra_i2c_dev *i2c_dev)
>> @@ -1090,7 +1090,7 @@ static int tegra_i2c_issue_bus_clear(struct i2c_adapter *adap)
>> I2C_BC_TERMINATE;
>> i2c_writel(i2c_dev, val, I2C_BUS_CLEAR_CNFG);
>>
>> - err = tegra_i2c_wait_for_config_load(i2c_dev);
>> + err = tegra_i2c_wait_for_config_load(i2c_dev, false);
>> if (err)
>> return err;
>>
>>
>
> What about a simpler variant:
>
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index 6f08c0c3238d..0727383f4940 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -533,7 +533,7 @@ static int tegra_i2c_poll_register(struct
> tegra_i2c_dev *i2c_dev,
> void __iomem *addr = i2c_dev->base + tegra_i2c_reg_addr(i2c_dev, reg);
> u32 val;
>
> - if (!i2c_dev->atomic_mode)
> + if (!i2c_dev->atomic_mode && !in_irq())
> return readl_relaxed_poll_timeout(addr, val, !(val & mask),
> delay_us, timeout_us);
>
Yep, I'll post a v2 with that.
Mikko
prev parent reply other threads:[~2021-01-11 15:53 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-11 13:55 [PATCH] i2c: tegra: Wait for config load atomically while in ISR Mikko Perttunen
2021-01-11 14:31 ` David Laight
2021-01-11 14:38 ` Mikko Perttunen
2021-01-11 15:14 ` Dmitry Osipenko
2021-01-11 15:52 ` Mikko Perttunen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a180bd42-2cd9-87b6-1848-61058f198d63@kapsi.fi \
--to=cyndis@kapsi.fi \
--cc=digetx@gmail.com \
--cc=jonathanh@nvidia.com \
--cc=ldewangan@nvidia.com \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=mperttunen@nvidia.com \
--cc=stable@vger.kernel.org \
--cc=thierry.reding@gmail.com \
--cc=wsa@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox