* [PATCH 3/4] arm64: Allow hw watchpoint of length 3,5,6 and 7
From: Yao Qi @ 2016-10-12 11:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <d6ebb3e9776154da4c1a41f17446e9e31c891a7d.1476251587.git.panand@redhat.com>
On Wed, Oct 12, 2016 at 6:58 AM, Pratyush Anand <panand@redhat.com> wrote:
> Since, arm64 can support all offset within a double word limit. Therefore,
> now support other lengths within that range as well.
How does ptracer (like GDB) detect kernel has already supported all byte
address select values? I suppose ptrace(NT_ARM_HW_WATCH, ) with
len is 3 or 5 fail on current kernel but is of success after your patches
applied.
GDB is aware of the byte address select limitation in kernel, so it always
sets 1,2,4,8 in len in ctrl. GDB needs to know whether the limitation is still
there or not.
--
Yao (??)
^ permalink raw reply
* [PATCH 0/2] ARM: at91: properly handle LPDDR poweroff
From: Alexandre Belloni @ 2016-10-12 11:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1791580.DIVsjjyavb@ws-stein>
On 10/10/2016 at 08:00:30 +0200, Alexander Stein wrote :
> Hi Alexandre,
>
> On Friday 07 October 2016 18:34:25, Alexandre Belloni wrote:
> > Hi,
> >
> > This patch set improves LPDDR support on SoCs using the Atmel MPDDR
> > controller.
> >
> > LPDDR memoris can only handle up to 400 uncontrolled power offs in their
> > life. The proper power off sequence has to be applied before shutting down
> > the SoC.
> >
> > I'm not too happy with the code duplication but this is a design choice
> > that has been made before because both shitdown controler are really
> ^^^^^^^^
>
> I guess you mean shutdown? :) Same for the suject of 2nd patch.
>
I'm planning to send a v2, hopefully fixing my typos ;)
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v3 0/5] arm64/mm: use the contiguous attribute for kernel mappings
From: Ard Biesheuvel @ 2016-10-12 11:23 UTC (permalink / raw)
To: linux-arm-kernel
This 5-piece series is a followup to the single patch 'arm64: mmu: set the
contiguous for kernel mappings when appropriate' sent out on the 10th [0].
Changes in v3:
- add support for contiguous PMDs for all granule sizes (not just 16k)
- add a separate patch to deal with contiguous PUDs (4k granule only), and
contiguous PMDs for 2 levels of translation (which requires special handling)
- avoid pmd_none/pud_none in the BUG() statements in patch #1, since they
may resolve in unexpected ways with folded PMDs/PUDs
Version v2 [1] addressed the following issues:
- the contiguous attribute is also useful for contigous PMD mappings on 16k
granule kernels (i.e., 1 GB blocks)
- the function parameter 'block_mappings_allowed' does not clearly convey
whether contiguous page mappings should be used, so it is renamed to
'page_mappings_only', and its meaning inverted
- instead of BUGging on changes in the PTE_CONT attribute in PMD or PTE entries
that have been populated already, BUG on any modification except for
permission attributes, which don't require break-before-make when changed.
[0] http://marc.info/?l=linux-arm-kernel&m=147612332130714
[1] http://marc.info/?l=linux-arm-kernel&m=147618975314593
An example memory map from a Seattle system with 64 GB running a 4k/3 levels
kernel with KASLR enabled is included below.
Ard Biesheuvel (5):
arm64: mm: BUG on unsupported manipulations of live kernel mappings
arm64: mm: replace 'block_mappings_allowed' with 'page_mappings_only'
arm64: mm: set the contiguous bit for kernel mappings where
appropriate
arm64: mm: support additional contiguous kernel mapping region sizes
arm64: mm: round memstart_addr to contiguous PUD/PMD size
arch/arm64/include/asm/kernel-pgtable.h | 11 +-
arch/arm64/include/asm/mmu.h | 2 +-
arch/arm64/include/asm/pgtable-hwdef.h | 6 +
arch/arm64/kernel/efi.c | 8 +-
arch/arm64/mm/mmu.c | 173 ++++++++++++++------
5 files changed, 140 insertions(+), 60 deletions(-)
--
2.7.4
// Kernel mapping
0xffffff8a70100000-0xffffff8a70200000 1M PTE ro x SHD AF CON
0xffffff8a70200000-0xffffff8a70600000 4M PMD ro x SHD AF BLK
0xffffff8a70600000-0xffffff8a706d0000 832K PTE ro x SHD AF CON
0xffffff8a706d0000-0xffffff8a70920000 2368K PTE ro NX SHD AF CON
0xffffff8a70a90000-0xffffff8a70b40000 704K PTE RW NX SHD AF CON
0xffffff8a70b40000-0xffffff8a70b44000 16K PTE RW NX SHD AF
...
---[ Linear Mapping ]---
0xffffffcc00e80000-0xffffffcc01000000 1536K PTE RW NX SHD AF CON
0xffffffcc01000000-0xffffffcc02000000 16M PMD RW NX SHD AF BLK
0xffffffcc02000000-0xffffffcc40000000 992M PMD RW NX SHD AF CON BLK
0xffffffcc40000000-0xffffffd180000000 21G PGD RW NX SHD AF BLK
0xffffffd180000000-0xffffffd198000000 384M PMD RW NX SHD AF CON BLK
0xffffffd198000000-0xffffffd199200000 18M PMD RW NX SHD AF BLK
0xffffffd199200000-0xffffffd199300000 1M PTE RW NX SHD AF CON
0xffffffd199300000-0xffffffd199400000 1M PTE ro NX SHD AF CON
0xffffffd199400000-0xffffffd199a00000 6M PMD ro NX SHD AF BLK
0xffffffd199a00000-0xffffffd199b20000 1152K PTE ro NX SHD AF CON
0xffffffd199b20000-0xffffffd199c00000 896K PTE RW NX SHD AF CON
0xffffffd199c00000-0xffffffd19a000000 4M PMD RW NX SHD AF BLK
0xffffffd19a000000-0xffffffd1c0000000 608M PMD RW NX SHD AF CON BLK
0xffffffd1c0000000-0xffffffd400000000 9G PGD RW NX SHD AF BLK
0xffffffd400000000-0xffffffd800000000 16G PGD RW NX SHD AF CON BLK
0xffffffd800000000-0xffffffdbc0000000 15G PGD RW NX SHD AF BLK
0xffffffdbc0000000-0xffffffdbf8000000 896M PMD RW NX SHD AF CON BLK
0xffffffdbf8000000-0xffffffdbf8600000 6M PMD RW NX SHD AF BLK
0xffffffdbf8600000-0xffffffdbf8700000 1M PTE RW NX SHD AF CON
0xffffffdbf87a0000-0xffffffdbf8800000 384K PTE RW NX SHD AF CON
0xffffffdbf8800000-0xffffffdbfb800000 48M PMD RW NX SHD AF BLK
0xffffffdbfb800000-0xffffffdbfb830000 192K PTE RW NX SHD AF CON
0xffffffdbfbc10000-0xffffffdbfbcd0000 768K PTE RW NX SHD AF CON
0xffffffdbfc000000-0xffffffdbfe000000 32M PMD RW NX SHD AF CON BLK
0xffffffdbfe000000-0xffffffdbffe00000 30M PMD RW NX SHD AF BLK
0xffffffdbffe00000-0xffffffdbfffd0000 1856K PTE RW NX SHD AF CON
0xffffffdbffff0000-0xffffffdc00000000 64K PTE RW NX SHD AF CON
^ permalink raw reply
* [PATCH v3 1/5] arm64: mm: BUG on unsupported manipulations of live kernel mappings
From: Ard Biesheuvel @ 2016-10-12 11:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476271425-19401-1-git-send-email-ard.biesheuvel@linaro.org>
Now that we take care not manipulate the live kernel page tables in a
way that may lead to TLB conflicts, the case where a table mapping is
replaced by a block mapping can no longer occur. So remove the handling
of this at the PUD and PMD levels, and instead, BUG() on any occurrence
of live kernel page table manipulations that modify anything other than
the permission bits.
Since mark_rodata_ro() is the only caller where the kernel mappings that
are being manipulated are actually live, drop the various conditional
flush_tlb_all() invocations, and add a single call to mark_rodata_ro()
instead.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/mm/mmu.c | 68 ++++++++++++--------
1 file changed, 41 insertions(+), 27 deletions(-)
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 05615a3fdc6f..e1c34e5a1d7d 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -28,8 +28,6 @@
#include <linux/memblock.h>
#include <linux/fs.h>
#include <linux/io.h>
-#include <linux/slab.h>
-#include <linux/stop_machine.h>
#include <asm/barrier.h>
#include <asm/cputype.h>
@@ -95,6 +93,12 @@ static phys_addr_t __init early_pgtable_alloc(void)
return phys;
}
+/*
+ * The following mapping attributes may be updated in live
+ * kernel mappings without the need for break-before-make.
+ */
+static const pteval_t modifiable_attr_mask = PTE_PXN | PTE_RDONLY | PTE_WRITE;
+
static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
unsigned long end, unsigned long pfn,
pgprot_t prot,
@@ -115,8 +119,18 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
pte = pte_set_fixmap_offset(pmd, addr);
do {
+ pte_t old_pte = *pte;
+
set_pte(pte, pfn_pte(pfn, prot));
pfn++;
+
+ /*
+ * After the PTE entry has been populated once, we
+ * only allow updates to the permission attributes.
+ */
+ BUG_ON(pte_val(old_pte) != 0 &&
+ ((pte_val(old_pte) ^ pte_val(*pte)) &
+ ~modifiable_attr_mask) != 0);
} while (pte++, addr += PAGE_SIZE, addr != end);
pte_clear_fixmap();
@@ -146,27 +160,28 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
pmd = pmd_set_fixmap_offset(pud, addr);
do {
+ pmd_t old_pmd = *pmd;
+
next = pmd_addr_end(addr, end);
+
/* try section mapping first */
if (((addr | next | phys) & ~SECTION_MASK) == 0 &&
allow_block_mappings) {
- pmd_t old_pmd =*pmd;
pmd_set_huge(pmd, phys, prot);
+
/*
- * Check for previous table entries created during
- * boot (__create_page_tables) and flush them.
+ * After the PMD entry has been populated once, we
+ * only allow updates to the permission attributes.
*/
- if (!pmd_none(old_pmd)) {
- flush_tlb_all();
- if (pmd_table(old_pmd)) {
- phys_addr_t table = pmd_page_paddr(old_pmd);
- if (!WARN_ON_ONCE(slab_is_available()))
- memblock_free(table, PAGE_SIZE);
- }
- }
+ BUG_ON(pmd_val(old_pmd) != 0 &&
+ ((pmd_val(old_pmd) ^ pmd_val(*pmd)) &
+ ~modifiable_attr_mask) != 0);
} else {
alloc_init_pte(pmd, addr, next, __phys_to_pfn(phys),
prot, pgtable_alloc);
+
+ BUG_ON(pmd_val(old_pmd) != 0 &&
+ pmd_val(old_pmd) != pmd_val(*pmd));
}
phys += next - addr;
} while (pmd++, addr = next, addr != end);
@@ -204,33 +219,29 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
pud = pud_set_fixmap_offset(pgd, addr);
do {
+ pud_t old_pud = *pud;
+
next = pud_addr_end(addr, end);
/*
* For 4K granule only, attempt to put down a 1GB block
*/
if (use_1G_block(addr, next, phys) && allow_block_mappings) {
- pud_t old_pud = *pud;
pud_set_huge(pud, phys, prot);
/*
- * If we have an old value for a pud, it will
- * be pointing to a pmd table that we no longer
- * need (from swapper_pg_dir).
- *
- * Look up the old pmd table and free it.
+ * After the PUD entry has been populated once, we
+ * only allow updates to the permission attributes.
*/
- if (!pud_none(old_pud)) {
- flush_tlb_all();
- if (pud_table(old_pud)) {
- phys_addr_t table = pud_page_paddr(old_pud);
- if (!WARN_ON_ONCE(slab_is_available()))
- memblock_free(table, PAGE_SIZE);
- }
- }
+ BUG_ON(pud_val(old_pud) != 0 &&
+ ((pud_val(old_pud) ^ pud_val(*pud)) &
+ ~modifiable_attr_mask) != 0);
} else {
alloc_init_pmd(pud, addr, next, phys, prot,
pgtable_alloc, allow_block_mappings);
+
+ BUG_ON(pud_val(old_pud) != 0 &&
+ pud_val(old_pud) != pud_val(*pud));
}
phys += next - addr;
} while (pud++, addr = next, addr != end);
@@ -396,6 +407,9 @@ void mark_rodata_ro(void)
section_size = (unsigned long)__init_begin - (unsigned long)__start_rodata;
create_mapping_late(__pa(__start_rodata), (unsigned long)__start_rodata,
section_size, PAGE_KERNEL_RO);
+
+ /* flush the TLBs after updating live kernel mappings */
+ flush_tlb_all();
}
static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
--
2.7.4
^ permalink raw reply related
* [PATCH v3 2/5] arm64: mm: replace 'block_mappings_allowed' with 'page_mappings_only'
From: Ard Biesheuvel @ 2016-10-12 11:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476271425-19401-1-git-send-email-ard.biesheuvel@linaro.org>
In preparation of adding support for contiguous PTE and PMD mappings,
let's replace 'block_mappings_allowed' with 'page_mappings_only', which
will be a more accurate description of the nature of the setting once we
add such contiguous mappings into the mix.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/include/asm/mmu.h | 2 +-
arch/arm64/kernel/efi.c | 8 ++---
arch/arm64/mm/mmu.c | 32 ++++++++++----------
3 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
index 8d9fce037b2f..a81454ad5455 100644
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -34,7 +34,7 @@ extern void __iomem *early_io_map(phys_addr_t phys, unsigned long virt);
extern void init_mem_pgprot(void);
extern void create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
unsigned long virt, phys_addr_t size,
- pgprot_t prot, bool allow_block_mappings);
+ pgprot_t prot, bool page_mappings_only);
extern void *fixmap_remap_fdt(phys_addr_t dt_phys);
#endif
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index ba9bee389fd5..5d17f377d905 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -62,8 +62,8 @@ struct screen_info screen_info __section(.data);
int __init efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md)
{
pteval_t prot_val = create_mapping_protection(md);
- bool allow_block_mappings = (md->type != EFI_RUNTIME_SERVICES_CODE &&
- md->type != EFI_RUNTIME_SERVICES_DATA);
+ bool page_mappings_only = (md->type == EFI_RUNTIME_SERVICES_CODE ||
+ md->type == EFI_RUNTIME_SERVICES_DATA);
if (!PAGE_ALIGNED(md->phys_addr) ||
!PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT)) {
@@ -76,12 +76,12 @@ int __init efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md)
* from the MMU routines. So avoid block mappings altogether in
* that case.
*/
- allow_block_mappings = false;
+ page_mappings_only = true;
}
create_pgd_mapping(mm, md->phys_addr, md->virt_addr,
md->num_pages << EFI_PAGE_SHIFT,
- __pgprot(prot_val | PTE_NG), allow_block_mappings);
+ __pgprot(prot_val | PTE_NG), page_mappings_only);
return 0;
}
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index e1c34e5a1d7d..bf1d71b62c4f 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -139,7 +139,7 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
phys_addr_t phys, pgprot_t prot,
phys_addr_t (*pgtable_alloc)(void),
- bool allow_block_mappings)
+ bool page_mappings_only)
{
pmd_t *pmd;
unsigned long next;
@@ -166,7 +166,7 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
/* try section mapping first */
if (((addr | next | phys) & ~SECTION_MASK) == 0 &&
- allow_block_mappings) {
+ !page_mappings_only) {
pmd_set_huge(pmd, phys, prot);
/*
@@ -204,7 +204,7 @@ static inline bool use_1G_block(unsigned long addr, unsigned long next,
static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
phys_addr_t phys, pgprot_t prot,
phys_addr_t (*pgtable_alloc)(void),
- bool allow_block_mappings)
+ bool page_mappings_only)
{
pud_t *pud;
unsigned long next;
@@ -226,7 +226,7 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
/*
* For 4K granule only, attempt to put down a 1GB block
*/
- if (use_1G_block(addr, next, phys) && allow_block_mappings) {
+ if (use_1G_block(addr, next, phys) && !page_mappings_only) {
pud_set_huge(pud, phys, prot);
/*
@@ -238,7 +238,7 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
~modifiable_attr_mask) != 0);
} else {
alloc_init_pmd(pud, addr, next, phys, prot,
- pgtable_alloc, allow_block_mappings);
+ pgtable_alloc, page_mappings_only);
BUG_ON(pud_val(old_pud) != 0 &&
pud_val(old_pud) != pud_val(*pud));
@@ -253,7 +253,7 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
unsigned long virt, phys_addr_t size,
pgprot_t prot,
phys_addr_t (*pgtable_alloc)(void),
- bool allow_block_mappings)
+ bool page_mappings_only)
{
unsigned long addr, length, end, next;
pgd_t *pgd = pgd_offset_raw(pgdir, virt);
@@ -273,7 +273,7 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
do {
next = pgd_addr_end(addr, end);
alloc_init_pud(pgd, addr, next, phys, prot, pgtable_alloc,
- allow_block_mappings);
+ page_mappings_only);
phys += next - addr;
} while (pgd++, addr = next, addr != end);
}
@@ -302,17 +302,17 @@ static void __init create_mapping_noalloc(phys_addr_t phys, unsigned long virt,
&phys, virt);
return;
}
- __create_pgd_mapping(init_mm.pgd, phys, virt, size, prot, NULL, true);
+ __create_pgd_mapping(init_mm.pgd, phys, virt, size, prot, NULL, false);
}
void __init create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
unsigned long virt, phys_addr_t size,
- pgprot_t prot, bool allow_block_mappings)
+ pgprot_t prot, bool page_mappings_only)
{
BUG_ON(mm == &init_mm);
__create_pgd_mapping(mm->pgd, phys, virt, size, prot,
- pgd_pgtable_alloc, allow_block_mappings);
+ pgd_pgtable_alloc, page_mappings_only);
}
static void create_mapping_late(phys_addr_t phys, unsigned long virt,
@@ -325,7 +325,7 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,
}
__create_pgd_mapping(init_mm.pgd, phys, virt, size, prot,
- NULL, !debug_pagealloc_enabled());
+ NULL, debug_pagealloc_enabled());
}
static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end)
@@ -343,7 +343,7 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end
__create_pgd_mapping(pgd, start, __phys_to_virt(start),
end - start, PAGE_KERNEL,
early_pgtable_alloc,
- !debug_pagealloc_enabled());
+ debug_pagealloc_enabled());
return;
}
@@ -356,13 +356,13 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end
__phys_to_virt(start),
kernel_start - start, PAGE_KERNEL,
early_pgtable_alloc,
- !debug_pagealloc_enabled());
+ debug_pagealloc_enabled());
if (kernel_end < end)
__create_pgd_mapping(pgd, kernel_end,
__phys_to_virt(kernel_end),
end - kernel_end, PAGE_KERNEL,
early_pgtable_alloc,
- !debug_pagealloc_enabled());
+ debug_pagealloc_enabled());
/*
* Map the linear alias of the [_text, __init_begin) interval as
@@ -372,7 +372,7 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end
*/
__create_pgd_mapping(pgd, kernel_start, __phys_to_virt(kernel_start),
kernel_end - kernel_start, PAGE_KERNEL_RO,
- early_pgtable_alloc, !debug_pagealloc_enabled());
+ early_pgtable_alloc, debug_pagealloc_enabled());
}
static void __init map_mem(pgd_t *pgd)
@@ -422,7 +422,7 @@ static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
BUG_ON(!PAGE_ALIGNED(size));
__create_pgd_mapping(pgd, pa_start, (unsigned long)va_start, size, prot,
- early_pgtable_alloc, !debug_pagealloc_enabled());
+ early_pgtable_alloc, debug_pagealloc_enabled());
vma->addr = va_start;
vma->phys_addr = pa_start;
--
2.7.4
^ permalink raw reply related
* [PATCH v3 3/5] arm64: mm: set the contiguous bit for kernel mappings where appropriate
From: Ard Biesheuvel @ 2016-10-12 11:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476271425-19401-1-git-send-email-ard.biesheuvel@linaro.org>
Now that we no longer allow live kernel PMDs to be split, it is safe to
start using the contiguous bit for kernel mappings. So set the contiguous
bit in the kernel page mappings for regions whose size and alignment are
suitable for this.
This enables the following contiguous range sizes for the virtual mapping
of the kernel image, and for the linear mapping:
granule size | cont PTE | cont PMD |
-------------+------------+------------+
4 KB | 64 KB | 32 MB |
16 KB | 2 MB | 1 GB* |
64 KB | 2 MB | 16 GB* |
* only when built for 3 or more levels of translation
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/mm/mmu.c | 35 +++++++++++++++++---
1 file changed, 31 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index bf1d71b62c4f..40be4979102d 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -102,8 +102,10 @@ static const pteval_t modifiable_attr_mask = PTE_PXN | PTE_RDONLY | PTE_WRITE;
static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
unsigned long end, unsigned long pfn,
pgprot_t prot,
- phys_addr_t (*pgtable_alloc)(void))
+ phys_addr_t (*pgtable_alloc)(void),
+ bool page_mappings_only)
{
+ pgprot_t __prot = prot;
pte_t *pte;
BUG_ON(pmd_sect(*pmd));
@@ -121,7 +123,18 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
do {
pte_t old_pte = *pte;
- set_pte(pte, pfn_pte(pfn, prot));
+ /*
+ * Set the contiguous bit for the subsequent group of PTEs if
+ * its size and alignment are appropriate.
+ */
+ if (((addr | PFN_PHYS(pfn)) & ~CONT_PTE_MASK) == 0) {
+ if (end - addr >= CONT_PTE_SIZE && !page_mappings_only)
+ __prot = __pgprot(pgprot_val(prot) | PTE_CONT);
+ else
+ __prot = prot;
+ }
+
+ set_pte(pte, pfn_pte(pfn, __prot));
pfn++;
/*
@@ -141,6 +154,7 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
phys_addr_t (*pgtable_alloc)(void),
bool page_mappings_only)
{
+ pgprot_t __prot = prot;
pmd_t *pmd;
unsigned long next;
@@ -167,7 +181,19 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
/* try section mapping first */
if (((addr | next | phys) & ~SECTION_MASK) == 0 &&
!page_mappings_only) {
- pmd_set_huge(pmd, phys, prot);
+ /*
+ * Set the contiguous bit for the subsequent group of
+ * PMDs if its size and alignment are appropriate.
+ */
+ if (((addr | phys) & ~CONT_PMD_MASK) == 0) {
+ if (end - addr >= CONT_PMD_SIZE)
+ __prot = __pgprot(pgprot_val(prot) |
+ PTE_CONT);
+ else
+ __prot = prot;
+ }
+
+ pmd_set_huge(pmd, phys, __prot);
/*
* After the PMD entry has been populated once, we
@@ -178,7 +204,8 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
~modifiable_attr_mask) != 0);
} else {
alloc_init_pte(pmd, addr, next, __phys_to_pfn(phys),
- prot, pgtable_alloc);
+ prot, pgtable_alloc,
+ page_mappings_only);
BUG_ON(pmd_val(old_pmd) != 0 &&
pmd_val(old_pmd) != pmd_val(*pmd));
--
2.7.4
^ permalink raw reply related
* [PATCH v3 4/5] arm64: mm: support additional contiguous kernel mapping region sizes
From: Ard Biesheuvel @ 2016-10-12 11:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476271425-19401-1-git-send-email-ard.biesheuvel@linaro.org>
Extend the basic support for kernel mappings using contiguous regions
by adding support for contiguous PUDs (4k granule only), either as a
discrete level or folded into the PGDs. In the same way, handle folded
PMDs so that contiguous PMDs (for 16k and 64k granule kernels) will
work as expected for 2 levels of translation as well.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/include/asm/pgtable-hwdef.h | 6 +++
arch/arm64/mm/mmu.c | 40 +++++++++++++++++++-
2 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index eb0c2bd90de9..4192072af932 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -93,12 +93,15 @@
#ifdef CONFIG_ARM64_64K_PAGES
#define CONT_PTE_SHIFT 5
#define CONT_PMD_SHIFT 5
+#define CONT_PUD_SHIFT 0
#elif defined(CONFIG_ARM64_16K_PAGES)
#define CONT_PTE_SHIFT 7
#define CONT_PMD_SHIFT 5
+#define CONT_PUD_SHIFT 0
#else
#define CONT_PTE_SHIFT 4
#define CONT_PMD_SHIFT 4
+#define CONT_PUD_SHIFT 4
#endif
#define CONT_PTES (1 << CONT_PTE_SHIFT)
@@ -107,6 +110,9 @@
#define CONT_PMDS (1 << CONT_PMD_SHIFT)
#define CONT_PMD_SIZE (CONT_PMDS * PMD_SIZE)
#define CONT_PMD_MASK (~(CONT_PMD_SIZE - 1))
+#define CONT_PUDS (1 << CONT_PUD_SHIFT)
+#define CONT_PUD_SIZE (CONT_PUDS * PUD_SIZE)
+#define CONT_PUD_MASK (~(CONT_PUD_SIZE - 1))
/* the the numerical offset of the PTE within a range of CONT_PTES */
#define CONT_RANGE_OFFSET(addr) (((addr)>>PAGE_SHIFT)&(CONT_PTES-1))
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 40be4979102d..0e0eca45b54a 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -233,6 +233,7 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
phys_addr_t (*pgtable_alloc)(void),
bool page_mappings_only)
{
+ pgprot_t __prot = prot;
pud_t *pud;
unsigned long next;
@@ -254,7 +255,19 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
* For 4K granule only, attempt to put down a 1GB block
*/
if (use_1G_block(addr, next, phys) && !page_mappings_only) {
- pud_set_huge(pud, phys, prot);
+ /*
+ * Set the contiguous bit for the subsequent group of
+ * PUDs if its size and alignment are appropriate.
+ */
+ if (((addr | phys) & ~CONT_PUD_MASK) == 0) {
+ if (end - addr >= CONT_PUD_SIZE)
+ __prot = __pgprot(pgprot_val(prot) |
+ PTE_CONT);
+ else
+ __prot = prot;
+ }
+
+ pud_set_huge(pud, phys, __prot);
/*
* After the PUD entry has been populated once, we
@@ -284,6 +297,7 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
{
unsigned long addr, length, end, next;
pgd_t *pgd = pgd_offset_raw(pgdir, virt);
+ pgprot_t __prot = prot;
/*
* If the virtual and physical address don't have the same offset
@@ -299,7 +313,29 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
end = addr + length;
do {
next = pgd_addr_end(addr, end);
- alloc_init_pud(pgd, addr, next, phys, prot, pgtable_alloc,
+
+ /*
+ * If any intermediate levels are folded into the PGDs, we
+ * need to deal with the contiguous attributes here, since
+ * the contiguity can only be observed at this level.
+ */
+ if (PGDIR_SHIFT == PMD_SHIFT && !page_mappings_only &&
+ ((addr | phys) & ~CONT_PMD_MASK) == 0) {
+ if (end - addr >= CONT_PMD_SIZE)
+ __prot = __pgprot(pgprot_val(prot) |
+ PTE_CONT);
+ else
+ __prot = prot;
+ } else if (PGDIR_SHIFT == PUD_SHIFT && CONT_PUD_SHIFT > 0 &&
+ !page_mappings_only &&
+ ((addr | phys) & ~CONT_PUD_MASK) == 0) {
+ if (end - addr >= CONT_PUD_SIZE)
+ __prot = __pgprot(pgprot_val(prot) |
+ PTE_CONT);
+ else
+ __prot = prot;
+ }
+ alloc_init_pud(pgd, addr, next, phys, __prot, pgtable_alloc,
page_mappings_only);
phys += next - addr;
} while (pgd++, addr = next, addr != end);
--
2.7.4
^ permalink raw reply related
* [PATCH v3 5/5] arm64: mm: round memstart_addr to contiguous PUD/PMD size
From: Ard Biesheuvel @ 2016-10-12 11:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476271425-19401-1-git-send-email-ard.biesheuvel@linaro.org>
Given that contiguous PUDs/PMDs allow the linear region to be mapped more
efficiently, ensure that the relative alignment between the linear virtual
and physical mappings of system RAM are equal modulo the contiguous PUD
size (or contiguous PMD size, for 16k/64k granule size).
Note that this reduces the KASLR randomization of the linear range by
PUD_SHIFT (or PMD_SHIFT) bits.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/include/asm/kernel-pgtable.h | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/include/asm/kernel-pgtable.h b/arch/arm64/include/asm/kernel-pgtable.h
index 7e51d1b57c0c..71dbd9e1e809 100644
--- a/arch/arm64/include/asm/kernel-pgtable.h
+++ b/arch/arm64/include/asm/kernel-pgtable.h
@@ -83,16 +83,13 @@
/*
* To make optimal use of block mappings when laying out the linear
* mapping, round down the base of physical memory to a size that can
- * be mapped efficiently, i.e., either PUD_SIZE (4k granule) or PMD_SIZE
- * (64k granule), or a multiple that can be mapped using contiguous bits
- * in the page tables: 32 * PMD_SIZE (16k granule)
+ * be mapped efficiently, i.e., either CONT_PUD_SIZE (4k granule) or
+ * CONT_PMD_SIZE (16k/64k granules)
*/
#if defined(CONFIG_ARM64_4K_PAGES)
-#define ARM64_MEMSTART_SHIFT PUD_SHIFT
-#elif defined(CONFIG_ARM64_16K_PAGES)
-#define ARM64_MEMSTART_SHIFT (PMD_SHIFT + 5)
+#define ARM64_MEMSTART_SHIFT (PUD_SHIFT + CONT_PUD_SHIFT)
#else
-#define ARM64_MEMSTART_SHIFT PMD_SHIFT
+#define ARM64_MEMSTART_SHIFT (PMD_SHIFT + CONT_PMD_SHIFT)
#endif
/*
--
2.7.4
^ permalink raw reply related
* [bug report] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver
From: Dan Carpenter @ 2016-10-12 11:32 UTC (permalink / raw)
To: linux-arm-kernel
Hello Tai Nguyen,
The patch 832c927d119b: "perf: xgene: Add APM X-Gene SoC Performance
Monitoring Unit driver" from Jul 15, 2016, leads to the following
static checker warning:
drivers/perf/xgene_pmu.c:1014 acpi_get_pmu_hw_inf()
warn: '&res' isn't an ERR_PTR
drivers/perf/xgene_pmu.c
992 static struct
993 xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu,
994 struct acpi_device *adev, u32 type)
995 {
996 struct device *dev = xgene_pmu->dev;
997 struct list_head resource_list;
998 struct xgene_pmu_dev_ctx *ctx;
999 const union acpi_object *obj;
1000 struct hw_pmu_info *inf;
1001 void __iomem *dev_csr;
1002 struct resource res;
1003 int enable_bit;
1004 int rc;
1005
1006 ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
1007 if (!ctx)
1008 return NULL;
1009
1010 INIT_LIST_HEAD(&resource_list);
1011 rc = acpi_dev_get_resources(adev, &resource_list,
1012 acpi_pmu_dev_add_resource, &res);
1013 acpi_dev_free_resource_list(&resource_list);
1014 if (rc < 0 || IS_ERR(&res)) {
^^^^
Obviously this is a stack address and not an error pointer. I'm not
sure what was intended here.
1015 dev_err(dev, "PMU type %d: No resource address found\n", type);
1016 goto err;
1017 }
1018
regards,
dan carpenter
^ permalink raw reply
* [PATCH RESEND] ARM: dts: keystone-k2*: Increase SPI Flash partition size for U-Boot
From: Russell King - ARM Linux @ 2016-10-12 11:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c3be6bfc-9cfa-bcd8-5988-ee8bdd9fa506@ti.com>
On Wed, Oct 12, 2016 at 04:30:28PM +0530, Vignesh R wrote:
> Hi,
>
> On Monday 10 October 2016 08:01 PM, Russell King - ARM Linux wrote:
> > On Mon, Oct 10, 2016 at 07:41:41PM +0530, Vignesh R wrote:
> >> U-Boot SPI Boot image is now more than 512KB for Keystone2 devices and
> >> cannot fit into existing partition. So, increase the SPI Flash partition
> >> for U-Boot to 1MB for all Keystone2 devices.
> >>
> >> Signed-off-by: Vignesh R <vigneshr@ti.com>
> >> ---
> >>
> >> This was submitted to v4.9 merge window but was never picked up:
> >> https://patchwork.kernel.org/patch/9135023/
> >
> > I think you need to explain why it's safe to change the layout of the
> > flash partitions like this.
> >
> > - What is this "misc" partition?
> >
>
> This partition seems to exists from the very beginning. I believe, this
> is just a spare area of flash that can be used as per end-user
> requirement. Either to store a small filesystem or kernel. Copying
> Murali who added above partition if he has any input here.
>
> > - Why is it safe to move the "misc" partition in this way?
> >
> > - Do users need to do anything with data stored in the "misc" partition
> > when changing kernels?
> >
>
> MTD layer will take care of most abstractions (like start address etc).
> Will add a note in commit message informing about the reduction in size
> of the partition.
>
> > If the "misc" partition is simply unused space on the flash device, why
> > list it in DT?
> >
>
> If the unused space is not listed in the DT, then there is no /dev/mtdX
> node created for the unused section. User will then have to manually
> edit DT, in order to get the node and mount it. Instead, lets make it
> available by default.
So, taken all together, your argument is:
- We want a user partition
- It's okay to destroy the data in the user's partition by moving it
around randomly between kernel versions.
The two do not naturally go together at all. You're messing with user
expectations in ways you should not be. This really is not an acceptable
approach.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Ziji Hu @ 2016-10-12 11:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1b015f16-aa37-591c-3299-9431f6e82cce@intel.com>
Hi Adrian,
Thank you very much for your review.
I will firstly fix the typo.
On 2016/10/11 20:37, Adrian Hunter wrote:
> On 07/10/16 18:22, Gregory CLEMENT wrote:
>> From: Ziji Hu <huziji@marvell.com>
>>
>> Add Xenon eMMC/SD/SDIO host controller core functionality.
>> Add Xenon specific intialization process.
>> Add Xenon specific mmc_host_ops APIs.
>> Add Xenon specific register definitions.
>>
>> Add CONFIG_MMC_SDHCI_XENON support in drivers/mmc/host/Kconfig.
>>
>> Marvell Xenon SDHC conforms to SD Physical Layer Specification
>> Version 3.01 and is designed according to the guidelines provided
>> in the SD Host Controller Standard Specification Version 3.00.
>>
>> Signed-off-by: Hu Ziji <huziji@marvell.com>
>> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>
> I looked at a couple of things but you need to sort out the issues with
> card_candidate before going further.
>
Understood.
I will improve the card_candidate. Please help check the details in below.
>> ---
<snip>
>> +
>> +static int xenon_emmc_signal_voltage_switch(struct mmc_host *mmc,
>> + struct mmc_ios *ios)
>> +{
>> + unsigned char voltage = ios->signal_voltage;
>> +
>> + if ((voltage == MMC_SIGNAL_VOLTAGE_330) ||
>> + (voltage == MMC_SIGNAL_VOLTAGE_180))
>> + return __emmc_signal_voltage_switch(mmc, voltage);
>> +
>> + dev_err(mmc_dev(mmc), "Unsupported signal voltage: %d\n",
>> + voltage);
>> + return -EINVAL;
>> +}
>> +
>> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
>> + struct mmc_ios *ios)
>> +{
>> + struct sdhci_host *host = mmc_priv(mmc);
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> +
>> + /*
>> + * Before SD/SDIO set signal voltage, SD bus clock should be
>> + * disabled. However, sdhci_set_clock will also disable the Internal
>> + * clock in mmc_set_signal_voltage().
>> + * If Internal clock is disabled, the 3.3V/1.8V bit can not be updated.
>> + * Thus here manually enable internal clock.
>> + *
>> + * After switch completes, it is unnecessary to disable internal clock,
>> + * since keeping internal clock active obeys SD spec.
>> + */
>> + enable_xenon_internal_clk(host);
>> +
>> + if (priv->card_candidate) {
>
> mmc_power_up() calls __mmc_set_signal_voltage() calls
> host->ops->start_signal_voltage_switch so priv->card_candidate could be an
> invalid reference to an old card.
>
> So that's not going to work if the card changes - not only for removable
> cards but even for eMMC if init fails and retries.
>
As you point out, this piece of code have defects, even though it actually works on Marvell multiple platforms, unless eMMC card is removable.
I can add a property to explicitly indicate eMMC type in DTS.
Then card_candidate access can be removed here.
Does it sounds more reasonable to you?
>> + if (mmc_card_mmc(priv->card_candidate))
>> + return xenon_emmc_signal_voltage_switch(mmc, ios);
>
> So if all you need to know is whether it is a eMMC, why can't DT tell you?
>
I can add an eMMC type property in DTS, to remove the card_candidate access here.
>> + }
>> +
>> + return sdhci_start_signal_voltage_switch(mmc, ios);
>> +}
>> +
>> +/*
>> + * After determining which slot is used for SDIO,
>> + * some additional task is required.
>> + */
>> +static void xenon_init_card(struct mmc_host *mmc, struct mmc_card *card)
>> +{
>> + struct sdhci_host *host = mmc_priv(mmc);
>> + u32 reg;
>> + u8 slot_idx;
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> +
>> + /* Link the card for delay adjustment */
>> + priv->card_candidate = card;
>
> You really need a better way to get the card. I suggest you take up the
> issue with Ulf. One possibility is to have mmc core set host->card = card
> much earlier.
>
Could you please tell me if any issue related to card_candidate still exists, after the card_candidate is removed from xenon_start_signal_voltage_switch() in above?
It seems that when init_card is called, the structure card has already been updated and stable in MMC/SD/SDIO initialization sequence.
May I keep it here?
>> + /* Set tuning functionality of this slot */
>> + xenon_slot_tuning_setup(host);
>> +
>> + slot_idx = priv->slot_idx;
>> + if (!mmc_card_sdio(card)) {
>> + /* Re-enable the Auto-CMD12 cap flag. */
>> + host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
>> + host->flags |= SDHCI_AUTO_CMD12;
>> +
>> + /* Clear SDIO Card Inserted indication */
>> + reg = sdhci_readl(host, SDHC_SYS_CFG_INFO);
>> + reg &= ~(1 << (slot_idx + SLOT_TYPE_SDIO_SHIFT));
>> + sdhci_writel(host, reg, SDHC_SYS_CFG_INFO);
>> +
>> + if (mmc_card_mmc(card)) {
>> + mmc->caps |= MMC_CAP_NONREMOVABLE;
>> + if (!(host->quirks2 & SDHCI_QUIRK2_NO_1_8_V))
>> + mmc->caps |= MMC_CAP_1_8V_DDR;
>> + /*
>> + * Force to clear BUS_TEST to
>> + * skip bus_test_pre and bus_test_post
>> + */
>> + mmc->caps &= ~MMC_CAP_BUS_WIDTH_TEST;
>> + mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ |
>> + MMC_CAP2_PACKED_CMD;
>> + if (mmc->caps & MMC_CAP_8_BIT_DATA)
>> + mmc->caps2 |= MMC_CAP2_HS400_1_8V;
>> + }
>> + } else {
>> + /*
>> + * Delete the Auto-CMD12 cap flag.
>> + * Otherwise, when sending multi-block CMD53,
>> + * Driver will set Transfer Mode Register to enable Auto CMD12.
>> + * However, SDIO device cannot recognize CMD12.
>> + * Thus SDHC will time-out for waiting for CMD12 response.
>> + */
>> + host->quirks &= ~SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
>> + host->flags &= ~SDHCI_AUTO_CMD12;
>
> sdhci_set_transfer_mode() won't enable auto-CMD12 for CMD53 anyway, so is
> this needed?
>
In Xenon driver, Auto-CMD12 flag is set to enable full support to Auto-CMD feature, both Auto-CMD12 and Auto-CMD23.
As a result, when Xenon SDHC slot can both support SD and SDIO, Auto-CMD12 is disabled when SDIO card is inserted, and renabled when SD is inserted.
I recheck the sdhci code to set Auto-CMD bit in Transfer Mode register, in sdhci_set_transfer_mode():
if (mmc_op_multi(cmd->opcode) || data->blocks > 1)
As you can see, as long as it is CMD18/CMD25 OR there are multiple data blocks, Auto-CMD field will be set.
CMD53 doesn't have CMD23. Thus Auto-CMD12 is selected since Auto-CMD12 flag is set.
Thus I have to clear Auto-CMD12 to avoid issuing Auto-CMD12 in SDIO transfer.
I just meet a similar issue in RPMB.
When Auto-CMD12 flag is set, eMMC RPMB access will trigger Auto-CMD12, since CMD25 is in use.
It will cause RPMB access failed.
One possible solution is to drop Auto-CMD12 support and use Auto-CMD23 only, in Xenon driver.
May I know you opinion, please?
>> +
>> + /*
>> + * Set SDIO Card Inserted indication
>> + * to inform that the current slot is for SDIO
>> + */
>> + reg = sdhci_readl(host, SDHC_SYS_CFG_INFO);
>> + reg |= (1 << (slot_idx + SLOT_TYPE_SDIO_SHIFT));
>> + sdhci_writel(host, reg, SDHC_SYS_CFG_INFO);
>> + }
>> +}
>> +
>> +static int xenon_execute_tuning(struct mmc_host *mmc, u32 opcode)
>> +{
>> + struct sdhci_host *host = mmc_priv(mmc);
>> +
>> + if (host->timing == MMC_TIMING_UHS_DDR50)
>> + return 0;
>> +
>> + return sdhci_execute_tuning(mmc, opcode);
>> +}
>> +
>> +static void xenon_replace_mmc_host_ops(struct sdhci_host *host)
>> +{
>> + host->mmc_host_ops.set_ios = xenon_set_ios;
>> + host->mmc_host_ops.start_signal_voltage_switch =
>> + xenon_start_signal_voltage_switch;
>> + host->mmc_host_ops.init_card = xenon_init_card;
>> + host->mmc_host_ops.execute_tuning = xenon_execute_tuning;
>> +}
>> +
>> +static int xenon_probe_dt(struct platform_device *pdev)
>> +{
>> + struct device_node *np = pdev->dev.of_node;
>> + struct sdhci_host *host = platform_get_drvdata(pdev);
>> + struct mmc_host *mmc = host->mmc;
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> + int err;
>> + u32 slot_idx, nr_slot;
>> + u32 tuning_count;
>> + u32 reg;
>> +
>> + /* Standard MMC property */
>> + err = mmc_of_parse(mmc);
>> + if (err)
>> + return err;
>> +
>> + /* Standard SDHCI property */
>> + sdhci_get_of_property(pdev);
>> +
>> + /*
>> + * Xenon Specific property:
>> + * slotno: the index of slot. Refer to SDHC_SYS_CFG_INFO register
>> + * tuning-count: the interval between re-tuning
>> + * PHY type: "sdhc phy", "emmc phy 5.0" or "emmc phy 5.1"
>> + */
>> + if (!of_property_read_u32(np, "xenon,slotno", &slot_idx)) {
>> + nr_slot = sdhci_readl(host, SDHC_SYS_CFG_INFO);
>> + nr_slot &= NR_SUPPORTED_SLOT_MASK;
>> + if (unlikely(slot_idx > nr_slot)) {
>> + dev_err(mmc_dev(mmc), "Slot Index %d exceeds Number of slots %d\n",
>> + slot_idx, nr_slot);
>> + return -EINVAL;
>> + }
>> + } else {
>> + priv->slot_idx = 0x0;
>> + }
>> +
>> + if (!of_property_read_u32(np, "xenon,tuning-count", &tuning_count)) {
>> + if (unlikely(tuning_count >= TMR_RETUN_NO_PRESENT)) {
>> + dev_err(mmc_dev(mmc), "Wrong Re-tuning Count. Set default value %d\n",
>> + DEF_TUNING_COUNT);
>> + tuning_count = DEF_TUNING_COUNT;
>> + }
>> + } else {
>> + priv->tuning_count = DEF_TUNING_COUNT;
>> + }
>> +
>> + if (of_property_read_bool(np, "xenon,mask-conflict-err")) {
>> + reg = sdhci_readl(host, SDHC_SYS_EXT_OP_CTRL);
>> + reg |= MASK_CMD_CONFLICT_ERROR;
>> + sdhci_writel(host, reg, SDHC_SYS_EXT_OP_CTRL);
>> + }
>> +
>> + return err;
>> +}
>> +
>> +static int xenon_slot_probe(struct sdhci_host *host)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> + u8 slot_idx = priv->slot_idx;
>> +
>> + /* Enable slot */
>> + xenon_enable_slot(host, slot_idx);
>> +
>> + /* Enable ACG */
>> + xenon_set_acg(host, true);
>> +
>> + /* Enable Parallel Transfer Mode */
>> + xenon_enable_slot_parallel_tran(host, slot_idx);
>> +
>> + priv->timing = MMC_TIMING_FAKE;
>> + priv->clock = 0;
>> +
>> + return 0;
>> +}
>> +
>> +static void xenon_slot_remove(struct sdhci_host *host)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> + u8 slot_idx = priv->slot_idx;
>> +
>> + /* disable slot */
>> + xenon_disable_slot(host, slot_idx);
>> +}
>> +
>> +static int sdhci_xenon_probe(struct platform_device *pdev)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host;
>> + struct sdhci_host *host;
>> + struct clk *clk, *axi_clk;
>> + struct sdhci_xenon_priv *priv;
>> + int err;
>> +
>> + host = sdhci_pltfm_init(pdev, &sdhci_xenon_pdata,
>> + sizeof(struct sdhci_xenon_priv));
>> + if (IS_ERR(host))
>> + return PTR_ERR(host);
>> +
>> + pltfm_host = sdhci_priv(host);
>> + priv = sdhci_pltfm_priv(pltfm_host);
>> +
>> + xenon_set_acg(host, false);
>> +
>> + /*
>> + * Link Xenon specific mmc_host_ops function,
>> + * to replace standard ones in sdhci_ops.
>> + */
>> + xenon_replace_mmc_host_ops(host);
>> +
>> + clk = devm_clk_get(&pdev->dev, "core");
>> + if (IS_ERR(clk)) {
>> + dev_err(&pdev->dev, "Failed to setup input clk.\n");
>> + err = PTR_ERR(clk);
>> + goto free_pltfm;
>> + }
>> + clk_prepare_enable(clk);
>> + pltfm_host->clk = clk;
>> +
>> + /*
>> + * Some SOCs require additional clock to
>> + * manage AXI bus clock.
>> + * It is optional.
>> + */
>> + axi_clk = devm_clk_get(&pdev->dev, "axi");
>> + if (!IS_ERR(axi_clk)) {
>> + clk_prepare_enable(axi_clk);
>> + priv->axi_clk = axi_clk;
>> + }
>> +
>> + err = xenon_probe_dt(pdev);
>> + if (err)
>> + goto err_clk;
>> +
>> + err = xenon_slot_probe(host);
>> + if (err)
>> + goto err_clk;
>> +
>> + err = sdhci_add_host(host);
>> + if (err)
>> + goto remove_slot;
>> +
>> + return 0;
>> +
>> +remove_slot:
>> + xenon_slot_remove(host);
>> +err_clk:
>> + clk_disable_unprepare(pltfm_host->clk);
>> + if (!IS_ERR(axi_clk))
>> + clk_disable_unprepare(axi_clk);
>> +free_pltfm:
>> + sdhci_pltfm_free(pdev);
>> + return err;
>> +}
>> +
>> +static int sdhci_xenon_remove(struct platform_device *pdev)
>> +{
>> + struct sdhci_host *host = platform_get_drvdata(pdev);
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> + int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xFFFFFFFF);
>> +
>> + xenon_slot_remove(host);
>> +
>> + sdhci_remove_host(host, dead);
>> +
>> + clk_disable_unprepare(pltfm_host->clk);
>> + clk_disable_unprepare(priv->axi_clk);
>> +
>> + sdhci_pltfm_free(pdev);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id sdhci_xenon_dt_ids[] = {
>> + { .compatible = "marvell,sdhci-xenon",},
>> + { .compatible = "marvell,armada-3700-sdhci",},
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids);
>> +
>> +static struct platform_driver sdhci_xenon_driver = {
>> + .driver = {
>> + .name = "sdhci-xenon",
>> + .of_match_table = sdhci_xenon_dt_ids,
>> + .pm = &sdhci_pltfm_pmops,
>> + },
>> + .probe = sdhci_xenon_probe,
>> + .remove = sdhci_xenon_remove,
>> +};
>> +
>> +module_platform_driver(sdhci_xenon_driver);
>> +
>> +MODULE_DESCRIPTION("SDHCI platform driver for Marvell Xenon SDHC");
>> +MODULE_AUTHOR("Hu Ziji <huziji@marvell.com>");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/mmc/host/sdhci-xenon.h b/drivers/mmc/host/sdhci-xenon.h
>> new file mode 100644
>> index 000000000000..c2370493fbe8
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-xenon.h
>> @@ -0,0 +1,134 @@
>> +/*
>> + * Copyright (C) 2016 Marvell, All Rights Reserved.
>> + *
>> + * Author: Hu Ziji <huziji@marvell.com>
>> + * Date: 2016-8-24
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + */
>> +#ifndef SDHCI_XENON_H_
>> +#define SDHCI_XENON_H_
>> +
>> +#include <linux/clk.h>
>> +#include <linux/mmc/card.h>
>> +#include <linux/of.h>
>> +#include "sdhci.h"
>> +
>> +/* Register Offset of SD Host Controller SOCP self-defined register */
>> +#define SDHC_SYS_CFG_INFO 0x0104
>> +#define SLOT_TYPE_SDIO_SHIFT 24
>> +#define SLOT_TYPE_EMMC_MASK 0xFF
>> +#define SLOT_TYPE_EMMC_SHIFT 16
>> +#define SLOT_TYPE_SD_SDIO_MMC_MASK 0xFF
>> +#define SLOT_TYPE_SD_SDIO_MMC_SHIFT 8
>> +#define NR_SUPPORTED_SLOT_MASK 0x7
>> +
>> +#define SDHC_SYS_OP_CTRL 0x0108
>> +#define AUTO_CLKGATE_DISABLE_MASK BIT(20)
>> +#define SDCLK_IDLEOFF_ENABLE_SHIFT 8
>> +#define SLOT_ENABLE_SHIFT 0
>> +
>> +#define SDHC_SYS_EXT_OP_CTRL 0x010C
>> +#define MASK_CMD_CONFLICT_ERROR BIT(8)
>> +
>> +#define SDHC_SLOT_OP_STATUS_CTRL 0x0128
>> +#define DELAY_90_DEGREE_MASK_EMMC5 BIT(7)
>> +#define DELAY_90_DEGREE_SHIFT_EMMC5 7
>> +#define EMMC_5_0_PHY_FIXED_DELAY_MASK 0x7F
>> +#define EMMC_PHY_FIXED_DELAY_MASK 0xFF
>> +#define EMMC_PHY_FIXED_DELAY_WINDOW_MIN (EMMC_PHY_FIXED_DELAY_MASK >> 3)
>> +#define SDH_PHY_FIXED_DELAY_MASK 0x1FF
>> +#define SDH_PHY_FIXED_DELAY_WINDOW_MIN (SDH_PHY_FIXED_DELAY_MASK >> 4)
>> +
>> +#define TUN_CONSECUTIVE_TIMES_SHIFT 16
>> +#define TUN_CONSECUTIVE_TIMES_MASK 0x7
>> +#define TUN_CONSECUTIVE_TIMES 0x4
>> +#define TUNING_STEP_SHIFT 12
>> +#define TUNING_STEP_MASK 0xF
>> +#define TUNING_STEP_DIVIDER BIT(6)
>> +
>> +#define FORCE_SEL_INVERSE_CLK_SHIFT 11
>> +
>> +#define SDHC_SLOT_EMMC_CTRL 0x0130
>> +#define ENABLE_DATA_STROBE BIT(24)
>> +#define SET_EMMC_RSTN BIT(16)
>> +#define DISABLE_RD_DATA_CRC BIT(14)
>> +#define DISABLE_CRC_STAT_TOKEN BIT(13)
>> +#define EMMC_VCCQ_MASK 0x3
>> +#define EMMC_VCCQ_1_8V 0x1
>> +#define EMMC_VCCQ_3_3V 0x3
>> +
>> +#define SDHC_SLOT_RETUNING_REQ_CTRL 0x0144
>> +/* retuning compatible */
>> +#define RETUNING_COMPATIBLE 0x1
>> +
>> +#define SDHC_SLOT_EXT_PRESENT_STATE 0x014C
>> +#define LOCK_STATE 0x1
>> +
>> +#define SDHC_SLOT_DLL_CUR_DLY_VAL 0x0150
>> +
>> +/* Tuning Parameter */
>> +#define TMR_RETUN_NO_PRESENT 0xF
>> +#define DEF_TUNING_COUNT 0x9
>> +
>> +#define MMC_TIMING_FAKE 0xFF
>> +
>> +#define DEFAULT_SDCLK_FREQ (400000)
>> +
>> +/* Xenon specific Mode Select value */
>> +#define XENON_SDHCI_CTRL_HS200 0x5
>> +#define XENON_SDHCI_CTRL_HS400 0x6
>> +
>> +struct sdhci_xenon_priv {
>> + /*
>> + * The bus_width, timing, and clock fields in below
>> + * record the current setting of Xenon SDHC.
>> + * Driver will call a Sampling Fixed Delay Adjustment
>> + * if any setting is changed.
>> + */
>> + unsigned char bus_width;
>> + unsigned char timing;
>> + unsigned char tuning_count;
>> + unsigned int clock;
>> + struct clk *axi_clk;
>> +
>> + /* Slot idx */
>> + u8 slot_idx;
>> +
>> + /*
>> + * When initializing card, Xenon has to determine card type and
>> + * adjust Sampling Fixed delay.
>> + * However, at that time, card structure is not linked to mmc_host.
>> + * Thus a card pointer is added here to provide
>> + * the delay adjustment function with the card structure
>> + * of the card during initialization
>> + */
>> + struct mmc_card *card_candidate;
>> +};
>> +
>> +static inline int enable_xenon_internal_clk(struct sdhci_host *host)
>> +{
>> + u32 reg;
>> + u8 timeout;
>> +
>> + reg = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
>> + reg |= SDHCI_CLOCK_INT_EN;
>> + sdhci_writel(host, reg, SDHCI_CLOCK_CONTROL);
>> + /* Wait max 20 ms */
>> + timeout = 20;
>> + while (!((reg = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
>> + & SDHCI_CLOCK_INT_STABLE)) {
>> + if (timeout == 0) {
>> + pr_err("%s: Internal clock never stabilised.\n",
>> + mmc_hostname(host->mmc));
>> + return -ETIMEDOUT;
>> + }
>> + timeout--;
>> + mdelay(1);
>> + }
>> +
>> + return 0;
>> +}
>> +#endif
>>
>
^ permalink raw reply
* [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ziji Hu @ 2016-10-12 12:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <bfe30317-bb3b-b3dc-e9df-87b6ba100307@intel.com>
Hi Adrian,
On 2016/10/11 20:39, Adrian Hunter wrote:
> On 07/10/16 18:22, Gregory CLEMENT wrote:
>> From: Ziji Hu <huziji@marvell.com>
>>
>> Marvell Xenon eMMC/SD/SDIO Host Controller contains PHY.
>> Three types of PHYs are supported.
>>
>> Add support to multiple types of PHYs init and configuration.
>> Add register definitions of PHYs.
>>
>> Signed-off-by: Hu Ziji <huziji@marvell.com>
>> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>> MAINTAINERS | 1 +-
>> drivers/mmc/host/Makefile | 2 +-
>> drivers/mmc/host/sdhci-xenon-phy.c | 1141 +++++++++++++++++++++++++++++-
>> drivers/mmc/host/sdhci-xenon-phy.h | 157 ++++-
>> drivers/mmc/host/sdhci-xenon.c | 4 +-
>> drivers/mmc/host/sdhci-xenon.h | 17 +-
>> 6 files changed, 1321 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/mmc/host/sdhci-xenon-phy.c
>> create mode 100644 drivers/mmc/host/sdhci-xenon-phy.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 859420e5dfd3..b5673c2ee5f2 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -7583,6 +7583,7 @@ M: Ziji Hu <huziji@marvell.com>
>> L: linux-mmc at vger.kernel.org
>> S: Supported
>> F: drivers/mmc/host/sdhci-xenon.*
>> +F: drivers/mmc/host/sdhci-xenon-phy.*
>> F: Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
>>
>> MATROX FRAMEBUFFER DRIVER
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index 75eaf743486c..4f2854556ff7 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -82,4 +82,4 @@ ifeq ($(CONFIG_CB710_DEBUG),y)
>> endif
>>
>> obj-$(CONFIG_MMC_SDHCI_XENON) += sdhci-xenon-driver.o
>> -sdhci-xenon-driver-y += sdhci-xenon.o
>> +sdhci-xenon-driver-y += sdhci-xenon.o sdhci-xenon-phy.o
>> diff --git a/drivers/mmc/host/sdhci-xenon-phy.c b/drivers/mmc/host/sdhci-xenon-phy.c
>> new file mode 100644
>> index 000000000000..4eb8fea1bec9
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-xenon-phy.c
>
> <SNIP>
>
>> +static int __xenon_emmc_delay_adj_test(struct mmc_card *card)
>> +{
>> + int err;
>> + u8 *ext_csd = NULL;
>> +
>> + err = mmc_get_ext_csd(card, &ext_csd);
>> + kfree(ext_csd);
>> +
>> + return err;
>> +}
>> +
>> +static int __xenon_sdio_delay_adj_test(struct mmc_card *card)
>> +{
>> + struct mmc_command cmd = {0};
>> + int err;
>> +
>> + cmd.opcode = SD_IO_RW_DIRECT;
>> + cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
>> +
>> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
>> + if (err)
>> + return err;
>> +
>> + if (cmd.resp[0] & R5_ERROR)
>> + return -EIO;
>> + if (cmd.resp[0] & R5_FUNCTION_NUMBER)
>> + return -EINVAL;
>> + if (cmd.resp[0] & R5_OUT_OF_RANGE)
>> + return -ERANGE;
>> + return 0;
>> +}
>> +
>> +static int __xenon_sd_delay_adj_test(struct mmc_card *card)
>> +{
>> + struct mmc_command cmd = {0};
>> + int err;
>> +
>> + cmd.opcode = MMC_SEND_STATUS;
>> + cmd.arg = card->rca << 16;
>> + cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
>> +
>> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
>> + return err;
>> +}
>> +
>> +static int xenon_delay_adj_test(struct mmc_card *card)
>> +{
>> + WARN_ON(!card);
>> + WARN_ON(!card->host);
>> +
>> + if (mmc_card_mmc(card))
>> + return __xenon_emmc_delay_adj_test(card);
>> + else if (mmc_card_sd(card))
>> + return __xenon_sd_delay_adj_test(card);
>> + else if (mmc_card_sdio(card))
>> + return __xenon_sdio_delay_adj_test(card);
>> + else
>> + return -EINVAL;
>> +}
>
> So you are issuing commands from the ->set_ios() callback. I would want to
> get Ulf's OK for that before going further.
>
Yes, you are correct.
In some speed mode, Xenon SDHC has to send a series of transfers to search for a perfect sampling point in PHY delay line.
It is like tuning process.
> One thing: you will need to ensure you don't trigger get HS400 re-tuning
> because it will call back into ->set_ios().
>
Could you please make the term "HS400 re-tuning" more detailed?
In current MMC driver, "HS400 re-tuning" will go back to HS200, execute HS200 tuning and come back to HS400.
I'm sure our Xenon SDHC will not execute it.
However, in coming eMMC 5.2, there is a real HS400 re-tuning, in which tuning can be directly executed in HS400 mode.
Our Xenon SDHC will neither trigger this HS400 re-tuning.
But since so far there is no such feature in MMC driver, I cannot give you a 100% guarantee now.
> And you have the problem that you need to get a reference to the card before
> the card device has been added. As I wrote in response to the previous
> patch, you should get Ulf's help with that too.
>
Sure.
I will get card_candidate solved at first.
Thank you again for your review and help.
Thank you.
Best regards,
Hu Ziji
>
^ permalink raw reply
* [RFC PATCH 00/11] pci: support for configurable PCI endpoint
From: Christoph Hellwig @ 2016-10-12 12:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57E8BB69.4020804@ti.com>
On Mon, Sep 26, 2016 at 11:38:41AM +0530, Kishon Vijay Abraham I wrote:
> > Ok, so in theory there can be other hardware (and quite likely is)
> > that supports multiple functions, and we can extend the framework
> > to support them without major obstacles, but your hardware doesn't,
> > so you kept it simple with one hardcoded function, right?
>
> right, PCIe can have upto 8 functions. So the issues with the current framework
> has to be fixed. I don't expect major obstacles with this as of now.
I wouldn't be too worried about. We have two kinds of functions in
PCIe: physical functions, or virtual functions using SR-IOV.
For the first one we pretty much just need the controller driver to
report them separately as there is almost no interaction between
functions.
SR-IOV support will be more interesting as the physical functions
controls creation of the associated virtual functions. I'd like to
defer that problem until we get hold of a software programmable
controller that supports SR-IOV and has open documentation. (That
beeing said, if someone has a pointer to such a beast send it my way!)
> > We should still find out whether it's important that you can have
> > a single PCI function with a software multi-function support of some
> > sort. We'd still be limited to six BARs in total, and would also need
> > something to identify those sub-functions, so implementing that might
> > get quite hairy.
> >
> > Possibly this could be done at a higher level, e.g. by implementing
> > a PCI-virtio multiplexer that can host multiple virtio based devices
> > inside of a single PCI function. If we think that would be a good idea,
> > we should make sure the configfs interface is extensible enough to
> > handle that.
>
> Okay. So here the main function (actual PCI function) *can* perform the work of
> virtio muliplexer if the platform wants to support sub-functions or it can be a
> normal PCI function. right?
I really don't think we should be worried about this multiplexer. It's
not something real PCIe devices do (sane ones anyway, the rest is
handled by ad-hoc multiplexers), and we should avoid creating our own
magic periphals for it.
> > One use case I have in mind for this is to have a PCI function that
> > can use virtio to provide rootfs (virtio-blk or 9pfs), network
> > and console to the system that implements the PCI function (note
> > that this is the opposite direction of what almost everyone else
> > uses PCI devices for).
>
> Do you mean the virtio should actually be in the host side? Even here the
> system that implements PCI function should have multiple functions right? (one
> for network, other for console etc..). So there should be a virtio multiplexer
> both in the host side and in the device side?
We already support virtio over phsysical PCIe buses to support intel MIC
devices. Take a look at drivers/misc/mic/bus/vop_bus.c and
drivers/misc/mic/vop (yes, what a horrible place for that code, not my
fault)
^ permalink raw reply
* [PATCH v2 0/2] PCI: aardvark: Cleanup
From: Bjorn Helgaas @ 2016-10-12 12:36 UTC (permalink / raw)
To: linux-arm-kernel
Add local "dev" pointers to reduce repetition of things like "&pdev->dev"
and remove unused drvdata.
Changes from v1:
I dropped the following because they added a lot of churn for
questionable benefit:
PCI: aardvark: Name private struct pointer "advk" consistently
PCI: aardvark: Reorder accessor functions
PCI: aardvark: Swap order of advk_write() reg/val arguments
---
Bjorn Helgaas (2):
PCI: aardvark: Add local struct device pointers
PCI: aardvark: Remove unused platform data
drivers/pci/host/pci-aardvark.c | 39 ++++++++++++++++++++-------------------
1 file changed, 20 insertions(+), 19 deletions(-)
^ permalink raw reply
* [PATCH v2 1/2] PCI: aardvark: Add local struct device pointers
From: Bjorn Helgaas @ 2016-10-12 12:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161012123001.22186.4053.stgit@bhelgaas-glaptop2.roam.corp.google.com>
Use a local "struct device *dev" for brevity and consistency with other
drivers. No functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/pci/host/pci-aardvark.c | 38 ++++++++++++++++++++------------------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c
index e4a5b7e..16421d1 100644
--- a/drivers/pci/host/pci-aardvark.c
+++ b/drivers/pci/host/pci-aardvark.c
@@ -230,20 +230,20 @@ static int advk_pcie_link_up(struct advk_pcie *pcie)
static int advk_pcie_wait_for_link(struct advk_pcie *pcie)
{
+ struct device *dev = &pcie->pdev->dev;
int retries;
/* check if the link is up or not */
for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
if (advk_pcie_link_up(pcie)) {
- dev_info(&pcie->pdev->dev, "link up\n");
+ dev_info(dev, "link up\n");
return 0;
}
usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
}
- dev_err(&pcie->pdev->dev, "link never came up\n");
-
+ dev_err(dev, "link never came up\n");
return -ETIMEDOUT;
}
@@ -376,6 +376,7 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
static void advk_pcie_check_pio_status(struct advk_pcie *pcie)
{
+ struct device *dev = &pcie->pdev->dev;
u32 reg;
unsigned int status;
char *strcomp_status, *str_posted;
@@ -407,12 +408,13 @@ static void advk_pcie_check_pio_status(struct advk_pcie *pcie)
else
str_posted = "Posted";
- dev_err(&pcie->pdev->dev, "%s PIO Response Status: %s, %#x @ %#x\n",
+ dev_err(dev, "%s PIO Response Status: %s, %#x @ %#x\n",
str_posted, strcomp_status, reg, advk_readl(pcie, PIO_ADDR_LS));
}
static int advk_pcie_wait_pio(struct advk_pcie *pcie)
{
+ struct device *dev = &pcie->pdev->dev;
unsigned long timeout;
timeout = jiffies + msecs_to_jiffies(PIO_TIMEOUT_MS);
@@ -426,7 +428,7 @@ static int advk_pcie_wait_pio(struct advk_pcie *pcie)
return 0;
}
- dev_err(&pcie->pdev->dev, "config read/write timed out\n");
+ dev_err(dev, "config read/write timed out\n");
return -ETIMEDOUT;
}
@@ -560,10 +562,11 @@ static int advk_pcie_alloc_msi(struct advk_pcie *pcie)
static void advk_pcie_free_msi(struct advk_pcie *pcie, int hwirq)
{
+ struct device *dev = &pcie->pdev->dev;
+
mutex_lock(&pcie->msi_used_lock);
if (!test_bit(hwirq, pcie->msi_irq_in_use))
- dev_err(&pcie->pdev->dev, "trying to free unused MSI#%d\n",
- hwirq);
+ dev_err(dev, "trying to free unused MSI#%d\n", hwirq);
else
clear_bit(hwirq, pcie->msi_irq_in_use);
mutex_unlock(&pcie->msi_used_lock);
@@ -910,6 +913,7 @@ out_release_res:
static int advk_pcie_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct advk_pcie *pcie;
struct resource *res;
struct pci_bus *bus, *child;
@@ -917,8 +921,7 @@ static int advk_pcie_probe(struct platform_device *pdev)
struct device_node *msi_node;
int ret, irq;
- pcie = devm_kzalloc(&pdev->dev, sizeof(struct advk_pcie),
- GFP_KERNEL);
+ pcie = devm_kzalloc(dev, sizeof(struct advk_pcie), GFP_KERNEL);
if (!pcie)
return -ENOMEM;
@@ -926,22 +929,22 @@ static int advk_pcie_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pcie);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pcie->base = devm_ioremap_resource(&pdev->dev, res);
+ pcie->base = devm_ioremap_resource(dev, res);
if (IS_ERR(pcie->base))
return PTR_ERR(pcie->base);
irq = platform_get_irq(pdev, 0);
- ret = devm_request_irq(&pdev->dev, irq, advk_pcie_irq_handler,
+ ret = devm_request_irq(dev, irq, advk_pcie_irq_handler,
IRQF_SHARED | IRQF_NO_THREAD, "advk-pcie",
pcie);
if (ret) {
- dev_err(&pdev->dev, "Failed to register interrupt\n");
+ dev_err(dev, "Failed to register interrupt\n");
return ret;
}
ret = advk_pcie_parse_request_of_pci_ranges(pcie);
if (ret) {
- dev_err(&pdev->dev, "Failed to parse resources\n");
+ dev_err(dev, "Failed to parse resources\n");
return ret;
}
@@ -949,24 +952,24 @@ static int advk_pcie_probe(struct platform_device *pdev)
ret = advk_pcie_init_irq_domain(pcie);
if (ret) {
- dev_err(&pdev->dev, "Failed to initialize irq\n");
+ dev_err(dev, "Failed to initialize irq\n");
return ret;
}
ret = advk_pcie_init_msi_irq_domain(pcie);
if (ret) {
- dev_err(&pdev->dev, "Failed to initialize irq\n");
+ dev_err(dev, "Failed to initialize irq\n");
advk_pcie_remove_irq_domain(pcie);
return ret;
}
- msi_node = of_parse_phandle(pdev->dev.of_node, "msi-parent", 0);
+ msi_node = of_parse_phandle(dev->of_node, "msi-parent", 0);
if (msi_node)
msi = of_pci_find_msi_chip_by_node(msi_node);
else
msi = NULL;
- bus = pci_scan_root_bus_msi(&pdev->dev, 0, &advk_pcie_ops,
+ bus = pci_scan_root_bus_msi(dev, 0, &advk_pcie_ops,
pcie, &pcie->resources, &pcie->msi);
if (!bus) {
advk_pcie_remove_msi_irq_domain(pcie);
@@ -980,7 +983,6 @@ static int advk_pcie_probe(struct platform_device *pdev)
pcie_bus_configure_settings(child);
pci_bus_add_devices(bus);
-
return 0;
}
^ permalink raw reply related
* [PATCH v2 2/2] PCI: aardvark: Remove unused platform data
From: Bjorn Helgaas @ 2016-10-12 12:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161012123001.22186.4053.stgit@bhelgaas-glaptop2.roam.corp.google.com>
The aardvark driver never uses the platform drvdata pointer, so don't
bother setting it. No functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/pci/host/pci-aardvark.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c
index 16421d1..4fce494 100644
--- a/drivers/pci/host/pci-aardvark.c
+++ b/drivers/pci/host/pci-aardvark.c
@@ -926,7 +926,6 @@ static int advk_pcie_probe(struct platform_device *pdev)
return -ENOMEM;
pcie->pdev = pdev;
- platform_set_drvdata(pdev, pcie);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
pcie->base = devm_ioremap_resource(dev, res);
^ permalink raw reply related
* [RFC PATCH 01/11] pci: endpoint: add EP core layer to enable EP controller and EP functions
From: Christoph Hellwig @ 2016-10-12 12:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473829927-20466-2-git-send-email-kishon@ti.com>
> +/**
> + * pci_epc_stop() - stop the PCI link
> + * @epc: the link of the EPC device that has to be stopped
> + *
> + * Invoke to stop the PCI link
> + */
> +void pci_epc_stop(struct pci_epc *epc)
> +{
> + if (IS_ERR(epc) || !epc->ops->stop)
> + return;
> +
> + spin_lock_irq(&epc->irq_lock);
> + epc->ops->stop(epc);
> + spin_unlock_irq(&epc->irq_lock);
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_stop);
Can you elaborate on the synchronization strategy here? It seems
like irq_lock is generally taken irq save and just around method
calls. Wou;dn't it be better to leave locking to the methods
themselves?
> +/**
> + * struct pci_epc - represents the PCI EPC device
> + * @dev: PCI EPC device
> + * @ops: function pointers for performing endpoint operations
> + * @mutex: mutex to protect pci_epc ops
> + */
> +struct pci_epc {
> + struct device dev;
> + /* support only single function PCI device for now */
> + struct pci_epf *epf;
> + const struct pci_epc_ops *ops;
> + spinlock_t irq_lock;
> +};
And this still documentes a mutex instead of the irq save spinlock,
while we're at it..
> +/**
> + * struct pci_epf_bar - represents the BAR of EPF device
> + * @phys_addr: physical address that should be mapped to the BAR
> + * @size: the size of the address space present in BAR
> + */
> +struct pci_epf_bar {
> + dma_addr_t phys_addr;
> + size_t size;
> +};
Just curious: shouldn't this be a phys_addr_t instead of a dma_addr_t?
Otherwise this looks like a nice little framework to get started!
^ permalink raw reply
* [RFC PATCH 02/11] pci: endpoint: introduce configfs entry for configuring EP functions
From: Christoph Hellwig @ 2016-10-12 12:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473829927-20466-3-git-send-email-kishon@ti.com>
On Wed, Sep 14, 2016 at 10:41:58AM +0530, Kishon Vijay Abraham I wrote:
> diff --git a/drivers/pci/endpoint/Kconfig b/drivers/pci/endpoint/Kconfig
> index a6d827c..f1dd206 100644
> --- a/drivers/pci/endpoint/Kconfig
> +++ b/drivers/pci/endpoint/Kconfig
> @@ -13,7 +13,9 @@ config PCI_ENDPOINT
>
> Enabling this option will build the endpoint library, which
> includes endpoint controller library and endpoint function
> - library.
> + library. This will also enable the configfs entry required to
> + configure the endpoint function and used to bind the
> + function with a endpoint controller.
>
> If in doubt, say "N" to disable Endpoint support.
This needs to grow a
depends on CONFIGFS_FS
> +/**
> + * pci-ep-cfs.c - configfs to configure the PCI endpoint
Please don't use the file name in the top of the file comment, it's
only bound to get out of date..
> +struct pci_epf_info {
> + struct config_item pci_epf;
> + struct pci_epf *epf;
> +};
Any reason not to simply embedd the config_item into the pci_epf structure?
^ permalink raw reply
* [RFC PATCH 05/11] pci: rename *host* directory to *controller*
From: Christoph Hellwig @ 2016-10-12 12:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473829927-20466-6-git-send-email-kishon@ti.com>
This is a big and painful change. I'd suggest to either drop it for
now or convince Bjorn to take it as a scripted renamed just after -rc1.
^ permalink raw reply
* [PATCH 2/2] power/reset: at91-poweroff: timely shitdown LPDDR memories
From: Jean-Jacques Hiblot @ 2016-10-12 12:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161007163427.11454-3-alexandre.belloni@free-electrons.com>
2016-10-07 18:34 GMT+02:00 Alexandre Belloni
<alexandre.belloni@free-electrons.com>:
> LPDDR memories can only handle up to 400 uncontrolled power off. Ensure the
> proper power off sequence is used before shutting down the platform.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
> drivers/power/reset/at91-poweroff.c | 52 +++++++++++++++++++++++++++++++-
> drivers/power/reset/at91-sama5d2_shdwc.c | 48 ++++++++++++++++++++++++++++-
> 2 files changed, 98 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/power/reset/at91-poweroff.c b/drivers/power/reset/at91-poweroff.c
> index e9e24df35f26..bf97390e6cd7 100644
> --- a/drivers/power/reset/at91-poweroff.c
> +++ b/drivers/power/reset/at91-poweroff.c
> @@ -14,9 +14,12 @@
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_address.h>
> #include <linux/platform_device.h>
> #include <linux/printk.h>
>
> +#include <soc/at91/at91sam9_ddrsdr.h>
> +
> #define AT91_SHDW_CR 0x00 /* Shut Down Control Register */
> #define AT91_SHDW_SHDW BIT(0) /* Shut Down command */
> #define AT91_SHDW_KEY (0xa5 << 24) /* KEY Password */
> @@ -50,6 +53,7 @@ static const char *shdwc_wakeup_modes[] = {
>
> static void __iomem *at91_shdwc_base;
> static struct clk *sclk;
> +static void __iomem *mpddrc_base;
>
> static void __init at91_wakeup_status(void)
> {
> @@ -73,6 +77,28 @@ static void at91_poweroff(void)
> writel(AT91_SHDW_KEY | AT91_SHDW_SHDW, at91_shdwc_base + AT91_SHDW_CR);
> }
>
> +static void at91_lpddr_poweroff(void)
> +{
> + asm volatile(
> + /* Align to cache lines */
> + ".balign 32\n\t"
> +
> + " ldr r6, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
At first sight, it looks useless. I assume it's used to preload the
TLB before the LPDDR is turned off.
A comment to explain why this line is useful would prevent its removal.
> +
> + /* Power down SDRAM0 */
> + " str %1, [%0, #" __stringify(AT91_DDRSDRC_LPR) "]\n\t"
> + /* Shutdown CPU */
> + " str %3, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
> +
> + " b .\n\t"
> + :
> + : "r" (mpddrc_base),
> + "r" cpu_to_le32(AT91_DDRSDRC_LPDDR2_PWOFF),
> + "r" (at91_shdwc_base),
> + "r" cpu_to_le32(AT91_SHDW_KEY | AT91_SHDW_SHDW)
> + : "r0");
> +}
> +
> static int at91_poweroff_get_wakeup_mode(struct device_node *np)
> {
> const char *pm;
> @@ -124,6 +150,8 @@ static void at91_poweroff_dt_set_wakeup_mode(struct platform_device *pdev)
> static int __init at91_poweroff_probe(struct platform_device *pdev)
> {
> struct resource *res;
> + struct device_node *np;
> + u32 ddr_type;
> int ret;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -150,12 +178,29 @@ static int __init at91_poweroff_probe(struct platform_device *pdev)
>
> pm_power_off = at91_poweroff;
>
> + np = of_find_compatible_node(NULL, NULL, "atmel,sama5d3-ddramc");
> + if (!np)
> + return 0;
> +
> + mpddrc_base = of_iomap(np, 0);
> + of_node_put(np);
> +
> + if (!mpddrc_base)
> + return 0;
> +
> + ddr_type = readl(mpddrc_base + AT91_DDRSDRC_MDR) & AT91_DDRSDRC_MD;
> + if ((ddr_type == AT91_DDRSDRC_MD_LPDDR2) ||
> + (ddr_type == AT91_DDRSDRC_MD_LPDDR3))
Souldn't there be something like "pm_power_off = at91_lpddr_poweroff;" here ?
Jean-Jacques
> + else
> + iounmap(mpddrc_base);
> +
> return 0;
> }
>
> static int __exit at91_poweroff_remove(struct platform_device *pdev)
> {
> - if (pm_power_off == at91_poweroff)
> + if (pm_power_off == at91_poweroff ||
> + pm_power_off == at91_lpddr_poweroff)
> pm_power_off = NULL;
>
> clk_disable_unprepare(sclk);
> @@ -163,6 +208,11 @@ static int __exit at91_poweroff_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct of_device_id at91_ramc_of_match[] = {
> + { .compatible = "atmel,sama5d3-ddramc", },
> + { /* sentinel */ }
> +};
> +
> static const struct of_device_id at91_poweroff_of_match[] = {
> { .compatible = "atmel,at91sam9260-shdwc", },
> { .compatible = "atmel,at91sam9rl-shdwc", },
> diff --git a/drivers/power/reset/at91-sama5d2_shdwc.c b/drivers/power/reset/at91-sama5d2_shdwc.c
> index 8a5ac9706c9c..5736f360b374 100644
> --- a/drivers/power/reset/at91-sama5d2_shdwc.c
> +++ b/drivers/power/reset/at91-sama5d2_shdwc.c
> @@ -22,9 +22,12 @@
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_address.h>
> #include <linux/platform_device.h>
> #include <linux/printk.h>
>
> +#include <soc/at91/at91sam9_ddrsdr.h>
> +
> #define SLOW_CLOCK_FREQ 32768
>
> #define AT91_SHDW_CR 0x00 /* Shut Down Control Register */
> @@ -75,6 +78,7 @@ struct shdwc {
> */
> static struct shdwc *at91_shdwc;
> static struct clk *sclk;
> +static void __iomem *mpddrc_base;
>
> static const unsigned long long sdwc_dbc_period[] = {
> 0, 3, 32, 512, 4096, 32768,
> @@ -108,6 +112,28 @@ static void at91_poweroff(void)
> at91_shdwc->at91_shdwc_base + AT91_SHDW_CR);
> }
>
> +static void at91_lpddr_poweroff(void)
> +{
> + asm volatile(
> + /* Align to cache lines */
> + ".balign 32\n\t"
> +
> + " ldr r6, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
> +
> + /* Power down SDRAM0 */
> + " str %1, [%0, #" __stringify(AT91_DDRSDRC_LPR) "]\n\t"
> + /* Shutdown CPU */
> + " str %3, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
> +
> + " b .\n\t"
> + :
> + : "r" (mpddrc_base),
> + "r" cpu_to_le32(AT91_DDRSDRC_LPDDR2_PWOFF),
> + "r" (at91_shdwc->at91_shdwc_base),
> + "r" cpu_to_le32(AT91_SHDW_KEY | AT91_SHDW_SHDW)
> + : "r0");
> +}
> +
> static u32 at91_shdwc_debouncer_value(struct platform_device *pdev,
> u32 in_period_us)
> {
> @@ -212,6 +238,8 @@ static int __init at91_shdwc_probe(struct platform_device *pdev)
> {
> struct resource *res;
> const struct of_device_id *match;
> + struct device_node *np;
> + u32 ddr_type;
> int ret;
>
> if (!pdev->dev.of_node)
> @@ -249,6 +277,23 @@ static int __init at91_shdwc_probe(struct platform_device *pdev)
>
> pm_power_off = at91_poweroff;
>
> + np = of_find_compatible_node(NULL, NULL, "atmel,sama5d3-ddramc");
> + if (!np)
> + return 0;
> +
> + mpddrc_base = of_iomap(np, 0);
> + of_node_put(np);
> +
> + if (!mpddrc_base)
> + return 0;
> +
> + ddr_type = readl(mpddrc_base + AT91_DDRSDRC_MDR) & AT91_DDRSDRC_MD;
> + if ((ddr_type == AT91_DDRSDRC_MD_LPDDR2) ||
> + (ddr_type == AT91_DDRSDRC_MD_LPDDR3))
> + pm_power_off = at91_lpddr_poweroff;
> + else
> + iounmap(mpddrc_base);
> +
> return 0;
> }
>
> @@ -256,7 +301,8 @@ static int __exit at91_shdwc_remove(struct platform_device *pdev)
> {
> struct shdwc *shdw = platform_get_drvdata(pdev);
>
> - if (pm_power_off == at91_poweroff)
> + if (pm_power_off == at91_poweroff ||
> + pm_power_off == at91_lpddr_poweroff)
> pm_power_off = NULL;
>
> /* Reset values to disable wake-up features */
> --
> 2.9.3
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Adrian Hunter @ 2016-10-12 13:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4009a66b-1e09-0780-b7c1-128ff1401b5f@marvell.com>
On 12/10/16 14:58, Ziji Hu wrote:
> Hi Adrian,
>
> Thank you very much for your review.
> I will firstly fix the typo.
>
> On 2016/10/11 20:37, Adrian Hunter wrote:
>> On 07/10/16 18:22, Gregory CLEMENT wrote:
>>> From: Ziji Hu <huziji@marvell.com>
>>>
>>> Add Xenon eMMC/SD/SDIO host controller core functionality.
>>> Add Xenon specific intialization process.
>>> Add Xenon specific mmc_host_ops APIs.
>>> Add Xenon specific register definitions.
>>>
>>> Add CONFIG_MMC_SDHCI_XENON support in drivers/mmc/host/Kconfig.
>>>
>>> Marvell Xenon SDHC conforms to SD Physical Layer Specification
>>> Version 3.01 and is designed according to the guidelines provided
>>> in the SD Host Controller Standard Specification Version 3.00.
>>>
>>> Signed-off-by: Hu Ziji <huziji@marvell.com>
>>> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>
>> I looked at a couple of things but you need to sort out the issues with
>> card_candidate before going further.
>>
> Understood.
> I will improve the card_candidate. Please help check the details in below.
>
>>> ---
> <snip>
>>> +
>>> +static int xenon_emmc_signal_voltage_switch(struct mmc_host *mmc,
>>> + struct mmc_ios *ios)
>>> +{
>>> + unsigned char voltage = ios->signal_voltage;
>>> +
>>> + if ((voltage == MMC_SIGNAL_VOLTAGE_330) ||
>>> + (voltage == MMC_SIGNAL_VOLTAGE_180))
>>> + return __emmc_signal_voltage_switch(mmc, voltage);
>>> +
>>> + dev_err(mmc_dev(mmc), "Unsupported signal voltage: %d\n",
>>> + voltage);
>>> + return -EINVAL;
>>> +}
>>> +
>>> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
>>> + struct mmc_ios *ios)
>>> +{
>>> + struct sdhci_host *host = mmc_priv(mmc);
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>>> +
>>> + /*
>>> + * Before SD/SDIO set signal voltage, SD bus clock should be
>>> + * disabled. However, sdhci_set_clock will also disable the Internal
>>> + * clock in mmc_set_signal_voltage().
>>> + * If Internal clock is disabled, the 3.3V/1.8V bit can not be updated.
>>> + * Thus here manually enable internal clock.
>>> + *
>>> + * After switch completes, it is unnecessary to disable internal clock,
>>> + * since keeping internal clock active obeys SD spec.
>>> + */
>>> + enable_xenon_internal_clk(host);
>>> +
>>> + if (priv->card_candidate) {
>>
>> mmc_power_up() calls __mmc_set_signal_voltage() calls
>> host->ops->start_signal_voltage_switch so priv->card_candidate could be an
>> invalid reference to an old card.
>>
>> So that's not going to work if the card changes - not only for removable
>> cards but even for eMMC if init fails and retries.
>>
> As you point out, this piece of code have defects, even though it actually works on Marvell multiple platforms, unless eMMC card is removable.
>
> I can add a property to explicitly indicate eMMC type in DTS.
> Then card_candidate access can be removed here.
> Does it sounds more reasonable to you?
Sure
>
>>> + if (mmc_card_mmc(priv->card_candidate))
>>> + return xenon_emmc_signal_voltage_switch(mmc, ios);
>>
>> So if all you need to know is whether it is a eMMC, why can't DT tell you?
>>
> I can add an eMMC type property in DTS, to remove the card_candidate access here.
>
>>> + }
>>> +
>>> + return sdhci_start_signal_voltage_switch(mmc, ios);
>>> +}
>>> +
>>> +/*
>>> + * After determining which slot is used for SDIO,
>>> + * some additional task is required.
>>> + */
>>> +static void xenon_init_card(struct mmc_host *mmc, struct mmc_card *card)
>>> +{
>>> + struct sdhci_host *host = mmc_priv(mmc);
>>> + u32 reg;
>>> + u8 slot_idx;
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>>> +
>>> + /* Link the card for delay adjustment */
>>> + priv->card_candidate = card;
>>
>> You really need a better way to get the card. I suggest you take up the
>> issue with Ulf. One possibility is to have mmc core set host->card = card
>> much earlier.
>>
> Could you please tell me if any issue related to card_candidate still exists, after the card_candidate is removed from xenon_start_signal_voltage_switch() in above?
> It seems that when init_card is called, the structure card has already been updated and stable in MMC/SD/SDIO initialization sequence.
> May I keep it here?
It works by accident rather than by design. We can do better.
>
>>> + /* Set tuning functionality of this slot */
>>> + xenon_slot_tuning_setup(host);
>>> +
>>> + slot_idx = priv->slot_idx;
>>> + if (!mmc_card_sdio(card)) {
>>> + /* Re-enable the Auto-CMD12 cap flag. */
>>> + host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
>>> + host->flags |= SDHCI_AUTO_CMD12;
>>> +
>>> + /* Clear SDIO Card Inserted indication */
>>> + reg = sdhci_readl(host, SDHC_SYS_CFG_INFO);
>>> + reg &= ~(1 << (slot_idx + SLOT_TYPE_SDIO_SHIFT));
>>> + sdhci_writel(host, reg, SDHC_SYS_CFG_INFO);
>>> +
>>> + if (mmc_card_mmc(card)) {
>>> + mmc->caps |= MMC_CAP_NONREMOVABLE;
>>> + if (!(host->quirks2 & SDHCI_QUIRK2_NO_1_8_V))
>>> + mmc->caps |= MMC_CAP_1_8V_DDR;
>>> + /*
>>> + * Force to clear BUS_TEST to
>>> + * skip bus_test_pre and bus_test_post
>>> + */
>>> + mmc->caps &= ~MMC_CAP_BUS_WIDTH_TEST;
>>> + mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ |
>>> + MMC_CAP2_PACKED_CMD;
>>> + if (mmc->caps & MMC_CAP_8_BIT_DATA)
>>> + mmc->caps2 |= MMC_CAP2_HS400_1_8V;
>>> + }
>>> + } else {
>>> + /*
>>> + * Delete the Auto-CMD12 cap flag.
>>> + * Otherwise, when sending multi-block CMD53,
>>> + * Driver will set Transfer Mode Register to enable Auto CMD12.
>>> + * However, SDIO device cannot recognize CMD12.
>>> + * Thus SDHC will time-out for waiting for CMD12 response.
>>> + */
>>> + host->quirks &= ~SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
>>> + host->flags &= ~SDHCI_AUTO_CMD12;
>>
>> sdhci_set_transfer_mode() won't enable auto-CMD12 for CMD53 anyway, so is
>> this needed?
>>
> In Xenon driver, Auto-CMD12 flag is set to enable full support to Auto-CMD feature, both Auto-CMD12 and Auto-CMD23.
> As a result, when Xenon SDHC slot can both support SD and SDIO, Auto-CMD12 is disabled when SDIO card is inserted, and renabled when SD is inserted.
>
> I recheck the sdhci code to set Auto-CMD bit in Transfer Mode register, in sdhci_set_transfer_mode():
> if (mmc_op_multi(cmd->opcode) || data->blocks > 1)
> As you can see, as long as it is CMD18/CMD25 OR there are multiple data blocks, Auto-CMD field will be set.
> CMD53 doesn't have CMD23. Thus Auto-CMD12 is selected since Auto-CMD12 flag is set.
> Thus I have to clear Auto-CMD12 to avoid issuing Auto-CMD12 in SDIO transfer.
The code is:
if (mmc_op_multi(cmd->opcode) || data->blocks > 1) {
mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI;
/*
* If we are sending CMD23, CMD12 never gets sent
* on successful completion (so no Auto-CMD12).
*/
if (sdhci_auto_cmd12(host, cmd->mrq) &&
(cmd->opcode != SD_IO_RW_EXTENDED))
mode |= SDHCI_TRNS_AUTO_CMD12;
else if (cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23)) {
mode |= SDHCI_TRNS_AUTO_CMD23;
sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2);
}
}
You can see the check for SD_IO_RW_EXTENDED which is CMD53.
>
> I just meet a similar issue in RPMB.
> When Auto-CMD12 flag is set, eMMC RPMB access will trigger Auto-CMD12, since CMD25 is in use.
> It will cause RPMB access failed.
Can you explain more about the RPMB issue. Doesn't it use CMD23, so CMD12
wouldn't be used - auto or manually.
>
> One possible solution is to drop Auto-CMD12 support and use Auto-CMD23 only, in Xenon driver.
> May I know you opinion, please?
I don't use auto-CMD12 because I don't know if it provides any benefit and
sdhci does not seem to have implemented Auto CMD12 Error Recovery, although
I have never looked at it closely.
^ permalink raw reply
* [PATCH v14 00/16] KVM PCIe/MSI passthrough on ARM/ARM64
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
This is the second respin on top of Robin's series [1], addressing Alex' comments.
Major changes are:
- MSI-doorbell API now is moved to DMA IOMMU API following Alex suggestion
to put all API pieces at the same place (so eventually in the IOMMU
subsystem)
- new iommu_domain_msi_resv struct and accessor through DOMAIN_ATTR_MSI_RESV
domain with mirror VFIO capability
- more robustness I think in the VFIO layer
- added "iommu/iova: fix __alloc_and_insert_iova_range" since with the current
code I failed allocating an IOVA page in a single page domain with upper part
reserved
IOVA range exclusion will be handled in a separate series
The priority really is to discuss and freeze the API and especially the MSI
doorbell's handling. Do we agree to put that in DMA IOMMU?
Note: the size computation does not take into account possible page overlaps
between doorbells but it would add quite a lot of complexity i think.
Tested on AMD Overdrive (single GICv2m frame) with I350 VF assignment.
dependency:
the series depends on Robin's generic-v7 branch:
[1] [PATCH v7 00/22] Generic DT bindings for PCI IOMMUs and ARM SMMU
http://www.spinics.net/lists/arm-kernel/msg531110.html
Best Regards
Eric
Git: complete series available at
https://github.com/eauger/linux/tree/generic-v7-pcie-passthru-v14
the above branch includes a temporary patch to work around a ThunderX pci
bus reset crash (which I think unrelated to this series):
"vfio: pci: HACK! workaround thunderx pci_try_reset_bus crash"
Do not take this one for other platforms.
Eric Auger (15):
iommu/iova: fix __alloc_and_insert_iova_range
iommu: Introduce DOMAIN_ATTR_MSI_RESV
iommu/dma: MSI doorbell alloc/free
iommu/dma: Introduce iommu_calc_msi_resv
iommu/arm-smmu: Implement domain_get_attr for DOMAIN_ATTR_MSI_RESV
irqchip/gic-v2m: Register the MSI doorbell
irqchip/gicv3-its: Register the MSI doorbell
vfio: Introduce a vfio_dma type field
vfio/type1: vfio_find_dma accepting a type argument
vfio/type1: Implement recursive vfio_find_dma_from_node
vfio/type1: Handle unmap/unpin and replay for VFIO_IOVA_RESERVED slots
vfio: Allow reserved msi iova registration
vfio/type1: Check doorbell safety
iommu/arm-smmu: Do not advertise IOMMU_CAP_INTR_REMAP
vfio/type1: Introduce MSI_RESV capability
Robin Murphy (1):
iommu/dma: Allow MSI-only cookies
drivers/iommu/Kconfig | 4 +-
drivers/iommu/arm-smmu-v3.c | 10 +-
drivers/iommu/arm-smmu.c | 10 +-
drivers/iommu/dma-iommu.c | 184 ++++++++++++++++++++++++++
drivers/iommu/iova.c | 2 +-
drivers/irqchip/irq-gic-v2m.c | 10 +-
drivers/irqchip/irq-gic-v3-its.c | 13 ++
drivers/vfio/vfio_iommu_type1.c | 279 +++++++++++++++++++++++++++++++++++++--
include/linux/dma-iommu.h | 59 +++++++++
include/linux/iommu.h | 8 ++
include/uapi/linux/vfio.h | 30 ++++-
11 files changed, 587 insertions(+), 22 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH v14 01/16] iommu/iova: fix __alloc_and_insert_iova_range
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
Fix the size check within start_pfn and limit_pfn.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
the issue was observed when playing with 1 page iova domain with
higher iova reserved.
---
drivers/iommu/iova.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index e23001b..ee29dbf 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -147,7 +147,7 @@ move_left:
if (!curr) {
if (size_aligned)
pad_size = iova_get_pad_size(size, limit_pfn);
- if ((iovad->start_pfn + size + pad_size) > limit_pfn) {
+ if ((iovad->start_pfn + size + pad_size - 1) > limit_pfn) {
spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);
return -ENOMEM;
}
--
1.9.1
^ permalink raw reply related
* [PATCH v14 02/16] iommu: Introduce DOMAIN_ATTR_MSI_RESV
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
Introduce a new DOMAIN_ATTR_MSI_RESV domain attribute and associated
iommu_domain msi_resv field. It comprises the size and alignment
of the IOVA reserved window dedicated to MSI mapping. This attribute
only is supported when MSI must be IOMMU mapped.
This is the case on ARM.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Suggested-by: Alex Williamson <alex.williamson@redhat.com>
---
v13 -> v14:
- new msi_resv type and name
v12 -> v13:
- reword the commit message
v8 -> v9:
- rename programmable into iommu_msi_supported
- add iommu_domain_msi_aperture_valid
v8: creation
- deprecates DOMAIN_ATTR_MSI_MAPPING flag
---
include/linux/iommu.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 436dc21..aaeb598 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -52,6 +52,12 @@ struct iommu_domain_geometry {
bool force_aperture; /* DMA only allowed in mappable range? */
};
+/* MSI reserved IOVA window requirements */
+struct iommu_domain_msi_resv {
+ size_t size; /* size in bytes */
+ size_t alignment; /* byte alignment */
+};
+
/* Domain feature flags */
#define __IOMMU_DOMAIN_PAGING (1U << 0) /* Support for iommu_map/unmap */
#define __IOMMU_DOMAIN_DMA_API (1U << 1) /* Domain for use in DMA-API
@@ -83,6 +89,7 @@ struct iommu_domain {
iommu_fault_handler_t handler;
void *handler_token;
struct iommu_domain_geometry geometry;
+ struct iommu_domain_msi_resv msi_resv;
void *iova_cookie;
};
@@ -108,6 +115,7 @@ enum iommu_cap {
enum iommu_attr {
DOMAIN_ATTR_GEOMETRY,
+ DOMAIN_ATTR_MSI_RESV,
DOMAIN_ATTR_PAGING,
DOMAIN_ATTR_WINDOWS,
DOMAIN_ATTR_FSL_PAMU_STASH,
--
1.9.1
^ permalink raw reply related
* [PATCH v14 03/16] iommu/dma: Allow MSI-only cookies
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
From: Robin Murphy <robin.murphy@arm.com>
IOMMU domain users such as VFIO face a similar problem to DMA API ops
with regard to mapping MSI messages in systems where the MSI write is
subject to IOMMU translation. With the relevant infrastructure now in
place for managed DMA domains, it's actually really simple for other
users to piggyback off that and reap the benefits without giving up
their own IOVA management, and without having to reinvent their own
wheel in the MSI layer.
Allow such users to opt into automatic MSI remapping by dedicating a
region of their IOVA space to a managed cookie.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v13 -> v14:
- restore reserve_iova for iova >= base + size
v1 -> v13 incorpration:
- compared to Robin's version
- add NULL last param to iommu_dma_init_domain
- set the msi_geometry aperture
- I removed
if (base < U64_MAX - size)
reserve_iova(iovad, iova_pfn(iovad, base + size), ULONG_MAX);
don't get why we would reserve something out of the scope of the iova domain?
what do I miss?
---
drivers/iommu/dma-iommu.c | 39 +++++++++++++++++++++++++++++++++++++++
include/linux/dma-iommu.h | 9 +++++++++
2 files changed, 48 insertions(+)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index c5ab866..d45f9a0 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -716,3 +716,42 @@ void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
msg->address_lo += lower_32_bits(msi_page->iova);
}
}
+
+/**
+ * iommu_get_dma_msi_region_cookie - Configure a domain for MSI remapping only
+ * @domain: IOMMU domain to prepare
+ * @base: Base address of IOVA region to use as the MSI remapping aperture
+ * @size: Size of the desired MSI aperture
+ *
+ * Users who manage their own IOVA allocation and do not want DMA API support,
+ * but would still like to take advantage of automatic MSI remapping, can use
+ * this to initialise their own domain appropriately.
+ */
+int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
+ dma_addr_t base, u64 size)
+{
+ struct iommu_dma_cookie *cookie;
+ struct iova_domain *iovad;
+ int ret;
+
+ if (domain->type == IOMMU_DOMAIN_DMA)
+ return -EINVAL;
+
+ ret = iommu_get_dma_cookie(domain);
+ if (ret)
+ return ret;
+
+ ret = iommu_dma_init_domain(domain, base, size, NULL);
+ if (ret) {
+ iommu_put_dma_cookie(domain);
+ return ret;
+ }
+
+ cookie = domain->iova_cookie;
+ iovad = &cookie->iovad;
+ if (base < U64_MAX - size)
+ reserve_iova(iovad, iova_pfn(iovad, base + size), ULONG_MAX);
+
+ return 0;
+}
+EXPORT_SYMBOL(iommu_get_dma_msi_region_cookie);
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 32c5890..05ab5b4 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -67,6 +67,9 @@ int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr);
/* The DMA API isn't _quite_ the whole story, though... */
void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
+int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
+ dma_addr_t base, u64 size);
+
#else
struct iommu_domain;
@@ -90,6 +93,12 @@ static inline void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
{
}
+static inline int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
+ dma_addr_t base, u64 size)
+{
+ return -ENODEV;
+}
+
#endif /* CONFIG_IOMMU_DMA */
#endif /* __KERNEL__ */
#endif /* __DMA_IOMMU_H */
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox