Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220620134758.1286480-2-aisheng.dong@nxp.com>
 <YrCM0reni+x/KWsG@sirena.org.uk> <DB9PR04MB84779EF2842D789FA66094C380B09@DB9PR04MB8477.eurprd04.prod.outlook.com>
 <YrCXILblKsp6DuN3@sirena.org.uk> <DB9PR04MB8477CD99D5847291A629994180B09@DB9PR04MB8477.eurprd04.prod.outlook.com>
 <YrCznap77OyHu4bO@sirena.org.uk> <DB9PR04MB847785E1861525FC1E4AD97280B39@DB9PR04MB8477.eurprd04.prod.outlook.com>
 <YrHkXH1M4NydBfQT@sirena.org.uk> <DB9PR04MB84772F73D4AA1B49F87BECCF80B39@DB9PR04MB8477.eurprd04.prod.outlook.com>
 <CAA+hA=RQVRHzdy_ZpwcC-ZB8mwUYuMYu_iLNAfuPcMCS1G9WXA@mail.gmail.com> <YrMMzizzE7rIji5x@sirena.org.uk>
In-Reply-To: <YrMMzizzE7rIji5x@sirena.org.uk>
From:   Dong Aisheng <dongas86@gmail.com>
Date:   Thu, 23 Jun 2022 00:05:46 +0800
Message-ID: <CAA+hA=SA7BY8WxrbuRAOsuHyz+bx-EJq-UT_YLW5wYMS+io+pg@mail.gmail.com>
Subject: Re: [PATCH RFC 1/2] regmap: add option to disable debugfs
To:     Mark Brown <broonie@kernel.org>
Cc:     Aisheng Dong <aisheng.dong@nxp.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "l.stach@pengutronix.de" <l.stach@pengutronix.de>,
        Peng Fan <peng.fan@nxp.com>,
        "shawnguo@kernel.org" <shawnguo@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Jun 22, 2022 at 8:36 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Jun 22, 2022 at 06:12:49PM +0800, Dong Aisheng wrote:
>
> > NOTE:  i didn't fix _regmap_write() as i.MX controls regmap write well in driver
> > with power enabled first, so don't have issues in reality.
>
> I can't tell what you think the problem is with _regmap_write()?
>

Because from what I see,  _regmap_write() seems still can write to HW
register even with cache_only mode set theoretically.

> > It can be fixed in a separate patch later if needed.
> > You may check if it's as your expected solution.
>
> > For syscon, I still have no idea how to fix it if I can't disable it.
> >
> > diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
> > index 2eaffd3224c9..da1702fd57cc 100644
> > --- a/drivers/base/regmap/regcache.c
> > +++ b/drivers/base/regmap/regcache.c
> > @@ -495,7 +495,7 @@ EXPORT_SYMBOL_GPL(regcache_drop_region);
> >  void regcache_cache_only(struct regmap *map, bool enable)
> >  {
> >         map->lock(map->lock_arg);
> > -       WARN_ON(map->cache_bypass && enable);
> > +//     WARN_ON(map->cache_bypass && enable);
> >         map->cache_only = enable;
> >         trace_regmap_cache_only(map, enable);
> >         map->unlock(map->lock_arg);
>
> What is the purpose of this change?  Why would the combination of cache
> only and bypass modes work be a good idea, and how should things behave
> in that case?
>

Because without this change, there will be a kernel dump caused by
WARN_ON when drivers call regcache_cache_only(map, true) after power
is off. There's no cache used in the imx blkctl driver. So map->cache_bypass
is default to true.

I did this by following your previous suggestion as follows:
"You don't actually need to have a cache to use cache only mode, if there
are no cached registers then you'll just get -EBUSY on any access to the
registers but that's hopefully fine since at the minute things will just
fall over anyway.  You shouldn't even need to flag registers as volatile
if there's no cache since it's not really relevant without a cache."

Am I misunderstanding your suggestions?

Regards
Aisheng