Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 07/14] arm: Use STACK_TOP when computing mmap base address
From: Alexandre Ghiti @ 2019-08-08  6:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Albert Ou, Kees Cook, Alexandre Ghiti, linux-mm, Catalin Marinas,
	Palmer Dabbelt, Will Deacon, Russell King, Ralf Baechle,
	linux-kernel, linux-fsdevel, Luis Chamberlain, Paul Burton,
	Paul Walmsley, James Hogan, linux-riscv, linux-mips,
	Christoph Hellwig, linux-arm-kernel, Alexander Viro
In-Reply-To: <20190808061756.19712-1-alex@ghiti.fr>

mmap base address must be computed wrt stack top address, using TASK_SIZE
is wrong since STACK_TOP and TASK_SIZE are not equivalent.

Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
Acked-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
---
 arch/arm/mm/mmap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/mmap.c b/arch/arm/mm/mmap.c
index bff3d00bda5b..0b94b674aa91 100644
--- a/arch/arm/mm/mmap.c
+++ b/arch/arm/mm/mmap.c
@@ -19,7 +19,7 @@
 
 /* gap between mmap and stack */
 #define MIN_GAP		(128*1024*1024UL)
-#define MAX_GAP		((TASK_SIZE)/6*5)
+#define MAX_GAP		((STACK_TOP)/6*5)
 #define STACK_RND_MASK	(0x7ff >> (PAGE_SHIFT - 12))
 
 static int mmap_is_legacy(struct rlimit *rlim_stack)
@@ -51,7 +51,7 @@ static unsigned long mmap_base(unsigned long rnd, struct rlimit *rlim_stack)
 	else if (gap > MAX_GAP)
 		gap = MAX_GAP;
 
-	return PAGE_ALIGN(TASK_SIZE - gap - rnd);
+	return PAGE_ALIGN(STACK_TOP - gap - rnd);
 }
 
 /*
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v2] arm64: dts: allwinner: a64: Enable eMMC on A64-OLinuXino
From: Sunil Mohan Adapa @ 2019-08-08  6:25 UTC (permalink / raw)
  To: Chen-Yu Tsai, Maxime Ripard; +Cc: Martin Ayotte, linux-arm-kernel
In-Reply-To: <CAGb2v66EjQ-_QNsJ+xR0LcR983whU-hF9SZ2dSX_i5v7qcEyww@mail.gmail.com>


[-- Attachment #1.1.1: Type: text/plain, Size: 4460 bytes --]



On 07/08/19 10:12 pm, Chen-Yu Tsai wrote:
> On Wed, Aug 7, 2019 at 10:45 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>>
>> Hi,
>>
>> On Wed, Aug 07, 2019 at 08:09:19PM +0800, Chen-Yu Tsai wrote:
>>> On Wed, Aug 7, 2019 at 8:01 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>>>>
>>>> On Tue, Aug 06, 2019 at 02:25:17PM +0800, Chen-Yu Tsai wrote:
>>>>> On Mon, Aug 5, 2019 at 8:58 PM Martin Ayotte <martinayotte@gmail.com> wrote:
>>>>>>
>>>>>> Fine for me too.
>>>>>>
>>>>>> Thanks .
>>>>>>
>>>>>> -----Message d'origine-----
>>>>>> De : Sunil Mohan Adapa [mailto:sunil@medhas.org]
>>>>>> Envoyé : Monday, August 05, 2019 1:25 AM
>>>>>> Ą : Chen-Yu Tsai
>>>>>> Cc : Maxime Ripard; Martin Ayotte; linux-arm-kernel
>>>>>> Objet : Re: [PATCH v2] arm64: dts: allwinner: a64: Enable eMMC on
>>>>>> A64-OLinuXino
>>>>>>
>>>>>> On 04/08/19 8:33 pm, Chen-Yu Tsai wrote:
>>>>>>> On Fri, Aug 2, 2019 at 2:47 AM Sunil Mohan Adapa <sunil@medhas.org> wrote:
>>>>>>>>
>>>>>>>> On 01/08/19 6:49 am, Martin Ayotte wrote:
>>>>>>>>> If my SOB could help here, I don't mind since I've done the commit
>>>>>>>>> more than a year ago for Armbian ...
>>>>>>>>>
>>>>>>>>> Signed-off-by: Martin Ayotte <martinayotte@gmai.com>
>>>>>>>>> Tested-by: Martin Ayotte <martinayotte@gmai.com>
>>>>>>>> gmai.com is likely a typo.
>>>>>>>>
>>>>>>>>> On Wed, Jul 31, 2019 at 10:42 PM Chen-Yu Tsai <wens@csie.org
>>>>>>>>>
>>>>>>>>>> Thanks. The patch looks good overall. The authorship is a little
>>>>>>>>>> confusing though. If it was initially done by Martin (CC-ed), then
>>>>>>>>>> he should be the author, and we should get his Signed-off-by if
>>>>>>>>>> possible.
>>>>>>>>
>>>>>>>> Martin is indeed the original author of the patch. Thank you for
>>>>>> reviewing.
>>>>>>>
>>>>>>> I'd like to apply this patch with Martin as the author, if that's OK with
>>>>>> you
>>>>>>> both?
>>>>>>
>>>>>> That is completely okay with me.
>>>>>
>>>>> Applied for 5.4.
>>>>>
>>>>> I reordered the tags so they make more sense:
>>>>>
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git/commit/?h=sunxi/dt-for-5.4&id=0834887732df5af41b59b2e4d530fc1f5478965f
>>>>
>>>> Sorry for being late on this, but it looks like the eMMC, NAND and SPI
>>>> pins are conflicting on the A64-Olinuxino design.
>>>>
>>>> There's no configuration with a NAND, so we don't really need to worry
>>>> about that, but if we merge this in the main DT, we'll prevent anyone
>>>> from using that DT on an olinuxino with a SPI flash.
>>>>
>>>> I think we should just create emmc and SPI-flash variants of that DT.
>>>
>>> Actually they aren't. Olimex specifically uses eMMC modules that don't
>>> use the data strobe line, so SPI can be used together.
>>
>> Ah, right.
>>
>> Still, this creates a precedent that I'm not really comfortable
>> with. Three actually.
>>
>> Merging this in the main DT means three things:
>>   - We're not consistent anymore, including within the olinuxino
>>     boards only. A20 Olinuxino is pretty much in the same situation,
>>     yet we dealt with it differently.
>>   - This means that this will create a spurious device and report
>>     errors in the kernel message and whenever someone will try to
>>     access the device on boards that don't have it wired. This
>>     shouldn't happen and we really shouldn't expose devices that just
>>     aren't there, just like you don't have all the devices that are
>>     not connected on your USB connector.
>>   - Finally, this means that in order to keep it somewhat consistent,
>>     we would have to merge the SPI flash in the main DT too. This will
>>     prevent people without a SPI flash to use the SPI signals on the
>>     UEXT connector for something else, which again goes against the
>>     policy we've had for basically any other board.
> 
> OK. Shall we back it out and figure out something else?

I can try to help with alternate implementation with some guidance. I
can also test any patches on an A64-OLinuXino model with eMMC. So, do we
create a new -emmc.dts like in case of A20 OLinuXino?

BTW, a basic question: how does u-boot know which variant of dtb (with
-emmc or without) it has load before booting Linux? Does this need to be
hardcoded now into the boot script? Currently, in Debian the DTB name is
chosen by u-boot and not present in the boot script.

Thanks,

-- 
Sunil


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v6 06/14] arm: Properly account for stack randomization and stack guard gap
From: Alexandre Ghiti @ 2019-08-08  6:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Albert Ou, Kees Cook, Alexandre Ghiti, linux-mm, Catalin Marinas,
	Palmer Dabbelt, Will Deacon, Russell King, Ralf Baechle,
	linux-kernel, linux-fsdevel, Luis Chamberlain, Paul Burton,
	Paul Walmsley, James Hogan, linux-riscv, linux-mips,
	Christoph Hellwig, linux-arm-kernel, Alexander Viro
In-Reply-To: <20190808061756.19712-1-alex@ghiti.fr>

This commit takes care of stack randomization and stack guard gap when
computing mmap base address and checks if the task asked for randomization.

This fixes the problem uncovered and not fixed for arm here:
https://lkml.kernel.org/r/20170622200033.25714-1-riel@redhat.com

Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
Acked-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
---
 arch/arm/mm/mmap.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/mmap.c b/arch/arm/mm/mmap.c
index f866870db749..bff3d00bda5b 100644
--- a/arch/arm/mm/mmap.c
+++ b/arch/arm/mm/mmap.c
@@ -18,8 +18,9 @@
 	 (((pgoff)<<PAGE_SHIFT) & (SHMLBA-1)))
 
 /* gap between mmap and stack */
-#define MIN_GAP (128*1024*1024UL)
-#define MAX_GAP ((TASK_SIZE)/6*5)
+#define MIN_GAP		(128*1024*1024UL)
+#define MAX_GAP		((TASK_SIZE)/6*5)
+#define STACK_RND_MASK	(0x7ff >> (PAGE_SHIFT - 12))
 
 static int mmap_is_legacy(struct rlimit *rlim_stack)
 {
@@ -35,6 +36,15 @@ static int mmap_is_legacy(struct rlimit *rlim_stack)
 static unsigned long mmap_base(unsigned long rnd, struct rlimit *rlim_stack)
 {
 	unsigned long gap = rlim_stack->rlim_cur;
+	unsigned long pad = stack_guard_gap;
+
+	/* Account for stack randomization if necessary */
+	if (current->flags & PF_RANDOMIZE)
+		pad += (STACK_RND_MASK << PAGE_SHIFT);
+
+	/* Values close to RLIM_INFINITY can overflow. */
+	if (gap + pad > gap)
+		gap += pad;
 
 	if (gap < MIN_GAP)
 		gap = MIN_GAP;
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v6 05/14] arm64, mm: Make randomization selected by generic topdown mmap layout
From: Alexandre Ghiti @ 2019-08-08  6:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Albert Ou, Kees Cook, Alexandre Ghiti, linux-mm, Catalin Marinas,
	Palmer Dabbelt, Will Deacon, Russell King, Ralf Baechle,
	linux-kernel, linux-fsdevel, Luis Chamberlain, Paul Burton,
	Paul Walmsley, James Hogan, linux-riscv, linux-mips,
	Christoph Hellwig, linux-arm-kernel, Alexander Viro
In-Reply-To: <20190808061756.19712-1-alex@ghiti.fr>

This commits selects ARCH_HAS_ELF_RANDOMIZE when an arch uses the generic
topdown mmap layout functions so that this security feature is on by
default.

Note that this commit also removes the possibility for arm64 to have elf
randomization and no MMU: without MMU, the security added by randomization
is worth nothing.

Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
---
 arch/Kconfig                |  1 +
 arch/arm64/Kconfig          |  1 -
 arch/arm64/kernel/process.c |  8 --------
 mm/util.c                   | 11 +++++++++--
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index a0bb6fa4d381..d4c1f0551dfe 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -705,6 +705,7 @@ config HAVE_ARCH_COMPAT_MMAP_BASES
 config ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
 	bool
 	depends on MMU
+	select ARCH_HAS_ELF_RANDOMIZE
 
 config HAVE_COPY_THREAD_TLS
 	bool
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 14a194e63458..399f595ef852 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -16,7 +16,6 @@ config ARM64
 	select ARCH_HAS_DMA_MMAP_PGPROT
 	select ARCH_HAS_DMA_PREP_COHERENT
 	select ARCH_HAS_ACPI_TABLE_UPGRADE if ACPI
-	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_FAST_MULTIPLIER
 	select ARCH_HAS_FORTIFY_SOURCE
 	select ARCH_HAS_GCOV_PROFILE_ALL
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index f674f28df663..8ddc2471b054 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -548,14 +548,6 @@ unsigned long arch_align_stack(unsigned long sp)
 	return sp & ~0xf;
 }
 
-unsigned long arch_randomize_brk(struct mm_struct *mm)
-{
-	if (is_compat_task())
-		return randomize_page(mm->brk, SZ_32M);
-	else
-		return randomize_page(mm->brk, SZ_1G);
-}
-
 /*
  * Called from setup_new_exec() after (COMPAT_)SET_PERSONALITY.
  */
diff --git a/mm/util.c b/mm/util.c
index 0781e5575cb3..16f1e56e2996 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -321,7 +321,15 @@ unsigned long randomize_stack_top(unsigned long stack_top)
 }
 
 #ifdef CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
-#ifdef CONFIG_ARCH_HAS_ELF_RANDOMIZE
+unsigned long arch_randomize_brk(struct mm_struct *mm)
+{
+	/* Is the current task 32bit ? */
+	if (!IS_ENABLED(CONFIG_64BIT) || is_compat_task())
+		return randomize_page(mm->brk, SZ_32M);
+
+	return randomize_page(mm->brk, SZ_1G);
+}
+
 unsigned long arch_mmap_rnd(void)
 {
 	unsigned long rnd;
@@ -335,7 +343,6 @@ unsigned long arch_mmap_rnd(void)
 
 	return rnd << PAGE_SHIFT;
 }
-#endif /* CONFIG_ARCH_HAS_ELF_RANDOMIZE */
 
 static int mmap_is_legacy(struct rlimit *rlim_stack)
 {
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v6 04/14] arm64, mm: Move generic mmap layout functions to mm
From: Alexandre Ghiti @ 2019-08-08  6:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Albert Ou, Kees Cook, Alexandre Ghiti, linux-mm, Catalin Marinas,
	Palmer Dabbelt, Will Deacon, Russell King, Ralf Baechle,
	linux-kernel, Christoph Hellwig, linux-fsdevel, Luis Chamberlain,
	Paul Burton, Paul Walmsley, James Hogan, linux-riscv, linux-mips,
	Christoph Hellwig, linux-arm-kernel, Alexander Viro
In-Reply-To: <20190808061756.19712-1-alex@ghiti.fr>

arm64 handles top-down mmap layout in a way that can be easily reused
by other architectures, so make it available in mm.

It then introduces a new config ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
that can be set by other architectures to benefit from those functions.
Note that this new config depends on MMU being enabled, if selected
without MMU support, a warning will be thrown.

Suggested-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
---
 arch/Kconfig                       | 10 ++++
 arch/arm64/Kconfig                 |  1 +
 arch/arm64/include/asm/processor.h |  2 -
 arch/arm64/mm/mmap.c               | 76 -----------------------------
 kernel/sysctl.c                    |  6 ++-
 mm/util.c                          | 78 +++++++++++++++++++++++++++++-
 6 files changed, 92 insertions(+), 81 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index a7b57dd42c26..a0bb6fa4d381 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -696,6 +696,16 @@ config HAVE_ARCH_COMPAT_MMAP_BASES
 	  and vice-versa 32-bit applications to call 64-bit mmap().
 	  Required for applications doing different bitness syscalls.
 
+# This allows to use a set of generic functions to determine mmap base
+# address by giving priority to top-down scheme only if the process
+# is not in legacy mode (compat task, unlimited stack size or
+# sysctl_legacy_va_layout).
+# Architecture that selects this option can provide its own version of:
+# - STACK_RND_MASK
+config ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
+	bool
+	depends on MMU
+
 config HAVE_COPY_THREAD_TLS
 	bool
 	help
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 3adcec05b1f6..14a194e63458 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -72,6 +72,7 @@ config ARM64
 	select ARCH_SUPPORTS_INT128 if GCC_VERSION >= 50000 || CC_IS_CLANG
 	select ARCH_SUPPORTS_NUMA_BALANCING
 	select ARCH_WANT_COMPAT_IPC_PARSE_VERSION if COMPAT
+	select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
 	select ARCH_WANT_FRAME_POINTERS
 	select ARCH_WANT_HUGE_PMD_SHARE if ARM64_4K_PAGES || (ARM64_16K_PAGES && !ARM64_VA_BITS_36)
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 844e2964b0f5..65e2de00913f 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -281,8 +281,6 @@ static inline void spin_lock_prefetch(const void *ptr)
 		     "nop") : : "p" (ptr));
 }
 
-#define HAVE_ARCH_PICK_MMAP_LAYOUT
-
 #endif
 
 extern unsigned long __ro_after_init signal_minsigstksz; /* sigframe size */
diff --git a/arch/arm64/mm/mmap.c b/arch/arm64/mm/mmap.c
index e4acaead67de..3028bacbc4e9 100644
--- a/arch/arm64/mm/mmap.c
+++ b/arch/arm64/mm/mmap.c
@@ -20,82 +20,6 @@
 
 #include <asm/cputype.h>
 
-/*
- * Leave enough space between the mmap area and the stack to honour ulimit in
- * the face of randomisation.
- */
-#define MIN_GAP (SZ_128M)
-#define MAX_GAP	(STACK_TOP/6*5)
-
-static int mmap_is_legacy(struct rlimit *rlim_stack)
-{
-	if (current->personality & ADDR_COMPAT_LAYOUT)
-		return 1;
-
-	if (rlim_stack->rlim_cur == RLIM_INFINITY)
-		return 1;
-
-	return sysctl_legacy_va_layout;
-}
-
-unsigned long arch_mmap_rnd(void)
-{
-	unsigned long rnd;
-
-#ifdef CONFIG_COMPAT
-	if (is_compat_task())
-		rnd = get_random_long() & ((1UL << mmap_rnd_compat_bits) - 1);
-	else
-#endif
-		rnd = get_random_long() & ((1UL << mmap_rnd_bits) - 1);
-	return rnd << PAGE_SHIFT;
-}
-
-static unsigned long mmap_base(unsigned long rnd, struct rlimit *rlim_stack)
-{
-	unsigned long gap = rlim_stack->rlim_cur;
-	unsigned long pad = stack_guard_gap;
-
-	/* Account for stack randomization if necessary */
-	if (current->flags & PF_RANDOMIZE)
-		pad += (STACK_RND_MASK << PAGE_SHIFT);
-
-	/* Values close to RLIM_INFINITY can overflow. */
-	if (gap + pad > gap)
-		gap += pad;
-
-	if (gap < MIN_GAP)
-		gap = MIN_GAP;
-	else if (gap > MAX_GAP)
-		gap = MAX_GAP;
-
-	return PAGE_ALIGN(STACK_TOP - gap - rnd);
-}
-
-/*
- * This function, called very early during the creation of a new process VM
- * image, sets up which VM layout function to use:
- */
-void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
-{
-	unsigned long random_factor = 0UL;
-
-	if (current->flags & PF_RANDOMIZE)
-		random_factor = arch_mmap_rnd();
-
-	/*
-	 * Fall back to the standard layout if the personality bit is set, or
-	 * if the expected stack growth is unlimited:
-	 */
-	if (mmap_is_legacy(rlim_stack)) {
-		mm->mmap_base = TASK_UNMAPPED_BASE + random_factor;
-		mm->get_unmapped_area = arch_get_unmapped_area;
-	} else {
-		mm->mmap_base = mmap_base(random_factor, rlim_stack);
-		mm->get_unmapped_area = arch_get_unmapped_area_topdown;
-	}
-}
-
 /*
  * You really shouldn't be using read() or write() on /dev/mem.  This might go
  * away in the future.
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 078950d9605b..00fcea236eba 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -264,7 +264,8 @@ extern struct ctl_table epoll_table[];
 extern struct ctl_table firmware_config_table[];
 #endif
 
-#ifdef HAVE_ARCH_PICK_MMAP_LAYOUT
+#if defined(HAVE_ARCH_PICK_MMAP_LAYOUT) || \
+    defined(CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT)
 int sysctl_legacy_va_layout;
 #endif
 
@@ -1573,7 +1574,8 @@ static struct ctl_table vm_table[] = {
 		.proc_handler	= proc_dointvec,
 		.extra1		= SYSCTL_ZERO,
 	},
-#ifdef HAVE_ARCH_PICK_MMAP_LAYOUT
+#if defined(HAVE_ARCH_PICK_MMAP_LAYOUT) || \
+    defined(CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT)
 	{
 		.procname	= "legacy_va_layout",
 		.data		= &sysctl_legacy_va_layout,
diff --git a/mm/util.c b/mm/util.c
index 15a4fb0f5473..0781e5575cb3 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -17,7 +17,12 @@
 #include <linux/vmalloc.h>
 #include <linux/userfaultfd_k.h>
 #include <linux/elf.h>
+#include <linux/elf-randomize.h>
+#include <linux/personality.h>
 #include <linux/random.h>
+#include <linux/processor.h>
+#include <linux/sizes.h>
+#include <linux/compat.h>
 
 #include <linux/uaccess.h>
 
@@ -315,7 +320,78 @@ unsigned long randomize_stack_top(unsigned long stack_top)
 #endif
 }
 
-#if defined(CONFIG_MMU) && !defined(HAVE_ARCH_PICK_MMAP_LAYOUT)
+#ifdef CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
+#ifdef CONFIG_ARCH_HAS_ELF_RANDOMIZE
+unsigned long arch_mmap_rnd(void)
+{
+	unsigned long rnd;
+
+#ifdef CONFIG_HAVE_ARCH_MMAP_RND_COMPAT_BITS
+	if (is_compat_task())
+		rnd = get_random_long() & ((1UL << mmap_rnd_compat_bits) - 1);
+	else
+#endif /* CONFIG_HAVE_ARCH_MMAP_RND_COMPAT_BITS */
+		rnd = get_random_long() & ((1UL << mmap_rnd_bits) - 1);
+
+	return rnd << PAGE_SHIFT;
+}
+#endif /* CONFIG_ARCH_HAS_ELF_RANDOMIZE */
+
+static int mmap_is_legacy(struct rlimit *rlim_stack)
+{
+	if (current->personality & ADDR_COMPAT_LAYOUT)
+		return 1;
+
+	if (rlim_stack->rlim_cur == RLIM_INFINITY)
+		return 1;
+
+	return sysctl_legacy_va_layout;
+}
+
+/*
+ * Leave enough space between the mmap area and the stack to honour ulimit in
+ * the face of randomisation.
+ */
+#define MIN_GAP		(SZ_128M)
+#define MAX_GAP		(STACK_TOP / 6 * 5)
+
+static unsigned long mmap_base(unsigned long rnd, struct rlimit *rlim_stack)
+{
+	unsigned long gap = rlim_stack->rlim_cur;
+	unsigned long pad = stack_guard_gap;
+
+	/* Account for stack randomization if necessary */
+	if (current->flags & PF_RANDOMIZE)
+		pad += (STACK_RND_MASK << PAGE_SHIFT);
+
+	/* Values close to RLIM_INFINITY can overflow. */
+	if (gap + pad > gap)
+		gap += pad;
+
+	if (gap < MIN_GAP)
+		gap = MIN_GAP;
+	else if (gap > MAX_GAP)
+		gap = MAX_GAP;
+
+	return PAGE_ALIGN(STACK_TOP - gap - rnd);
+}
+
+void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
+{
+	unsigned long random_factor = 0UL;
+
+	if (current->flags & PF_RANDOMIZE)
+		random_factor = arch_mmap_rnd();
+
+	if (mmap_is_legacy(rlim_stack)) {
+		mm->mmap_base = TASK_UNMAPPED_BASE + random_factor;
+		mm->get_unmapped_area = arch_get_unmapped_area;
+	} else {
+		mm->mmap_base = mmap_base(random_factor, rlim_stack);
+		mm->get_unmapped_area = arch_get_unmapped_area_topdown;
+	}
+}
+#elif defined(CONFIG_MMU) && !defined(HAVE_ARCH_PICK_MMAP_LAYOUT)
 void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
 {
 	mm->mmap_base = TASK_UNMAPPED_BASE;
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v2] arm64: mm: print hexadecimal EC value in mem_abort_decode()
From: Anshuman Khandual @ 2019-08-08  6:21 UTC (permalink / raw)
  To: Miles Chen
  Cc: Mark Rutland, wsd_upstream, Catalin Marinas, Will Deacon,
	linux-kernel, linux-mediatek, James Morse, linux-arm-kernel
In-Reply-To: <1565244075.26350.3.camel@mtkswgap22>



On 08/08/2019 11:31 AM, Miles Chen wrote:
> On Thu, 2019-08-08 at 11:19 +0530, Anshuman Khandual wrote:
>>
>> On 08/07/2019 06:03 AM, Miles Chen wrote:
>>> This change prints the hexadecimal EC value in mem_abort_decode(),
>>> which makes it easier to lookup the corresponding EC in
>>> the ARM Architecture Reference Manual.
>>>
>>> The commit 1f9b8936f36f ("arm64: Decode information from ESR upon mem
>>> faults") prints useful information when memory abort occurs. It would
>>> be easier to lookup "0x25" instead of "DABT" in the document. Then we
>>> can check the corresponding ISS.
>>>
>>> For example:
>>> Current	info	  	Document
>>> 		  	EC	Exception class
>>> "CP15 MCR/MRC"		0x3	"MCR or MRC access to CP15a..."
>>> "ASIMD"			0x7	"Access to SIMD or floating-point..."
>>> "DABT (current EL)" 	0x25	"Data Abort taken without..."
>>> ...
>>>
>>> Before:
>>> Unable to handle kernel paging request at virtual address 000000000000c000
>>> Mem abort info:
>>>   ESR = 0x96000046
>>>   Exception class = DABT (current EL), IL = 32 bits
>>>   SET = 0, FnV = 0
>>>   EA = 0, S1PTW = 0
>>> Data abort info:
>>>   ISV = 0, ISS = 0x00000046
>>>   CM = 0, WnR = 1
>>>
>>> After:
>>> Unable to handle kernel paging request at virtual address 000000000000c000
>>> Mem abort info:
>>>   ESR = 0x96000046
>>>   EC = 0x25: DABT (current EL), IL = 32 bits
>>>   SET = 0, FnV = 0
>>>   EA = 0, S1PTW = 0
>>> Data abort info:
>>>   ISV = 0, ISS = 0x00000046
>>>   CM = 0, WnR = 1
>>>
>>> Change since v1:
>>> print "EC" instead of "Exception class"
>>> print EC in fixwidth
>>>
>>> Cc: Mark Rutland <Mark.rutland@arm.com>
>>> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
>>> Cc: James Morse <james.morse@arm.com>
>>> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
>>
>> This version implements the suggestion, hence it should have
>> also contained acked-by tag from Mark from earlier version.
>>
> 
> No problem. Sorry for not including the tag.
> I was not sure if I should add the acked-by tag from Mark in patch v2.

Yeah because V2 has now implemented the suggestion as required for
getting the tag per Mark in V1.

> 
>> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> 
> If I send patch v3, I should include acked-by tag from Mark and
> Reviewed-by tag from you, right?

Right.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v6 03/14] arm64: Consider stack randomization for mmap base only when necessary
From: Alexandre Ghiti @ 2019-08-08  6:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Albert Ou, Kees Cook, Alexandre Ghiti, linux-mm, Catalin Marinas,
	Palmer Dabbelt, Will Deacon, Russell King, Ralf Baechle,
	linux-kernel, linux-fsdevel, Luis Chamberlain, Paul Burton,
	Paul Walmsley, James Hogan, linux-riscv, linux-mips,
	Christoph Hellwig, linux-arm-kernel, Alexander Viro
In-Reply-To: <20190808061756.19712-1-alex@ghiti.fr>

Do not offset mmap base address because of stack randomization if
current task does not want randomization.
Note that x86 already implements this behaviour.

Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
---
 arch/arm64/mm/mmap.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/mm/mmap.c b/arch/arm64/mm/mmap.c
index bb0140afed66..e4acaead67de 100644
--- a/arch/arm64/mm/mmap.c
+++ b/arch/arm64/mm/mmap.c
@@ -54,7 +54,11 @@ unsigned long arch_mmap_rnd(void)
 static unsigned long mmap_base(unsigned long rnd, struct rlimit *rlim_stack)
 {
 	unsigned long gap = rlim_stack->rlim_cur;
-	unsigned long pad = (STACK_RND_MASK << PAGE_SHIFT) + stack_guard_gap;
+	unsigned long pad = stack_guard_gap;
+
+	/* Account for stack randomization if necessary */
+	if (current->flags & PF_RANDOMIZE)
+		pad += (STACK_RND_MASK << PAGE_SHIFT);
 
 	/* Values close to RLIM_INFINITY can overflow. */
 	if (gap + pad > gap)
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* RE: [PATCH] arm64: mm: add missing PTE_SPECIAL in pte_mkdevmap on arm64
From: Justin He (Arm Technology China) @ 2019-08-08  6:20 UTC (permalink / raw)
  To: Anshuman Khandual, Catalin Marinas, Will Deacon, Mark Rutland,
	James Morse
  Cc: Christoffer Dall, linux-kernel@vger.kernel.org, Jun Yao, Qian Cai,
	Punit Agrawal, Thomas Gleixner, Robin Murphy, Alex Van Brunt,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <ce0be561-117c-ef94-6a26-f88c3ba21096@arm.com>

Hi Anshuman
Thanks for the comments, please see my comments below

> -----Original Message-----
> From: Anshuman Khandual <anshuman.khandual@arm.com>
> Sent: 2019年8月8日 13:19
> To: Justin He (Arm Technology China) <Justin.He@arm.com>; Catalin
> Marinas <Catalin.Marinas@arm.com>; Will Deacon <will@kernel.org>;
> Mark Rutland <Mark.Rutland@arm.com>; James Morse
> <James.Morse@arm.com>
> Cc: Christoffer Dall <Christoffer.Dall@arm.com>; Punit Agrawal
> <punitagrawal@gmail.com>; Qian Cai <cai@lca.pw>; Jun Yao
> <yaojun8558363@gmail.com>; Alex Van Brunt <avanbrunt@nvidia.com>;
> Robin Murphy <Robin.Murphy@arm.com>; Thomas Gleixner
> <tglx@linutronix.de>; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] arm64: mm: add missing PTE_SPECIAL in
> pte_mkdevmap on arm64
>
[...]
> > diff --git a/arch/arm64/include/asm/pgtable.h
> b/arch/arm64/include/asm/pgtable.h
> > index 5fdcfe237338..e09760ece844 100644
> > --- a/arch/arm64/include/asm/pgtable.h
> > +++ b/arch/arm64/include/asm/pgtable.h
> > @@ -209,7 +209,7 @@ static inline pmd_t pmd_mkcont(pmd_t pmd)
> >
> >  static inline pte_t pte_mkdevmap(pte_t pte)
> >  {
> > -   return set_pte_bit(pte, __pgprot(PTE_DEVMAP));
> > +   return set_pte_bit(pte, __pgprot(PTE_DEVMAP | PTE_SPECIAL));
> >  }
> >
> >  static inline void set_pte(pte_t *ptep, pte_t pte)
> > @@ -396,7 +396,10 @@ static inline int pmd_protnone(pmd_t pmd)
> >  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> >  #define pmd_devmap(pmd)            pte_devmap(pmd_pte(pmd))
> >  #endif
> > -#define pmd_mkdevmap(pmd)
>       pte_pmd(pte_mkdevmap(pmd_pte(pmd)))
> > +static inline pmd_t pmd_mkdevmap(pmd_t pmd)
> > +{
> > +   return pte_pmd(set_pte_bit(pmd_pte(pmd),
> __pgprot(PTE_DEVMAP)));
> > +}
>
> Though I could see other platforms like powerpc and x86 following same
> approach (DEVMAP + SPECIAL) for pte so that it checks positive for
> pte_special() but then just DEVMAP for pmd which could never have a
> pmd_special(). But a more fundamental question is - why should a devmap
> be a special pte as well ?

IIUC, special pte bit make things handling easier compare with those arches which
have no special bit. The memory codes will regard devmap page as a special one
compared with normal page.
Devmap page structure can be stored in ram/pmem/none.

>
> Also in vm_normal_page() why cannot it tests for pte_devmap() before it
> starts looking for CONFIG_ARCH_HAS_PTE_SPECIAL. Is this the only path
> for

AFAICT, yes, but it changes to much besides arm codes. 😊

> which we need to set SPECIAL bit on a devmap pte or there are other paths
> where this semantics is assumed ?

No idea


--
Cheers,
Justin (Jia He)


IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v6 02/14] arm64: Make use of is_compat_task instead of hardcoding this test
From: Alexandre Ghiti @ 2019-08-08  6:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Albert Ou, Kees Cook, Alexandre Ghiti, linux-mm, Catalin Marinas,
	Palmer Dabbelt, Will Deacon, Russell King, Ralf Baechle,
	linux-kernel, linux-fsdevel, Luis Chamberlain, Paul Burton,
	Paul Walmsley, James Hogan, linux-riscv, linux-mips,
	Christoph Hellwig, linux-arm-kernel, Alexander Viro
In-Reply-To: <20190808061756.19712-1-alex@ghiti.fr>

Each architecture has its own way to determine if a task is a compat task,
by using is_compat_task in arch_mmap_rnd, it allows more genericity and
then it prepares its moving to mm/.

Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
---
 arch/arm64/mm/mmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/mm/mmap.c b/arch/arm64/mm/mmap.c
index b050641b5139..bb0140afed66 100644
--- a/arch/arm64/mm/mmap.c
+++ b/arch/arm64/mm/mmap.c
@@ -43,7 +43,7 @@ unsigned long arch_mmap_rnd(void)
 	unsigned long rnd;
 
 #ifdef CONFIG_COMPAT
-	if (test_thread_flag(TIF_32BIT))
+	if (is_compat_task())
 		rnd = get_random_long() & ((1UL << mmap_rnd_compat_bits) - 1);
 	else
 #endif
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v6 01/14] mm, fs: Move randomize_stack_top from fs to mm
From: Alexandre Ghiti @ 2019-08-08  6:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Albert Ou, Kees Cook, Alexandre Ghiti, linux-mm, Catalin Marinas,
	Palmer Dabbelt, Will Deacon, Russell King, Ralf Baechle,
	linux-kernel, linux-fsdevel, Luis Chamberlain, Paul Burton,
	Paul Walmsley, James Hogan, linux-riscv, linux-mips,
	Christoph Hellwig, linux-arm-kernel, Alexander Viro
In-Reply-To: <20190808061756.19712-1-alex@ghiti.fr>

This preparatory commit moves this function so that further introduction
of generic topdown mmap layout is contained only in mm/util.c.

Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
Acked-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
---
 fs/binfmt_elf.c    | 20 --------------------
 include/linux/mm.h |  2 ++
 mm/util.c          | 22 ++++++++++++++++++++++
 3 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index d4e11b2e04f6..cec3b4146440 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -670,26 +670,6 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
  * libraries.  There is no binary dependent code anywhere else.
  */
 
-#ifndef STACK_RND_MASK
-#define STACK_RND_MASK (0x7ff >> (PAGE_SHIFT - 12))	/* 8MB of VA */
-#endif
-
-static unsigned long randomize_stack_top(unsigned long stack_top)
-{
-	unsigned long random_variable = 0;
-
-	if (current->flags & PF_RANDOMIZE) {
-		random_variable = get_random_long();
-		random_variable &= STACK_RND_MASK;
-		random_variable <<= PAGE_SHIFT;
-	}
-#ifdef CONFIG_STACK_GROWSUP
-	return PAGE_ALIGN(stack_top) + random_variable;
-#else
-	return PAGE_ALIGN(stack_top) - random_variable;
-#endif
-}
-
 static int load_elf_binary(struct linux_binprm *bprm)
 {
 	struct file *interpreter = NULL; /* to shut gcc up */
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0334ca97c584..ae0e5d241eb8 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2351,6 +2351,8 @@ extern int install_special_mapping(struct mm_struct *mm,
 				   unsigned long addr, unsigned long len,
 				   unsigned long flags, struct page **pages);
 
+unsigned long randomize_stack_top(unsigned long stack_top);
+
 extern unsigned long get_unmapped_area(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
 
 extern unsigned long mmap_region(struct file *file, unsigned long addr,
diff --git a/mm/util.c b/mm/util.c
index e6351a80f248..15a4fb0f5473 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -16,6 +16,8 @@
 #include <linux/hugetlb.h>
 #include <linux/vmalloc.h>
 #include <linux/userfaultfd_k.h>
+#include <linux/elf.h>
+#include <linux/random.h>
 
 #include <linux/uaccess.h>
 
@@ -293,6 +295,26 @@ int vma_is_stack_for_current(struct vm_area_struct *vma)
 	return (vma->vm_start <= KSTK_ESP(t) && vma->vm_end >= KSTK_ESP(t));
 }
 
+#ifndef STACK_RND_MASK
+#define STACK_RND_MASK (0x7ff >> (PAGE_SHIFT - 12))     /* 8MB of VA */
+#endif
+
+unsigned long randomize_stack_top(unsigned long stack_top)
+{
+	unsigned long random_variable = 0;
+
+	if (current->flags & PF_RANDOMIZE) {
+		random_variable = get_random_long();
+		random_variable &= STACK_RND_MASK;
+		random_variable <<= PAGE_SHIFT;
+	}
+#ifdef CONFIG_STACK_GROWSUP
+	return PAGE_ALIGN(stack_top) + random_variable;
+#else
+	return PAGE_ALIGN(stack_top) - random_variable;
+#endif
+}
+
 #if defined(CONFIG_MMU) && !defined(HAVE_ARCH_PICK_MMAP_LAYOUT)
 void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
 {
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v6 00/14] Provide generic top-down mmap layout functions
From: Alexandre Ghiti @ 2019-08-08  6:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Albert Ou, Kees Cook, Alexandre Ghiti, linux-mm, Catalin Marinas,
	Palmer Dabbelt, Will Deacon, Russell King, Ralf Baechle,
	linux-kernel, linux-fsdevel, Luis Chamberlain, Paul Burton,
	Paul Walmsley, James Hogan, linux-riscv, linux-mips,
	Christoph Hellwig, linux-arm-kernel, Alexander Viro

This series introduces generic functions to make top-down mmap layout
easily accessible to architectures, in particular riscv which was
the initial goal of this series.
The generic implementation was taken from arm64 and used successively
by arm, mips and finally riscv.

Note that in addition the series fixes 2 issues:
- stack randomization was taken into account even if not necessary.
- [1] fixed an issue with mmap base which did not take into account
  randomization but did not report it to arm and mips, so by moving
  arm64 into a generic library, this problem is now fixed for both
  architectures.

This work is an effort to factorize architecture functions to avoid
code duplication and oversights as in [1].

[1]: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1429066.html

Changes in v6:
  - Do not handle sv48 as it will be correctly implemented later: assume
    64BIT <=> sv39.
  - Add acked-by from Paul

Changes in v5:
  - Fix [PATCH 11/14]
  - Rebase on top of v5.3rc2 and commit
    "riscv: kbuild: add virtual memory system selection"
  - [PATCH 14/14] now takes into account the various virtual memory systems

Changes in v4:
  - Make ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT select ARCH_HAS_ELF_RANDOMIZE
    by default as suggested by Kees,
  - ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT depends on MMU and defines the
    functions needed by ARCH_HAS_ELF_RANDOMIZE => architectures that use
    the generic mmap topdown functions cannot have ARCH_HAS_ELF_RANDOMIZE
    selected without MMU, but I think it's ok since randomization without
    MMU does not add much security anyway.
  - There is no common API to determine if a process is 32b, so I came up with
    !IS_ENABLED(CONFIG_64BIT) || is_compat_task() in [PATCH v4 12/14].
  - Mention in the change log that x86 already takes care of not offseting mmap
    base address if the task does not want randomization.
  - Re-introduce a comment that should not have been removed.
  - Add Reviewed/Acked-By from Paul, Christoph and Kees, thank you for that.
  - I tried to minimize the changes from the commits in v3 in order to make
    easier the review of the v4, the commits changed or added are:
    - [PATCH v4 5/14]
    - [PATCH v4 8/14]
    - [PATCH v4 11/14]
    - [PATCH v4 12/14]
    - [PATCH v4 13/14]

Changes in v3:
  - Split into small patches to ease review as suggested by Christoph
    Hellwig and Kees Cook
  - Move help text of new config as a comment, as suggested by Christoph
  - Make new config depend on MMU, as suggested by Christoph

Changes in v2 as suggested by Christoph Hellwig:
  - Preparatory patch that moves randomize_stack_top
  - Fix duplicate config in riscv
  - Align #if defined on next line => this gives rise to a checkpatch
    warning. I found this pattern all around the tree, in the same proportion
    as the previous pattern which was less pretty:
    git grep -C 1 -n -P "^#if defined.+\|\|.*\\\\$" 

Alexandre Ghiti (14):
  mm, fs: Move randomize_stack_top from fs to mm
  arm64: Make use of is_compat_task instead of hardcoding this test
  arm64: Consider stack randomization for mmap base only when necessary
  arm64, mm: Move generic mmap layout functions to mm
  arm64, mm: Make randomization selected by generic topdown mmap layout
  arm: Properly account for stack randomization and stack guard gap
  arm: Use STACK_TOP when computing mmap base address
  arm: Use generic mmap top-down layout and brk randomization
  mips: Properly account for stack randomization and stack guard gap
  mips: Use STACK_TOP when computing mmap base address
  mips: Adjust brk randomization offset to fit generic version
  mips: Replace arch specific way to determine 32bit task with generic
    version
  mips: Use generic mmap top-down layout and brk randomization
  riscv: Make mmap allocation top-down by default

 arch/Kconfig                       |  11 +++
 arch/arm/Kconfig                   |   2 +-
 arch/arm/include/asm/processor.h   |   2 -
 arch/arm/kernel/process.c          |   5 --
 arch/arm/mm/mmap.c                 |  52 --------------
 arch/arm64/Kconfig                 |   2 +-
 arch/arm64/include/asm/processor.h |   2 -
 arch/arm64/kernel/process.c        |   8 ---
 arch/arm64/mm/mmap.c               |  72 -------------------
 arch/mips/Kconfig                  |   2 +-
 arch/mips/include/asm/processor.h  |   5 --
 arch/mips/mm/mmap.c                |  84 ----------------------
 arch/riscv/Kconfig                 |  12 ++++
 fs/binfmt_elf.c                    |  20 ------
 include/linux/mm.h                 |   2 +
 kernel/sysctl.c                    |   6 +-
 mm/util.c                          | 107 ++++++++++++++++++++++++++++-
 17 files changed, 138 insertions(+), 256 deletions(-)

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* RE: [PATCH V4 1/2] perf: imx8_ddr_perf: add AXI ID filter support
From: Joakim Zhang @ 2019-08-08  6:11 UTC (permalink / raw)
  To: Robin Murphy, will@kernel.org, mark.rutland@arm.com, Frank Li
  Cc: kernel@pengutronix.de, linux-arm-kernel@lists.infradead.org,
	dl-linux-imx
In-Reply-To: <DB7PR04MB46180FDE0FD470C035AF2892E6DA0@DB7PR04MB4618.eurprd04.prod.outlook.com>


> -----Original Message-----
> From: Joakim Zhang
> Sent: 2019年8月5日 18:33
> To: Robin Murphy <robin.murphy@arm.com>; will@kernel.org;
> mark.rutland@arm.com; Frank Li <frank.li@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org; kernel@pengutronix.de; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: RE: [PATCH V4 1/2] perf: imx8_ddr_perf: add AXI ID filter support
> 
> 
> > -----Original Message-----
> > From: Joakim Zhang
> > Sent: 2019年8月2日 15:20
> > To: Robin Murphy <robin.murphy@arm.com>; will@kernel.org;
> > mark.rutland@arm.com; Frank Li <frank.li@nxp.com>
> > Cc: linux-arm-kernel@lists.infradead.org; kernel@pengutronix.de;
> > dl-linux-imx <linux-imx@nxp.com>
> > Subject: RE: [PATCH V4 1/2] perf: imx8_ddr_perf: add AXI ID filter
> > support
> >
> >
> > > -----Original Message-----
> > > From: Robin Murphy <robin.murphy@arm.com>
> > > Sent: 2019年8月1日 18:00
> > > To: Joakim Zhang <qiangqing.zhang@nxp.com>; will@kernel.org;
> > > mark.rutland@arm.com; Frank Li <frank.li@nxp.com>
> > > Cc: linux-arm-kernel@lists.infradead.org; kernel@pengutronix.de;
> > > dl-linux-imx <linux-imx@nxp.com>
> > > Subject: Re: [PATCH V4 1/2] perf: imx8_ddr_perf: add AXI ID filter
> > > support
> > >
> > > On 2019-08-01 6:25 am, Joakim Zhang wrote:
> > > [...]
> > > >>> @@ -195,6 +214,18 @@ static int ddr_perf_event_init(struct
> > > >>> perf_event
> > > >> *event)
> > > >>>    	struct hw_perf_event *hwc = &event->hw;
> > > >>>    	struct perf_event *sibling;
> > > >>>
> > > >>> +	if (pmu->devtype_data->quirks & DDR_CAP_AXI_ID_FILTER) {
> > > >>> +		if (event->attr.config == 0x41)
> > > >>> +			pmu->axi_id_read = event->attr.config1;
> > > >>> +
> > > >>> +		if (event->attr.config == 0x42)
> > > >>> +			pmu->axi_id_write = event->attr.config1;
> > > >>> +
> > > >>> +		if (pmu->axi_id_read && pmu->axi_id_write &&
> > > >>> +		    (pmu->axi_id_read != pmu->axi_id_write))
> > > >>> +			return -EINVAL;
> > > >>> +	}
> > > >>
> > > >> This isn't the correct approach that Mark outlined :(
> > > >>
> > > >> In event_init, you should validate that any filtering for the
> > > >> given event is compatible with any other sibling events in the
> > > >> same group, but you should not consider (and should definitely
> > > >> not
> > > >> change) the current state of the PMU at that point. This step is
> > > >> about rejecting event configurations which could
> > > >> *never* be successfully scheduled (since a group represents a set
> > > >> of events which must be scheduled all at the same time).
> > > >>
> > > >> In event_add, you know the given event/group is sufficiently
> > > >> valid to
> > > >> *potentially* be scheduled, given that it has passed the
> > > >> event_init checks, but you then need to check that the filtering
> > > >> is compatible with all other events
> > > >> *currently* counting on the PMU. If this fails, perf core will
> > > >> try to reschedule the current events until the new one is able to run.
> > > >> That's why you need the additional step of validating groups
> > > >> beforehand, because otherwise you could end up with contradictory
> > > >> scheduling constraints at this point and never make progress.
> > > >
> > > > Hi Mark and Robin,
> > > >
> > > > Thanks for all your kindly detailed explanation firstly.
> > > >
> > > > My understanding from your comments, I need to validate the filtering
> (i.e.
> > > config1/axi_id) for *all* events in same group during event_init, right?
> > > > But it's so strange for that axi_id is only for axi-id-read and
> > > > axi-id-write
> > event.
> > > We don't need to specify axi_id for any other events when mixed with
> > > these two events.
> > >
> > > Sorry, I implicitly meant all *relevant* events - obviously there's
> > > nothing to check for events which don't have filtering anyway. All
> > > that matters is the case where we're asked to create a read/write
> > > event in a group which already has at least one other read/write
> > > event as a sibling. I've sketched out a quick (completely untested)
> > > example of one way to do that part below. The logic for event_add
> > > would be very similar, except instead of comparing the sibling
> > > against the event, there you'd compare the event against the current PMU
> state.
> > >
> > > > If I can just check the axi-id-read and axi-id-write event during
> > > > event_add and then pass the axi_id value to the filter register.
> > > > Don't check the case that user specify both of them at the same
> > > > time with different
> > > filtering value. Instead of checking it in the driver, I add the doc
> > > in Documentation/admin-guide/perf/ directory to note that
> > > axi-id-read and axi-id-write event should be specified separately,
> > > or specified at the same time with same axi_id value.
> > >
> > > Sure, we could just rely on the user to get it right, but that means
> > > there's a fair chance that the user can inadvertently get it wrong,
> > > get nonsensical results, and waste a week trying to debug a
> > > perceived problem which doesn't actually exist. It's not difficult
> > > for the driver to perform the correct validation, so it's better for everyone if
> it does.
> > >
> > > It also seems reasonable that a user might want to intentionally
> > > measure events on different IDs over the same run (but not in the
> > > same group), e.g. to compare the relative average bandwidth of two
> > > devices, perhaps to tune QoS parameters. That requires perf core to
> > > know it needs to rotate the events during the run, which will only
> > > happen if
> > event_add does the right thing.
> > >
> > > Robin.
> >
> > Hi Robin,
> >
> > I completely understood what you said now, thank you very much. But I
> > came across another issue when I test this case. You can see below.
> >
> > [...]
> > >          for_each_sibling_event(sibling, event->group_leader) {
> > >                  if (sibling->pmu != event->pmu &&
> > >                                  !is_software_event(sibling))
> > >                          return -EINVAL;
> > > +
> > > +               if (is_filtered && ddr_perf_is_filtered(sibling) &&
> > > +                   ddr_perf_filter_val(sibling) != filter_val)
> > > +                       return -EINVAL;
> > >          }
> > [...]
> >
> > Need to check the axi id value of sibling events in one same group
> > with for_each_sibling_event ():
> > #define for_each_sibling_event(sibling, event)                  \
> >          if ((event)->group_leader == (event))                   \
> >                  list_for_each_entry((sibling),
> > &(event)->sibling_list,
> > sibling_list)
> >
> > But I found that all check in this For loop will never be checked,
> > that means the code never runs into this For loop.
> >                if (sibling->pmu != event->pmu &&
> >                               !is_software_event(sibling))
> >                        return -EINVAL;
> >
> >                if (is_filtered && ddr_perf_is_filtered(sibling) &&
> >                   ddr_perf_filter_val(sibling) != filter_val)
> >                        return -EINVAL;
> >
> > Finally I found that it can't iterate over the list with
> > list_for_each_entry((sibling), &(event)->sibling_list, sibling_list).
> > So driver can't reject event group with axi id illegal value during
> > event_init() now. Do you know why it can't iterate to sibling events?
> 
> Hi Robin,
> 
> I have an another question, with below configuration, if this means cycles, read
> and write events is an event group?
> 	perf stat -e imx8_ddr0/cycles/,imx8_ddr0/read/,imx8_ddr0/write/ sleep 1
> 
> If yes, I found that perf event core attach event group(perf_group_attach() in
> kernel/events/core.c) after perf event initialization (perf_try_init_event() call
> pmu->event_init() callback in kernel/events/core.c).
> Is it reasonable as we always check the sibling event form event_init callback?
> And each perf event pass to perf_group_attach() always point to it's group
> leader, so for_each_sibling_event() in event_init() can't iterate to it's sibling
> events, as it has no sibling events from perf_froup_attach().
> 
> Do I misunderstand angthing?

Hi Robin,

I have known that below is correct method to configure an event group:
	perf stat -e '{imx8_ddr0/cycles/,imx8_ddr0/read/,imx8_ddr0/write/}' sleep 1

I will send out PATCH V5, please help review, thank you.
 
Best Regards,
Joakim Zhang
> Best Regards,
> Joakim Zhang
> > Best Regards,
> > Joakim Zhang
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2] arm64: mm: print hexadecimal EC value in mem_abort_decode()
From: Miles Chen @ 2019-08-08  6:01 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: Mark Rutland, wsd_upstream, Catalin Marinas, Will Deacon,
	linux-kernel, linux-mediatek, James Morse, linux-arm-kernel
In-Reply-To: <98bdbcfb-24ed-fcd8-4b2c-f2c78b245dda@arm.com>

On Thu, 2019-08-08 at 11:19 +0530, Anshuman Khandual wrote:
> 
> On 08/07/2019 06:03 AM, Miles Chen wrote:
> > This change prints the hexadecimal EC value in mem_abort_decode(),
> > which makes it easier to lookup the corresponding EC in
> > the ARM Architecture Reference Manual.
> > 
> > The commit 1f9b8936f36f ("arm64: Decode information from ESR upon mem
> > faults") prints useful information when memory abort occurs. It would
> > be easier to lookup "0x25" instead of "DABT" in the document. Then we
> > can check the corresponding ISS.
> > 
> > For example:
> > Current	info	  	Document
> > 		  	EC	Exception class
> > "CP15 MCR/MRC"		0x3	"MCR or MRC access to CP15a..."
> > "ASIMD"			0x7	"Access to SIMD or floating-point..."
> > "DABT (current EL)" 	0x25	"Data Abort taken without..."
> > ...
> > 
> > Before:
> > Unable to handle kernel paging request at virtual address 000000000000c000
> > Mem abort info:
> >   ESR = 0x96000046
> >   Exception class = DABT (current EL), IL = 32 bits
> >   SET = 0, FnV = 0
> >   EA = 0, S1PTW = 0
> > Data abort info:
> >   ISV = 0, ISS = 0x00000046
> >   CM = 0, WnR = 1
> > 
> > After:
> > Unable to handle kernel paging request at virtual address 000000000000c000
> > Mem abort info:
> >   ESR = 0x96000046
> >   EC = 0x25: DABT (current EL), IL = 32 bits
> >   SET = 0, FnV = 0
> >   EA = 0, S1PTW = 0
> > Data abort info:
> >   ISV = 0, ISS = 0x00000046
> >   CM = 0, WnR = 1
> > 
> > Change since v1:
> > print "EC" instead of "Exception class"
> > print EC in fixwidth
> > 
> > Cc: Mark Rutland <Mark.rutland@arm.com>
> > Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> > Cc: James Morse <james.morse@arm.com>
> > Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> 
> This version implements the suggestion, hence it should have
> also contained acked-by tag from Mark from earlier version.
> 

No problem. Sorry for not including the tag.
I was not sure if I should add the acked-by tag from Mark in patch v2.

> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

If I send patch v3, I should include acked-by tag from Mark and
Reviewed-by tag from you, right?


Miles


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2] arm64: mm: print hexadecimal EC value in mem_abort_decode()
From: Anshuman Khandual @ 2019-08-08  5:49 UTC (permalink / raw)
  To: Miles Chen, Catalin Marinas, Will Deacon
  Cc: Mark Rutland, wsd_upstream, linux-kernel, James Morse,
	linux-mediatek, linux-arm-kernel
In-Reply-To: <20190807003336.28040-1-miles.chen@mediatek.com>



On 08/07/2019 06:03 AM, Miles Chen wrote:
> This change prints the hexadecimal EC value in mem_abort_decode(),
> which makes it easier to lookup the corresponding EC in
> the ARM Architecture Reference Manual.
> 
> The commit 1f9b8936f36f ("arm64: Decode information from ESR upon mem
> faults") prints useful information when memory abort occurs. It would
> be easier to lookup "0x25" instead of "DABT" in the document. Then we
> can check the corresponding ISS.
> 
> For example:
> Current	info	  	Document
> 		  	EC	Exception class
> "CP15 MCR/MRC"		0x3	"MCR or MRC access to CP15a..."
> "ASIMD"			0x7	"Access to SIMD or floating-point..."
> "DABT (current EL)" 	0x25	"Data Abort taken without..."
> ...
> 
> Before:
> Unable to handle kernel paging request at virtual address 000000000000c000
> Mem abort info:
>   ESR = 0x96000046
>   Exception class = DABT (current EL), IL = 32 bits
>   SET = 0, FnV = 0
>   EA = 0, S1PTW = 0
> Data abort info:
>   ISV = 0, ISS = 0x00000046
>   CM = 0, WnR = 1
> 
> After:
> Unable to handle kernel paging request at virtual address 000000000000c000
> Mem abort info:
>   ESR = 0x96000046
>   EC = 0x25: DABT (current EL), IL = 32 bits
>   SET = 0, FnV = 0
>   EA = 0, S1PTW = 0
> Data abort info:
>   ISV = 0, ISS = 0x00000046
>   CM = 0, WnR = 1
> 
> Change since v1:
> print "EC" instead of "Exception class"
> print EC in fixwidth
> 
> Cc: Mark Rutland <Mark.rutland@arm.com>
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Signed-off-by: Miles Chen <miles.chen@mediatek.com>

This version implements the suggestion, hence it should have
also contained acked-by tag from Mark from earlier version.

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [linux-sunxi] [PATCH 0/3] Add basic support for RTC on Allwinner H6 SoC
From: Chen-Yu Tsai @ 2019-08-08  5:48 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Mark Rutland, Alessandro Zummo, devicetree, Maxime Ripard,
	linux-kernel, linux-sunxi, Rob Herring, linux-arm-kernel,
	linux-rtc
In-Reply-To: <20190807105502.GK3600@piout.net>

On Wed, Aug 7, 2019 at 6:55 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> Hi,
>
> On 06/08/2019 20:30:45+0200, Ondřej Jirman wrote:
> > Maybe whether XO or DCXO is used also matters if you want to do some fine
> > tunning of DCXO (control register has pletny of options), but that's probably
> > better done in u-boot. And there's still no need to read HOSC source from DT.
> > The driver can just check compatible, and if it is H6 and OSC_CLK_SRC_SEL is 1,
> > it can do it's DCXO tunning, or whatever. But neither OS nor bootloader will
> > be using this info to gate/disable the osciallator.
> >
>
> It is actually useful to be able to tweak the crystal tuning at
> runtime to be able to reduce clock drift and compare with a reliable
> source (e.g. NTP).
> I'm curious, what kind of options does this RTC have?

It has options to set the current, trim cap value, band gap voltage, and also
change the mode to just accept an external clock signal, instead of driving
a crystal. The settings for the former parameters are not explained though.

See page 364 of
http://linux-sunxi.org/File:Allwinner_H6_V200_User_Manual_V1.1.pdf

ChenYu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 38/41] powerpc: convert put_page() to put_user_page*()
From: Michael Ellerman @ 2019-08-08  5:42 UTC (permalink / raw)
  To: john.hubbard, Andrew Morton
  Cc: linux-fbdev, Jan Kara, kvm, Benjamin Herrenschmidt, Dave Hansen,
	Dave Chinner, dri-devel, linux-mm, sparclinux, Ira Weiny,
	ceph-devel, devel, rds-devel, linux-rdma, x86, amd-gfx,
	Christoph Hellwig, Christoph Hellwig, Jason Gunthorpe, xen-devel,
	devel, linux-media, John Hubbard, intel-gfx, linux-block,
	Jérôme Glisse, linux-rpi-kernel, Dan Williams,
	linux-arm-kernel, linux-nfs, netdev, LKML, linux-xfs,
	linux-crypto, linux-fsdevel, linuxppc-dev
In-Reply-To: <20190807013340.9706-39-jhubbard@nvidia.com>

Hi John,

john.hubbard@gmail.com writes:
> diff --git a/arch/powerpc/mm/book3s64/iommu_api.c b/arch/powerpc/mm/book3s64/iommu_api.c
> index b056cae3388b..e126193ba295 100644
> --- a/arch/powerpc/mm/book3s64/iommu_api.c
> +++ b/arch/powerpc/mm/book3s64/iommu_api.c
> @@ -203,6 +202,7 @@ static void mm_iommu_unpin(struct mm_iommu_table_group_mem_t *mem)
>  {
>  	long i;
>  	struct page *page = NULL;
> +	bool dirty = false;

I don't think you need that initialisation do you?

>  	if (!mem->hpas)
>  		return;
> @@ -215,10 +215,9 @@ static void mm_iommu_unpin(struct mm_iommu_table_group_mem_t *mem)
>  		if (!page)
>  			continue;
>  
> -		if (mem->hpas[i] & MM_IOMMU_TABLE_GROUP_PAGE_DIRTY)
> -			SetPageDirty(page);
> +		dirty = mem->hpas[i] & MM_IOMMU_TABLE_GROUP_PAGE_DIRTY;
> -		put_page(page);
> +		put_user_pages_dirty_lock(&page, 1, dirty);
>  		mem->hpas[i] = 0;
>  	}
>  }

cheers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 5/8] soc: ti: omap-prm: add omap4 PRM data
From: Keerthy @ 2019-08-08  5:30 UTC (permalink / raw)
  To: Tero Kristo, ssantosh, linux-arm-kernel, linux-omap, robh+dt
  Cc: tony, devicetree
In-Reply-To: <1565164139-21886-6-git-send-email-t-kristo@ti.com>



On 07/08/19 1:18 PM, Tero Kristo wrote:
> Add PRM data for omap4 family of SoCs.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> ---
>   drivers/soc/ti/omap_prm.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/soc/ti/omap_prm.c b/drivers/soc/ti/omap_prm.c
> index 870515e3..9b8d5945 100644
> --- a/drivers/soc/ti/omap_prm.c
> +++ b/drivers/soc/ti/omap_prm.c
> @@ -54,7 +54,27 @@ struct omap_reset_data {
>   
>   #define OMAP_PRM_NO_RSTST	BIT(0)
>   
> +struct omap_prm_data omap4_prm_data[] = {
> +	{ .name = "mpu", .base = 0x4a306300, .pwstst = 0x4 },
> +	{ .name = "tesla", .base = 0x4a306400, .pwstst = 0x4, .rstctl = 0x10, .rstst = 0x14 },
> +	{ .name = "abe", .base = 0x4a306500, .pwstst = 0x4 },
> +	{ .name = "always_on_core", .base = 0x4a306600, .pwstst = 0x4 },
> +	{ .name = "core", .base = 0x4a306700, .pwstst = 0x4, .rstctl = 0x210, .rstst = 0x214 },
> +	{ .name = "ivahd", .base = 0x4a306f00, .pwstst = 0x4, .rstctl = 0x10, .rstst = 0x14 },
> +	{ .name = "cam", .base = 0x4a307000, .pwstst = 0x4 },
> +	{ .name = "dss", .base = 0x4a307100, .pwstst = 0x4 },
> +	{ .name = "gfx", .base = 0x4a307200, .pwstst = 0x4 },
> +	{ .name = "l3init", .base = 0x4a307300, .pwstst = 0x4 },
> +	{ .name = "l4per", .base = 0x4a307400, .pwstst = 0x4 },
> +	{ .name = "cefuse", .base = 0x4a307600, .pwstst = 0x4 },
> +	{ .name = "wkup", .base = 0x4a307700, .pwstst = 0x4 },
> +	{ .name = "emu", .base = 0x4a307900, .pwstst = 0x4 },
> +	{ .name = "device", .base = 0x4a307b00, .rstctl = 0x0, .rstst = 0x4 },
> +	{ },
> +};

So at some point arch/arm/mach-omap2/powerdomains44xx_data.c
duplicated data will be removed?

> +
>   static const struct of_device_id omap_prm_id_table[] = {
> +	{ .compatible = "ti,omap4-prm-inst", .data = omap4_prm_data },
>   	{ },
>   };
>   
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 2/8] soc: ti: add initial PRM driver with reset control support
From: Keerthy @ 2019-08-08  5:26 UTC (permalink / raw)
  To: Tero Kristo, ssantosh, linux-arm-kernel, linux-omap, robh+dt
  Cc: tony, devicetree
In-Reply-To: <1565164139-21886-3-git-send-email-t-kristo@ti.com>



On 07/08/19 1:18 PM, Tero Kristo wrote:
> Add initial PRM (Power and Reset Management) driver for TI OMAP class
> SoCs. Initially this driver only supports reset control, but can be
> extended to support rest of the functionality, like powerdomain
> control, PRCM irq support etc.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> ---
>   arch/arm/mach-omap2/Kconfig |   1 +
>   drivers/soc/ti/Makefile     |   1 +
>   drivers/soc/ti/omap_prm.c   | 216 ++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 218 insertions(+)
>   create mode 100644 drivers/soc/ti/omap_prm.c
> 
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index fdb6743..42ad063 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -109,6 +109,7 @@ config ARCH_OMAP2PLUS
>   	select TI_SYSC
>   	select OMAP_IRQCHIP
>   	select CLKSRC_TI_32K
> +	select RESET_CONTROLLER
>   	help
>   	  Systems based on OMAP2, OMAP3, OMAP4 or OMAP5
>   
> diff --git a/drivers/soc/ti/Makefile b/drivers/soc/ti/Makefile
> index b3868d3..788b5cd 100644
> --- a/drivers/soc/ti/Makefile
> +++ b/drivers/soc/ti/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_KEYSTONE_NAVIGATOR_QMSS)	+= knav_qmss.o
>   knav_qmss-y := knav_qmss_queue.o knav_qmss_acc.o
>   obj-$(CONFIG_KEYSTONE_NAVIGATOR_DMA)	+= knav_dma.o
>   obj-$(CONFIG_AMX3_PM)			+= pm33xx.o
> +obj-$(CONFIG_ARCH_OMAP2PLUS)		+= omap_prm.o
>   obj-$(CONFIG_WKUP_M3_IPC)		+= wkup_m3_ipc.o
>   obj-$(CONFIG_TI_SCI_PM_DOMAINS)		+= ti_sci_pm_domains.o
>   obj-$(CONFIG_TI_SCI_INTA_MSI_DOMAIN)	+= ti_sci_inta_msi.o
> diff --git a/drivers/soc/ti/omap_prm.c b/drivers/soc/ti/omap_prm.c
> new file mode 100644
> index 0000000..7c89eb8
> --- /dev/null
> +++ b/drivers/soc/ti/omap_prm.c
> @@ -0,0 +1,216 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * OMAP2+ PRM driver
> + *
> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
> + *	Tero Kristo <t-kristo@ti.com>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/delay.h>
> +
> +struct omap_rst_map {
> +	s8 rst;
> +	s8 st;
> +};
> +
> +struct omap_prm_data {
> +	u32 base;
> +	const char *name;
> +	u16 pwstctrl;
> +	u16 pwstst;
> +	u16 rstctl;
> +	u16 rstst;
> +	struct omap_rst_map *rstmap;
> +	u8 flags;
> +};
> +
> +struct omap_prm {
> +	const struct omap_prm_data *data;
> +	void __iomem *base;
> +};
> +
> +struct omap_reset_data {
> +	struct reset_controller_dev rcdev;
> +	struct omap_prm *prm;
> +};
> +
> +#define to_omap_reset_data(p) container_of((p), struct omap_reset_data, rcdev)
> +
> +#define OMAP_MAX_RESETS		8
> +#define OMAP_RESET_MAX_WAIT	10000
> +
> +#define OMAP_PRM_NO_RSTST	BIT(0)
> +
> +static const struct of_device_id omap_prm_id_table[] = {
> +	{ },
> +};

This table is blank and we are doing of_match_device against it.

> +
> +static int omap_reset_status(struct reset_controller_dev *rcdev,
> +			     unsigned long id)
> +{
> +	struct omap_reset_data *reset = to_omap_reset_data(rcdev);
> +	u32 v;
> +
> +	v = readl_relaxed(reset->prm->base + reset->prm->data->rstst);
> +	v &= 1 << id;
> +	v >>= id;
> +
> +	return v;
> +}
> +
> +static int omap_reset_assert(struct reset_controller_dev *rcdev,
> +			     unsigned long id)
> +{
> +	struct omap_reset_data *reset = to_omap_reset_data(rcdev);
> +	u32 v;
> +
> +	/* assert the reset control line */
> +	v = readl_relaxed(reset->prm->base + reset->prm->data->rstctl);
> +	v |= 1 << id;
> +	writel_relaxed(v, reset->prm->base + reset->prm->data->rstctl);
> +
> +	return 0;
> +}
> +
> +static int omap_reset_get_st_bit(struct omap_reset_data *reset,
> +				 unsigned long id)
> +{
> +	struct omap_rst_map *map = reset->prm->data->rstmap;
> +
> +	while (map && map->rst >= 0) {
> +		if (map->rst == id)
> +			return map->st;
> +
> +		map++;
> +	}
> +
> +	return id;
> +}
> +
> +/*
> + * Note that status will not change until clocks are on, and clocks cannot be
> + * enabled until reset is deasserted. Consumer drivers must check status
> + * separately after enabling clocks.
> + */
> +static int omap_reset_deassert(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	struct omap_reset_data *reset = to_omap_reset_data(rcdev);
> +	u32 v;
> +	int st_bit = id;
> +	bool has_rstst;
> +
> +	/* check the current status to avoid de-asserting the line twice */
> +	v = readl_relaxed(reset->prm->base + reset->prm->data->rstctl);
> +	if (!(v & BIT(id)))
> +		return -EEXIST;
> +
> +	has_rstst = !(reset->prm->data->flags & OMAP_PRM_NO_RSTST);
> +
> +	if (has_rstst) {
> +		st_bit = omap_reset_get_st_bit(reset, id);
> +
> +		/* Clear the reset status by writing 1 to the status bit */
> +		v = readl_relaxed(reset->prm->base + reset->prm->data->rstst);
> +		v |= 1 << st_bit;
> +		writel_relaxed(v, reset->prm->base + reset->prm->data->rstst);
> +	}
> +
> +	/* de-assert the reset control line */
> +	v = readl_relaxed(reset->prm->base + reset->prm->data->rstctl);
> +	v &= ~(1 << id);
> +	writel_relaxed(v, reset->prm->base + reset->prm->data->rstctl);
> +
> +	return 0;
> +}
> +
> +static const struct reset_control_ops omap_reset_ops = {
> +	.assert		= omap_reset_assert,
> +	.deassert	= omap_reset_deassert,
> +	.status		= omap_reset_status,
> +};
> +
> +static int omap_prm_reset_probe(struct platform_device *pdev,
> +				struct omap_prm *prm)
> +{
> +	struct omap_reset_data *reset;
> +
> +	/*
> +	 * Check if we have resets. If either rstctl or rstst is
> +	 * non-zero, we have reset registers in place. Additionally
> +	 * the flag OMAP_PRM_NO_RSTST implies that we have resets.
> +	 */
> +	if (!prm->data->rstctl && !prm->data->rstst &&
> +	    !(prm->data->flags & OMAP_PRM_NO_RSTST))
> +		return 0;
> +
> +	reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
> +	if (!reset)
> +		return -ENOMEM;
> +
> +	reset->rcdev.owner = THIS_MODULE;
> +	reset->rcdev.ops = &omap_reset_ops;
> +	reset->rcdev.of_node = pdev->dev.of_node;
> +	reset->rcdev.nr_resets = OMAP_MAX_RESETS;
> +
> +	reset->prm = prm;
> +
> +	return devm_reset_controller_register(&pdev->dev, &reset->rcdev);
> +}
> +
> +static int omap_prm_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	const struct omap_prm_data *data;
> +	struct omap_prm *prm;
> +	const struct of_device_id *match;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENODEV;
> +
> +	match = of_match_device(omap_prm_id_table, &pdev->dev);
> +	if (!match)
> +		return -ENOTSUPP;
> +
> +	prm = devm_kzalloc(&pdev->dev, sizeof(*prm), GFP_KERNEL);
> +	if (!prm)
> +		return -ENOMEM;
> +
> +	data = match->data;
> +
> +	while (data->base != res->start) {
> +		if (!data->base)
> +			return -EINVAL;
> +		data++;
> +	}
> +
> +	prm->data = data;
> +
> +	prm->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (!prm->base)
> +		return -ENOMEM;
> +
> +	return omap_prm_reset_probe(pdev, prm);
> +}
> +
> +static struct platform_driver omap_prm_driver = {
> +	.probe = omap_prm_probe,
> +	.driver = {
> +		.name		= KBUILD_MODNAME,
> +		.of_match_table	= omap_prm_id_table,
> +	},
> +};
> +builtin_platform_driver(omap_prm_driver);
> +
> +MODULE_ALIAS("platform:prm");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("omap2+ prm driver");

It is a builtin_platform_driver so do we need the MODULE*?

> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64: mm: add missing PTE_SPECIAL in pte_mkdevmap on arm64
From: Anshuman Khandual @ 2019-08-08  5:18 UTC (permalink / raw)
  To: Jia He, Catalin Marinas, Will Deacon, Mark Rutland, James Morse
  Cc: Christoffer Dall, linux-kernel, Jun Yao, Qian Cai, Punit Agrawal,
	Thomas Gleixner, Robin Murphy, Alex Van Brunt, linux-arm-kernel
In-Reply-To: <20190807045851.10772-1-justin.he@arm.com>



On 08/07/2019 10:28 AM, Jia He wrote:
> Without this patch, the MAP_SYNC test case will cause a print_bad_pte
> warning on arm64 as follows:
> [   25.542693] BUG: Bad page map in process mapdax333
> pte:2e8000448800f53 pmd:41ff5f003
> [   25.546360] page:ffff7e0010220000 refcount:1 mapcount:-1
> mapping:ffff8003e29c7440 index:0x0
> [   25.550281] ext4_dax_aops
> [   25.550282] name:"__aaabbbcccddd__"
> [   25.551553] flags: 0x3ffff0000001002(referenced|reserved)
> [   25.555802] raw: 03ffff0000001002 ffff8003dfffa908 0000000000000000
> ffff8003e29c7440
> [   25.559446] raw: 0000000000000000 0000000000000000 00000001fffffffe
> 0000000000000000
> [   25.563075] page dumped because: bad pte
> [   25.564938] addr:0000ffffbe05b000 vm_flags:208000fb
> anon_vma:0000000000000000 mapping:ffff8003e29c7440 index:0
> [   25.574272] file:__aaabbbcccddd__ fault:ext4_dax_fault
> mmmmap:ext4_file_mmap readpage:0x0
> [   25.578799] CPU: 1 PID: 1180 Comm: mapdax333 Not tainted 5.2.0+ #21
> [   25.581702] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0
> 02/06/2015
> [   25.585624] Call trace:
> [   25.587008]  dump_backtrace+0x0/0x178
> [   25.588799]  show_stack+0x24/0x30
> [   25.590328]  dump_stack+0xa8/0xcc
> [   25.591901]  print_bad_pte+0x18c/0x218
> [   25.593628]  unmap_page_range+0x778/0xc00
> [   25.595506]  unmap_single_vma+0x94/0xe8
> [   25.597304]  unmap_vmas+0x90/0x108
> [   25.598901]  unmap_region+0xc0/0x128
> [   25.600566]  __do_munmap+0x284/0x3f0
> [   25.602245]  __vm_munmap+0x78/0xe0
> [   25.603820]  __arm64_sys_munmap+0x34/0x48
> [   25.605709]  el0_svc_common.constprop.0+0x78/0x168
> [   25.607956]  el0_svc_handler+0x34/0x90
> [   25.609698]  el0_svc+0x8/0xc
> [   25.611103] Disabling lock debugging due to kernel taint
> [   25.613573] BUG: Bad page state in process mapdax333  pfn:448800
> [   25.616359] page:ffff7e0010220000 refcount:0 mapcount:-1
> mapping:ffff8003e29c7440 index:0x1
> [   25.620236] ext4_dax_aops
> [   25.620237] name:"__aaabbbcccddd__"
> [   25.621495] flags: 0x3ffff0000000000()
> [   25.624912] raw: 03ffff0000000000 dead000000000100 dead000000000200
> ffff8003e29c7440
> [   25.628502] raw: 0000000000000001 0000000000000000 00000000fffffffe
> 0000000000000000
> [   25.632097] page dumped because: non-NULL mapping
> [...]
> [   25.656567] CPU: 1 PID: 1180 Comm: mapdax333 Tainted: G    B
> 5.2.0+ #21
> [   25.660131] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0
> 02/06/2015
> [   25.663324] Call trace:
> [   25.664466]  dump_backtrace+0x0/0x178
> [   25.666163]  show_stack+0x24/0x30
> [   25.667721]  dump_stack+0xa8/0xcc
> [   25.669270]  bad_page+0xf0/0x150
> [   25.670772]  free_pages_check_bad+0x84/0xa0
> [   25.672724]  free_pcppages_bulk+0x45c/0x708
> [   25.674675]  free_unref_page_commit+0xcc/0x100
> [   25.676751]  free_unref_page_list+0x13c/0x200
> [   25.678801]  release_pages+0x350/0x420
> [   25.680539]  free_pages_and_swap_cache+0xf8/0x128
> [   25.682738]  tlb_flush_mmu+0x164/0x2b0
> [   25.684485]  unmap_page_range+0x648/0xc00
> [   25.686349]  unmap_single_vma+0x94/0xe8
> [   25.688131]  unmap_vmas+0x90/0x108
> [   25.689739]  unmap_region+0xc0/0x128
> [   25.691392]  __do_munmap+0x284/0x3f0
> [   25.693079]  __vm_munmap+0x78/0xe0
> [   25.694658]  __arm64_sys_munmap+0x34/0x48
> [   25.696530]  el0_svc_common.constprop.0+0x78/0x168
> [   25.698772]  el0_svc_handler+0x34/0x90
> [   25.700512]  el0_svc+0x8/0xc
> 
> The root cause is in _vm_normal_page, without the PTE_SPECIAL bit,
> the return value will be incorrectly set to pfn_to_page(pfn) instead
> of NULL. Besides, this patch also rewrite the pmd_mkdevmap to avoid
> setting PTE_SPECIAL for pmd
> 
> The MAP_SYNC test case is as follows(Provided by Yibo Cai)
> $#include <stdio.h>
> $#include <string.h>
> $#include <unistd.h>
> $#include <sys/file.h>
> $#include <sys/mman.h>
> 
> $#ifndef MAP_SYNC
> $#define MAP_SYNC 0x80000
> $#endif
> 
> /* mount -o dax /dev/pmem0 /mnt */
> $#define F "/mnt/__aaabbbcccddd__"
> 
> int main(void)
> {
>     int fd;
>     char buf[4096];
>     void *addr;
> 
>     if ((fd = open(F, O_CREAT|O_TRUNC|O_RDWR, 0644)) < 0) {
>         perror("open1");
>         return 1;
>     }
> 
>     if (write(fd, buf, 4096) != 4096) {
>         perror("lseek");
>         return 1;
>     }
> 
>     addr = mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_SYNC,
> fd, 0);
>     if (addr == MAP_FAILED) {
>         perror("mmap");
>         printf("did you mount with '-o dax'?\n");
>         return 1;
>     }
> 
>     memset(addr, 0x55, 4096);
> 
>     if (munmap(addr, 4096) == -1) {
>         perror("munmap");
>         return 1;
>     }
> 
>     close(fd);
> 
>     return 0;
> }
> 
> Fixes: 73b20c84d42d ("arm64: mm: implement pte_devmap support")
> Reported-by: Yibo Cai <Yibo.Cai@arm.com>
> Signed-off-by: Jia He <justin.he@arm.com>
> Acked-by: Robin Murphy <Robin.Murphy@arm.com>
> ---
>  arch/arm64/include/asm/pgtable.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 5fdcfe237338..e09760ece844 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -209,7 +209,7 @@ static inline pmd_t pmd_mkcont(pmd_t pmd)
>  
>  static inline pte_t pte_mkdevmap(pte_t pte)
>  {
> -	return set_pte_bit(pte, __pgprot(PTE_DEVMAP));
> +	return set_pte_bit(pte, __pgprot(PTE_DEVMAP | PTE_SPECIAL));
>  }
>  
>  static inline void set_pte(pte_t *ptep, pte_t pte)
> @@ -396,7 +396,10 @@ static inline int pmd_protnone(pmd_t pmd)
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  #define pmd_devmap(pmd)		pte_devmap(pmd_pte(pmd))
>  #endif
> -#define pmd_mkdevmap(pmd)	pte_pmd(pte_mkdevmap(pmd_pte(pmd)))
> +static inline pmd_t pmd_mkdevmap(pmd_t pmd)
> +{
> +	return pte_pmd(set_pte_bit(pmd_pte(pmd), __pgprot(PTE_DEVMAP)));
> +}

Though I could see other platforms like powerpc and x86 following same
approach (DEVMAP + SPECIAL) for pte so that it checks positive for
pte_special() but then just DEVMAP for pmd which could never have a
pmd_special(). But a more fundamental question is - why should a devmap
be a special pte as well ?

Also in vm_normal_page() why cannot it tests for pte_devmap() before it
starts looking for CONFIG_ARCH_HAS_PTE_SPECIAL. Is this the only path for
which we need to set SPECIAL bit on a devmap pte or there are other paths
where this semantics is assumed ?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 4/5] clk: Add driver for Bitmain BM1880 SoC clock controller
From: Stephen Boyd @ 2019-08-08  5:15 UTC (permalink / raw)
  To: Manivannan Sadhasivam, mturquette, robh+dt
  Cc: devicetree, Manivannan Sadhasivam, darren.tsao, linux-kernel,
	linux-arm-kernel, fisher.cheng, alec.lin, linux-clk, haitao.suo
In-Reply-To: <20190705151440.20844-5-manivannan.sadhasivam@linaro.org>

Quoting Manivannan Sadhasivam (2019-07-05 08:14:39)
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index fc1e0cf44995..ffc61ed85ade 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -304,6 +304,12 @@ config COMMON_CLK_FIXED_MMIO
>         help
>           Support for Memory Mapped IO Fixed clocks
>  
> +config COMMON_CLK_BM1880
> +       bool "Clock driver for Bitmain BM1880 SoC"
> +       depends on ARCH_BITMAIN || COMPILE_TEST
> +       help
> +         This driver supports the clocks on Bitmain BM1880 SoC.

Can you add this config somewhere else besides the end? Preferably
close to alphabetically in this file.

> +
>  source "drivers/clk/actions/Kconfig"
>  source "drivers/clk/analogbits/Kconfig"
>  source "drivers/clk/bcm/Kconfig"
> diff --git a/drivers/clk/clk-bm1880.c b/drivers/clk/clk-bm1880.c
> new file mode 100644
> index 000000000000..26cdb75bb936
> --- /dev/null
> +++ b/drivers/clk/clk-bm1880.c
> @@ -0,0 +1,947 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Bitmain BM1880 SoC clock driver
> + *
> + * Copyright (c) 2019 Linaro Ltd.
> + * Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>

Should probably add kernel.h for at least container_of()

> +
> +#include <dt-bindings/clock/bm1880-clock.h>
> +
> +#define BM1880_CLK_MPLL_CTL    0x00
> +#define BM1880_CLK_SPLL_CTL    0x04
> +#define BM1880_CLK_FPLL_CTL    0x08
> +#define BM1880_CLK_DDRPLL_CTL  0x0c
> +
> +#define BM1880_CLK_ENABLE0     0x00
> +#define BM1880_CLK_ENABLE1     0x04
> +#define BM1880_CLK_SELECT      0x20
> +#define BM1880_CLK_DIV0                0x40
> +#define BM1880_CLK_DIV1                0x44
> +#define BM1880_CLK_DIV2                0x48
> +#define BM1880_CLK_DIV3                0x4c
> +#define BM1880_CLK_DIV4                0x50
> +#define BM1880_CLK_DIV5                0x54
> +#define BM1880_CLK_DIV6                0x58
> +#define BM1880_CLK_DIV7                0x5c
> +#define BM1880_CLK_DIV8                0x60
> +#define BM1880_CLK_DIV9                0x64
> +#define BM1880_CLK_DIV10       0x68
> +#define BM1880_CLK_DIV11       0x6c
> +#define BM1880_CLK_DIV12       0x70
> +#define BM1880_CLK_DIV13       0x74
> +#define BM1880_CLK_DIV14       0x78
> +#define BM1880_CLK_DIV15       0x7c
> +#define BM1880_CLK_DIV16       0x80
> +#define BM1880_CLK_DIV17       0x84
> +#define BM1880_CLK_DIV18       0x88
> +#define BM1880_CLK_DIV19       0x8c
> +#define BM1880_CLK_DIV20       0x90
> +#define BM1880_CLK_DIV21       0x94
> +#define BM1880_CLK_DIV22       0x98
> +#define BM1880_CLK_DIV23       0x9c
> +#define BM1880_CLK_DIV24       0xa0
> +#define BM1880_CLK_DIV25       0xa4
> +#define BM1880_CLK_DIV26       0xa8
> +#define BM1880_CLK_DIV27       0xac
> +#define BM1880_CLK_DIV28       0xb0
> +
> +#define to_bm1880_pll_clk(_hw) container_of(_hw, struct bm1880_pll_hw_clock, hw)
> +#define to_bm1880_div_clk(_hw) container_of(_hw, struct bm1880_div_hw_clock, hw)
> +
> +static DEFINE_SPINLOCK(bm1880_clk_lock);
> +
> +struct bm1880_clock_data {
> +       void __iomem *pll_base;
> +       void __iomem *sys_base;
> +       struct clk_onecell_data clk_data;
> +};
> +
> +struct bm1880_gate_clock {
> +       unsigned int    id;
> +       const char      *name;
> +       const char      *parent;
> +       u32             gate_reg;
> +       s8              gate_shift;
> +       unsigned long   flags;
> +};
> +
> +struct bm1880_mux_clock {
> +       unsigned int    id;
> +       const char      *name;
> +       const char      * const * parents;
> +       s8              num_parents;
> +       u32             reg;
> +       s8              shift;
> +       unsigned long   flags;
> +};
> +
> +struct bm1880_div_clock {
> +       unsigned int    id;
> +       const char      *name;
> +       const char      *parent;
> +       u32             reg;
> +       u8              shift;
> +       u8              width;
> +       u32             initval;
> +       struct clk_div_table *table;
> +       unsigned long   flags;
> +};
> +
> +struct bm1880_div_hw_clock {
> +       struct bm1880_div_clock div;
> +       void __iomem *base;
> +       spinlock_t *lock;
> +       struct clk_hw hw;
> +};
> +
> +struct bm1880_composite_clock {
> +       unsigned int    id;
> +       const char      *name;
> +       const char      *parent;
> +       const char      * const * parents;
> +       unsigned int    num_parents;
> +       unsigned long   flags;
> +
> +       u32             gate_reg;
> +       u32             mux_reg;
> +       u32             div_reg;
> +
> +       s8              gate_shift;
> +       s8              mux_shift;
> +       s8              div_shift;
> +       s8              div_width;
> +       s16             div_initval;
> +       struct clk_div_table *table;
> +};
> +
> +struct bm1880_pll_clock {
> +       unsigned int    id;
> +       const char      *name;
> +       const char      *parent;
> +       u32             reg;
> +       unsigned long   flags;
> +};
> +
> +struct bm1880_pll_hw_clock {
> +       struct bm1880_pll_clock pll;
> +       void __iomem *base;
> +       struct clk_hw hw;
> +};
> +
> +#define GATE_DIV(_id, _name, _parent, _gate_reg, _gate_shift, _div_reg,        \
> +                       _div_shift, _div_width, _div_initval, _table,   \
> +                       _flags) {                                       \
> +               .id = _id,                                              \
> +               .parent = _parent,                                      \
> +               .name = _name,                                          \
> +               .gate_reg = _gate_reg,                                  \
> +               .gate_shift = _gate_shift,                              \
> +               .div_reg = _div_reg,                                    \
> +               .div_shift = _div_shift,                                \
> +               .div_width = _div_width,                                \
> +               .div_initval = _div_initval,                            \
> +               .table = _table,                                        \
> +               .mux_shift = -1,                                        \
> +               .flags = _flags,                                        \
> +       }
> +
> +#define GATE_MUX(_id, _name, _parents, _gate_reg, _gate_shift,         \
> +                       _mux_reg, _mux_shift, _flags) {                 \
> +               .id = _id,                                              \
> +               .parents = _parents,                                    \
> +               .num_parents = ARRAY_SIZE(_parents),                    \
> +               .name = _name,                                          \
> +               .gate_reg = _gate_reg,                                  \
> +               .gate_shift = _gate_shift,                              \
> +               .div_shift = -1,                                        \
> +               .mux_reg = _mux_reg,                                    \
> +               .mux_shift = _mux_shift,                                \
> +               .flags = _flags,                                        \
> +       }
> +
> +static const struct bm1880_pll_clock bm1880_pll_clks[] = {
> +       { BM1880_CLK_MPLL, "clk_mpll", "osc", BM1880_CLK_MPLL_CTL,
> +         CLK_IS_CRITICAL },
> +       { BM1880_CLK_SPLL, "clk_spll", "osc", BM1880_CLK_SPLL_CTL,
> +         CLK_IS_CRITICAL },
> +       { BM1880_CLK_FPLL, "clk_fpll", "osc", BM1880_CLK_FPLL_CTL,
> +         CLK_IS_CRITICAL },
> +       { BM1880_CLK_DDRPLL, "clk_ddrpll", "osc", BM1880_CLK_DDRPLL_CTL,
> +         CLK_IS_CRITICAL },
> +};
> +
> +static const struct bm1880_gate_clock bm1880_gate_clks[] = {
> +       { BM1880_CLK_AHB_ROM, "clk_ahb_rom", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 2, CLK_IS_CRITICAL },
> +       { BM1880_CLK_AXI_SRAM, "clk_axi_sram", "clk_axi1",
> +         BM1880_CLK_ENABLE0, 3, CLK_IS_CRITICAL },
> +       { BM1880_CLK_DDR_AXI, "clk_ddr_axi", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 4, CLK_IS_CRITICAL },
> +       { BM1880_CLK_APB_EFUSE, "clk_apb_efuse", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 6, CLK_IS_CRITICAL },
> +       { BM1880_CLK_AXI5_EMMC, "clk_axi5_emmc", "clk_axi5",
> +         BM1880_CLK_ENABLE0, 7, 0 },
> +       { BM1880_CLK_AXI5_SD, "clk_axi5_sd", "clk_axi5",
> +         BM1880_CLK_ENABLE0, 10, 0 },
> +       { BM1880_CLK_AXI4_ETH0, "clk_axi4_eth0", "clk_axi4",
> +         BM1880_CLK_ENABLE0, 14, 0 },
> +       { BM1880_CLK_AXI4_ETH1, "clk_axi4_eth1", "clk_axi4",
> +         BM1880_CLK_ENABLE0, 16, 0 },
> +       { BM1880_CLK_AXI1_GDMA, "clk_axi1_gdma", "clk_axi1",
> +         BM1880_CLK_ENABLE0, 17, 0 },
> +       /* Don't gate GPIO clocks as it is not owned by the GPIO driver */
> +       { BM1880_CLK_APB_GPIO, "clk_apb_gpio", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 18, CLK_IGNORE_UNUSED },
> +       { BM1880_CLK_APB_GPIO_INTR, "clk_apb_gpio_intr", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 19, CLK_IGNORE_UNUSED },
> +       { BM1880_CLK_AXI1_MINER, "clk_axi1_miner", "clk_axi1",
> +         BM1880_CLK_ENABLE0, 21, 0 },
> +       { BM1880_CLK_AHB_SF, "clk_ahb_sf", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 22, 0 },
> +       { BM1880_CLK_SDMA_AXI, "clk_sdma_axi", "clk_axi5",
> +         BM1880_CLK_ENABLE0, 23, 0 },
> +       { BM1880_CLK_APB_I2C, "clk_apb_i2c", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 25, 0 },
> +       { BM1880_CLK_APB_WDT, "clk_apb_wdt", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 26, 0 },
> +       { BM1880_CLK_APB_JPEG, "clk_apb_jpeg", "clk_axi6",
> +         BM1880_CLK_ENABLE0, 27, 0 },
> +       { BM1880_CLK_AXI5_NF, "clk_axi5_nf", "clk_axi5",
> +         BM1880_CLK_ENABLE0, 29, 0 },
> +       { BM1880_CLK_APB_NF, "clk_apb_nf", "clk_axi6",
> +         BM1880_CLK_ENABLE0, 30, 0 },
> +       { BM1880_CLK_APB_PWM, "clk_apb_pwm", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE1, 0, 0 },
> +       { BM1880_CLK_RV, "clk_rv", "clk_mux_rv",
> +         BM1880_CLK_ENABLE1, 1, 0 },
> +       { BM1880_CLK_APB_SPI, "clk_apb_spi", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE1, 2, 0 },
> +       { BM1880_CLK_UART_500M, "clk_uart_500m", "clk_div_uart_500m",
> +         BM1880_CLK_ENABLE1, 4, 0 },
> +       { BM1880_CLK_APB_UART, "clk_apb_uart", "clk_axi6",
> +         BM1880_CLK_ENABLE1, 5, 0 },
> +       { BM1880_CLK_APB_I2S, "clk_apb_i2s", "clk_axi6",
> +         BM1880_CLK_ENABLE1, 6, 0 },
> +       { BM1880_CLK_AXI4_USB, "clk_axi4_usb", "clk_axi4",
> +         BM1880_CLK_ENABLE1, 7, 0 },
> +       { BM1880_CLK_APB_USB, "clk_apb_usb", "clk_axi6",
> +         BM1880_CLK_ENABLE1, 8, 0 },
> +       { BM1880_CLK_12M_USB, "clk_12m_usb", "clk_div_12m_usb",
> +         BM1880_CLK_ENABLE1, 11, 0 },
> +       { BM1880_CLK_APB_VIDEO, "clk_apb_video", "clk_axi6",
> +         BM1880_CLK_ENABLE1, 12, 0 },
> +       { BM1880_CLK_APB_VPP, "clk_apb_vpp", "clk_axi6",
> +         BM1880_CLK_ENABLE1, 15, 0 },
> +       { BM1880_CLK_AXI6, "clk_axi6", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE1, 21, CLK_IS_CRITICAL },
> +};
> +
> +static const char * const clk_a53_parents[] = { "clk_spll", "clk_mpll" };
> +static const char * const clk_rv_parents[] = { "clk_div_1_rv", "clk_div_0_rv" };
> +static const char * const clk_axi1_parents[] = { "clk_div_1_axi1", "clk_div_0_axi1" };
> +static const char * const clk_axi6_parents[] = { "clk_div_1_axi6", "clk_div_0_axi6" };
> +
> +static const struct bm1880_mux_clock bm1880_mux_clks[] = {
> +       { BM1880_CLK_MUX_RV, "clk_mux_rv", clk_rv_parents, 2,
> +         BM1880_CLK_SELECT, 1, 0 },
> +       { BM1880_CLK_MUX_AXI6, "clk_mux_axi6", clk_axi6_parents, 2,
> +         BM1880_CLK_SELECT, 3, 0 },
> +};
> +
> +static struct clk_div_table bm1880_div_table_0[] = {

Can these tables be const?

> +       { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> +       { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> +       { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> +       { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> +       { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> +       { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> +       { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> +       { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> +       { 0, 0 }
> +};
> +
> +static struct clk_div_table bm1880_div_table_1[] = {
> +       { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> +       { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> +       { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> +       { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> +       { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> +       { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> +       { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> +       { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> +       { 127, 128 }, { 0, 0 }
> +};
> +
> +static struct clk_div_table bm1880_div_table_2[] = {
> +       { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> +       { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> +       { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> +       { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> +       { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> +       { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> +       { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> +       { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> +       { 127, 128 }, { 255, 256 }, { 0, 0 }
> +};
> +
> +static struct clk_div_table bm1880_div_table_3[] = {
> +       { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> +       { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> +       { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> +       { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> +       { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> +       { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> +       { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> +       { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> +       { 127, 128 }, { 255, 256 }, { 511, 512 }, { 0, 0 }
> +};
> +
> +static struct clk_div_table bm1880_div_table_4[] = {
> +       { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> +       { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> +       { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> +       { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> +       { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> +       { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> +       { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> +       { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> +       { 127, 128 }, { 255, 256 }, { 511, 512 }, { 65535, 65536 },
> +       { 0, 0 }
> +};
> +
> +static const struct bm1880_div_clock bm1880_div_clks[] = {
> +       { BM1880_CLK_DIV_0_RV, "clk_div_0_rv", "clk_spll",
> +         BM1880_CLK_DIV12, 16, 5, 1, bm1880_div_table_0, CLK_IGNORE_UNUSED },
> +       { BM1880_CLK_DIV_1_RV, "clk_div_1_rv", "clk_fpll",
> +         BM1880_CLK_DIV13, 16, 5, 1, bm1880_div_table_0, CLK_IGNORE_UNUSED },
> +       { BM1880_CLK_DIV_UART_500M, "clk_div_uart_500m", "clk_fpll",
> +         BM1880_CLK_DIV15, 16, 7, 3, bm1880_div_table_1, 0 },
> +       { BM1880_CLK_DIV_0_AXI1, "clk_div_0_axi1", "clk_mpll",
> +         BM1880_CLK_DIV21, 16, 5, 2, bm1880_div_table_0, CLK_IS_CRITICAL },
> +       { BM1880_CLK_DIV_1_AXI1, "clk_div_1_axi1", "clk_fpll",
> +         BM1880_CLK_DIV22, 16, 5, 3, bm1880_div_table_0, CLK_IS_CRITICAL },
> +       { BM1880_CLK_DIV_0_AXI6, "clk_div_0_axi6", "clk_fpll",
> +         BM1880_CLK_DIV27, 16, 5, 15, bm1880_div_table_0, CLK_IS_CRITICAL },
> +       { BM1880_CLK_DIV_1_AXI6, "clk_div_1_axi6", "clk_mpll",
> +         BM1880_CLK_DIV28, 16, 5, 11, bm1880_div_table_0, CLK_IS_CRITICAL },
> +       { BM1880_CLK_DIV_12M_USB, "clk_div_12m_usb", "clk_fpll",
> +         BM1880_CLK_DIV18, 16, 7, 125, bm1880_div_table_1, 0 },
> +};
> +
> +static struct bm1880_composite_clock bm1880_composite_clks[] = {
> +       GATE_MUX(BM1880_CLK_A53, "clk_a53", clk_a53_parents,
> +                BM1880_CLK_ENABLE0, 0, BM1880_CLK_SELECT, 0,
> +                CLK_IS_CRITICAL),

Please document why CLK_IS_CRITICAL. Maybe CPU clk so must be kept on?

> +       GATE_DIV(BM1880_CLK_50M_A53, "clk_50m_a53", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 1, BM1880_CLK_DIV0, 16, 5, 30,
> +                bm1880_div_table_0, CLK_IS_CRITICAL),
> +       GATE_DIV(BM1880_CLK_EFUSE, "clk_efuse", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 5, BM1880_CLK_DIV1, 16, 7, 60,
> +                bm1880_div_table_1, 0),
> +       GATE_DIV(BM1880_CLK_EMMC, "clk_emmc", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 8, BM1880_CLK_DIV2, 16, 5, 15,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_100K_EMMC, "clk_100k_emmc", "clk_div_12m_usb",
> +                BM1880_CLK_ENABLE0, 9, BM1880_CLK_DIV3, 16, 8, 120,
> +                bm1880_div_table_2, 0),
> +       GATE_DIV(BM1880_CLK_SD, "clk_sd", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 11, BM1880_CLK_DIV4, 16, 5, 15,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_100K_SD, "clk_100k_sd", "clk_div_12m_usb",
> +                BM1880_CLK_ENABLE0, 12, BM1880_CLK_DIV5, 16, 8, 120,
> +                bm1880_div_table_2, 0),
> +       GATE_DIV(BM1880_CLK_500M_ETH0, "clk_500m_eth0", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 13, BM1880_CLK_DIV6, 16, 5, 3,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_500M_ETH1, "clk_500m_eth1", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 15, BM1880_CLK_DIV7, 16, 5, 3,
> +                bm1880_div_table_0, 0),
> +       /* Don't gate GPIO clocks as it is not owned by the GPIO driver */
> +       GATE_DIV(BM1880_CLK_GPIO_DB, "clk_gpio_db", "clk_div_12m_usb",
> +                BM1880_CLK_ENABLE0, 20, BM1880_CLK_DIV8, 16, 16, 120,
> +                bm1880_div_table_4, CLK_IGNORE_UNUSED),
> +       GATE_DIV(BM1880_CLK_SDMA_AUD, "clk_sdma_aud", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 24, BM1880_CLK_DIV9, 16, 7, 61,
> +                bm1880_div_table_1, 0),
> +       GATE_DIV(BM1880_CLK_JPEG_AXI, "clk_jpeg_axi", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 28, BM1880_CLK_DIV10, 16, 5, 4,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_NF, "clk_nf", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 31, BM1880_CLK_DIV11, 16, 5, 30,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_TPU_AXI, "clk_tpu_axi", "clk_spll",
> +                BM1880_CLK_ENABLE1, 3, BM1880_CLK_DIV14, 16, 5, 1,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_125M_USB, "clk_125m_usb", "clk_fpll",
> +                BM1880_CLK_ENABLE1, 9, BM1880_CLK_DIV16, 16, 5, 12,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_33K_USB, "clk_33k_usb", "clk_div_12m_usb",
> +                BM1880_CLK_ENABLE1, 10, BM1880_CLK_DIV17, 16, 9, 363,
> +                bm1880_div_table_3, 0),
> +       GATE_DIV(BM1880_CLK_VIDEO_AXI, "clk_video_axi", "clk_fpll",
> +                BM1880_CLK_ENABLE1, 13, BM1880_CLK_DIV19, 16, 5, 4,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_VPP_AXI, "clk_vpp_axi", "clk_fpll",
> +                BM1880_CLK_ENABLE1, 14, BM1880_CLK_DIV20, 16, 5, 4,
> +                bm1880_div_table_0, 0),
> +       GATE_MUX(BM1880_CLK_AXI1, "clk_axi1", clk_axi1_parents,
> +                BM1880_CLK_ENABLE1, 15, BM1880_CLK_SELECT, 2,
> +                CLK_IS_CRITICAL),
> +       GATE_DIV(BM1880_CLK_AXI2, "clk_axi2", "clk_fpll",
> +                BM1880_CLK_ENABLE1, 17, BM1880_CLK_DIV23, 16, 5, 3,
> +                bm1880_div_table_0, CLK_IS_CRITICAL),
> +       GATE_DIV(BM1880_CLK_AXI3, "clk_axi3", "clk_mux_rv",
> +                BM1880_CLK_ENABLE1, 18, BM1880_CLK_DIV24, 16, 5, 2,
> +                bm1880_div_table_0, CLK_IS_CRITICAL),
> +       GATE_DIV(BM1880_CLK_AXI4, "clk_axi4", "clk_fpll",
> +                BM1880_CLK_ENABLE1, 19, BM1880_CLK_DIV25, 16, 5, 6,
> +                bm1880_div_table_0, CLK_IS_CRITICAL),
> +       GATE_DIV(BM1880_CLK_AXI5, "clk_axi5", "clk_fpll",
> +                BM1880_CLK_ENABLE1, 20, BM1880_CLK_DIV26, 16, 5, 15,
> +                bm1880_div_table_0, CLK_IS_CRITICAL),
> +};
> +
> +static unsigned long bm1880_pll_rate_calc(u32 regval, unsigned long parent_rate)
> +{
> +       u32 fbdiv, fref, refdiv;
> +       u32 postdiv1, postdiv2;
> +       unsigned long rate, numerator, denominator;
> +
> +       fbdiv = (regval >> 16) & 0xfff;
> +       fref = parent_rate;
> +       refdiv = regval & 0x1f;
> +       postdiv1 = (regval >> 8) & 0x7;
> +       postdiv2 = (regval >> 12) & 0x7;
> +
> +       numerator = parent_rate * fbdiv;
> +       denominator = refdiv * postdiv1 * postdiv2;
> +       do_div(numerator, denominator);
> +       rate = numerator;
> +
> +       return rate;
> +}
> +
> +static unsigned long bm1880_pll_recalc_rate(struct clk_hw *hw,
> +                                           unsigned long parent_rate)
> +{
> +       struct bm1880_pll_hw_clock *pll_hw = to_bm1880_pll_clk(hw);
> +       unsigned long rate;
> +       u32 regval;
> +
> +       regval = readl(pll_hw->base + pll_hw->pll.reg);
> +       rate = bm1880_pll_rate_calc(regval, parent_rate);
> +
> +       return rate;
> +}
> +
> +static const struct clk_ops bm1880_pll_ops = {
> +       .recalc_rate    = bm1880_pll_recalc_rate,
> +};
> +
> +struct clk *bm1880_clk_register_pll(const struct bm1880_pll_clock *pll_clk,
> +                                   void __iomem *sys_base)
> +{
> +       struct bm1880_pll_hw_clock *pll_hw;
> +       struct clk_init_data init;
> +       struct clk_hw *hw;
> +       int err;
> +
> +       pll_hw = kzalloc(sizeof(*pll_hw), GFP_KERNEL);
> +       if (!pll_hw)
> +               return ERR_PTR(-ENOMEM);
> +
> +       init.name = pll_clk->name;
> +       init.ops = &bm1880_pll_ops;
> +       init.flags = pll_clk->flags;
> +       init.parent_names = &pll_clk->parent;

Can you use the new way of specifying parents instead of using strings
for everything?

> +       init.num_parents = 1;
> +
> +       pll_hw->hw.init = &init;
> +       pll_hw->pll.reg = pll_clk->reg;
> +       pll_hw->base = sys_base;
> +
> +       hw = &pll_hw->hw;
> +       err = clk_hw_register(NULL, hw);
> +
> +       if (err) {
> +               kfree(pll_hw);
> +               return ERR_PTR(err);
> +       }
> +
> +       return hw->clk;

Can this return the clk_hw pointer instead?

> +}
> +
> +void bm1880_clk_unregister_pll(struct clk *clk)

Should this be static?

> +{
> +       struct bm1880_pll_hw_clock *pll_hw;
> +       struct clk_hw *hw;
> +
> +       hw = __clk_get_hw(clk);
> +       if (!hw)
> +               return;
> +
> +       pll_hw = to_bm1880_pll_clk(hw);
> +
> +       clk_unregister(clk);
> +       kfree(pll_hw);
> +}
> +
> +int bm1880_clk_register_plls(const struct bm1880_pll_clock *clks,
> +                            int num_clks, struct bm1880_clock_data *data)
> +{
> +       struct clk *clk;
> +       void __iomem *pll_base = data->pll_base;
> +       int i;
> +
> +       for (i = 0; i < num_clks; i++) {
> +               const struct bm1880_pll_clock *bm1880_clk = &clks[i];
> +
> +               clk = bm1880_clk_register_pll(bm1880_clk, pll_base);
> +               if (IS_ERR(clk)) {
> +                       pr_err("%s: failed to register clock %s\n",
> +                              __func__, bm1880_clk->name);
> +                       goto err_clk;
> +               }
> +
> +               data->clk_data.clks[clks[i].id] = clk;
> +       }
> +
> +       return 0;
> +
> +err_clk:
> +       while (i--)

I guess while (--i) is more idiomatic but this works too.

> +               bm1880_clk_unregister_pll(data->clk_data.clks[clks[i].id]);
> +
> +       return PTR_ERR(clk);
> +}
> +
> +int bm1880_clk_register_mux(const struct bm1880_mux_clock *clks,
> +                           int num_clks, struct bm1880_clock_data *data)
> +{
> +       struct clk *clk;
> +       void __iomem *sys_base = data->sys_base;
> +       int i;
> +
> +       for (i = 0; i < num_clks; i++) {
> +               clk = clk_register_mux(NULL, clks[i].name,

Can you use the clk_hw based APIs for generic type clks?

> +                                      clks[i].parents,
> +                                      clks[i].num_parents,
> +                                      clks[i].flags,
> +                                      sys_base + clks[i].reg,
> +                                      clks[i].shift, 1, 0,
> +                                      &bm1880_clk_lock);
> +               if (IS_ERR(clk)) {
> +                       pr_err("%s: failed to register clock %s\n",
> +                              __func__, clks[i].name);
> +                       goto err_clk;
> +               }
> +
> +               data->clk_data.clks[clks[i].id] = clk;
> +       }
> +
> +       return 0;
> +
> +err_clk:
> +       while (i--)
> +               clk_unregister_gate(data->clk_data.clks[clks[i].id]);
> +
> +       return PTR_ERR(clk);
> +}
> +
> +static unsigned long bm1880_clk_div_recalc_rate(struct clk_hw *hw,
> +                                               unsigned long parent_rate)
> +{
> +       struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
> +       struct bm1880_div_clock *div = &div_hw->div;
> +       void __iomem *reg_addr = div_hw->base + div->reg;
> +       unsigned int val;
> +       unsigned long rate;
> +
> +       if (!(readl(reg_addr) & BIT(3))) {
> +               val = div->initval;
> +       } else {
> +               val = readl(reg_addr) >> div->shift;
> +               val &= clk_div_mask(div->width);
> +       }
> +
> +       rate = divider_recalc_rate(hw, parent_rate, val, div->table,
> +                                  div->flags, div->width);
> +
> +       return rate;
> +}
> +
> +static long bm1880_clk_div_round_rate(struct clk_hw *hw, unsigned long rate,
> +                                     unsigned long *prate)
> +{
> +       struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
> +       struct bm1880_div_clock *div = &div_hw->div;
> +       void __iomem *reg_addr = div_hw->base + div->reg;
> +
> +       if (div->flags & CLK_DIVIDER_READ_ONLY) {
> +               u32 val;
> +
> +               val = readl(reg_addr) >> div->shift;
> +               val &= clk_div_mask(div->width);
> +
> +               return divider_ro_round_rate(hw, rate, prate, div->table,
> +                                            div->width, div->flags,
> +                                            val);
> +       }
> +
> +       return divider_round_rate(hw, rate, prate, div->table,
> +                                 div->width, div->flags);
> +}
> +
> +static int bm1880_clk_div_set_rate(struct clk_hw *hw, unsigned long rate,
> +                                  unsigned long parent_rate)
> +{
> +       struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
> +       struct bm1880_div_clock *div = &div_hw->div;
> +       void __iomem *reg_addr = div_hw->base + div->reg;
> +       unsigned long flags = 0;
> +       int value;
> +       u32 val;
> +
> +       value = divider_get_val(rate, parent_rate, div->table,
> +                               div->width, div_hw->div.flags);
> +       if (value < 0)
> +               return value;
> +
> +       if (div_hw->lock)
> +               spin_lock_irqsave(div_hw->lock, flags);
> +       else
> +               __acquire(div_hw->lock);
> +
> +       if (div->flags & CLK_DIVIDER_HIWORD_MASK) {
> +               val = clk_div_mask(div->width) << (div_hw->div.shift + 16);
> +       } else {
> +               val = readl(reg_addr);
> +               val &= ~(clk_div_mask(div->width) << div_hw->div.shift);
> +       }
> +       val |= (u32)value << div->shift;
> +       writel(val, reg_addr);
> +
> +       if (div_hw->lock)
> +               spin_unlock_irqrestore(div_hw->lock, flags);
> +       else
> +               __release(div_hw->lock);
> +
> +       return 0;
> +}
> +
> +const struct clk_ops bm1880_clk_div_ops = {

static?

> +       .recalc_rate = bm1880_clk_div_recalc_rate,
> +       .round_rate = bm1880_clk_div_round_rate,
> +       .set_rate = bm1880_clk_div_set_rate,
> +};
> +
> +struct clk *bm1880_clk_register_div(const struct bm1880_div_clock *div_clk,
> +                                   void __iomem *sys_base)
> +{
> +       struct bm1880_div_hw_clock *div_hw;
> +       struct clk_init_data init;
> +       struct clk_hw *hw;
> +       int err;
> +
> +       div_hw = kzalloc(sizeof(*div_hw), GFP_KERNEL);
> +       if (!div_hw)
> +               return ERR_PTR(-ENOMEM);
> +
> +       init.name = div_clk->name;
> +       init.ops = &bm1880_clk_div_ops;
> +       init.flags = div_clk->flags;
> +       init.parent_names = &div_clk->parent;
> +       init.num_parents = 1;
> +
> +       div_hw->hw.init = &init;
> +       div_hw->div.reg = div_clk->reg;
> +       div_hw->div.shift = div_clk->shift;
> +       div_hw->div.width = div_clk->width;
> +       div_hw->div.initval = div_clk->initval;
> +       div_hw->div.table = div_clk->table;
> +       div_hw->div.flags = CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO;
> +       div_hw->base = sys_base;
> +       div_hw->lock = &bm1880_clk_lock;
> +
> +       hw = &div_hw->hw;
> +       err = clk_hw_register(NULL, hw);
> +
> +       if (err) {
> +               kfree(div_hw);
> +               return ERR_PTR(err);
> +       }
> +
> +       return hw->clk;
> +}
> +
> +void bm1880_clk_unregister_div(struct clk *clk)
> +{
> +       struct bm1880_div_hw_clock *div_hw;
> +       struct clk_hw *hw;
> +
> +       hw = __clk_get_hw(clk);
> +       if (!hw)
> +               return;
> +
> +       div_hw = to_bm1880_div_clk(hw);
> +
> +       clk_unregister(clk);
> +       kfree(div_hw);
> +}
> +
> +int bm1880_clk_register_divs(const struct bm1880_div_clock *clks,
> +                            int num_clks, struct bm1880_clock_data *data)
> +{
> +       struct clk *clk;
> +       void __iomem *sys_base = data->sys_base;
> +       int i;
> +
> +       for (i = 0; i < num_clks; i++) {
> +               const struct bm1880_div_clock *bm1880_clk = &clks[i];
> +
> +               clk = bm1880_clk_register_div(bm1880_clk, sys_base);
> +               if (IS_ERR(clk)) {
> +                       pr_err("%s: failed to register clock %s\n",
> +                              __func__, bm1880_clk->name);
> +                       goto err_clk;
> +               }
> +
> +               data->clk_data.clks[clks[i].id] = clk;
> +       }
> +
> +       return 0;
> +
> +err_clk:
> +       while (i--)
> +               bm1880_clk_unregister_div(data->clk_data.clks[clks[i].id]);
> +
> +       return PTR_ERR(clk);
> +}
> +
> +int bm1880_clk_register_gate(const struct bm1880_gate_clock *clks,
> +                            int num_clks, struct bm1880_clock_data *data)
> +{
> +       struct clk *clk;
> +       void __iomem *sys_base = data->sys_base;
> +       int i;
> +
> +       for (i = 0; i < num_clks; i++) {
> +               clk = clk_register_gate(NULL, clks[i].name,
> +                                       clks[i].parent,
> +                                       clks[i].flags,
> +                                       sys_base + clks[i].gate_reg,
> +                                       clks[i].gate_shift,
> +                                       0,
> +                                       &bm1880_clk_lock);
> +               if (IS_ERR(clk)) {
> +                       pr_err("%s: failed to register clock %s\n",
> +                              __func__, clks[i].name);
> +                       goto err_clk;
> +               }
> +
> +               data->clk_data.clks[clks[i].id] = clk;
> +       }
> +
> +       return 0;
> +
> +err_clk:
> +       while (i--)
> +               clk_unregister_gate(data->clk_data.clks[clks[i].id]);
> +
> +       return PTR_ERR(clk);
> +}
> +
> +struct clk *bm1880_clk_register_composite(struct bm1880_composite_clock *clks,
> +                                         void __iomem *sys_base)
> +{
> +       struct clk *clk;
> +       struct clk_mux *mux = NULL;
> +       struct clk_gate *gate = NULL;
> +       struct bm1880_div_hw_clock *div_hws = NULL;
> +       struct clk_hw *mux_hw = NULL, *gate_hw = NULL, *div_hw = NULL;
> +       const struct clk_ops *mux_ops = NULL, *gate_ops = NULL, *div_ops = NULL;
> +       const char * const *parent_names;
> +       const char *parent;
> +       int num_parents;
> +       int ret;
> +
> +       if (clks->mux_shift >= 0) {
> +               mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> +               if (!mux)
> +                       return ERR_PTR(-ENOMEM);
> +
> +               mux->reg = sys_base + clks->mux_reg;
> +               mux->mask = 1;
> +               mux->shift = clks->mux_shift;
> +               mux_hw = &mux->hw;
> +               mux_ops = &clk_mux_ops;
> +               mux->lock = &bm1880_clk_lock;
> +
> +               parent_names = clks->parents;
> +               num_parents = clks->num_parents;
> +       } else {
> +               parent = clks->parent;
> +               parent_names = &parent;
> +               num_parents = 1;
> +       }
> +
> +       if (clks->gate_shift >= 0) {
> +               gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> +               if (!gate) {
> +                       ret = -ENOMEM;
> +                       goto err_out;
> +               }
> +
> +               gate->reg = sys_base + clks->gate_reg;
> +               gate->bit_idx = clks->gate_shift;
> +               gate->lock = &bm1880_clk_lock;
> +
> +               gate_hw = &gate->hw;
> +               gate_ops = &clk_gate_ops;
> +       }
> +
> +       if (clks->div_shift >= 0) {
> +               div_hws = kzalloc(sizeof(*div_hws), GFP_KERNEL);
> +               if (!div_hws) {
> +                       ret = -ENOMEM;
> +                       goto err_out;
> +               }
> +
> +               div_hws->base = sys_base;
> +               div_hws->div.reg = clks->div_reg;
> +               div_hws->div.shift = clks->div_shift;
> +               div_hws->div.width = clks->div_width;
> +               div_hws->div.table = clks->table;
> +               div_hws->div.initval = clks->div_initval;
> +               div_hws->lock = &bm1880_clk_lock;
> +               div_hws->div.flags = CLK_DIVIDER_ONE_BASED |
> +                                    CLK_DIVIDER_ALLOW_ZERO;
> +
> +               div_hw = &div_hws->hw;
> +               div_ops = &bm1880_clk_div_ops;
> +       }
> +
> +       clk = clk_register_composite(NULL, clks->name, parent_names,
> +                                    num_parents, mux_hw, mux_ops, div_hw,
> +                                    div_ops, gate_hw, gate_ops, (clks->flags));
> +
> +       if (IS_ERR(clk)) {
> +               ret = PTR_ERR(clk);
> +               goto err_out;
> +       }
> +
> +       return clk;
> +
> +err_out:
> +       kfree(div_hws);
> +       kfree(gate);
> +       kfree(mux);
> +
> +       return ERR_PTR(ret);
> +}
> +
> +int bm1880_clk_register_composites(struct bm1880_composite_clock *clks,
> +                                  int num_clks, struct bm1880_clock_data *data)
> +{
> +       struct clk *clk;
> +       void __iomem *sys_base = data->sys_base;
> +       int i;
> +
> +       for (i = 0; i < num_clks; i++) {
> +               struct bm1880_composite_clock *bm1880_clk = &clks[i];
> +
> +               clk = bm1880_clk_register_composite(bm1880_clk, sys_base);
> +               if (IS_ERR(clk)) {
> +                       pr_err("%s: failed to register clock %s\n",
> +                              __func__, bm1880_clk->name);
> +                       goto err_clk;
> +               }
> +
> +               data->clk_data.clks[clks[i].id] = clk;
> +       }
> +
> +       return 0;
> +
> +err_clk:
> +       while (i--)
> +               clk_unregister_composite(data->clk_data.clks[clks[i].id]);
> +
> +       return PTR_ERR(clk);
> +}
> +
> +static void bm1880_clk_init(struct device_node *np)
> +{
> +       struct bm1880_clock_data *clk_data;
> +       struct clk **clk_table;
> +       void __iomem *pll_base, *sys_base;
> +       int num_clks;
> +
> +       pll_base = of_iomap(np, 0);
> +       if (!pll_base) {
> +               pr_err("%pOFn: unable to map pll resource", np);
> +               of_node_put(np);
> +               return;
> +       }
> +
> +       sys_base = of_iomap(np, 1);
> +       if (!sys_base) {
> +               pr_err("%pOFn: unable to map sys resource", np);
> +               of_node_put(np);
> +               return;
> +       }
> +
> +       clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
> +       if (!clk_data)
> +               return;
> +
> +       clk_data->pll_base = pll_base;
> +       clk_data->sys_base = sys_base;
> +       num_clks = ARRAY_SIZE(bm1880_gate_clks) +
> +                  ARRAY_SIZE(bm1880_composite_clks);
> +
> +       clk_table = kcalloc(num_clks, sizeof(*clk_table), GFP_KERNEL);
> +       if (!clk_table)
> +               goto err_out;
> +
> +       clk_data->clk_data.clks = clk_table;
> +       clk_data->clk_data.clk_num = num_clks;
> +
> +       /* Register PLL clocks */
> +       bm1880_clk_register_plls(bm1880_pll_clks,
> +                                ARRAY_SIZE(bm1880_pll_clks),
> +                                clk_data);
> +
> +       /* Register Divider clocks */

Please remove these comments, they provide no useful information.

> +       bm1880_clk_register_divs(bm1880_div_clks,
> +                                ARRAY_SIZE(bm1880_div_clks),
> +                                clk_data);
> +
> +       /* Register Mux clocks */
> +       bm1880_clk_register_mux(bm1880_mux_clks,
> +                               ARRAY_SIZE(bm1880_mux_clks),
> +                               clk_data);
> +
> +       /* Register Composite clocks */
> +       bm1880_clk_register_composites(bm1880_composite_clks,
> +                                      ARRAY_SIZE(bm1880_composite_clks),
> +                                      clk_data);
> +
> +       /* Register Gate clocks */
> +       bm1880_clk_register_gate(bm1880_gate_clks,
> +                                ARRAY_SIZE(bm1880_gate_clks),
> +                                clk_data);
> +
> +       of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data->clk_data);
> +
> +       return;
> +
> +err_out:
> +       kfree(clk_data);
> +}
> +
> +CLK_OF_DECLARE_DRIVER(bm1880_clk, "bitmain,bm1880-clk", bm1880_clk_init);

Is there a reason why it can't be a platform driver?


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2] arm64: dts: allwinner: a64: Enable eMMC on A64-OLinuXino
From: Chen-Yu Tsai @ 2019-08-08  5:12 UTC (permalink / raw)
  To: Maxime Ripard; +Cc: Sunil Mohan Adapa, Martin Ayotte, linux-arm-kernel
In-Reply-To: <20190807144533.bbgtmkva34su7c5v@flea>

On Wed, Aug 7, 2019 at 10:45 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> Hi,
>
> On Wed, Aug 07, 2019 at 08:09:19PM +0800, Chen-Yu Tsai wrote:
> > On Wed, Aug 7, 2019 at 8:01 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Tue, Aug 06, 2019 at 02:25:17PM +0800, Chen-Yu Tsai wrote:
> > > > On Mon, Aug 5, 2019 at 8:58 PM Martin Ayotte <martinayotte@gmail.com> wrote:
> > > > >
> > > > > Fine for me too.
> > > > >
> > > > > Thanks .
> > > > >
> > > > > -----Message d'origine-----
> > > > > De : Sunil Mohan Adapa [mailto:sunil@medhas.org]
> > > > > Envoyé : Monday, August 05, 2019 1:25 AM
> > > > > Ą : Chen-Yu Tsai
> > > > > Cc : Maxime Ripard; Martin Ayotte; linux-arm-kernel
> > > > > Objet : Re: [PATCH v2] arm64: dts: allwinner: a64: Enable eMMC on
> > > > > A64-OLinuXino
> > > > >
> > > > > On 04/08/19 8:33 pm, Chen-Yu Tsai wrote:
> > > > > > On Fri, Aug 2, 2019 at 2:47 AM Sunil Mohan Adapa <sunil@medhas.org> wrote:
> > > > > >>
> > > > > >> On 01/08/19 6:49 am, Martin Ayotte wrote:
> > > > > >>> If my SOB could help here, I don't mind since I've done the commit
> > > > > >>> more than a year ago for Armbian ...
> > > > > >>>
> > > > > >>> Signed-off-by: Martin Ayotte <martinayotte@gmai.com>
> > > > > >>> Tested-by: Martin Ayotte <martinayotte@gmai.com>
> > > > > >> gmai.com is likely a typo.
> > > > > >>
> > > > > >>> On Wed, Jul 31, 2019 at 10:42 PM Chen-Yu Tsai <wens@csie.org
> > > > > >>>
> > > > > >>>> Thanks. The patch looks good overall. The authorship is a little
> > > > > >>>> confusing though. If it was initially done by Martin (CC-ed), then
> > > > > >>>> he should be the author, and we should get his Signed-off-by if
> > > > > >>>> possible.
> > > > > >>
> > > > > >> Martin is indeed the original author of the patch. Thank you for
> > > > > reviewing.
> > > > > >
> > > > > > I'd like to apply this patch with Martin as the author, if that's OK with
> > > > > you
> > > > > > both?
> > > > >
> > > > > That is completely okay with me.
> > > >
> > > > Applied for 5.4.
> > > >
> > > > I reordered the tags so they make more sense:
> > > >
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git/commit/?h=sunxi/dt-for-5.4&id=0834887732df5af41b59b2e4d530fc1f5478965f
> > >
> > > Sorry for being late on this, but it looks like the eMMC, NAND and SPI
> > > pins are conflicting on the A64-Olinuxino design.
> > >
> > > There's no configuration with a NAND, so we don't really need to worry
> > > about that, but if we merge this in the main DT, we'll prevent anyone
> > > from using that DT on an olinuxino with a SPI flash.
> > >
> > > I think we should just create emmc and SPI-flash variants of that DT.
> >
> > Actually they aren't. Olimex specifically uses eMMC modules that don't
> > use the data strobe line, so SPI can be used together.
>
> Ah, right.
>
> Still, this creates a precedent that I'm not really comfortable
> with. Three actually.
>
> Merging this in the main DT means three things:
>   - We're not consistent anymore, including within the olinuxino
>     boards only. A20 Olinuxino is pretty much in the same situation,
>     yet we dealt with it differently.
>   - This means that this will create a spurious device and report
>     errors in the kernel message and whenever someone will try to
>     access the device on boards that don't have it wired. This
>     shouldn't happen and we really shouldn't expose devices that just
>     aren't there, just like you don't have all the devices that are
>     not connected on your USB connector.
>   - Finally, this means that in order to keep it somewhat consistent,
>     we would have to merge the SPI flash in the main DT too. This will
>     prevent people without a SPI flash to use the SPI signals on the
>     UEXT connector for something else, which again goes against the
>     policy we've had for basically any other board.

OK. Shall we back it out and figure out something else?

ChenYu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/5] dt-bindings: clock: Add Bitmain BM1880 SoC clock controller binding
From: Stephen Boyd @ 2019-08-08  5:01 UTC (permalink / raw)
  To: Manivannan Sadhasivam, mturquette, robh+dt
  Cc: devicetree, Manivannan Sadhasivam, darren.tsao, linux-kernel,
	linux-arm-kernel, fisher.cheng, alec.lin, linux-clk, haitao.suo
In-Reply-To: <20190705151440.20844-2-manivannan.sadhasivam@linaro.org>

Quoting Manivannan Sadhasivam (2019-07-05 08:14:36)
> Add devicetree binding for Bitmain BM1880 SoC clock controller.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  .../bindings/clock/bitmain,bm1880-clk.txt     | 47 +++++++++++

Can you convert this to YAML? It's all the rage right now.

>  include/dt-bindings/clock/bm1880-clock.h      | 82 +++++++++++++++++++
>  2 files changed, 129 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.txt
>  create mode 100644 include/dt-bindings/clock/bm1880-clock.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.txt b/Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.txt
> new file mode 100644
> index 000000000000..9c967095d430
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.txt
> @@ -0,0 +1,47 @@
> +* Bitmain BM1880 Clock Controller
> +
> +The Bitmain BM1880 clock controler generates and supplies clock to
> +various peripherals within the SoC.
> +
> +Required Properties:
> +
> +- compatible: Should be "bitmain,bm1880-clk"
> +- reg :        Register address and size of PLL and SYS control domains
> +- reg-names : Register domain names: "pll" and "sys"
> +- clocks : Phandle of the input reference clock.
> +- #clock-cells: Should be 1.
> +
> +Each clock is assigned an identifier, and client nodes can use this identifier
> +to specify the clock which they consume.
> +
> +All available clocks are defined as preprocessor macros in corresponding
> +dt-bindings/clock/bm1880-clock.h header and can be used in device tree sources.
> +
> +External clocks:
> +
> +The osc clock used as the input for the plls is generated outside the SoC.
> +It is expected that it is defined using standard clock bindings as "osc".
> +
> +Example: 
> +
> +        clk: clock-controller@800 {
> +                compatible = "bitmain,bm1880-clk";
> +                reg = <0xe8 0x0c>,<0x800 0xb0>;

It looks weird still. What hardware module is this actually part of?
Some larger power manager block?

> +                reg-names = "pll", "sys";
> +                clocks = <&osc>;
> +                #clock-cells = <1>;
> +        };
> +

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64/cache: silence -Woverride-init warnings
From: Nathan Chancellor @ 2019-08-08  4:50 UTC (permalink / raw)
  To: Qian Cai
  Cc: mark.rutland, catalin.marinas, linux-kernel, clang-built-linux,
	will, linux-arm-kernel
In-Reply-To: <20190808032916.879-1-cai@lca.pw>

On Wed, Aug 07, 2019 at 11:29:16PM -0400, Qian Cai wrote:
> The commit 155433cb365e ("arm64: cache: Remove support for ASID-tagged
> VIVT I-caches") introduced some compiation warnings from GCC (and
> Clang) with -Winitializer-overrides),
> 
> arch/arm64/kernel/cpuinfo.c:38:26: warning: initialized field
> overwritten [-Woverride-init]
> [ICACHE_POLICY_VIPT]  = "VIPT",
>                         ^~~~~~
> arch/arm64/kernel/cpuinfo.c:38:26: note: (near initialization for
> 'icache_policy_str[2]')
> arch/arm64/kernel/cpuinfo.c:39:26: warning: initialized field
> overwritten [-Woverride-init]
> [ICACHE_POLICY_PIPT]  = "PIPT",
>                         ^~~~~~
> arch/arm64/kernel/cpuinfo.c:39:26: note: (near initialization for
> 'icache_policy_str[3]')
> arch/arm64/kernel/cpuinfo.c:40:27: warning: initialized field
> overwritten [-Woverride-init]
> [ICACHE_POLICY_VPIPT]  = "VPIPT",
>                          ^~~~~~~
> arch/arm64/kernel/cpuinfo.c:40:27: note: (near initialization for
> 'icache_policy_str[0]')
> 
> because it initializes icache_policy_str[0 ... 3] twice. Since
> arm64 developers are keen to keep the style of initializing a static
> array with a non-zero pattern first, just disable those warnings for
> both GCC and Clang of this file.
> 
> Fixes: 155433cb365e ("arm64: cache: Remove support for ASID-tagged VIVT I-caches")
> Signed-off-by: Qian Cai <cai@lca.pw>

It's a shame we can't just use one cc-disable-warning statement but
-Woverride-init wasn't added for GCC compatibility until clang 8.0.0
and we don't have an established minimum clang version.

With that said, I applied your patch and I don't see with warning with
W=1 anymore and I see both options get added to the clang command line
with V=1.

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

Cheers!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 1/4] clk: core: introduce clk_hw_set_parent()
From: Stephen Boyd @ 2019-08-08  4:47 UTC (permalink / raw)
  To: Jerome Brunet, Neil Armstrong
  Cc: Neil Armstrong, Martin Blumenstingl, linux-kernel, linux-amlogic,
	linux-clk, linux-arm-kernel
In-Reply-To: <1j36iewvdo.fsf@starbuckisacylon.baylibre.com>

Quoting Jerome Brunet (2019-08-06 01:28:19)
> On Wed 31 Jul 2019 at 10:40, Neil Armstrong <narmstrong@baylibre.com> wrote:
> 
> > Introduce the clk_hw_set_parent() provider call to change parent of
> > a clock by using the clk_hw pointers.
> >
> > This eases the clock reparenting from clock rate notifiers and
> > implementing DVFS with simpler code avoiding the boilerplates
> > functions as __clk_lookup(clk_hw_get_name()) then clk_set_parent().
> >
> > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> > Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> 
> Looks ok to me but we will obviously need Stephen's ack to apply it

Acked-by: Stephen Boyd <sboyd@kernel.org>

> 
> > ---
> >  drivers/clk/clk.c            | 6 ++++++
> >  include/linux/clk-provider.h | 1 +
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index c0990703ce54..c11b1781d24a 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -2487,6 +2487,12 @@ static int clk_core_set_parent_nolock(struct clk_core *core,
> >       return ret;
> >  }
> >  
> > +int clk_hw_set_parent(struct clk_hw *hw, struct clk_hw *parent)
> > +{
> > +     return clk_core_set_parent_nolock(hw->core, parent->core);

I wonder if you really want to call all those things in
clk_core_set_parent_nolock(). For example, do you want notifiers to run
again and for rates to be speculated? Maybe all you want to do is
overwrite some value for the clk's parent by calling into the ops for
the clk and generically parse the value that's passed as the "parent"
here.

I ask because it may be good to avoid doing all that work and updating
bookkeeping when we're deep in a notifier. If we can clearly understand
what the driver wants to do from the notifier then it's simpler to
change in the future to use things such as the coordinated clk rate
vaporware.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC/RFT v3 07/14] clk: meson: g12a: add notifiers to handle cpu clock change
From: Stephen Boyd @ 2019-08-08  4:43 UTC (permalink / raw)
  To: Martin Blumenstingl, Neil Armstrong, dbasehore
  Cc: khilman, linux-kernel, linux-gpio, linux-amlogic, linux-clk,
	linux-arm-kernel, jbrunet
In-Reply-To: <CAFBinCAnKeGYkyCmEMugWuQaSxgBp4DqtHN3b0rLJY6jwOF0QA@mail.gmail.com>

Quoting Martin Blumenstingl (2019-07-02 16:28:55)
> Hi Stephen, Hi Neil,
> 
> On Mon, Jul 1, 2019 at 11:13 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
> >
> > In order to implement clock switching for the CLKID_CPU_CLK and
> > CLKID_CPUB_CLK, notifiers are added on specific points of the
> > clock tree :
> >
> > cpu_clk / cpub_clk
> > |   \- cpu_clk_dyn
> > |      |  \- cpu_clk_premux0
> > |      |        |- cpu_clk_postmux0
> > |      |        |    |- cpu_clk_dyn0_div
> > |      |        |    \- xtal/fclk_div2/fclk_div3
> > |      |        \- xtal/fclk_div2/fclk_div3
> > |      \- cpu_clk_premux1
> > |            |- cpu_clk_postmux1
> > |            |    |- cpu_clk_dyn1_div
> > |            |    \- xtal/fclk_div2/fclk_div3
> > |            \- xtal/fclk_div2/fclk_div3
> > \ sys_pll / sys1_pll
> >
> > This for each cluster, a single one for G12A, two for G12B.
> >
> > Each cpu_clk_premux1 tree is marked as read-only and CLK_SET_RATE_NO_REPARENT,
> > to be used as "parking" clock in a safe clock frequency.
> it seems that this is one case where the "coordinated clocks" feature
> would come handy: [0]
> Stephen, do you know if those patches stopped in March or if there's
> still some ongoing effort to get them ready?
> 

Derek told me yesterday he wants to work on it again, but I don't know
his timeline. If Derek doesn't reply here then maybe it can be picked up
by someone else.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox