[PATCH V1] i2c: tegra: fix tegra186 hw supported features

[PATCH V1] i2c: tegra: fix tegra186 hw supported features

[Sowjanya Komatineni]

Tegra186 does not support multi-master mode and also there is no
master fifo control register.

This patch fixes supported features of Tegra186 and prevents
crashing during boot as master fifo control register are not
present on Tegra186 and prior.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index a4cd79c9f7a7..24c206db70a4 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -1434,9 +1434,9 @@ static const struct tegra_i2c_hw_feature tegra186_i2c_hw = {
 	.clk_divisor_fast_mode = 0x19,
 	.clk_divisor_fast_plus_mode = 0x10,
 	.has_config_load_reg = true,
-	.has_multi_master_mode = true,
+	.has_multi_master_mode = false,
 	.has_slcg_override_reg = true,
-	.has_mst_fifo = true,
+	.has_mst_fifo = false,
 	.quirks = &tegra_i2c_quirks,
 	.supports_bus_clear = true,
 	.has_apb_dma = false,
-- 
2.7.4

Re: [PATCH V1] i2c: tegra: fix tegra186 hw supported features

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 703 bytes --]

On Sat, Feb 16, 2019 at 08:33:07AM -0800, Sowjanya Komatineni wrote:
> Tegra186 does not support multi-master mode and also there is no
> master fifo control register.
> 
> This patch fixes supported features of Tegra186 and prevents
> crashing during boot as master fifo control register are not
> present on Tegra186 and prior.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

We really should've caught this earlier. Jon, let's think about ways to
make it easier to catch these things in the future on our test system.

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH V1] i2c: tegra: fix tegra186 hw supported features

[Jon Hunter]

On 18/02/2019 08:42, Thierry Reding wrote:
> On Sat, Feb 16, 2019 at 08:33:07AM -0800, Sowjanya Komatineni wrote:
>> Tegra186 does not support multi-master mode and also there is no
>> master fifo control register.
>>
>> This patch fixes supported features of Tegra186 and prevents
>> crashing during boot as master fifo control register are not
>> present on Tegra186 and prior.
>>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>>  drivers/i2c/busses/i2c-tegra.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> We really should've caught this earlier. Jon, let's think about ways to
> make it easier to catch these things in the future on our test system.

Indeed. This has fixed the boot regression I was seeing over the weekend
on -next for Tegra186. However, I am bit confused here, because when I
look at the Tegra186 TRM it states that the I2C supports multi-master
mode which disagrees with this patch. Furthermore, it seems odd that
Tegra210 and Tegra194 would support multi-master mode but Tegra186 does
not. So is this really correct?

Cheers
Jon

-- 
nvpublic

Re: [PATCH V1] i2c: tegra: fix tegra186 hw supported features

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1740 bytes --]

On Mon, Feb 18, 2019 at 09:34:38AM +0000, Jon Hunter wrote:
> 
> On 18/02/2019 08:42, Thierry Reding wrote:
> > On Sat, Feb 16, 2019 at 08:33:07AM -0800, Sowjanya Komatineni wrote:
> >> Tegra186 does not support multi-master mode and also there is no
> >> master fifo control register.
> >>
> >> This patch fixes supported features of Tegra186 and prevents
> >> crashing during boot as master fifo control register are not
> >> present on Tegra186 and prior.
> >>
> >> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> >> ---
> >>  drivers/i2c/busses/i2c-tegra.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > We really should've caught this earlier. Jon, let's think about ways to
> > make it easier to catch these things in the future on our test system.
> 
> Indeed. This has fixed the boot regression I was seeing over the weekend
> on -next for Tegra186. However, I am bit confused here, because when I
> look at the Tegra186 TRM it states that the I2C supports multi-master
> mode which disagrees with this patch. Furthermore, it seems odd that
> Tegra210 and Tegra194 would support multi-master mode but Tegra186 does
> not. So is this really correct?

I would expect at least the missing master FIFO registers to cause a
crash on Tegra186. The internal architecture specification also says
that multi-master is supported.

Sowjanya: do you have other documents that suggest we don't support
multi-master on Tegra186 specifically? Or is this to work around an
issue specific to Tegra186?

As it is this contradicts documentation, so we need to either fix the
commit to remove only master FIFO support or we need to fix the docs
to reflect reality.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH V1] i2c: tegra: fix tegra186 hw supported features

[Sowjanya Komatineni]

> > 
> > On 18/02/2019 08:42, Thierry Reding wrote:
> > > On Sat, Feb 16, 2019 at 08:33:07AM -0800, Sowjanya Komatineni wrote:
> > >> Tegra186 does not support multi-master mode and also there is no 
> > >> master fifo control register.
> > >>
> > >> This patch fixes supported features of Tegra186 and prevents 
> > >> crashing during boot as master fifo control register are not 
> > >> present on Tegra186 and prior.
> > >>
> > >> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> > >> ---
> > >>  drivers/i2c/busses/i2c-tegra.c | 4 ++--
> > >>  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > We really should've caught this earlier. Jon, let's think about ways 
> > > to make it easier to catch these things in the future on our test system.
> > 
> > Indeed. This has fixed the boot regression I was seeing over the 
> > weekend on -next for Tegra186. However, I am bit confused here, 
> > because when I look at the Tegra186 TRM it states that the I2C 
> > supports multi-master mode which disagrees with this patch. 
> > Furthermore, it seems odd that
> > Tegra210 and Tegra194 would support multi-master mode but Tegra186 
> > does not. So is this really correct?
> >
> > I would expect at least the missing master FIFO registers to cause a crash on Tegra186. The internal architecture specification also says that multi-master is supported.
> >
> > Sowjanya: do you have other documents that suggest we don't support multi-master on Tegra186 specifically? Or is this to work around an issue specific to Tegra186?
> >
> > As it is this contradicts documentation, so we need to either fix the commit to remove only master FIFO support or we need to fix the docs to reflect reality.
> >
> > Thierry

Thierry/Jonathan,

Design wise we support but due to known hw bugs we defeatured multi-master mode. TRM docs also says no multi master support for T210, T186 and multi-master is supported only on T194.

T210 also has multi master enabled in driver. We should disable that also.

Thanks
sowjanya

Re: [PATCH V1] i2c: tegra: fix tegra186 hw supported features

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2503 bytes --]

On Mon, Feb 18, 2019 at 04:13:46PM +0000, Sowjanya Komatineni wrote:
> > > 
> > > On 18/02/2019 08:42, Thierry Reding wrote:
> > > > On Sat, Feb 16, 2019 at 08:33:07AM -0800, Sowjanya Komatineni wrote:
> > > >> Tegra186 does not support multi-master mode and also there is no 
> > > >> master fifo control register.
> > > >>
> > > >> This patch fixes supported features of Tegra186 and prevents 
> > > >> crashing during boot as master fifo control register are not 
> > > >> present on Tegra186 and prior.
> > > >>
> > > >> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> > > >> ---
> > > >>  drivers/i2c/busses/i2c-tegra.c | 4 ++--
> > > >>  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > We really should've caught this earlier. Jon, let's think about ways 
> > > > to make it easier to catch these things in the future on our test system.
> > > 
> > > Indeed. This has fixed the boot regression I was seeing over the 
> > > weekend on -next for Tegra186. However, I am bit confused here, 
> > > because when I look at the Tegra186 TRM it states that the I2C 
> > > supports multi-master mode which disagrees with this patch. 
> > > Furthermore, it seems odd that
> > > Tegra210 and Tegra194 would support multi-master mode but Tegra186 
> > > does not. So is this really correct?
> > >
> > > I would expect at least the missing master FIFO registers to cause a crash on Tegra186. The internal architecture specification also says that multi-master is supported.
> > >
> > > Sowjanya: do you have other documents that suggest we don't support multi-master on Tegra186 specifically? Or is this to work around an issue specific to Tegra186?
> > >
> > > As it is this contradicts documentation, so we need to either fix the commit to remove only master FIFO support or we need to fix the docs to reflect reality.
> > >
> > > Thierry
> 
> Thierry/Jonathan,
> 
> Design wise we support but due to known hw bugs we defeatured
> multi-master mode. TRM docs also says no multi master support for
> T210, T186 and multi-master is supported only on T194.
> 
> T210 also has multi master enabled in driver. We should disable that
> also.

Okay, I think it makes sense to split this into two patches:

1) remove master FIFO support on Tegra186 because that leads to a
   crash

2) remove multi-master support on Tegra210 and Tegra186 with a commit
   message that explains that it was defeatured because of known bugs

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]