Hi Shengjiu,
On Thu, Jun 13, 2019 at 03:00:58AM +0000, S.j. Wang wrote:
Commit 8973112aa41b ("ASoC: fsl_esai: ETDR and TX0~5 registers are non volatile") removed TX data registers from the volatile_reg list and appended default values for them. However, being data registers of TX, they should not have been removed from the list because they should not be cached -- see the following reason.
When doing regcache_sync(), this operation might accidentally write some dirty data to these registers, in case that cached data happen to be different from the default ones, which might also result in a channel shift or swap situation, since the number of write-via-sync operations at ETDR would very unlikely match the channel number.
So this patch reverts the original commit to keep TX data registers in volatile_reg list in order to prevent them from being written by regcache_sync().
Note: this revert is not a complete revert as it keeps those macros of registers remaining in the default value list while the original commit also changed other entries in the list. And this patch isn't very necessary to Cc stable tree since there has been always a FIFO reset operation around the regcache_sync() call, even prior to this reverted commit.
Signed-off-by: Nicolin Chen nicoleotsuka@gmail.com Cc: Shengjiu Wang shengjiu.wang@nxp.com
Hi Mark, In case there's no objection against the patch, I'd still like to wait for a Tested-by from NXP folks before submitting it. Thanks!
bool regmap_volatile(struct regmap *map, unsigned int reg) { if (!map->format.format_write && !regmap_readable(map, reg)) return false;
Actually with this patch, the regcache_sync will write the 0 to ETDR, even It is declared volatile, the reason is that in regmap_volatile(), the first condition
(!map->format.format_write && !regmap_readable(map, reg)) is true.
So the regmap_volatile will return false.
Interesting finding.....so a write-only register will not be treated as a volatile register (to avoid regcache_sync) at all....
And in regcache_reg_needs_sync(), because there is no default value It will return true, then the ETDR need be synced, and be written 0.
Looks like either way of keeping them in or out of volatile_reg list might have the same result of having a data being written, while our current code at least would not force to write 0.
So I think having a FIFO reset won't be a bad idea at all. And since our suspend/resume() functions are already doing regcache_sync() with a FIFO reset, we can just reuse that code for your reset routine.
Thanks a lot Nicolin