Re: [PATCH 2/5] i2c: tegra: Restore pinmux on system resume

[Thierry Reding <thierry.reding@gmail.com>]

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

On Fri, May 08, 2020 at 12:50:09AM +0300, Dmitry Osipenko wrote:
> 06.05.2020 22:33, Thierry Reding пишет:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Depending on the board design, the I2C controllers found on Tegra SoCs
> > may require pinmuxing in order to function. This is done as part of the
> > driver's runtime suspend/resume operations. However, the PM core does
> > not allow devices to go into runtime suspend during system sleep to
> > avoid potential races with the suspend/resume of their parents.
> > 
> > As a result of this, when Tegra SoCs resume from system suspend, their
> > I2C controllers may have lost the pinmux state in hardware, whereas the
> > pinctrl subsystem is not aware of this. To fix this, make sure that if
> > the I2C controller is not runtime suspended, the runtime suspend code is
> > still executed in order to disable the module clock (which we don't need
> > to be enabled during sleep) and set the pinmux to the idle state.
> > 
> > Conversely, make sure that the I2C controller is properly resumed when
> > waking up from sleep so that pinmux settings are properly restored.
> > 
> > This fixes a bug seen with DDC transactions to an HDMI monitor timing
> > out when resuming from system suspend.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/i2c/busses/i2c-tegra.c | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> > index 7c88611c732c..db142d897604 100644
> > --- a/drivers/i2c/busses/i2c-tegra.c
> > +++ b/drivers/i2c/busses/i2c-tegra.c
> > @@ -1769,10 +1769,14 @@ static int tegra_i2c_remove(struct platform_device *pdev)
> >  static int __maybe_unused tegra_i2c_suspend(struct device *dev)
> >  {
> >  	struct tegra_i2c_dev *i2c_dev = dev_get_drvdata(dev);
> > +	int err = 0;
> >  
> >  	i2c_mark_adapter_suspended(&i2c_dev->adapter);
> >  
> > -	return 0;
> > +	if (!pm_runtime_status_suspended(dev))
> > +		err = tegra_i2c_runtime_suspend(dev);
> > +
> > +	return err;
> >  }
> >  
> >  static int __maybe_unused tegra_i2c_resume(struct device *dev)
> > @@ -1788,9 +1792,11 @@ static int __maybe_unused tegra_i2c_resume(struct device *dev)
> >  	if (err)
> >  		return err;
> >  
> > -	err = tegra_i2c_runtime_suspend(dev);
> > -	if (err)
> > -		return err;
> > +	if (pm_runtime_status_suspended(dev)) {
> > +		err = tegra_i2c_runtime_suspend(dev);
> > +		if (err)
> > +			return err;
> > +	}
> >  
> >  	i2c_mark_adapter_resumed(&i2c_dev->adapter);
> >  
> > 
> 
> Is it legal to touch DPAUX registers while DPAUX is in a suspended state?

DPAUX is never runtime suspended and the dependency from the I2C
controller on DPAUX should ensure that they are suspended and resumed in
the right order during system sleep.

Thierry

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