* [PATCH V4 2/3] arm64: entry: Move the trampoline to be before PAN
From: Steve Capper @ 2018-01-11 10:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180111101159.9748-1-steve.capper@arm.com>
The trampoline page tables are positioned after the early page tables in
the kernel linker script.
As we are about to change the early page table logic to resolve the
swapper size at link time as opposed to compile time, the
SWAPPER_DIR_SIZE variable (currently used to locate the trampline)
will be rendered unsuitable for low level assembler.
This patch solves this issue by moving the trampoline before the PAN
page tables. The offset to the trampoline from ttbr1 can then be
expressed by: PAGE_SIZE + RESERVED_TTBR0_SIZE, which is available to the
entry assembler.
Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Steve Capper <steve.capper@arm.com>
---
Changed in V4: added Ard's tags
---
arch/arm64/kernel/entry.S | 4 ++--
arch/arm64/kernel/vmlinux.lds.S | 11 ++++++-----
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 031392ee5f47..7902d8145b9a 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -987,7 +987,7 @@ __ni_sys_trace:
.macro tramp_map_kernel, tmp
mrs \tmp, ttbr1_el1
- sub \tmp, \tmp, #(SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE)
+ add \tmp, \tmp, #(PAGE_SIZE + RESERVED_TTBR0_SIZE)
bic \tmp, \tmp, #USER_ASID_FLAG
msr ttbr1_el1, \tmp
#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1003
@@ -1006,7 +1006,7 @@ alternative_else_nop_endif
.macro tramp_unmap_kernel, tmp
mrs \tmp, ttbr1_el1
- add \tmp, \tmp, #(SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE)
+ sub \tmp, \tmp, #(PAGE_SIZE + RESERVED_TTBR0_SIZE)
orr \tmp, \tmp, #USER_ASID_FLAG
msr ttbr1_el1, \tmp
/*
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 8e567de8f369..4c7112a47469 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -218,6 +218,12 @@ SECTIONS
. = ALIGN(PAGE_SIZE);
idmap_pg_dir = .;
. += IDMAP_DIR_SIZE;
+
+#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
+ tramp_pg_dir = .;
+ . += PAGE_SIZE;
+#endif
+
#ifdef CONFIG_ARM64_SW_TTBR0_PAN
reserved_ttbr0 = .;
. += RESERVED_TTBR0_SIZE;
@@ -225,11 +231,6 @@ SECTIONS
swapper_pg_dir = .;
. += SWAPPER_DIR_SIZE;
-#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
- tramp_pg_dir = .;
- . += PAGE_SIZE;
-#endif
-
__pecoff_data_size = ABSOLUTE(. - __initdata_begin);
_end = .;
--
2.11.0
^ permalink raw reply related
* [PATCH V4 3/3] arm64: Extend early page table code to allow for larger kernels
From: Steve Capper @ 2018-01-11 10:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180111101159.9748-1-steve.capper@arm.com>
Currently the early assembler page table code assumes that precisely
1xpgd, 1xpud, 1xpmd are sufficient to represent the early kernel text
mappings.
Unfortunately this is rarely the case when running with a 16KB granule,
and we also run into limits with 4KB granule when building much larger
kernels.
This patch re-writes the early page table logic to compute indices of
mappings for each level of page table, and if multiple indices are
required, the next-level page table is scaled up accordingly.
Also the required size of the swapper_pg_dir is computed at link time
to cover the mapping [KIMAGE_ADDR + VOFFSET, _end]. When KASLR is
enabled, an extra page is set aside for each level that may require extra
entries at runtime.
Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Steve Capper <steve.capper@arm.com>
---
Changed in V4:
local loop variable now unique to macro instatiation,
count logic simplified
---
arch/arm64/include/asm/kernel-pgtable.h | 47 ++++++++++-
arch/arm64/include/asm/pgtable.h | 1 +
arch/arm64/kernel/head.S | 144 +++++++++++++++++++++++---------
arch/arm64/kernel/vmlinux.lds.S | 1 +
arch/arm64/mm/mmu.c | 3 +-
5 files changed, 156 insertions(+), 40 deletions(-)
diff --git a/arch/arm64/include/asm/kernel-pgtable.h b/arch/arm64/include/asm/kernel-pgtable.h
index 77a27af01371..82386e860dd2 100644
--- a/arch/arm64/include/asm/kernel-pgtable.h
+++ b/arch/arm64/include/asm/kernel-pgtable.h
@@ -52,7 +52,52 @@
#define IDMAP_PGTABLE_LEVELS (ARM64_HW_PGTABLE_LEVELS(PHYS_MASK_SHIFT))
#endif
-#define SWAPPER_DIR_SIZE (SWAPPER_PGTABLE_LEVELS * PAGE_SIZE)
+
+/*
+ * If KASLR is enabled, then an offset K is added to the kernel address
+ * space. The bottom 21 bits of this offset are zero to guarantee 2MB
+ * alignment for PA and VA.
+ *
+ * For each pagetable level of the swapper, we know that the shift will
+ * be larger than 21 (for the 4KB granule case we use section maps thus
+ * the smallest shift is actually 30) thus there is the possibility that
+ * KASLR can increase the number of pagetable entries by 1, so we make
+ * room for this extra entry.
+ *
+ * Note KASLR cannot increase the number of required entries for a level
+ * by more than one because it increments both the virtual start and end
+ * addresses equally (the extra entry comes from the case where the end
+ * address is just pushed over a boundary and the start address isn't).
+ */
+
+#ifdef CONFIG_RANDOMIZE_BASE
+#define EARLY_KASLR (1)
+#else
+#define EARLY_KASLR (0)
+#endif
+
+#define EARLY_ENTRIES(vstart, vend, shift) (((vend) >> (shift)) \
+ - ((vstart) >> (shift)) + 1 + EARLY_KASLR)
+
+#define EARLY_PGDS(vstart, vend) (EARLY_ENTRIES(vstart, vend, PGDIR_SHIFT))
+
+#if SWAPPER_PGTABLE_LEVELS > 3
+#define EARLY_PUDS(vstart, vend) (EARLY_ENTRIES(vstart, vend, PUD_SHIFT))
+#else
+#define EARLY_PUDS(vstart, vend) (0)
+#endif
+
+#if SWAPPER_PGTABLE_LEVELS > 2
+#define EARLY_PMDS(vstart, vend) (EARLY_ENTRIES(vstart, vend, SWAPPER_TABLE_SHIFT))
+#else
+#define EARLY_PMDS(vstart, vend) (0)
+#endif
+
+#define EARLY_PAGES(vstart, vend) ( 1 /* PGDIR page */ \
+ + EARLY_PGDS((vstart), (vend)) /* each PGDIR needs a next level page table */ \
+ + EARLY_PUDS((vstart), (vend)) /* each PUD needs a next level page table */ \
+ + EARLY_PMDS((vstart), (vend))) /* each PMD needs a next level page table */
+#define SWAPPER_DIR_SIZE (PAGE_SIZE * EARLY_PAGES(KIMAGE_VADDR + TEXT_OFFSET, _end))
#define IDMAP_DIR_SIZE (IDMAP_PGTABLE_LEVELS * PAGE_SIZE)
#ifdef CONFIG_ARM64_SW_TTBR0_PAN
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index bfa237e892f1..54b0a8398055 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -706,6 +706,7 @@ static inline void pmdp_set_wrprotect(struct mm_struct *mm,
#endif
extern pgd_t swapper_pg_dir[PTRS_PER_PGD];
+extern pgd_t swapper_pg_end[];
extern pgd_t idmap_pg_dir[PTRS_PER_PGD];
extern pgd_t tramp_pg_dir[PTRS_PER_PGD];
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 66f01869e97c..95748a00eb89 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -191,44 +191,109 @@ ENDPROC(preserve_boot_args)
.endm
/*
- * Macro to populate the PGD (and possibily PUD) for the corresponding
- * block entry in the next level (tbl) for the given virtual address.
+ * Macro to populate page table entries, these entries can be pointers to the next level
+ * or last level entries pointing to physical memory.
*
- * Preserves: tbl, next, virt
- * Corrupts: ptrs_per_pgd, tmp1, tmp2
+ * tbl: page table address
+ * rtbl: pointer to page table or physical memory
+ * index: start index to write
+ * eindex: end index to write - [index, eindex] written to
+ * flags: flags for pagetable entry to or in
+ * inc: increment to rtbl between each entry
+ * tmp1: temporary variable
+ *
+ * Preserves: tbl, eindex, flags, inc
+ * Corrupts: index, tmp1
+ * Returns: rtbl
*/
- .macro create_pgd_entry, tbl, virt, ptrs_per_pgd, tmp1, tmp2
- create_table_entry \tbl, \virt, PGDIR_SHIFT, \ptrs_per_pgd, \tmp1, \tmp2
-#if SWAPPER_PGTABLE_LEVELS > 3
- mov \ptrs_per_pgd, PTRS_PER_PUD
- create_table_entry \tbl, \virt, PUD_SHIFT, \ptrs_per_pgd, \tmp1, \tmp2
-#endif
-#if SWAPPER_PGTABLE_LEVELS > 2
- mov \ptrs_per_pgd, PTRS_PER_PTE
- create_table_entry \tbl, \virt, SWAPPER_TABLE_SHIFT, \ptrs_per_pgd, \tmp1, \tmp2
-#endif
+ .macro populate_entries, tbl, rtbl, index, eindex, flags, inc, tmp1
+.Lpe\@: phys_to_pte \rtbl, \tmp1
+ orr \tmp1, \tmp1, \flags // tmp1 = table entry
+ str \tmp1, [\tbl, \index, lsl #3]
+ add \rtbl, \rtbl, \inc // rtbl = pa next level
+ add \index, \index, #1
+ cmp \index, \eindex
+ b.ls .Lpe\@
+ .endm
+
+/*
+ * Compute indices of table entries from virtual address range. If multiple entries
+ * were needed in the previous page table level then the next page table level is assumed
+ * to be composed of multiple pages. (This effectively scales the end index).
+ *
+ * vstart: virtual address of start of range
+ * vend: virtual address of end of range
+ * shift: shift used to transform virtual address into index
+ * ptrs: number of entries in page table
+ * istart: index in table corresponding to vstart
+ * iend: index in table corresponding to vend
+ * count: On entry: how many extra entries were required in previous level, scales
+ * our end index.
+ * On exit: returns how many extra entries required for next page table level
+ *
+ * Preserves: vstart, vend, shift, ptrs
+ * Returns: istart, iend, count
+ */
+ .macro compute_indices, vstart, vend, shift, ptrs, istart, iend, count
+ lsr \iend, \vend, \shift
+ mov \istart, \ptrs
+ sub \istart, \istart, #1
+ and \iend, \iend, \istart // iend = (vend >> shift) & (ptrs - 1)
+ mov \istart, \ptrs
+ mul \istart, \istart, \count
+ add \iend, \iend, \istart // iend += (count - 1) * ptrs
+ // our entries span multiple tables
+
+ lsr \istart, \vstart, \shift
+ mov \count, \ptrs
+ sub \count, \count, #1
+ and \istart, \istart, \count
+
+ sub \count, \iend, \istart
.endm
/*
- * Macro to populate block entries in the page table for the start..end
- * virtual range (inclusive).
+ * Map memory for specified virtual address range. Each level of page table needed supports
+ * multiple entries. If a level requires n entries the next page table level is assumed to be
+ * formed from n pages.
+ *
+ * tbl: location of page table
+ * rtbl: address to be used for first level page table entry (typically tbl + PAGE_SIZE)
+ * vstart: start address to map
+ * vend: end address to map - we map [vstart, vend]
+ * flags: flags to use to map last level entries
+ * phys: physical address corresponding to vstart - physical memory is contiguous
+ * pgds: the number of pgd entries
*
- * Preserves: tbl, flags
- * Corrupts: phys, start, end, tmp, pstate
+ * Temporaries: istart, iend, tmp, count, sv - these need to be different registers
+ * Preserves: vstart, vend, flags
+ * Corrupts: tbl, rtbl, istart, iend, tmp, count, sv
*/
- .macro create_block_map, tbl, flags, phys, start, end, tmp
- lsr \start, \start, #SWAPPER_BLOCK_SHIFT
- and \start, \start, #PTRS_PER_PTE - 1 // table index
- bic \phys, \phys, #SWAPPER_BLOCK_SIZE - 1
- lsr \end, \end, #SWAPPER_BLOCK_SHIFT
- and \end, \end, #PTRS_PER_PTE - 1 // table end index
-9999: phys_to_pte \phys, \tmp
- orr \tmp, \tmp, \flags // table entry
- str \tmp, [\tbl, \start, lsl #3] // store the entry
- add \start, \start, #1 // next entry
- add \phys, \phys, #SWAPPER_BLOCK_SIZE // next block
- cmp \start, \end
- b.ls 9999b
+ .macro map_memory, tbl, rtbl, vstart, vend, flags, phys, pgds, istart, iend, tmp, count, sv
+ add \rtbl, \tbl, #PAGE_SIZE
+ mov \sv, \rtbl
+ mov \count, #0
+ compute_indices \vstart, \vend, #PGDIR_SHIFT, \pgds, \istart, \iend, \count
+ populate_entries \tbl, \rtbl, \istart, \iend, #PMD_TYPE_TABLE, #PAGE_SIZE, \tmp
+ mov \tbl, \sv
+ mov \sv, \rtbl
+
+#if SWAPPER_PGTABLE_LEVELS > 3
+ compute_indices \vstart, \vend, #PUD_SHIFT, #PTRS_PER_PUD, \istart, \iend, \count
+ populate_entries \tbl, \rtbl, \istart, \iend, #PMD_TYPE_TABLE, #PAGE_SIZE, \tmp
+ mov \tbl, \sv
+ mov \sv, \rtbl
+#endif
+
+#if SWAPPER_PGTABLE_LEVELS > 2
+ compute_indices \vstart, \vend, #SWAPPER_TABLE_SHIFT, #PTRS_PER_PMD, \istart, \iend, \count
+ populate_entries \tbl, \rtbl, \istart, \iend, #PMD_TYPE_TABLE, #PAGE_SIZE, \tmp
+ mov \tbl, \sv
+#endif
+
+ compute_indices \vstart, \vend, #SWAPPER_BLOCK_SHIFT, #PTRS_PER_PTE, \istart, \iend, \count
+ bic \count, \phys, #SWAPPER_BLOCK_SIZE - 1
+ populate_entries \tbl, \count, \istart, \iend, \flags, #SWAPPER_BLOCK_SIZE, \tmp
.endm
/*
@@ -246,14 +311,16 @@ __create_page_tables:
* dirty cache lines being evicted.
*/
adrp x0, idmap_pg_dir
- ldr x1, =(IDMAP_DIR_SIZE + SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE)
+ adrp x1, swapper_pg_end
+ sub x1, x1, x0
bl __inval_dcache_area
/*
* Clear the idmap and swapper page tables.
*/
adrp x0, idmap_pg_dir
- ldr x1, =(IDMAP_DIR_SIZE + SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE)
+ adrp x1, swapper_pg_end
+ sub x1, x1, x0
1: stp xzr, xzr, [x0], #16
stp xzr, xzr, [x0], #16
stp xzr, xzr, [x0], #16
@@ -318,10 +385,10 @@ __create_page_tables:
#endif
1:
ldr_l x4, idmap_ptrs_per_pgd
- create_pgd_entry x0, x3, x4, x5, x6
mov x5, x3 // __pa(__idmap_text_start)
adr_l x6, __idmap_text_end // __pa(__idmap_text_end)
- create_block_map x0, x7, x3, x5, x6, x4
+
+ map_memory x0, x1, x3, x6, x7, x3, x4, x10, x11, x12, x13, x14
/*
* Map the kernel image (starting with PHYS_OFFSET).
@@ -330,12 +397,12 @@ __create_page_tables:
mov_q x5, KIMAGE_VADDR + TEXT_OFFSET // compile time __va(_text)
add x5, x5, x23 // add KASLR displacement
mov x4, PTRS_PER_PGD
- create_pgd_entry x0, x5, x4, x3, x6
adrp x6, _end // runtime __pa(_end)
adrp x3, _text // runtime __pa(_text)
sub x6, x6, x3 // _end - _text
add x6, x6, x5 // runtime __va(_end)
- create_block_map x0, x7, x3, x5, x6, x4
+
+ map_memory x0, x1, x5, x6, x7, x3, x4, x10, x11, x12, x13, x14
/*
* Since the page tables have been populated with non-cacheable
@@ -343,7 +410,8 @@ __create_page_tables:
* tables again to remove any speculatively loaded cache lines.
*/
adrp x0, idmap_pg_dir
- ldr x1, =(IDMAP_DIR_SIZE + SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE)
+ adrp x1, swapper_pg_end
+ sub x1, x1, x0
dmb sy
bl __inval_dcache_area
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 4c7112a47469..0221aca6493d 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -230,6 +230,7 @@ SECTIONS
#endif
swapper_pg_dir = .;
. += SWAPPER_DIR_SIZE;
+ swapper_pg_end = .;
__pecoff_data_size = ABSOLUTE(. - __initdata_begin);
_end = .;
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 4071602031ed..fdac11979bae 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -644,7 +644,8 @@ void __init paging_init(void)
* allocated with it.
*/
memblock_free(__pa_symbol(swapper_pg_dir) + PAGE_SIZE,
- SWAPPER_DIR_SIZE - PAGE_SIZE);
+ __pa_symbol(swapper_pg_end) - __pa_symbol(swapper_pg_dir)
+ - PAGE_SIZE);
}
/*
--
2.11.0
^ permalink raw reply related
* [linux-sunxi] [PATCH 1/7] pinctrl: sunxi: add support for pin controllers without bus gate
From: Chen-Yu Tsai @ 2018-01-11 10:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2ca1ee96-8dc6-c80b-ae11-45895d6a8484@arm.com>
On Thu, Jan 11, 2018 at 6:08 PM, Andre Przywara <andre.przywara@arm.com> wrote:
> Hi,
>
> On 06/01/18 04:23, Icenowy Zheng wrote:
>> The Allwinner H6 pin controllers (both the main one and the CPUs one)
>> have no bus gate clocks.
>>
>> Add support for this kind of pin controllers.
>>
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> ---
>> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 30 ++++++++++++++++++++----------
>> drivers/pinctrl/sunxi/pinctrl-sunxi.h | 1 +
>> 2 files changed, 21 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> index 4b6cb25bc796..68cd505679d9 100644
>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> @@ -1182,7 +1182,12 @@ static int sunxi_pinctrl_setup_debounce(struct sunxi_pinctrl *pctl,
>> unsigned int hosc_div, losc_div;
>> struct clk *hosc, *losc;
>> u8 div, src;
>> - int i, ret;
>> + int i, ret, clk_count;
>> +
>> + if (pctl->desc->without_bus_gate)
>> + clk_count = 2;
>> + else
>> + clk_count = 3;
>>
>> /* Deal with old DTs that didn't have the oscillators */
>> if (of_count_phandle_with_args(node, "clocks", "#clock-cells") != 3)
>> @@ -1360,15 +1365,19 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
>> goto gpiochip_error;
>> }
>>
>> - clk = devm_clk_get(&pdev->dev, NULL);
>> - if (IS_ERR(clk)) {
>> - ret = PTR_ERR(clk);
>> - goto gpiochip_error;
>> - }
>> + if (!desc->without_bus_gate) {
>
> Do we really need explicit support for that case?
> Can't we have something that works automatically?
>
> if (node has clock-names property) (A)
> use clocks as enumerated and named there
You still need to know if the hardware has a bus gate or not.
If it's missing, and it's disabled, you end up with unusable
hardware.
Unless you are fully trusting the device tree to be correct.
IMHO that makes for hard to find bugs during SoC bringup.
ChenYu
> else if (node has one clock reference) (B)
> use this as gate clock, no debounce support
> else if (node has no clock property at all) (C)
> no gate clock needed, no debounce support
>
> On top of that we should add the clock-names property to all DTs, even
> for those with only a "apb" clock. Shouldn't hurt existing kernels.
> Possibly even add debounce support for those on the way, if applicable.
>
> So we would just support case (B) and (C) for legacy reasons.
>
> Does that make sense?
>
> Cheers,
> Andre.
>
>> + clk = devm_clk_get(&pdev->dev, NULL);
>> + if (IS_ERR(clk)) {
>> + ret = PTR_ERR(clk);
>> + goto gpiochip_error;
>> + }
>>
>> - ret = clk_prepare_enable(clk);
>> - if (ret)
>> - goto gpiochip_error;
>> + ret = clk_prepare_enable(clk);
>> + if (ret)
>> + goto gpiochip_error;
>> + } else {
>> + clk = NULL;
>> + }
>>
>> pctl->irq = devm_kcalloc(&pdev->dev,
>> pctl->desc->irq_banks,
>> @@ -1425,7 +1434,8 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
>> return 0;
>>
>> clk_error:
>> - clk_disable_unprepare(clk);
>> + if (clk)
>> + clk_disable_unprepare(clk);
>> gpiochip_error:
>> gpiochip_remove(pctl->chip);
>> return ret;
>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
>> index 11b128f54ed2..ccb6230f0bb5 100644
>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
>> @@ -113,6 +113,7 @@ struct sunxi_pinctrl_desc {
>> unsigned irq_bank_base;
>> bool irq_read_needs_mux;
>> bool disable_strict_mode;
>> + bool without_bus_gate;
>> };
>>
>> struct sunxi_pinctrl_function {
>>
^ permalink raw reply
* [linux-sunxi] [PATCH 1/7] pinctrl: sunxi: add support for pin controllers without bus gate
From: Icenowy Zheng @ 2018-01-11 10:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2ca1ee96-8dc6-c80b-ae11-45895d6a8484@arm.com>
? 2018?1?11? GMT+08:00 ??6:08:19, Andre Przywara <andre.przywara@arm.com> ??:
>Hi,
>
>On 06/01/18 04:23, Icenowy Zheng wrote:
>> The Allwinner H6 pin controllers (both the main one and the CPUs one)
>> have no bus gate clocks.
>>
>> Add support for this kind of pin controllers.
>>
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> ---
>> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 30
>++++++++++++++++++++----------
>> drivers/pinctrl/sunxi/pinctrl-sunxi.h | 1 +
>> 2 files changed, 21 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> index 4b6cb25bc796..68cd505679d9 100644
>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> @@ -1182,7 +1182,12 @@ static int sunxi_pinctrl_setup_debounce(struct
>sunxi_pinctrl *pctl,
>> unsigned int hosc_div, losc_div;
>> struct clk *hosc, *losc;
>> u8 div, src;
>> - int i, ret;
>> + int i, ret, clk_count;
>> +
>> + if (pctl->desc->without_bus_gate)
>> + clk_count = 2;
>> + else
>> + clk_count = 3;
>>
>> /* Deal with old DTs that didn't have the oscillators */
>> if (of_count_phandle_with_args(node, "clocks", "#clock-cells") !=
>3)
>> @@ -1360,15 +1365,19 @@ int sunxi_pinctrl_init_with_variant(struct
>platform_device *pdev,
>> goto gpiochip_error;
>> }
>>
>> - clk = devm_clk_get(&pdev->dev, NULL);
>> - if (IS_ERR(clk)) {
>> - ret = PTR_ERR(clk);
>> - goto gpiochip_error;
>> - }
>> + if (!desc->without_bus_gate) {
>
>Do we really need explicit support for that case?
>Can't we have something that works automatically?
It can be a sanity check. When a SoC comes with bus gate
support but no apb is provided, there's something wrong.
>
>if (node has clock-names property) (A)
> use clocks as enumerated and named there
>else if (node has one clock reference) (B)
> use this as gate clock, no debounce support
>else if (node has no clock property at all) (C)
> no gate clock needed, no debounce support
This should not happen in practice, as no gate clock is implemented
after debounce.
>
>On top of that we should add the clock-names property to all DTs, even
>for those with only a "apb" clock. Shouldn't hurt existing kernels.
>Possibly even add debounce support for those on the way, if applicable.
>
>So we would just support case (B) and (C) for legacy reasons.
>
>Does that make sense?
>
>Cheers,
>Andre.
>
>> + clk = devm_clk_get(&pdev->dev, NULL);
>> + if (IS_ERR(clk)) {
>> + ret = PTR_ERR(clk);
>> + goto gpiochip_error;
>> + }
>>
>> - ret = clk_prepare_enable(clk);
>> - if (ret)
>> - goto gpiochip_error;
>> + ret = clk_prepare_enable(clk);
>> + if (ret)
>> + goto gpiochip_error;
>> + } else {
>> + clk = NULL;
>> + }
>>
>> pctl->irq = devm_kcalloc(&pdev->dev,
>> pctl->desc->irq_banks,
>> @@ -1425,7 +1434,8 @@ int sunxi_pinctrl_init_with_variant(struct
>platform_device *pdev,
>> return 0;
>>
>> clk_error:
>> - clk_disable_unprepare(clk);
>> + if (clk)
>> + clk_disable_unprepare(clk);
>> gpiochip_error:
>> gpiochip_remove(pctl->chip);
>> return ret;
>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
>b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
>> index 11b128f54ed2..ccb6230f0bb5 100644
>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
>> @@ -113,6 +113,7 @@ struct sunxi_pinctrl_desc {
>> unsigned irq_bank_base;
>> bool irq_read_needs_mux;
>> bool disable_strict_mode;
>> + bool without_bus_gate;
>> };
>>
>> struct sunxi_pinctrl_function {
>>
^ permalink raw reply
* [linux-sunxi] [PATCH 1/7] pinctrl: sunxi: add support for pin controllers without bus gate
From: Andre Przywara @ 2018-01-11 10:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v64E-XOHVJG0Q-uZeCAdxF37QSVCDfiYmqtv7ydvseQwrw@mail.gmail.com>
Hi,
On 11/01/18 10:14, Chen-Yu Tsai wrote:
> On Thu, Jan 11, 2018 at 6:08 PM, Andre Przywara <andre.przywara@arm.com> wrote:
>> Hi,
>>
>> On 06/01/18 04:23, Icenowy Zheng wrote:
>>> The Allwinner H6 pin controllers (both the main one and the CPUs one)
>>> have no bus gate clocks.
>>>
>>> Add support for this kind of pin controllers.
>>>
>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>>> ---
>>> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 30 ++++++++++++++++++++----------
>>> drivers/pinctrl/sunxi/pinctrl-sunxi.h | 1 +
>>> 2 files changed, 21 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>>> index 4b6cb25bc796..68cd505679d9 100644
>>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>>> @@ -1182,7 +1182,12 @@ static int sunxi_pinctrl_setup_debounce(struct sunxi_pinctrl *pctl,
>>> unsigned int hosc_div, losc_div;
>>> struct clk *hosc, *losc;
>>> u8 div, src;
>>> - int i, ret;
>>> + int i, ret, clk_count;
>>> +
>>> + if (pctl->desc->without_bus_gate)
>>> + clk_count = 2;
>>> + else
>>> + clk_count = 3;
>>>
>>> /* Deal with old DTs that didn't have the oscillators */
>>> if (of_count_phandle_with_args(node, "clocks", "#clock-cells") != 3)
>>> @@ -1360,15 +1365,19 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
>>> goto gpiochip_error;
>>> }
>>>
>>> - clk = devm_clk_get(&pdev->dev, NULL);
>>> - if (IS_ERR(clk)) {
>>> - ret = PTR_ERR(clk);
>>> - goto gpiochip_error;
>>> - }
>>> + if (!desc->without_bus_gate) {
>>
>> Do we really need explicit support for that case?
>> Can't we have something that works automatically?
>>
>> if (node has clock-names property) (A)
>> use clocks as enumerated and named there
>
> You still need to know if the hardware has a bus gate or not.
> If it's missing, and it's disabled, you end up with unusable
> hardware.
Yes. So what? If you have a broken DT, it will not work. Just don't do
it. I don't understand why we want to defend against this case.
> Unless you are fully trusting the device tree to be correct.
Sorry, but what else do we trust?
> IMHO that makes for hard to find bugs during SoC bringup.
I am not sure if that is really an issue. I would expect people doing
SoC bringup to be able to cope with those kinds of problems.
Cheers,
Andre.
>
> ChenYu
>
>> else if (node has one clock reference) (B)
>> use this as gate clock, no debounce support
>> else if (node has no clock property at all) (C)
>> no gate clock needed, no debounce support
>>
>> On top of that we should add the clock-names property to all DTs, even
>> for those with only a "apb" clock. Shouldn't hurt existing kernels.
>> Possibly even add debounce support for those on the way, if applicable.
>>
>> So we would just support case (B) and (C) for legacy reasons.
>>
>> Does that make sense?
>>
>> Cheers,
>> Andre.
>>
>>> + clk = devm_clk_get(&pdev->dev, NULL);
>>> + if (IS_ERR(clk)) {
>>> + ret = PTR_ERR(clk);
>>> + goto gpiochip_error;
>>> + }
>>>
>>> - ret = clk_prepare_enable(clk);
>>> - if (ret)
>>> - goto gpiochip_error;
>>> + ret = clk_prepare_enable(clk);
>>> + if (ret)
>>> + goto gpiochip_error;
>>> + } else {
>>> + clk = NULL;
>>> + }
>>>
>>> pctl->irq = devm_kcalloc(&pdev->dev,
>>> pctl->desc->irq_banks,
>>> @@ -1425,7 +1434,8 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
>>> return 0;
>>>
>>> clk_error:
>>> - clk_disable_unprepare(clk);
>>> + if (clk)
>>> + clk_disable_unprepare(clk);
>>> gpiochip_error:
>>> gpiochip_remove(pctl->chip);
>>> return ret;
>>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
>>> index 11b128f54ed2..ccb6230f0bb5 100644
>>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
>>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
>>> @@ -113,6 +113,7 @@ struct sunxi_pinctrl_desc {
>>> unsigned irq_bank_base;
>>> bool irq_read_needs_mux;
>>> bool disable_strict_mode;
>>> + bool without_bus_gate;
>>> };
>>>
>>> struct sunxi_pinctrl_function {
>>>
^ permalink raw reply
* [PATCH 34/38] arm: Implement thread_struct whitelist for hardened usercopy
From: Russell King - ARM Linux @ 2018-01-11 10:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1515636190-24061-35-git-send-email-keescook@chromium.org>
On Wed, Jan 10, 2018 at 06:03:06PM -0800, Kees Cook wrote:
> ARM does not carry FPU state in the thread structure, so it can declare
> no usercopy whitelist at all.
This comment seems to be misleading. We have stored FP state in the
thread structure for a long time - for example, VFP state is stored
in thread->vfpstate.hard, so we _do_ have floating point state in
the thread structure.
What I think this commit message needs to describe is why we don't
need a whitelist _despite_ having FP state in the thread structure.
At the moment, the commit message is making me think that this patch
is wrong and will introduce a regression.
Thanks.
>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
> Cc: linux-arm-kernel at lists.infradead.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> arch/arm/Kconfig | 1 +
> arch/arm/include/asm/processor.h | 7 +++++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 51c8df561077..3ea00d65f35d 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -50,6 +50,7 @@ config ARM
> select HAVE_ARCH_KGDB if !CPU_ENDIAN_BE32 && MMU
> select HAVE_ARCH_MMAP_RND_BITS if MMU
> select HAVE_ARCH_SECCOMP_FILTER if (AEABI && !OABI_COMPAT)
> + select HAVE_ARCH_THREAD_STRUCT_WHITELIST
> select HAVE_ARCH_TRACEHOOK
> select HAVE_ARM_SMCCC if CPU_V7
> select HAVE_EBPF_JIT if !CPU_ENDIAN_BE32
> diff --git a/arch/arm/include/asm/processor.h b/arch/arm/include/asm/processor.h
> index 338cbe0a18ef..01a41be58d43 100644
> --- a/arch/arm/include/asm/processor.h
> +++ b/arch/arm/include/asm/processor.h
> @@ -45,6 +45,13 @@ struct thread_struct {
> struct debug_info debug;
> };
>
> +/* Nothing needs to be usercopy-whitelisted from thread_struct. */
> +static inline void arch_thread_struct_whitelist(unsigned long *offset,
> + unsigned long *size)
> +{
> + *offset = *size = 0;
> +}
> +
> #define INIT_THREAD { }
>
> #define start_thread(regs,pc,sp) \
> --
> 2.7.4
>
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* [PATCH] IIO: ADC: stm32-dfsdm: avoid unused-variable warning
From: Arnd Bergmann @ 2018-01-11 10:34 UTC (permalink / raw)
To: linux-arm-kernel
Building with CONFIG_OF disabled produces a compiler warning:
drivers/iio/adc/stm32-dfsdm-core.c: In function 'stm32_dfsdm_probe':
drivers/iio/adc/stm32-dfsdm-core.c:245:22: error: unused variable 'pnode' [-Werror=unused-variable]
This removes the variable and open-codes it in the only place
it gets used to avoid that warning.
Fixes: bed73904e76f ("IIO: ADC: add stm32 DFSDM core support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/iio/adc/stm32-dfsdm-core.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/iio/adc/stm32-dfsdm-core.c b/drivers/iio/adc/stm32-dfsdm-core.c
index 72427414db7f..6cd655f8239b 100644
--- a/drivers/iio/adc/stm32-dfsdm-core.c
+++ b/drivers/iio/adc/stm32-dfsdm-core.c
@@ -242,7 +242,6 @@ MODULE_DEVICE_TABLE(of, stm32_dfsdm_of_match);
static int stm32_dfsdm_probe(struct platform_device *pdev)
{
struct dfsdm_priv *priv;
- struct device_node *pnode = pdev->dev.of_node;
const struct of_device_id *of_id;
const struct stm32_dfsdm_dev_data *dev_data;
struct stm32_dfsdm *dfsdm;
@@ -254,7 +253,7 @@ static int stm32_dfsdm_probe(struct platform_device *pdev)
priv->pdev = pdev;
- of_id = of_match_node(stm32_dfsdm_of_match, pnode);
+ of_id = of_match_node(stm32_dfsdm_of_match, pdev->dev.of_node);
if (!of_id->data) {
dev_err(&pdev->dev, "Data associated to device is missing\n");
return -EINVAL;
--
2.9.0
^ permalink raw reply related
* [linux-sunxi] [PATCH 1/7] pinctrl: sunxi: add support for pin controllers without bus gate
From: Maxime Ripard @ 2018-01-11 10:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ba83f00d-c1e5-6f6a-84df-04169860c7eb@arm.com>
On Thu, Jan 11, 2018 at 10:23:52AM +0000, Andre Przywara wrote:
> Hi,
>
> On 11/01/18 10:14, Chen-Yu Tsai wrote:
> > On Thu, Jan 11, 2018 at 6:08 PM, Andre Przywara <andre.przywara@arm.com> wrote:
> >> Hi,
> >>
> >> On 06/01/18 04:23, Icenowy Zheng wrote:
> >>> The Allwinner H6 pin controllers (both the main one and the CPUs one)
> >>> have no bus gate clocks.
> >>>
> >>> Add support for this kind of pin controllers.
> >>>
> >>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> >>> ---
> >>> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 30 ++++++++++++++++++++----------
> >>> drivers/pinctrl/sunxi/pinctrl-sunxi.h | 1 +
> >>> 2 files changed, 21 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> >>> index 4b6cb25bc796..68cd505679d9 100644
> >>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> >>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> >>> @@ -1182,7 +1182,12 @@ static int sunxi_pinctrl_setup_debounce(struct sunxi_pinctrl *pctl,
> >>> unsigned int hosc_div, losc_div;
> >>> struct clk *hosc, *losc;
> >>> u8 div, src;
> >>> - int i, ret;
> >>> + int i, ret, clk_count;
> >>> +
> >>> + if (pctl->desc->without_bus_gate)
> >>> + clk_count = 2;
> >>> + else
> >>> + clk_count = 3;
> >>>
> >>> /* Deal with old DTs that didn't have the oscillators */
> >>> if (of_count_phandle_with_args(node, "clocks", "#clock-cells") != 3)
> >>> @@ -1360,15 +1365,19 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
> >>> goto gpiochip_error;
> >>> }
> >>>
> >>> - clk = devm_clk_get(&pdev->dev, NULL);
> >>> - if (IS_ERR(clk)) {
> >>> - ret = PTR_ERR(clk);
> >>> - goto gpiochip_error;
> >>> - }
> >>> + if (!desc->without_bus_gate) {
> >>
> >> Do we really need explicit support for that case?
> >> Can't we have something that works automatically?
> >>
> >> if (node has clock-names property) (A)
> >> use clocks as enumerated and named there
> >
> > You still need to know if the hardware has a bus gate or not.
> > If it's missing, and it's disabled, you end up with unusable
> > hardware.
>
> Yes. So what? If you have a broken DT, it will not work. Just don't do
> it. I don't understand why we want to defend against this case.
This is not the point, but rather: if we have a way to detect easily
that the device tree is missing a property that is missing in our
binding, why shouldn't we do it?
We're already doing it for reg and interrupts for example, why not for
the clocks?
> > Unless you are fully trusting the device tree to be correct.
>
> Sorry, but what else do we trust?
>
> > IMHO that makes for hard to find bugs during SoC bringup.
>
> I am not sure if that is really an issue. I would expect people
> doing SoC bringup to be able to cope with those kinds of problems.
Riiiight, because it worked so well in the past. We definitely didn't
overlooked some clocks used for debouncing in this particular driver,
or some to get the timekeeping right in the RTC.
The argument that "anyone who codes in the kernel should just know
better" doesn't work, on multiple levels. Because anyone that actually
knows better can make a mistake or overlook some feature (because you
didn't have your morning coffee yet, or because it was undocumented)
and because you just make someone that doesn't feel bad.
So, yes, we cannot not trust the device tree. But if we have a way to
detect simple mistakes in the binding, we should also do it.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180111/5ebbcbd0/attachment-0001.sig>
^ permalink raw reply
* [linux-sunxi] [PATCH 1/7] pinctrl: sunxi: add support for pin controllers without bus gate
From: Andre Przywara @ 2018-01-11 10:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <08CB03C2-1253-4072-B8AC-D3F1253C8A85@aosc.io>
Hi,
On 11/01/18 10:15, Icenowy Zheng wrote:
>
>
> ? 2018?1?11? GMT+08:00 ??6:08:19, Andre Przywara <andre.przywara@arm.com> ??:
>> Hi,
>>
>> On 06/01/18 04:23, Icenowy Zheng wrote:
>>> The Allwinner H6 pin controllers (both the main one and the CPUs one)
>>> have no bus gate clocks.
>>>
>>> Add support for this kind of pin controllers.
>>>
>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>>> ---
>>> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 30
>> ++++++++++++++++++++----------
>>> drivers/pinctrl/sunxi/pinctrl-sunxi.h | 1 +
>>> 2 files changed, 21 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>>> index 4b6cb25bc796..68cd505679d9 100644
>>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>>> @@ -1182,7 +1182,12 @@ static int sunxi_pinctrl_setup_debounce(struct
>> sunxi_pinctrl *pctl,
>>> unsigned int hosc_div, losc_div;
>>> struct clk *hosc, *losc;
>>> u8 div, src;
>>> - int i, ret;
>>> + int i, ret, clk_count;
>>> +
>>> + if (pctl->desc->without_bus_gate)
>>> + clk_count = 2;
>>> + else
>>> + clk_count = 3;
>>>
>>> /* Deal with old DTs that didn't have the oscillators */
>>> if (of_count_phandle_with_args(node, "clocks", "#clock-cells") !=
>> 3)
Just spotted: I guess you wanted to compare against that computed value
here?
But I wonder if we can get rid of this check at all? Don't we rely on
clock-names and input-debounce anyway? So we will bail out later anyway
if the DT does not have those?
Why do we need this check then?
>>> @@ -1360,15 +1365,19 @@ int sunxi_pinctrl_init_with_variant(struct
>> platform_device *pdev,
>>> goto gpiochip_error;
>>> }
>>>
>>> - clk = devm_clk_get(&pdev->dev, NULL);
>>> - if (IS_ERR(clk)) {
>>> - ret = PTR_ERR(clk);
>>> - goto gpiochip_error;
>>> - }
>>> + if (!desc->without_bus_gate) {
>>
>> Do we really need explicit support for that case?
>> Can't we have something that works automatically?
>
> It can be a sanity check. When a SoC comes with bus gate
> support but no apb is provided, there's something wrong.
>
>>
>> if (node has clock-names property) (A)
>> use clocks as enumerated and named there
>> else if (node has one clock reference) (B)
>> use this as gate clock, no debounce support
>> else if (node has no clock property at all) (C)
>> no gate clock needed, no debounce support
>
> This should not happen in practice, as no gate clock is implemented
> after debounce.
But still you seem to somewhat support it with your changes above - by
bailing out if there aren't two clocks.
This kind of explicitly checking for a certain number of clocks sounds
not very robust and future proof to me, hence the suggestion to get rid
of it.
Cheers,
Andre.
>>
>> On top of that we should add the clock-names property to all DTs, even
>> for those with only a "apb" clock. Shouldn't hurt existing kernels.
>> Possibly even add debounce support for those on the way, if applicable.
>>
>> So we would just support case (B) and (C) for legacy reasons.
>>
>> Does that make sense?
>>
>> Cheers,
>> Andre.
>>
>>> + clk = devm_clk_get(&pdev->dev, NULL);
>>> + if (IS_ERR(clk)) {
>>> + ret = PTR_ERR(clk);
>>> + goto gpiochip_error;
>>> + }
>>>
>>> - ret = clk_prepare_enable(clk);
>>> - if (ret)
>>> - goto gpiochip_error;
>>> + ret = clk_prepare_enable(clk);
>>> + if (ret)
>>> + goto gpiochip_error;
>>> + } else {
>>> + clk = NULL;
>>> + }
>>>
>>> pctl->irq = devm_kcalloc(&pdev->dev,
>>> pctl->desc->irq_banks,
>>> @@ -1425,7 +1434,8 @@ int sunxi_pinctrl_init_with_variant(struct
>> platform_device *pdev,
>>> return 0;
>>>
>>> clk_error:
>>> - clk_disable_unprepare(clk);
>>> + if (clk)
>>> + clk_disable_unprepare(clk);
>>> gpiochip_error:
>>> gpiochip_remove(pctl->chip);
>>> return ret;
>>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
>> b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
>>> index 11b128f54ed2..ccb6230f0bb5 100644
>>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
>>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
>>> @@ -113,6 +113,7 @@ struct sunxi_pinctrl_desc {
>>> unsigned irq_bank_base;
>>> bool irq_read_needs_mux;
>>> bool disable_strict_mode;
>>> + bool without_bus_gate;
>>> };
>>>
>>> struct sunxi_pinctrl_function {
>>>
^ permalink raw reply
* [linux-sunxi] [PATCH 1/7] pinctrl: sunxi: add support for pin controllers without bus gate
From: Icenowy Zheng @ 2018-01-11 10:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180111104100.j5rwitma3wgtdivm@flea.lan>
? 2018?1?11???? CST ??6:41:00?Maxime Ripard ???
> On Thu, Jan 11, 2018 at 10:23:52AM +0000, Andre Przywara wrote:
> > Hi,
> >
> > On 11/01/18 10:14, Chen-Yu Tsai wrote:
> > > On Thu, Jan 11, 2018 at 6:08 PM, Andre Przywara <andre.przywara@arm.com>
wrote:
> > >> Hi,
> > >>
> > >> On 06/01/18 04:23, Icenowy Zheng wrote:
> > >>> The Allwinner H6 pin controllers (both the main one and the CPUs one)
> > >>> have no bus gate clocks.
> > >>>
> > >>> Add support for this kind of pin controllers.
> > >>>
> > >>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > >>> ---
> > >>>
> > >>> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 30
> > >>> ++++++++++++++++++++----------
> > >>> drivers/pinctrl/sunxi/pinctrl-sunxi.h | 1 +
> > >>> 2 files changed, 21 insertions(+), 10 deletions(-)
> > >>>
> > >>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> > >>> b/drivers/pinctrl/sunxi/pinctrl-sunxi.c index
> > >>> 4b6cb25bc796..68cd505679d9 100644
> > >>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> > >>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> > >>> @@ -1182,7 +1182,12 @@ static int sunxi_pinctrl_setup_debounce(struct
> > >>> sunxi_pinctrl *pctl,> >>>
> > >>> unsigned int hosc_div, losc_div;
> > >>> struct clk *hosc, *losc;
> > >>> u8 div, src;
> > >>>
> > >>> - int i, ret;
> > >>> + int i, ret, clk_count;
> > >>> +
> > >>> + if (pctl->desc->without_bus_gate)
> > >>> + clk_count = 2;
> > >>> + else
> > >>> + clk_count = 3;
> > >>>
> > >>> /* Deal with old DTs that didn't have the oscillators */
> > >>> if (of_count_phandle_with_args(node, "clocks", "#clock-cells")
> > >>> != 3)
> > >>>
> > >>> @@ -1360,15 +1365,19 @@ int sunxi_pinctrl_init_with_variant(struct
> > >>> platform_device *pdev,> >>>
> > >>> goto gpiochip_error;
> > >>>
> > >>> }
> > >>>
> > >>> - clk = devm_clk_get(&pdev->dev, NULL);
> > >>> - if (IS_ERR(clk)) {
> > >>> - ret = PTR_ERR(clk);
> > >>> - goto gpiochip_error;
> > >>> - }
> > >>> + if (!desc->without_bus_gate) {
> > >>
> > >> Do we really need explicit support for that case?
> > >> Can't we have something that works automatically?
> > >>
> > >> if (node has clock-names property) (A)
> > >>
> > >> use clocks as enumerated and named there
> > >
> > > You still need to know if the hardware has a bus gate or not.
> > > If it's missing, and it's disabled, you end up with unusable
> > > hardware.
> >
> > Yes. So what? If you have a broken DT, it will not work. Just don't do
> > it. I don't understand why we want to defend against this case.
>
> This is not the point, but rather: if we have a way to detect easily
> that the device tree is missing a property that is missing in our
> binding, why shouldn't we do it?
>
> We're already doing it for reg and interrupts for example, why not for
> the clocks?
>
> > > Unless you are fully trusting the device tree to be correct.
> >
> > Sorry, but what else do we trust?
> >
> > > IMHO that makes for hard to find bugs during SoC bringup.
> >
> > I am not sure if that is really an issue. I would expect people
> > doing SoC bringup to be able to cope with those kinds of problems.
>
> Riiiight, because it worked so well in the past. We definitely didn't
> overlooked some clocks used for debouncing in this particular driver,
> or some to get the timekeeping right in the RTC.
>
> The argument that "anyone who codes in the kernel should just know
> better" doesn't work, on multiple levels. Because anyone that actually
> knows better can make a mistake or overlook some feature (because you
> didn't have your morning coffee yet, or because it was undocumented)
> and because you just make someone that doesn't feel bad.
I agree it here -- when I'm doing initial trial on H6 I didn't found that apb
gate is missing ;-)
>
> So, yes, we cannot not trust the device tree. But if we have a way to
> detect simple mistakes in the binding, we should also do it.
>
> Maxime
^ permalink raw reply
* net-next/master boot bisection: v4.15-rc7-1221-g45f8982 on sama53d #4343-staging
From: Guillaume Tucker @ 2018-01-11 10:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1961011202.207.1515665766663.JavaMail.jenkins@ip-172-30-0-246>
Please see below - the kernelci.org automated bisection tool has
found this commit caused a boot failure on one platform in the
Free Electrons lab. Several bisections pointed to the same
commit over the past couple of days, with various kernel configs.
More details about the boot failure can be found here:
https://staging.kernelci.org/boot/all/job/net-next/branch/master/kernel/v4.15-rc5-999-gfea23fb-lava-bisect-v2-b-staging-4343-8/
Full boot log:
http://staging-storage.kernelci.org/net-next/master/v4.15-rc5-999-gfea23fb-lava-bisect-v2-b-staging-4343-8/arm/multi_v7_defconfig/lab-free-electrons-dev/boot-at91-sama5d3_xplained.html
It looks like a network interface initialisation error on this
platform. I haven't investigated any further and the automated
bisection tool is still experimental, so please take this with a
pinch of salt and sorry if this patch isn't the actual culprit.
Hope this helps!
Guillaume
On 11/01/18 10:16, kernelci.org bot wrote:
> Bisection result for net-next/master (v4.15-rc7-1221-g45f8982) on sama53d
>
> Good known revision:
>
> 72bca20 liquidio: Use zeroing memory allocator than allocator/memset
>
> Bad known revision:
>
> 45f8982 Merge branch 'hns3-next'
>
> Extra parameters:
>
> Tree: net-next
> URL: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
> Branch: master
> Target: sama53d
> Lab: lab-free-electrons
> Defconfig: multi_v7_defconfig
> Plan: boot
>
>
> Breaking commit found:
>
> -------------------------------------------------------------------------------
> commit fea23fb591cce99546baca043d2a068228e87a79
> Author: Russell King <rmk+kernel@armlinux.org.uk>
> Date: Tue Jan 2 10:58:58 2018 +0000
>
> net: phy: convert read-modify-write to phy_modify()
>
> Convert read-modify-write sequences in at803x, Marvell and core phylib
> to use phy_modify() to ensure safety.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 1e190f3b..c271590 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -215,34 +215,22 @@ static int at803x_suspend(struct phy_device *phydev)
> int value;
> int wol_enabled;
>
> - mutex_lock(&phydev->lock);
> -
> value = phy_read(phydev, AT803X_INTR_ENABLE);
> wol_enabled = value & AT803X_INTR_ENABLE_WOL;
>
> - value = phy_read(phydev, MII_BMCR);
> -
> if (wol_enabled)
> - value |= BMCR_ISOLATE;
> + value = BMCR_ISOLATE;
> else
> - value |= BMCR_PDOWN;
> + value = BMCR_PDOWN;
>
> - phy_write(phydev, MII_BMCR, value);
> -
> - mutex_unlock(&phydev->lock);
> + phy_modify(phydev, MII_BMCR, 0, value);
>
> return 0;
> }
>
> static int at803x_resume(struct phy_device *phydev)
> {
> - int value;
> -
> - value = phy_read(phydev, MII_BMCR);
> - value &= ~(BMCR_PDOWN | BMCR_ISOLATE);
> - phy_write(phydev, MII_BMCR, value);
> -
> - return 0;
> + return phy_modify(phydev, MII_BMCR, ~(BMCR_PDOWN | BMCR_ISOLATE), 0);
> }
>
> static int at803x_probe(struct phy_device *phydev)
> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> index ac39d90..2bd3896 100644
> --- a/drivers/net/phy/marvell.c
> +++ b/drivers/net/phy/marvell.c
> @@ -471,7 +471,7 @@ static int m88e1121_config_aneg(struct phy_device *phydev)
>
> if (phy_interface_is_rgmii(phydev)) {
> err = m88e1121_config_aneg_rgmii_delays(phydev);
> - if (err)
> + if (err < 0)
> return err;
> }
>
> @@ -664,19 +664,14 @@ static int m88e1116r_config_init(struct phy_device *phydev)
>
> static int m88e3016_config_init(struct phy_device *phydev)
> {
> - int reg;
> + int ret;
>
> /* Enable Scrambler and Auto-Crossover */
> - reg = phy_read(phydev, MII_88E3016_PHY_SPEC_CTRL);
> - if (reg < 0)
> - return reg;
> -
> - reg &= ~MII_88E3016_DISABLE_SCRAMBLER;
> - reg |= MII_88E3016_AUTO_MDIX_CROSSOVER;
> -
> - reg = phy_write(phydev, MII_88E3016_PHY_SPEC_CTRL, reg);
> - if (reg < 0)
> - return reg;
> + ret = phy_modify(phydev, MII_88E3016_PHY_SPEC_CTRL,
> + ~MII_88E3016_DISABLE_SCRAMBLER,
> + MII_88E3016_AUTO_MDIX_CROSSOVER);
> + if (ret < 0)
> + return ret;
>
> return marvell_config_init(phydev);
> }
> @@ -685,42 +680,34 @@ static int m88e1111_config_init_hwcfg_mode(struct phy_device *phydev,
> u16 mode,
> int fibre_copper_auto)
> {
> - int temp;
> -
> - temp = phy_read(phydev, MII_M1111_PHY_EXT_SR);
> - if (temp < 0)
> - return temp;
> -
> - temp &= ~(MII_M1111_HWCFG_MODE_MASK |
> - MII_M1111_HWCFG_FIBER_COPPER_AUTO |
> - MII_M1111_HWCFG_FIBER_COPPER_RES);
> - temp |= mode;
> -
> if (fibre_copper_auto)
> - temp |= MII_M1111_HWCFG_FIBER_COPPER_AUTO;
> + mode |= MII_M1111_HWCFG_FIBER_COPPER_AUTO;
>
> - return phy_write(phydev, MII_M1111_PHY_EXT_SR, temp);
> + return phy_modify(phydev, MII_M1111_PHY_EXT_SR,
> + (u16)~(MII_M1111_HWCFG_MODE_MASK |
> + MII_M1111_HWCFG_FIBER_COPPER_AUTO |
> + MII_M1111_HWCFG_FIBER_COPPER_RES),
> + mode);
> }
>
> static int m88e1111_config_init_rgmii_delays(struct phy_device *phydev)
> {
> - int temp;
> -
> - temp = phy_read(phydev, MII_M1111_PHY_EXT_CR);
> - if (temp < 0)
> - return temp;
> + int delay;
>
> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
> - temp |= (MII_M1111_RGMII_RX_DELAY | MII_M1111_RGMII_TX_DELAY);
> + delay = MII_M1111_RGMII_RX_DELAY | MII_M1111_RGMII_TX_DELAY;
> } else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
> - temp &= ~MII_M1111_RGMII_TX_DELAY;
> - temp |= MII_M1111_RGMII_RX_DELAY;
> + delay = MII_M1111_RGMII_RX_DELAY;
> } else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> - temp &= ~MII_M1111_RGMII_RX_DELAY;
> - temp |= MII_M1111_RGMII_TX_DELAY;
> + delay = MII_M1111_RGMII_TX_DELAY;
> + } else {
> + delay = 0;
> }
>
> - return phy_write(phydev, MII_M1111_PHY_EXT_CR, temp);
> + return phy_modify(phydev, MII_M1111_PHY_EXT_CR,
> + (u16)~(MII_M1111_RGMII_RX_DELAY |
> + MII_M1111_RGMII_TX_DELAY),
> + delay);
> }
>
> static int m88e1111_config_init_rgmii(struct phy_device *phydev)
> @@ -766,7 +753,7 @@ static int m88e1111_config_init_rtbi(struct phy_device *phydev)
> int err;
>
> err = m88e1111_config_init_rgmii_delays(phydev);
> - if (err)
> + if (err < 0)
> return err;
>
> err = m88e1111_config_init_hwcfg_mode(
> @@ -793,7 +780,7 @@ static int m88e1111_config_init(struct phy_device *phydev)
>
> if (phy_interface_is_rgmii(phydev)) {
> err = m88e1111_config_init_rgmii(phydev);
> - if (err)
> + if (err < 0)
> return err;
> }
>
> @@ -834,7 +821,6 @@ static int m88e1121_config_init(struct phy_device *phydev)
> static int m88e1510_config_init(struct phy_device *phydev)
> {
> int err;
> - int temp;
>
> /* SGMII-to-Copper mode initialization */
> if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
> @@ -846,16 +832,15 @@ static int m88e1510_config_init(struct phy_device *phydev)
> return err;
>
> /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
> - temp = phy_read(phydev, MII_88E1510_GEN_CTRL_REG_1);
> - temp &= ~MII_88E1510_GEN_CTRL_REG_1_MODE_MASK;
> - temp |= MII_88E1510_GEN_CTRL_REG_1_MODE_SGMII;
> - err = phy_write(phydev, MII_88E1510_GEN_CTRL_REG_1, temp);
> + err = phy_modify(phydev, MII_88E1510_GEN_CTRL_REG_1,
> + ~MII_88E1510_GEN_CTRL_REG_1_MODE_MASK,
> + MII_88E1510_GEN_CTRL_REG_1_MODE_SGMII);
> if (err < 0)
> return err;
>
> /* PHY reset is necessary after changing MODE[2:0] */
> - temp |= MII_88E1510_GEN_CTRL_REG_1_RESET;
> - err = phy_write(phydev, MII_88E1510_GEN_CTRL_REG_1, temp);
> + err = phy_modify(phydev, MII_88E1510_GEN_CTRL_REG_1, 0,
> + MII_88E1510_GEN_CTRL_REG_1_RESET);
> if (err < 0)
> return err;
>
> @@ -961,7 +946,6 @@ static int m88e1149_config_init(struct phy_device *phydev)
>
> static int m88e1145_config_init_rgmii(struct phy_device *phydev)
> {
> - int temp;
> int err;
>
> err = m88e1111_config_init_rgmii_delays(phydev);
> @@ -973,15 +957,9 @@ static int m88e1145_config_init_rgmii(struct phy_device *phydev)
> if (err < 0)
> return err;
>
> - temp = phy_read(phydev, 0x1e);
> - if (temp < 0)
> - return temp;
> -
> - temp &= 0xf03f;
> - temp |= 2 << 9; /* 36 ohm */
> - temp |= 2 << 6; /* 39 ohm */
> -
> - err = phy_write(phydev, 0x1e, temp);
> + err = phy_modify(phydev, 0x1e, 0xf03f,
> + 2 << 9 | /* 36 ohm */
> + 2 << 6); /* 39 ohm */
> if (err < 0)
> return err;
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index be13b5d..2c5b2e0 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1368,9 +1368,8 @@ static int genphy_config_eee_advert(struct phy_device *phydev)
> */
> int genphy_setup_forced(struct phy_device *phydev)
> {
> - int ctl = phy_read(phydev, MII_BMCR);
> + u16 ctl = 0;
>
> - ctl &= BMCR_LOOPBACK | BMCR_ISOLATE | BMCR_PDOWN;
> phydev->pause = 0;
> phydev->asym_pause = 0;
>
> @@ -1382,7 +1381,8 @@ int genphy_setup_forced(struct phy_device *phydev)
> if (DUPLEX_FULL == phydev->duplex)
> ctl |= BMCR_FULLDPLX;
>
> - return phy_write(phydev, MII_BMCR, ctl);
> + return phy_modify(phydev, MII_BMCR,
> + BMCR_LOOPBACK | BMCR_ISOLATE | BMCR_PDOWN, ctl);
> }
> EXPORT_SYMBOL(genphy_setup_forced);
>
> @@ -1392,17 +1392,9 @@ EXPORT_SYMBOL(genphy_setup_forced);
> */
> int genphy_restart_aneg(struct phy_device *phydev)
> {
> - int ctl = phy_read(phydev, MII_BMCR);
> -
> - if (ctl < 0)
> - return ctl;
> -
> - ctl |= BMCR_ANENABLE | BMCR_ANRESTART;
> -
> /* Don't isolate the PHY if we're negotiating */
> - ctl &= ~BMCR_ISOLATE;
> -
> - return phy_write(phydev, MII_BMCR, ctl);
> + return phy_modify(phydev, MII_BMCR, ~BMCR_ISOLATE,
> + BMCR_ANENABLE | BMCR_ANRESTART);
> }
> EXPORT_SYMBOL(genphy_restart_aneg);
>
> @@ -1668,44 +1660,20 @@ EXPORT_SYMBOL(genphy_config_init);
>
> int genphy_suspend(struct phy_device *phydev)
> {
> - int value;
> -
> - mutex_lock(&phydev->lock);
> -
> - value = phy_read(phydev, MII_BMCR);
> - phy_write(phydev, MII_BMCR, value | BMCR_PDOWN);
> -
> - mutex_unlock(&phydev->lock);
> -
> - return 0;
> + return phy_modify(phydev, MII_BMCR, 0, BMCR_PDOWN);
> }
> EXPORT_SYMBOL(genphy_suspend);
>
> int genphy_resume(struct phy_device *phydev)
> {
> - int value;
> -
> - value = phy_read(phydev, MII_BMCR);
> - phy_write(phydev, MII_BMCR, value & ~BMCR_PDOWN);
> -
> - return 0;
> + return phy_modify(phydev, MII_BMCR, ~BMCR_PDOWN, 0);
> }
> EXPORT_SYMBOL(genphy_resume);
>
> int genphy_loopback(struct phy_device *phydev, bool enable)
> {
> - int value;
> -
> - value = phy_read(phydev, MII_BMCR);
> - if (value < 0)
> - return value;
> -
> - if (enable)
> - value |= BMCR_LOOPBACK;
> - else
> - value &= ~BMCR_LOOPBACK;
> -
> - return phy_write(phydev, MII_BMCR, value);
> + return phy_modify(phydev, MII_BMCR, ~BMCR_LOOPBACK,
> + enable ? BMCR_LOOPBACK : 0);
> }
> EXPORT_SYMBOL(genphy_loopback);
> -------------------------------------------------------------------------------
>
>
> Git bisection log:
>
> -------------------------------------------------------------------------------
> git bisect start
> # good: [72bca2084a21edda74b802bc076083d5951f67b4] liquidio: Use zeroing memory allocator than allocator/memset
> git bisect good 72bca2084a21edda74b802bc076083d5951f67b4
> # bad: [45f8982253ad3fd314e3580cb9209183ea3faa71] Merge branch 'hns3-next'
> git bisect bad 45f8982253ad3fd314e3580cb9209183ea3faa71
> # good: [44596f86826df00ab3ab1086b25f3cdcc11156a1] Merge tag 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma
> git bisect good 44596f86826df00ab3ab1086b25f3cdcc11156a1
> # bad: [0befd061af59c4ba426588930f09eb9ea2475534] netfilter: nf_tables: remove nft_dereference()
> git bisect bad 0befd061af59c4ba426588930f09eb9ea2475534
> # bad: [d0adb51edb73c94a595bfa9d9bd8b35977e74fbf] nfp: add basic multicast filtering
> git bisect bad d0adb51edb73c94a595bfa9d9bd8b35977e74fbf
> # bad: [8a4816cad00bf14642f0ed6043b32d29a05006ce] tg3: Add Macronix NVRAM support
> git bisect bad 8a4816cad00bf14642f0ed6043b32d29a05006ce
> # good: [34dc08e4be208539b7c4aa8154a610e1736705e8] net: mdiobus: add unlocked accessors
> git bisect good 34dc08e4be208539b7c4aa8154a610e1736705e8
> # bad: [ee7e16b66a766e8f922aafe5edf9353b9f37a424] net: mdio: Only perform gpio reset for PHYs
> git bisect bad ee7e16b66a766e8f922aafe5edf9353b9f37a424
> # good: [424ca4c5512173e42b4086322dafd33ee882baf3] net: phy: marvell: fix paged access races
> git bisect good 424ca4c5512173e42b4086322dafd33ee882baf3
> # bad: [fea23fb591cce99546baca043d2a068228e87a79] net: phy: convert read-modify-write to phy_modify()
> git bisect bad fea23fb591cce99546baca043d2a068228e87a79
> # good: [2b74e5be17d25fbca4be236a19efcd2ecae81cb2] net: phy: add phy_modify() accessor
> git bisect good 2b74e5be17d25fbca4be236a19efcd2ecae81cb2
> # first bad commit: [fea23fb591cce99546baca043d2a068228e87a79] net: phy: convert read-modify-write to phy_modify()
> -------------------------------------------------------------------------------
>
^ permalink raw reply
* [RFC PATCH 0/2] drivers: clk: Add ZynqMP clock driver support
From: Sudeep Holla @ 2018-01-11 10:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1515449797-5629-1-git-send-email-jollys@xilinx.com>
On 08/01/18 22:16, Jolly Shah wrote:
> Add clock driver for ZynqMP
>
Can you post the complete series in one set for easy of review. It's
currently broken into *6 - 8* different sets by different authors and
it's extremely hard to get the full view of how these pieces are
connected and extremely misleading. Can someone post them collectively
as you would already have it if you are doing some testing building on
each patch set.
IIUC mailbox is used for all the communication, right ?
If so, to start with just have 2 series one for mailbox/transport and
another for the whole protocol(eemi?) and users of it.
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH -next] ASoC: stm32: fix a typo in stm32_adfsdm_probe()
From: Wei Yongjun @ 2018-01-11 11:11 UTC (permalink / raw)
To: linux-arm-kernel
Fix a typo, we should return PTR_ERR(priv->iio_cb) instead of
PTR_ERR(priv->iio_ch).
Fixes: 55da094824c4 ("ASoC: stm32: add DFSDM DAI support")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
sound/soc/stm/stm32_adfsdm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/stm/stm32_adfsdm.c b/sound/soc/stm/stm32_adfsdm.c
index af50891..7306e3e 100644
--- a/sound/soc/stm/stm32_adfsdm.c
+++ b/sound/soc/stm/stm32_adfsdm.c
@@ -320,7 +320,7 @@ static int stm32_adfsdm_probe(struct platform_device *pdev)
priv->iio_cb = iio_channel_get_all_cb(&pdev->dev, NULL, NULL);
if (IS_ERR(priv->iio_cb))
- return PTR_ERR(priv->iio_ch);
+ return PTR_ERR(priv->iio_cb);
ret = devm_snd_soc_register_platform(&pdev->dev,
&stm32_adfsdm_soc_platform);
^ permalink raw reply related
* [PATCH -next] IIO: ADC: fix return value check in stm32_dfsdm_adc_probe()
From: Wei Yongjun @ 2018-01-11 11:12 UTC (permalink / raw)
To: linux-arm-kernel
In case of error, the function devm_iio_device_alloc() returns NULL
pointer not ERR_PTR(). The IS_ERR() test in the return value check
should be replaced with NULL test.
Fixes: e2e6771c6462 ("IIO: ADC: add STM32 DFSDM sigma delta ADC support")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
drivers/iio/adc/stm32-dfsdm-adc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
index e628d04..5e87140 100644
--- a/drivers/iio/adc/stm32-dfsdm-adc.c
+++ b/drivers/iio/adc/stm32-dfsdm-adc.c
@@ -1100,9 +1100,9 @@ static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
dev_data = (const struct stm32_dfsdm_dev_data *)of_id->data;
iio = devm_iio_device_alloc(dev, sizeof(*adc));
- if (IS_ERR(iio)) {
+ if (!iio) {
dev_err(dev, "%s: Failed to allocate IIO\n", __func__);
- return PTR_ERR(iio);
+ return -ENOMEM;
}
adc = iio_priv(iio);
^ permalink raw reply related
* [PATCH 1/9] iommu/of: Drop early initialisation hooks
From: JeffyChen @ 2018-01-11 11:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <60fdd375-ca74-0fea-c598-39a68b9e7735@samsung.com>
Hi Marek,
Thanks for your reply.
On 01/11/2018 05:40 PM, Marek Szyprowski wrote:
> Hi Jeffy,
>
> On 2018-01-11 09:22, Jeffy Chen wrote:
>> With the probe-deferral mechanism, early initialisation hooks are no
>> longer needed.
>>
>> Suggested-by: Robin Murphy <robin.murphy@arm.com>
>> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
>> ---
>>
>> drivers/iommu/arm-smmu-v3.c | 2 +-
>> drivers/iommu/arm-smmu.c | 12 ++++++------
>> drivers/iommu/exynos-iommu.c | 2 +-
>
> For Exynos IOMMU:
> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
>
> IPMMU and MSM IOMMU are no longer multi-platform safe after this patch.
> It breaks them in the same way as my commit 928055a01b3f ("iommu/exynos:
> Remove custom platform device registration code") broke Exynos IOMMU.
>
> You need a similar fix for them:
> https://www.spinics.net/lists/arm-kernel/msg627648.html
hmmm, right, i did saw this fix in the rockchip iommu driver too.
and there're also some other iommu drivers put bus_set_iommu in their
probe() to avoid that.
maybe we can do it in the iommu framework?
for example:
1/ add a bus type member to struct iommu_device
2/ and a iommu_device_set_bus()
3/ do the bus_set_iommu stuff in iommu_device_register()
4/ undo bus_set_iommu in iommu_device_unregister()
^ permalink raw reply
* [PATCH v3 0/7] Marvell NAND controller rework with ->exec_op()
From: Boris Brezillon @ 2018-01-11 11:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180109103637.23798-1-miquel.raynal@free-electrons.com>
On Tue, 9 Jan 2018 11:36:30 +0100
Miquel Raynal <miquel.raynal@free-electrons.com> wrote:
> Hi,
>
> After the addition of the NAND framework ->exec_op() interface (see [1]
> for the series preparing it and [2] for the last version of the
> core-side implementation of ->exec_op() itself), this series replaces
> the current Marvell NAND controller driver pxa3xx_nand.c with a rework
> called marvell_nand.c.
>
> Aside the fact that it drops the big state machine, improves the overall
> speed and implements raw accesses, it is the first driver-side
> implementation of the ->exec_op() interface and may be used as reference
> for latter reworks of the same type.
>
> One may find more detail about why a completely new driver is needed in
> the commit log of:
>
> "mtd: nand: add reworked Marvell NAND controller driver"
>
> Device tree NAND node definition for all platforms referring to the
> Marvell driver using the new bindings have already been accepted by the
> MVEBU DT maintainers and will be merged after the driver. They are more
> hierarchical and fit the real organization of the hardware, by having
> NAND partitions that are part of NAND chip nodes, themselves part of the
> NAND controller node.
>
> These changes have been tested on:
> - PXA3xx platform with a CM-X300 board (2kiB page NAND, 1b/512B
> strength, Hamming ECC engine) [32 bits]
> - Armada 385 DB AP (4kiB page NAND, 4b/512B, BCH ECC engine) [32 bits]
> - Armada 398 DB (4kiB page NAND, 8b/512B, BCH ECC engine using a layout
> with a last chunk different than the others) [32 bits]
> - Armada 7040 DB and Armada 8040 DB (4kiB page NAND, 4b/512B, BCH ECC
> engine) [64 bits]
> - Triax dvb-tc board (2kiB page NAND, 4b/512B, BCH ECC engine) [32 bits]
>
> This version is known not to be stable yet with a Zylonite based setup but
> otherwise looks good for Marvell EBU platforms.
So, here is the plan: since the driver has been tested on various mvebu
platforms and is known to work fine on these platforms, I'd like to
queue the driver and the patch modifying mvebu defconfigs (patches 1 to
4) for 4.16.
I'll leave other patches for 4.17, which means I'd like remaining bugs
to be fixed during the 4.16 release cycle so that we can eventually get
rid of the old driver. That's really important to me that we don't keep
both drivers around for too long, because my previous experience showed
that, when you have 2 drivers for the same HW, people don't switch to
the new one until they're forced to do it.
Robert, are you fine with this approach? What about the tests you were
doing? Did you make any progress? Did you find other issues?
>
> For people who would like to test it easily, a branch ready to be tested
> is available at [3]. It is based on nand/next and has all the changes
> brought by the previously mentionned series as well as this one.
>
> Thank you,
> Miqu?l
>
>
> [1] https://www.spinics.net/lists/arm-kernel/msg619633.html
> [2] http://lists.infradead.org/pipermail/linux-mtd/2017-December/077965.html
> [3] https://github.com/miquelraynal/linux/tree/marvell/nand-next/nfc
>
>
> Changes since v2:
> - Added a patch to create the nand-rb property in the Documentation
> - Rewording in the Documentation according to Rob's comments
> - Moved from marvell,rb to nand-rb property in the code
> - Disociated using this driver with Marvell EBU platforms than using
> it with PXA ones
> - Fixed the handling of 16-bit buses
> - Fixed SPDX comment style
> - Reorganized registers offsets/bit fields definitions as requested
> - Moved to Kernel doc
> - Changed the logic in ->select_chip() to use a prepared value of NDCR
> only instead of recalculating it
> - Fixed the presence of the SPARE_EN bit, reworked a bit the
> hmg_do_read/write() helpers
> - Fixed the OOB layouts that were unusable (all spare data first, then
> all ECC bytes)
> - Additional check on mtd->writesize when using NFCv1 (all sizes not
> supported)
> - Various typos/rewording
>
> Changes since v1:
> - Rewording
> - Fixed BCH ->read/write_page() hooks for 2kiB pages NAND chips
> - Removed license text, used SPDX tag instead
> - Removed read_page_data()
> - Enhanced the DT bindings document with the label property and the
> deprecated bindings.
> - Simplified the read_chunk() helper (OOB always read).
> - Simplified the ->bch_read_page() hook by removing the addition raw
> read to get ECC bytes.
> - Fixed the ->correct() function that did not check for bitflips in
> ECC bytes in erased pages.
>
>
> Miquel Raynal (7):
> dt-bindings: mtd: document new nand-rb property
> dt-bindings: mtd: add Marvell NAND controller documentation
> mtd: nand: add reworked Marvell NAND controller driver
> mtd: nand: use reworked NAND controller driver with Marvell EBU SoCs
> mtd: nand: use Marvell reworked NAND controller driver with all
> platforms
> dt-bindings: mtd: remove pxa3xx NAND controller documentation
> mtd: nand: remove useless fields from pxa3xx NAND platform data
>
> .../devicetree/bindings/mtd/marvell-nand.txt | 123 +
> Documentation/devicetree/bindings/mtd/nand.txt | 1 +
> .../devicetree/bindings/mtd/pxa3xx-nand.txt | 50 -
> arch/arm/configs/cm_x300_defconfig | 2 +-
> arch/arm/configs/mvebu_v7_defconfig | 2 +-
> arch/arm/configs/pxa3xx_defconfig | 3 +-
> arch/arm/configs/pxa_defconfig | 2 +-
> arch/arm/configs/raumfeld_defconfig | 2 +-
> arch/arm/mach-mmp/ttc_dkb.c | 4 +-
> arch/arm/mach-pxa/cm-x300.c | 8 +-
> arch/arm/mach-pxa/colibri-pxa3xx.c | 8 +-
> arch/arm/mach-pxa/colibri.h | 2 +-
> arch/arm/mach-pxa/littleton.c | 10 +-
> arch/arm/mach-pxa/mxm8x10.c | 10 +-
> arch/arm/mach-pxa/raumfeld.c | 6 +-
> arch/arm/mach-pxa/zylonite.c | 10 +-
> arch/arm64/configs/defconfig | 2 +-
> drivers/mtd/nand/Kconfig | 18 +-
> drivers/mtd/nand/Makefile | 2 +-
> drivers/mtd/nand/marvell_nand.c | 2896 ++++++++++++++++++++
> drivers/mtd/nand/pxa3xx_nand.c | 2104 --------------
> include/linux/platform_data/mtd-nand-pxa3xx.h | 43 +-
> 22 files changed, 3072 insertions(+), 2236 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mtd/marvell-nand.txt
> delete mode 100644 Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt
> create mode 100644 drivers/mtd/nand/marvell_nand.c
> delete mode 100644 drivers/mtd/nand/pxa3xx_nand.c
>
^ permalink raw reply
* [PATCH v1 0/4] pwm support for allwinner sun8i R40/V40/T3 SOCs.
From: hao_zhang @ 2018-01-11 11:31 UTC (permalink / raw)
To: linux-arm-kernel
This patchset add pwm support for allwinner sun8i R40/V40/T3 SOCs.
Beause of my mistake, previous patch set from v4.
this patch set i resend it from v1 and fix some error.
PATCH LOG:
[v4,1/4] dt-bindings: pwm: binding allwinner sun8i R40/V40/T3.
https://patchwork.kernel.org/patch/10110177/
[v4,2/4] ARM: PWM: add allwinner sun8i R40/V40/T3 pwm support.
https://patchwork.kernel.org/patch/10110185/
[v4,3/4] ARM: dts: add pwm node for r40.
https://patchwork.kernel.org/patch/10110191/
Fixed:
1.change readl/writel to regmap.
2.undo the previous polarity, do clk_disable_unprepare()
and unlock spinlock while error return.
Regards,
Hao Zhang
hao_zhang (4):
dt-bindings: pwm: binding allwinner sun8i R40/V40/T3.
ARM: PWM: add allwinner sun8i R40/V40/T3 pwm support.
ARM: dts: add pwm pins for r40.
ARM: dts: add pwm node for r40.
.../devicetree/bindings/pwm/pwm-sun8i.txt | 18 +
arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts | 6 +
arch/arm/boot/dts/sun8i-r40.dtsi | 13 +
drivers/pwm/Kconfig | 10 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-sun8i-r40.c | 394 +++++++++++++++++++++
6 files changed, 442 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/pwm-sun8i.txt
create mode 100644 drivers/pwm/pwm-sun8i-r40.c
--
2.7.4
^ permalink raw reply
* [PATCH v1 1/4] dt-bindings: pwm: binding allwinner sun8i R40/V40/T3.
From: hao_zhang @ 2018-01-11 11:31 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds allwinner R40, V40, T3 pwm binding documents.
Signed-off-by: hao_zhang <hao5781286@gmail.com>
---
Documentation/devicetree/bindings/pwm/pwm-sun8i.txt | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/pwm-sun8i.txt
diff --git a/Documentation/devicetree/bindings/pwm/pwm-sun8i.txt b/Documentation/devicetree/bindings/pwm/pwm-sun8i.txt
new file mode 100644
index 0000000..2c86c67
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-sun8i.txt
@@ -0,0 +1,18 @@
+Allwinner sun8i R40/V40/T3 SoC PWM controller
+
+Required properties:
+ - compatible: should be one of:
+ - "allwinner,sun8i-r40-pwm"
+ - reg: physical base address and length of the controller's registers
+ - #pwm-cells: should be 3. See pwm.txt in this directory for a description of
+ the cells format.
+ - clocks: From common clock binding, handle to the parent clock.
+
+Example:
+
+pwm: pwm at 1c23400 {
+ compatible = "allwinner,sun8i-r40-pwm";
+ reg = <0x01c23400 0x154>;
+ clocks = <&osc24M>;
+ #pwm-cells = <3>;
+};
--
2.7.4
^ permalink raw reply related
* [PATCH] arm64: kdump: retain reserved memory regions
From: AKASHI Takahiro @ 2018-01-11 11:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu9QX-r7Q5Dsze6MYRb8hbRyq=C__3BsZTBvvSyv1Y-KMg@mail.gmail.com>
On Wed, Jan 10, 2018 at 11:09:32AM +0000, Ard Biesheuvel wrote:
> On 10 January 2018 at 10:09, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > This is a fix against the issue that crash dump kernel may hang up
> > during booting, which can happen on any ACPI-based system with "ACPI
> > Reclaim Memory."
> >
> > <kicking off kdump after panic>
> > Bye!
> > (snip...)
> > ACPI: Core revision 20170728
> > pud=000000002e7d0003, *pmd=000000002e7c0003, *pte=00e8000039710707
> > Internal error: Oops: 96000021 [#1] SMP
> > Modules linked in:
> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc6 #1
> > task: ffff000008d05180 task.stack: ffff000008cc0000
> > PC is at acpi_ns_lookup+0x25c/0x3c0
> > LR is at acpi_ds_load1_begin_op+0xa4/0x294
> > (snip...)
> > Process swapper/0 (pid: 0, stack limit = 0xffff000008cc0000)
> > Call trace:
> > (snip...)
> > [<ffff0000084a6764>] acpi_ns_lookup+0x25c/0x3c0
> > [<ffff00000849b4f8>] acpi_ds_load1_begin_op+0xa4/0x294
> > [<ffff0000084ad4ac>] acpi_ps_build_named_op+0xc4/0x198
> > [<ffff0000084ad6cc>] acpi_ps_create_op+0x14c/0x270
> > [<ffff0000084acfa8>] acpi_ps_parse_loop+0x188/0x5c8
> > [<ffff0000084ae048>] acpi_ps_parse_aml+0xb0/0x2b8
> > [<ffff0000084a8e10>] acpi_ns_one_complete_parse+0x144/0x184
> > [<ffff0000084a8e98>] acpi_ns_parse_table+0x48/0x68
> > [<ffff0000084a82cc>] acpi_ns_load_table+0x4c/0xdc
> > [<ffff0000084b32f8>] acpi_tb_load_namespace+0xe4/0x264
> > [<ffff000008baf9b4>] acpi_load_tables+0x48/0xc0
> > [<ffff000008badc20>] acpi_early_init+0x9c/0xd0
> > [<ffff000008b70d50>] start_kernel+0x3b4/0x43c
> > Code: b9008fb9 2a000318 36380054 32190318 (b94002c0)
> > ---[ end trace c46ed37f9651c58e ]---
> > Kernel panic - not syncing: Fatal exception
> > Rebooting in 10 seconds..
> >
> > (diagnosis)
> > * This fault is a data abort, alignment fault (ESR=0x96000021)
> > during reading out ACPI table.
> > * Initial ACPI tables are normally stored in system ram and marked as
> > "ACPI Reclaim memory" by the firmware.
> > * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
> > memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
> > removing NOMAP bit and they are instead "memblock-reserved".
> > * When crash dump kernel boots up, it tries to accesses ACPI tables by
> > ioremap'ing them (through acpi_os_ioremap()).
> > * Since those regions are not included in device tree's
> > "usable-memory-range" and so not recognized as part of crash dump
> > kernel's system ram, ioremap() will create a non-cacheable mapping here.
> > * ACPI accessor/helper functions are compiled in without unaligned access
> > support (ACPI_MISALIGNMENT_NOT_SUPPORTED), eventually ending up a fatal
> > panic when accessing ACPI tables.
> >
> > With this patch, all the reserved memory regions, as well as NOMAP-
> > attributed ones which are presumably ACPI runtime code and data, are set
> > to be retained in system ram even if they are outside of usable memory
> > range specified by device tree blob. Accordingly, ACPI tables are mapped
> > as cacheable and can be safely accessed without causing unaligned access
> > faults.
> >
> > Reported-by: Bhupesh Sharma <bhsharma@redhat.com>
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > arch/arm64/mm/init.c | 16 ++++++++++++++--
> > 1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > index 2d5a443b205c..e4a8b64a09b1 100644
> > --- a/arch/arm64/mm/init.c
> > +++ b/arch/arm64/mm/init.c
> > @@ -352,11 +352,23 @@ static void __init fdt_enforce_memory_region(void)
> > struct memblock_region reg = {
> > .size = 0,
> > };
> > + u64 idx;
> > + phys_addr_t start, end;
> >
> > of_scan_flat_dt(early_init_dt_scan_usablemem, ®);
> >
> > - if (reg.size)
> > - memblock_cap_memory_range(reg.base, reg.size);
>
> Given that memblock_cap_memory_range() was introduced by you for
> kdump, is there any way to handle it there?
Indeed, but I'm not sure that the new semantics of this function
is quite generic.
> If not, should we remove it?
I prefer to remove it.
Thanks,
-Takahiro AKASHI
> > + if (reg.size) {
> > +retry:
> > + /* exclude usable & !reserved memory */
> > + for_each_free_mem_range(idx, NUMA_NO_NODE, MEMBLOCK_NONE,
> > + &start, &end, NULL) {
> > + memblock_remove(start, end - start);
> > + goto retry;
> > + }
> > +
> > + /* add back fdt's usable memory */
> > + memblock_add(reg.base, reg.size);
> > + }
> > }
> >
> > void __init arm64_memblock_init(void)
> > --
> > 2.15.1
> >
^ permalink raw reply
* [PATCH v1 2/4] ARM: PWM: add allwinner sun8i R40/V40/T3 pwm support.
From: hao_zhang @ 2018-01-11 11:32 UTC (permalink / raw)
To: linux-arm-kernel
This patch add allwinner sun8i R40/V40/T3 pwm support.
Signed-off-by: hao_zhang <hao5781286@gmail.com>
---
drivers/pwm/Kconfig | 10 ++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-sun8i-r40.c | 394 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 405 insertions(+)
create mode 100644 drivers/pwm/pwm-sun8i-r40.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 763ee50..cde5a70 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -444,6 +444,16 @@ config PWM_SUN4I
To compile this driver as a module, choose M here: the module
will be called pwm-sun4i.
+config PWM_SUN8I_R40
+ tristate "Allwinner PWM SUN8I R40 support"
+ depends on ARCH_SUNXI || COMPILE_TEST
+ depends on HAS_IOMEM && COMMON_CLK
+ help
+ Generic PWM framework driver for Allwinner SoCs R40, V40, T3.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-sun8i-r40.
+
config PWM_TEGRA
tristate "NVIDIA Tegra PWM support"
depends on ARCH_TEGRA
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 0258a74..026a55b 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_PWM_STM32) += pwm-stm32.o
obj-$(CONFIG_PWM_STM32_LP) += pwm-stm32-lp.o
obj-$(CONFIG_PWM_STMPE) += pwm-stmpe.o
obj-$(CONFIG_PWM_SUN4I) += pwm-sun4i.o
+obj-$(CONFIG_PWM_SUN8I_R40) += pwm-sun8i-r40.o
obj-$(CONFIG_PWM_TEGRA) += pwm-tegra.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
diff --git a/drivers/pwm/pwm-sun8i-r40.c b/drivers/pwm/pwm-sun8i-r40.c
new file mode 100644
index 0000000..3d34285
--- /dev/null
+++ b/drivers/pwm/pwm-sun8i-r40.c
@@ -0,0 +1,394 @@
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/time.h>
+#include <linux/regmap.h>
+
+#define PWM_IRQ_ENABLE_REG 0x0000
+#define PCIE(ch) BIT(ch)
+
+#define PWM_IRQ_STATUS_REG 0x0004
+#define PIS(ch) BIT(ch)
+
+#define CAPTURE_IRQ_ENABLE_REG 0x0010
+#define CFIE(ch) BIT(ch << 1 + 1)
+#define CRIE(ch) BIT(ch << 1)
+
+#define CAPTURE_IRQ_STATUS_REG 0x0014
+#define CFIS(ch) BIT(ch << 1 + 1)
+#define CRIS(ch) BIT(ch << 1)
+
+#define CLK_CFG_REG(ch) (0x0020 + (ch >> 1) * 4)
+#define CLK_SRC BIT(7)
+#define CLK_SRC_BYPASS_SEC BIT(6)
+#define CLK_SRC_BYPASS_FIR BIT(5)
+#define CLK_GATING BIT(4)
+#define CLK_DIV_M GENMASK(3, 0)
+
+#define PWM_DZ_CTR_REG(ch) (0x0030 + (ch >> 1) * 4)
+#define PWM_DZ_INTV GENMASK(15, 8)
+#define PWM_DZ_EN BIT(0)
+
+#define PWM_ENABLE_REG 0x0040
+#define PWM_EN(ch) BIT(ch)
+
+#define CAPTURE_ENABLE_REG 0x0044
+#define CAP_EN(ch) BIT(ch)
+
+#define PWM_CTR_REG(ch) (0x0060 + ch * 0x20)
+#define PWM_PERIOD_RDY BIT(11)
+#define PWM_PUL_START BIT(10)
+#define PWM_MODE BIT(9)
+#define PWM_ACT_STA BIT(8)
+#define PWM_PRESCAL_K GENMASK(7, 0)
+
+#define PWM_PERIOD_REG(ch) (0x0064 + ch * 0x20)
+#define PWM_ENTIRE_CYCLE GENMASK(31, 16)
+#define PWM_ACT_CYCLE GENMASK(15, 0)
+
+#define PWM_CNT_REG(ch) (0x0068 + ch * 0x20)
+#define PWM_CNT_VAL GENMASK(15, 0)
+
+#define CAPTURE_CTR_REG(ch) (0x006c + ch * 0x20)
+#define CAPTURE_CRLF BIT(2)
+#define CAPTURE_CFLF BIT(1)
+#define CAPINV BIT(0)
+
+#define CAPTURE_RISE_REG(ch) (0x0070 + ch * 0x20)
+#define CAPTURE_CRLR GENMASK(15, 0)
+
+#define CAPTURE_FALL_REG(ch) (0x0074 + ch * 0x20)
+#define CAPTURE_CFLR GENMASK(15, 0)
+
+struct sun8i_pwm_data {
+ bool has_prescaler_bypass;
+ bool has_rdy;
+ unsigned int npwm;
+};
+
+struct sun8i_pwm_chip {
+ struct pwm_chip chip;
+ struct clk *clk;
+ void __iomem *base;
+ spinlock_t ctrl_lock;
+ const struct sun8i_pwm_data *data;
+ struct regmap *regmap;
+};
+
+static const u16 div_m_table[] = {
+ 1,
+ 2,
+ 4,
+ 8,
+ 16,
+ 32,
+ 64,
+ 128,
+ 256
+};
+
+static inline struct sun8i_pwm_chip *to_sun8i_pwm_chip(struct pwm_chip *chip)
+{
+ return container_of(chip, struct sun8i_pwm_chip, chip);
+}
+
+static u32 sun8i_pwm_read(struct sun8i_pwm_chip *sun8i_pwm,
+ unsigned long offset)
+{
+ u32 val;
+
+ regmap_read(sun8i_pwm->regmap, offset, &val);
+
+ return val;
+}
+
+static inline void sun8i_pwm_set_bit(struct sun8i_pwm_chip *sun8i_pwm,
+ unsigned long reg, u32 bit)
+{
+ regmap_update_bits(sun8i_pwm->regmap, reg, bit, bit);
+}
+
+static inline void sun8i_pwm_clear_bit(struct sun8i_pwm_chip *sun8i_pwm,
+ unsigned long reg, u32 bit)
+{
+ regmap_update_bits(sun8i_pwm->regmap, reg, bit, 0);
+}
+
+static inline void sun8i_pwm_set_value(struct sun8i_pwm_chip *sun8i_pwm,
+ unsigned long reg, u32 mask, u32 val)
+{
+ regmap_update_bits(sun8i_pwm->regmap, reg, mask, val);
+}
+
+static void sun8i_pwm_set_polarity(struct sun8i_pwm_chip *chip, u32 ch,
+ enum pwm_polarity polarity)
+{
+ if (polarity == PWM_POLARITY_NORMAL)
+ sun8i_pwm_set_bit(chip, PWM_CTR_REG(ch), PWM_ACT_STA);
+ else
+ sun8i_pwm_clear_bit(chip, PWM_CTR_REG(ch), PWM_ACT_STA);
+}
+
+static int sun8i_pwm_config(struct sun8i_pwm_chip *sun8i_pwm, u8 ch,
+ struct pwm_state *state)
+{
+ u64 clk_rate, clk_div, val;
+ u16 prescaler = 0;
+ u8 id = 0;
+
+ clk_rate = clk_get_rate(sun8i_pwm->clk);
+
+ if (clk_rate == 24000000)
+ sun8i_pwm_clear_bit(sun8i_pwm, CLK_CFG_REG(ch), CLK_SRC);
+ else
+ sun8i_pwm_set_bit(sun8i_pwm, CLK_CFG_REG(ch), CLK_SRC);
+
+ if (sun8i_pwm->data->has_prescaler_bypass) {
+ /* pwm output bypass */
+ if (ch % 2)
+ sun8i_pwm_set_bit(sun8i_pwm, CLK_CFG_REG(ch),
+ CLK_SRC_BYPASS_FIR);
+ else
+ sun8i_pwm_set_bit(sun8i_pwm, CLK_CFG_REG(ch),
+ CLK_SRC_BYPASS_SEC);
+ return 0;
+ }
+
+ val = state->period * clk_rate;
+ do_div(val, NSEC_PER_SEC);
+ if (val < 1) {
+ dev_err(sun8i_pwm->chip.dev,
+ "period expects a larger value\n");
+ return -EINVAL;
+ }
+
+ /* calculate and set prescalar, div table, pwn entrie cycle */
+ clk_div = val;
+
+ while (clk_div > 65535) {
+ prescaler++;
+ clk_div = val;
+ do_div(clk_div, prescaler + 1);
+ do_div(clk_div, div_m_table[id]);
+
+ if (prescaler == 255) {
+ prescaler = 0;
+ id++;
+ if (id == 9)
+ return -EINVAL;
+ }
+ }
+
+ sun8i_pwm_set_value(sun8i_pwm, PWM_PERIOD_REG(ch),
+ PWM_ENTIRE_CYCLE, clk_div << 16);
+ sun8i_pwm_set_value(sun8i_pwm, PWM_CTR_REG(ch),
+ PWM_PRESCAL_K, prescaler << 0);
+ sun8i_pwm_set_value(sun8i_pwm, CLK_CFG_REG(ch),
+ CLK_DIV_M, id << 0);
+
+ /* set duty cycle */
+ val = (prescaler + 1) * div_m_table[id] * clk_div;
+ val = state->period;
+ do_div(val, clk_div);
+ clk_div = state->duty_cycle;
+ do_div(clk_div, val);
+
+ sun8i_pwm_set_value(sun8i_pwm, PWM_PERIOD_REG(ch),
+ PWM_ACT_CYCLE, clk_div << 0);
+
+ return 0;
+}
+
+static int sun8i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ int ret;
+ struct sun8i_pwm_chip *sun8i_pwm = to_sun8i_pwm_chip(chip);
+ struct pwm_state cstate;
+
+ pwm_get_state(pwm, &cstate);
+ if (!cstate.enabled) {
+ ret = clk_prepare_enable(sun8i_pwm->clk);
+ if (ret) {
+ dev_err(chip->dev, "failed to enable PWM clock\n");
+ return ret;
+ }
+ }
+
+ spin_lock(&sun8i_pwm->ctrl_lock);
+
+ if ((cstate.period != state->period) ||
+ (cstate.duty_cycle != state->duty_cycle)) {
+ ret = sun8i_pwm_config(sun8i_pwm, pwm->hwpwm, state);
+ if (ret) {
+ clk_disable_unprepare(sun8i_pwm->clk);
+ spin_unlock(&sun8i_pwm->ctrl_lock);
+ dev_err(chip->dev, "failed to config PWM\n");
+ return ret;
+ }
+ }
+
+ if (state->polarity != cstate.polarity)
+ sun8i_pwm_set_polarity(sun8i_pwm, pwm->hwpwm, state->polarity);
+
+ if (state->enabled) {
+ sun8i_pwm_set_bit(sun8i_pwm,
+ CLK_CFG_REG(pwm->hwpwm), CLK_GATING);
+
+ sun8i_pwm_set_bit(sun8i_pwm,
+ PWM_ENABLE_REG, PWM_EN(pwm->hwpwm));
+ } else {
+ sun8i_pwm_clear_bit(sun8i_pwm,
+ CLK_CFG_REG(pwm->hwpwm), CLK_GATING);
+
+ sun8i_pwm_clear_bit(sun8i_pwm,
+ PWM_ENABLE_REG, PWM_EN(pwm->hwpwm));
+ }
+
+ spin_unlock(&sun8i_pwm->ctrl_lock);
+
+ return 0;
+}
+
+static void sun8i_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ struct sun8i_pwm_chip *sun8i_pwm = to_sun8i_pwm_chip(chip);
+ u64 clk_rate, tmp;
+ u32 val;
+ u16 clk_div, act_cycle;
+ u8 prescal, id;
+
+ clk_rate = clk_get_rate(sun8i_pwm->clk);
+
+ val = sun8i_pwm_read(sun8i_pwm, PWM_CTR_REG(pwm->hwpwm));
+ if (PWM_ACT_STA & val)
+ state->polarity = PWM_POLARITY_NORMAL;
+ else
+ state->polarity = PWM_POLARITY_INVERSED;
+
+ prescal = PWM_PRESCAL_K & val;
+
+ val = sun8i_pwm_read(sun8i_pwm, PWM_ENABLE_REG);
+ if (PWM_EN(pwm->hwpwm) & val)
+ state->enabled = true;
+ else
+ state->enabled = false;
+
+ val = sun8i_pwm_read(sun8i_pwm, PWM_PERIOD_REG(pwm->hwpwm));
+ act_cycle = PWM_ACT_CYCLE & val;
+ clk_div = val >> 16;
+
+ val = sun8i_pwm_read(sun8i_pwm, CLK_CFG_REG(pwm->hwpwm));
+ id = CLK_DIV_M & val;
+
+ tmp = act_cycle * prescal * div_m_table[id] * NSEC_PER_SEC;
+ state->duty_cycle = DIV_ROUND_CLOSEST_ULL(tmp, clk_rate);
+ tmp = clk_div * prescal * div_m_table[id] * NSEC_PER_SEC;
+ state->period = DIV_ROUND_CLOSEST_ULL(tmp, clk_rate);
+}
+
+static const struct regmap_config sun8i_pwm_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = CAPTURE_FALL_REG(7),
+};
+
+static const struct pwm_ops sun8i_pwm_ops = {
+ .apply = sun8i_pwm_apply,
+ .get_state = sun8i_pwm_get_state,
+ .owner = THIS_MODULE,
+};
+
+static const struct sun8i_pwm_data sun8i_pwm_data_r40 = {
+ .has_prescaler_bypass = false,
+ .has_rdy = true,
+ .npwm = 8,
+};
+
+static const struct of_device_id sun8i_pwm_dt_ids[] = {
+ {
+ .compatible = "allwinner,sun8i-r40-pwm",
+ .data = &sun8i_pwm_data_r40,
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, sun8i_pwm_dt_ids);
+
+static int sun8i_pwm_probe(struct platform_device *pdev)
+{
+ struct sun8i_pwm_chip *pwm;
+ struct resource *res;
+ int ret;
+ const struct of_device_id *match;
+
+ match = of_match_device(sun8i_pwm_dt_ids, &pdev->dev);
+
+ pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
+ if (!pwm)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ pwm->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(pwm->base))
+ return PTR_ERR(pwm->base);
+
+ pwm->regmap = devm_regmap_init_mmio(&pdev->dev, pwm->base,
+ &sun8i_pwm_regmap_config);
+
+ pwm->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(pwm->clk))
+ return PTR_ERR(pwm->clk);
+
+ pwm->data = match->data;
+ pwm->chip.dev = &pdev->dev;
+ pwm->chip.ops = &sun8i_pwm_ops;
+ pwm->chip.base = -1;
+ pwm->chip.npwm = pwm->data->npwm;
+ pwm->chip.of_xlate = of_pwm_xlate_with_flags;
+ pwm->chip.of_pwm_n_cells = 3;
+
+ spin_lock_init(&pwm->ctrl_lock);
+
+ ret = pwmchip_add(&pwm->chip);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to add PWM chip: %d\n", ret);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, pwm);
+
+ return 0;
+}
+
+static int sun8i_pwm_remove(struct platform_device *pdev)
+{
+ struct sun8i_pwm_chip *pwm = platform_get_drvdata(pdev);
+
+ return pwmchip_remove(&pwm->chip);
+}
+
+static struct platform_driver sun8i_pwm_driver = {
+ .driver = {
+ .name = "sun8i-r40-pwm",
+ .of_match_table = sun8i_pwm_dt_ids,
+ },
+ .probe = sun8i_pwm_probe,
+ .remove = sun8i_pwm_remove,
+};
+module_platform_driver(sun8i_pwm_driver);
+
+MODULE_ALIAS("platform:sun8i-r40-pwm");
+MODULE_AUTHOR("Hao Zhang <hao5781286@gmail.com>");
+MODULE_DESCRIPTION("Allwinner sun8i-r40 PWM driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related
* [PATCH v1 3/4] ARM: dts: add pwm pins for r40.
From: hao_zhang @ 2018-01-11 11:33 UTC (permalink / raw)
To: linux-arm-kernel
This patch add pwm pins for r40.
Signed-off-by: hao_zhang <hao5781286@gmail.com>
---
arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
index 8c5efe2..6cf6273 100644
--- a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
+++ b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
@@ -196,6 +196,12 @@
status = "okay";
};
+&pwm {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm_pins>;
+ status = "okay";
+};
+
&uart0 {
pinctrl-names = "default";
pinctrl-0 = <&uart0_pb_pins>;
--
2.7.4
^ permalink raw reply related
* [PATCH v1 4/4] ARM: dts: add pwm node for r40.
From: hao_zhang @ 2018-01-11 11:34 UTC (permalink / raw)
To: linux-arm-kernel
This patch add pwm node for r40.
Signed-off-by: hao_zhang <hao5781286@gmail.com>
---
arch/arm/boot/dts/sun8i-r40.dtsi | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
index 173dcc1..84c963c 100644
--- a/arch/arm/boot/dts/sun8i-r40.dtsi
+++ b/arch/arm/boot/dts/sun8i-r40.dtsi
@@ -295,6 +295,11 @@
bias-pull-up;
};
+ pwm_pins: pwm-pins {
+ pins = "PB2", "PB3";
+ function = "pwm";
+ };
+
uart0_pb_pins: uart0-pb-pins {
pins = "PB22", "PB23";
function = "uart0";
@@ -306,6 +311,14 @@
reg = <0x01c20c90 0x10>;
};
+ pwm: pwm at 1c23400 {
+ compatible = "allwinner,sun8i-r40-pwm";
+ reg = <0x01c23400 0x154>;
+ clocks = <&osc24M>;
+ #pwm-cells = <3>;
+ status = "disabled";
+ };
+
uart0: serial at 1c28000 {
compatible = "snps,dw-apb-uart";
reg = <0x01c28000 0x400>;
--
2.7.4
^ permalink raw reply related
* [PATCH] arm64: kdump: retain reserved memory regions
From: AKASHI Takahiro @ 2018-01-11 11:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5A55F87F.2080501@arm.com>
James,
On Wed, Jan 10, 2018 at 11:26:55AM +0000, James Morse wrote:
> Hi Akashi,
>
> On 10/01/18 10:09, AKASHI Takahiro wrote:
> > This is a fix against the issue that crash dump kernel may hang up
> > during booting, which can happen on any ACPI-based system with "ACPI
> > Reclaim Memory."
> >
> > <kicking off kdump after panic>
> > Bye!
> > (snip...)
> > ACPI: Core revision 20170728
> > pud=000000002e7d0003, *pmd=000000002e7c0003, *pte=00e8000039710707
> > Internal error: Oops: 96000021 [#1] SMP
> > Modules linked in:
> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc6 #1
> > task: ffff000008d05180 task.stack: ffff000008cc0000
> > PC is at acpi_ns_lookup+0x25c/0x3c0
> > LR is at acpi_ds_load1_begin_op+0xa4/0x294
> > (snip...)
> > Process swapper/0 (pid: 0, stack limit = 0xffff000008cc0000)
> > Call trace:
> > (snip...)
> > [<ffff0000084a6764>] acpi_ns_lookup+0x25c/0x3c0
> > [<ffff00000849b4f8>] acpi_ds_load1_begin_op+0xa4/0x294
> > [<ffff0000084ad4ac>] acpi_ps_build_named_op+0xc4/0x198
> > [<ffff0000084ad6cc>] acpi_ps_create_op+0x14c/0x270
> > [<ffff0000084acfa8>] acpi_ps_parse_loop+0x188/0x5c8
> > [<ffff0000084ae048>] acpi_ps_parse_aml+0xb0/0x2b8
> > [<ffff0000084a8e10>] acpi_ns_one_complete_parse+0x144/0x184
> > [<ffff0000084a8e98>] acpi_ns_parse_table+0x48/0x68
> > [<ffff0000084a82cc>] acpi_ns_load_table+0x4c/0xdc
> > [<ffff0000084b32f8>] acpi_tb_load_namespace+0xe4/0x264
> > [<ffff000008baf9b4>] acpi_load_tables+0x48/0xc0
> > [<ffff000008badc20>] acpi_early_init+0x9c/0xd0
> > [<ffff000008b70d50>] start_kernel+0x3b4/0x43c
> > Code: b9008fb9 2a000318 36380054 32190318 (b94002c0)
> > ---[ end trace c46ed37f9651c58e ]---
> > Kernel panic - not syncing: Fatal exception
> > Rebooting in 10 seconds..
> >
> > (diagnosis)
> > * This fault is a data abort, alignment fault (ESR=0x96000021)
> > during reading out ACPI table.
> > * Initial ACPI tables are normally stored in system ram and marked as
> > "ACPI Reclaim memory" by the firmware.
> > * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
> > memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
> > removing NOMAP bit and they are instead "memblock-reserved".
> > * When crash dump kernel boots up, it tries to accesses ACPI tables by
> > ioremap'ing them (through acpi_os_ioremap()).
> > * Since those regions are not included in device tree's
> > "usable-memory-range" and so not recognized as part of crash dump
> > kernel's system ram, ioremap() will create a non-cacheable mapping here.
>
> Ugh, because acpi_os_ioremap() looks at the efi memory map through the prism of
> what we pulled into memblock, which is different during kdump.
>
> Is an alternative to teach acpi_os_ioremap() to ask
> efi_mem_attributes() directly for the attributes to use?
> (e.g. arch_apei_get_mem_attribute())
I didn't think of this approach.
Do you mean a change like the patch below?
(I'm still debugging this code since the kernel fails to boot.)
Thanks,
-Takahiro AKASHI
>
> > * ACPI accessor/helper functions are compiled in without unaligned access
> > support (ACPI_MISALIGNMENT_NOT_SUPPORTED), eventually ending up a fatal
> > panic when accessing ACPI tables.
> >
> > With this patch, all the reserved memory regions, as well as NOMAP-
> > attributed ones which are presumably ACPI runtime code and data, are set
> > to be retained in system ram even if they are outside of usable memory
> > range specified by device tree blob. Accordingly, ACPI tables are mapped
> > as cacheable and can be safely accessed without causing unaligned access
> > faults.
>
>
> Thanks,
>
> James
===8<===
diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 32f465a80e4e..6953aaaf2bfa 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -16,6 +16,7 @@
#include <linux/psci.h>
#include <asm/cputype.h>
+#include <asm/io.h>
#include <asm/smp_plat.h>
#include <asm/tlbflush.h>
@@ -29,18 +30,13 @@
/* Basic configuration for ACPI */
#ifdef CONFIG_ACPI
+pgprot_t __acpi_get_mem_attribute(phys_addr_t addr);
+
/* ACPI table mapping after acpi_permanent_mmap is set */
static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys,
acpi_size size)
{
- /*
- * EFI's reserve_regions() call adds memory with the WB attribute
- * to memblock via early_init_dt_add_memory_arch().
- */
- if (!memblock_is_memory(phys))
- return ioremap(phys, size);
-
- return ioremap_cache(phys, size);
+ return __ioremap(phys, size, __acpi_get_mem_attribute(phys));
}
#define acpi_os_ioremap acpi_os_ioremap
@@ -125,7 +121,10 @@ static inline const char *acpi_get_enable_method(int cpu)
* for compatibility.
*/
#define acpi_disable_cmcff 1
-pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr);
+static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+{
+ return __acpi_get_mem_attribute(addr);
+}
#endif /* CONFIG_ACPI_APEI */
#ifdef CONFIG_ACPI_NUMA
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3162715ed78..43e9d8371f88 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -239,8 +239,7 @@ void __init acpi_boot_table_init(void)
}
}
-#ifdef CONFIG_ACPI_APEI
-pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
{
/*
* According to "Table 8 Map: EFI memory types to AArch64 memory
@@ -261,4 +260,3 @@ pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
return __pgprot(PROT_NORMAL_NC);
return __pgprot(PROT_DEVICE_nGnRnE);
}
-#endif
^ permalink raw reply related
* [linux-sunxi] [PATCH 1/7] pinctrl: sunxi: add support for pin controllers without bus gate
From: Andre Przywara @ 2018-01-11 11:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180111104100.j5rwitma3wgtdivm@flea.lan>
Hi,
On 11/01/18 10:41, Maxime Ripard wrote:
> On Thu, Jan 11, 2018 at 10:23:52AM +0000, Andre Przywara wrote:
>> Hi,
>>
>> On 11/01/18 10:14, Chen-Yu Tsai wrote:
>>> On Thu, Jan 11, 2018 at 6:08 PM, Andre Przywara <andre.przywara@arm.com> wrote:
>>>> Hi,
>>>>
>>>> On 06/01/18 04:23, Icenowy Zheng wrote:
>>>>> The Allwinner H6 pin controllers (both the main one and the CPUs one)
>>>>> have no bus gate clocks.
>>>>>
>>>>> Add support for this kind of pin controllers.
>>>>>
>>>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>>>>> ---
>>>>> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 30 ++++++++++++++++++++----------
>>>>> drivers/pinctrl/sunxi/pinctrl-sunxi.h | 1 +
>>>>> 2 files changed, 21 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>>>>> index 4b6cb25bc796..68cd505679d9 100644
>>>>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>>>>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>>>>> @@ -1182,7 +1182,12 @@ static int sunxi_pinctrl_setup_debounce(struct sunxi_pinctrl *pctl,
>>>>> unsigned int hosc_div, losc_div;
>>>>> struct clk *hosc, *losc;
>>>>> u8 div, src;
>>>>> - int i, ret;
>>>>> + int i, ret, clk_count;
>>>>> +
>>>>> + if (pctl->desc->without_bus_gate)
>>>>> + clk_count = 2;
>>>>> + else
>>>>> + clk_count = 3;
>>>>>
>>>>> /* Deal with old DTs that didn't have the oscillators */
>>>>> if (of_count_phandle_with_args(node, "clocks", "#clock-cells") != 3)
>>>>> @@ -1360,15 +1365,19 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
>>>>> goto gpiochip_error;
>>>>> }
>>>>>
>>>>> - clk = devm_clk_get(&pdev->dev, NULL);
>>>>> - if (IS_ERR(clk)) {
>>>>> - ret = PTR_ERR(clk);
>>>>> - goto gpiochip_error;
>>>>> - }
>>>>> + if (!desc->without_bus_gate) {
>>>>
>>>> Do we really need explicit support for that case?
>>>> Can't we have something that works automatically?
>>>>
>>>> if (node has clock-names property) (A)
>>>> use clocks as enumerated and named there
>>>
>>> You still need to know if the hardware has a bus gate or not.
>>> If it's missing, and it's disabled, you end up with unusable
>>> hardware.
>>
>> Yes. So what? If you have a broken DT, it will not work. Just don't do
>> it. I don't understand why we want to defend against this case.
>
> This is not the point, but rather: if we have a way to detect easily
> that the device tree is missing a property that is missing in our
> binding, why shouldn't we do it?
>
> We're already doing it for reg and interrupts for example, why not for
> the clocks?
>
>>> Unless you are fully trusting the device tree to be correct.
>>
>> Sorry, but what else do we trust?
>>
>>> IMHO that makes for hard to find bugs during SoC bringup.
>>
>> I am not sure if that is really an issue. I would expect people
>> doing SoC bringup to be able to cope with those kinds of problems.
>
> Riiiight, because it worked so well in the past. We definitely didn't
> overlooked some clocks used for debouncing in this particular driver,
> or some to get the timekeeping right in the RTC.
I think that's a different issue, because debouncing is an optional
feature. How would those kind of explicit molly guards here have
prevented this omission in the past, when we only discovered that later?
> The argument that "anyone who codes in the kernel should just know
> better" doesn't work, on multiple levels. Because anyone that actually
> knows better can make a mistake or overlook some feature (because you
> didn't have your morning coffee yet, or because it was undocumented)
> and because you just make someone that doesn't feel bad.
I agree to that. But: If something doesn't work, checking clocks and
reset would be my first impulse. And Icenowy did exactly that and
quickly found it.
Plus this only protects against known pitfalls.
> So, yes, we cannot not trust the device tree. But if we have a way to
> detect simple mistakes in the binding, we should also do it.
I totally honour that, I am just wondering what price we pay for that.
This kind of: "We need three clocks here, or wait, two clock in this
particular case" sounds a bit dodgy and little future proof to me.
Which is somewhat confirmed by the fact that we need to adjust this
check now. So I suggest we remove it, as we have more, actual checks
afterwards anyway. That should cover future extensions without further ado:
The clock-names property should cater nicely for those cases, hence my
suggestion to rely on it. Plus we need to support the legacy DTs with
just a single clock and no clock-names. Done.
So I think we should change the devm_get_clk(..., NULL) to
devm_get_clk(..., "apb"), and then check for just a single unnamed clock
if that fails (older DTs), or no clock at all, if we need to support
future SoCs without debouncing.
Looking deeper I actually think we are not binding compliant at the
moment, as we rely on the "apb" clock to be the first one, however
clock-names = "hosc", "losc", "apb" would be perfectly legal as well, as
we don't document a certain order of the clock - which is not necessary
with clock-names.
I can make a patch if we agree on that.
Cheers,
Andre.
^ permalink raw reply
* [linux-sunxi] [PATCH 1/7] pinctrl: sunxi: add support for pin controllers without bus gate
From: Andre Przywara @ 2018-01-11 11:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180106042326.46519-1-icenowy@aosc.io>
Hi,
another take to avoid this patch at all, I just remembered this from an
IRC discussion before:
On 06/01/18 04:23, Icenowy Zheng wrote:
> The Allwinner H6 pin controllers (both the main one and the CPUs one)
> have no bus gate clocks.
I don't think this is true. The pin controller *needs* an APB clock,
it's just not gate-able or not exposed or documented.
The "system bus tree" on page 90 in the manual shows that the "GPIO"
block is located on the APB1 bus.
So can't we just reference this apb clock directly? That would be much
cleaner, "more" correct and require less changes: "The best patch is no
patch":
clocks = <&ccu APB1>, <&osc24M>, <&osc32k>;
/* or whatever this APB clock is actually called. */
clock-names = "apb", "hosc", "losc";
Cheers,
Andre.
>
> Add support for this kind of pin controllers.
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 30 ++++++++++++++++++++----------
> drivers/pinctrl/sunxi/pinctrl-sunxi.h | 1 +
> 2 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 4b6cb25bc796..68cd505679d9 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -1182,7 +1182,12 @@ static int sunxi_pinctrl_setup_debounce(struct sunxi_pinctrl *pctl,
> unsigned int hosc_div, losc_div;
> struct clk *hosc, *losc;
> u8 div, src;
> - int i, ret;
> + int i, ret, clk_count;
> +
> + if (pctl->desc->without_bus_gate)
> + clk_count = 2;
> + else
> + clk_count = 3;
>
> /* Deal with old DTs that didn't have the oscillators */
> if (of_count_phandle_with_args(node, "clocks", "#clock-cells") != 3)
> @@ -1360,15 +1365,19 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
> goto gpiochip_error;
> }
>
> - clk = devm_clk_get(&pdev->dev, NULL);
> - if (IS_ERR(clk)) {
> - ret = PTR_ERR(clk);
> - goto gpiochip_error;
> - }
> + if (!desc->without_bus_gate) {
> + clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(clk)) {
> + ret = PTR_ERR(clk);
> + goto gpiochip_error;
> + }
>
> - ret = clk_prepare_enable(clk);
> - if (ret)
> - goto gpiochip_error;
> + ret = clk_prepare_enable(clk);
> + if (ret)
> + goto gpiochip_error;
> + } else {
> + clk = NULL;
> + }
>
> pctl->irq = devm_kcalloc(&pdev->dev,
> pctl->desc->irq_banks,
> @@ -1425,7 +1434,8 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
> return 0;
>
> clk_error:
> - clk_disable_unprepare(clk);
> + if (clk)
> + clk_disable_unprepare(clk);
> gpiochip_error:
> gpiochip_remove(pctl->chip);
> return ret;
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> index 11b128f54ed2..ccb6230f0bb5 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> @@ -113,6 +113,7 @@ struct sunxi_pinctrl_desc {
> unsigned irq_bank_base;
> bool irq_read_needs_mux;
> bool disable_strict_mode;
> + bool without_bus_gate;
> };
>
> struct sunxi_pinctrl_function {
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox