RE: [PATCH/RFC v4 2/4] regulator: fixed: add regulator_ops members for suspend/resume

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx/

[2]
https://lore.kernel.org/linux-renesas-soc/CAPDyKFpiBU1D+a7zb+Ggm0_HZ+YR4=LXJZ5MPytXtT=uBEdjPA@xxxxxxxxxxxxxx/

Best regards,
Yoshihiro Shimoda





[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux