linux-mmc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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


  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).