RE: [PATCH v2 4/5] spi: tegra210-quad: add acpi support

[Krishna Yarlagadda]

> -----Original Message-----
> From: Mark Brown <broonie@xxxxxxxxxx>
> Sent: 24 February 2022 23:52
> To: Krishna Yarlagadda <kyarlagadda@xxxxxxxxxx>
> Cc: thierry.reding@xxxxxxxxx; Jonathan Hunter <jonathanh@xxxxxxxxxx>; linux-spi@xxxxxxxxxxxxxxx; linux-tegra@xxxxxxxxxxxxxxx;
> Sowjanya Komatineni <skomatineni@xxxxxxxxxx>; Laxman Dewangan <ldewangan@xxxxxxxxxx>; robh+dt@xxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; p.zabel@xxxxxxxxxxxxxx
> Subject: Re: [PATCH v2 4/5] spi: tegra210-quad: add acpi support
> 
> On Wed, Feb 23, 2022 at 06:32:56AM +0000, Krishna Yarlagadda wrote:
> 
> > > > +	if (has_acpi_companion(tqspi->dev))
> > > > +		return 0;
> > > >  	/* flush all write which are in PPSB queue by reading back */
> > > >  	tegra_qspi_readl(tqspi, QSPI_COMMAND1);
> 
> > > As well as clock stuff this is also skipping flushing of pending writes
> > > - is that intentional?  It's not called out in the changelog and seems like
> > > something that could cause issues if someone runs on a system where
> > > the firmware does implement runtime suspend.
> 
> > Runtime suspend is not enabled with ACPI firmware. Converted compiler flag in v1 to runtime check.
> > We must add more changes like setting DPM flags for runtime pm support with ACPI.
> > Can take this as part of a different series.
> 
> It at least needs to be clearer what's going on here, the changelog
> doesn't match the code and it's not obvious from the code that ACPI
> won't kick in and power manage the device as things stand.
Understood Mark. I will add comments to make it clear.
Also update change log indicating runtime suspend does not work
with ACPI.