| Message ID | 20200908224006.25636-10-digetx@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Improvements for Tegra I2C driver | expand |
| Context | Check | Description |
|---|---|---|
| tagr/GVS | success | 1124251 |
| tagr/GVS-1122491 | pending | None |
| tagr/GVS-1122491 | fail | None |
| tagr/GVS | pending | 1122533 |
| tagr/GVS | success | 1122533 |
| tagr/GVS | pending | 1124251 |
On Wed, Sep 09, 2020 at 01:39:41AM +0300, Dmitry Osipenko wrote: > Use a single reset_control_reset() instead of assert/deasset couple in > order to make code cleaner a tad. Note that the reset_control_reset() > uses 1 microsecond delay instead of 2 that was used previously, but this > shouldn't matter because one microsecond is a default reset time for most > of Tegra peripherals and TRM doesn't mention anything special in regards > to I2C controller's reset propagation time. > > In addition don't ignore potential error of the reset control by emitting > a noisy warning if it fails, which will indicate an existence of a severe > problem, while still allow machine to boot up. > > Reviewed-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/i2c/busses/i2c-tegra.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) Acked-by: Thierry Reding <treding@nvidia.com>
On Wed, 09 Sep 2020 01:39:41 +0300, Dmitry Osipenko wrote: > Use a single reset_control_reset() instead of assert/deasset couple in > order to make code cleaner a tad. Note that the reset_control_reset() > uses 1 microsecond delay instead of 2 that was used previously, but this > shouldn't matter because one microsecond is a default reset time for most > of Tegra peripherals and TRM doesn't mention anything special in regards > to I2C controller's reset propagation time. > > In addition don't ignore potential error of the reset control by emitting > a noisy warning if it fails, which will indicate an existence of a severe > problem, while still allow machine to boot up. > > Reviewed-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > Acked-by: Thierry Reding <treding@nvidia.com> > --- > drivers/i2c/busses/i2c-tegra.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) Tested-by: Thierry Reding <treding@nvidia.com>
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index b813c0976c10..90ba2f5327c5 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -785,9 +785,16 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev) u32 tsu_thd; u8 tlow, thigh; - reset_control_assert(i2c_dev->rst); - udelay(2); - reset_control_deassert(i2c_dev->rst); + /* + * The reset shouldn't ever fail in practice. The failure will be a + * sign of a severe problem that needs to be resolved. Still we don't + * want to fail the initialization completely because this may break + * kernel boot up since voltage regulators use I2C. Hence, we will + * emit a noisy warning on error, which won't stay unnoticed and + * won't hose machine entirely. + */ + err = reset_control_reset(i2c_dev->rst); + WARN_ON_ONCE(err); if (i2c_dev->is_dvc) tegra_dvc_init(i2c_dev);