All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leonard Crestez <leonard.crestez@nxp.com>
To: Lucas Stach <l.stach@pengutronix.de>
Cc: Mark Brown <broonie@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <kernel@pengutronix.de>,
	Mark Rutland <mark.rutland@arm.com>, <devicetree@vger.kernel.org>,
	Anson Huang <Anson.Huang@nxp.com>,
	Irina Tirdea <irina.tirdea@nxp.com>, <linux-pm@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, Rob Herring <robh+dt@kernel.org>,
	"Octavian Purdila" <octavian.purdila@nxp.com>,
	Fabio Estevam <fabio.estevam@nxp.com>,
	Robin Gong <yibin.gong@nxp.com>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC 7/8] cpufreq: imx6q: Initialize LDO bypass
Date: Wed, 22 Mar 2017 19:48:01 +0200	[thread overview]
Message-ID: <1490204881.17057.16.camel@nxp.com> (raw)
In-Reply-To: <1490202585.29056.5.camel@pengutronix.de>

On Wed, 2017-03-22 at 18:09 +0100, Lucas Stach wrote:
> Am Mittwoch, den 22.03.2017, 18:53 +0200 schrieb Leonard Crestez:
> > 
> > Several imx6* socs have three built-in regulators LDO_ARM LDO_SOC and
> > LDO_PU used to control internal chip voltages. "ldo-bypass" mode refers
> > to placing these regulators in bypass mode and controlling voltages from
> > an external power management chip instead. This is intended to save
> > power at the expense of an extra PMIC on the board.
> > 
> > The voltages for these regulators are controlled from the imxq6 cpufreq
> > driver so it makes sense to also control LDO bypass from here. The gpc
> > driver also fetches a reference to LDO_PU and uses it to gate power to
> > graphics blocks.
> > 
> > The LDO regulator has a minimum dropout voltage of 125mv so enabling
> > bypass mode will raise the effective voltage by that amount. We need set
> > the minimum frequency first to avoid overvolting when enabling LDO
> > bypass.
> > 
> > The binding is an u32 fsl,ldo-bypass in the gpc node because that's how
> > it was defined in the freescale vendor tree for a long time and
> > compatibility is desirable. Otherwise it would be a bool.
> > 
> > Some versions of u-boot shipped by freescale check this same property
> > and set regulators in bypass mode before linux actually starts so we
> > check for that scenario as well and finish early.
> I've not looked at the patch at all, but this feels like the wrong
> location to implement this. Using bypass mode or not should really be a
> internal decision of the regulator driver, influenced by a DT property
> to allow bypass mode.
> 
> The regulator driver can also implement the correct sequencing of first
> lowering external voltage to min + dropout, then going into bypass mode,
> then lower the external voltage by the amount of the dropout. I don't
> see why we need a frequency switch for this at all.

Because minimum voltages are dictated by core frequency. At high frequency
the (minimum voltage for frequency + dropout) is too high and would go beyond
the maximum of 1300 mv when bypass is enabled. It doesn't actually instantly
break, this is based on the "operating ranges" from this document:

http://www.nxp.com/assets/documents/data/en/data-sheets/IMX6DQCEC.pdf

> Implementing this in the consumers seems like the wrong spot.

It doesn't belong in drivers for individual regulators either, it's a piece
of board-level global configuration.

--
Regards,
Leonard

WARNING: multiple messages have this Message-ID (diff)
From: Leonard Crestez <leonard.crestez-3arQi8VN3Tc@public.gmane.org>
To: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Liam Girdwood <lgirdwood-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Viresh Kumar
	<viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	"Rafael J. Wysocki" <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org>,
	Shawn Guo <shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Anson Huang <Anson.Huang-3arQi8VN3Tc@public.gmane.org>,
	Irina Tirdea <irina.tirdea-3arQi8VN3Tc@public.gmane.org>,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Octavian Purdila <octavian.purdila-3arQi8VN3Tc@public.gmane.org>,
	Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>,
	Robin Gong <yibin.gong-3arQi8VN3Tc@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [RFC 7/8] cpufreq: imx6q: Initialize LDO bypass
Date: Wed, 22 Mar 2017 19:48:01 +0200	[thread overview]
Message-ID: <1490204881.17057.16.camel@nxp.com> (raw)
In-Reply-To: <1490202585.29056.5.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

On Wed, 2017-03-22 at 18:09 +0100, Lucas Stach wrote:
> Am Mittwoch, den 22.03.2017, 18:53 +0200 schrieb Leonard Crestez:
> > 
> > Several imx6* socs have three built-in regulators LDO_ARM LDO_SOC and
> > LDO_PU used to control internal chip voltages. "ldo-bypass" mode refers
> > to placing these regulators in bypass mode and controlling voltages from
> > an external power management chip instead. This is intended to save
> > power at the expense of an extra PMIC on the board.
> > 
> > The voltages for these regulators are controlled from the imxq6 cpufreq
> > driver so it makes sense to also control LDO bypass from here. The gpc
> > driver also fetches a reference to LDO_PU and uses it to gate power to
> > graphics blocks.
> > 
> > The LDO regulator has a minimum dropout voltage of 125mv so enabling
> > bypass mode will raise the effective voltage by that amount. We need set
> > the minimum frequency first to avoid overvolting when enabling LDO
> > bypass.
> > 
> > The binding is an u32 fsl,ldo-bypass in the gpc node because that's how
> > it was defined in the freescale vendor tree for a long time and
> > compatibility is desirable. Otherwise it would be a bool.
> > 
> > Some versions of u-boot shipped by freescale check this same property
> > and set regulators in bypass mode before linux actually starts so we
> > check for that scenario as well and finish early.
> I've not looked at the patch at all, but this feels like the wrong
> location to implement this. Using bypass mode or not should really be a
> internal decision of the regulator driver, influenced by a DT property
> to allow bypass mode.
> 
> The regulator driver can also implement the correct sequencing of first
> lowering external voltage to min + dropout, then going into bypass mode,
> then lower the external voltage by the amount of the dropout. I don't
> see why we need a frequency switch for this at all.

Because minimum voltages are dictated by core frequency. At high frequency
the (minimum voltage for frequency + dropout) is too high and would go beyond
the maximum of 1300 mv when bypass is enabled. It doesn't actually instantly
break, this is based on the "operating ranges" from this document:

http://www.nxp.com/assets/documents/data/en/data-sheets/IMX6DQCEC.pdf

> Implementing this in the consumers seems like the wrong spot.

It doesn't belong in drivers for individual regulators either, it's a piece
of board-level global configuration.

--
Regards,
Leonard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: leonard.crestez@nxp.com (Leonard Crestez)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 7/8] cpufreq: imx6q: Initialize LDO bypass
Date: Wed, 22 Mar 2017 19:48:01 +0200	[thread overview]
Message-ID: <1490204881.17057.16.camel@nxp.com> (raw)
In-Reply-To: <1490202585.29056.5.camel@pengutronix.de>

On Wed, 2017-03-22 at 18:09 +0100, Lucas Stach wrote:
> Am Mittwoch, den 22.03.2017, 18:53 +0200 schrieb Leonard Crestez:
> > 
> > Several imx6* socs have three built-in regulators LDO_ARM LDO_SOC and
> > LDO_PU used to control internal chip voltages. "ldo-bypass" mode refers
> > to placing these regulators in bypass mode and controlling voltages from
> > an external power management chip instead. This is intended to save
> > power at the expense of an extra PMIC on the board.
> > 
> > The voltages for these regulators are controlled from the imxq6 cpufreq
> > driver so it makes sense to also control LDO bypass from here. The gpc
> > driver also fetches a reference to LDO_PU and uses it to gate power to
> > graphics blocks.
> > 
> > The LDO regulator has a minimum dropout voltage of 125mv so enabling
> > bypass mode will raise the effective voltage by that amount. We need set
> > the minimum frequency first to avoid overvolting when enabling LDO
> > bypass.
> > 
> > The binding is an u32 fsl,ldo-bypass in the gpc node because that's how
> > it was defined in the freescale vendor tree for a long time and
> > compatibility is desirable. Otherwise it would be a bool.
> > 
> > Some versions of u-boot shipped by freescale check this same property
> > and set regulators in bypass mode before linux actually starts so we
> > check for that scenario as well and finish early.
> I've not looked at the patch at all, but this feels like the wrong
> location to implement this. Using bypass mode or not should really be a
> internal decision of the regulator driver, influenced by a DT property
> to allow bypass mode.
> 
> The regulator driver can also implement the correct sequencing of first
> lowering external voltage to min + dropout, then going into bypass mode,
> then lower the external voltage by the amount of the dropout. I don't
> see why we need a frequency switch for this at all.

Because minimum voltages are dictated by core frequency. At high frequency
the (minimum voltage for frequency + dropout) is too high and would go beyond
the maximum of 1300 mv when bypass is enabled. It doesn't actually instantly
break, this is based on the "operating ranges" from this document:

http://www.nxp.com/assets/documents/data/en/data-sheets/IMX6DQCEC.pdf

> Implementing this in the consumers seems like the wrong spot.

It doesn't belong in drivers for individual regulators either, it's a piece
of board-level global configuration.

--
Regards,
Leonard

  reply	other threads:[~2017-03-22 17:48 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-22 16:53 [RFC 0/8] ARM: imx: Upstream fsl,ldo-bypass Leonard Crestez
2017-03-22 16:53 ` Leonard Crestez
2017-03-22 16:53 ` Leonard Crestez
2017-03-22 16:53 ` [RFC 1/8] ARM: imx: gpc: Do not print error message for EPROBE_DEFER Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-22 16:53 ` [RFC 2/8] cpufreq: imx6q: Fix handling EPROBE_DEFER from regulator Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-23  4:33   ` Viresh Kumar
2017-03-23  4:33     ` Viresh Kumar
2017-03-23  4:33     ` Viresh Kumar
2017-03-22 16:53 ` [RFC 3/8] cpufreq: imx6q: Set max suspend_freq to avoid changes during suspend Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-23  4:34   ` Viresh Kumar
2017-03-23  4:34     ` Viresh Kumar
2017-03-28 20:03     ` Leonard Crestez
2017-03-28 20:03       ` Leonard Crestez
2017-03-28 20:03       ` Leonard Crestez
2017-03-28 20:50       ` Rafael J. Wysocki
2017-03-28 20:50         ` Rafael J. Wysocki
2017-03-28 20:50         ` Rafael J. Wysocki
2017-03-28 22:23         ` Rafael J. Wysocki
2017-03-28 22:23           ` Rafael J. Wysocki
2017-03-22 16:53 ` [RFC 4/8] regulator: core: Check enabling bypass respects constraints Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-24 12:52   ` Mark Brown
2017-03-24 12:52     ` Mark Brown
2017-03-24 12:52     ` Mark Brown
2017-03-28 12:39     ` Leonard Crestez
2017-03-28 12:39       ` Leonard Crestez
2017-03-28 12:39       ` Leonard Crestez
2017-03-28 16:47       ` Mark Brown
2017-03-28 16:47         ` Mark Brown
2017-03-28 16:47         ` Mark Brown
2017-03-28 19:49         ` Leonard Crestez
2017-03-28 19:49           ` Leonard Crestez
2017-03-28 19:49           ` Leonard Crestez
2017-04-06 18:52           ` Mark Brown
2017-04-06 18:52             ` Mark Brown
2017-04-06 18:52             ` Mark Brown
2017-04-07 10:51             ` Leonard Crestez
2017-04-07 10:51               ` Leonard Crestez
2017-04-07 10:51               ` Leonard Crestez
2017-04-07 11:22               ` Mark Brown
2017-04-07 11:22                 ` Mark Brown
2017-04-13 20:46                 ` Leonard Crestez
2017-04-13 20:46                   ` Leonard Crestez
2017-04-13 20:46                   ` Leonard Crestez
2017-03-22 16:53 ` [RFC 5/8] regulator: anatop: fix min dropout for bypass mode Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-24 12:54   ` Mark Brown
2017-03-24 12:54     ` Mark Brown
2017-03-24 12:54     ` Mark Brown
2017-03-28 11:52     ` Leonard Crestez
2017-03-28 11:52       ` Leonard Crestez
2017-03-28 11:52       ` Leonard Crestez
2017-03-22 16:53 ` [RFC 6/8] regulator: core: Add regulator_is_bypass function Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-24 12:55   ` Mark Brown
2017-03-24 12:55     ` Mark Brown
2017-03-24 12:55     ` Mark Brown
2017-03-28 14:53     ` Leonard Crestez
2017-03-28 14:53       ` Leonard Crestez
2017-03-28 14:53       ` Leonard Crestez
2017-03-22 16:53 ` [RFC 7/8] cpufreq: imx6q: Initialize LDO bypass Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-22 17:09   ` Lucas Stach
2017-03-22 17:09     ` Lucas Stach
2017-03-22 17:09     ` Lucas Stach
2017-03-22 17:48     ` Leonard Crestez [this message]
2017-03-22 17:48       ` Leonard Crestez
2017-03-22 17:48       ` Leonard Crestez
2017-03-22 18:00       ` Lucas Stach
2017-03-22 18:00         ` Lucas Stach
2017-03-22 16:53 ` [RFC 8/8] ARM: dts: imx6qdl-sabresd: Enable fsl,ldo-bypass Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-22 16:53   ` Leonard Crestez
2017-03-22 17:13   ` Lucas Stach
2017-03-22 17:13     ` Lucas Stach
2017-03-29 13:32     ` Leonard Crestez
2017-03-29 13:32       ` Leonard Crestez
2017-03-29 13:32       ` Leonard Crestez

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=1490204881.17057.16.camel@nxp.com \
    --to=leonard.crestez@nxp.com \
    --cc=Anson.Huang@nxp.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=fabio.estevam@nxp.com \
    --cc=irina.tirdea@nxp.com \
    --cc=kernel@pengutronix.de \
    --cc=l.stach@pengutronix.de \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=octavian.purdila@nxp.com \
    --cc=rjw@rjwysocki.net \
    --cc=robh+dt@kernel.org \
    --cc=shawnguo@kernel.org \
    --cc=viresh.kumar@linaro.org \
    --cc=yibin.gong@nxp.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.