* [PATCH v11 5/8] clocksource/drivers/arm_arch_timer: Simplify ACPI support code.
From: Fu Wei @ 2016-09-13 9:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CADyBb7uX79UP=mUxvP-EPGi56-46y2SCMPsWO=Uw-=N0GQERhA@mail.gmail.com>
Hi Thomas, Daniel,
For these arm_arch_timer patches, do you have any other suggestion or comment?
I have deleted "skipping" in the error message.
I have prepared v12 (rebase to rc6 and on the top of IORT v11),
should I send it now (if you are OK with my arm_arch_timer patches ),
or anything I can do to improve this patchset ?
Thanks.
On 7 September 2016 at 17:23, Fu Wei <fu.wei@linaro.org> wrote:
> Hi Thomas
>
> On 6 September 2016 at 22:36, Thomas Gleixner <tglx@linutronix.de> wrote:
>> On Tue, 6 Sep 2016, fu.wei at linaro.org wrote:
>>> + if (timer_count < 0)
>>> + pr_err("Failed to get platform timer info, skipping.\n");
>>
>> So this prints something about skipping. But then it continues as if
>> nothing went wrong. That's either wrong or confusing or both.
>
> yes, you are right, this info is confusing.
> maybe we just delete the "skipping" ?
>
> ?timer_count < 0? is caused by some firmware bug, in gtdt.c:
> ----
> int __init acpi_gtdt_init(struct acpi_table_header *table)
> {
> ......
> if (start < (void *)table + sizeof(struct acpi_table_gtdt)) {
> pr_err(FW_BUG "Failed to retrieve timer info from firmware:
> invalid data.\n");
> return -EINVAL;
> ......
> }
> ----
>
> But in this situation( without platform timers ), system still can work.
> So I thing we just need to print a error.
>
>>
>>> - arch_timer_init();
>>> - return 0;
>>> + return arch_timer_init();
>>
>> Thanks,
>>
>> tglx
>
>
>
> --
> Best regards,
>
> Fu Wei
> Software Engineer
> Red Hat
--
Best regards,
Fu Wei
Software Engineer
Red Hat
^ permalink raw reply
* [PATCH v4 01/10] arm64: KVM: Use static keys for selecting the GIC backend
From: Christoffer Dall @ 2016-09-13 9:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D7C2AE.908@arm.com>
On Tue, Sep 13, 2016 at 10:11:10AM +0100, Marc Zyngier wrote:
> On 13/09/16 09:20, Christoffer Dall wrote:
> > On Mon, Sep 12, 2016 at 03:49:15PM +0100, Vladimir Murzin wrote:
> >> Currently GIC backend is selected via alternative framework and this
> >> is fine. We are going to introduce vgic-v3 to 32-bit world and there
> >> we don't have patching framework in hand, so we can either check
> >> support for GICv3 every time we need to choose which backend to use or
> >> try to optimise it by using static keys. The later looks quite
> >> promising because we can share logic involved in selecting GIC backend
> >> between architectures if both uses static keys.
> >>
> >> This patch moves arm64 from alternative to static keys framework for
> >> selecting GIC backend. For that we embed static key into vgic_global
> >> and enable the key during vgic initialisation based on what has
> >> already been exposed by the host GIC driver.
> >>
> >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> >> ---
> >> arch/arm64/kvm/hyp/switch.c | 21 +++++++++++----------
> >> include/kvm/arm_vgic.h | 4 ++++
> >> virt/kvm/arm/vgic/vgic-init.c | 4 ++++
> >> virt/kvm/arm/vgic/vgic.c | 2 +-
> >> 4 files changed, 20 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> >> index 5a84b45..d5c4cc5 100644
> >> --- a/arch/arm64/kvm/hyp/switch.c
> >> +++ b/arch/arm64/kvm/hyp/switch.c
> >> @@ -16,6 +16,8 @@
> >> */
> >>
> >> #include <linux/types.h>
> >> +#include <linux/jump_label.h>
> >> +
> >> #include <asm/kvm_asm.h>
> >> #include <asm/kvm_hyp.h>
> >>
> >> @@ -126,17 +128,13 @@ static void __hyp_text __deactivate_vm(struct kvm_vcpu *vcpu)
> >> write_sysreg(0, vttbr_el2);
> >> }
> >>
> >> -static hyp_alternate_select(__vgic_call_save_state,
> >> - __vgic_v2_save_state, __vgic_v3_save_state,
> >> - ARM64_HAS_SYSREG_GIC_CPUIF);
> >> -
> >> -static hyp_alternate_select(__vgic_call_restore_state,
> >> - __vgic_v2_restore_state, __vgic_v3_restore_state,
> >> - ARM64_HAS_SYSREG_GIC_CPUIF);
> >> -
> >> static void __hyp_text __vgic_save_state(struct kvm_vcpu *vcpu)
> >> {
> >> - __vgic_call_save_state()(vcpu);
> >> + if (static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif))
> >
> > It's a bit weird that we use _unlikely for GICv3 (at least if/when GICv3
> > hardware becomes mainstream), but as we don't have another primitive for
> > the 'default disabled' case, I suppose that's the best we can do.
>
> We could always revert the "likelihood" of that test once GICv3 has
> conquered the world. Or start patching the 32bit kernel like we do for
> 64bit...
>
> >
> >> + __vgic_v3_save_state(vcpu);
> >> + else
> >> + __vgic_v2_save_state(vcpu);
> >> +
> >> write_sysreg(read_sysreg(hcr_el2) & ~HCR_INT_OVERRIDE, hcr_el2);
> >> }
> >>
> >> @@ -149,7 +147,10 @@ static void __hyp_text __vgic_restore_state(struct kvm_vcpu *vcpu)
> >> val |= vcpu->arch.irq_lines;
> >> write_sysreg(val, hcr_el2);
> >>
> >> - __vgic_call_restore_state()(vcpu);
> >> + if (static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif))
> >> + __vgic_v3_restore_state(vcpu);
> >> + else
> >> + __vgic_v2_restore_state(vcpu);
> >> }
> >>
> >> static bool __hyp_text __true_value(void)
> >> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> >> index 19b698e..994665a 100644
> >> --- a/include/kvm/arm_vgic.h
> >> +++ b/include/kvm/arm_vgic.h
> >> @@ -23,6 +23,7 @@
> >> #include <linux/types.h>
> >> #include <kvm/iodev.h>
> >> #include <linux/list.h>
> >> +#include <linux/jump_label.h>
> >>
> >> #define VGIC_V3_MAX_CPUS 255
> >> #define VGIC_V2_MAX_CPUS 8
> >> @@ -63,6 +64,9 @@ struct vgic_global {
> >>
> >> /* Only needed for the legacy KVM_CREATE_IRQCHIP */
> >> bool can_emulate_gicv2;
> >> +
> >> + /* GIC system register CPU interface */
> >> + struct static_key_false gicv3_cpuif;
> >
> > Documentation/static-keys.txt says that we are not supposed to use
> > struct static_key_false directly. This will obviously work quite
> > nicely, but we could consider adding a pair of
> > DECLARE_STATIC_KEY_TRUE/FALSE macros that don't have the assignments,
> > but obviously this will need an ack from other maintainers.
> >
> > Thoughts?
>
> Grepping through the tree shows that we're not the only abusers of this
> (dynamic debug is far worse!). Happy to write the additional macros and
> submit them if nobody beats me to it.
>
> >
> >
> >> };
> >>
> >> extern struct vgic_global kvm_vgic_global_state;
> >> diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
> >> index 83777c1..14d6718 100644
> >> --- a/virt/kvm/arm/vgic/vgic-init.c
> >> +++ b/virt/kvm/arm/vgic/vgic-init.c
> >> @@ -405,6 +405,10 @@ int kvm_vgic_hyp_init(void)
> >> break;
> >> case GIC_V3:
> >> ret = vgic_v3_probe(gic_kvm_info);
> >> + if (!ret) {
> >> + static_branch_enable(&kvm_vgic_global_state.gicv3_cpuif);
> >> + kvm_info("GIC system register CPU interface\n");
> >
> > nit: add enabled to the info message?
> >
> >> + }
> >> break;
> >> default:
> >> ret = -ENODEV;
> >> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> >> index e83b7fe..8a529a7 100644
> >> --- a/virt/kvm/arm/vgic/vgic.c
> >> +++ b/virt/kvm/arm/vgic/vgic.c
> >> @@ -29,7 +29,7 @@
> >> #define DEBUG_SPINLOCK_BUG_ON(p)
> >> #endif
> >>
> >> -struct vgic_global __section(.hyp.text) kvm_vgic_global_state;
> >> +struct vgic_global __section(.hyp.text) kvm_vgic_global_state = {.gicv3_cpuif = STATIC_KEY_FALSE_INIT,};
> >>
> >> /*
> >> * Locking order is always:
> >> --
> >> 1.7.9.5
> >>
> >
> > Overall this looks really nice, as long as we're clear on the static
> > keys stuff.
>
> Indeed, we should get this sorted, though I'm not sure this should be a
> blocker for this code.
>
Agreed, let's ship it!
-Christoffer
^ permalink raw reply
* [RFCv3][PATCH 3/5] arm64: Implement ARCH_HAS_FORCE_CACHE
From: Will Deacon @ 2016-09-13 9:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473715978-11633-4-git-send-email-labbott@redhat.com>
Hi Laura,
On Mon, Sep 12, 2016 at 02:32:56PM -0700, Laura Abbott wrote:
>
> arm64 may need to guarantee the caches are synced. Implement versions of
> the kernel_force_cache API to allow this.
>
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
> v3: Switch to calling cache operations directly instead of relying on
> DMA mapping.
> ---
> arch/arm64/include/asm/cacheflush.h | 8 ++++++++
> arch/arm64/mm/cache.S | 24 ++++++++++++++++++++----
> arch/arm64/mm/flush.c | 11 +++++++++++
> 3 files changed, 39 insertions(+), 4 deletions(-)
I'm really hesitant to expose these cache routines as an API solely to
support a driver sitting in staging/. I appreciate that there's a chicken
and egg problem here, but we *really* don't want people using these routines
in preference to the DMA API, and I fear that we'll simply grow a bunch
more users of these things if we promote it as an API like you're proposing.
Can the code not be contained under staging/, as part of ion?
Will
^ permalink raw reply
* [PATCH V4 3/4] ARM: bcm2835: add thermal node to device-tree of bcm283x
From: Stefan Wahren @ 2016-09-13 9:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <704dd583-5bd8-fa77-a4ab-18f0845b9c4e@martin.sperl.org>
Am 13.09.2016 um 11:06 schrieb Martin Sperl:
>
>
> On 09.09.2016 21:02, Stefan Wahren wrote:
>> bcm2837.dtsi
>>
>> &thermal {
>> compatible = "brcm,bcm2837-thermal";
>> status = "okay";
>> };
>
> Note that there is only a bcm2837.dtsi in arm64/boot/dts/broadcom/
> but not in arm/boot/dts - at least with the current upstream kernel.
>
> So I will just patch those dtsi that exist right now.
That's correct as long as the bcm2837.dtsi change is a separate patch
for ARM64 and this was the reason for my "warning" in my first reply.
Stefan
>
> Martin
^ permalink raw reply
* [PATCH v4 01/10] arm64: KVM: Use static keys for selecting the GIC backend
From: Marc Zyngier @ 2016-09-13 9:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160913082019.GB5680@cbox>
On 13/09/16 09:20, Christoffer Dall wrote:
> On Mon, Sep 12, 2016 at 03:49:15PM +0100, Vladimir Murzin wrote:
>> Currently GIC backend is selected via alternative framework and this
>> is fine. We are going to introduce vgic-v3 to 32-bit world and there
>> we don't have patching framework in hand, so we can either check
>> support for GICv3 every time we need to choose which backend to use or
>> try to optimise it by using static keys. The later looks quite
>> promising because we can share logic involved in selecting GIC backend
>> between architectures if both uses static keys.
>>
>> This patch moves arm64 from alternative to static keys framework for
>> selecting GIC backend. For that we embed static key into vgic_global
>> and enable the key during vgic initialisation based on what has
>> already been exposed by the host GIC driver.
>>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>> ---
>> arch/arm64/kvm/hyp/switch.c | 21 +++++++++++----------
>> include/kvm/arm_vgic.h | 4 ++++
>> virt/kvm/arm/vgic/vgic-init.c | 4 ++++
>> virt/kvm/arm/vgic/vgic.c | 2 +-
>> 4 files changed, 20 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
>> index 5a84b45..d5c4cc5 100644
>> --- a/arch/arm64/kvm/hyp/switch.c
>> +++ b/arch/arm64/kvm/hyp/switch.c
>> @@ -16,6 +16,8 @@
>> */
>>
>> #include <linux/types.h>
>> +#include <linux/jump_label.h>
>> +
>> #include <asm/kvm_asm.h>
>> #include <asm/kvm_hyp.h>
>>
>> @@ -126,17 +128,13 @@ static void __hyp_text __deactivate_vm(struct kvm_vcpu *vcpu)
>> write_sysreg(0, vttbr_el2);
>> }
>>
>> -static hyp_alternate_select(__vgic_call_save_state,
>> - __vgic_v2_save_state, __vgic_v3_save_state,
>> - ARM64_HAS_SYSREG_GIC_CPUIF);
>> -
>> -static hyp_alternate_select(__vgic_call_restore_state,
>> - __vgic_v2_restore_state, __vgic_v3_restore_state,
>> - ARM64_HAS_SYSREG_GIC_CPUIF);
>> -
>> static void __hyp_text __vgic_save_state(struct kvm_vcpu *vcpu)
>> {
>> - __vgic_call_save_state()(vcpu);
>> + if (static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif))
>
> It's a bit weird that we use _unlikely for GICv3 (at least if/when GICv3
> hardware becomes mainstream), but as we don't have another primitive for
> the 'default disabled' case, I suppose that's the best we can do.
We could always revert the "likelihood" of that test once GICv3 has
conquered the world. Or start patching the 32bit kernel like we do for
64bit...
>
>> + __vgic_v3_save_state(vcpu);
>> + else
>> + __vgic_v2_save_state(vcpu);
>> +
>> write_sysreg(read_sysreg(hcr_el2) & ~HCR_INT_OVERRIDE, hcr_el2);
>> }
>>
>> @@ -149,7 +147,10 @@ static void __hyp_text __vgic_restore_state(struct kvm_vcpu *vcpu)
>> val |= vcpu->arch.irq_lines;
>> write_sysreg(val, hcr_el2);
>>
>> - __vgic_call_restore_state()(vcpu);
>> + if (static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif))
>> + __vgic_v3_restore_state(vcpu);
>> + else
>> + __vgic_v2_restore_state(vcpu);
>> }
>>
>> static bool __hyp_text __true_value(void)
>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
>> index 19b698e..994665a 100644
>> --- a/include/kvm/arm_vgic.h
>> +++ b/include/kvm/arm_vgic.h
>> @@ -23,6 +23,7 @@
>> #include <linux/types.h>
>> #include <kvm/iodev.h>
>> #include <linux/list.h>
>> +#include <linux/jump_label.h>
>>
>> #define VGIC_V3_MAX_CPUS 255
>> #define VGIC_V2_MAX_CPUS 8
>> @@ -63,6 +64,9 @@ struct vgic_global {
>>
>> /* Only needed for the legacy KVM_CREATE_IRQCHIP */
>> bool can_emulate_gicv2;
>> +
>> + /* GIC system register CPU interface */
>> + struct static_key_false gicv3_cpuif;
>
> Documentation/static-keys.txt says that we are not supposed to use
> struct static_key_false directly. This will obviously work quite
> nicely, but we could consider adding a pair of
> DECLARE_STATIC_KEY_TRUE/FALSE macros that don't have the assignments,
> but obviously this will need an ack from other maintainers.
>
> Thoughts?
Grepping through the tree shows that we're not the only abusers of this
(dynamic debug is far worse!). Happy to write the additional macros and
submit them if nobody beats me to it.
>
>
>> };
>>
>> extern struct vgic_global kvm_vgic_global_state;
>> diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
>> index 83777c1..14d6718 100644
>> --- a/virt/kvm/arm/vgic/vgic-init.c
>> +++ b/virt/kvm/arm/vgic/vgic-init.c
>> @@ -405,6 +405,10 @@ int kvm_vgic_hyp_init(void)
>> break;
>> case GIC_V3:
>> ret = vgic_v3_probe(gic_kvm_info);
>> + if (!ret) {
>> + static_branch_enable(&kvm_vgic_global_state.gicv3_cpuif);
>> + kvm_info("GIC system register CPU interface\n");
>
> nit: add enabled to the info message?
>
>> + }
>> break;
>> default:
>> ret = -ENODEV;
>> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
>> index e83b7fe..8a529a7 100644
>> --- a/virt/kvm/arm/vgic/vgic.c
>> +++ b/virt/kvm/arm/vgic/vgic.c
>> @@ -29,7 +29,7 @@
>> #define DEBUG_SPINLOCK_BUG_ON(p)
>> #endif
>>
>> -struct vgic_global __section(.hyp.text) kvm_vgic_global_state;
>> +struct vgic_global __section(.hyp.text) kvm_vgic_global_state = {.gicv3_cpuif = STATIC_KEY_FALSE_INIT,};
>>
>> /*
>> * Locking order is always:
>> --
>> 1.7.9.5
>>
>
> Overall this looks really nice, as long as we're clear on the static
> keys stuff.
Indeed, we should get this sorted, though I'm not sure this should be a
blocker for this code.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH 6/8 v2] arm: orion5x: Add DT-based support for Netgear WNR854T
From: Jamie Lentin @ 2016-09-13 9:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912220344.GM11400@lunn.ch>
On 2016-09-12 23:03, Andrew Lunn wrote:
>> Maybe we can instead leave out the PCI support from the new
>> file for now and not delete the legacy board file?
This seems a reasonable compromise. The PCI card the router comes with
isn't supported by mwl8k mainline anyway (There's STA-only firmware that
can be extracted from a windows driver and PCI IDs added, but stats
reporting uses a different format), so it's not a huge loss, although
many did replace the card with something Atheros-based.
Unfortunately the power regulator on my spare router has stopped
regulating, which will make more development tricky. But will have a
look at the PCI conversion if I can get hold of another.
> Jamie, which interrupt do you see the WiFi card using? If it is
> IRQ_ORION5X_PCIE0_INT, (1 + 11), that is probably easier to deal with
> than if it uses GPIO 4.
Definitely uses GPIO 4.
# cat /proc/interrupts
CPU0
17: 165966 bridge-interrupt-ctrl 2 Edge orion_event
22: 5119 interrupt-controller 3 Edge serial
24: 0 bridge-interrupt-ctrl 3 Edge orion_wdt
25: 89504 interrupt-controller 22 Edge f1072004.mdio-bus
28: 1 f1010100.gpio 1 Edge Reset Button
31: 598 f1010100.gpio 4 Level mwl8k
59: 985 interrupt-controller 21 Edge eth0
Err: 0
# iw dev wlan0 scan > /dev/null ; cat /proc/interrupts
CPU0
17: 166424 bridge-interrupt-ctrl 2 Edge orion_event
22: 5250 interrupt-controller 3 Edge serial
24: 0 bridge-interrupt-ctrl 3 Edge orion_wdt
25: 89748 interrupt-controller 22 Edge f1072004.mdio-bus
28: 1 f1010100.gpio 1 Edge Reset Button
31: 708 f1010100.gpio 4 Level mwl8k
59: 985 interrupt-controller 21 Edge eth0
Err: 0
# sleep 10 ; cat /proc/interrupts
CPU0
17: 167985 bridge-interrupt-ctrl 2 Edge orion_event
22: 5349 interrupt-controller 3 Edge serial
24: 0 bridge-interrupt-ctrl 3 Edge orion_wdt
25: 90603 interrupt-controller 22 Edge f1072004.mdio-bus
28: 1 f1010100.gpio 1 Edge Reset Button
31: 708 f1010100.gpio 4 Level mwl8k
59: 995 interrupt-controller 21 Edge eth0
Err: 0
# iw dev wlan0 scan > /dev/null ; cat /proc/interrupts
CPU0
17: 168270 bridge-interrupt-ctrl 2 Edge orion_event
22: 5480 interrupt-controller 3 Edge serial
24: 0 bridge-interrupt-ctrl 3 Edge orion_wdt
25: 90727 interrupt-controller 22 Edge f1072004.mdio-bus
28: 1 f1010100.gpio 1 Edge Reset Button
31: 818 f1010100.gpio 4 Level mwl8k
59: 996 interrupt-controller 21 Edge eth0
Err: 0
>
> Thanks
> Andrew
^ permalink raw reply
* [PATCH v2 3/4] ARM: dts: Add NextThing GR8 dtsi
From: Linus Walleij @ 2016-09-13 9:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1975640.ctuo4Obpao@avalon>
On Mon, Sep 12, 2016 at 2:47 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 12 Sep 2016 14:40:15 Linus Walleij wrote:
>> HOWEVER it often turns out that while you can programmatically
>> and individually set pins to any function (and biasing etc), the
>> person designing the hardware was not thinking that you should
>> be able to do whatever you like, e.g. even if it is possible to
>> take two pins and use one of them for half an SPI bus and the
>> other for half an I2C bus, that doesn't mean that this is useful
>> or makes any kind of electronic sense, it just makes "software
>> sense".
(...)
> I'd argue that you would find out about lots of clever/insane use cases that
> don't fit this model if you looked at all the hardware available out there,
> especially non-phone devices. Your SPI example is a good one, I've seen SPI
> being used in unidirectional mode only, with only MISO or MOSI mattering. In
> that case the other pin could be used as a GPIO for a totally unrelated
> purpose when the design is short on GPIOs or when GPIOs have been allocated
> without any knowledge of the Linux pinctrl subsystem.
That is true sometimes. It is a tradeoff, I can also imagine actually
driving an I2C bus just to use the SCL line as a clock for something,
constantly feeding nonsense data through the I2C block and
ignoring SDA and reusing that pin as GPIO. (And a lot of other
theoretical usecases.)
Some pin controller hardware helpully only let you select groups
and makes such hacks impossible.
Also I guess the target audience of the SoC will affect the
hackishness of the usecases, and affect what they might attempt
to shoehorn into the design.
So model on whatever makes most sense, is usually how I think about
it. Or as the IETF says "rough consensus and running code".
I guess it is a bit of grayzone, and that is why both solutions coexist.
> Looking at the sh-pfc driver, I wish the hardware had followed the pinctrl-
> single model. sh-pfc is a good example of how bloated a pinctrl driver can
> become when there is no choice but model all the relationships betweens pins
> and functions in C code.
It might be true, there are so many variables to the equation that
I cannot tell.
Debuggability and readability of code and device trees and different
groups of people reading code vs device trees is another factor.
Scaringly, what is best for me as subsystem maintainer (that all
drivers look identical) is not always best for the users.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH 1/5] clk: add support for runtime pm
From: Marek Szyprowski @ 2016-09-13 9:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912223105.GI7243@codeaurora.org>
Hi Stephen,
On 2016-09-13 00:31, Stephen Boyd wrote:
> On 09/12, Marek Szyprowski wrote:
>> Hi Stephen,
>>
>>
>> On 2016-09-08 02:19, Stephen Boyd wrote:
>>> On 09/01, Marek Szyprowski wrote:
>>>> Registers for some clocks might be located in the SOC area, which are under the
>>>> power domain. To enable access to those registers respective domain has to be
>>>> turned on. Additionally, registers for such clocks will usually loose its
>>>> contents when power domain is turned off, so additional saving and restoring of
>>>> them might be needed in the clock controller driver.
>>>>
>>>> This patch adds basic infrastructure in the clocks core to allow implementing
>>>> driver for such clocks under power domains. Clock provider can supply a
>>>> struct device pointer, which is the used by clock core for tracking and managing
>>>> clock's controller runtime pm state. Each clk_prepare() operation
>>>> will first call pm_runtime_get_sync() on the supplied device, while
>>>> clk_unprepare() will do pm_runtime_put() at the end.
>>>>
>>>> Additional calls to pm_runtime_get/put functions are required to ensure that any
>>>> register access (like calculating/chaning clock rates) will be done with clock
>>>> controller in active runtime state.
>>>>
>>>> Special handling of the case when runtime pm is disabled for clock controller's
>>>> device is needed to let this feature work properly also during system sleep
>>>> suspend/resume operations (runtime pm is first disabled before entering sleep
>>>> state's, but controller is usually still operational until its suspend pm
>>>> callback is called).
>>>>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> My "knee jerk" concern is that we're going to take a runtime PM
>>> lock underneath the prepare lock. That seems like a situation
>>> where we could hit a lock inversion if the runtime PM callbacks
>>> themselves acquire the prepare lock by calling clk APIs? But this
>>> concern is false right? We release the runtime PM lock before
>>> calling the PM callback, so we shouldn't hit any deadlock and
>>> lockdep won't complain?
>> Runtime PM uses fine grained locking based on per-device locks, so there
>> should be no problem with global clock prepare lock. The only lock
>> interaction
>> is between clock controller device's rpm lock and clocks global
>> prepare lock, but
>> it always done with the same access pattern. I've tested it
>> extensively (also
>> with lock dep) with various use cases and found no problems.
>>
> Great! So you have runtime PM callbacks that are calling
> clk_prepare/unprepare?
Well, not really. clock controller's runtime pm functions must not call
clk_prepare/unprepare yet.
I didn't get your question. I thought that you are asking if my change
won't introduce any deadlock related to prepare and dev->pm locks. My
runtime pm functions doesn't do any call to clk_prepare/unprepare.
Although global clock prepare lock is re-entrant from the same process, it
would cause deadlock if called from runtime pm functions, because runtime
pm functions might be called from the worker running on the different
cpu/process.
I hope that the work started by Krzysztof Kozlowski on splitting prepare
lock on per-controller basis will solve limitation and one would be able
to call clk_prapare/unprepare on clocks from other controllers even from
the runtime pm functions.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* [PATCH V4 3/4] ARM: bcm2835: add thermal node to device-tree of bcm283x
From: Martin Sperl @ 2016-09-13 9:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <677965855.33392.597ccfbe-225b-403b-8ffa-3f42292996d4.open-xchange@email.1und1.de>
On 09.09.2016 21:02, Stefan Wahren wrote:
> bcm2837.dtsi
>
> &thermal {
> compatible = "brcm,bcm2837-thermal";
> status = "okay";
> };
Note that there is only a bcm2837.dtsi in arm64/boot/dts/broadcom/
but not in arm/boot/dts - at least with the current upstream kernel.
So I will just patch those dtsi that exist right now.
Martin
^ permalink raw reply
* Tegra baseline test results for v4.8-rc6
From: Jon Hunter @ 2016-09-13 9:05 UTC (permalink / raw)
To: linux-arm-kernel
Here are some basic Tegra test results for Linux v4.8-rc6.
Logs and other details at:
https://nvtb.github.io//linux/test_v4.8-rc6/20160912030104/
Test summary
------------
Build: zImage:
Pass: ( 2/ 2): multi_v7_defconfig, tegra_defconfig
Build: Image:
Pass: ( 1/ 1): defconfig
Boot to userspace: defconfig:
Pass: ( 4/ 4): qemu-vexpress64, tegra132-norrin,
tegra210-p2371-0000, tegra210-smaug
Boot to userspace: multi_v7_defconfig:
Pass: ( 5/ 5): tegra114-dalmore-a04, tegra124-jetson-tk1,
tegra124-nyan-big, tegra20-trimslice, tegra30-beaver
Boot to userspace: tegra_defconfig:
Pass: ( 5/ 5): tegra114-dalmore-a04, tegra124-jetson-tk1,
tegra124-nyan-big, tegra20-trimslice, tegra30-beaver
PM: System suspend: multi_v7_defconfig:
Pass: ( 5/ 5): tegra114-dalmore-a04, tegra124-jetson-tk1,
tegra124-nyan-big, tegra20-trimslice, tegra30-beaver
PM: System suspend: tegra_defconfig:
Pass: ( 5/ 5): tegra114-dalmore-a04, tegra124-jetson-tk1,
tegra124-nyan-big, tegra20-trimslice, tegra30-beaver
vmlinux object size
(delta in bytes from test_v4.8-rc5 (c6935931c1894ff857616ff8549b61236a19148f)):
text data bss total kernel
-4 +128 0 +124 defconfig
+528 0 0 +528 multi_v7_defconfig
+472 +128 0 +600 tegra_defconfig
Boot-time memory difference
(delta in bytes from test_v4.8-rc5 (c6935931c1894ff857616ff8549b61236a19148f))
avail rsrvd high freed board kconfig dtb
. . . . qemu-vexpress64 defconfig __internal
. . . . tegra114-dalmore-a04 multi_v7_defconfig tegra114-dalmore
. . . . tegra114-dalmore-a04 tegra_defconfig tegra114-dalmore
. . . . tegra124-jetson-tk1 multi_v7_defconfig tegra124-jetson-tk1
. . . . tegra124-jetson-tk1 tegra_defconfig tegra124-jetson-tk1
. . . . tegra124-nyan-big multi_v7_defconfig tegra124-nyan-big
. . . . tegra124-nyan-big tegra_defconfig tegra124-nyan-big
. . . . tegra132-norrin defconfig tegra132-norrin
. . . . tegra20-trimslice multi_v7_defconfig tegra20-trimslice
. . . . tegra20-trimslice tegra_defconfig tegra20-trimslice
. . . . tegra210-p2371-0000 defconfig tegra210-p2371-0000
. . . . tegra210-smaug defconfig tegra210-smaug
. . . . tegra30-beaver multi_v7_defconfig tegra30-beaver
. . . . tegra30-beaver tegra_defconfig tegra30-beaver
^ permalink raw reply
* [PATCH v4 07/10] ARM: Introduce MPIDR_LEVEL_SHIFT macro
From: Vladimir Murzin @ 2016-09-13 9:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160913083855.GC5680@cbox>
On 13/09/16 09:38, Christoffer Dall wrote:
> On Mon, Sep 12, 2016 at 03:49:21PM +0100, Vladimir Murzin wrote:
>> vgic-v3 driver uses architecture specific MPIDR_LEVEL_SHIFT macro to
>> encode the affinity in a form compatible with ICC_SGI* registers.
>> Unfortunately, that macro is missing on ARM, so let's add it.
>>
>> Cc: Russell King <rmk+kernel@armlinux.org.uk>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>> ---
>> arch/arm/include/asm/cputype.h | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
>> index 1ee94c7..e2d94c1 100644
>> --- a/arch/arm/include/asm/cputype.h
>> +++ b/arch/arm/include/asm/cputype.h
>> @@ -55,6 +55,7 @@
>>
>> #define MPIDR_LEVEL_BITS 8
>> #define MPIDR_LEVEL_MASK ((1 << MPIDR_LEVEL_BITS) - 1)
>> +#define MPIDR_LEVEL_SHIFT(level) (MPIDR_LEVEL_BITS * level)
>>
>
> I'm not sure I follow the correctness of this completely.
>
> This is called from vgic_v3_dispatch_sgi, which takes a u64 value, which
> may have something in the Aff3 field, which we now shift left 24 bits,
> but that is not the Aff3 field of AArch32's MPIDR.
>
> What is the rationale for this making sense again?
IIUC, in such case we construct mpidr which won't match in match_mpidr()
with the value we get from kvm_vcpu_get_mpidr_aff() and no SGI will be
sent to the guest.
Since we get that u64 value from the guest, I'd think it is something
wrong is going on in the guest in case Aff3 is non-zero; however, we can
hide it by zeroing out SGI Aff3 bits in access_gic_sgi().
Cheers
Vladimir
>
> Thanks,
> -Christoffer
>
>> #define MPIDR_AFFINITY_LEVEL(mpidr, level) \
>> ((mpidr >> (MPIDR_LEVEL_BITS * level)) & MPIDR_LEVEL_MASK)
>> --
>> 1.7.9.5
>>
>
>
^ permalink raw reply
* [PATCH 1/3] arm64: alternative: add auto-nop infrastructure
From: Ard Biesheuvel @ 2016-09-13 8:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160913085704.GC19689@leverpostej>
On 13 September 2016 at 09:57, Mark Rutland <mark.rutland@arm.com> wrote:
> On Tue, Sep 13, 2016 at 09:36:14AM +0100, Ard Biesheuvel wrote:
>> On 7 September 2016 at 11:07, Mark Rutland <mark.rutland@arm.com> wrote:
>> > In some cases, one side of an alternative sequence is simply a number of
>> > NOPs used to balance the other side. Keeping track of this manually is
>> > tedious, and the presence of large chains of NOPs makes the code more
>> > painful to read than necessary.
>> >
>> > To ameliorate matters, this patch adds a new alternative_else_nop_endif,
>> > which automatically balances an alternative sequence with a trivial NOP
>> > sled.
>> >
>> > In many cases, we would like a NOP-sled in the default case, and
>> > instructions patched in in the presence of a feature. To enable the NOPs
>> > to be generated automatically for this case, this patch also adds a new
>> > alternative_if, and updates alternative_else and alternative_endif to
>> > work with either alternative_if or alternative_endif.
>
> [...]
>
>> > +/*
>> > + * Begin an alternative code sequence.
>> > */
>> > .macro alternative_if_not cap
>> > + .set .Lasm_alt_mode, 0
>>
>> Given that only a single copy of this symbol will exist in an object
>> file, is it still possible to use both variants in a single
>> compilation/assembly unit?
>
> Yes.
>
> GAS allows the symbol to be set multiple times (so long as the
> assignments are constant values). The last assignment "wins" when it
> comes to output, but assembler macros are evaluated before this, and use
> the most recent assignment.
>
> In testing I hacked __kvm_call_hyp to use both:
>
> ENTRY(__kvm_call_hyp)
> alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> str lr, [sp, #-16]!
> hvc #0
> ldr lr, [sp], #16
> ret
> alternative_else_nop_endif
> alternative_if ARM64_HAS_VIRT_HOST_EXTN
> hvc #0x539
> alternative_else_nop_endif
> b __vhe_hyp_call
> ENDPROC(__kvm_call_hyp)
>
> Which, according to objdump gives me the expected result:
>
> Disassembly of section .text:
>
> 0000000000000000 <__kvm_call_hyp>:
> 0: f81f0ffe str x30, [sp,#-16]!
> 4: d4000002 hvc #0x0
> 8: f84107fe ldr x30, [sp],#16
> c: d65f03c0 ret
> 10: d503201f nop
> 14: 14000000 b 0 <__vhe_hyp_call>
>
> Disassembly of section .altinstr_replacement:
>
> 0000000000000000 <.altinstr_replacement>:
> 0: d503201f nop
> 4: d503201f nop
> 8: d503201f nop
> c: d503201f nop
> 10: d400a722 hvc #0x539
>
Thanks for the clarification,
Ard.
^ permalink raw reply
* [PATCH 1/3] arm64: alternative: add auto-nop infrastructure
From: Mark Rutland @ 2016-09-13 8:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu-KHePVj3gs+Q0wfgkVrD0nB0Yi+xLdbnGz-YZTTsPxFw@mail.gmail.com>
On Tue, Sep 13, 2016 at 09:36:14AM +0100, Ard Biesheuvel wrote:
> On 7 September 2016 at 11:07, Mark Rutland <mark.rutland@arm.com> wrote:
> > In some cases, one side of an alternative sequence is simply a number of
> > NOPs used to balance the other side. Keeping track of this manually is
> > tedious, and the presence of large chains of NOPs makes the code more
> > painful to read than necessary.
> >
> > To ameliorate matters, this patch adds a new alternative_else_nop_endif,
> > which automatically balances an alternative sequence with a trivial NOP
> > sled.
> >
> > In many cases, we would like a NOP-sled in the default case, and
> > instructions patched in in the presence of a feature. To enable the NOPs
> > to be generated automatically for this case, this patch also adds a new
> > alternative_if, and updates alternative_else and alternative_endif to
> > work with either alternative_if or alternative_endif.
[...]
> > +/*
> > + * Begin an alternative code sequence.
> > */
> > .macro alternative_if_not cap
> > + .set .Lasm_alt_mode, 0
>
> Given that only a single copy of this symbol will exist in an object
> file, is it still possible to use both variants in a single
> compilation/assembly unit?
Yes.
GAS allows the symbol to be set multiple times (so long as the
assignments are constant values). The last assignment "wins" when it
comes to output, but assembler macros are evaluated before this, and use
the most recent assignment.
In testing I hacked __kvm_call_hyp to use both:
ENTRY(__kvm_call_hyp)
alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
str lr, [sp, #-16]!
hvc #0
ldr lr, [sp], #16
ret
alternative_else_nop_endif
alternative_if ARM64_HAS_VIRT_HOST_EXTN
hvc #0x539
alternative_else_nop_endif
b __vhe_hyp_call
ENDPROC(__kvm_call_hyp)
Which, according to objdump gives me the expected result:
Disassembly of section .text:
0000000000000000 <__kvm_call_hyp>:
0: f81f0ffe str x30, [sp,#-16]!
4: d4000002 hvc #0x0
8: f84107fe ldr x30, [sp],#16
c: d65f03c0 ret
10: d503201f nop
14: 14000000 b 0 <__vhe_hyp_call>
Disassembly of section .altinstr_replacement:
0000000000000000 <.altinstr_replacement>:
0: d503201f nop
4: d503201f nop
8: d503201f nop
c: d503201f nop
10: d400a722 hvc #0x539
Thanks,
Mark.
^ permalink raw reply
* [PATCH v8 6/9] drm/mediatek: add dsi interrupt control
From: CK Hu @ 2016-09-13 8:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473681672-47144-7-git-send-email-yt.shen@mediatek.com>
Hi, YT:
On Mon, 2016-09-12 at 20:01 +0800, YT Shen wrote:
> From: shaoming chen <shaoming.chen@mediatek.com>
>
> add dsi interrupt control
>
> Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 78 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 78 insertions(+)
>
[snip...]
>
> +static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
> +{
> + u32 inten = DSI_INT_ALL_BITS;
> +
> + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO)
> + inten &= ~(TE_RDY_INT_FLAG | EXT_TE_RDY_INT_FLAG);
> +
> + writel(inten, dsi->regs + DSI_INTEN);
> +}
> +
[snip...]
> +
> +static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
> +{
> + struct mtk_dsi *dsi = dev_id;
> + u32 status, tmp;
> + u32 flag = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
Why do you only process these three irq? You also enable TE_RDY_INT_FLAG
& EXT_TE_RDY_INT_FLAG in mtk_dsi_set_interrupt_enable(). Process these
two irq here or not enable them in mtk_dsi_set_interrupt_enable().
Regards,
CK
> +
> + status = readl(dsi->regs + DSI_INTSTA) & flag;
> +
> + if (status) {
> + do {
> + mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK);
> + tmp = readl(dsi->regs + DSI_INTSTA);
> + } while (tmp & DSI_BUSY);
> +
> + mtk_dsi_mask(dsi, DSI_INTSTA, status, 0);
> + mtk_dsi_irq_data_set(dsi, status);
> + wake_up_interruptible(&dsi->irq_wait_queue);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
^ permalink raw reply
* [PATCH v4 10/10] ARM: KVM: Support vgic-v3
From: Christoffer Dall @ 2016-09-13 8:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473691764-29424-11-git-send-email-vladimir.murzin@arm.com>
On Mon, Sep 12, 2016 at 03:49:24PM +0100, Vladimir Murzin wrote:
> This patch allows to build and use vgic-v3 in 32-bit mode.
>
> Unfortunately, it can not be split in several steps without extra
> stubs to keep patches independent and bisectable. For instance,
> virt/kvm/arm/vgic/vgic-v3.c uses function from vgic-v3-sr.c, handling
> access to GICv3 cpu interface from the guest requires vgic_v3.vgic_sre
> to be already defined.
>
> It is how support has been done:
>
> * handle SGI requests from the guest
>
> * report configured SRE on access to GICv3 cpu interface from the guest
>
> * required vgic-v3 macros are provided via uapi.h
>
> * static keys are used to select GIC backend
>
> * to make vgic-v3 build KVM_ARM_VGIC_V3 guard is removed along with
> the static inlines
>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply
* [PATCH v4 09/10] ARM: gic-v3: Introduce 32-to-64-bit mappings for GICv3 cpu registers
From: Christoffer Dall @ 2016-09-13 8:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473691764-29424-10-git-send-email-vladimir.murzin@arm.com>
On Mon, Sep 12, 2016 at 03:49:23PM +0100, Vladimir Murzin wrote:
> vgic-v3 save/restore routines are written in such way that they map
> arm64 system register naming nicely, but it does not fit to arm
> world. To keep virt/kvm/arm/hyp/vgic-v3-sr.c untouched we create a
> mapping with a function for each register mapping the 32-bit to the
> 64-bit accessors.
>
> Please, note that 64-bit wide ICH_LR is split in two 32-bit halves
> (ICH_LR and ICH_LRC) accessed independently.
>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply
* [PATCH v4 08/10] ARM: Move system register accessors to asm/cp15.h
From: Christoffer Dall @ 2016-09-13 8:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473691764-29424-9-git-send-email-vladimir.murzin@arm.com>
On Mon, Sep 12, 2016 at 03:49:22PM +0100, Vladimir Murzin wrote:
> Headers linux/irqchip/arm-gic.v3.h and arch/arm/include/asm/kvm_hyp.h
> are included in virt/kvm/arm/hyp/vgic-v3-sr.c and both define macros
> called __ACCESS_CP15 and __ACCESS_CP15_64 which obviously creates a
> conflict. These macros were introduced independently for GIC and KVM
> and, in fact, do the same thing.
>
> As an option we could add prefixes to KVM and GIC version of macros so
> they won't clash, but it'd introduce code duplication. Alternatively,
> we could keep macro in, say, GIC header and include it in KVM one (or
> vice versa), but such dependency would not look nicer.
>
> So we follow arm64 way (it handles this via sysreg.h) and move only
> single set of macros to asm/cp15.h
>
> Cc: Russell King <rmk+kernel@armlinux.org.uk>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
> arch/arm/include/asm/arch_gicv3.h | 27 +++++++++++----------------
> arch/arm/include/asm/cp15.h | 15 +++++++++++++++
> arch/arm/include/asm/kvm_hyp.h | 15 +--------------
> 3 files changed, 27 insertions(+), 30 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch_gicv3.h b/arch/arm/include/asm/arch_gicv3.h
> index e08d151..af25c32 100644
> --- a/arch/arm/include/asm/arch_gicv3.h
> +++ b/arch/arm/include/asm/arch_gicv3.h
> @@ -22,9 +22,7 @@
>
> #include <linux/io.h>
> #include <asm/barrier.h>
> -
> -#define __ACCESS_CP15(CRn, Op1, CRm, Op2) p15, Op1, %0, CRn, CRm, Op2
> -#define __ACCESS_CP15_64(Op1, CRm) p15, Op1, %Q0, %R0, CRm
> +#include <asm/cp15.h>
>
> #define ICC_EOIR1 __ACCESS_CP15(c12, 0, c12, 1)
> #define ICC_DIR __ACCESS_CP15(c12, 0, c11, 1)
> @@ -102,58 +100,55 @@
>
> static inline void gic_write_eoir(u32 irq)
> {
> - asm volatile("mcr " __stringify(ICC_EOIR1) : : "r" (irq));
> + write_sysreg(irq, ICC_EOIR1);
> isb();
> }
>
> static inline void gic_write_dir(u32 val)
> {
> - asm volatile("mcr " __stringify(ICC_DIR) : : "r" (val));
> + write_sysreg(val, ICC_DIR);
> isb();
> }
>
> static inline u32 gic_read_iar(void)
> {
> - u32 irqstat;
> + u32 irqstat = read_sysreg(ICC_IAR1);
>
> - asm volatile("mrc " __stringify(ICC_IAR1) : "=r" (irqstat));
> dsb(sy);
> +
> return irqstat;
> }
>
> static inline void gic_write_pmr(u32 val)
> {
> - asm volatile("mcr " __stringify(ICC_PMR) : : "r" (val));
> + write_sysreg(val, ICC_PMR);
> }
>
> static inline void gic_write_ctlr(u32 val)
> {
> - asm volatile("mcr " __stringify(ICC_CTLR) : : "r" (val));
> + write_sysreg(val, ICC_CTLR);
> isb();
> }
>
> static inline void gic_write_grpen1(u32 val)
> {
> - asm volatile("mcr " __stringify(ICC_IGRPEN1) : : "r" (val));
> + write_sysreg(val, ICC_IGRPEN1);
> isb();
> }
>
> static inline void gic_write_sgi1r(u64 val)
> {
> - asm volatile("mcrr " __stringify(ICC_SGI1R) : : "r" (val));
> + write_sysreg(val, ICC_SGI1R);
> }
>
> static inline u32 gic_read_sre(void)
> {
> - u32 val;
> -
> - asm volatile("mrc " __stringify(ICC_SRE) : "=r" (val));
> - return val;
> + return read_sysreg(ICC_SRE);
> }
>
> static inline void gic_write_sre(u32 val)
> {
> - asm volatile("mcr " __stringify(ICC_SRE) : : "r" (val));
> + write_sysreg(val, ICC_SRE);
> isb();
> }
>
> diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
> index c3f1152..dbdbce1 100644
> --- a/arch/arm/include/asm/cp15.h
> +++ b/arch/arm/include/asm/cp15.h
> @@ -49,6 +49,21 @@
>
> #ifdef CONFIG_CPU_CP15
>
> +#define __ACCESS_CP15(CRn, Op1, CRm, Op2) \
> + "mrc", "mcr", __stringify(p15, Op1, %0, CRn, CRm, Op2), u32
> +#define __ACCESS_CP15_64(Op1, CRm) \
> + "mrrc", "mcrr", __stringify(p15, Op1, %Q0, %R0, CRm), u64
> +
> +#define __read_sysreg(r, w, c, t) ({ \
> + t __val; \
> + asm volatile(r " " c : "=r" (__val)); \
> + __val; \
> +})
> +#define read_sysreg(...) __read_sysreg(__VA_ARGS__)
> +
> +#define __write_sysreg(v, r, w, c, t) asm volatile(w " " c : : "r" ((t)(v)))
> +#define write_sysreg(v, ...) __write_sysreg(v, __VA_ARGS__)
> +
> extern unsigned long cr_alignment; /* defined in entry-armv.S */
>
> static inline unsigned long get_cr(void)
> diff --git a/arch/arm/include/asm/kvm_hyp.h b/arch/arm/include/asm/kvm_hyp.h
> index 6eaff28..e604ad68 100644
> --- a/arch/arm/include/asm/kvm_hyp.h
> +++ b/arch/arm/include/asm/kvm_hyp.h
> @@ -20,28 +20,15 @@
>
> #include <linux/compiler.h>
> #include <linux/kvm_host.h>
> +#include <asm/cp15.h>
> #include <asm/kvm_mmu.h>
> #include <asm/vfp.h>
>
> #define __hyp_text __section(.hyp.text) notrace
>
> -#define __ACCESS_CP15(CRn, Op1, CRm, Op2) \
> - "mrc", "mcr", __stringify(p15, Op1, %0, CRn, CRm, Op2), u32
> -#define __ACCESS_CP15_64(Op1, CRm) \
> - "mrrc", "mcrr", __stringify(p15, Op1, %Q0, %R0, CRm), u64
> #define __ACCESS_VFP(CRn) \
> "mrc", "mcr", __stringify(p10, 7, %0, CRn, cr0, 0), u32
>
> -#define __write_sysreg(v, r, w, c, t) asm volatile(w " " c : : "r" ((t)(v)))
> -#define write_sysreg(v, ...) __write_sysreg(v, __VA_ARGS__)
> -
> -#define __read_sysreg(r, w, c, t) ({ \
> - t __val; \
> - asm volatile(r " " c : "=r" (__val)); \
> - __val; \
> -})
> -#define read_sysreg(...) __read_sysreg(__VA_ARGS__)
> -
> #define write_special(v, r) \
> asm volatile("msr " __stringify(r) ", %0" : : "r" (v))
> #define read_special(r) ({ \
> --
> 1.7.9.5
>
^ permalink raw reply
* [PATCH v4 06/10] KVM: arm: vgic: Support 64-bit data manipulation on 32-bit host systems
From: Christoffer Dall @ 2016-09-13 8:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473691764-29424-7-git-send-email-vladimir.murzin@arm.com>
On Mon, Sep 12, 2016 at 03:49:20PM +0100, Vladimir Murzin wrote:
> We have couple of 64-bit registers defined in GICv3 architecture, so
> unsigned long accesses to these registers will only access a single
> 32-bit part of that regitser. On the other hand these registers can't
> be accessed as 64-bit with a single instruction like ldrd/strd or
> ldmia/stmia if we run a 32-bit host because KVM does not support
> access to MMIO space done by these instructions.
>
> It means that a 32-bit guest accesses these registers in 32-bit
> chunks, so the only thing we need to do is to ensure that
> extract_bytes() always takes 64-bit data.
>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
> virt/kvm/arm/vgic/vgic-mmio-v3.c | 2 +-
> virt/kvm/arm/vgic/vgic-mmio.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> index 6385ed5..0d3c76a 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> @@ -23,7 +23,7 @@
> #include "vgic-mmio.h"
>
> /* extract @num bytes at @offset bytes offset in data */
> -unsigned long extract_bytes(unsigned long data, unsigned int offset,
> +unsigned long extract_bytes(u64 data, unsigned int offset,
> unsigned int num)
> {
> return (data >> (offset * 8)) & GENMASK_ULL(num * 8 - 1, 0);
> diff --git a/virt/kvm/arm/vgic/vgic-mmio.h b/virt/kvm/arm/vgic/vgic-mmio.h
> index 0b3ecf9..80f92ce 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio.h
> +++ b/virt/kvm/arm/vgic/vgic-mmio.h
> @@ -96,7 +96,7 @@ unsigned long vgic_data_mmio_bus_to_host(const void *val, unsigned int len);
> void vgic_data_host_to_mmio_bus(void *buf, unsigned int len,
> unsigned long data);
>
> -unsigned long extract_bytes(unsigned long data, unsigned int offset,
> +unsigned long extract_bytes(u64 data, unsigned int offset,
> unsigned int num);
>
> u64 update_64bit_reg(u64 reg, unsigned int offset, unsigned int len,
> --
> 1.7.9.5
>
^ permalink raw reply
* [PATCH v4 05/10] KVM: arm: vgic: Fix compiler warnings when built for 32-bit
From: Christoffer Dall @ 2016-09-13 8:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473691764-29424-6-git-send-email-vladimir.murzin@arm.com>
On Mon, Sep 12, 2016 at 03:49:19PM +0100, Vladimir Murzin wrote:
> Well, this patch is looking ahead of time, but we'll get following
> compiler warnings as soon as we introduce vgic-v3 to 32-bit world
>
> CC arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-mmio-v3.o
> arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-mmio-v3.c: In function 'vgic_mmio_read_v3r_typer':
> arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-mmio-v3.c:184:35: warning: left shift count >= width of type [-Wshift-count-overflow]
> value = (mpidr & GENMASK(23, 0)) << 32;
> ^
> In file included from ./include/linux/kernel.h:10:0,
> from ./include/asm-generic/bug.h:13,
> from ./arch/arm/include/asm/bug.h:59,
> from ./include/linux/bug.h:4,
> from ./include/linux/io.h:23,
> from ./arch/arm/include/asm/arch_gicv3.h:23,
> from ./include/linux/irqchip/arm-gic-v3.h:411,
> from arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-mmio-v3.c:14:
> arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-mmio-v3.c: In function 'vgic_v3_dispatch_sgi':
> ./include/linux/bitops.h:6:24: warning: left shift count >= width of type [-Wshift-count-overflow]
> #define BIT(nr) (1UL << (nr))
> ^
> arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-mmio-v3.c:614:20: note: in expansion of macro 'BIT'
> broadcast = reg & BIT(ICC_SGI1R_IRQ_ROUTING_MODE_BIT);
> ^
> Let's fix them now.
>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply
* [PATCH v4 04/10] KVM: arm64: vgic-its: Introduce config option to guard ITS specific code
From: Christoffer Dall @ 2016-09-13 8:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473691764-29424-5-git-send-email-vladimir.murzin@arm.com>
On Mon, Sep 12, 2016 at 03:49:18PM +0100, Vladimir Murzin wrote:
> By now ITS code guarded with KVM_ARM_VGIC_V3 config option which was
> introduced to hide everything specific to vgic-v3 from 32-bit world.
> We are going to support vgic-v3 in 32-bit world and KVM_ARM_VGIC_V3
> will gone, but we don't have support for ITS there yet and we need to
> continue keeping ITS away.
> Introduce the new config option to prevent ITS code being build in
> 32-bit mode when support for vgic-v3 is done.
>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply
* [PATCH v4 03/10] arm64: KVM: Move vgic-v3 save/restore to virt/kvm/arm/hyp
From: Christoffer Dall @ 2016-09-13 8:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473691764-29424-4-git-send-email-vladimir.murzin@arm.com>
On Mon, Sep 12, 2016 at 03:49:17PM +0100, Vladimir Murzin wrote:
> So we can reuse the code under arch/arm
>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply
* [PATCH 1/5] clk: add support for runtime pm
From: Ulf Hansson @ 2016-09-13 8:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1472737551-15272-2-git-send-email-m.szyprowski@samsung.com>
On 1 September 2016 at 15:45, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> Registers for some clocks might be located in the SOC area, which are under the
> power domain. To enable access to those registers respective domain has to be
> turned on. Additionally, registers for such clocks will usually loose its
> contents when power domain is turned off, so additional saving and restoring of
> them might be needed in the clock controller driver.
This is indeed correct, I can confirm that the UX500 SoC's PRCC clock
controllers also needs to be managed like this.
>
> This patch adds basic infrastructure in the clocks core to allow implementing
> driver for such clocks under power domains. Clock provider can supply a
> struct device pointer, which is the used by clock core for tracking and managing
> clock's controller runtime pm state. Each clk_prepare() operation
> will first call pm_runtime_get_sync() on the supplied device, while
> clk_unprepare() will do pm_runtime_put() at the end.
This make sense!
>
> Additional calls to pm_runtime_get/put functions are required to ensure that any
> register access (like calculating/chaning clock rates) will be done with clock
/s/chaning/changing
> controller in active runtime state.
/s/active runtime/runtime resumed
>
> Special handling of the case when runtime pm is disabled for clock controller's
> device is needed to let this feature work properly also during system sleep
> suspend/resume operations (runtime pm is first disabled before entering sleep
> state's, but controller is usually still operational until its suspend pm
> callback is called).
This needs to be clarified. I agree we need to cover system PM as
well, but let's try be a bit more precise about it.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
I would also like to extend the change log to describe a little bit of
how a clk provider should interact with this new and nice feature.
Something like:
*) It needs to provide a struct device to the core when registering
the provider.
**) It needs to enable runtime PM.
***) It needs to make sure the runtime PM status of the controller
device reflects the HW state.
> ---
> drivers/clk/clk.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 76 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 820a939fb6bb..a1934e9b4e95 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -21,6 +21,7 @@
> #include <linux/of.h>
> #include <linux/device.h>
> #include <linux/init.h>
> +#include <linux/pm_runtime.h>
> #include <linux/sched.h>
> #include <linux/clkdev.h>
>
> @@ -46,6 +47,7 @@ struct clk_core {
> const struct clk_ops *ops;
> struct clk_hw *hw;
> struct module *owner;
> + struct device *dev;
> struct clk_core *parent;
> const char **parent_names;
> struct clk_core **parents;
> @@ -87,6 +89,42 @@ struct clk {
> struct hlist_node clks_node;
> };
>
> +/*** runtime pm ***/
> +static int clk_pm_runtime_get(struct clk_core *core)
> +{
> + int ret = 0;
> +
> + if (!core->dev)
> + return 0;
> +
> + if (pm_runtime_enabled(core->dev)) {
Why do you need to check for this?
> + ret = pm_runtime_get_sync(core->dev);
> + } else {
> + if (!pm_runtime_status_suspended(core->dev))
> + pm_runtime_get_noresume(core->dev);
This looks weird. I guess it's related to the system PM case somehow?
> + }
> + return ret < 0 ? ret : 0;
> +}
> +
> +static void clk_pm_runtime_put(struct clk_core *core)
> +{
Similar comments as for clk_pm_runtime_get().
> + if (!core->dev)
> + return;
> +
> + if (pm_runtime_enabled(core->dev))
> + pm_runtime_put(core->dev);
> + else
> + pm_runtime_put_noidle(core->dev);
> +}
> +
> +static bool clk_pm_runtime_suspended(struct clk_core *core)
> +{
> + if (!core->dev)
> + return 0;
> +
> + return pm_runtime_suspended(core->dev);
> +}
> +
> /*** locking ***/
> static void clk_prepare_lock(void)
> {
> @@ -150,6 +188,9 @@ static void clk_enable_unlock(unsigned long flags)
>
> static bool clk_core_is_prepared(struct clk_core *core)
> {
> + if (clk_pm_runtime_suspended(core))
> + return false;
> +
This isn't safe, as even if the clock controller is runtime resumed at
this point, that's *not* a guarantee that is stays runtime resumed
while invoking the ->ops->is_prepared().
Instead you must call a pm_runtime_get_noresume() before you check the
runtime PM status, as that should avoid the device from being runtime
suspended. Then when the ->ops->is_prepared() has been invoked, we
should call pm_runtime_put().
Although, I am not sure the above change becomes entirely correct as I
think we are mixing the runtime PM status with the clock prepare
status here. In other words, the next time the clock controller
becomes runtime resumed, it may very well restore some register
context which may prepare the clock, unless someone explicitly has
unprepared it.
Of course, it all depends on how clk_core_is_prepared() is used by the
clock framework.
> /*
> * .is_prepared is optional for clocks that can prepare
> * fall back to software usage counter if it is missing
> @@ -162,6 +203,9 @@ static bool clk_core_is_prepared(struct clk_core *core)
>
> static bool clk_core_is_enabled(struct clk_core *core)
> {
> + if (clk_pm_runtime_suspended(core))
> + return false;
> +
Similar comment as for clk_core_is_prepared().
> /*
> * .is_enabled is only mandatory for clocks that gate
> * fall back to software usage counter if .is_enabled is missing
> @@ -489,6 +533,8 @@ static void clk_core_unprepare(struct clk_core *core)
> if (core->ops->unprepare)
> core->ops->unprepare(core->hw);
>
> + clk_pm_runtime_put(core);
> +
> trace_clk_unprepare_complete(core);
> clk_core_unprepare(core->parent);
> }
> @@ -530,10 +576,14 @@ static int clk_core_prepare(struct clk_core *core)
> return 0;
>
> if (core->prepare_count == 0) {
> - ret = clk_core_prepare(core->parent);
> + ret = clk_pm_runtime_get(core);
> if (ret)
> return ret;
>
> + ret = clk_core_prepare(core->parent);
> + if (ret)
> + goto runtime_put;
> +
> trace_clk_prepare(core);
>
> if (core->ops->prepare)
> @@ -541,15 +591,18 @@ static int clk_core_prepare(struct clk_core *core)
>
> trace_clk_prepare_complete(core);
>
> - if (ret) {
> - clk_core_unprepare(core->parent);
> - return ret;
> - }
> + if (ret)
> + goto unprepare;
> }
>
> core->prepare_count++;
>
> return 0;
> +unprepare:
> + clk_core_unprepare(core->parent);
> +runtime_put:
> + clk_pm_runtime_put(core);
> + return ret;
> }
>
> static int clk_core_prepare_lock(struct clk_core *core)
> @@ -1563,6 +1616,7 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
> {
> struct clk_core *top, *fail_clk;
> unsigned long rate = req_rate;
> + int ret = 0;
>
> if (!core)
> return 0;
> @@ -1579,21 +1633,28 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
> if (!top)
> return -EINVAL;
>
> + ret = clk_pm_runtime_get(core);
> + if (ret)
> + return ret;
> +
> /* notify that we are about to change rates */
> fail_clk = clk_propagate_rate_change(top, PRE_RATE_CHANGE);
> if (fail_clk) {
> pr_debug("%s: failed to set %s rate\n", __func__,
> fail_clk->name);
> clk_propagate_rate_change(top, ABORT_RATE_CHANGE);
> - return -EBUSY;
> + ret = -EBUSY;
> + goto err;
> }
>
> /* change the rates */
> clk_change_rate(top);
>
> core->req_rate = req_rate;
> +err:
> + clk_pm_runtime_put(core);
>
> - return 0;
> + return ret;
> }
>
> /**
> @@ -1824,12 +1885,16 @@ static int clk_core_set_parent(struct clk_core *core, struct clk_core *parent)
> p_rate = parent->rate;
> }
>
> + ret = clk_pm_runtime_get(core);
> + if (ret)
> + goto out;
> +
> /* propagate PRE_RATE_CHANGE notifications */
> ret = __clk_speculate_rates(core, p_rate);
>
> /* abort if a driver objects */
> if (ret & NOTIFY_STOP_MASK)
> - goto out;
> + goto runtime_put;
>
> /* do the re-parent */
> ret = __clk_set_parent(core, parent, p_index);
> @@ -1842,6 +1907,8 @@ static int clk_core_set_parent(struct clk_core *core, struct clk_core *parent)
> __clk_recalc_accuracies(core);
> }
>
> +runtime_put:
> + clk_pm_runtime_put(core);
> out:
> clk_prepare_unlock();
>
> @@ -2546,6 +2613,7 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw)
> goto fail_name;
> }
> core->ops = hw->init->ops;
> + core->dev = dev;
> if (dev && dev->driver)
> core->owner = dev->driver->owner;
> core->hw = hw;
> --
> 1.9.1
>
I believe we are also accessing the clock controller HW from the
late_initcall_sync(clk_disable_unused) function.
More precisely, in clk_disable_unused_subtree(), we probably need a
pm_runtime_get_sync() before calling clk_core_is_enabled(). And then
restore that with a pm_runtime_put() after the clock has been
disabled.
The similar is needed in clk_unprepare_unused_subtree().
Kind regards
Uffe
^ permalink raw reply
* [PATCH 2/2] crypto: arm64/aes-ctr: fix NULL dereference in tail processing
From: Ard Biesheuvel @ 2016-09-13 8:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473756533-21078-1-git-send-email-ard.biesheuvel@linaro.org>
The AES-CTR glue code avoids calling into the blkcipher API for the
tail portion of the walk, by comparing the remainder of walk.nbytes
modulo AES_BLOCK_SIZE with the residual nbytes, and jumping straight
into the tail processing block if they are equal. This tail processing
block checks whether nbytes != 0, and does nothing otherwise.
However, in case of an allocation failure in the blkcipher layer, we
may enter this code with walk.nbytes == 0, while nbytes > 0. In this
case, we should not dereference the source and destination pointers,
since they may be NULL. So instead of checking for nbytes != 0, check
for (walk.nbytes % AES_BLOCK_SIZE) != 0, which implies the former in
non-error conditions.
Fixes: 49788fe2a128 ("arm64/crypto: AES-ECB/CBC/CTR/XTS using ARMv8 NEON and Crypto Extensions")
Reported-by: xiakaixu <xiakaixu@huawei.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/crypto/aes-glue.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/crypto/aes-glue.c b/arch/arm64/crypto/aes-glue.c
index 5c888049d061..6b2aa0fd6cd0 100644
--- a/arch/arm64/crypto/aes-glue.c
+++ b/arch/arm64/crypto/aes-glue.c
@@ -216,7 +216,7 @@ static int ctr_encrypt(struct blkcipher_desc *desc, struct scatterlist *dst,
err = blkcipher_walk_done(desc, &walk,
walk.nbytes % AES_BLOCK_SIZE);
}
- if (nbytes) {
+ if (walk.nbytes % AES_BLOCK_SIZE) {
u8 *tdst = walk.dst.virt.addr + blocks * AES_BLOCK_SIZE;
u8 *tsrc = walk.src.virt.addr + blocks * AES_BLOCK_SIZE;
u8 __aligned(8) tail[AES_BLOCK_SIZE];
--
2.7.4
^ permalink raw reply related
* [PATCH 1/2] crypto: arm/aes-ctr: fix NULL dereference in tail processing
From: Ard Biesheuvel @ 2016-09-13 8:48 UTC (permalink / raw)
To: linux-arm-kernel
The AES-CTR glue code avoids calling into the blkcipher API for the
tail portion of the walk, by comparing the remainder of walk.nbytes
modulo AES_BLOCK_SIZE with the residual nbytes, and jumping straight
into the tail processing block if they are equal. This tail processing
block checks whether nbytes != 0, and does nothing otherwise.
However, in case of an allocation failure in the blkcipher layer, we
may enter this code with walk.nbytes == 0, while nbytes > 0. In this
case, we should not dereference the source and destination pointers,
since they may be NULL. So instead of checking for nbytes != 0, check
for (walk.nbytes % AES_BLOCK_SIZE) != 0, which implies the former in
non-error conditions.
Fixes: 86464859cc77 ("crypto: arm - AES in ECB/CBC/CTR/XTS modes using ARMv8 Crypto Extensions")
Reported-by: xiakaixu <xiakaixu@huawei.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm/crypto/aes-ce-glue.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/crypto/aes-ce-glue.c b/arch/arm/crypto/aes-ce-glue.c
index da3c0428507b..aef022a87c53 100644
--- a/arch/arm/crypto/aes-ce-glue.c
+++ b/arch/arm/crypto/aes-ce-glue.c
@@ -284,7 +284,7 @@ static int ctr_encrypt(struct blkcipher_desc *desc, struct scatterlist *dst,
err = blkcipher_walk_done(desc, &walk,
walk.nbytes % AES_BLOCK_SIZE);
}
- if (nbytes) {
+ if (walk.nbytes % AES_BLOCK_SIZE) {
u8 *tdst = walk.dst.virt.addr + blocks * AES_BLOCK_SIZE;
u8 *tsrc = walk.src.virt.addr + blocks * AES_BLOCK_SIZE;
u8 __aligned(8) tail[AES_BLOCK_SIZE];
--
2.7.4
^ permalink raw reply related
* [PATCH v5 07/14] drivers: acpi: iort: add support for ARM SMMU platform devices creation
From: Hanjun Guo @ 2016-09-13 8:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160913082441.GA28804@red-moon>
On 2016/9/13 16:24, Lorenzo Pieralisi wrote:
> On Tue, Sep 13, 2016 at 04:15:31PM +0800, Hanjun Guo wrote:
>
> [...]
>
>>>> +static acpi_status __init iort_match_iommu_callback(struct
>>>> acpi_iort_node *node,
>>>> + void *context)
>>>> +{
>>>> + int ret;
>>>> + struct fwnode_handle *fwnode;
>>>> +
>>>> + fwnode = iort_get_fwnode(node);
>>>> +
>>>> + if (!fwnode)
>>>> + return AE_NOT_FOUND;
>>>> +
>>>> + ret = iort_add_smmu_platform_device(fwnode, node);
>>>> + if (ret) {
>>>> + pr_err("Error in platform device creation\n");
>>>> + return AE_ERROR;
>>>> + }
>>>> +
>>>> + return AE_OK;
>>>> +}
>>>> +
>>>> +static void __init iort_smmu_init(void)
>>>> +{
>>>> + iort_scan_node(ACPI_IORT_NODE_SMMU, iort_match_iommu_callback,
>>>> NULL);
>>>> + iort_scan_node(ACPI_IORT_NODE_SMMU_V3, iort_match_iommu_callback,
>>>> NULL);
>>>
>>> Since iort_scan_node() returns after the first successful match it finds,
>>> only the first SMMU_V3 in my IORT is being enumerated. I think you need
>>> to go back to the "iterator" like approach you had been using or make
>>> iort_match_iommu_callback() always return a non-AE_OK value so the scan
>>> continues and has a chance to visit all of the SMMU_V3 nodes.
>>
>> Please use the updated version of IORT patch (aka Tomasz's v11)
>> then things will work fine.
>
> Nate is right, I was too keen on using iort_scan_node(), it does
> not really work here (unless as he said I return a value !AE_OK in
> the callback, which is horrible), I reverted back to the iterator
> approach and I can push out a fixed up branch if useful before next
> posting.
Ah, sorry, I just noticed "the first SMMU_V3" which is pretty similar
with the second problem which is noticed by Nate...
Thanks
Hanjun
^ 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