* [PATCH 03/61] ARM: dts: r8a7778: Use R-Car Gen1 Ether fallback compat string
From: Simon Horman @ 2017-12-06 10:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1512555546.git.horms+renesas@verge.net.au>
Use newly added R-Car Gen1 Ether fallback compat string
in the DT of the r8a7778 SoC.
This should have no run-time effect as the driver matches against
the per-SoC compat string before considering the fallback compat string.
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
arch/arm/boot/dts/r8a7778.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/r8a7778.dtsi b/arch/arm/boot/dts/r8a7778.dtsi
index a39472aab867..d6e3c0400ec3 100644
--- a/arch/arm/boot/dts/r8a7778.dtsi
+++ b/arch/arm/boot/dts/r8a7778.dtsi
@@ -51,7 +51,8 @@
};
ether: ethernet at fde00000 {
- compatible = "renesas,ether-r8a7778";
+ compatible = "renesas,ether-r8a7778",
+ "renesas,rcar-gen1-ether";
reg = <0xfde00000 0x400>;
interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&mstp1_clks R8A7778_CLK_ETHER>;
--
2.11.0
^ permalink raw reply related
* [PATCH 02/61] ARM: dts: r8a7745: Use R-Car Gen2 Ether fallback compat string
From: Simon Horman @ 2017-12-06 10:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1512555546.git.horms+renesas@verge.net.au>
Use newly added R-Car Gen2 Ether fallback compat string
in the DT of the r8a7745 SoC.
This should have no run-time effect as the driver matches against
the per-SoC compat string before considering the fallback compat string.
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
arch/arm/boot/dts/r8a7745.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/r8a7745.dtsi b/arch/arm/boot/dts/r8a7745.dtsi
index 3a50f703601c..6ad93f0deb8f 100644
--- a/arch/arm/boot/dts/r8a7745.dtsi
+++ b/arch/arm/boot/dts/r8a7745.dtsi
@@ -615,7 +615,8 @@
};
ether: ethernet at ee700000 {
- compatible = "renesas,ether-r8a7745";
+ compatible = "renesas,ether-r8a7745",
+ "renesas,rcar-gen2-ether";
reg = <0 0xee700000 0 0x400>;
interrupts = <GIC_SPI 162 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cpg CPG_MOD 813>;
--
2.11.0
^ permalink raw reply related
* [PATCH 01/61] ARM: dts: r8a7743: Use R-Car Gen2 Ether fallback compat string
From: Simon Horman @ 2017-12-06 10:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1512555546.git.horms+renesas@verge.net.au>
Use newly added R-Car Gen2 Ether fallback compat string
in the DT of the r8a7743 SoC.
This should have no run-time effect as the driver matches against
the per-SoC compat string before considering the fallback compat string.
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
arch/arm/boot/dts/r8a7743.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/r8a7743.dtsi b/arch/arm/boot/dts/r8a7743.dtsi
index 7bbba4a36f31..f647b86c0205 100644
--- a/arch/arm/boot/dts/r8a7743.dtsi
+++ b/arch/arm/boot/dts/r8a7743.dtsi
@@ -827,7 +827,8 @@
};
ether: ethernet at ee700000 {
- compatible = "renesas,ether-r8a7743";
+ compatible = "renesas,ether-r8a7743",
+ "renesas,rcar-gen2-ether";
reg = <0 0xee700000 0 0x400>;
interrupts = <GIC_SPI 162 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cpg CPG_MOD 813>;
--
2.11.0
^ permalink raw reply related
* [PATCH 1/1] soc: renesas: Identify R-Car M3-W ES1.1
From: Simon Horman @ 2017-12-06 10:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1512553442.git.horms+renesas@verge.net.au>
From: Geert Uytterhoeven <geert+renesas@glider.be>
The Product Register of R-Car M3-W ES1.1 incorrectly identifies the SoC
revision as ES2.0. Add a workaround to fix this.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
drivers/soc/renesas/renesas-soc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c
index 9f4ee2567c72..926b7fd6db2d 100644
--- a/drivers/soc/renesas/renesas-soc.c
+++ b/drivers/soc/renesas/renesas-soc.c
@@ -250,6 +250,9 @@ static int __init renesas_soc_init(void)
if (chipid) {
product = readl(chipid);
iounmap(chipid);
+ /* R-Car M3-W ES1.1 incorrectly identifies as ES2.0 */
+ if ((product & 0x7fff) == 0x5210)
+ product ^= 0x11;
if (soc->id && ((product >> 8) & 0xff) != soc->id) {
pr_warn("SoC mismatch (product = 0x%x)\n", product);
return -ENODEV;
--
2.11.0
^ permalink raw reply related
* [GIT PULL] Renesas ARM Based SoC Updates for v4.16
From: Simon Horman @ 2017-12-06 10:21 UTC (permalink / raw)
To: linux-arm-kernel
Hi Olof, Hi Kevin, Hi Arnd,
Please consider these Renesas ARM based SoC updates for v4.16.
The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:
Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)
are available in the git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git tags/renesas-soc-for-v4.16
for you to fetch changes up to 90f0d2b344313a8a4c366ef60d0df33008d2be84:
soc: renesas: Identify R-Car M3-W ES1.1 (2017-11-27 11:40:57 +0100)
----------------------------------------------------------------
Renesas ARM Based SoC Updates for v4.16
* Identify R-Car M3-W ES1.1
Geert Uytterhoeven says "The Product Register of R-Car M3-W ES1.1
incorrectly identifies the SoC revision as ES2.0. Add a workaround to
fix this."
It is my understanding that this is likely to be forwards-compatibile.
----------------------------------------------------------------
Geert Uytterhoeven (1):
soc: renesas: Identify R-Car M3-W ES1.1
drivers/soc/renesas/renesas-soc.c | 3 +++
1 file changed, 3 insertions(+)
^ permalink raw reply
* [PATCH 2/2] arm64: renesas: document V3MSK board bindings
From: Simon Horman @ 2017-12-06 10:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1512553735.git.horms+renesas@verge.net.au>
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Document the V3M Starter Kit device tree bindings, listing it as
a supported board.
This allows to use checkpatch.pl to validate .dts files referring to
the V3MSK board.
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
Documentation/devicetree/bindings/arm/shmobile.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt b/Documentation/devicetree/bindings/arm/shmobile.txt
index 062520a0e74c..5c3af7ef0761 100644
--- a/Documentation/devicetree/bindings/arm/shmobile.txt
+++ b/Documentation/devicetree/bindings/arm/shmobile.txt
@@ -112,6 +112,8 @@ Boards:
compatible = "renesas,sk-rzg1e", "renesas,r8a7745"
- SK-RZG1M (YR8A77430S000BE)
compatible = "renesas,sk-rzg1m", "renesas,r8a7743"
+ - V3MSK
+ compatible = "renesas,v3msk", "renesas,r8a77970"
- Wheat
compatible = "renesas,wheat", "renesas,r8a7792"
--
2.11.0
^ permalink raw reply related
* [PATCH 1/2] ARM: shmobile: Document Renesas M3-W-based Salvator-XS board DT bindings
From: Simon Horman @ 2017-12-06 10:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1512553735.git.horms+renesas@verge.net.au>
From: Geert Uytterhoeven <geert+renesas@glider.be>
The Renesas Salvator-XS (Salvator-X 2nd version) development board can
be equipped with either an R-Car H3 ES2.0 or M3-W ES1.x SiP, which are
pin-compatible.
Document board part number and compatible values for the version with
R-Car M3-W.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
Documentation/devicetree/bindings/arm/shmobile.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt b/Documentation/devicetree/bindings/arm/shmobile.txt
index 020d758fc0c5..062520a0e74c 100644
--- a/Documentation/devicetree/bindings/arm/shmobile.txt
+++ b/Documentation/devicetree/bindings/arm/shmobile.txt
@@ -104,6 +104,8 @@ Boards:
compatible = "renesas,salvator-x", "renesas,r8a7796"
- Salvator-XS (Salvator-X 2nd version, RTP0RC7795SIPB0012S)
compatible = "renesas,salvator-xs", "renesas,r8a7795"
+ - Salvator-XS (Salvator-X 2nd version, RTP0RC7796SIPB0012S)
+ compatible = "renesas,salvator-xs", "renesas,r8a7796"
- SILK (RTP0RC7794LCB00011S)
compatible = "renesas,silk", "renesas,r8a7794"
- SK-RZG1E (YR8A77450S000BE)
--
2.11.0
^ permalink raw reply related
* [GIT PULL] Renesas ARM Based SoC DT Bindings Updates for v4.16
From: Simon Horman @ 2017-12-06 10:21 UTC (permalink / raw)
To: linux-arm-kernel
Hi Olof, Hi Kevin, Hi Arnd,
Please consider these Renesas ARM based SoC DT bindings updates for v4.16.
The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:
Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)
are available in the git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git tags/renesas-dt-bindings-for-v4.16
for you to fetch changes up to eebd0732136fd293c8f15a435978c1c34cd7f32f:
arm64: renesas: document V3MSK board bindings (2017-11-29 09:18:24 +0100)
----------------------------------------------------------------
Renesas ARM Based SoC DT Bindings Updates for v4.16
* Document V3MSK board bindings
These are the bindings for the R-Car V3M Starter Kit
* Document M3-W-based Salvator-XS board bingigns
Geert Uytterhoeven says "The Renesas Salvator-XS (Salvator-X 2nd version)
development board can be equipped with either an R-Car H3 ES2.0 or M3-W
ES1.x SiP, which are pin-compatible."
----------------------------------------------------------------
Geert Uytterhoeven (1):
ARM: shmobile: Document Renesas M3-W-based Salvator-XS board DT bindings
Sergei Shtylyov (1):
arm64: renesas: document V3MSK board bindings
Documentation/devicetree/bindings/arm/shmobile.txt | 4 ++++
1 file changed, 4 insertions(+)
^ permalink raw reply
* [PATCH v2] firmware: qcom: scm: Fix incorrect of_node_put call in scm_init
From: Loys Ollivier @ 2017-12-06 10:21 UTC (permalink / raw)
To: linux-arm-kernel
When using other platform architectures, in the init of the qcom_scm
driver, of_node_put is called on /firmware if no qcom dt is found.
This results in a kernel error: Bad of_node_put() on /firmware.
The call to of_node_put from the qcom_scm init is unnecessary as
of_find_matching_node is calling it automatically.
Remove this of_node_put().
Fixes: d0f6fa7ba2d6 ("firmware: qcom: scm: Convert SCM to platform driver")
Signed-off-by: Loys Ollivier <lollivier@baylibre.com>
---
Change since v1, remove the curly brackets as if statement becomes a
one liner.
drivers/firmware/qcom_scm.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index af4c75217ea6..5beb6a6adcf6 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -632,10 +632,8 @@ static int __init qcom_scm_init(void)
np = of_find_matching_node(fw_np, qcom_scm_dt_match);
- if (!np) {
- of_node_put(fw_np);
+ if (!np)
return -ENODEV;
- }
of_node_put(np);
--
2.7.4
^ permalink raw reply related
* [PATCH] arm64/mm: Do not write ASID generation to ttbr0
From: Julien Thierry @ 2017-12-06 10:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5A26F485.10402@arm.com>
Hi James,
On 05/12/17 19:33, James Morse wrote:
> Hi Julien,
>
> On 05/12/17 17:38, Julien Thierry wrote:
>> When writing the user ASID to ttbr0, 16bit get copied to ttbr0, potentially
>> including part of the ASID generation in the ttbr0.ASID. If the kernel is
>> using less than 16bits ASIDs and the other ttbr0 bits aren't RES0, two
>> tasks using the same mm context might end up running with different
>> ttbr0.ASID values.
>> This would be triggered by one of the threads being scheduled before a
>> roll-over and the second one scheduled after a roll-over.
>>
>> Pad the generation out of the 16bits of the mm id that are written to
>> ttbr0. Thus, what the hardware sees is what the kernel considers ASID.
>
> Is this to fix a system with a mix of 8/16 asid bits? Or someone being clever
> and reducing asid_bits to stress rollover?
>
So it was motivated by the later. But hopefully the fix, pushing the
generation always over 16bits is also suitable for hardware that mixes
8/16bits asids. If it is not, do call it out.
> (I think we should do something like this so we can test rollover.)
>
>
>> diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
>> index 6f40170..a7c72d4 100644
>> --- a/arch/arm64/mm/context.c
>> +++ b/arch/arm64/mm/context.c
>> @@ -180,6 +190,13 @@ static u64 new_context(struct mm_struct *mm, unsigned int cpu)
>> /* We're out of ASIDs, so increment the global generation count */
>> generation = atomic64_add_return_relaxed(ASID_FIRST_VERSION,
>> &asid_generation);
>> +
>> + /*
>> + * It is unlikely the generation will ever overflow, but if this
>> + * happens, let it be known strange things can occur.
>> + */
>> + WARN_ON(generation == ASID_FIRST_VERSION);
>
> Won't generation wrap to zero, not '1<<16'?
Yes it will! Silly me...
>
> asid_generation-is-never-zero is one of the nasty things in this code.
>
> In check_and_switch_context() we switch to the 'fast path' where the current
> asid is usable if: the generation matches and the active_asid isn't 0 (which
> indicates a rollover has occurred).
>
> from mm/context.c::check_and_switch_context():
>> if (!((asid ^ atomic64_read(&asid_generation)) >> asid_bits)
>> && atomic64_xchg_relaxed(&per_cpu(active_asids, cpu), asid))
>> goto switch_mm_fastpath;
>
>
> If asid_generation is ever zero, (even briefly) multiple new tasks with
> different pages-tables will pass the generation test, and run with asid=0.
>
> Short of xchg(ASID_MAX_VERSION, ASID_FIRST_VERSION), every time, just in case, I
> don't think this is something we can handle.
>
> But, this thing is layers on layers of subtle behaviour, so I may have missed
> something...
>
>
I had not thought of that. However I believe we checked with 48bits and
the case of the generation overflowing would take a human life span (or
something on that scale) of a system running and spawning continuous
processes before reaching this. Which is why we decided on having a
WARN_ON for now. So I don't know if we want to change things for this
corner case which really means "things are going bad" more than anything
else.
> Instead, do you think we can duplicate just the asid bits (asid & ASID_MASK)
> into a new 16bit field in mm_context_t, which we then use instead of the ASID()
> and mmid macros? (We only set a new asid in one place as returned by new_context()).
>
I'm not sure I understand why this prevents running with asid = 0 when
generation is 0.
> This would let context.c's asid_bits be arbitrary, as the hardware only uses the
> masked value it exposes in the new field.
>
> This doesn't solve the mixed 8/16 bit case. I think we should force those
> systems to use 8bit asids via cpufeature to preserve as much of the generation
> counter as possible.
>
Hmmm right, I see that today we just panic the kernel when we have a
smaller asid size than the boot cpu...
So if we boot on a 8bit asid cpu it should work but not the other way
around... I agree we probably should find a way to reduce the size of
software asids system wide when we find a cpu has less bits available.
Thanks,
--
Julien Thierry
^ permalink raw reply
* [PATCH v2 5/5] rtc: add mxc driver for i.MX53 SRTC
From: Patrick Brünn @ 2017-12-06 10:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171206083618.eea63zqmpgaaazwl@pengutronix.de>
>From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
>Sent: Mittwoch, 6. Dezember 2017 09:36
>On Tue, Dec 05, 2017 at 03:06:46PM +0100, linux-kernel-dev at beckhoff.com
>wrote:
>> +static int mxc_rtc_write_alarm_locked(struct mxc_rtc_data *const pdata,
>> + struct rtc_time *alarm_tm)
>> +{
>> + void __iomem *const ioaddr = pdata->ioaddr;
>> + unsigned long time;
>> +
>> + rtc_tm_to_time(alarm_tm, &time);
>> +
>> + if (time > U32_MAX) {
>> + pr_err("Hopefully I am out of service by then :-(\n");
>> + return -EINVAL;
>> + }
>
>This will never happen as on your target hardware unsigned long is a
>32bit type. Not sure what is best to do here. Maybe you should test
>the return value of rtc_tm_to_time. ATM it returns 0 unconditionally,
>but rtc_tm_to_time could detect when the input time doesn't fit into
>its return type and return an error in this case.
>Also I just realized that it's unsigned and only overflows in the year
>2106. I'm most likely dead then so I don't care that much ;)
>
please see my response to Alexandre's follow up
>> +/* This function is the RTC interrupt service routine. */
>> +static irqreturn_t mxc_rtc_interrupt(int irq, void *dev_id)
>> +{
>> + struct platform_device *pdev = dev_id;
>> + struct mxc_rtc_data *pdata = platform_get_drvdata(pdev);
>> + void __iomem *ioaddr = pdata->ioaddr;
>> + unsigned long flags;
>> + u32 events = 0;
>> + u32 lp_status;
>> + u32 lp_cr;
>> +
>> + spin_lock_irqsave(&pdata->lock, flags);
>> + if (clk_prepare_enable(pdata->clk)) {
>> + spin_unlock_irqrestore(&pdata->lock, flags);
>> + return IRQ_NONE;
>> + }
>
>You are not allowed to do a clk_prepare under a spinlock. That was the
>original reason to split enabling a clk into clk_prepare and clk_enable.
>Everything that can block is done in clk_prepare and only non blocking
>things are done in clk_enable.
>If you want to enable/disable the clock on demand you can clk_prepare()
>in probe and clk_enable when you actually need it.
>
Thanks for clarification. To be honest when I read Lothar's suggestion it was
the first time I thought about the idea of keeping the clk disabled most of
the time. I am not very experienced with this. But a rtctest loop run for
hours so I assume it would be okay to keep the clk disabled until hw access.
If there is no objection from somebody who knows the i.MX53 SRTC HW
better, I will stick to the clock on demand model and make sure I avoid
blocking.
>> +
>> +static int mxc_rtc_read_time(struct device *dev, struct rtc_time *tm)
>> +{
>> + struct mxc_rtc_data *pdata = dev_get_drvdata(dev);
>> + time_t now;
>> + int ret = mxc_rtc_lock(pdata);
>> +
>> + if (ret)
>> + return ret;
>> +
>> + now = readl(pdata->ioaddr + SRTC_LPSCMR);
>> + rtc_time_to_tm(now, tm);
>> + ret = rtc_valid_tm(tm);
>> + mxc_rtc_unlock(pdata);
>
>I don't think this needs to be locked.
I will change this to only enable the clock for the readl()
>> +static int mxc_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>> +{
>> + struct mxc_rtc_data *pdata = dev_get_drvdata(dev);
>> + int ret = mxc_rtc_lock(pdata);
>> +
>> + if (ret)
>> + return ret;
>> +
>> + ret = mxc_rtc_write_alarm_locked(pdata, &alrm->time);
>
>Is it worth it to make this a separate function?
Maybe not, I think it is an artifact from a refactoring. I will reconsider this
for the next version.
>
>> + if (!ret) {
>> + mxc_rtc_alarm_irq_enable_locked(pdata, alrm->enabled);
>> + mxc_rtc_sync_lp_locked(pdata->ioaddr);
>> + }
>> + mxc_rtc_unlock(pdata);
>> + return ret;
>> +}
>> +
>> +static const struct rtc_class_ops mxc_rtc_ops = {
>> + .read_time = mxc_rtc_read_time,
>> + .set_time = mxc_rtc_set_time,
>> + .read_alarm = mxc_rtc_read_alarm,
>> + .set_alarm = mxc_rtc_set_alarm,
>> + .alarm_irq_enable = mxc_rtc_alarm_irq_enable,
>> +};
>> +
>> +static int mxc_rtc_wait_for_flag(void *__iomem ioaddr, int flag)
>> +{
>> + unsigned int timeout = REG_READ_TIMEOUT;
>> +
>> + while (!(readl(ioaddr) & flag)) {
>> + if (!--timeout) {
>> + pr_err("Wait timeout for 0x%x@%p!\n", flag, ioaddr);
>
>Please use dev_* functions for printing. In this case the message should
>probably be printed from the caller.
Do you have a link at hand about dev_* vs. pr_*? I just choose pr_err here,
because I would have to change the functions signature to get a device.
However, I will drop the message and move it to the caller.
>> + /* clear lp interrupt status */
>> + writel(0xFFFFFFFF, ioaddr + SRTC_LPSR);
>> +
>> + /* move out of init state */
>> + writel((SRTC_LPCR_IE | SRTC_LPCR_NSA), ioaddr + SRTC_LPCR);
>> + xc_rtc_wait_for_flag(ioaddr + SRTC_LPSR, SRTC_LPSR_IES);
>
>If this can fail, shouldn't you test for an error?
very probably
>> +
>> + /* move out of non-valid state */
>> + writel((SRTC_LPCR_IE | SRTC_LPCR_NVE | SRTC_LPCR_NSA |
>> + SRTC_LPCR_EN_LP), ioaddr + SRTC_LPCR);
>> + mxc_rtc_wait_for_flag(ioaddr + SRTC_LPSR, SRTC_LPSR_NVES);
>
>dito
sure
Thanks,
Patrick
---
Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
^ permalink raw reply
* [PATCH] arm64: fpsimd: Prevent registers leaking from dead tasks
From: Ard Biesheuvel @ 2017-12-06 9:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512485802-29489-1-git-send-email-Dave.Martin@arm.com>
On 5 December 2017 at 14:56, Dave Martin <Dave.Martin@arm.com> wrote:
> Currently, loading of a task's fpsimd state into the CPU registers
> is skipped if that task's state is already present in the registers
> of that CPU.
>
> However, the code relies on the struct fpsimd_state * (and by
> extension struct task_struct *) to unambiguously identify a task.
>
> There is a particular case in which this doesn't work reliably:
> when a task exits, its task_struct may be recycled to describe a
> new task.
>
> Consider the following scenario:
>
> 1) Task P loads its fpsimd state onto cpu C.
> per_cpu(fpsimd_last_state, C) := P;
> P->thread.fpsimd_state.cpu := C;
>
> 2) Task X is scheduled onto C and loads its fpsimd state on C.
> per_cpu(fpsimd_last_state, C) := X;
> X->thread.fpsimd_state.cpu := C;
>
> 3) X exits, causing X's task_struct to be freed.
>
> 4) P forks a new child T, which obtains X's recycled task_struct.
> T == X.
> T->thread.fpsimd_state.cpu == C (inherited from P).
>
> 5) T is scheduled on C.
> T's fpsimd state is not loaded, because
> per_cpu(fpsimd_last_state, C) == T (== X) &&
> T->thread.fpsimd_state.cpu == C.
>
> (This is the check performed by fpsimd_thread_switch().)
>
> So, T gets X's registers because the last registers loaded onto C
> were those of X, in (2).
>
> This patch fixes the problem by ensuring that the sched-in check
> fails in (5): fpsimd_flush_task_state(T) is called when T is
> forked, so that T->thread.fpsimd_state.cpu == C cannot be true.
> This relies on the fact that T is not schedulable until after
> copy_thread() completes.
>
> Once T's fpsimd state has been loaded on some CPU C there may still
> be other cpus D for which per_cpu(fpsimd_last_state, D) ==
> &X->thread.fpsimd_state. But D is necessarily != C in this case,
> and the check in (5) must fail.
>
> An alternative fix would be to do refcounting on task_struct. This
> would result in each CPU holding a reference to the last task whose
> fpsimd state was loaded there. It's not clear whether this is
> preferable, and it involves higher overhead than the fix proposed
> in this patch. It would also moves all the task_struct freeing
move
> work into the context switch critical section, or otherwise some
> deferred cleanup mechanism would need to be introduced, neither of
> which seems obviously justified.
>
> Fixes: 005f78cd8849 ("arm64: defer reloading a task's FPSIMD state to userland resume")
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/kernel/process.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index b2adcce..5387d15 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -313,6 +313,7 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start,
> */
> clear_tsk_thread_flag(p, TIF_SVE);
> p->thread.sve_state = NULL;
> + fpsimd_flush_task_state(p);
>
> if (likely(!(p->flags & PF_KTHREAD))) {
> *childregs = *current_pt_regs();
> --
> 2.1.4
>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
^ permalink raw reply
* [kernel-hardening][PATCH v3 1/3] arm: mm: dump: make page table dumping reusable
From: Jinbum Park @ 2017-12-06 9:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGXu5j+UHqJcxygGXk0jXYkK7AHzi1rU5SqvowfYT-OVKc6r5Q@mail.gmail.com>
>> +#ifndef __ASM_PTDUMP_H
>> +#define __ASM_PTDUMP_H
>> +
>> +#ifdef CONFIG_ARM_PTDUMP_CORE
>
> Is this #ifdef needed? I think this file is only included in dump.c
> and ptdump_debugfs.c, both of which are only built when
> CONFIG_ARM_PTDUMP_CORE is defined.
Looking at next patch in this patch-set series ([PATCH v3 3/3] arm:
mm: dump: add checking for writable and executable pages),
Not only dump.c and ptdump_debugfs.c but also arch/arm/mm/init.c
include this file (ptdump.h) to call debug_checkwx().
mm/init.c is not built only when CONFIG_ARM_PTDUMP_CORE is defined.
So, This #ifdef seems not be needed for this patch, but is needed for
this patch-set series.
>> +static int ptdump_init(void)
>> +{
>> + ptdump_initialize();
>> + return ptdump_debugfs_register(&kernel_ptdump_info,
>> + "kernel_page_tables");
>
> This changes the return value of ptdump_init. This should do similar
> to what was done before:
>
> return ptdump_debugfs_register(&kernel_ptdump_info,
> "kernel_page_tables") ? 0 : -ENOMEM;
ptdump_debugfs_register() already returns what you think.
>> +int ptdump_debugfs_register(struct ptdump_info *info, const char *name)
>> +{
>> + struct dentry *pe;
>> +
>> + pe = debugfs_create_file(name, 0400, NULL, info, &ptdump_fops);
>> + return pe ? 0 : -ENOMEM;
>> +
>> +}
So "return ptdump_debugfs_register(~~)" is fine.
Thanks.
Jinbum Park.
^ permalink raw reply
* [PATCH v2 5/5] rtc: add mxc driver for i.MX53 SRTC
From: Patrick Brünn @ 2017-12-06 9:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171206085825.GM21780@piout.net>
>From: Alexandre Belloni [mailto:alexandre.belloni at free-electrons.com]
>Sent: Mittwoch, 6. Dezember 2017 09:58
>On 06/12/2017 at 09:36:18 +0100, Sascha Hauer wrote:
>> > +/*
>> > + * This function updates the RTC alarm registers and then clears all the
>> > + * interrupt status bits.
>> > + * The caller should hold the pdata->lock
>> > + *
>> > + * @param alrm the new alarm value to be updated in the RTC
>> > + *
>> > + * @return 0 if successful; non-zero otherwise.
>> > + */
>> > +static int mxc_rtc_write_alarm_locked(struct mxc_rtc_data *const pdata,
>> > + struct rtc_time *alarm_tm)
>> > +{
>> > + void __iomem *const ioaddr = pdata->ioaddr;
>> > + unsigned long time;
>> > +
>> > + rtc_tm_to_time(alarm_tm, &time);
>> > +
>> > + if (time > U32_MAX) {
>> > + pr_err("Hopefully I am out of service by then :-(\n");
>> > + return -EINVAL;
>> > + }
>>
>> This will never happen as on your target hardware unsigned long is a
>> 32bit type. Not sure what is best to do here. Maybe you should test
>> the return value of rtc_tm_to_time. ATM it returns 0 unconditionally,
>> but rtc_tm_to_time could detect when the input time doesn't fit into
>> its return type and return an error in this case.
>> Also I just realized that it's unsigned and only overflows in the year
>> 2106. I'm most likely dead then so I don't care that much ;)
>>
>
>One solution is to use the 64bit version instead so it doesn't overflow.
>This makes the time > U32_MAX work.
>Also, I'll send (hopefully soon) a series adding proper range checking
>for the whole RTC subsystem. And yes, it not urgent as I don't think I
>will care so much in 2106 too ;)
>
I just noticed that in mxc_rtc_set_time() I am using the 64bit version.
After thinking a while about this issue, I think the 64bit version is better
suited for my use case. It makes explicitly clear that I need to push the
time into a 32bit hw register and "unsigned long" is just by accident the
correct size for me.
>> > +/*
>> > + * This function reads the current RTC time into tm in Gregorian date.
>> > + *
>> > + * @param tm contains the RTC time value upon return
>> > + *
>> > + * @return 0 if successful; non-zero otherwise.
>> > + */
>> > +static int mxc_rtc_read_time(struct device *dev, struct rtc_time *tm)
>> > +{
>> > + struct mxc_rtc_data *pdata = dev_get_drvdata(dev);
>> > + time_t now;
>> > + int ret = mxc_rtc_lock(pdata);
>> > +
>> > + if (ret)
>> > + return ret;
>> > +
>> > + now = readl(pdata->ioaddr + SRTC_LPSCMR);
>> > + rtc_time_to_tm(now, tm);
>> > + ret = rtc_valid_tm(tm);
>
>This check is useless for two reasons: you know that rtc_time_to_tm will
>generate a valid tm and the core always checks the tm anyway.
I will remove this with the next version
Thanks for your time and help,
Patrick
Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
^ permalink raw reply
* [PATCH 1/2] drm/nouveau/bar/gf100: fix hang when calling ->fini() before ->init()
From: Guillaume Tucker @ 2017-12-06 9:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CABDvA=krahNw3q+qpEPbbv8Y9fvX4jXc_B54qafwtgFyWpqKWg@mail.gmail.com>
On 05/12/17 18:32, Ben Skeggs wrote:
> On Wed, Dec 6, 2017 at 12:30 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
>
>>
>> On 04/12/17 18:37, Guillaume Tucker wrote:
>>> If the firmware fails to load then ->fini() will be called before the
>>> device has been initialised, causing the kernel to hang while trying
>>> to write to a register. Add a test in ->fini() to avoid this issue.
>>>
>>> This fixes a kernel hang on tegra124.
>>>
>>> Fixes: b17de35a2ebbe ("drm/nouveau/bar: implement bar1 teardown")
>>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
>>> CC: Ben Skeggs <bskeggs@redhat.com>
>>> ---
>>> drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c | 7 +++++--
>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c
>> b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c
>>> index a3ba7f50198b..95e2aba64aad 100644
>>> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c
>>> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c
>>> @@ -43,9 +43,12 @@ gf100_bar_bar1_wait(struct nvkm_bar *base)
>>> }
>>>
>>> void
>>> -gf100_bar_bar1_fini(struct nvkm_bar *bar)
>>> +gf100_bar_bar1_fini(struct nvkm_bar *base)
>>> {
>>> - nvkm_mask(bar->subdev.device, 0x001704, 0x80000000, 0x00000000);
>>> + struct nvkm_device *device = base->subdev.device;
>>> +
>>> + if (base->subdev.oneinit)
>>> + nvkm_mask(device, 0x001704, 0x80000000, 0x00000000);
>>> }
>>>
>>> void
>>
>> I have tested this and it works for me. Thanks for fixing this! Would be
>> good to get Ben's ACK, but you can have my ...
>>
> I'd love to get a good explanation as to why it hangs without this change,
> as, on the surface, it's not immediately obvious as to why it's hanging.
To be fair I'm not entirely sure either why this causes a hang, I
haven't read the TRM... The iomem has been mapped at this point,
so accessing the register should work. One clue is when you look
at _bar1_init(), the 0x1704 register is initialised with
some (device instance?) memory address. So it's possible that
the hardware does something special when you set this to 0 as in
_bar1_fini(), which may fail in particular if it was previously
not initialised with a valid address.
This is merely guesswork, would be interested to find out the
real explanation though.
>> Tested-by: Jon Hunter <jonathanh@nvidia.com>
Thanks!
Guillaume
^ permalink raw reply
* [kernel-hardening][PATCH v3 3/3] arm: mm: dump: add checking for writable and executable pages
From: Jinbum Park @ 2017-12-06 9:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <82ab0116-ac67-c80a-73d5-a812e38eb547@redhat.com>
2017-12-06 8:51 GMT+09:00 Laura Abbott <labbott@redhat.com>:
> On 12/04/2017 06:27 AM, Jinbum Park wrote:
>>
>> Page mappings with full RWX permissions are a security risk.
>> x86, arm64 has an option to walk the page tables
>> and dump any bad pages.
>>
>> (1404d6f13e47
>> ("arm64: dump: Add checking for writable and exectuable pages"))
>> Add a similar implementation for arm.
>>
>> Signed-off-by: Jinbum Park <jinb.park7@gmail.com>
>> ---
>> v3: Reuse pg_level, prot_bits to check ro, nx prot.
>>
>> arch/arm/Kconfig.debug | 27 +++++++++++++++++++++++
>> arch/arm/include/asm/ptdump.h | 8 +++++++
>> arch/arm/mm/dump.c | 51
>> +++++++++++++++++++++++++++++++++++++++++++
>> arch/arm/mm/init.c | 2 ++
>> 4 files changed, 88 insertions(+)
>>
>> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
>> index e7b94db..78a6470 100644
>> --- a/arch/arm/Kconfig.debug
>> +++ b/arch/arm/Kconfig.debug
>> @@ -20,6 +20,33 @@ config ARM_PTDUMP_DEBUGFS
>> kernel.
>> If in doubt, say "N"
>> +config DEBUG_WX
>> + bool "Warn on W+X mappings at boot"
>> + select ARM_PTDUMP_CORE
>> + ---help---
>> + Generate a warning if any W+X mappings are found at boot.
>> +
>> + This is useful for discovering cases where the kernel is
>> leaving
>> + W+X mappings after applying NX, as such mappings are a
>> security risk.
>> +
>> + Look for a message in dmesg output like this:
>> +
>> + arm/mm: Checked W+X mappings: passed, no W+X pages
>> found.
>> +
>> + or like this, if the check failed:
>> +
>> + arm/mm: Checked W+X mappings: FAILED, <N> W+X
>> pages found.
>> +
>> + Note that even if the check fails, your kernel is possibly
>> + still fine, as W+X mappings are not a security hole in
>> + themselves, what they do is that they make the
>> exploitation
>> + of other unfixed kernel bugs easier.
>> +
>> + There is no runtime or memory usage effect of this option
>> + once the kernel has booted up - it's a one time check.
>> +
>> + If in doubt, say "Y".
>> +
>> # RMK wants arm kernels compiled with frame pointers or stack unwinding.
>> # If you know what you are doing and are willing to live without stack
>> # traces, you can get a slightly smaller kernel by setting this option
>> to
>> diff --git a/arch/arm/include/asm/ptdump.h b/arch/arm/include/asm/ptdump.h
>> index 3a6c0b7..b6a0162 100644
>> --- a/arch/arm/include/asm/ptdump.h
>> +++ b/arch/arm/include/asm/ptdump.h
>> @@ -43,6 +43,14 @@ static inline int ptdump_debugfs_register(struct
>> ptdump_info *info,
>> }
>> #endif /* CONFIG_ARM_PTDUMP_DEBUGFS */
>> +void ptdump_check_wx(void);
>> +
>> #endif /* CONFIG_ARM_PTDUMP_CORE */
>> +#ifdef CONFIG_DEBUG_WX
>> +#define debug_checkwx() ptdump_check_wx()
>> +#else
>> +#define debug_checkwx() do { } while (0)
>> +#endif
>> +
>> #endif /* __ASM_PTDUMP_H */
>> diff --git a/arch/arm/mm/dump.c b/arch/arm/mm/dump.c
>> index 43a2bee..3e2e6f0 100644
>> --- a/arch/arm/mm/dump.c
>> +++ b/arch/arm/mm/dump.c
>> @@ -52,6 +52,8 @@ struct pg_state {
>> unsigned long start_address;
>> unsigned level;
>> u64 current_prot;
>> + bool check_wx;
>> + unsigned long wx_pages;
>> const char *current_domain;
>> };
>> @@ -194,6 +196,8 @@ struct pg_level {
>> const struct prot_bits *bits;
>> size_t num;
>> u64 mask;
>> + const struct prot_bits *ro_bit;
>> + const struct prot_bits *nx_bit;
>> };
It looks better~:)
I'll prepare new version.
>> static struct pg_level pg_level[] = {
>> @@ -203,9 +207,17 @@ struct pg_level {
>> }, { /* pmd */
>> .bits = section_bits,
>> .num = ARRAY_SIZE(section_bits),
>> + #ifdef CONFIG_ARM_LPAE
>> + .ro_bit = section_bits + 1,
>> + #else
>> + .ro_bit = section_bits,
>> + #endif
>> + .nx_bit = section_bits + ARRAY_SIZE(section_bits) - 2,
>> }, { /* pte */
>> .bits = pte_bits,
>> .num = ARRAY_SIZE(pte_bits),
>> + .ro_bit = pte_bits + 1,
>> + .nx_bit = pte_bits + 2,
>> },
>> };
>>
>
>
> This is better but the addition offset from the array is still
> prone to breakage if we add entries. Maybe something like this
> on top of yours:
>
> diff --git a/arch/arm/mm/dump.c b/arch/arm/mm/dump.c
> index 3e2e6f06e4d9..572cbc4dc247 100644
> --- a/arch/arm/mm/dump.c
> +++ b/arch/arm/mm/dump.c
> @@ -62,6 +62,8 @@ struct prot_bits {
> u64 val;
> const char *set;
> const char *clear;
> + bool ro_bit;
> + bool x_bit;
> };
> static const struct prot_bits pte_bits[] = {
> @@ -75,11 +77,13 @@ static const struct prot_bits pte_bits[] = {
> .val = L_PTE_RDONLY,
> .set = "ro",
> .clear = "RW",
> + .ro_bit = true,
> }, {
> .mask = L_PTE_XN,
> .val = L_PTE_XN,
> .set = "NX",
> .clear = "x ",
> + .x_bit = true,
> }, {
> .mask = L_PTE_SHARED,
> .val = L_PTE_SHARED,
> @@ -143,11 +147,13 @@ static const struct prot_bits section_bits[] = {
> .val = L_PMD_SECT_RDONLY | PMD_SECT_AP2,
> .set = "ro",
> .clear = "RW",
> + .ro_bit = true,
> #elif __LINUX_ARM_ARCH__ >= 6
> {
> .mask = PMD_SECT_APX | PMD_SECT_AP_READ |
> PMD_SECT_AP_WRITE,
> .val = PMD_SECT_APX | PMD_SECT_AP_WRITE,
> .set = " ro",
> + .ro_bit = true,
> }, {
> .mask = PMD_SECT_APX | PMD_SECT_AP_READ |
> PMD_SECT_AP_WRITE,
> .val = PMD_SECT_AP_WRITE,
> @@ -166,6 +172,7 @@ static const struct prot_bits section_bits[] = {
> .mask = PMD_SECT_AP_READ | PMD_SECT_AP_WRITE,
> .val = 0,
> .set = " ro",
> + .ro_bit = true,
> }, {
> .mask = PMD_SECT_AP_READ | PMD_SECT_AP_WRITE,
> .val = PMD_SECT_AP_WRITE,
> @@ -184,6 +191,7 @@ static const struct prot_bits section_bits[] = {
> .val = PMD_SECT_XN,
> .set = "NX",
> .clear = "x ",
> + .x_bit = true,
> }, {
> .mask = PMD_SECT_S,
> .val = PMD_SECT_S,
> @@ -207,17 +215,9 @@ static struct pg_level pg_level[] = {
> }, { /* pmd */
> .bits = section_bits,
> .num = ARRAY_SIZE(section_bits),
> - #ifdef CONFIG_ARM_LPAE
> - .ro_bit = section_bits + 1,
> - #else
> - .ro_bit = section_bits,
> - #endif
> - .nx_bit = section_bits + ARRAY_SIZE(section_bits) - 2,
> }, { /* pte */
> .bits = pte_bits,
> .num = ARRAY_SIZE(pte_bits),
> - .ro_bit = pte_bits + 1,
> - .nx_bit = pte_bits + 2,
> },
> };
> @@ -410,8 +410,13 @@ static void ptdump_initialize(void)
> for (i = 0; i < ARRAY_SIZE(pg_level); i++)
> if (pg_level[i].bits)
> - for (j = 0; j < pg_level[i].num; j++)
> + for (j = 0; j < pg_level[i].num; j++) {
> pg_level[i].mask |=
> pg_level[i].bits[j].mask;
> + if (pg_level[i].bits[j].ro_bit)
> + pg_level[i].ro_bit =
> &pg_level[i].bits[j];
> + if (pg_level[i].bits[j].x_bit)
> + pg_level[i].nx_bit =
> &pg_level[i].bits[j];
> + }
> address_markers[2].start_address = VMALLOC_START;
> }
>
>
>
>
>>
>> @@ -226,6 +238,23 @@ static void dump_prot(struct pg_state *st, const
>> struct prot_bits *bits, size_t
>> }
>> }
>> +static void note_prot_wx(struct pg_state *st, unsigned long addr)
>> +{
>> + if (!st->check_wx)
>> + return;
>> + if ((st->current_prot & pg_level[st->level].ro_bit->mask) ==
>> + pg_level[st->level].ro_bit->val)
>> + return;
>> + if ((st->current_prot & pg_level[st->level].nx_bit->mask) ==
>> + pg_level[st->level].nx_bit->val)
>> + return;
>> +
>> + WARN_ONCE(1, "arm/mm: Found insecure W+X mapping at address
>> %p/%pS\n",
>> + (void *)st->start_address, (void *)st->start_address);
>> +
>
>
> With the new %p hashing, printing just %p is not useful, so just drop
> it and just have the %pS.
ok, I'll drop the %p.
>
> Thanks,
> Laura
>
^ permalink raw reply
* [PATCH v2 5/5] rtc: add mxc driver for i.MX53 SRTC
From: Alexandre Belloni @ 2017-12-06 8:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171206083618.eea63zqmpgaaazwl@pengutronix.de>
On 06/12/2017 at 09:36:18 +0100, Sascha Hauer wrote:
> > +/*
> > + * This function updates the RTC alarm registers and then clears all the
> > + * interrupt status bits.
> > + * The caller should hold the pdata->lock
> > + *
> > + * @param alrm the new alarm value to be updated in the RTC
> > + *
> > + * @return 0 if successful; non-zero otherwise.
> > + */
> > +static int mxc_rtc_write_alarm_locked(struct mxc_rtc_data *const pdata,
> > + struct rtc_time *alarm_tm)
> > +{
> > + void __iomem *const ioaddr = pdata->ioaddr;
> > + unsigned long time;
> > +
> > + rtc_tm_to_time(alarm_tm, &time);
> > +
> > + if (time > U32_MAX) {
> > + pr_err("Hopefully I am out of service by then :-(\n");
> > + return -EINVAL;
> > + }
>
> This will never happen as on your target hardware unsigned long is a
> 32bit type. Not sure what is best to do here. Maybe you should test
> the return value of rtc_tm_to_time. ATM it returns 0 unconditionally,
> but rtc_tm_to_time could detect when the input time doesn't fit into
> its return type and return an error in this case.
> Also I just realized that it's unsigned and only overflows in the year
> 2106. I'm most likely dead then so I don't care that much ;)
>
One solution is to use the 64bit version instead so it doesn't overflow.
This makes the time > U32_MAX work.
Also, I'll send (hopefully soon) a series adding proper range checking
for the whole RTC subsystem. And yes, it not urgent as I don't think I
will care so much in 2106 too ;)
> > +/*
> > + * This function reads the current RTC time into tm in Gregorian date.
> > + *
> > + * @param tm contains the RTC time value upon return
> > + *
> > + * @return 0 if successful; non-zero otherwise.
> > + */
> > +static int mxc_rtc_read_time(struct device *dev, struct rtc_time *tm)
> > +{
> > + struct mxc_rtc_data *pdata = dev_get_drvdata(dev);
> > + time_t now;
> > + int ret = mxc_rtc_lock(pdata);
> > +
> > + if (ret)
> > + return ret;
> > +
> > + now = readl(pdata->ioaddr + SRTC_LPSCMR);
> > + rtc_time_to_tm(now, tm);
> > + ret = rtc_valid_tm(tm);
This check is useless for two reasons: you know that rtc_time_to_tm will
generate a valid tm and the core always checks the tm anyway.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v2 0/4] arm64: defconfig: enable additional led triggers
From: Amit Kucheria @ 2017-12-06 8:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1509951550.git.amit.kucheria@linaro.org>
(Adding Arnd)
Now that the merge window rush has abated, can you please apply this
trivial series?
On Mon, Nov 6, 2017 at 12:38 PM, Amit Kucheria <amit.kucheria@linaro.org> wrote:
> This patchset enables the kernel panic and disk-activity trigger for LEDs
> and then enables the panic trigger for three 96Boards - DB410c, Hikey and
> Hikey960.
>
> DB410c and Hikey panic behaviour has been tested by triggering a panic
> through /proc/sysrq-trigger, while Hikey960 is only compile-tested.
>
>
> Amit Kucheria (4):
> arm64: defconfig: enable new trigger modes for leds
> arm64: dts: qcom: apq8016-sbc: Allow USR4 LED to notify kernel panic
> arm64: dts: hisilicon: hi6220-hikey: Allow USR1 LED to notify kernel
> panic
> arm64: dts: hisilicon: hi3660-hikey960: Allow USR4 LED to notify
> kernel panic
>
> arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 1 +
> arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 1 +
> arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 1 +
> arch/arm64/configs/defconfig | 2 ++
> 4 files changed, 5 insertions(+)
>
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v6 6/8] KVM: arm/arm64: Support VGIC dist pend/active changes for mapped IRQs
From: Marc Zyngier @ 2017-12-06 8:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171205223900.znmruf3pfpg3pvqa@yury-thinkpad>
On 05/12/17 22:39, Yury Norov wrote:
> On Tue, Dec 05, 2017 at 04:47:46PM +0000, Marc Zyngier wrote:
>> On 05/12/17 15:03, Yury Norov wrote:
>>> On Mon, Dec 04, 2017 at 09:05:04PM +0100, Christoffer Dall wrote:
>>>> From: Christoffer Dall <christoffer.dall@linaro.org>
>>>>
>>>> For mapped IRQs (with the HW bit set in the LR) we have to follow some
>>>> rules of the architecture. One of these rules is that VM must not be
>>>> allowed to deactivate a virtual interrupt with the HW bit set unless the
>>>> physical interrupt is also active.
>>>>
>>>> This works fine when injecting mapped interrupts, because we leave it up
>>>> to the injector to either set EOImode==1 or manually set the active
>>>> state of the physical interrupt.
>>>>
>>>> However, the guest can set virtual interrupt to be pending or active by
>>>> writing to the virtual distributor, which could lead to deactivating a
>>>> virtual interrupt with the HW bit set without the physical interrupt
>>>> being active.
>>>>
>>>> We could set the physical interrupt to active whenever we are about to
>>>> enter the VM with a HW interrupt either pending or active, but that
>>>> would be really slow, especially on GICv2. So we take the long way
>>>> around and do the hard work when needed, which is expected to be
>>>> extremely rare.
>>>>
>>>> When the VM sets the pending state for a HW interrupt on the virtual
>>>> distributor we set the active state on the physical distributor, because
>>>> the virtual interrupt can become active and then the guest can
>>>> deactivate it.
>>>>
>>>> When the VM clears the pending state we also clear it on the physical
>>>> side, because the injector might otherwise raise the interrupt. We also
>>>> clear the physical active state when the virtual interrupt is not
>>>> active, since otherwise a SPEND/CPEND sequence from the guest would
>>>> prevent signaling of future interrupts.
>>>>
>>>> Changing the state of mapped interrupts from userspace is not supported,
>>>> and it's expected that userspace unmaps devices from VFIO before
>>>> attempting to set the interrupt state, because the interrupt state is
>>>> driven by hardware.
>>>>
>>>> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
>>>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
>>>> ---
>>>> virt/kvm/arm/vgic/vgic-mmio.c | 71 +++++++++++++++++++++++++++++++++++++++----
>>>> virt/kvm/arm/vgic/vgic.c | 7 +++++
>>>> virt/kvm/arm/vgic/vgic.h | 1 +
>>>> 3 files changed, 73 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
>>>> index 747b0a3b4784..8d173d99a7a4 100644
>>>> --- a/virt/kvm/arm/vgic/vgic-mmio.c
>>>> +++ b/virt/kvm/arm/vgic/vgic-mmio.c
>>>> @@ -16,6 +16,7 @@
>>>> #include <linux/kvm.h>
>>>> #include <linux/kvm_host.h>
>>>> #include <kvm/iodev.h>
>>>> +#include <kvm/arm_arch_timer.h>
>>>> #include <kvm/arm_vgic.h>
>>>>
>>>> #include "vgic.h"
>>>> @@ -143,10 +144,22 @@ static struct kvm_vcpu *vgic_get_mmio_requester_vcpu(void)
>>>> return vcpu;
>>>> }
>>>>
>>>> +/* Must be called with irq->irq_lock held */
>>>
>>> Instead of enforcing this rule in comment, you can enforce it in code:
>>>
>>> BUG_ON(!spin_is_locked(irq->irq_lock))
>>
>> Are we going to litter the kernel with such assertions? I don't think
>> that's a valuable thing to do.
>
> That's what I agree - BUG-ifiyng is somewhat debugging technique and
> should be avoided in release code. But as I can see, in kvm code BUG()s
> are widely used:
> $ find . -name "*.c" | xargs grep -w 'BUG\|BUG_ON' | grep kvm | wc -l
> 155
>
> So I tuned my littering detector. :)
>
> In this patchset new BUG()s are added in patches 4 and 6. In patch 6
> BUG() has meaning of TODO:
>
> + if (vintid == vcpu_vtimer(vcpu)->irq.irq)
> + timer = vcpu_vtimer(vcpu);
> + else
> + BUG(); /* We only map the vtimer so far */
> +
>
> Which is far from original purpose of BUG().
>
> If you think that BUG() is not OK in this case (and I agree with it),
> I think they should be also removed from patches 4 and 6. 6 - for sure.
There is a small, but important difference here. Your earlier suggestion
had the implication that we should check for the irq->irq_lock on all
code paths, as there is no point in only checking it in a single
function. That assertion must be true in a lot of places in the vgic
code, including places that are on the fast path.
This would turn the code into a pretty horrible mess, and bring very
little to the table (unless you consider slowing down your system to be
a feature). Also, we have better ways of detecting locking issues, such
as lockdep.
On the other hand, the BUG you quote above check for a condition that
only makes sense in this particular function.
So I'd suggest that instead of blindly counting the number of assertion,
you look at whether that makes sense at that particular point. Yes, we
probably have too many BUG_ONs, and we usually weed them after gaining
some confidence that the code is sane. For new code, a BUG_ON is a very
reassuring point that we don't hit anything unexpected.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v2 5/5] rtc: add mxc driver for i.MX53 SRTC
From: Sascha Hauer @ 2017-12-06 8:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171205140646.30367-6-linux-kernel-dev@beckhoff.com>
On Tue, Dec 05, 2017 at 03:06:46PM +0100, linux-kernel-dev at beckhoff.com wrote:
> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>
> Neither rtc-imxdi, rtc-mxc nor rtc-snvs are compatible with i.MX53.
>
> This is driver enables support for the low power domain SRTC features:
> - 32-bit MSB of non-rollover time counter
> - 32-bit alarm register
>
> Based on:
> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/rtc/rtc-mxc_v2.c?h=imx_2.6.35_11.09.01
>
> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
>
> ---
>
> v2:
> - have seperate patches for dt-binding, CONFIG option, imx53.dtsi and driver
> - add SPDX-License-Identifier and cleanup copyright notice
> - replace __raw_readl/writel() with readl/writel()
> - fix PM_SLEEP callbacks
> - add CONFIG_RTC_DRV_MXC_V2 to build rtc-mxc_v2.c
> - remove misleading or obvious comments and fix style of the remaining
> - avoid endless loop while waiting for hw
> - implement consistent locking; make spinlock a member of dev struct
> - enable clk only for register accesses
> - remove all udelay() calls since they are obsolete or redundant
> (we are already waiting for register flags to change)
> - init platform_data before registering irq callback
> - let set_time() fail, when 32 bit rtc counter exceeded
> - make names more consistent
> - cleanup and reorder includes
> - cleanup and remove unused defines
>
> To: Alessandro Zummo <a.zummo@towertech.it>
> To: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
> Cc: linux-rtc at vger.kernel.org (open list:REAL TIME CLOCK (RTC) SUBSYSTEM)
> Cc: devicetree at vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
> Cc: linux-kernel at vger.kernel.org (open list)
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Juergen Borleis <jbe@pengutronix.de>
> Cc: Noel Vellemans <Noel.Vellemans@visionbms.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de> (maintainer:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE)
> Cc: Russell King <linux@armlinux.org.uk> (maintainer:ARM PORT)
> Cc: linux-arm-kernel at lists.infradead.org (moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE)
>
> Cc: Philippe Ombredanne <pombredanne@nexb.com>
> Cc: Lothar Wa?mann <LW@KARO-electronics.de>
> ---
> drivers/rtc/Makefile | 1 +
> drivers/rtc/rtc-mxc_v2.c | 433 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 434 insertions(+)
> create mode 100644 drivers/rtc/rtc-mxc_v2.c
>
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index f2f50c11dc38..dcf60e61ae5c 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -106,6 +106,7 @@ obj-$(CONFIG_RTC_DRV_MT6397) += rtc-mt6397.o
> obj-$(CONFIG_RTC_DRV_MT7622) += rtc-mt7622.o
> obj-$(CONFIG_RTC_DRV_MV) += rtc-mv.o
> obj-$(CONFIG_RTC_DRV_MXC) += rtc-mxc.o
> +obj-$(CONFIG_RTC_DRV_MXC_V2) += rtc-mxc_v2.o
> obj-$(CONFIG_RTC_DRV_NUC900) += rtc-nuc900.o
> obj-$(CONFIG_RTC_DRV_OMAP) += rtc-omap.o
> obj-$(CONFIG_RTC_DRV_OPAL) += rtc-opal.o
> diff --git a/drivers/rtc/rtc-mxc_v2.c b/drivers/rtc/rtc-mxc_v2.c
> new file mode 100644
> index 000000000000..c5a6d2c293bb
> --- /dev/null
> +++ b/drivers/rtc/rtc-mxc_v2.c
> @@ -0,0 +1,433 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Real Time Clock (RTC) Driver for i.MX53
> + * Copyright (c) 2004-2011 Freescale Semiconductor, Inc.
> + * Copyright (c) 2017 Beckhoff Automation GmbH & Co. KG
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/rtc.h>
> +
> +#define SRTC_LPPDR_INIT 0x41736166 /* init for glitch detect */
> +
> +#define SRTC_LPCR_EN_LP BIT(3) /* lp enable */
> +#define SRTC_LPCR_WAE BIT(4) /* lp wakeup alarm enable */
> +#define SRTC_LPCR_ALP BIT(7) /* lp alarm flag */
> +#define SRTC_LPCR_NSA BIT(11) /* lp non secure access */
> +#define SRTC_LPCR_NVE BIT(14) /* lp non valid state exit bit */
> +#define SRTC_LPCR_IE BIT(15) /* lp init state exit bit */
> +
> +#define SRTC_LPSR_ALP BIT(3) /* lp alarm flag */
> +#define SRTC_LPSR_NVES BIT(14) /* lp non-valid state exit status */
> +#define SRTC_LPSR_IES BIT(15) /* lp init state exit status */
> +
> +#define SRTC_LPSCMR 0x00 /* LP Secure Counter MSB Reg */
> +#define SRTC_LPSCLR 0x04 /* LP Secure Counter LSB Reg */
> +#define SRTC_LPSAR 0x08 /* LP Secure Alarm Reg */
> +#define SRTC_LPCR 0x10 /* LP Control Reg */
> +#define SRTC_LPSR 0x14 /* LP Status Reg */
> +#define SRTC_LPPDR 0x18 /* LP Power Supply Glitch Detector Reg */
> +
> +/* max. number of retries to read registers, 120 was max during test */
> +#define REG_READ_TIMEOUT 2000
> +
> +struct mxc_rtc_data {
> + struct rtc_device *rtc;
> + void __iomem *ioaddr;
> + struct clk *clk;
> + spinlock_t lock; /* protects register access */
> + int irq;
> +};
> +
> +/*
> + * This function does write synchronization for writes to the lp srtc block.
> + * To take care of the asynchronous CKIL clock, all writes from the IP domain
> + * will be synchronized to the CKIL domain.
> + * The caller should hold the pdata->lock
> + */
> +static inline void mxc_rtc_sync_lp_locked(void __iomem *ioaddr)
> +{
> + unsigned int i;
> +
> + /* Wait for 3 CKIL cycles */
> + for (i = 0; i < 3; i++) {
> + const u32 count = readl(ioaddr + SRTC_LPSCLR);
> + unsigned int timeout = REG_READ_TIMEOUT;
> +
> + while ((readl(ioaddr + SRTC_LPSCLR)) == count) {
> + if (!--timeout) {
> + pr_err("SRTC_LPSCLR stuck! Check your hw.\n");
> + return;
> + }
> + }
> + }
> +}
> +
> +/*
> + * This function updates the RTC alarm registers and then clears all the
> + * interrupt status bits.
> + * The caller should hold the pdata->lock
> + *
> + * @param alrm the new alarm value to be updated in the RTC
> + *
> + * @return 0 if successful; non-zero otherwise.
> + */
> +static int mxc_rtc_write_alarm_locked(struct mxc_rtc_data *const pdata,
> + struct rtc_time *alarm_tm)
> +{
> + void __iomem *const ioaddr = pdata->ioaddr;
> + unsigned long time;
> +
> + rtc_tm_to_time(alarm_tm, &time);
> +
> + if (time > U32_MAX) {
> + pr_err("Hopefully I am out of service by then :-(\n");
> + return -EINVAL;
> + }
This will never happen as on your target hardware unsigned long is a
32bit type. Not sure what is best to do here. Maybe you should test
the return value of rtc_tm_to_time. ATM it returns 0 unconditionally,
but rtc_tm_to_time could detect when the input time doesn't fit into
its return type and return an error in this case.
Also I just realized that it's unsigned and only overflows in the year
2106. I'm most likely dead then so I don't care that much ;)
> +
> + writel((u32)time, ioaddr + SRTC_LPSAR);
> +
> + /* clear alarm interrupt status bit */
> + writel(SRTC_LPSR_ALP, ioaddr + SRTC_LPSR);
> +
> + mxc_rtc_sync_lp_locked(ioaddr);
> + return 0;
> +}
> +
> +/* This function is the RTC interrupt service routine. */
> +static irqreturn_t mxc_rtc_interrupt(int irq, void *dev_id)
> +{
> + struct platform_device *pdev = dev_id;
> + struct mxc_rtc_data *pdata = platform_get_drvdata(pdev);
> + void __iomem *ioaddr = pdata->ioaddr;
> + unsigned long flags;
> + u32 events = 0;
> + u32 lp_status;
> + u32 lp_cr;
> +
> + spin_lock_irqsave(&pdata->lock, flags);
> + if (clk_prepare_enable(pdata->clk)) {
> + spin_unlock_irqrestore(&pdata->lock, flags);
> + return IRQ_NONE;
> + }
You are not allowed to do a clk_prepare under a spinlock. That was the
original reason to split enabling a clk into clk_prepare and clk_enable.
Everything that can block is done in clk_prepare and only non blocking
things are done in clk_enable.
If you want to enable/disable the clock on demand you can clk_prepare()
in probe and clk_enable when you actually need it.
> +
> + lp_status = readl(ioaddr + SRTC_LPSR);
> + lp_cr = readl(ioaddr + SRTC_LPCR);
> +
> + /* update irq data & counter */
> + if (lp_status & SRTC_LPSR_ALP) {
> + if (lp_cr & SRTC_LPCR_ALP)
> + events = (RTC_AF | RTC_IRQF);
> +
> + /* disable further lp alarm interrupts */
> + lp_cr &= ~(SRTC_LPCR_ALP | SRTC_LPCR_WAE);
> + }
> +
> + /* Update interrupt enables */
> + writel(lp_cr, ioaddr + SRTC_LPCR);
> +
> + /* clear interrupt status */
> + writel(lp_status, ioaddr + SRTC_LPSR);
> +
> + mxc_rtc_sync_lp_locked(ioaddr);
> + rtc_update_irq(pdata->rtc, 1, events);
> + clk_disable_unprepare(pdata->clk);
> + spin_unlock_irqrestore(&pdata->lock, flags);
> + return IRQ_HANDLED;
> +}
> +
> +/*
> + * Enable clk and aquire spinlock
> + * @return 0 if successful; non-zero otherwise.
> + */
> +static int mxc_rtc_lock(struct mxc_rtc_data *const pdata)
> +{
> + int ret;
> +
> + spin_lock_irq(&pdata->lock);
> + ret = clk_prepare_enable(pdata->clk);
> + if (ret) {
> + spin_unlock_irq(&pdata->lock);
> + return ret;
> + }
> + return 0;
> +}
> +
> +static int mxc_rtc_unlock(struct mxc_rtc_data *const pdata)
> +{
> + clk_disable_unprepare(pdata->clk);
> + spin_unlock_irq(&pdata->lock);
> + return 0;
> +}
> +
> +/*
> + * This function reads the current RTC time into tm in Gregorian date.
> + *
> + * @param tm contains the RTC time value upon return
> + *
> + * @return 0 if successful; non-zero otherwise.
> + */
> +static int mxc_rtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> + struct mxc_rtc_data *pdata = dev_get_drvdata(dev);
> + time_t now;
> + int ret = mxc_rtc_lock(pdata);
> +
> + if (ret)
> + return ret;
> +
> + now = readl(pdata->ioaddr + SRTC_LPSCMR);
> + rtc_time_to_tm(now, tm);
> + ret = rtc_valid_tm(tm);
> + mxc_rtc_unlock(pdata);
I don't think this needs to be locked.
> + return ret;
> +}
> +
> +/*
> + * This function sets the internal RTC time based on tm in Gregorian date.
> + *
> + * @param tm the time value to be set in the RTC
> + *
> + * @return 0 if successful; non-zero otherwise.
> + */
> +static int mxc_rtc_set_time(struct device *dev, struct rtc_time *tm)
> +{
> + struct mxc_rtc_data *pdata = dev_get_drvdata(dev);
> + time64_t time = rtc_tm_to_time64(tm);
> + int ret;
> +
> + if (time > U32_MAX) {
> + dev_err(dev, "RTC exceeded by %llus\n", time - U32_MAX);
> + return -EINVAL;
> + }
> +
> + ret = mxc_rtc_lock(pdata);
> + if (ret)
> + return ret;
> +
> + writel(time, pdata->ioaddr + SRTC_LPSCMR);
> + mxc_rtc_sync_lp_locked(pdata->ioaddr);
> + return mxc_rtc_unlock(pdata);
> +}
> +
> +/*
> + * This function reads the current alarm value into the passed in \b alrm
> + * argument. It updates the \b alrm's pending field value based on the whether
> + * an alarm interrupt occurs or not.
> + *
> + * @param alrm contains the RTC alarm value upon return
> + *
> + * @return 0 if successful; non-zero otherwise.
> + */
> +static int mxc_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> + struct mxc_rtc_data *pdata = dev_get_drvdata(dev);
> + void __iomem *ioaddr = pdata->ioaddr;
> + int ret;
> +
> + ret = mxc_rtc_lock(pdata);
> + if (ret)
> + return ret;
> +
> + rtc_time_to_tm(readl(ioaddr + SRTC_LPSAR), &alrm->time);
> + alrm->pending = !!(readl(ioaddr + SRTC_LPSR) & SRTC_LPSR_ALP);
> + return mxc_rtc_unlock(pdata);
> +}
> +
> +/*
> + * Enable/Disable alarm interrupt
> + * The caller should hold the pdata->lock
> + */
> +static void mxc_rtc_alarm_irq_enable_locked(struct mxc_rtc_data *pdata,
> + unsigned int enable)
> +{
> + u32 lp_cr = readl(pdata->ioaddr + SRTC_LPCR);
> +
> + if (enable)
> + lp_cr |= (SRTC_LPCR_ALP | SRTC_LPCR_WAE);
> + else
> + lp_cr &= ~(SRTC_LPCR_ALP | SRTC_LPCR_WAE);
> +
> + writel(lp_cr, pdata->ioaddr + SRTC_LPCR);
> +}
> +
> +static int mxc_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
> +{
> + struct mxc_rtc_data *pdata = dev_get_drvdata(dev);
> + int ret = mxc_rtc_lock(pdata);
> +
> + if (ret)
> + return ret;
> +
> + mxc_rtc_alarm_irq_enable_locked(pdata, enable);
> + return mxc_rtc_unlock(pdata);
> +}
> +
> +/*
> + * This function sets the RTC alarm based on passed in alrm.
> + *
> + * @param alrm the alarm value to be set in the RTC
> + *
> + * @return 0 if successful; non-zero otherwise.
> + */
> +static int mxc_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> + struct mxc_rtc_data *pdata = dev_get_drvdata(dev);
> + int ret = mxc_rtc_lock(pdata);
> +
> + if (ret)
> + return ret;
> +
> + ret = mxc_rtc_write_alarm_locked(pdata, &alrm->time);
Is it worth it to make this a separate function?
> + if (!ret) {
> + mxc_rtc_alarm_irq_enable_locked(pdata, alrm->enabled);
> + mxc_rtc_sync_lp_locked(pdata->ioaddr);
> + }
> + mxc_rtc_unlock(pdata);
> + return ret;
> +}
> +
> +static const struct rtc_class_ops mxc_rtc_ops = {
> + .read_time = mxc_rtc_read_time,
> + .set_time = mxc_rtc_set_time,
> + .read_alarm = mxc_rtc_read_alarm,
> + .set_alarm = mxc_rtc_set_alarm,
> + .alarm_irq_enable = mxc_rtc_alarm_irq_enable,
> +};
> +
> +static int mxc_rtc_wait_for_flag(void *__iomem ioaddr, int flag)
> +{
> + unsigned int timeout = REG_READ_TIMEOUT;
> +
> + while (!(readl(ioaddr) & flag)) {
> + if (!--timeout) {
> + pr_err("Wait timeout for 0x%x@%p!\n", flag, ioaddr);
Please use dev_* functions for printing. In this case the message should
probably be printed from the caller.
> + return -EBUSY;
> + }
> + }
> + return 0;
> +}
> +
> +static int mxc_rtc_probe(struct platform_device *pdev)
> +{
> + struct mxc_rtc_data *pdata;
> + struct resource *res;
> + void __iomem *ioaddr;
> + int ret = 0;
> +
> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENODEV;
> +
> + pdata->ioaddr = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(pdata->ioaddr))
> + return PTR_ERR(pdata->ioaddr);
> +
> + ioaddr = pdata->ioaddr;
> +
> + pdata->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(pdata->clk)) {
> + dev_err(&pdev->dev, "unable to get rtc clock!\n");
> + return PTR_ERR(pdata->clk);
> + }
> +
> + spin_lock_init(&pdata->lock);
> + pdata->irq = platform_get_irq(pdev, 0);
> + if (pdata->irq < 0)
> + return pdata->irq;
> +
> + device_init_wakeup(&pdev->dev, 1);
> +
> + ret = clk_prepare_enable(pdata->clk);
> + if (ret)
> + return ret;
> + /* initialize glitch detect */
> + writel(SRTC_LPPDR_INIT, ioaddr + SRTC_LPPDR);
> +
> + /* clear lp interrupt status */
> + writel(0xFFFFFFFF, ioaddr + SRTC_LPSR);
> +
> + /* move out of init state */
> + writel((SRTC_LPCR_IE | SRTC_LPCR_NSA), ioaddr + SRTC_LPCR);
> + xc_rtc_wait_for_flag(ioaddr + SRTC_LPSR, SRTC_LPSR_IES);
If this can fail, shouldn't you test for an error?
> +
> + /* move out of non-valid state */
> + writel((SRTC_LPCR_IE | SRTC_LPCR_NVE | SRTC_LPCR_NSA |
> + SRTC_LPCR_EN_LP), ioaddr + SRTC_LPCR);
> + mxc_rtc_wait_for_flag(ioaddr + SRTC_LPSR, SRTC_LPSR_NVES);
dito
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [RFC PATCH 2/5] perf jevents: add support for arch recommended events
From: John Garry @ 2017-12-06 8:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171205172748.GI3070@tassilo.jf.intel.com>
On 05/12/2017 17:27, Andi Kleen wrote:
> On Wed, Dec 06, 2017 at 12:13:16AM +0800, John Garry wrote:
>> For some architectures (like arm64), there are architecture-
>> defined recommended events. Vendors may not be obliged to
>> follow the recommendation and may implement their own pmu
>> event for a specific event cod
>
> I would just duplicate the architected events into the different
> vendor files. Then you wouldn't need all this mess.
>
This is what we were originally doing:
https://patchwork.kernel.org/patch/10010859/
But then we thought that we could avoid duplicating all these events for
every platform from every vendor. Most, if not all, vendors will
implement the events as recommended for any platform, so much
unnecessary duplication.
cheers,
John
> -Andi
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
> .
>
^ permalink raw reply
* [PATCH v6 0/5] clk: Add Aspeed clock driver
From: Joel Stanley @ 2017-12-06 7:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171128071908.12279-1-joel@jms.id.au>
Hello clk maintainers,
Has anyone had a chance to review these patches?
On Tue, Nov 28, 2017 at 5:49 PM, Joel Stanley <joel@jms.id.au> wrote:
> This driver supports the ast2500, ast2400 (and derivative) BMC SoCs from
> Aspeed.
>
> This is v6. See patches for detailed changelogs.
>
> v6: Added reviewed-bys
> v5: Address review from Andrew
> v4: Address review from Andrew and Stephen.
> v3: Address review from Andrew and has seen more testing on hardware
> v2: split the driver out into a series of patches to make them easier to
> review.
>
> All of the important clocks are supported, with most non-essential ones
> also implemented where information is available. I am working with
> Aspeed to clear up some of the missing information, including the
> missing parent-sibling relationships.
>
> We need to know the rate of the apb clock in order to correctly program
> the clocksource driver, so the apb and it's parents are created in the
> CLK_OF_DECLARE_DRIVER callback.
>
> The rest of the clocks are created at normal driver probe time. I
> followed the Gemini driver's lead with using the regmap where I could,
> but also having a pointer to the base address for use with the common
> clock callbacks.
>
> The driver borrows from the clk_gate common clock infrastructure, but modifies
> it in order to support the clock gate and reset pair that most of the clocks
> have. This pair must be reset-ungated-released, with appropriate delays,
> according to the datasheet.
>
> The first patch introduces the core clock registration parts, and describes
> the clocks. The second creates the core clocks, giving the system enough to
> boot (but without uart). Next come the non-core clocks, and finally the reset
> controller that is used for the few cocks that don't have a gate to go with their
> reset pair.
>
> Please review!
>
> Cheers,
>
> Joel
>
> Joel Stanley (5):
> clk: Add clock driver for ASPEED BMC SoCs
> clk: aspeed: Register core clocks
> clk: aspeed: Add platform driver and register PLLs
> clk: aspeed: Register gated clocks
> clk: aspeed: Add reset controller
>
> drivers/clk/Kconfig | 12 +
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-aspeed.c | 657 +++++++++++++++++++++++++++++++
> include/dt-bindings/clock/aspeed-clock.h | 54 +++
> 4 files changed, 724 insertions(+)
> create mode 100644 drivers/clk/clk-aspeed.c
> create mode 100644 include/dt-bindings/clock/aspeed-clock.h
>
> --
> 2.14.1
>
^ permalink raw reply
* [PATCH v6 6/6] ARM: dts: imx6: RIoTboard provide standby on power off option
From: Oleksij Rempel @ 2017-12-06 7:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171206072402.11694-1-o.rempel@pengutronix.de>
This board, as well as some other boards with i.MX6 and a PMIC, uses a
"PMIC_STBY_REQ" line to notify the PMIC about a state change.
The PMIC is programmed for a specific state change before triggering the
line.
In this case, PMIC_STBY_REQ can be used for stand by, sleep
and power off modes.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
arch/arm/boot/dts/imx6dl-riotboard.dts | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/imx6dl-riotboard.dts b/arch/arm/boot/dts/imx6dl-riotboard.dts
index 275c6c05219d..574f6b261ecd 100644
--- a/arch/arm/boot/dts/imx6dl-riotboard.dts
+++ b/arch/arm/boot/dts/imx6dl-riotboard.dts
@@ -90,6 +90,10 @@
status = "okay";
};
+&clks {
+ fsl,pmic-stby-poweroff;
+};
+
&fec {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_enet>;
@@ -170,6 +174,7 @@
reg = <0x08>;
interrupt-parent = <&gpio5>;
interrupts = <16 8>;
+ fsl,pmic-stby-poweroff;
regulators {
reg_vddcore: sw1ab { /* VDDARM_IN */
--
2.11.0
^ permalink raw reply related
* [PATCH v6 5/6] regulator: pfuze100-regulator: provide pm_power_off_prepare handler
From: Oleksij Rempel @ 2017-12-06 7:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171206072402.11694-1-o.rempel@pengutronix.de>
On some boards the SoC can use one pin "PMIC_STBY_REQ" to notify th PMIC
about state changes. In this case internal state of PMIC must be
preconfigured for upcomming state change.
It works fine with the current regulator framework, except with the
power-off case.
This patch is providing an optional pm_power_off_prepare handler
which will configure standby state of the PMIC to disable all power lines.
In my power consumption test on RIoTBoard, I got the following results:
power off without this patch: 320 mA
power off with this patch: 2 mA
suspend to ram: 40 mA
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/regulator/pfuze100-regulator.c | 92 ++++++++++++++++++++++++++++++++++
1 file changed, 92 insertions(+)
diff --git a/drivers/regulator/pfuze100-regulator.c b/drivers/regulator/pfuze100-regulator.c
index 63922a2167e5..f6c276ed91d8 100644
--- a/drivers/regulator/pfuze100-regulator.c
+++ b/drivers/regulator/pfuze100-regulator.c
@@ -28,6 +28,7 @@
#include <linux/regulator/pfuze100.h>
#include <linux/i2c.h>
#include <linux/slab.h>
+#include <linux/kallsyms.h>
#include <linux/regmap.h>
#define PFUZE_NUMREGS 128
@@ -42,11 +43,17 @@
#define PFUZE100_COINVOL 0x1a
#define PFUZE100_SW1ABVOL 0x20
+#define PFUZE100_SW1ABMODE 0x23
#define PFUZE100_SW1CVOL 0x2e
+#define PFUZE100_SW1CMODE 0x31
#define PFUZE100_SW2VOL 0x35
+#define PFUZE100_SW2MODE 0x38
#define PFUZE100_SW3AVOL 0x3c
+#define PFUZE100_SW3AMODE 0x3f
#define PFUZE100_SW3BVOL 0x43
+#define PFUZE100_SW3BMODE 0x46
#define PFUZE100_SW4VOL 0x4a
+#define PFUZE100_SW4MODE 0x4d
#define PFUZE100_SWBSTCON1 0x66
#define PFUZE100_VREFDDRCON 0x6a
#define PFUZE100_VSNVSVOL 0x6b
@@ -57,6 +64,13 @@
#define PFUZE100_VGEN5VOL 0x70
#define PFUZE100_VGEN6VOL 0x71
+#define PFUZE100_SWxMODE_MASK 0xf
+#define PFUZE100_SWxMODE_APS_APS 0x8
+#define PFUZE100_SWxMODE_APS_OFF 0x4
+
+#define PFUZE100_VGENxLPWR BIT(6)
+#define PFUZE100_VGENxSTBY BIT(5)
+
enum chips { PFUZE100, PFUZE200, PFUZE3000 = 3 };
struct pfuze_regulator {
@@ -489,6 +503,69 @@ static inline struct device_node *match_of_node(int index)
}
#endif
+static struct pfuze_chip *syspm_pfuze_chip;
+
+static void pfuze_power_off_prepare(void)
+{
+ dev_info(syspm_pfuze_chip->dev, "Configure standy mode for power off");
+
+ /* Switch from default mode: APS/APS to APS/Off */
+ regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW1ABMODE,
+ PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+ regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW1CMODE,
+ PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+ regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW2MODE,
+ PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+ regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW3AMODE,
+ PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+ regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW3BMODE,
+ PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+ regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW4MODE,
+ PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+
+ regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN1VOL,
+ PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+ PFUZE100_VGENxSTBY);
+ regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN2VOL,
+ PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+ PFUZE100_VGENxSTBY);
+ regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN3VOL,
+ PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+ PFUZE100_VGENxSTBY);
+ regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN4VOL,
+ PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+ PFUZE100_VGENxSTBY);
+ regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN5VOL,
+ PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+ PFUZE100_VGENxSTBY);
+ regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN6VOL,
+ PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+ PFUZE100_VGENxSTBY);
+}
+
+static int pfuze_power_off_prepare_init(struct pfuze_chip *pfuze_chip)
+{
+ if (pfuze_chip->chip_id != PFUZE100) {
+ dev_warn(pfuze_chip->dev, "Requested pm_power_off_prepare handler for not supported chip\n");
+ return -ENODEV;
+ }
+
+ if (pm_power_off_prepare) {
+ dev_warn(pfuze_chip->dev, "pm_power_off_prepare is already registered.\n");
+ return -EBUSY;
+ }
+
+ if (syspm_pfuze_chip) {
+ dev_warn(pfuze_chip->dev, "syspm_pfuze_chip is already set.\n");
+ return -EBUSY;
+ }
+
+ syspm_pfuze_chip = pfuze_chip;
+ pm_power_off_prepare = pfuze_power_off_prepare;
+
+ return 0;
+}
+
static int pfuze_identify(struct pfuze_chip *pfuze_chip)
{
unsigned int value;
@@ -659,6 +736,20 @@ static int pfuze100_regulator_probe(struct i2c_client *client,
}
}
+ if (of_property_read_bool(client->dev.of_node,
+ "fsl,pmic-stby-poweroff"))
+ return pfuze_power_off_prepare_init(pfuze_chip);
+
+ return 0;
+}
+
+static int pfuze100_regulator_remove(struct i2c_client *client)
+{
+ if (syspm_pfuze_chip) {
+ syspm_pfuze_chip = NULL;
+ pm_power_off_prepare = NULL;
+ }
+
return 0;
}
@@ -669,6 +760,7 @@ static struct i2c_driver pfuze_driver = {
.of_match_table = pfuze_dt_ids,
},
.probe = pfuze100_regulator_probe,
+ .remove = pfuze100_regulator_remove,
};
module_i2c_driver(pfuze_driver);
--
2.11.0
^ permalink raw reply related
* [PATCH v6 4/6] regulator: pfuze100: add fsl, pmic-stby-poweroff property
From: Oleksij Rempel @ 2017-12-06 7:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171206072402.11694-1-o.rempel@pengutronix.de>
Document the new optional "fsl,pmic-stby-poweroff" property.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Acked-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/regulator/pfuze100.txt | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/regulator/pfuze100.txt b/Documentation/devicetree/bindings/regulator/pfuze100.txt
index 444c47831a40..197f1a52e960 100644
--- a/Documentation/devicetree/bindings/regulator/pfuze100.txt
+++ b/Documentation/devicetree/bindings/regulator/pfuze100.txt
@@ -4,6 +4,13 @@ Required properties:
- compatible: "fsl,pfuze100", "fsl,pfuze200", "fsl,pfuze3000"
- reg: I2C slave address
+Optional properties:
+- fsl,pmic-stby-poweroff: if present, configure the PMIC to shutdown all
+ power rails when PMIC_STBY_REQ line is asserted during the power off sequence.
+ Use this option if the SoC should be powered off by external power
+ management IC (PMIC) on PMIC_STBY_REQ signal.
+ As opposite to PMIC_STBY_REQ boards can implement PMIC_ON_REQ signal.
+
Required child node:
- regulators: This is the list of child nodes that specify the regulator
initialization data for defined regulators. Please refer to below doc
--
2.11.0
^ permalink raw reply related
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