Linux-Samsung-soc Archive on
 help / color / Atom feed
From: Marek Szyprowski <>
To: Dmitry Osipenko <>,,,
	Mark Brown <>
Cc: Liam Girdwood <>,
	Lucas Stach <>,
	Bartlomiej Zolnierkiewicz <>,
	Krzysztof Kozlowski <>,
	Viresh Kumar <>,, Nishanth Menon <>,
	Stephen Boyd <>,
	Vincent Guittot <>,
	Rafael Wysocki <>,,
	Chanwoo Choi <>
Subject: Re: [PATCH v2] soc: samsung: Add simple voltage coupler for Exynos5800
Date: Thu, 4 Jun 2020 15:28:24 +0200
Message-ID: <> (raw)
In-Reply-To: <>

Hi Dmitry,

On 02.06.2020 17:15, Dmitry Osipenko wrote:
> 02.06.2020 16:02, Marek Szyprowski пишет:
>> Add a simple custom voltage regulator coupler for Exynos5800 SoCs, which
>> require coupling between "vdd_arm" and "vdd_int" regulators. This coupler
>> ensures that the voltage balancing for the coupled regulators is done
>> only when clients for the each regulator apply their constraints, so the
>> voltage values don't go beyond the bootloader-selected operation point
>> during the boot process. This also ensures proper voltage balancing if
>> any of the client driver is missing.
>> Signed-off-by: Marek Szyprowski <>
>> ---
>>   (...)
> Hello Marek,
> Does this mean that you're going to allow to violate the coupling
> constraints while coupled regulator has no consumers?
> I don't think that you may want to skip the coupled balancing ever.
> Instead you may want to assume that the min-voltage constraint equals to
> the current regulator's voltage while the coupled regulator has no
> consumers.
> Yours variant of the balancer doesn't prevent the voltage dropping on
> regulator's enabling while coupled regulator doesn't have active
> consumers. This is the problem which we previously had once OPP code was
> changed to enable regulator.
> Secondly, yours variant of the balancer also doesn't handle the case
> where set_voltage() is invoked while one of the couples doesn't have
> active consumers because voltage of this couple may drop more than
> allowed on the voltage re-balancing.
Indeed. I've focused on disabling balancing when there are no consumers 
and I didn't notice that the max_spread might be violated in such case.
> I'd suggest to simply change the regulator_get_optimal_voltage() to
> limit the desired_min_uV to the current voltage if coupled regulator has
> no consumers.

Right, this sounds like a best solution. I have an idea to try to add it 
again to the core as a simple check: if regultor is boot_on, use current 
voltage as min_uV until a consumer is registered. I've checked and this 
approach fixes the issue. I will submit a patch in a few minutes.

> I don't think that any of the today's upstream kernel coupled-regulator
> users really need to support the case where a regulator couple is
> allowed *not* to have active consumers, so for now it should be fine to
> change the core code to accommodate the needs of the Exynos regulators
> (IMO). We may get back to this later on once there will be a real need
> support that case.
> Please also note that I'm assuming that each of the coupled regulators
> doesn't have more than one consumer at a time in yours case (correct?),
> because yours solution won't work well in a case of multiple consumers.
> There is no universal solution for this bootstrapping problem yet.

There are only a single consumers for each coupled regulator (cpufreq 
for vdd_arm and devfreq for vdd_int).

Best regards
Marek Szyprowski, PhD
Samsung R&D Institute Poland

  reply index

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <>
2020-05-29 12:49 ` [PATCH 0/2] Fix regulators coupling " Marek Szyprowski
     [not found]   ` <>
2020-05-29 12:49     ` [PATCH 1/2] regulator: extract voltage balancing code to the separate function Marek Szyprowski
     [not found]   ` <>
2020-05-29 12:49     ` [PATCH 2/2] soc: samsung: Add simple voltage coupler for Exynos5800 Marek Szyprowski
2020-05-29 17:43       ` Krzysztof Kozlowski
2020-06-01  6:28         ` Marek Szyprowski
     [not found]           ` <>
2020-06-02 13:02             ` [PATCH v2] " Marek Szyprowski
2020-06-02 15:15               ` Dmitry Osipenko
2020-06-04 13:28                 ` Marek Szyprowski [this message]
2020-05-29 16:52   ` [PATCH 0/2] Fix regulators coupling " Mark Brown
2020-05-29 16:58     ` Mark Brown
2020-05-29 17:33       ` Krzysztof Kozlowski

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Samsung-soc Archive on

Archives are clonable:
	git clone --mirror linux-samsung-soc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-samsung-soc linux-samsung-soc/ \
	public-inbox-index linux-samsung-soc

Example config snippet for mirrors

Newsgroup available over NNTP:

AGPL code for this site: git clone