LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] powerpc/dma: Fix invalid DMA mmap behavior
From: Michael Ellerman @ 2019-07-22 23:09 UTC (permalink / raw)
  To: Shawn Anastasio, Arnd Bergmann, Christoph Hellwig
  Cc: Alexey Kardashevskiy, Sam Bobroff, open list:IOMMU DRIVERS,
	Oliver O'Halloran, linuxppc-dev, Marek Szyprowski
In-Reply-To: <ff0c5578-1cb4-af29-ca40-ef2c6b246d2f@anastas.io>

Shawn Anastasio <shawn@anastas.io> writes:
> On 7/22/19 7:16 AM, Michael Ellerman wrote:
>> Arnd Bergmann <arnd@arndb.de> writes:
>>> On Thu, Jul 18, 2019 at 11:52 AM Christoph Hellwig <hch@lst.de> wrote:
>>>> On Thu, Jul 18, 2019 at 10:49:34AM +0200, Christoph Hellwig wrote:
>>>>> On Thu, Jul 18, 2019 at 01:45:16PM +1000, Oliver O'Halloran wrote:
>>>>>>> Other than m68k, mips, and arm64, everybody else that doesn't have
>>>>>>> ARCH_NO_COHERENT_DMA_MMAP set uses this default implementation, so
>>>>>>> I assume this behavior is acceptable on those architectures.
>>>>>>
>>>>>> It might be acceptable, but there's no reason to use pgport_noncached
>>>>>> if the platform supports cache-coherent DMA.
>>>>>>
>>>>>> Christoph (+cc) made the change so maybe he saw something we're missing.
>>>>>
>>>>> I always found the forcing of noncached access even for coherent
>>>>> devices a little odd, but this was inherited from the previous
>>>>> implementation, which surprised me a bit as the different attributes
>>>>> are usually problematic even on x86.  Let me dig into the history a
>>>>> bit more, but I suspect the righ fix is to default to cached mappings
>>>>> for coherent devices.
>>>>
>>>> Ok, some history:
>>>>
>>>> The generic dma mmap implementation, which we are effectively still
>>>> using today was added by:
>>>>
>>>> commit 64ccc9c033c6089b2d426dad3c56477ab066c999
>>>> Author: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> Date:   Thu Jun 14 13:03:04 2012 +0200
>>>>
>>>>      common: dma-mapping: add support for generic dma_mmap_* calls
>>>>
>>>> and unconditionally uses pgprot_noncached in dma_common_mmap, which is
>>>> then used as the fallback by dma_mmap_attrs if no ->mmap method is
>>>> present.  At that point we already had the powerpc implementation
>>>> that only uses pgprot_noncached for non-coherent mappings, and
>>>> the arm one, which uses pgprot_writecombine if DMA_ATTR_WRITE_COMBINE
>>>> is set and otherwise pgprot_dmacoherent, which seems to be uncached.
>>>> Arm did support coherent platforms at that time, but they might have
>>>> been an afterthought and not handled properly.
>>>
>>> Cache-coherent devices are still very rare on 32-bit ARM.
>>>
>>> Among the callers of dma_mmap_coherent(), almost all are in platform
>>> specific device drivers that only ever run on noncoherent ARM SoCs,
>>> which explains why nobody would have noticed problems.
>>>
>>> There is also a difference in behavior between ARM and PowerPC
>>> when dealing with mismatched cacheability attributes: If the same
>>> page is mapped as both cached and uncached to, this may
>>> cause silent undefined behavior on ARM, while PowerPC should
>>> enter a checkstop as soon as it notices.
>> 
>> On newer Power CPUs it's actually more like the ARM behaviour.
>> 
>> I don't know for sure that it will *never* checkstop but there are at
>> least cases where it won't. There's some (not much) detail in the
>> Power8/9 user manuals.
>
> The issue was discovered due to sporadic checkstops on P9, so it
> seems like it will happen at least sometimes.

Yeah true. I wasn't sure if that checkstop was actually caused by a
cached/uncached mismatch or something else, but looks like it was, from
the hostboot issue (https://github.com/open-power/hostboot/issues/180):

  12.47015|  Signature Description    : pu.ex:k0:n0:s0:p00:c0 (L2FIR[16]) Cache line inhibited hit cacheable space


So I'm not really sure how to square that with the documentation in the
user manual:

  If a caching-inhibited load instruction hits in the L1 data cache, the
  load data is serviced from the L1 data cache and no request is sent to
  the NCU.
  If a caching-inhibited store instruction hits in the L1 data cache,
  the store data is written to the L1 data cache and sent to the NCU.
  Note that the L1 data cache and L2 cache are no longer coherent.

I guess I'm either misinterpreting that section or there's some *other*
documentation somewhere I haven't found that says that it will also
checkstop.

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/dma: Fix invalid DMA mmap behavior
From: Shawn Anastasio @ 2019-07-22 19:23 UTC (permalink / raw)
  To: Michael Ellerman, Arnd Bergmann, Christoph Hellwig
  Cc: Alexey Kardashevskiy, Sam Bobroff, open list:IOMMU DRIVERS,
	Oliver O'Halloran, linuxppc-dev, Marek Szyprowski
In-Reply-To: <87ef2i6z99.fsf@concordia.ellerman.id.au>



On 7/22/19 7:16 AM, Michael Ellerman wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
>> On Thu, Jul 18, 2019 at 11:52 AM Christoph Hellwig <hch@lst.de> wrote:
>>> On Thu, Jul 18, 2019 at 10:49:34AM +0200, Christoph Hellwig wrote:
>>>> On Thu, Jul 18, 2019 at 01:45:16PM +1000, Oliver O'Halloran wrote:
>>>>>> Other than m68k, mips, and arm64, everybody else that doesn't have
>>>>>> ARCH_NO_COHERENT_DMA_MMAP set uses this default implementation, so
>>>>>> I assume this behavior is acceptable on those architectures.
>>>>>
>>>>> It might be acceptable, but there's no reason to use pgport_noncached
>>>>> if the platform supports cache-coherent DMA.
>>>>>
>>>>> Christoph (+cc) made the change so maybe he saw something we're missing.
>>>>
>>>> I always found the forcing of noncached access even for coherent
>>>> devices a little odd, but this was inherited from the previous
>>>> implementation, which surprised me a bit as the different attributes
>>>> are usually problematic even on x86.  Let me dig into the history a
>>>> bit more, but I suspect the righ fix is to default to cached mappings
>>>> for coherent devices.
>>>
>>> Ok, some history:
>>>
>>> The generic dma mmap implementation, which we are effectively still
>>> using today was added by:
>>>
>>> commit 64ccc9c033c6089b2d426dad3c56477ab066c999
>>> Author: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Date:   Thu Jun 14 13:03:04 2012 +0200
>>>
>>>      common: dma-mapping: add support for generic dma_mmap_* calls
>>>
>>> and unconditionally uses pgprot_noncached in dma_common_mmap, which is
>>> then used as the fallback by dma_mmap_attrs if no ->mmap method is
>>> present.  At that point we already had the powerpc implementation
>>> that only uses pgprot_noncached for non-coherent mappings, and
>>> the arm one, which uses pgprot_writecombine if DMA_ATTR_WRITE_COMBINE
>>> is set and otherwise pgprot_dmacoherent, which seems to be uncached.
>>> Arm did support coherent platforms at that time, but they might have
>>> been an afterthought and not handled properly.
>>
>> Cache-coherent devices are still very rare on 32-bit ARM.
>>
>> Among the callers of dma_mmap_coherent(), almost all are in platform
>> specific device drivers that only ever run on noncoherent ARM SoCs,
>> which explains why nobody would have noticed problems.
>>
>> There is also a difference in behavior between ARM and PowerPC
>> when dealing with mismatched cacheability attributes: If the same
>> page is mapped as both cached and uncached to, this may
>> cause silent undefined behavior on ARM, while PowerPC should
>> enter a checkstop as soon as it notices.
> 
> On newer Power CPUs it's actually more like the ARM behaviour.
> 
> I don't know for sure that it will *never* checkstop but there are at
> least cases where it won't. There's some (not much) detail in the
> Power8/9 user manuals.

The issue was discovered due to sporadic checkstops on P9, so it
seems like it will happen at least sometimes.

> cheers

^ permalink raw reply

* Re: [PATCH v2] powerpc: slightly improve cache helpers
From: Segher Boessenkool @ 2019-07-22 17:58 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: LKML, clang-built-linux, Paul Mackerras, Nathan Chancellor,
	linuxppc-dev
In-Reply-To: <CAKwvOd=KRVsFkT8dLFoitky9OF8tKmbn00-OPi6kBygyx4QwHg@mail.gmail.com>

On Mon, Jul 22, 2019 at 10:21:07AM -0700, Nick Desaulniers wrote:
> On Sun, Jul 21, 2019 at 11:19 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> > On Sun, Jul 21, 2019 at 07:41:40PM -0700, Nathan Chancellor wrote:
> > > On Sun, Jul 21, 2019 at 01:01:50PM -0500, Segher Boessenkool wrote:
> > > > On Sun, Jul 21, 2019 at 12:58:46AM -0700, Nathan Chancellor wrote:
> > > > > 0000017c clear_user_page:
> > > > >      17c: 94 21 ff f0                     stwu 1, -16(1)
> > > > >      180: 38 80 00 80                     li 4, 128
> > > > >      184: 38 63 ff e0                     addi 3, 3, -32
> > > > >      188: 7c 89 03 a6                     mtctr 4
> > > > >      18c: 38 81 00 0f                     addi 4, 1, 15
> > > > >      190: 8c c3 00 20                     lbzu 6, 32(3)
> > > > >      194: 98 c1 00 0f                     stb 6, 15(1)
> > > > >      198: 7c 00 27 ec                     dcbz 0, 4
> > > > >      19c: 42 00 ff f4                     bdnz .+65524
> > > >
> > > > Uh, yeah, well, I have no idea what clang tried here, but that won't
> > > > work.  It's copying a byte from each target cache line to the stack,
> > > > and then does clears the cache line containing that byte on the stack.
> > > >
> > > > I *guess* this is about "Z" and not about "%y", but you'll have to ask
> > > > the clang people.
> > > >
> > > > Or it may be that they do not treat inline asm operands as lvalues
> > > > properly?  That rings some bells.  Yeah that looks like it.
> >
> > The code is
> >   __asm__ __volatile__ ("dcbz %y0" : : "Z"(*(u8 *)addr) : "memory");
> >
> > so yeah it looks like clang took that  *(u8 *)addr  as rvalue, and
> > stored that in stack, and then used *that* as memory.
> 
> What's the %y modifier supposed to mean here?

It prints a memory address for an indexed operand.

If you write just "%0" it prints addresses that are a single register
as "0(r3)" instead of "0,r3".  Some instructions do not allow offset
form.

> addr is in the list of
> inputs, so what's wrong with using it as an rvalue?

It seems to use *(u8 *)addr as rvalue.  Asm operands are lvalues.  It
matters a lot for memory operands.

> > Maybe clang simply does not not to treat "Z" the same as "m"?  (And "Y"
> > and "Q" and "es" and a whole bunch of "w*", what about those?)


Segher

^ permalink raw reply

* Re: [PATCH 2/2] powerpc: avoid adjusting memory_limit for capture kernel memory reservation
From: Michal Suchánek @ 2019-07-22 17:49 UTC (permalink / raw)
  To: Hari Bathini
  Cc: linuxppc-dev, Mahesh J Salgaonkar, Ananth N Mavinakayanahalli
In-Reply-To: <156166327993.13320.10788410344711883330.stgit@hbathini.in.ibm.com>

On Fri, 28 Jun 2019 00:51:19 +0530
Hari Bathini <hbathini@linux.ibm.com> wrote:

> Currently, if memory_limit is specified and it overlaps with memory to
> be reserved for capture kernel, memory_limit is adjusted to accommodate
> capture kernel. With memory reservation for capture kernel moved later
> (after enforcing memory limit), this adjustment no longer holds water.
> So, avoid adjusting memory_limit and error out instead.

Can you split out the memory limit adjustment out of memory reservation
so it can still be adjusted?

Thanks

Michal
> 
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>  arch/powerpc/kernel/fadump.c        |   16 ----------------
>  arch/powerpc/kernel/machine_kexec.c |   22 +++++++++++-----------
>  2 files changed, 11 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 4eab972..a784695 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -476,22 +476,6 @@ int __init fadump_reserve_mem(void)
>  #endif
>  	}
>  
> -	/*
> -	 * Calculate the memory boundary.
> -	 * If memory_limit is less than actual memory boundary then reserve
> -	 * the memory for fadump beyond the memory_limit and adjust the
> -	 * memory_limit accordingly, so that the running kernel can run with
> -	 * specified memory_limit.
> -	 */
> -	if (memory_limit && memory_limit < memblock_end_of_DRAM()) {
> -		size = get_fadump_area_size();
> -		if ((memory_limit + size) < memblock_end_of_DRAM())
> -			memory_limit += size;
> -		else
> -			memory_limit = memblock_end_of_DRAM();
> -		printk(KERN_INFO "Adjusted memory_limit for firmware-assisted"
> -				" dump, now %#016llx\n", memory_limit);
> -	}
>  	if (memory_limit)
>  		memory_boundary = memory_limit;
>  	else
> diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
> index c4ed328..fc5533b 100644
> --- a/arch/powerpc/kernel/machine_kexec.c
> +++ b/arch/powerpc/kernel/machine_kexec.c
> @@ -125,10 +125,8 @@ void __init reserve_crashkernel(void)
>  		crashk_res.end = crash_base + crash_size - 1;
>  	}
>  
> -	if (crashk_res.end == crashk_res.start) {
> -		crashk_res.start = crashk_res.end = 0;
> -		return;
> -	}
> +	if (crashk_res.end == crashk_res.start)
> +		goto error_out;
>  
>  	/* We might have got these values via the command line or the
>  	 * device tree, either way sanitise them now. */
> @@ -170,15 +168,13 @@ void __init reserve_crashkernel(void)
>  	if (overlaps_crashkernel(__pa(_stext), _end - _stext)) {
>  		printk(KERN_WARNING
>  			"Crash kernel can not overlap current kernel\n");
> -		crashk_res.start = crashk_res.end = 0;
> -		return;
> +		goto error_out;
>  	}
>  
>  	/* Crash kernel trumps memory limit */
>  	if (memory_limit && memory_limit <= crashk_res.end) {
> -		memory_limit = crashk_res.end + 1;
> -		printk("Adjusted memory limit for crashkernel, now 0x%llx\n",
> -		       memory_limit);
> +		pr_err("Crash kernel size can't exceed memory_limit\n");
> +		goto error_out;
>  	}
>  
>  	printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
> @@ -190,9 +186,13 @@ void __init reserve_crashkernel(void)
>  	if (!memblock_is_region_memory(crashk_res.start, crash_size) ||
>  	    memblock_reserve(crashk_res.start, crash_size)) {
>  		pr_err("Failed to reserve memory for crashkernel!\n");
> -		crashk_res.start = crashk_res.end = 0;
> -		return;
> +		goto error_out;
>  	}
> +
> +	return;
> +error_out:
> +	crashk_res.start = crashk_res.end = 0;
> +	return;
>  }
>  
>  int overlaps_crashkernel(unsigned long start, unsigned long size)
> 


^ permalink raw reply

* [RFC PATCH 4/4] powerpc/64: Add VIRTUAL_BUG_ON checks for __va and __pa addresses
From: Nicholas Piggin @ 2019-07-22 17:47 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Anju T Sudhakar, Madhavan Srinivasan, Reza Arbab, Nicholas Piggin
In-Reply-To: <20190722174700.11483-1-npiggin@gmail.com>

Ensure __va is given an address below PAGE_OFFSET, and __pa is given
one above PAGE_OFFSET.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/page.h | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 0d52f57fca04..c8bb14ff4713 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -215,9 +215,19 @@ static inline bool pfn_valid(unsigned long pfn)
 /*
  * gcc miscompiles (unsigned long)(&static_var) - PAGE_OFFSET
  * with -mcmodel=medium, so we use & and | instead of - and + on 64-bit.
+ * This also results in better code generation.
  */
-#define __va(x) ((void *)(unsigned long)((phys_addr_t)(x) | PAGE_OFFSET))
-#define __pa(x) ((unsigned long)(x) & 0x0fffffffffffffffUL)
+#define __va(x)								\
+({									\
+	VIRTUAL_BUG_ON((unsigned long)(x) >= PAGE_OFFSET);		\
+	(void *)(unsigned long)((phys_addr_t)(x) | PAGE_OFFSET);	\
+})
+
+#define __pa(x)								\
+({									\
+	VIRTUAL_BUG_ON((unsigned long)(x) < PAGE_OFFSET);		\
+	(unsigned long)(x) & 0x0fffffffffffffffUL;			\
+})
 
 #else /* 32-bit, non book E */
 #define __va(x) ((void *)(unsigned long)((phys_addr_t)(x) + PAGE_OFFSET - MEMORY_START))
-- 
2.20.1


^ permalink raw reply related

* [RFC PATCH 3/4] powerpc/perf: fix imc allocation failure
From: Nicholas Piggin @ 2019-07-22 17:46 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Anju T Sudhakar, Madhavan Srinivasan, Reza Arbab, Nicholas Piggin
In-Reply-To: <20190722174700.11483-1-npiggin@gmail.com>

alloc_pages_node return value should be tested before applying
page_address.

Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/perf/imc-pmu.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index dea243185ea4..cb50a9e1fd2d 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -577,6 +577,7 @@ static int core_imc_mem_init(int cpu, int size)
 {
 	int nid, rc = 0, core_id = (cpu / threads_per_core);
 	struct imc_mem_info *mem_info;
+	struct page *page;
 
 	/*
 	 * alloc_pages_node() will allocate memory for core in the
@@ -587,11 +588,12 @@ static int core_imc_mem_init(int cpu, int size)
 	mem_info->id = core_id;
 
 	/* We need only vbase for core counters */
-	mem_info->vbase = page_address(alloc_pages_node(nid,
-					  GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE |
-					  __GFP_NOWARN, get_order(size)));
-	if (!mem_info->vbase)
+	page = alloc_pages_node(nid,
+				GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE |
+				__GFP_NOWARN, get_order(size));
+	if (!page)
 		return -ENOMEM;
+	mem_info->vbase = page_address(page);
 
 	/* Init the mutex */
 	core_imc_refc[core_id].id = core_id;
@@ -849,15 +851,17 @@ static int thread_imc_mem_alloc(int cpu_id, int size)
 	int nid = cpu_to_node(cpu_id);
 
 	if (!local_mem) {
+		struct page *page;
 		/*
 		 * This case could happen only once at start, since we dont
 		 * free the memory in cpu offline path.
 		 */
-		local_mem = page_address(alloc_pages_node(nid,
+		page = alloc_pages_node(nid,
 				  GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE |
-				  __GFP_NOWARN, get_order(size)));
-		if (!local_mem)
+				  __GFP_NOWARN, get_order(size));
+		if (!page)
 			return -ENOMEM;
+		local_mem = page_address(page);
 
 		per_cpu(thread_imc_mem, cpu_id) = local_mem;
 	}
@@ -1095,11 +1099,14 @@ static int trace_imc_mem_alloc(int cpu_id, int size)
 	int core_id = (cpu_id / threads_per_core);
 
 	if (!local_mem) {
-		local_mem = page_address(alloc_pages_node(phys_id,
-					GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE |
-					__GFP_NOWARN, get_order(size)));
-		if (!local_mem)
+		struct page *page;
+
+		page = alloc_pages_node(phys_id,
+				GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE |
+				__GFP_NOWARN, get_order(size));
+		if (!page)
 			return -ENOMEM;
+		local_mem = page_address(page);
 		per_cpu(trace_imc_mem, cpu_id) = local_mem;
 
 		/* Initialise the counters for trace mode */
-- 
2.20.1


^ permalink raw reply related

* [RFC PATCH 2/4] powerpc/64s/radix: Fix memory hot-unplug page table split
From: Nicholas Piggin @ 2019-07-22 17:46 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Anju T Sudhakar, Madhavan Srinivasan, Reza Arbab, Nicholas Piggin
In-Reply-To: <20190722174700.11483-1-npiggin@gmail.com>

create_physical_mapping expects physical addresses, but splitting
these mapping on hot unplug is supplying virtual (effective)
addresses.

[I'm not sure how to test this one]

Cc: Balbir Singh <bsingharora@gmail.com>
Fixes: 4dd5f8a99e791 ("powerpc/mm/radix: Split linear mapping on hot-unplug")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/mm/book3s64/radix_pgtable.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
index c5cc16ab1954..2204d8eeb784 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -737,8 +737,8 @@ static int __meminit stop_machine_change_mapping(void *data)
 
 	spin_unlock(&init_mm.page_table_lock);
 	pte_clear(&init_mm, params->aligned_start, params->pte);
-	create_physical_mapping(params->aligned_start, params->start, -1);
-	create_physical_mapping(params->end, params->aligned_end, -1);
+	create_physical_mapping(__pa(params->aligned_start), __pa(params->start), -1);
+	create_physical_mapping(__pa(params->end), __pa(params->aligned_end), -1);
 	spin_lock(&init_mm.page_table_lock);
 	return 0;
 }
-- 
2.20.1


^ permalink raw reply related

* [RFC PATCH 1/4] powerpc/64s/radix: Fix memory hotplug section page table creation
From: Nicholas Piggin @ 2019-07-22 17:46 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Anju T Sudhakar, Madhavan Srinivasan, Reza Arbab, Nicholas Piggin
In-Reply-To: <20190722174700.11483-1-npiggin@gmail.com>

create_physical_mapping expects physical addresses, but creating and
splitting these mappings after boot is supplying virtual (effective)
addresses. This can be hit by booting with limited memory then probing
new physical memory sections.

Cc: Reza Arbab <arbab@linux.vnet.ibm.com>
Fixes: 6cc27341b21a8 ("powerpc/mm: add radix__create_section_mapping()")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/mm/book3s64/radix_pgtable.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
index b4ca9e95e678..c5cc16ab1954 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -902,7 +902,7 @@ int __meminit radix__create_section_mapping(unsigned long start, unsigned long e
 		return -1;
 	}
 
-	return create_physical_mapping(start, end, nid);
+	return create_physical_mapping(__pa(start), __pa(end), nid);
 }
 
 int __meminit radix__remove_section_mapping(unsigned long start, unsigned long end)
-- 
2.20.1


^ permalink raw reply related

* [RFC PATCH 0/4] assorted virtual / physical address fixes
From: Nicholas Piggin @ 2019-07-22 17:46 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Anju T Sudhakar, Madhavan Srinivasan, Reza Arbab, Nicholas Piggin

Implementing VIRTUAL_BUG_ON to catch incorrect usage of __va and __pa
showed up a few possible issues. Actually patch 1 was found by
inspection (I will check whether I may attribute the reporter).

Thanks,
Nick

Nicholas Piggin (4):
  powerpc/64s/radix: Fix memory hotplug section page table creation
  powerpc/64s/radix: Fix memory hot-unplug page table split
  powerpc/perf: fix imc allocation failure
  powerpc/64: Add VIRTUAL_BUG_ON checks for __va and __pa addresses

 arch/powerpc/include/asm/page.h          | 14 ++++++++++--
 arch/powerpc/mm/book3s64/radix_pgtable.c |  6 ++---
 arch/powerpc/perf/imc-pmu.c              | 29 +++++++++++++++---------
 3 files changed, 33 insertions(+), 16 deletions(-)

-- 
2.20.1


^ permalink raw reply

* Re: [PATCH v2] powerpc: slightly improve cache helpers
From: Nick Desaulniers @ 2019-07-22 17:21 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: LKML, clang-built-linux, Paul Mackerras, Nathan Chancellor,
	linuxppc-dev
In-Reply-To: <20190722061940.GZ20882@gate.crashing.org>

On Sun, Jul 21, 2019 at 11:19 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Sun, Jul 21, 2019 at 07:41:40PM -0700, Nathan Chancellor wrote:
> > Hi Segher,
> >
> > On Sun, Jul 21, 2019 at 01:01:50PM -0500, Segher Boessenkool wrote:
> > > On Sun, Jul 21, 2019 at 12:58:46AM -0700, Nathan Chancellor wrote:
> > > > 0000017c clear_user_page:
> > > >      17c: 94 21 ff f0                     stwu 1, -16(1)
> > > >      180: 38 80 00 80                     li 4, 128
> > > >      184: 38 63 ff e0                     addi 3, 3, -32
> > > >      188: 7c 89 03 a6                     mtctr 4
> > > >      18c: 38 81 00 0f                     addi 4, 1, 15
> > > >      190: 8c c3 00 20                     lbzu 6, 32(3)
> > > >      194: 98 c1 00 0f                     stb 6, 15(1)
> > > >      198: 7c 00 27 ec                     dcbz 0, 4
> > > >      19c: 42 00 ff f4                     bdnz .+65524
> > >
> > > Uh, yeah, well, I have no idea what clang tried here, but that won't
> > > work.  It's copying a byte from each target cache line to the stack,
> > > and then does clears the cache line containing that byte on the stack.
> > >
> > > I *guess* this is about "Z" and not about "%y", but you'll have to ask
> > > the clang people.
> > >
> > > Or it may be that they do not treat inline asm operands as lvalues
> > > properly?  That rings some bells.  Yeah that looks like it.
>
> The code is
>   __asm__ __volatile__ ("dcbz %y0" : : "Z"(*(u8 *)addr) : "memory");
>
> so yeah it looks like clang took that  *(u8 *)addr  as rvalue, and
> stored that in stack, and then used *that* as memory.

What's the %y modifier supposed to mean here?  addr is in the list of
inputs, so what's wrong with using it as an rvalue?

>
> Maybe clang simply does not not to treat "Z" the same as "m"?  (And "Y"
> and "Q" and "es" and a whole bunch of "w*", what about those?)

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply

* [PATCH v9 05/21] powerpc: mm: Add p?d_leaf() definitions
From: Steven Price @ 2019-07-22 15:41 UTC (permalink / raw)
  To: linux-mm
  Cc: Mark Rutland, Peter Zijlstra, Dave Hansen, Paul Mackerras,
	H. Peter Anvin, Will Deacon, Liang, Kan, x86, Steven Price,
	Ingo Molnar, Catalin Marinas, Arnd Bergmann, kvm-ppc,
	Jérôme Glisse, Borislav Petkov, Andy Lutomirski,
	Thomas Gleixner, linux-arm-kernel, Ard Biesheuvel, linux-kernel,
	James Morse, Andrew Morton, linuxppc-dev
In-Reply-To: <20190722154210.42799-1-steven.price@arm.com>

walk_page_range() is going to be allowed to walk page tables other than
those of user space. For this it needs to know when it has reached a
'leaf' entry in the page tables. This information is provided by the
p?d_leaf() functions/macros.

For powerpc pmd_large() already exists and does what we want, so hoist
it out of the CONFIG_TRANSPARENT_HUGEPAGE condition and implement the
other levels. Macros are used to provide the generic p?d_leaf() names.

CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: linuxppc-dev@lists.ozlabs.org
CC: kvm-ppc@vger.kernel.org
Signed-off-by: Steven Price <steven.price@arm.com>
---
 arch/powerpc/include/asm/book3s/64/pgtable.h | 30 ++++++++++++++------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 8308f32e9782..84270666355c 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -921,6 +921,12 @@ static inline int pud_present(pud_t pud)
 	return !!(pud_raw(pud) & cpu_to_be64(_PAGE_PRESENT));
 }
 
+#define pud_leaf	pud_large
+static inline int pud_large(pud_t pud)
+{
+	return !!(pud_raw(pud) & cpu_to_be64(_PAGE_PTE));
+}
+
 extern struct page *pud_page(pud_t pud);
 extern struct page *pmd_page(pmd_t pmd);
 static inline pte_t pud_pte(pud_t pud)
@@ -964,6 +970,12 @@ static inline int pgd_present(pgd_t pgd)
 	return !!(pgd_raw(pgd) & cpu_to_be64(_PAGE_PRESENT));
 }
 
+#define pgd_leaf	pgd_large
+static inline int pgd_large(pgd_t pgd)
+{
+	return !!(pgd_raw(pgd) & cpu_to_be64(_PAGE_PTE));
+}
+
 static inline pte_t pgd_pte(pgd_t pgd)
 {
 	return __pte_raw(pgd_raw(pgd));
@@ -1131,6 +1143,15 @@ static inline bool pmd_access_permitted(pmd_t pmd, bool write)
 	return pte_access_permitted(pmd_pte(pmd), write);
 }
 
+#define pmd_leaf	pmd_large
+/*
+ * returns true for pmd migration entries, THP, devmap, hugetlb
+ */
+static inline int pmd_large(pmd_t pmd)
+{
+	return !!(pmd_raw(pmd) & cpu_to_be64(_PAGE_PTE));
+}
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 extern pmd_t pfn_pmd(unsigned long pfn, pgprot_t pgprot);
 extern pmd_t mk_pmd(struct page *page, pgprot_t pgprot);
@@ -1157,15 +1178,6 @@ pmd_hugepage_update(struct mm_struct *mm, unsigned long addr, pmd_t *pmdp,
 	return hash__pmd_hugepage_update(mm, addr, pmdp, clr, set);
 }
 
-/*
- * returns true for pmd migration entries, THP, devmap, hugetlb
- * But compile time dependent on THP config
- */
-static inline int pmd_large(pmd_t pmd)
-{
-	return !!(pmd_raw(pmd) & cpu_to_be64(_PAGE_PTE));
-}
-
 static inline pmd_t pmd_mknotpresent(pmd_t pmd)
 {
 	return __pmd(pmd_val(pmd) & ~_PAGE_PRESENT);
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v2] powerpc: slightly improve cache helpers
From: Segher Boessenkool @ 2019-07-22 15:18 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: linux-kernel, clang-built-linux, Paul Mackerras,
	Nathan Chancellor, linuxppc-dev
In-Reply-To: <87imru74ul.fsf@concordia.ellerman.id.au>

On Mon, Jul 22, 2019 at 08:15:14PM +1000, Michael Ellerman wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> > On Sun, Jul 21, 2019 at 12:58:46AM -0700, Nathan Chancellor wrote:
> >> 0000017c clear_user_page:
> >>      17c: 94 21 ff f0                  	stwu 1, -16(1)
> >>      180: 38 80 00 80                  	li 4, 128
> >>      184: 38 63 ff e0                  	addi 3, 3, -32
> >>      188: 7c 89 03 a6                  	mtctr 4
> >>      18c: 38 81 00 0f                  	addi 4, 1, 15
> >>      190: 8c c3 00 20                  	lbzu 6, 32(3)
> >>      194: 98 c1 00 0f                  	stb 6, 15(1)
> >>      198: 7c 00 27 ec                  	dcbz 0, 4
> >>      19c: 42 00 ff f4                  	bdnz .+65524
> >
> > Uh, yeah, well, I have no idea what clang tried here, but that won't
> > work.  It's copying a byte from each target cache line to the stack,
> > and then does clears the cache line containing that byte on the stack.
> 
> So it seems like this is a clang bug.
> 
> None of the distros we support use clang, but we would still like to
> keep it working if we can.

Which version?  Which versions *are* broken?

> Looking at the original patch, the only upside is that the compiler
> can use both RA and RB to compute the address, rather than us forcing RA
> to 0.
> 
> But at least with my compiler here (GCC 8 vintage) I don't actually see
> GCC ever using both GPRs even with the patch. Or at least, there's no
> difference before/after the patch as far as I can see.

The benefit is small, certainly.

> So my inclination is to revert the original patch. We can try again in a
> few years :D
> 
> Thoughts?

I think you should give the clang people time to figure out what is
going on.


Segher

^ permalink raw reply

* Re: [PATCH] powerpc: Wire up clone3 syscall
From: Christian Brauner @ 2019-07-22 13:37 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20190722132231.10169-1-mpe@ellerman.id.au>

On Mon, Jul 22, 2019 at 11:22:31PM +1000, Michael Ellerman wrote:
> Wire up the new clone3 syscall added in commit 7f192e3cd316 ("fork:
> add clone3").
> 
> This requires a ppc_clone3 wrapper, in order to save the non-volatile
> GPRs before calling into the generic syscall code. Otherwise we hit
> the BUG_ON in CHECK_FULL_REGS in copy_thread().
> 
> Lightly tested using Christian's test code on a Power8 LE VM.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Thank you, Michael!

One comment below, otherwise:

Acked-by: Christian Brauner <christian@brauner.io>

> ---
>  arch/powerpc/include/asm/unistd.h        | 1 +
>  arch/powerpc/kernel/entry_64.S           | 5 +++++
>  arch/powerpc/kernel/syscalls/syscall.tbl | 1 +
>  3 files changed, 7 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
> index 68473c3c471c..b0720c7c3fcf 100644
> --- a/arch/powerpc/include/asm/unistd.h
> +++ b/arch/powerpc/include/asm/unistd.h
> @@ -49,6 +49,7 @@
>  #define __ARCH_WANT_SYS_FORK
>  #define __ARCH_WANT_SYS_VFORK
>  #define __ARCH_WANT_SYS_CLONE
> +#define __ARCH_WANT_SYS_CLONE3
>  
>  #endif		/* __ASSEMBLY__ */
>  #endif /* _ASM_POWERPC_UNISTD_H_ */
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index d9105fcf4021..0a0b5310f54a 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -487,6 +487,11 @@ _GLOBAL(ppc_clone)
>  	bl	sys_clone
>  	b	.Lsyscall_exit
>  
> +_GLOBAL(ppc_clone3)
> +       bl      save_nvgprs
> +       bl      sys_clone3
> +       b       .Lsyscall_exit
> +
>  _GLOBAL(ppc32_swapcontext)
>  	bl	save_nvgprs
>  	bl	compat_sys_swapcontext
> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
> index f2c3bda2d39f..6886ecb590d5 100644
> --- a/arch/powerpc/kernel/syscalls/syscall.tbl
> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
> @@ -516,3 +516,4 @@
>  432	common	fsmount				sys_fsmount
>  433	common	fspick				sys_fspick
>  434	common	pidfd_open			sys_pidfd_open
> +435	common	clone3				ppc_clone3

Note that in v5.3-rc1 there's now a comment that 435 is reserved in
there. So this will likely cause a merge conflict. You might want to
base your change off of v5.3-rc1 instead to avoid that. :)

So basically:

From 10b2e4176d712e45c7cb22af4aed4ce09818785c Mon Sep 17 00:00:00 2001
From: Michael Ellerman <mpe@ellerman.id.au>
Date: Mon, 22 Jul 2019 23:22:31 +1000
Subject: [PATCH] powerpc: Wire up clone3 syscall

Wire up the new clone3 syscall added in commit 7f192e3cd316 ("fork:
add clone3").

This requires a ppc_clone3 wrapper, in order to save the non-volatile
GPRs before calling into the generic syscall code. Otherwise we hit
the BUG_ON in CHECK_FULL_REGS in copy_thread().

Lightly tested using Christian's test code on a Power8 LE VM.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Acked-by: Christian Brauner <christian@brauner.io>
Link: https://lore.kernel.org/r/20190722132231.10169-1-mpe@ellerman.id.au
---
 arch/powerpc/include/asm/unistd.h        | 1 +
 arch/powerpc/kernel/entry_64.S           | 5 +++++
 arch/powerpc/kernel/syscalls/syscall.tbl | 2 +-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index 68473c3c471c..b0720c7c3fcf 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -49,6 +49,7 @@
 #define __ARCH_WANT_SYS_FORK
 #define __ARCH_WANT_SYS_VFORK
 #define __ARCH_WANT_SYS_CLONE
+#define __ARCH_WANT_SYS_CLONE3
 
 #endif		/* __ASSEMBLY__ */
 #endif /* _ASM_POWERPC_UNISTD_H_ */
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index d9105fcf4021..0a0b5310f54a 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -487,6 +487,11 @@ _GLOBAL(ppc_clone)
 	bl	sys_clone
 	b	.Lsyscall_exit
 
+_GLOBAL(ppc_clone3)
+       bl      save_nvgprs
+       bl      sys_clone3
+       b       .Lsyscall_exit
+
 _GLOBAL(ppc32_swapcontext)
 	bl	save_nvgprs
 	bl	compat_sys_swapcontext
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index 3331749aab20..6886ecb590d5 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -516,4 +516,4 @@
 432	common	fsmount				sys_fsmount
 433	common	fspick				sys_fspick
 434	common	pidfd_open			sys_pidfd_open
-# 435 reserved for clone3
+435	common	clone3				ppc_clone3
-- 
2.22.0


^ permalink raw reply related

* [PATCH] powerpc: Wire up clone3 syscall
From: Michael Ellerman @ 2019-07-22 13:22 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: linux-kernel, christian

Wire up the new clone3 syscall added in commit 7f192e3cd316 ("fork:
add clone3").

This requires a ppc_clone3 wrapper, in order to save the non-volatile
GPRs before calling into the generic syscall code. Otherwise we hit
the BUG_ON in CHECK_FULL_REGS in copy_thread().

Lightly tested using Christian's test code on a Power8 LE VM.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/unistd.h        | 1 +
 arch/powerpc/kernel/entry_64.S           | 5 +++++
 arch/powerpc/kernel/syscalls/syscall.tbl | 1 +
 3 files changed, 7 insertions(+)

diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index 68473c3c471c..b0720c7c3fcf 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -49,6 +49,7 @@
 #define __ARCH_WANT_SYS_FORK
 #define __ARCH_WANT_SYS_VFORK
 #define __ARCH_WANT_SYS_CLONE
+#define __ARCH_WANT_SYS_CLONE3
 
 #endif		/* __ASSEMBLY__ */
 #endif /* _ASM_POWERPC_UNISTD_H_ */
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index d9105fcf4021..0a0b5310f54a 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -487,6 +487,11 @@ _GLOBAL(ppc_clone)
 	bl	sys_clone
 	b	.Lsyscall_exit
 
+_GLOBAL(ppc_clone3)
+       bl      save_nvgprs
+       bl      sys_clone3
+       b       .Lsyscall_exit
+
 _GLOBAL(ppc32_swapcontext)
 	bl	save_nvgprs
 	bl	compat_sys_swapcontext
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index f2c3bda2d39f..6886ecb590d5 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -516,3 +516,4 @@
 432	common	fsmount				sys_fsmount
 433	common	fspick				sys_fspick
 434	common	pidfd_open			sys_pidfd_open
+435	common	clone3				ppc_clone3
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH 06/10] ASoC: dt-bindings: Document dl_mask property
From: Lucas Stach @ 2019-07-22 12:56 UTC (permalink / raw)
  To: Daniel Baluta, broonie
  Cc: alsa-devel, viorel.suman, timur, Xiubo.Lee, linuxppc-dev,
	shengjiu.wang, angus, tiwai, perex, nicoleotsuka, linux-imx,
	kernel, festevam, linux-kernel
In-Reply-To: <20190722124833.28757-7-daniel.baluta@nxp.com>

Am Montag, den 22.07.2019, 15:48 +0300 schrieb Daniel Baluta:
> SAI supports up to 8 data lines. This property let the user
> configure how many data lines should be used per transfer
> direction (Tx/Rx).
> 
> > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
>  Documentation/devicetree/bindings/sound/fsl-sai.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/fsl-sai.txt b/Documentation/devicetree/bindings/sound/fsl-sai.txt
> index 2e726b983845..59f4d965a5fb 100644
> --- a/Documentation/devicetree/bindings/sound/fsl-sai.txt
> +++ b/Documentation/devicetree/bindings/sound/fsl-sai.txt
> @@ -49,6 +49,11 @@ Optional properties:
>  
> >    - big-endian		: Boolean property, required if all the SAI
> >  			  registers are big-endian rather than little-endian.
> > +  - fsl,dl_mask		: list of two integers (bitmask, first for RX, second
> > +			  for TX) representing enabled datalines. Bit 0
> > +			  represents first data line, bit 1 represents second
> > +			  data line and so on. Data line is enabled if
> > +			  corresponding bit is set to 1.

No underscores in property names, please. Also this should document the
default value used by the driver when the property is absent.

Regards,
Lucas

^ permalink raw reply

* Re: [PATCH 05/10] ASoC: fsl_sai: Add support to enable multiple data lines
From: Lucas Stach @ 2019-07-22 12:55 UTC (permalink / raw)
  To: Daniel Baluta, broonie
  Cc: alsa-devel, viorel.suman, timur, Xiubo.Lee, linuxppc-dev,
	shengjiu.wang, angus, tiwai, perex, nicoleotsuka, linux-imx,
	kernel, festevam, linux-kernel
In-Reply-To: <20190722124833.28757-6-daniel.baluta@nxp.com>

Am Montag, den 22.07.2019, 15:48 +0300 schrieb Daniel Baluta:
> SAI supports up to 8 Rx/Tx data lines which can be enabled
> using TCE/RCE bits of TCR3/RCR3 registers.
> 
> Data lines to be enabled are read from DT fsl,dl_mask property.
> By default (if no DT entry is provided) only data line 0 is enabled.
> 
> Note:
> We can only enable consecutive data lines starting with data line #0.

Why is the property a bitmask then? To me this sounds like we want to
have the number of lanes in the DT and convert to the bitmask inside
the driver.

> > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
>  sound/soc/fsl/fsl_sai.c | 10 +++++++++-
>  sound/soc/fsl/fsl_sai.h |  6 ++++--
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index 768341608695..d0fa02188b7c 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -601,7 +601,7 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,
>  
> >  	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx),
> >  			   FSL_SAI_CR3_TRCE_MASK,
> > -			   FSL_SAI_CR3_TRCE);
> > +			   FSL_SAI_CR3_TRCE(sai->soc_data->dl_mask[tx]);
>  
> >  	ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
> >  			SNDRV_PCM_HW_PARAM_RATE, &fsl_sai_rate_constraints);
> @@ -887,6 +887,14 @@ static int fsl_sai_probe(struct platform_device *pdev)
> >  		}
> >  	}
>  
> > +	/* active data lines mask for TX/RX, defaults to 1 (only the first
> > +	 * data line is enabled
> +	 */

Comment style not in line with Linux coding style.

Regards,
Lucas

> +	sai->dl_mask[RX] = 1;
> > +	sai->dl_mask[TX] = 1;
> > +	of_property_read_u32_index(np, "fsl,dl_mask", RX, &sai->dl_mask[RX]);
> > +	of_property_read_u32_index(np, "fsl,dl_mask", TX, &sai->dl_mask[TX]);
> +
> >  	irq = platform_get_irq(pdev, 0);
> >  	if (irq < 0) {
> >  		dev_err(&pdev->dev, "no irq for node %s\n", pdev->name);
> diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
> index b1abeed2f78e..6d32f0950ec5 100644
> --- a/sound/soc/fsl/fsl_sai.h
> +++ b/sound/soc/fsl/fsl_sai.h
> @@ -109,8 +109,8 @@
> >  #define FSL_SAI_CR2_DIV_MASK	0xff
>  
>  /* SAI Transmit and Receive Configuration 3 Register */
> > -#define FSL_SAI_CR3_TRCE	BIT(16)
> > -#define FSL_SAI_CR3_TRCE_MASK	GENMASK(16, 23)
> > +#define FSL_SAI_CR3_TRCE(x)	((x) << 16)
> > +#define FSL_SAI_CR3_TRCE_MASK	GENMASK(23, 16)
> >  #define FSL_SAI_CR3_WDFL(x)	(x)
> >  #define FSL_SAI_CR3_WDFL_MASK	0x1f
>  
> @@ -176,6 +176,8 @@ struct fsl_sai {
> >  	unsigned int slots;
> >  	unsigned int slot_width;
>  
> > +	unsigned int dl_mask[2];
> +
> >  	const struct fsl_sai_soc_data *soc_data;
> >  	struct snd_dmaengine_dai_dma_data dma_params_rx;
> >  	struct snd_dmaengine_dai_dma_data dma_params_tx;

^ permalink raw reply

* Re: [PATCH 10/10] ASoC: fsl_sai: Add support for imx7ulp/imx8mq
From: Lucas Stach @ 2019-07-22 13:04 UTC (permalink / raw)
  To: Daniel Baluta, broonie
  Cc: alsa-devel, viorel.suman, timur, Xiubo.Lee, linuxppc-dev,
	shengjiu.wang, angus, tiwai, perex, nicoleotsuka, linux-imx,
	kernel, festevam, linux-kernel
In-Reply-To: <20190722124833.28757-11-daniel.baluta@nxp.com>

Am Montag, den 22.07.2019, 15:48 +0300 schrieb Daniel Baluta:
> SAI module on imx7ulp/imx8m features 2 new registers (VERID and PARAM)
> at the beginning of register address space.
> 
> On imx7ulp FIFOs can held up to 16 x 32 bit samples.
> On imx8mq FIFOs can held up to 128 x 32 bit samples.
> 
> > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
>  sound/soc/fsl/fsl_sai.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index f2441b84877e..b05837465b5a 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -1065,10 +1065,24 @@ static const struct fsl_sai_soc_data fsl_sai_imx6sx_data = {
> >  	.reg_offset = 0,
>  };
>  
> +static const struct fsl_sai_soc_data fsl_sai_imx7ulp_data = {
> > +	.use_imx_pcm = true,
> > +	.fifo_depth = 16,
> > +	.reg_offset = 8,
> +};
> +
> +static const struct fsl_sai_soc_data fsl_sai_imx8mq_data = {
> > +	.use_imx_pcm = true,
> > +	.fifo_depth = 128,
> > +	.reg_offset = 8,
> +};
> +
>  static const struct of_device_id fsl_sai_ids[] = {
> >  	{ .compatible = "fsl,vf610-sai", .data = &fsl_sai_vf610_data },
> >  	{ .compatible = "fsl,imx6sx-sai", .data = &fsl_sai_imx6sx_data },
> >  	{ .compatible = "fsl,imx6ul-sai", .data = &fsl_sai_imx6sx_data },
> > +	{ .compatible = "fsl,imx7ulp-sai", .data = &fsl_sai_imx7ulp_data },
> > +	{ .compatible = "fsl,imx8mq-sai", .data = &fsl_sai_imx8mq_data },
> > 
Those two new compatibles need to be documented in the DT bindings.

Regards,
Lucas

^ permalink raw reply

* Re: [PATCH 07/10] ASoC: fsl_sai: Add support for FIFO combine mode
From: Lucas Stach @ 2019-07-22 13:01 UTC (permalink / raw)
  To: Daniel Baluta, broonie
  Cc: alsa-devel, viorel.suman, timur, Xiubo.Lee, linuxppc-dev,
	shengjiu.wang, angus, tiwai, perex, nicoleotsuka, linux-imx,
	kernel, festevam, linux-kernel
In-Reply-To: <20190722124833.28757-8-daniel.baluta@nxp.com>

Am Montag, den 22.07.2019, 15:48 +0300 schrieb Daniel Baluta:
> FIFO combining mode allows the separate FIFOs for multiple data
> channels
> to be used as a single FIFO for either software accesses or a single
> data
> channel or both.
> 
> FIFO combined mode is described in chapter 13.10.3.5.4 from i.MX8MQ
> reference manual [1].
> 
> For each direction (RX/TX) fifo combine mode is read from fsl,fcomb-
> mode
> DT property. By default, if no property is specified fifo combine
> mode
> is disabled.
> 
> [1]https://cache.nxp.com/secured/assets/documents/en/reference-manual
> /IMX8MDQLQRM.pdf?__gda__=1563728701_38bea7f0f726472cc675cb141b91bec7&
> fileExt=.pdf
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
>  sound/soc/fsl/fsl_sai.c | 37 +++++++++++++++++++++++++++++++++++++
>  sound/soc/fsl/fsl_sai.h |  9 +++++++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index d0fa02188b7c..140014901fce 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -475,6 +475,35 @@ static int fsl_sai_hw_params(struct
> snd_pcm_substream *substream,
>  		}
>  	}
>  
> +	switch (sai->soc_data->fcomb_mode[tx]) {
> +	case FSL_SAI_FCOMB_NONE:
> +		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
> +				   FSL_SAI_CR4_FCOMB_SOFT |
> +				   FSL_SAI_CR4_FCOMB_SHIFT, 0);
> +		break;
> +	case FSL_SAI_FCOMB_SHIFT:
> +		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
> +				   FSL_SAI_CR4_FCOMB_SOFT |
> +				   FSL_SAI_CR4_FCOMB_SHIFT,
> +				   FSL_SAI_CR4_FCOMB_SHIFT);
> +		break;
> +	case FSL_SAI_FCOMB_SOFT:
> +		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
> +				   FSL_SAI_CR4_FCOMB_SOFT |
> +				   FSL_SAI_CR4_FCOMB_SHIFT,
> +				   FSL_SAI_CR4_FCOMB_SOFT);
> +		break;
> +	case FSL_SAI_FCOMB_BOTH:
> +		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
> +				   FSL_SAI_CR4_FCOMB_SOFT |
> +				   FSL_SAI_CR4_FCOMB_SHIFT,
> +				   FSL_SAI_CR4_FCOMB_SOFT |
> +				   FSL_SAI_CR4_FCOMB_SHIFT);
> +		break;
> +	default:
> +		break;
> +	}

This would probably look less redundant if you only select the bits to
set in the switch statement and move the regmap_update_bits after the
switch.

Regards,
Lucas

>  	regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
>  			   FSL_SAI_CR4_SYWD_MASK |
> FSL_SAI_CR4_FRSZ_MASK,
>  			   val_cr4);
> @@ -887,6 +916,14 @@ static int fsl_sai_probe(struct platform_device
> *pdev)
>  		}
>  	}
>  
> +	/* FIFO combine mode for TX/RX, defaults to disabled */
> +	sai->fcomb_mode[RX] = FSL_SAI_FCOMB_NONE;
> +	sai->fcomb_mode[TX] = FSL_SAI_FCOMB_NONE;
> +	of_property_read_u32_index(np, "fsl,fcomb-mode", RX,
> +				   &sai->fcomb_mode[RX]);
> +	of_property_read_u32_index(np, "fsl,fcomb-mode", TX,
> +				   &sai->fcomb_mode[TX]);
> +
>  	/* active data lines mask for TX/RX, defaults to 1 (only the
> first
>  	 * data line is enabled
>  	 */
> diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
> index 6d32f0950ec5..abf140951187 100644
> --- a/sound/soc/fsl/fsl_sai.h
> +++ b/sound/soc/fsl/fsl_sai.h
> @@ -115,6 +115,8 @@
>  #define FSL_SAI_CR3_WDFL_MASK	0x1f
>  
>  /* SAI Transmit and Receive Configuration 4 Register */
> +#define FSL_SAI_CR4_FCOMB_SHIFT BIT(26)
> +#define FSL_SAI_CR4_FCOMB_SOFT  BIT(27)
>  #define FSL_SAI_CR4_FRSZ(x)	(((x) - 1) << 16)
>  #define FSL_SAI_CR4_FRSZ_MASK	(0x1f << 16)
>  #define FSL_SAI_CR4_SYWD(x)	(((x) - 1) << 8)
> @@ -155,6 +157,12 @@
>  #define FSL_SAI_MAXBURST_TX 6
>  #define FSL_SAI_MAXBURST_RX 6
>  
> +/* FIFO combine modes */
> +#define FSL_SAI_FCOMB_NONE     0
> +#define FSL_SAI_FCOMB_SHIFT    1
> +#define FSL_SAI_FCOMB_SOFT     2
> +#define FSL_SAI_FCOMB_BOTH     3
> +
>  struct fsl_sai_soc_data {
>  	bool use_imx_pcm;
>  	unsigned int fifo_depth;
> @@ -177,6 +185,7 @@ struct fsl_sai {
>  	unsigned int slot_width;
>  
>  	unsigned int dl_mask[2];
> +	unsigned int fcomb_mode[2];
>  
>  	const struct fsl_sai_soc_data *soc_data;
>  	struct snd_dmaengine_dai_dma_data dma_params_rx;

^ permalink raw reply

* [PATCH 09/10] ASoC: fsl_sai: Add support for SAI new version
From: Daniel Baluta @ 2019-07-22 12:48 UTC (permalink / raw)
  To: broonie
  Cc: Daniel Baluta, alsa-devel, viorel.suman, timur, Xiubo.Lee,
	linuxppc-dev, shengjiu.wang, angus, tiwai, perex, nicoleotsuka,
	linux-imx, kernel, festevam, linux-kernel, l.stach
In-Reply-To: <20190722124833.28757-1-daniel.baluta@nxp.com>

New IP version introduces Version ID and Parameter registers
and optionally added Timestamp feature.

VERID and PARAM registers are placed at the top of registers
address space and some registers are shifted according to
the following table:

Tx/Rx data registers and Tx/Rx FIFO registers keep their
addresses, all other registers are shifted by 8.

SAI Memory map is described in chapter 13.10.4.1.1 I2S Memory map
of the Reference Manual [1].

In order to make as less changes as possible we attach an offset
to each register offset to each changed register definition. The
offset is read from each board private data.

[1]https://cache.nxp.com/secured/assets/documents/en/reference-manual/IMX8MDQLQRM.pdf?__gda__=1563728701_38bea7f0f726472cc675cb141b91bec7&fileExt=.pdf

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 sound/soc/fsl/fsl_sai.c | 240 +++++++++++++++++++++++-----------------
 sound/soc/fsl/fsl_sai.h |  41 +++----
 2 files changed, 162 insertions(+), 119 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 140014901fce..f2441b84877e 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -40,6 +40,7 @@ static const struct snd_pcm_hw_constraint_list fsl_sai_rate_constraints = {
 static irqreturn_t fsl_sai_isr(int irq, void *devid)
 {
 	struct fsl_sai *sai = (struct fsl_sai *)devid;
+	unsigned int ofs = sai->soc_data->reg_offset;
 	struct device *dev = &sai->pdev->dev;
 	u32 flags, xcsr, mask;
 	bool irq_none = true;
@@ -52,7 +53,7 @@ static irqreturn_t fsl_sai_isr(int irq, void *devid)
 	mask = (FSL_SAI_FLAGS >> FSL_SAI_CSR_xIE_SHIFT) << FSL_SAI_CSR_xF_SHIFT;
 
 	/* Tx IRQ */
-	regmap_read(sai->regmap, FSL_SAI_TCSR, &xcsr);
+	regmap_read(sai->regmap, FSL_SAI_TCSR(ofs), &xcsr);
 	flags = xcsr & mask;
 
 	if (flags)
@@ -82,11 +83,11 @@ static irqreturn_t fsl_sai_isr(int irq, void *devid)
 	xcsr &= ~FSL_SAI_CSR_xF_MASK;
 
 	if (flags)
-		regmap_write(sai->regmap, FSL_SAI_TCSR, flags | xcsr);
+		regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), flags | xcsr);
 
 irq_rx:
 	/* Rx IRQ */
-	regmap_read(sai->regmap, FSL_SAI_RCSR, &xcsr);
+	regmap_read(sai->regmap, FSL_SAI_RCSR(ofs), &xcsr);
 	flags = xcsr & mask;
 
 	if (flags)
@@ -116,7 +117,7 @@ static irqreturn_t fsl_sai_isr(int irq, void *devid)
 	xcsr &= ~FSL_SAI_CSR_xF_MASK;
 
 	if (flags)
-		regmap_write(sai->regmap, FSL_SAI_RCSR, flags | xcsr);
+		regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), flags | xcsr);
 
 out:
 	if (irq_none)
@@ -140,6 +141,7 @@ static int fsl_sai_set_dai_sysclk_tr(struct snd_soc_dai *cpu_dai,
 		int clk_id, unsigned int freq, int fsl_dir)
 {
 	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+	unsigned int ofs = sai->soc_data->reg_offset;
 	bool tx = fsl_dir == FSL_FMT_TRANSMITTER;
 	u32 val_cr2 = 0;
 
@@ -160,7 +162,7 @@ static int fsl_sai_set_dai_sysclk_tr(struct snd_soc_dai *cpu_dai,
 		return -EINVAL;
 	}
 
-	regmap_update_bits(sai->regmap, FSL_SAI_xCR2(tx),
+	regmap_update_bits(sai->regmap, FSL_SAI_xCR2(tx, ofs),
 			   FSL_SAI_CR2_MSEL_MASK, val_cr2);
 
 	return 0;
@@ -193,6 +195,7 @@ static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai,
 				unsigned int fmt, int fsl_dir)
 {
 	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+	unsigned int ofs = sai->soc_data->reg_offset;
 	bool tx = fsl_dir == FSL_FMT_TRANSMITTER;
 	u32 val_cr2 = 0, val_cr4 = 0;
 
@@ -287,9 +290,9 @@ static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai,
 		return -EINVAL;
 	}
 
-	regmap_update_bits(sai->regmap, FSL_SAI_xCR2(tx),
+	regmap_update_bits(sai->regmap, FSL_SAI_xCR2(tx, ofs),
 			   FSL_SAI_CR2_BCP | FSL_SAI_CR2_BCD_MSTR, val_cr2);
-	regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
+	regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs),
 			   FSL_SAI_CR4_MF | FSL_SAI_CR4_FSE |
 			   FSL_SAI_CR4_FSP | FSL_SAI_CR4_FSD_MSTR, val_cr4);
 
@@ -316,6 +319,7 @@ static int fsl_sai_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
 static int fsl_sai_set_bclk(struct snd_soc_dai *dai, bool tx, u32 freq)
 {
 	struct fsl_sai *sai = snd_soc_dai_get_drvdata(dai);
+	unsigned int ofs = sai->soc_data->reg_offset;
 	unsigned long clk_rate;
 	u32 savediv = 0, ratio, savesub = freq;
 	u32 id;
@@ -378,17 +382,17 @@ static int fsl_sai_set_bclk(struct snd_soc_dai *dai, bool tx, u32 freq)
 	 */
 	if ((sai->synchronous[TX] && !sai->synchronous[RX]) ||
 	    (!tx && !sai->synchronous[RX])) {
-		regmap_update_bits(sai->regmap, FSL_SAI_RCR2,
+		regmap_update_bits(sai->regmap, FSL_SAI_RCR2(ofs),
 				   FSL_SAI_CR2_MSEL_MASK,
 				   FSL_SAI_CR2_MSEL(sai->mclk_id[tx]));
-		regmap_update_bits(sai->regmap, FSL_SAI_RCR2,
+		regmap_update_bits(sai->regmap, FSL_SAI_RCR2(ofs),
 				   FSL_SAI_CR2_DIV_MASK, savediv - 1);
 	} else if ((sai->synchronous[RX] && !sai->synchronous[TX]) ||
 		   (tx && !sai->synchronous[TX])) {
-		regmap_update_bits(sai->regmap, FSL_SAI_TCR2,
+		regmap_update_bits(sai->regmap, FSL_SAI_TCR2(ofs),
 				   FSL_SAI_CR2_MSEL_MASK,
 				   FSL_SAI_CR2_MSEL(sai->mclk_id[tx]));
-		regmap_update_bits(sai->regmap, FSL_SAI_TCR2,
+		regmap_update_bits(sai->regmap, FSL_SAI_TCR2(ofs),
 				   FSL_SAI_CR2_DIV_MASK, savediv - 1);
 	}
 
@@ -403,6 +407,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *cpu_dai)
 {
 	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+	unsigned int ofs = sai->soc_data->reg_offset;
 	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
 	unsigned int channels = params_channels(params);
 	u32 word_width = params_width(params);
@@ -455,19 +460,19 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
 
 	if (!sai->is_slave_mode) {
 		if (!sai->synchronous[TX] && sai->synchronous[RX] && !tx) {
-			regmap_update_bits(sai->regmap, FSL_SAI_TCR4,
+			regmap_update_bits(sai->regmap, FSL_SAI_TCR4(ofs),
 				FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
 				val_cr4);
-			regmap_update_bits(sai->regmap, FSL_SAI_TCR5,
+			regmap_update_bits(sai->regmap, FSL_SAI_TCR5(ofs),
 				FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
 				FSL_SAI_CR5_FBT_MASK, val_cr5);
 			regmap_write(sai->regmap, FSL_SAI_TMR,
 				~0UL - ((1 << channels) - 1));
 		} else if (!sai->synchronous[RX] && sai->synchronous[TX] && tx) {
-			regmap_update_bits(sai->regmap, FSL_SAI_RCR4,
+			regmap_update_bits(sai->regmap, FSL_SAI_RCR4(ofs),
 				FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
 				val_cr4);
-			regmap_update_bits(sai->regmap, FSL_SAI_RCR5,
+			regmap_update_bits(sai->regmap, FSL_SAI_RCR5(ofs),
 				FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
 				FSL_SAI_CR5_FBT_MASK, val_cr5);
 			regmap_write(sai->regmap, FSL_SAI_RMR,
@@ -475,26 +480,26 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
 		}
 	}
 
-	switch (sai->soc_data->fcomb_mode[tx]) {
+	switch (sai->fcomb_mode[tx]) {
 	case FSL_SAI_FCOMB_NONE:
-		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
+		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs),
 				   FSL_SAI_CR4_FCOMB_SOFT |
 				   FSL_SAI_CR4_FCOMB_SHIFT, 0);
 		break;
 	case FSL_SAI_FCOMB_SHIFT:
-		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
+		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs),
 				   FSL_SAI_CR4_FCOMB_SOFT |
 				   FSL_SAI_CR4_FCOMB_SHIFT,
 				   FSL_SAI_CR4_FCOMB_SHIFT);
 		break;
 	case FSL_SAI_FCOMB_SOFT:
-		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
+		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs),
 				   FSL_SAI_CR4_FCOMB_SOFT |
 				   FSL_SAI_CR4_FCOMB_SHIFT,
 				   FSL_SAI_CR4_FCOMB_SOFT);
 		break;
 	case FSL_SAI_FCOMB_BOTH:
-		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
+		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs),
 				   FSL_SAI_CR4_FCOMB_SOFT |
 				   FSL_SAI_CR4_FCOMB_SHIFT,
 				   FSL_SAI_CR4_FCOMB_SOFT |
@@ -504,10 +509,10 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
 		break;
 	}
 
-	regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
+	regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs),
 			   FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
 			   val_cr4);
-	regmap_update_bits(sai->regmap, FSL_SAI_xCR5(tx),
+	regmap_update_bits(sai->regmap, FSL_SAI_xCR5(tx, ofs),
 			   FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
 			   FSL_SAI_CR5_FBT_MASK, val_cr5);
 	regmap_write(sai->regmap, FSL_SAI_xMR(tx), ~0UL - ((1 << channels) - 1));
@@ -535,6 +540,8 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 		struct snd_soc_dai *cpu_dai)
 {
 	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+	unsigned int ofs = sai->soc_data->reg_offset;
+
 	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
 	u32 xcsr, count = 100;
 
@@ -543,9 +550,9 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 	 * Rx sync with Tx clocks: Clear SYNC for Tx, set it for Rx.
 	 * Tx sync with Rx clocks: Clear SYNC for Rx, set it for Tx.
 	 */
-	regmap_update_bits(sai->regmap, FSL_SAI_TCR2, FSL_SAI_CR2_SYNC,
-		           sai->synchronous[TX] ? FSL_SAI_CR2_SYNC : 0);
-	regmap_update_bits(sai->regmap, FSL_SAI_RCR2, FSL_SAI_CR2_SYNC,
+	regmap_update_bits(sai->regmap, FSL_SAI_TCR2(ofs), FSL_SAI_CR2_SYNC,
+			   sai->synchronous[TX] ? FSL_SAI_CR2_SYNC : 0);
+	regmap_update_bits(sai->regmap, FSL_SAI_RCR2(ofs), FSL_SAI_CR2_SYNC,
 			   sai->synchronous[RX] ? FSL_SAI_CR2_SYNC : 0);
 
 	/*
@@ -556,43 +563,44 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
+		regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
 				   FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
 
-		regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
+		regmap_update_bits(sai->regmap, FSL_SAI_RCSR(ofs),
 				   FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
-		regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
+		regmap_update_bits(sai->regmap, FSL_SAI_TCSR(ofs),
 				   FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
 
-		regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
+		regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
 				   FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
+		regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
 				   FSL_SAI_CSR_FRDE, 0);
-		regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
+		regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
 				   FSL_SAI_CSR_xIE_MASK, 0);
 
 		/* Check if the opposite FRDE is also disabled */
-		regmap_read(sai->regmap, FSL_SAI_xCSR(!tx), &xcsr);
+		regmap_read(sai->regmap, FSL_SAI_xCSR(!tx, ofs), &xcsr);
 		if (!(xcsr & FSL_SAI_CSR_FRDE)) {
 			/* Disable both directions and reset their FIFOs */
-			regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
+			regmap_update_bits(sai->regmap, FSL_SAI_TCSR(ofs),
 					   FSL_SAI_CSR_TERE, 0);
-			regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
+			regmap_update_bits(sai->regmap, FSL_SAI_RCSR(ofs),
 					   FSL_SAI_CSR_TERE, 0);
 
 			/* TERE will remain set till the end of current frame */
 			do {
 				udelay(10);
-				regmap_read(sai->regmap, FSL_SAI_xCSR(tx), &xcsr);
+				regmap_read(sai->regmap,
+					    FSL_SAI_xCSR(tx, ofs), &xcsr);
 			} while (--count && xcsr & FSL_SAI_CSR_TERE);
 
-			regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
+			regmap_update_bits(sai->regmap, FSL_SAI_TCSR(ofs),
 					   FSL_SAI_CSR_FR, FSL_SAI_CSR_FR);
-			regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
+			regmap_update_bits(sai->regmap, FSL_SAI_RCSR(ofs),
 					   FSL_SAI_CSR_FR, FSL_SAI_CSR_FR);
 
 			/*
@@ -604,13 +612,13 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 			 */
 			if (!sai->is_slave_mode) {
 				/* Software Reset for both Tx and Rx */
-				regmap_write(sai->regmap,
-					     FSL_SAI_TCSR, FSL_SAI_CSR_SR);
-				regmap_write(sai->regmap,
-					     FSL_SAI_RCSR, FSL_SAI_CSR_SR);
+				regmap_write(sai->regmap, FSL_SAI_TCSR(ofs),
+					     FSL_SAI_CSR_SR);
+				regmap_write(sai->regmap, FSL_SAI_RCSR(ofs),
+					     FSL_SAI_CSR_SR);
 				/* Clear SR bit to finish the reset */
-				regmap_write(sai->regmap, FSL_SAI_TCSR, 0);
-				regmap_write(sai->regmap, FSL_SAI_RCSR, 0);
+				regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), 0);
+				regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), 0);
 			}
 		}
 		break;
@@ -625,12 +633,13 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *cpu_dai)
 {
 	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+	unsigned int ofs = sai->soc_data->reg_offset;
 	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
 	int ret;
 
-	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx),
+	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
 			   FSL_SAI_CR3_TRCE_MASK,
-			   FSL_SAI_CR3_TRCE(sai->soc_data->dl_mask[tx]);
+			   FSL_SAI_CR3_TRCE(sai->dl_mask[tx]));
 
 	ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
 			SNDRV_PCM_HW_PARAM_RATE, &fsl_sai_rate_constraints);
@@ -642,9 +651,10 @@ static void fsl_sai_shutdown(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *cpu_dai)
 {
 	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+	unsigned int ofs = sai->soc_data->reg_offset;
 	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
 
-	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx),
+	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
 			   FSL_SAI_CR3_TRCE_MASK, 0);
 }
 
@@ -662,18 +672,20 @@ static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
 static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai)
 {
 	struct fsl_sai *sai = dev_get_drvdata(cpu_dai->dev);
+	unsigned int ofs = sai->soc_data->reg_offset;
 
 	/* Software Reset for both Tx and Rx */
-	regmap_write(sai->regmap, FSL_SAI_TCSR, FSL_SAI_CSR_SR);
-	regmap_write(sai->regmap, FSL_SAI_RCSR, FSL_SAI_CSR_SR);
+	regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), FSL_SAI_CSR_SR);
+	regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), FSL_SAI_CSR_SR);
 	/* Clear SR bit to finish the reset */
-	regmap_write(sai->regmap, FSL_SAI_TCSR, 0);
-	regmap_write(sai->regmap, FSL_SAI_RCSR, 0);
+	regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), 0);
+	regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), 0);
 
-	regmap_update_bits(sai->regmap, FSL_SAI_TCR1, FSL_SAI_CR1_RFW_MASK,
+	regmap_update_bits(sai->regmap, FSL_SAI_TCR1(ofs),
+			   FSL_SAI_CR1_RFW_MASK,
 			   sai->soc_data->fifo_depth - FSL_SAI_MAXBURST_TX);
-	regmap_update_bits(sai->regmap, FSL_SAI_RCR1, FSL_SAI_CR1_RFW_MASK,
-			   FSL_SAI_MAXBURST_RX - 1);
+	regmap_update_bits(sai->regmap, FSL_SAI_RCR1(ofs),
+			   FSL_SAI_CR1_RFW_MASK, FSL_SAI_MAXBURST_RX - 1);
 
 	snd_soc_dai_init_dma_data(cpu_dai, &sai->dma_params_tx,
 				&sai->dma_params_rx);
@@ -710,12 +722,12 @@ static const struct snd_soc_component_driver fsl_component = {
 	.name           = "fsl-sai",
 };
 
-static struct reg_default fsl_sai_reg_defaults[] = {
-	{FSL_SAI_TCR1, 0},
-	{FSL_SAI_TCR2, 0},
-	{FSL_SAI_TCR3, 0},
-	{FSL_SAI_TCR4, 0},
-	{FSL_SAI_TCR5, 0},
+static struct reg_default fsl_sai_reg_defaults_ofs0[] = {
+	{FSL_SAI_TCR1(0), 0},
+	{FSL_SAI_TCR2(0), 0},
+	{FSL_SAI_TCR3(0), 0},
+	{FSL_SAI_TCR4(0), 0},
+	{FSL_SAI_TCR5(0), 0},
 	{FSL_SAI_TDR0, 0},
 	{FSL_SAI_TDR1, 0},
 	{FSL_SAI_TDR2, 0},
@@ -724,24 +736,50 @@ static struct reg_default fsl_sai_reg_defaults[] = {
 	{FSL_SAI_TDR5, 0},
 	{FSL_SAI_TDR6, 0},
 	{FSL_SAI_TDR7, 0},
-	{FSL_SAI_TMR,  0},
-	{FSL_SAI_RCR1, 0},
-	{FSL_SAI_RCR2, 0},
-	{FSL_SAI_RCR3, 0},
-	{FSL_SAI_RCR4, 0},
-	{FSL_SAI_RCR5, 0},
-	{FSL_SAI_RMR,  0},
+	{FSL_SAI_TMR, 0},
+	{FSL_SAI_RCR1(0), 0},
+	{FSL_SAI_RCR2(0), 0},
+	{FSL_SAI_RCR3(0), 0},
+	{FSL_SAI_RCR4(0), 0},
+	{FSL_SAI_RCR5(0), 0},
+	{FSL_SAI_RMR, 0},
+};
+
+static struct reg_default fsl_sai_reg_defaults_ofs8[] = {
+	{FSL_SAI_TCR1(8), 0},
+	{FSL_SAI_TCR2(8), 0},
+	{FSL_SAI_TCR3(8), 0},
+	{FSL_SAI_TCR4(8), 0},
+	{FSL_SAI_TCR5(8), 0},
+	{FSL_SAI_TDR0, 0},
+	{FSL_SAI_TDR1, 0},
+	{FSL_SAI_TDR2, 0},
+	{FSL_SAI_TDR3, 0},
+	{FSL_SAI_TDR4, 0},
+	{FSL_SAI_TDR5, 0},
+	{FSL_SAI_TDR6, 0},
+	{FSL_SAI_TDR7, 0},
+	{FSL_SAI_TMR, 0},
+	{FSL_SAI_RCR1(8), 0},
+	{FSL_SAI_RCR2(8), 0},
+	{FSL_SAI_RCR3(8), 0},
+	{FSL_SAI_RCR4(8), 0},
+	{FSL_SAI_RCR5(8), 0},
+	{FSL_SAI_RMR, 0},
 };
 
 static bool fsl_sai_readable_reg(struct device *dev, unsigned int reg)
 {
+	struct fsl_sai *sai = dev_get_drvdata(dev);
+	unsigned int ofs = sai->soc_data->reg_offset;
+
+	if (reg >= FSL_SAI_TCSR(ofs) && reg <= FSL_SAI_TCR5(ofs))
+		return true;
+
+	if (reg >= FSL_SAI_RCSR(ofs) && reg <= FSL_SAI_RCR5(ofs))
+		return true;
+
 	switch (reg) {
-	case FSL_SAI_TCSR:
-	case FSL_SAI_TCR1:
-	case FSL_SAI_TCR2:
-	case FSL_SAI_TCR3:
-	case FSL_SAI_TCR4:
-	case FSL_SAI_TCR5:
 	case FSL_SAI_TFR0:
 	case FSL_SAI_TFR1:
 	case FSL_SAI_TFR2:
@@ -751,12 +789,6 @@ static bool fsl_sai_readable_reg(struct device *dev, unsigned int reg)
 	case FSL_SAI_TFR6:
 	case FSL_SAI_TFR7:
 	case FSL_SAI_TMR:
-	case FSL_SAI_RCSR:
-	case FSL_SAI_RCR1:
-	case FSL_SAI_RCR2:
-	case FSL_SAI_RCR3:
-	case FSL_SAI_RCR4:
-	case FSL_SAI_RCR5:
 	case FSL_SAI_RDR0:
 	case FSL_SAI_RDR1:
 	case FSL_SAI_RDR2:
@@ -782,9 +814,13 @@ static bool fsl_sai_readable_reg(struct device *dev, unsigned int reg)
 
 static bool fsl_sai_volatile_reg(struct device *dev, unsigned int reg)
 {
+	struct fsl_sai *sai = dev_get_drvdata(dev);
+	unsigned int ofs = sai->soc_data->reg_offset;
+
+	if (reg == FSL_SAI_TCSR(ofs) || reg == FSL_SAI_RCSR(ofs))
+		return true;
+
 	switch (reg) {
-	case FSL_SAI_TCSR:
-	case FSL_SAI_RCSR:
 	case FSL_SAI_TFR0:
 	case FSL_SAI_TFR1:
 	case FSL_SAI_TFR2:
@@ -817,13 +853,16 @@ static bool fsl_sai_volatile_reg(struct device *dev, unsigned int reg)
 
 static bool fsl_sai_writeable_reg(struct device *dev, unsigned int reg)
 {
+	struct fsl_sai *sai = dev_get_drvdata(dev);
+	unsigned int ofs = sai->soc_data->reg_offset;
+
+	if (reg >= FSL_SAI_TCSR(ofs) && reg <= FSL_SAI_TCR5(ofs))
+		return true;
+
+	if (reg >= FSL_SAI_RCSR(ofs) && reg <= FSL_SAI_RCR5(ofs))
+		return true;
+
 	switch (reg) {
-	case FSL_SAI_TCSR:
-	case FSL_SAI_TCR1:
-	case FSL_SAI_TCR2:
-	case FSL_SAI_TCR3:
-	case FSL_SAI_TCR4:
-	case FSL_SAI_TCR5:
 	case FSL_SAI_TDR0:
 	case FSL_SAI_TDR1:
 	case FSL_SAI_TDR2:
@@ -833,12 +872,6 @@ static bool fsl_sai_writeable_reg(struct device *dev, unsigned int reg)
 	case FSL_SAI_TDR6:
 	case FSL_SAI_TDR7:
 	case FSL_SAI_TMR:
-	case FSL_SAI_RCSR:
-	case FSL_SAI_RCR1:
-	case FSL_SAI_RCR2:
-	case FSL_SAI_RCR3:
-	case FSL_SAI_RCR4:
-	case FSL_SAI_RCR5:
 	case FSL_SAI_RMR:
 		return true;
 	default:
@@ -846,14 +879,14 @@ static bool fsl_sai_writeable_reg(struct device *dev, unsigned int reg)
 	}
 }
 
-static const struct regmap_config fsl_sai_regmap_config = {
+static struct regmap_config fsl_sai_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
 	.val_bits = 32,
 
 	.max_register = FSL_SAI_RMR,
-	.reg_defaults = fsl_sai_reg_defaults,
-	.num_reg_defaults = ARRAY_SIZE(fsl_sai_reg_defaults),
+	.reg_defaults = fsl_sai_reg_defaults_ofs0,
+	.num_reg_defaults = ARRAY_SIZE(fsl_sai_reg_defaults_ofs0),
 	.readable_reg = fsl_sai_readable_reg,
 	.volatile_reg = fsl_sai_volatile_reg,
 	.writeable_reg = fsl_sai_writeable_reg,
@@ -885,6 +918,12 @@ static int fsl_sai_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	if (sai->soc_data->reg_offset == 8) {
+		fsl_sai_regmap_config.reg_defaults = fsl_sai_reg_defaults_ofs8;
+		fsl_sai_regmap_config.num_reg_defaults =
+			ARRAY_SIZE(fsl_sai_reg_defaults_ofs8);
+	}
+
 	sai->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
 			"bus", base, &fsl_sai_regmap_config);
 
@@ -1017,11 +1056,13 @@ static int fsl_sai_remove(struct platform_device *pdev)
 static const struct fsl_sai_soc_data fsl_sai_vf610_data = {
 	.use_imx_pcm = false,
 	.fifo_depth = 32,
+	.reg_offset = 0,
 };
 
 static const struct fsl_sai_soc_data fsl_sai_imx6sx_data = {
 	.use_imx_pcm = true,
 	.fifo_depth = 32,
+	.reg_offset = 0,
 };
 
 static const struct of_device_id fsl_sai_ids[] = {
@@ -1054,6 +1095,7 @@ static int fsl_sai_runtime_suspend(struct device *dev)
 static int fsl_sai_runtime_resume(struct device *dev)
 {
 	struct fsl_sai *sai = dev_get_drvdata(dev);
+	unsigned int ofs = sai->soc_data->reg_offset;
 	int ret;
 
 	ret = clk_prepare_enable(sai->bus_clk);
@@ -1075,11 +1117,11 @@ static int fsl_sai_runtime_resume(struct device *dev)
 	}
 
 	regcache_cache_only(sai->regmap, false);
-	regmap_write(sai->regmap, FSL_SAI_TCSR, FSL_SAI_CSR_SR);
-	regmap_write(sai->regmap, FSL_SAI_RCSR, FSL_SAI_CSR_SR);
+	regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), FSL_SAI_CSR_SR);
+	regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), FSL_SAI_CSR_SR);
 	usleep_range(1000, 2000);
-	regmap_write(sai->regmap, FSL_SAI_TCSR, 0);
-	regmap_write(sai->regmap, FSL_SAI_RCSR, 0);
+	regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), 0);
+	regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), 0);
 
 	ret = regcache_sync(sai->regmap);
 	if (ret)
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index abf140951187..d20f16cc2a80 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -14,12 +14,12 @@
 			 SNDRV_PCM_FMTBIT_S32_LE)
 
 /* SAI Register Map Register */
-#define FSL_SAI_TCSR	0x00 /* SAI Transmit Control */
-#define FSL_SAI_TCR1	0x04 /* SAI Transmit Configuration 1 */
-#define FSL_SAI_TCR2	0x08 /* SAI Transmit Configuration 2 */
-#define FSL_SAI_TCR3	0x0c /* SAI Transmit Configuration 3 */
-#define FSL_SAI_TCR4	0x10 /* SAI Transmit Configuration 4 */
-#define FSL_SAI_TCR5	0x14 /* SAI Transmit Configuration 5 */
+#define FSL_SAI_TCSR(ofs)	(0x00 + ofs) /* SAI Transmit Control */
+#define FSL_SAI_TCR1(ofs)	(0x04 + ofs) /* SAI Transmit Configuration 1 */
+#define FSL_SAI_TCR2(ofs)	(0x08 + ofs) /* SAI Transmit Configuration 2 */
+#define FSL_SAI_TCR3(ofs)	(0x0c + ofs) /* SAI Transmit Configuration 3 */
+#define FSL_SAI_TCR4(ofs)	(0x10 + ofs) /* SAI Transmit Configuration 4 */
+#define FSL_SAI_TCR5(ofs)	(0x14 + ofs) /* SAI Transmit Configuration 5 */
 #define FSL_SAI_TDR0	0x20 /* SAI Transmit Data 0 */
 #define FSL_SAI_TDR1	0x24 /* SAI Transmit Data 1 */
 #define FSL_SAI_TDR2	0x28 /* SAI Transmit Data 2 */
@@ -37,12 +37,12 @@
 #define FSL_SAI_TFR6	0x58 /* SAI Transmit FIFO 6 */
 #define FSL_SAI_TFR7	0x5C /* SAI Transmit FIFO 7 */
 #define FSL_SAI_TMR	0x60 /* SAI Transmit Mask */
-#define FSL_SAI_RCSR	0x80 /* SAI Receive Control */
-#define FSL_SAI_RCR1	0x84 /* SAI Receive Configuration 1 */
-#define FSL_SAI_RCR2	0x88 /* SAI Receive Configuration 2 */
-#define FSL_SAI_RCR3	0x8c /* SAI Receive Configuration 3 */
-#define FSL_SAI_RCR4	0x90 /* SAI Receive Configuration 4 */
-#define FSL_SAI_RCR5	0x94 /* SAI Receive Configuration 5 */
+#define FSL_SAI_RCSR(ofs)	(0x80 + ofs) /* SAI Receive Control */
+#define FSL_SAI_RCR1(ofs)	(0x84 + ofs)/* SAI Receive Configuration 1 */
+#define FSL_SAI_RCR2(ofs)	(0x88 + ofs) /* SAI Receive Configuration 2 */
+#define FSL_SAI_RCR3(ofs)	(0x8c + ofs) /* SAI Receive Configuration 3 */
+#define FSL_SAI_RCR4(ofs)	(0x90 + ofs) /* SAI Receive Configuration 4 */
+#define FSL_SAI_RCR5(ofs)	(0x94 + ofs) /* SAI Receive Configuration 5 */
 #define FSL_SAI_RDR0	0xa0 /* SAI Receive Data 0 */
 #define FSL_SAI_RDR1	0xa4 /* SAI Receive Data 1 */
 #define FSL_SAI_RDR2	0xa8 /* SAI Receive Data 2 */
@@ -61,14 +61,14 @@
 #define FSL_SAI_RFR7	0xdc /* SAI Receive FIFO 7 */
 #define FSL_SAI_RMR	0xe0 /* SAI Receive Mask */
 
-#define FSL_SAI_xCSR(tx)	(tx ? FSL_SAI_TCSR : FSL_SAI_RCSR)
-#define FSL_SAI_xCR1(tx)	(tx ? FSL_SAI_TCR1 : FSL_SAI_RCR1)
-#define FSL_SAI_xCR2(tx)	(tx ? FSL_SAI_TCR2 : FSL_SAI_RCR2)
-#define FSL_SAI_xCR3(tx)	(tx ? FSL_SAI_TCR3 : FSL_SAI_RCR3)
-#define FSL_SAI_xCR4(tx)	(tx ? FSL_SAI_TCR4 : FSL_SAI_RCR4)
-#define FSL_SAI_xCR5(tx)	(tx ? FSL_SAI_TCR5 : FSL_SAI_RCR5)
-#define FSL_SAI_xDR(tx)		(tx ? FSL_SAI_TDR : FSL_SAI_RDR)
-#define FSL_SAI_xFR(tx)		(tx ? FSL_SAI_TFR : FSL_SAI_RFR)
+#define FSL_SAI_xCSR(tx, ofs)	(tx ? FSL_SAI_TCSR(ofs) : FSL_SAI_RCSR(ofs))
+#define FSL_SAI_xCR1(tx, ofs)	(tx ? FSL_SAI_TCR1(ofs) : FSL_SAI_RCR1(ofs))
+#define FSL_SAI_xCR2(tx, ofs)	(tx ? FSL_SAI_TCR2(ofs) : FSL_SAI_RCR2(ofs))
+#define FSL_SAI_xCR3(tx, ofs)	(tx ? FSL_SAI_TCR3(ofs) : FSL_SAI_RCR3(ofs))
+#define FSL_SAI_xCR4(tx, ofs)	(tx ? FSL_SAI_TCR4(ofs) : FSL_SAI_RCR4(ofs))
+#define FSL_SAI_xCR5(tx, ofs)	(tx ? FSL_SAI_TCR5(ofs) : FSL_SAI_RCR5(ofs))
+#define FSL_SAI_xDR(tx, ofs)	(tx ? FSL_SAI_TDR(ofs) : FSL_SAI_RDR(ofs))
+#define FSL_SAI_xFR(tx, ofs)	(tx ? FSL_SAI_TFR(ofs) : FSL_SAI_RFR(ofs))
 #define FSL_SAI_xMR(tx)		(tx ? FSL_SAI_TMR : FSL_SAI_RMR)
 
 /* SAI Transmit/Receive Control Register */
@@ -166,6 +166,7 @@
 struct fsl_sai_soc_data {
 	bool use_imx_pcm;
 	unsigned int fifo_depth;
+	unsigned int reg_offset;
 };
 
 struct fsl_sai {
-- 
2.17.1


^ permalink raw reply related

* [PATCH 04/10] ASoC: fsl_sai: Update Tx/Rx channel enable mask
From: Daniel Baluta @ 2019-07-22 12:48 UTC (permalink / raw)
  To: broonie
  Cc: Daniel Baluta, alsa-devel, viorel.suman, timur, Xiubo.Lee,
	linuxppc-dev, shengjiu.wang, angus, tiwai, perex, nicoleotsuka,
	linux-imx, kernel, festevam, linux-kernel, l.stach
In-Reply-To: <20190722124833.28757-1-daniel.baluta@nxp.com>

Tx channel enable (TCE) / Rx channel enable (RCE) bits
enable corresponding data channel for Tx/Rx operation.

Because SAI supports up the 8 channels TCE/RCE occupy
up the 8 bits inside TCR3/RCR3 registers we need to extend
the mask to reflect this.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 sound/soc/fsl/fsl_sai.c | 6 ++++--
 sound/soc/fsl/fsl_sai.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 7f8823fe4b90..768341608695 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -599,7 +599,8 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,
 	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
 	int ret;
 
-	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx), FSL_SAI_CR3_TRCE,
+	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx),
+			   FSL_SAI_CR3_TRCE_MASK,
 			   FSL_SAI_CR3_TRCE);
 
 	ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
@@ -614,7 +615,8 @@ static void fsl_sai_shutdown(struct snd_pcm_substream *substream,
 	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
 	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
 
-	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx), FSL_SAI_CR3_TRCE, 0);
+	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx),
+			   FSL_SAI_CR3_TRCE_MASK, 0);
 }
 
 static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 4bb478041d67..b1abeed2f78e 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -110,6 +110,7 @@
 
 /* SAI Transmit and Receive Configuration 3 Register */
 #define FSL_SAI_CR3_TRCE	BIT(16)
+#define FSL_SAI_CR3_TRCE_MASK	GENMASK(16, 23)
 #define FSL_SAI_CR3_WDFL(x)	(x)
 #define FSL_SAI_CR3_WDFL_MASK	0x1f
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH 07/10] ASoC: fsl_sai: Add support for FIFO combine mode
From: Daniel Baluta @ 2019-07-22 12:48 UTC (permalink / raw)
  To: broonie
  Cc: Daniel Baluta, alsa-devel, viorel.suman, timur, Xiubo.Lee,
	linuxppc-dev, shengjiu.wang, angus, tiwai, perex, nicoleotsuka,
	linux-imx, kernel, festevam, linux-kernel, l.stach
In-Reply-To: <20190722124833.28757-1-daniel.baluta@nxp.com>

FIFO combining mode allows the separate FIFOs for multiple data channels
to be used as a single FIFO for either software accesses or a single data
channel or both.

FIFO combined mode is described in chapter 13.10.3.5.4 from i.MX8MQ
reference manual [1].

For each direction (RX/TX) fifo combine mode is read from fsl,fcomb-mode
DT property. By default, if no property is specified fifo combine mode
is disabled.

[1]https://cache.nxp.com/secured/assets/documents/en/reference-manual/IMX8MDQLQRM.pdf?__gda__=1563728701_38bea7f0f726472cc675cb141b91bec7&fileExt=.pdf

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 sound/soc/fsl/fsl_sai.c | 37 +++++++++++++++++++++++++++++++++++++
 sound/soc/fsl/fsl_sai.h |  9 +++++++++
 2 files changed, 46 insertions(+)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index d0fa02188b7c..140014901fce 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -475,6 +475,35 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
 		}
 	}
 
+	switch (sai->soc_data->fcomb_mode[tx]) {
+	case FSL_SAI_FCOMB_NONE:
+		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
+				   FSL_SAI_CR4_FCOMB_SOFT |
+				   FSL_SAI_CR4_FCOMB_SHIFT, 0);
+		break;
+	case FSL_SAI_FCOMB_SHIFT:
+		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
+				   FSL_SAI_CR4_FCOMB_SOFT |
+				   FSL_SAI_CR4_FCOMB_SHIFT,
+				   FSL_SAI_CR4_FCOMB_SHIFT);
+		break;
+	case FSL_SAI_FCOMB_SOFT:
+		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
+				   FSL_SAI_CR4_FCOMB_SOFT |
+				   FSL_SAI_CR4_FCOMB_SHIFT,
+				   FSL_SAI_CR4_FCOMB_SOFT);
+		break;
+	case FSL_SAI_FCOMB_BOTH:
+		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
+				   FSL_SAI_CR4_FCOMB_SOFT |
+				   FSL_SAI_CR4_FCOMB_SHIFT,
+				   FSL_SAI_CR4_FCOMB_SOFT |
+				   FSL_SAI_CR4_FCOMB_SHIFT);
+		break;
+	default:
+		break;
+	}
+
 	regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
 			   FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
 			   val_cr4);
@@ -887,6 +916,14 @@ static int fsl_sai_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* FIFO combine mode for TX/RX, defaults to disabled */
+	sai->fcomb_mode[RX] = FSL_SAI_FCOMB_NONE;
+	sai->fcomb_mode[TX] = FSL_SAI_FCOMB_NONE;
+	of_property_read_u32_index(np, "fsl,fcomb-mode", RX,
+				   &sai->fcomb_mode[RX]);
+	of_property_read_u32_index(np, "fsl,fcomb-mode", TX,
+				   &sai->fcomb_mode[TX]);
+
 	/* active data lines mask for TX/RX, defaults to 1 (only the first
 	 * data line is enabled
 	 */
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 6d32f0950ec5..abf140951187 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -115,6 +115,8 @@
 #define FSL_SAI_CR3_WDFL_MASK	0x1f
 
 /* SAI Transmit and Receive Configuration 4 Register */
+#define FSL_SAI_CR4_FCOMB_SHIFT BIT(26)
+#define FSL_SAI_CR4_FCOMB_SOFT  BIT(27)
 #define FSL_SAI_CR4_FRSZ(x)	(((x) - 1) << 16)
 #define FSL_SAI_CR4_FRSZ_MASK	(0x1f << 16)
 #define FSL_SAI_CR4_SYWD(x)	(((x) - 1) << 8)
@@ -155,6 +157,12 @@
 #define FSL_SAI_MAXBURST_TX 6
 #define FSL_SAI_MAXBURST_RX 6
 
+/* FIFO combine modes */
+#define FSL_SAI_FCOMB_NONE     0
+#define FSL_SAI_FCOMB_SHIFT    1
+#define FSL_SAI_FCOMB_SOFT     2
+#define FSL_SAI_FCOMB_BOTH     3
+
 struct fsl_sai_soc_data {
 	bool use_imx_pcm;
 	unsigned int fifo_depth;
@@ -177,6 +185,7 @@ struct fsl_sai {
 	unsigned int slot_width;
 
 	unsigned int dl_mask[2];
+	unsigned int fcomb_mode[2];
 
 	const struct fsl_sai_soc_data *soc_data;
 	struct snd_dmaengine_dai_dma_data dma_params_rx;
-- 
2.17.1


^ permalink raw reply related

* [PATCH 01/10] ASoC: fsl_sai: add of_match data
From: Daniel Baluta @ 2019-07-22 12:48 UTC (permalink / raw)
  To: broonie
  Cc: alsa-devel, viorel.suman, timur, Xiubo.Lee, linuxppc-dev,
	shengjiu.wang, angus, tiwai, perex, nicoleotsuka, linux-imx,
	kernel, festevam, linux-kernel, l.stach
In-Reply-To: <20190722124833.28757-1-daniel.baluta@nxp.com>

From: Lucas Stach <l.stach@pengutronix.de>

New revisions of the SAI IP block have even more differences that need
be taken into account by the driver. To avoid sprinking compatible
checks all over the driver move the current differences into of_match_data.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 sound/soc/fsl/fsl_sai.c | 22 ++++++++++++++--------
 sound/soc/fsl/fsl_sai.h |  6 +++++-
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index d58cc3ae90d8..ed0432e7327a 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -9,6 +9,7 @@
 #include <linux/dmaengine.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
@@ -788,10 +789,7 @@ static int fsl_sai_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	sai->pdev = pdev;
-
-	if (of_device_is_compatible(np, "fsl,imx6sx-sai") ||
-	    of_device_is_compatible(np, "fsl,imx6ul-sai"))
-		sai->sai_on_imx = true;
+	sai->soc_data = of_device_get_match_data(&pdev->dev);
 
 	sai->is_lsb_first = of_property_read_bool(np, "lsb-first");
 
@@ -900,7 +898,7 @@ static int fsl_sai_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	if (sai->sai_on_imx)
+	if (sai->soc_data->use_imx_pcm)
 		return imx_pcm_dma_init(pdev, IMX_SAI_DMABUF_SIZE);
 	else
 		return devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
@@ -913,10 +911,18 @@ static int fsl_sai_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct fsl_sai_soc_data fsl_sai_vf610_data = {
+	.use_imx_pcm = false,
+};
+
+static const struct fsl_sai_soc_data fsl_sai_imx6sx_data = {
+	.use_imx_pcm = true,
+};
+
 static const struct of_device_id fsl_sai_ids[] = {
-	{ .compatible = "fsl,vf610-sai", },
-	{ .compatible = "fsl,imx6sx-sai", },
-	{ .compatible = "fsl,imx6ul-sai", },
+	{ .compatible = "fsl,vf610-sai", .data = &fsl_sai_vf610_data },
+	{ .compatible = "fsl,imx6sx-sai", .data = &fsl_sai_imx6sx_data },
+	{ .compatible = "fsl,imx6ul-sai", .data = &fsl_sai_imx6sx_data },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, fsl_sai_ids);
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 24cb156bf995..83e2bfe05b1b 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -126,6 +126,10 @@
 #define FSL_SAI_MAXBURST_TX 6
 #define FSL_SAI_MAXBURST_RX 6
 
+struct fsl_sai_soc_data {
+	bool use_imx_pcm;
+};
+
 struct fsl_sai {
 	struct platform_device *pdev;
 	struct regmap *regmap;
@@ -135,7 +139,6 @@ struct fsl_sai {
 	bool is_slave_mode;
 	bool is_lsb_first;
 	bool is_dsp_mode;
-	bool sai_on_imx;
 	bool synchronous[2];
 
 	unsigned int mclk_id[2];
@@ -143,6 +146,7 @@ struct fsl_sai {
 	unsigned int slots;
 	unsigned int slot_width;
 
+	const struct fsl_sai_soc_data *soc_data;
 	struct snd_dmaengine_dai_dma_data dma_params_rx;
 	struct snd_dmaengine_dai_dma_data dma_params_tx;
 };
-- 
2.17.1


^ permalink raw reply related

* [PATCH 10/10] ASoC: fsl_sai: Add support for imx7ulp/imx8mq
From: Daniel Baluta @ 2019-07-22 12:48 UTC (permalink / raw)
  To: broonie
  Cc: Daniel Baluta, alsa-devel, viorel.suman, timur, Xiubo.Lee,
	linuxppc-dev, shengjiu.wang, angus, tiwai, perex, nicoleotsuka,
	linux-imx, kernel, festevam, linux-kernel, l.stach
In-Reply-To: <20190722124833.28757-1-daniel.baluta@nxp.com>

SAI module on imx7ulp/imx8m features 2 new registers (VERID and PARAM)
at the beginning of register address space.

On imx7ulp FIFOs can held up to 16 x 32 bit samples.
On imx8mq FIFOs can held up to 128 x 32 bit samples.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 sound/soc/fsl/fsl_sai.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index f2441b84877e..b05837465b5a 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -1065,10 +1065,24 @@ static const struct fsl_sai_soc_data fsl_sai_imx6sx_data = {
 	.reg_offset = 0,
 };
 
+static const struct fsl_sai_soc_data fsl_sai_imx7ulp_data = {
+	.use_imx_pcm = true,
+	.fifo_depth = 16,
+	.reg_offset = 8,
+};
+
+static const struct fsl_sai_soc_data fsl_sai_imx8mq_data = {
+	.use_imx_pcm = true,
+	.fifo_depth = 128,
+	.reg_offset = 8,
+};
+
 static const struct of_device_id fsl_sai_ids[] = {
 	{ .compatible = "fsl,vf610-sai", .data = &fsl_sai_vf610_data },
 	{ .compatible = "fsl,imx6sx-sai", .data = &fsl_sai_imx6sx_data },
 	{ .compatible = "fsl,imx6ul-sai", .data = &fsl_sai_imx6sx_data },
+	{ .compatible = "fsl,imx7ulp-sai", .data = &fsl_sai_imx7ulp_data },
+	{ .compatible = "fsl,imx8mq-sai", .data = &fsl_sai_imx8mq_data },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, fsl_sai_ids);
-- 
2.17.1


^ permalink raw reply related

* [PATCH 02/10] ASoC: fsl_sai: derive TX FIFO watermark from FIFO depth
From: Daniel Baluta @ 2019-07-22 12:48 UTC (permalink / raw)
  To: broonie
  Cc: alsa-devel, viorel.suman, timur, Xiubo.Lee, linuxppc-dev,
	shengjiu.wang, angus, tiwai, perex, nicoleotsuka, linux-imx,
	kernel, festevam, linux-kernel, l.stach
In-Reply-To: <20190722124833.28757-1-daniel.baluta@nxp.com>

From: Lucas Stach <l.stach@pengutronix.de>

The DMA request schould be triggered as soon as the FIFO has space
for another burst. As different versions of the SAI block have
different FIFO sizes, the watrmark level needs to be derived from
version specific data.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 sound/soc/fsl/fsl_sai.c | 4 +++-
 sound/soc/fsl/fsl_sai.h | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index ed0432e7327a..1d1a447163e3 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -640,7 +640,7 @@ static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai)
 	regmap_write(sai->regmap, FSL_SAI_RCSR, 0);
 
 	regmap_update_bits(sai->regmap, FSL_SAI_TCR1, FSL_SAI_CR1_RFW_MASK,
-			   FSL_SAI_MAXBURST_TX * 2);
+			   sai->soc_data->fifo_depth - FSL_SAI_MAXBURST_TX);
 	regmap_update_bits(sai->regmap, FSL_SAI_RCR1, FSL_SAI_CR1_RFW_MASK,
 			   FSL_SAI_MAXBURST_RX - 1);
 
@@ -913,10 +913,12 @@ static int fsl_sai_remove(struct platform_device *pdev)
 
 static const struct fsl_sai_soc_data fsl_sai_vf610_data = {
 	.use_imx_pcm = false,
+	.fifo_depth = 32,
 };
 
 static const struct fsl_sai_soc_data fsl_sai_imx6sx_data = {
 	.use_imx_pcm = true,
+	.fifo_depth = 32,
 };
 
 static const struct of_device_id fsl_sai_ids[] = {
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 83e2bfe05b1b..7c1ef671da28 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -128,6 +128,7 @@
 
 struct fsl_sai_soc_data {
 	bool use_imx_pcm;
+	unsigned int fifo_depth;
 };
 
 struct fsl_sai {
-- 
2.17.1


^ permalink raw reply related

* [PATCH 08/10] ASoC: dt-bindings: Document fcomb_mode property
From: Daniel Baluta @ 2019-07-22 12:48 UTC (permalink / raw)
  To: broonie
  Cc: Daniel Baluta, alsa-devel, viorel.suman, timur, Xiubo.Lee,
	linuxppc-dev, shengjiu.wang, angus, tiwai, perex, nicoleotsuka,
	linux-imx, kernel, festevam, linux-kernel, l.stach
In-Reply-To: <20190722124833.28757-1-daniel.baluta@nxp.com>

This allows combining multiple-data-line FIFOs into a
single-data-line FIFO.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 Documentation/devicetree/bindings/sound/fsl-sai.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/fsl-sai.txt b/Documentation/devicetree/bindings/sound/fsl-sai.txt
index 59f4d965a5fb..ca27afd840ba 100644
--- a/Documentation/devicetree/bindings/sound/fsl-sai.txt
+++ b/Documentation/devicetree/bindings/sound/fsl-sai.txt
@@ -54,6 +54,10 @@ Optional properties:
 			  represents first data line, bit 1 represents second
 			  data line and so on. Data line is enabled if
 			  corresponding bit is set to 1.
+  - fsl,fcomb_mode	: list of two integers (first for RX, second for TX)
+			  representing FIFO combine mode. Possible values for
+			  combined mode are: 0 - disabled, 1 - Rx/Tx from shift
+			  registers, 2 - Rx/Tx by software, 3 - both.
 
 Optional properties (for mx6ul):
 
-- 
2.17.1


^ permalink raw reply related


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