[PATCH v3 0/4] Raspberry Pi 4 DMA addressing support
Hi all, this series attempts to address some issues we found while bringing up the new Raspberry Pi 4 in arm64 and it's intended to serve as a follow up of these discussions: v2: https://lkml.org/lkml/2019/8/20/767 v1: https://lkml.org/lkml/2019/7/31/922 RFC: https://lkml.org/lkml/2019/7/17/476 The new Raspberry Pi 4 has up to 4GB of memory but most peripherals can only address the first GB: their DMA address range is 0xc000-0xfc00 which is aliased to the first GB of physical memory 0x-0x3c00. Note that only some peripherals have these limitations: the PCIe, V3D, GENET, and 40-bit DMA channels have a wider view of the address space by virtue of being hooked up trough a second interconnect. Part of this is solved on arm32 by setting up the machine specific '.dma_zone_size = SZ_1G', which takes care of reserving the coherent memory area at the right spot. That said no buffer bouncing (needed for dma streaming) is available at the moment, but that's a story for another series. Unfortunately there is no such thing as 'dma_zone_size' in arm64. Only ZONE_DMA32 is created which is interpreted by dma-direct and the arm64 arch code as if all peripherals where be able to address the first 4GB of memory. In the light of this, the series implements the following changes: - Create both DMA zones in arm64, ZONE_DMA will contain the first 1G area and ZONE_DMA32 the rest of the 32 bit addressable memory. So far the RPi4 is the only arm64 device with such DMA addressing limitations so this hardcoded solution was deemed preferable. - Properly set ARCH_ZONE_DMA_BITS. - Reserve the CMA area in a place suitable for all peripherals. This series has been tested on multiple devices both by checking the zones setup matches the expectations and by double-checking physical addresses on pages allocated on the three relevant areas GFP_DMA, GFP_DMA32, GFP_KERNEL: - On an RPi4 with variations on the ram memory size. But also forcing the situation where all three memory zones are nonempty by setting a 3G ZONE_DMA32 ceiling on a 4G setup. Both with and without NUMA support. - On a Synquacer box[1] with 32G of memory. - On an ACPI based Huawei TaiShan server[2] with 256G of memory. - On a QEMU virtual machine running arm64's OpenSUSE Tumbleweed. That's all. Regards, Nicolas [1] https://www.96boards.org/product/developerbox/ [2] https://e.huawei.com/en/products/cloud-computing-dc/servers/taishan-server/taishan-2280-v2 --- Changes in v3: - Fixed ZONE_DMA's size to 1G - Update mmzone.h's comment to match changes in arm64 - Remove all dma-direct patches Changes in v2: - Update comment to reflect new zones split - ZONE_DMA will never be left empty - Try another approach merging both ZONE_DMA comments into one - Address Christoph's comments - If this approach doesn't get much traction I'll just drop the patch from the series as it's not really essential Nicolas Saenz Julienne (4): arm64: mm: use arm64_dma_phys_limit instead of calling max_zone_dma_phys() arm64: rename variables used to calculate ZONE_DMA32's size arm64: use both ZONE_DMA and ZONE_DMA32 mm: refresh ZONE_DMA and ZONE_DMA32 comments in 'enum zone_type' arch/arm64/Kconfig| 4 ++ arch/arm64/include/asm/page.h | 2 + arch/arm64/mm/init.c | 71 +-- include/linux/mmzone.h| 45 -- 4 files changed, 83 insertions(+), 39 deletions(-) -- 2.23.0
[PATCH v3 1/4] arm64: mm: use arm64_dma_phys_limit instead of calling max_zone_dma_phys()
By the time we call zones_sizes_init() arm64_dma_phys_limit already contains the result of max_zone_dma_phys(). We use the variable instead of calling the function directly to save some precious cpu time. Signed-off-by: Nicolas Saenz Julienne --- Changes in v3: None Changes in v2: None arch/arm64/mm/init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index f3c795278def..6112d6c90fa8 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -181,7 +181,7 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max) unsigned long max_zone_pfns[MAX_NR_ZONES] = {0}; #ifdef CONFIG_ZONE_DMA32 - max_zone_pfns[ZONE_DMA32] = PFN_DOWN(max_zone_dma_phys()); + max_zone_pfns[ZONE_DMA32] = PFN_DOWN(arm64_dma_phys_limit); #endif max_zone_pfns[ZONE_NORMAL] = max; -- 2.23.0
[PATCH v2 01/11] asm-generic: add dma_zone_size
Some architectures have platform specific DMA addressing limitations. This will allow for hardware description code to provide the constraints in a generic manner, so as for arch code to properly setup it's memory zones and DMA mask. Signed-off-by: Nicolas Saenz Julienne --- Changes in v2: None include/asm-generic/dma.h | 8 +++- mm/page_alloc.c | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/include/asm-generic/dma.h b/include/asm-generic/dma.h index 43d0c8af8058..c2f39cdb64f6 100644 --- a/include/asm-generic/dma.h +++ b/include/asm-generic/dma.h @@ -8,7 +8,13 @@ * * Some code relies on seeing MAX_DMA_ADDRESS though. */ -#define MAX_DMA_ADDRESS PAGE_OFFSET +#define MAX_DMA_ADDRESS (PAGE_OFFSET + dma_zone_size) + +/* + * Some architectures may have platform specific DMA addressing constraints. + * Firmware can use this to fine tune the device's DMA memory zone. + */ +extern u64 dma_zone_size __ro_after_init; extern int request_dma(unsigned int dmanr, const char *device_id); extern void free_dma(unsigned int dmanr); diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 272c6de1bf4e..b514afee5451 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -133,6 +133,9 @@ EXPORT_SYMBOL(_totalram_pages); unsigned long totalreserve_pages __read_mostly; unsigned long totalcma_pages __read_mostly; +u64 dma_zone_size __ro_after_init; +EXPORT_SYMBOL(dma_zone_size); + int percpu_pagelist_fraction; gfp_t gfp_allowed_mask __read_mostly = GFP_BOOT_MASK; #ifdef CONFIG_INIT_ON_ALLOC_DEFAULT_ON -- 2.22.0
[PATCH v2 02/11] arm: use generic dma_zone_size
'dma_zone_size' was created as a generic replacement to 'arm_dma_zone_size'. Use it accordingly. Signed-off-by: Nicolas Saenz Julienne --- Changes in v2: None arch/arm/include/asm/dma.h | 8 +--- arch/arm/mm/init.c | 12 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/arch/arm/include/asm/dma.h b/arch/arm/include/asm/dma.h index a81dda65c576..52d19ffd92b4 100644 --- a/arch/arm/include/asm/dma.h +++ b/arch/arm/include/asm/dma.h @@ -2,16 +2,18 @@ #ifndef __ASM_ARM_DMA_H #define __ASM_ARM_DMA_H +#include + /* * This is the maximum virtual address which can be DMA'd from. */ +#undef MAX_DMA_ADDRESS #ifndef CONFIG_ZONE_DMA #define MAX_DMA_ADDRESS0xUL #else #define MAX_DMA_ADDRESS({ \ - extern phys_addr_t arm_dma_zone_size; \ - arm_dma_zone_size && arm_dma_zone_size < (0x1000 - PAGE_OFFSET) ? \ - (PAGE_OFFSET + arm_dma_zone_size) : 0xUL; }) + dma_zone_size && dma_zone_size < (0x1000 - PAGE_OFFSET) ? \ + (PAGE_OFFSET + dma_zone_size) : 0xUL; }) #endif #ifdef CONFIG_ISA_DMA_API diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c index 16d373d587c4..95680bad245a 100644 --- a/arch/arm/mm/init.c +++ b/arch/arm/mm/init.c @@ -79,10 +79,6 @@ static void __init find_limits(unsigned long *min, unsigned long *max_low, } #ifdef CONFIG_ZONE_DMA - -phys_addr_t arm_dma_zone_size __read_mostly; -EXPORT_SYMBOL(arm_dma_zone_size); - /* * The DMA mask corresponding to the maximum bus address allocatable * using GFP_DMA. The default here places no restriction on DMA @@ -109,8 +105,8 @@ void __init setup_dma_zone(const struct machine_desc *mdesc) { #ifdef CONFIG_ZONE_DMA if (mdesc->dma_zone_size) { - arm_dma_zone_size = mdesc->dma_zone_size; - arm_dma_limit = PHYS_OFFSET + arm_dma_zone_size - 1; + dma_zone_size = mdesc->dma_zone_size; + arm_dma_limit = PHYS_OFFSET + dma_zone_size - 1; } else arm_dma_limit = 0x; arm_dma_pfn_limit = arm_dma_limit >> PAGE_SHIFT; @@ -164,9 +160,9 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max_low, * Adjust the sizes according to any special requirements for * this machine type. */ - if (arm_dma_zone_size) + if (dma_zone_size) arm_adjust_dma_zone(zone_size, zhole_size, - arm_dma_zone_size >> PAGE_SHIFT); + dma_zone_size >> PAGE_SHIFT); #endif free_area_init_node(0, zone_size, min, zhole_size); -- 2.22.0
[PATCH v2 00/11] Raspberry Pi 4 DMA addressing support
Hi all, this series attempts to address some issues we found while bringing up the new Raspberry Pi 4 in arm64 and it's intended to serve as a follow up of these discussions: v1: https://lkml.org/lkml/2019/7/31/922 RFC: https://lkml.org/lkml/2019/7/17/476 The new Raspberry Pi 4 has up to 4GB of memory but most peripherals can only address the first GB: their DMA address range is 0xc000-0xfc00 which is aliased to the first GB of physical memory 0x-0x3c00. Note that only some peripherals have these limitations: the PCIe, V3D, GENET, and 40-bit DMA channels have a wider view of the address space by virtue of being hooked up trough a second interconnect. Part of this is solved in arm32 by setting up the machine specific '.dma_zone_size = SZ_1G', which takes care of reserving the coherent memory area at the right spot. That said no buffer bouncing (needed for dma streaming) is available at the moment, but that's a story for another series. Unfortunately there is no such thing as 'dma_zone_size' in arm64. Only ZONE_DMA32 is created which is interpreted by dma-direct and the arm64 arch code as if all peripherals where be able to address the first 4GB of memory. In the light of this, the series implements the following changes: - Create generic 'dma_zone_size' in order for hardware description code to set it up when needed. - Add a function in early_init_dt_scan() to setup 'dma_zone_size' for the RPi4. - Create both DMA zones in arm64, ZONE_DMA will contain the area addressable by all peripherals and ZONE_DMA32 the rest of the 32 bit addressable memory. ZONE_DMA32 might be left empty. - Reserve the CMA area in a place suitable for all peripherals. - Inform dma-direct of the new runtime calculated min_mask. This series has been tested on multiple devices both by checking the zones setup matches the expectations and by double-checking physical addresses on pages allocated on the three relevant areas GFP_DMA, GFP_DMA32, GFP_KERNEL: - On an RPi4 with variations on the ram memory size. But also forcing the situation where all three memory zones are nonempty by setting a 3G ZONE_DMA32 ceiling on a 4G setup. Both with and without NUMA support. - On a Synquacer box[1] with 32G of memory. - On an ACPI based Huawei TaiShan server[2] with 256G of memory. - On a QEMU virtual machine running arm64's OpenSUSE Tumbleweed. That's all. Regards, Nicolas [1] https://www.96boards.org/product/developerbox/ [2] https://e.huawei.com/en/products/cloud-computing-dc/servers/taishan-server/taishan-2280-v2 --- Changes in v2: - More in depth testing. - Create new global 'dma_zone_size'. - New approach to getting the dma_zone_size, instead of parsing the dts we hardcode it conditionally to the machine compatible name. - Fix ZONE_DMA and ZONE_DMA32 split, now ZONE_DMA32 remains empty if ZONE_DMA fits the whole 32 bit addressable space. - Take into account devices with DMA offset. - Rename new dma-direct variable to zone_dma_bits. - Try new approach by merging both ZONE_DMA and ZONE_DMA32 comments in mmzone.h, add new up to date examples. Nicolas Saenz Julienne (11): asm-generic: add dma_zone_size arm: use generic dma_zone_size of/fdt: add of_fdt_machine_is_compatible function of/fdt: add early_init_dt_get_dma_zone_size() arm64: mm: use arm64_dma_phys_limit instead of calling max_zone_dma_phys() arm64: rename variables used to calculate ZONE_DMA32's size arm64: re-introduce max_zone_dma_phys() arm64: use both ZONE_DMA and ZONE_DMA32 dma-direct: turn ARCH_ZONE_DMA_BITS into a variable arm64: edit zone_dma_bits to fine tune dma-direct min mask mm: refresh ZONE_DMA and ZONE_DMA32 comments in 'enum zone_type' arch/arm/include/asm/dma.h | 8 ++-- arch/arm/mm/init.c | 12 ++ arch/arm64/Kconfig | 4 ++ arch/arm64/mm/init.c| 73 + arch/powerpc/include/asm/page.h | 9 arch/powerpc/mm/mem.c | 16 +--- arch/s390/include/asm/page.h| 2 - arch/s390/mm/init.c | 1 + drivers/of/fdt.c| 15 +++ include/asm-generic/dma.h | 8 +++- include/linux/dma-direct.h | 2 + include/linux/mmzone.h | 46 - kernel/dma/direct.c | 13 +++--- mm/page_alloc.c | 3 ++ 14 files changed, 140 insertions(+), 72 deletions(-) -- 2.22.0
[PATCH v2 07/11] arm64: re-introduce max_zone_dma_phys()
Some devices might have multiple interconnects with different DMA addressing limitations. This function provides the higher physical address accessible by all peripherals on the SoC. If such limitation doesn't exist it'll return the maximum physical address of the 32 bit addressable area. Signed-off-by: Nicolas Saenz Julienne --- Changes in v2: - Update function's behavior to fit new dma zones split - Use dma_zone_size - Take into account devices with a hardcoded DMA offset arch/arm64/mm/init.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index 8956c22634dd..bc7999020c71 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -174,6 +174,17 @@ static phys_addr_t __init max_zone_dma32_phys(void) return min(offset + (1ULL << 32), memblock_end_of_DRAM()); } +static phys_addr_t __init max_zone_dma_phys(void) + +{ + phys_addr_t offset = memblock_start_of_DRAM() & GENMASK_ULL(63, 32); + + if (dma_zone_size) + return min(offset + dma_zone_size, memblock_end_of_DRAM()); + else + return max_zone_dma32_phys(); +} + #ifdef CONFIG_NUMA static void __init zone_sizes_init(unsigned long min, unsigned long max) -- 2.22.0
[PATCH v2 04/11] of/fdt: add early_init_dt_get_dma_zone_size()
Some devices might have weird DMA addressing limitations that only apply to a subset of the available peripherals. For example the Raspberry Pi 4 has two interconnects, one able to address the whole lower 4G memory area and another one limited to the lower 1G. Being an uncommon situation we simply hardcode the device wide DMA addressable memory size conditionally to the machine compatible name and set 'dma_zone_size' accordingly. Signed-off-by: Nicolas Saenz Julienne --- Changes in v2: - New approach to getting dma_zone_size, instead of parsing the dts we hardcode it conditionally to the machine compatible name. drivers/of/fdt.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index 06ffbd39d9af..f756e8c05a77 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -27,6 +27,7 @@ #include /* for COMMAND_LINE_SIZE */ #include +#include/* for dma_zone_size */ #include "of_private.h" @@ -1195,6 +1196,12 @@ void __init early_init_dt_scan_nodes(void) of_scan_flat_dt(early_init_dt_scan_memory, NULL); } +void __init early_init_dt_get_dma_zone_size(void) +{ + if (of_fdt_machine_is_compatible("brcm,bcm2711")) + dma_zone_size = 0x3c00; +} + bool __init early_init_dt_scan(void *params) { bool status; @@ -1204,6 +1211,7 @@ bool __init early_init_dt_scan(void *params) return false; early_init_dt_scan_nodes(); + early_init_dt_get_dma_zone_size(); return true; } -- 2.22.0
[PATCH v2 05/11] arm64: mm: use arm64_dma_phys_limit instead of calling max_zone_dma_phys()
By the time we call zones_sizes_init() arm64_dma_phys_limit already contains the result of max_zone_dma_phys(). We use the variable instead of calling the function directly to save some precious cpu time. Signed-off-by: Nicolas Saenz Julienne --- Changes in v2: None arch/arm64/mm/init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index f3c795278def..6112d6c90fa8 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -181,7 +181,7 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max) unsigned long max_zone_pfns[MAX_NR_ZONES] = {0}; #ifdef CONFIG_ZONE_DMA32 - max_zone_pfns[ZONE_DMA32] = PFN_DOWN(max_zone_dma_phys()); + max_zone_pfns[ZONE_DMA32] = PFN_DOWN(arm64_dma_phys_limit); #endif max_zone_pfns[ZONE_NORMAL] = max; -- 2.22.0
Re: [PATCH v2] HID: input: fix a4tech horizontal wheel custom usage
On Tue, 2019-06-11 at 14:13 +0200, Nicolas Saenz Julienne wrote: > Some a4tech mice use the 'GenericDesktop.00b8' usage to inform whether > the previous wheel report was horizontal or vertical. Before > c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key") this > usage was being mapped to 'Relative.Misc'. After the patch it's simply > ignored (usage->type == 0 & usage->code == 0). Which ultimately makes > hid-a4tech ignore the WHEEL/HWHEEL selection event, as it has no > usage->type. > > We shouldn't rely on a mapping for that usage as it's nonstandard and > doesn't really map to an input event. So we bypass the mapping and make > sure the custom event handling properly handles both reports. > > Fixes: c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key") > Signed-off-by: Nicolas Saenz Julienne > --- It would be nice for this patch not to get lost. It fixes issues both repoted on opensuse and fedora. I can resubmit it if needed. Regards, Nicolas signature.asc Description: This is a digitally signed message part
[PATCH 0/8] Raspberry Pi 4 DMA addressing support
Hi all, this series attempts to address some issues we found while bringing up the new Raspberry Pi 4 in arm64 and it's intended to serve as a follow up of this discussion: https://lkml.org/lkml/2019/7/17/476 The new Raspberry Pi 4 has up to 4GB of memory but most peripherals can only address the first GB: their DMA address range is 0xc000-0xfc00 which is aliased to the first GB of physical memory 0x-0x3c00. Note that only some peripherals have these limitations: the ARM cores, PCIe, V3D, GENET, and 40-bit DMA channels have a wider view of the address space. Part of this is solved in arm32 by setting up the machine specific '.dma_zone_size = SZ_1G', which takes care of the allocating the coherent memory area at the right spot. Yet no buffer bouncing (needed for dma streaming) is available at the moment, but that's a story for another series. Unfortunately there is no such thing as '.dma_zone_size' in arm64 also only ZONE_DMA32 is created which is interpreted by dma-direct and the arm64 code as if all peripherals where be able to address the first 4GB of memory. In the light of this, the series implements the following changes: - Add code that parses the device-tree in oder to find the SoC's common DMA area. - Create a ZONE_DMA whenever that area is needed and add the rest of the lower 4 GB of memory to ZONE_DMA32*. - Create the CMA area in a place suitable for all peripherals. - Inform dma-direct of the new runtime calculated min_mask*. That's all. Regards, Nicolas * These solutions where already discussed on the previous RFC (see link above). --- Nicolas Saenz Julienne (8): arm64: mm: use arm64_dma_phys_limit instead of calling max_zone_dma_phys() arm64: rename variables used to calculate ZONE_DMA32's size of/fdt: add function to get the SoC wide DMA addressable memory size arm64: re-introduce max_zone_dma_phys() arm64: use ZONE_DMA on DMA addressing limited devices dma-direct: turn ARCH_ZONE_DMA_BITS into a variable arm64: update arch_zone_dma_bits to fine tune dma-direct min mask mm: comment arm64's usage of 'enum zone_type' arch/arm64/Kconfig | 4 ++ arch/arm64/mm/init.c| 78 ++--- arch/powerpc/include/asm/page.h | 9 arch/powerpc/mm/mem.c | 14 +- arch/s390/include/asm/page.h| 2 - arch/s390/mm/init.c | 1 + drivers/of/fdt.c| 72 ++ include/linux/dma-direct.h | 2 + include/linux/mmzone.h | 21 - include/linux/of_fdt.h | 2 + kernel/dma/direct.c | 8 ++-- 11 files changed, 168 insertions(+), 45 deletions(-) -- 2.22.0
[PATCH 1/8] arm64: mm: use arm64_dma_phys_limit instead of calling max_zone_dma_phys()
By the time we call zones_sizes_init() arm64_dma_phys_limit already contains the result of max_zone_dma_phys(). We use the variable instead of calling the function directly to save some precious cpu time. Signed-off-by: Nicolas Saenz Julienne --- arch/arm64/mm/init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index f3c795278def..6112d6c90fa8 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -181,7 +181,7 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max) unsigned long max_zone_pfns[MAX_NR_ZONES] = {0}; #ifdef CONFIG_ZONE_DMA32 - max_zone_pfns[ZONE_DMA32] = PFN_DOWN(max_zone_dma_phys()); + max_zone_pfns[ZONE_DMA32] = PFN_DOWN(arm64_dma_phys_limit); #endif max_zone_pfns[ZONE_NORMAL] = max; -- 2.22.0
[RFC] ARM: bcm2835: register dmabounce on devices hooked to main interconnect
NOTE: This patch builds upon Stefan's series providing basic support for RPi4[1]. I'm mostly interested in verifying if this is the correct approach to the issue stated below. If so I assume this will be added to Stefan's v2 series. The new Raspberry Pi 4 happens to have weird DMA constraints. Even though it might contain up to 4 GB of ram, most devices can only access the first lower GB of memory. This breaks the overall assumption DMA API makes whereas 32-bit DMA masks are always supported[2], and potentially breaks DMA addressing for all streaming DMA users. This has already been observed with 'sdhci-iproc' but might as well happen elsewhere. Note that contiguous allocations are safe as 'dma_zone_size' is set accordingly. To get around that limitation we register arm's dmabounce dma-ops on all devices hooked to the SoC's main interconnect. [1] https://www.spinics.net/lists/arm-kernel/msg742120.html [2] https://www.spinics.net/lists/arm-kernel/msg742736.html Signed-off-by: Nicolas Saenz Julienne --- arch/arm/mach-bcm/Kconfig | 1 + arch/arm/mach-bcm/board_bcm2835.c | 29 + 2 files changed, 30 insertions(+) diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig index 5e5f1fabc3d4..588326f7e269 100644 --- a/arch/arm/mach-bcm/Kconfig +++ b/arch/arm/mach-bcm/Kconfig @@ -168,6 +168,7 @@ config ARCH_BCM2835 select PINCTRL select PINCTRL_BCM2835 select MFD_CORE + select DMABOUNCE if ARCH_MULTI_V7 help This enables support for the Broadcom BCM2835 and BCM2836 SoCs. This SoC is used in the Raspberry Pi and Roku 2 devices. diff --git a/arch/arm/mach-bcm/board_bcm2835.c b/arch/arm/mach-bcm/board_bcm2835.c index c09cf25596af..7aff29f77ca7 100644 --- a/arch/arm/mach-bcm/board_bcm2835.c +++ b/arch/arm/mach-bcm/board_bcm2835.c @@ -3,6 +3,8 @@ * Copyright (C) 2010 Broadcom */ +#include +#include #include #include #include @@ -24,8 +26,35 @@ static const char * const bcm2835_compat[] = { NULL }; +static int bcm2835_needs_bounce(struct device *dev, dma_addr_t dma_addr, size_t size) +{ + /* +* The accepted dma addresses are [0xc000, 0x] which map to +* ram's [0x, 0x3fff]. +*/ + return dma_addr < 3ULL * SZ_1G; +} + +static int bcm2835_platform_notify(struct device *dev) +{ + if (dev->parent && !strcmp("soc", dev_name(dev->parent))) { + dev->dma_mask = >coherent_dma_mask; + dev->coherent_dma_mask = DMA_BIT_MASK(30); + dmabounce_register_dev(dev, 2048, 4096, bcm2835_needs_bounce); + } + + return 0; +} + +void __init bcm2835_init_early(void) +{ + if(of_machine_is_compatible("brcm,bcm2711")) + platform_notify = bcm2835_platform_notify; +} + DT_MACHINE_START(BCM2835, "BCM2835") .dma_zone_size = SZ_1G, .dt_compat = bcm2835_compat, .smp = smp_ops(bcm2836_smp_ops), + .init_early = bcm2835_init_early, MACHINE_END -- 2.22.0
[RFC 2/4] arm64: mm: parse dma-ranges in order to better estimate arm64_dma_phys_limit
The dma physical limit has so far been calculated based on the memory size and the assumption that dma would be at least able to address the first 4 GB. This turned out no to be true with the Raspberry Pi 4 which, on it's main interconnect, can only address the first GB of memory, even though it might have up to 4 GB. With the current miscalculated dma physical limit the contiguous memory reserve is located in an inaccessible area for most of the board's devices. To solve this we now scan the device tree for the 'dma-ranges' property on the root or interconnect nodes, which allows us to calculate the lowest common denominator dma physical limit. If no dma-ranges is available, we'll default to the old scheme. Signed-off-by: Nicolas Saenz Julienne --- arch/arm64/mm/init.c | 61 +--- 1 file changed, 57 insertions(+), 4 deletions(-) diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index 6112d6c90fa8..5708adf0db52 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -163,14 +163,67 @@ static void __init reserve_elfcorehdr(void) { } #endif /* CONFIG_CRASH_DUMP */ + +static int __init early_init_dt_scan_dma_ranges(unsigned long node, + const char *uname, int depth, void *data) +{ + phys_addr_t *dma_phys_limit = data; + u64 phys_addr, dma_addr, size; + int dma_addr_cells; + const __be32 *reg; + const void *prop; + int len; + + /* We keep looking if this isn't the root node or an interconnect */ + if (!(depth == 0 || + (depth == 1 && of_flat_dt_is_compatible(node, "simple-bus" + return 0; + + prop = of_get_flat_dt_prop(node, "#address-cells", NULL); + if (prop) + dma_addr_cells = be32_to_cpup(prop); + else + dma_addr_cells = 1; /* arm64's default addr_cell size */ + + reg = of_get_flat_dt_prop(node, "dma-ranges", ); + if (!reg || + len < (dma_addr_cells + dt_root_addr_cells + dt_root_size_cells)) + return 0; + + dma_addr = dt_mem_next_cell(dma_addr_cells, ); + phys_addr = dt_mem_next_cell(dt_root_addr_cells, ); + size = dt_mem_next_cell(dt_root_size_cells, ); + + /* We're in ZONE_DMA32 */ + if (size > (1ULL << 32)) + size = 1ULL << 32; + + if (*dma_phys_limit > (phys_addr + size)) + *dma_phys_limit = phys_addr + size; + + return 0; +} + /* - * Return the maximum physical address for ZONE_DMA32 (DMA_BIT_MASK(32)). It - * currently assumes that for memory starting above 4G, 32-bit devices will - * use a DMA offset. + * Return the maximum physical address for ZONE_DMA32. It currently assumes + * that for memory starting above 4G, 32-bit devices will use a DMA offset. */ static phys_addr_t __init max_zone_dma_phys(void) { - phys_addr_t offset = memblock_start_of_DRAM() & GENMASK_ULL(63, 32); + phys_addr_t dma_phys_limit = ~0; + phys_addr_t offset; + + /* +* We walk the whole fdt looking for nodes with dma-ranges, calculate +* the max_zone_dma_phys for them and keep going. We end-up getting the +* lowest common denominator of all the matches. +*/ + of_scan_flat_dt(early_init_dt_scan_dma_ranges, _phys_limit); + if (dma_phys_limit != ~0) + return dma_phys_limit; + + /* If no dma-ranges property was found we try to infer the value */ + offset = memblock_start_of_DRAM() & GENMASK_ULL(63, 32); return min(offset + (1ULL << 32), memblock_end_of_DRAM()); } -- 2.22.0
[RFC 4/4] arm64: mm: set direct_dma_min_mask according to dma-ranges
Now that we parse the dma-ranges during initialization we can fine-tune the DMA mask used by the direct DMA implementation. We set the mask based on the size of the DMA addressable memory, and if bigger than 4GB we force it to DMA_BIT_MASK(32) as it's always been. Signed-off-by: Nicolas Saenz Julienne --- arch/arm64/mm/init.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index 5708adf0db52..f8af2c99667c 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -52,6 +52,8 @@ EXPORT_SYMBOL(memstart_addr); phys_addr_t arm64_dma_phys_limit __ro_after_init; +extern u64 dma_direct_min_mask; + #ifdef CONFIG_KEXEC_CORE /* * reserve_crashkernel() - reserves memory for crash kernel @@ -198,8 +200,12 @@ static int __init early_init_dt_scan_dma_ranges(unsigned long node, if (size > (1ULL << 32)) size = 1ULL << 32; - if (*dma_phys_limit > (phys_addr + size)) + if (*dma_phys_limit > (phys_addr + size)) { + /* Set min DMA mask in case is was smaller than 32 */ + dma_direct_min_mask = size - 1; + *dma_phys_limit = phys_addr + size; + } return 0; } -- 2.22.0
[RFC 1/4] arm64: mm: use arm64_dma_phys_limit instead of calling max_zone_dma_phys()
By the time we call zones_sizes_init() arm64_dma_phys_limit already contains the result of max_zone_dma_phys(). We use the variable instead of calling the function directly to save some precious cpu time. Signed-off-by: Nicolas Saenz Julienne --- arch/arm64/mm/init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index f3c795278def..6112d6c90fa8 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -181,7 +181,7 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max) unsigned long max_zone_pfns[MAX_NR_ZONES] = {0}; #ifdef CONFIG_ZONE_DMA32 - max_zone_pfns[ZONE_DMA32] = PFN_DOWN(max_zone_dma_phys()); + max_zone_pfns[ZONE_DMA32] = PFN_DOWN(arm64_dma_phys_limit); #endif max_zone_pfns[ZONE_NORMAL] = max; -- 2.22.0
Re: [PATCH v2] HID: input: fix a4tech horizontal wheel custom usage
On Fri, 2019-06-14 at 16:25 +0200, Nicolas Saenz Julienne wrote: > On Fri, 2019-06-14 at 15:36 +0200, Benjamin Tissoires wrote: > > Hi Wolfgang, > > > > On Thu, Jun 13, 2019 at 1:49 PM Wolfgang Bauer wrote: > > > On Tuesday, 11. Juni 2019, 16:42:37 Benjamin Tissoires wrote: > > > > On Tue, Jun 11, 2019 at 2:13 PM Nicolas Saenz Julienne > > > > > > > > wrote: > > > > > NOTE: I CC'd Wolfgang as he's the one who can test this. > > > > > > > > I'll wait for Wolfram to confirm that the patch works before pushing > > > > then. > > > > > > My name is Wolfgang, not Wolfram... ;-) > > > > ouch, sorry for that (I am more used to talk to the I2C maintainer > > apparently) > > > > > But never mind. > > > > > > I tested the patch meanwhile on top of kernel 5.2.rc4, where the mouse > > > wheel > > > actually worked. > > > > Actually, I am a little bit lost here. > > > > The patch mentions a fix of c01908a14bf73, which is in 5.1 final. > > So if your mouse works in 5.2.rc4, I am not sure how > > HID-a4tech-fix-horizontal-scrolling.patch could break it. > > > > Could you be slightly more specific in what "works" and what doesn't? > > Hi Benjamin, > > First of all here's the descriptor: > 0x05, 0x01, /* Usage Page (Desktop), */ > 0x09, 0x02, /* Usage (Mouse), */ > 0xA1, 0x01, /* Collection (Application), */ > 0x09, 0x01, /* Usage (Pointer),*/ > 0xA1, 0x00, /* Collection (Physical), */ > 0x05, 0x09, /* Usage Page (Button),*/ > 0x19, 0x01, /* Usage Minimum (01h),*/ > 0x29, 0x08, /* Usage Maximum (08h),*/ > 0x15, 0x00, /* Logical Minimum (0),*/ > 0x25, 0x01, /* Logical Maximum (1),*/ > 0x75, 0x01, /* Report Size (1),*/ > 0x95, 0x08, /* Report Count (8), */ > 0x81, 0x02, /* Input (Variable), */ > 0x05, 0x01, /* Usage Page (Desktop), */ > 0x09, 0x30, /* Usage (X), */ > 0x09, 0x31, /* Usage (Y), */ > 0x09, 0x38, /* Usage (Wheel), */ > 0x09, 0xB8, /* Usage (B8h),*/ > 0x15, 0x81, /* Logical Minimum (-127), */ > 0x25, 0x7F, /* Logical Maximum (127), */ > 0x75, 0x08, /* Report Size (8),*/ > 0x95, 0x04, /* Report Count (4), */ > 0x81, 0x06, /* Input (Variable, Relative), */ > 0xC0, /* End Collection, */ > 0xC0/* End Collection > > > Sorry for the confusion, I'll try to explain the situation: > > In v5.2-rc4 without "HID-a4tech-fix-horizontal-scrolling.patch" the vertical > wheel works out of luck as it's mapped to REL_WHEEL_HIGH_RES, which hid-a4tech > ignores and lets hid-input process, the horizontal wheel is broken. On top of > that Usage(0xB8) is also ignored by both hid-a4tech and hid-input as it isn't > mapped to anything. > > There are two distinct bugs here: > - High resolution wheel processing in hid-a4tech not being implemented, > breaking horizontal wheels. > - hid-a4tech not taking care of Usage(0xB8) correctly as it depended on it > being mapped to "Rel.Misc". That behaviour changed in v5.1 with "HID: > input: add mapping for "Toggle Display" key". > > Once high resolution wheel reports are fixed and handled in hid-a4tech's > custom > event, the mouse breaks as it's the processing of Usage(0xB8) that triggers > the > input_events, which is being ignored. > > You'll probably ask how come we didn't see this when > "HID-a4tech-fix-horizontal-scrolling.patch" was merged. It's due to the fact > it > was tested on an older kernel, v5.0.15, that didn't contain "HID: input: add > mapping for "Toggle Display" key"[1]. > > So that's why I added that specific fix tag. For LTS kernels, it is possible > that "Toggle Display" support was back-ported but not the high resolution > wheels support. Hi, Any thoughts on this? Regards, Nicolas signature.asc Description: This is a digitally signed message part
[PATCH v2] xhci: clear port_remote_wakeup after resume failure
This was seen on a Dell Precision 5520 using it's WD15 dock. The dock's Ethernet connectivity is provided though USB. While idle, both the Ethernet device and XHCI are suspended by runtime PM. To be then resumed on behalf of the Ethernet device, which has remote wake-up capabilities. The Ethernet device was observed to randomly disconnect from the USB port shortly after submitting it's remote wake-up request. Probably a weird timing issue yet to be investigated. This causes runtime PM to busyloop causing some tangible CPU load. The reason is the port gets stuck in the middle of a remote wake-up operation, waiting for the device to switch to U0. This never happens, leaving "port_remote_wakeup" enabled, and automatically triggering a failure on any further suspend operation. This patch clears "port_remote_wakeup" upon detecting a device with a wrong PORT_CONNECT state. Making sure the above mentioned situation doesn't trigger a PM busyloop. Signed-off-by: Nicolas Saenz Julienne --- Changes since v1: - Do not trigger clear based on PLS_MASK != XDEV_RESUME to avoid a potential race condition between the irq handler and hub thread. drivers/usb/host/xhci-hub.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 3abe70ff1b1e..05cd46a11c0c 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -1047,8 +1047,8 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd, xhci_get_usb2_port_status(port, , raw_port_status, flags); /* -* Clear stale usb2 resume signalling variables in case port changed -* state during resume signalling. For example on error +* Clear stale resume signalling variables in case port changed +* state during resume signalling. For example on error. */ if ((bus_state->resume_done[wIndex] || test_bit(wIndex, _state->resuming_ports)) && @@ -1057,6 +1057,9 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd, bus_state->resume_done[wIndex] = 0; clear_bit(wIndex, _state->resuming_ports); usb_hcd_end_port_resume(>self, wIndex); + } else if (bus_state->port_remote_wakeup & (1 << port->hcd_portnum) && + !(raw_port_status & PORT_CONNECT)) { + bus_state->port_remote_wakeup &= ~(1 << port->hcd_portnum); } if (bus_state->port_c_suspend & (1 << wIndex)) -- 2.22.0
Re: [PATCH v2] HID: input: fix a4tech horizontal wheel custom usage
On Fri, 2019-06-14 at 15:36 +0200, Benjamin Tissoires wrote: > Hi Wolfgang, > > On Thu, Jun 13, 2019 at 1:49 PM Wolfgang Bauer wrote: > > On Tuesday, 11. Juni 2019, 16:42:37 Benjamin Tissoires wrote: > > > On Tue, Jun 11, 2019 at 2:13 PM Nicolas Saenz Julienne > > > > > > wrote: > > > > NOTE: I CC'd Wolfgang as he's the one who can test this. > > > > > > I'll wait for Wolfram to confirm that the patch works before pushing then. > > > > My name is Wolfgang, not Wolfram... ;-) > > ouch, sorry for that (I am more used to talk to the I2C maintainer apparently) > > > But never mind. > > > > I tested the patch meanwhile on top of kernel 5.2.rc4, where the mouse wheel > > actually worked. > > Actually, I am a little bit lost here. > > The patch mentions a fix of c01908a14bf73, which is in 5.1 final. > So if your mouse works in 5.2.rc4, I am not sure how > HID-a4tech-fix-horizontal-scrolling.patch could break it. > > Could you be slightly more specific in what "works" and what doesn't? Hi Benjamin, First of all here's the descriptor: 0x05, 0x01, /* Usage Page (Desktop), */ 0x09, 0x02, /* Usage (Mouse), */ 0xA1, 0x01, /* Collection (Application), */ 0x09, 0x01, /* Usage (Pointer),*/ 0xA1, 0x00, /* Collection (Physical), */ 0x05, 0x09, /* Usage Page (Button),*/ 0x19, 0x01, /* Usage Minimum (01h),*/ 0x29, 0x08, /* Usage Maximum (08h),*/ 0x15, 0x00, /* Logical Minimum (0),*/ 0x25, 0x01, /* Logical Maximum (1),*/ 0x75, 0x01, /* Report Size (1),*/ 0x95, 0x08, /* Report Count (8), */ 0x81, 0x02, /* Input (Variable), */ 0x05, 0x01, /* Usage Page (Desktop), */ 0x09, 0x30, /* Usage (X), */ 0x09, 0x31, /* Usage (Y), */ 0x09, 0x38, /* Usage (Wheel), */ 0x09, 0xB8, /* Usage (B8h),*/ 0x15, 0x81, /* Logical Minimum (-127), */ 0x25, 0x7F, /* Logical Maximum (127), */ 0x75, 0x08, /* Report Size (8),*/ 0x95, 0x04, /* Report Count (4), */ 0x81, 0x06, /* Input (Variable, Relative), */ 0xC0, /* End Collection, */ 0xC0/* End Collection Sorry for the confusion, I'll try to explain the situation: In v5.2-rc4 without "HID-a4tech-fix-horizontal-scrolling.patch" the vertical wheel works out of luck as it's mapped to REL_WHEEL_HIGH_RES, which hid-a4tech ignores and lets hid-input process, the horizontal wheel is broken. On top of that Usage(0xB8) is also ignored by both hid-a4tech and hid-input as it isn't mapped to anything. There are two distinct bugs here: - High resolution wheel processing in hid-a4tech not being implemented, breaking horizontal wheels. - hid-a4tech not taking care of Usage(0xB8) correctly as it depended on it being mapped to "Rel.Misc". That behaviour changed in v5.1 with "HID: input: add mapping for "Toggle Display" key". Once high resolution wheel reports are fixed and handled in hid-a4tech's custom event, the mouse breaks as it's the processing of Usage(0xB8) that triggers the input_events, which is being ignored. You'll probably ask how come we didn't see this when "HID-a4tech-fix-horizontal-scrolling.patch" was merged. It's due to the fact it was tested on an older kernel, v5.0.15, that didn't contain "HID: input: add mapping for "Toggle Display" key"[1]. So that's why I added that specific fix tag. For LTS kernels, it is possible that "Toggle Display" support was back-ported but not the high resolution wheels support. Hope it made things more clear. Regards, Nicolas [1] https://lkml.kernel.org/lkml/nycvar.yfh.7.76.1906010028440.1...@cbobk.fhfr.pm/T/ > > Do we have the report descriptors available somewhere? > And if not, could you run hid-recorder from > https://gitlab.freedesktop.org/libevdev/hid-tools and attach the logs > when you move the horizontal wheel? > > Cheers, > Benjamin > > > As the patch didn't apply cleanly (it's obviously based upon > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=abf82e8f7e9af40a49e3d905187c662a43c96c8f , called > > "HID- > > a4tech-fix-horizontal-scrolling.patch" below), I added that patch as well. > > > > My results: > > kernel 5.2.rc4 works > > kernel 5.2.rc4 + HID-a4tech-fix-horizontal-scrolling.patch is broken > > kernel 5.2.rc4 + HID-a4tech-fix-horizontal-scrolling.patch + > > HID-input-fix-a4tech-horizontal-wheel-custom-usage.pat
Re: Regression post "HID: core: move Usage Page concatenation to Main item"
On Fri, 2019-06-14 at 10:52 +0200, Nicolas Saenz Julienne wrote: > On Fri, 2019-06-14 at 09:02 +0900, Jean-Baptiste Théou wrote: > > Sorry - Please find the public link: > > > > > https://android.googlesource.com/platform/cts/+/master/tests/tests/hardware/res/raw/asus_gamepad_register.json > > Best regards > > > > On Fri, Jun 14, 2019 at 9:01 AM Jean-Baptiste Théou > > wrote: > > > Hi, > > > > > > This patch (58e75155009cc85629955d3482f36a1e0eec) is triggering a > > > regression with the following descriptor (report not working as > > > expected) > > > > > > > https://partner-android.googlesource.com/platform/cts/+/refs/heads/q-fs-release/tests/tests/hardware/res/raw/asus_gamepad_register.json > > > Didn't see anything obviously wrong with this gamepad descriptor, so > > > not sure what's trigger the regression. > > > > > > Thanks a lot > > > > > > Best regards > > Added Benjamin to the mail thread. > > For the record here's the report descritor, I have the feeling the issue is > related to the Usage Page being defined in the middle of the Usage > ennumeration. > > 0x05, 0x01, /* Usage Page (Desktop), */ > 0x09, 0x05, /* Usage (Gamepad),*/ > 0xA1, 0x01, /* Collection (Application), */ > 0x85, 0x01, /* Report ID (1), */ > 0x05, 0x09, /* Usage Page (Button),*/ > 0x0A, 0x01, 0x00, /* Usage (01h),*/ > 0x0A, 0x02, 0x00, /* Usage (02h),*/ > 0x0A, 0x04, 0x00, /* Usage (04h),*/ > 0x0A, 0x05, 0x00, /* Usage (05h),*/ > 0x0A, 0x07, 0x00, /* Usage (07h),*/ > 0x0A, 0x08, 0x00, /* Usage (08h),*/ > 0x0A, 0x0E, 0x00, /* Usage (0Eh),*/ > 0x0A, 0x0F, 0x00, /* Usage (0Fh),*/ > 0x0A, 0x0D, 0x00, /* Usage (0Dh),*/ > 0x05, 0x0C, /* Usage Page (Consumer), */ > 0x0A, 0x24, 0x02, /* Usage (AC Back),*/ > 0x0A, 0x23, 0x02, /* Usage (AC Home),*/ > 0x15, 0x00, /* Logical Minimum (0),*/ > 0x25, 0x01, /* Logical Maximum (1),*/ > 0x75, 0x01, /* Report Size (1),*/ > 0x95, 0x0B, /* Report Count (11), */ > 0x81, 0x02, /* Input (Variable), */ > 0x75, 0x01, /* Report Size (1),*/ > 0x95, 0x01, /* Report Count (1), */ > 0x81, 0x03, /* Input (Constant, Variable), */ > 0x05, 0x01, /* Usage Page (Desktop), */ > 0x75, 0x04, /* Report Size (4),*/ > 0x95, 0x01, /* Report Count (1), */ > 0x25, 0x07, /* Logical Maximum (7),*/ > 0x46, 0x3B, 0x01, /* Physical Maximum (315), */ > 0x66, 0x14, 0x00, /* Unit (Degrees), */ > 0x09, 0x39, /* Usage (Hat Switch), */ > 0x81, 0x42, /* Input (Variable, Null State), */ > 0x66, 0x00, 0x00, /* Unit, */ > 0x09, 0x01, /* Usage (Pointer),*/ > 0xA1, 0x00, /* Collection (Physical), */ > 0x09, 0x30, /* Usage (X), */ > 0x09, 0x31, /* Usage (Y), */ > 0x09, 0x32, /* Usage (Z), */ > 0x09, 0x35, /* Usage (Rz), */ > 0x05, 0x02, /* Usage Page (Simulation),*/ > 0x09, 0xC5, /* Usage (C5h),*/ > 0x09, 0xC4, /* Usage (C4h),*/ > 0x15, 0x00, /* Logical Minimum (0),*/ > 0x26, 0xFF, 0x00, /* Logical Maximum (255), */ > 0x35, 0x00, /* Physical Minimum (0), */ > 0x46, 0xFF, 0x00, /* Physical Maximum (255), */ > 0x75, 0x08, /* Report Size (8),*/ > 0x95, 0x06, /* Report Count (6), */ > 0x81, 0x02, /* Input (Variable), */ Well as it was stated in 57e75155009 ("HID: core: move Usage Page concatenation to Main item"): The spec is not pretty clear as 5.2.2.7 states "Any usage that follows is interpreted as a Usage ID and concatenated with the Usage Page". While 5.2.2.8 states "When the parser encounte
Re: Regression post "HID: core: move Usage Page concatenation to Main item"
On Fri, 2019-06-14 at 09:02 +0900, Jean-Baptiste Théou wrote: > Sorry - Please find the public link: > > https://android.googlesource.com/platform/cts/+/master/tests/tests/hardware/res/raw/asus_gamepad_register.json > > Best regards > > On Fri, Jun 14, 2019 at 9:01 AM Jean-Baptiste Théou > wrote: > > Hi, > > > > This patch (58e75155009cc85629955d3482f36a1e0eec) is triggering a > > regression with the following descriptor (report not working as > > expected) > > > > https://partner-android.googlesource.com/platform/cts/+/refs/heads/q-fs-release/tests/tests/hardware/res/raw/asus_gamepad_register.json > > > > Didn't see anything obviously wrong with this gamepad descriptor, so > > not sure what's trigger the regression. > > > > Thanks a lot > > > > Best regards Added Benjamin to the mail thread. For the record here's the report descritor, I have the feeling the issue is related to the Usage Page being defined in the middle of the Usage ennumeration. 0x05, 0x01, /* Usage Page (Desktop), */ 0x09, 0x05, /* Usage (Gamepad),*/ 0xA1, 0x01, /* Collection (Application), */ 0x85, 0x01, /* Report ID (1), */ 0x05, 0x09, /* Usage Page (Button),*/ 0x0A, 0x01, 0x00, /* Usage (01h),*/ 0x0A, 0x02, 0x00, /* Usage (02h),*/ 0x0A, 0x04, 0x00, /* Usage (04h),*/ 0x0A, 0x05, 0x00, /* Usage (05h),*/ 0x0A, 0x07, 0x00, /* Usage (07h),*/ 0x0A, 0x08, 0x00, /* Usage (08h),*/ 0x0A, 0x0E, 0x00, /* Usage (0Eh),*/ 0x0A, 0x0F, 0x00, /* Usage (0Fh),*/ 0x0A, 0x0D, 0x00, /* Usage (0Dh),*/ 0x05, 0x0C, /* Usage Page (Consumer), */ 0x0A, 0x24, 0x02, /* Usage (AC Back),*/ 0x0A, 0x23, 0x02, /* Usage (AC Home),*/ 0x15, 0x00, /* Logical Minimum (0),*/ 0x25, 0x01, /* Logical Maximum (1),*/ 0x75, 0x01, /* Report Size (1),*/ 0x95, 0x0B, /* Report Count (11), */ 0x81, 0x02, /* Input (Variable), */ 0x75, 0x01, /* Report Size (1),*/ 0x95, 0x01, /* Report Count (1), */ 0x81, 0x03, /* Input (Constant, Variable), */ 0x05, 0x01, /* Usage Page (Desktop), */ 0x75, 0x04, /* Report Size (4),*/ 0x95, 0x01, /* Report Count (1), */ 0x25, 0x07, /* Logical Maximum (7),*/ 0x46, 0x3B, 0x01, /* Physical Maximum (315), */ 0x66, 0x14, 0x00, /* Unit (Degrees), */ 0x09, 0x39, /* Usage (Hat Switch), */ 0x81, 0x42, /* Input (Variable, Null State), */ 0x66, 0x00, 0x00, /* Unit, */ 0x09, 0x01, /* Usage (Pointer),*/ 0xA1, 0x00, /* Collection (Physical), */ 0x09, 0x30, /* Usage (X), */ 0x09, 0x31, /* Usage (Y), */ 0x09, 0x32, /* Usage (Z), */ 0x09, 0x35, /* Usage (Rz), */ 0x05, 0x02, /* Usage Page (Simulation),*/ 0x09, 0xC5, /* Usage (C5h),*/ 0x09, 0xC4, /* Usage (C4h),*/ 0x15, 0x00, /* Logical Minimum (0),*/ 0x26, 0xFF, 0x00, /* Logical Maximum (255), */ 0x35, 0x00, /* Physical Minimum (0), */ 0x46, 0xFF, 0x00, /* Physical Maximum (255), */ 0x75, 0x08, /* Report Size (8),*/ 0x95, 0x06, /* Report Count (6), */ 0x81, 0x02, /* Input (Variable), */ 0xC0, /* End Collection, */ 0x85, 0x02, /* Report ID (2), */ 0x05, 0x08, /* Usage Page (LED), */ 0x0A, 0x01, 0x00, /* Usage (01h),*/ 0x0A, 0x02, 0x00, /* Usage (02h),*/ 0x0A, 0x03, 0x00, /* Usage (03h),*/ 0x0A, 0x04, 0x00, /* Usage (04h),*/ 0x15, 0x00, /* Logical Minimum (0),*/ 0x25, 0x01, /* Logical Maximum (1),*/ 0x75, 0x01, /* Report Size (1),*/ 0x95, 0x04, /* Report Count (4), */ 0x91, 0x02, /* Output (Variable), */ 0x75, 0x04, /* Report Size (4),*/ 0x95, 0x01, /* Report Count (1),
Re: Regression post "HID: core: move Usage Page concatenation to Main item"
On Fri, 2019-06-14 at 09:01 +0900, Jean-Baptiste Théou wrote: > Hi, > > This patch (58e75155009cc85629955d3482f36a1e0eec) is triggering a > regression with the following descriptor (report not working as > expected) > > https://partner-android.googlesource.com/platform/cts/+/refs/heads/q-fs-release/tests/tests/hardware/res/raw/asus_gamepad_register.json > > Didn't see anything obviously wrong with this gamepad descriptor, so > not sure what's trigger the regression. > I'll have a look at it. Do you have any more information on the regression? What exactly isn't working? Regards, Nicolas signature.asc Description: This is a digitally signed message part
[PATCH v4 4/7] cpufreq: add driver for Raspberry Pi
Raspberry Pi's firmware offers and interface though which update it's performance requirements. It allows us to request for specific runtime frequencies, which the firmware might or might not respect, depending on the firmware configuration and thermals. As the maximum and minimum frequencies are configurable in the firmware there is no way to know in advance their values. So the Raspberry Pi cpufreq driver queries them, builds an opp frequency table to then launch cpufreq-dt. Also, as the firmware interface might be configured as a module, making the cpu clock unavailable during init, this implements a full fledged driver, as opposed to most drivers registering cpufreq-dt, which only make use of an init routine. Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt Reviewed-by: Stephen Boyd --- Changes since v3: - Fix spelling error in commit's subject Changes since v2: - Round OPP tables Changes since v1: - Remove compatible checks - Add module support, now full fledged driver - Use NULL in clk_get() drivers/cpufreq/Kconfig.arm | 8 +++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/raspberrypi-cpufreq.c | 97 +++ 3 files changed, 106 insertions(+) create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 6f65b7f05496..56c31a78c692 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -142,6 +142,14 @@ config ARM_QCOM_CPUFREQ_HW The driver implements the cpufreq interface for this HW engine. Say Y if you want to support CPUFreq HW. +config ARM_RASPBERRYPI_CPUFREQ + tristate "Raspberry Pi cpufreq support" + depends on CLK_RASPBERRYPI || COMPILE_TEST + help + This adds the CPUFreq driver for Raspberry Pi + + If in doubt, say N. + config ARM_S3C_CPUFREQ bool help diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index 7bcda2273d0c..5a6c70d26c98 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -65,6 +65,7 @@ obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)+= qcom-cpufreq-kryo.o +obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ) += raspberrypi-cpufreq.o obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o obj-$(CONFIG_ARM_S3C2416_CPUFREQ) += s3c2416-cpufreq.o diff --git a/drivers/cpufreq/raspberrypi-cpufreq.c b/drivers/cpufreq/raspberrypi-cpufreq.c new file mode 100644 index ..2bc7d9734272 --- /dev/null +++ b/drivers/cpufreq/raspberrypi-cpufreq.c @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Raspberry Pi cpufreq driver + * + * Copyright (C) 2019, Nicolas Saenz Julienne + */ + +#include +#include +#include +#include +#include +#include + +#define RASPBERRYPI_FREQ_INTERVAL 1 + +static struct platform_device *cpufreq_dt; + +static int raspberrypi_cpufreq_probe(struct platform_device *pdev) +{ + struct device *cpu_dev; + unsigned long min, max; + unsigned long rate; + struct clk *clk; + int ret; + + cpu_dev = get_cpu_device(0); + if (!cpu_dev) { + pr_err("Cannot get CPU for cpufreq driver\n"); + return -ENODEV; + } + + clk = clk_get(cpu_dev, NULL); + if (IS_ERR(clk)) { + dev_err(cpu_dev, "Cannot get clock for CPU0\n"); + return PTR_ERR(clk); + } + + /* +* The max and min frequencies are configurable in the Raspberry Pi +* firmware, so we query them at runtime. +*/ + min = roundup(clk_round_rate(clk, 0), RASPBERRYPI_FREQ_INTERVAL); + max = roundup(clk_round_rate(clk, ULONG_MAX), RASPBERRYPI_FREQ_INTERVAL); + clk_put(clk); + + for (rate = min; rate <= max; rate += RASPBERRYPI_FREQ_INTERVAL) { + ret = dev_pm_opp_add(cpu_dev, rate, 0); + if (ret) + goto remove_opp; + } + + cpufreq_dt = platform_device_register_simple("cpufreq-dt", -1, NULL, 0); + ret = PTR_ERR_OR_ZERO(cpufreq_dt); + if (ret) { + dev_err(cpu_dev, "Failed to create platform device, %d\n", ret); + goto remove_opp; + } + + return 0; + +remove_opp: + dev_pm_opp_remove_all_dynamic(cpu_dev); + + return ret; +} + +static int raspberrypi_cpufreq_remove(struct platform_device *pdev) +{ + struct device *cpu_dev; + + cpu_dev = get_cpu_device(0); + if (cpu_dev) + dev_pm_opp_remove_all_dynamic(cpu_dev); + + platform_device_unregister(cpufreq_dt); + + return 0; +} + +/* + * Since the driver depends on cl
[PATCH v4 3/7] firmware: raspberrypi: register clk device
Since clk-raspberrypi is tied to the VC4 firmware instead of particular hardware it's registration should be performed by the firmware driver. Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt --- drivers/firmware/raspberrypi.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c index 61be15d9df7d..da26a584dca0 100644 --- a/drivers/firmware/raspberrypi.c +++ b/drivers/firmware/raspberrypi.c @@ -20,6 +20,7 @@ #define MBOX_CHAN_PROPERTY 8 static struct platform_device *rpi_hwmon; +static struct platform_device *rpi_clk; struct rpi_firmware { struct mbox_client cl; @@ -207,6 +208,12 @@ rpi_register_hwmon_driver(struct device *dev, struct rpi_firmware *fw) -1, NULL, 0); } +static void rpi_register_clk_driver(struct device *dev) +{ + rpi_clk = platform_device_register_data(dev, "raspberrypi-clk", + -1, NULL, 0); +} + static int rpi_firmware_probe(struct platform_device *pdev) { struct device *dev = >dev; @@ -234,6 +241,7 @@ static int rpi_firmware_probe(struct platform_device *pdev) rpi_firmware_print_firmware_revision(fw); rpi_register_hwmon_driver(dev, fw); + rpi_register_clk_driver(dev); return 0; } @@ -254,6 +262,8 @@ static int rpi_firmware_remove(struct platform_device *pdev) platform_device_unregister(rpi_hwmon); rpi_hwmon = NULL; + platform_device_unregister(rpi_clk); + rpi_clk = NULL; mbox_free_channel(fw->chan); return 0; -- 2.21.0
[PATCH v4 5/7] clk: raspberrypi: register platform device for raspberrypi-cpufreq
As 'clk-raspberrypi' depends on RPi's firmware interface, which might be configured as a module, the cpu clock might not be available for the cpufreq driver during it's init process. So we register the 'raspberrypi-cpufreq' platform device after the probe sequence succeeds. Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt --- Changes since v2: - Use raspberrypi_clk struct to store cpufreq platform_device drivers/clk/bcm/clk-raspberrypi.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c index fef1f7caee4f..1654fd0eedc9 100644 --- a/drivers/clk/bcm/clk-raspberrypi.c +++ b/drivers/clk/bcm/clk-raspberrypi.c @@ -34,6 +34,7 @@ struct raspberrypi_clk { struct device *dev; struct rpi_firmware *firmware; + struct platform_device *cpufreq; unsigned long min_rate; unsigned long max_rate; @@ -272,6 +273,7 @@ static int raspberrypi_clk_probe(struct platform_device *pdev) rpi->dev = dev; rpi->firmware = firmware; + platform_set_drvdata(pdev, rpi); ret = raspberrypi_register_pllb(rpi); if (ret) { @@ -283,6 +285,18 @@ static int raspberrypi_clk_probe(struct platform_device *pdev) if (ret) return ret; + rpi->cpufreq = platform_device_register_data(dev, "raspberrypi-cpufreq", +-1, NULL, 0); + + return 0; +} + +static int raspberrypi_clk_remove(struct platform_device *pdev) +{ + struct raspberrypi_clk *rpi = platform_get_drvdata(pdev); + + platform_device_unregister(rpi->cpufreq); + return 0; } @@ -291,6 +305,7 @@ static struct platform_driver raspberrypi_clk_driver = { .name = "raspberrypi-clk", }, .probe = raspberrypi_clk_probe, + .remove = raspberrypi_clk_remove, }; module_platform_driver(raspberrypi_clk_driver); -- 2.21.0
[PATCH v4 6/7] ARM: defconfig: enable cpufreq driver for RPi
This enables on both multi_v7_defconfig and bcm2835_defconfig the new firmware based clock and cpufreq drivers for the Raspberry Pi platform. In the case of bcm2835_defconfig, as the cpufreq subsystem was disabled, the conservative governor was selected as default since it better handles the high frequency transition latency. Signed-off-by: Nicolas Saenz Julienne Acked-by: Stefan Wahren --- Changes since v2: - Change default governor to conservative in bcm2835_defconfig - Set all as builtin in bcm2835_defconfig arch/arm/configs/bcm2835_defconfig | 9 + arch/arm/configs/multi_v7_defconfig | 2 ++ 2 files changed, 11 insertions(+) diff --git a/arch/arm/configs/bcm2835_defconfig b/arch/arm/configs/bcm2835_defconfig index dcf7610cfe55..519ff58e67b3 100644 --- a/arch/arm/configs/bcm2835_defconfig +++ b/arch/arm/configs/bcm2835_defconfig @@ -37,6 +37,14 @@ CONFIG_CMA=y CONFIG_SECCOMP=y CONFIG_KEXEC=y CONFIG_CRASH_DUMP=y +CONFIG_CPU_FREQ=y +CONFIG_CPU_FREQ_STAT=y +CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE=y +CONFIG_CPU_FREQ_GOV_POWERSAVE=y +CONFIG_CPU_FREQ_GOV_USERSPACE=y +CONFIG_CPU_FREQ_GOV_ONDEMAND=y +CONFIG_CPUFREQ_DT=y +CONFIG_ARM_RASPBERRYPI_CPUFREQ=y CONFIG_VFP=y # CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set # CONFIG_SUSPEND is not set @@ -132,6 +140,7 @@ CONFIG_DMA_BCM2835=y CONFIG_STAGING=y CONFIG_SND_BCM2835=m CONFIG_VIDEO_BCM2835=m +CONFIG_CLK_RASPBERRYPI=y CONFIG_MAILBOX=y CONFIG_BCM2835_MBOX=y # CONFIG_IOMMU_SUPPORT is not set diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index 6b748f214eae..0fd60a83f768 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -102,6 +102,7 @@ CONFIG_CPU_FREQ_GOV_CONSERVATIVE=m CONFIG_CPU_FREQ_GOV_SCHEDUTIL=y CONFIG_CPUFREQ_DT=y CONFIG_ARM_IMX6Q_CPUFREQ=y +CONFIG_ARM_RASPBERRYPI_CPUFREQ=y CONFIG_QORIQ_CPUFREQ=y CONFIG_CPU_IDLE=y CONFIG_ARM_CPUIDLE=y @@ -899,6 +900,7 @@ CONFIG_STAGING_BOARD=y CONFIG_COMMON_CLK_MAX77686=y CONFIG_COMMON_CLK_RK808=m CONFIG_COMMON_CLK_S2MPS11=m +CONFIG_CLK_RASPBERRYPI=y CONFIG_COMMON_CLK_QCOM=y CONFIG_QCOM_CLK_RPM=y CONFIG_APQ_MMCC_8084=y -- 2.21.0
[PATCH v4 2/7] clk: bcm283x: add driver interfacing with Raspberry Pi's firmware
Raspberry Pi's firmware offers an interface though which update it's clock's frequencies. This is specially useful in order to change the CPU clock (pllb_arm) which is 'owned' by the firmware and we're unable to scale using the register interface provided by clk-bcm2835. Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt --- Changes since v3: - Fix sparse warnings, use le32_to_cpu to interface with firmware - Correct extra 0 in RPI_FIRMWARE_ARM_CLK_ID Changes since v2: - Remove redundant error message - Rebase to linux next - Fix spelling - Fix checkpatch.pl errors Changes since v1: - Use BIT() - Add Kconfig entry, with compile test - remove prepare/unprepare - Fix uninitialized init.name in pllb registration - Add MODULE_ALIAS() - Use determine_rate() instead of round_rate() - Add small introduction explaining need for driver drivers/clk/bcm/Kconfig | 7 + drivers/clk/bcm/Makefile | 1 + drivers/clk/bcm/clk-raspberrypi.c | 300 ++ 3 files changed, 308 insertions(+) create mode 100644 drivers/clk/bcm/clk-raspberrypi.c diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig index 766a838ad9af..16e508eba6e5 100644 --- a/drivers/clk/bcm/Kconfig +++ b/drivers/clk/bcm/Kconfig @@ -74,3 +74,10 @@ config CLK_BCM_SR default ARCH_BCM_IPROC help Enable common clock framework support for the Broadcom Stingray SoC + +config CLK_RASPBERRYPI + tristate "Raspberry Pi firmware based clock support" + depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE) + help + Enable common clock framework support for Raspberry Pi's firmware + dependent clocks diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile index e924f25bc6c8..004e9526d6f6 100644 --- a/drivers/clk/bcm/Makefile +++ b/drivers/clk/bcm/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm21664.o obj-$(CONFIG_COMMON_CLK_IPROC) += clk-iproc-armpll.o clk-iproc-pll.o clk-iproc-asiu.o obj-$(CONFIG_CLK_BCM2835) += clk-bcm2835.o obj-$(CONFIG_CLK_BCM2835) += clk-bcm2835-aux.o +obj-$(CONFIG_CLK_RASPBERRYPI) += clk-raspberrypi.o obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o obj-$(CONFIG_CLK_BCM_CYGNUS) += clk-cygnus.o obj-$(CONFIG_CLK_BCM_HR2) += clk-hr2.o diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c new file mode 100644 index ..fef1f7caee4f --- /dev/null +++ b/drivers/clk/bcm/clk-raspberrypi.c @@ -0,0 +1,300 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Raspberry Pi driver for firmware controlled clocks + * + * Even though clk-bcm2835 provides an interface to the hardware registers for + * the system clocks we've had to factor out 'pllb' as the firmware 'owns' it. + * We're not allowed to change it directly as we might race with the + * over-temperature and under-voltage protections provided by the firmware. + * + * Copyright (C) 2019 Nicolas Saenz Julienne + */ + +#include +#include +#include +#include +#include + +#include + +#define RPI_FIRMWARE_ARM_CLK_ID0x0003 + +#define RPI_FIRMWARE_STATE_ENABLE_BIT BIT(0) +#define RPI_FIRMWARE_STATE_WAIT_BITBIT(1) + +/* + * Even though the firmware interface alters 'pllb' the frequencies are + * provided as per 'pllb_arm'. We need to scale before passing them trough. + */ +#define RPI_FIRMWARE_PLLB_ARM_DIV_RATE 2 + +#define A2W_PLL_FRAC_BITS 20 + +struct raspberrypi_clk { + struct device *dev; + struct rpi_firmware *firmware; + + unsigned long min_rate; + unsigned long max_rate; + + struct clk_hw pllb; + struct clk_hw *pllb_arm; + struct clk_lookup *pllb_arm_lookup; +}; + +/* + * Structure of the message passed to Raspberry Pi's firmware in order to + * change clock rates. The 'disable_turbo' option is only available to the ARM + * clock (pllb) which we enable by default as turbo mode will alter multiple + * clocks at once. + * + * Even though we're able to access the clock registers directly we're bound to + * use the firmware interface as the firmware ultimately takes care of + * mitigating overheating/undervoltage situations and we would be changing + * frequencies behind his back. + * + * For more information on the firmware interface check: + * https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface + */ +struct raspberrypi_firmware_prop { + __le32 id; + __le32 val; + __le32 disable_turbo; +} __packed; + +static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 tag, + u32 clk, u32 *val) +{ + struct raspberrypi_firmware_prop msg = { + .id = cpu_to_le32(clk), + .val = cpu_to_le32(*val), + .disable_turbo = cpu_to_le32(1), + }; + int ret; + + ret = rpi_firmware_property(firmware,
[PATCH v4 7/7] arm64: defconfig: enable cpufreq support for RPi3
This enables both the new firmware clock driver and cpufreq driver available for the RPi3 family of boards. Signed-off-by: Nicolas Saenz Julienne Acked-by: Stefan Wahren --- Changes since v2: - Build both drivers as modules arch/arm64/configs/defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 5a8e853833cf..5e322e61b101 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -83,6 +83,7 @@ CONFIG_ACPI_CPPC_CPUFREQ=m CONFIG_ARM_ARMADA_37XX_CPUFREQ=y CONFIG_ARM_SCPI_CPUFREQ=y CONFIG_ARM_IMX_CPUFREQ_DT=m +CONFIG_ARM_RASPBERRYPI_CPUFREQ=m CONFIG_ARM_TEGRA186_CPUFREQ=y CONFIG_ARM_SCPI_PROTOCOL=y CONFIG_RASPBERRYPI_FIRMWARE=y @@ -653,6 +654,7 @@ CONFIG_COMMON_CLK_CS2000_CP=y CONFIG_COMMON_CLK_S2MPS11=y CONFIG_CLK_QORIQ=y CONFIG_COMMON_CLK_PWM=y +CONFIG_CLK_RASPBERRYPI=m CONFIG_CLK_IMX8MM=y CONFIG_CLK_IMX8MQ=y CONFIG_CLK_IMX8QXP=y -- 2.21.0
[PATCH v4 0/7] cpufreq support for Raspberry Pi
Hi all, this aims at adding cpufreq support to the Raspberry Pi family of boards. The series first factors out 'pllb' from clk-bcm2385 and creates a new clk driver that operates it over RPi's firmware interface[1]. We are forced to do so as the firmware 'owns' the pll and we're not allowed to change through the register interface directly as we might race with the over-temperature and under-voltage protections provided by the firmware. Next it creates a minimal cpufreq driver that populates the CPU's opp table, and registers cpufreq-dt. Which is needed as the firmware controls the max and min frequencies available. This was tested on a RPi3b+ and RPI2b, both using multi_v7_defconfig and arm64's defconfig. That's all, kind regards, Nicolas [1] https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface --- Changes since v3: - Fix sparse warnings in clk-raspberrypi.c - Minor cleanups Changes since v2: - Fixed configs to match Stefan's comments - Round OPP frequencies - Rebase onto linux-next - Minor cleanups & checkpatch.pl Changes since v1: - Enabled by default on the whole family of devices - Added/Fixed module support - clk device now registered by firmware driver - raspberrypi-cpufreq device now registered by clk driver - Reimplemented clk rounding unsing determine_rate() - Enabled in configs for arm and arm64 Changes since RFC: - Move firmware clk device into own driver Nicolas Saenz Julienne (7): clk: bcm2835: remove pllb clk: bcm283x: add driver interfacing with Raspberry Pi's firmware firmware: raspberrypi: register clk device cpufreq: add driver for Raspberry Pi clk: raspberrypi: register platform device for raspberrypi-cpufreq ARM: defconfig: enable cpufreq driver for RPi arm64: defconfig: enable cpufreq support for RPi3 arch/arm/configs/bcm2835_defconfig| 9 + arch/arm/configs/multi_v7_defconfig | 2 + arch/arm64/configs/defconfig | 2 + drivers/clk/bcm/Kconfig | 7 + drivers/clk/bcm/Makefile | 1 + drivers/clk/bcm/clk-bcm2835.c | 28 +-- drivers/clk/bcm/clk-raspberrypi.c | 315 ++ drivers/cpufreq/Kconfig.arm | 8 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/raspberrypi-cpufreq.c | 97 drivers/firmware/raspberrypi.c| 10 + 11 files changed, 456 insertions(+), 24 deletions(-) create mode 100644 drivers/clk/bcm/clk-raspberrypi.c create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c -- 2.21.0
[PATCH v4 1/7] clk: bcm2835: remove pllb
Raspberry Pi's firmware controls this pll, we should use the firmware interface to access it. Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt --- Changes since v1: - Add comment to explain why pllb isn't there anymore drivers/clk/bcm/clk-bcm2835.c | 28 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index 770bb01f523e..867ae3c20041 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -1651,30 +1651,10 @@ static const struct bcm2835_clk_desc clk_desc_array[] = { .fixed_divider = 1, .flags = CLK_SET_RATE_PARENT), - /* PLLB is used for the ARM's clock. */ - [BCM2835_PLLB] = REGISTER_PLL( - .name = "pllb", - .cm_ctrl_reg = CM_PLLB, - .a2w_ctrl_reg = A2W_PLLB_CTRL, - .frac_reg = A2W_PLLB_FRAC, - .ana_reg_base = A2W_PLLB_ANA0, - .reference_enable_mask = A2W_XOSC_CTRL_PLLB_ENABLE, - .lock_mask = CM_LOCK_FLOCKB, - - .ana = _ana_default, - - .min_rate = 6u, - .max_rate = 30u, - .max_fb_rate = BCM2835_MAX_FB_RATE), - [BCM2835_PLLB_ARM] = REGISTER_PLL_DIV( - .name = "pllb_arm", - .source_pll = "pllb", - .cm_reg = CM_PLLB, - .a2w_reg = A2W_PLLB_ARM, - .load_mask = CM_PLLB_LOADARM, - .hold_mask = CM_PLLB_HOLDARM, - .fixed_divider = 1, - .flags = CLK_SET_RATE_PARENT), + /* +* PLLB is used for the ARM's clock. Controlled by firmware, see +* clk-raspberrypi.c. +*/ /* * PLLC is the core PLL, used to drive the core VPU clock. -- 2.21.0
Re: [PATCH v2 1/2] net: ethernet: wiznet: w5X00 add device tree support
On Wed, 2019-06-12 at 09:52 -0700, David Miller wrote: > From: Nicolas Saenz Julienne > Date: Wed, 12 Jun 2019 14:25:25 +0200 > > > The w5X00 chip provides an SPI to Ethernet inteface. This patch allows > > platform devices to be defined through the device tree. > > > > Signed-off-by: Nicolas Saenz Julienne > > Applied to net-next. Thanks! signature.asc Description: This is a digitally signed message part
[PATCH v2 2/2] dt-bindings: net: wiznet: add w5x00 support
Add bindings for Wiznet's w5x00 series of SPI interfaced Ethernet chips. Based on the bindings for microchip,enc28j60. Signed-off-by: Nicolas Saenz Julienne --- Changes since v1: - one compatible sting per line - use correct 'ethenet@0' phandle name .../devicetree/bindings/net/wiznet,w5x00.txt | 51 +++ 1 file changed, 51 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/wiznet,w5x00.txt diff --git a/Documentation/devicetree/bindings/net/wiznet,w5x00.txt b/Documentation/devicetree/bindings/net/wiznet,w5x00.txt new file mode 100644 index ..901946dea3de --- /dev/null +++ b/Documentation/devicetree/bindings/net/wiznet,w5x00.txt @@ -0,0 +1,51 @@ +* Wiznet w5x00 + +This is a standalone 10/100 MBit Ethernet controller with SPI interface. + +For each device connected to a SPI bus, define a child node within +the SPI master node. + +Required properties: +- compatible: Should be one of the following strings: + "wiznet,w5100" + "wiznet,w5200" + "wiznet,w5500" +- reg: Specify the SPI chip select the chip is wired to. +- interrupts: Specify the interrupt index within the interrupt controller (referred + to above in interrupt-parent) and interrupt type. w5x00 natively + generates falling edge interrupts, however, additional board logic + might invert the signal. +- pinctrl-names: List of assigned state names, see pinctrl binding documentation. +- pinctrl-0: List of phandles to configure the GPIO pin used as interrupt line, + see also generic and your platform specific pinctrl binding + documentation. + +Optional properties: +- spi-max-frequency: Maximum frequency of the SPI bus when accessing the w5500. + According to the w5500 datasheet, the chip allows a maximum of 80 MHz, however, + board designs may need to limit this value. +- local-mac-address: See ethernet.txt in the same directory. + + +Example (for Raspberry Pi with pin control stuff for GPIO irq): + + { + ethernet@0: w5500@0 { + compatible = "wiznet,w5500"; + reg = <0>; + pinctrl-names = "default"; + pinctrl-0 = <_pins>; + interrupt-parent = <>; + interrupts = <25 IRQ_TYPE_EDGE_FALLING>; + spi-max-frequency = <3000>; + }; +}; + + { + eth1_pins: eth1_pins { + brcm,pins = <25>; + brcm,function = <0>; /* in */ + brcm,pull = <0>; /* none */ + }; +}; + -- 2.21.0
[PATCH v2 1/2] net: ethernet: wiznet: w5X00 add device tree support
The w5X00 chip provides an SPI to Ethernet inteface. This patch allows platform devices to be defined through the device tree. Signed-off-by: Nicolas Saenz Julienne --- drivers/net/ethernet/wiznet/w5100-spi.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/wiznet/w5100-spi.c b/drivers/net/ethernet/wiznet/w5100-spi.c index 918b3e50850a..2b4126d2427d 100644 --- a/drivers/net/ethernet/wiznet/w5100-spi.c +++ b/drivers/net/ethernet/wiznet/w5100-spi.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include "w5100.h" @@ -409,14 +410,32 @@ static const struct w5100_ops w5500_ops = { .init = w5500_spi_init, }; +static const struct of_device_id w5100_of_match[] = { + { .compatible = "wiznet,w5100", .data = (const void*)W5100, }, + { .compatible = "wiznet,w5200", .data = (const void*)W5200, }, + { .compatible = "wiznet,w5500", .data = (const void*)W5500, }, + { }, +}; +MODULE_DEVICE_TABLE(of, w5100_of_match); + static int w5100_spi_probe(struct spi_device *spi) { - const struct spi_device_id *id = spi_get_device_id(spi); + const struct of_device_id *of_id; const struct w5100_ops *ops; + kernel_ulong_t driver_data; int priv_size; const void *mac = of_get_mac_address(spi->dev.of_node); - switch (id->driver_data) { + if (spi->dev.of_node) { + of_id = of_match_device(w5100_of_match, >dev); + if (!of_id) + return -ENODEV; + driver_data = (kernel_ulong_t)of_id->data; + } else { + driver_data = spi_get_device_id(spi)->driver_data; + } + + switch (driver_data) { case W5100: ops = _spi_ops; priv_size = 0; @@ -453,6 +472,7 @@ static struct spi_driver w5100_spi_driver = { .driver = { .name = "w5100", .pm = _pm_ops, + .of_match_table = w5100_of_match, }, .probe = w5100_spi_probe, .remove = w5100_spi_remove, -- 2.21.0
Re: [PATCH 1/2] input: edt-ft5x06 - add polled input support
On Tue, 2019-04-30 at 20:58 +0200, Nicolas Saenz Julienne wrote: > Some hardware configurations might pass on providing an interrupt line. > In that case there is always the option to use a polled input approach. > This patch adapts the driver for it. > > The polled approach is only triggered if no interrupt is provided by the > firmware or platform data. > > Signed-off-by: Nicolas Saenz Julienne > --- Ping :) > drivers/input/touchscreen/edt-ft5x06.c | 100 ++--- > 1 file changed, 72 insertions(+), 28 deletions(-) > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c > b/drivers/input/touchscreen/edt-ft5x06.c > index 702bfda7ee77..e58645c72c2f 100644 > --- a/drivers/input/touchscreen/edt-ft5x06.c > +++ b/drivers/input/touchscreen/edt-ft5x06.c > @@ -39,6 +39,7 @@ > #include > #include > #include > +#include > #include > > #define WORK_REGISTER_THRESHOLD 0x00 > @@ -97,6 +98,7 @@ struct edt_reg_addr { > struct edt_ft5x06_ts_data { > struct i2c_client *client; > struct input_dev *input; > + struct input_polled_dev *poll_dev; > struct touchscreen_properties prop; > u16 num_x; > u16 num_y; > @@ -181,9 +183,8 @@ static bool edt_ft5x06_ts_check_crc(struct > edt_ft5x06_ts_data *tsdata, > return true; > } > > -static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id) > +static void edt_ft5x06_process(struct edt_ft5x06_ts_data *tsdata) > { > - struct edt_ft5x06_ts_data *tsdata = dev_id; > struct device *dev = >client->dev; > u8 cmd; > u8 rdbuf[63]; > @@ -210,7 +211,7 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void > *dev_id) > break; > > default: > - goto out; > + return; > } > > memset(rdbuf, 0, sizeof(rdbuf)); > @@ -222,7 +223,7 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void > *dev_id) > if (error) { > dev_err_ratelimited(dev, "Unable to fetch data, error: %d\n", > error); > - goto out; > + return; > } > > /* M09/M12 does not send header or CRC */ > @@ -232,11 +233,11 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void > *dev_id) > dev_err_ratelimited(dev, > "Unexpected header: %02x%02x%02x!\n", > rdbuf[0], rdbuf[1], rdbuf[2]); > - goto out; > + return; > } > > if (!edt_ft5x06_ts_check_crc(tsdata, rdbuf, datalen)) > - goto out; > + return; > } > > for (i = 0; i < tsdata->max_support_points; i++) { > @@ -273,11 +274,23 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void > *dev_id) > > input_mt_report_pointer_emulation(tsdata->input, true); > input_sync(tsdata->input); > +} > > -out: > +static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id) > +{ > + struct edt_ft5x06_ts_data *tsdata = dev_id; > + > + edt_ft5x06_process(tsdata); > return IRQ_HANDLED; > } > > +static void edt_ft5x06_poll(struct input_polled_dev *dev) > +{ > + struct edt_ft5x06_ts_data *tsdata = dev->private; > + > + edt_ft5x06_process(tsdata); > +} > + > static int edt_ft5x06_register_write(struct edt_ft5x06_ts_data *tsdata, >u8 addr, u8 value) > { > @@ -1059,7 +1072,9 @@ static int edt_ft5x06_ts_probe(struct i2c_client > *client, >const struct i2c_device_id *id) > { > const struct edt_i2c_chip_data *chip_data; > + struct input_polled_dev *poll_dev = NULL; > struct edt_ft5x06_ts_data *tsdata; > + bool polled = !(client->irq); > struct input_dev *input; > unsigned long irq_flags; > int error; > @@ -1112,15 +1127,38 @@ static int edt_ft5x06_ts_probe(struct i2c_client > *client, > msleep(300); > } > > - input = devm_input_allocate_device(>dev); > - if (!input) { > - dev_err(>dev, "failed to allocate input device.\n"); > - return -ENOMEM; > + if (polled) { > + poll_dev = devm_input_allocate_polled_device(>dev); > + if (!poll_dev) { > + dev_err(>dev, > + "failed to allocate polled input device.\n"); > + return -ENOMEM; > + } > + > + poll_dev->pol
Re: [PATCH v3 4/7] cpufreq: add driver for Raspbery Pi
On Tue, 2019-06-11 at 19:58 +0200, Nicolas Saenz Julienne wrote: > Raspberry Pi's firmware offers and interface though which update it's > performance requirements. It allows us to request for specific runtime > frequencies, which the firmware might or might not respect, depending on > the firmware configuration and thermals. > > As the maximum and minimum frequencies are configurable in the firmware > there is no way to know in advance their values. So the Raspberry Pi > cpufreq driver queries them, builds an opp frequency table to then > launch cpufreq-dt. > > Also, as the firmware interface might be configured as a module, making > the cpu clock unavailable during init, this implements a full fledged > driver, as opposed to most drivers registering cpufreq-dt, which only > make use of an init routine. > > Signed-off-by: Nicolas Saenz Julienne > Acked-by: Eric Anholt > Reviewed-by: Stephen Boyd > --- Changes since v2: - Round OPP rates > Changes since v1: > - Remove compatible checks > - Add module support, now full fledged driver > - Use NULL in clk_get() > > drivers/cpufreq/Kconfig.arm | 8 +++ > drivers/cpufreq/Makefile | 1 + > drivers/cpufreq/raspberrypi-cpufreq.c | 97 +++ > 3 files changed, 106 insertions(+) > create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c > > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm > index 6f65b7f05496..56c31a78c692 100644 > --- a/drivers/cpufreq/Kconfig.arm > +++ b/drivers/cpufreq/Kconfig.arm > @@ -142,6 +142,14 @@ config ARM_QCOM_CPUFREQ_HW > The driver implements the cpufreq interface for this HW engine. > Say Y if you want to support CPUFreq HW. > > +config ARM_RASPBERRYPI_CPUFREQ > + tristate "Raspberry Pi cpufreq support" > + depends on CLK_RASPBERRYPI || COMPILE_TEST > + help > + This adds the CPUFreq driver for Raspberry Pi > + > + If in doubt, say N. > + > config ARM_S3C_CPUFREQ > bool > help > diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile > index 7bcda2273d0c..5a6c70d26c98 100644 > --- a/drivers/cpufreq/Makefile > +++ b/drivers/cpufreq/Makefile > @@ -65,6 +65,7 @@ obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)+= pxa2xx-cpufreq.o > obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o > obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW)+= qcom-cpufreq-hw.o > obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO) += qcom-cpufreq-kryo.o > +obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ)+= raspberrypi-cpufreq.o > obj-$(CONFIG_ARM_S3C2410_CPUFREQ)+= s3c2410-cpufreq.o > obj-$(CONFIG_ARM_S3C2412_CPUFREQ)+= s3c2412-cpufreq.o > obj-$(CONFIG_ARM_S3C2416_CPUFREQ)+= s3c2416-cpufreq.o > diff --git a/drivers/cpufreq/raspberrypi-cpufreq.c > b/drivers/cpufreq/raspberrypi-cpufreq.c > new file mode 100644 > index ..2bc7d9734272 > --- /dev/null > +++ b/drivers/cpufreq/raspberrypi-cpufreq.c > @@ -0,0 +1,97 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Raspberry Pi cpufreq driver > + * > + * Copyright (C) 2019, Nicolas Saenz Julienne > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define RASPBERRYPI_FREQ_INTERVAL1 > + > +static struct platform_device *cpufreq_dt; > + > +static int raspberrypi_cpufreq_probe(struct platform_device *pdev) > +{ > + struct device *cpu_dev; > + unsigned long min, max; > + unsigned long rate; > + struct clk *clk; > + int ret; > + > + cpu_dev = get_cpu_device(0); > + if (!cpu_dev) { > + pr_err("Cannot get CPU for cpufreq driver\n"); > + return -ENODEV; > + } > + > + clk = clk_get(cpu_dev, NULL); > + if (IS_ERR(clk)) { > + dev_err(cpu_dev, "Cannot get clock for CPU0\n"); > + return PTR_ERR(clk); > + } > + > + /* > + * The max and min frequencies are configurable in the Raspberry Pi > + * firmware, so we query them at runtime. > + */ > + min = roundup(clk_round_rate(clk, 0), RASPBERRYPI_FREQ_INTERVAL); > + max = roundup(clk_round_rate(clk, ULONG_MAX), > RASPBERRYPI_FREQ_INTERVAL); > + clk_put(clk); > + > + for (rate = min; rate <= max; rate += RASPBERRYPI_FREQ_INTERVAL) { > + ret = dev_pm_opp_add(cpu_dev, rate, 0); > + if (ret) > + goto remove_opp; > + } > + > + cpufreq_dt = platform_device_register_simple("cpufreq-dt", -1, NULL, 0); > + ret = PTR_ERR_OR_ZERO(cpufreq_dt); > + if (ret) { > + dev_err(cpu_dev, "Fail
[PATCH v3 7/7] arm64: defconfig: enable cpufreq support for RPi3
This enables both the new firmware clock driver and cpufreq driver available for the RPi3 family of boards. Signed-off-by: Nicolas Saenz Julienne --- Changes since v2: - Build both drivers as modules arch/arm64/configs/defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index bb0705e1f52e..73fb2067a905 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -83,6 +83,7 @@ CONFIG_ACPI_CPPC_CPUFREQ=m CONFIG_ARM_ARMADA_37XX_CPUFREQ=y CONFIG_ARM_SCPI_CPUFREQ=y CONFIG_ARM_IMX_CPUFREQ_DT=m +CONFIG_ARM_RASPBERRYPI_CPUFREQ=m CONFIG_ARM_TEGRA186_CPUFREQ=y CONFIG_ARM_SCPI_PROTOCOL=y CONFIG_RASPBERRYPI_FIRMWARE=y @@ -652,6 +653,7 @@ CONFIG_COMMON_CLK_CS2000_CP=y CONFIG_COMMON_CLK_S2MPS11=y CONFIG_CLK_QORIQ=y CONFIG_COMMON_CLK_PWM=y +CONFIG_CLK_RASPBERRYPI=m CONFIG_CLK_IMX8MM=y CONFIG_CLK_IMX8MQ=y CONFIG_CLK_IMX8QXP=y -- 2.21.0
[PATCH v3 6/7] ARM: defconfig: enable cpufreq driver for RPi
This enables on both multi_v7_defconfig and bcm2835_defconfig the new firmware based clock and cpufreq drivers for the Raspberry Pi platform. In the case of bcm2835_defconfig, as the cpufreq subsystem was disabled, the conservative governor was selected as default since it better handles the high frequency transition latency. Signed-off-by: Nicolas Saenz Julienne --- Changes since v2: - Change default governor to conservative in bcm2835_defconfig - Set all as builtin in bcm2835_defconfig arch/arm/configs/bcm2835_defconfig | 9 + arch/arm/configs/multi_v7_defconfig | 2 ++ 2 files changed, 11 insertions(+) diff --git a/arch/arm/configs/bcm2835_defconfig b/arch/arm/configs/bcm2835_defconfig index dcf7610cfe55..519ff58e67b3 100644 --- a/arch/arm/configs/bcm2835_defconfig +++ b/arch/arm/configs/bcm2835_defconfig @@ -37,6 +37,14 @@ CONFIG_CMA=y CONFIG_SECCOMP=y CONFIG_KEXEC=y CONFIG_CRASH_DUMP=y +CONFIG_CPU_FREQ=y +CONFIG_CPU_FREQ_STAT=y +CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE=y +CONFIG_CPU_FREQ_GOV_POWERSAVE=y +CONFIG_CPU_FREQ_GOV_USERSPACE=y +CONFIG_CPU_FREQ_GOV_ONDEMAND=y +CONFIG_CPUFREQ_DT=y +CONFIG_ARM_RASPBERRYPI_CPUFREQ=y CONFIG_VFP=y # CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set # CONFIG_SUSPEND is not set @@ -132,6 +140,7 @@ CONFIG_DMA_BCM2835=y CONFIG_STAGING=y CONFIG_SND_BCM2835=m CONFIG_VIDEO_BCM2835=m +CONFIG_CLK_RASPBERRYPI=y CONFIG_MAILBOX=y CONFIG_BCM2835_MBOX=y # CONFIG_IOMMU_SUPPORT is not set diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index 6b748f214eae..0fd60a83f768 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -102,6 +102,7 @@ CONFIG_CPU_FREQ_GOV_CONSERVATIVE=m CONFIG_CPU_FREQ_GOV_SCHEDUTIL=y CONFIG_CPUFREQ_DT=y CONFIG_ARM_IMX6Q_CPUFREQ=y +CONFIG_ARM_RASPBERRYPI_CPUFREQ=y CONFIG_QORIQ_CPUFREQ=y CONFIG_CPU_IDLE=y CONFIG_ARM_CPUIDLE=y @@ -899,6 +900,7 @@ CONFIG_STAGING_BOARD=y CONFIG_COMMON_CLK_MAX77686=y CONFIG_COMMON_CLK_RK808=m CONFIG_COMMON_CLK_S2MPS11=m +CONFIG_CLK_RASPBERRYPI=y CONFIG_COMMON_CLK_QCOM=y CONFIG_QCOM_CLK_RPM=y CONFIG_APQ_MMCC_8084=y -- 2.21.0
[PATCH v3 5/7] clk: raspberrypi: register platform device for raspberrypi-cpufreq
As 'clk-raspberrypi' depends on RPi's firmware interface, which might be configured as a module, the cpu clock might not be available for the cpufreq driver during it's init process. So we register the 'raspberrypi-cpufreq' platform device after the probe sequence succeeds. Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt --- Changes since v2: - Use raspberrypi_clk struct to store cpufreq platform_device drivers/clk/bcm/clk-raspberrypi.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c index 467933767106..7f9b001f8d70 100644 --- a/drivers/clk/bcm/clk-raspberrypi.c +++ b/drivers/clk/bcm/clk-raspberrypi.c @@ -34,6 +34,7 @@ struct raspberrypi_clk { struct device *dev; struct rpi_firmware *firmware; + struct platform_device *cpufreq; unsigned long min_rate; unsigned long max_rate; @@ -272,6 +273,7 @@ static int raspberrypi_clk_probe(struct platform_device *pdev) rpi->dev = dev; rpi->firmware = firmware; + platform_set_drvdata(pdev, rpi); ret = raspberrypi_register_pllb(rpi); if (ret) { @@ -283,6 +285,18 @@ static int raspberrypi_clk_probe(struct platform_device *pdev) if (ret) return ret; + rpi->cpufreq = platform_device_register_data(dev, "raspberrypi-cpufreq", +-1, NULL, 0); + + return 0; +} + +static int raspberrypi_clk_remove(struct platform_device *pdev) +{ + struct raspberrypi_clk *rpi = platform_get_drvdata(pdev); + + platform_device_unregister(rpi->cpufreq); + return 0; } @@ -291,6 +305,7 @@ static struct platform_driver raspberrypi_clk_driver = { .name = "raspberrypi-clk", }, .probe = raspberrypi_clk_probe, + .remove = raspberrypi_clk_remove, }; module_platform_driver(raspberrypi_clk_driver); -- 2.21.0
[PATCH v3 4/7] cpufreq: add driver for Raspbery Pi
Raspberry Pi's firmware offers and interface though which update it's performance requirements. It allows us to request for specific runtime frequencies, which the firmware might or might not respect, depending on the firmware configuration and thermals. As the maximum and minimum frequencies are configurable in the firmware there is no way to know in advance their values. So the Raspberry Pi cpufreq driver queries them, builds an opp frequency table to then launch cpufreq-dt. Also, as the firmware interface might be configured as a module, making the cpu clock unavailable during init, this implements a full fledged driver, as opposed to most drivers registering cpufreq-dt, which only make use of an init routine. Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt Reviewed-by: Stephen Boyd --- Changes since v1: - Remove compatible checks - Add module support, now full fledged driver - Use NULL in clk_get() drivers/cpufreq/Kconfig.arm | 8 +++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/raspberrypi-cpufreq.c | 97 +++ 3 files changed, 106 insertions(+) create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 6f65b7f05496..56c31a78c692 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -142,6 +142,14 @@ config ARM_QCOM_CPUFREQ_HW The driver implements the cpufreq interface for this HW engine. Say Y if you want to support CPUFreq HW. +config ARM_RASPBERRYPI_CPUFREQ + tristate "Raspberry Pi cpufreq support" + depends on CLK_RASPBERRYPI || COMPILE_TEST + help + This adds the CPUFreq driver for Raspberry Pi + + If in doubt, say N. + config ARM_S3C_CPUFREQ bool help diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index 7bcda2273d0c..5a6c70d26c98 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -65,6 +65,7 @@ obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)+= qcom-cpufreq-kryo.o +obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ) += raspberrypi-cpufreq.o obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o obj-$(CONFIG_ARM_S3C2416_CPUFREQ) += s3c2416-cpufreq.o diff --git a/drivers/cpufreq/raspberrypi-cpufreq.c b/drivers/cpufreq/raspberrypi-cpufreq.c new file mode 100644 index ..2bc7d9734272 --- /dev/null +++ b/drivers/cpufreq/raspberrypi-cpufreq.c @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Raspberry Pi cpufreq driver + * + * Copyright (C) 2019, Nicolas Saenz Julienne + */ + +#include +#include +#include +#include +#include +#include + +#define RASPBERRYPI_FREQ_INTERVAL 1 + +static struct platform_device *cpufreq_dt; + +static int raspberrypi_cpufreq_probe(struct platform_device *pdev) +{ + struct device *cpu_dev; + unsigned long min, max; + unsigned long rate; + struct clk *clk; + int ret; + + cpu_dev = get_cpu_device(0); + if (!cpu_dev) { + pr_err("Cannot get CPU for cpufreq driver\n"); + return -ENODEV; + } + + clk = clk_get(cpu_dev, NULL); + if (IS_ERR(clk)) { + dev_err(cpu_dev, "Cannot get clock for CPU0\n"); + return PTR_ERR(clk); + } + + /* +* The max and min frequencies are configurable in the Raspberry Pi +* firmware, so we query them at runtime. +*/ + min = roundup(clk_round_rate(clk, 0), RASPBERRYPI_FREQ_INTERVAL); + max = roundup(clk_round_rate(clk, ULONG_MAX), RASPBERRYPI_FREQ_INTERVAL); + clk_put(clk); + + for (rate = min; rate <= max; rate += RASPBERRYPI_FREQ_INTERVAL) { + ret = dev_pm_opp_add(cpu_dev, rate, 0); + if (ret) + goto remove_opp; + } + + cpufreq_dt = platform_device_register_simple("cpufreq-dt", -1, NULL, 0); + ret = PTR_ERR_OR_ZERO(cpufreq_dt); + if (ret) { + dev_err(cpu_dev, "Failed to create platform device, %d\n", ret); + goto remove_opp; + } + + return 0; + +remove_opp: + dev_pm_opp_remove_all_dynamic(cpu_dev); + + return ret; +} + +static int raspberrypi_cpufreq_remove(struct platform_device *pdev) +{ + struct device *cpu_dev; + + cpu_dev = get_cpu_device(0); + if (cpu_dev) + dev_pm_opp_remove_all_dynamic(cpu_dev); + + platform_device_unregister(cpufreq_dt); + + return 0; +} + +/* + * Since the driver depends on clk-raspberrypi, which may return EPROBE_DEFER, + * all the activity is performed in the pro
[PATCH v3 3/7] firmware: raspberrypi: register clk device
Since clk-raspberrypi is tied to the VC4 firmware instead of particular hardware it's registration should be performed by the firmware driver. Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt --- drivers/firmware/raspberrypi.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c index 61be15d9df7d..da26a584dca0 100644 --- a/drivers/firmware/raspberrypi.c +++ b/drivers/firmware/raspberrypi.c @@ -20,6 +20,7 @@ #define MBOX_CHAN_PROPERTY 8 static struct platform_device *rpi_hwmon; +static struct platform_device *rpi_clk; struct rpi_firmware { struct mbox_client cl; @@ -207,6 +208,12 @@ rpi_register_hwmon_driver(struct device *dev, struct rpi_firmware *fw) -1, NULL, 0); } +static void rpi_register_clk_driver(struct device *dev) +{ + rpi_clk = platform_device_register_data(dev, "raspberrypi-clk", + -1, NULL, 0); +} + static int rpi_firmware_probe(struct platform_device *pdev) { struct device *dev = >dev; @@ -234,6 +241,7 @@ static int rpi_firmware_probe(struct platform_device *pdev) rpi_firmware_print_firmware_revision(fw); rpi_register_hwmon_driver(dev, fw); + rpi_register_clk_driver(dev); return 0; } @@ -254,6 +262,8 @@ static int rpi_firmware_remove(struct platform_device *pdev) platform_device_unregister(rpi_hwmon); rpi_hwmon = NULL; + platform_device_unregister(rpi_clk); + rpi_clk = NULL; mbox_free_channel(fw->chan); return 0; -- 2.21.0
[PATCH v3 2/7] clk: bcm283x: add driver interfacing with Raspberry Pi's firmware
Raspberry Pi's firmware offers an interface though which update it's clock's frequencies. This is specially useful in order to change the CPU clock (pllb_arm) which is 'owned' by the firmware and we're unable to scale using the register interface provided by clk-bcm2835. Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt --- Changes since v2: - Remove redundant error message - Rebase to linux next - Fix spelling - Fix checkpatch.pl errors Changes since v1: - Use BIT() - Add Kconfig entry, with compile test - remove prepare/unprepare - Fix uninitialized init.name in pllb registration - Add MODULE_ALIAS() - Use determine_rate() instead of round_rate() - Add small introduction explaining need for driver drivers/clk/bcm/Kconfig | 7 + drivers/clk/bcm/Makefile | 1 + drivers/clk/bcm/clk-raspberrypi.c | 300 ++ 3 files changed, 308 insertions(+) create mode 100644 drivers/clk/bcm/clk-raspberrypi.c diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig index 766a838ad9af..16e508eba6e5 100644 --- a/drivers/clk/bcm/Kconfig +++ b/drivers/clk/bcm/Kconfig @@ -74,3 +74,10 @@ config CLK_BCM_SR default ARCH_BCM_IPROC help Enable common clock framework support for the Broadcom Stingray SoC + +config CLK_RASPBERRYPI + tristate "Raspberry Pi firmware based clock support" + depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE) + help + Enable common clock framework support for Raspberry Pi's firmware + dependent clocks diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile index e924f25bc6c8..004e9526d6f6 100644 --- a/drivers/clk/bcm/Makefile +++ b/drivers/clk/bcm/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm21664.o obj-$(CONFIG_COMMON_CLK_IPROC) += clk-iproc-armpll.o clk-iproc-pll.o clk-iproc-asiu.o obj-$(CONFIG_CLK_BCM2835) += clk-bcm2835.o obj-$(CONFIG_CLK_BCM2835) += clk-bcm2835-aux.o +obj-$(CONFIG_CLK_RASPBERRYPI) += clk-raspberrypi.o obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o obj-$(CONFIG_CLK_BCM_CYGNUS) += clk-cygnus.o obj-$(CONFIG_CLK_BCM_HR2) += clk-hr2.o diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c new file mode 100644 index ..467933767106 --- /dev/null +++ b/drivers/clk/bcm/clk-raspberrypi.c @@ -0,0 +1,300 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Raspberry Pi driver for firmware controlled clocks + * + * Even though clk-bcm2835 provides an interface to the hardware registers for + * the system clocks we've had to factor out 'pllb' as the firmware 'owns' it. + * We're not allowed to change it directly as we might race with the + * over-temperature and under-voltage protections provided by the firmware. + * + * Copyright (C) 2019 Nicolas Saenz Julienne + */ + +#include +#include +#include +#include +#include + +#include + +#define RPI_FIRMWARE_ARM_CLK_ID0x3 + +#define RPI_FIRMWARE_STATE_ENABLE_BIT BIT(0) +#define RPI_FIRMWARE_STATE_WAIT_BITBIT(1) + +/* + * Even though the firmware interface alters 'pllb' the frequencies are + * provided as per 'pllb_arm'. We need to scale before passing them trough. + */ +#define RPI_FIRMWARE_PLLB_ARM_DIV_RATE 2 + +#define A2W_PLL_FRAC_BITS 20 + +struct raspberrypi_clk { + struct device *dev; + struct rpi_firmware *firmware; + + unsigned long min_rate; + unsigned long max_rate; + + struct clk_hw pllb; + struct clk_hw *pllb_arm; + struct clk_lookup *pllb_arm_lookup; +}; + +/* + * Structure of the message passed to Raspberry Pi's firmware in order to + * change clock rates. The 'disable_turbo' option is only available to the ARM + * clock (pllb) which we enable by default as turbo mode will alter multiple + * clocks at once. + * + * Even though we're able to access the clock registers directly we're bound to + * use the firmware interface as the firmware ultimately takes care of + * mitigating overheating/undervoltage situations and we would be changing + * frequencies behind his back. + * + * For more information on the firmware interface check: + * https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface + */ +struct raspberrypi_firmware_prop { + __le32 id; + __le32 val; + __le32 disable_turbo; +} __packed; + +static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 tag, + u32 clk, u32 *val) +{ + struct raspberrypi_firmware_prop msg = { + .id = clk, + .val = *val, + .disable_turbo = 1, + }; + int ret; + + ret = rpi_firmware_property(firmware, tag, , sizeof(msg)); + if (ret) + return ret; + + *val = msg.val; + + return 0; +} + +static int raspberrypi_fw_pll_is_on(struct clk_hw *hw)
[PATCH v3 1/7] clk: bcm2835: remove pllb
Raspberry Pi's firmware controls this pll, we should use the firmware interface to access it. Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt --- Changes since v1: - Add comment to explain why pllb isn't there anymore drivers/clk/bcm/clk-bcm2835.c | 28 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index 770bb01f523e..867ae3c20041 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -1651,30 +1651,10 @@ static const struct bcm2835_clk_desc clk_desc_array[] = { .fixed_divider = 1, .flags = CLK_SET_RATE_PARENT), - /* PLLB is used for the ARM's clock. */ - [BCM2835_PLLB] = REGISTER_PLL( - .name = "pllb", - .cm_ctrl_reg = CM_PLLB, - .a2w_ctrl_reg = A2W_PLLB_CTRL, - .frac_reg = A2W_PLLB_FRAC, - .ana_reg_base = A2W_PLLB_ANA0, - .reference_enable_mask = A2W_XOSC_CTRL_PLLB_ENABLE, - .lock_mask = CM_LOCK_FLOCKB, - - .ana = _ana_default, - - .min_rate = 6u, - .max_rate = 30u, - .max_fb_rate = BCM2835_MAX_FB_RATE), - [BCM2835_PLLB_ARM] = REGISTER_PLL_DIV( - .name = "pllb_arm", - .source_pll = "pllb", - .cm_reg = CM_PLLB, - .a2w_reg = A2W_PLLB_ARM, - .load_mask = CM_PLLB_LOADARM, - .hold_mask = CM_PLLB_HOLDARM, - .fixed_divider = 1, - .flags = CLK_SET_RATE_PARENT), + /* +* PLLB is used for the ARM's clock. Controlled by firmware, see +* clk-raspberrypi.c. +*/ /* * PLLC is the core PLL, used to drive the core VPU clock. -- 2.21.0
[PATCH v3 0/7] cpufreq support for Raspberry Pi
Hi all, this aims at adding cpufreq support to the Raspberry Pi family of boards. The series first factors out 'pllb' from clk-bcm2385 and creates a new clk driver that operates it over RPi's firmware interface[1]. We are forced to do so as the firmware 'owns' the pll and we're not allowed to change through the register interface directly as we might race with the over-temperature and under-voltage protections provided by the firmware. Next it creates a minimal cpufreq driver that populates the CPU's opp table, and registers cpufreq-dt. Which is needed as the firmware controls the max and min frequencies available. This was tested on a RPi3b+ and RPI2b, both using multi_v7_defconfig and arm64's defconfig. That's all, kind regards, Nicolas [1] https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface --- Changes since v2: - Fixed configs to match Stefan's comments - Round OPP frequencies - Rebase onto linux-next - Minor cleanups & checkpatch.pl Changes since v1: - Enabled by default on the whole family of devices - Added/Fixed module support - clk device now registered by firmware driver - raspberrypi-cpufreq device now registered by clk driver - Reimplemented clk rounding unsing determine_rate() - Enabled in configs for arm and arm64 Changes since RFC: - Move firmware clk device into own driver Nicolas Saenz Julienne (7): clk: bcm2835: remove pllb clk: bcm283x: add driver interfacing with Raspberry Pi's firmware firmware: raspberrypi: register clk device cpufreq: add driver for Raspbery Pi clk: raspberrypi: register platform device for raspberrypi-cpufreq ARM: defconfig: enable cpufreq driver for RPi arm64: defconfig: enable cpufreq support for RPi3 arch/arm/configs/bcm2835_defconfig| 9 + arch/arm/configs/multi_v7_defconfig | 2 + arch/arm64/configs/defconfig | 2 + drivers/clk/bcm/Kconfig | 7 + drivers/clk/bcm/Makefile | 1 + drivers/clk/bcm/clk-bcm2835.c | 28 +-- drivers/clk/bcm/clk-raspberrypi.c | 315 ++ drivers/cpufreq/Kconfig.arm | 8 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/raspberrypi-cpufreq.c | 97 drivers/firmware/raspberrypi.c| 10 + 11 files changed, 456 insertions(+), 24 deletions(-) create mode 100644 drivers/clk/bcm/clk-raspberrypi.c create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c -- 2.21.0
[PATCH v2] HID: input: fix a4tech horizontal wheel custom usage
Some a4tech mice use the 'GenericDesktop.00b8' usage to inform whether the previous wheel report was horizontal or vertical. Before c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key") this usage was being mapped to 'Relative.Misc'. After the patch it's simply ignored (usage->type == 0 & usage->code == 0). Which ultimately makes hid-a4tech ignore the WHEEL/HWHEEL selection event, as it has no usage->type. We shouldn't rely on a mapping for that usage as it's nonstandard and doesn't really map to an input event. So we bypass the mapping and make sure the custom event handling properly handles both reports. Fixes: c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key") Signed-off-by: Nicolas Saenz Julienne --- NOTE: I CC'd Wolfgang as he's the one who can test this. Changes since v1: - new approach, moved fix into hid-a4tech drivers/hid/hid-a4tech.c | 30 +++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/drivers/hid/hid-a4tech.c b/drivers/hid/hid-a4tech.c index 98bf694626f7..3a8c4a5971f7 100644 --- a/drivers/hid/hid-a4tech.c +++ b/drivers/hid/hid-a4tech.c @@ -23,12 +23,36 @@ #define A4_2WHEEL_MOUSE_HACK_7 0x01 #define A4_2WHEEL_MOUSE_HACK_B80x02 +#define A4_WHEEL_ORIENTATION (HID_UP_GENDESK | 0x00b8) + struct a4tech_sc { unsigned long quirks; unsigned int hw_wheel; __s32 delayed_value; }; +static int a4_input_mapping(struct hid_device *hdev, struct hid_input *hi, + struct hid_field *field, struct hid_usage *usage, + unsigned long **bit, int *max) +{ + struct a4tech_sc *a4 = hid_get_drvdata(hdev); + + if (a4->quirks & A4_2WHEEL_MOUSE_HACK_B8 && + usage->hid == A4_WHEEL_ORIENTATION) { + /* +* We do not want to have this usage mapped to anything as it's +* nonstandard and doesn't really behave like an HID report. +* It's only selecting the orientation (vertical/horizontal) of +* the previous mouse wheel report. The input_events will be +* generated once both reports are recorded in a4_event(). +*/ + return -1; + } + + return 0; + +} + static int a4_input_mapped(struct hid_device *hdev, struct hid_input *hi, struct hid_field *field, struct hid_usage *usage, unsigned long **bit, int *max) @@ -52,8 +76,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field, struct a4tech_sc *a4 = hid_get_drvdata(hdev); struct input_dev *input; - if (!(hdev->claimed & HID_CLAIMED_INPUT) || !field->hidinput || - !usage->type) + if (!(hdev->claimed & HID_CLAIMED_INPUT) || !field->hidinput) return 0; input = field->hidinput->input; @@ -64,7 +87,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field, return 1; } - if (usage->hid == 0x000100b8) { + if (usage->hid == A4_WHEEL_ORIENTATION) { input_event(input, EV_REL, value ? REL_HWHEEL : REL_WHEEL, a4->delayed_value); input_event(input, EV_REL, value ? REL_HWHEEL_HI_RES : @@ -131,6 +154,7 @@ MODULE_DEVICE_TABLE(hid, a4_devices); static struct hid_driver a4_driver = { .name = "a4tech", .id_table = a4_devices, + .input_mapping = a4_input_mapping, .input_mapped = a4_input_mapped, .event = a4_event, .probe = a4_probe, -- 2.21.0
Re: [PATCH] HID: input: fix a4tech horizontal wheel custom usage id
On Tue, 2019-06-11 at 10:43 +0200, Benjamin Tissoires wrote: > Hi Nicolas, > > On Mon, Jun 10, 2019 at 8:54 PM Nicolas Saenz Julienne > wrote: > > Some a4tech mice use the 'GenericDesktop.00b8' usage id to inform > > whether the previous wheel report was horizontal or vertical. Before > > c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key") this > > usage id was being mapped to 'Relative.Misc'. After the patch it's > > simply ignored (usage->type == 0 & usage->code == 0). Checking the HID > > Usage Tables it turns out it's a reserved usage_id, so it makes sense to > > map it the way it was. Ultimately this makes hid-a4tech ignore the > > WHEEL/HWHEEL selection event, as it has no usage->type. > > > > The patch reverts the handling of the usage id back to it's previous > > behavior. > > Hmm, if A4Tech is using a reserved usage, we shouldn't fix this in > hid-input.c but in hid-a4tech instead. > Because you won't know when someone else in the HID consortium will > remap this usage to some other random axis, and your mouse will be > broken again. > > How about you add a .input_mapping callback in hid-a4tech and map this > usage there to your needs? This way you will be sure that such a > situation will not happen again. I agree it would be a cleaner solution. In summary the first report indicates the wheel relative value, the second the orientation. The first report is already being mapped to REL_WHEEL and REL_WHEEL (or the high-res versions), but what would be a correct code for the second report? The way I see it, we shouldn't map it to anything. And then catch both events in the custom driver to build the input_event() accordinlgy (as it's almost being done already). Is this somewhat correct? I'll send a followup patch anyway so we have something more tangible comment on. > > Fixes: c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key") > > Signed-off-by: Nicolas Saenz Julienne > > --- > > drivers/hid/hid-input.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c > > index 63855f275a38..6a956d5a195e 100644 > > --- a/drivers/hid/hid-input.c > > +++ b/drivers/hid/hid-input.c > > @@ -671,7 +671,7 @@ static void hidinput_configure_usage(struct hid_input > > *hidinput, struct hid_fiel > > if ((usage->hid & 0xf0) == 0xb0) { /* SC - Display */ > > switch (usage->hid & 0xf) { > > case 0x05: map_key_clear(KEY_SWITCHVIDEOMODE); > > break; > > - default: goto ignore; > > + default: goto unknown; > > } > > break; > > } > > -- > > 2.21.0 > > signature.asc Description: This is a digitally signed message part
[PATCH] HID: input: fix a4tech horizontal wheel custom usage id
Some a4tech mice use the 'GenericDesktop.00b8' usage id to inform whether the previous wheel report was horizontal or vertical. Before c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key") this usage id was being mapped to 'Relative.Misc'. After the patch it's simply ignored (usage->type == 0 & usage->code == 0). Checking the HID Usage Tables it turns out it's a reserved usage_id, so it makes sense to map it the way it was. Ultimately this makes hid-a4tech ignore the WHEEL/HWHEEL selection event, as it has no usage->type. The patch reverts the handling of the usage id back to it's previous behavior. Fixes: c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key") Signed-off-by: Nicolas Saenz Julienne --- drivers/hid/hid-input.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index 63855f275a38..6a956d5a195e 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -671,7 +671,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel if ((usage->hid & 0xf0) == 0xb0) { /* SC - Display */ switch (usage->hid & 0xf) { case 0x05: map_key_clear(KEY_SWITCHVIDEOMODE); break; - default: goto ignore; + default: goto unknown; } break; } -- 2.21.0
Re: [PATCH] xhci: clear port_remote_wakeup after resume failure
On Tue, 2019-06-04 at 16:53 +0300, Mathias Nyman wrote: > On 27.5.2019 14.28, Nicolas Saenz Julienne wrote: > > Hi Matthias, > > thanks for the review. > > > > On Mon, 2019-05-27 at 14:16 +0300, Mathias Nyman wrote: > > > On 24.5.2019 17.52, Nicolas Saenz Julienne wrote: > > > > This was seen on a Dell Precision 5520 using it's WD15 dock. The dock's > > > > Ethernet device interfaces with the laptop through one of it's USB3 > > > > ports. While idle, the Ethernet device and HCD are suspended by runtime > > > > PM, being the only device connected on the bus. Then, both are resumed > > > > on > > > > behalf of the Ethernet device, which has remote wake-up capabilities. > > > > > > > > The Ethernet device was observed to randomly disconnect from the USB > > > > port shortly after submitting it's remote wake-up request. Probably a > > > > weird timing issue yet to be investigated. This causes runtime PM to > > > > busyloop causing some tangible CPU load. The reason is the port gets > > > > stuck in the middle of a remote wake-up operation, waiting for the > > > > device to switch to U0. This never happens, leaving "port_remote_wakeup" > > > > enabled, and automatically triggering a failure on any further suspend > > > > operation. > > > > > > > > This patch clears "port_remote_wakeup" upon detecting a device with a > > > > wrong resuming port state (see Table 4-9 in 4.15.2.3). Making sure the > > > > above mentioned situation doesn't trigger a PM busyloop. > > > > > > > > > > There was a similar case where the USB3 link had transitioned to a > > > lower power U1 or U2 state after resume, before driver read the state, > > > leaving port_remote_wakeup flag uncleared. > > > > > > This was fixed in 5.1 kernel by commit: > > > > > > 6cbcf59 xhci: Fix port resume done detection for SS ports with LPM enable > > > > > > Can you check if you have it? > > > It should be in recent stable releases as well. > > > > I was aware of that patch, unfortunately it doesn't address the same issue. > > In > > my case I never get a second port status event (so no PLC == 1 or any state > > change seen in PLS). The device simply disconnects from the bus. > > > > I see, ok, then we need to clear the flag in the hub thread. > > But to me it looks like this patch could cause a small race risk in the > successful > device initiated resume cases. > > If the hub thread, i.e. the get_port_status() function, notices the U0 state > before > the interrupt handler, i.e. handle_port_status() function, then > port_remote_wakeup > flag is cleared in the hub thread and the wakeup notification is never called > from > handle_port_status(). > > Would it be enough to just check for (port_remote_wakeup flag && > !PORT_CONNECT) in the hub thread? > USB3 PORT_CONNECT bit is lost in most error cases. I get your concerns, there is a race indeed. On top of that, checking PORT_CONNECT works fine for me. So if I undertood your suggestion right, would this be fine? diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 3abe70ff1b1e..253957dc62de 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -1057,6 +1057,9 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd, bus_state->resume_done[wIndex] = 0; clear_bit(wIndex, _state->resuming_ports); usb_hcd_end_port_resume(>self, wIndex); + } else if (bus_state->port_remote_wakeup & (1 << port->hcd_portnum) && + !(raw_port_status & PORT_CONNECT)) { + bus_state->port_remote_wakeup &= ~(1 << port->hcd_portnum); } if (bus_state->port_c_suspend & (1 << wIndex)) Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 4/7] cpufreq: add driver for Raspbery Pi
On Fri, 2019-06-07 at 13:42 +0200, Stefan Wahren wrote: > Hi Nicolas, > > Am 06.06.19 um 16:22 schrieb Nicolas Saenz Julienne: > > Raspberry Pi's firmware offers and interface though which update it's > > performance requirements. It allows us to request for specific runtime > > frequencies, which the firmware might or might not respect, depending on > > the firmware configuration and thermals. > > > > As the maximum and minimum frequencies are configurable in the firmware > > there is no way to know in advance their values. So the Raspberry Pi > > cpufreq driver queries them, builds an opp frequency table to then > > launch cpufreq-dt. > > > > Also, as the firmware interface might be configured as a module, making > > the cpu clock unavailable during init, this implements a full fledged > > driver, as opposed to most drivers registering cpufreq-dt, which only > > make use of an init routine. > > > > Signed-off-by: Nicolas Saenz Julienne > > Acked-by: Eric Anholt > > > > --- > > > > Changes since v1: > > - Remove compatible checks > > - Add module support, now full fledged driver > > - Use NULL in clk_get() > > > > drivers/cpufreq/Kconfig.arm | 8 +++ > > drivers/cpufreq/Makefile | 1 + > > drivers/cpufreq/raspberrypi-cpufreq.c | 100 ++ > > 3 files changed, 109 insertions(+) > > create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c > > > > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm > > index f8129edc145e..5e9204d443ff 100644 > > --- a/drivers/cpufreq/Kconfig.arm > > +++ b/drivers/cpufreq/Kconfig.arm > > @@ -133,6 +133,14 @@ config ARM_QCOM_CPUFREQ_HW > > The driver implements the cpufreq interface for this HW engine. > > Say Y if you want to support CPUFreq HW. > > > > +config ARM_RASPBERRYPI_CPUFREQ > > + tristate "Raspberry Pi cpufreq support" > > + depends on CLK_RASPBERRYPI || COMPILE_TEST > > + help > > + This adds the CPUFreq driver for Raspberry Pi > > + > > + If in doubt, say N. > > + > > config ARM_S3C_CPUFREQ > > bool > > help > > diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile > > index 689b26c6f949..121c1acb66c0 100644 > > --- a/drivers/cpufreq/Makefile > > +++ b/drivers/cpufreq/Makefile > > @@ -64,6 +64,7 @@ obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o > > obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o > > obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o > > obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)+= qcom-cpufreq-kryo.o > > +obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ) += raspberrypi-cpufreq.o > > obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o > > obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o > > obj-$(CONFIG_ARM_S3C2416_CPUFREQ) += s3c2416-cpufreq.o > > diff --git a/drivers/cpufreq/raspberrypi-cpufreq.c > > b/drivers/cpufreq/raspberrypi-cpufreq.c > > new file mode 100644 > > index ..99b59d5a50aa > > --- /dev/null > > +++ b/drivers/cpufreq/raspberrypi-cpufreq.c > > @@ -0,0 +1,100 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Raspberry Pi cpufreq driver > > + * > > + * Copyright (C) 2019, Nicolas Saenz Julienne > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +static struct platform_device *cpufreq_dt; > > + > > +static int raspberrypi_cpufreq_probe(struct platform_device *pdev) > > +{ > > + struct device *cpu_dev; > > + unsigned long min, max; > > + unsigned long rate; > > + struct clk *clk; > > + int ret; > > + > > + cpu_dev = get_cpu_device(0); > > + if (!cpu_dev) { > > + pr_err("Cannot get CPU for cpufreq driver\n"); > > + return -ENODEV; > > + } > > + > > + clk = clk_get(cpu_dev, NULL); > > + if (IS_ERR(clk)) { > > + dev_err(cpu_dev, "Cannot get clock for CPU0\n"); > > + return PTR_ERR(clk); > > + } > > + > > + /* > > +* The max and min frequencies are configurable in the Raspberry Pi > > +* firmware, so we query them at runtime > > +*/ > > + min = clk_round_rate(clk, 0); > > + max = clk_round_rate(clk, ULONG_MAX); > > + clk_put(clk); > > + > > + for (rate = min; rate < max; rate += 1) { > > + ret = dev_pm_opp_add(cpu_dev, rate, 0); > > + if (ret) > > + goto remove_opp; > > + } > > i played a little bit with my Raspberry Pi Zero W and this series. Looks > fine so far. > > Sorry for this nitpicking, but i expect user questions about the > differences between sysfs and vcgencmd measure_clock. > > scaling_available_frequencies gives > > 69 79 89 99 > > but vcgencmd measure_clock return the rounded up values. > > I know we shouldn't fake anything, but adding the OPPs rounded up may > avoid confusion. > > Stefan Agree, I'll change this in v3. signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 7/7] arm64: defconfig: enable cpufreq support for RPi3
On Fri, 2019-06-07 at 12:19 +0200, Stefan Wahren wrote: > Hi Nicolas, > > Am 06.06.19 um 16:23 schrieb Nicolas Saenz Julienne: > > This enables both the new firmware clock driver and cpufreq driver > > available for the RPi3 family of boards. > > > > Signed-off-by: Nicolas Saenz Julienne > > --- > > arch/arm64/configs/defconfig | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig > > index 4d583514258c..3b7baffb3087 100644 > > --- a/arch/arm64/configs/defconfig > > +++ b/arch/arm64/configs/defconfig > > @@ -82,6 +82,7 @@ CONFIG_CPUFREQ_DT=y > > CONFIG_ACPI_CPPC_CPUFREQ=m > > CONFIG_ARM_ARMADA_37XX_CPUFREQ=y > > CONFIG_ARM_SCPI_CPUFREQ=y > > +CONFIG_ARM_RASPBERRYPI_CPUFREQ=y > > the arm64 kernel tends to get very big, so i suggested to build it as a > kernel module. > > Any reason why you choose to make it builtin? Not really, I missed your suggestion. I'll fix in v3. signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 2/7] clk: bcm283x: add driver interfacing with Raspberry Pi's firmware
On Fri, 2019-06-07 at 11:26 +0200, Stefan Wahren wrote: > Hi Nicolas, > > Am 06.06.19 um 16:22 schrieb Nicolas Saenz Julienne: > > Raspberry Pi's firmware offers an interface though which update it's > > clock's frequencies. This is specially useful in order to change the CPU > > clock (pllb_arm) which is 'owned' by the firmware and we're unable to > > scale using the register interface provided by clk-bcm2835. > > > > Signed-off-by: Nicolas Saenz Julienne > > Acked-by: Eric Anholt > > > > --- > > > > Changes since v1: > > - Use BIT() > > - Add Kconfig entry, with compile test > > - remove prepare/unprepare > > - Fix uninitialized init.name in pllb registration > > - Add MODULE_ALIAS() > > - Use determine_rate() instead of round_rate() > > - Add small introduction explaining need for driver > > > > drivers/clk/bcm/Kconfig | 7 + > > drivers/clk/bcm/Makefile | 1 + > > drivers/clk/bcm/clk-raspberrypi.c | 302 ++ > > 3 files changed, 310 insertions(+) > > create mode 100644 drivers/clk/bcm/clk-raspberrypi.c > > > > diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig > > index 29ee7b776cd4..a4a2775d65e1 100644 > > --- a/drivers/clk/bcm/Kconfig > > +++ b/drivers/clk/bcm/Kconfig > > @@ -64,3 +64,10 @@ config CLK_BCM_SR > > default ARCH_BCM_IPROC > > help > > Enable common clock framework support for the Broadcom Stingray SoC > > + > > +config CLK_RASPBERRYPI > > + tristate "Raspberry Pi firmware based clock support" > > + depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && > > !RASPBERRYPI_FIRMWARE) > > + help > > + Enable common clock framework support for Raspberry Pi's firmware > > + dependent clocks > > diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile > > index 002661d39128..eb7159099d82 100644 > > --- a/drivers/clk/bcm/Makefile > > +++ b/drivers/clk/bcm/Makefile > > @@ -7,6 +7,7 @@ obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm21664.o > > obj-$(CONFIG_COMMON_CLK_IPROC) += clk-iproc-armpll.o clk-iproc-pll.o > > clk-iproc-asiu.o > > obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o > > obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835-aux.o > > +obj-$(CONFIG_CLK_RASPBERRYPI) += clk-raspberrypi.o > > obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o > > obj-$(CONFIG_CLK_BCM_CYGNUS) += clk-cygnus.o > > obj-$(CONFIG_CLK_BCM_HR2) += clk-hr2.o > > not your fault but you better rebase your next version on linux-next > because Florian's latest patches ("clk: bcm: Make BCM2835 clock drivers > selectable") collide with this patch. > > Checkpatch gives the following output about this patch: > > WARNING: 'harware' may be misspelled - perhaps 'hardware'? > > #58: FILE: drivers/clk/bcm/clk-raspberrypi.c:5: > + * Even though clk-bcm2835 provides an interface to the harware > registers for > > ERROR: code indent should use tabs where possible > #197: FILE: drivers/clk/bcm/clk-raspberrypi.c:144: > +^I^I^I^I struct clk_rate_request *req)$ Noted, thanks. As this seems fairly calm, should I send v3 or wait little more? signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 4/7] cpufreq: add driver for Raspbery Pi
On Thu, 2019-06-06 at 11:23 -0700, Stephen Boyd wrote: > Quoting Nicolas Saenz Julienne (2019-06-06 11:10:04) > > On Thu, 2019-06-06 at 10:36 -0700, Stephen Boyd wrote: > > > Quoting Nicolas Saenz Julienne (2019-06-06 10:22:16) > > > > Hi Stephen, > > > > Thanks for the review. > > > > > > > > On Thu, 2019-06-06 at 10:09 -0700, Stephen Boyd wrote: > > > > > Quoting Nicolas Saenz Julienne (2019-06-06 07:22:56) > > > > > > diff --git a/drivers/cpufreq/raspberrypi-cpufreq.c > > > > > > b/drivers/cpufreq/raspberrypi-cpufreq.c > > > > > > new file mode 100644 > > > > > > index ..99b59d5a50aa > > > > > > --- /dev/null > > > > > > +++ b/drivers/cpufreq/raspberrypi-cpufreq.c > > > > > [...] > > > > > > + > > > > > > +/* > > > > > > + * Since the driver depends on clk-raspberrypi, which may return > > > > > > EPROBE_DEFER, > > > > > > + * all the activity is performed in the probe, which may be defered > > > > > > as > > > > > > well. > > > > > > + */ > > > > > > +static struct platform_driver raspberrypi_cpufreq_driver = { > > > > > > + .driver = { > > > > > > + .name = "raspberrypi-cpufreq", > > > > > > + }, > > > > > > + .probe = raspberrypi_cpufreq_probe, > > > > > > + .remove = raspberrypi_cpufreq_remove, > > > > > > +}; > > > > > > +module_platform_driver(raspberrypi_cpufreq_driver); > > > > > > > > > > How does this driver probe? Do you have a node in DT named > > > > > raspberrypi-cpufreq that matches and probes this? I would think this > > > > > would follow the drivers/cpufreq/cpufreq-dt-platdev.c design where > > > > > it's > > > > > an initcall that probes the board compatible string. > > > > > > > > > > Or, if it depends on clk-raspberrypi probing, maybe it could create > > > > > the > > > > > platform device in that drivers probe function. > > > > > > > > Well you just reviewed that patch :) > > > > > > Ok. So what's your plan? > > > > So as discussed previously with the RPi mantainers, they preferred for the > > platform device for raspberrypi-clk to be created by the firmware interface > > driver. IIRC Stefan said it was more flexible and the approach used with > > RPi's > > hwmon driver already. Also, it's not really clear whether this driver really > > fits the device tree as it wouldn't be describing hardware. > > > > As far as raspberrypi-cpufreq is concerned the max and min frequencies are > > configurable in the firmware. So we can't really integrate cpufreq into the > > device tree as we need to create the opp table dynamically. Hence the > > dedicated > > driver. On top of that the CPU might not have a clock during the init > > process, > > as both the firmware interface and raspberrypi-clk can be compiled as > > modules. > > So I decided the simplest solution was to create the raspberrypi-cpufreq > > platform device at the end of raspberrypi-clk's probe. > > > > Once raspberrypi-cpufreq is loaded it queries the min/max frequencies, > > populates the CPU's opp table and creates an instance of cpufreq-dt. Which > > finally can operate, without the need of any dt info, as opp tables are > > populated and CPUs have a clock. > > > > I hope this makes it a little more clear :). > > > > Yes, thanks. I see that largely follows the commit description so it > looks OK to me. > Thanks! signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 4/7] cpufreq: add driver for Raspbery Pi
On Thu, 2019-06-06 at 10:36 -0700, Stephen Boyd wrote: > Quoting Nicolas Saenz Julienne (2019-06-06 10:22:16) > > Hi Stephen, > > Thanks for the review. > > > > On Thu, 2019-06-06 at 10:09 -0700, Stephen Boyd wrote: > > > Quoting Nicolas Saenz Julienne (2019-06-06 07:22:56) > > > > diff --git a/drivers/cpufreq/raspberrypi-cpufreq.c > > > > b/drivers/cpufreq/raspberrypi-cpufreq.c > > > > new file mode 100644 > > > > index ..99b59d5a50aa > > > > --- /dev/null > > > > +++ b/drivers/cpufreq/raspberrypi-cpufreq.c > > > [...] > > > > + > > > > +/* > > > > + * Since the driver depends on clk-raspberrypi, which may return > > > > EPROBE_DEFER, > > > > + * all the activity is performed in the probe, which may be defered as > > > > well. > > > > + */ > > > > +static struct platform_driver raspberrypi_cpufreq_driver = { > > > > + .driver = { > > > > + .name = "raspberrypi-cpufreq", > > > > + }, > > > > + .probe = raspberrypi_cpufreq_probe, > > > > + .remove = raspberrypi_cpufreq_remove, > > > > +}; > > > > +module_platform_driver(raspberrypi_cpufreq_driver); > > > > > > How does this driver probe? Do you have a node in DT named > > > raspberrypi-cpufreq that matches and probes this? I would think this > > > would follow the drivers/cpufreq/cpufreq-dt-platdev.c design where it's > > > an initcall that probes the board compatible string. > > > > > > Or, if it depends on clk-raspberrypi probing, maybe it could create the > > > platform device in that drivers probe function. > > > > Well you just reviewed that patch :) > > Ok. So what's your plan? So as discussed previously with the RPi mantainers, they preferred for the platform device for raspberrypi-clk to be created by the firmware interface driver. IIRC Stefan said it was more flexible and the approach used with RPi's hwmon driver already. Also, it's not really clear whether this driver really fits the device tree as it wouldn't be describing hardware. As far as raspberrypi-cpufreq is concerned the max and min frequencies are configurable in the firmware. So we can't really integrate cpufreq into the device tree as we need to create the opp table dynamically. Hence the dedicated driver. On top of that the CPU might not have a clock during the init process, as both the firmware interface and raspberrypi-clk can be compiled as modules. So I decided the simplest solution was to create the raspberrypi-cpufreq platform device at the end of raspberrypi-clk's probe. Once raspberrypi-cpufreq is loaded it queries the min/max frequencies, populates the CPU's opp table and creates an instance of cpufreq-dt. Which finally can operate, without the need of any dt info, as opp tables are populated and CPUs have a clock. I hope this makes it a little more clear :). > > > > + > > > > +MODULE_AUTHOR("Nicolas Saenz Julienne > > > +MODULE_DESCRIPTION("Raspberry Pi cpufreq driver"); > > > > +MODULE_LICENSE("GPL"); > > > > +MODULE_ALIAS("platform:raspberrypi-cpufreq"); > > > > > > I don't think the module alias is needed anymore. > > > > That's surprising. I remember the driver not being loaded by udev without > > it. > > > > Maybe I'm wrong. Could be not needed for DT based platform devices with > an OF table. As explained in the previous paragraph, I'm not using DT. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 4/7] cpufreq: add driver for Raspbery Pi
Hi Stephen, Thanks for the review. On Thu, 2019-06-06 at 10:09 -0700, Stephen Boyd wrote: > Quoting Nicolas Saenz Julienne (2019-06-06 07:22:56) > > diff --git a/drivers/cpufreq/raspberrypi-cpufreq.c > > b/drivers/cpufreq/raspberrypi-cpufreq.c > > new file mode 100644 > > index ..99b59d5a50aa > > --- /dev/null > > +++ b/drivers/cpufreq/raspberrypi-cpufreq.c > [...] > > + > > +/* > > + * Since the driver depends on clk-raspberrypi, which may return > > EPROBE_DEFER, > > + * all the activity is performed in the probe, which may be defered as > > well. > > + */ > > +static struct platform_driver raspberrypi_cpufreq_driver = { > > + .driver = { > > + .name = "raspberrypi-cpufreq", > > + }, > > + .probe = raspberrypi_cpufreq_probe, > > + .remove = raspberrypi_cpufreq_remove, > > +}; > > +module_platform_driver(raspberrypi_cpufreq_driver); > > How does this driver probe? Do you have a node in DT named > raspberrypi-cpufreq that matches and probes this? I would think this > would follow the drivers/cpufreq/cpufreq-dt-platdev.c design where it's > an initcall that probes the board compatible string. > > Or, if it depends on clk-raspberrypi probing, maybe it could create the > platform device in that drivers probe function. Well you just reviewed that patch :) > > + > > +MODULE_AUTHOR("Nicolas Saenz Julienne > +MODULE_DESCRIPTION("Raspberry Pi cpufreq driver"); > > +MODULE_LICENSE("GPL"); > > +MODULE_ALIAS("platform:raspberrypi-cpufreq"); > > I don't think the module alias is needed anymore. That's surprising. I remember the driver not being loaded by udev without it. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 5/7] clk: raspberrypi: register platform device for raspberrypi-cpufreq
On Thu, 2019-06-06 at 10:05 -0700, Stephen Boyd wrote: > Quoting Nicolas Saenz Julienne (2019-06-06 07:22:58) > > diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk- > > raspberrypi.c > > index b1365cf19f3a..052296b5fbe4 100644 > > --- a/drivers/clk/bcm/clk-raspberrypi.c > > +++ b/drivers/clk/bcm/clk-raspberrypi.c > > @@ -63,6 +63,8 @@ struct raspberrypi_firmware_prop { > > __le32 disable_turbo; > > } __packed; > > > > +static struct platform_device *rpi_cpufreq; > > Why can't this be stored in platform driver data? It actually could, I just followed the same pattern as the code found in patch #3. I'll update it in the next version if you prefer it. > > > + > > static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 > > tag, > > u32 clk, u32 *val) > > { > > @@ -285,6 +287,17 @@ static int raspberrypi_clk_probe(struct platform_device > > *pdev) > > return ret; > > } > > > > + rpi_cpufreq = platform_device_register_data(dev, "raspberrypi- > > cpufreq", > > + -1, NULL, 0); > > + > > + return 0; > > +} > > + > > +static int raspberrypi_clk_remove(struct platform_device *pdev) > > +{ > > + platform_device_unregister(rpi_cpufreq); > > + rpi_cpufreq = NULL; > > This assignment to NULL looks unnecessary. > Same as above, but now that you pointed out, it's true it doesn't have any effect > > + > > return 0; > > } > > > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 2/7] clk: bcm283x: add driver interfacing with Raspberry Pi's firmware
On Thu, 2019-06-06 at 16:22 +0200, Nicolas Saenz Julienne wrote: > Raspberry Pi's firmware offers an interface though which update it's > clock's frequencies. This is specially useful in order to change the CPU > clock (pllb_arm) which is 'owned' by the firmware and we're unable to > scale using the register interface provided by clk-bcm2835. > > Signed-off-by: Nicolas Saenz Julienne > Acked-by: Eric Anholt > > --- > > Changes since v1: > - Use BIT() > - Add Kconfig entry, with compile test > - remove prepare/unprepare > - Fix uninitialized init.name in pllb registration > - Add MODULE_ALIAS() > - Use determine_rate() instead of round_rate() > - Add small introduction explaining need for driver > > drivers/clk/bcm/Kconfig | 7 + > drivers/clk/bcm/Makefile | 1 + > drivers/clk/bcm/clk-raspberrypi.c | 302 ++ > 3 files changed, 310 insertions(+) > create mode 100644 drivers/clk/bcm/clk-raspberrypi.c > > diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig > index 29ee7b776cd4..a4a2775d65e1 100644 > --- a/drivers/clk/bcm/Kconfig > +++ b/drivers/clk/bcm/Kconfig > @@ -64,3 +64,10 @@ config CLK_BCM_SR > default ARCH_BCM_IPROC > help > Enable common clock framework support for the Broadcom Stingray SoC > + > +config CLK_RASPBERRYPI > + tristate "Raspberry Pi firmware based clock support" > + depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && > !RASPBERRYPI_FIRMWARE) > + help > + Enable common clock framework support for Raspberry Pi's firmware > + dependent clocks > diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile > index 002661d39128..eb7159099d82 100644 > --- a/drivers/clk/bcm/Makefile > +++ b/drivers/clk/bcm/Makefile > @@ -7,6 +7,7 @@ obj-$(CONFIG_CLK_BCM_KONA)+= clk-bcm21664.o > obj-$(CONFIG_COMMON_CLK_IPROC) += clk-iproc-armpll.o clk-iproc-pll.o > clk-iproc-asiu.o > obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o > obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835-aux.o > +obj-$(CONFIG_CLK_RASPBERRYPI)+= clk-raspberrypi.o > obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o > obj-$(CONFIG_CLK_BCM_CYGNUS) += clk-cygnus.o > obj-$(CONFIG_CLK_BCM_HR2)+= clk-hr2.o > diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk- > raspberrypi.c > new file mode 100644 > index ..b1365cf19f3a > --- /dev/null > +++ b/drivers/clk/bcm/clk-raspberrypi.c > @@ -0,0 +1,302 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Raspberry Pi driver for firmware controlled clocks > + * > + * Even though clk-bcm2835 provides an interface to the harware registers for > + * the system clocks we've had to factor out 'pllb' as the firmware 'owns' > it. > + * We're not allowed to change it directly as we might race with the > + * over-temperature and under-voltage protections provided by the firmware. > + * > + * Copyright (C) 2019 Nicolas Saenz Julienne > + */ > + > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#define RPI_FIRMWARE_ARM_CLK_ID 0x3 > + > +#define RPI_FIRMWARE_STATE_ENABLE_BITBIT(0) > +#define RPI_FIRMWARE_STATE_WAIT_BIT BIT(1) > + > +/* > + * Even though the firmware interface alters 'pllb' the frequencies are > + * provided as per 'pllb_arm'. We need to scale before passing them trough. > + */ > +#define RPI_FIRMWARE_PLLB_ARM_DIV_RATE 2 > + > +#define A2W_PLL_FRAC_BITS20 > + > +struct raspberrypi_clk { > + struct device *dev; > + struct rpi_firmware *firmware; > + > + unsigned long min_rate; > + unsigned long max_rate; > + > + struct clk_hw pllb; > + struct clk_hw *pllb_arm; > + struct clk_lookup *pllb_arm_lookup; > +}; > + > +/* > + * Structure of the message passed to Raspberry Pi's firmware in order to > + * change clock rates. The 'disable_turbo' option is only available to the > ARM > + * clock (pllb) which we enable by default as turbo mode will alter multiple > + * clocks at once. > + * > + * Even though we're able to access the clock registers directly we're bound > to > + * use the firmware interface as the firmware ultimately takes care of > + * mitigating overheating/undervoltage situations and we would be changing > + * frequencies behind his back. > + * > + * For more information on the firmware interface check: > + * https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface > + */ > +struct raspberrypi_firmware_prop { > + __le32 id; > + __le32 val; > + __le32 disable_turbo;
[PATCH v2 7/7] arm64: defconfig: enable cpufreq support for RPi3
This enables both the new firmware clock driver and cpufreq driver available for the RPi3 family of boards. Signed-off-by: Nicolas Saenz Julienne --- arch/arm64/configs/defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 4d583514258c..3b7baffb3087 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -82,6 +82,7 @@ CONFIG_CPUFREQ_DT=y CONFIG_ACPI_CPPC_CPUFREQ=m CONFIG_ARM_ARMADA_37XX_CPUFREQ=y CONFIG_ARM_SCPI_CPUFREQ=y +CONFIG_ARM_RASPBERRYPI_CPUFREQ=y CONFIG_ARM_TEGRA186_CPUFREQ=y CONFIG_ARM_SCPI_PROTOCOL=y CONFIG_RASPBERRYPI_FIRMWARE=y @@ -639,6 +640,7 @@ CONFIG_COMMON_CLK_CS2000_CP=y CONFIG_COMMON_CLK_S2MPS11=y CONFIG_CLK_QORIQ=y CONFIG_COMMON_CLK_PWM=y +CONFIG_CLK_RASPBERRYPI=y CONFIG_CLK_IMX8MQ=y CONFIG_CLK_IMX8QXP=y CONFIG_TI_SCI_CLK=y -- 2.21.0
[PATCH v2 5/7] clk: raspberrypi: register platform device for raspberrypi-cpufreq
As 'clk-raspberrypi' depends on RPi's firmware interface, which might be configured as a module, the cpu clock might not be available for the cpufreq driver during it's init process. So we register the 'raspberrypi-cpufreq' platform device after the probe sequence succeeds. Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt --- drivers/clk/bcm/clk-raspberrypi.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c index b1365cf19f3a..052296b5fbe4 100644 --- a/drivers/clk/bcm/clk-raspberrypi.c +++ b/drivers/clk/bcm/clk-raspberrypi.c @@ -63,6 +63,8 @@ struct raspberrypi_firmware_prop { __le32 disable_turbo; } __packed; +static struct platform_device *rpi_cpufreq; + static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 tag, u32 clk, u32 *val) { @@ -285,6 +287,17 @@ static int raspberrypi_clk_probe(struct platform_device *pdev) return ret; } + rpi_cpufreq = platform_device_register_data(dev, "raspberrypi-cpufreq", + -1, NULL, 0); + + return 0; +} + +static int raspberrypi_clk_remove(struct platform_device *pdev) +{ + platform_device_unregister(rpi_cpufreq); + rpi_cpufreq = NULL; + return 0; } @@ -293,6 +306,7 @@ static struct platform_driver raspberrypi_clk_driver = { .name = "raspberrypi-clk", }, .probe = raspberrypi_clk_probe, + .remove = raspberrypi_clk_remove, }; module_platform_driver(raspberrypi_clk_driver); -- 2.21.0
[PATCH v2 6/7] ARM: defconfig: enable cpufreq driver for RPi
This enables on both multi_v7_defconfig and bcm2835_defconfig the new firmware based clock and cpufreq drivers for the Raspberry Pi platform. In the case of bcm2835_defconfig, as the cpufreq subsystem was disabled, the subsystem configuration was copied from multi_v7_defconfig (default governor, statistics, etc...). Signed-off-by: Nicolas Saenz Julienne --- arch/arm/configs/bcm2835_defconfig | 9 + arch/arm/configs/multi_v7_defconfig | 2 ++ 2 files changed, 11 insertions(+) diff --git a/arch/arm/configs/bcm2835_defconfig b/arch/arm/configs/bcm2835_defconfig index dcf7610cfe55..3fd90bfd5fec 100644 --- a/arch/arm/configs/bcm2835_defconfig +++ b/arch/arm/configs/bcm2835_defconfig @@ -37,6 +37,14 @@ CONFIG_CMA=y CONFIG_SECCOMP=y CONFIG_KEXEC=y CONFIG_CRASH_DUMP=y +CONFIG_CPU_FREQ=y +CONFIG_CPU_FREQ_STAT=y +CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y +CONFIG_CPU_FREQ_GOV_POWERSAVE=m +CONFIG_CPU_FREQ_GOV_USERSPACE=m +CONFIG_CPU_FREQ_GOV_CONSERVATIVE=m +CONFIG_CPUFREQ_DT=y +CONFIG_ARM_RASPBERRYPI_CPUFREQ=y CONFIG_VFP=y # CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set # CONFIG_SUSPEND is not set @@ -132,6 +140,7 @@ CONFIG_DMA_BCM2835=y CONFIG_STAGING=y CONFIG_SND_BCM2835=m CONFIG_VIDEO_BCM2835=m +CONFIG_CLK_RASPBERRYPI=y CONFIG_MAILBOX=y CONFIG_BCM2835_MBOX=y # CONFIG_IOMMU_SUPPORT is not set diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index 6b748f214eae..0fd60a83f768 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -102,6 +102,7 @@ CONFIG_CPU_FREQ_GOV_CONSERVATIVE=m CONFIG_CPU_FREQ_GOV_SCHEDUTIL=y CONFIG_CPUFREQ_DT=y CONFIG_ARM_IMX6Q_CPUFREQ=y +CONFIG_ARM_RASPBERRYPI_CPUFREQ=y CONFIG_QORIQ_CPUFREQ=y CONFIG_CPU_IDLE=y CONFIG_ARM_CPUIDLE=y @@ -899,6 +900,7 @@ CONFIG_STAGING_BOARD=y CONFIG_COMMON_CLK_MAX77686=y CONFIG_COMMON_CLK_RK808=m CONFIG_COMMON_CLK_S2MPS11=m +CONFIG_CLK_RASPBERRYPI=y CONFIG_COMMON_CLK_QCOM=y CONFIG_QCOM_CLK_RPM=y CONFIG_APQ_MMCC_8084=y -- 2.21.0
[PATCH v2 4/7] cpufreq: add driver for Raspbery Pi
Raspberry Pi's firmware offers and interface though which update it's performance requirements. It allows us to request for specific runtime frequencies, which the firmware might or might not respect, depending on the firmware configuration and thermals. As the maximum and minimum frequencies are configurable in the firmware there is no way to know in advance their values. So the Raspberry Pi cpufreq driver queries them, builds an opp frequency table to then launch cpufreq-dt. Also, as the firmware interface might be configured as a module, making the cpu clock unavailable during init, this implements a full fledged driver, as opposed to most drivers registering cpufreq-dt, which only make use of an init routine. Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt --- Changes since v1: - Remove compatible checks - Add module support, now full fledged driver - Use NULL in clk_get() drivers/cpufreq/Kconfig.arm | 8 +++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/raspberrypi-cpufreq.c | 100 ++ 3 files changed, 109 insertions(+) create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index f8129edc145e..5e9204d443ff 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -133,6 +133,14 @@ config ARM_QCOM_CPUFREQ_HW The driver implements the cpufreq interface for this HW engine. Say Y if you want to support CPUFreq HW. +config ARM_RASPBERRYPI_CPUFREQ + tristate "Raspberry Pi cpufreq support" + depends on CLK_RASPBERRYPI || COMPILE_TEST + help + This adds the CPUFreq driver for Raspberry Pi + + If in doubt, say N. + config ARM_S3C_CPUFREQ bool help diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index 689b26c6f949..121c1acb66c0 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -64,6 +64,7 @@ obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)+= qcom-cpufreq-kryo.o +obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ) += raspberrypi-cpufreq.o obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o obj-$(CONFIG_ARM_S3C2416_CPUFREQ) += s3c2416-cpufreq.o diff --git a/drivers/cpufreq/raspberrypi-cpufreq.c b/drivers/cpufreq/raspberrypi-cpufreq.c new file mode 100644 index ..99b59d5a50aa --- /dev/null +++ b/drivers/cpufreq/raspberrypi-cpufreq.c @@ -0,0 +1,100 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Raspberry Pi cpufreq driver + * + * Copyright (C) 2019, Nicolas Saenz Julienne + */ + +#include +#include +#include +#include +#include +#include + +static struct platform_device *cpufreq_dt; + +static int raspberrypi_cpufreq_probe(struct platform_device *pdev) +{ + struct device *cpu_dev; + unsigned long min, max; + unsigned long rate; + struct clk *clk; + int ret; + + cpu_dev = get_cpu_device(0); + if (!cpu_dev) { + pr_err("Cannot get CPU for cpufreq driver\n"); + return -ENODEV; + } + + clk = clk_get(cpu_dev, NULL); + if (IS_ERR(clk)) { + dev_err(cpu_dev, "Cannot get clock for CPU0\n"); + return PTR_ERR(clk); + } + + /* +* The max and min frequencies are configurable in the Raspberry Pi +* firmware, so we query them at runtime +*/ + min = clk_round_rate(clk, 0); + max = clk_round_rate(clk, ULONG_MAX); + clk_put(clk); + + for (rate = min; rate < max; rate += 1) { + ret = dev_pm_opp_add(cpu_dev, rate, 0); + if (ret) + goto remove_opp; + } + + ret = dev_pm_opp_add(cpu_dev, max, 0); + if (ret) + goto remove_opp; + + cpufreq_dt = platform_device_register_simple("cpufreq-dt", -1, NULL, 0); + ret = PTR_ERR_OR_ZERO(cpufreq_dt); + if (ret) { + dev_err(cpu_dev, "Failed to create platform device, %d\n", ret); + goto remove_opp; + } + + return 0; + +remove_opp: + dev_pm_opp_remove_all_dynamic(cpu_dev); + + return ret; +} + +static int raspberrypi_cpufreq_remove(struct platform_device *pdev) +{ + struct device *cpu_dev; + + cpu_dev = get_cpu_device(0); + if (cpu_dev) + dev_pm_opp_remove_all_dynamic(cpu_dev); + + platform_device_unregister(cpufreq_dt); + cpufreq_dt = NULL; + + return 0; +} + +/* + * Since the driver depends on clk-raspberrypi, which may return EPROBE_DEFER, + * all the activity is performed in the probe, which may be defered as well. + */ +sta
[PATCH v2 3/7] firmware: raspberrypi: register clk device
Since clk-raspberrypi is tied to the VC4 firmware instead of particular hardware it's registration should be performed by the firmware driver. Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt --- drivers/firmware/raspberrypi.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c index 61be15d9df7d..da26a584dca0 100644 --- a/drivers/firmware/raspberrypi.c +++ b/drivers/firmware/raspberrypi.c @@ -20,6 +20,7 @@ #define MBOX_CHAN_PROPERTY 8 static struct platform_device *rpi_hwmon; +static struct platform_device *rpi_clk; struct rpi_firmware { struct mbox_client cl; @@ -207,6 +208,12 @@ rpi_register_hwmon_driver(struct device *dev, struct rpi_firmware *fw) -1, NULL, 0); } +static void rpi_register_clk_driver(struct device *dev) +{ + rpi_clk = platform_device_register_data(dev, "raspberrypi-clk", + -1, NULL, 0); +} + static int rpi_firmware_probe(struct platform_device *pdev) { struct device *dev = >dev; @@ -234,6 +241,7 @@ static int rpi_firmware_probe(struct platform_device *pdev) rpi_firmware_print_firmware_revision(fw); rpi_register_hwmon_driver(dev, fw); + rpi_register_clk_driver(dev); return 0; } @@ -254,6 +262,8 @@ static int rpi_firmware_remove(struct platform_device *pdev) platform_device_unregister(rpi_hwmon); rpi_hwmon = NULL; + platform_device_unregister(rpi_clk); + rpi_clk = NULL; mbox_free_channel(fw->chan); return 0; -- 2.21.0
[PATCH v2 2/7] clk: bcm283x: add driver interfacing with Raspberry Pi's firmware
Raspberry Pi's firmware offers an interface though which update it's clock's frequencies. This is specially useful in order to change the CPU clock (pllb_arm) which is 'owned' by the firmware and we're unable to scale using the register interface provided by clk-bcm2835. Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt --- Changes since v1: - Use BIT() - Add Kconfig entry, with compile test - remove prepare/unprepare - Fix uninitialized init.name in pllb registration - Add MODULE_ALIAS() - Use determine_rate() instead of round_rate() - Add small introduction explaining need for driver drivers/clk/bcm/Kconfig | 7 + drivers/clk/bcm/Makefile | 1 + drivers/clk/bcm/clk-raspberrypi.c | 302 ++ 3 files changed, 310 insertions(+) create mode 100644 drivers/clk/bcm/clk-raspberrypi.c diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig index 29ee7b776cd4..a4a2775d65e1 100644 --- a/drivers/clk/bcm/Kconfig +++ b/drivers/clk/bcm/Kconfig @@ -64,3 +64,10 @@ config CLK_BCM_SR default ARCH_BCM_IPROC help Enable common clock framework support for the Broadcom Stingray SoC + +config CLK_RASPBERRYPI + tristate "Raspberry Pi firmware based clock support" + depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE) + help + Enable common clock framework support for Raspberry Pi's firmware + dependent clocks diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile index 002661d39128..eb7159099d82 100644 --- a/drivers/clk/bcm/Makefile +++ b/drivers/clk/bcm/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm21664.o obj-$(CONFIG_COMMON_CLK_IPROC) += clk-iproc-armpll.o clk-iproc-pll.o clk-iproc-asiu.o obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835-aux.o +obj-$(CONFIG_CLK_RASPBERRYPI) += clk-raspberrypi.o obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o obj-$(CONFIG_CLK_BCM_CYGNUS) += clk-cygnus.o obj-$(CONFIG_CLK_BCM_HR2) += clk-hr2.o diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c new file mode 100644 index ..b1365cf19f3a --- /dev/null +++ b/drivers/clk/bcm/clk-raspberrypi.c @@ -0,0 +1,302 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Raspberry Pi driver for firmware controlled clocks + * + * Even though clk-bcm2835 provides an interface to the harware registers for + * the system clocks we've had to factor out 'pllb' as the firmware 'owns' it. + * We're not allowed to change it directly as we might race with the + * over-temperature and under-voltage protections provided by the firmware. + * + * Copyright (C) 2019 Nicolas Saenz Julienne + */ + +#include +#include +#include +#include +#include + +#include + +#define RPI_FIRMWARE_ARM_CLK_ID0x3 + +#define RPI_FIRMWARE_STATE_ENABLE_BIT BIT(0) +#define RPI_FIRMWARE_STATE_WAIT_BITBIT(1) + +/* + * Even though the firmware interface alters 'pllb' the frequencies are + * provided as per 'pllb_arm'. We need to scale before passing them trough. + */ +#define RPI_FIRMWARE_PLLB_ARM_DIV_RATE 2 + +#define A2W_PLL_FRAC_BITS 20 + +struct raspberrypi_clk { + struct device *dev; + struct rpi_firmware *firmware; + + unsigned long min_rate; + unsigned long max_rate; + + struct clk_hw pllb; + struct clk_hw *pllb_arm; + struct clk_lookup *pllb_arm_lookup; +}; + +/* + * Structure of the message passed to Raspberry Pi's firmware in order to + * change clock rates. The 'disable_turbo' option is only available to the ARM + * clock (pllb) which we enable by default as turbo mode will alter multiple + * clocks at once. + * + * Even though we're able to access the clock registers directly we're bound to + * use the firmware interface as the firmware ultimately takes care of + * mitigating overheating/undervoltage situations and we would be changing + * frequencies behind his back. + * + * For more information on the firmware interface check: + * https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface + */ +struct raspberrypi_firmware_prop { + __le32 id; + __le32 val; + __le32 disable_turbo; +} __packed; + +static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 tag, + u32 clk, u32 *val) +{ + struct raspberrypi_firmware_prop msg = { + .id = clk, + .val = *val, + .disable_turbo = 1, + }; + int ret; + + ret = rpi_firmware_property(firmware, tag, , sizeof(msg)); + if (ret) + return ret; + + *val = msg.val; + + return 0; +} + +static int raspberrypi_fw_pll_is_on(struct clk_hw *hw) +{ + struct raspberrypi_clk *rpi = container_of(hw, struct raspberrypi_clk, + pllb
[PATCH v2 1/7] clk: bcm2835: remove pllb
Raspberry Pi's firmware controls this pll, we should use the firmware interface to access it. Signed-off-by: Nicolas Saenz Julienne Acked-by: Eric Anholt --- Changes since v1: - Add comment to explain why pllb isn't there anymore drivers/clk/bcm/clk-bcm2835.c | 28 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index 770bb01f523e..867ae3c20041 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -1651,30 +1651,10 @@ static const struct bcm2835_clk_desc clk_desc_array[] = { .fixed_divider = 1, .flags = CLK_SET_RATE_PARENT), - /* PLLB is used for the ARM's clock. */ - [BCM2835_PLLB] = REGISTER_PLL( - .name = "pllb", - .cm_ctrl_reg = CM_PLLB, - .a2w_ctrl_reg = A2W_PLLB_CTRL, - .frac_reg = A2W_PLLB_FRAC, - .ana_reg_base = A2W_PLLB_ANA0, - .reference_enable_mask = A2W_XOSC_CTRL_PLLB_ENABLE, - .lock_mask = CM_LOCK_FLOCKB, - - .ana = _ana_default, - - .min_rate = 6u, - .max_rate = 30u, - .max_fb_rate = BCM2835_MAX_FB_RATE), - [BCM2835_PLLB_ARM] = REGISTER_PLL_DIV( - .name = "pllb_arm", - .source_pll = "pllb", - .cm_reg = CM_PLLB, - .a2w_reg = A2W_PLLB_ARM, - .load_mask = CM_PLLB_LOADARM, - .hold_mask = CM_PLLB_HOLDARM, - .fixed_divider = 1, - .flags = CLK_SET_RATE_PARENT), + /* +* PLLB is used for the ARM's clock. Controlled by firmware, see +* clk-raspberrypi.c. +*/ /* * PLLC is the core PLL, used to drive the core VPU clock. -- 2.21.0
[PATCH v2 0/7] cpufreq support for Raspberry Pi
Hi all, this aims at adding cpufreq support to the Raspberry Pi family of boards. The series first factors out 'pllb' from clk-bcm2385 and creates a new clk driver that operates it over RPi's firmware interface[1]. We are forced to do so as the firmware 'owns' the pll and we're not allowed to change through the register interface directly as we might race with the over-temperature and under-voltage protections provided by the firmware. Next it creates a minimal cpufreq driver that populates the CPU's opp table, and registers cpufreq-dt. Which is needed as the firmware controls the max and min frequencies available. This was tested on a RPi3b+ and RPI2b, both using multi_v7_defconfig and arm64's defconfig. @Eric: Even though some things change, namely to properly support being built as a module, I still added your Acked-by as it didn't change the overall design. I hope it's OK. That's all, kind regards, Nicolas [1] https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface --- Changes since v1: - Enabled by default on the whole family of devices - Added/Fixed module support - clk device now registered by firmware driver - raspberrypi-cpufreq device now registered by clk driver - Reimplemented clk rounding unsing determine_rate() - Enabled in configs for arm and arm64 Changes since RFC: - Move firmware clk device into own driver Nicolas Saenz Julienne (7): clk: bcm2835: remove pllb clk: bcm283x: add driver interfacing with Raspberry Pi's firmware firmware: raspberrypi: register clk device cpufreq: add driver for Raspbery Pi clk: raspberrypi: register platform device for raspberrypi-cpufreq ARM: defconfig: enable cpufreq driver for RPi arm64: defconfig: enable cpufreq support for RPi3 arch/arm/configs/bcm2835_defconfig| 9 + arch/arm/configs/multi_v7_defconfig | 2 + arch/arm64/configs/defconfig | 2 + drivers/clk/bcm/Kconfig | 7 + drivers/clk/bcm/Makefile | 1 + drivers/clk/bcm/clk-bcm2835.c | 28 +-- drivers/clk/bcm/clk-raspberrypi.c | 316 ++ drivers/cpufreq/Kconfig.arm | 8 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/raspberrypi-cpufreq.c | 100 drivers/firmware/raspberrypi.c| 10 + 11 files changed, 460 insertions(+), 24 deletions(-) create mode 100644 drivers/clk/bcm/clk-raspberrypi.c create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c -- 2.21.0
Re: [PATCH 0/4] cpufreq support for the Raspberry Pi
On Wed, 2019-06-05 at 13:34 +0200, Stefan Wahren wrote: > Hi, > > Am 05.06.19 um 13:00 schrieb Nicolas Saenz Julienne: > > Hi Stefan, > > thanks for the review, I took note of your code comments. > > > > On Wed, 2019-06-05 at 11:46 +0200, Stefan Wahren wrote: > > > Hi Nicolas, > > > > > > Am 04.06.19 um 19:32 schrieb Nicolas Saenz Julienne: > > > > Hi all, > > > > this series aims at adding cpufreq support to the Raspberry Pi family of > > > > boards. > > > > > > > > The previous revision can be found at: > > > > https://lkml.org/lkml/2019/5/20/431 > > > > > > > > The series first factors out 'pllb' from clk-bcm2385 and creates a new > > > > clk driver that operates it over RPi's firmware interface[1]. We are > > > > forced to do so as the firmware 'owns' the pll and we're not allowed to > > > > change through the register interface directly as we might race with the > > > > over-temperature and under-voltage protections provided by the firmware. > > > it would be nice to preserve such design decision in the driver as a > > > comment, because the cover letter usually get lost. > > > > Next it creates a minimal cpufreq driver that populates the CPU's opp > > > > table, and registers cpufreq-dt. Which is needed as the firmware > > > > controls the max and min frequencies available. > > > I tested your series on top of Linux 5.2-rc1 with multi_v7_defconfig and > > > manually enable this drivers. During boot with Raspbian rootfs i'm > > > getting the following: > > > > > > [1.177009] cpu cpu0: failed to get clock: -2 > > > [1.183643] cpufreq-dt: probe of cpufreq-dt failed with error -2 > > This is surprising, who could be creating a platform_device for cpufreq-dt > > apart from raspberrypi-cpufreq? Just to make things clear, you're using the > > device tree from v5.2-rc1 (as opposed to the Raspbian one)? > > sorry my fault, i thought it already has been replaced. The behavior in > this unexpected case is fine, since it doesn't crash. > > I replaced the the DTB with the mainline one, but now i'm getting this: > > [4.566068] cpufreq: cpufreq_online: CPU0: Running at unlisted freq: > 60 KHz > [4.580690] cpu cpu0: dev_pm_opp_set_rate: Invalid target frequency 0 > [4.594391] cpufreq: __target_index: Failed to change cpu frequency: -22 For the record this fixes it: diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index aa51756fd4d6..edb71eefe9cf 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1293,7 +1293,7 @@ static int clk_core_determine_round_nolock(struct clk_core *core, } else if (core->ops->round_rate) { rate = core->ops->round_rate(core->hw, req->rate, >best_parent_rate); - if (rate < 0) + if (IS_ERR_VALUE(rate)) return rate; req->rate = rate; round_rate() returns a 'long' value, yet 'pllb' in rpi3b+ goes as high as 2.8GHz, which only fits in an 'unsigned long'. This explains why I didn't see this issue with RPI2b. I'll add the patch to the series. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH 0/4] cpufreq support for the Raspberry Pi
Hi Stefan, On Wed, 2019-06-05 at 13:34 +0200, Stefan Wahren wrote: > Hi, > > Am 05.06.19 um 13:00 schrieb Nicolas Saenz Julienne: > > Hi Stefan, > > thanks for the review, I took note of your code comments. > > > > On Wed, 2019-06-05 at 11:46 +0200, Stefan Wahren wrote: > > > Hi Nicolas, > > > > > > Am 04.06.19 um 19:32 schrieb Nicolas Saenz Julienne: > > > > Hi all, > > > > this series aims at adding cpufreq support to the Raspberry Pi family of > > > > boards. > > > > > > > > The previous revision can be found at: > > > > https://lkml.org/lkml/2019/5/20/431 > > > > > > > > The series first factors out 'pllb' from clk-bcm2385 and creates a new > > > > clk driver that operates it over RPi's firmware interface[1]. We are > > > > forced to do so as the firmware 'owns' the pll and we're not allowed to > > > > change through the register interface directly as we might race with the > > > > over-temperature and under-voltage protections provided by the firmware. > > > it would be nice to preserve such design decision in the driver as a > > > comment, because the cover letter usually get lost. > > > > Next it creates a minimal cpufreq driver that populates the CPU's opp > > > > table, and registers cpufreq-dt. Which is needed as the firmware > > > > controls the max and min frequencies available. > > > I tested your series on top of Linux 5.2-rc1 with multi_v7_defconfig and > > > manually enable this drivers. During boot with Raspbian rootfs i'm > > > getting the following: > > > > > > [1.177009] cpu cpu0: failed to get clock: -2 > > > [1.183643] cpufreq-dt: probe of cpufreq-dt failed with error -2 > > This is surprising, who could be creating a platform_device for cpufreq-dt > > apart from raspberrypi-cpufreq? Just to make things clear, you're using the > > device tree from v5.2-rc1 (as opposed to the Raspbian one)? > > sorry my fault, i thought it already has been replaced. The behavior in > this unexpected case is fine, since it doesn't crash. > > I replaced the the DTB with the mainline one, but now i'm getting this: > > [4.566068] cpufreq: cpufreq_online: CPU0: Running at unlisted freq: > 60 KHz > [4.580690] cpu cpu0: dev_pm_opp_set_rate: Invalid target frequency 0 > [4.594391] cpufreq: __target_index: Failed to change cpu frequency: -22 Ok, this looks more more like my fault, probably an overflow error somewhere. I saw something similar while testing it on RPI2b. Which board & config was this run with? Could you confirm the clk-raspberrypi.c message verifying the max and min frequencies showed up and was correct. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH 2/4] clk: bcm283x: add driver interfacing with Raspberry Pi's firmware
Hi Stefan, thanks for your review. On Wed, 2019-06-05 at 12:44 +0200, Stefan Wahren wrote: > Hi Nicolas, > > Am 04.06.19 um 19:32 schrieb Nicolas Saenz Julienne: > > Raspberry Pi's firmware offers and interface though which update it's > > clock's frequencies. This is specially useful in order to change the CPU > > clock (pllb_arm) which is 'owned' by the firmware and we're unable to > > scale using the register interface. > > > > Signed-off-by: Nicolas Saenz Julienne > > --- > > > > Changes since RFC: > > - Moved firmware interface into own driver > > - Use of_find_compatible_node() > > - Remove error message on rpi_firmware_get() failure > > - Ratelimit messages on set_rate() failure > > - Use __le32 on firmware interface definition > > > > drivers/clk/bcm/Makefile | 1 + > > drivers/clk/bcm/clk-raspberrypi.c | 316 ++ > > 2 files changed, 317 insertions(+) > > create mode 100644 drivers/clk/bcm/clk-raspberrypi.c > > > > diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile > > index 002661d39128..07abe92df9d1 100644 > > --- a/drivers/clk/bcm/Makefile > > +++ b/drivers/clk/bcm/Makefile > > @@ -7,6 +7,7 @@ obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm21664.o > > obj-$(CONFIG_COMMON_CLK_IPROC) += clk-iproc-armpll.o clk-iproc-pll.o > > clk-iproc-asiu.o > > obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o > > obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835-aux.o > > +obj-$(CONFIG_ARCH_BCM2835) += clk-raspberrypi.o > Hm, on the one side it would be nice to avoid building this driver in > case the firmware driver is disabled on the other side it would be good > to keep compile test. > > obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o > > obj-$(CONFIG_CLK_BCM_CYGNUS) += clk-cygnus.o > > obj-$(CONFIG_CLK_BCM_HR2) += clk-hr2.o > > diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk- > > raspberrypi.c > > new file mode 100644 > > index ..485c00288414 > > --- /dev/null > > +++ b/drivers/clk/bcm/clk-raspberrypi.c > > @@ -0,0 +1,316 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (C) 2019 Nicolas Saenz Julienne > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > + > > +#define RPI_FIRMWARE_ARM_CLK_ID0x3 > > + > > +#define RPI_FIRMWARE_STATE_ENABLE_BIT 0x1 > > +#define RPI_FIRMWARE_STATE_WAIT_BIT0x2 > how about using the BIT() macro? > > + > > +/* > > + * Even though the firmware interface alters 'pllb' the frequencies are > > + * provided as per 'pllb_arm'. We need to scale before passing them trough. > > + */ > > +#define RPI_FIRMWARE_PLLB_ARM_DIV_RATE 2 > > + > > +#define A2W_PLL_FRAC_BITS 20 > > + > > +struct raspberrypi_clk { > > + struct device *dev; > > + struct rpi_firmware *firmware; > > + > > + unsigned long min_rate; > > + unsigned long max_rate; > > + > > + struct clk_hw pllb; > > + struct clk_hw *pllb_arm; > > + struct clk_lookup *pllb_arm_lookup; > > +}; > > + > > +/* > > + * Structure of the message passed to Raspberry Pi's firmware in order to > > + * change clock rates. The 'disable_turbo' option is only available to the > > ARM > > + * clock (pllb) which we enable by default as turbo mode will alter > > multiple > > + * clocks at once. > > + * > > + * Even though we're able to access the clock registers directly we're > > bound to > > + * use the firmware interface as the firmware ultimately takes care of > > + * mitigating overheating/undervoltage situations and we would be changing > > + * frequencies behind his back. > > + * > > + * For more information on the firmware interface check: > > + * https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface > > + */ > > +struct raspberrypi_firmware_prop { > > + __le32 id; > > + __le32 val; > > + __le32 disable_turbo; > > +} __packed; > > + > > +static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 > > tag, > > + u32 clk, u32 *val) > > +{ > > + struct raspberrypi_firmware_prop msg = { > > + .id = clk, > > + .val = *val, > > + .disable_turbo = 1, > > + }; > > + int ret; > > + > > + ret = rpi_firmware
Re: [PATCH 0/4] cpufreq support for the Raspberry Pi
Hi Stefan, thanks for the review, I took note of your code comments. On Wed, 2019-06-05 at 11:46 +0200, Stefan Wahren wrote: > Hi Nicolas, > > Am 04.06.19 um 19:32 schrieb Nicolas Saenz Julienne: > > Hi all, > > this series aims at adding cpufreq support to the Raspberry Pi family of > > boards. > > > > The previous revision can be found at: https://lkml.org/lkml/2019/5/20/431 > > > > The series first factors out 'pllb' from clk-bcm2385 and creates a new > > clk driver that operates it over RPi's firmware interface[1]. We are > > forced to do so as the firmware 'owns' the pll and we're not allowed to > > change through the register interface directly as we might race with the > > over-temperature and under-voltage protections provided by the firmware. > it would be nice to preserve such design decision in the driver as a > comment, because the cover letter usually get lost. > > Next it creates a minimal cpufreq driver that populates the CPU's opp > > table, and registers cpufreq-dt. Which is needed as the firmware > > controls the max and min frequencies available. > > I tested your series on top of Linux 5.2-rc1 with multi_v7_defconfig and > manually enable this drivers. During boot with Raspbian rootfs i'm > getting the following: > > [1.177009] cpu cpu0: failed to get clock: -2 > [1.183643] cpufreq-dt: probe of cpufreq-dt failed with error -2 This is surprising, who could be creating a platform_device for cpufreq-dt apart from raspberrypi-cpufreq? Just to make things clear, you're using the device tree from v5.2-rc1 (as opposed to the Raspbian one)? > [1.192117] sdhci: Secure Digital Host Controller Interface driver > [1.198725] sdhci: Copyright(c) Pierre Ossman > [1.207005] Synopsys Designware Multimedia Card Interface Driver > [1.319936] sdhost-bcm2835 3f202000.mmc: loaded - DMA enabled (>1) > [1.326641] sdhci-pltfm: SDHCI platform and OF driver helper > [1.336568] ledtrig-cpu: registered to indicate activity on CPUs > [1.343713] usbcore: registered new interface driver usbhid > [1.350275] usbhid: USB HID core driver > [1.357639] bcm2835-mbox 3f00b880.mailbox: mailbox enabled > [1.367490] NET: Registered protocol family 10 > [1.375013] Segment Routing with IPv6 > [1.381696] sit: IPv6, IPv4 and MPLS over IPv4 tunneling driver > [1.388980] NET: Registered protocol family 17 > [1.395624] can: controller area network core (rev 20170425 abi 9) > [1.402358] NET: Registered protocol family 29 > [1.408997] can: raw protocol (rev 20170425) > [1.415599] can: broadcast manager protocol (rev 20170425 t) > [1.422219] can: netlink gateway (rev 20170425) max_hops=1 > [1.429369] Key type dns_resolver registered > [1.437190] Registering SWP/SWPB emulation handler > [1.43] Loading compiled-in X.509 certificates > [1.455693] 3f201000.serial: ttyAMA0 at MMIO 0x3f201000 (irq = 81, > base_baud = 0) is a PL011 rev2 > [1.462768] serial serial0: tty port ttyAMA0 registered > [1.478755] mmc0: host does not support reading read-only switch, > assuming write-enable > [1.488792] mmc0: new high speed SDHC card at address 0007 > [1.495766] raspberrypi-firmware soc:firmware: Attached to firmware > from 2019-03-27 15:45 > [1.496862] mmcblk0: mmc0:0007 SDCIT 14.6 GiB > [1.512768] raspberrypi-clk raspberrypi-clk: CPU frequency range: min > 6, max 14 > [1.513012] mmcblk0: p1 p2 > [1.558085] dwc2 3f98.usb: 3f98.usb supply vusb_d not found, > using dummy regulator > [1.565355] dwc2 3f98.usb: 3f98.usb supply vusb_a not found, > using dummy regulator > [1.623246] dwc2 3f98.usb: DWC OTG Controller > [1.630318] dwc2 3f98.usb: new USB bus registered, assigned bus > number 1 > [1.637439] dwc2 3f98.usb: irq 33, io mem 0x3f98 > [1.645268] hub 1-0:1.0: USB hub found > [1.652317] hub 1-0:1.0: 1 port detected > [1.665867] sdhci-iproc 3f30.sdhci: allocated mmc-pwrseq > [1.704788] mmc1: SDHCI controller on 3f30.sdhci [3f30.sdhci] > using PIO > [1.717694] hctosys: unable to open rtc device (rtc0) > [1.724967] sysfs: cannot create duplicate filename > '/devices/platform/cpufreq-dt' For the record, this is raspberrypi-cpufreq creating the platform device for cpufreq-dt. > [1.732120] CPU: 1 PID: 1 Comm: swapper/0 Not tainted > 5.2.0-rc1-4-g5aa6d98-dirty #2 > [1.739288] Hardware name: BCM2835 > [1.746421] [] (unwind_backtrace) from [] > (show_stack+0x10/0x14) > [1.753636] [] (show_stack) from [] > (dump_stack+0xb4/0xc8) > [1.760840] [] (dump_stack) from [] > (sysfs_warn_dup+0x58/0x64) > [1
Re: [PATCH 4/4] cpufreq: add driver for Raspbery Pi
Hi Eric, On Tue, 2019-06-04 at 17:18 -0700, Eric Anholt wrote: > Nicolas Saenz Julienne writes: > > > Raspberry Pi's firmware offers and interface though which update it's > > performance requirements. It allows us to request for specific runtime > > frequencies, which the firmware might or might not respect, depending on > > the firmware configuration and thermals. > > > > As the maximum and minimum frequencies are configurable in the firmware > > there is no way to know in advance their values. So the Raspberry Pi > > cpufreq driver queries them, builds an opp frequency table to then > > launch cpufreq-dt. > > > > Signed-off-by: Nicolas Saenz Julienne > > --- > > > > Changes since RFC: > > - Alphabetically ordered relevant stuff > > - Updated Kconfig to select firmware interface > > - Correctly unref clk_dev after use > > - Remove all opps on failure > > - Remove use of dev_pm_opp_set_sharing_cpus() > > > > drivers/cpufreq/Kconfig.arm | 8 +++ > > drivers/cpufreq/Makefile | 1 + > > drivers/cpufreq/raspberrypi-cpufreq.c | 84 +++ > > 3 files changed, 93 insertions(+) > > create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c > > > > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm > > index f8129edc145e..556d432cc826 100644 > > --- a/drivers/cpufreq/Kconfig.arm > > +++ b/drivers/cpufreq/Kconfig.arm > > @@ -133,6 +133,14 @@ config ARM_QCOM_CPUFREQ_HW > > The driver implements the cpufreq interface for this HW engine. > > Say Y if you want to support CPUFreq HW. > > > > +config ARM_RASPBERRYPI_CPUFREQ > > + tristate "Raspberry Pi cpufreq support" > > + select RASPBERRYPI_FIRMWARE > > + help > > + This adds the CPUFreq driver for Raspberry Pi > > + > > + If in doubt, say N. > > + > > config ARM_S3C_CPUFREQ > > bool > > help > > diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile > > index 689b26c6f949..121c1acb66c0 100644 > > --- a/drivers/cpufreq/Makefile > > +++ b/drivers/cpufreq/Makefile > > @@ -64,6 +64,7 @@ obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o > > obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o > > obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o > > obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)+= qcom-cpufreq-kryo.o > > +obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ) += raspberrypi-cpufreq.o > > obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o > > obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o > > obj-$(CONFIG_ARM_S3C2416_CPUFREQ) += s3c2416-cpufreq.o > > diff --git a/drivers/cpufreq/raspberrypi-cpufreq.c > > b/drivers/cpufreq/raspberrypi-cpufreq.c > > new file mode 100644 > > index ..2b3a195a9d37 > > --- /dev/null > > +++ b/drivers/cpufreq/raspberrypi-cpufreq.c > > @@ -0,0 +1,84 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Raspberry Pi cpufreq driver > > + * > > + * Copyright (C) 2019, Nicolas Saenz Julienne > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +static const struct of_device_id machines[] __initconst = { > > + { .compatible = "raspberrypi,3-model-b-plus" }, > > + { .compatible = "raspberrypi,3-model-b" }, > > + { .compatible = "raspberrypi,2-model-b" }, > > + { /* sentinel */ } > > +}; > > I think I'd skip the compatible string check here. The firmware's > clock-management should be well-tested by folks playing with clocking in > the downstream tree. There aren't any firmware differences in the > processing of these clock management packets, to my recollection. Fair enough, I'll remove it. > Other than that, I'm happy with the series and would give it my > acked-by. Thanks! Regads, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH 3/4] clk: bcm2835: register Raspberry Pi's firmware clk device
On Tue, 2019-06-04 at 17:00 -0700, Eric Anholt wrote: > Nicolas Saenz Julienne writes: > > > Registers clk-raspberrypi as a platform device as part of the driver's > > probe sequence. > > Similar to how we have VCHI register platform devices for the services > VCHI provides, shouldn't we have the firmware driver register the device > for clk_raspberrypi? Or put the clk provider in the fw driver instead > of a separate driver (no opinion on my part). Makes sense to me, I'll move the platform driver registration into the firmware driver. signature.asc Description: This is a digitally signed message part
[PATCH 4/4] cpufreq: add driver for Raspbery Pi
Raspberry Pi's firmware offers and interface though which update it's performance requirements. It allows us to request for specific runtime frequencies, which the firmware might or might not respect, depending on the firmware configuration and thermals. As the maximum and minimum frequencies are configurable in the firmware there is no way to know in advance their values. So the Raspberry Pi cpufreq driver queries them, builds an opp frequency table to then launch cpufreq-dt. Signed-off-by: Nicolas Saenz Julienne --- Changes since RFC: - Alphabetically ordered relevant stuff - Updated Kconfig to select firmware interface - Correctly unref clk_dev after use - Remove all opps on failure - Remove use of dev_pm_opp_set_sharing_cpus() drivers/cpufreq/Kconfig.arm | 8 +++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/raspberrypi-cpufreq.c | 84 +++ 3 files changed, 93 insertions(+) create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index f8129edc145e..556d432cc826 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -133,6 +133,14 @@ config ARM_QCOM_CPUFREQ_HW The driver implements the cpufreq interface for this HW engine. Say Y if you want to support CPUFreq HW. +config ARM_RASPBERRYPI_CPUFREQ + tristate "Raspberry Pi cpufreq support" + select RASPBERRYPI_FIRMWARE + help + This adds the CPUFreq driver for Raspberry Pi + + If in doubt, say N. + config ARM_S3C_CPUFREQ bool help diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index 689b26c6f949..121c1acb66c0 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -64,6 +64,7 @@ obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)+= qcom-cpufreq-kryo.o +obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ) += raspberrypi-cpufreq.o obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o obj-$(CONFIG_ARM_S3C2416_CPUFREQ) += s3c2416-cpufreq.o diff --git a/drivers/cpufreq/raspberrypi-cpufreq.c b/drivers/cpufreq/raspberrypi-cpufreq.c new file mode 100644 index ..2b3a195a9d37 --- /dev/null +++ b/drivers/cpufreq/raspberrypi-cpufreq.c @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Raspberry Pi cpufreq driver + * + * Copyright (C) 2019, Nicolas Saenz Julienne + */ + +#include +#include +#include +#include +#include +#include +#include + +static const struct of_device_id machines[] __initconst = { + { .compatible = "raspberrypi,3-model-b-plus" }, + { .compatible = "raspberrypi,3-model-b" }, + { .compatible = "raspberrypi,2-model-b" }, + { /* sentinel */ } +}; + +static int __init raspberrypi_cpufreq_driver_init(void) +{ + struct platform_device *pdev; + struct device *cpu_dev; + unsigned long min, max; + unsigned long rate; + struct clk *clk; + int ret; + + if (!of_match_node(machines, of_root)) + return -ENODEV; + + cpu_dev = get_cpu_device(0); + if (!cpu_dev) { + pr_err("Cannot get CPU for cpufreq driver\n"); + return -ENODEV; + } + + clk = clk_get(cpu_dev, 0); + if (IS_ERR(clk)) { + dev_err(cpu_dev, "Cannot get clock for CPU0\n"); + return PTR_ERR(clk); + } + + /* +* The max and min frequencies are configurable in the Raspberry Pi +* firmware, so we query them at runtime +*/ + min = clk_round_rate(clk, 0); + max = clk_round_rate(clk, ULONG_MAX); + clk_put(clk); + + for (rate = min; rate < max; rate += 1) { + ret = dev_pm_opp_add(cpu_dev, rate, 0); + if (ret) + goto remove_opp; + } + + ret = dev_pm_opp_add(cpu_dev, max, 0); + if (ret) + goto remove_opp; + + pdev = platform_device_register_data(NULL, "cpufreq-dt", -1, NULL, 0); + ret = PTR_ERR_OR_ZERO(pdev); + if (ret) { + dev_err(cpu_dev, "Failed to create platform device, %d\n", ret); + goto remove_opp; + } + + return 0; + +remove_opp: + dev_pm_opp_remove_all_dynamic(cpu_dev); + + return ret; +} + +late_initcall(raspberrypi_cpufreq_driver_init); + +MODULE_AUTHOR("Nicolas Saenz Julienne
[PATCH 3/4] clk: bcm2835: register Raspberry Pi's firmware clk device
Registers clk-raspberrypi as a platform device as part of the driver's probe sequence. Signed-off-by: Nicolas Saenz Julienne --- drivers/clk/bcm/clk-bcm2835.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index ccb0319fc2e9..6f370e6bafed 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -2098,6 +2098,7 @@ static int bcm2835_mark_sdc_parent_critical(struct clk *sdc) static int bcm2835_clk_probe(struct platform_device *pdev) { + struct platform_device *rpi_fw_clk; struct device *dev = >dev; struct clk_hw **hws; struct bcm2835_cprman *cprman; @@ -2150,8 +2151,18 @@ static int bcm2835_clk_probe(struct platform_device *pdev) if (ret) return ret; - return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get, - >onecell); + ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get, +>onecell); + if (ret) + return ret; + + rpi_fw_clk = platform_device_register_data(NULL, "raspberrypi-clk", -1, + NULL, 0); + ret = PTR_ERR_OR_ZERO(rpi_fw_clk); + if (ret) + dev_err(dev, "Failed to create platform device, %d\n", ret); + + return ret; } static const struct of_device_id bcm2835_clk_of_match[] = { -- 2.21.0
[PATCH 2/4] clk: bcm283x: add driver interfacing with Raspberry Pi's firmware
Raspberry Pi's firmware offers and interface though which update it's clock's frequencies. This is specially useful in order to change the CPU clock (pllb_arm) which is 'owned' by the firmware and we're unable to scale using the register interface. Signed-off-by: Nicolas Saenz Julienne --- Changes since RFC: - Moved firmware interface into own driver - Use of_find_compatible_node() - Remove error message on rpi_firmware_get() failure - Ratelimit messages on set_rate() failure - Use __le32 on firmware interface definition drivers/clk/bcm/Makefile | 1 + drivers/clk/bcm/clk-raspberrypi.c | 316 ++ 2 files changed, 317 insertions(+) create mode 100644 drivers/clk/bcm/clk-raspberrypi.c diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile index 002661d39128..07abe92df9d1 100644 --- a/drivers/clk/bcm/Makefile +++ b/drivers/clk/bcm/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm21664.o obj-$(CONFIG_COMMON_CLK_IPROC) += clk-iproc-armpll.o clk-iproc-pll.o clk-iproc-asiu.o obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835-aux.o +obj-$(CONFIG_ARCH_BCM2835) += clk-raspberrypi.o obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o obj-$(CONFIG_CLK_BCM_CYGNUS) += clk-cygnus.o obj-$(CONFIG_CLK_BCM_HR2) += clk-hr2.o diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c new file mode 100644 index ..485c00288414 --- /dev/null +++ b/drivers/clk/bcm/clk-raspberrypi.c @@ -0,0 +1,316 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2019 Nicolas Saenz Julienne + */ + +#include +#include +#include +#include +#include + +#include + +#define RPI_FIRMWARE_ARM_CLK_ID0x3 + +#define RPI_FIRMWARE_STATE_ENABLE_BIT 0x1 +#define RPI_FIRMWARE_STATE_WAIT_BIT0x2 + +/* + * Even though the firmware interface alters 'pllb' the frequencies are + * provided as per 'pllb_arm'. We need to scale before passing them trough. + */ +#define RPI_FIRMWARE_PLLB_ARM_DIV_RATE 2 + +#define A2W_PLL_FRAC_BITS 20 + +struct raspberrypi_clk { + struct device *dev; + struct rpi_firmware *firmware; + + unsigned long min_rate; + unsigned long max_rate; + + struct clk_hw pllb; + struct clk_hw *pllb_arm; + struct clk_lookup *pllb_arm_lookup; +}; + +/* + * Structure of the message passed to Raspberry Pi's firmware in order to + * change clock rates. The 'disable_turbo' option is only available to the ARM + * clock (pllb) which we enable by default as turbo mode will alter multiple + * clocks at once. + * + * Even though we're able to access the clock registers directly we're bound to + * use the firmware interface as the firmware ultimately takes care of + * mitigating overheating/undervoltage situations and we would be changing + * frequencies behind his back. + * + * For more information on the firmware interface check: + * https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface + */ +struct raspberrypi_firmware_prop { + __le32 id; + __le32 val; + __le32 disable_turbo; +} __packed; + +static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 tag, + u32 clk, u32 *val) +{ + struct raspberrypi_firmware_prop msg = { + .id = clk, + .val = *val, + .disable_turbo = 1, + }; + int ret; + + ret = rpi_firmware_property(firmware, tag, , sizeof(msg)); + if (ret) + return ret; + + *val = msg.val; + + return 0; +} + +static int raspberrypi_fw_pll_is_on(struct clk_hw *hw) +{ + struct raspberrypi_clk *rpi = container_of(hw, struct raspberrypi_clk, + pllb); + u32 val = 0; + int ret; + + ret = raspberrypi_clock_property(rpi->firmware, +RPI_FIRMWARE_GET_CLOCK_STATE, +RPI_FIRMWARE_ARM_CLK_ID, ); + if (ret) + return 0; + + return !!(val & RPI_FIRMWARE_STATE_ENABLE_BIT); +} + + +static unsigned long raspberrypi_fw_pll_get_rate(struct clk_hw *hw, +unsigned long parent_rate) +{ + struct raspberrypi_clk *rpi = container_of(hw, struct raspberrypi_clk, + pllb); + u32 val = 0; + int ret; + + ret = raspberrypi_clock_property(rpi->firmware, +RPI_FIRMWARE_GET_CLOCK_RATE, +RPI_FIRMWARE_ARM_CLK_ID, +); + if (ret) + return ret; + + return val * RPI_FIRMWARE_PLLB_ARM_DIV_RATE; +} + +static int raspberrypi_fw_pll_on(struct clk_hw *hw) +{ + struct raspberrypi_clk *rpi = conta
[PATCH 1/4] clk: bcm2835: remove pllb
Raspberry Pi's firmware controls this pll, we should use the firmware interface to access it. Signed-off-by: Nicolas Saenz Julienne --- drivers/clk/bcm/clk-bcm2835.c | 25 - 1 file changed, 25 deletions(-) diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index 770bb01f523e..ccb0319fc2e9 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -1651,31 +1651,6 @@ static const struct bcm2835_clk_desc clk_desc_array[] = { .fixed_divider = 1, .flags = CLK_SET_RATE_PARENT), - /* PLLB is used for the ARM's clock. */ - [BCM2835_PLLB] = REGISTER_PLL( - .name = "pllb", - .cm_ctrl_reg = CM_PLLB, - .a2w_ctrl_reg = A2W_PLLB_CTRL, - .frac_reg = A2W_PLLB_FRAC, - .ana_reg_base = A2W_PLLB_ANA0, - .reference_enable_mask = A2W_XOSC_CTRL_PLLB_ENABLE, - .lock_mask = CM_LOCK_FLOCKB, - - .ana = _ana_default, - - .min_rate = 6u, - .max_rate = 30u, - .max_fb_rate = BCM2835_MAX_FB_RATE), - [BCM2835_PLLB_ARM] = REGISTER_PLL_DIV( - .name = "pllb_arm", - .source_pll = "pllb", - .cm_reg = CM_PLLB, - .a2w_reg = A2W_PLLB_ARM, - .load_mask = CM_PLLB_LOADARM, - .hold_mask = CM_PLLB_HOLDARM, - .fixed_divider = 1, - .flags = CLK_SET_RATE_PARENT), - /* * PLLC is the core PLL, used to drive the core VPU clock. * -- 2.21.0
[PATCH 0/4] cpufreq support for the Raspberry Pi
Hi all, this series aims at adding cpufreq support to the Raspberry Pi family of boards. The previous revision can be found at: https://lkml.org/lkml/2019/5/20/431 The series first factors out 'pllb' from clk-bcm2385 and creates a new clk driver that operates it over RPi's firmware interface[1]. We are forced to do so as the firmware 'owns' the pll and we're not allowed to change through the register interface directly as we might race with the over-temperature and under-voltage protections provided by the firmware. Next it creates a minimal cpufreq driver that populates the CPU's opp table, and registers cpufreq-dt. Which is needed as the firmware controls the max and min frequencies available. This was tested on a RPi3b+ and RPI2b which are the boards I have access to. Until this is tested broadly the cpufreq driver takes care of filtering out the rest of boards. That's all, kind regards, Nicolas [1] https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface --- Changes since RFC: - Addressed Viresh's comments in cpufreq driver - Addressed Stefan's comments in both cpufreq & clk drivers - Moved all firmware clk operations into it's own driver Nicolas Saenz Julienne (4): clk: bcm2835: remove pllb clk: bcm283x: add driver interfacing with Raspberry Pi's firmware clk: bcm2835: register Raspberry Pi's firmware clk device cpufreq: add driver for Raspbery Pi drivers/clk/bcm/Makefile | 1 + drivers/clk/bcm/clk-bcm2835.c | 40 ++-- drivers/clk/bcm/clk-raspberrypi.c | 316 ++ drivers/cpufreq/Kconfig.arm | 8 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/raspberrypi-cpufreq.c | 84 +++ 6 files changed, 423 insertions(+), 27 deletions(-) create mode 100644 drivers/clk/bcm/clk-raspberrypi.c create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c -- 2.21.0
Re: [PATCH] xhci: clear port_remote_wakeup after resume failure
Hi Matthias, thanks for the review. On Mon, 2019-05-27 at 14:16 +0300, Mathias Nyman wrote: > On 24.5.2019 17.52, Nicolas Saenz Julienne wrote: > > This was seen on a Dell Precision 5520 using it's WD15 dock. The dock's > > Ethernet device interfaces with the laptop through one of it's USB3 > > ports. While idle, the Ethernet device and HCD are suspended by runtime > > PM, being the only device connected on the bus. Then, both are resumed on > > behalf of the Ethernet device, which has remote wake-up capabilities. > > > > The Ethernet device was observed to randomly disconnect from the USB > > port shortly after submitting it's remote wake-up request. Probably a > > weird timing issue yet to be investigated. This causes runtime PM to > > busyloop causing some tangible CPU load. The reason is the port gets > > stuck in the middle of a remote wake-up operation, waiting for the > > device to switch to U0. This never happens, leaving "port_remote_wakeup" > > enabled, and automatically triggering a failure on any further suspend > > operation. > > > > This patch clears "port_remote_wakeup" upon detecting a device with a > > wrong resuming port state (see Table 4-9 in 4.15.2.3). Making sure the > > above mentioned situation doesn't trigger a PM busyloop. > > > > There was a similar case where the USB3 link had transitioned to a > lower power U1 or U2 state after resume, before driver read the state, > leaving port_remote_wakeup flag uncleared. > > This was fixed in 5.1 kernel by commit: > > 6cbcf59 xhci: Fix port resume done detection for SS ports with LPM enable > > Can you check if you have it? > It should be in recent stable releases as well. I was aware of that patch, unfortunately it doesn't address the same issue. In my case I never get a second port status event (so no PLC == 1 or any state change seen in PLS). The device simply disconnects from the bus. I did test both the issue and fix on top of last week's master branch. Regards, Nicolas signature.asc Description: This is a digitally signed message part
[PATCH] xhci: clear port_remote_wakeup after resume failure
This was seen on a Dell Precision 5520 using it's WD15 dock. The dock's Ethernet device interfaces with the laptop through one of it's USB3 ports. While idle, the Ethernet device and HCD are suspended by runtime PM, being the only device connected on the bus. Then, both are resumed on behalf of the Ethernet device, which has remote wake-up capabilities. The Ethernet device was observed to randomly disconnect from the USB port shortly after submitting it's remote wake-up request. Probably a weird timing issue yet to be investigated. This causes runtime PM to busyloop causing some tangible CPU load. The reason is the port gets stuck in the middle of a remote wake-up operation, waiting for the device to switch to U0. This never happens, leaving "port_remote_wakeup" enabled, and automatically triggering a failure on any further suspend operation. This patch clears "port_remote_wakeup" upon detecting a device with a wrong resuming port state (see Table 4-9 in 4.15.2.3). Making sure the above mentioned situation doesn't trigger a PM busyloop. Signed-off-by: Nicolas Saenz Julienne --- drivers/usb/host/xhci-hub.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 3abe70ff1b1e..53f5ee50ef8c 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -1047,8 +1047,8 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd, xhci_get_usb2_port_status(port, , raw_port_status, flags); /* -* Clear stale usb2 resume signalling variables in case port changed -* state during resume signalling. For example on error +* Clear stale resume signalling variables in case port changed +* state during resume signalling. For example on error. */ if ((bus_state->resume_done[wIndex] || test_bit(wIndex, _state->resuming_ports)) && @@ -1057,6 +1057,12 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd, bus_state->resume_done[wIndex] = 0; clear_bit(wIndex, _state->resuming_ports); usb_hcd_end_port_resume(>self, wIndex); + } else if (bus_state->port_remote_wakeup & (1 << port->hcd_portnum) && + ((raw_port_status & PORT_PLS_MASK) != XDEV_RESUME || + !(raw_port_status & PORT_CONNECT) || + !(raw_port_status & PORT_PE) || + raw_port_status & PORT_OC)) { + bus_state->port_remote_wakeup &= ~(1 << port->hcd_portnum); } if (bus_state->port_c_suspend & (1 << wIndex)) -- 2.21.0
Re: [RFC v2 3/5] clk: bcm2835: use firmware interface to update pllb
On Tue, 2019-05-21 at 23:43 +0200, Stefan Wahren wrote: > > Nicolas Saenz Julienne hat am 21. Mai 2019 um 17:47 > > geschrieben: > > > > > > Hi Stefan, thanks for your comments! > > > > On Tue, 2019-05-21 at 14:40 +0200, Stefan Wahren wrote: > > > Hi Nicolas, > > > > > > On 20.05.19 14:11, Stefan Wahren wrote: > > > > Hi Nicolas, > > > > > > > > the following comments applies only in case Eric is fine with the whole > > > > approach. > > > > > > > > On 20.05.19 12:47, Nicolas Saenz Julienne wrote: > > > > > Raspberry Pi's firmware, which runs in a dedicated processor, keeps > > > > maybe we should clarify that the firmware is running in the VPU > > > > > track of the board's temperature and voltage. It's resposible for > > > > > scaling the CPU frequency whenever it deems the device reached an > > > > > unsafe > > > > > state. On top of that the firmware provides an interface which allows > > > > > Linux to to query the clock's state or change it's frequency. > > > > I think this requires a separate update of the devicetree binding. > > > > > Being the sole user of the bcm2835 clock driver, this integrates the > > > > > firmware interface into the clock driver and adds a first user: the > > > > > CPU > > > > > pll, also known as 'pllb'. > > > > Please verify that the kernel still works (and this clock driver probe) > > > > under the following conditions: > > > > > > > > - CONFIG_RASPBERRYPI_FIRMWARE=n > > > > - CONFIG_RASPBERRYPI_FIRMWARE=m > > > > - older DTBs without patch #1 > > > i thought about this and the case this driver would return > > > -EPROBE_DEFER. The clock driver is too essential for doing such a thing. > > > So i think the best solution would be to move these changes into a > > > separate driver which should be register by the clock driver (similiar > > > to vchiq). This also avoid the need of a new device tree binding. > > > > I understand your concerns. > > > > Wouldn't you prefer registering the device trough the device tree? I'd go > > with > > the same approach as the firmware touchscreen driver, which is registered > > after > > the firmware's probe trough dt's 'simple-bus'. That said, it's not a > > strongly > > held opinion, I'm happy with whatever solution as long as it works. > > A devicetree binding always introduce some kind of inflexibility. In case > someone finds a better solution later things can get really messy. A recent > example is the clock handling for i2c-bcm2835. Fair enough. > > I get from your comments that you'd like the register based version of > > 'pllb' > > and 'pllb_arm' to be loaded if for some reason the firmware isn't available. > > Is > > that right? > > This wasn't my intention. I would prefer a simple approch here (no handover). > > > The main problem I see with this is the duplication of 'pllb' and > > 'pllb_arm'. Both drivers will create the same clock device through different > > interfaces. Any suggestions on how to deal with that? If not I can simply > > remove 'pllb' and 'pllb_arm' from clk-bcm2835.c. > > Yes. So even if this driver is disabled, there shouldn't be a regression. Or > did i miss something? No, there shoudn't be any regressions as these clocks are not being used at the moment. I'll send a follow-up series soon :) Regrads, Nicolas signature.asc Description: This is a digitally signed message part
Re: [RFC v2 3/5] clk: bcm2835: use firmware interface to update pllb
Hi Stefan, thanks for your comments! On Tue, 2019-05-21 at 14:40 +0200, Stefan Wahren wrote: > Hi Nicolas, > > On 20.05.19 14:11, Stefan Wahren wrote: > > Hi Nicolas, > > > > the following comments applies only in case Eric is fine with the whole > > approach. > > > > On 20.05.19 12:47, Nicolas Saenz Julienne wrote: > > > Raspberry Pi's firmware, which runs in a dedicated processor, keeps > > maybe we should clarify that the firmware is running in the VPU > > > track of the board's temperature and voltage. It's resposible for > > > scaling the CPU frequency whenever it deems the device reached an unsafe > > > state. On top of that the firmware provides an interface which allows > > > Linux to to query the clock's state or change it's frequency. > > I think this requires a separate update of the devicetree binding. > > > Being the sole user of the bcm2835 clock driver, this integrates the > > > firmware interface into the clock driver and adds a first user: the CPU > > > pll, also known as 'pllb'. > > Please verify that the kernel still works (and this clock driver probe) > > under the following conditions: > > > > - CONFIG_RASPBERRYPI_FIRMWARE=n > > - CONFIG_RASPBERRYPI_FIRMWARE=m > > - older DTBs without patch #1 > i thought about this and the case this driver would return > -EPROBE_DEFER. The clock driver is too essential for doing such a thing. > So i think the best solution would be to move these changes into a > separate driver which should be register by the clock driver (similiar > to vchiq). This also avoid the need of a new device tree binding. I understand your concerns. Wouldn't you prefer registering the device trough the device tree? I'd go with the same approach as the firmware touchscreen driver, which is registered after the firmware's probe trough dt's 'simple-bus'. That said, it's not a strongly held opinion, I'm happy with whatever solution as long as it works. I get from your comments that you'd like the register based version of 'pllb' and 'pllb_arm' to be loaded if for some reason the firmware isn't available. Is that right? The main problem I see with this is the duplication of 'pllb' and 'pllb_arm'. Both drivers will create the same clock device through different interfaces. Any suggestions on how to deal with that? If not I can simply remove 'pllb' and 'pllb_arm' from clk-bcm2835.c. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [RFC v2 3/5] clk: bcm2835: use firmware interface to update pllb
On Tue, 2019-05-21 at 14:14 +0200, Petr Tesarik wrote: > On Tue, 21 May 2019 13:39:31 +0200 > Nicolas Saenz Julienne wrote: > > > Hi Oliver, thanks for the review. > > > > On Mon, 2019-05-20 at 14:43 +0200, Oliver Neukum wrote: > > > On Mo, 2019-05-20 at 12:47 +0200, Nicolas Saenz Julienne wrote: > > > > + * For more information on the firmware interface check: > > > > + * > > > > https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface > > > > + */ > > > > +struct bcm2835_firmware_prop { > > > > + u32 id; > > > > + u32 val; > > > > + u32 disable_turbo; > > > > +} __packed; > > > > > > Hi, > > > > > > technically we are not in arch and those fields have a defined > > > endianness. > > > > > > > Well I set it as packed since it's 'sent' through a memory mapped firmware > > interface. Hence the need for the structure format to be fixed. So I guessed > > we're safer with it, as I'm not 100% sure what the different compilers are > > going to do with it (although it's very likely it'll stay the same). BTW > > this > > will be built both for arm & arm64. > > I believe that's not the point Oliver was trying to make. You should > use __le32 instead of u32. > > That's because u32 means "host byte order" and this code is not located > under arch/, so host endianness is unknown, but the mailbox interface > requires little-endian. > > It's nit-picking, and that's why Oliver writes 'technically'; there is > probably no way this firmware interface could be used on a big-endian > CPU... Understood, thanks for the clarification. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [RFC v2 0/5] cpufreq support for the Raspberry Pi
Hi Viresh, thanks for the comments. On Mon, 2019-05-20 at 16:21 +0530, Viresh Kumar wrote: > On 20-05-19, 12:47, Nicolas Saenz Julienne wrote: > > Hi all, > > as some of you may recall I've been spending some time looking into > > providing 'cpufreq' support for the Raspberry Pi platform[1]. I think > > I'm close to something workable, so I'd love for you to comment on it. > > > > There has been some design changes since the last version. Namely the > > fact that I now make sure *only* the CPU frequency is updated. The > > firmware API we use has two modes, with or without turbo. Enabling turbo > > implies not only scaling the CPU clock but also the VPU and other > > peripheral related clocks. This is problematic as some of them are not > > prepared for this kind frequency changes. I spent some time adapting the > > peripheral drivers, but the result was disappointing as they poorly > > support live frequency changes (which most other chips accept, think for > > instance I2C and clock stretching) but also turned out hard to integrate > > into the kernel. As we were planning to use 'clk_notifiers' which turns > > out not to be such a good idea as it's prone to deadlocks and not > > recommended by the clock maintainers[2]. It's also worth mentioning that > > the foundation kernel doesn't support VPU frequency scaling either. > > > > With this in mind, and as suggested by clock maintainers[2], I've > > decided to integrate the firmware clock interface into the bcm2835 clock > > driver. This, in my opinion, provides the least friction with the > > firmware and lets us write very simple and portable higher level > > drivers. As I did with the 'cpufreq' driver which simply queries the max > > and min frequencies available, which are configurable in the firmware, > > to then trigger the generic 'cpufreq-dt'. > > > > In the future we could further integrate other firmware dependent clocks > > into the main driver. For instance to be able to scale the VPU clock, > > which should be operated through a 'devfreq' driver. > > > > This was tested on a RPi3b+ and if the series is well received I'll test > > it further on all platforms I own. > > Please always supply version history on what has changed from V1. Will do > And why do you keep sending it as RFC ? Well it's because of patch #3 which integrates the firmware interface into the clock driver. I want some approval from the maintainers before cleaning it up testing it on all RPi versions. > Just keep the default PATCH thing,the patches are in good shape I would say. Thanks :) Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [RFC v2 4/5] dts: bcm2837: add per-cpu clock devices
On Mon, 2019-05-20 at 14:19 +0200, Stefan Wahren wrote: > Hi Nicolas, > > On 20.05.19 12:47, Nicolas Saenz Julienne wrote: > > The four CPUs share a same clock source called pllb_arm. The clock can > > be scaled through the raspberrypi firmware interface. > do you see a problem with applying this also to bcm2835.dtsi and > bcm2836.dtsi? Not at all. I just need to test it first. signature.asc Description: This is a digitally signed message part
Re: [RFC v2 3/5] clk: bcm2835: use firmware interface to update pllb
Hi Oliver, thanks for the review. On Mon, 2019-05-20 at 14:43 +0200, Oliver Neukum wrote: > On Mo, 2019-05-20 at 12:47 +0200, Nicolas Saenz Julienne wrote: > > + * For more information on the firmware interface check: > > + * https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface > > + */ > > +struct bcm2835_firmware_prop { > > + u32 id; > > + u32 val; > > + u32 disable_turbo; > > +} __packed; > > Hi, > > technically we are not in arch and those fields have a defined > endianness. > Well I set it as packed since it's 'sent' through a memory mapped firmware interface. Hence the need for the structure format to be fixed. So I guessed we're safer with it, as I'm not 100% sure what the different compilers are going to do with it (although it's very likely it'll stay the same). BTW this will be built both for arm & arm64. Regards, Nicolas signature.asc Description: This is a digitally signed message part
[RFC v2 2/5] clk: bcm2835: set pllb_arm divisor as readonly
This divisor is controlled by the firmware, we don't want the clock subsystem to update it inadvertently. Signed-off-by: Nicolas Saenz Julienne --- drivers/clk/bcm/clk-bcm2835.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index c2772dfb155a..5aea110672f3 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -465,6 +465,7 @@ struct bcm2835_pll_divider_data { u32 hold_mask; u32 fixed_divider; u32 flags; + u32 div_flags; }; struct bcm2835_clock_data { @@ -1349,7 +1350,7 @@ bcm2835_register_pll_divider(struct bcm2835_cprman *cprman, divider->div.reg = cprman->regs + data->a2w_reg; divider->div.shift = A2W_PLL_DIV_SHIFT; divider->div.width = A2W_PLL_DIV_BITS; - divider->div.flags = CLK_DIVIDER_MAX_AT_ZERO; + divider->div.flags = data->div_flags | CLK_DIVIDER_MAX_AT_ZERO; divider->div.lock = >regs_lock; divider->div.hw.init = divider->div.table = NULL; @@ -1676,7 +1677,8 @@ static const struct bcm2835_clk_desc clk_desc_array[] = { .load_mask = CM_PLLB_LOADARM, .hold_mask = CM_PLLB_HOLDARM, .fixed_divider = 1, - .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE), + .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE, + .div_flags = CLK_DIVIDER_READ_ONLY), /* * PLLC is the core PLL, used to drive the core VPU clock. -- 2.21.0
[RFC v2 0/5] cpufreq support for the Raspberry Pi
Hi all, as some of you may recall I've been spending some time looking into providing 'cpufreq' support for the Raspberry Pi platform[1]. I think I'm close to something workable, so I'd love for you to comment on it. There has been some design changes since the last version. Namely the fact that I now make sure *only* the CPU frequency is updated. The firmware API we use has two modes, with or without turbo. Enabling turbo implies not only scaling the CPU clock but also the VPU and other peripheral related clocks. This is problematic as some of them are not prepared for this kind frequency changes. I spent some time adapting the peripheral drivers, but the result was disappointing as they poorly support live frequency changes (which most other chips accept, think for instance I2C and clock stretching) but also turned out hard to integrate into the kernel. As we were planning to use 'clk_notifiers' which turns out not to be such a good idea as it's prone to deadlocks and not recommended by the clock maintainers[2]. It's also worth mentioning that the foundation kernel doesn't support VPU frequency scaling either. With this in mind, and as suggested by clock maintainers[2], I've decided to integrate the firmware clock interface into the bcm2835 clock driver. This, in my opinion, provides the least friction with the firmware and lets us write very simple and portable higher level drivers. As I did with the 'cpufreq' driver which simply queries the max and min frequencies available, which are configurable in the firmware, to then trigger the generic 'cpufreq-dt'. In the future we could further integrate other firmware dependent clocks into the main driver. For instance to be able to scale the VPU clock, which should be operated through a 'devfreq' driver. This was tested on a RPi3b+ and if the series is well received I'll test it further on all platforms I own. That's all, kind regards, Nicolas [1] https://lists.infradead.org/pipermail/linux-rpi-kernel/2019-April/008634.html [2] https://www.spinics.net/lists/linux-clk/msg36937.html --- Changes since v1: - Addressed Viresh's comments in cpufreq driver - Resend with (hopefully) proper CCs Nicolas Saenz Julienne (5): clk: bcm2835: set CLK_GET_RATE_NOCACHE on CPU clocks clk: bcm2835: set pllb_arm divisor as readonly clk: bcm2835: use firmware interface to update pllb dts: bcm2837: add per-cpu clock devices cpufreq: add driver for Raspbery Pi arch/arm/boot/dts/bcm2837.dtsi| 8 + drivers/clk/bcm/clk-bcm2835.c | 284 -- drivers/cpufreq/Kconfig.arm | 8 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/raspberrypi-cpufreq.c | 83 5 files changed, 366 insertions(+), 18 deletions(-) create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c -- 2.21.0
[RFC v2 1/5] clk: bcm2835: set CLK_GET_RATE_NOCACHE on CPU clocks
Raspberry Pi's firmware is responsible for updating the cpu clocks and pll. This makes sure we get the right rates anytime. Signed-off-by: Nicolas Saenz Julienne --- drivers/clk/bcm/clk-bcm2835.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index 770bb01f523e..c2772dfb155a 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -411,6 +411,7 @@ struct bcm2835_pll_data { u32 reference_enable_mask; /* Bit in CM_LOCK to indicate when the PLL has locked. */ u32 lock_mask; + u32 flags; const struct bcm2835_pll_ana_bits *ana; @@ -1299,7 +1300,7 @@ static struct clk_hw *bcm2835_register_pll(struct bcm2835_cprman *cprman, init.num_parents = 1; init.name = data->name; init.ops = _pll_clk_ops; - init.flags = CLK_IGNORE_UNUSED; + init.flags = data->flags | CLK_IGNORE_UNUSED; pll = kzalloc(sizeof(*pll), GFP_KERNEL); if (!pll) @@ -1660,6 +1661,7 @@ static const struct bcm2835_clk_desc clk_desc_array[] = { .ana_reg_base = A2W_PLLB_ANA0, .reference_enable_mask = A2W_XOSC_CTRL_PLLB_ENABLE, .lock_mask = CM_LOCK_FLOCKB, + .flags = CLK_GET_RATE_NOCACHE, .ana = _ana_default, @@ -1674,7 +1676,7 @@ static const struct bcm2835_clk_desc clk_desc_array[] = { .load_mask = CM_PLLB_LOADARM, .hold_mask = CM_PLLB_HOLDARM, .fixed_divider = 1, - .flags = CLK_SET_RATE_PARENT), + .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE), /* * PLLC is the core PLL, used to drive the core VPU clock. -- 2.21.0
[RFC v2 4/5] dts: bcm2837: add per-cpu clock devices
The four CPUs share a same clock source called pllb_arm. The clock can be scaled through the raspberrypi firmware interface. Signed-off-by: Nicolas Saenz Julienne --- arch/arm/boot/dts/bcm2837.dtsi | 8 1 file changed, 8 insertions(+) diff --git a/arch/arm/boot/dts/bcm2837.dtsi b/arch/arm/boot/dts/bcm2837.dtsi index beb6c502dadc..a8fea6696b42 100644 --- a/arch/arm/boot/dts/bcm2837.dtsi +++ b/arch/arm/boot/dts/bcm2837.dtsi @@ -44,6 +44,8 @@ reg = <0>; enable-method = "spin-table"; cpu-release-addr = <0x0 0x00d8>; + clocks = < BCM2835_PLLB_ARM>; + clock-names = "pllb_arm"; }; cpu1: cpu@1 { @@ -52,6 +54,8 @@ reg = <1>; enable-method = "spin-table"; cpu-release-addr = <0x0 0x00e0>; + clocks = < BCM2835_PLLB_ARM>; + clock-names = "pllb_arm"; }; cpu2: cpu@2 { @@ -60,6 +64,8 @@ reg = <2>; enable-method = "spin-table"; cpu-release-addr = <0x0 0x00e8>; + clocks = < BCM2835_PLLB_ARM>; + clock-names = "pllb_arm"; }; cpu3: cpu@3 { @@ -68,6 +74,8 @@ reg = <3>; enable-method = "spin-table"; cpu-release-addr = <0x0 0x00f0>; + clocks = < BCM2835_PLLB_ARM>; + clock-names = "pllb_arm"; }; }; }; -- 2.21.0
[RFC v2 3/5] clk: bcm2835: use firmware interface to update pllb
Raspberry Pi's firmware, which runs in a dedicated processor, keeps track of the board's temperature and voltage. It's resposible for scaling the CPU frequency whenever it deems the device reached an unsafe state. On top of that the firmware provides an interface which allows Linux to to query the clock's state or change it's frequency. Being the sole user of the bcm2835 clock driver, this integrates the firmware interface into the clock driver and adds a first user: the CPU pll, also known as 'pllb'. Signed-off-by: Nicolas Saenz Julienne --- drivers/clk/bcm/clk-bcm2835.c | 274 -- 1 file changed, 259 insertions(+), 15 deletions(-) diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index 5aea110672f3..ce5b16f3704e 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -35,6 +35,7 @@ #include #include #include +#include #define CM_PASSWORD0x5a00 @@ -289,6 +290,30 @@ #define LOCK_TIMEOUT_NS1 #define BCM2835_MAX_FB_RATE175000u +#define RPI_FIRMWARE_ARM_CLK_ID0x3 +#define RPI_FIRMWARE_STATE_ENABLE_BIT 0x1 +#define RPI_FIRMWARE_STATE_WAIT_BIT0x2 + +/* + * Structure of the message passed to Raspberry Pi's firmware in order to + * change clock rates. The 'disable_turbo' option is only available to the ARM + * clock (pllb) which we enable by default as turbo mode will alter multiple + * clocks at once. + * + * Even though we're able to access the clock registers directly we're bound to + * use the firmware interface as the firmware ultimately takes care of + * mitigating overheating/undervoltage situations and we would be changing + * frequencies behind his back. + * + * For more information on the firmware interface check: + * https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface + */ +struct bcm2835_firmware_prop { + u32 id; + u32 val; + u32 disable_turbo; +} __packed; + /* * Names of clocks used within the driver that need to be replaced * with an external parent's name. This array is in the order that @@ -316,6 +341,8 @@ struct bcm2835_cprman { */ const char *real_parent_names[ARRAY_SIZE(cprman_parent_names)]; + struct rpi_firmware *firmware; + /* Must be last */ struct clk_hw_onecell_data onecell; }; @@ -424,6 +451,23 @@ struct bcm2835_pll_data { unsigned long max_fb_rate; }; +struct bcm2835_fw_pll_data { + const char *name; + int firmware_clk_id; + u32 flags; + + unsigned long min_rate; + unsigned long max_rate; + + /* +* The rate provided to the firmware interface might not always match +* the clock subsystem's. For instance, in the case of the ARM cpu +* clock, even though the firmware ultimately edits 'pllb' it takes the +* expected 'pllb_arm' rate as it's input. +*/ + unsigned int rate_rescale; +}; + struct bcm2835_pll_ana_bits { u32 mask0; u32 set0; @@ -505,6 +549,7 @@ struct bcm2835_pll { struct clk_hw hw; struct bcm2835_cprman *cprman; const struct bcm2835_pll_data *data; + const struct bcm2835_fw_pll_data *fw_data; }; static int bcm2835_pll_is_on(struct clk_hw *hw) @@ -517,6 +562,41 @@ static int bcm2835_pll_is_on(struct clk_hw *hw) A2W_PLL_CTRL_PRST_DISABLE; } +static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 tag, + u32 clk, u32 *val) +{ + int ret; + struct bcm2835_firmware_prop msg = { + .id = clk, + .val = *val, + .disable_turbo = 1, + }; + + ret = rpi_firmware_property(firmware, tag, , sizeof(msg)); + if (ret) + return ret; + + *val = msg.val; + + return 0; +} + +static int bcm2835_fw_pll_is_on(struct clk_hw *hw) +{ + struct bcm2835_pll *pll = container_of(hw, struct bcm2835_pll, hw); + struct bcm2835_cprman *cprman = pll->cprman; + u32 val = 0; + int ret; + + ret = raspberrypi_clock_property(cprman->firmware, +RPI_FIRMWARE_GET_CLOCK_STATE, +pll->fw_data->firmware_clk_id, ); + if (ret) + return 0; + + return !!(val & RPI_FIRMWARE_STATE_ENABLE_BIT); +} + static void bcm2835_pll_choose_ndiv_and_fdiv(unsigned long rate, unsigned long parent_rate, u32 *ndiv, u32 *fdiv) @@ -547,16 +627,37 @@ static long bcm2835_pll_round_rate(struct clk_hw *hw, unsigned long rate, unsigned long *parent_rate) { struct bcm2835_pll *pll = container_of(hw, struct bcm2835_pll, hw); - const struct bcm2835_pll_data *data = pll->data; u32 ndiv, f
[RFC v2 5/5] cpufreq: add driver for Raspbery Pi
Raspberry Pi's firmware offers and interface though which update it's performance requirements. It allows us to request for specific runtime frequencies, which the firmware might or might not respect, depending on the firmware configuration and thermals. As the maximum and minimum frequencies are configurable in the firmware there is no way to know in advance their values. So the Raspberry Pi cpufreq driver queries them, builds an opp frequency table to then launch cpufreq-dt. Signed-off-by: Nicolas Saenz Julienne --- drivers/cpufreq/Kconfig.arm | 8 +++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/raspberrypi-cpufreq.c | 83 +++ 3 files changed, 92 insertions(+) create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 179a1d302f48..f6eba7ae50d0 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -308,3 +308,11 @@ config ARM_PXA2xx_CPUFREQ This add the CPUFreq driver support for Intel PXA2xx SOCs. If in doubt, say N. + +config ARM_RASPBERRYPI_CPUFREQ + tristate "Raspberry Pi cpufreq support" + depends on RASPBERRYPI_FIRMWARE || COMPILE_TEST + help + This adds the CPUFreq driver for Raspberry Pi + + If in doubt, say N. diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index 689b26c6f949..02678e9b2ff5 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -84,6 +84,7 @@ obj-$(CONFIG_ARM_TEGRA124_CPUFREQ)+= tegra124-cpufreq.o obj-$(CONFIG_ARM_TEGRA186_CPUFREQ) += tegra186-cpufreq.o obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o +obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ) += raspberrypi-cpufreq.o ## diff --git a/drivers/cpufreq/raspberrypi-cpufreq.c b/drivers/cpufreq/raspberrypi-cpufreq.c new file mode 100644 index ..a85988867d56 --- /dev/null +++ b/drivers/cpufreq/raspberrypi-cpufreq.c @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Raspberry Pi cpufreq driver + * + * Copyright (C) 2019, Nicolas Saenz Julienne + */ + +#include +#include +#include +#include +#include +#include +#include + +static const struct of_device_id machines[] __initconst = { + { .compatible = "raspberrypi,3-model-b-plus" }, + { .compatible = "raspberrypi,3-model-b" }, + { /* sentinel */ } +}; + +static int __init raspberrypi_cpufreq_driver_init(void) +{ + struct platform_device *pdev; + struct device *cpu_dev; + struct clk *clk; + long min, max; + long rate; + int ret; + + if (!of_match_node(machines, of_root)) + return -ENODEV; + + cpu_dev = get_cpu_device(0); + if (!cpu_dev) { + pr_err("Cannot get CPU for cpufreq driver\n"); + return -ENODEV; + } + + clk = clk_get(cpu_dev, 0); + if (IS_ERR(clk)) { + dev_err(cpu_dev, "Cannot get clock for CPU0\n"); + return PTR_ERR(clk); + } + + /* +* The max and min frequencies are configurable in the Raspberry Pi +* firmware, so we query them at runtime +*/ + min = clk_round_rate(clk, 0); + max = clk_round_rate(clk, ULONG_MAX); + clk_put(clk); + + for (rate = min; rate < max; rate += 1) { + ret = dev_pm_opp_add(cpu_dev, rate, 0); + if (ret) + goto remove_opp; + } + + ret = dev_pm_opp_add(cpu_dev, max, 0); + if (ret) + goto remove_opp; + + pdev = platform_device_register_data(NULL, "cpufreq-dt", -1, NULL, 0); + ret = PTR_ERR_OR_ZERO(pdev); + if (ret) { + dev_err(cpu_dev, "Failed to create platform device, %d\n", ret); + goto remove_opp; + } + + return 0; + +remove_opp: + dev_pm_opp_remove_all_dynamic(cpu_dev); + + return ret; +} + +late_initcall(raspberrypi_cpufreq_driver_init); + +MODULE_AUTHOR("Nicolas Saenz Julienne
[RFC 4/5] dts: bcm2837: add per-cpu clock devices
The four CPUs share a same clock source called pllb_arm. The clock can be scaled through the raspberrypi firmware interface. Signed-off-by: Nicolas Saenz Julienne --- arch/arm/boot/dts/bcm2837.dtsi | 8 1 file changed, 8 insertions(+) diff --git a/arch/arm/boot/dts/bcm2837.dtsi b/arch/arm/boot/dts/bcm2837.dtsi index beb6c502dadc..a8fea6696b42 100644 --- a/arch/arm/boot/dts/bcm2837.dtsi +++ b/arch/arm/boot/dts/bcm2837.dtsi @@ -44,6 +44,8 @@ reg = <0>; enable-method = "spin-table"; cpu-release-addr = <0x0 0x00d8>; + clocks = < BCM2835_PLLB_ARM>; + clock-names = "pllb_arm"; }; cpu1: cpu@1 { @@ -52,6 +54,8 @@ reg = <1>; enable-method = "spin-table"; cpu-release-addr = <0x0 0x00e0>; + clocks = < BCM2835_PLLB_ARM>; + clock-names = "pllb_arm"; }; cpu2: cpu@2 { @@ -60,6 +64,8 @@ reg = <2>; enable-method = "spin-table"; cpu-release-addr = <0x0 0x00e8>; + clocks = < BCM2835_PLLB_ARM>; + clock-names = "pllb_arm"; }; cpu3: cpu@3 { @@ -68,6 +74,8 @@ reg = <3>; enable-method = "spin-table"; cpu-release-addr = <0x0 0x00f0>; + clocks = < BCM2835_PLLB_ARM>; + clock-names = "pllb_arm"; }; }; }; -- 2.21.0
[RFC 0/5] cpufreq support for the Raspberry Pi
Hi all, as some of you may recall I've been spending some time looking into providing 'cpufreq' support for the Raspberry Pi platform[1]. I think I'm close to something workable, so I'd love for you to comment on it. There has been some design changes since the last version. Namely the fact that I now make sure *only* the CPU frequency is updated. The firmware API we use has two modes, with or without turbo. Enabling turbo implies not only scaling the CPU clock but also the VPU and other peripheral related clocks. This is problematic as some of them are not prepared for this kind frequency changes. I spent some time adapting the peripheral drivers, but the result was disappointing as they poorly support live frequency changes (which most other chips accept, think for instance I2C and clock stretching) but also turned out hard to integrate into the kernel. As we were planning to use 'clk_notifiers' which turns out not to be such a good idea as it's prone to deadlocks and not recommended by the clock maintainers[2]. It's also worth mentioning that the foundation kernel doesn't support VPU frequency scaling either. With this in mind, and as suggested by clock maintainers[2], I've decided to integrate the firmware clock interface into the bcm2835 clock driver. This, in my opinion, provides the least friction with the firmware and lets us write very simple and portable higher level drivers. As I did with the 'cpufreq' driver which simply queries the max and min frequencies available, which are configurable in the firmware, to then trigger the generic 'cpufreq-dt'. In the future we could further integrate other firmware dependent clocks into the main driver. For instance to be able to scale the VPU clock, which should be operated through a 'devfreq' driver. This was tested on a RPi3b+ and if the series is well received I'll test it further on all platforms I own. That's all, kind regards, Nicolas [1] https://lists.infradead.org/pipermail/linux-rpi-kernel/2019-April/008634.html [2] https://www.spinics.net/lists/linux-clk/msg36937.html --- Nicolas Saenz Julienne (5): clk: bcm2835: set CLK_GET_RATE_NOCACHE on CPU clocks clk: bcm2835: set pllb_arm divisor as readonly clk: bcm2835: use firmware interface to update pllb dts: bcm2837: add per-cpu clock devices cpufreq: add driver for Raspbery Pi arch/arm/boot/dts/bcm2837.dtsi| 8 + drivers/clk/bcm/clk-bcm2835.c | 284 -- drivers/cpufreq/Kconfig.arm | 8 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/raspberrypi-cpufreq.c | 79 +++ 5 files changed, 362 insertions(+), 18 deletions(-) create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c -- 2.21.0
[RFC 5/5] cpufreq: add driver for Raspbery Pi
Raspberry Pi's firmware offers and interface though which update it's performance requirements. It allows us to request for specific runtime frequencies, which the firmware might or might not respect, depending on the firmware configuration and thermals. As the maximum and minimum frequencies are configurable in the firmware there is no way to know in advance their values. So the Raspberry Pi cpufreq driver queries them, builds an opp frequency table to then launch cpufreq-dt. Signed-off-by: Nicolas Saenz Julienne --- drivers/cpufreq/Kconfig.arm | 8 +++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/raspberrypi-cpufreq.c | 79 +++ 3 files changed, 88 insertions(+) create mode 100644 drivers/cpufreq/raspberrypi-cpufreq.c diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 179a1d302f48..70e5f13f7632 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -308,3 +308,11 @@ config ARM_PXA2xx_CPUFREQ This add the CPUFreq driver support for Intel PXA2xx SOCs. If in doubt, say N. + +config ARM_RASPBERRYPI_CPUFREQ + tristate "Raspberry Pi cpufreq support" + depends on RASPBERRYPI_FIRMWARE || (RASPBERRYPI_FIRMWARE=n && COMPILE_TEST) + help + This adds the CPUFreq driver for Raspberry Pi + + If in doubt, say N. diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index 689b26c6f949..02678e9b2ff5 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -84,6 +84,7 @@ obj-$(CONFIG_ARM_TEGRA124_CPUFREQ)+= tegra124-cpufreq.o obj-$(CONFIG_ARM_TEGRA186_CPUFREQ) += tegra186-cpufreq.o obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o +obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ) += raspberrypi-cpufreq.o ## diff --git a/drivers/cpufreq/raspberrypi-cpufreq.c b/drivers/cpufreq/raspberrypi-cpufreq.c new file mode 100644 index ..53cb3e5a8457 --- /dev/null +++ b/drivers/cpufreq/raspberrypi-cpufreq.c @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Raspberry Pi cpufreq driver + * + * Copyright (C) 2019, Nicolas Saenz Julienne + */ + +#include +#include +#include +#include +#include +#include +#include + +static const struct of_device_id machines[] __initconst = { + { .compatible = "raspberrypi,3-model-b-plus" }, + { .compatible = "raspberrypi,3-model-b" }, + { /* sentinel */ } +}; + +static int __init raspberrypi_cpufreq_driver_init(void) +{ + struct platform_device *pdev; + struct cpumask shared_cpus; + struct device *cpu_dev; + struct clk *clk; + long min, max; + long rate; + int ret; + + if (!of_match_node(machines, of_root)) + return -ENODEV; + + cpu_dev = get_cpu_device(0); + if (!cpu_dev) { + pr_err("Cannot get CPU for cpufreq driver\n"); + return -ENODEV; + } + + clk = clk_get(cpu_dev, 0); + if (IS_ERR(clk)) { + dev_err(cpu_dev, "Cannot get clock for CPU0\n"); + return PTR_ERR(clk); + } + + /* +* The max and min frequencies are configurable in the Raspberry Pi +* firmware, so we query them at runtime +*/ + min = clk_round_rate(clk, 0); + max = clk_round_rate(clk, ULONG_MAX); + + for (rate = min; rate < max; rate += 1) { + ret = dev_pm_opp_add(cpu_dev, rate, 0); + if (ret) + return ret; + } + + ret = dev_pm_opp_add(cpu_dev, max, 0); + if (ret) + return ret; + + cpumask_setall(_cpus); + dev_pm_opp_set_sharing_cpus(cpu_dev, _cpus); + + pdev = platform_device_register_data(NULL, "cpufreq-dt", -1, NULL, 0); + ret = PTR_ERR_OR_ZERO(pdev); + if (ret) + dev_err(cpu_dev, "Failed to create platform device, %d\n", ret); + + return ret; +} + +late_initcall(raspberrypi_cpufreq_driver_init); + +MODULE_AUTHOR("Nicolas Saenz Julienne
[RFC 3/5] clk: bcm2835: use firmware interface to update pllb
Raspberry Pi's firmware, which runs in a dedicated processor, keeps track of the board's temperature and voltage. It's resposible for scaling the CPU frequency whenever it deems the device reached an unsafe state. On top of that the firmware provides an interface which allows Linux to to query the clock's state or change it's frequency. Being the sole user of the bcm2835 clock driver, this integrates the firmware interface into the clock driver and adds a first user: the CPU pll, also known as 'pllb'. Signed-off-by: Nicolas Saenz Julienne --- drivers/clk/bcm/clk-bcm2835.c | 274 -- 1 file changed, 259 insertions(+), 15 deletions(-) diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index 5aea110672f3..ce5b16f3704e 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -35,6 +35,7 @@ #include #include #include +#include #define CM_PASSWORD0x5a00 @@ -289,6 +290,30 @@ #define LOCK_TIMEOUT_NS1 #define BCM2835_MAX_FB_RATE175000u +#define RPI_FIRMWARE_ARM_CLK_ID0x3 +#define RPI_FIRMWARE_STATE_ENABLE_BIT 0x1 +#define RPI_FIRMWARE_STATE_WAIT_BIT0x2 + +/* + * Structure of the message passed to Raspberry Pi's firmware in order to + * change clock rates. The 'disable_turbo' option is only available to the ARM + * clock (pllb) which we enable by default as turbo mode will alter multiple + * clocks at once. + * + * Even though we're able to access the clock registers directly we're bound to + * use the firmware interface as the firmware ultimately takes care of + * mitigating overheating/undervoltage situations and we would be changing + * frequencies behind his back. + * + * For more information on the firmware interface check: + * https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface + */ +struct bcm2835_firmware_prop { + u32 id; + u32 val; + u32 disable_turbo; +} __packed; + /* * Names of clocks used within the driver that need to be replaced * with an external parent's name. This array is in the order that @@ -316,6 +341,8 @@ struct bcm2835_cprman { */ const char *real_parent_names[ARRAY_SIZE(cprman_parent_names)]; + struct rpi_firmware *firmware; + /* Must be last */ struct clk_hw_onecell_data onecell; }; @@ -424,6 +451,23 @@ struct bcm2835_pll_data { unsigned long max_fb_rate; }; +struct bcm2835_fw_pll_data { + const char *name; + int firmware_clk_id; + u32 flags; + + unsigned long min_rate; + unsigned long max_rate; + + /* +* The rate provided to the firmware interface might not always match +* the clock subsystem's. For instance, in the case of the ARM cpu +* clock, even though the firmware ultimately edits 'pllb' it takes the +* expected 'pllb_arm' rate as it's input. +*/ + unsigned int rate_rescale; +}; + struct bcm2835_pll_ana_bits { u32 mask0; u32 set0; @@ -505,6 +549,7 @@ struct bcm2835_pll { struct clk_hw hw; struct bcm2835_cprman *cprman; const struct bcm2835_pll_data *data; + const struct bcm2835_fw_pll_data *fw_data; }; static int bcm2835_pll_is_on(struct clk_hw *hw) @@ -517,6 +562,41 @@ static int bcm2835_pll_is_on(struct clk_hw *hw) A2W_PLL_CTRL_PRST_DISABLE; } +static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 tag, + u32 clk, u32 *val) +{ + int ret; + struct bcm2835_firmware_prop msg = { + .id = clk, + .val = *val, + .disable_turbo = 1, + }; + + ret = rpi_firmware_property(firmware, tag, , sizeof(msg)); + if (ret) + return ret; + + *val = msg.val; + + return 0; +} + +static int bcm2835_fw_pll_is_on(struct clk_hw *hw) +{ + struct bcm2835_pll *pll = container_of(hw, struct bcm2835_pll, hw); + struct bcm2835_cprman *cprman = pll->cprman; + u32 val = 0; + int ret; + + ret = raspberrypi_clock_property(cprman->firmware, +RPI_FIRMWARE_GET_CLOCK_STATE, +pll->fw_data->firmware_clk_id, ); + if (ret) + return 0; + + return !!(val & RPI_FIRMWARE_STATE_ENABLE_BIT); +} + static void bcm2835_pll_choose_ndiv_and_fdiv(unsigned long rate, unsigned long parent_rate, u32 *ndiv, u32 *fdiv) @@ -547,16 +627,37 @@ static long bcm2835_pll_round_rate(struct clk_hw *hw, unsigned long rate, unsigned long *parent_rate) { struct bcm2835_pll *pll = container_of(hw, struct bcm2835_pll, hw); - const struct bcm2835_pll_data *data = pll->data; u32 ndiv, f
[RFC 1/5] clk: bcm2835: set CLK_GET_RATE_NOCACHE on CPU clocks
Raspberry Pi's firmware is responsible for updating the cpu clocks and pll. This makes sure we get the right rates anytime. Signed-off-by: Nicolas Saenz Julienne --- drivers/clk/bcm/clk-bcm2835.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index 770bb01f523e..c2772dfb155a 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -411,6 +411,7 @@ struct bcm2835_pll_data { u32 reference_enable_mask; /* Bit in CM_LOCK to indicate when the PLL has locked. */ u32 lock_mask; + u32 flags; const struct bcm2835_pll_ana_bits *ana; @@ -1299,7 +1300,7 @@ static struct clk_hw *bcm2835_register_pll(struct bcm2835_cprman *cprman, init.num_parents = 1; init.name = data->name; init.ops = _pll_clk_ops; - init.flags = CLK_IGNORE_UNUSED; + init.flags = data->flags | CLK_IGNORE_UNUSED; pll = kzalloc(sizeof(*pll), GFP_KERNEL); if (!pll) @@ -1660,6 +1661,7 @@ static const struct bcm2835_clk_desc clk_desc_array[] = { .ana_reg_base = A2W_PLLB_ANA0, .reference_enable_mask = A2W_XOSC_CTRL_PLLB_ENABLE, .lock_mask = CM_LOCK_FLOCKB, + .flags = CLK_GET_RATE_NOCACHE, .ana = _ana_default, @@ -1674,7 +1676,7 @@ static const struct bcm2835_clk_desc clk_desc_array[] = { .load_mask = CM_PLLB_LOADARM, .hold_mask = CM_PLLB_HOLDARM, .fixed_divider = 1, - .flags = CLK_SET_RATE_PARENT), + .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE), /* * PLLC is the core PLL, used to drive the core VPU clock. -- 2.21.0
[RFC 2/5] clk: bcm2835: set pllb_arm divisor as readonly
This divisor is controlled by the firmware, we don't want the clock subsystem to update it inadvertently. Signed-off-by: Nicolas Saenz Julienne --- drivers/clk/bcm/clk-bcm2835.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index c2772dfb155a..5aea110672f3 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -465,6 +465,7 @@ struct bcm2835_pll_divider_data { u32 hold_mask; u32 fixed_divider; u32 flags; + u32 div_flags; }; struct bcm2835_clock_data { @@ -1349,7 +1350,7 @@ bcm2835_register_pll_divider(struct bcm2835_cprman *cprman, divider->div.reg = cprman->regs + data->a2w_reg; divider->div.shift = A2W_PLL_DIV_SHIFT; divider->div.width = A2W_PLL_DIV_BITS; - divider->div.flags = CLK_DIVIDER_MAX_AT_ZERO; + divider->div.flags = data->div_flags | CLK_DIVIDER_MAX_AT_ZERO; divider->div.lock = >regs_lock; divider->div.hw.init = divider->div.table = NULL; @@ -1676,7 +1677,8 @@ static const struct bcm2835_clk_desc clk_desc_array[] = { .load_mask = CM_PLLB_LOADARM, .hold_mask = CM_PLLB_HOLDARM, .fixed_divider = 1, - .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE), + .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE, + .div_flags = CLK_DIVIDER_READ_ONLY), /* * PLLC is the core PLL, used to drive the core VPU clock. -- 2.21.0
[PATCH] spi: bcm2835: only split transfers that exceed DLEN if DMA available
There is no use for this when performing non DMA operations. So we bypass the split. Signed-off-by: Nicolas Saenz Julienne --- drivers/spi/spi-bcm2835.c | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c index 37893313e595..649fd6caed35 100644 --- a/drivers/spi/spi-bcm2835.c +++ b/drivers/spi/spi-bcm2835.c @@ -861,15 +861,17 @@ static int bcm2835_spi_prepare_message(struct spi_master *master, u32 cs = bcm2835_rd(bs, BCM2835_SPI_CS); int ret; - /* -* DMA transfers are limited to 16 bit (0 to 65535 bytes) by the SPI HW -* due to DLEN. Split up transfers (32-bit FIFO aligned) if the limit is -* exceeded. -*/ - ret = spi_split_transfers_maxsize(master, msg, 65532, - GFP_KERNEL | GFP_DMA); - if (ret) - return ret; + if (master->can_dma) { + /* +* DMA transfers are limited to 16 bit (0 to 65535 bytes) by +* the SPI HW due to DLEN. Split up transfers (32-bit FIFO +* aligned) if the limit is exceeded. +*/ + ret = spi_split_transfers_maxsize(master, msg, 65532, + GFP_KERNEL | GFP_DMA); + if (ret) + return ret; + } cs &= ~(BCM2835_SPI_CS_CPOL | BCM2835_SPI_CS_CPHA); -- 2.21.0
[PATCH v3 3/4] staging: vchiq: make wait events interruptible
The killable version of wait_event() is meant to be used on situations where it should not fail at all costs, but still have the convenience of being able to kill it if really necessary. Wait events in VCHIQ doesn't fit this criteria, as it's mainly used as an interface to V4L2 and ALSA devices. Fixes: 852b2876a8a8 ("staging: vchiq: rework remove_event handling") Signed-off-by: Nicolas Saenz Julienne --- .../vc04_services/interface/vchiq_arm/vchiq_core.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c index c65cf1e6f910..44bfa890e0e5 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -395,13 +395,21 @@ remote_event_create(wait_queue_head_t *wq, struct remote_event *event) init_waitqueue_head(wq); } +/* + * All the event waiting routines in VCHIQ used a custom semaphore + * implementation that filtered most signals. This achieved a behaviour similar + * to the "killable" family of functions. While cleaning up this code all the + * routines where switched to the "interruptible" family of functions, as the + * former was deemed unjustified and the use "killable" set all VCHIQ's + * threads in D state. + */ static inline int remote_event_wait(wait_queue_head_t *wq, struct remote_event *event) { if (!event->fired) { event->armed = 1; dsb(sy); - if (wait_event_killable(*wq, event->fired)) { + if (wait_event_interruptible(*wq, event->fired)) { event->armed = 0; return 0; } -- 2.21.0
[PATCH v3 1/4] staging: vchiq_2835_arm: revert "quit using custom down_interruptible()"
The killable version of down() is meant to be used on situations where it should not fail at all costs, but still have the convenience of being able to kill it if really necessary. VCHIQ doesn't fit this criteria, as it's mainly used as an interface to V4L2 and ALSA devices. Fixes: ff5979ad8636 ("staging: vchiq_2835_arm: quit using custom down_interruptible()") Signed-off-by: Nicolas Saenz Julienne --- .../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c index a9a22917ecdb..49d3b39b1059 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c @@ -514,7 +514,7 @@ create_pagelist(char __user *buf, size_t count, unsigned short type) (g_cache_line_size - 1 { char *fragments; - if (down_killable(_free_fragments_sema)) { + if (down_interruptible(_free_fragments_sema) != 0) { cleanup_pagelistinfo(pagelistinfo); return NULL; } -- 2.21.0
[PATCH v3 2/4] staging: vchiq: revert "switch to wait_for_completion_killable"
The killable version of wait_for_completion() is meant to be used on situations where it should not fail at all costs, but still have the convenience of being able to kill it if really necessary. VCHIQ doesn't fit this criteria, as it's mainly used as an interface to V4L2 and ALSA devices. Fixes: a772f116702e ("staging: vchiq: switch to wait_for_completion_killable") Signed-off-by: Nicolas Saenz Julienne --- .../interface/vchiq_arm/vchiq_arm.c | 21 ++- .../interface/vchiq_arm/vchiq_core.c | 21 ++- .../interface/vchiq_arm/vchiq_util.c | 6 +++--- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index ab7d6a0ce94c..62d8f599e765 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -532,7 +532,8 @@ add_completion(VCHIQ_INSTANCE_T instance, VCHIQ_REASON_T reason, vchiq_log_trace(vchiq_arm_log_level, "%s - completion queue full", __func__); DEBUG_COUNT(COMPLETION_QUEUE_FULL_COUNT); - if (wait_for_completion_killable(>remove_event)) { + if (wait_for_completion_interruptible( + >remove_event)) { vchiq_log_info(vchiq_arm_log_level, "service_callback interrupted"); return VCHIQ_RETRY; @@ -643,7 +644,7 @@ service_callback(VCHIQ_REASON_T reason, struct vchiq_header *header, } DEBUG_TRACE(SERVICE_CALLBACK_LINE); - if (wait_for_completion_killable( + if (wait_for_completion_interruptible( _service->remove_event) != 0) { vchiq_log_info(vchiq_arm_log_level, @@ -978,7 +979,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) has been closed until the client library calls the CLOSE_DELIVERED ioctl, signalling close_event. */ if (user_service->close_pending && - wait_for_completion_killable( + wait_for_completion_interruptible( _service->close_event)) status = VCHIQ_RETRY; break; @@ -1154,7 +1155,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) DEBUG_TRACE(AWAIT_COMPLETION_LINE); mutex_unlock(>completion_mutex); - rc = wait_for_completion_killable( + rc = wait_for_completion_interruptible( >insert_event); mutex_lock(>completion_mutex); if (rc != 0) { @@ -1324,7 +1325,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) do { spin_unlock(_queue_spinlock); DEBUG_TRACE(DEQUEUE_MESSAGE_LINE); - if (wait_for_completion_killable( + if (wait_for_completion_interruptible( _service->insert_event)) { vchiq_log_info(vchiq_arm_log_level, "DEQUEUE_MESSAGE interrupted"); @@ -2328,7 +2329,7 @@ vchiq_keepalive_thread_func(void *v) while (1) { long rc = 0, uc = 0; - if (wait_for_completion_killable(_state->ka_evt) + if (wait_for_completion_interruptible(_state->ka_evt) != 0) { vchiq_log_error(vchiq_susp_log_level, "%s interrupted", __func__); @@ -2579,7 +2580,7 @@ block_resume(struct vchiq_arm_state *arm_state) write_unlock_bh(_state->susp_res_lock); vchiq_log_info(vchiq_susp_log_level, "%s wait for previously " "blocked clients", __func__); - if (wait_for_completion_killable_timeout( + if (wait_for_completion_interruptible_timeout( _state->blocked_blocker, timeout_val) <= 0) { vchiq_log_error(vchiq_susp_log_level, "%s wait for " @@ -2605,7 +2606,7 @@ block_resume(struct vchiq_arm_state *arm_state) write_unlock_bh(_state->susp_res_lock); vchiq_log_info(vc
[PATCH v3 0/4] staging: vchiq: use interruptible waits
Hi, this series tries to address an issue that came up in Raspbian's kernel tree [1] and upstream distros [2][3]. We adopted some changes that moved wait calls from a custom implementation to the more standard killable family of functions. Users complained that all the VCHIQ threads showed up in D state (which is the expected behaviour). The custom implementation we deleted tried to mimic the killable family of functions, yet accepted more signals than the later; SIGKILL | SIGINT | SIGQUIT | SIGTRAP | SIGSTOP | SIGCONT for the custom implementation as opposed to plain old SIGKILL. Raspbian maintainers decided roll back some of those changes and leave the wait functions as interruptible. Hence creating some divergence between both trees. One could argue that not liking having the threads stuck in D state is not really a software issue. It's more a cosmetic thing that can scare people when they look at "uptime". On the other hand, if we are ever to unstage this driver, we'd really need a proper justification for using the killable family of functions. Which I think it's not really clear at the moment. As Raspbian's kernel has been working for a while with interruptible waits I propose we follow through. If needed we can always go back to killable. But at least we'll have a proper understanding on the actual needs. In the end the driver is in staging, and the potential for errors small. The first 3 commits fix the issue, and should probably get in as soon as possible, the last commit is just cosmetic and can wait until 5.3. Regards, Nicolas [1] https://github.com/raspberrypi/linux/issues/2881 [2] https://archlinuxarm.org/forum/viewtopic.php?f=65=13485 [3] https://lists.fedoraproject.org/archives/list/a...@lists.fedoraproject.org/message/GBXGJ7DOV5CQQXFPOZCXTRD6W4BEPT4Q/ -- Changes since v2: - Cleaned up revert commit message - Rebase & merge conflict resolutions - Add code cleanup suggested by Dan Carpenter Changes since v1: - Proplery format revert commits - Add code comment to remind of this issue - Add Fixes tags Nicolas Saenz Julienne (4): staging: vchiq_2835_arm: revert "quit using custom down_interruptible()" staging: vchiq: revert "switch to wait_for_completion_killable" staging: vchiq: make wait events interruptible staging: vchiq: stop explicitly comparing with zero to catch errors .../bcm2835-camera/bcm2835-camera.c | 11 ++- .../interface/vchiq_arm/vchiq_2835_arm.c | 2 +- .../interface/vchiq_arm/vchiq_arm.c | 85 +-- .../interface/vchiq_arm/vchiq_connected.c | 4 +- .../interface/vchiq_arm/vchiq_core.c | 53 +++- .../interface/vchiq_arm/vchiq_debugfs.c | 4 +- .../interface/vchiq_arm/vchiq_util.c | 6 +- 7 files changed, 82 insertions(+), 83 deletions(-) -- 2.21.0
[PATCH v3 4/4] staging: vchiq: stop explicitly comparing with zero to catch errors
The vchiq code tends to follow a coding pattern that's not accepted as per the Linux kernel coding style We have this: if (expression != 0) We want this: if (expression) We make an exception if the expression refers to a size, in which case it's accepted for the sake of clarity. Signed-off-by: Nicolas Saenz Julienne --- .../bcm2835-camera/bcm2835-camera.c | 11 ++-- .../interface/vchiq_arm/vchiq_2835_arm.c | 2 +- .../interface/vchiq_arm/vchiq_arm.c | 66 --- .../interface/vchiq_arm/vchiq_connected.c | 4 +- .../interface/vchiq_arm/vchiq_core.c | 28 .../interface/vchiq_arm/vchiq_debugfs.c | 4 +- 6 files changed, 52 insertions(+), 63 deletions(-) diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c index 68f08dc18da9..57f79c153277 100644 --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c @@ -327,7 +327,7 @@ static void buffer_cb(struct vchiq_mmal_instance *instance, "%s: status:%d, buf:%p, length:%lu, flags %u, pts %lld\n", __func__, status, buf, length, mmal_flags, pts); - if (status != 0) { + if (status) { /* error in transfer */ if (buf) { /* there was a buffer with the error so return it */ @@ -359,8 +359,7 @@ static void buffer_cb(struct vchiq_mmal_instance *instance, } } else { if (dev->capture.frame_count) { - if (dev->capture.vc_start_timestamp != -1 && - pts != 0) { + if (dev->capture.vc_start_timestamp != -1 && pts) { ktime_t timestamp; s64 runtime_us = pts - dev->capture.vc_start_timestamp; @@ -826,7 +825,7 @@ static int vidioc_enum_input(struct file *file, void *priv, struct v4l2_input *inp) { /* only a single camera input */ - if (inp->index != 0) + if (inp->index) return -EINVAL; inp->type = V4L2_INPUT_TYPE_CAMERA; @@ -842,7 +841,7 @@ static int vidioc_g_input(struct file *file, void *priv, unsigned int *i) static int vidioc_s_input(struct file *file, void *priv, unsigned int i) { - if (i != 0) + if (i) return -EINVAL; return 0; @@ -1281,7 +1280,7 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void *priv, } ret = mmal_setup_components(dev, f); - if (ret != 0) { + if (ret) { v4l2_err(>v4l2_dev, "%s: failed to setup mmal components: %d\n", __func__, ret); diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c index 49d3b39b1059..cb588c0b9364 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c @@ -514,7 +514,7 @@ create_pagelist(char __user *buf, size_t count, unsigned short type) (g_cache_line_size - 1 { char *fragments; - if (down_interruptible(_free_fragments_sema) != 0) { + if (down_interruptible(_free_fragments_sema)) { cleanup_pagelistinfo(pagelistinfo); return NULL; } diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 62d8f599e765..9264a07cf160 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -238,7 +238,7 @@ VCHIQ_STATUS_T vchiq_shutdown(VCHIQ_INSTANCE_T instance) vchiq_log_trace(vchiq_core_log_level, "%s(%p) called", __func__, instance); - if (mutex_lock_killable(>mutex) != 0) + if (mutex_lock_killable(>mutex)) return VCHIQ_RETRY; /* Remove all services */ @@ -280,7 +280,7 @@ VCHIQ_STATUS_T vchiq_connect(VCHIQ_INSTANCE_T instance) vchiq_log_trace(vchiq_core_log_level, "%s(%p) called", __func__, instance); - if (mutex_lock_killable(>mutex) != 0) { + if (mutex_lock_killable(>mutex)) { vchiq_log_trace(vchiq_core_log_level, "%s: call to mutex_lock failed", __func__); status = VCHIQ_RETRY; @@ -645,8 +645,7 @@ service_callback(VCHIQ_REASON_T reason, struct vchiq_header *header,
Re: [PATCH v2 0/3] staging: vchiq: use interruptible waits
On Mon, 2019-05-06 at 20:12 +0200, Stefan Wahren wrote: > Hi Nicolas, > > Am 06.05.19 um 16:40 schrieb Nicolas Saenz Julienne: > > Hi, > > ... > > > > Regards, > > Nicolas > > > > [1] https://github.com/raspberrypi/linux/issues/2881 > > [2] https://archlinuxarm.org/forum/viewtopic.php?f=65=13485 > > [3] > > https://lists.fedoraproject.org/archives/list/a...@lists.fedoraproject.org/message/GBXGJ7DOV5CQQXFPOZCXTRD6W4BEPT4Q/ > > > > -- > > > > Changes since v1: > > - Proplery format revert commits > > - Add code comment to remind of this issue > > - Add Fixes tags > > > > Nicolas Saenz Julienne (3): > > staging: vchiq_2835_arm: revert "quit using custom > > down_interruptible()" > > staging: vchiq: revert "switch to wait_for_completion_killable" > > staging: vchiq: make wait events interruptible > > > > .../interface/vchiq_arm/vchiq_2835_arm.c | 2 +- > > .../interface/vchiq_arm/vchiq_arm.c | 21 +++-- > > .../interface/vchiq_arm/vchiq_core.c | 31 --- > > .../interface/vchiq_arm/vchiq_util.c | 6 ++-- > > 4 files changed, 35 insertions(+), 25 deletions(-) > > > against which tree should this series apply? > > Since the merge window opened the current staging-linus wont be > available soon. I don't know if that's what you meant, but I guess we should wait for 5.2-rc1 and then push it, the fixes will eventually get into the stable version of 5.1. Regards, Nicolas signature.asc Description: This is a digitally signed message part