* [PATCH v2 02/14] arm64, hibernate: create_safe_exec_page cleanup
From: Pavel Tatashin @ 2019-08-17 2:46 UTC (permalink / raw)
To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190817024629.26611-1-pasha.tatashin@soleen.com>
create_safe_exec_page() is going to be split into two parts in preparation
of moving page table handling code out of hibernate.c
Remove allocator parameter, and rename dst to page. Also, remove the
goto's, as we can return directly without cleanups.
Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/kernel/hibernate.c | 60 +++++++++++++++--------------------
1 file changed, 26 insertions(+), 34 deletions(-)
diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index 9341fcc6e809..96b6f8da7e49 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -196,57 +196,51 @@ EXPORT_SYMBOL(arch_hibernation_header_restore);
*/
static int create_safe_exec_page(void *src_start, size_t length,
unsigned long dst_addr,
- phys_addr_t *phys_dst_addr,
- void *(*allocator)(gfp_t mask),
- gfp_t mask)
+ phys_addr_t *phys_dst_addr)
{
- int rc = 0;
+ void *page = (void *)get_safe_page(GFP_ATOMIC);
+ pgd_t *trans_table;
pgd_t *pgdp;
pud_t *pudp;
pmd_t *pmdp;
pte_t *ptep;
- unsigned long dst = (unsigned long)allocator(mask);
- if (!dst) {
- rc = -ENOMEM;
- goto out;
- }
+ if (!page)
+ return -ENOMEM;
+
+ memcpy((void *)page, src_start, length);
+ __flush_icache_range((unsigned long)page, (unsigned long)page + length);
- memcpy((void *)dst, src_start, length);
- __flush_icache_range(dst, dst + length);
+ trans_table = (void *)get_safe_page(GFP_ATOMIC);
+ if (!trans_table)
+ return -ENOMEM;
- pgdp = pgd_offset_raw(allocator(mask), dst_addr);
+ pgdp = pgd_offset_raw(trans_table, dst_addr);
if (pgd_none(READ_ONCE(*pgdp))) {
- pudp = allocator(mask);
- if (!pudp) {
- rc = -ENOMEM;
- goto out;
- }
+ pudp = (void *)get_safe_page(GFP_ATOMIC);
+ if (!pudp)
+ return -ENOMEM;
pgd_populate(&init_mm, pgdp, pudp);
}
pudp = pud_offset(pgdp, dst_addr);
if (pud_none(READ_ONCE(*pudp))) {
- pmdp = allocator(mask);
- if (!pmdp) {
- rc = -ENOMEM;
- goto out;
- }
+ pmdp = (void *)get_safe_page(GFP_ATOMIC);
+ if (!pmdp)
+ return -ENOMEM;
pud_populate(&init_mm, pudp, pmdp);
}
pmdp = pmd_offset(pudp, dst_addr);
if (pmd_none(READ_ONCE(*pmdp))) {
- ptep = allocator(mask);
- if (!ptep) {
- rc = -ENOMEM;
- goto out;
- }
+ ptep = (void *)get_safe_page(GFP_ATOMIC);
+ if (!ptep)
+ return -ENOMEM;
pmd_populate_kernel(&init_mm, pmdp, ptep);
}
ptep = pte_offset_kernel(pmdp, dst_addr);
- set_pte(ptep, pfn_pte(virt_to_pfn(dst), PAGE_KERNEL_EXEC));
+ set_pte(ptep, pfn_pte(virt_to_pfn(page), PAGE_KERNEL_EXEC));
/*
* Load our new page tables. A strict BBM approach requires that we
@@ -262,13 +256,12 @@ static int create_safe_exec_page(void *src_start, size_t length,
*/
cpu_set_reserved_ttbr0();
local_flush_tlb_all();
- write_sysreg(phys_to_ttbr(virt_to_phys(pgdp)), ttbr0_el1);
+ write_sysreg(phys_to_ttbr(virt_to_phys(trans_table)), ttbr0_el1);
isb();
- *phys_dst_addr = virt_to_phys((void *)dst);
+ *phys_dst_addr = virt_to_phys((void *)page);
-out:
- return rc;
+ return 0;
}
#define dcache_clean_range(start, end) __flush_dcache_area(start, (end - start))
@@ -523,8 +516,7 @@ int swsusp_arch_resume(void)
*/
rc = create_safe_exec_page(__hibernate_exit_text_start, exit_size,
(unsigned long)hibernate_exit,
- &phys_hibernate_exit,
- (void *)get_safe_page, GFP_ATOMIC);
+ &phys_hibernate_exit);
if (rc) {
pr_err("Failed to create safe executable page for hibernate_exit code.\n");
goto out;
--
2.22.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 v2 01/14] kexec: quiet down kexec reboot
From: Pavel Tatashin @ 2019-08-17 2:46 UTC (permalink / raw)
To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190817024629.26611-1-pasha.tatashin@soleen.com>
Here is a regular kexec command sequence and output:
=====
$ kexec --reuse-cmdline -i --load Image
$ kexec -e
[ 161.342002] kexec_core: Starting new kernel
Welcome to Buildroot
buildroot login:
=====
Even when "quiet" kernel parameter is specified, "kexec_core: Starting
new kernel" is printed.
This message has KERN_EMERG level, but there is no emergency, it is a
normal kexec operation, so quiet it down to appropriate KERN_NOTICE.
Machines that have slow console baud rate benefit from less output.
Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
Reviewed-by: Simon Horman <horms@verge.net.au>
---
kernel/kexec_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index d5870723b8ad..2c5b72863b7b 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1169,7 +1169,7 @@ int kernel_kexec(void)
* CPU hotplug again; so re-enable it here.
*/
cpu_hotplug_enable();
- pr_emerg("Starting new kernel\n");
+ pr_notice("Starting new kernel\n");
machine_shutdown();
}
--
2.22.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 v2 03/14] arm64, hibernate: add trans_table public functions
From: Pavel Tatashin @ 2019-08-17 2:46 UTC (permalink / raw)
To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190817024629.26611-1-pasha.tatashin@soleen.com>
trans_table_create_copy() and trans_table_map_page() are going to be
the basis for public interface of new subsystem that handles page
tables for cases which are between kernels: kexec, and hibernate.
Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/kernel/hibernate.c | 96 ++++++++++++++++++++++-------------
1 file changed, 61 insertions(+), 35 deletions(-)
diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index 96b6f8da7e49..449d69b5651c 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -182,39 +182,15 @@ int arch_hibernation_header_restore(void *addr)
}
EXPORT_SYMBOL(arch_hibernation_header_restore);
-/*
- * Copies length bytes, starting at src_start into an new page,
- * perform cache maintentance, then maps it at the specified address low
- * address as executable.
- *
- * This is used by hibernate to copy the code it needs to execute when
- * overwriting the kernel text. This function generates a new set of page
- * tables, which it loads into ttbr0.
- *
- * Length is provided as we probably only want 4K of data, even on a 64K
- * page system.
- */
-static int create_safe_exec_page(void *src_start, size_t length,
- unsigned long dst_addr,
- phys_addr_t *phys_dst_addr)
+int trans_table_map_page(pgd_t *trans_table, void *page,
+ unsigned long dst_addr,
+ pgprot_t pgprot)
{
- void *page = (void *)get_safe_page(GFP_ATOMIC);
- pgd_t *trans_table;
pgd_t *pgdp;
pud_t *pudp;
pmd_t *pmdp;
pte_t *ptep;
- if (!page)
- return -ENOMEM;
-
- memcpy((void *)page, src_start, length);
- __flush_icache_range((unsigned long)page, (unsigned long)page + length);
-
- trans_table = (void *)get_safe_page(GFP_ATOMIC);
- if (!trans_table)
- return -ENOMEM;
-
pgdp = pgd_offset_raw(trans_table, dst_addr);
if (pgd_none(READ_ONCE(*pgdp))) {
pudp = (void *)get_safe_page(GFP_ATOMIC);
@@ -242,6 +218,44 @@ static int create_safe_exec_page(void *src_start, size_t length,
ptep = pte_offset_kernel(pmdp, dst_addr);
set_pte(ptep, pfn_pte(virt_to_pfn(page), PAGE_KERNEL_EXEC));
+ return 0;
+}
+
+/*
+ * Copies length bytes, starting at src_start into an new page,
+ * perform cache maintentance, then maps it at the specified address low
+ * address as executable.
+ *
+ * This is used by hibernate to copy the code it needs to execute when
+ * overwriting the kernel text. This function generates a new set of page
+ * tables, which it loads into ttbr0.
+ *
+ * Length is provided as we probably only want 4K of data, even on a 64K
+ * page system.
+ */
+static int create_safe_exec_page(void *src_start, size_t length,
+ unsigned long dst_addr,
+ phys_addr_t *phys_dst_addr)
+{
+ void *page = (void *)get_safe_page(GFP_ATOMIC);
+ pgd_t *trans_table;
+ int rc;
+
+ if (!page)
+ return -ENOMEM;
+
+ memcpy(page, src_start, length);
+ __flush_icache_range((unsigned long)page, (unsigned long)page + length);
+
+ trans_table = (void *)get_safe_page(GFP_ATOMIC);
+ if (!trans_table)
+ return -ENOMEM;
+
+ rc = trans_table_map_page(trans_table, page, dst_addr,
+ PAGE_KERNEL_EXEC);
+ if (rc)
+ return rc;
+
/*
* Load our new page tables. A strict BBM approach requires that we
* ensure that TLBs are free of any entries that may overlap with the
@@ -259,7 +273,7 @@ static int create_safe_exec_page(void *src_start, size_t length,
write_sysreg(phys_to_ttbr(virt_to_phys(trans_table)), ttbr0_el1);
isb();
- *phys_dst_addr = virt_to_phys((void *)page);
+ *phys_dst_addr = virt_to_phys(page);
return 0;
}
@@ -462,6 +476,24 @@ static int copy_page_tables(pgd_t *dst_pgdp, unsigned long start,
return 0;
}
+int trans_table_create_copy(pgd_t **dst_pgdp, unsigned long start,
+ unsigned long end)
+{
+ int rc;
+ pgd_t *trans_table = (pgd_t *)get_safe_page(GFP_ATOMIC);
+
+ if (!trans_table) {
+ pr_err("Failed to allocate memory for temporary page tables.\n");
+ return -ENOMEM;
+ }
+
+ rc = copy_page_tables(trans_table, start, end);
+ if (!rc)
+ *dst_pgdp = trans_table;
+
+ return rc;
+}
+
/*
* Setup then Resume from the hibernate image using swsusp_arch_suspend_exit().
*
@@ -483,13 +515,7 @@ int swsusp_arch_resume(void)
* Create a second copy of just the linear map, and use this when
* restoring.
*/
- tmp_pg_dir = (pgd_t *)get_safe_page(GFP_ATOMIC);
- if (!tmp_pg_dir) {
- pr_err("Failed to allocate memory for temporary page tables.\n");
- rc = -ENOMEM;
- goto out;
- }
- rc = copy_page_tables(tmp_pg_dir, PAGE_OFFSET, 0);
+ rc = trans_table_create_copy(&tmp_pg_dir, PAGE_OFFSET, 0);
if (rc)
goto out;
--
2.22.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 v2 06/14] arm64, trans_table: add trans_table_create_empty
From: Pavel Tatashin @ 2019-08-17 2:46 UTC (permalink / raw)
To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190817024629.26611-1-pasha.tatashin@soleen.com>
This functions returns a zeroed trans_table using the allocator that is
specified in the info argument.
trans_tables should be created by using this function.
Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/include/asm/trans_table.h | 4 ++++
arch/arm64/kernel/hibernate.c | 6 +++---
arch/arm64/mm/trans_table.c | 12 ++++++++++++
3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/trans_table.h b/arch/arm64/include/asm/trans_table.h
index 1a57af09ded5..02d3a0333dc9 100644
--- a/arch/arm64/include/asm/trans_table.h
+++ b/arch/arm64/include/asm/trans_table.h
@@ -40,6 +40,10 @@ struct trans_table_info {
unsigned long trans_flags;
};
+/* Create and empty trans table. */
+int trans_table_create_empty(struct trans_table_info *info,
+ pgd_t **trans_table);
+
int trans_table_create_copy(pgd_t **dst_pgdp, unsigned long start,
unsigned long end);
diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index 524b68ec3233..3a7b362e5a58 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -216,9 +216,9 @@ static int create_safe_exec_page(void *src_start, size_t length,
memcpy(page, src_start, length);
__flush_icache_range((unsigned long)page, (unsigned long)page + length);
- trans_table = (void *)get_safe_page(GFP_ATOMIC);
- if (!trans_table)
- return -ENOMEM;
+ rc = trans_table_create_empty(&trans_info, &trans_table);
+ if (rc)
+ return rc;
rc = trans_table_map_page(&trans_info, trans_table, page, dst_addr,
PAGE_KERNEL_EXEC);
diff --git a/arch/arm64/mm/trans_table.c b/arch/arm64/mm/trans_table.c
index 12f4b3cab6d6..6deb35f83118 100644
--- a/arch/arm64/mm/trans_table.c
+++ b/arch/arm64/mm/trans_table.c
@@ -164,6 +164,18 @@ static int copy_page_tables(pgd_t *dst_pgdp, unsigned long start,
return 0;
}
+int trans_table_create_empty(struct trans_table_info *info, pgd_t **trans_table)
+{
+ pgd_t *dst_pgdp = trans_alloc(info);
+
+ if (!dst_pgdp)
+ return -ENOMEM;
+
+ *trans_table = dst_pgdp;
+
+ return 0;
+}
+
int trans_table_create_copy(pgd_t **dst_pgdp, unsigned long start,
unsigned long end)
{
--
2.22.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 v2 04/14] arm64, hibernate: move page handling function to new trans_table.c
From: Pavel Tatashin @ 2019-08-17 2:46 UTC (permalink / raw)
To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190817024629.26611-1-pasha.tatashin@soleen.com>
Now, that we abstracted the required functions move them to a new home.
Later, we will generalize these function in order to be useful outside
of hibernation.
Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/Kconfig | 4 +
arch/arm64/include/asm/trans_table.h | 21 +++
arch/arm64/kernel/hibernate.c | 199 +------------------------
arch/arm64/mm/Makefile | 1 +
arch/arm64/mm/trans_table.c | 212 +++++++++++++++++++++++++++
5 files changed, 239 insertions(+), 198 deletions(-)
create mode 100644 arch/arm64/include/asm/trans_table.h
create mode 100644 arch/arm64/mm/trans_table.c
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 3adcec05b1f6..91a7416ffe4e 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -999,6 +999,10 @@ config CRASH_DUMP
For more details see Documentation/admin-guide/kdump/kdump.rst
+config TRANS_TABLE
+ def_bool y
+ depends on HIBERNATION || KEXEC_CORE
+
config XEN_DOM0
def_bool y
depends on XEN
diff --git a/arch/arm64/include/asm/trans_table.h b/arch/arm64/include/asm/trans_table.h
new file mode 100644
index 000000000000..f57b2ab2a0b8
--- /dev/null
+++ b/arch/arm64/include/asm/trans_table.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Copyright (c) 2019, Microsoft Corporation.
+ * Pavel Tatashin <patatash@linux.microsoft.com>
+ */
+
+#ifndef _ASM_TRANS_TABLE_H
+#define _ASM_TRANS_TABLE_H
+
+#include <linux/bits.h>
+#include <asm/pgtable-types.h>
+
+int trans_table_create_copy(pgd_t **dst_pgdp, unsigned long start,
+ unsigned long end);
+
+int trans_table_map_page(pgd_t *trans_table, void *page,
+ unsigned long dst_addr,
+ pgprot_t pgprot);
+
+#endif /* _ASM_TRANS_TABLE_H */
diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index 449d69b5651c..0cb858b3f503 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -16,7 +16,6 @@
#define pr_fmt(x) "hibernate: " x
#include <linux/cpu.h>
#include <linux/kvm_host.h>
-#include <linux/mm.h>
#include <linux/pm.h>
#include <linux/sched.h>
#include <linux/suspend.h>
@@ -31,14 +30,12 @@
#include <asm/kexec.h>
#include <asm/memory.h>
#include <asm/mmu_context.h>
-#include <asm/pgalloc.h>
-#include <asm/pgtable.h>
-#include <asm/pgtable-hwdef.h>
#include <asm/sections.h>
#include <asm/smp.h>
#include <asm/smp_plat.h>
#include <asm/suspend.h>
#include <asm/sysreg.h>
+#include <asm/trans_table.h>
#include <asm/virt.h>
/*
@@ -182,45 +179,6 @@ int arch_hibernation_header_restore(void *addr)
}
EXPORT_SYMBOL(arch_hibernation_header_restore);
-int trans_table_map_page(pgd_t *trans_table, void *page,
- unsigned long dst_addr,
- pgprot_t pgprot)
-{
- pgd_t *pgdp;
- pud_t *pudp;
- pmd_t *pmdp;
- pte_t *ptep;
-
- pgdp = pgd_offset_raw(trans_table, dst_addr);
- if (pgd_none(READ_ONCE(*pgdp))) {
- pudp = (void *)get_safe_page(GFP_ATOMIC);
- if (!pudp)
- return -ENOMEM;
- pgd_populate(&init_mm, pgdp, pudp);
- }
-
- pudp = pud_offset(pgdp, dst_addr);
- if (pud_none(READ_ONCE(*pudp))) {
- pmdp = (void *)get_safe_page(GFP_ATOMIC);
- if (!pmdp)
- return -ENOMEM;
- pud_populate(&init_mm, pudp, pmdp);
- }
-
- pmdp = pmd_offset(pudp, dst_addr);
- if (pmd_none(READ_ONCE(*pmdp))) {
- ptep = (void *)get_safe_page(GFP_ATOMIC);
- if (!ptep)
- return -ENOMEM;
- pmd_populate_kernel(&init_mm, pmdp, ptep);
- }
-
- ptep = pte_offset_kernel(pmdp, dst_addr);
- set_pte(ptep, pfn_pte(virt_to_pfn(page), PAGE_KERNEL_EXEC));
-
- return 0;
-}
-
/*
* Copies length bytes, starting at src_start into an new page,
* perform cache maintentance, then maps it at the specified address low
@@ -339,161 +297,6 @@ int swsusp_arch_suspend(void)
return ret;
}
-static void _copy_pte(pte_t *dst_ptep, pte_t *src_ptep, unsigned long addr)
-{
- pte_t pte = READ_ONCE(*src_ptep);
-
- if (pte_valid(pte)) {
- /*
- * Resume will overwrite areas that may be marked
- * read only (code, rodata). Clear the RDONLY bit from
- * the temporary mappings we use during restore.
- */
- set_pte(dst_ptep, pte_mkwrite(pte));
- } else if (debug_pagealloc_enabled() && !pte_none(pte)) {
- /*
- * debug_pagealloc will removed the PTE_VALID bit if
- * the page isn't in use by the resume kernel. It may have
- * been in use by the original kernel, in which case we need
- * to put it back in our copy to do the restore.
- *
- * Before marking this entry valid, check the pfn should
- * be mapped.
- */
- BUG_ON(!pfn_valid(pte_pfn(pte)));
-
- set_pte(dst_ptep, pte_mkpresent(pte_mkwrite(pte)));
- }
-}
-
-static int copy_pte(pmd_t *dst_pmdp, pmd_t *src_pmdp, unsigned long start,
- unsigned long end)
-{
- pte_t *src_ptep;
- pte_t *dst_ptep;
- unsigned long addr = start;
-
- dst_ptep = (pte_t *)get_safe_page(GFP_ATOMIC);
- if (!dst_ptep)
- return -ENOMEM;
- pmd_populate_kernel(&init_mm, dst_pmdp, dst_ptep);
- dst_ptep = pte_offset_kernel(dst_pmdp, start);
-
- src_ptep = pte_offset_kernel(src_pmdp, start);
- do {
- _copy_pte(dst_ptep, src_ptep, addr);
- } while (dst_ptep++, src_ptep++, addr += PAGE_SIZE, addr != end);
-
- return 0;
-}
-
-static int copy_pmd(pud_t *dst_pudp, pud_t *src_pudp, unsigned long start,
- unsigned long end)
-{
- pmd_t *src_pmdp;
- pmd_t *dst_pmdp;
- unsigned long next;
- unsigned long addr = start;
-
- if (pud_none(READ_ONCE(*dst_pudp))) {
- dst_pmdp = (pmd_t *)get_safe_page(GFP_ATOMIC);
- if (!dst_pmdp)
- return -ENOMEM;
- pud_populate(&init_mm, dst_pudp, dst_pmdp);
- }
- dst_pmdp = pmd_offset(dst_pudp, start);
-
- src_pmdp = pmd_offset(src_pudp, start);
- do {
- pmd_t pmd = READ_ONCE(*src_pmdp);
-
- next = pmd_addr_end(addr, end);
- if (pmd_none(pmd))
- continue;
- if (pmd_table(pmd)) {
- if (copy_pte(dst_pmdp, src_pmdp, addr, next))
- return -ENOMEM;
- } else {
- set_pmd(dst_pmdp,
- __pmd(pmd_val(pmd) & ~PMD_SECT_RDONLY));
- }
- } while (dst_pmdp++, src_pmdp++, addr = next, addr != end);
-
- return 0;
-}
-
-static int copy_pud(pgd_t *dst_pgdp, pgd_t *src_pgdp, unsigned long start,
- unsigned long end)
-{
- pud_t *dst_pudp;
- pud_t *src_pudp;
- unsigned long next;
- unsigned long addr = start;
-
- if (pgd_none(READ_ONCE(*dst_pgdp))) {
- dst_pudp = (pud_t *)get_safe_page(GFP_ATOMIC);
- if (!dst_pudp)
- return -ENOMEM;
- pgd_populate(&init_mm, dst_pgdp, dst_pudp);
- }
- dst_pudp = pud_offset(dst_pgdp, start);
-
- src_pudp = pud_offset(src_pgdp, start);
- do {
- pud_t pud = READ_ONCE(*src_pudp);
-
- next = pud_addr_end(addr, end);
- if (pud_none(pud))
- continue;
- if (pud_table(pud)) {
- if (copy_pmd(dst_pudp, src_pudp, addr, next))
- return -ENOMEM;
- } else {
- set_pud(dst_pudp,
- __pud(pud_val(pud) & ~PMD_SECT_RDONLY));
- }
- } while (dst_pudp++, src_pudp++, addr = next, addr != end);
-
- return 0;
-}
-
-static int copy_page_tables(pgd_t *dst_pgdp, unsigned long start,
- unsigned long end)
-{
- unsigned long next;
- unsigned long addr = start;
- pgd_t *src_pgdp = pgd_offset_k(start);
-
- dst_pgdp = pgd_offset_raw(dst_pgdp, start);
- do {
- next = pgd_addr_end(addr, end);
- if (pgd_none(READ_ONCE(*src_pgdp)))
- continue;
- if (copy_pud(dst_pgdp, src_pgdp, addr, next))
- return -ENOMEM;
- } while (dst_pgdp++, src_pgdp++, addr = next, addr != end);
-
- return 0;
-}
-
-int trans_table_create_copy(pgd_t **dst_pgdp, unsigned long start,
- unsigned long end)
-{
- int rc;
- pgd_t *trans_table = (pgd_t *)get_safe_page(GFP_ATOMIC);
-
- if (!trans_table) {
- pr_err("Failed to allocate memory for temporary page tables.\n");
- return -ENOMEM;
- }
-
- rc = copy_page_tables(trans_table, start, end);
- if (!rc)
- *dst_pgdp = trans_table;
-
- return rc;
-}
-
/*
* Setup then Resume from the hibernate image using swsusp_arch_suspend_exit().
*
diff --git a/arch/arm64/mm/Makefile b/arch/arm64/mm/Makefile
index 849c1df3d214..3794fff18659 100644
--- a/arch/arm64/mm/Makefile
+++ b/arch/arm64/mm/Makefile
@@ -6,6 +6,7 @@ obj-y := dma-mapping.o extable.o fault.o init.o \
obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o
obj-$(CONFIG_ARM64_PTDUMP_CORE) += dump.o
obj-$(CONFIG_ARM64_PTDUMP_DEBUGFS) += ptdump_debugfs.o
+obj-$(CONFIG_TRANS_TABLE) += trans_table.o
obj-$(CONFIG_NUMA) += numa.o
obj-$(CONFIG_DEBUG_VIRTUAL) += physaddr.o
KASAN_SANITIZE_physaddr.o += n
diff --git a/arch/arm64/mm/trans_table.c b/arch/arm64/mm/trans_table.c
new file mode 100644
index 000000000000..b4bbb559d9cf
--- /dev/null
+++ b/arch/arm64/mm/trans_table.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (c) 2019, Microsoft Corporation.
+ * Pavel Tatashin <patatash@linux.microsoft.com>
+ */
+
+/*
+ * Transitional tables are used during system transferring from one world to
+ * another: such as during hibernate restore, and kexec reboots. During these
+ * phases one cannot rely on page table not being overwritten.
+ *
+ */
+
+#include <asm/trans_table.h>
+#include <asm/pgalloc.h>
+#include <asm/pgtable.h>
+#include <linux/suspend.h>
+
+static void _copy_pte(pte_t *dst_ptep, pte_t *src_ptep, unsigned long addr)
+{
+ pte_t pte = READ_ONCE(*src_ptep);
+
+ if (pte_valid(pte)) {
+ /*
+ * Resume will overwrite areas that may be marked
+ * read only (code, rodata). Clear the RDONLY bit from
+ * the temporary mappings we use during restore.
+ */
+ set_pte(dst_ptep, pte_mkwrite(pte));
+ } else if (debug_pagealloc_enabled() && !pte_none(pte)) {
+ /*
+ * debug_pagealloc will removed the PTE_VALID bit if
+ * the page isn't in use by the resume kernel. It may have
+ * been in use by the original kernel, in which case we need
+ * to put it back in our copy to do the restore.
+ *
+ * Before marking this entry valid, check the pfn should
+ * be mapped.
+ */
+ BUG_ON(!pfn_valid(pte_pfn(pte)));
+
+ set_pte(dst_ptep, pte_mkpresent(pte_mkwrite(pte)));
+ }
+}
+
+static int copy_pte(pmd_t *dst_pmdp, pmd_t *src_pmdp, unsigned long start,
+ unsigned long end)
+{
+ pte_t *src_ptep;
+ pte_t *dst_ptep;
+ unsigned long addr = start;
+
+ dst_ptep = (pte_t *)get_safe_page(GFP_ATOMIC);
+ if (!dst_ptep)
+ return -ENOMEM;
+ pmd_populate_kernel(&init_mm, dst_pmdp, dst_ptep);
+ dst_ptep = pte_offset_kernel(dst_pmdp, start);
+
+ src_ptep = pte_offset_kernel(src_pmdp, start);
+ do {
+ _copy_pte(dst_ptep, src_ptep, addr);
+ } while (dst_ptep++, src_ptep++, addr += PAGE_SIZE, addr != end);
+
+ return 0;
+}
+
+static int copy_pmd(pud_t *dst_pudp, pud_t *src_pudp, unsigned long start,
+ unsigned long end)
+{
+ pmd_t *src_pmdp;
+ pmd_t *dst_pmdp;
+ unsigned long next;
+ unsigned long addr = start;
+
+ if (pud_none(READ_ONCE(*dst_pudp))) {
+ dst_pmdp = (pmd_t *)get_safe_page(GFP_ATOMIC);
+ if (!dst_pmdp)
+ return -ENOMEM;
+ pud_populate(&init_mm, dst_pudp, dst_pmdp);
+ }
+ dst_pmdp = pmd_offset(dst_pudp, start);
+
+ src_pmdp = pmd_offset(src_pudp, start);
+ do {
+ pmd_t pmd = READ_ONCE(*src_pmdp);
+
+ next = pmd_addr_end(addr, end);
+ if (pmd_none(pmd))
+ continue;
+ if (pmd_table(pmd)) {
+ if (copy_pte(dst_pmdp, src_pmdp, addr, next))
+ return -ENOMEM;
+ } else {
+ set_pmd(dst_pmdp,
+ __pmd(pmd_val(pmd) & ~PMD_SECT_RDONLY));
+ }
+ } while (dst_pmdp++, src_pmdp++, addr = next, addr != end);
+
+ return 0;
+}
+
+static int copy_pud(pgd_t *dst_pgdp, pgd_t *src_pgdp, unsigned long start,
+ unsigned long end)
+{
+ pud_t *dst_pudp;
+ pud_t *src_pudp;
+ unsigned long next;
+ unsigned long addr = start;
+
+ if (pgd_none(READ_ONCE(*dst_pgdp))) {
+ dst_pudp = (pud_t *)get_safe_page(GFP_ATOMIC);
+ if (!dst_pudp)
+ return -ENOMEM;
+ pgd_populate(&init_mm, dst_pgdp, dst_pudp);
+ }
+ dst_pudp = pud_offset(dst_pgdp, start);
+
+ src_pudp = pud_offset(src_pgdp, start);
+ do {
+ pud_t pud = READ_ONCE(*src_pudp);
+
+ next = pud_addr_end(addr, end);
+ if (pud_none(pud))
+ continue;
+ if (pud_table(pud)) {
+ if (copy_pmd(dst_pudp, src_pudp, addr, next))
+ return -ENOMEM;
+ } else {
+ set_pud(dst_pudp,
+ __pud(pud_val(pud) & ~PMD_SECT_RDONLY));
+ }
+ } while (dst_pudp++, src_pudp++, addr = next, addr != end);
+
+ return 0;
+}
+
+static int copy_page_tables(pgd_t *dst_pgdp, unsigned long start,
+ unsigned long end)
+{
+ unsigned long next;
+ unsigned long addr = start;
+ pgd_t *src_pgdp = pgd_offset_k(start);
+
+ dst_pgdp = pgd_offset_raw(dst_pgdp, start);
+ do {
+ next = pgd_addr_end(addr, end);
+ if (pgd_none(READ_ONCE(*src_pgdp)))
+ continue;
+ if (copy_pud(dst_pgdp, src_pgdp, addr, next))
+ return -ENOMEM;
+ } while (dst_pgdp++, src_pgdp++, addr = next, addr != end);
+
+ return 0;
+}
+
+int trans_table_create_copy(pgd_t **dst_pgdp, unsigned long start,
+ unsigned long end)
+{
+ int rc;
+ pgd_t *trans_table = (pgd_t *)get_safe_page(GFP_ATOMIC);
+
+ if (!trans_table) {
+ pr_err("Failed to allocate memory for temporary page tables.\n");
+ return -ENOMEM;
+ }
+
+ rc = copy_page_tables(trans_table, start, end);
+ if (!rc)
+ *dst_pgdp = trans_table;
+
+ return rc;
+}
+
+int trans_table_map_page(pgd_t *trans_table, void *page,
+ unsigned long dst_addr,
+ pgprot_t pgprot)
+{
+ pgd_t *pgdp;
+ pud_t *pudp;
+ pmd_t *pmdp;
+ pte_t *ptep;
+
+ pgdp = pgd_offset_raw(trans_table, dst_addr);
+ if (pgd_none(READ_ONCE(*pgdp))) {
+ pudp = (void *)get_safe_page(GFP_ATOMIC);
+ if (!pudp)
+ return -ENOMEM;
+ pgd_populate(&init_mm, pgdp, pudp);
+ }
+
+ pudp = pud_offset(pgdp, dst_addr);
+ if (pud_none(READ_ONCE(*pudp))) {
+ pmdp = (void *)get_safe_page(GFP_ATOMIC);
+ if (!pmdp)
+ return -ENOMEM;
+ pud_populate(&init_mm, pudp, pmdp);
+ }
+
+ pmdp = pmd_offset(pudp, dst_addr);
+ if (pmd_none(READ_ONCE(*pmdp))) {
+ ptep = (void *)get_safe_page(GFP_ATOMIC);
+ if (!ptep)
+ return -ENOMEM;
+ pmd_populate_kernel(&init_mm, pmdp, ptep);
+ }
+
+ ptep = pte_offset_kernel(pmdp, dst_addr);
+ set_pte(ptep, pfn_pte(virt_to_pfn(page), PAGE_KERNEL_EXEC));
+
+ return 0;
+}
--
2.22.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 v2 05/14] arm64, trans_table: make trans_table_map_page generic
From: Pavel Tatashin @ 2019-08-17 2:46 UTC (permalink / raw)
To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190817024629.26611-1-pasha.tatashin@soleen.com>
Currently, trans_table_map_page has assumptions that are relevant to
hibernate. But, to make it generic we must allow it to use any allocator
and also, can't assume that entries do not exist in the page table
already. Also, we can't use init_mm here.
Also, add "flags" for trans_table_info, they are going to be used
in copy functions once they are generalized.
Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/include/asm/trans_table.h | 40 +++++++++++++-
arch/arm64/kernel/hibernate.c | 13 ++++-
arch/arm64/mm/trans_table.c | 83 +++++++++++++++++++---------
3 files changed, 107 insertions(+), 29 deletions(-)
diff --git a/arch/arm64/include/asm/trans_table.h b/arch/arm64/include/asm/trans_table.h
index f57b2ab2a0b8..1a57af09ded5 100644
--- a/arch/arm64/include/asm/trans_table.h
+++ b/arch/arm64/include/asm/trans_table.h
@@ -11,11 +11,45 @@
#include <linux/bits.h>
#include <asm/pgtable-types.h>
+/*
+ * trans_alloc_page
+ * - Allocator that should return exactly one uninitilaized page, if this
+ * allocator fails, trans_table returns -ENOMEM error.
+ *
+ * trans_alloc_arg
+ * - Passed to trans_alloc_page as an argument
+ *
+ * trans_flags
+ * - bitmap with flags that control how page table is filled.
+ * TRANS_MKWRITE: during page table copy make PTE, PME, and PUD page
+ * writeable by removing RDONLY flag from PTE.
+ * TRANS_MKVALID: during page table copy, if PTE present, but not valid,
+ * make it valid.
+ * TRANS_CHECKPFN: During page table copy, for every PTE entry check that
+ * PFN that this PTE points to is valid. Otherwise return
+ * -ENXIO
+ */
+
+#define TRANS_MKWRITE BIT(0)
+#define TRANS_MKVALID BIT(1)
+#define TRANS_CHECKPFN BIT(2)
+
+struct trans_table_info {
+ void * (*trans_alloc_page)(void *arg);
+ void *trans_alloc_arg;
+ unsigned long trans_flags;
+};
+
int trans_table_create_copy(pgd_t **dst_pgdp, unsigned long start,
unsigned long end);
-int trans_table_map_page(pgd_t *trans_table, void *page,
- unsigned long dst_addr,
- pgprot_t pgprot);
+/*
+ * Add map entry to trans_table for a base-size page at PTE level.
+ * page: page to be mapped.
+ * dst_addr: new VA address for the pages
+ * pgprot: protection for the page.
+ */
+int trans_table_map_page(struct trans_table_info *info, pgd_t *trans_table,
+ void *page, unsigned long dst_addr, pgprot_t pgprot);
#endif /* _ASM_TRANS_TABLE_H */
diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index 0cb858b3f503..524b68ec3233 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -179,6 +179,12 @@ int arch_hibernation_header_restore(void *addr)
}
EXPORT_SYMBOL(arch_hibernation_header_restore);
+static void *
+hibernate_page_alloc(void *arg)
+{
+ return (void *)get_safe_page((gfp_t)(unsigned long)arg);
+}
+
/*
* Copies length bytes, starting at src_start into an new page,
* perform cache maintentance, then maps it at the specified address low
@@ -195,6 +201,11 @@ static int create_safe_exec_page(void *src_start, size_t length,
unsigned long dst_addr,
phys_addr_t *phys_dst_addr)
{
+ struct trans_table_info trans_info = {
+ .trans_alloc_page = hibernate_page_alloc,
+ .trans_alloc_arg = (void *)GFP_ATOMIC,
+ .trans_flags = 0,
+ };
void *page = (void *)get_safe_page(GFP_ATOMIC);
pgd_t *trans_table;
int rc;
@@ -209,7 +220,7 @@ static int create_safe_exec_page(void *src_start, size_t length,
if (!trans_table)
return -ENOMEM;
- rc = trans_table_map_page(trans_table, page, dst_addr,
+ rc = trans_table_map_page(&trans_info, trans_table, page, dst_addr,
PAGE_KERNEL_EXEC);
if (rc)
return rc;
diff --git a/arch/arm64/mm/trans_table.c b/arch/arm64/mm/trans_table.c
index b4bbb559d9cf..12f4b3cab6d6 100644
--- a/arch/arm64/mm/trans_table.c
+++ b/arch/arm64/mm/trans_table.c
@@ -17,6 +17,16 @@
#include <asm/pgtable.h>
#include <linux/suspend.h>
+static void *trans_alloc(struct trans_table_info *info)
+{
+ void *page = info->trans_alloc_page(info->trans_alloc_arg);
+
+ if (page)
+ clear_page(page);
+
+ return page;
+}
+
static void _copy_pte(pte_t *dst_ptep, pte_t *src_ptep, unsigned long addr)
{
pte_t pte = READ_ONCE(*src_ptep);
@@ -172,41 +182,64 @@ int trans_table_create_copy(pgd_t **dst_pgdp, unsigned long start,
return rc;
}
-int trans_table_map_page(pgd_t *trans_table, void *page,
- unsigned long dst_addr,
- pgprot_t pgprot)
+int trans_table_map_page(struct trans_table_info *info, pgd_t *trans_table,
+ void *page, unsigned long dst_addr, pgprot_t pgprot)
{
- pgd_t *pgdp;
- pud_t *pudp;
- pmd_t *pmdp;
- pte_t *ptep;
-
- pgdp = pgd_offset_raw(trans_table, dst_addr);
- if (pgd_none(READ_ONCE(*pgdp))) {
- pudp = (void *)get_safe_page(GFP_ATOMIC);
- if (!pudp)
+ int pgd_idx = pgd_index(dst_addr);
+ int pud_idx = pud_index(dst_addr);
+ int pmd_idx = pmd_index(dst_addr);
+ int pte_idx = pte_index(dst_addr);
+ pgd_t *pgdp = trans_table;
+ pgd_t pgd = READ_ONCE(pgdp[pgd_idx]);
+ pud_t *pudp, pud;
+ pmd_t *pmdp, pmd;
+ pte_t *ptep, pte;
+
+ if (pgd_none(pgd)) {
+ pud_t *t = trans_alloc(info);
+
+ if (!t)
return -ENOMEM;
- pgd_populate(&init_mm, pgdp, pudp);
+
+ __pgd_populate(&pgdp[pgd_idx], __pa(t), PUD_TYPE_TABLE);
+ pgd = READ_ONCE(pgdp[pgd_idx]);
}
- pudp = pud_offset(pgdp, dst_addr);
- if (pud_none(READ_ONCE(*pudp))) {
- pmdp = (void *)get_safe_page(GFP_ATOMIC);
- if (!pmdp)
+ pudp = __va(pgd_page_paddr(pgd));
+ pud = READ_ONCE(pudp[pud_idx]);
+ if (pud_sect(pud)) {
+ return -ENXIO;
+ } else if (pud_none(pud) || pud_sect(pud)) {
+ pmd_t *t = trans_alloc(info);
+
+ if (!t)
return -ENOMEM;
- pud_populate(&init_mm, pudp, pmdp);
+
+ __pud_populate(&pudp[pud_idx], __pa(t), PMD_TYPE_TABLE);
+ pud = READ_ONCE(pudp[pud_idx]);
}
- pmdp = pmd_offset(pudp, dst_addr);
- if (pmd_none(READ_ONCE(*pmdp))) {
- ptep = (void *)get_safe_page(GFP_ATOMIC);
- if (!ptep)
+ pmdp = __va(pud_page_paddr(pud));
+ pmd = READ_ONCE(pmdp[pmd_idx]);
+ if (pmd_sect(pmd)) {
+ return -ENXIO;
+ } else if (pmd_none(pmd) || pmd_sect(pmd)) {
+ pte_t *t = trans_alloc(info);
+
+ if (!t)
return -ENOMEM;
- pmd_populate_kernel(&init_mm, pmdp, ptep);
+
+ __pmd_populate(&pmdp[pmd_idx], __pa(t), PTE_TYPE_PAGE);
+ pmd = READ_ONCE(pmdp[pmd_idx]);
}
- ptep = pte_offset_kernel(pmdp, dst_addr);
- set_pte(ptep, pfn_pte(virt_to_pfn(page), PAGE_KERNEL_EXEC));
+ ptep = __va(pmd_page_paddr(pmd));
+ pte = READ_ONCE(ptep[pte_idx]);
+
+ if (!pte_none(pte))
+ return -ENXIO;
+
+ set_pte(&ptep[pte_idx], pfn_pte(virt_to_pfn(page), pgprot));
return 0;
}
--
2.22.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 v2 08/14] arm64, trans_table: add PUD_SECT_RDONLY
From: Pavel Tatashin @ 2019-08-17 2:46 UTC (permalink / raw)
To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190817024629.26611-1-pasha.tatashin@soleen.com>
There is PMD_SECT_RDONLY that is used in pud_* function which is confusing.
Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/include/asm/pgtable-hwdef.h | 1 +
arch/arm64/mm/trans_table.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index db92950bb1a0..dcb4f13c7888 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -110,6 +110,7 @@
#define PUD_TABLE_BIT (_AT(pudval_t, 1) << 1)
#define PUD_TYPE_MASK (_AT(pudval_t, 3) << 0)
#define PUD_TYPE_SECT (_AT(pudval_t, 1) << 0)
+#define PUD_SECT_RDONLY (_AT(pudval_t, 1) << 7) /* AP[2] */
/*
* Level 2 descriptor (PMD).
diff --git a/arch/arm64/mm/trans_table.c b/arch/arm64/mm/trans_table.c
index 634293ffb54c..815e40bb1316 100644
--- a/arch/arm64/mm/trans_table.c
+++ b/arch/arm64/mm/trans_table.c
@@ -138,7 +138,7 @@ static int copy_pud(pgd_t *dst_pgdp, pgd_t *src_pgdp, unsigned long start,
return -ENOMEM;
} else {
set_pud(dst_pudp,
- __pud(pud_val(pud) & ~PMD_SECT_RDONLY));
+ __pud(pud_val(pud) & ~PUD_SECT_RDONLY));
}
} while (dst_pudp++, src_pudp++, addr = next, addr != end);
--
2.22.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 v2 07/14] arm64, trans_table: adjust trans_table_create_copy interface
From: Pavel Tatashin @ 2019-08-17 2:46 UTC (permalink / raw)
To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190817024629.26611-1-pasha.tatashin@soleen.com>
Make trans_table_create_copy inline with the other functions in
trans_table: use the trans_table_info argument, and also use the
trans_table_create_empty.
Note, that the functions that are called by trans_table_create_copy are
not yet adjusted to be compliant with trans_table: they do not yet use
the provided allocator, do not check for generic errors, and do not yet
use the flags in info argument.
Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/include/asm/trans_table.h | 7 ++++++-
arch/arm64/kernel/hibernate.c | 31 ++++++++++++++++++++++++++--
arch/arm64/mm/trans_table.c | 17 ++++++---------
3 files changed, 41 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/include/asm/trans_table.h b/arch/arm64/include/asm/trans_table.h
index 02d3a0333dc9..8c296bd3e10f 100644
--- a/arch/arm64/include/asm/trans_table.h
+++ b/arch/arm64/include/asm/trans_table.h
@@ -44,7 +44,12 @@ struct trans_table_info {
int trans_table_create_empty(struct trans_table_info *info,
pgd_t **trans_table);
-int trans_table_create_copy(pgd_t **dst_pgdp, unsigned long start,
+/*
+ * Create trans table and copy entries from from_table to trans_table in range
+ * [start, end)
+ */
+int trans_table_create_copy(struct trans_table_info *info, pgd_t **trans_table,
+ pgd_t *from_table, unsigned long start,
unsigned long end);
/*
diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index 3a7b362e5a58..6fbaff769c1d 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -323,15 +323,42 @@ int swsusp_arch_resume(void)
phys_addr_t phys_hibernate_exit;
void __noreturn (*hibernate_exit)(phys_addr_t, phys_addr_t, void *,
void *, phys_addr_t, phys_addr_t);
+ struct trans_table_info trans_info = {
+ .trans_alloc_page = hibernate_page_alloc,
+ .trans_alloc_arg = (void *)GFP_ATOMIC,
+ /*
+ * Resume will overwrite areas that may be marked read only
+ * (code, rodata). Clear the RDONLY bit from the temporary
+ * mappings we use during restore.
+ */
+ .trans_flags = TRANS_MKWRITE,
+ };
+
+ /*
+ * debug_pagealloc will removed the PTE_VALID bit if the page isn't in
+ * use by the resume kernel. It may have been in use by the original
+ * kernel, in which case we need to put it back in our copy to do the
+ * restore.
+ *
+ * Before marking this entry valid, check the pfn should be mapped.
+ */
+ if (debug_pagealloc_enabled())
+ trans_info.trans_flags |= (TRANS_MKVALID | TRANS_CHECKPFN);
/*
* Restoring the memory image will overwrite the ttbr1 page tables.
* Create a second copy of just the linear map, and use this when
* restoring.
*/
- rc = trans_table_create_copy(&tmp_pg_dir, PAGE_OFFSET, 0);
- if (rc)
+ rc = trans_table_create_copy(&trans_info, &tmp_pg_dir, init_mm.pgd,
+ PAGE_OFFSET, 0);
+ if (rc) {
+ if (rc == -ENOMEM)
+ pr_err("Failed to allocate memory for temporary page tables.\n");
+ else if (rc == -ENXIO)
+ pr_err("Tried to set PTE for PFN that does not exist\n");
goto out;
+ }
/*
* We need a zero page that is zero before & after resume in order to
diff --git a/arch/arm64/mm/trans_table.c b/arch/arm64/mm/trans_table.c
index 6deb35f83118..634293ffb54c 100644
--- a/arch/arm64/mm/trans_table.c
+++ b/arch/arm64/mm/trans_table.c
@@ -176,22 +176,17 @@ int trans_table_create_empty(struct trans_table_info *info, pgd_t **trans_table)
return 0;
}
-int trans_table_create_copy(pgd_t **dst_pgdp, unsigned long start,
+int trans_table_create_copy(struct trans_table_info *info, pgd_t **trans_table,
+ pgd_t *from_table, unsigned long start,
unsigned long end)
{
int rc;
- pgd_t *trans_table = (pgd_t *)get_safe_page(GFP_ATOMIC);
- if (!trans_table) {
- pr_err("Failed to allocate memory for temporary page tables.\n");
- return -ENOMEM;
- }
-
- rc = copy_page_tables(trans_table, start, end);
- if (!rc)
- *dst_pgdp = trans_table;
+ rc = trans_table_create_empty(info, trans_table);
+ if (rc)
+ return rc;
- return rc;
+ return copy_page_tables(*trans_table, start, end);
}
int trans_table_map_page(struct trans_table_info *info, pgd_t *trans_table,
--
2.22.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 v2 10/14] kexec: add machine_kexec_post_load()
From: Pavel Tatashin @ 2019-08-17 2:46 UTC (permalink / raw)
To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190817024629.26611-1-pasha.tatashin@soleen.com>
It is the same as machine_kexec_prepare(), but is called after segments are
loaded. This way, can do processing work with already loaded relocation
segments. One such example is arm64: it has to have segments loaded in
order to create a page table, but it cannot do it during kexec time,
because at that time allocations won't be possible anymore.
Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
kernel/kexec.c | 4 ++++
kernel/kexec_core.c | 6 ++++++
kernel/kexec_file.c | 4 ++++
kernel/kexec_internal.h | 2 ++
4 files changed, 16 insertions(+)
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 1b018f1a6e0d..27b71dc7b35a 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -159,6 +159,10 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
kimage_terminate(image);
+ ret = machine_kexec_post_load(image);
+ if (ret)
+ goto out;
+
/* Install the new kernel and uninstall the old */
image = xchg(dest_image, image);
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 2c5b72863b7b..8360645d1bbe 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -587,6 +587,12 @@ static void kimage_free_extra_pages(struct kimage *image)
kimage_free_page_list(&image->unusable_pages);
}
+
+int __weak machine_kexec_post_load(struct kimage *image)
+{
+ return 0;
+}
+
void kimage_terminate(struct kimage *image)
{
if (*image->entry != 0)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index b8cc032d5620..cb531d768114 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -391,6 +391,10 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
kimage_terminate(image);
+ ret = machine_kexec_post_load(image);
+ if (ret)
+ goto out;
+
/*
* Free up any temporary buffers allocated which are not needed
* after image has been loaded
diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
index 48aaf2ac0d0d..39d30ccf8d87 100644
--- a/kernel/kexec_internal.h
+++ b/kernel/kexec_internal.h
@@ -13,6 +13,8 @@ void kimage_terminate(struct kimage *image);
int kimage_is_destination_range(struct kimage *image,
unsigned long start, unsigned long end);
+int machine_kexec_post_load(struct kimage *image);
+
extern struct mutex kexec_mutex;
#ifdef CONFIG_KEXEC_FILE
--
2.22.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 v2 09/14] arm64, trans_table: complete generalization of trans_tables
From: Pavel Tatashin @ 2019-08-17 2:46 UTC (permalink / raw)
To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190817024629.26611-1-pasha.tatashin@soleen.com>
Make the last private functions in page table copy path generlized for use
outside of hibernate.
Switch to use the provided allocator, flags, and source page table. Also,
unify all copy function implementations to reduce the possibility of bugs.
All page table levels are implemented symmetrically.
Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/mm/trans_table.c | 204 ++++++++++++++++++++----------------
1 file changed, 113 insertions(+), 91 deletions(-)
diff --git a/arch/arm64/mm/trans_table.c b/arch/arm64/mm/trans_table.c
index 815e40bb1316..ce0f24806eaa 100644
--- a/arch/arm64/mm/trans_table.c
+++ b/arch/arm64/mm/trans_table.c
@@ -27,139 +27,161 @@ static void *trans_alloc(struct trans_table_info *info)
return page;
}
-static void _copy_pte(pte_t *dst_ptep, pte_t *src_ptep, unsigned long addr)
+static int trans_table_copy_pte(struct trans_table_info *info, pte_t *dst_ptep,
+ pte_t *src_ptep, unsigned long start,
+ unsigned long end)
{
- pte_t pte = READ_ONCE(*src_ptep);
-
- if (pte_valid(pte)) {
- /*
- * Resume will overwrite areas that may be marked
- * read only (code, rodata). Clear the RDONLY bit from
- * the temporary mappings we use during restore.
- */
- set_pte(dst_ptep, pte_mkwrite(pte));
- } else if (debug_pagealloc_enabled() && !pte_none(pte)) {
- /*
- * debug_pagealloc will removed the PTE_VALID bit if
- * the page isn't in use by the resume kernel. It may have
- * been in use by the original kernel, in which case we need
- * to put it back in our copy to do the restore.
- *
- * Before marking this entry valid, check the pfn should
- * be mapped.
- */
- BUG_ON(!pfn_valid(pte_pfn(pte)));
-
- set_pte(dst_ptep, pte_mkpresent(pte_mkwrite(pte)));
- }
-}
-
-static int copy_pte(pmd_t *dst_pmdp, pmd_t *src_pmdp, unsigned long start,
- unsigned long end)
-{
- pte_t *src_ptep;
- pte_t *dst_ptep;
unsigned long addr = start;
+ int i = pte_index(addr);
- dst_ptep = (pte_t *)get_safe_page(GFP_ATOMIC);
- if (!dst_ptep)
- return -ENOMEM;
- pmd_populate_kernel(&init_mm, dst_pmdp, dst_ptep);
- dst_ptep = pte_offset_kernel(dst_pmdp, start);
-
- src_ptep = pte_offset_kernel(src_pmdp, start);
do {
- _copy_pte(dst_ptep, src_ptep, addr);
- } while (dst_ptep++, src_ptep++, addr += PAGE_SIZE, addr != end);
+ pte_t src_pte = READ_ONCE(src_ptep[i]);
+
+ if (pte_none(src_pte))
+ continue;
+ if (info->trans_flags & TRANS_MKWRITE)
+ src_pte = pte_mkwrite(src_pte);
+ if (info->trans_flags & TRANS_MKVALID)
+ src_pte = pte_mkpresent(src_pte);
+ if (info->trans_flags & TRANS_CHECKPFN) {
+ if (!pfn_valid(pte_pfn(src_pte)))
+ return -ENXIO;
+ }
+ set_pte(&dst_ptep[i], src_pte);
+ } while (addr += PAGE_SIZE, i++, addr != end && i < PTRS_PER_PTE);
return 0;
}
-static int copy_pmd(pud_t *dst_pudp, pud_t *src_pudp, unsigned long start,
- unsigned long end)
+static int trans_table_copy_pmd(struct trans_table_info *info, pmd_t *dst_pmdp,
+ pmd_t *src_pmdp, unsigned long start,
+ unsigned long end)
{
- pmd_t *src_pmdp;
- pmd_t *dst_pmdp;
unsigned long next;
unsigned long addr = start;
+ int i = pmd_index(addr);
+ int rc;
- if (pud_none(READ_ONCE(*dst_pudp))) {
- dst_pmdp = (pmd_t *)get_safe_page(GFP_ATOMIC);
- if (!dst_pmdp)
- return -ENOMEM;
- pud_populate(&init_mm, dst_pudp, dst_pmdp);
- }
- dst_pmdp = pmd_offset(dst_pudp, start);
-
- src_pmdp = pmd_offset(src_pudp, start);
do {
- pmd_t pmd = READ_ONCE(*src_pmdp);
+ pmd_t src_pmd = READ_ONCE(src_pmdp[i]);
+ pmd_t dst_pmd = READ_ONCE(dst_pmdp[i]);
+ pte_t *dst_ptep, *src_ptep;
next = pmd_addr_end(addr, end);
- if (pmd_none(pmd))
+ if (pmd_none(src_pmd))
+ continue;
+
+ if (!pmd_table(src_pmd)) {
+ if (info->trans_flags & TRANS_MKWRITE)
+ pmd_val(src_pmd) &= ~PMD_SECT_RDONLY;
+ set_pmd(&dst_pmdp[i], src_pmd);
continue;
- if (pmd_table(pmd)) {
- if (copy_pte(dst_pmdp, src_pmdp, addr, next))
+ }
+
+ if (pmd_none(dst_pmd)) {
+ pte_t *t = trans_alloc(info);
+
+ if (!t)
return -ENOMEM;
- } else {
- set_pmd(dst_pmdp,
- __pmd(pmd_val(pmd) & ~PMD_SECT_RDONLY));
+
+ __pmd_populate(&dst_pmdp[i], __pa(t), PTE_TYPE_PAGE);
+ dst_pmd = READ_ONCE(dst_pmdp[i]);
}
- } while (dst_pmdp++, src_pmdp++, addr = next, addr != end);
+
+ src_ptep = __va(pmd_page_paddr(src_pmd));
+ dst_ptep = __va(pmd_page_paddr(dst_pmd));
+
+ rc = trans_table_copy_pte(info, dst_ptep, src_ptep, addr, next);
+ if (rc)
+ return rc;
+ } while (addr = next, i++, addr != end && i < PTRS_PER_PMD);
return 0;
}
-static int copy_pud(pgd_t *dst_pgdp, pgd_t *src_pgdp, unsigned long start,
- unsigned long end)
+static int trans_table_copy_pud(struct trans_table_info *info, pud_t *dst_pudp,
+ pud_t *src_pudp, unsigned long start,
+ unsigned long end)
{
- pud_t *dst_pudp;
- pud_t *src_pudp;
unsigned long next;
unsigned long addr = start;
+ int i = pud_index(addr);
+ int rc;
- if (pgd_none(READ_ONCE(*dst_pgdp))) {
- dst_pudp = (pud_t *)get_safe_page(GFP_ATOMIC);
- if (!dst_pudp)
- return -ENOMEM;
- pgd_populate(&init_mm, dst_pgdp, dst_pudp);
- }
- dst_pudp = pud_offset(dst_pgdp, start);
-
- src_pudp = pud_offset(src_pgdp, start);
do {
- pud_t pud = READ_ONCE(*src_pudp);
+ pud_t src_pud = READ_ONCE(src_pudp[i]);
+ pud_t dst_pud = READ_ONCE(dst_pudp[i]);
+ pmd_t *dst_pmdp, *src_pmdp;
next = pud_addr_end(addr, end);
- if (pud_none(pud))
+ if (pud_none(src_pud))
continue;
- if (pud_table(pud)) {
- if (copy_pmd(dst_pudp, src_pudp, addr, next))
+
+ if (!pud_table(src_pud)) {
+ if (info->trans_flags & TRANS_MKWRITE)
+ pud_val(src_pud) &= ~PUD_SECT_RDONLY;
+ set_pud(&dst_pudp[i], src_pud);
+ continue;
+ }
+
+ if (pud_none(dst_pud)) {
+ pmd_t *t = trans_alloc(info);
+
+ if (!t)
return -ENOMEM;
- } else {
- set_pud(dst_pudp,
- __pud(pud_val(pud) & ~PUD_SECT_RDONLY));
+
+ __pud_populate(&dst_pudp[i], __pa(t), PMD_TYPE_TABLE);
+ dst_pud = READ_ONCE(dst_pudp[i]);
}
- } while (dst_pudp++, src_pudp++, addr = next, addr != end);
+
+ src_pmdp = __va(pud_page_paddr(src_pud));
+ dst_pmdp = __va(pud_page_paddr(dst_pud));
+
+ rc = trans_table_copy_pmd(info, dst_pmdp, src_pmdp, addr, next);
+ if (rc)
+ return rc;
+ } while (addr = next, i++, addr != end && i < PTRS_PER_PUD);
return 0;
}
-static int copy_page_tables(pgd_t *dst_pgdp, unsigned long start,
- unsigned long end)
+static int trans_table_copy_pgd(struct trans_table_info *info, pgd_t *dst_pgdp,
+ pgd_t *src_pgdp, unsigned long start,
+ unsigned long end)
{
unsigned long next;
unsigned long addr = start;
- pgd_t *src_pgdp = pgd_offset_k(start);
+ int i = pgd_index(addr);
+ int rc;
- dst_pgdp = pgd_offset_raw(dst_pgdp, start);
do {
+ pgd_t src_pgd;
+ pgd_t dst_pgd;
+ pud_t *dst_pudp, *src_pudp;
+
+ src_pgd = READ_ONCE(src_pgdp[i]);
+ dst_pgd = READ_ONCE(dst_pgdp[i]);
next = pgd_addr_end(addr, end);
- if (pgd_none(READ_ONCE(*src_pgdp)))
+ if (pgd_none(src_pgd))
continue;
- if (copy_pud(dst_pgdp, src_pgdp, addr, next))
- return -ENOMEM;
- } while (dst_pgdp++, src_pgdp++, addr = next, addr != end);
+
+ if (pgd_none(dst_pgd)) {
+ pud_t *t = trans_alloc(info);
+
+ if (!t)
+ return -ENOMEM;
+
+ __pgd_populate(&dst_pgdp[i], __pa(t), PUD_TYPE_TABLE);
+ dst_pgd = READ_ONCE(dst_pgdp[i]);
+ }
+
+ src_pudp = __va(pgd_page_paddr(src_pgd));
+ dst_pudp = __va(pgd_page_paddr(dst_pgd));
+
+ rc = trans_table_copy_pud(info, dst_pudp, src_pudp, addr, next);
+ if (rc)
+ return rc;
+ } while (addr = next, i++, addr != end && i < PTRS_PER_PGD);
return 0;
}
@@ -186,7 +208,7 @@ int trans_table_create_copy(struct trans_table_info *info, pgd_t **trans_table,
if (rc)
return rc;
- return copy_page_tables(*trans_table, start, end);
+ return trans_table_copy_pgd(info, *trans_table, from_table, start, end);
}
int trans_table_map_page(struct trans_table_info *info, pgd_t *trans_table,
--
2.22.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 v2 11/14] arm64, kexec: move relocation function setup and clean up
From: Pavel Tatashin @ 2019-08-17 2:46 UTC (permalink / raw)
To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190817024629.26611-1-pasha.tatashin@soleen.com>
Currently, kernel relocation function is configured in machine_kexec()
at the time of kexec reboot by using control_code_page.
This operation, however, is more logical to be done during kexec_load,
and thus remove from reboot time. Move, setup of this function to
newly added machine_kexec_post_load().
In addition, do some cleanup: add infor about reloction function to
kexec_image_info(), and remove extra messages from machine_kexec().
Make dtb_mem, always available, if CONFIG_KEXEC_FILE is not configured
dtb_mem is set to zero anyway.
Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/include/asm/kexec.h | 3 +-
arch/arm64/kernel/machine_kexec.c | 49 +++++++++++--------------------
2 files changed, 19 insertions(+), 33 deletions(-)
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index 12a561a54128..d15ca1ca1e83 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -90,14 +90,15 @@ static inline void crash_prepare_suspend(void) {}
static inline void crash_post_resume(void) {}
#endif
-#ifdef CONFIG_KEXEC_FILE
#define ARCH_HAS_KIMAGE_ARCH
struct kimage_arch {
void *dtb;
unsigned long dtb_mem;
+ unsigned long kern_reloc;
};
+#ifdef CONFIG_KEXEC_FILE
extern const struct kexec_file_ops kexec_image_ops;
struct kimage;
diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c
index 0df8493624e0..9b41da50e6f7 100644
--- a/arch/arm64/kernel/machine_kexec.c
+++ b/arch/arm64/kernel/machine_kexec.c
@@ -42,6 +42,7 @@ static void _kexec_image_info(const char *func, int line,
pr_debug(" start: %lx\n", kimage->start);
pr_debug(" head: %lx\n", kimage->head);
pr_debug(" nr_segments: %lu\n", kimage->nr_segments);
+ pr_debug(" kern_reloc: %pa\n", &kimage->arch.kern_reloc);
for (i = 0; i < kimage->nr_segments; i++) {
pr_debug(" segment[%lu]: %016lx - %016lx, 0x%lx bytes, %lu pages\n",
@@ -58,6 +59,19 @@ void machine_kexec_cleanup(struct kimage *kimage)
/* Empty routine needed to avoid build errors. */
}
+int machine_kexec_post_load(struct kimage *kimage)
+{
+ unsigned long kern_reloc;
+
+ kern_reloc = page_to_phys(kimage->control_code_page);
+ memcpy(__va(kern_reloc), arm64_relocate_new_kernel,
+ arm64_relocate_new_kernel_size);
+ kimage->arch.kern_reloc = kern_reloc;
+
+ kexec_image_info(kimage);
+ return 0;
+}
+
/**
* machine_kexec_prepare - Prepare for a kexec reboot.
*
@@ -67,8 +81,6 @@ void machine_kexec_cleanup(struct kimage *kimage)
*/
int machine_kexec_prepare(struct kimage *kimage)
{
- kexec_image_info(kimage);
-
if (kimage->type != KEXEC_TYPE_CRASH && cpus_are_stuck_in_kernel()) {
pr_err("Can't kexec: CPUs are stuck in the kernel.\n");
return -EBUSY;
@@ -143,8 +155,7 @@ static void kexec_segment_flush(const struct kimage *kimage)
*/
void machine_kexec(struct kimage *kimage)
{
- phys_addr_t reboot_code_buffer_phys;
- void *reboot_code_buffer;
+ void *reboot_code_buffer = phys_to_virt(kimage->arch.kern_reloc);
bool in_kexec_crash = (kimage == kexec_crash_image);
bool stuck_cpus = cpus_are_stuck_in_kernel();
@@ -155,30 +166,8 @@ void machine_kexec(struct kimage *kimage)
WARN(in_kexec_crash && (stuck_cpus || smp_crash_stop_failed()),
"Some CPUs may be stale, kdump will be unreliable.\n");
- reboot_code_buffer_phys = page_to_phys(kimage->control_code_page);
- reboot_code_buffer = phys_to_virt(reboot_code_buffer_phys);
-
kexec_image_info(kimage);
- pr_debug("%s:%d: control_code_page: %p\n", __func__, __LINE__,
- kimage->control_code_page);
- pr_debug("%s:%d: reboot_code_buffer_phys: %pa\n", __func__, __LINE__,
- &reboot_code_buffer_phys);
- pr_debug("%s:%d: reboot_code_buffer: %p\n", __func__, __LINE__,
- reboot_code_buffer);
- pr_debug("%s:%d: relocate_new_kernel: %p\n", __func__, __LINE__,
- arm64_relocate_new_kernel);
- pr_debug("%s:%d: relocate_new_kernel_size: 0x%lx(%lu) bytes\n",
- __func__, __LINE__, arm64_relocate_new_kernel_size,
- arm64_relocate_new_kernel_size);
-
- /*
- * Copy arm64_relocate_new_kernel to the reboot_code_buffer for use
- * after the kernel is shut down.
- */
- memcpy(reboot_code_buffer, arm64_relocate_new_kernel,
- arm64_relocate_new_kernel_size);
-
/* Flush the reboot_code_buffer in preparation for its execution. */
__flush_dcache_area(reboot_code_buffer, arm64_relocate_new_kernel_size);
@@ -214,12 +203,8 @@ void machine_kexec(struct kimage *kimage)
* userspace (kexec-tools).
* In kexec_file case, the kernel starts directly without purgatory.
*/
- cpu_soft_restart(reboot_code_buffer_phys, kimage->head, kimage->start,
-#ifdef CONFIG_KEXEC_FILE
- kimage->arch.dtb_mem);
-#else
- 0);
-#endif
+ cpu_soft_restart(kimage->arch.kern_reloc, kimage->head, kimage->start,
+ kimage->arch.dtb_mem);
BUG(); /* Should never get here. */
}
--
2.22.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 v2 12/14] arm64, kexec: add expandable argument to relocation function
From: Pavel Tatashin @ 2019-08-17 2:46 UTC (permalink / raw)
To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190817024629.26611-1-pasha.tatashin@soleen.com>
Currently, kexec relocation function (arm64_relocate_new_kernel) accepts
the following arguments:
head: start of array that contains relocation information.
entry: entry point for new kernel or purgatory.
dtb_mem: first and only argument to entry.
The number of arguments cannot be easily expended, because this
function is also called from HVC_SOFT_RESTART, which preserves only
three arguments. And, also arm64_relocate_new_kernel is written in
assembly but called without stack, thus no place to move extra
arguments to free registers.
Soon, we will need to pass more arguments: once we enable MMU we
will need to pass information about page tables.
Another benefit of allowing this function to accept more arguments, is that
kernel can actually accept up to 4 arguments (x0-x3), however currently
only one is used, but if in the future we will need for more (for example,
pass information about when previous kernel exited to have a precise
measurement in time spent in purgatory), we won't be easilty do that
if arm64_relocate_new_kernel can't accept more arguments.
So, add a new struct: kern_reloc_arg, and place it in kexec safe page (i.e
memory that is not overwritten during relocation).
Thus, make arm64_relocate_new_kernel to only take one argument, that
contains all the needed information.
Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/include/asm/kexec.h | 18 ++++++
arch/arm64/kernel/asm-offsets.c | 9 +++
arch/arm64/kernel/cpu-reset.S | 4 +-
arch/arm64/kernel/cpu-reset.h | 8 +--
arch/arm64/kernel/machine_kexec.c | 28 ++++++++-
arch/arm64/kernel/relocate_kernel.S | 88 ++++++++++-------------------
6 files changed, 86 insertions(+), 69 deletions(-)
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index d15ca1ca1e83..d5b79d4c7fae 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -90,12 +90,30 @@ static inline void crash_prepare_suspend(void) {}
static inline void crash_post_resume(void) {}
#endif
+/*
+ * kern_reloc_arg is passed to kernel relocation function as an argument.
+ * head kimage->head, allows to traverse through relocation segments.
+ * entry_addr kimage->start, where to jump from relocation function (new
+ * kernel, or purgatory entry address).
+ * kern_arg0 first argument to kernel is its dtb address. The other
+ * arguments are currently unused, and must be set to 0
+ */
+struct kern_reloc_arg {
+ unsigned long head;
+ unsigned long entry_addr;
+ unsigned long kern_arg0;
+ unsigned long kern_arg1;
+ unsigned long kern_arg2;
+ unsigned long kern_arg3;
+};
+
#define ARCH_HAS_KIMAGE_ARCH
struct kimage_arch {
void *dtb;
unsigned long dtb_mem;
unsigned long kern_reloc;
+ unsigned long kern_reloc_arg;
};
#ifdef CONFIG_KEXEC_FILE
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 214685760e1c..900394907fd8 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -23,6 +23,7 @@
#include <asm/suspend.h>
#include <linux/kbuild.h>
#include <linux/arm-smccc.h>
+#include <linux/kexec.h>
int main(void)
{
@@ -126,6 +127,14 @@ int main(void)
#ifdef CONFIG_ARM_SDE_INTERFACE
DEFINE(SDEI_EVENT_INTREGS, offsetof(struct sdei_registered_event, interrupted_regs));
DEFINE(SDEI_EVENT_PRIORITY, offsetof(struct sdei_registered_event, priority));
+#endif
+#ifdef CONFIG_KEXEC_CORE
+ DEFINE(KRELOC_HEAD, offsetof(struct kern_reloc_arg, head));
+ DEFINE(KRELOC_ENTRY_ADDR, offsetof(struct kern_reloc_arg, entry_addr));
+ DEFINE(KRELOC_KERN_ARG0, offsetof(struct kern_reloc_arg, kern_arg0));
+ DEFINE(KRELOC_KERN_ARG1, offsetof(struct kern_reloc_arg, kern_arg1));
+ DEFINE(KRELOC_KERN_ARG2, offsetof(struct kern_reloc_arg, kern_arg2));
+ DEFINE(KRELOC_KERN_ARG3, offsetof(struct kern_reloc_arg, kern_arg3));
#endif
return 0;
}
diff --git a/arch/arm64/kernel/cpu-reset.S b/arch/arm64/kernel/cpu-reset.S
index 6ea337d464c4..64c78a42919f 100644
--- a/arch/arm64/kernel/cpu-reset.S
+++ b/arch/arm64/kernel/cpu-reset.S
@@ -43,9 +43,7 @@ ENTRY(__cpu_soft_restart)
hvc #0 // no return
1: mov x18, x1 // entry
- mov x0, x2 // arg0
- mov x1, x3 // arg1
- mov x2, x4 // arg2
+ mov x0, x2 // arg
br x18
ENDPROC(__cpu_soft_restart)
diff --git a/arch/arm64/kernel/cpu-reset.h b/arch/arm64/kernel/cpu-reset.h
index ed50e9587ad8..7a8720ff186f 100644
--- a/arch/arm64/kernel/cpu-reset.h
+++ b/arch/arm64/kernel/cpu-reset.h
@@ -11,12 +11,10 @@
#include <asm/virt.h>
void __cpu_soft_restart(unsigned long el2_switch, unsigned long entry,
- unsigned long arg0, unsigned long arg1, unsigned long arg2);
+ unsigned long arg);
static inline void __noreturn cpu_soft_restart(unsigned long entry,
- unsigned long arg0,
- unsigned long arg1,
- unsigned long arg2)
+ unsigned long arg)
{
typeof(__cpu_soft_restart) *restart;
@@ -25,7 +23,7 @@ static inline void __noreturn cpu_soft_restart(unsigned long entry,
restart = (void *)__pa_symbol(__cpu_soft_restart);
cpu_install_idmap();
- restart(el2_switch, entry, arg0, arg1, arg2);
+ restart(el2_switch, entry, arg);
unreachable();
}
diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c
index 9b41da50e6f7..d745ea2051df 100644
--- a/arch/arm64/kernel/machine_kexec.c
+++ b/arch/arm64/kernel/machine_kexec.c
@@ -43,6 +43,7 @@ static void _kexec_image_info(const char *func, int line,
pr_debug(" head: %lx\n", kimage->head);
pr_debug(" nr_segments: %lu\n", kimage->nr_segments);
pr_debug(" kern_reloc: %pa\n", &kimage->arch.kern_reloc);
+ pr_debug(" kern_reloc_arg: %pa\n", &kimage->arch.kern_reloc_arg);
for (i = 0; i < kimage->nr_segments; i++) {
pr_debug(" segment[%lu]: %016lx - %016lx, 0x%lx bytes, %lu pages\n",
@@ -59,14 +60,38 @@ void machine_kexec_cleanup(struct kimage *kimage)
/* Empty routine needed to avoid build errors. */
}
+/* Allocates pages for kexec page table */
+static void *kexec_page_alloc(void *arg)
+{
+ struct kimage *kimage = (struct kimage *)arg;
+ struct page *page = kimage_alloc_control_pages(kimage, 0);
+
+ if (!page)
+ return NULL;
+
+ return page_address(page);
+}
+
int machine_kexec_post_load(struct kimage *kimage)
{
unsigned long kern_reloc;
+ struct kern_reloc_arg *kern_reloc_arg;
kern_reloc = page_to_phys(kimage->control_code_page);
memcpy(__va(kern_reloc), arm64_relocate_new_kernel,
arm64_relocate_new_kernel_size);
+
+ kern_reloc_arg = kexec_page_alloc(kimage);
+ if (!kern_reloc_arg)
+ return -ENOMEM;
+ memset(kern_reloc_arg, 0, sizeof(struct kern_reloc_arg));
+
kimage->arch.kern_reloc = kern_reloc;
+ kimage->arch.kern_reloc_arg = __pa(kern_reloc_arg);
+
+ kern_reloc_arg->head = kimage->head;
+ kern_reloc_arg->entry_addr = kimage->start;
+ kern_reloc_arg->kern_arg0 = kimage->arch.dtb_mem;
kexec_image_info(kimage);
return 0;
@@ -203,8 +228,7 @@ void machine_kexec(struct kimage *kimage)
* userspace (kexec-tools).
* In kexec_file case, the kernel starts directly without purgatory.
*/
- cpu_soft_restart(kimage->arch.kern_reloc, kimage->head, kimage->start,
- kimage->arch.dtb_mem);
+ cpu_soft_restart(kimage->arch.kern_reloc, kimage->arch.kern_reloc_arg);
BUG(); /* Should never get here. */
}
diff --git a/arch/arm64/kernel/relocate_kernel.S b/arch/arm64/kernel/relocate_kernel.S
index c1d7db71a726..d352faf7cbe6 100644
--- a/arch/arm64/kernel/relocate_kernel.S
+++ b/arch/arm64/kernel/relocate_kernel.S
@@ -8,7 +8,7 @@
#include <linux/kexec.h>
#include <linux/linkage.h>
-
+#include <asm/asm-offsets.h>
#include <asm/assembler.h>
#include <asm/kexec.h>
#include <asm/page.h>
@@ -17,86 +17,58 @@
/*
* arm64_relocate_new_kernel - Put a 2nd stage image in place and boot it.
*
- * The memory that the old kernel occupies may be overwritten when coping the
+ * The memory that the old kernel occupies may be overwritten when copying the
* new image to its final location. To assure that the
* arm64_relocate_new_kernel routine which does that copy is not overwritten,
* all code and data needed by arm64_relocate_new_kernel must be between the
* symbols arm64_relocate_new_kernel and arm64_relocate_new_kernel_end. The
* machine_kexec() routine will copy arm64_relocate_new_kernel to the kexec
- * control_code_page, a special page which has been set up to be preserved
- * during the copy operation.
+ * safe memory that has been set up to be preserved during the copy operation.
*/
ENTRY(arm64_relocate_new_kernel)
-
- /* Setup the list loop variables. */
- mov x18, x2 /* x18 = dtb address */
- mov x17, x1 /* x17 = kimage_start */
- mov x16, x0 /* x16 = kimage_head */
- raw_dcache_line_size x15, x0 /* x15 = dcache line size */
- mov x14, xzr /* x14 = entry ptr */
- mov x13, xzr /* x13 = copy dest */
-
/* Clear the sctlr_el2 flags. */
- mrs x0, CurrentEL
- cmp x0, #CurrentEL_EL2
+ mrs x2, CurrentEL
+ cmp x2, #CurrentEL_EL2
b.ne 1f
- mrs x0, sctlr_el2
+ mrs x2, sctlr_el2
ldr x1, =SCTLR_ELx_FLAGS
- bic x0, x0, x1
+ bic x2, x2, x1
pre_disable_mmu_workaround
- msr sctlr_el2, x0
+ msr sctlr_el2, x2
isb
-1:
-
- /* Check if the new image needs relocation. */
+1: /* Check if the new image needs relocation. */
+ ldr x16, [x0, #KRELOC_HEAD] /* x16 = kimage_head */
tbnz x16, IND_DONE_BIT, .Ldone
-
+ raw_dcache_line_size x15, x1 /* x15 = dcache line size */
.Lloop:
and x12, x16, PAGE_MASK /* x12 = addr */
-
/* Test the entry flags. */
.Ltest_source:
tbz x16, IND_SOURCE_BIT, .Ltest_indirection
/* Invalidate dest page to PoC. */
- mov x0, x13
- add x20, x0, #PAGE_SIZE
+ mov x2, x13
+ add x20, x2, #PAGE_SIZE
sub x1, x15, #1
- bic x0, x0, x1
-2: dc ivac, x0
- add x0, x0, x15
- cmp x0, x20
+ bic x2, x2, x1
+2: dc ivac, x2
+ add x2, x2, x15
+ cmp x2, x20
b.lo 2b
dsb sy
- mov x20, x13
- mov x21, x12
- copy_page x20, x21, x0, x1, x2, x3, x4, x5, x6, x7
-
- /* dest += PAGE_SIZE */
- add x13, x13, PAGE_SIZE
+ copy_page x13, x12, x1, x2, x3, x4, x5, x6, x7, x8
b .Lnext
-
.Ltest_indirection:
tbz x16, IND_INDIRECTION_BIT, .Ltest_destination
-
- /* ptr = addr */
- mov x14, x12
+ mov x14, x12 /* ptr = addr */
b .Lnext
-
.Ltest_destination:
tbz x16, IND_DESTINATION_BIT, .Lnext
-
- /* dest = addr */
- mov x13, x12
-
+ mov x13, x12 /* dest = addr */
.Lnext:
- /* entry = *ptr++ */
- ldr x16, [x14], #8
-
- /* while (!(entry & DONE)) */
- tbz x16, IND_DONE_BIT, .Lloop
-
+ ldr x16, [x14], #8 /* entry = *ptr++ */
+ tbz x16, IND_DONE_BIT, .Lloop /* while (!(entry & DONE)) */
.Ldone:
/* wait for writes from copy_page to finish */
dsb nsh
@@ -105,18 +77,16 @@ ENTRY(arm64_relocate_new_kernel)
isb
/* Start new image. */
- mov x0, x18
- mov x1, xzr
- mov x2, xzr
- mov x3, xzr
- br x17
-
-ENDPROC(arm64_relocate_new_kernel)
+ ldr x4, [x0, #KRELOC_ENTRY_ADDR] /* x4 = kimage_start */
+ ldr x3, [x0, #KRELOC_KERN_ARG3]
+ ldr x2, [x0, #KRELOC_KERN_ARG2]
+ ldr x1, [x0, #KRELOC_KERN_ARG1]
+ ldr x0, [x0, #KRELOC_KERN_ARG0] /* x0 = dtb address */
+ br x4
+END(arm64_relocate_new_kernel)
.ltorg
-
.align 3 /* To keep the 64-bit values below naturally aligned. */
-
.Lcopy_end:
.org KEXEC_CONTROL_PAGE_SIZE
--
2.22.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 v2 13/14] arm64, kexec: configure transitional page table for kexec
From: Pavel Tatashin @ 2019-08-17 2:46 UTC (permalink / raw)
To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190817024629.26611-1-pasha.tatashin@soleen.com>
Configure a page table located in kexec-safe memory that has
the following mappings:
1. identity mapping for text of relocation function with executable permission.
2. identity mapping for argument for relocation function.
3. linear mappings for all source ranges
4. linear mappings for all destination ranges.
Also, configure el2_vector, that is used to jump to new kernel from EL2 on
non-VHE kernels.
Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/include/asm/kexec.h | 32 +++++++
arch/arm64/kernel/asm-offsets.c | 6 ++
arch/arm64/kernel/machine_kexec.c | 129 ++++++++++++++++++++++++++--
arch/arm64/kernel/relocate_kernel.S | 16 +++-
4 files changed, 174 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index d5b79d4c7fae..450d8440f597 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -90,6 +90,23 @@ static inline void crash_prepare_suspend(void) {}
static inline void crash_post_resume(void) {}
#endif
+#if defined(CONFIG_KEXEC_CORE)
+/* Global variables for the arm64_relocate_new_kernel routine. */
+extern const unsigned char arm64_relocate_new_kernel[];
+extern const unsigned long arm64_relocate_new_kernel_size;
+
+/* Body of the vector for escalating to EL2 from relocation routine */
+extern const unsigned char kexec_el1_sync[];
+extern const unsigned long kexec_el1_sync_size;
+
+#define KEXEC_EL2_VECTOR_TABLE_SIZE 2048
+#define KEXEC_EL2_SYNC_OFFSET (KEXEC_EL2_VECTOR_TABLE_SIZE / 2)
+
+#endif
+
+#define KEXEC_SRC_START PAGE_OFFSET
+#define KEXEC_DST_START (PAGE_OFFSET + \
+ ((UL(0xffffffffffffffff) - PAGE_OFFSET) >> 1) + 1)
/*
* kern_reloc_arg is passed to kernel relocation function as an argument.
* head kimage->head, allows to traverse through relocation segments.
@@ -97,6 +114,15 @@ static inline void crash_post_resume(void) {}
* kernel, or purgatory entry address).
* kern_arg0 first argument to kernel is its dtb address. The other
* arguments are currently unused, and must be set to 0
+ * trans_ttbr0 idmap for relocation function and its argument
+ * trans_ttbr1 linear map for source/destination addresses.
+ * el2_vector If present means that relocation routine will go to EL1
+ * from EL2 to do the copy, and then back to EL2 to do the jump
+ * to new world. This vector contains only the final jump
+ * instruction at KEXEC_EL2_SYNC_OFFSET.
+ * src_addr linear map for source pages.
+ * dst_addr linear map for destination pages.
+ * copy_len Number of bytes that need to be copied
*/
struct kern_reloc_arg {
unsigned long head;
@@ -105,6 +131,12 @@ struct kern_reloc_arg {
unsigned long kern_arg1;
unsigned long kern_arg2;
unsigned long kern_arg3;
+ unsigned long trans_ttbr0;
+ unsigned long trans_ttbr1;
+ unsigned long el2_vector;
+ unsigned long src_addr;
+ unsigned long dst_addr;
+ unsigned long copy_len;
};
#define ARCH_HAS_KIMAGE_ARCH
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 900394907fd8..7c2ba09a8ceb 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -135,6 +135,12 @@ int main(void)
DEFINE(KRELOC_KERN_ARG1, offsetof(struct kern_reloc_arg, kern_arg1));
DEFINE(KRELOC_KERN_ARG2, offsetof(struct kern_reloc_arg, kern_arg2));
DEFINE(KRELOC_KERN_ARG3, offsetof(struct kern_reloc_arg, kern_arg3));
+ DEFINE(KRELOC_TRANS_TTBR0, offsetof(struct kern_reloc_arg, trans_ttbr0));
+ DEFINE(KRELOC_TRANS_TTBR1, offsetof(struct kern_reloc_arg, trans_ttbr1));
+ DEFINE(KRELOC_EL2_VECTOR, offsetof(struct kern_reloc_arg, el2_vector));
+ DEFINE(KRELOC_SRC_ADDR, offsetof(struct kern_reloc_arg, src_addr));
+ DEFINE(KRELOC_DST_ADDR, offsetof(struct kern_reloc_arg, dst_addr));
+ DEFINE(KRELOC_COPY_LEN, offsetof(struct kern_reloc_arg, copy_len));
#endif
return 0;
}
diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c
index d745ea2051df..16f761fc50c8 100644
--- a/arch/arm64/kernel/machine_kexec.c
+++ b/arch/arm64/kernel/machine_kexec.c
@@ -20,13 +20,10 @@
#include <asm/mmu.h>
#include <asm/mmu_context.h>
#include <asm/page.h>
+#include <asm/trans_table.h>
#include "cpu-reset.h"
-/* Global variables for the arm64_relocate_new_kernel routine. */
-extern const unsigned char arm64_relocate_new_kernel[];
-extern const unsigned long arm64_relocate_new_kernel_size;
-
/**
* kexec_image_info - For debugging output.
*/
@@ -72,15 +69,128 @@ static void *kexec_page_alloc(void *arg)
return page_address(page);
}
+/*
+ * Map source segments starting from KEXEC_SRC_START, and map destination
+ * segments starting from KEXEC_DST_START, and return size of copy in
+ * *copy_len argument.
+ * Relocation function essentially needs to do:
+ * memcpy(KEXEC_DST_START, KEXEC_SRC_START, copy_len);
+ */
+static int map_segments(struct kimage *kimage, pgd_t *pgdp,
+ struct trans_table_info *info,
+ unsigned long *copy_len)
+{
+ unsigned long *ptr = 0;
+ unsigned long dest = 0;
+ unsigned long src_va = KEXEC_SRC_START;
+ unsigned long dst_va = KEXEC_DST_START;
+ unsigned long len = 0;
+ unsigned long entry, addr;
+ int rc;
+
+ for (entry = kimage->head; !(entry & IND_DONE); entry = *ptr++) {
+ addr = entry & PAGE_MASK;
+
+ switch (entry & IND_FLAGS) {
+ case IND_DESTINATION:
+ dest = addr;
+ break;
+ case IND_INDIRECTION:
+ ptr = __va(addr);
+ if (rc)
+ return rc;
+ break;
+ case IND_SOURCE:
+ rc = trans_table_map_page(info, pgdp, __va(addr),
+ src_va, PAGE_KERNEL);
+ if (rc)
+ return rc;
+ rc = trans_table_map_page(info, pgdp, __va(dest),
+ dst_va, PAGE_KERNEL);
+ if (rc)
+ return rc;
+ dest += PAGE_SIZE;
+ src_va += PAGE_SIZE;
+ dst_va += PAGE_SIZE;
+ len += PAGE_SIZE;
+ }
+ }
+ *copy_len = len;
+
+ return 0;
+}
+
+static int mmu_relocate_setup(struct kimage *kimage, unsigned long kern_reloc,
+ struct kern_reloc_arg *kern_reloc_arg)
+{
+ struct trans_table_info info = {
+ .trans_alloc_page = kexec_page_alloc,
+ .trans_alloc_arg = kimage,
+ .trans_flags = 0,
+ };
+ pgd_t *trans_ttbr0, *trans_ttbr1;
+ int rc;
+
+ rc = trans_table_create_empty(&info, &trans_ttbr0);
+ if (rc)
+ return rc;
+
+ rc = trans_table_create_empty(&info, &trans_ttbr1);
+ if (rc)
+ return rc;
+
+ rc = map_segments(kimage, trans_ttbr1, &info,
+ &kern_reloc_arg->copy_len);
+ if (rc)
+ return rc;
+
+ /* Map relocation function va == pa */
+ rc = trans_table_map_page(&info, trans_ttbr0, __va(kern_reloc),
+ kern_reloc, PAGE_KERNEL_EXEC);
+ if (rc)
+ return rc;
+
+ /* Map relocation function argument va == pa */
+ rc = trans_table_map_page(&info, trans_ttbr0, kern_reloc_arg,
+ __pa(kern_reloc_arg), PAGE_KERNEL);
+ if (rc)
+ return rc;
+
+ kern_reloc_arg->trans_ttbr0 = phys_to_ttbr(__pa(trans_ttbr0));
+ kern_reloc_arg->trans_ttbr1 = phys_to_ttbr(__pa(trans_ttbr1));
+ kern_reloc_arg->src_addr = KEXEC_SRC_START;
+ kern_reloc_arg->dst_addr = KEXEC_DST_START;
+
+ return 0;
+}
+
int machine_kexec_post_load(struct kimage *kimage)
{
+ unsigned long el2_vector = 0;
unsigned long kern_reloc;
struct kern_reloc_arg *kern_reloc_arg;
+ int rc = 0;
+
+ /*
+ * Sanity check that relocation function + el2_vector fit into one
+ * page.
+ */
+ if (arm64_relocate_new_kernel_size > KEXEC_EL2_VECTOR_TABLE_SIZE) {
+ pr_err("can't fit relocation function and el2_vector in one page");
+ return -ENOMEM;
+ }
kern_reloc = page_to_phys(kimage->control_code_page);
memcpy(__va(kern_reloc), arm64_relocate_new_kernel,
arm64_relocate_new_kernel_size);
+ /* Setup vector table only when EL2 is available, but no VHE */
+ if (is_hyp_mode_available() && !is_kernel_in_hyp_mode()) {
+ el2_vector = kern_reloc + KEXEC_EL2_VECTOR_TABLE_SIZE;
+ memcpy(__va(el2_vector + KEXEC_EL2_SYNC_OFFSET), kexec_el1_sync,
+ kexec_el1_sync_size);
+ }
+
kern_reloc_arg = kexec_page_alloc(kimage);
if (!kern_reloc_arg)
return -ENOMEM;
@@ -91,10 +201,19 @@ int machine_kexec_post_load(struct kimage *kimage)
kern_reloc_arg->head = kimage->head;
kern_reloc_arg->entry_addr = kimage->start;
+ kern_reloc_arg->el2_vector = el2_vector;
kern_reloc_arg->kern_arg0 = kimage->arch.dtb_mem;
+ /*
+ * If relocation is not needed, we do not need to enable MMU in
+ * relocation routine, therefore do not create page tables for
+ * scenarios such as crash kernel
+ */
+ if (!(kimage->head & IND_DONE))
+ rc = mmu_relocate_setup(kimage, kern_reloc, kern_reloc_arg);
+
kexec_image_info(kimage);
- return 0;
+ return rc;
}
/**
diff --git a/arch/arm64/kernel/relocate_kernel.S b/arch/arm64/kernel/relocate_kernel.S
index d352faf7cbe6..14243a678277 100644
--- a/arch/arm64/kernel/relocate_kernel.S
+++ b/arch/arm64/kernel/relocate_kernel.S
@@ -83,17 +83,25 @@ ENTRY(arm64_relocate_new_kernel)
ldr x1, [x0, #KRELOC_KERN_ARG1]
ldr x0, [x0, #KRELOC_KERN_ARG0] /* x0 = dtb address */
br x4
+.ltorg
+.Larm64_relocate_new_kernel_end:
END(arm64_relocate_new_kernel)
-.ltorg
+ENTRY(kexec_el1_sync)
+ br x4 /* Jump to new world from el2 */
+.Lkexec_el1_sync_end:
+END(kexec_el1_sync)
+
.align 3 /* To keep the 64-bit values below naturally aligned. */
-.Lcopy_end:
.org KEXEC_CONTROL_PAGE_SIZE
-
/*
* arm64_relocate_new_kernel_size - Number of bytes to copy to the
* control_code_page.
*/
.globl arm64_relocate_new_kernel_size
arm64_relocate_new_kernel_size:
- .quad .Lcopy_end - arm64_relocate_new_kernel
+ .quad .Larm64_relocate_new_kernel_end - arm64_relocate_new_kernel
+
+.globl kexec_el1_sync_size
+kexec_el1_sync_size:
+ .quad .Lkexec_el1_sync_end - kexec_el1_sync
--
2.22.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 v2 14/14] arm64, kexec: enable MMU during kexec relocation
From: Pavel Tatashin @ 2019-08-17 2:46 UTC (permalink / raw)
To: pasha.tatashin, jmorris, sashal, ebiederm, kexec, linux-kernel,
corbet, catalin.marinas, will, linux-arm-kernel, marc.zyngier,
james.morse, vladimir.murzin, matthias.bgg, bhsharma, linux-mm
In-Reply-To: <20190817024629.26611-1-pasha.tatashin@soleen.com>
Now, that we have transitional page tables configured, temporarily enable
MMU to allow faster relocation of segments to final destination.
The performance data: for a moderate size kernel + initramfs: 25M the
relocation was taking 0.382s, with enabled MMU it now takes
0.019s only or x20 improvement.
The time is proportional to the size of relocation, therefore if initramfs
is larger, 100M it could take over a second.
Also, remove reloc_arg->head, as it is not needed anymore once MMU is
enabled.
Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/include/asm/kexec.h | 2 -
arch/arm64/kernel/asm-offsets.c | 1 -
arch/arm64/kernel/machine_kexec.c | 1 -
arch/arm64/kernel/relocate_kernel.S | 136 +++++++++++++++++-----------
4 files changed, 84 insertions(+), 56 deletions(-)
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index 450d8440f597..ad81ed3e5751 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -109,7 +109,6 @@ extern const unsigned long kexec_el1_sync_size;
((UL(0xffffffffffffffff) - PAGE_OFFSET) >> 1) + 1)
/*
* kern_reloc_arg is passed to kernel relocation function as an argument.
- * head kimage->head, allows to traverse through relocation segments.
* entry_addr kimage->start, where to jump from relocation function (new
* kernel, or purgatory entry address).
* kern_arg0 first argument to kernel is its dtb address. The other
@@ -125,7 +124,6 @@ extern const unsigned long kexec_el1_sync_size;
* copy_len Number of bytes that need to be copied
*/
struct kern_reloc_arg {
- unsigned long head;
unsigned long entry_addr;
unsigned long kern_arg0;
unsigned long kern_arg1;
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 7c2ba09a8ceb..13ad00b1b90f 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -129,7 +129,6 @@ int main(void)
DEFINE(SDEI_EVENT_PRIORITY, offsetof(struct sdei_registered_event, priority));
#endif
#ifdef CONFIG_KEXEC_CORE
- DEFINE(KRELOC_HEAD, offsetof(struct kern_reloc_arg, head));
DEFINE(KRELOC_ENTRY_ADDR, offsetof(struct kern_reloc_arg, entry_addr));
DEFINE(KRELOC_KERN_ARG0, offsetof(struct kern_reloc_arg, kern_arg0));
DEFINE(KRELOC_KERN_ARG1, offsetof(struct kern_reloc_arg, kern_arg1));
diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c
index 16f761fc50c8..b5ff5fdb4777 100644
--- a/arch/arm64/kernel/machine_kexec.c
+++ b/arch/arm64/kernel/machine_kexec.c
@@ -199,7 +199,6 @@ int machine_kexec_post_load(struct kimage *kimage)
kimage->arch.kern_reloc = kern_reloc;
kimage->arch.kern_reloc_arg = __pa(kern_reloc_arg);
- kern_reloc_arg->head = kimage->head;
kern_reloc_arg->entry_addr = kimage->start;
kern_reloc_arg->el2_vector = el2_vector;
kern_reloc_arg->kern_arg0 = kimage->arch.dtb_mem;
diff --git a/arch/arm64/kernel/relocate_kernel.S b/arch/arm64/kernel/relocate_kernel.S
index 14243a678277..96ff6760bd9c 100644
--- a/arch/arm64/kernel/relocate_kernel.S
+++ b/arch/arm64/kernel/relocate_kernel.S
@@ -4,6 +4,8 @@
*
* Copyright (C) Linaro.
* Copyright (C) Huawei Futurewei Technologies.
+ * Copyright (c) 2019, Microsoft Corporation.
+ * Pavel Tatashin <patatash@linux.microsoft.com>
*/
#include <linux/kexec.h>
@@ -14,6 +16,49 @@
#include <asm/page.h>
#include <asm/sysreg.h>
+/* Invalidae TLB */
+.macro tlb_invalidate
+ dsb sy
+ dsb ish
+ tlbi vmalle1
+ dsb ish
+ isb
+.endm
+
+/* Turn-off mmu at level specified by sctlr */
+.macro turn_off_mmu sctlr, tmp1, tmp2
+ mrs \tmp1, \sctlr
+ ldr \tmp2, =SCTLR_ELx_FLAGS
+ bic \tmp1, \tmp1, \tmp2
+ pre_disable_mmu_workaround
+ msr \sctlr, \tmp1
+ isb
+.endm
+
+/* Turn-on mmu at level specified by sctlr */
+.macro turn_on_mmu sctlr, tmp1, tmp2
+ mrs \tmp1, \sctlr
+ ldr \tmp2, =SCTLR_ELx_FLAGS
+ orr \tmp1, \tmp1, \tmp2
+ msr \sctlr, \tmp1
+ ic iallu
+ dsb nsh
+ isb
+.endm
+
+/*
+ * Set ttbr0 and ttbr1, called while MMU is disabled, so no need to temporarily
+ * set zero_page table. Invalidate TLB after new tables are set.
+ */
+.macro set_ttbr arg, tmp
+ ldr \tmp, [\arg, #KRELOC_TRANS_TTBR0]
+ msr ttbr0_el1, \tmp
+ ldr \tmp, [\arg, #KRELOC_TRANS_TTBR1]
+ offset_ttbr1 \tmp
+ msr ttbr1_el1, \tmp
+ isb
+.endm
+
/*
* arm64_relocate_new_kernel - Put a 2nd stage image in place and boot it.
*
@@ -24,65 +69,52 @@
* symbols arm64_relocate_new_kernel and arm64_relocate_new_kernel_end. The
* machine_kexec() routine will copy arm64_relocate_new_kernel to the kexec
* safe memory that has been set up to be preserved during the copy operation.
+ *
+ * This function temporarily enables MMU if kernel relocation is needed.
+ * Also, if we enter this function at EL2 on non-VHE kernel, we temporarily go
+ * to EL1 to enable MMU, and escalate back to EL2 at the end to do the jump to
+ * the new kernel. This is determined by presence of el2_vector.
*/
ENTRY(arm64_relocate_new_kernel)
- /* Clear the sctlr_el2 flags. */
- mrs x2, CurrentEL
- cmp x2, #CurrentEL_EL2
+ mrs x1, CurrentEL
+ cmp x1, #CurrentEL_EL2
b.ne 1f
- mrs x2, sctlr_el2
- ldr x1, =SCTLR_ELx_FLAGS
- bic x2, x2, x1
- pre_disable_mmu_workaround
- msr sctlr_el2, x2
- isb
-1: /* Check if the new image needs relocation. */
- ldr x16, [x0, #KRELOC_HEAD] /* x16 = kimage_head */
- tbnz x16, IND_DONE_BIT, .Ldone
- raw_dcache_line_size x15, x1 /* x15 = dcache line size */
-.Lloop:
- and x12, x16, PAGE_MASK /* x12 = addr */
- /* Test the entry flags. */
-.Ltest_source:
- tbz x16, IND_SOURCE_BIT, .Ltest_indirection
-
- /* Invalidate dest page to PoC. */
- mov x2, x13
- add x20, x2, #PAGE_SIZE
- sub x1, x15, #1
- bic x2, x2, x1
-2: dc ivac, x2
- add x2, x2, x15
- cmp x2, x20
- b.lo 2b
- dsb sy
-
- copy_page x13, x12, x1, x2, x3, x4, x5, x6, x7, x8
- b .Lnext
-.Ltest_indirection:
- tbz x16, IND_INDIRECTION_BIT, .Ltest_destination
- mov x14, x12 /* ptr = addr */
- b .Lnext
-.Ltest_destination:
- tbz x16, IND_DESTINATION_BIT, .Lnext
- mov x13, x12 /* dest = addr */
-.Lnext:
- ldr x16, [x14], #8 /* entry = *ptr++ */
- tbz x16, IND_DONE_BIT, .Lloop /* while (!(entry & DONE)) */
-.Ldone:
- /* wait for writes from copy_page to finish */
- dsb nsh
- ic iallu
- dsb nsh
- isb
-
- /* Start new image. */
- ldr x4, [x0, #KRELOC_ENTRY_ADDR] /* x4 = kimage_start */
+ turn_off_mmu sctlr_el2, x1, x2 /* Turn off MMU at EL2 */
+1: mov x20, xzr /* x20 will hold vector value */
+ ldr x11, [x0, #KRELOC_COPY_LEN]
+ cbz x11, 5f /* Check if need to relocate */
+ ldr x20, [x0, #KRELOC_EL2_VECTOR]
+ cbz x20, 2f /* need to reduce to EL1? */
+ msr vbar_el2, x20 /* el2_vector present, means */
+ adr x1, 2f /* we will do copy in el1 but */
+ msr elr_el2, x1 /* do final jump from el2 */
+ eret /* Reduce to EL1 */
+2: set_ttbr x0, x1 /* Set our page tables */
+ tlb_invalidate
+ turn_on_mmu sctlr_el1, x1, x2 /* Turn MMU back on */
+ ldr x1, [x0, #KRELOC_DST_ADDR];
+ ldr x2, [x0, #KRELOC_SRC_ADDR];
+ mov x12, x1 /* x12 dst backup */
+3: copy_page x1, x2, x3, x4, x5, x6, x7, x8, x9, x10
+ sub x11, x11, #PAGE_SIZE
+ cbnz x11, 3b /* page copy loop */
+ raw_dcache_line_size x2, x3 /* x2 = dcache line size */
+ sub x3, x2, #1 /* x3 = dcache_size - 1 */
+ bic x12, x12, x3
+4: dc cvau, x12 /* Flush D-cache */
+ add x12, x12, x2
+ cmp x12, x1 /* Compare to dst + len */
+ b.ne 4b /* D-cache flush loop */
+ turn_off_mmu sctlr_el1, x1, x2 /* Turn off MMU */
+ tlb_invalidate /* Invalidate TLB */
+5: ldr x4, [x0, #KRELOC_ENTRY_ADDR] /* x4 = kimage_start */
ldr x3, [x0, #KRELOC_KERN_ARG3]
ldr x2, [x0, #KRELOC_KERN_ARG2]
ldr x1, [x0, #KRELOC_KERN_ARG1]
ldr x0, [x0, #KRELOC_KERN_ARG0] /* x0 = dtb address */
- br x4
+ cbnz x20, 6f /* need to escalate to el2? */
+ br x4 /* Jump to new world */
+6: hvc #0 /* enters kexec_el1_sync */
.ltorg
.Larm64_relocate_new_kernel_end:
END(arm64_relocate_new_kernel)
--
2.22.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 1/5] dt-bindings: clock: Add Bitmain BM1880 SoC clock controller binding
From: Manivannan Sadhasivam @ 2019-08-17 3:34 UTC (permalink / raw)
To: Stephen Boyd
Cc: devicetree, mturquette, linux-kernel, darren.tsao, robh+dt,
haitao.suo, fisher.cheng, alec.lin, linux-clk, linux-arm-kernel
In-Reply-To: <20190808050128.E3DA52186A@mail.kernel.org>
Hi Stephen,
On Wed, Aug 07, 2019 at 10:01:28PM -0700, Stephen Boyd wrote:
> 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.
>
Sure.
> > 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?
>
These are all part of the sysctrl block (clock + pinctrl + reset) and the
register domains got split between system and pll.
Thanks,
Mani
> > + 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 V5 3/5] iommu/dma-iommu: Handle deferred devices
From: Hillf Danton @ 2019-08-17 3:39 UTC (permalink / raw)
To: Tom Murphy
Cc: Heiko Stuebner, virtualization, linux-tegra, Thierry Reding,
Will Deacon, Marek Szyprowski, Jean-Philippe Brucker,
linux-samsung-soc, Joerg Roedel, iommu, Krzysztof Kozlowski,
Jonathan Hunter, linux-rockchip, Andy Gross, Gerald Schaefer,
linux-s390, linux-arm-msm, linux-mediatek, Matthias Brugger,
linux-arm-kernel, David Woodhouse, linux-kernel, Rob Clark,
Kukjin Kim, Robin Murphy
In-Reply-To: <20190815110944.3579-1-murphyt7@tcd.ie>
On Thu, 15 Aug 2019 12:09:41 +0100 Tom Murphy wrote:
>
> Handle devices which defer their attach to the iommu in the dma-iommu api
>
> Signed-off-by: Tom Murphy <murphyt7@tcd.ie>
> ---
> drivers/iommu/dma-iommu.c | 27 ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 2712fbc68b28..906b7fa14d3c 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -22,6 +22,7 @@
> #include <linux/pci.h>
> #include <linux/scatterlist.h>
> #include <linux/vmalloc.h>
> +#include <linux/crash_dump.h>
>
> struct iommu_dma_msi_page {
> struct list_head list;
> @@ -351,6 +352,21 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
> return iova_reserve_iommu_regions(dev, domain);
> }
>
> +static int handle_deferred_device(struct device *dev,
> + struct iommu_domain *domain)
> +{
> + const struct iommu_ops *ops = domain->ops;
> +
> + if (!is_kdump_kernel())
> + return 0;
> +
> + if (unlikely(ops->is_attach_deferred &&
> + ops->is_attach_deferred(domain, dev)))
> + return iommu_attach_device(domain, dev);
> +
> + return 0;
> +}
> +
> /**
> * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
> * page flags.
> @@ -463,6 +479,9 @@ static dma_addr_t __iommu_dma_map(struct device *dev, phys_addr_t phys,
> size_t iova_off = iova_offset(iovad, phys);
> dma_addr_t iova;
>
> + if (unlikely(handle_deferred_device(dev, domain)))
> + return DMA_MAPPING_ERROR;
> +
> size = iova_align(iovad, size + iova_off);
>
> iova = iommu_dma_alloc_iova(domain, size, dma_get_mask(dev), dev);
iommu_map_atomic() is applied to __iommu_dma_map() in 2/5.
Is it an atomic context currently given the mutex_lock() in
iommu_attach_device()?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* re: clk: actions: Don't reference clk_init_data after registration [bug report]
From: Stephen Boyd @ 2019-08-17 3:44 UTC (permalink / raw)
To: linux-arm-kernel@lists.infradead.org, Andreas Färber,
Colin Ian King, Manivannan Sadhasivam, Michael Turquette,
linux-clk
Cc: linux-kernel@vger.kernel.org
In-Reply-To: <7132b72b-bd73-f53f-8966-a9b88dd444e4@canonical.com>
Quoting Colin Ian King (2019-08-16 15:31:11)
> Hi,
>
> Static analysis with Coverity Scan on linux-next has found an issue with
> the following commit:
Thanks, it's already been fixed.
_______________________________________________
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-17 3:46 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: devicetree, mturquette, linux-kernel, darren.tsao, robh+dt,
haitao.suo, fisher.cheng, alec.lin, linux-clk, linux-arm-kernel
In-Reply-To: <20190817033422.GB14652@Mani-XPS-13-9360>
Quoting Manivannan Sadhasivam (2019-08-16 20:34:22)
> On Wed, Aug 07, 2019 at 10:01:28PM -0700, Stephen Boyd wrote:
> > Quoting Manivannan Sadhasivam (2019-07-05 08:14:36)
> > > +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?
> >
>
> These are all part of the sysctrl block (clock + pinctrl + reset) and the
> register domains got split between system and pll.
>
And that can't be one node that probes the clk, pinctrl, and reset
drivers from C code?
_______________________________________________
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/6] clk: imx8mn: Add necessary frequency support for ARM PLL table
From: Stephen Boyd @ 2019-08-17 3:52 UTC (permalink / raw)
To: Anson.Huang, abel.vesa, devicetree, festevam, kernel,
leonard.crestez, linux-arm-kernel, linux-clk, linux-kernel,
linux-pm, mark.rutland, mturquette, rjw, robh+dt, s.hauer,
shawnguo, viresh.kumar
Cc: Linux-imx
In-Reply-To: <1565866783-19672-5-git-send-email-Anson.Huang@nxp.com>
Quoting Anson.Huang@nxp.com (2019-08-15 03:59:42)
> diff --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c
> index ecd1062..3f1239a 100644
> --- a/drivers/clk/imx/clk-imx8mn.c
> +++ b/drivers/clk/imx/clk-imx8mn.c
> @@ -82,6 +84,7 @@ static struct imx_pll14xx_clk imx8mn_dram_pll = {
> static struct imx_pll14xx_clk imx8mn_arm_pll = {
> .type = PLL_1416X,
> .rate_table = imx8mn_pll1416x_tbl,
> + .rate_count = ARRAY_SIZE(imx8mn_pll1416x_tbl),
Why is rate_count added? That's not described in the commit text.
_______________________________________________
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,V2,1/2] media: dt-bindings: media: i2c: Add bindings for OV02A10
From: Dongchun Zhu @ 2019-08-17 3:53 UTC (permalink / raw)
To: sakari.ailus
Cc: mark.rutland, devicetree, drinkcat, srv_heupstream, menghui.lin,
tfiga, shengnan.wang, louis.kuo, sj.huang, robh+dt,
linux-mediatek, matthias.bgg, bingbu.cao, mchehab,
linux-arm-kernel, linux-media
In-Reply-To: <ef65288c523f405396991bd6d757bba0@mtkmbs02n1.mediatek.inc>
Hi Sakari,
On Tue, 2019-07-23 at 10:41 +0300, Sakari Ailus wrote:
>
> Hi Dongchun,
>
> On Thu, Jul 04, 2019 at 04:46:50PM +0800, dongchun.zhu@mediatek.com wrote:
> > From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> >
> > Add device tree binding documentation for the OV02A10 camera sensor.
> >
> > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > ---
> > .../devicetree/bindings/media/i2c/ov02a10.txt | 43 ++++++++++++++++++++++
> > MAINTAINERS | 7 ++++
> > 2 files changed, 50 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/media/i2c/ov02a10.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov02a10.txt
> > b/Documentation/devicetree/bindings/media/i2c/ov02a10.txt
> > new file mode 100644
> > index 0000000..d40aa87
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov02a10.txt
> > @@ -0,0 +1,43 @@
> > +* Omnivision OV02A10 MIPI CSI-2 sensor
> > +
> > +Required Properties:
> > +- compatible: shall be "ovti,ov02a10"
> > +- clocks: reference to the xvclk input clock
> > +- clock-names: shall be "xvclk"
> > +- avdd-supply: Analog voltage supply, 2.8 volts
> > +- dovdd-supply: Digital I/O voltage supply, 1.8 volts
> > +- dvdd-supply: Digital core voltage supply, 1.8 volts
> > +- reset-gpios: Low active reset gpio
> > +
> > +The device node shall contain one 'port' child node with an
> > +'endpoint' subnode for its digital output video port, in accordance
> > +with the video interface bindings defined in
> > +Documentation/devicetree/bindings/media/video-interfaces.txt.
> > +The endpoint optional property 'data-lanes' shall be "<1>".
>
> How many lanes does the module (or the sensor) have?
>
From sensor datasheet, OV02A10 supports 1-lane MIPI interface.
> > +
> > +Example:
> > +&i2c4 {
> > +ov02a10: camera-sensor@3d {
> > +compatible = "ovti,ov02a10";
> > +reg = <0x3d>;
> > +pinctrl-names = "default";
> > +pinctrl-0 = <&camera_pins_cam1_mclk_on>;
> > +
> > +clocks = <&topckgen CLK_TOP_MUX_CAMTG2>,
> > +<&topckgen CLK_TOP_UNIVP_192M_D8>;
> > +clock-names = "xvclk", "freq_mux";
> > +
> > +avdd-supply = <&mt6358_vcama1_reg>;
> > +dvdd-supply = <&mt6358_vcn18_reg>;
> > +dovdd-supply = <&mt6358_vcamio_reg>;
> > +pwdn-gpios = <&pio 107 1>;
> > +reset-gpios = <&pio 109 1>;
> > +
> > +port {
> > + ov02a10_core: endpoint {
> > + remote-endpoint = <&ov02a10_0>;
>
> Indentation is still wrong. :-(
>
Fixed in next release.
> > + data-lanes = <1>;
> > +};
> > +};
> > +};
> > +};
> > diff --git a/MAINTAINERS b/MAINTAINERS index 5cfbea4..62b81ff 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -11571,6 +11571,13 @@ T:git git://linuxtv.org/media_tree.git
> > S:Maintained
> > F:drivers/media/i2c/ov13858.c
> >
> > +OMNIVISION OV02A10 SENSOR DRIVER
> > +M:Dongchun Zhu <dongchun.zhu@mediatek.com>
> > +L:linux-media@vger.kernel.org
> > +T:git git://linuxtv.org/media_tree.git
> > +S:Maintained
> > +F:Documentation/devicetree/bindings/media/i2c/ov02a10.txt
> > +
> > OMNIVISION OV2680 SENSOR DRIVER
> > M:Rui Miguel Silva <rmfrfs@gmail.com>
> > L:linux-media@vger.kernel.org
>
> --
> Kind regards,
>
> Sakari Ailus
> sakari.ailus@linux.intel.com
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> *********************MEDIATEK Confidential/Internal Use*********************
_______________________________________________
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: Manivannan Sadhasivam @ 2019-08-17 3:55 UTC (permalink / raw)
To: Stephen Boyd
Cc: devicetree, mturquette, linux-kernel, darren.tsao, robh+dt,
haitao.suo, fisher.cheng, alec.lin, linux-clk, linux-arm-kernel
In-Reply-To: <20190808051600.4EF7D2186A@mail.kernel.org>
Hi Stephen,
On Wed, Aug 07, 2019 at 10:15:59PM -0700, Stephen Boyd wrote:
> 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.
>
Okay. I got confused by the fact that Makefile is sorted but not the
Kconfig.
> > +
> > 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()
>
okay.
> > +
> > +#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?
>
Ack.
> > + { 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?
>
Yeah, will add comments for all critical clocks.
> > + 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?
>
Sure, will do it for clocks which doesn't use helper APIs.
> > + 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?
>
What is the benefit? I see that only hw:init is going to be NULL in future.
So, I'll keep it as it is.
> > +}
> > +
> > +void bm1880_clk_unregister_pll(struct clk *clk)
>
> Should this be static?
>
Ack.
> > +{
> > + 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?
>
IMO using helper APIs greatly reduce code size and makes the driver
look more clean. So I prefer to use the helpers wherever applicable.
When you plan to deprecate those, I'll switch over to plain clk_hw APIs.
> > + 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?
>
Ack.
> > + .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.
>
Ack.
> > + 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?
>
Hmm, I looked into the majority of drivers which live under `driver/clk/`.
Most of them are using CLK_OF_DECLARE_DRIVER, so I thought that only drivers
which have a separate directory are preferred by the maintainers to use
platform driver way.
Anyway, I can switch over to platform driver and that's what I prefer.
Thanks,
Mani
_______________________________________________
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: Manivannan Sadhasivam @ 2019-08-17 3:58 UTC (permalink / raw)
To: Stephen Boyd
Cc: devicetree, mturquette, linux-kernel, darren.tsao, robh+dt,
haitao.suo, fisher.cheng, alec.lin, linux-clk, linux-arm-kernel
In-Reply-To: <20190817034612.6DA7E21721@mail.kernel.org>
On Fri, Aug 16, 2019 at 08:46:11PM -0700, Stephen Boyd wrote:
> Quoting Manivannan Sadhasivam (2019-08-16 20:34:22)
> > On Wed, Aug 07, 2019 at 10:01:28PM -0700, Stephen Boyd wrote:
> > > Quoting Manivannan Sadhasivam (2019-07-05 08:14:36)
> > > > +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?
> > >
> >
> > These are all part of the sysctrl block (clock + pinctrl + reset) and the
> > register domains got split between system and pll.
> >
>
> And that can't be one node that probes the clk, pinctrl, and reset
> drivers from C code?
It is not a MFD for sure. It's just grouping of the register domains together.
Thanks,
Mani
>
_______________________________________________
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,V2,1/2] media: dt-bindings: media: i2c: Add bindings for OV02A10
From: Dongchun Zhu @ 2019-08-17 5:03 UTC (permalink / raw)
To: Rob Herring
Cc: mark.rutland, devicetree, drinkcat, srv_heupstream, menghui.lin,
Sakari Ailus, tfiga, shengnan.wang, louis.kuo, sj.huang, robh+dt,
linux-mediatek, matthias.bgg, bingbu.cao, mchehab,
linux-arm-kernel, linux-media
In-Reply-To: <5d5d644af9334609bb7c3ff745b2e754@mtkmbs02n1.mediatek.inc>
Hi Rob,
On Mon, 2019-07-22 at 18:49 -0500, Rob Herring wrote:
>
> On Thu, Jul 04, 2019 at 04:46:50PM +0800, dongchun.zhu@mediatek.com wrote:
> > From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> >
> > Add device tree binding documentation for the OV02A10 camera sensor.
> >
> > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > ---
> > .../devicetree/bindings/media/i2c/ov02a10.txt | 43 ++++++++++++++++++++++
> > MAINTAINERS | 7 ++++
> > 2 files changed, 50 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/media/i2c/ov02a10.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov02a10.txt
> > b/Documentation/devicetree/bindings/media/i2c/ov02a10.txt
> > new file mode 100644
> > index 0000000..d40aa87
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov02a10.txt
> > @@ -0,0 +1,43 @@
> > +* Omnivision OV02A10 MIPI CSI-2 sensor
> > +
> > +Required Properties:
> > +- compatible: shall be "ovti,ov02a10"
> > +- clocks: reference to the xvclk input clock
> > +- clock-names: shall be "xvclk"
> > +- avdd-supply: Analog voltage supply, 2.8 volts
> > +- dovdd-supply: Digital I/O voltage supply, 1.8 volts
> > +- dvdd-supply: Digital core voltage supply, 1.8 volts
> > +- reset-gpios: Low active reset gpio
> > +
> > +The device node shall contain one 'port' child node with an
> > +'endpoint' subnode for its digital output video port, in accordance
> > +with the video interface bindings defined in
> > +Documentation/devicetree/bindings/media/video-interfaces.txt.
> > +The endpoint optional property 'data-lanes' shall be "<1>".
> > +
> > +Example:
> > +&i2c4 {
> > +ov02a10: camera-sensor@3d {
> > +compatible = "ovti,ov02a10";
> > +reg = <0x3d>;
> > +pinctrl-names = "default";
> > +pinctrl-0 = <&camera_pins_cam1_mclk_on>;
> > +
> > +clocks = <&topckgen CLK_TOP_MUX_CAMTG2>,
> > +<&topckgen CLK_TOP_UNIVP_192M_D8>;
> > +clock-names = "xvclk", "freq_mux";
> > +
> > +avdd-supply = <&mt6358_vcama1_reg>;
> > +dvdd-supply = <&mt6358_vcn18_reg>;
> > +dovdd-supply = <&mt6358_vcamio_reg>;
> > +pwdn-gpios = <&pio 107 1>;
>
> Not documented. If you need this, use 'powerdown-gpios' instead.
>
Fixed in next release.
> > +reset-gpios = <&pio 109 1>;
> > +
> > +port {
> > + ov02a10_core: endpoint {
> > + remote-endpoint = <&ov02a10_0>;
> > + data-lanes = <1>;
> > +};
> > +};
> > +};
> > +};
> > diff --git a/MAINTAINERS b/MAINTAINERS index 5cfbea4..62b81ff 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -11571,6 +11571,13 @@ T:git git://linuxtv.org/media_tree.git
> > S:Maintained
> > F:drivers/media/i2c/ov13858.c
> >
> > +OMNIVISION OV02A10 SENSOR DRIVER
> > +M:Dongchun Zhu <dongchun.zhu@mediatek.com>
> > +L:linux-media@vger.kernel.org
> > +T:git git://linuxtv.org/media_tree.git
> > +S:Maintained
> > +F:Documentation/devicetree/bindings/media/i2c/ov02a10.txt
> > +
> > OMNIVISION OV2680 SENSOR DRIVER
> > M:Rui Miguel Silva <rmfrfs@gmail.com>
> > L:linux-media@vger.kernel.org
> > --
> > 2.9.2
> >
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> *********************MEDIATEK Confidential/Internal Use*********************
_______________________________________________
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,V2,2/2] media: i2c: Add Omnivision OV02A10 camera sensor driver
From: Dongchun Zhu @ 2019-08-17 5:35 UTC (permalink / raw)
To: Bingbu Cao
Cc: mark.rutland, devicetree, drinkcat, srv_heupstream, tfiga,
shengnan.wang, louis.kuo, sj.huang, robh+dt, linux-mediatek,
matthias.bgg, bingbu.cao, mchehab, linux-arm-kernel, linux-media
In-Reply-To: <8639e67ad6db40b78e48b00c875187dc@mtkmbs02n1.mediatek.inc>
Hi Bingbu,
On Tue, 2019-07-08 at 15:52 +0800, Bingbu Cao wrote:
> -----Original Message-----
> From: Linux-mediatek [mailto:linux-mediatek-bounces@lists.infradead.org] On Behalf Of Bingbu Cao
> Sent: Monday, July 08, 2019 3:53 PM
> To: Dongchun Zhu (朱东春) <Dongchun.Zhu@mediatek.com>; mchehab@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; sakari.ailus@linux.intel.com; --to=drinkcat@chromium.org; tfiga@chromium.org; matthias.bgg@gmail.com; bingbu.cao@intel.com
> Cc: devicetree@vger.kernel.org; srv_heupstream <srv_heupstream@mediatek.com>; Shengnan Wang (王圣男) <shengnan.wang@mediatek.com>; Sj Huang (黃信璋) <sj.huang@mediatek.com>; linux-mediatek@lists.infradead.org; Louis Kuo (郭德寧) <louis.kuo@mediatek.com>; linux-arm-kernel@lists.infradead.org; linux-media@vger.kernel.org
> Subject: Re: [RFC,V2,2/2] media: i2c: Add Omnivision OV02A10 camera sensor driver
>
>
>
> On 7/4/19 4:46 PM, dongchun.zhu@mediatek.com wrote:
> > From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> >
> > This patch adds a V4L2 sub-device driver for OV02A10 image sensor.
> > The OV02A10 is a 1/5" CMOS sensor from Omnivision.
> > Supports output format: 10-bit Raw.
> > The OV02A10 has a single MIPI lane interface and use the I2C bus for control and the CSI-2 bus for data.
> >
> > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> >
> snip
> > +
> > +/* MIPI color bar enable output */
> > +static const struct regval ov02a10_test_pattern_enable_regs[] = {
> > +{0xfd, 0x01},
> > +{0x0d, 0x00},
> > +{0xb6, 0x01},
> > +{0x01, 0x01},
> > +{0xfd, 0x01},
> > +{0xac, 0x01},
> > +{REG_NULL, 0x00}
> Actually, you can use ARRAY_SIZE to write the regs, the REG_NULL can be removed. However, it is not a problem.
> ...
> snip...
Thanks for kindly reminder.
It would remain, just as OV5695/OV2685 did.
> > +static int ov02a10_set_fmt(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *fmt) {
> > +struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
> > +int ret = 0;
> > +
> > +mutex_lock(&ov02a10->mutex);
> > +
> > +if (ov02a10->streaming) {
> > +ret = -EBUSY;
> > +goto unlock;
> I like return -EBUSY directly after mutex_unlock here and return 0 below.
Got it.
Fixed in next release.
> > +}
> > +
> > +/* only one mode supported */
> > +mbus_fmt->code = ov02a10->fmt.code;
> > +ov02a10_fill_fmt(ov02a10->cur_mode, mbus_fmt);
> > +ov02a10->fmt = fmt->format;
> > +
> > +unlock:
> > +mutex_unlock(&ov02a10->mutex);
> > +
> > +return ret;
> > +}
> > +
> > +static int ov02a10_get_fmt(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *fmt) {
> > +struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
> > +
> > +mutex_lock(&ov02a10->mutex);
> > +
> > +fmt->format = ov02a10->fmt;
> > +mbus_fmt->code = ov02a10->fmt.code;
> > +ov02a10_fill_fmt(ov02a10->cur_mode, mbus_fmt);
> > +
> > +mutex_unlock(&ov02a10->mutex);
> > +
> > +return 0;
> > +}
> > +
> > +static int ov02a10_enum_mbus_code(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_mbus_code_enum *code) {
> > +struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +
> > +if (code->index >= ARRAY_SIZE(supported_modes) || !(code->index))
> > +return -EINVAL;
> > +
> > +code->code = ov02a10->fmt.code;
> > +
> > +return 0;
> > +}
> > +
> > +static int ov02a10_enum_frame_sizes(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_frame_size_enum *fse) {
> > +if (fse->index >= ARRAY_SIZE(supported_modes) || !(fse->index))
> > +return -EINVAL;
> > +
> > +fse->min_width = supported_modes[fse->index].width;
> > +fse->max_width = supported_modes[fse->index].width;
> > +fse->max_height = supported_modes[fse->index].height;
> > +fse->min_height = supported_modes[fse->index].height;
> > +
> > +return 0;
> > +}
> > +
> > +static int __ov02a10_power_on(struct ov02a10 *ov02a10) {
> > +struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > +struct device *dev = &client->dev;
> > +int ret;
> > +
> > +ret = clk_prepare_enable(ov02a10->xvclk);
> > +if (ret < 0) {
> > +dev_err(dev, "Failed to enable xvclk\n");
> > +return ret;
> > +}
> > +
> > +/* note: set 0 is high, set 1 is low */
> > +gpiod_set_value_cansleep(ov02a10->reset_gpio, 1);
> > +gpiod_set_value_cansleep(ov02a10->pwdn_gpio, 0);
> > +
> > +ret = regulator_bulk_enable(OV02A10_NUM_SUPPLIES, ov02a10->supplies);
> > +if (ret < 0) {
> > +dev_err(dev, "Failed to enable regulators\n");
> > +goto disable_clk;
> > +}
> > +msleep_range(7);
> > +
> > +gpiod_set_value_cansleep(ov02a10->pwdn_gpio, 1);
> > +msleep_range(10);
> > +
> > +gpiod_set_value_cansleep(ov02a10->reset_gpio, 0);
> > +msleep_range(10);
> > +
> > +return 0;
> > +
> > +disable_clk:
> > +clk_disable_unprepare(ov02a10->xvclk);
> > +
> > +return ret;
> > +}
> > +
> > +static void __ov02a10_power_off(struct ov02a10 *ov02a10) {
> > +clk_disable_unprepare(ov02a10->xvclk);
> > +gpiod_set_value_cansleep(ov02a10->reset_gpio, 1);
> > +gpiod_set_value_cansleep(ov02a10->pwdn_gpio, 1);
> > +regulator_bulk_disable(OV02A10_NUM_SUPPLIES, ov02a10->supplies); }
> > +
> > +static int __ov02a10_start_stream(struct ov02a10 *ov02a10) {
> > +struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > +int ret;
> > +
> > +/* Apply default values of current mode */
> > +ret = ov02a10_write_array(ov02a10, ov02a10->cur_mode->reg_list);
> > +if (ret)
> > +return ret;
> > +
> > +/* Apply customized values from user */
> > +ret = __v4l2_ctrl_handler_setup(ov02a10->subdev.ctrl_handler);
> > +if (ret)
> > +return ret;
> > +
> > +/* Set Orientation be 180 degree */
> Set orientation to 180 degree
Got it.
Fixed in next release.
> > +if (ov02a10->upside_down) {
> > +ret = ov02a10_write_reg(ov02a10, REG_MIRROR_FLIP_CONTROL,
> > +REG_CONFIG_MIRROR_FLIP);
> > +if (ret) {
> > +dev_err(&client->dev, "%s failed to set orientation\n",
> > +__func__);
> > +return ret;
> > +}
> > +ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> > +REG_ENABLE);
> > +if (ret < 0)
> > +return ret;
> > +}
> > +
> > +/* set stream on register */
> > +return ov02a10_write_reg(ov02a10,
> > + REG_SC_CTRL_MODE, SC_CTRL_MODE_STREAMING); }
> > +
> > +static int __ov02a10_stop_stream(struct ov02a10 *ov02a10) {
> > +return ov02a10_write_reg(ov02a10,
> > + REG_SC_CTRL_MODE, SC_CTRL_MODE_STANDBY); }
> > +
> > +static int ov02a10_entity_init_cfg(struct v4l2_subdev *subdev,
> > + struct v4l2_subdev_pad_config *cfg) {
> > +struct v4l2_subdev_format fmt = { 0 };
> > +
> > +fmt.which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
> > +fmt.format.width = 1600;
> > +fmt.format.height = 1200;
> > +
> > +ov02a10_set_fmt(subdev, cfg, &fmt);
> > +
> > +return 0;
> > +}
> > +
> > +static int ov02a10_s_stream(struct v4l2_subdev *sd, int on) {
> > +struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > +int ret = 0;
> > +
> > +dev_dbg(&client->dev, "ov02a10 s_stream (%d)\n", on);
> > +mutex_lock(&ov02a10->mutex);
> > +
> > +if (ov02a10->streaming == on)
> > +goto unlock_and_return;
> > +
> > +if (on) {
> > +ret = pm_runtime_get_sync(&client->dev);
> > +if (ret < 0) {
> > +pm_runtime_put_noidle(&client->dev);
> > +goto unlock_and_return;
> > +}
> > +
> > +ret = __ov02a10_start_stream(ov02a10);
> > +if (ret) {
> > +__ov02a10_stop_stream(ov02a10);
> > +ov02a10->streaming = !on;
> > +goto err_rpm_put;
> > +}
> > +} else {
> > +__ov02a10_stop_stream(ov02a10);
> > +pm_runtime_put(&client->dev);
> > +}
> > +
> > +ov02a10->streaming = on;
> > +mutex_unlock(&ov02a10->mutex);
> > +
> > +return ret;
> > +
> > +err_rpm_put:
> > +pm_runtime_put(&client->dev);
> > +unlock_and_return:
> > +mutex_unlock(&ov02a10->mutex);
> > +
> > +return ret;
> > +}
> > +
> > +static int ov02a10_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh
> > +*fh) {
> > +struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +struct v4l2_mbus_framefmt *try_fmt = v4l2_subdev_get_try_format(sd,
> > +fh->pad,
> > +0);
> > +
> > +mutex_lock(&ov02a10->mutex);
> > +/* Initialize try_fmt */
> > +try_fmt->code = ov02a10->fmt.code;
> > +ov02a10_fill_fmt(&supported_modes[0], try_fmt);
> > +
> > +mutex_unlock(&ov02a10->mutex);
> > +
> > +return 0;
> > +}
> > +
> > +static int __maybe_unused ov02a10_runtime_resume(struct device *dev)
> > +{
> > +struct i2c_client *client = to_i2c_client(dev);
> > +struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > +struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +
> > +return __ov02a10_power_on(ov02a10);
> > +}
> > +
> > +static int __maybe_unused ov02a10_runtime_suspend(struct device *dev)
> > +{
> > +struct i2c_client *client = to_i2c_client(dev);
> > +struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > +struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +
> > +__ov02a10_power_off(ov02a10);
> > +
> > +return 0;
> > +}
> > +
> > +static const struct dev_pm_ops ov02a10_pm_ops = {
> > +SET_RUNTIME_PM_OPS(ov02a10_runtime_suspend,
> > + ov02a10_runtime_resume, NULL)
> > +};
> > +
> > +static int ov02a10_set_test_pattern(struct ov02a10 *ov02a10, s32
> > +value) {
> > +if (value)
> > +return ov02a10_write_array(ov02a10,
> > + ov02a10_test_pattern_enable_regs);
> > +
> > +return ov02a10_write_array(ov02a10,
> > +ov02a10_test_pattern_disable_regs);
> > +}
> > +
> > +static int ov02a10_set_ctrl(struct v4l2_ctrl *ctrl) {
> > +struct ov02a10 *ov02a10 = container_of(ctrl->handler,
> > + struct ov02a10, ctrl_handler);
> > +struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > +s64 max_expo;
> > +int ret;
> > +
> > +/* Propagate change of current control to all related controls */
> > +if (ctrl->id == V4L2_CID_VBLANK) {
> > +/* Update max exposure while meeting expected vblanking */
> > +max_expo = ov02a10->cur_mode->height + ctrl->val - 4;
> > +__v4l2_ctrl_modify_range(ov02a10->exposure,
> > + ov02a10->exposure->minimum, max_expo,
> > + ov02a10->exposure->step,
> > + ov02a10->exposure->default_value);
> > +}
> > +
> > +/* V4L2 controls values will be applied only when power is already up */
> > +if (!pm_runtime_get_if_in_use(&client->dev))
> > +return 0;
> > +
> > +switch (ctrl->id) {
> > +case V4L2_CID_EXPOSURE:
> > +ret = ov02a10_write_reg(ov02a10, REG_PAGE_SWITCH, REG_ENABLE);
> > +if (ret < 0)
> > +return ret;
> > +ret = ov02a10_write_reg(ov02a10, OV02A10_REG_EXPOSURE_H,
> > +((ctrl->val >> 8) & 0xFF));
> > +if (!ret) {
> > +ret = ov02a10_write_reg(ov02a10, OV02A10_REG_EXPOSURE_L,
> > +(ctrl->val & 0xFF));
> > +if (ret < 0)
> > +return ret;
> > +}
> > +ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> > +REG_ENABLE);
> > +if (ret < 0)
> > +return ret;
> > +break;
> > +case V4L2_CID_ANALOGUE_GAIN:
> > +ret = ov02a10_write_reg(ov02a10, REG_PAGE_SWITCH, REG_ENABLE);
> > +if (ret < 0)
> > +return ret;
> > +ret = ov02a10_write_reg(ov02a10, OV02A10_REG_GAIN,
> > +(ctrl->val & 0xFF));
> > +if (ret < 0)
> > +return ret;
> > +ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> > +REG_ENABLE);
> > +if (ret < 0)
> > +return ret;
> > +break;
> > +case V4L2_CID_VBLANK:
> > +ret = ov02a10_write_reg(ov02a10, REG_PAGE_SWITCH, REG_ENABLE);
> > +if (ret < 0)
> > +return ret;
> > +ret = ov02a10_write_reg(ov02a10, OV02A10_REG_VTS_H,
> > +(((ctrl->val +
> > +ov02a10->cur_mode->height - 1224) >> 8)
> > +& 0xFF));
> 1224 is hard-coded here, could you add some comments? Is it a sensor setting sensitive value?
This would be fixed in next release.
1224 is the basic line for OV02a10 CMOS sensor.
In fact, there is one relationship between framelength and v-blanking.
That is, framelength = basic_line + v-blanking.
Thus we could modify the value of v-blanking register to update
framelength.
> > +if (!ret) {
> > +ret = ov02a10_write_reg(ov02a10, OV02A10_REG_VTS_L,
> > +((ctrl->val +
> > +ov02a10->cur_mode->height -
> > +1224) & 0xFF));
> ditto.
Fixed in next release.
> > +if (ret < 0)
> > +return ret;
> > +}
> > +ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> > +REG_ENABLE);
> > +if (ret < 0)
> > +return ret;
> > +break;
> > +case V4L2_CID_TEST_PATTERN:
> > +ret = ov02a10_set_test_pattern(ov02a10, ctrl->val);
> > +if (ret < 0)
> > +return ret;
> > +break;
> > +default:
> > +dev_warn(&client->dev, "%s Unhandled id:0x%x, val:0x%x\n",
> > + __func__, ctrl->id, ctrl->val);
> > +ret = -EINVAL;
> > +break;
> > +};
> > +
> > +pm_runtime_put(&client->dev);
> > +
> > +return ret;
> > +}
> > +
> > +static const struct v4l2_subdev_video_ops ov02a10_video_ops = {
> > +.s_stream = ov02a10_s_stream,
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops ov02a10_pad_ops = {
> > +.init_cfg = ov02a10_entity_init_cfg,
> > +.enum_mbus_code = ov02a10_enum_mbus_code,
> > +.enum_frame_size = ov02a10_enum_frame_sizes,
> > +.get_fmt = ov02a10_get_fmt,
> > +.set_fmt = ov02a10_set_fmt,
> > +};
> > +
> > +static const struct v4l2_subdev_ops ov02a10_subdev_ops = {
> > +.video= &ov02a10_video_ops,
> > +.pad= &ov02a10_pad_ops,
> > +};
> > +
> > +static const struct media_entity_operations ov02a10_subdev_entity_ops = {
> > +.link_validate = v4l2_subdev_link_validate, };
> > +
> > +static const struct v4l2_subdev_internal_ops ov02a10_internal_ops = {
> > +.open = ov02a10_open,
> > +};
> > +
> > +static const struct v4l2_ctrl_ops ov02a10_ctrl_ops = {
> > +.s_ctrl = ov02a10_set_ctrl,
> > +};
> > +
> > +static int ov02a10_initialize_controls(struct ov02a10 *ov02a10) {
> > +struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > +const struct ov02a10_mode *mode;
> > +struct v4l2_ctrl_handler *handler;
> > +struct v4l2_ctrl *ctrl;
> > +u64 exposure_max;
> > +u32 pixel_rate, h_blank;
> > +int ret;
> > +
> > +handler = &ov02a10->ctrl_handler;
> > +mode = ov02a10->cur_mode;
> > +ret = v4l2_ctrl_handler_init(handler, 10);
> > +if (ret)
> > +return ret;
> > +handler->lock = &ov02a10->mutex;
> > +
> > +ctrl = v4l2_ctrl_new_int_menu(handler, NULL, V4L2_CID_LINK_FREQ,
> > + 0, 0, link_freq_menu_items);
> > +if (ctrl)
> > +ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > +pixel_rate = (link_freq_menu_items[0] * 2 * OV02A10_LANES) /
> > + OV02A10_BITS_PER_SAMPLE;
> > +v4l2_ctrl_new_std(handler, NULL, V4L2_CID_PIXEL_RATE,
> > + 0, pixel_rate, 1, pixel_rate);
> > +
> > +h_blank = mode->hts_def - mode->width;
> > +ov02a10->hblank = v4l2_ctrl_new_std(handler, NULL, V4L2_CID_HBLANK,
> > + h_blank, h_blank, 1, h_blank);
> > +if (ov02a10->hblank)
> > +ov02a10->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > +ov02a10->vblank = v4l2_ctrl_new_std(handler, &ov02a10_ctrl_ops,
> > + V4L2_CID_VBLANK, mode->vts_def -
> > + mode->height,
> > + OV02A10_VTS_MAX - mode->height, 1,
> > + mode->vts_def - mode->height);
> > +
> > +exposure_max = mode->vts_def - 4;
> > +ov02a10->exposure = v4l2_ctrl_new_std(handler, &ov02a10_ctrl_ops,
> > + V4L2_CID_EXPOSURE,
> > + OV02A10_EXPOSURE_MIN,
> > + exposure_max,
> > + OV02A10_EXPOSURE_STEP,
> > + mode->exp_def);
> > +
> > +ov02a10->anal_gain = v4l2_ctrl_new_std(handler, &ov02a10_ctrl_ops,
> > + V4L2_CID_ANALOGUE_GAIN,
> > + OV02A10_GAIN_MIN,
> > + OV02A10_GAIN_MAX,
> > + OV02A10_GAIN_STEP,
> > + OV02A10_GAIN_DEFAULT);
> > +
> > +ov02a10->test_pattern =
> > + v4l2_ctrl_new_std_menu_items(handler,
> > +&ov02a10_ctrl_ops,
> > +V4L2_CID_TEST_PATTERN,
> > +ARRAY_SIZE(ov02a10_test_pattern_menu) -
> > +1, 0, 0, ov02a10_test_pattern_menu);
> > +
> > +if (handler->error) {
> > +ret = handler->error;
> > +dev_err(&client->dev,
> > +"Failed to init controls(%d)\n", ret);
> > +goto err_free_handler;
> > +}
> > +
> > +ov02a10->subdev.ctrl_handler = handler;
> > +
> > +return 0;
> > +
> > +err_free_handler:
> > +v4l2_ctrl_handler_free(handler);
> > +
> > +return ret;
> > +}
> > +
> > +static int ov02a10_check_sensor_id(struct ov02a10 *ov02a10) {
> > +struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > +u16 id;
> > +u8 pid = 0;
> > +u8 ver = 0;
> > +int ret;
> > +
> > +/* Check sensor revision */
> > +ret = ov02a10_read_reg(ov02a10, OV02A10_REG_CHIP_ID_H, &pid);
> > +if (ret)
> > +return ret;
> > +
> > +ret = ov02a10_read_reg(ov02a10, OV02A10_REG_CHIP_ID_L, &ver);
> > +if (ret)
> > +return ret;
> > +
> > +id = OV02A10_ID(pid, ver);
> > +if (id != CHIP_ID) {
> > +dev_err(&client->dev, "Unexpected sensor id(%04x)\n", id);
> > +return ret;
> > +}
> > +dev_dbg(&client->dev, "Detected OV%04X sensor\n", id);
> > +
> > +return 0;
> > +}
> > +
> > +static int ov02a10_configure_regulators(struct ov02a10 *ov02a10) {
> > +struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > +unsigned int i;
> > +
> > +for (i = 0; i < OV02A10_NUM_SUPPLIES; i++)
> > +ov02a10->supplies[i].supply = ov02a10_supply_names[i];
> > +
> > +return devm_regulator_bulk_get(&client->dev,
> > + OV02A10_NUM_SUPPLIES,
> > + ov02a10->supplies);
> > +}
> > +
> > +static int ov02a10_probe(struct i2c_client *client) {
> > +struct device *dev = &client->dev;
> > +struct ov02a10 *ov02a10;
> > +u32 rotation;
> > +u32 xclk_freq;
> > +int ret;
> > +
> > +dev_dbg(dev, "ov02a10 probe\n");
> I think this line is not needed at all.
Removed in next release.
> > +ov02a10 = devm_kzalloc(dev, sizeof(*ov02a10), GFP_KERNEL);
> > +if (!ov02a10)
> > +return -ENOMEM;
> > +
> > +v4l2_i2c_subdev_init(&ov02a10->subdev, client, &ov02a10_subdev_ops);
> > +ov02a10->fmt.code = MEDIA_BUS_FMT_SBGGR10_1X10;
> > +
> > +/* optional indication of physical rotation of sensor */
> > +ret = fwnode_property_read_u32(dev_fwnode(dev), "rotation",
> > + &rotation);
> > +if (!ret) {
> > +switch (rotation) {
> > +case 180:
> > +ov02a10->upside_down = true;
> > +ov02a10->fmt.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> > +break;
> > +case 0:
> > +break;
> > +default:
> > +dev_warn(dev, "%u degrees rotation is not supported, ignoring...\n",
> > + rotation);
> > +}
> > +}
> > +
> > +/* get system clock (xvclk) */
> > +ov02a10->xvclk = devm_clk_get(dev, "xvclk");
> > +if (IS_ERR(ov02a10->xvclk)) {
> > +dev_err(dev, "Failed to get xvclk\n");
> > +return -EINVAL;
> > +}
> > +
> > +ret = of_property_read_u32(dev->of_node, "clock-frequency", &xclk_freq);
> > +if (ret) {
> > +dev_err(dev, "Failed to get xclk frequency\n");
> > +return ret;
> > +}
> > +
> > +/* external clock must be 24MHz, allow 1% tolerance */
> > +if (xclk_freq < 23760000 || xclk_freq > 24240000) {
> > +dev_err(dev, "external clock frequency %u is not supported\n",
> > +xclk_freq);
> > +return -EINVAL;
> > +}
> > +dev_dbg(dev, "external clock frequency %u\n", xclk_freq);
> > +
> > +ret = clk_set_rate(ov02a10->xvclk, xclk_freq);
> > +if (ret) {
> > +dev_err(dev, "Failed to set xvclk frequency (24MHz)\n");
> > +return ret;
> > +}
> > +
> > +ov02a10->pwdn_gpio = devm_gpiod_get(dev, "pwdn", GPIOD_OUT_LOW);
> > +if (IS_ERR(ov02a10->pwdn_gpio)) {
> > +dev_err(dev, "Failed to get powerdown-gpios\n");
> > +return -EINVAL;
> > +}
> > +
> > +ov02a10->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> > +if (IS_ERR(ov02a10->reset_gpio)) {
> > +dev_err(dev, "Failed to get reset-gpios\n");
> > +return -EINVAL;
> > +}
> > +
> > +ret = ov02a10_configure_regulators(ov02a10);
> > +if (ret) {
> > +dev_err(dev, "Failed to get power regulators\n");
> > +return ret;
> > +}
> > +
> > +mutex_init(&ov02a10->mutex);
> > +ov02a10->cur_mode = &supported_modes[0];
> > +ret = ov02a10_initialize_controls(ov02a10);
> > +if (ret) {
> > +dev_err(dev, "Failed to initialize controls\n");
> > +goto err_destroy_mutex;
> > +}
> > +
> > +ret = __ov02a10_power_on(ov02a10);
> > +if (ret)
> > +goto err_free_handler;
> > +
> > +ret = ov02a10_check_sensor_id(ov02a10);
> > +if (ret)
> > +goto err_power_off;
> > +
> > +ov02a10->subdev.internal_ops = &ov02a10_internal_ops;
> > +ov02a10->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > +ov02a10->subdev.entity.ops = &ov02a10_subdev_entity_ops;
> > +ov02a10->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > +ov02a10->pad.flags = MEDIA_PAD_FL_SOURCE;
> > +ret = media_entity_pads_init(&ov02a10->subdev.entity, 1, &ov02a10->pad);
> > +if (ret < 0) {
> > +dev_err(dev, "failed to init entity pads: %d", ret);
> > +goto err_power_off;
> > +}
> > +
> > +ret = v4l2_async_register_subdev(&ov02a10->subdev);
> > +if (ret) {
> > +dev_err(dev, "failed to register V4L2 subdev: %d",
> > +ret);
> > +goto err_clean_entity;
> > +}
> > +
> > +pm_runtime_set_active(dev);
> > +pm_runtime_enable(dev);
> > +pm_runtime_idle(dev);
> > +
> > +return 0;
> > +
> > +err_clean_entity:
> > +media_entity_cleanup(&ov02a10->subdev.entity);
> > +err_power_off:
> > +__ov02a10_power_off(ov02a10);
> > +err_free_handler:
> > +v4l2_ctrl_handler_free(ov02a10->subdev.ctrl_handler);
> > +err_destroy_mutex:
> > +mutex_destroy(&ov02a10->mutex);
> > +
> > +return ret;
> > +}
> > +
> > +static int ov02a10_remove(struct i2c_client *client) {
> > +struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > +struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +
> > +v4l2_async_unregister_subdev(sd);
> > +media_entity_cleanup(&sd->entity);
> > +v4l2_ctrl_handler_free(sd->ctrl_handler);
> > +pm_runtime_disable(&client->dev);
> > +if (!pm_runtime_status_suspended(&client->dev))
> > +__ov02a10_power_off(ov02a10);
> > +pm_runtime_set_suspended(&client->dev);
> > +mutex_destroy(&ov02a10->mutex);
> > +
> > +return 0;
> > +}
> > +
> > +#if IS_ENABLED(CONFIG_OF)
> > +static const struct of_device_id ov02a10_of_match[] = {
> > +{ .compatible = "ovti,ov02a10" },
> > +{},
> > +};
> > +MODULE_DEVICE_TABLE(of, ov02a10_of_match); #endif
> > +
> > +static struct i2c_driver ov02a10_i2c_driver = {
> > +.driver = {
> > +.name = "ov02a10",
> > +.pm = &ov02a10_pm_ops,
> > +.of_match_table = ov02a10_of_match,
> > +},
> > +.probe_new= &ov02a10_probe,
> > +.remove= &ov02a10_remove,
> > +};
> > +
> > +module_i2c_driver(ov02a10_i2c_driver);
> > +
> > +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> > +MODULE_DESCRIPTION("OmniVision OV02A10 sensor driver");
> > +MODULE_LICENSE("GPL v2");
> > +
> >
> Other part looks good to me.
Many thanks for your review.
I really appreciate the effort you all made for the work.
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> *********************MEDIATEK Confidential/Internal Use*********************
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [Xen-devel] [PATCH 07/11] swiotlb-xen: provide a single page-coherent.h header
From: Christoph Hellwig @ 2019-08-17 6:50 UTC (permalink / raw)
To: Julien Grall
Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, x86, linux-kernel,
iommu, xen-devel, Christoph Hellwig, linux-arm-kernel
In-Reply-To: <9a3261c6-5d92-cf6b-1ae8-3a8e8b5ef0d4@arm.com>
On Fri, Aug 16, 2019 at 11:40:43PM +0100, Julien Grall wrote:
> I am not sure I agree with this rename. The implementation of the helpers
> are very Arm specific as this is assuming Dom0 is 1:1 mapped.
>
> This was necessary due to the lack of IOMMU on Arm platforms back then.
> But this is now a pain to get rid of it on newer platform...
So if you look at the final version of the header after the whole
series, what assumes a 1:1 mapping? It all just is
if (pfn_valid())
local cache sync;
else
call into the arch code;
are you concerned that the local cache sync might have to be split
up more for a non-1:1 map in that case? We could just move
the xen_dma_* routines into the arch instead of __xen_dma, but it
really helps to have a common interface header.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox