* [PATCH 3/6] soc: renesas: rcar-sysc: Provide helpers to power up/down CPUs
From: Geert Uytterhoeven @ 2018-05-30 15:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527693916-11215-1-git-send-email-geert+renesas@glider.be>
Provide helpers to control CPU power areas from platform code, taking
just a CPU index. This will avoid having to pass full CPU power area
parameter blocks, and thus duplicating information already provided by
SoC-specific SYSC drivers.
This will be used on R-Car H1 only.
Later R-Car generations rely on APMU/RST for CPU power area control.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/soc/renesas/rcar-sysc.c | 40 +++++++++++++++++++++++++++++++++++
include/linux/soc/renesas/rcar-sysc.h | 2 ++
2 files changed, 42 insertions(+)
diff --git a/drivers/soc/renesas/rcar-sysc.c b/drivers/soc/renesas/rcar-sysc.c
index 95120acc4d806da6..4ad6dcd19420a045 100644
--- a/drivers/soc/renesas/rcar-sysc.c
+++ b/drivers/soc/renesas/rcar-sysc.c
@@ -310,6 +310,8 @@ struct rcar_pm_domains {
struct generic_pm_domain *domains[RCAR_PD_ALWAYS_ON + 1];
};
+static struct genpd_onecell_data *rcar_sysc_onecell_data;
+
static int __init rcar_sysc_pd_init(void)
{
const struct rcar_sysc_info *info;
@@ -356,6 +358,7 @@ static int __init rcar_sysc_pd_init(void)
domains->onecell_data.domains = domains->domains;
domains->onecell_data.num_domains = ARRAY_SIZE(domains->domains);
+ rcar_sysc_onecell_data = &domains->onecell_data;
for (i = 0, syscier = 0; i < info->num_areas; i++)
syscier |= BIT(info->areas[i].isr_bit);
@@ -449,3 +452,40 @@ void __init rcar_sysc_init(phys_addr_t base, u32 syscier)
pr_debug("%s: syscier = 0x%08x\n", __func__, syscier);
iowrite32(syscier, rcar_sysc_base + SYSCIER);
}
+
+#ifdef CONFIG_ARCH_R8A7779
+static int rcar_sysc_power_cpu(unsigned int idx, bool on)
+{
+ struct generic_pm_domain *genpd;
+ struct rcar_sysc_pd *pd;
+ unsigned int i;
+
+ if (!rcar_sysc_onecell_data)
+ return -ENODEV;
+
+ for (i = 0; i < rcar_sysc_onecell_data->num_domains; i++) {
+ genpd = rcar_sysc_onecell_data->domains[i];
+ if (!genpd)
+ continue;
+
+ pd = to_rcar_pd(genpd);
+ if (!(pd->flags & PD_CPU) || pd->ch.chan_bit != idx)
+ continue;
+
+ return on ? rcar_sysc_power_up(&pd->ch)
+ : rcar_sysc_power_down(&pd->ch);
+ }
+
+ return -ENOENT;
+}
+
+int rcar_sysc_power_down_cpu(unsigned int cpu)
+{
+ return rcar_sysc_power_cpu(cpu, false);
+}
+
+int rcar_sysc_power_up_cpu(unsigned int cpu)
+{
+ return rcar_sysc_power_cpu(cpu, true);
+}
+#endif /* CONFIG_ARCH_R8A7779 */
diff --git a/include/linux/soc/renesas/rcar-sysc.h b/include/linux/soc/renesas/rcar-sysc.h
index 8a6086d2e9c37585..9020da2111fdeea9 100644
--- a/include/linux/soc/renesas/rcar-sysc.h
+++ b/include/linux/soc/renesas/rcar-sysc.h
@@ -13,5 +13,7 @@ struct rcar_sysc_ch {
int rcar_sysc_power_down(const struct rcar_sysc_ch *sysc_ch);
int rcar_sysc_power_up(const struct rcar_sysc_ch *sysc_ch);
void rcar_sysc_init(phys_addr_t base, u32 syscier);
+int rcar_sysc_power_down_cpu(unsigned int cpu);
+int rcar_sysc_power_up_cpu(unsigned int cpu);
#endif /* __LINUX_SOC_RENESAS_RCAR_SYSC_H__ */
--
2.7.4
^ permalink raw reply related
* [PATCH 4/6] ARM: shmobile: r8a7779: Use rcar_sysc_power_{down, up}_cpu()
From: Geert Uytterhoeven @ 2018-05-30 15:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527693916-11215-1-git-send-email-geert+renesas@glider.be>
The r8a7779 SMP code calls rcar_sysc_power_{down,up}() to control power
to the SYSC power areas containing CPUs. This requires passing full CPU
power area parameter blocks.
Migrate the code to call the new rcar_sysc_power_{down,up}_cpu()
helpers, which just take a CPU index, and use the SYSC power area
definitions from the r8a7779-sysc driver.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
arch/arm/mach-shmobile/smp-r8a7779.c | 47 +++++-------------------------------
1 file changed, 6 insertions(+), 41 deletions(-)
diff --git a/arch/arm/mach-shmobile/smp-r8a7779.c b/arch/arm/mach-shmobile/smp-r8a7779.c
index ff1e6fc0b861c4fc..3036f910046118ee 100644
--- a/arch/arm/mach-shmobile/smp-r8a7779.c
+++ b/arch/arm/mach-shmobile/smp-r8a7779.c
@@ -31,59 +31,24 @@
#define AVECR IOMEM(0xfe700040)
#define R8A7779_SCU_BASE 0xf0000000
-static const struct rcar_sysc_ch r8a7779_ch_cpu1 = {
- .chan_offs = 0x40, /* PWRSR0 .. PWRER0 */
- .chan_bit = 1, /* ARM1 */
- .isr_bit = 1, /* ARM1 */
-};
-
-static const struct rcar_sysc_ch r8a7779_ch_cpu2 = {
- .chan_offs = 0x40, /* PWRSR0 .. PWRER0 */
- .chan_bit = 2, /* ARM2 */
- .isr_bit = 2, /* ARM2 */
-};
-
-static const struct rcar_sysc_ch r8a7779_ch_cpu3 = {
- .chan_offs = 0x40, /* PWRSR0 .. PWRER0 */
- .chan_bit = 3, /* ARM3 */
- .isr_bit = 3, /* ARM3 */
-};
-
-static const struct rcar_sysc_ch * const r8a7779_ch_cpu[4] = {
- [1] = &r8a7779_ch_cpu1,
- [2] = &r8a7779_ch_cpu2,
- [3] = &r8a7779_ch_cpu3,
-};
-
static int r8a7779_platform_cpu_kill(unsigned int cpu)
{
- const struct rcar_sysc_ch *ch = NULL;
int ret = -EIO;
cpu = cpu_logical_map(cpu);
-
- if (cpu < ARRAY_SIZE(r8a7779_ch_cpu))
- ch = r8a7779_ch_cpu[cpu];
-
- if (ch)
- ret = rcar_sysc_power_down(ch);
+ if (cpu)
+ ret = rcar_sysc_power_down_cpu(cpu);
return ret ? ret : 1;
}
static int r8a7779_boot_secondary(unsigned int cpu, struct task_struct *idle)
{
- const struct rcar_sysc_ch *ch = NULL;
- unsigned int lcpu = cpu_logical_map(cpu);
- int ret;
-
- if (lcpu < ARRAY_SIZE(r8a7779_ch_cpu))
- ch = r8a7779_ch_cpu[lcpu];
+ int ret = -EIO;
- if (ch)
- ret = rcar_sysc_power_up(ch);
- else
- ret = -EIO;
+ cpu = cpu_logical_map(cpu);
+ if (cpu)
+ ret = rcar_sysc_power_up_cpu(cpu);
return ret;
}
--
2.7.4
^ permalink raw reply related
* [PATCH 5/6] ARM: shmobile: r8a7779: Remove explicit SYSC config and init
From: Geert Uytterhoeven @ 2018-05-30 15:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527693916-11215-1-git-send-email-geert+renesas@glider.be>
If the R-Car H1 system controller is described in DT, the rcar-sysc
driver configures SYSCIER and SYSCIMR based on the SoC-specific power
area definitions in r8a7779-sysc. The platform code still passed this
information to the rcar-sysc driver, for compatibility with old DTBs
predating commit b2df3aa487395a1b ("ARM: dts: r8a7779: Add SYSC PM
Domains") in v4.7. The time has come to drop backwards compatibility,
and delegate everything to the DT enabled rcar-sysc driver.
After stopping powering down secondary CPUs during early boot, there is
no longer a need to force an early initialization of the rcar-sysc
driver. It will be initialized in time for secondary CPU bringup by its
early_initcall().
Hence all explicit SYSC configuration and initialization can be removed
from the R-Car H1 platform code.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
-------------------------------------------------------
---
arch/arm/mach-shmobile/Makefile | 2 +-
arch/arm/mach-shmobile/pm-r8a7779.c | 41 ------------------------------------
arch/arm/mach-shmobile/r8a7779.h | 2 --
arch/arm/mach-shmobile/smp-r8a7779.c | 2 --
4 files changed, 1 insertion(+), 46 deletions(-)
delete mode 100644 arch/arm/mach-shmobile/pm-r8a7779.c
diff --git a/arch/arm/mach-shmobile/Makefile b/arch/arm/mach-shmobile/Makefile
index 2b8d3896e1ebbe6a..05ba728ed4f67cf2 100644
--- a/arch/arm/mach-shmobile/Makefile
+++ b/arch/arm/mach-shmobile/Makefile
@@ -11,7 +11,7 @@ obj-$(CONFIG_ARCH_SH73A0) += setup-sh73a0.o
obj-$(CONFIG_ARCH_R8A73A4) += setup-r8a73a4.o
obj-$(CONFIG_ARCH_R8A7740) += setup-r8a7740.o
obj-$(CONFIG_ARCH_R8A7778) += setup-r8a7778.o
-obj-$(CONFIG_ARCH_R8A7779) += setup-r8a7779.o pm-r8a7779.o
+obj-$(CONFIG_ARCH_R8A7779) += setup-r8a7779.o
obj-$(CONFIG_ARCH_EMEV2) += setup-emev2.o
obj-$(CONFIG_ARCH_R7S72100) += setup-r7s72100.o
diff --git a/arch/arm/mach-shmobile/pm-r8a7779.c b/arch/arm/mach-shmobile/pm-r8a7779.c
deleted file mode 100644
index 5c9a93f5e650181a..0000000000000000
--- a/arch/arm/mach-shmobile/pm-r8a7779.c
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * r8a7779 Power management support
- *
- * Copyright (C) 2011 Renesas Solutions Corp.
- * Copyright (C) 2011 Magnus Damm
- *
- * This file is subject to the terms and conditions of the GNU General Public
- * License. See the file "COPYING" in the main directory of this archive
- * for more details.
- */
-
-#include <linux/soc/renesas/rcar-sysc.h>
-
-#include <asm/io.h>
-
-#include "r8a7779.h"
-
-/* SYSC */
-#define SYSCIER 0x0c
-#define SYSCIMR 0x10
-
-#if defined(CONFIG_PM) || defined(CONFIG_SMP)
-
-static void __init r8a7779_sysc_init(void)
-{
- rcar_sysc_init(0xffd85000, 0x0131000e);
-}
-
-#else /* CONFIG_PM || CONFIG_SMP */
-
-static inline void r8a7779_sysc_init(void) {}
-
-#endif /* CONFIG_PM || CONFIG_SMP */
-
-void __init r8a7779_pm_init(void)
-{
- static int once;
-
- if (!once++)
- r8a7779_sysc_init();
-}
diff --git a/arch/arm/mach-shmobile/r8a7779.h b/arch/arm/mach-shmobile/r8a7779.h
index 30668aa6acc34647..ca9db8fde2f791cc 100644
--- a/arch/arm/mach-shmobile/r8a7779.h
+++ b/arch/arm/mach-shmobile/r8a7779.h
@@ -2,8 +2,6 @@
#ifndef __ASM_R8A7779_H__
#define __ASM_R8A7779_H__
-extern void r8a7779_pm_init(void);
-
extern const struct smp_operations r8a7779_smp_ops;
#endif /* __ASM_R8A7779_H__ */
diff --git a/arch/arm/mach-shmobile/smp-r8a7779.c b/arch/arm/mach-shmobile/smp-r8a7779.c
index 3036f910046118ee..0d3ec5c2b8fcbe7c 100644
--- a/arch/arm/mach-shmobile/smp-r8a7779.c
+++ b/arch/arm/mach-shmobile/smp-r8a7779.c
@@ -60,8 +60,6 @@ static void __init r8a7779_smp_prepare_cpus(unsigned int max_cpus)
/* setup r8a7779 specific SCU bits */
shmobile_smp_scu_prepare_cpus(R8A7779_SCU_BASE, max_cpus);
-
- r8a7779_pm_init();
}
#ifdef CONFIG_HOTPLUG_CPU
--
2.7.4
^ permalink raw reply related
* [PATCH 6/6] soc: renesas: rcar-sysc: Drop legacy handling
From: Geert Uytterhoeven @ 2018-05-30 15:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527693916-11215-1-git-send-email-geert+renesas@glider.be>
Now the R-Car platform code no longer supports DTBs lacking a SYSC
device node in DT, all legacy handling can be dropped from the R-Car
SYSC driver:
- Make rcar_sysc_ch private to the driver,
- Make rcar_sysc_power_{down,up}() static (they have been replaced by
rcar_sysc_power_{down,up}_cpu()),
- Remove the legacy wrapper rcar_sysc_init(), and the check for double
initialization (only the early_initcall is left).
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/soc/renesas/rcar-sysc.c | 38 ++++++++---------------------------
include/linux/soc/renesas/rcar-sysc.h | 11 ----------
2 files changed, 8 insertions(+), 41 deletions(-)
diff --git a/drivers/soc/renesas/rcar-sysc.c b/drivers/soc/renesas/rcar-sysc.c
index 4ad6dcd19420a045..41af9c7b912f09ca 100644
--- a/drivers/soc/renesas/rcar-sysc.c
+++ b/drivers/soc/renesas/rcar-sysc.c
@@ -58,6 +58,12 @@
#define RCAR_PD_ALWAYS_ON 32 /* Always-on power area */
+struct rcar_sysc_ch {
+ u16 chan_offs;
+ u8 chan_bit;
+ u8 isr_bit;
+};
+
static void __iomem *rcar_sysc_base;
static DEFINE_SPINLOCK(rcar_sysc_lock); /* SMP CPUs + I/O devices */
@@ -143,12 +149,12 @@ static int rcar_sysc_power(const struct rcar_sysc_ch *sysc_ch, bool on)
return ret;
}
-int rcar_sysc_power_down(const struct rcar_sysc_ch *sysc_ch)
+static int rcar_sysc_power_down(const struct rcar_sysc_ch *sysc_ch)
{
return rcar_sysc_power(sysc_ch, false);
}
-int rcar_sysc_power_up(const struct rcar_sysc_ch *sysc_ch)
+static int rcar_sysc_power_up(const struct rcar_sysc_ch *sysc_ch)
{
return rcar_sysc_power(sysc_ch, true);
}
@@ -323,9 +329,6 @@ static int __init rcar_sysc_pd_init(void)
unsigned int i;
int error;
- if (rcar_sysc_base)
- return 0;
-
np = of_find_matching_node_and_match(NULL, rcar_sysc_matches, &match);
if (!np)
return -ENODEV;
@@ -428,31 +431,6 @@ void __init rcar_sysc_nullify(struct rcar_sysc_area *areas,
}
}
-void __init rcar_sysc_init(phys_addr_t base, u32 syscier)
-{
- u32 syscimr;
-
- if (!rcar_sysc_pd_init())
- return;
-
- rcar_sysc_base = ioremap_nocache(base, PAGE_SIZE);
-
- /*
- * Mask all interrupt sources to prevent the CPU from receiving them.
- * Make sure not to clear reserved bits that were set before.
- */
- syscimr = ioread32(rcar_sysc_base + SYSCIMR);
- syscimr |= syscier;
- pr_debug("%s: syscimr = 0x%08x\n", __func__, syscimr);
- iowrite32(syscimr, rcar_sysc_base + SYSCIMR);
-
- /*
- * SYSC needs all interrupt sources enabled to control power.
- */
- pr_debug("%s: syscier = 0x%08x\n", __func__, syscier);
- iowrite32(syscier, rcar_sysc_base + SYSCIER);
-}
-
#ifdef CONFIG_ARCH_R8A7779
static int rcar_sysc_power_cpu(unsigned int idx, bool on)
{
diff --git a/include/linux/soc/renesas/rcar-sysc.h b/include/linux/soc/renesas/rcar-sysc.h
index 9020da2111fdeea9..00fae6fd234d7895 100644
--- a/include/linux/soc/renesas/rcar-sysc.h
+++ b/include/linux/soc/renesas/rcar-sysc.h
@@ -2,17 +2,6 @@
#ifndef __LINUX_SOC_RENESAS_RCAR_SYSC_H__
#define __LINUX_SOC_RENESAS_RCAR_SYSC_H__
-#include <linux/types.h>
-
-struct rcar_sysc_ch {
- u16 chan_offs;
- u8 chan_bit;
- u8 isr_bit;
-};
-
-int rcar_sysc_power_down(const struct rcar_sysc_ch *sysc_ch);
-int rcar_sysc_power_up(const struct rcar_sysc_ch *sysc_ch);
-void rcar_sysc_init(phys_addr_t base, u32 syscier);
int rcar_sysc_power_down_cpu(unsigned int cpu);
int rcar_sysc_power_up_cpu(unsigned int cpu);
--
2.7.4
^ permalink raw reply related
* [PATCH v2 1/6] ARM: dra762: hwmod: Add MCAN support
From: Tony Lindgren @ 2018-05-30 15:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cfa9c9df-57a2-69f6-9509-440dc18292e7@ti.com>
* Tero Kristo <t-kristo@ti.com> [180530 15:18]:
> For the OCP if part, I think that is still needed until we switch over to
> full sysc driver. clkctrl_offs you probably also need because that is used
> for mapping the omap_hwmod against a specific clkctrl clock. Those can be
> only removed once we are done with hwmod (or figure out some other way to
> assign the clkctrl clock to a hwmod.)
Hmm might be worth testing. I thought your commit 70f05be32133
("ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available")
already parses the clkctrl from dts?
Regards,
Tony
^ permalink raw reply
* [PATCH] PCI: Add pci=safemode option
From: Sinan Kaya @ 2018-05-30 15:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180530145650.GA39853@bhelgaas-glaptop.roam.corp.google.com>
On 5/30/2018 10:56 AM, Bjorn Helgaas wrote:
> This *was* my idea, but I'm starting to think it was a bad idea.
>
> I don't want people to use pci= parameters as the normal way to get a
> system to boot. That would be a huge support hassle (putting things
> in release notes, diagnosing problems when people forget it, etc).
>
> But the parameters *are* useful for debugging. If we had a
> "pci=safemode" and it avoided some problem, the next step would be to
> narrow it down by using the more specific flags (pci=nomsi, pci=noari,
> pci=no_ext_tags, etc). So I think 95% of the value is in the specific
> flags, and a "pci=safemode" might add a little bit of value but at the
> cost of a small but nagging maintenance concern and code clutter.
OK. Let's try noXYZ feature. Can you enumerate the XYZ features that you
want to see?
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Doug Anderson @ 2018-05-30 15:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180530150241.GO6920@sirena.org.uk>
Hi,
On Wed, May 30, 2018 at 8:02 AM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, May 30, 2018 at 07:46:50AM -0700, Doug Anderson wrote:
>> On Wed, May 30, 2018 at 2:37 AM, Mark Brown <broonie@kernel.org> wrote:
>
>> >> Linux vote for the lowest voltage it's comfortable with. Linux keeps
>> >> track of the true voltage that the driver wants and will always change
>> >> its vote back to that before enabling. Thus (assuming Linux is OK
>> >> with 1.2 V - 1.4 V for a rail):
>
>> > That's pretty much what it should do anyway with normally designed
>> > hardware.
>
>> I guess the question is: do we insist that the driver include this
>> workaround, or are we OK with letting the hardware behave as the
>> hardware does?
>
> What you're describing sounds like what we should be doing normally, if
> we're not doing that we should probably be fixing the core.
I'm not convinced that this behavior makes sense to move to the core.
On most regulators I'd expect that when the regulator driver says to
turn them off that they will output no voltage. The reason RPMh is
special is that there's an aggregation outside of Linux. Specifically
I like to make it concrete use the example where both Linux and "the
modem" make requests for the same regulator and RPMh aggregates these
requests. This aggregation happens separately for "enabled" and
"voltage".
Thus if you consider:
Modem: vote for 1.3V and enabled=true
Linux: vote for 1.4V and enabled=false
The aggregated voltage is 1.4V and the aggregated enabled is true.
Said another way: Linux's vote for the voltage affected the state of
the rail even though Linux wanted the rail disabled.
In any other system when Linux disabled the regulator it wouldn't
matter that you left it set at 1.4V. Thus RPMh is special and IMO the
workaround belongs there.
-Doug
^ permalink raw reply
* [RFT v3 1/4] perf cs-etm: Generate branch sample for missed packets
From: Leo Yan @ 2018-05-30 15:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJ9a7VhC7EHJ1DtTEH1uERGiBpCJgseboaH0cqgpsU3ZMcgjgQ@mail.gmail.com>
Hi Mike,
On Wed, May 30, 2018 at 04:04:34PM +0100, Mike Leach wrote:
[...]
> >>> + /* Generate sample for exception packet */
> >>> + if (etmq->prev_packet->exc == true)
> >>> + generate_sample = true;
> >>
> >>
> >> Please don't do that. Exception packets have a type of their own and can
> >> be
> >> added to the decoder packet queue the same way INST_RANGE and TRACE_ON
> >> packets
> >> are. Moreover exception packet containt an address that, if I'm reading
> >> the
> >> documenation properly, can be used to keep track of instructions that were
> >> executed between the last address of the previous range packet and the
> >> address
> >> executed just before the exception occurred. Mike and Rob will have to
> >> confirm
> >> this as the decoder may be doing all that hard work for us.
> >>
>
> clarification on the exception packets....
>
> The Opencsd output exception packet gives you the exception number,
> and optionally the preferred return address. If this address is
> present does depend a lot on the underlying protocol - will normally
> be there with ETMv4.
> Exceptions are marked differently in the underlying protocol - the
> OCSD packets abstract away these differences.
>
> consider the code:
>
> 0x1000: <some instructions>
> 0x1100: BR 0x2000
> ....
> 0x2000: <some instructions>
> 0x2020 BZ r4
>
> Without an exception this would result in the packets
>
> OCSD_RANGE(0x1000,0x1104, Last instr type=Br, taken) // recall that
> range packets have start addr inclusive, end addr exclusive.
> OCSD_RANGE(0x2000,0x2024, Last instr type=Br, <taken / not taken -
> depends on condition>
>
> Now consider an exception occurring before the BR 0x2000
>
> this will result in:-
> OCSD_RANGE(0x1000, 0x1100, Last instr type=Other)
> OCSD_EXECEPTION(IRQ, ret-addr 0x1100)
> OCSD_RANGE(IRQ_START, IRQ_END+4, Last instr type = BR, taken) //
> this is more likely to have multiple ranges / branches before any
> return, but simplified here.
> OCSD_EXCEPTION_RETURN() // present if exception returns are
> explicitly marked in underlying trace - may not always be depending on
> circumstances.
> OCSD_RANGE(0x1100,0x1104, Last=BR, taken) // continue on with short
> range - just the branch
> OCSD_RANGE(0x2000,0x2024, Last instr type=Br, <taken / not taken -
> depends on condition>
>
> Now consider the exception occurring after the BR, but before any
> other instructions are executed.
>
> OCSD_RANGE(0x1000,0x1104, Last instr type=Br, taken) // recall that
> range packets have start addr inclusive, end addr exclusive.
> OCSD_EXECEPTION(IRQ, ret-addr 0x2000) // here the preferred return
> address is actually the target of the branch.
> OCSD_RANGE(IRQ_START, IRQ_END+4, Last instr type = BR, taken) //
> this is more likely to have multiple ranges / branches before any
> return, but simplified here.
> OCSD_RANGE(0x2000,0x2024, Last instr type=Br, <taken / not taken -
> depends on condition>
>
> So in general it is possible to arrive in the IRQ_START range with the
> previous packet having been either a taken branch, a not taken branch,
> or not a branch.
> Care must be taken - whether AutoFDO or normal trace disassembly not
> to assume that having the last range packet as a taken branch means
> that the next range packet is the target, if there is an intervening
> exception packet.
Thanks a lot for detailed explaination.
IIUC, AutoFDO will not have such issue due every range packet will be
handled for it. On the other hand, as you remind, the branch samples
(and its consumer trace disassembler) is very dependent on the flag
'last_instr_taken_branch'.
According to your explaination, I think we consider the branch is
taken for below situations:
- The new coming packet is exception packet (both for exception entry
and exit packets);
- The previous packet is expcetion packet;
- The previous packet is normal range packet with
'last_instr_taken_branch' = true;
So I'd like to use below function to demonstrate my understanding for
exception packets handling. I also will send out one new patch for
support exception packet for reviewing.
If you have concern or I miss anything, please let me know.
static bool cs_etm__is_taken_branch(struct cs_etm_packet *prev_packet,
struct cs_etm_packet *packet,)
{
/* The branch is taken for normal range packet with taken branch flag */
if (prev_packet->sample_type == CS_ETM_RANGE &&
prev_packet->last_instr_taken_branch)
return true;
/* The branch is taken if previous packet is exception packet */
if (prev_packet->sample_type == CS_ETM_EXCEPTION ||
prev_packet->sample_type == CS_ETM_EXCEPTION_RET)
return true;
/* The branch is taken for an intervening exception packet */
if (packet->sample_type == CS_ETM_EXCEPTION ||
packet->sample_type == CS_ETM_EXCEPTION_RET)
return true;
return false;
}
[...]
Thanks,
Leo Yan
^ permalink raw reply
* [PATCH v3 3/3] x86/mm: add TLB purge to free pmd/pte page interfaces
From: Kani, Toshi @ 2018-05-30 15:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180530045927.GP18595@8bytes.org>
On Wed, 2018-05-30 at 06:59 +0200, joro at 8bytes.org wrote:
> On Tue, May 29, 2018 at 04:10:24PM +0000, Kani, Toshi wrote:
> > Can you explain why you think allocating a page here is a major problem?
>
> Because a larger allocation is more likely to fail. And if you fail the
> allocation, you also fail to free more pages, which _is_ a problem. So
> better avoid any allocations in code paths that are about freeing
> memory.
It only allocates a single page. If it fails to allocate a single page,
this pud mapping request simply falls to pmd mappings, which is only as
bad as your suggested plain revert always does for both pud and pmd
mappings. Also, this func is called from ioremap() when a driver
initializes its mapping. If the system does not have a single page to
allocate, the driver has a much bigger issue to deal with than its
request falling back to pmd mappings. Please also note that this func
is not called from free-memory path.
> > If we just revert, please apply patch 1/3 first. This patch address the
> > BUG_ON issue on PAE. This is a real issue that needs a fix ASAP.
>
> It does not address the problem of dirty page-walk caches on x86-64.
This patch 3/3 fixes it. Hope my explanation above clarified.
> > The page-directory cache issue on x64, which is addressed by patch 3/3,
> > is a theoretical issue that I could not hit by putting ioremap() calls
> > into a loop for a whole day. Nobody hit this issue, either.
>
> How do you know you didn't hit that issue? It might cause silent data
> corruption, which might not be easily detected.
If the test hit that issue, it would have caused a kernel page fault
(freed & cleared pte page still unmapped, or this page reused and
attribute data invalid) or MCE (pte page reused and phys addr invalid)
when it accessed to ioremap'd address.
Causing silent data corruption requires that this freed & cleared pte
page to be reused and re-initialized with a valid pte entry data. While
this case is possible, the chance of my test only hitting this case
without ever hitting much more likely cases of page fault or MCE is
basically zero.
> > The simple revert patch Joerg posted a while ago causes
> > pmd_free_pte_page() to fail on x64. This causes multiple pmd mappings
> > to fall into pte mappings on my test systems. This can be seen as a
> > degradation, and I am afraid that it is more harmful than good.
>
> The plain revert just removes all the issues with the dirty TLB that the
> original patch introduced and prevents huge mappings from being
> established when there have been smaller mappings before. This is not
> ideal, but at least its is consistent and does not leak pages and leaves
> no dirty TLBs. So this is the easiest and most reliable fix for this
> stage in the release process.
If you think the page directory issue needs a fix ASAP, I believe taking
patch 3/3 is much better option than the plain revert, which will
introduce the fall-back issue I explained.
Thanks,
-Toshi
^ permalink raw reply
* [PATCH v2 1/6] ARM: dra762: hwmod: Add MCAN support
From: Tero Kristo @ 2018-05-30 15:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180530152825.GG5705@atomide.com>
On 30/05/18 18:28, Tony Lindgren wrote:
> * Tero Kristo <t-kristo@ti.com> [180530 15:18]:
>> For the OCP if part, I think that is still needed until we switch over to
>> full sysc driver. clkctrl_offs you probably also need because that is used
>> for mapping the omap_hwmod against a specific clkctrl clock. Those can be
>> only removed once we are done with hwmod (or figure out some other way to
>> assign the clkctrl clock to a hwmod.)
>
> Hmm might be worth testing. I thought your commit 70f05be32133
> ("ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available")
> already parses the clkctrl from dts?
It maps the clkctrl clock to be used by hwmod, if those are available.
We didn't add any specific clock entries to DT for mapping the actual
clkctrl clock without the hwmod_data hints yet though, as that was
deemed temporary solution only due to transition to interconnect driver.
I.e., you would need something like this in DT for every device node:
&uart3 {
clocks = <l4per_clkctrl UART3_CLK 0>;
clock-names = "clkctrl";
};
... which is currently not present.
Alternatively you could rename the main_clk in the hwmod_data to point
towards the clkctrl clock, but again, that would be a temporary solution
only.
-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply
* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Mark Brown @ 2018-05-30 15:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=XJd-udEE6az0xP7i6x0oGsnd=BSCyZ0Og_+RjNCMXnCQ@mail.gmail.com>
On Wed, May 30, 2018 at 08:34:50AM -0700, Doug Anderson wrote:
> On Wed, May 30, 2018 at 8:02 AM, Mark Brown <broonie@kernel.org> wrote:
> > What you're describing sounds like what we should be doing normally, if
> > we're not doing that we should probably be fixing the core.
> I'm not convinced that this behavior makes sense to move to the core.
> On most regulators I'd expect that when the regulator driver says to
> turn them off that they will output no voltage. The reason RPMh is
Oh, you mean while the regulator is off... TBH I don't see a huge
problem doing that anyway and just reverting to the bottom of the
constraints when everything gets turned off since we have to see if we
need to adjust the voltage every time we enabled anyway.
> In any other system when Linux disabled the regulator it wouldn't
> matter that you left it set at 1.4V. Thus RPMh is special and IMO the
> workaround belongs there.
Without the core doing something the regulator isn't going to get told
that anything updated voltages anyway...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180530/35e9ccdc/attachment.sig>
^ permalink raw reply
* [RFT v3 1/4] perf cs-etm: Generate branch sample for missed packets
From: Leo Yan @ 2018-05-30 15:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180530153900.GA10925@leoy-ThinkPad-X240s>
On Wed, May 30, 2018 at 11:39:00PM +0800, Leo Yan wrote:
> Hi Mike,
>
> On Wed, May 30, 2018 at 04:04:34PM +0100, Mike Leach wrote:
>
> [...]
>
> > >>> + /* Generate sample for exception packet */
> > >>> + if (etmq->prev_packet->exc == true)
> > >>> + generate_sample = true;
> > >>
> > >>
> > >> Please don't do that. Exception packets have a type of their own and can
> > >> be
> > >> added to the decoder packet queue the same way INST_RANGE and TRACE_ON
> > >> packets
> > >> are. Moreover exception packet containt an address that, if I'm reading
> > >> the
> > >> documenation properly, can be used to keep track of instructions that were
> > >> executed between the last address of the previous range packet and the
> > >> address
> > >> executed just before the exception occurred. Mike and Rob will have to
> > >> confirm
> > >> this as the decoder may be doing all that hard work for us.
> > >>
> >
> > clarification on the exception packets....
> >
> > The Opencsd output exception packet gives you the exception number,
> > and optionally the preferred return address. If this address is
> > present does depend a lot on the underlying protocol - will normally
> > be there with ETMv4.
> > Exceptions are marked differently in the underlying protocol - the
> > OCSD packets abstract away these differences.
> >
> > consider the code:
> >
> > 0x1000: <some instructions>
> > 0x1100: BR 0x2000
> > ....
> > 0x2000: <some instructions>
> > 0x2020 BZ r4
> >
> > Without an exception this would result in the packets
> >
> > OCSD_RANGE(0x1000,0x1104, Last instr type=Br, taken) // recall that
> > range packets have start addr inclusive, end addr exclusive.
> > OCSD_RANGE(0x2000,0x2024, Last instr type=Br, <taken / not taken -
> > depends on condition>
> >
> > Now consider an exception occurring before the BR 0x2000
> >
> > this will result in:-
> > OCSD_RANGE(0x1000, 0x1100, Last instr type=Other)
> > OCSD_EXECEPTION(IRQ, ret-addr 0x1100)
> > OCSD_RANGE(IRQ_START, IRQ_END+4, Last instr type = BR, taken) //
> > this is more likely to have multiple ranges / branches before any
> > return, but simplified here.
> > OCSD_EXCEPTION_RETURN() // present if exception returns are
> > explicitly marked in underlying trace - may not always be depending on
> > circumstances.
> > OCSD_RANGE(0x1100,0x1104, Last=BR, taken) // continue on with short
> > range - just the branch
> > OCSD_RANGE(0x2000,0x2024, Last instr type=Br, <taken / not taken -
> > depends on condition>
> >
> > Now consider the exception occurring after the BR, but before any
> > other instructions are executed.
> >
> > OCSD_RANGE(0x1000,0x1104, Last instr type=Br, taken) // recall that
> > range packets have start addr inclusive, end addr exclusive.
> > OCSD_EXECEPTION(IRQ, ret-addr 0x2000) // here the preferred return
> > address is actually the target of the branch.
> > OCSD_RANGE(IRQ_START, IRQ_END+4, Last instr type = BR, taken) //
> > this is more likely to have multiple ranges / branches before any
> > return, but simplified here.
> > OCSD_RANGE(0x2000,0x2024, Last instr type=Br, <taken / not taken -
> > depends on condition>
> >
> > So in general it is possible to arrive in the IRQ_START range with the
> > previous packet having been either a taken branch, a not taken branch,
> > or not a branch.
> > Care must be taken - whether AutoFDO or normal trace disassembly not
> > to assume that having the last range packet as a taken branch means
> > that the next range packet is the target, if there is an intervening
> > exception packet.
>
> Thanks a lot for detailed explaination.
>
> IIUC, AutoFDO will not have such issue due every range packet will be
> handled for it. On the other hand, as you remind, the branch samples
> (and its consumer trace disassembler) is very dependent on the flag
> 'last_instr_taken_branch'.
>
> According to your explaination, I think we consider the branch is
> taken for below situations:
>
> - The new coming packet is exception packet (both for exception entry
> and exit packets);
> - The previous packet is expcetion packet;
> - The previous packet is normal range packet with
> 'last_instr_taken_branch' = true;
>
> So I'd like to use below function to demonstrate my understanding for
> exception packets handling. I also will send out one new patch for
> support exception packet for reviewing.
>
> If you have concern or I miss anything, please let me know.
>
> static bool cs_etm__is_taken_branch(struct cs_etm_packet *prev_packet,
> struct cs_etm_packet *packet,)
> {
> /* The branch is taken for normal range packet with taken branch flag */
> if (prev_packet->sample_type == CS_ETM_RANGE &&
> prev_packet->last_instr_taken_branch)
> return true;
>
> /* The branch is taken if previous packet is exception packet */
> if (prev_packet->sample_type == CS_ETM_EXCEPTION ||
> prev_packet->sample_type == CS_ETM_EXCEPTION_RET)
> return true;
>
> /* The branch is taken for an intervening exception packet */
> if (packet->sample_type == CS_ETM_EXCEPTION ||
> packet->sample_type == CS_ETM_EXCEPTION_RET)
> return true;
>
> return false;
> }
Just clarify, I missed to mention I introduce two extra sample types:
CS_ETM_EXCEPTION and CS_ETM_EXCEPTION_RET, one is for exception
entry packet and another is for exception exit packet. If this is
hard for understanding, you could hold on for reveiwing new patch.
Thanks,
Leo Yan
^ permalink raw reply
* [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Mark Brown @ 2018-05-30 15:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=WTTKXwPchuWRh-RNTc8=X8Xo28oTqk=3wW4+SiTHhxuA@mail.gmail.com>
On Wed, May 30, 2018 at 07:54:47AM -0700, Doug Anderson wrote:
> On Wed, May 30, 2018 at 3:37 AM, Mark Brown <broonie@kernel.org> wrote:
> > I'm confused as to why we are specifying the maximum current the device
> > can deliver in a given mode in the DT - surely that's a fixed property
> > of the hardware?
> Said another way: you're saying that you'd expect the "max-microamps"
> table to have one fewer element than the listed modes? So in the
> above example you'd have:
No, I'm saying that I don't know why that property exists at all. This
sounds like it's intended to be the amount of current the regulator can
deliver in each mode which is normally a design property of the silicon.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180530/ae2343e4/attachment.sig>
^ permalink raw reply
* [PATCH v2 1/6] ARM: dra762: hwmod: Add MCAN support
From: Tony Lindgren @ 2018-05-30 15:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b0476932-a6e9-6ef1-69ad-c9a8c55d9739@ti.com>
* Tero Kristo <t-kristo@ti.com> [180530 15:44]:
> On 30/05/18 18:28, Tony Lindgren wrote:
> > * Tero Kristo <t-kristo@ti.com> [180530 15:18]:
> > > For the OCP if part, I think that is still needed until we switch over to
> > > full sysc driver. clkctrl_offs you probably also need because that is used
> > > for mapping the omap_hwmod against a specific clkctrl clock. Those can be
> > > only removed once we are done with hwmod (or figure out some other way to
> > > assign the clkctrl clock to a hwmod.)
> >
> > Hmm might be worth testing. I thought your commit 70f05be32133
> > ("ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available")
> > already parses the clkctrl from dts?
>
> It maps the clkctrl clock to be used by hwmod, if those are available. We
> didn't add any specific clock entries to DT for mapping the actual clkctrl
> clock without the hwmod_data hints yet though, as that was deemed temporary
> solution only due to transition to interconnect driver. I.e., you would need
> something like this in DT for every device node:
>
> &uart3 {
> clocks = <l4per_clkctrl UART3_CLK 0>;
> clock-names = "clkctrl";
> };
>
> ... which is currently not present.
Hmm is that not the "fck" clkctrl clock we have already in
the dts files for the interconnect target modules?
We can also use pdata callbacks to pass the clock node if
needed. But I guess I don't quite still understand what we
are missing :)
Regards,
Tony
^ permalink raw reply
* [PATCH v9 01/15] ARM: Add Krait L2 register accessor functions
From: Stephen Boyd @ 2018-05-30 15:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <664089d4-b30d-99bb-2021-12128b2895ba@codeaurora.org>
Quoting Sricharan R (2018-05-24 22:40:11)
> Hi Bjorn,
>
> On 5/24/2018 11:09 PM, Bjorn Andersson wrote:
> > On Tue 06 Mar 06:38 PST 2018, Sricharan R wrote:
> >
> >> From: Stephen Boyd <sboyd@codeaurora.org>
> >>
> >> Krait CPUs have a handful of L2 cache controller registers that
> >> live behind a cp15 based indirection register. First you program
> >> the indirection register (l2cpselr) to point the L2 'window'
> >> register (l2cpdr) at what you want to read/write. Then you
> >> read/write the 'window' register to do what you want. The
> >> l2cpselr register is not banked per-cpu so we must lock around
> >> accesses to it to prevent other CPUs from re-pointing l2cpdr
> >> underneath us.
> >>
> >> Cc: Mark Rutland <mark.rutland@arm.com>
> >> Cc: Russell King <linux@arm.linux.org.uk>
> >> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> >
> > This should have your signed-off-by here as well.
> >
>
> ok.
>
> > Apart from that:
> >
> > Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> >
>
Will these patches come around again? I'll do a quick sweep on them
today but I expect them to be resent.
^ permalink raw reply
* [PATCH v2 4/4] mmc: sdhci-msm: Add sdhci msm register write APIs which wait for pwr irq
From: Georgi Djakov @ 2018-05-30 15:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6c84f6fc-270c-d826-6d87-7f9f6fc5015f@codeaurora.org>
Hi Vijay,
On 05/30/2018 10:11 AM, Vijay Viswanath wrote:
> Hi Georgi,
>
> Thanks for testing the patch on 8096 and pointing out this issue.
> The issue is coming because, when card is removed, the HOST_CONTROL2
> register is retaining the 1.8V Signalling enable bit till SDHCI reset
> happens after a new card is inserted.
>
> Adding the change you suggested can avoid this wait, but I feel a better
> solution is to clear the 1.8V signalling bit when the card is removed.
> When a new card is inserted, we shouldn't be keeping the 1.8V bit set
> until we send cmd11 to the SD card. A new SD card should start with 3V.
>
> One solution is to explicitly clear the HOST_CONTROL2 register when card
> is removed.
>
> Other way is to revert the commit:
> 9718f84b85396e090ca42fafa730410d286d61e3 "mmc: sdhci-msm: Do not reset
> the controller if no card in the slot"
>
> The sdhci-msm doesn't require "SDHCI_QUIRK_NO_CARD_NO_RESET". The issue
> which above commit is trying to avoid is fixed by the pwr-irq patches.
> Resetting the controller will clear the HOST_CONTROL2 register and avoid
> this issue.
>
> Can you please try this ? I tested reverting the QUIRK on two platforms:
> db410c(8916) and sdm845. SD card insert/remove worked fine after that
> and I didn't get any "Reset 0x1 never completed" error during card
> insert/remove or shutdown.
Thank you! I have submitted a patch to remove the quirk and tested it on
db410c and db820c.
BR,
Georgi
^ permalink raw reply
* [PATCH v2 00/17] arm64 SSBD (aka Spectre-v4) mitigation
From: Will Deacon @ 2018-05-30 15:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180529121121.24927-1-marc.zyngier@arm.com>
Hi Marc,
On Tue, May 29, 2018 at 01:11:04PM +0100, Marc Zyngier wrote:
> This patch series implements the Linux kernel side of the "Spectre-v4"
> (CVE-2018-3639) mitigation known as "Speculative Store Bypass Disable"
> (SSBD).
>
> More information can be found at:
>
> https://bugs.chromium.org/p/project-zero/issues/detail?id=1528
> https://developer.arm.com/support/arm-security-updates/speculative-processor-vulnerability
>
> For all released Arm Cortex-A CPUs that are affected by this issue, then
> the preferred mitigation is simply to set a chicken bit in the firmware
> during CPU initialisation and therefore no change to Linux is required.
> Other CPUs may require the chicken bit to be toggled dynamically (for
> example, when switching between user-mode and kernel-mode) and this is
> achieved by calling into EL3 via an SMC which has been published as part
> of the latest SMCCC specification:
>
> https://developer.arm.com/cache-speculation-vulnerability-firmware-specification
>
> as well as an ATF update for the released ARM cores affected by SSBD:
>
> https://github.com/ARM-software/arm-trusted-firmware/pull/1392
>
> These patches provide the following:
>
> 1. Safe probing of firmware to establish which CPUs in the system
> require calling into EL3 as part of the mitigation.
>
> 2. For CPUs that require it, call into EL3 on exception entry/exit
> from EL0 to apply the SSBD mitigation when running at EL1.
>
> 3. A command-line option to force the SSBD mitigation to be always on,
> always off, or dymamically toggled (default) for CPUs that require
> the EL3 call.
>
> 4. An initial implementation of a prctl() backend for arm64 that allows
> userspace tasks to opt-in to the mitigation explicitly. This is
> intended to match the interface provided by x86, and so we rely on
> their core changes here. The seccomp interface is provided as an
> extra set of patches, which I'd like *not* to see merged. The main
> reason is that it is invasive, has ugly/unclear semantics, and could
> probably be left to the existing prctl interface.
I agree with you here. For patches 1-10, then:
Acked-by: Will Deacon <will.deacon@arm.com>
but I'd prefer to leave the seccomp stuff alone for the moment because I
don't think the implicit enabling is necessarily the right thing to do
there and supporting it comes at a cost.
Will
^ permalink raw reply
* [PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Jon Hunter @ 2018-05-30 16:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180529100421.31022-9-ulf.hansson@linaro.org>
On 29/05/18 11:04, Ulf Hansson wrote:
> To support devices being partitioned across multiple PM domains, let's
> begin with extending genpd to cope with these kind of configurations.
>
> Therefore, add a new exported function genpd_dev_pm_attach_by_id(), which
> is similar to the existing genpd_dev_pm_attach(), but with the difference
> that it allows its callers to provide an index to the PM domain that it
> wants to attach.
>
> Note that, genpd_dev_pm_attach_by_id() shall only be called by the driver
> core / PM core, similar to how the existing dev_pm_domain_attach() makes
> use of genpd_dev_pm_attach(). However, this is implemented by following
> changes on top.
>
> Because, only one PM domain can be attached per device, genpd needs to
> create a virtual device that it can attach/detach instead. More precisely,
> let the new function genpd_dev_pm_attach_by_id() register a virtual struct
> device via calling device_register(). Then let it attach this device to the
> corresponding PM domain, rather than the one that is provided by the
> caller. The actual attaching is done via re-using the existing genpd OF
> functions.
>
> At successful attachment, genpd_dev_pm_attach_by_id() returns the created
> virtual device, which allows the caller to operate on it to deal with power
> management. Following changes on top, provides more details in this
> regards.
>
> To deal with detaching of a PM domain for the multiple PM domains case,
> let's also extend the existing genpd_dev_pm_detach() function, to cover the
> cleanup of the created virtual device, via make it call device_unregister()
> on it. In this way, there is no need to introduce a new function to deal
> with detach for the multiple PM domain case, but instead the existing one
> is re-used.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>
> Changes in v2:
> - Fixed comments from Jon. Clarified function descriptions
> and changelog.
>
> ---
> drivers/base/power/domain.c | 80 +++++++++++++++++++++++++++++++++++++
> include/linux/pm_domain.h | 8 ++++
> 2 files changed, 88 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 2af99bfcbe3c..2b496d79159d 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2171,6 +2171,15 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
> }
> EXPORT_SYMBOL_GPL(of_genpd_remove_last);
>
> +static void genpd_release_dev(struct device *dev)
> +{
> + kfree(dev);
> +}
> +
> +static struct bus_type genpd_bus_type = {
> + .name = "genpd",
> +};
> +
> /**
> * genpd_dev_pm_detach - Detach a device from its PM domain.
> * @dev: Device to detach.
> @@ -2208,6 +2217,10 @@ static void genpd_dev_pm_detach(struct device *dev, bool power_off)
>
> /* Check if PM domain can be powered off after removing this device. */
> genpd_queue_power_off_work(pd);
> +
> + /* Unregister the device if it was created by genpd. */
> + if (dev->bus == &genpd_bus_type)
> + device_unregister(dev);
> }
>
> static void genpd_dev_pm_sync(struct device *dev)
> @@ -2298,6 +2311,67 @@ int genpd_dev_pm_attach(struct device *dev)
> }
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
> +/**
> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
> + * @dev: Device to attach.
Can you update the description of the above as well?
Thanks
Jon
--
nvpublic
^ permalink raw reply
* [PATCH v2 6/9] PM / Domains: Don't attach devices in genpd with multi PM domains
From: Jon Hunter @ 2018-05-30 16:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180529100421.31022-7-ulf.hansson@linaro.org>
On 29/05/18 11:04, Ulf Hansson wrote:
> The power-domain DT property may now contain a list of PM domain
> specifiers, which represents that a device are partitioned across multiple
> PM domains. This leads to a new situation in genpd_dev_pm_attach(), as only
> one PM domain can be attached per device.
>
> To remain things simple for the most common configuration, when a single PM
> domain is used, let's treat the multiple PM domain case as being specific.
>
> In other words, let's change genpd_dev_pm_attach() to check for multiple PM
> domains and prevent it from attach any PM domain for this case. Instead,
> leave this to be managed separately, from following changes to genpd.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree at vger.kernel.org
> Suggested-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>
> Changes in v2:
> - Minor update to changelog to mention "PM domain specifiers" rather
> than a "list of phandles".
>
> ---
> drivers/base/power/domain.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 7ebf7993273a..12a20f21974d 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2229,10 +2229,10 @@ static void genpd_dev_pm_sync(struct device *dev)
> * attaches the device to retrieved pm_domain ops.
> *
> * Returns 1 on successfully attached PM domain, 0 when the device don't need a
> - * PM domain or a negative error code in case of failures. Note that if a
> - * power-domain exists for the device, but it cannot be found or turned on,
> - * then return -EPROBE_DEFER to ensure that the device is not probed and to
> - * re-try again later.
> + * PM domain or when multiple power-domains exists for it, else a negative error
> + * code. Note that if a power-domain exists for the device, but it cannot be
> + * found or turned on, then return -EPROBE_DEFER to ensure that the device is
> + * not probed and to re-try again later.
> */
> int genpd_dev_pm_attach(struct device *dev)
> {
> @@ -2243,10 +2243,18 @@ int genpd_dev_pm_attach(struct device *dev)
> if (!dev->of_node)
> return 0;
>
> + /*
> + * Devices with multiple PM domains must be attached separately, as we
> + * can only attach one PM domain per device.
> + */
> + if (of_count_phandle_with_args(dev->of_node, "power-domains",
> + "#power-domain-cells") != 1)
> + return 0;
> +
> ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
> "#power-domain-cells", 0, &pd_args);
> if (ret < 0)
> - return 0;
> + return ret;
>
> mutex_lock(&gpd_list_lock);
> pd = genpd_get_from_provider(&pd_args);
>
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>
Cheers
Jon
--
nvpublic
^ permalink raw reply
* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Doug Anderson @ 2018-05-30 16:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180530154849.GQ6920@sirena.org.uk>
Hi,
On Wed, May 30, 2018 at 8:48 AM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, May 30, 2018 at 08:34:50AM -0700, Doug Anderson wrote:
>> On Wed, May 30, 2018 at 8:02 AM, Mark Brown <broonie@kernel.org> wrote:
>
>> > What you're describing sounds like what we should be doing normally, if
>> > we're not doing that we should probably be fixing the core.
>
>> I'm not convinced that this behavior makes sense to move to the core.
>> On most regulators I'd expect that when the regulator driver says to
>> turn them off that they will output no voltage. The reason RPMh is
>
> Oh, you mean while the regulator is off... TBH I don't see a huge
> problem doing that anyway and just reverting to the bottom of the
> constraints when everything gets turned off since we have to see if we
> need to adjust the voltage every time we enabled anyway.
>
>> In any other system when Linux disabled the regulator it wouldn't
>> matter that you left it set at 1.4V. Thus RPMh is special and IMO the
>> workaround belongs there.
>
> Without the core doing something the regulator isn't going to get told
> that anything updated voltages anyway...
I was just suggesting that when the core tells the regulator driver to
disable itself that the regulator driver tell RPMh to not only disable
itself but also (temporarily) vote for a lower voltage. When the core
tells the regulator to re-enable itself then the regulator driver
restores the original voltage vote (or applies any vote that might
have been attempted while the regulator was disabled). That wouldn't
require any regulator core changes.
-Doug
^ permalink raw reply
* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Mark Brown @ 2018-05-30 16:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=X2u=hMyWinSDtim-PmFwAy5mXcwg3HeYojAHcsUFhV3g@mail.gmail.com>
On Wed, May 30, 2018 at 09:06:16AM -0700, Doug Anderson wrote:
> On Wed, May 30, 2018 at 8:48 AM, Mark Brown <broonie@kernel.org> wrote:
> > Without the core doing something the regulator isn't going to get told
> > that anything updated voltages anyway...
> I was just suggesting that when the core tells the regulator driver to
> disable itself that the regulator driver tell RPMh to not only disable
> itself but also (temporarily) vote for a lower voltage. When the core
> tells the regulator to re-enable itself then the regulator driver
> restores the original voltage vote (or applies any vote that might
> have been attempted while the regulator was disabled). That wouldn't
> require any regulator core changes.
It needs something to tell it what the new voltage to set is.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180530/383a4352/attachment-0001.sig>
^ permalink raw reply
* [PATCH v2 7/9] PM / Domains: Split genpd_dev_pm_attach()
From: Jon Hunter @ 2018-05-30 16:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180529100421.31022-8-ulf.hansson@linaro.org>
On 29/05/18 11:04, Ulf Hansson wrote:
> To extend genpd to deal with allowing multiple PM domains per device, some
> of the code in genpd_dev_pm_attach() can be re-used. Let's prepare for this
> by moving some of the code into a sub-function.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> drivers/base/power/domain.c | 60 ++++++++++++++++++++-----------------
> 1 file changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 12a20f21974d..2af99bfcbe3c 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2221,38 +2221,15 @@ static void genpd_dev_pm_sync(struct device *dev)
> genpd_queue_power_off_work(pd);
> }
>
> -/**
> - * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
> - * @dev: Device to attach.
> - *
> - * Parse device's OF node to find a PM domain specifier. If such is found,
> - * attaches the device to retrieved pm_domain ops.
> - *
> - * Returns 1 on successfully attached PM domain, 0 when the device don't need a
> - * PM domain or when multiple power-domains exists for it, else a negative error
> - * code. Note that if a power-domain exists for the device, but it cannot be
> - * found or turned on, then return -EPROBE_DEFER to ensure that the device is
> - * not probed and to re-try again later.
> - */
> -int genpd_dev_pm_attach(struct device *dev)
> +static int __genpd_dev_pm_attach(struct device *dev, struct device_node *np,
> + unsigned int index)
> {
> struct of_phandle_args pd_args;
> struct generic_pm_domain *pd;
> int ret;
>
> - if (!dev->of_node)
> - return 0;
> -
> - /*
> - * Devices with multiple PM domains must be attached separately, as we
> - * can only attach one PM domain per device.
> - */
> - if (of_count_phandle_with_args(dev->of_node, "power-domains",
> - "#power-domain-cells") != 1)
> - return 0;
> -
> - ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
> - "#power-domain-cells", 0, &pd_args);
> + ret = of_parse_phandle_with_args(np, "power-domains",
> + "#power-domain-cells", index, &pd_args);
> if (ret < 0)
> return ret;
>
> @@ -2290,6 +2267,35 @@ int genpd_dev_pm_attach(struct device *dev)
>
> return ret ? -EPROBE_DEFER : 1;
> }
> +
> +/**
> + * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
> + * @dev: Device to attach.
> + *
> + * Parse device's OF node to find a PM domain specifier. If such is found,
> + * attaches the device to retrieved pm_domain ops.
> + *
> + * Returns 1 on successfully attached PM domain, 0 when the device don't need a
> + * PM domain or when multiple power-domains exists for it, else a negative error
> + * code. Note that if a power-domain exists for the device, but it cannot be
> + * found or turned on, then return -EPROBE_DEFER to ensure that the device is
> + * not probed and to re-try again later.
> + */
> +int genpd_dev_pm_attach(struct device *dev)
> +{
> + if (!dev->of_node)
> + return 0;
> +
> + /*
> + * Devices with multiple PM domains must be attached separately, as we
> + * can only attach one PM domain per device.
> + */
> + if (of_count_phandle_with_args(dev->of_node, "power-domains",
> + "#power-domain-cells") != 1)
> + return 0;
> +
> + return __genpd_dev_pm_attach(dev, dev->of_node, 0);
> +}
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
> static const struct of_device_id idle_state_match[] = {
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>
Cheers
Jon
--
nvpublic
^ permalink raw reply
* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Doug Anderson @ 2018-05-30 16:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180530160744.GS6920@sirena.org.uk>
Hi,
On Wed, May 30, 2018 at 9:07 AM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, May 30, 2018 at 09:06:16AM -0700, Doug Anderson wrote:
>> On Wed, May 30, 2018 at 8:48 AM, Mark Brown <broonie@kernel.org> wrote:
>
>> > Without the core doing something the regulator isn't going to get told
>> > that anything updated voltages anyway...
>
>> I was just suggesting that when the core tells the regulator driver to
>> disable itself that the regulator driver tell RPMh to not only disable
>> itself but also (temporarily) vote for a lower voltage. When the core
>> tells the regulator to re-enable itself then the regulator driver
>> restores the original voltage vote (or applies any vote that might
>> have been attempted while the regulator was disabled). That wouldn't
>> require any regulator core changes.
>
> It needs something to tell it what the new voltage to set is.
The regulator driver has its own cache of what voltage was most
recently requested by Linux. It can use that, can't it?
-Doug
^ permalink raw reply
* [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Doug Anderson @ 2018-05-30 16:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180530155044.GR6920@sirena.org.uk>
Hi,
On Wed, May 30, 2018 at 8:50 AM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, May 30, 2018 at 07:54:47AM -0700, Doug Anderson wrote:
>> On Wed, May 30, 2018 at 3:37 AM, Mark Brown <broonie@kernel.org> wrote:
>
>> > I'm confused as to why we are specifying the maximum current the device
>> > can deliver in a given mode in the DT - surely that's a fixed property
>> > of the hardware?
>
>> Said another way: you're saying that you'd expect the "max-microamps"
>> table to have one fewer element than the listed modes? So in the
>> above example you'd have:
>
> No, I'm saying that I don't know why that property exists at all. This
> sounds like it's intended to be the amount of current the regulator can
> deliver in each mode which is normally a design property of the silicon.
Ah, got it. So the whole point here is to be able to implement either
the function "set_load" or the function "get_optimum_mode". We need
some sort of table to convert from current to mode. That's what this
table does.
My argument to David was that since set_load / get_optimum_mode were
features of the regulator core these should actually be standard
properties and not Qualcomm-specific ones.
-Doug
^ permalink raw reply
* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Mark Brown @ 2018-05-30 16:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=WVGu0f+=q-C9kiEmaLEUJqGY+Jbns83h72uF-9iasrUw@mail.gmail.com>
On Wed, May 30, 2018 at 09:09:02AM -0700, Doug Anderson wrote:
> On Wed, May 30, 2018 at 9:07 AM, Mark Brown <broonie@kernel.org> wrote:
> > It needs something to tell it what the new voltage to set is.
> The regulator driver has its own cache of what voltage was most
> recently requested by Linux. It can use that, can't it?
If we're just going to use the most recently set voltage then hopefully
the hardware already knew that, and it might not be the lowest available
voltage if the last consumer to get turned off was holding the voltage
higher.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180530/10a575f9/attachment.sig>
^ 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