* [PATCH 1/2] ARM: mm: fix set_memory_*() bounds checks
From: Dave Gerlach @ 2016-11-29 15:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <E1c8r91-0007wL-Fl@rmk-PC.armlinux.org.uk>
Hi,
On 11/21/2016 10:08 AM, Russell King wrote:
> The set_memory_*() bounds checks are buggy on several fronts:
>
> 1. They fail to round the region size up if the passed address is not
> page aligned.
> 2. The region check was incomplete, and didn't correspond with what
> was being asked of apply_to_page_range()
>
> So, rework change_memory_common() to fix these problems, adding an
> "in_region()" helper to determine whether the start & size fit within
> the provided region start and stop addresses.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> arch/arm/mm/pageattr.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/arch/arm/mm/pageattr.c b/arch/arm/mm/pageattr.c
> index d19b1ad29b07..db09f2e7efda 100644
> --- a/arch/arm/mm/pageattr.c
> +++ b/arch/arm/mm/pageattr.c
> @@ -34,28 +34,28 @@ static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long addr,
> return 0;
> }
>
> +static bool in_range(unsigned long start, unsigned long size,
> + unsigned long range_start, unsigned long range_end)
> +{
> + return start >= range_start && start < range_end &&
> + size <= range_end - start;
> +}
> +
> static int change_memory_common(unsigned long addr, int numpages,
> pgprot_t set_mask, pgprot_t clear_mask)
> {
> - unsigned long start = addr;
> - unsigned long size = PAGE_SIZE*numpages;
> - unsigned long end = start + size;
> + unsigned long start = addr & PAGE_SIZE;
This doesn't work as is, I believe 'start' should be set to
PAGE_ALIGN(addr), addr & PAGE_SIZE as it is doesn't make sense. If I
make this change this code works ok.
Regards,
Dave
> + unsigned long end = PAGE_ALIGN(addr) + numpages * PAGE_SIZE;
> + unsigned long size = end - start;
> int ret;
> struct page_change_data data;
>
> - if (!IS_ALIGNED(addr, PAGE_SIZE)) {
> - start &= PAGE_MASK;
> - end = start + size;
> - WARN_ON_ONCE(1);
> - }
> + WARN_ON_ONCE(start != addr);
>
> - if (!numpages)
> + if (!size)
> return 0;
>
> - if (start < MODULES_VADDR || start >= MODULES_END)
> - return -EINVAL;
> -
> - if (end < MODULES_VADDR || start >= MODULES_END)
> + if (!in_range(start, size, MODULES_VADDR, MODULES_END))
> return -EINVAL;
>
> data.set_mask = set_mask;
>
^ permalink raw reply
* [PATCH v3 13/13] clocksource/drivers/rockchip_timer: Prevent ftrace recursion
From: Alexander Kochetkov @ 2016-11-29 15:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7837485.H9nfDmPu0F@diego>
Hello Heiko!
Thank you for patch review!
> 29 ????. 2016 ?., ? 18:01, Heiko St?bner <heiko@sntech.de> ???????(?):
>
> you introduced the issue yourself in patch 11/13. In general any patch should
> never leave the kernel in a worse state than it was before, so no patch should
> ever introduce known issues itself.
Agree.
> In that line of thought, don't patches 10+11 introduce warnings about unused
> functions as well when applied without patch 12?
7 - yes
10 - not
11 - yes
> 29 ????. 2016 ?., ? 18:03, Heiko St?bner <heiko@sntech.de> ???????(?):
>
> Then why do you need another patch to remove them and don't do that in the
> patch removing their respective usage?
To make 7 more readable. Overwise patch messed up and unreadable.
It?s hard to track what going on in the patch.
> Maybe merge them?
I'll merge all of them.
P.S.
here comment from another thead about the patches.
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-November/thread.html#470957
> 29 ????. 2016 ?., ? 18:07, Robin Murphy <robin.murphy@arm.com> ???????(?):
>
> 3288 (and probably anything newer) is irrelevant to this discussion, as
> it has the arch timer interface - that may be busted in other ways (such
> as not being correctly set up by firmware and not being always-on as it
> should), but frequency is not one of them. This only affects
> Cortex-A9/A5 based parts.
So I update comments for patches.
Looks, like I study archeology with my patches, as all new ARM chips not
affected by the problem anymore.
Regards,
Alexander.
^ permalink raw reply
* [PATCH V5 03/10] efi: parse ARMv8 processor error
From: Baicar, Tyler @ 2016-11-29 15:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <58388193.6000202@arm.com>
Hello James,
On 11/25/2016 11:23 AM, James Morse wrote:
> Hi Tyler,
>
> On 21/11/16 22:35, Tyler Baicar wrote:
>> Add support for ARMv8 Common Platform Error Record (CPER).
>> UEFI 2.6 specification adds support for ARMv8 specific
>> processor error information to be reported as part of the
>> CPER records. This provides more detail on for processor error logs.
> I think I'm missing a big part of the puzzle here, I will come back to this next
> week. I can't quite line up some of the masks and shifts with the table
> descriptions in the UEFI spec[0].
It looks like there was some misunderstanding when the context info
parsing was added here
(probably because the spec has some issues that I describe below).
I'll need to clean quite a bit of the context info parsing up. I didn't
catch this earlier because
we aren't reporting context info in firmware right now for the errors I
have been testing.
>> diff --git a/include/linux/cper.h b/include/linux/cper.h
>> index 13ea41c..2a9d553 100644
>> --- a/include/linux/cper.h
>> +++ b/include/linux/cper.h
>> @@ -180,6 +185,10 @@ enum {
>> #define CPER_SEC_PROC_IPF \
>> UUID_LE(0xE429FAF1, 0x3CB7, 0x11D4, 0x0B, 0xCA, 0x07, 0x00, \
>> 0x80, 0xC7, 0x3C, 0x88, 0x81)
>> +/* Processor Specific: ARMv8 */
>> +#define CPER_SEC_PROC_ARMV8 \
>> + UUID_LE(0xE19E3D16, 0xBC11, 0x11E4, 0x9C, 0xAA, 0xC2, 0x05, \
>> + 0x1D, 0x5D, 0x46, 0xB0)
> Nit: UEFI v2.6 N.2.2 (table 249) describes this as 'ARM' not 'ARMV8' (which is
> an architectural version).
I'll change it in the next set.
>> /* Platform Memory */
>> #define CPER_SEC_PLATFORM_MEM \
>> UUID_LE(0xA5BC1114, 0x6F64, 0x4EDE, 0xB8, 0x63, 0x3E, 0x83, \
>> @@ -255,6 +264,34 @@ enum {
>>
>> #define CPER_PCIE_SLOT_SHIFT 3
>>
>> +#define CPER_ARMV8_ERR_INFO_NUM_MASK 0x00000000000000FF
>> +#define CPER_ARMV8_CTX_INFO_NUM_MASK 0x0000000000FFFF00
> Table 260 describes both ERR_INFO_NUM and CONTEXT_INFO_NUM for as both being
> 2bytes long, as does your struct cper_sec_proc_armv8 below. Are these for
> something else? Do these correspond with one of the four bitfield formats
> described in Table 262->265?
>
> I can't see where they are used, and they look like they are reaching across
> multiple fields in a struct.
I will remove these as they aren't needed.
>> +#define CPER_ARMV8_CTX_INFO_NUM_SHIFT 8
>> +
>> +#define CPER_ARMV8_VALID_MPIDR 0x00000001
>> +#define CPER_ARMV8_VALID_AFFINITY_LEVEL 0x00000002
>> +#define CPER_ARMV8_VALID_RUNNING_STATE 0x00000004
>> +#define CPER_ARMV8_VALID_VENDOR_INFO 0x00000008
>> +
>> +#define CPER_ARMV8_INFO_VALID_MULTI_ERR 0x0001
>> +#define CPER_ARMV8_INFO_VALID_FLAGS 0x0002
>> +#define CPER_ARMV8_INFO_VALID_ERR_INFO 0x0004
>> +#define CPER_ARMV8_INFO_VALID_VIRT_ADDR 0x0008
>> +#define CPER_ARMV8_INFO_VALID_PHYSICAL_ADDR 0x0010
>> +
>> +#define CPER_ARMV8_INFO_FLAGS_FIRST 0x0001
>> +#define CPER_ARMV8_INFO_FLAGS_LAST 0x0002
>> +#define CPER_ARMV8_INFO_FLAGS_PROPAGATED 0x0004
>> +
>> +#define CPER_AARCH64_CTX_LEN 368
>> +#define CPER_AARCH32_CTX_LEN 256
> Are these the worst case sizes for combinations of the structures in N2.4.4.2?
> (Tables 266 to 273)
>
> If so is there any chance they could be sizeof(<some union of structs>), even if
> the structs are things like:
>> /* ARMv8 AArch64 GPRs (Type 4) - defined in UEFI Spec N2.4.4.2 */
>> struct cper_armv8_aarch64_gprs {
>> u64 regs[32];
>> }
> This way its easier to check the number is correct, and if a new type is added
> this won't get forgotten.
These were representing the sizes of table 266 and table 267, but
looking at this more it seems
like some of the spec doesn't make sense:
Table 260 has the Processor Context field which only mentions tables 266
and 267.
I think that should really be tables 266 - 274 representing all 9
context types.
Table 265 then has the Register Array field which mentions the contents
of the array
are described in tables 267 - 271. I think this also should be tables
266 - 274 to cover
all 9 context types.
And then the text before table 274 is clearly wrong calling it table
275...seems like there
are several mistakes in the table numbering mentioned in this section.
I'm going to need to update the context info parsing code and add the
other register array
sizes based on all of the context tables. Looks like the code will need
to be restructured
some because otherwise there will be quite a bit of duplication.
>> +#define CPER_ARMV8_CTX_TYPE_MASK 0x000000000000000F
>> +#define CPER_ARMV8_CTX_EL_MASK 0x0000000000000070
>> +#define CPER_ARMV8_CTX_NS_MASK 0x0000000000000080
>> +#define CPER_ARMV8_CTX_EL_SHIFT 4
>> +#define CPER_ARMV8_CTX_NS_SHIFT 7
>> +
> Again, I can't work out what these correspond to. I can't see a secure bit or EL
> field in any of those UEFI tables.
>
> Is this one of the 'ARM Vendor Specific Micro-Architecture Error Structure's? If
> so we should have some infrastructure for picking the correct (or unknown)
> decode function based on a range of MIDRs.
These will be removed. The exception level and secure context
information will be covered by
which register context type is being reported.
0 ? AArch32 GPRs (General Purpose Registers).
1 -- AArch32 EL1 context registers
2 -- AArch32 EL2 context registers
3 -- Aarch32 secure context registers
4 ? AArch64 GPRs
5 -- AArch64 EL1 context registers
6 ? Aarch64 EL2 context registers
7 -- AArch64 EL3 context registers
8 ? Misc. System Register Structure
>> #define acpi_hest_generic_data_error_length(gdata) \
>> (((struct acpi_hest_generic_data *)(gdata))->error_data_length)
>> #define acpi_hest_generic_data_size(gdata) \
>> @@ -352,6 +389,41 @@ struct cper_ia_proc_ctx {
>> __u64 mm_reg_addr;
>> };
>>
>> +/* ARMv8 Processor Error Section */
>> +struct cper_sec_proc_armv8 {
>> + __u32 validation_bits;
>> + __u16 err_info_num; /* Number of Processor Error Info */
>> + __u16 context_info_num; /* Number of Processor Context Info Records*/
>> + __u32 section_length;
>> + __u8 affinity_level;
>> + __u8 reserved[3]; /* must be zero */
>> + __u64 mpidr;
>> + __u64 midr;
>> + __u32 running_state; /* Bit 0 set - Processor running. PSCI = 0 */
>> + __u32 psci_state;
>> +};
>> +
>> +/* ARMv8 Processor Error Information Structure */
>> +struct cper_armv8_err_info {
>> + __u8 version;
>> + __u8 length;
>> + __u16 validation_bits;
>> + __u8 type;
>> + __u16 multiple_error;
>> + __u8 flags;
>> + __u64 error_info;
>> + __u64 virt_fault_addr;
>> + __u64 physical_fault_addr;
>> +};
>
>> +/* ARMv8 AARCH64 Processor Context Information Structure */
>> +struct cper_armv8_aarch64_ctx {
>> + __u8 type_el_ns;
>> + __u8 reserved[7]; /* must be zero */
>> + __u8 gpr[288];
>> + __u8 spr[68];
>> +};
> Is this:
> "Table 265. ARM Processor Error Context Information Header Structure"?
This structure should be removed, it doesn't get used in code now.
Thanks,
Tyler
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic
From: Joerg Roedel @ 2016-11-29 16:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161116095615.GA25656@red-moon>
On Wed, Nov 16, 2016 at 09:56:15AM +0000, Lorenzo Pieralisi wrote:
> I can easily make the changes Robin suggests above, I need to know
> what to do with this patch it is the last blocking point for this
> series and time is running out I can revert to using dev->bus to
> retrieve iommu_ops (even though I do not think it makes sense given
> what Robin outlines below) but I need to know please, we can't gate
> an entire series for this patch that is just syntactic sugar.
Well, I didn't really object to the approach per-se, I just wanted to
know the rationale behind the need for the iommu-ops pointer. So through
which tree should this series be merged?
I think I can live with the pointer for now, we can later convert it to
an iommu-instance pointer.
Joerg
^ permalink raw reply
* [RFC v3 04/10] iommu: iommu_alloc_resv_region
From: Joerg Roedel @ 2016-11-29 16:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479215363-2898-5-git-send-email-eric.auger@redhat.com>
On Tue, Nov 15, 2016 at 01:09:17PM +0000, Eric Auger wrote:
> +static inline struct iommu_resv_region *
> +iommu_alloc_resv_region(phys_addr_t start, size_t length, unsigned int prot)
> +{
> + return NULL;
> +}
> +
Will this function be called outside of iommu code?
Joerg
^ permalink raw reply
* [PATCH v4 0/9] Implement clocksource for rockchip SoC using rockchip timer
From: Alexander Kochetkov @ 2016-11-29 16:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480343486-25539-1-git-send-email-al.kochet@gmail.com>
Hello,
This patch series contain:
- devicetree bindings clarification for rockchip timers
- dts files fixes for rk3228-evb, rk3229-evb and rk3188
- implementation of clocksource and sched clock for rockchip SoC
The clock supplying the arm-global-timer on the rk3188 is coming from the
the cpu clock itself and thus changes its rate everytime cpufreq adjusts
the cpu frequency making this timer unsuitable as a stable clocksource.
The rk3188, rk3288 and following socs share a separate timer block already
handled by the rockchip-timer driver. Therefore adapt this driver to also
be able to act as clocksource on rk3188.
In order to test clocksource you can run following commands and check
how much time it take in real. On rk3188 it take about ~45 seconds.
cpufreq-set -f 1.6GHZ
date; sleep 60; date
rk3288 (and probably anything newer) is irrelevant to this patch,
as it has the arch timer interface. This patch may be usefull
for Cortex-A9/A5 based parts.
Regards,
Alexander.
This is try 4. Please discard all v1, v2, v3 patches.
Changes in v4:
merged 7 and 8 from series 3
merged 10, 11, 12, 13 from series 3
fixed commit message
Changes in v3:
added patches:
ARM: dts: rockchip: disable arm-global-timer for rk3188
clocksource/drivers/rockchip_timer: Prevent ftrace recursion
devicetree v1 patches:
https://patchwork.ozlabs.org/patch/699019/
https://patchwork.ozlabs.org/patch/699020/
kernel v1 patches:
https://patchwork.kernel.org/patch/9443975/
https://patchwork.kernel.org/patch/9443971/
https://patchwork.kernel.org/patch/9443959/
https://patchwork.kernel.org/patch/9443963/
https://patchwork.kernel.org/patch/9443979/
https://patchwork.kernel.org/patch/9443989/
https://patchwork.kernel.org/patch/9443987/
https://patchwork.kernel.org/patch/9443977/
https://patchwork.kernel.org/patch/9443991/
Alexander Kochetkov (9):
dt-bindings: clarify compatible property for rockchip timers
ARM: dts: rockchip: update compatible property for rk3228 timer
ARM: dts: rockchip: update compatible property for rk3229 timer
ARM: dts: rockchip: add timer entries to rk3188 SoC
ARM: dts: rockchip: disable arm-global-timer for rk3188
clocksource/drivers/rockchip_timer: split bc_timer into rk_timer and
rk_clock_event_device
clocksource/drivers/rockchip_timer: low level routines take rk_timer
as parameter
clocksource/drivers/rockchip_timer: move TIMER_INT_UNMASK out of
rk_timer_enable()
clocksource/drivers/rockchip_timer: implement clocksource timer
.../bindings/timer/rockchip,rk-timer.txt | 12 +-
arch/arm/boot/dts/rk3188.dtsi | 17 ++
arch/arm/boot/dts/rk3228-evb.dts | 4 +
arch/arm/boot/dts/rk3229-evb.dts | 4 +
drivers/clocksource/rockchip_timer.c | 207 +++++++++++++++-----
5 files changed, 190 insertions(+), 54 deletions(-)
--
1.7.9.5
^ permalink raw reply
* [PATCH v4 1/9] dt-bindings: clarify compatible property for rockchip timers
From: Alexander Kochetkov @ 2016-11-29 16:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480436092-10728-1-git-send-email-al.kochet@gmail.com>
Make all properties description in form '"rockchip,<chip>-timer",
"rockchip,rk3288-timer"' for all chips found in linux kernel.
Suggested-by: Heiko St?bner <heiko@sntech.de>
Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
.../bindings/timer/rockchip,rk-timer.txt | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
index a41b184..16a5f45 100644
--- a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
+++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
@@ -1,9 +1,15 @@
Rockchip rk timer
Required properties:
-- compatible: shall be one of:
- "rockchip,rk3288-timer" - for rk3066, rk3036, rk3188, rk322x, rk3288, rk3368
- "rockchip,rk3399-timer" - for rk3399
+- compatible: should be:
+ "rockchip,rk3036-timer", "rockchip,rk3288-timer": for Rockchip RK3036
+ "rockchip,rk3066-timer", "rockchip,rk3288-timer": for Rockchip RK3066
+ "rockchip,rk3188-timer", "rockchip,rk3288-timer": for Rockchip RK3188
+ "rockchip,rk3228-timer", "rockchip,rk3288-timer": for Rockchip RK3228
+ "rockchip,rk3229-timer", "rockchip,rk3288-timer": for Rockchip RK3229
+ "rockchip,rk3288-timer": for Rockchip RK3288
+ "rockchip,rk3368-timer", "rockchip,rk3288-timer": for Rockchip RK3368
+ "rockchip,rk3399-timer": for Rockchip RK3399
- reg: base address of the timer register starting with TIMERS CONTROL register
- interrupts: should contain the interrupts for Timer0
- clocks : must contain an entry for each entry in clock-names
--
1.7.9.5
^ permalink raw reply related
* [PATCH v4 2/9] ARM: dts: rockchip: update compatible property for rk3228 timer
From: Alexander Kochetkov @ 2016-11-29 16:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480436092-10728-1-git-send-email-al.kochet@gmail.com>
Property set to '"rockchip,rk3228-timer", "rockchip,rk3288-timer"'
to match devicetree bindings.
Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
arch/arm/boot/dts/rk3228-evb.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/rk3228-evb.dts b/arch/arm/boot/dts/rk3228-evb.dts
index 904668e..38eab87 100644
--- a/arch/arm/boot/dts/rk3228-evb.dts
+++ b/arch/arm/boot/dts/rk3228-evb.dts
@@ -70,3 +70,7 @@
&uart2 {
status = "okay";
};
+
+&timer {
+ compatible = "rockchip,rk3228-timer", "rockchip,rk3288-timer";
+};
--
1.7.9.5
^ permalink raw reply related
* [PATCH v4 3/9] ARM: dts: rockchip: update compatible property for rk3229 timer
From: Alexander Kochetkov @ 2016-11-29 16:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480436092-10728-1-git-send-email-al.kochet@gmail.com>
Property set to '"rockchip,rk3229-timer", "rockchip,rk3288-timer"'
to match devicetree bindings.
Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
arch/arm/boot/dts/rk3229-evb.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/rk3229-evb.dts b/arch/arm/boot/dts/rk3229-evb.dts
index b6a1203..6629769 100644
--- a/arch/arm/boot/dts/rk3229-evb.dts
+++ b/arch/arm/boot/dts/rk3229-evb.dts
@@ -88,3 +88,7 @@
&uart2 {
status = "okay";
};
+
+&timer {
+ compatible = "rockchip,rk3229-timer", "rockchip,rk3288-timer";
+};
--
1.7.9.5
^ permalink raw reply related
* [PATCH v4 4/9] ARM: dts: rockchip: add timer entries to rk3188 SoC
From: Alexander Kochetkov @ 2016-11-29 16:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480436092-10728-1-git-send-email-al.kochet@gmail.com>
The patch add two timers to all rk3188 based boards.
The first timer is from alive subsystem and it act as a backup
for the local timers at sleep time. It act the same as other
SoC rockchip timers already present in kernel.
The second timer is from CPU subsystem and act as replacement
for the arm-global-timer clocksource and sched clock. It run
at stable frequency 24MHz.
Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
arch/arm/boot/dts/rk3188.dtsi | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index 31f81b2..0dc52fe 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -106,6 +106,22 @@
};
};
+ timer3: timer at 2000e000 {
+ compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer";
+ reg = <0x2000e000 0x20>;
+ interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cru SCLK_TIMER3>, <&cru PCLK_TIMER3>;
+ clock-names = "timer", "pclk";
+ };
+
+ timer6: timer at 200380a0 {
+ compatible = "rockchip,rk3188-timer", "rockchip,rk3288-timer";
+ reg = <0x200380a0 0x20>;
+ interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cru SCLK_TIMER6>, <&cru PCLK_TIMER0>;
+ clock-names = "timer", "pclk";
+ };
+
i2s0: i2s at 1011a000 {
compatible = "rockchip,rk3188-i2s", "rockchip,rk3066-i2s";
reg = <0x1011a000 0x2000>;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v4 5/9] ARM: dts: rockchip: disable arm-global-timer for rk3188
From: Alexander Kochetkov @ 2016-11-29 16:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480436092-10728-1-git-send-email-al.kochet@gmail.com>
arm-global-timer can provide clockevents, clocksource and shed_clock. But
on rk3188 platform it provide only clocksource and shed_clock. clockevents
from arm-global-timer is not used by kernel because there is another
clockevent provider with higher rating (smp-twd).
My commit from the series implement clocksource and shed_clock using
rockchip_timer. But sched clock from rk_timer is not selected by kernel
due to lower frequency than arm-global-timer, and clocksource from
rk_timer is not selected by kernel due to lower rating than
arm-global-timer. And I don't want to increase clocksource rating
because ratings greater 300 used for high frequency clocksources.
clocksource and shed_clock is quite unstable, because their rate depends
on cpu frequency. So disable arm-global-timer and use clocksource and
sched_clock from rockchip_timer.
Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
arch/arm/boot/dts/rk3188.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index 0dc52fe..44da3d42 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -546,6 +546,7 @@
&global_timer {
interrupts = <GIC_PPI 11 0xf04>;
+ status = "disabled";
};
&local_timer {
--
1.7.9.5
^ permalink raw reply related
* [PATCH v4 6/9] clocksource/drivers/rockchip_timer: split bc_timer into rk_timer and rk_clock_event_device
From: Alexander Kochetkov @ 2016-11-29 16:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480436092-10728-1-git-send-email-al.kochet@gmail.com>
The patch move ce field out of struct bc_timer into struct
rk_clock_event_device and rename struct bc_timer to struct rk_timer.
This is refactoring step without functional changes.
Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
drivers/clocksource/rockchip_timer.c | 33 ++++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
index 23e267a..6d68d4c 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -29,18 +29,28 @@
#define TIMER_MODE_USER_DEFINED_COUNT (1 << 1)
#define TIMER_INT_UNMASK (1 << 2)
-struct bc_timer {
- struct clock_event_device ce;
+struct rk_timer {
void __iomem *base;
void __iomem *ctrl;
u32 freq;
};
-static struct bc_timer bc_timer;
+struct rk_clock_event_device {
+ struct clock_event_device ce;
+ struct rk_timer timer;
+};
+
+static struct rk_clock_event_device bc_timer;
+
+static inline struct rk_clock_event_device*
+rk_clock_event_device(struct clock_event_device *ce)
+{
+ return container_of(ce, struct rk_clock_event_device, ce);
+}
-static inline struct bc_timer *rk_timer(struct clock_event_device *ce)
+static inline struct rk_timer *rk_timer(struct clock_event_device *ce)
{
- return container_of(ce, struct bc_timer, ce);
+ return &rk_clock_event_device(ce)->timer;
}
static inline void __iomem *rk_base(struct clock_event_device *ce)
@@ -116,16 +126,17 @@ static irqreturn_t rk_timer_interrupt(int irq, void *dev_id)
static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
{
struct clock_event_device *ce = &bc_timer.ce;
+ struct rk_timer *timer = &bc_timer.timer;
struct clk *timer_clk;
struct clk *pclk;
int ret = -EINVAL, irq;
- bc_timer.base = of_iomap(np, 0);
- if (!bc_timer.base) {
+ timer->base = of_iomap(np, 0);
+ if (!timer->base) {
pr_err("Failed to get base address for '%s'\n", TIMER_NAME);
return -ENXIO;
}
- bc_timer.ctrl = bc_timer.base + ctrl_reg;
+ timer->ctrl = timer->base + ctrl_reg;
pclk = of_clk_get_by_name(np, "pclk");
if (IS_ERR(pclk)) {
@@ -153,7 +164,7 @@ static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
goto out_timer_clk;
}
- bc_timer.freq = clk_get_rate(timer_clk);
+ timer->freq = clk_get_rate(timer_clk);
irq = irq_of_parse_and_map(np, 0);
if (!irq) {
@@ -181,7 +192,7 @@ static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
goto out_irq;
}
- clockevents_config_and_register(ce, bc_timer.freq, 1, UINT_MAX);
+ clockevents_config_and_register(ce, timer->freq, 1, UINT_MAX);
return 0;
@@ -190,7 +201,7 @@ out_irq:
out_timer_clk:
clk_disable_unprepare(pclk);
out_unmap:
- iounmap(bc_timer.base);
+ iounmap(timer->base);
return ret;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v4 7/9] clocksource/drivers/rockchip_timer: low level routines take rk_timer as parameter
From: Alexander Kochetkov @ 2016-11-29 16:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480436092-10728-1-git-send-email-al.kochet@gmail.com>
Pass rk_timer instead of clock_event_device to low lever timer routines.
So that code could be reused by clocksource implementation.
Drop rk_base() and rk_ctrl().
This is refactoring step without functional changes.
Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
drivers/clocksource/rockchip_timer.c | 57 ++++++++++++++++------------------
1 file changed, 27 insertions(+), 30 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
index 6d68d4c..a17dc61 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -53,70 +53,67 @@ static inline struct rk_timer *rk_timer(struct clock_event_device *ce)
return &rk_clock_event_device(ce)->timer;
}
-static inline void __iomem *rk_base(struct clock_event_device *ce)
+static inline void rk_timer_disable(struct rk_timer *timer)
{
- return rk_timer(ce)->base;
+ writel_relaxed(TIMER_DISABLE, timer->ctrl);
}
-static inline void __iomem *rk_ctrl(struct clock_event_device *ce)
-{
- return rk_timer(ce)->ctrl;
-}
-
-static inline void rk_timer_disable(struct clock_event_device *ce)
-{
- writel_relaxed(TIMER_DISABLE, rk_ctrl(ce));
-}
-
-static inline void rk_timer_enable(struct clock_event_device *ce, u32 flags)
+static inline void rk_timer_enable(struct rk_timer *timer, u32 flags)
{
writel_relaxed(TIMER_ENABLE | TIMER_INT_UNMASK | flags,
- rk_ctrl(ce));
+ timer->ctrl);
}
static void rk_timer_update_counter(unsigned long cycles,
- struct clock_event_device *ce)
+ struct rk_timer *timer)
{
- writel_relaxed(cycles, rk_base(ce) + TIMER_LOAD_COUNT0);
- writel_relaxed(0, rk_base(ce) + TIMER_LOAD_COUNT1);
+ writel_relaxed(cycles, timer->base + TIMER_LOAD_COUNT0);
+ writel_relaxed(0, timer->base + TIMER_LOAD_COUNT1);
}
-static void rk_timer_interrupt_clear(struct clock_event_device *ce)
+static void rk_timer_interrupt_clear(struct rk_timer *timer)
{
- writel_relaxed(1, rk_base(ce) + TIMER_INT_STATUS);
+ writel_relaxed(1, timer->base + TIMER_INT_STATUS);
}
static inline int rk_timer_set_next_event(unsigned long cycles,
struct clock_event_device *ce)
{
- rk_timer_disable(ce);
- rk_timer_update_counter(cycles, ce);
- rk_timer_enable(ce, TIMER_MODE_USER_DEFINED_COUNT);
+ struct rk_timer *timer = rk_timer(ce);
+
+ rk_timer_disable(timer);
+ rk_timer_update_counter(cycles, timer);
+ rk_timer_enable(timer, TIMER_MODE_USER_DEFINED_COUNT);
return 0;
}
static int rk_timer_shutdown(struct clock_event_device *ce)
{
- rk_timer_disable(ce);
+ struct rk_timer *timer = rk_timer(ce);
+
+ rk_timer_disable(timer);
return 0;
}
static int rk_timer_set_periodic(struct clock_event_device *ce)
{
- rk_timer_disable(ce);
- rk_timer_update_counter(rk_timer(ce)->freq / HZ - 1, ce);
- rk_timer_enable(ce, TIMER_MODE_FREE_RUNNING);
+ struct rk_timer *timer = rk_timer(ce);
+
+ rk_timer_disable(timer);
+ rk_timer_update_counter(timer->freq / HZ - 1, timer);
+ rk_timer_enable(timer, TIMER_MODE_FREE_RUNNING);
return 0;
}
static irqreturn_t rk_timer_interrupt(int irq, void *dev_id)
{
struct clock_event_device *ce = dev_id;
+ struct rk_timer *timer = rk_timer(ce);
- rk_timer_interrupt_clear(ce);
+ rk_timer_interrupt_clear(timer);
if (clockevent_state_oneshot(ce))
- rk_timer_disable(ce);
+ rk_timer_disable(timer);
ce->event_handler(ce);
@@ -183,8 +180,8 @@ static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
ce->cpumask = cpu_possible_mask;
ce->rating = 250;
- rk_timer_interrupt_clear(ce);
- rk_timer_disable(ce);
+ rk_timer_interrupt_clear(timer);
+ rk_timer_disable(timer);
ret = request_irq(irq, rk_timer_interrupt, IRQF_TIMER, TIMER_NAME, ce);
if (ret) {
--
1.7.9.5
^ permalink raw reply related
* [PATCH v4 8/9] clocksource/drivers/rockchip_timer: move TIMER_INT_UNMASK out of rk_timer_enable()
From: Alexander Kochetkov @ 2016-11-29 16:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480436092-10728-1-git-send-email-al.kochet@gmail.com>
This allow to enable timer without enabling interrupts from it.
As that mode will be used in clocksource implementation.
This is refactoring step without functional changes.
Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
drivers/clocksource/rockchip_timer.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
index a17dc61..61c3bb1 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -60,8 +60,7 @@ static inline void rk_timer_disable(struct rk_timer *timer)
static inline void rk_timer_enable(struct rk_timer *timer, u32 flags)
{
- writel_relaxed(TIMER_ENABLE | TIMER_INT_UNMASK | flags,
- timer->ctrl);
+ writel_relaxed(TIMER_ENABLE | flags, timer->ctrl);
}
static void rk_timer_update_counter(unsigned long cycles,
@@ -83,7 +82,8 @@ static inline int rk_timer_set_next_event(unsigned long cycles,
rk_timer_disable(timer);
rk_timer_update_counter(cycles, timer);
- rk_timer_enable(timer, TIMER_MODE_USER_DEFINED_COUNT);
+ rk_timer_enable(timer, TIMER_MODE_USER_DEFINED_COUNT |
+ TIMER_INT_UNMASK);
return 0;
}
@@ -101,7 +101,7 @@ static int rk_timer_set_periodic(struct clock_event_device *ce)
rk_timer_disable(timer);
rk_timer_update_counter(timer->freq / HZ - 1, timer);
- rk_timer_enable(timer, TIMER_MODE_FREE_RUNNING);
+ rk_timer_enable(timer, TIMER_MODE_FREE_RUNNING | TIMER_INT_UNMASK);
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v4 9/9] clocksource/drivers/rockchip_timer: implement clocksource timer
From: Alexander Kochetkov @ 2016-11-29 16:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480436092-10728-1-git-send-email-al.kochet@gmail.com>
The clock supplying the arm-global-timer on the rk3188 is coming from the
the cpu clock itself and thus changes its rate everytime cpufreq adjusts
the cpu frequency making this timer unsuitable as a stable clocksource
and sched clock.
The rk3188, rk3288 and following socs share a separate timer block already
handled by the rockchip-timer driver. Therefore adapt this driver to also
be able to act as clocksource and sched clock on rk3188.
In order to test clocksource you can run following commands and check
how much time it take in real. On rk3188 it take about ~45 seconds.
cpufreq-set -f 1.6GHZ
date; sleep 60; date
In order to use the patch you need to declare two timers in the dts
file. The first timer will be initialized as clockevent provider
and the second one as clocksource. The clockevent must be from
alive subsystem as it used as backup for the local timers at sleep
time.
The patch does not break compatibility with older device tree files.
The older device tree files contain only one timer. The timer
will be initialized as clockevent, as expected.
rk3288 (and probably anything newer) is irrelevant to this patch,
as it has the arch timer interface. This patch may be usefull
for Cortex-A9/A5 based parts.
Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
drivers/clocksource/rockchip_timer.c | 137 +++++++++++++++++++++++++++++-----
1 file changed, 117 insertions(+), 20 deletions(-)
diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
index 61c3bb1..a127822 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -11,6 +11,7 @@
#include <linux/clockchips.h>
#include <linux/init.h>
#include <linux/interrupt.h>
+#include <linux/sched_clock.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
@@ -19,6 +20,8 @@
#define TIMER_LOAD_COUNT0 0x00
#define TIMER_LOAD_COUNT1 0x04
+#define TIMER_CURRENT_VALUE0 0x08
+#define TIMER_CURRENT_VALUE1 0x0C
#define TIMER_CONTROL_REG3288 0x10
#define TIMER_CONTROL_REG3399 0x1c
#define TIMER_INT_STATUS 0x18
@@ -40,7 +43,19 @@ struct rk_clock_event_device {
struct rk_timer timer;
};
+struct rk_clocksource {
+ struct clocksource cs;
+ struct rk_timer timer;
+};
+
+enum {
+ ROCKCHIP_CLKSRC_CLOCKEVENT = 0,
+ ROCKCHIP_CLKSRC_CLOCKSOURCE = 1,
+};
+
static struct rk_clock_event_device bc_timer;
+static struct rk_clocksource cs_timer;
+static int rk_next_clksrc = ROCKCHIP_CLKSRC_CLOCKEVENT;
static inline struct rk_clock_event_device*
rk_clock_event_device(struct clock_event_device *ce)
@@ -63,11 +78,37 @@ static inline void rk_timer_enable(struct rk_timer *timer, u32 flags)
writel_relaxed(TIMER_ENABLE | flags, timer->ctrl);
}
-static void rk_timer_update_counter(unsigned long cycles,
- struct rk_timer *timer)
+static void rk_timer_update_counter(u64 cycles, struct rk_timer *timer)
+{
+ u32 lower = cycles & 0xFFFFFFFF;
+ u32 upper = (cycles >> 32) & 0xFFFFFFFF;
+
+ writel_relaxed(lower, timer->base + TIMER_LOAD_COUNT0);
+ writel_relaxed(upper, timer->base + TIMER_LOAD_COUNT1);
+}
+
+static u64 notrace _rk_timer_counter_read(struct rk_timer *timer)
{
- writel_relaxed(cycles, timer->base + TIMER_LOAD_COUNT0);
- writel_relaxed(0, timer->base + TIMER_LOAD_COUNT1);
+ u64 counter;
+ u32 lower;
+ u32 upper, old_upper;
+
+ upper = readl_relaxed(timer->base + TIMER_CURRENT_VALUE1);
+ do {
+ old_upper = upper;
+ lower = readl_relaxed(timer->base + TIMER_CURRENT_VALUE0);
+ upper = readl_relaxed(timer->base + TIMER_CURRENT_VALUE1);
+ } while (upper != old_upper);
+
+ counter = upper;
+ counter <<= 32;
+ counter |= lower;
+ return counter;
+}
+
+static u64 rk_timer_counter_read(struct rk_timer *timer)
+{
+ return _rk_timer_counter_read(timer);
}
static void rk_timer_interrupt_clear(struct rk_timer *timer)
@@ -120,13 +161,46 @@ static irqreturn_t rk_timer_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static cycle_t rk_timer_clocksource_read(struct clocksource *cs)
+{
+ struct rk_clocksource *_cs =
+ container_of(cs, struct rk_clocksource, cs);
+
+ return ~rk_timer_counter_read(&_cs->timer);
+}
+
+static u64 notrace rk_timer_sched_clock_read(void)
+{
+ struct rk_clocksource *_cs = &cs_timer;
+
+ return ~_rk_timer_counter_read(&_cs->timer);
+}
+
static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
{
- struct clock_event_device *ce = &bc_timer.ce;
- struct rk_timer *timer = &bc_timer.timer;
+ struct clock_event_device *ce = NULL;
+ struct clocksource *cs = NULL;
+ struct rk_timer *timer = NULL;
struct clk *timer_clk;
struct clk *pclk;
int ret = -EINVAL, irq;
+ int clksrc;
+
+ clksrc = rk_next_clksrc;
+ rk_next_clksrc++;
+
+ switch (clksrc) {
+ case ROCKCHIP_CLKSRC_CLOCKEVENT:
+ ce = &bc_timer.ce;
+ timer = &bc_timer.timer;
+ break;
+ case ROCKCHIP_CLKSRC_CLOCKSOURCE:
+ cs = &cs_timer.cs;
+ timer = &cs_timer.timer;
+ break;
+ default:
+ return -ENODEV;
+ }
timer->base = of_iomap(np, 0);
if (!timer->base) {
@@ -170,26 +244,49 @@ static int __init rk_timer_init(struct device_node *np, u32 ctrl_reg)
goto out_irq;
}
- ce->name = TIMER_NAME;
- ce->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT |
- CLOCK_EVT_FEAT_DYNIRQ;
- ce->set_next_event = rk_timer_set_next_event;
- ce->set_state_shutdown = rk_timer_shutdown;
- ce->set_state_periodic = rk_timer_set_periodic;
- ce->irq = irq;
- ce->cpumask = cpu_possible_mask;
- ce->rating = 250;
+ if (ce) {
+ ce->name = TIMER_NAME;
+ ce->features = CLOCK_EVT_FEAT_PERIODIC |
+ CLOCK_EVT_FEAT_ONESHOT |
+ CLOCK_EVT_FEAT_DYNIRQ;
+ ce->set_next_event = rk_timer_set_next_event;
+ ce->set_state_shutdown = rk_timer_shutdown;
+ ce->set_state_periodic = rk_timer_set_periodic;
+ ce->irq = irq;
+ ce->cpumask = cpu_possible_mask;
+ ce->rating = 250;
+ }
+
+ if (cs) {
+ cs->name = TIMER_NAME;
+ cs->flags = CLOCK_SOURCE_IS_CONTINUOUS;
+ cs->mask = CLOCKSOURCE_MASK(64);
+ cs->read = rk_timer_clocksource_read;
+ cs->rating = 250;
+ }
rk_timer_interrupt_clear(timer);
rk_timer_disable(timer);
- ret = request_irq(irq, rk_timer_interrupt, IRQF_TIMER, TIMER_NAME, ce);
- if (ret) {
- pr_err("Failed to initialize '%s': %d\n", TIMER_NAME, ret);
- goto out_irq;
+ if (ce) {
+ ret = request_irq(irq, rk_timer_interrupt, IRQF_TIMER,
+ TIMER_NAME, ce);
+ if (ret) {
+ pr_err("Failed to initialize '%s': %d\n",
+ TIMER_NAME, ret);
+ goto out_irq;
+ }
+
+ clockevents_config_and_register(ce, timer->freq, 1, UINT_MAX);
}
- clockevents_config_and_register(ce, timer->freq, 1, UINT_MAX);
+ if (cs) {
+ rk_timer_update_counter(U64_MAX, timer);
+ rk_timer_enable(timer, 0);
+ clocksource_register_hz(cs, timer->freq);
+ sched_clock_register(rk_timer_sched_clock_read, 64,
+ timer->freq);
+ }
return 0;
--
1.7.9.5
^ permalink raw reply related
* [PATCH] ARM: davinci: da8xx: Fix sleeping function called from invalid context
From: David Lechner @ 2016-11-29 16:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <aac017b2-fc5b-6773-46b3-fd797dc904a6@ti.com>
On 11/29/2016 06:34 AM, Sekhar Nori wrote:
> On Tuesday 29 November 2016 04:46 PM, Alexandre Bailon wrote:
>> On 11/29/2016 11:48 AM, Sekhar Nori wrote:
>>> On Monday 28 November 2016 09:59 PM, Alexandre Bailon wrote:
>>>> Everytime the usb20 phy is enabled, there is a
>>>> "sleeping function called from invalid context" BUG.
>>>
>>> Who calls PHY clk_enable() from non-preemptible context? Can you provide
>>> the call stack?
>> Actually, clk_enable() is called from preemptible context (from phy
>> driver) but it disables interrupts before to call the clk_enable()
>> callback.
>> I attached the call stack that is probably more understandable than
>> my explanation.
>
> Thanks! So this happens due to spin_lock_irqsave() in clk_enable() in
> arch/arm/mach-davinci/clock.c. Can you add reference to this in your
> commit description.
>
> Also +David since this issue should have shown up since the time this
> code was added.
I'm not trying to pass the blame, but it was actually Axel that added
the clk_get() code, so it will be good if he knows about the issue too. :-)
I don't think I have been building kernels with any of the kernel
hacking options turned on, so I will start doing this to make sure we
catch bugs like this.
>
>>>> pr_err("could not get usb20 clk: %ld\n", PTR_ERR(usb20_clk));
>>>> return;
>>>> }
>>>>
>>>> /* The USB 2.O PLL requires that the USB 2.O PSC is enabled as well. */
>>>> - err = clk_prepare_enable(usb20_clk);
>>>> + err = clk_enable(usb20_clk);
>>>> if (err) {
>>>> pr_err("failed to enable usb20 clk: %d\n", err);
>>>> clk_put(usb20_clk);
>>>> @@ -197,8 +197,7 @@ static void usb20_phy_clk_enable(struct clk *clk)
>>>>
>>>> pr_err("Timeout waiting for USB 2.0 PHY clock good\n");
>>>> done:
>>>> - clk_disable_unprepare(usb20_clk);
>>>> - clk_put(usb20_clk);
>>>> + clk_disable(usb20_clk);
>>>
>>>
>>> I noticed that we are missing clk_disable(usb20_clk) in
>>> usb20_phy_clk_disable(). It will now be easier to do that after this
>>> patch. Can you add that in a separate patch?
>>>
>> I don't think we need it.
>> What we don't see in this patch is that usb20_clk is enabled and,
>> it is disabled right after the PHY PLL is ready in usb20_phy_clk_enable().
>
> Agreed.
>
> Thanks,
> Sekhar
>
^ permalink raw reply
* [PATCH V7 2/3] ACPI: Add support for ResourceSource/IRQ domain mapping
From: Rafael J. Wysocki @ 2016-11-29 16:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c13596d9b4f86f1b603fb979a0083889@codeaurora.org>
On Tue, Nov 29, 2016 at 4:20 PM, Agustin Vega-Frias
<agustinv@codeaurora.org> wrote:
> Hi Rafael,
>
>
> On 2016-11-29 07:43, Rafael J. Wysocki wrote:
>>
>> On Tue, Nov 29, 2016 at 1:11 PM, Lorenzo Pieralisi
>> <lorenzo.pieralisi@arm.com> wrote:
>>>
>>> Hi Agustin,
>>>
>>> On Mon, Nov 28, 2016 at 05:40:24PM -0500, Agustin Vega-Frias wrote:
>>>>
>>>> Hi Rafael,
>>>>
>>>> Can you chime in on Lorenzo's feedback and the discussion below?
>>>> It would be great if you can comment on the reason ACPI does things
>>>> in a certain way.
>>>>
>>>> Hi Lorenzo,
>>>>
>>>> On 2016-11-25 06:40, Lorenzo Pieralisi wrote:
>>>> >Hi Agustin,
>>>> >
>>>> >On Thu, Nov 24, 2016 at 04:15:48PM +0000, Lorenzo Pieralisi wrote:
>>>> >
>>>> >[...]
>>>> >
>>>> >>> @@ -448,6 +449,7 @@ bool acpi_dev_resource_interrupt(struct
>>>> >>> acpi_resource *ares, int index,
>>>> >>> {
>>>> >>> struct acpi_resource_irq *irq;
>>>> >>> struct acpi_resource_extended_irq *ext_irq;
>>>> >>> + struct fwnode_handle *src;
>>>> >>>
>>>> >>> switch (ares->type) {
>>>> >>> case ACPI_RESOURCE_TYPE_IRQ:
>>>> >>> @@ -460,7 +462,7 @@ bool acpi_dev_resource_interrupt(struct
>>>> >>> acpi_resource *ares, int index,
>>>> >>> acpi_dev_irqresource_disabled(res, 0);
>>>> >>> return false;
>>>> >>> }
>>>> >>> - acpi_dev_get_irqresource(res, irq->interrupts[index],
>>>> >>> + acpi_dev_get_irqresource(res, irq->interrupts[index],
>>>> >>> NULL,
>>>> >>> irq->triggering, irq->polarity,
>>>> >>> irq->sharable, true);
>>>> >>> break;
>>>> >>> @@ -470,7 +472,8 @@ bool acpi_dev_resource_interrupt(struct
>>>> >>> acpi_resource *ares, int index,
>>>> >>> acpi_dev_irqresource_disabled(res, 0);
>>>> >>> return false;
>>>> >>> }
>>>> >>> - acpi_dev_get_irqresource(res, ext_irq->interrupts[index],
>>>> >>> + src =
>>>> >>> acpi_get_irq_source_fwhandle(&ext_irq->resource_source);
>>>> >>
>>>> >>Is there a reason why we need to do the domain look-up here ?
>>>>
>>>> Because we need to pass the resource down to acpi_dev_get_irqresource
>>>> which does the mapping through acpi_register_irq/acpi_register_gsi.
>>>>
>>>> >>
>>>> >>I would like to understand if, by reshuffling the code (and by
>>>> >>returning
>>>> >>the resource_source to the calling code - somehow), it would be
>>>> >>possible
>>>> >>to just mirror what the OF code does in of_irq_get(), namely:
>>>> >>
>>>> >>(1) parse the irq entry -> of_irq_parse_one()
>>>> >>(2) look the domain up -> irq_find_host()
>>>> >>(3) create the mapping -> irq_create_of_mapping()
>>>> >>
>>>> >>You wrote the code already, I think it is just a matter of shuffling
>>>> >>it around (well, minus returning the resource_source to the caller
>>>> >>which is phandle equivalent in DT).
>>>>
>>>> This is one area in which DT and ACPI are fundamentally different. In DT
>>>> once the flattened blob is expanded the data is fixed. In ACPI the data
>>>> returned by a method can change. In reality most methods like CRS return
>>>> constants, but given that per-spec they are methods the interpreter has
>>>> to be involved, which makes it an expensive operation. I believe that is
>>>> the reason the resource parsing code in ACPI attempts all mappings
>>>> during
>>>> the bus scan. Rafael can you comment on this?
>>>>
>>>> One way to do what you suggest would be to defer IRQ mapping by, e.g.,
>>>> populating res->start with the HW IRQ number and res->end with the
>>>> fwnode.
>>>> That way we can avoid having to walk the resource buffer when a mapping
>>>> is needed. I don't think that approach would deviate much more from
>>>> the spec from what the current ahead-of-time mapping does, but it would
>>>> require more changes in the core code. An alternative would be to do
>>>> that only for resources that fail to map.
>>>>
>>>> >>
>>>> >>You abstracted away (2) and (3) behind acpi_register_irq(), that
>>>> >>on anything than does not use ACPI_GENERIC_GSI is just glue code
>>>> >>to acpi_register_gsi().
>>>> >>
>>>> >>Also, it is not a question on this patch but I ask it here because it
>>>> >>is related. On ACPI you are doing the reverse of what is done in
>>>> >>DT in platform_get_irq():
>>>> >>
>>>> >>- get the resources already parsed -> platform_get_resource()
>>>> >>- if they are disabled -> acpi_irq_get()
>>>> >>
>>>> >>and I think the ordering is tied to my question above because
>>>> >>you carry out the domain look up in acpi_dev_resource_interrupt()
>>>> >>so that if for any reason it fails the corresponding resource
>>>> >>is disabled so that we try to get it again through acpi_irq_get().
>>>> >>
>>>> >>I suspect you did it this way to make sure:
>>>> >>
>>>> >>a) keep the current ACPI IRQ parsing interface changes to a mininum
>>>> >>b) avoid changing the behaviour on x86/ia64; in particular, calling
>>>> >> acpi_register_gsi() for the _same_ mapping (an IRQ that was already
>>>> >> registered at device creation resource parsing) multiple times can
>>>> >> trigger issues on x86/ia64
>>>>
>>>> You are correct about my reasons. I wanted to keep ACPI core code
>>>> changes
>>>> to a minimum, and I also needed to work within the current
>>>> implementation
>>>> which uses the pre-converted IRQ resources.
>>>>
>>>> >>
>>>> >>I think that's a reasonable approach but I wanted to get these
>>>> >>clarifications, I do not think you are far from getting this
>>>> >>done but since it is a significant change I think it is worth
>>>> >>discussing the points I raised above because I think the DT code
>>>> >>sequence in of_irq_get() (1-2-3 above) is cleaner from an IRQ
>>>> >>layer perspective (instead of having the domain look-up buried
>>>> >>inside the ACPI IRQ resource parsing API).
>>>> >
>>>> >I had another look and to achieve the above one way of doing that is to
>>>> >implement acpi_irq_get() only for ACPI_GENERIC_GSI and stub it out for
>>>> >!ACPI_GENERIC_GSI (ie return an error code so that on !ACPI_GENERIC_GSI
>>>> >we would fall back to current solution for ACPI). Within acpi_irq_get()
>>>> >you can easily carry out the same steps (1->2->3) above in ACPI
>>>> >you have
>>>> >the code already there I think it is easy to change the
>>>> >acpi_irq_get_cb() interface to return a filled in struct irq_fwspec and
>>>> >the interface would become identical to of_irq_get() that is an
>>>> >advantage to maintain it from an IRQ maintainership perspective I
>>>> >think,
>>>> >that's my opinion.
>>>>
>>>> I think I get what you mean. I'll take a stab at implementing
>>>> acpi_irq_get()
>>>> in the way you suggest.
>>>>
>>>> >
>>>> >There is still a nagging snag though. When platform devices are
>>>> >created, core ACPI code parse the resources through:
>>>> >
>>>> >acpi_dev_get_resources()
>>>> >
>>>> >and we _have_ to have way to avoid initializing IRQ resources that
>>>> >have a dependency (ie there is a resource_source pointer that is valid
>>>> >in their descriptors) that's easy to do if we think that's the right
>>>> >thing to do and can hardly break current code (which ignores the
>>>> >resource_source altogether).
>>>>
>>>> I'd rather keep the core code as-is with regard to the ahead-of-time
>>>> conversion. Whether a resource source is available at the time of
>>>> the bus
>>>> scan should be transparent to the code in drivers/acpi/resource.c, and
>>>> we need the initialization as a disabled resource to signal the need
>>>> to retry anyway.
>>>
>>>
>>> Yes, exactly that's the nub. Your current code works, I am trying to
>>> make it more modular and similar to the DT/irqdomain IRQ look-up path,
>>> which has its advantages.
>>>
>>> There are two options IMHO:
>>>
>>> - always disable the resource if it has a resource_source dependency and
>>> defer
>>> its parsing to acpi_irq_get() (where you can easily implement steps
>>> 1-2-3 above).
>>> What I wanted to say is that, by disabling the resource if it has a
>>> resource_source dependency you can't break x86/ia64 (it is ignored at
>>> present - hopefully there is nothing that we are not aware of behind
>>> that choice). On x86/ia64 acpi_irq_get() would be an empty stub.
>>> This way you would keep the irqdomain look-up out of the ACPI resource
>>> parsing API, correct ?
>>> - keep code as-is
>>>
>>> Your point on _CRS being _current_ resource setting is perfectly valid
>>> so platform_get_resource() in platform_get_irq() must always take
>>> precedence over acpi_irq_get() (which should just apply to disabled
>>> resources), I am not sure that doing it the other way around is safe.
>>>
>>>> Rafael, do you have any other suggestions/feedback on how to go about
>>>> doing this?
>>>
>>>
>>> Yes, comments very appreciated, these changes are not trivial and need
>>> agreement.
>>
>>
>> So I need more time.
>
>
> Please wait for V8 which will address some issues raised by Lorenzo.
>
>>
>> But basically, _CRS can't really change on the fly AFAICS. I'm not
>> even sure it is valid for it to change at all after the first
>> evaluation if _SRS/_PRS are not present.
>
>
> That's good to know and it opens more possibilities.
Actually, to me it follows from the very purpose of _CRS that, as long
as the device is enabled, it should be expected to return the same
data every time it is evaluated, unless _SRS is invoked in the
meantime. Otherwise, it would be possible for the device's resources
to change unexpectedly under a driver using it.
Thanks,
Rafael
^ permalink raw reply
* [PATCH] ARM: davinci: da8xx: Fix sleeping function called from invalid context
From: Axel Haslam @ 2016-11-29 16:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1a24c04b-8c7e-9e6c-eada-0c5a0a6c8881@lechnology.com>
On Tue, Nov 29, 2016 at 5:22 PM, David Lechner <david@lechnology.com> wrote:
> On 11/29/2016 06:34 AM, Sekhar Nori wrote:
>>
>> On Tuesday 29 November 2016 04:46 PM, Alexandre Bailon wrote:
>>>
>>> On 11/29/2016 11:48 AM, Sekhar Nori wrote:
>>>>
>>>> On Monday 28 November 2016 09:59 PM, Alexandre Bailon wrote:
>>>>>
>>>>> Everytime the usb20 phy is enabled, there is a
>>>>> "sleeping function called from invalid context" BUG.
>>>>
>>>>
>>>> Who calls PHY clk_enable() from non-preemptible context? Can you provide
>>>> the call stack?
>>>
>>> Actually, clk_enable() is called from preemptible context (from phy
>>> driver) but it disables interrupts before to call the clk_enable()
>>> callback.
>>> I attached the call stack that is probably more understandable than
>>> my explanation.
>>
>>
>> Thanks! So this happens due to spin_lock_irqsave() in clk_enable() in
>> arch/arm/mach-davinci/clock.c. Can you add reference to this in your
>> commit description.
>>
>> Also +David since this issue should have shown up since the time this
>> code was added.
>
>
> I'm not trying to pass the blame, but it was actually Axel that added the
> clk_get() code, so it will be good if he knows about the issue too. :-)
>
> I don't think I have been building kernels with any of the kernel hacking
> options turned on, so I will start doing this to make sure we catch bugs
> like this.
>
Yes, sorry i missed this. i did not have that option enabled, and did
not catch this, thanks Alex!
>>
>>>>> pr_err("could not get usb20 clk: %ld\n",
>>>>> PTR_ERR(usb20_clk));
>>>>> return;
>>>>> }
>>>>>
>>>>> /* The USB 2.O PLL requires that the USB 2.O PSC is enabled as
>>>>> well. */
>>>>> - err = clk_prepare_enable(usb20_clk);
>>>>> + err = clk_enable(usb20_clk);
>>>>> if (err) {
>>>>> pr_err("failed to enable usb20 clk: %d\n", err);
>>>>> clk_put(usb20_clk);
>>>>> @@ -197,8 +197,7 @@ static void usb20_phy_clk_enable(struct clk *clk)
>>>>>
>>>>> pr_err("Timeout waiting for USB 2.0 PHY clock good\n");
>>>>> done:
>>>>> - clk_disable_unprepare(usb20_clk);
>>>>> - clk_put(usb20_clk);
>>>>> + clk_disable(usb20_clk);
>>>>
>>>>
>>>>
>>>> I noticed that we are missing clk_disable(usb20_clk) in
>>>> usb20_phy_clk_disable(). It will now be easier to do that after this
>>>> patch. Can you add that in a separate patch?
>>>>
>>> I don't think we need it.
>>> What we don't see in this patch is that usb20_clk is enabled and,
>>> it is disabled right after the PHY PLL is ready in
>>> usb20_phy_clk_enable().
>>
>>
>> Agreed.
>>
>> Thanks,
>> Sekhar
>>
>
^ permalink raw reply
* [PATCH v9 07/11] arm/arm64: vgic: Implement KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO ioctl
From: Vijay Kilari @ 2016-11-29 16:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161128195028.GH18170@cbox>
On Tue, Nov 29, 2016 at 1:20 AM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Wed, Nov 23, 2016 at 06:31:54PM +0530, vijay.kilari at gmail.com wrote:
>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>
>> Userspace requires to store and restore of line_level for
>> level triggered interrupts using ioctl KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO.
>>
>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>> ---
>> arch/arm/include/uapi/asm/kvm.h | 7 ++++++
>> arch/arm64/include/uapi/asm/kvm.h | 6 +++++
>> virt/kvm/arm/vgic/vgic-kvm-device.c | 49 ++++++++++++++++++++++++++++++++++++-
>> virt/kvm/arm/vgic/vgic-mmio-v3.c | 11 +++++++++
>> virt/kvm/arm/vgic/vgic-mmio.c | 38 ++++++++++++++++++++++++++++
>> virt/kvm/arm/vgic/vgic-mmio.h | 5 ++++
>> virt/kvm/arm/vgic/vgic.h | 2 ++
>> 7 files changed, 117 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
>> index 98658d9d..f347779 100644
>> --- a/arch/arm/include/uapi/asm/kvm.h
>> +++ b/arch/arm/include/uapi/asm/kvm.h
>> @@ -191,6 +191,13 @@ struct kvm_arch_memory_slot {
>> #define KVM_DEV_ARM_VGIC_GRP_CTRL 4
>> #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
>> #define KVM_DEV_ARM_VGIC_CPU_SYSREGS 6
>> +#define KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO 7
>> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT 10
>> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK \
>> + (0x3fffffULL << KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT)
>> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK 0x3ff
>> +#define VGIC_LEVEL_INFO_LINE_LEVEL 0
>> +
>> #define KVM_DEV_ARM_VGIC_CTRL_INIT 0
>>
>> /* KVM_IRQ_LINE irq field index values */
>> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>> index 91c7137..4100f8c 100644
>> --- a/arch/arm64/include/uapi/asm/kvm.h
>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>> @@ -211,6 +211,12 @@ struct kvm_arch_memory_slot {
>> #define KVM_DEV_ARM_VGIC_GRP_CTRL 4
>> #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
>> #define KVM_DEV_ARM_VGIC_CPU_SYSREGS 6
>> +#define KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO 7
>> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT 10
>> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK \
>> + (0x3fffffULL << KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT)
>> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK 0x3ff
>> +#define VGIC_LEVEL_INFO_LINE_LEVEL 0
>>
>> #define KVM_DEV_ARM_VGIC_CTRL_INIT 0
>>
>> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
>> index b6266fe..52ed00b 100644
>> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
>> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
>> @@ -510,6 +510,25 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
>> regid, reg);
>> break;
>> }
>> + case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
>> + unsigned int info, intid;
>> +
>> + info = (attr->attr & KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK) >>
>> + KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT;
>> + if (info == VGIC_LEVEL_INFO_LINE_LEVEL) {
>> + if (is_write)
>> + tmp32 = *reg;
>> + intid = attr->attr &
>> + KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK;
>> + ret = vgic_v3_line_level_info_uaccess(vcpu, is_write,
>> + intid, &tmp32);
>> + if (!is_write)
>> + *reg = tmp32;
>
> I had a comment here about not having to use the tmp32 by modifying the
> line_level_info function, that you seem to have missed.
>
> Hint: The level info is not called from an MMIO path so you should be
> able to just write it in a natural way.
Ok. Changed the prototype of vgic_v3_line_level_info_uaccess() to take
u64 reg instead of tmp32
>
>> + } else {
>> + ret = -EINVAL;
>> + }
>> + break;
>> + }
>> default:
>> ret = -EINVAL;
>> break;
>> @@ -552,6 +571,17 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
>>
>> return vgic_v3_attr_regs_access(dev, attr, ®, true);
>> }
>> + case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
>> + u32 __user *uaddr = (u32 __user *)(long)attr->addr;
>> + u64 reg;
>> + u32 tmp32;
>> +
>> + if (get_user(tmp32, uaddr))
>> + return -EFAULT;
>> +
>> + reg = tmp32;
>> + return vgic_v3_attr_regs_access(dev, attr, ®, true);
>> + }
>> }
>> return -ENXIO;
>> }
>> @@ -587,8 +617,18 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
>> return ret;
>> return put_user(reg, uaddr);
>> }
>> - }
>> + case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
>> + u32 __user *uaddr = (u32 __user *)(long)attr->addr;
>> + u64 reg;
>> + u32 tmp32;
>>
>> + ret = vgic_v3_attr_regs_access(dev, attr, ®, false);
>> + if (ret)
>> + return ret;
>> + tmp32 = reg;
>> + return put_user(tmp32, uaddr);
>> + }
>> + }
>> return -ENXIO;
>> }
>>
>> @@ -609,6 +649,13 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
>> return vgic_v3_has_attr_regs(dev, attr);
>> case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
>> return 0;
>> + case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
>> + if (((attr->attr & KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK) >>
>> + KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT) ==
>> + VGIC_LEVEL_INFO_LINE_LEVEL)
>> + return 0;
>> + break;
>> + }
>> case KVM_DEV_ARM_VGIC_GRP_CTRL:
>> switch (attr->attr) {
>> case KVM_DEV_ARM_VGIC_CTRL_INIT:
>> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
>> index 2f7b4ed..4d7d93d 100644
>> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
>> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
>> @@ -808,3 +808,14 @@ int vgic_v3_redist_uaccess(struct kvm_vcpu *vcpu, bool is_write,
>> return vgic_uaccess(vcpu, &rd_dev, is_write,
>> offset, val);
>> }
>> +
>> +int vgic_v3_line_level_info_uaccess(struct kvm_vcpu *vcpu, bool is_write,
>> + u32 intid, u32 *val)
>> +{
>> + if (is_write)
>> + vgic_write_irq_line_level_info(vcpu, intid, *val);
>> + else
>> + *val = vgic_read_irq_line_level_info(vcpu, intid);
>> +
>> + return 0;
>> +}
>> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
>> index f81e0e5..d602081 100644
>> --- a/virt/kvm/arm/vgic/vgic-mmio.c
>> +++ b/virt/kvm/arm/vgic/vgic-mmio.c
>> @@ -371,6 +371,44 @@ void vgic_mmio_write_config(struct kvm_vcpu *vcpu,
>> }
>> }
>>
>> +unsigned long vgic_read_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid)
>> +{
>> + int i;
>> + unsigned long val = 0;
>> +
>> + for (i = 0; i < 32; i++) {
>> + struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
>> +
>> + if (irq->line_level)
>> + val |= (1U << i);
>> +
>> + vgic_put_irq(vcpu->kvm, irq);
>> + }
>> +
>> + return val;
>> +}
>> +
>> +void vgic_write_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid,
>> + const unsigned long val)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < 32; i++) {
>> + struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
>> +
>> + spin_lock(&irq->irq_lock);
>> + if (val & (1U << i)) {
>> + irq->line_level = true;
>> + vgic_queue_irq_unlock(vcpu->kvm, irq);
>> + } else {
>> + irq->line_level = false;
>> + spin_unlock(&irq->irq_lock);
>> + }
>
> I think you also missed my comment about having to keep the pending
> state in sync with the level state.
>
> Which means you have to set the pending state when the line_level goes
> up, and lower it when it goes down unless soft_pending is also set,
> assuming it's configured as a level triggered interrupt.
>
> If it's an edge-triggered interrupt, I think you only need to set the
> pending state on a line being asserted and the rest should be adjusted
> in case the user restores the configuration state to level triggered
> later.
Is this ok?
void vgic_write_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid,
const u64 val)
{
int i;
for (i = 0; i < 32; i++) {
struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
spin_lock(&irq->irq_lock);
if (val & (1U << i)) {
irq->line_level = true;
irq->pending = true;
vgic_queue_irq_unlock(vcpu->kvm, irq);
} else {
if (irq->config == VGIC_CONFIG_EDGE ||
(irq->config == VGIC_CONFIG_LEVEL &&
!irq->soft_pending))
irq->line_level = false;
spin_unlock(&irq->irq_lock);
}
vgic_put_irq(vcpu->kvm, irq);
}
}
>
> Thanks,
> -Christoffer
^ permalink raw reply
* [PATCH 2/2] ARM: omap: timers: reduce rating of gp_timer clocksource
From: Grygorii Strashko @ 2016-11-29 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479968355-18860-2-git-send-email-j-keerthy@ti.com>
On 11/24/2016 12:19 AM, Keerthy wrote:
> From: Grygorii Strashko <grygorii.strashko@ti.com>
>
> Now ARM Global timer (rating 300) will not be selected as clocksource,
> because it's initialized after OMAP GP Timer (rating 300) and
> Timekeeping core will not allow to replace clocksource with new one if
> both of them have the same rating.
>
> Reduce rating of OMAP GP Timer (300->290) when it's used as
> clocksource device - this will allow to select ARM Global timer (300)
> as clocksource when enabled.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
Unfortunately, this patch has dependency [1] and can't be used alone as
it will cause ARM Global timer to be selected as clocksource
always on am437x and this will kill cpuidle, because ARM Global timer
is not in always_on domain.
The intention of enabling ARM Global timer is only for non-pm aware use
cases for RT-kernel latency improvement - where deep cpuidle states are
not enabled.
[1] https://patchwork.kernel.org/patch/8940051/
> ---
> arch/arm/mach-omap2/timer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index b2f2448..a0dbb0b 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -376,7 +376,7 @@ static cycle_t clocksource_read_cycles(struct clocksource *cs)
> }
>
> static struct clocksource clocksource_gpt = {
> - .rating = 300,
> + .rating = 290,
> .read = clocksource_read_cycles,
> .mask = CLOCKSOURCE_MASK(32),
> .flags = CLOCK_SOURCE_IS_CONTINUOUS,
>
--
regards,
-grygorii
^ permalink raw reply
* [PATCH 0/9] coresight: next v4.9-rc7
From: Mathieu Poirier @ 2016-11-29 16:47 UTC (permalink / raw)
To: linux-arm-kernel
Hi Greg,
These are the patches I'd like you to consider for inclusion in the
4.10 cycle. They were sent earlier this month [1] but I haven't received
the usual automated acknowledgment, hence sending again. Everything
applies cleanly on your 'char-misc-next' branch (38d1790644ef).
Thanks,
Mathieu
[1]. https://www.spinics.net/lists/kernel/msg2380312.html
Chunyan Zhang (1):
coresight: stm: return error code instead of zero in .packet()
Mathieu Poirier (1):
coresight: reset "enable_sink" flag when need be
Muhammad Abdul WAHAB (2):
coresight: etm3x: indentation fix (extra space removed)
coresight: etm3x: Adding missing features of Coresight PTM components
Quentin Lambert (1):
coresight: perf: Add a missing call to etm_free_aux
Suzuki K Poulose (1):
coresight: Add support for ARM Coresight STM-500
Suzuki K. Poulose (3):
coresight: tmc: Cleanup operation mode handling
coresight: tmc: Get rid of mode parameter for helper routines
coresight: tmc: Remove duplicate memset
drivers/hwtracing/coresight/coresight-etm-perf.c | 31 ++++-----
drivers/hwtracing/coresight/coresight-etm.h | 5 ++
.../hwtracing/coresight/coresight-etm3x-sysfs.c | 12 +++-
drivers/hwtracing/coresight/coresight-priv.h | 4 +-
drivers/hwtracing/coresight/coresight-stm.c | 9 ++-
drivers/hwtracing/coresight/coresight-tmc-etf.c | 48 ++++++--------
drivers/hwtracing/coresight/coresight-tmc-etr.c | 43 +++++--------
drivers/hwtracing/coresight/coresight-tmc.h | 2 +-
drivers/hwtracing/coresight/coresight.c | 74 ++++++++++++++++++++--
9 files changed, 144 insertions(+), 84 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 1/9] coresight: stm: return error code instead of zero in .packet()
From: Mathieu Poirier @ 2016-11-29 16:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480438039-24960-1-git-send-email-mathieu.poirier@linaro.org>
From: Chunyan Zhang <zhang.chunyan@linaro.org>
In STM framework driver, the trace data writing loop would keep running
until it received a negative return value or the whole trace packet has
been written to STM device. So if the .packet() of STM device always
returns zero since the device is not enabled or the parameter isn't
supported, STM framework driver will stall into a dead loop.
Returning -EACCES (Permission denied) in .packet() if the device is
disabled makes more sense, and this is the same for returning -EINVAL
if the channel passed into is not supported.
Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
drivers/hwtracing/coresight/coresight-stm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
index 49e0f1b925a5..d397849c2c6a 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -419,10 +419,10 @@ static ssize_t stm_generic_packet(struct stm_data *stm_data,
struct stm_drvdata, stm);
if (!(drvdata && local_read(&drvdata->mode)))
- return 0;
+ return -EACCES;
if (channel >= drvdata->numsp)
- return 0;
+ return -EINVAL;
ch_addr = (unsigned long)stm_channel_addr(drvdata, channel);
--
2.7.4
^ permalink raw reply related
* [PATCH 2/9] coresight: etm3x: indentation fix (extra space removed)
From: Mathieu Poirier @ 2016-11-29 16:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480438039-24960-1-git-send-email-mathieu.poirier@linaro.org>
From: Muhammad Abdul WAHAB <muhammadabdul.wahab@centralesupelec.fr>
An extra space is removed.
Signed-off-by: Muhammad Abdul Wahab <muhammadabdul.wahab@centralesupelec.fr>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
drivers/hwtracing/coresight/coresight-etm3x-sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
index e9b071953f80..5ea090955c08 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
@@ -146,7 +146,7 @@ static ssize_t mode_store(struct device *dev,
goto err_unlock;
}
config->ctrl |= ETMCR_STALL_MODE;
- } else
+ } else
config->ctrl &= ~ETMCR_STALL_MODE;
if (config->mode & ETM_MODE_TIMESTAMP) {
--
2.7.4
^ permalink raw reply related
* [PATCH 3/9] coresight: etm3x: Adding missing features of Coresight PTM components
From: Mathieu Poirier @ 2016-11-29 16:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480438039-24960-1-git-send-email-mathieu.poirier@linaro.org>
From: Muhammad Abdul WAHAB <muhammadabdul.wahab@centralesupelec.fr>
In the current driver for Coresight components, two features of PTM
components are missing:
1. Branch Broadcasting (present also in ETM but called Branch Output)
2. Return Stack (only present in PTM v1.0 and PTMv1.1)
These features can be added simply to the code using `mode` field of
`etm_config` struct.
1. **Branch Broadcast** : The branch broadcast feature is present in ETM
components as well and is called Branch output. It allows to retrieve
addresses for direct branch addresses alongside the indirect branch
addresses. For example, it could be useful in cases when tracing without
source code.
2. **Return Stack** : The return stack option allows to retrieve the
return addresses of function calls. It can be useful to avoid CRA
(Code Reuse Attacks) by keeping a shadowstack.
Signed-off-by: Muhammad Abdul Wahab <muhammadabdul.wahab@centralesupelec.fr>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
drivers/hwtracing/coresight/coresight-etm.h | 5 +++++
drivers/hwtracing/coresight/coresight-etm3x-sysfs.c | 10 ++++++++++
2 files changed, 15 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h
index 4a18ee499965..ad063d7444e1 100644
--- a/drivers/hwtracing/coresight/coresight-etm.h
+++ b/drivers/hwtracing/coresight/coresight-etm.h
@@ -89,11 +89,13 @@
/* ETMCR - 0x00 */
#define ETMCR_PWD_DWN BIT(0)
#define ETMCR_STALL_MODE BIT(7)
+#define ETMCR_BRANCH_BROADCAST BIT(8)
#define ETMCR_ETM_PRG BIT(10)
#define ETMCR_ETM_EN BIT(11)
#define ETMCR_CYC_ACC BIT(12)
#define ETMCR_CTXID_SIZE (BIT(14)|BIT(15))
#define ETMCR_TIMESTAMP_EN BIT(28)
+#define ETMCR_RETURN_STACK BIT(29)
/* ETMCCR - 0x04 */
#define ETMCCR_FIFOFULL BIT(23)
/* ETMPDCR - 0x310 */
@@ -110,8 +112,11 @@
#define ETM_MODE_STALL BIT(2)
#define ETM_MODE_TIMESTAMP BIT(3)
#define ETM_MODE_CTXID BIT(4)
+#define ETM_MODE_BBROAD BIT(5)
+#define ETM_MODE_RET_STACK BIT(6)
#define ETM_MODE_ALL (ETM_MODE_EXCLUDE | ETM_MODE_CYCACC | \
ETM_MODE_STALL | ETM_MODE_TIMESTAMP | \
+ ETM_MODE_BBROAD | ETM_MODE_RET_STACK | \
ETM_MODE_CTXID | ETM_MODE_EXCL_KERN | \
ETM_MODE_EXCL_USER)
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
index 5ea090955c08..ca98ad13bb8c 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
@@ -164,6 +164,16 @@ static ssize_t mode_store(struct device *dev,
else
config->ctrl &= ~ETMCR_CTXID_SIZE;
+ if (config->mode & ETM_MODE_BBROAD)
+ config->ctrl |= ETMCR_BRANCH_BROADCAST;
+ else
+ config->ctrl &= ~ETMCR_BRANCH_BROADCAST;
+
+ if (config->mode & ETM_MODE_RET_STACK)
+ config->ctrl |= ETMCR_RETURN_STACK;
+ else
+ config->ctrl &= ~ETMCR_RETURN_STACK;
+
if (config->mode & (ETM_MODE_EXCL_KERN | ETM_MODE_EXCL_USER))
etm_config_trace_mode(config);
--
2.7.4
^ permalink raw reply related
* [PATCH 4/9] coresight: reset "enable_sink" flag when need be
From: Mathieu Poirier @ 2016-11-29 16:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480438039-24960-1-git-send-email-mathieu.poirier@linaro.org>
When using coresight from the perf interface sinks are specified
as part of the perf command line. As such the sink needs to be
disabled once it has been acknowledged by the coresight framework.
Otherwise the sink stays enabled, which may interfere with other
sessions.
This patch removes the sink selection check from the build path
process and make it a function on it's own. The function is
then used when operating from sysFS or perf to determine what
sink has been selected.
If operated from perf the status of the "enable_sink" flag is
reset so that concurrent session can use a different sink. When
used from sysFS the status of the flag is left untouched since
users have full control.
The implementation doesn't handle a scenario where a sink has
been enabled from sysFS and another sink is selected from the
perf command line as both modes of operation are mutually
exclusive.
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
drivers/hwtracing/coresight/coresight-etm-perf.c | 31 +++++-----
drivers/hwtracing/coresight/coresight-priv.h | 4 +-
drivers/hwtracing/coresight/coresight.c | 74 ++++++++++++++++++++++--
3 files changed, 87 insertions(+), 22 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 2cd7c718198a..5a346fc8ce06 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -202,6 +202,21 @@ static void *etm_setup_aux(int event_cpu, void **pages,
if (!event_data)
return NULL;
+ /*
+ * In theory nothing prevent tracers in a trace session from being
+ * associated with different sinks, nor having a sink per tracer. But
+ * until we have HW with this kind of topology we need to assume tracers
+ * in a trace session are using the same sink. Therefore go through
+ * the coresight bus and pick the first enabled sink.
+ *
+ * When operated from sysFS users are responsible to enable the sink
+ * while from perf, the perf tools will do it based on the choice made
+ * on the cmd line. As such the "enable_sink" flag in sysFS is reset.
+ */
+ sink = coresight_get_enabled_sink(true);
+ if (!sink)
+ return NULL;
+
INIT_WORK(&event_data->work, free_event_data);
mask = &event_data->mask;
@@ -219,25 +234,11 @@ static void *etm_setup_aux(int event_cpu, void **pages,
* list of devices from source to sink that can be
* referenced later when the path is actually needed.
*/
- event_data->path[cpu] = coresight_build_path(csdev);
+ event_data->path[cpu] = coresight_build_path(csdev, sink);
if (IS_ERR(event_data->path[cpu]))
goto err;
}
- /*
- * In theory nothing prevent tracers in a trace session from being
- * associated with different sinks, nor having a sink per tracer. But
- * until we have HW with this kind of topology and a way to convey
- * sink assignement from the perf cmd line we need to assume tracers
- * in a trace session are using the same sink. Therefore pick the sink
- * found at the end of the first available path.
- */
- cpu = cpumask_first(mask);
- /* Grab the sink at the end of the path */
- sink = coresight_get_sink(event_data->path[cpu]);
- if (!sink)
- goto err;
-
if (!sink_ops(sink)->alloc_buffer)
goto err;
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 196a14be4b3d..ef9d8e93e3b2 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -111,7 +111,9 @@ static inline void CS_UNLOCK(void __iomem *addr)
void coresight_disable_path(struct list_head *path);
int coresight_enable_path(struct list_head *path, u32 mode);
struct coresight_device *coresight_get_sink(struct list_head *path);
-struct list_head *coresight_build_path(struct coresight_device *csdev);
+struct coresight_device *coresight_get_enabled_sink(bool reset);
+struct list_head *coresight_build_path(struct coresight_device *csdev,
+ struct coresight_device *sink);
void coresight_release_path(struct list_head *path);
#ifdef CONFIG_CORESIGHT_SOURCE_ETM3X
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
index 7bf00a0beb6f..0c37356e417c 100644
--- a/drivers/hwtracing/coresight/coresight.c
+++ b/drivers/hwtracing/coresight/coresight.c
@@ -368,6 +368,52 @@ struct coresight_device *coresight_get_sink(struct list_head *path)
return csdev;
}
+static int coresight_enabled_sink(struct device *dev, void *data)
+{
+ bool *reset = data;
+ struct coresight_device *csdev = to_coresight_device(dev);
+
+ if ((csdev->type == CORESIGHT_DEV_TYPE_SINK ||
+ csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) &&
+ csdev->activated) {
+ /*
+ * Now that we have a handle on the sink for this session,
+ * disable the sysFS "enable_sink" flag so that possible
+ * concurrent perf session that wish to use another sink don't
+ * trip on it. Doing so has no ramification for the current
+ * session.
+ */
+ if (*reset)
+ csdev->activated = false;
+
+ return 1;
+ }
+
+ return 0;
+}
+
+/**
+ * coresight_get_enabled_sink - returns the first enabled sink found on the bus
+ * @deactivate: Whether the 'enable_sink' flag should be reset
+ *
+ * When operated from perf the deactivate parameter should be set to 'true'.
+ * That way the "enabled_sink" flag of the sink that was selected can be reset,
+ * allowing for other concurrent perf sessions to choose a different sink.
+ *
+ * When operated from sysFS users have full control and as such the deactivate
+ * parameter should be set to 'false', hence mandating users to explicitly
+ * clear the flag.
+ */
+struct coresight_device *coresight_get_enabled_sink(bool deactivate)
+{
+ struct device *dev = NULL;
+
+ dev = bus_find_device(&coresight_bustype, NULL, &deactivate,
+ coresight_enabled_sink);
+
+ return dev ? to_coresight_device(dev) : NULL;
+}
+
/**
* _coresight_build_path - recursively build a path from a @csdev to a sink.
* @csdev: The device to start from.
@@ -380,6 +426,7 @@ struct coresight_device *coresight_get_sink(struct list_head *path)
* last one.
*/
static int _coresight_build_path(struct coresight_device *csdev,
+ struct coresight_device *sink,
struct list_head *path)
{
int i;
@@ -387,15 +434,15 @@ static int _coresight_build_path(struct coresight_device *csdev,
struct coresight_node *node;
/* An activated sink has been found. Enqueue the element */
- if ((csdev->type == CORESIGHT_DEV_TYPE_SINK ||
- csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) && csdev->activated)
+ if (csdev == sink)
goto out;
/* Not a sink - recursively explore each port found on this element */
for (i = 0; i < csdev->nr_outport; i++) {
struct coresight_device *child_dev = csdev->conns[i].child_dev;
- if (child_dev && _coresight_build_path(child_dev, path) == 0) {
+ if (child_dev &&
+ _coresight_build_path(child_dev, sink, path) == 0) {
found = true;
break;
}
@@ -422,18 +469,22 @@ static int _coresight_build_path(struct coresight_device *csdev,
return 0;
}
-struct list_head *coresight_build_path(struct coresight_device *csdev)
+struct list_head *coresight_build_path(struct coresight_device *source,
+ struct coresight_device *sink)
{
struct list_head *path;
int rc;
+ if (!sink)
+ return ERR_PTR(-EINVAL);
+
path = kzalloc(sizeof(struct list_head), GFP_KERNEL);
if (!path)
return ERR_PTR(-ENOMEM);
INIT_LIST_HEAD(path);
- rc = _coresight_build_path(csdev, path);
+ rc = _coresight_build_path(source, sink, path);
if (rc) {
kfree(path);
return ERR_PTR(rc);
@@ -497,6 +548,7 @@ static int coresight_validate_source(struct coresight_device *csdev,
int coresight_enable(struct coresight_device *csdev)
{
int cpu, ret = 0;
+ struct coresight_device *sink;
struct list_head *path;
mutex_lock(&coresight_mutex);
@@ -508,7 +560,17 @@ int coresight_enable(struct coresight_device *csdev)
if (csdev->enable)
goto out;
- path = coresight_build_path(csdev);
+ /*
+ * Search for a valid sink for this session but don't reset the
+ * "enable_sink" flag in sysFS. Users get to do that explicitly.
+ */
+ sink = coresight_get_enabled_sink(false);
+ if (!sink) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ path = coresight_build_path(csdev, sink);
if (IS_ERR(path)) {
pr_err("building path(s) failed\n");
ret = PTR_ERR(path);
--
2.7.4
^ 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