This patch fixes the Tegra DMA config option processing in the
i2c-tegra driver.
Tegra processors prior to Tegra186 used APB DMA for I2C requiring
CONFIG_TEGRA20_APB_DMA=y while Tegra186 and later use GPC DMA requiring
CONFIG_TEGRA186_GPC_DMA=y.
The check for if the processor uses APB DMA is inverted and so the wrong
DMA config options are checked.
This means if CONFIG_TEGRA20_APB_DMA=y but CONFIG_TEGRA186_GPC_DMA=n
with a Tegra186 or later processor the driver will incorrectly think DMA is
enabled and attempt to request DMA channels that will never be availible,
leaving the driver in a perpetual EPROBE_DEFER state.
Signed-off-by: Parker Newman <[email protected]>
---
drivers/i2c/busses/i2c-tegra.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index bcbbf23aa530..dc6ed3a8d69e 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -442,7 +442,7 @@ static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev)
if (IS_VI(i2c_dev))
return 0;
- if (!i2c_dev->hw->has_apb_dma) {
+ if (i2c_dev->hw->has_apb_dma) {
if (!IS_ENABLED(CONFIG_TEGRA20_APB_DMA)) {
dev_dbg(i2c_dev->dev, "APB DMA support not enabled\n");
return 0;
--
2.17.1
BTW...
On Thu, Aug 03, 2023 at 05:10:02PM +0000, Parker Newman wrote:
>
you have a blank line here.
> This patch fixes the Tegra DMA config option processing in the
> i2c-tegra driver.
>
> Tegra processors prior to Tegra186 used APB DMA for I2C requiring
> CONFIG_TEGRA20_APB_DMA=y while Tegra186 and later use GPC DMA requiring
> CONFIG_TEGRA186_GPC_DMA=y.
>
> The check for if the processor uses APB DMA is inverted and so the wrong
> DMA config options are checked.
>
> This means if CONFIG_TEGRA20_APB_DMA=y but CONFIG_TEGRA186_GPC_DMA=n
> with a Tegra186 or later processor the driver will incorrectly think DMA is
> enabled and attempt to request DMA channels that will never be availible,
> leaving the driver in a perpetual EPROBE_DEFER state.
>
> Signed-off-by: Parker Newman <[email protected]>
As this is a fix you also need to add
Fixes: 48cb6356fae1 ("i2c: tegra: Add GPCDMA support")
Cc: Akhil R <[email protected]>
Cc: <[email protected]> # v6.1+
Cc'eing Akhil as well for his opinion on this.
Andi
Hi Laxman and/or Dmitry,
On Thu, Aug 03, 2023 at 05:10:02PM +0000, Parker Newman wrote:
>
> This patch fixes the Tegra DMA config option processing in the
> i2c-tegra driver.
>
> Tegra processors prior to Tegra186 used APB DMA for I2C requiring
> CONFIG_TEGRA20_APB_DMA=y while Tegra186 and later use GPC DMA requiring
> CONFIG_TEGRA186_GPC_DMA=y.
>
> The check for if the processor uses APB DMA is inverted and so the wrong
> DMA config options are checked.
>
> This means if CONFIG_TEGRA20_APB_DMA=y but CONFIG_TEGRA186_GPC_DMA=n
> with a Tegra186 or later processor the driver will incorrectly think DMA is
> enabled and attempt to request DMA channels that will never be availible,
> leaving the driver in a perpetual EPROBE_DEFER state.
>
> Signed-off-by: Parker Newman <[email protected]>
> ---
> drivers/i2c/busses/i2c-tegra.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index bcbbf23aa530..dc6ed3a8d69e 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -442,7 +442,7 @@ static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev)
> if (IS_VI(i2c_dev))
> return 0;
>
> - if (!i2c_dev->hw->has_apb_dma) {
> + if (i2c_dev->hw->has_apb_dma) {
> if (!IS_ENABLED(CONFIG_TEGRA20_APB_DMA)) {
> dev_dbg(i2c_dev->dev, "APB DMA support not enabled\n");
> return 0;
Can I have your opinion here, please?
Andi
>
> BTW...
>
> On Thu, Aug 03, 2023 at 05:10:02PM +0000, Parker Newman wrote:
> >
>
> you have a blank line here.
>
> > This patch fixes the Tegra DMA config option processing in the
> > i2c-tegra driver.
> >
> > Tegra processors prior to Tegra186 used APB DMA for I2C requiring
> > CONFIG_TEGRA20_APB_DMA=y while Tegra186 and later use GPC DMA
> > requiring CONFIG_TEGRA186_GPC_DMA=y.
> >
> > The check for if the processor uses APB DMA is inverted and so the
> > wrong DMA config options are checked.
> >
> > This means if CONFIG_TEGRA20_APB_DMA=y but
> CONFIG_TEGRA186_GPC_DMA=n
> > with a Tegra186 or later processor the driver will incorrectly think
> > DMA is enabled and attempt to request DMA channels that will never be
> > availible, leaving the driver in a perpetual EPROBE_DEFER state.
> >
> > Signed-off-by: Parker Newman <[email protected]>
>
> As this is a fix you also need to add
>
> Fixes: 48cb6356fae1 ("i2c: tegra: Add GPCDMA support")
> Cc: Akhil R <[email protected]>
> Cc: <[email protected]> # v6.1+
>
> Cc'eing Akhil as well for his opinion on this.
The fix looks valid to me. Must have been a typo there.
Regards,
Akhil
On 2023-08-06 10:21, Akhil R wrote:
>>
>> BTW...
>>
>> On Thu, Aug 03, 2023 at 05:10:02PM +0000, Parker Newman wrote:
>>>
>>
>> you have a blank line here.
>>
>>> This patch fixes the Tegra DMA config option processing in the
>>> i2c-tegra driver.
>>>
>>> Tegra processors prior to Tegra186 used APB DMA for I2C requiring
>>> CONFIG_TEGRA20_APB_DMA=y while Tegra186 and later use GPC DMA
>>> requiring CONFIG_TEGRA186_GPC_DMA=y.
>>>
>>> The check for if the processor uses APB DMA is inverted and so the
>>> wrong DMA config options are checked.
>>>
>>> This means if CONFIG_TEGRA20_APB_DMA=y but
>> CONFIG_TEGRA186_GPC_DMA=n
>>> with a Tegra186 or later processor the driver will incorrectly think
>>> DMA is enabled and attempt to request DMA channels that will never be
>>> availible, leaving the driver in a perpetual EPROBE_DEFER state.
>>>
>>> Signed-off-by: Parker Newman <[email protected]>
>>
>> As this is a fix you also need to add
>>
>> Fixes: 48cb6356fae1 ("i2c: tegra: Add GPCDMA support")
>> Cc: Akhil R <[email protected]>
>> Cc: <[email protected]> # v6.1+
>>
>> Cc'eing Akhil as well for his opinion on this.
> The fix looks valid to me. Must have been a typo there.
>
> Regards,
> Akhil
>
Yes it appears to be a simple typo and if you have both DMA Config options
set the bug would get missed.
I am new to the Linux mailing list, should I send a new message to
[email protected] or CC them on this one?
Thanks,
Parker
Hi Parker,
On Tue, Aug 08, 2023 at 12:42:28PM +0000, Parker Newman wrote:
> On 2023-08-06 10:21, Akhil R wrote:
> >>
> >> BTW...
> >>
> >> On Thu, Aug 03, 2023 at 05:10:02PM +0000, Parker Newman wrote:
> >>>
> >>
> >> you have a blank line here.
> >>
> >>> This patch fixes the Tegra DMA config option processing in the
> >>> i2c-tegra driver.
> >>>
> >>> Tegra processors prior to Tegra186 used APB DMA for I2C requiring
> >>> CONFIG_TEGRA20_APB_DMA=y while Tegra186 and later use GPC DMA
> >>> requiring CONFIG_TEGRA186_GPC_DMA=y.
> >>>
> >>> The check for if the processor uses APB DMA is inverted and so the
> >>> wrong DMA config options are checked.
> >>>
> >>> This means if CONFIG_TEGRA20_APB_DMA=y but
> >> CONFIG_TEGRA186_GPC_DMA=n
> >>> with a Tegra186 or later processor the driver will incorrectly think
> >>> DMA is enabled and attempt to request DMA channels that will never be
> >>> availible, leaving the driver in a perpetual EPROBE_DEFER state.
> >>>
> >>> Signed-off-by: Parker Newman <[email protected]>
> >>
> >> As this is a fix you also need to add
> >>
> >> Fixes: 48cb6356fae1 ("i2c: tegra: Add GPCDMA support")
> >> Cc: Akhil R <[email protected]>
> >> Cc: <[email protected]> # v6.1+
> >>
> >> Cc'eing Akhil as well for his opinion on this.
> > The fix looks valid to me. Must have been a typo there.
> >
> > Regards,
> > Akhil
> >
> Yes it appears to be a simple typo and if you have both DMA Config options
> set the bug would get missed.
>
> I am new to the Linux mailing list, should I send a new message to
> [email protected] or CC them on this one?
First of all... welcome to the Linux mailing list :)
No need to resend, I can add it... I was hoping for an official
ack, but as that's not coming, I will ack it
Acked-by: Andi Shyti <[email protected]>
Thank you Parker and wishing to see more patches from you ;)
Andi
Hi
On Thu, 03 Aug 2023 17:10:02 +0000, Parker Newman wrote:
> This patch fixes the Tegra DMA config option processing in the
> i2c-tegra driver.
>
> Tegra processors prior to Tegra186 used APB DMA for I2C requiring
> CONFIG_TEGRA20_APB_DMA=y while Tegra186 and later use GPC DMA requiring
> CONFIG_TEGRA186_GPC_DMA=y.
>
> [...]
With the Fixes tag added, applied to i2c/andi-for-current on
https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git
Please note that this patch may still undergo further evaluation
and the final decision will be made in collaboration with
Wolfram.
Thank you,
Andi
Patches applied
===============
[1/1] i2c: tegra: Fix i2c-tegra DMA config option processing
commit: fc9a464f3d9a2a361e8bcb960345270a9dc46054
On Thu, Aug 03, 2023 at 05:10:02PM +0000, Parker Newman wrote:
>
> This patch fixes the Tegra DMA config option processing in the
> i2c-tegra driver.
>
> Tegra processors prior to Tegra186 used APB DMA for I2C requiring
> CONFIG_TEGRA20_APB_DMA=y while Tegra186 and later use GPC DMA requiring
> CONFIG_TEGRA186_GPC_DMA=y.
>
> The check for if the processor uses APB DMA is inverted and so the wrong
> DMA config options are checked.
>
> This means if CONFIG_TEGRA20_APB_DMA=y but CONFIG_TEGRA186_GPC_DMA=n
> with a Tegra186 or later processor the driver will incorrectly think DMA is
> enabled and attempt to request DMA channels that will never be availible,
> leaving the driver in a perpetual EPROBE_DEFER state.
>
> Signed-off-by: Parker Newman <[email protected]>
Applied to for-current, thanks!
> > Cc'eing Akhil as well for his opinion on this.
> The fix looks valid to me. Must have been a typo there.
I'll count this as an ack. Thanks!
> > > Cc'eing Akhil as well for his opinion on this.
> > The fix looks valid to me. Must have been a typo there.
>
> I'll count this as an ack. Thanks!
Oh Sorry. I just realized I didn't put an ack.
Acked-by: Akhil R <[email protected]>
05.08.2023 00:49, Andi Shyti пишет:
> Hi Laxman and/or Dmitry,
>
> On Thu, Aug 03, 2023 at 05:10:02PM +0000, Parker Newman wrote:
>>
>> This patch fixes the Tegra DMA config option processing in the
>> i2c-tegra driver.
>>
>> Tegra processors prior to Tegra186 used APB DMA for I2C requiring
>> CONFIG_TEGRA20_APB_DMA=y while Tegra186 and later use GPC DMA requiring
>> CONFIG_TEGRA186_GPC_DMA=y.
>>
>> The check for if the processor uses APB DMA is inverted and so the wrong
>> DMA config options are checked.
>>
>> This means if CONFIG_TEGRA20_APB_DMA=y but CONFIG_TEGRA186_GPC_DMA=n
>> with a Tegra186 or later processor the driver will incorrectly think DMA is
>> enabled and attempt to request DMA channels that will never be availible,
>> leaving the driver in a perpetual EPROBE_DEFER state.
>>
>> Signed-off-by: Parker Newman <[email protected]>
>> ---
>> drivers/i2c/busses/i2c-tegra.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>> index bcbbf23aa530..dc6ed3a8d69e 100644
>> --- a/drivers/i2c/busses/i2c-tegra.c
>> +++ b/drivers/i2c/busses/i2c-tegra.c
>> @@ -442,7 +442,7 @@ static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev)
>> if (IS_VI(i2c_dev))
>> return 0;
>>
>> - if (!i2c_dev->hw->has_apb_dma) {
>> + if (i2c_dev->hw->has_apb_dma) {
>> if (!IS_ENABLED(CONFIG_TEGRA20_APB_DMA)) {
>> dev_dbg(i2c_dev->dev, "APB DMA support not enabled\n");
>> return 0;
>
> Can I have your opinion here, please?
The patch looks good, thanks Parker for fixing it. I'll be able to test
it only sometime later and let you all know if there will be any
problem. Previously I haven't noticed any Tegra I2C regressions, maybe
we should change that dev_dbg to dev_warn.
On 2023-08-14 11:25, Dmitry Osipenko wrote:
> 05.08.2023 00:49, Andi Shyti пишет:
>> Hi Laxman and/or Dmitry,
>>
>> On Thu, Aug 03, 2023 at 05:10:02PM +0000, Parker Newman wrote:
>>>
>>> This patch fixes the Tegra DMA config option processing in the
>>> i2c-tegra driver.
>>>
>>> Tegra processors prior to Tegra186 used APB DMA for I2C requiring
>>> CONFIG_TEGRA20_APB_DMA=y while Tegra186 and later use GPC DMA requiring
>>> CONFIG_TEGRA186_GPC_DMA=y.
>>>
>>> The check for if the processor uses APB DMA is inverted and so the wrong
>>> DMA config options are checked.
>>>
>>> This means if CONFIG_TEGRA20_APB_DMA=y but CONFIG_TEGRA186_GPC_DMA=n
>>> with a Tegra186 or later processor the driver will incorrectly think DMA is
>>> enabled and attempt to request DMA channels that will never be availible,
>>> leaving the driver in a perpetual EPROBE_DEFER state.
>>>
>>> Signed-off-by: Parker Newman <[email protected]>
>>> ---
>>> drivers/i2c/busses/i2c-tegra.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>>> index bcbbf23aa530..dc6ed3a8d69e 100644
>>> --- a/drivers/i2c/busses/i2c-tegra.c
>>> +++ b/drivers/i2c/busses/i2c-tegra.c
>>> @@ -442,7 +442,7 @@ static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev)
>>> if (IS_VI(i2c_dev))
>>> return 0;
>>>
>>> - if (!i2c_dev->hw->has_apb_dma) {
>>> + if (i2c_dev->hw->has_apb_dma) {
>>> if (!IS_ENABLED(CONFIG_TEGRA20_APB_DMA)) {
>>> dev_dbg(i2c_dev->dev, "APB DMA support not enabled\n");
>>> return 0;
>>
>> Can I have your opinion here, please?
>
> The patch looks good, thanks Parker for fixing it. I'll be able to test
> it only sometime later and let you all know if there will be any
> problem. Previously I haven't noticed any Tegra I2C regressions, maybe
> we should change that dev_dbg to dev_warn.
>
Hi Dmitry,
You will not notice any issues if both options are set (or not set) as it
will fall through and configure the DMA or skip DMA setup as expected.
I only noticed the issue after I enabled CONFIG_TEGRA20_APB_DMA which is a
KConfig requirement for the Tegra HS-UART driver and my I2C stopped working.
Which now as I write this I realize is possibly another "bug"... As far as I know
the HS UARTs also use GPC DMA on T186 or later? I would need to look into that.
I don't think the print needs to be a warning, not having DMA enabled
is a valid option, it just needed the correct CONFIG options to be checked for
the DMA type (APB versus GPC).
-Parker