* [PATCH v2] power: supply: ab8500: remove set but not used variables 'vbup33_vrtcn' and 'bup_vch_range'
From: YueHaibing @ 2019-07-17 14:18 UTC (permalink / raw)
To: sre, linus.walleij, lee.jones, loic.pallardy
Cc: linux-kernel, linux-pm, YueHaibing
Fixes gcc '-Wunused-but-set-variable' warnings:
drivers/power/supply/ab8500_charger.c:
In function ab8500_charger_init_hw_registers:
drivers/power/supply/ab8500_charger.c:3013:24: warning:
variable vbup33_vrtcn set but not used [-Wunused-but-set-variable]
drivers/power/supply/ab8500_charger.c:3013:5: warning:
variable bup_vch_range set but not used [-Wunused-but-set-variable]
They are not used since commit 4c4268dc97c4 ("power:
supply: ab8500: Drop AB8540/9540 support")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
v2: Fix patch title
---
drivers/power/supply/ab8500_charger.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/power/supply/ab8500_charger.c b/drivers/power/supply/ab8500_charger.c
index 30de448..270a48a 100644
--- a/drivers/power/supply/ab8500_charger.c
+++ b/drivers/power/supply/ab8500_charger.c
@@ -3010,7 +3010,6 @@ static int ab8500_charger_usb_get_property(struct power_supply *psy,
static int ab8500_charger_init_hw_registers(struct ab8500_charger *di)
{
int ret = 0;
- u8 bup_vch_range = 0, vbup33_vrtcn = 0;
/* Setup maximum charger current and voltage for ABB cut2.0 */
if (!is_ab8500_1p1_or_earlier(di->parent)) {
@@ -3111,12 +3110,6 @@ static int ab8500_charger_init_hw_registers(struct ab8500_charger *di)
goto out;
}
- /* Backup battery voltage and current */
- if (di->bm->bkup_bat_v > BUP_VCH_SEL_3P1V)
- bup_vch_range = BUP_VCH_RANGE;
- if (di->bm->bkup_bat_v == BUP_VCH_SEL_3P3V)
- vbup33_vrtcn = VBUP33_VRTCN;
-
ret = abx500_set_register_interruptible(di->dev,
AB8500_RTC,
AB8500_RTC_BACKUP_CHG_REG,
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 1/3] power: supply: ab8500: remove set but not used variables 'vbup33_vrtcn' and 'bup_vch_range'
From: Yuehaibing @ 2019-07-17 14:17 UTC (permalink / raw)
To: sre, linus.walleij, lee.jones, loic.pallardy; +Cc: linux-kernel, linux-pm
In-Reply-To: <20190717141502.53968-1-yuehaibing@huawei.com>
Sorry, Pls ignore this, I will fix patch title.
On 2019/7/17 22:15, YueHaibing wrote:
> Fixes gcc '-Wunused-but-set-variable' warnings:
>
> drivers/power/supply/ab8500_charger.c:
> In function ab8500_charger_init_hw_registers:
> drivers/power/supply/ab8500_charger.c:3013:24: warning:
> variable vbup33_vrtcn set but not used [-Wunused-but-set-variable]
> drivers/power/supply/ab8500_charger.c:3013:5: warning:
> variable bup_vch_range set but not used [-Wunused-but-set-variable]
>
> They are not used since commit 4c4268dc97c4 ("power:
> supply: ab8500: Drop AB8540/9540 support")
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> drivers/power/supply/ab8500_charger.c | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/drivers/power/supply/ab8500_charger.c b/drivers/power/supply/ab8500_charger.c
> index 30de448..270a48a 100644
> --- a/drivers/power/supply/ab8500_charger.c
> +++ b/drivers/power/supply/ab8500_charger.c
> @@ -3010,7 +3010,6 @@ static int ab8500_charger_usb_get_property(struct power_supply *psy,
> static int ab8500_charger_init_hw_registers(struct ab8500_charger *di)
> {
> int ret = 0;
> - u8 bup_vch_range = 0, vbup33_vrtcn = 0;
>
> /* Setup maximum charger current and voltage for ABB cut2.0 */
> if (!is_ab8500_1p1_or_earlier(di->parent)) {
> @@ -3111,12 +3110,6 @@ static int ab8500_charger_init_hw_registers(struct ab8500_charger *di)
> goto out;
> }
>
> - /* Backup battery voltage and current */
> - if (di->bm->bkup_bat_v > BUP_VCH_SEL_3P1V)
> - bup_vch_range = BUP_VCH_RANGE;
> - if (di->bm->bkup_bat_v == BUP_VCH_SEL_3P3V)
> - vbup33_vrtcn = VBUP33_VRTCN;
> -
> ret = abx500_set_register_interruptible(di->dev,
> AB8500_RTC,
> AB8500_RTC_BACKUP_CHG_REG,
>
^ permalink raw reply
* [PATCH 1/3] power: supply: ab8500: remove set but not used variables 'vbup33_vrtcn' and 'bup_vch_range'
From: YueHaibing @ 2019-07-17 14:15 UTC (permalink / raw)
To: sre, linus.walleij, lee.jones, loic.pallardy
Cc: linux-kernel, linux-pm, YueHaibing
Fixes gcc '-Wunused-but-set-variable' warnings:
drivers/power/supply/ab8500_charger.c:
In function ab8500_charger_init_hw_registers:
drivers/power/supply/ab8500_charger.c:3013:24: warning:
variable vbup33_vrtcn set but not used [-Wunused-but-set-variable]
drivers/power/supply/ab8500_charger.c:3013:5: warning:
variable bup_vch_range set but not used [-Wunused-but-set-variable]
They are not used since commit 4c4268dc97c4 ("power:
supply: ab8500: Drop AB8540/9540 support")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/power/supply/ab8500_charger.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/power/supply/ab8500_charger.c b/drivers/power/supply/ab8500_charger.c
index 30de448..270a48a 100644
--- a/drivers/power/supply/ab8500_charger.c
+++ b/drivers/power/supply/ab8500_charger.c
@@ -3010,7 +3010,6 @@ static int ab8500_charger_usb_get_property(struct power_supply *psy,
static int ab8500_charger_init_hw_registers(struct ab8500_charger *di)
{
int ret = 0;
- u8 bup_vch_range = 0, vbup33_vrtcn = 0;
/* Setup maximum charger current and voltage for ABB cut2.0 */
if (!is_ab8500_1p1_or_earlier(di->parent)) {
@@ -3111,12 +3110,6 @@ static int ab8500_charger_init_hw_registers(struct ab8500_charger *di)
goto out;
}
- /* Backup battery voltage and current */
- if (di->bm->bkup_bat_v > BUP_VCH_SEL_3P1V)
- bup_vch_range = BUP_VCH_RANGE;
- if (di->bm->bkup_bat_v == BUP_VCH_SEL_3P3V)
- vbup33_vrtcn = VBUP33_VRTCN;
-
ret = abx500_set_register_interruptible(di->dev,
AB8500_RTC,
AB8500_RTC_BACKUP_CHG_REG,
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2 1/4] opp: core: add regulators enable and disable
From: Kamil Konieczny @ 2019-07-17 14:14 UTC (permalink / raw)
To: Viresh Kumar
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
linux-pm, linux-samsung-soc
In-Reply-To: <20190716100539.4uqelbxqz7bmtmea@vireshk-i7>
On 16.07.2019 12:05, Viresh Kumar wrote:
> On 15-07-19, 14:04, Kamil Konieczny wrote:
>> Add enable regulators to dev_pm_opp_set_regulators() and disable
>> regulators to dev_pm_opp_put_regulators(). This prepares for
>> converting exynos-bus devfreq driver to use dev_pm_opp_set_rate().
>>
>> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
>> --
>> Changes in v2:
>>
>> - move regulator enable and disable into loop
>>
>> ---
>> drivers/opp/core.c | 18 +++++++++++++++---
>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index 0e7703fe733f..069c5cf8827e 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -1570,6 +1570,10 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
>> goto free_regulators;
>> }
>>
>> + ret = regulator_enable(reg);
>> + if (ret < 0)
>> + goto disable;
>
> The name of this label is logically incorrect because we won't disable
> the regulator from there but put it. Over that, I would rather prefer
> to remove the label and add regulator_put() here itself.
I will change this and following according to your suggestions and will send v3.
>> +
>> opp_table->regulators[i] = reg;
>> }
>>
>> @@ -1582,9 +1586,15 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
>>
>> return opp_table;
>>
>> +disable:
>> + regulator_put(reg);
>> + --i;
>> +
>> free_regulators:
>> - while (i != 0)
>> - regulator_put(opp_table->regulators[--i]);
>> + for (; i >= 0; --i) {
>> + regulator_disable(opp_table->regulators[i]);
>> + regulator_put(opp_table->regulators[i]);
>
> This is incorrect as this will now try to put/disable the regulator
> which we failed to acquire. As --i happens only after the loop has run
> once. You can rather do:
>
> while (i--) {
> regulator_disable(opp_table->regulators[i]);
> regulator_put(opp_table->regulators[i]);
> }
>
>
>> + }
>>
>> kfree(opp_table->regulators);
>> opp_table->regulators = NULL;
>> @@ -1610,8 +1620,10 @@ void dev_pm_opp_put_regulators(struct opp_table *opp_table)
>> /* Make sure there are no concurrent readers while updating opp_table */
>> WARN_ON(!list_empty(&opp_table->opp_list));
>>
>> - for (i = opp_table->regulator_count - 1; i >= 0; i--)
>> + for (i = opp_table->regulator_count - 1; i >= 0; i--) {
>> + regulator_disable(opp_table->regulators[i]);
>> regulator_put(opp_table->regulators[i]);
>> + }
>>
>> _free_set_opp_data(opp_table);
>>
>> --
>> 2.22.0
>
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [PATCH v2 1/4] opp: core: add regulators enable and disable
From: Kamil Konieczny @ 2019-07-17 14:12 UTC (permalink / raw)
To: Chanwoo Choi
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Krzysztof Kozlowski,
Kukjin Kim, Kyungmin Park, Mark Rutland, MyungJoo Ham,
Nishanth Menon, Rob Herring, Stephen Boyd, Viresh Kumar,
devicetree, linux-arm-kernel, linux-kernel, linux-pm,
linux-samsung-soc
In-Reply-To: <9acc7dd0-614b-ccd3-a485-eeca3dab494b@samsung.com>
On 16.07.2019 06:03, Chanwoo Choi wrote:
> Hi Kamil,
>
> On 19. 7. 15. 오후 9:04, Kamil Konieczny wrote:
>> Add enable regulators to dev_pm_opp_set_regulators() and disable
>> regulators to dev_pm_opp_put_regulators(). This prepares for
>> converting exynos-bus devfreq driver to use dev_pm_opp_set_rate().
>
> IMHO, it is not proper to mention the specific driver name.
> If you explain the reason why enable the regulator before using it,
> it is enough description.
>
>>
>> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
>> --
>> Changes in v2:
>>
>> - move regulator enable and disable into loop
>>
>> ---
>> drivers/opp/core.c | 18 +++++++++++++++---
>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index 0e7703fe733f..069c5cf8827e 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -1570,6 +1570,10 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
>> goto free_regulators;
>> }
>>
>> + ret = regulator_enable(reg);
>> + if (ret < 0)
>> + goto disable;
>> +
>> opp_table->regulators[i] = reg;
>> }
>>
>> @@ -1582,9 +1586,15 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
>>
>> return opp_table;
>>
>> +disable:
>> + regulator_put(reg);
>> + --i;
>> +
>> free_regulators:
>> - while (i != 0)
>> - regulator_put(opp_table->regulators[--i]);
>> + for (; i >= 0; --i) {
>> + regulator_disable(opp_table->regulators[i]);
>> + regulator_put(opp_table->regulators[i]);
>> + }
>>
>> kfree(opp_table->regulators);
>> opp_table->regulators = NULL;
>> @@ -1610,8 +1620,10 @@ void dev_pm_opp_put_regulators(struct opp_table *opp_table)
>> /* Make sure there are no concurrent readers while updating opp_table */
>> WARN_ON(!list_empty(&opp_table->opp_list));
>>
>> - for (i = opp_table->regulator_count - 1; i >= 0; i--)
>> + for (i = opp_table->regulator_count - 1; i >= 0; i--) {
>> + regulator_disable(opp_table->regulators[i]);
>> regulator_put(opp_table->regulators[i]);
>> + }
>>
>> _free_set_opp_data(opp_table);
>>
>>
>
> I agree to enable the regulator before using it.
> The bootloader might not enable the regulators
> and the kernel need to enable regulator in order to increase
> the reference count explicitly event if bootloader enables it.
>
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Thank you, I will change commit description and send v3.
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [PATCH] cpuidle: Always stop scheduler tick on adaptive-tick CPUs
From: Frederic Weisbecker @ 2019-07-17 13:21 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael J. Wysocki, Linux PM, Thomas Lindroth, LKML,
Peter Zijlstra
In-Reply-To: <CAJZ5v0gB0AHTebjpp87YKA1wmE+tCw5V=eaRE2XDM3nyQYndnA@mail.gmail.com>
On Wed, Jul 17, 2019 at 09:55:08AM +0200, Rafael J. Wysocki wrote:
> On Tue, Jul 16, 2019 at 11:40 PM Frederic Weisbecker
> <frederic@kernel.org> wrote:
> >
> > On Tue, Jul 16, 2019 at 05:25:10PM +0200, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > >
> > > Running the scheduler tick on idle adaptive-tick CPUs is not useful
> >
> > Judging by the below change, you mean full dynticks, right?
>
> Right.
>
> > > and it may also be not expected by users (as reported by Thomas), so
> > > add a check to cpuidle_idle_call() to always stop the tick on them
> > > regardless of the idle duration predicted by the governor.
> > >
> > > Fixes: 554c8aa8ecad ("sched: idle: Select idle state before stopping the tick")
> > > Reported-by: Thomas Lindroth <thomas.lindroth@gmail.com>
> > > Tested-by: Thomas Lindroth <thomas.lindroth@gmail.com>
> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > ---
> > > kernel/sched/idle.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > Index: linux-pm/kernel/sched/idle.c
> > > ===================================================================
> > > --- linux-pm.orig/kernel/sched/idle.c
> > > +++ linux-pm/kernel/sched/idle.c
> > > @@ -191,7 +191,8 @@ static void cpuidle_idle_call(void)
> > > */
> > > next_state = cpuidle_select(drv, dev, &stop_tick);
> > >
> > > - if (stop_tick || tick_nohz_tick_stopped())
> > > + if (stop_tick || tick_nohz_tick_stopped() ||
> > > + !housekeeping_cpu(dev->cpu, HK_FLAG_TICK))
> >
> > But tick_nohz_tick_stopped() also works on full dynticks CPUs. If the
> > tick isn't stopped on a full dynticks CPU by the time we reach this path,
> > it means that the conditions for the tick to be stopped are not met anyway
> > (eg: more than one task and sched tick is needed, perf event requires the tick,
> > posix CPU timer, etc...)
>
> First of all, according to Thomas, the patch does make a difference,
> so evidently on his system(s) the full dynticks CPUs enter the idle
> loop with running tick.
>
> This means that, indeed, the conditions for the tick to be stopped
> have not been met up to that point, but if the (full dynticks) CPU
> becomes idle, that's because it has been made idle on purpose
> (presumably by a user-space "orchestrator" or the sysadmin), so the
> kernel can assume that it will remain idle indefinitely. That, in
> turn, is when the tick would be stopped on it regardless of everything
> else (even if it wasn't a full dynticks CPU).
Well I think we disagree on that assumption that if a nohz_full CPU is put
idle, it will remain there indefinitely. Nohz_full CPUs aren't really special
in this regard, they can sleep on an IO, wait for a short event just like
any other CPU.
The only difference with other CPUs is that they _might_ enter the idle loop
with the tick already stopped. Ok that should be the case most of the time
with regular full dynticks usecases, but there can be initialization work
or stuff that make the CPU run with periodic tick for some time.
Thanks.
>
> I guess I should add the above to the changelog.
>
> > Or am I missing something else?
>
> Well, if full dynticks CPUs are expected to always enter the idle loop
> with stopped tick, then something appears to be amiss, but I'm not
> sure if that expectation is entirely realistic.
>
> Cheers!
^ permalink raw reply
* [LPC 2019] Power Management and Thermal Control MC: Call for topics
From: Rafael J. Wysocki @ 2019-07-17 10:48 UTC (permalink / raw)
To: Linux PM; +Cc: Linux Kernel Mailing List, Eduardo Valentin
Hi All,
Now that the PM+Thermal MC has been officially announced
https://linuxplumbersconf.org/event/4/page/34-accepted-microconferences#pm-tc
everyone interested in the problem space in question and having a
topic for discussion is kindly requested to submit a topic proposal
for that MC via the LPC web site:
https://linuxplumbersconf.org/event/4/abstracts/
To do that (after following the link above):
1. "Press" the "Submit new proposal" button.
2. Log in to the LPC CFP system (* see below in case you don't want to
create an account).
3. Put a one line title of your topic into the "Title" field.
4. Put your abstract into the "Content" field (please keep the
abstracts concise).
5. Add yourself as the author (there may be multiple authors, in which
case add all of them).
6. In the "Comments" field say how much time the topic is expected to take.
7. Choose "Power Management and Thermal Control MC topic" in the
"Track" selection.
8. Answer "Yes" to the anti-harassment policy compliance confirmation.
9. "Press" the "Submit" button.
* In case you don't have an account in the LPC web site and you don't
want to create one, you can send the topic title and abstract to me,
and I will put the proposal into the LPC CFP system for you. Still,
please do consider adding an LPC account and following the steps above
yourself.
Please note that the most relevant topics are those aimed at solving
specific problems in the Linux "plumbing" area: the kernel, libraries,
system management tools etc.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH] clk: mvebu: armada-37xx-periph: Fix initialization for cpu clocks
From: Viresh Kumar @ 2019-07-17 10:33 UTC (permalink / raw)
To: Gregory CLEMENT, Ilias Apalodimas
Cc: Christian Neubert, Stephen Boyd, Mike Turquette, linux-clk,
linux-kernel, Rafael J. Wysocki, linux-pm, Vincent Guittot,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Thomas Petazzoni, linux-arm-kernel, Antoine Tenart,
Miquèl Raynal, Maxime Chevallier, stable
In-Reply-To: <20190610101918.sypafywc6fn4jsbo@vireshk-i7>
On 10-06-19, 15:49, Viresh Kumar wrote:
> On 22-05-19, 12:36, Viresh Kumar wrote:
> > On 22-05-19, 10:03, Ilias Apalodimas wrote:
> > > Hi Viresh, Gregory
> > > On Mon, May 20, 2019 at 04:50:42PM +0530, Viresh Kumar wrote:
> > > > On 25-04-19, 15:33, Ilias Apalodimas wrote:
> > > > > Hi Viresh,
> > > > >
> > > > > > > > Also, during this week-end, Christian suggested that the issue might
> > > > > > > > come from the AVS support.
> > > > > > > >
> > > > > > > > Could you disable it and check you still have the issue?
> > > > > > > >
> > > > > > > > For this, you just have to remove the avs node in
> > > > > > > > arch/arm64/boot/dts/marvell/armada-37xx.dtsi and rebuild the dtb.
> > > > > > > Sure. You'll have to wait for a week though. Currently on a trip. I'll run that
> > > > > > > once i return
> > > > > >
> > > > > > @Ilias: Can you please try this now and confirm to Gregory ?
> > > > > I am more overloaded than usual and totally forgot about this. Apologies.
> > > > > I'll try finding some time and do this.
> > > >
> > > > Ping Ilias.
> > > Sorry for the huge delay.
> > > Applying this patch and removing tha 'avs' node from
> > > arch/arm64/boot/dts/marvell/armada-37xx.dtsi seems to work.
> > > Changing between governors does not freeze the board any more. I haven't checked
> > > the actual impact on the CPU speed but the values on
> > > /sys/devices/system/cpu/cpufreq/policy0/scaling_governor are correct
> >
> > Thanks for testing it out. Lets see what Gregory has to say now.
>
> @Gregory: Do you have any further advice for Ilias ?
Ping.
--
viresh
^ permalink raw reply
* Re: [PATCH v3 0/6] Introduce Bandwidth OPPs for interconnect paths
From: Viresh Kumar @ 2019-07-17 10:32 UTC (permalink / raw)
To: Saravana Kannan
Cc: Georgi Djakov, Rob Herring, Mark Rutland, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, vincent.guittot,
seansw, daidavid1, Rajendra Nayak, sibis, bjorn.andersson,
evgreen, kernel-team, linux-pm, devicetree, linux-kernel
In-Reply-To: <20190703011020.151615-1-saravanak@google.com>
On 02-07-19, 18:10, Saravana Kannan wrote:
> Interconnects and interconnect paths quantify their performance levels in
> terms of bandwidth and not in terms of frequency. So similar to how we have
> frequency based OPP tables in DT and in the OPP framework, we need
> bandwidth OPP table support in the OPP framework and in DT. Since there can
> be more than one interconnect path used by a device, we also need a way to
> assign a bandwidth OPP table to an interconnect path.
>
> This patch series:
> - Adds opp-peak-KBps and opp-avg-KBps properties to OPP DT bindings
> - Adds interconnect-opp-table property to interconnect DT bindings
> - Adds OPP helper functions for bandwidth OPP tables
> - Adds icc_get_opp_table() to get the OPP table for an interconnect path
>
> So with the DT bindings added in this patch series, the DT for a GPU
> that does bandwidth voting from GPU to Cache and GPU to DDR would look
> something like this:
>
> gpu_cache_opp_table: gpu_cache_opp_table {
> compatible = "operating-points-v2";
>
> gpu_cache_3000: opp-3000 {
> opp-peak-KBps = <3000>;
> opp-avg-KBps = <1000>;
> };
> gpu_cache_6000: opp-6000 {
> opp-peak-KBps = <6000>;
> opp-avg-KBps = <2000>;
> };
> gpu_cache_9000: opp-9000 {
> opp-peak-KBps = <9000>;
> opp-avg-KBps = <9000>;
> };
> };
>
> gpu_ddr_opp_table: gpu_ddr_opp_table {
> compatible = "operating-points-v2";
>
> gpu_ddr_1525: opp-1525 {
> opp-peak-KBps = <1525>;
> opp-avg-KBps = <452>;
> };
> gpu_ddr_3051: opp-3051 {
> opp-peak-KBps = <3051>;
> opp-avg-KBps = <915>;
> };
> gpu_ddr_7500: opp-7500 {
> opp-peak-KBps = <7500>;
> opp-avg-KBps = <3000>;
> };
> };
Who is going to use the above tables and how ? These are the maximum
BW available over these paths, right ?
> gpu_opp_table: gpu_opp_table {
> compatible = "operating-points-v2";
> opp-shared;
>
> opp-200000000 {
> opp-hz = /bits/ 64 <200000000>;
> };
> opp-400000000 {
> opp-hz = /bits/ 64 <400000000>;
> };
> };
Shouldn't this link back to the above tables via required-opp, etc ?
How will we know how much BW is required by the GPU device for all the
paths ?
> gpu@7864000 {
> ...
> operating-points-v2 = <&gpu_opp_table>, <&gpu_cache_opp_table>, <&gpu_ddr_opp_table>;
> interconnects = <&mmnoc MASTER_GPU_1 &bimc SLAVE_SYSTEM_CACHE>,
> <&mmnoc MASTER_GPU_1 &bimc SLAVE_DDR>;
> interconnect-names = "gpu-cache", "gpu-mem";
> interconnect-opp-table = <&gpu_cache_opp_table>, <&gpu_ddr_opp_table>
> };
--
viresh
^ permalink raw reply
* [PATCH V2 2/2] opp: Return genpd virtual devices from dev_pm_opp_attach_genpd()
From: Viresh Kumar @ 2019-07-17 10:19 UTC (permalink / raw)
To: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki
Cc: Viresh Kumar, linux-pm, Vincent Guittot, niklas.cassel,
Rajendra Nayak, linux-kernel
In-Reply-To: <90ba3941b56e50ea5548dd1e90bf3bcd8c9da4fe.1563358721.git.viresh.kumar@linaro.org>
The cpufreq drivers don't need to do runtime PM operations on the
virtual devices returned by dev_pm_domain_attach_by_name() and so the
virtual devices weren't shared with the callers of
dev_pm_opp_attach_genpd() earlier.
But the IO device drivers would want to do that. This patch updates the
prototype of dev_pm_opp_attach_genpd() to accept another argument to
return the pointer to the array of genpd virtual devices.
Reported-by: Rajendra Nayak <rnayak@codeaurora.org>
Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V1->V2:
- Check virt_devs pointer before assigning.
drivers/opp/core.c | 6 +++++-
include/linux/pm_opp.h | 4 ++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 4e2a81698cd2..1f11f8c92337 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1775,6 +1775,7 @@ static void _opp_detach_genpd(struct opp_table *opp_table)
* dev_pm_opp_attach_genpd - Attach genpd(s) for the device and save virtual device pointer
* @dev: Consumer device for which the genpd is getting attached.
* @names: Null terminated array of pointers containing names of genpd to attach.
+ * @virt_devs: Pointer to return the array of virtual devices.
*
* Multiple generic power domains for a device are supported with the help of
* virtual genpd devices, which are created for each consumer device - genpd
@@ -1792,7 +1793,8 @@ static void _opp_detach_genpd(struct opp_table *opp_table)
* The order of entries in the names array must match the order in which
* "required-opps" are added in DT.
*/
-struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char **names)
+struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
+ const char **names, struct device ***virt_devs)
{
struct opp_table *opp_table;
struct device *virt_dev;
@@ -1846,6 +1848,8 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char **names
name++;
}
+ if (virt_devs)
+ *virt_devs = opp_table->genpd_virt_devs;
mutex_unlock(&opp_table->genpd_virt_dev_lock);
return opp_table;
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index be570761b77a..7c2fe2952f40 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -131,7 +131,7 @@ struct opp_table *dev_pm_opp_set_clkname(struct device *dev, const char * name);
void dev_pm_opp_put_clkname(struct opp_table *opp_table);
struct opp_table *dev_pm_opp_register_set_opp_helper(struct device *dev, int (*set_opp)(struct dev_pm_set_opp_data *data));
void dev_pm_opp_unregister_set_opp_helper(struct opp_table *opp_table);
-struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char **names);
+struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char **names, struct device ***virt_devs);
void dev_pm_opp_detach_genpd(struct opp_table *opp_table);
int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct opp_table *dst_table, unsigned int pstate);
int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
@@ -295,7 +295,7 @@ static inline struct opp_table *dev_pm_opp_set_clkname(struct device *dev, const
static inline void dev_pm_opp_put_clkname(struct opp_table *opp_table) {}
-static inline struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char **names)
+static inline struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char **names, struct device ***virt_devs)
{
return ERR_PTR(-ENOTSUPP);
}
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related
* [PATCH 1/2] opp: Not all power-domains are scalable
From: Viresh Kumar @ 2019-07-17 10:19 UTC (permalink / raw)
To: Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: Viresh Kumar, linux-pm, Vincent Guittot, Rafael Wysocki,
niklas.cassel, Rajendra Nayak, linux-kernel
A device may have multiple power-domains and not all of them may be
scalable (i.e. support performance states). But
dev_pm_opp_attach_genpd() doesn't take that into account currently.
Fix that by not verifying the names argument with "power-domain-names"
DT property and finding the index into the required-opps array. The
names argument will anyway get verified later on when we call
dev_pm_domain_attach_by_name().
Fixes: 6319aee10e53 ("opp: Attach genpds to devices from within OPP core")
Reported-by: Rajendra Nayak <rnayak@codeaurora.org>
Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/opp/core.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 2958cc7bbb58..4e2a81698cd2 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1788,12 +1788,15 @@ static void _opp_detach_genpd(struct opp_table *opp_table)
*
* This helper needs to be called once with a list of all genpd to attach.
* Otherwise the original device structure will be used instead by the OPP core.
+ *
+ * The order of entries in the names array must match the order in which
+ * "required-opps" are added in DT.
*/
struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char **names)
{
struct opp_table *opp_table;
struct device *virt_dev;
- int index, ret = -EINVAL;
+ int index = 0, ret = -EINVAL;
const char **name = names;
opp_table = dev_pm_opp_get_opp_table(dev);
@@ -1819,14 +1822,6 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char **names
goto unlock;
while (*name) {
- index = of_property_match_string(dev->of_node,
- "power-domain-names", *name);
- if (index < 0) {
- dev_err(dev, "Failed to find power domain: %s (%d)\n",
- *name, index);
- goto err;
- }
-
if (index >= opp_table->required_opp_count) {
dev_err(dev, "Index can't be greater than required-opp-count - 1, %s (%d : %d)\n",
*name, opp_table->required_opp_count, index);
@@ -1847,6 +1842,7 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char **names
}
opp_table->genpd_virt_devs[index] = virt_dev;
+ index++;
name++;
}
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related
* Re: [PATCH] opp: Return genpd virtual devices from dev_pm_opp_attach_genpd()
From: Viresh Kumar @ 2019-07-17 10:14 UTC (permalink / raw)
To: Rajendra Nayak
Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
linux-pm, Vincent Guittot, linux-kernel
In-Reply-To: <2fbfc0fc-7d61-bf8c-67de-28183d03e26a@codeaurora.org>
On 17-07-19, 15:34, Rajendra Nayak wrote:
>
>
> On 7/17/2019 11:17 AM, Viresh Kumar wrote:
> > On 11-07-19, 15:09, Rajendra Nayak wrote:
> > > Sorry for the delay
> >
> > Same here :)
> >
> > > I seem to have completely missed this patch.
> > > I just gave this a try and here are some observations,
> > >
> > > I have a case where I have one device with 2 power domains, one of them
> > > is scale-able (supports perf state) and the other one supports only being
> > > turned on and off.
> > >
> > > 1. In the driver I now need to use dev_pm_domain_attach_by_name/id to attach the
> > > power domain which supports only on/off and then use dev_pm_opp_attach_genpd()
> > > for the one which supports perf states.
> > >
> > > 2. My OPP table has only 1 required_opps, so the required_opp_count for the OPP table is 1.
> > > Now if my device tree has my scale-able powerdomain at index 1 (it works if its at index 0)
> > > then I end up with this error
> > >
> > > [ 2.858628] ufshcd-qcom 1d84000.ufshc: Index can't be greater than required-opp-count - 1, rpmh_pd (1 : 1)
> > >
> > > so it looks like a lot of the OPP core today just assumes that if a device has multiple power domains,
> > > all of them are scale-able which isn't necessarily true.
> >
> > I don't think a lot of OPP core has these problems, but maybe only
> > this place. I was taking care of this since the beginning just forgot
> > it now.
> >
> > What about this over this commit:
>
> Yes, this does seem to fix my concern mentioned in 2. above.
Great. I will include your Tested-by:, Lemme know if you have any
objections.
--
viresh
^ permalink raw reply
* Re: [PATCH] opp: Return genpd virtual devices from dev_pm_opp_attach_genpd()
From: Rajendra Nayak @ 2019-07-17 10:04 UTC (permalink / raw)
To: Viresh Kumar
Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
linux-pm, Vincent Guittot, linux-kernel
In-Reply-To: <20190717054713.vn65cfiqebhzdvjq@vireshk-i7>
On 7/17/2019 11:17 AM, Viresh Kumar wrote:
> On 11-07-19, 15:09, Rajendra Nayak wrote:
>> Sorry for the delay
>
> Same here :)
>
>> I seem to have completely missed this patch.
>> I just gave this a try and here are some observations,
>>
>> I have a case where I have one device with 2 power domains, one of them
>> is scale-able (supports perf state) and the other one supports only being
>> turned on and off.
>>
>> 1. In the driver I now need to use dev_pm_domain_attach_by_name/id to attach the
>> power domain which supports only on/off and then use dev_pm_opp_attach_genpd()
>> for the one which supports perf states.
>>
>> 2. My OPP table has only 1 required_opps, so the required_opp_count for the OPP table is 1.
>> Now if my device tree has my scale-able powerdomain at index 1 (it works if its at index 0)
>> then I end up with this error
>>
>> [ 2.858628] ufshcd-qcom 1d84000.ufshc: Index can't be greater than required-opp-count - 1, rpmh_pd (1 : 1)
>>
>> so it looks like a lot of the OPP core today just assumes that if a device has multiple power domains,
>> all of them are scale-able which isn't necessarily true.
>
> I don't think a lot of OPP core has these problems, but maybe only
> this place. I was taking care of this since the beginning just forgot
> it now.
>
> What about this over this commit:
Yes, this does seem to fix my concern mentioned in 2. above.
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index d76ead4eff4c..1f11f8c92337 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -1789,13 +1789,16 @@ static void _opp_detach_genpd(struct opp_table *opp_table)
> *
> * This helper needs to be called once with a list of all genpd to attach.
> * Otherwise the original device structure will be used instead by the OPP core.
> + *
> + * The order of entries in the names array must match the order in which
> + * "required-opps" are added in DT.
> */
> struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
> const char **names, struct device ***virt_devs)
> {
> struct opp_table *opp_table;
> struct device *virt_dev;
> - int index, ret = -EINVAL;
> + int index = 0, ret = -EINVAL;
> const char **name = names;
>
> opp_table = dev_pm_opp_get_opp_table(dev);
> @@ -1821,14 +1824,6 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
> goto unlock;
>
> while (*name) {
> - index = of_property_match_string(dev->of_node,
> - "power-domain-names", *name);
> - if (index < 0) {
> - dev_err(dev, "Failed to find power domain: %s (%d)\n",
> - *name, index);
> - goto err;
> - }
> -
> if (index >= opp_table->required_opp_count) {
> dev_err(dev, "Index can't be greater than required-opp-count - 1, %s (%d : %d)\n",
> *name, opp_table->required_opp_count, index);
> @@ -1849,6 +1844,7 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
> }
>
> opp_table->genpd_virt_devs[index] = virt_dev;
> + index++;
> name++;
> }
>
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH] cpuidle: Always stop scheduler tick on adaptive-tick CPUs
From: Rafael J. Wysocki @ 2019-07-17 7:55 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Rafael J. Wysocki, Linux PM, Thomas Lindroth, LKML,
Peter Zijlstra
In-Reply-To: <20190716214024.GA8345@lenoir>
On Tue, Jul 16, 2019 at 11:40 PM Frederic Weisbecker
<frederic@kernel.org> wrote:
>
> On Tue, Jul 16, 2019 at 05:25:10PM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Running the scheduler tick on idle adaptive-tick CPUs is not useful
>
> Judging by the below change, you mean full dynticks, right?
Right.
> > and it may also be not expected by users (as reported by Thomas), so
> > add a check to cpuidle_idle_call() to always stop the tick on them
> > regardless of the idle duration predicted by the governor.
> >
> > Fixes: 554c8aa8ecad ("sched: idle: Select idle state before stopping the tick")
> > Reported-by: Thomas Lindroth <thomas.lindroth@gmail.com>
> > Tested-by: Thomas Lindroth <thomas.lindroth@gmail.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> > kernel/sched/idle.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Index: linux-pm/kernel/sched/idle.c
> > ===================================================================
> > --- linux-pm.orig/kernel/sched/idle.c
> > +++ linux-pm/kernel/sched/idle.c
> > @@ -191,7 +191,8 @@ static void cpuidle_idle_call(void)
> > */
> > next_state = cpuidle_select(drv, dev, &stop_tick);
> >
> > - if (stop_tick || tick_nohz_tick_stopped())
> > + if (stop_tick || tick_nohz_tick_stopped() ||
> > + !housekeeping_cpu(dev->cpu, HK_FLAG_TICK))
>
> But tick_nohz_tick_stopped() also works on full dynticks CPUs. If the
> tick isn't stopped on a full dynticks CPU by the time we reach this path,
> it means that the conditions for the tick to be stopped are not met anyway
> (eg: more than one task and sched tick is needed, perf event requires the tick,
> posix CPU timer, etc...)
First of all, according to Thomas, the patch does make a difference,
so evidently on his system(s) the full dynticks CPUs enter the idle
loop with running tick.
This means that, indeed, the conditions for the tick to be stopped
have not been met up to that point, but if the (full dynticks) CPU
becomes idle, that's because it has been made idle on purpose
(presumably by a user-space "orchestrator" or the sysadmin), so the
kernel can assume that it will remain idle indefinitely. That, in
turn, is when the tick would be stopped on it regardless of everything
else (even if it wasn't a full dynticks CPU).
I guess I should add the above to the changelog.
> Or am I missing something else?
Well, if full dynticks CPUs are expected to always enter the idle loop
with stopped tick, then something appears to be amiss, but I'm not
sure if that expectation is entirely realistic.
Cheers!
^ permalink raw reply
* Re: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings
From: Viresh Kumar @ 2019-07-17 7:54 UTC (permalink / raw)
To: Saravana Kannan
Cc: Georgi Djakov, Rob Herring, Mark Rutland, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, vincent.guittot,
seansw, daidavid1, Rajendra Nayak, sibis, bjorn.andersson,
evgreen, kernel-team, linux-pm, devicetree, linux-kernel
In-Reply-To: <20190703011020.151615-2-saravanak@google.com>
On 02-07-19, 18:10, Saravana Kannan wrote:
> Interconnects often quantify their performance points in terms of
> bandwidth. So, add opp-peak-KBps (required) and opp-avg-KBps (optional) to
> allow specifying Bandwidth OPP tables in DT.
>
> opp-peak-KBps is a required property that replace opp-hz for Bandwidth OPP
> tables.
>
> opp-avg-KBps is an optional property that can be used in Bandwidth OPP
> tables.
>
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
> Documentation/devicetree/bindings/opp/opp.txt | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index 76b6c79604a5..c869e87caa2a 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -83,9 +83,14 @@ properties.
>
> Required properties:
> - opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. This is a
> - required property for all device nodes but devices like power domains. The
> - power domain nodes must have another (implementation dependent) property which
> - uniquely identifies the OPP nodes.
> + required property for all device nodes but for devices like power domains or
> + bandwidth opp tables. The power domain nodes must have another (implementation
> + dependent) property which uniquely identifies the OPP nodes. The interconnect
> + opps are required to have the opp-peak-bw property.
??
> +
> +- opp-peak-KBps: Peak bandwidth in kilobytes per second, expressed as a 32-bit
> + big-endian integer. This is a required property for all devices that don't
> + have opp-hz. For example, bandwidth OPP tables for interconnect paths.
>
> Optional properties:
> - opp-microvolt: voltage in micro Volts.
> @@ -132,6 +137,10 @@ Optional properties:
> - opp-level: A value representing the performance level of the device,
> expressed as a 32-bit integer.
>
> +- opp-avg-KBps: Average bandwidth in kilobytes per second, expressed as a
> + 32-bit big-endian integer. This property is only meaningful in OPP tables
> + where opp-peak-KBps is present.
> +
> - clock-latency-ns: Specifies the maximum possible transition latency (in
> nanoseconds) for switching to this OPP from any other OPP.
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
--
viresh
^ permalink raw reply
* Re: [PATCH v4 11/24] PM / devfreq: tegra30: Add debug messages
From: Chanwoo Choi @ 2019-07-17 6:45 UTC (permalink / raw)
To: Dmitry Osipenko, Thierry Reding, MyungJoo Ham, Kyungmin Park,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <53cd0ba5-f814-cd9b-19c5-1d42717ca58c@gmail.com>
On 19. 7. 16. 오후 10:26, Dmitry Osipenko wrote:
> 16.07.2019 15:23, Chanwoo Choi пишет:
>> Hi Dmitry,
>>
>> Usually, the kernel log print for all users
>> such as changing the frequency, fail or success.
>>
>> But, if the log just show the register dump,
>> it is not useful for all users. It is just used
>> for only specific developer.
>>
>> I recommend that you better to add more exception handling
>> code on many points instead of just showing the register dump.
>
> The debug messages are not users, but for developers. Yes, I primarily
> made the debugging to be useful for myself and will be happy to change
> the way debugging is done if there will be any other active developer
> for this driver. The registers dump is more than enough in order to
> understand what's going on, I don't see any real need to change anything
> here for now.
Basically, we have to develop code and add the log for anyone.
As you commented, even if there are no other developer, we never
guarantee this assumption forever. And also, if added debug message
for only you, you can add them when testing it temporarily.
If you want to add the just register dump log for you,
I can't agree. Once again, I hope that anyone understand
the meaning of debug message as much possible as.
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply
* Re: [PATCH v4 03/24] PM / devfreq: tegra30: Handle possible round-rate error
From: Chanwoo Choi @ 2019-07-17 6:38 UTC (permalink / raw)
To: Dmitry Osipenko, Thierry Reding, MyungJoo Ham, Kyungmin Park,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <e9fd7f68-8ad1-058f-7c87-ccd280ba0b0e@gmail.com>
On 19. 7. 16. 오후 10:09, Dmitry Osipenko wrote:
> 16.07.2019 14:50, Chanwoo Choi пишет:
>> On 19. 7. 8. 오전 7:32, Dmitry Osipenko wrote:
>>> The EMC clock rate rounding technically could fail, hence let's handle
>>> the error cases properly.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>> drivers/devfreq/tegra30-devfreq.c | 17 +++++++++++++++--
>>> 1 file changed, 15 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
>>> index 5e2b133babdd..5e606ae3f238 100644
>>> --- a/drivers/devfreq/tegra30-devfreq.c
>>> +++ b/drivers/devfreq/tegra30-devfreq.c
>>> @@ -592,8 +592,8 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>> struct tegra_devfreq_device *dev;
>>> struct tegra_devfreq *tegra;
>>> struct devfreq *devfreq;
>>> - unsigned long rate;
>>> unsigned int i;
>>> + long rate;
>>> int err;
>>>
>>> tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
>>> @@ -650,8 +650,14 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>>
>>> reset_control_deassert(tegra->reset);
>>>
>>> - tegra->max_freq = clk_round_rate(tegra->emc_clock, ULONG_MAX) / KHZ;
>>> + rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
>>> + if (rate < 0) {
>>> + dev_err(&pdev->dev, "Failed to round clock rate: %ld\n", rate);
>>> + return rate;
>>> + }
>>> +
>>> tegra->cur_freq = clk_get_rate(tegra->emc_clock) / KHZ;
>>> + tegra->max_freq = rate / KHZ;
>>>
>>> for (i = 0; i < ARRAY_SIZE(actmon_device_configs); i++) {
>>> dev = tegra->devices + i;
>>> @@ -662,6 +668,13 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>> for (rate = 0; rate <= tegra->max_freq * KHZ; rate++) {
>>> rate = clk_round_rate(tegra->emc_clock, rate);
>>>
>>
>> Please remove unneeded blank line.
>
> I can remove it, but it was added specifically to ease reading of the code.
>
>>> + if (rate < 0) {
>>> + dev_err(&pdev->dev,
>>> + "Failed to round clock rate: %ld\n", rate);
>>> + err = rate;
>>> + goto remove_opps;
>>> + }
>>
>> Also, this patch doesn't contain code which restore the previous
>> tegra->cur_freq/max_freq when error happen.
>
> The error here results in abortion of the driver's probing, hence
> nothing need to be restored in that case because nothing was changed at
> this point yet.
>
>
OK.
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply
* [PATCH 2/2] thermal: db8500: Use dev helper variable
From: Linus Walleij @ 2019-07-17 6:32 UTC (permalink / raw)
To: Zhang Rui, Eduardo Valentin, Daniel Lezcano; +Cc: linux-pm, Linus Walleij
In-Reply-To: <20190717063222.5902-1-linus.walleij@linaro.org>
The code gets easier to read like this.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/thermal/db8500_thermal.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/thermal/db8500_thermal.c b/drivers/thermal/db8500_thermal.c
index d650ae5fdf2a..d250bcf3c10c 100644
--- a/drivers/thermal/db8500_thermal.c
+++ b/drivers/thermal/db8500_thermal.c
@@ -313,16 +313,16 @@ static void db8500_thermal_work(struct work_struct *work)
}
static struct db8500_thsens_platform_data*
- db8500_thermal_parse_dt(struct platform_device *pdev)
+ db8500_thermal_parse_dt(struct device *dev)
{
struct db8500_thsens_platform_data *ptrips;
- struct device_node *np = pdev->dev.of_node;
+ struct device_node *np = dev->of_node;
char prop_name[32];
const char *tmp_str;
u32 tmp_data;
int i, j;
- ptrips = devm_kzalloc(&pdev->dev, sizeof(*ptrips), GFP_KERNEL);
+ ptrips = devm_kzalloc(dev, sizeof(*ptrips), GFP_KERNEL);
if (!ptrips)
return NULL;
@@ -377,7 +377,7 @@ static struct db8500_thsens_platform_data*
return ptrips;
err_parse_dt:
- dev_err(&pdev->dev, "Parsing device tree data error.\n");
+ dev_err(dev, "Parsing device tree data error.\n");
return NULL;
}
@@ -385,18 +385,19 @@ static int db8500_thermal_probe(struct platform_device *pdev)
{
struct db8500_thermal_zone *pzone = NULL;
struct db8500_thsens_platform_data *ptrips = NULL;
- struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
int low_irq, high_irq, ret = 0;
unsigned long dft_low, dft_high;
if (!np)
return -EINVAL;
- ptrips = db8500_thermal_parse_dt(pdev);
+ ptrips = db8500_thermal_parse_dt(dev);
if (!ptrips)
return -EINVAL;
- pzone = devm_kzalloc(&pdev->dev, sizeof(*pzone), GFP_KERNEL);
+ pzone = devm_kzalloc(dev, sizeof(*pzone), GFP_KERNEL);
if (!pzone)
return -ENOMEM;
@@ -410,31 +411,31 @@ static int db8500_thermal_probe(struct platform_device *pdev)
low_irq = platform_get_irq_byname(pdev, "IRQ_HOTMON_LOW");
if (low_irq < 0) {
- dev_err(&pdev->dev, "Get IRQ_HOTMON_LOW failed.\n");
+ dev_err(dev, "Get IRQ_HOTMON_LOW failed.\n");
ret = low_irq;
goto out_unlock;
}
- ret = devm_request_threaded_irq(&pdev->dev, low_irq, NULL,
+ ret = devm_request_threaded_irq(dev, low_irq, NULL,
prcmu_low_irq_handler, IRQF_NO_SUSPEND | IRQF_ONESHOT,
"dbx500_temp_low", pzone);
if (ret < 0) {
- dev_err(&pdev->dev, "Failed to allocate temp low irq.\n");
+ dev_err(dev, "Failed to allocate temp low irq.\n");
goto out_unlock;
}
high_irq = platform_get_irq_byname(pdev, "IRQ_HOTMON_HIGH");
if (high_irq < 0) {
- dev_err(&pdev->dev, "Get IRQ_HOTMON_HIGH failed.\n");
+ dev_err(dev, "Get IRQ_HOTMON_HIGH failed.\n");
ret = high_irq;
goto out_unlock;
}
- ret = devm_request_threaded_irq(&pdev->dev, high_irq, NULL,
+ ret = devm_request_threaded_irq(dev, high_irq, NULL,
prcmu_high_irq_handler, IRQF_NO_SUSPEND | IRQF_ONESHOT,
"dbx500_temp_high", pzone);
if (ret < 0) {
- dev_err(&pdev->dev, "Failed to allocate temp high irq.\n");
+ dev_err(dev, "Failed to allocate temp high irq.\n");
goto out_unlock;
}
@@ -442,11 +443,11 @@ static int db8500_thermal_probe(struct platform_device *pdev)
ptrips->num_trips, 0, pzone, &thdev_ops, NULL, 0, 0);
if (IS_ERR(pzone->therm_dev)) {
- dev_err(&pdev->dev, "Register thermal zone device failed.\n");
+ dev_err(dev, "Register thermal zone device failed.\n");
ret = PTR_ERR(pzone->therm_dev);
goto out_unlock;
}
- dev_info(&pdev->dev, "Thermal zone device registered.\n");
+ dev_info(dev, "Thermal zone device registered.\n");
dft_low = PRCMU_DEFAULT_LOW_TEMP;
dft_high = ptrips->trip_points[0].temp;
--
2.21.0
^ permalink raw reply related
* [PATCH 1/2] thermal: db8500: Finalize device tree conversion
From: Linus Walleij @ 2019-07-17 6:32 UTC (permalink / raw)
To: Zhang Rui, Eduardo Valentin, Daniel Lezcano
Cc: linux-pm, Linus Walleij, Lee Jones
At some point there was an attempt to convert the DB8500
thermal sensor to device tree: a probe path was added
and the device tree was augmented for the Snowball board.
The switchover was never completed: instead the thermal
devices came from from the PRCMU MFD device and the probe
on the Snowball was confused as another set of configuration
appeared from the device tree.
Move over to a device-tree only approach, as we fixed up
the device trees.
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Lee: it'd be great if you could ACK this, it is a file
with low change rate so we should likely not see any
collisions.
---
drivers/mfd/db8500-prcmu.c | 53 +-------------------
drivers/thermal/Kconfig | 2 +-
drivers/thermal/db8500_thermal.c | 30 +++++------
include/linux/platform_data/db8500_thermal.h | 29 -----------
4 files changed, 17 insertions(+), 97 deletions(-)
delete mode 100644 include/linux/platform_data/db8500_thermal.h
diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
index 3f21e26b8d36..a1e09bf06977 100644
--- a/drivers/mfd/db8500-prcmu.c
+++ b/drivers/mfd/db8500-prcmu.c
@@ -36,7 +36,6 @@
#include <linux/regulator/db8500-prcmu.h>
#include <linux/regulator/machine.h>
#include <linux/platform_data/ux500_wdt.h>
-#include <linux/platform_data/db8500_thermal.h>
#include "dbx500-prcmu-regs.h"
/* Index of different voltages to be used when accessing AVSData */
@@ -2982,53 +2981,6 @@ static struct ux500_wdt_data db8500_wdt_pdata = {
.timeout = 600, /* 10 minutes */
.has_28_bits_resolution = true,
};
-/*
- * Thermal Sensor
- */
-
-static struct resource db8500_thsens_resources[] = {
- {
- .name = "IRQ_HOTMON_LOW",
- .start = IRQ_PRCMU_HOTMON_LOW,
- .end = IRQ_PRCMU_HOTMON_LOW,
- .flags = IORESOURCE_IRQ,
- },
- {
- .name = "IRQ_HOTMON_HIGH",
- .start = IRQ_PRCMU_HOTMON_HIGH,
- .end = IRQ_PRCMU_HOTMON_HIGH,
- .flags = IORESOURCE_IRQ,
- },
-};
-
-static struct db8500_thsens_platform_data db8500_thsens_data = {
- .trip_points[0] = {
- .temp = 70000,
- .type = THERMAL_TRIP_ACTIVE,
- .cdev_name = {
- [0] = "thermal-cpufreq-0",
- },
- },
- .trip_points[1] = {
- .temp = 75000,
- .type = THERMAL_TRIP_ACTIVE,
- .cdev_name = {
- [0] = "thermal-cpufreq-0",
- },
- },
- .trip_points[2] = {
- .temp = 80000,
- .type = THERMAL_TRIP_ACTIVE,
- .cdev_name = {
- [0] = "thermal-cpufreq-0",
- },
- },
- .trip_points[3] = {
- .temp = 85000,
- .type = THERMAL_TRIP_CRITICAL,
- },
- .num_trips = 4,
-};
static const struct mfd_cell common_prcmu_devs[] = {
{
@@ -3052,10 +3004,7 @@ static const struct mfd_cell db8500_prcmu_devs[] = {
},
{
.name = "db8500-thermal",
- .num_resources = ARRAY_SIZE(db8500_thsens_resources),
- .resources = db8500_thsens_resources,
- .platform_data = &db8500_thsens_data,
- .pdata_size = sizeof(db8500_thsens_data),
+ .of_compatible = "stericsson,db8500-thermal",
},
};
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 9966364a6deb..001a21abcc28 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -310,7 +310,7 @@ config DOVE_THERMAL
config DB8500_THERMAL
tristate "DB8500 thermal management"
- depends on MFD_DB8500_PRCMU
+ depends on MFD_DB8500_PRCMU && OF
default y
help
Adds DB8500 thermal management implementation according to the thermal
diff --git a/drivers/thermal/db8500_thermal.c b/drivers/thermal/db8500_thermal.c
index b71a999d17d6..d650ae5fdf2a 100644
--- a/drivers/thermal/db8500_thermal.c
+++ b/drivers/thermal/db8500_thermal.c
@@ -13,13 +13,24 @@
#include <linux/mfd/dbx500-prcmu.h>
#include <linux/module.h>
#include <linux/of.h>
-#include <linux/platform_data/db8500_thermal.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/thermal.h>
#define PRCMU_DEFAULT_MEASURE_TIME 0xFFF
#define PRCMU_DEFAULT_LOW_TEMP 0
+#define COOLING_DEV_MAX 8
+
+struct db8500_trip_point {
+ unsigned long temp;
+ enum thermal_trip_type type;
+ char cdev_name[COOLING_DEV_MAX][THERMAL_NAME_LENGTH];
+};
+
+struct db8500_thsens_platform_data {
+ struct db8500_trip_point trip_points[THERMAL_MAX_TRIPS];
+ int num_trips;
+};
struct db8500_thermal_zone {
struct thermal_zone_device *therm_dev;
@@ -301,7 +312,6 @@ static void db8500_thermal_work(struct work_struct *work)
dev_dbg(&pzone->therm_dev->device, "thermal work finished.\n");
}
-#ifdef CONFIG_OF
static struct db8500_thsens_platform_data*
db8500_thermal_parse_dt(struct platform_device *pdev)
{
@@ -370,13 +380,6 @@ static struct db8500_thsens_platform_data*
dev_err(&pdev->dev, "Parsing device tree data error.\n");
return NULL;
}
-#else
-static inline struct db8500_thsens_platform_data*
- db8500_thermal_parse_dt(struct platform_device *pdev)
-{
- return NULL;
-}
-#endif
static int db8500_thermal_probe(struct platform_device *pdev)
{
@@ -386,11 +389,10 @@ static int db8500_thermal_probe(struct platform_device *pdev)
int low_irq, high_irq, ret = 0;
unsigned long dft_low, dft_high;
- if (np)
- ptrips = db8500_thermal_parse_dt(pdev);
- else
- ptrips = dev_get_platdata(&pdev->dev);
+ if (!np)
+ return -EINVAL;
+ ptrips = db8500_thermal_parse_dt(pdev);
if (!ptrips)
return -EINVAL;
@@ -498,13 +500,11 @@ static int db8500_thermal_resume(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_OF
static const struct of_device_id db8500_thermal_match[] = {
{ .compatible = "stericsson,db8500-thermal" },
{},
};
MODULE_DEVICE_TABLE(of, db8500_thermal_match);
-#endif
static struct platform_driver db8500_thermal_driver = {
.driver = {
diff --git a/include/linux/platform_data/db8500_thermal.h b/include/linux/platform_data/db8500_thermal.h
deleted file mode 100644
index 55e55750a165..000000000000
--- a/include/linux/platform_data/db8500_thermal.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * db8500_thermal.h - DB8500 Thermal Management Implementation
- *
- * Copyright (C) 2012 ST-Ericsson
- * Copyright (C) 2012 Linaro Ltd.
- *
- * Author: Hongbo Zhang <hongbo.zhang@linaro.com>
- */
-
-#ifndef _DB8500_THERMAL_H_
-#define _DB8500_THERMAL_H_
-
-#include <linux/thermal.h>
-
-#define COOLING_DEV_MAX 8
-
-struct db8500_trip_point {
- unsigned long temp;
- enum thermal_trip_type type;
- char cdev_name[COOLING_DEV_MAX][THERMAL_NAME_LENGTH];
-};
-
-struct db8500_thsens_platform_data {
- struct db8500_trip_point trip_points[THERMAL_MAX_TRIPS];
- int num_trips;
-};
-
-#endif /* _DB8500_THERMAL_H_ */
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v4 02/24] PM / devfreq: tegra30: Keep interrupt disabled while governor is stopped
From: Chanwoo Choi @ 2019-07-17 6:37 UTC (permalink / raw)
To: Dmitry Osipenko, Thierry Reding, MyungJoo Ham, Kyungmin Park,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <6c517d04-cf99-f907-e74d-9fba99405a53@gmail.com>
On 19. 7. 16. 오후 10:03, Dmitry Osipenko wrote:
> 16.07.2019 14:47, Chanwoo Choi пишет:
>> On 19. 7. 8. 오전 7:32, Dmitry Osipenko wrote:
>>> There is no real need to keep interrupt always-enabled, will be nicer
>>> to keep it disabled while governor is inactive.
>>>
>>> Suggested-by: Thierry Reding <thierry.reding@gmail.com>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>> drivers/devfreq/tegra30-devfreq.c | 43 ++++++++++++++++---------------
>>> 1 file changed, 22 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
>>> index a27300f40b0b..5e2b133babdd 100644
>>> --- a/drivers/devfreq/tegra30-devfreq.c
>>> +++ b/drivers/devfreq/tegra30-devfreq.c
>>> @@ -11,6 +11,7 @@
>>> #include <linux/devfreq.h>
>>> #include <linux/interrupt.h>
>>> #include <linux/io.h>
>>> +#include <linux/irq.h>
>>> #include <linux/module.h>
>>> #include <linux/mod_devicetable.h>
>>> #include <linux/platform_device.h>
>>> @@ -416,8 +417,6 @@ static void tegra_actmon_start(struct tegra_devfreq *tegra)
>>> {
>>> unsigned int i;
>>>
>>> - disable_irq(tegra->irq);
>>> -
>>> actmon_writel(tegra, ACTMON_SAMPLING_PERIOD - 1,
>>> ACTMON_GLB_PERIOD_CTRL);
>>>
>>> @@ -442,8 +441,6 @@ static void tegra_actmon_stop(struct tegra_devfreq *tegra)
>>> }
>>>
>>> actmon_write_barrier(tegra);
>>> -
>>> - enable_irq(tegra->irq);
>>> }
>>>
>>> static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
>>> @@ -552,6 +549,12 @@ static int tegra_governor_event_handler(struct devfreq *devfreq,
>>> {
>>> struct tegra_devfreq *tegra = dev_get_drvdata(devfreq->dev.parent);
>>>
>>> + /*
>>> + * Couple device with the governor early as it is needed at
>>> + * the moment of governor's start (used by ISR).
>>> + */
>>> + tegra->devfreq = devfreq;
>>
>> I'm not sure it is necessary. Almost devfreq device get
>> the devfreq instance on probe timing through devfreq_add_device directly.
>
> This is necessary because this assignment is for the "governor" and not
> the "device". Governor is started during of devfreq_add_device(), hence
> there is no better way to assign device to the driver's governor.
OK. I understand.
But, I have a question. Is it working before this patch?
How can you test it on that tegra->devfreq is NULL?
>
>>> +
>>> switch (event) {
>>> case DEVFREQ_GOV_START:
>>> devfreq_monitor_start(devfreq);
>>> @@ -586,10 +589,11 @@ static struct devfreq_governor tegra_devfreq_governor = {
>>>
>>> static int tegra_devfreq_probe(struct platform_device *pdev)
>>> {
>>> - struct tegra_devfreq *tegra;
>>> struct tegra_devfreq_device *dev;
>>> - unsigned int i;
>>> + struct tegra_devfreq *tegra;
>>> + struct devfreq *devfreq;
>>> unsigned long rate;
>>> + unsigned int i;
>>> int err;
>>>
>>> tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
>>> @@ -625,6 +629,16 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>> }
>>> tegra->irq = err;
>>>
>>> + irq_set_status_flags(tegra->irq, IRQ_NOAUTOEN);
>>> +
>>> + err = devm_request_threaded_irq(&pdev->dev, tegra->irq, NULL,
>>> + actmon_thread_isr, IRQF_ONESHOT,
>>> + "tegra-devfreq", tegra);
>>> + if (err) {
>>> + dev_err(&pdev->dev, "Interrupt request failed: %d\n", err);
>>> + return err;
>>> + }
>>> +
>>> reset_control_assert(tegra->reset);
>>>
>>> err = clk_prepare_enable(tegra->clock);
>>> @@ -672,28 +686,15 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>> }
>>>
>>> tegra_devfreq_profile.initial_freq = clk_get_rate(tegra->emc_clock);
>>> - tegra->devfreq = devfreq_add_device(&pdev->dev,
>>> - &tegra_devfreq_profile,
>>> - "tegra_actmon",
>>> - NULL);
>>> + devfreq = devfreq_add_device(&pdev->dev, &tegra_devfreq_profile,
>>> + "tegra_actmon", NULL);
>>> if (IS_ERR(tegra->devfreq)) {
>>
>> Have to check 'devfreq' instead of 'tegra->devfreq'.
>> Did you test it? It might be failed because 'tegra->devfreq is NULL.
>
> That's a good catch! Thank you very much.
>
>>> err = PTR_ERR(tegra->devfreq);
>>
>> ditto.
>
> Ok
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply
* [PATCH v3 09/12] kbuild: create *.mod with full directory path and remove MODVERDIR
From: Masahiro Yamada @ 2019-07-17 6:17 UTC (permalink / raw)
To: linux-kbuild
Cc: Joe Lawrence, Masahiro Yamada, Jonathan Corbet, Michal Marek,
Shuah Khan, Thomas Renninger, linux-doc, linux-kernel, linux-pm
In-Reply-To: <20190717061800.10018-1-yamada.masahiro@socionext.com>
While descending directories, Kbuild produces objects for modules,
but do not link final *.ko files; it is done in the modpost.
To keep track of modules, Kbuild creates a *.mod file in $(MODVERDIR)
for every module it is building. Some post-processing steps read the
necessary information from *.mod files. This avoids descending into
directories again. This mechanism was introduced in 2003 or so.
Later, commit 551559e13af1 ("kbuild: implement modules.order") added
modules.order. So, we can simply read it out to know all the modules
with directory paths. This is easier than parsing the first line of
*.mod files.
$(MODVERDIR) has a flat directory structure, that is, *.mod files
are named only with base names. This is based on the assumption that
the module name is unique across the tree. This assumption is really
fragile.
Stephen Rothwell reported a race condition caused by a module name
conflict:
https://lkml.org/lkml/2019/5/13/991
In parallel building, two different threads could write to the same
$(MODVERDIR)/*.mod simultaneously.
Non-unique module names are the source of all kind of troubles, hence
commit 3a48a91901c5 ("kbuild: check uniqueness of module names")
introduced a new checker script.
However, it is still fragile in the build system point of view because
this race happens before scripts/modules-check.sh is invoked. If it
happens again, the modpost will emit unclear error messages.
To fix this issue completely, create *.mod with full directory path
so that two threads never attempt to write to the same file.
$(MODVERDIR) is no longer needed.
Since modules with directory paths are listed in modules.order, Kbuild
is still able to find *.mod files without additional descending.
I also killed cmd_secanalysis; scripts/mod/sumversion.c computes MD4 hash
for modules with MODULE_VERSION(). When CONFIG_DEBUG_SECTION_MISMATCH=y,
it occurs not only in the modpost stage, but also during directory
descending, where sumversion.c may parse stale *.mod files. It would emit
'No such file or directory' warning when an object consisting a module is
renamed, or when a single-obj module is turned into a multi-obj module or
vice versa.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Acked-by: Nicolas Pitre <nico@fluxnic.net>
---
Changes in v3:
- Fix build error of allnoconfig
- Remove cmd_secanalysis
- Fix up comment in scripts/adjust_autoksyms.sh
- Fix up tools/power/cpupower/debug/kernel/Makefile
Changes in v2:
- Remove -r of xargs, which is a GNU extension
- Add '--' for extra safety
.gitignore | 1 +
Documentation/dontdiff | 1 +
Makefile | 20 +++-----------------
lib/Kconfig.debug | 12 +-----------
scripts/Makefile.build | 15 +++------------
scripts/Makefile.modpost | 4 ++--
scripts/adjust_autoksyms.sh | 14 +++++---------
scripts/mod/sumversion.c | 16 +++-------------
scripts/package/mkspec | 2 +-
tools/power/cpupower/debug/kernel/Makefile | 4 ++--
10 files changed, 22 insertions(+), 67 deletions(-)
diff --git a/.gitignore b/.gitignore
index 7587ef56b92d..8f5422cba6e2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -30,6 +30,7 @@
*.lz4
*.lzma
*.lzo
+*.mod
*.mod.c
*.o
*.o.*
diff --git a/Documentation/dontdiff b/Documentation/dontdiff
index 5eba889ea84d..9f4392876099 100644
--- a/Documentation/dontdiff
+++ b/Documentation/dontdiff
@@ -30,6 +30,7 @@
*.lzo
*.mo
*.moc
+*.mod
*.mod.c
*.o
*.o.*
diff --git a/Makefile b/Makefile
index 396cd5e525d1..9ad9f8d1130d 100644
--- a/Makefile
+++ b/Makefile
@@ -486,11 +486,6 @@ export KBUILD_AFLAGS_MODULE KBUILD_CFLAGS_MODULE KBUILD_LDFLAGS_MODULE
export KBUILD_AFLAGS_KERNEL KBUILD_CFLAGS_KERNEL
export KBUILD_ARFLAGS
-# When compiling out-of-tree modules, put MODVERDIR in the module
-# tree rather than in the kernel tree. The kernel tree might
-# even be read-only.
-export MODVERDIR := $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/).tmp_versions
-
# Files to ignore in find ... statements
export RCS_FIND_IGNORE := \( -name SCCS -o -name BitKeeper -o -name .svn -o \
@@ -1029,8 +1024,8 @@ vmlinux-deps := $(KBUILD_LDS) $(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS)
# Recurse until adjust_autoksyms.sh is satisfied
PHONY += autoksyms_recursive
-autoksyms_recursive: $(vmlinux-deps)
ifdef CONFIG_TRIM_UNUSED_KSYMS
+autoksyms_recursive: $(vmlinux-deps) modules.order
$(Q)$(CONFIG_SHELL) $(srctree)/scripts/adjust_autoksyms.sh \
"$(MAKE) -f $(srctree)/Makefile vmlinux"
endif
@@ -1113,7 +1108,6 @@ endif
prepare1: prepare3 outputmakefile asm-generic $(version_h) $(autoksyms_h) \
include/generated/utsrelease.h
- $(cmd_crmodverdir)
archprepare: archheaders archscripts prepare1 scripts
@@ -1371,7 +1365,7 @@ endif # CONFIG_MODULES
# make distclean Remove editor backup files, patch leftover files and the like
# Directories & files removed with 'make clean'
-CLEAN_DIRS += $(MODVERDIR) include/ksym
+CLEAN_DIRS += include/ksym
CLEAN_FILES += modules.builtin.modinfo
# Directories & files removed with 'make mrproper'
@@ -1641,7 +1635,6 @@ PHONY += $(clean-dirs) clean
$(clean-dirs):
$(Q)$(MAKE) $(clean)=$(patsubst _clean_%,%,$@)
-clean: rm-dirs := $(MODVERDIR)
clean: rm-files := $(KBUILD_EXTMOD)/Module.symvers
PHONY += help
@@ -1655,8 +1648,6 @@ help:
@echo ''
PHONY += prepare
-prepare:
- $(cmd_crmodverdir)
endif # KBUILD_EXTMOD
clean: $(clean-dirs)
@@ -1667,7 +1658,7 @@ clean: $(clean-dirs)
-o -name '*.ko.*' \
-o -name '*.dtb' -o -name '*.dtb.S' -o -name '*.dt.yaml' \
-o -name '*.dwo' -o -name '*.lst' \
- -o -name '*.su' \
+ -o -name '*.su' -o -name '*.mod' \
-o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \
-o -name '*.lex.c' -o -name '*.tab.[ch]' \
-o -name '*.asn1.[ch]' \
@@ -1796,11 +1787,6 @@ quiet_cmd_depmod = DEPMOD $(KERNELRELEASE)
cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \
$(KERNELRELEASE)
-# Create temporary dir for module support files
-# clean it up only when building all modules
-cmd_crmodverdir = $(Q)mkdir -p $(MODVERDIR) \
- $(if $(KBUILD_MODULES),; rm -f $(MODVERDIR)/*)
-
# read saved command lines for existing targets
existing-targets := $(wildcard $(sort $(targets)))
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 4ac4ca21a30a..cde5675340ba 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -353,23 +353,13 @@ config DEBUG_SECTION_MISMATCH
which results in the code/data being placed in specific sections.
The section mismatch analysis is always performed after a full
kernel build, and enabling this option causes the following
- additional steps to occur:
+ additional step to occur:
- Add the option -fno-inline-functions-called-once to gcc commands.
When inlining a function annotated with __init in a non-init
function, we would lose the section information and thus
the analysis would not catch the illegal reference.
This option tells gcc to inline less (but it does result in
a larger kernel).
- - Run the section mismatch analysis for each module/built-in.a file.
- When we run the section mismatch analysis on vmlinux.o, we
- lose valuable information about where the mismatch was
- introduced.
- Running the analysis for each module/built-in.a file
- tells where the mismatch happens much closer to the
- source. The drawback is that the same mismatch is
- reported at least twice.
- - Enable verbose reporting from modpost in order to help resolve
- the section mismatches that are reported.
config SECTION_MISMATCH_WARN_ONLY
bool "Make section mismatch errors non-fatal"
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index be32a3752de4..c6dfcc028f56 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -67,8 +67,6 @@ ifeq ($(CONFIG_MODULES)$(need-modorder),y1)
modorder-target := $(obj)/modules.order
endif
-# We keep a list of all modules in $(MODVERDIR)
-
__build: $(if $(KBUILD_BUILTIN),$(builtin-target) $(lib-target) $(extra-y)) \
$(if $(KBUILD_MODULES),$(obj-m) $(modorder-target)) \
$(subdir-ym) $(always)
@@ -87,11 +85,6 @@ ifneq ($(KBUILD_ENABLE_EXTRA_GCC_CHECKS),)
cmd_checkdoc = $(srctree)/scripts/kernel-doc -none $<
endif
-# Do section mismatch analysis for each module/built-in.a
-ifdef CONFIG_DEBUG_SECTION_MISMATCH
- cmd_secanalysis = ; scripts/mod/modpost $@
-endif
-
# Compile C sources (.c)
# ---------------------------------------------------------------------------
@@ -278,13 +271,11 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
$(call cmd,force_checksrc)
$(call if_changed_rule,cc_o_c)
-# Single-part modules are special since we need to mark them in $(MODVERDIR)
-
$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
$(call cmd,force_checksrc)
$(call if_changed_rule,cc_o_c)
@{ echo $(@:.o=.ko); echo $@; \
- $(cmd_undef_syms); } > $(MODVERDIR)/$(@F:.o=.mod)
+ $(cmd_undef_syms); } > $(patsubst %.o,%.mod,$@)
quiet_cmd_cc_lst_c = MKLST $@
cmd_cc_lst_c = $(CC) $(c_flags) -g -c -o $*.o $< && \
@@ -461,12 +452,12 @@ endif
# module is turned into a multi object module, $^ will contain header file
# dependencies recorded in the .*.cmd file.
quiet_cmd_link_multi-m = LD [M] $@
-cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ $(filter %.o,$^) $(cmd_secanalysis)
+ cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ $(filter %.o,$^)
$(multi-used-m): FORCE
$(call if_changed,link_multi-m)
@{ echo $(@:.o=.ko); echo $(filter %.o,$^); \
- $(cmd_undef_syms); } > $(MODVERDIR)/$(@F:.o=.mod)
+ $(cmd_undef_syms); } > $(patsubst %.o,%.mod,$@)
$(call multi_depend, $(multi-used-m), .o, -objs -y -m)
targets += $(multi-used-m)
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 5841508ffca9..6b19c1a4eae5 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -6,8 +6,8 @@
# Stage one of module building created the following:
# a) The individual .o files used for the module
# b) A <module>.o file which is the .o files above linked together
-# c) A <module>.mod file in $(MODVERDIR)/, listing the name of the
-# the preliminary <module>.o file, plus all .o files
+# c) A <module>.mod file, listing the name of the preliminary <module>.o file,
+# plus all .o files
# d) modules.order, which lists all the modules
# Stage 2 is handled by this file and does the following
diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
index aab4e299d7a2..2e4a7320bfb4 100755
--- a/scripts/adjust_autoksyms.sh
+++ b/scripts/adjust_autoksyms.sh
@@ -8,8 +8,7 @@
#
# Create/update the include/generated/autoksyms.h file from the list
-# of all module's needed symbols as recorded on the third line of
-# .tmp_versions/*.mod files.
+# of all module's needed symbols as recorded on the third line of *.mod files.
#
# For each symbol being added or removed, the corresponding dependency
# file's timestamp is updated to force a rebuild of the affected source
@@ -47,13 +46,10 @@ cat > "$new_ksyms_file" << EOT
*/
EOT
-[ "$(ls -A "$MODVERDIR")" ] &&
-for mod in "$MODVERDIR"/*.mod; do
- sed -n -e '3{s/ /\n/g;/^$/!p;}' "$mod"
-done | sort -u |
-while read sym; do
- echo "#define __KSYM_${sym} 1"
-done >> "$new_ksyms_file"
+sed 's/ko$/mod/' modules.order |
+xargs -n1 sed -n -e '3{s/ /\n/g;/^$/!p;}' -- |
+sort -u |
+sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$new_ksyms_file"
# Special case for modversions (see modpost.c)
if [ -n "$CONFIG_MODVERSIONS" ]; then
diff --git a/scripts/mod/sumversion.c b/scripts/mod/sumversion.c
index 0f6dcb4011a8..166f3fa247a9 100644
--- a/scripts/mod/sumversion.c
+++ b/scripts/mod/sumversion.c
@@ -396,21 +396,11 @@ void get_src_version(const char *modname, char sum[], unsigned sumlen)
unsigned long len;
struct md4_ctx md;
char *sources, *end, *fname;
- const char *basename;
char filelist[PATH_MAX + 1];
- char *modverdir = getenv("MODVERDIR");
- if (!modverdir)
- modverdir = ".";
-
- /* Source files for module are in .tmp_versions/modname.mod,
- after the first line. */
- if (strrchr(modname, '/'))
- basename = strrchr(modname, '/') + 1;
- else
- basename = modname;
- snprintf(filelist, sizeof(filelist), "%s/%.*s.mod", modverdir,
- (int) strlen(basename) - 2, basename);
+ /* objects for a module are listed in the second line of *.mod file. */
+ snprintf(filelist, sizeof(filelist), "%.*smod",
+ (int)strlen(modname) - 1, modname);
file = grab_file(filelist, &len);
if (!file)
diff --git a/scripts/package/mkspec b/scripts/package/mkspec
index 2d29df4a0a53..8640c278f1aa 100755
--- a/scripts/package/mkspec
+++ b/scripts/package/mkspec
@@ -29,7 +29,7 @@ fi
PROVIDES="$PROVIDES kernel-$KERNELRELEASE"
__KERNELRELEASE=$(echo $KERNELRELEASE | sed -e "s/-/_/g")
-EXCLUDES="$RCS_TAR_IGNORE --exclude=.tmp_versions --exclude=*vmlinux* \
+EXCLUDES="$RCS_TAR_IGNORE --exclude=*vmlinux* --exclude=*.mod \
--exclude=*.o --exclude=*.ko --exclude=*.cmd --exclude=Documentation \
--exclude=.config.old --exclude=.missing-syscalls.d --exclude=*.s"
diff --git a/tools/power/cpupower/debug/kernel/Makefile b/tools/power/cpupower/debug/kernel/Makefile
index c23e5a6ceb7e..7b5c43684be1 100644
--- a/tools/power/cpupower/debug/kernel/Makefile
+++ b/tools/power/cpupower/debug/kernel/Makefile
@@ -12,8 +12,8 @@ default:
$(MAKE) -C $(KDIR) M=$(CURDIR)
clean:
- - rm -rf *.o *.ko .tmp-versions .*.cmd .*.mod.* *.mod.c
- - rm -rf .tmp_versions* Module.symvers modules.order
+ - rm -rf *.o *.ko .*.cmd .*.mod.* *.mod.c
+ - rm -rf Module.symvers modules.order
install: default
install -d $(KMISC)
--
2.17.1
^ permalink raw reply related
* [PATCH v3 00/12] kbuild: create *.mod with directory path and remove MODVERDIR
From: Masahiro Yamada @ 2019-07-17 6:17 UTC (permalink / raw)
To: linux-kbuild
Cc: Joe Lawrence, Masahiro Yamada, James E.J. Bottomley,
Jonathan Corbet, Martin K. Petersen, Michal Marek, Shuah Khan,
Thomas Renninger, linux-doc, linux-kernel, linux-pm, linux-scsi
This series kills the long standing MODVERDIR.
Since MODVERDIR has a flat structure, it cannot avoid a race
condition when somebody introduces a module name conflict.
Kbuild now reads modules.order to get the list of all modules.
The post-processing/installation stages will be more robust
and simpler.
Masahiro Yamada (12):
kbuild: do not create empty modules.order in the prepare stage
kbuild: get rid of kernel/ prefix from in-tree modules.{order,builtin}
kbuild: remove duplication from modules.order in sub-directories
scsi: remove pointless $(MODVERDIR)/$(obj)/53c700.ver
kbuild: modinst: read modules.order instead of $(MODVERDIR)/*.mod
kbuild: modsign: read modules.order instead of $(MODVERDIR)/*.mod
kbuild: modpost: read modules.order instead of $(MODVERDIR)/*.mod
kbuild: export_report: read modules.order instead of
.tmp_versions/*.mod
kbuild: create *.mod with full directory path and remove MODVERDIR
kbuild: remove the first line of *.mod files
kbuild: remove 'prepare1' target
kbuild: split out *.mod out of {single,multi}-used-m rules
.gitignore | 1 +
Documentation/dontdiff | 1 +
Makefile | 36 ++++++-------------
drivers/scsi/Makefile | 2 +-
lib/Kconfig.debug | 12 +------
scripts/Makefile.build | 40 +++++++++-------------
scripts/Makefile.modbuiltin | 2 +-
scripts/Makefile.modinst | 5 +--
scripts/Makefile.modpost | 19 +++++-----
scripts/Makefile.modsign | 3 +-
scripts/adjust_autoksyms.sh | 14 +++-----
scripts/export_report.pl | 11 +++---
scripts/mod/sumversion.c | 23 +++----------
scripts/modules-check.sh | 2 +-
scripts/package/mkspec | 2 +-
tools/power/cpupower/debug/kernel/Makefile | 4 +--
16 files changed, 62 insertions(+), 115 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH RFC v2 2/2] cpufreq: mediatek: Support vproc shared by multiple component
From: Hsin-Yi Wang @ 2019-07-17 6:11 UTC (permalink / raw)
To: linux-arm-kernel
Cc: MyungJoo Ham ), Kyungmin Park, Chanwoo Choi, Matthias Brugger,
Rafael J . Wysocki, Viresh Kumar, linux-pm, linux-mediatek,
linux-kernel
In-Reply-To: <20190717061124.453-1-hsinyi@chromium.org>
mt8183-cci shares vproc with small cluster. If the regulator is shared
between several devices then the lowest request voltage that meets the
system constraints will be used.
However, previous mediatek cpufreq implementation would cause race condition
if vproc is shared by multiple devices, which would crash device due to
incorrect voltage supply.
A race condition example:
cci sets vproc 90 --> vproc=90
cpu0 sets vproc 50 --> vproc=max(50,90)=90
cpu0 sets vproc 70 --> cpu0 reads vproc 90, target is lower, so decide to scale
up frequency first, but before it set voltage...
cci sets vproc 60 --> vproc=max(60,50)=60. cpu0 already set freq to 70, but
before it set voltage, vproc becomes 60, which is not
sufficient for cpu0.
Let cpu and cci manages their own previous target voltage can avoid such race.
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
drivers/cpufreq/mediatek-cpufreq.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
index 7282834e8fe2..f5e737b862f0 100644
--- a/drivers/cpufreq/mediatek-cpufreq.c
+++ b/drivers/cpufreq/mediatek-cpufreq.c
@@ -46,6 +46,7 @@ struct mtk_cpu_dvfs_info {
struct notifier_block opp_nb;
int opp_cpu;
unsigned long opp_freq;
+ int old_vproc;
};
static LIST_HEAD(dvfs_info_list);
@@ -196,11 +197,16 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
static int mtk_cpufreq_set_voltage(struct mtk_cpu_dvfs_info *info, int vproc)
{
+ int ret;
+
if (info->need_voltage_tracking)
- return mtk_cpufreq_voltage_tracking(info, vproc);
+ ret = mtk_cpufreq_voltage_tracking(info, vproc);
else
- return regulator_set_voltage(info->proc_reg, vproc,
- vproc + VOLT_TOL);
+ ret = regulator_set_voltage(info->proc_reg, vproc,
+ MAX_VOLT_LIMIT);
+ if (!ret)
+ info->old_vproc = vproc;
+ return ret;
}
static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
@@ -218,7 +224,9 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy,
inter_vproc = info->intermediate_voltage;
old_freq_hz = clk_get_rate(cpu_clk);
- old_vproc = regulator_get_voltage(info->proc_reg);
+ old_vproc = info->old_vproc;
+ if (old_vproc == 0)
+ old_vproc = regulator_get_voltage(info->proc_reg);
if (old_vproc < 0) {
pr_err("%s: invalid Vproc value: %d\n", __func__, old_vproc);
return old_vproc;
--
2.20.1
^ permalink raw reply related
* [PATCH RFC v2 1/2] devfreq: mt8183-cci: using cpu based scaling passive_governor
From: Hsin-Yi Wang @ 2019-07-17 6:11 UTC (permalink / raw)
To: linux-arm-kernel
Cc: MyungJoo Ham ), Kyungmin Park, Chanwoo Choi, Matthias Brugger,
Rafael J . Wysocki, Viresh Kumar, linux-pm, linux-mediatek,
linux-kernel
In-Reply-To: <20190717061124.453-1-hsinyi@chromium.org>
This is based on mediatek's devfreq patches[1].
In MT8183 SoC, CCI and little core cluster share same regulator. In original
implementation, CCI frequency depends on regulator voltage, which results in
bad memory access performance if tasks are loaded on other cpus other than
little cluster (cpus 0-3).
Using cpu based scaling devfreq passive governor can improve this situation,
since in most cases, higher cpufreq implies higher loadings on the CCI, and CCI
should consider all cpu loadings instead of only the little cluster.
[1] https://patchwork.kernel.org/patch/10946063/
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
drivers/devfreq/mt8183-cci-devfreq.c | 239 +++++++--------------------
1 file changed, 56 insertions(+), 183 deletions(-)
diff --git a/drivers/devfreq/mt8183-cci-devfreq.c b/drivers/devfreq/mt8183-cci-devfreq.c
index 250c963789f3..4e8e5948ed41 100644
--- a/drivers/devfreq/mt8183-cci-devfreq.c
+++ b/drivers/devfreq/mt8183-cci-devfreq.c
@@ -11,189 +11,82 @@
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/regulator/consumer.h>
+#include <linux/time.h>
#include "governor.h"
+#define MAX_VOLT_LIMIT (1150000)
+
struct cci_devfreq {
struct devfreq *devfreq;
struct regulator *proc_reg;
- unsigned long proc_reg_uV;
struct clk *cci_clk;
- unsigned long freq;
- struct notifier_block nb;
- struct notifier_block opp_nb;
- int cci_min_freq;
+ int old_vproc;
+ unsigned long old_freq;
};
-static int cci_devfreq_regulator_notifier(struct notifier_block *nb,
- unsigned long val, void *data)
-{
- int ret;
- struct cci_devfreq *cci_df =
- container_of(nb, struct cci_devfreq, nb);
-
- /* deal with reduce frequency */
- if (val & REGULATOR_EVENT_PRE_VOLTAGE_CHANGE) {
- struct pre_voltage_change_data *pvc_data = data;
-
- if (pvc_data->min_uV < pvc_data->old_uV) {
- cci_df->proc_reg_uV =
- (unsigned long)(pvc_data->min_uV);
- mutex_lock(&cci_df->devfreq->lock);
- ret = update_devfreq(cci_df->devfreq);
- if (ret)
- pr_err("Fail to reduce cci frequency: %d\n",
- ret);
- mutex_unlock(&cci_df->devfreq->lock);
- }
- } else if ((val & REGULATOR_EVENT_ABORT_VOLTAGE_CHANGE) &&
- ((unsigned long)data > cci_df->proc_reg_uV)) {
- cci_df->proc_reg_uV = (unsigned long)data;
- mutex_lock(&cci_df->devfreq->lock);
- ret = update_devfreq(cci_df->devfreq);
- if (ret)
- pr_err("Fail to raise cci frequency back: %d\n", ret);
- mutex_unlock(&cci_df->devfreq->lock);
- } else if ((val & REGULATOR_EVENT_VOLTAGE_CHANGE) &&
- (cci_df->proc_reg_uV < (unsigned long)data)) {
- /* deal with increase frequency */
- cci_df->proc_reg_uV = (unsigned long)data;
- mutex_lock(&cci_df->devfreq->lock);
- ret = update_devfreq(cci_df->devfreq);
- if (ret)
- pr_err("Fail to raise cci frequency: %d\n", ret);
- mutex_unlock(&cci_df->devfreq->lock);
- }
-
- return 0;
-}
-
-static int ccidevfreq_opp_notifier(struct notifier_block *nb,
-unsigned long event, void *data)
-{
- int ret;
- struct dev_pm_opp *opp = data;
- struct cci_devfreq *cci_df = container_of(nb, struct cci_devfreq,
- opp_nb);
- unsigned long freq, volt, cur_volt;
-
- if (event == OPP_EVENT_ADJUST_VOLTAGE) {
- freq = dev_pm_opp_get_freq(opp);
- /* current opp item is changed */
- if (freq == cci_df->freq) {
- volt = dev_pm_opp_get_voltage(opp);
- cur_volt = regulator_get_voltage(cci_df->proc_reg);
-
- if (volt > cur_volt) {
- /* need reduce freq */
- mutex_lock(&cci_df->devfreq->lock);
- ret = update_devfreq(cci_df->devfreq);
- if (ret)
- pr_err("Fail to reduce cci frequency by opp notification: %d\n",
- ret);
- mutex_unlock(&cci_df->devfreq->lock);
- }
- }
-
- if (freq == cci_df->cci_min_freq) {
- volt = dev_pm_opp_get_voltage(opp);
- regulator_set_voltage(cci_df->proc_reg, volt, INT_MAX);
- }
- } else if (event == OPP_EVENT_DISABLE) {
- }
-
- return 0;
-}
-
-
-static int mtk_cci_governor_get_target(struct devfreq *devfreq,
- unsigned long *freq)
+static int mtk_cci_set_voltage(struct cci_devfreq *cci_df, int vproc)
{
- struct cci_devfreq *cci_df;
- struct dev_pm_opp *opp;
int ret;
- cci_df = dev_get_drvdata(devfreq->dev.parent);
-
- /* find available frequency */
- opp = dev_pm_opp_find_freq_ceil_by_volt(devfreq->dev.parent,
- cci_df->proc_reg_uV);
- ret = PTR_ERR_OR_ZERO(opp);
- if (ret) {
- pr_err("%s[%d], cannot find opp with voltage=%d: %d\n",
- __func__, __LINE__, cci_df->proc_reg_uV, ret);
- return ret;
- }
- *freq = dev_pm_opp_get_freq(opp);
-
- return 0;
-}
-
-static int mtk_cci_governor_event_handler(struct devfreq *devfreq,
- unsigned int event, void *data)
-{
- int ret;
- struct cci_devfreq *cci_df;
- struct notifier_block *nb;
- struct notifier_block *opp_nb;
-
- cci_df = dev_get_drvdata(devfreq->dev.parent);
- nb = &cci_df->nb;
- opp_nb = &cci_df->opp_nb;
-
- switch (event) {
- case DEVFREQ_GOV_START:
- case DEVFREQ_GOV_RESUME:
- nb->notifier_call = cci_devfreq_regulator_notifier;
- ret = regulator_register_notifier(cci_df->proc_reg,
- nb);
- if (ret)
- pr_err("%s: failed to add governor: %d\n", __func__,
- ret);
- opp_nb->notifier_call = ccidevfreq_opp_notifier;
- dev_pm_opp_register_notifier(devfreq->dev.parent, opp_nb);
- break;
-
- case DEVFREQ_GOV_STOP:
- case DEVFREQ_GOV_SUSPEND:
- ret = regulator_unregister_notifier(cci_df->proc_reg,
- nb);
- if (ret)
- pr_err("%s: failed to add governor: %d\n", __func__,
- ret);
- break;
-
- default:
- break;
- }
-
- return 0;
+ ret = regulator_set_voltage(cci_df->proc_reg, vproc,
+ MAX_VOLT_LIMIT);
+ if (!ret)
+ cci_df->old_vproc = vproc;
+ return ret;
}
-static struct devfreq_governor mtk_cci_devfreq_governor = {
- .name = "mtk_cci_vmon",
- .get_target_freq = mtk_cci_governor_get_target,
- .event_handler = mtk_cci_governor_event_handler,
- .immutable = true
-};
-
static int mtk_cci_devfreq_target(struct device *dev, unsigned long *freq,
u32 flags)
{
int ret;
struct cci_devfreq *cci_df = dev_get_drvdata(dev);
+ struct dev_pm_opp *opp;
+ unsigned long opp_rate, opp_voltage, old_voltage;
if (!cci_df)
return -EINVAL;
+ if (cci_df->old_freq == *freq)
+ return 0;
+
+ opp_rate = *freq;
+ opp = dev_pm_opp_find_freq_floor(dev, &opp_rate);
+ opp_voltage = dev_pm_opp_get_voltage(opp);
+ dev_pm_opp_put(opp);
+
+ old_voltage = cci_df->old_vproc;
+ if (old_voltage == 0)
+ old_voltage = regulator_get_voltage(cci_df->proc_reg);
+
+ // scale up: set voltage first then freq
+ if (opp_voltage > old_voltage) {
+ ret = mtk_cci_set_voltage(cci_df, opp_voltage);
+ if (ret) {
+ pr_err("cci: failed to scale up voltage\n");
+ return ret;
+ }
+ }
+
ret = clk_set_rate(cci_df->cci_clk, *freq);
if (ret) {
pr_err("%s: failed cci to set rate: %d\n", __func__,
ret);
+ mtk_cci_set_voltage(cci_df, old_voltage);
return ret;
}
- cci_df->freq = *freq;
+ // scale down: set freq first then voltage
+ if (opp_voltage < old_voltage) {
+ ret = mtk_cci_set_voltage(cci_df, opp_voltage);
+ if (ret) {
+ pr_err("cci: failed to scale down voltage\n");
+ clk_set_rate(cci_df->cci_clk, cci_df->old_freq);
+ return ret;
+ }
+ }
+
+ cci_df->old_freq = *freq;
return 0;
}
@@ -206,8 +99,7 @@ static int mtk_cci_devfreq_probe(struct platform_device *pdev)
{
struct device *cci_dev = &pdev->dev;
struct cci_devfreq *cci_df;
- unsigned long freq, volt;
- struct dev_pm_opp *opp;
+ struct devfreq_passive_data *passive_data;
int ret;
cci_df = devm_kzalloc(cci_dev, sizeof(*cci_df), GFP_KERNEL);
@@ -237,19 +129,18 @@ static int mtk_cci_devfreq_probe(struct platform_device *pdev)
return ret;
}
- /* set voltage lower bound */
- freq = 1;
- opp = dev_pm_opp_find_freq_ceil(cci_dev, &freq);
- cci_df->cci_min_freq = dev_pm_opp_get_freq(opp);
- volt = dev_pm_opp_get_voltage(opp);
- dev_pm_opp_put(opp);
-
platform_set_drvdata(pdev, cci_df);
+ passive_data = devm_kzalloc(cci_dev, sizeof(*passive_data), GFP_KERNEL);
+ if (!passive_data)
+ return -ENOMEM;
+
+ passive_data->cpufreq_type = true;
+
cci_df->devfreq = devm_devfreq_add_device(cci_dev,
&cci_devfreq_profile,
- "mtk_cci_vmon",
- NULL);
+ DEVFREQ_GOV_PASSIVE,
+ passive_data);
if (IS_ERR(cci_df->devfreq)) {
ret = PTR_ERR(cci_df->devfreq);
dev_err(cci_dev, "cannot create cci devfreq device:%d\n", ret);
@@ -277,30 +168,12 @@ static struct platform_driver cci_devfreq_driver = {
static int __init mtk_cci_devfreq_init(void)
{
- int ret;
-
- ret = devfreq_add_governor(&mtk_cci_devfreq_governor);
- if (ret) {
- pr_err("%s: failed to add governor: %d\n", __func__, ret);
- return ret;
- }
-
- ret = platform_driver_register(&cci_devfreq_driver);
- if (ret)
- devfreq_remove_governor(&mtk_cci_devfreq_governor);
-
- return ret;
+ return platform_driver_register(&cci_devfreq_driver);
}
module_init(mtk_cci_devfreq_init)
static void __exit mtk_cci_devfreq_exit(void)
{
- int ret;
-
- ret = devfreq_remove_governor(&mtk_cci_devfreq_governor);
- if (ret)
- pr_err("%s: failed to remove governor: %d\n", __func__, ret);
-
platform_driver_unregister(&cci_devfreq_driver);
}
module_exit(mtk_cci_devfreq_exit)
--
2.20.1
^ permalink raw reply related
* [PATCH RFC v2 0/2] Use cpu based scaling passive governor for MT8183 CCI
From: Hsin-Yi Wang @ 2019-07-17 6:11 UTC (permalink / raw)
To: linux-arm-kernel
Cc: MyungJoo Ham ), Kyungmin Park, Chanwoo Choi, Matthias Brugger,
Rafael J . Wysocki, Viresh Kumar, linux-pm, linux-mediatek,
linux-kernel
This series depends on following series:
1. mt8183 cpufreq and cci devfreq from andrew-sh.cheng
https://patchwork.kernel.org/cover/10946047/
2. cpu based scaling support to passive_governor from Sibi Sankar
https://lore.kernel.org/patchwork/patch/1101049/
This series uses cpu based scaling passive governor for mt8183-cci to improve
performance and deal with shared regulator voltage setting issue.
Hsin-Yi Wang (2):
devfreq: mt8183-cci: using cpu based scaling passive_governor
cpufreq: mediatek: Support vproc shared by multiple component
drivers/cpufreq/mediatek-cpufreq.c | 16 +-
drivers/devfreq/mt8183-cci-devfreq.c | 239 +++++++--------------------
2 files changed, 68 insertions(+), 187 deletions(-)
--
2.20.1
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox