linux-kernel - RE: [RFC/RFT PATCH v2] ASoC: fsl_esai: Revert "ETDR and TX0~5 registers are non volatile"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <VE1PR04MB6479D4B1D5F00B07C5CECC5BE3EF0@VE1PR04MB6479.eurprd04.prod.outlook.com>
Date:   Thu, 13 Jun 2019 03:00:58 +0000
From:   "S.j. Wang" <shengjiu.wang@....com>
To:     Nicolin Chen <nicoleotsuka@...il.com>,
        "broonie@...nel.org" <broonie@...nel.org>
CC:     "timur@...nel.org" <timur@...nel.org>,
        "Xiubo.Lee@...il.com" <Xiubo.Lee@...il.com>,
        "festevam@...il.com" <festevam@...il.com>,
        "lgirdwood@...il.com" <lgirdwood@...il.com>,
        "perex@...ex.cz" <perex@...ex.cz>,
        "tiwai@...e.com" <tiwai@...e.com>,
        "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFC/RFT PATCH v2] ASoC: fsl_esai: Revert "ETDR and TX0~5
 registers are non volatile"

Hi
> 
> 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@...il.com>
> Cc: Shengjiu Wang <shengjiu.wang@....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.

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.

Here is the code for regcache_default_sync()

static int regcache_default_sync(struct regmap *map, unsigned int min,
                                 unsigned int max)
{
        unsigned int reg;

        for (reg = min; reg <= max; reg += map->reg_stride) {
                unsigned int val;
                int ret;

                if (regmap_volatile(map, reg) ||
                    !regmap_writeable(map, reg))
                        continue;

                ret = regcache_read(map, reg, &val);
                if (ret)
                        return ret;

                if (!regcache_reg_needs_sync(map, reg, val))
                        continue;

                map->cache_bypass = true;
                ret = _regmap_write(map, reg, val);
                map->cache_bypass = false;

Best regards
Wang shengjiu