From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
To: Mark Brown <broonie@kernel.org>
Cc: "ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
"lgirdwood@gmail.com" <lgirdwood@gmail.com>,
"geert+renesas@glider.be" <geert+renesas@glider.be>,
"magnus.damm@gmail.com" <magnus.damm@gmail.com>,
"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-renesas-soc@vger.kernel.org"
<linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume
Date: Mon, 29 Jun 2020 02:42:26 +0000 [thread overview]
Message-ID: <TY2PR01MB3692A3B12CEF7F9708A8A59CD86E0@TY2PR01MB3692.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <20200626143914.GE5289@sirena.org.uk>
Hi Mark,
> From: Mark Brown, Sent: Friday, June 26, 2020 11:39 PM
>
> On Fri, Jun 26, 2020 at 06:32:20PM +0900, Yoshihiro Shimoda wrote:
>
> > +static int reg_is_enabled(struct regulator_dev *rdev)
> > +{
> > + struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
> > +
> > + return !priv->disabled_in_suspend;
> > +}
>
> This is broken, the state of the regualtor during system runtime need
> have no connection with the state of the regulator during system
> suspend.
>
> > +static int reg_prepare_disable(struct regulator_dev *rdev)
> > +{
> > + struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
> > +
> > + priv->disabled_in_suspend = true;
> > +
> > + return 0;
> > +}
>
> According to the changelog this is all about reflecting changes in the
> system state done by firmware but there's no interaction with firmware
> here which means this will be at best fragile. If we need to reflect
> changes in firmware configuration I'd expect there to be some
> interaction with firmware about how it is configured, or at least that
> the configuration would come from the same source.
I should have described background of previous patch series though,
according to previous discussion [1] the firmware side (like PSCI) is
also fragile unfortunately... So, I thought using regulator-off-in-suspend
in a regulator was better.
On other hand, Ulf is talking about either adding a property (perhaps like
regulator-off-in-suspend) into a regulator or just adding a new property
into MMC [2]. What do you think about Ulf' comment? I'm thinking
adding a new property "full-pwr-cycle-in-suspend" is the best solution.
This is because using a regulator property and reflecting a state of regulator without
firmware is fragile, as you said.
[1]
https://lore.kernel.org/linux-renesas-soc/CAMuHMdXjU7N4oG89YsozGijMpjgKGN6ezw2qm6FeGX=JyRhsvg@mail.gmail.com/
[2]
https://lore.kernel.org/linux-renesas-soc/CAPDyKFpiBU1D+a7zb+Ggm0_HZ+YR4=LXJZ5MPytXtT=uBEdjPA@mail.gmail.com/
Best regards,
Yoshihiro Shimoda
next prev parent reply other threads:[~2020-06-29 2:42 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-26 9:32 [PATCH/RFC v4 0/4] treewide: add regulator condition on _mmc_suspend() Yoshihiro Shimoda
2020-06-26 9:32 ` [PATCH/RFC v4 1/4] regulator: core: add prepare and resume_early Yoshihiro Shimoda
2020-06-26 14:30 ` Mark Brown
2020-06-29 2:12 ` Yoshihiro Shimoda
2020-06-26 9:32 ` [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume Yoshihiro Shimoda
2020-06-26 14:39 ` Mark Brown
2020-06-29 2:42 ` Yoshihiro Shimoda [this message]
2020-06-29 12:57 ` Mark Brown
2020-06-29 13:40 ` Sudeep Holla
2020-06-29 14:15 ` Geert Uytterhoeven
2020-06-29 15:07 ` Sudeep Holla
2020-06-29 16:14 ` Mark Brown
2020-06-29 16:42 ` Sudeep Holla
2020-06-29 17:26 ` Mark Brown
2020-06-29 17:42 ` Sudeep Holla
2020-06-30 8:29 ` Yoshihiro Shimoda
2020-06-26 9:32 ` [PATCH/RFC v4 3/4] mmc: core: Call mmc_poweroff_nofity() if regulators are disabled Yoshihiro Shimoda
2020-06-26 15:13 ` Mark Brown
2020-06-29 2:49 ` Yoshihiro Shimoda
2020-06-26 15:53 ` Sergei Shtylyov
2020-06-29 5:16 ` Yoshihiro Shimoda
2020-06-26 9:32 ` [PATCH/RFC v4 4/4] arm64: dts: renesas: add regulator-off-in-suspend property for eMMC Yoshihiro Shimoda
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=TY2PR01MB3692A3B12CEF7F9708A8A59CD86E0@TY2PR01MB3692.jpnprd01.prod.outlook.com \
--to=yoshihiro.shimoda.uh@renesas.com \
--cc=broonie@kernel.org \
--cc=geert+renesas@glider.be \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=magnus.damm@gmail.com \
--cc=ulf.hansson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).