| Message ID | 1550334787-27703-1-git-send-email-skomatineni@nvidia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [V1] i2c: tegra: fix tegra186 hw supported features | expand |
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>
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
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
> > > > 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
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
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,
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(-)