LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v10 00/17] powerpc: "paca->soft_enabled" based local atomic operation implementation
From: Madhavan Srinivasan @ 2017-12-20  3:55 UTC (permalink / raw)
  To: mpe; +Cc: benh, anton, paulus, npiggin, linuxppc-dev, Madhavan Srinivasan

Local atomic operations are fast and highly reentrant per CPU counters.
Used for percpu variable updates. Local atomic operations only guarantee
variable modification atomicity wrt the CPU which owns the data and
these needs to be executed in a preemption safe way.

Here is the design of the patchset. Since local_* operations
are only need to be atomic to interrupts (IIUC), we have two options.
Either replay the "op" if interrupted or replay the interrupt after
the "op". Initial patchset posted was based on implementing local_* operation
based on CR5 which replay's the "op". Patchset had issues in case of
rewinding the address pointor from an array. This make the slow path
really slow. Since CR5 based implementation proposed using __ex_table to find
the rewind address, this rasied concerns about size of __ex_table and vmlinux.

https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-December/123115.html

But this patchset uses Benjamin Herrenschmidt suggestion of using
arch_local_irq_disable() to soft_disable interrupts (including PMIs).
After finishing the "op", arch_local_irq_restore() called and correspondingly
interrupts are replayed if any occured.

Current paca->soft_enabled logic is reserved and MASKABLE_EXCEPTION_* macros
are extended to support this feature.

patch re-write the current local_* functions to use arch_local_irq_disbale.
Base flow for each function is

 {
        powerpc_local_irq_pmu_save(flags)
        load
        ..
        store
        powerpc_local_irq_pmu_restore(flags)
 }

Reason for the approach is that, currently l[w/d]arx/st[w/d]cx.
instruction pair is used for local_* operations, which are heavy
on cycle count and they dont support a local variant. So to
see whether the new implementation helps, used a modified
version of Rusty's benchmark code on local_t.

https://lkml.org/lkml/2008/12/16/450

Modifications to Rusty's benchmark code:
 - Executed only local_t test

Here are the values with the patch.

Time in ns per iteration

Local_t         Without Patch           With Patch

_inc                    38              10
_add                    38              10
_read                   4               4
_add_return             38              10

Currently only asm/local.h has been rewritten, and also
the entire change is tested only in PPC64 (pseries guest)
and PPC64 LE host. Have only compile tested ppc64e_*.

Changelog v9:
 - Split some patches to make reviewing easier
 - Included couple fixes
 - Updated the comment and commit message
 - Renamed variables and functions

Changelog v8:
 - Rearranged series.
 - Updated the comments
 - Fixed a hang in embedded version (thanks to mpe)
 - Added couple of more cleanup patches to the series
 - Updated commit messages.

Changelog v7:
1)Missed first patch in the series

Changelog v6:
1)Moved the renaming of soft_enabled to soft_disable_mask patch earlier in the series.
2)Added code to hardwire "softe" value in pt_regs for userspace to be always 1
3)rebased to latest upstream.

Changelog v5:
1)Fixed the check in hard_irq_disable() macro for soft_disabled_mask

Changelog v4:
1)split the __SOFT_ENABLED logic check from patch 7 and merged to soft_enabled
logic reversing patch.
2)Made changes to commit messages
3)Added a new IRQ_DISBALE_MASK_ALL to include supported disabled mask bits. 

Changelog v3:
1)Made suggest to commit messages
2)Added a new patch (patch 12) to rename the soft_enabled to soft_disabled_mask

Changelog v2:
Rebased to latest upstream

Changelog v1:
1)squashed patches 1/2 together and 8/9/10 together for readability
2)Created a separate patch for the kconfig changes
3)Moved the new mask value commit to patch 11.
4)Renamed local_irq_pmu_*() to powerpc_irq_pmu_*() to avoid
  namespaces matches with generic kernel local_irq*() functions
5)Renamed __EXCEPTION_PROLOG_1 macro to MASKABLE_EXCEPTION_PROLOG_1 macro
6)Made changes to commit messages
7)Add more comments to codes

Changelog RFC v5:
1)Implemented new set of soft_enabled manipulation functions
2)rewritten arch_local_irq_* functions to use the new soft_enabled_*()
3)Add WARN_ON to identify invalid soft_enabled transitions
4)Added powerpc_local_irq_pmu_save() and powerpc_local_irq_pmu_restore() to
  support masking of irqs (with PMI).
5)Added local_irq_pmu_*()s macros with trace_hardirqs_on|off() to match
  include/linux/irqflags.h

Changelog RFC v4:
1)Fix build breaks in in ppc64e_defconfig compilation
2)Merged PMI replay code with the exception vector changes patch
3)Renamed the new API to set PMI mask bit as suggested
4)Modified the current arch_local_save and new API function call to
  "OR" and store the value to ->soft_enabled instead of just store.
5)Updated the check in the arch_local_irq_restore() to alway check for
  greather than or zero to _LINUX mask bit.
6)Updated the commit messages.

Changelog RFC v3:
1)Squashed PMI masked interrupt patch and replay patch together
2)Have created a new patch which includes a new Kconfig and set_irq_set_mask()
3)Fixed the compilation issue with IRQ_DISABLE_MASK_* macros in book3e_*

Changelog RFC v2:
1)Renamed IRQ_DISABLE_LEVEL_* to IRQ_DISABLE_MASK_* and made logic changes
  to treat soft_enabled as a mask and not a flag or level.
2)Added a new Kconfig variable to support a WARN_ON
3)Refactored patchset for eaiser review.
4)Made changes to commit messages.
5)Made changes for BOOK3E version

Changelog RFC v1:

1)Commit messages are improved.
2)Renamed the arch_local_irq_disable_var to soft_irq_set_level as suggested
3)Renamed the LAZY_INTERRUPT* macro to IRQ_DISABLE_LEVEL_* as suggested
4)Extended the MASKABLE_EXCEPTION* macros to support additional parameter.
5)Each MASKABLE_EXCEPTION_* macro will carry a "mask_level"
6)Logic to decide on jump to maskable_handler in SOFTEN_TEST is now based on
  "mask_level"
7)__EXCEPTION_PROLOG_1 is factored out to support "mask_level" parameter.
  This reduced the code changes needed for supporting "mask_level" parameters.

Madhavan Srinivasan (15):
  powerpc/64: Add #defines for paca->soft_enabled flags
  powerpc/64: Fix arch_local_irq_disable() prototype
  powerpc/64: move set_soft_enabled(), rename it, add memory clobber
  powerpc/64: Implement and use soft_enabled_return API
  powerpc/64: Implement and use soft_enabled_set_return API
  powerpc/64: Cleanup hard_irq_disable() macro
  powerpc/64: Change soft_enabled from flag to bitmask
  powerpc/64: Rename soft_enabled to irq_soft_mask
  powerpc/64s: Avoid using EXCEPTION_PROLOG_1 macro in MASKABLE_*
  powerpc/64s: Add support to take additional parameter in MASKABLE_*
    macro
  powerpc/64s: Add support to mask perf interrupts and replay them
  powerpc: Add new kconfig IRQ_DEBUG_SUPPORT
  powerpc/64s: Add new set of irq_soft_mask_ functions for PMI masking
  powerpc: use generic atomic implementation for local_t
  powerpc/64s: Implement local_t using irq soft masking

Nicholas Piggin (2):
  powerpc/64: do not trace irqs-off at interrupt return to soft-disabled
    context
  powerpc/64: Improve inline asm in arch_local_irq_disable

 arch/powerpc/Kconfig.debug               |   4 +
 arch/powerpc/include/asm/exception-64s.h | 103 ++++++++++------
 arch/powerpc/include/asm/head-64.h       |  40 +++----
 arch/powerpc/include/asm/hw_irq.h        | 165 ++++++++++++++++++++++---
 arch/powerpc/include/asm/irqflags.h      |  14 +--
 arch/powerpc/include/asm/kvm_ppc.h       |   2 +-
 arch/powerpc/include/asm/local.h         | 200 +++++++++++++------------------
 arch/powerpc/include/asm/paca.h          |   2 +-
 arch/powerpc/kernel/asm-offsets.c        |   2 +-
 arch/powerpc/kernel/entry_64.S           |  50 ++++----
 arch/powerpc/kernel/exceptions-64e.S     |  20 ++--
 arch/powerpc/kernel/exceptions-64s.S     |  38 +++---
 arch/powerpc/kernel/head_64.S            |  11 +-
 arch/powerpc/kernel/idle_book3e.S        |   5 +-
 arch/powerpc/kernel/idle_power4.S        |   5 +-
 arch/powerpc/kernel/irq.c                |  29 ++---
 arch/powerpc/kernel/optprobes_head.S     |   2 +-
 arch/powerpc/kernel/process.c            |   3 +-
 arch/powerpc/kernel/ptrace.c             |  12 ++
 arch/powerpc/kernel/setup_64.c           |   5 +-
 arch/powerpc/kernel/signal_32.c          |   8 ++
 arch/powerpc/kernel/signal_64.c          |   3 +
 arch/powerpc/kernel/time.c               |   6 +-
 arch/powerpc/kvm/book3s_hv_rmhandlers.S  |   2 +-
 arch/powerpc/mm/hugetlbpage.c            |   2 +-
 arch/powerpc/perf/core-book3s.c          |   2 +-
 arch/powerpc/xmon/xmon.c                 |   4 +-
 27 files changed, 458 insertions(+), 281 deletions(-)

-- 
2.7.4

^ permalink raw reply

* Re: [PATCH] powerpc: Add aacraid and nvme to powernv_defconfig
From: Benjamin Herrenschmidt @ 2017-12-20  2:14 UTC (permalink / raw)
  To: Alexey Kardashevskiy, linuxppc-dev
In-Reply-To: <ed9aaffc-bb59-a39a-7ddc-55d664a93d8c@ozlabs.ru>

On Wed, 2017-12-20 at 12:59 +1100, Alexey Kardashevskiy wrote:
> On 20/12/17 12:51, Benjamin Herrenschmidt wrote:
> > These adapters can be found in a number of our systems, so let's
> > enable the corresponding drivers by default.
> > 
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> > --- a/arch/powerpc/configs/powernv_defconfig	2017-12-19 18:37:24.803470591 -0600
> > +++ b/arch/powerpc/configs/powernv_defconfig	2017-12-19 19:47:57.931952417 -0600
> > @@ -97,6 +97,7 @@
> >  CONFIG_BLK_DEV_RAM=y
> >  CONFIG_BLK_DEV_RAM_SIZE=65536
> >  CONFIG_VIRTIO_BLK=m
> > +CONFIG_BLK_DEV_NVME=y
> >  CONFIG_IDE=y
> >  CONFIG_BLK_DEV_IDECD=y
> >  CONFIG_BLK_DEV_GENERIC=y
> > @@ -113,6 +114,7 @@
> >  CONFIG_SCSI_CXGB4_ISCSI=m
> >  CONFIG_SCSI_BNX2_ISCSI=m
> >  CONFIG_BE2ISCSI=m
> > +CONFIG_SCSI_AACRAID=y
> >  CONFIG_SCSI_MPT2SAS=m
> >  CONFIG_SCSI_SYM53C8XX_2=m
> >  CONFIG_SCSI_SYM53C8XX_DMA_ADDRESSING_MODE=0
> > 
> 
> "y", not "m"?

Yes. I like my boot devices to be "y"

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH] powerpc: Add aacraid and nvme to powernv_defconfig
From: Alexey Kardashevskiy @ 2017-12-20  1:59 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev
In-Reply-To: <1513734660.2743.14.camel@kernel.crashing.org>

On 20/12/17 12:51, Benjamin Herrenschmidt wrote:
> These adapters can be found in a number of our systems, so let's
> enable the corresponding drivers by default.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> --- a/arch/powerpc/configs/powernv_defconfig	2017-12-19 18:37:24.803470591 -0600
> +++ b/arch/powerpc/configs/powernv_defconfig	2017-12-19 19:47:57.931952417 -0600
> @@ -97,6 +97,7 @@
>  CONFIG_BLK_DEV_RAM=y
>  CONFIG_BLK_DEV_RAM_SIZE=65536
>  CONFIG_VIRTIO_BLK=m
> +CONFIG_BLK_DEV_NVME=y
>  CONFIG_IDE=y
>  CONFIG_BLK_DEV_IDECD=y
>  CONFIG_BLK_DEV_GENERIC=y
> @@ -113,6 +114,7 @@
>  CONFIG_SCSI_CXGB4_ISCSI=m
>  CONFIG_SCSI_BNX2_ISCSI=m
>  CONFIG_BE2ISCSI=m
> +CONFIG_SCSI_AACRAID=y
>  CONFIG_SCSI_MPT2SAS=m
>  CONFIG_SCSI_SYM53C8XX_2=m
>  CONFIG_SCSI_SYM53C8XX_DMA_ADDRESSING_MODE=0
> 

"y", not "m"?


-- 
Alexey

^ permalink raw reply

* [PATCH] powerpc: Add aacraid and nvme to powernv_defconfig
From: Benjamin Herrenschmidt @ 2017-12-20  1:51 UTC (permalink / raw)
  To: linuxppc-dev

These adapters can be found in a number of our systems, so let's
enable the corresponding drivers by default.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
--- a/arch/powerpc/configs/powernv_defconfig	2017-12-19 18:37:24.803470591 -0600
+++ b/arch/powerpc/configs/powernv_defconfig	2017-12-19 19:47:57.931952417 -0600
@@ -97,6 +97,7 @@
 CONFIG_BLK_DEV_RAM=y
 CONFIG_BLK_DEV_RAM_SIZE=65536
 CONFIG_VIRTIO_BLK=m
+CONFIG_BLK_DEV_NVME=y
 CONFIG_IDE=y
 CONFIG_BLK_DEV_IDECD=y
 CONFIG_BLK_DEV_GENERIC=y
@@ -113,6 +114,7 @@
 CONFIG_SCSI_CXGB4_ISCSI=m
 CONFIG_SCSI_BNX2_ISCSI=m
 CONFIG_BE2ISCSI=m
+CONFIG_SCSI_AACRAID=y
 CONFIG_SCSI_MPT2SAS=m
 CONFIG_SCSI_SYM53C8XX_2=m
 CONFIG_SCSI_SYM53C8XX_DMA_ADDRESSING_MODE=0

^ permalink raw reply

* [PATCH 1/1] powerpc: Emulate paste instruction
From: Sukadev Bhattiprolu @ 2017-12-20  1:10 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Benjamin Herrenschmidt, mikey, hbabu, aneesh.kumar, linuxppc-dev,
	linux-kernel

From: Michael Neuling <mikey@neuling.org>

On POWER9 DD2.1 and below there are issues when the paste instruction
generates an error. If an error occurs when thread reconfiguration
happens (ie another thread in the core goes into/out of powersave) the
core may hang.

To avoid this a special sequence is required which stops thread
configuration so that the paste can be safely executed.

This patch assumes paste executed in userspace are trapped into the
illegal instruction exception at 0xe40.

Here we re-execute the paste instruction but with the required
sequence to ensure thread reconfiguration doesn't occur.

Signed-off-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
Changelog[v4]:
	- We need to disable pagefaults after all when modifying the thread
	  reconfig registers. Use a mutex, rather than a spinlock around
	  the thread reconfig registers. Acquire the mutex first then block
	  interrupts so we don't sleep on the mutex with interrupts disabled.

Changlog[v3]:
	- [Michael Ellerman] We don't need to disable/enable pagefaults
	  when emulating paste;
	- [Michael Ellerman, Aneesh Kumar] Fix retval from emulate_paste()

Changelog[v2]:
	[Sukadev]: Use PPC_PASTE() rather than the paste instruction since
	in older versions the instruction required a third parameter.
---
 arch/powerpc/include/asm/emulated_ops.h |  1 +
 arch/powerpc/include/asm/ppc-opcode.h   |  1 +
 arch/powerpc/include/asm/reg.h          |  2 +
 arch/powerpc/kernel/traps.c             | 73 +++++++++++++++++++++++++++++++++
 4 files changed, 77 insertions(+)

diff --git a/arch/powerpc/include/asm/emulated_ops.h b/arch/powerpc/include/asm/emulated_ops.h
index 651e135..fdc95cf 100644
--- a/arch/powerpc/include/asm/emulated_ops.h
+++ b/arch/powerpc/include/asm/emulated_ops.h
@@ -59,6 +59,7 @@ extern struct ppc_emulated {
 	struct ppc_emulated_entry lxvh8x;
 	struct ppc_emulated_entry lxvd2x;
 	struct ppc_emulated_entry lxvb16x;
+	struct ppc_emulated_entry paste;
 #endif
 } ppc_emulated;
 
diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index ce0930d..a55d2ef 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -229,6 +229,7 @@
 #define PPC_INST_MTTMR			0x7c0003dc
 #define PPC_INST_NOP			0x60000000
 #define PPC_INST_PASTE			0x7c20070d
+#define PPC_INST_PASTE_MASK		0xfc2007ff
 #define PPC_INST_POPCNTB		0x7c0000f4
 #define PPC_INST_POPCNTB_MASK		0xfc0007fe
 #define PPC_INST_POPCNTD		0x7c0003f4
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index b779f3c..3495ecf 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -469,6 +469,8 @@
 #define SPRN_DBAT7U	0x23E	/* Data BAT 7 Upper Register */
 #define SPRN_PPR	0x380	/* SMT Thread status Register */
 #define SPRN_TSCR	0x399	/* Thread Switch Control Register */
+#define SPRN_TRIG1	0x371	/* WAT Trigger 1 */
+#define SPRN_TRIG2	0x372	/* WAT Trigger 2 */
 
 #define SPRN_DEC	0x016		/* Decrement Register */
 #define SPRN_DER	0x095		/* Debug Enable Register */
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index f3eb61b..e1ea3be 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1153,6 +1153,74 @@ static inline bool tm_abort_check(struct pt_regs *regs, int reason)
 }
 #endif
 
+static DEFINE_MUTEX(paste_emulation_mutex);
+
+static inline int paste(void *i)
+{
+	int cr;
+	long retval = 0;
+
+	/* Need per core lock to ensure trig1/2 writes don't race */
+	mutex_lock(&paste_emulation_mutex);
+
+	hard_irq_disable();
+
+	mtspr(SPRN_TRIG1, 0); /* data doesn't matter */
+	mtspr(SPRN_TRIG1, 0); /* HW says do this twice */
+	asm volatile(
+		"1: " PPC_PASTE(0, %2) "\n"
+		"2: mfcr %1\n"
+		".section .fixup,\"ax\"\n"
+		"3:	li %0,%3\n"
+		"	li %2,0\n"
+		"	b 2b\n"
+		".previous\n"
+		EX_TABLE(1b, 3b)
+		: "=r" (retval), "=r" (cr)
+		: "b" (i), "i" (-EFAULT), "0" (retval));
+	mtspr(SPRN_TRIG2, 0);
+
+	local_irq_enable();
+
+	mutex_unlock(&paste_emulation_mutex);
+
+	return retval ? retval : cr;
+}
+
+static int emulate_paste(struct pt_regs *regs, u32 instword)
+{
+	const void __user *addr;
+	unsigned long ea;
+	u8 ra, rb;
+	int rc;
+
+	if (!cpu_has_feature(CPU_FTR_ARCH_300))
+		return -EINVAL;
+
+	ra = (instword >> 16) & 0x1f;
+	rb = (instword >> 11) & 0x1f;
+
+	ea = regs->gpr[rb] + (ra ? regs->gpr[ra] : 0ul);
+	if (is_32bit_task())
+		ea &= 0xfffffffful;
+	addr = (__force const void __user *)ea;
+
+	if (!access_ok(VERIFY_WRITE, addr, 128)) // cacheline size == 128
+		return -EFAULT;
+
+	pagefault_disable();
+
+	PPC_WARN_EMULATED(paste, regs);
+	rc = paste((void *)addr);
+
+	/* set cr0 to 0 to indicate a paste failure */
+	regs->ccr = (rc == -EFAULT) ? 0 : rc;
+
+	pagefault_enable();
+
+	return (rc == -EFAULT) ? rc : 0;
+}
+
 static int emulate_instruction(struct pt_regs *regs)
 {
 	u32 instword;
@@ -1165,6 +1233,10 @@ static int emulate_instruction(struct pt_regs *regs)
 	if (get_user(instword, (u32 __user *)(regs->nip)))
 		return -EFAULT;
 
+	/* Emulate the paste RA, RB. */
+	if ((instword & PPC_INST_PASTE_MASK) == PPC_INST_PASTE)
+		return emulate_paste(regs, instword);
+
 	/* Emulate the mfspr rD, PVR. */
 	if ((instword & PPC_INST_MFSPR_PVR_MASK) == PPC_INST_MFSPR_PVR) {
 		PPC_WARN_EMULATED(mfpvr, regs);
@@ -2088,6 +2160,7 @@ struct ppc_emulated ppc_emulated = {
 	WARN_EMULATED_SETUP(lxvh8x),
 	WARN_EMULATED_SETUP(lxvd2x),
 	WARN_EMULATED_SETUP(lxvb16x),
+	WARN_EMULATED_SETUP(paste),
 #endif
 };
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/1] vas: vas_window_init_dbgdir: fix order of cleanup.
From: Sukadev Bhattiprolu @ 2017-12-20  1:10 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Benjamin Herrenschmidt, mikey, hbabu, aneesh.kumar, linuxppc-dev,
	linux-kernel
In-Reply-To: <1513732229-10066-1-git-send-email-sukadev@linux.vnet.ibm.com>

Fix the order of cleanup to ensure we free the name buffer in case
of an error creating 'hvwc' or 'info' files.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/vas-debug.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/vas-debug.c b/arch/powerpc/platforms/powernv/vas-debug.c
index ca22f1e..b4de4c6 100644
--- a/arch/powerpc/platforms/powernv/vas-debug.c
+++ b/arch/powerpc/platforms/powernv/vas-debug.c
@@ -166,13 +166,13 @@ void vas_window_init_dbgdir(struct vas_window *window)
 
 	return;
 
-free_name:
-	kfree(window->dbgname);
-	window->dbgname = NULL;
-
 remove_dir:
 	debugfs_remove_recursive(window->dbgdir);
 	window->dbgdir = NULL;
+
+free_name:
+	kfree(window->dbgname);
+	window->dbgname = NULL;
 }
 
 void vas_instance_init_dbgdir(struct vas_instance *vinst)
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] On ppc64le we HAVE_RELIABLE_STACKTRACE
From: Josh Poimboeuf @ 2017-12-19 21:46 UTC (permalink / raw)
  To: Torsten Duwe
  Cc: Nicholas Piggin, linux-kernel, Jiri Kosina, live-patching,
	linuxppc-dev
In-Reply-To: <20171219112833.GA8758@lst.de>

On Tue, Dec 19, 2017 at 12:28:33PM +0100, Torsten Duwe wrote:
> On Mon, Dec 18, 2017 at 12:56:22PM -0600, Josh Poimboeuf wrote:
> > On Mon, Dec 18, 2017 at 03:33:34PM +1000, Nicholas Piggin wrote:
> > > On Sun, 17 Dec 2017 20:58:54 -0600
> > > Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > > 
> > > > On Fri, Dec 15, 2017 at 07:40:09PM +1000, Nicholas Piggin wrote:
> > > > > On Tue, 12 Dec 2017 08:05:01 -0600
> > > > > Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > > > >   
> > > > > > What about leaf functions?  If a leaf function doesn't establish a stack
> > > > > > frame, and it has inline asm which contains a blr to another function,
> > > > > > this ABI is broken.  
> > > > 
> > > > Oops, I meant to say "bl" instead of "blr".
> 
> You need to save LR, one way or the other. If gcc thinks it's a leaf function and
> does not do it, nor does your asm code, you'll return in an endless loop => bug.

Ah, so the function's return path would be corrupted, and an unreliable
stack trace would be the least of our problems.

So it sounds like .c files should be fine, unless I'm missing something
else.

> > > > > > Also, even for non-leaf functions, is it possible for GCC to insert the
> > > > > > inline asm before it sets up the stack frame?  (This is an occasional
> > > > > > problem on x86.)  
> > > > > 
> > > > > Inline asm must not have control transfer out of the statement unless
> > > > > it is asm goto.  
> > > > 
> > > > Can inline asm have calls to other functions?
> > > 
> > > I don't believe so.
> > 
> > It's allowed on x86, I don't see why it wouldn't be allowed on powerpc.
> > As you mentioned, GCC doesn't pay attention to what's inside asm("").
> > 
> > > > > > Also, what about hand-coded asm?  
> > > > > 
> > > > > Should follow the same rules if it uses the stack.  
> > > > 
> > > > How is that enforced?
> > > 
> > > It's not, AFAIK. Gcc doesn't understand what's inside asm("").
> > 
> > Here I was talking about .S files.
> 
> asm("") or .S ... the ABI spec is clear, and it's quite easy to follow. You
> need a place to save LR before you call another function, and STDU is so
> convenient to create a stack frame with a single instruction.
> My impression is one would have to be very determined to break the ABI
> deliberately.

Ok.  However, I perused the powerpc crypto code, because that was the
source of a lot of frame pointer breakage in x86.  I noticed that some
of the crypto functions create their stack frame *before* writing the LR
to the caller's stack, which is the opposite of what compiled C code
does.  That might confuse the unwinder if it were preempted in between.

But more on that below...

> > > > > > In addition to fixing the above issues, the unwinder also needs to
> > > > > > detect interrupts (i.e., preemption) and page faults on the stack of a
> > > > > > blocked task.  If a function were preempted before it created a stack
> > > > > > frame, or if a leaf function blocked on a page fault, the stack trace
> > > > > > will skip the function's caller, so such a trace will need to be
> > > > > > reported to livepatch as unreliable.  
> > > > > 
> > > > > I don't think there is much problem there for powerpc. Stack frame
> > > > > creation and function call with return pointer are each atomic.  
> > > > 
> > > > What if the function is interrupted before it creates the stack frame?
> 
> There should be a pt_regs that shows exactly this situation, see below.
> 
> > > Then there will be no stack frame, but you still get the caller address
> > > because it's saved in LR register as part of the function call. Then
> > > you get the caller's caller in its stack frame.
> > 
> > Ok.  So what about the interrupted function itself?  Looking at the
> > powerpc version of save_context_stack(), it doesn't do anything special
> > for exception frames like checking regs->nip.
> > 
> > Though it looks like that should be possible since show_stack() has a
> > way to identify exception frames.
> 
> IIRC x86 errors out if a task was interrupted in kernel context. PPC
> save_stack_trace_tsk_reliable() could do the same.
> 
> Would that be sufficient?

Yes, I think that would cover most of my concerns, including the above
crypto asm issue.

(Otherwise, we'd at least need to make sure that nothing gets skipped by
the unwinder when getting preempted/page faulted in a variety of
scenarios, including before LR is saved to caller, after LR is saved to
caller, the crypto issue I mentioned above, etc)

So with your proposal, I think I'm convinced that we don't need objtool
for ppc64le.  Does anyone disagree?

There are still a few more things that need to be looked at:

1) With function graph tracing enabled, is the unwinder smart enough to
   get the original function return address, e.g. by calling
   ftrace_graph_ret_addr()?

2) Similar question for kretprobes.

3) Any other issues with generated code (e.g., bpf, ftrace trampolines),
   runtime patching (e.g., CPU feature alternatives), kprobes, paravirt,
   etc, that might confuse the unwinder?

4) As a sanity check, it *might* be a good idea for
   save_stack_trace_tsk_reliable() to ensure that it always reaches the
   end of the stack.  There are several ways to do that:

   - If the syscall entry stack frame is always the same size, then the
     "end" would simply mean that the stack pointer is at a certain
     offset from the end of the task stack page.  However this might not
     work for kthreads and idle tasks, unless their stacks also start at
     the same offset.  (On x86 we actually standardized the end of stack
     location for all tasks, both user and kernel.)

   - If the unwinder can get to the syscall frame, it can presumably
     examine regs->msr to check the PR bit to ensure it got all the way
     to syscall entry.  But again this might only work for user tasks,
     depending on how kernel task stacks are set up.

   - Or a different approach would be to do error checking along the
     way, and reporting an error for any unexpected conditions.

   However, given that backlink/LR corruption doesn't seem possible with
   this architecture, maybe #4 would be overkill.  Personally I would
   feel more comfortable with an "end" check and a WARN() if it doesn't
   reach the end.  But I could just be overly paranoid due to my x86
   frame pointer experiences.

-- 
Josh

^ permalink raw reply

* Re: [PATCH v9 29/51] mm/mprotect, powerpc/mm/pkeys, x86/mm/pkeys: Add sysfs interface
From: Benjamin Herrenschmidt @ 2017-12-19 21:34 UTC (permalink / raw)
  To: Dave Hansen, Ram Pai
  Cc: mpe, mingo, akpm, corbet, arnd, linuxppc-dev, linux-mm, x86,
	linux-arch, linux-doc, linux-kselftest, linux-kernel, paulus,
	khandual, aneesh.kumar, bsingharora, hbabu, mhocko, bauerman,
	ebiederm
In-Reply-To: <e7971d03-6ad1-40d5-9b79-f01242db5293@intel.com>

On Mon, 2017-12-18 at 14:28 -0800, Dave Hansen wrote:
> > We do not have generic support for something like that on ppc.
> > The kernel looks at the device tree to determine what hardware features
> > are available. But does not have mechanism to tell the hardware to track
> > which of its features are currently enabled/used by the kernel; atleast
> > not for the memory-key feature.
> 
> Bummer.  You're missing out.
> 
> But, you could still do this with a syscall.  "Hey, kernel, do you
> support this feature?"

I'm not sure I understand Ram's original (quoted) point, but informing
userspace of CPU features is what AT_HWCAP's are about.

Ben.

^ permalink raw reply

* Re: revamp vmem_altmap / dev_pagemap handling V2
From: Dan Williams @ 2017-12-19 20:36 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jérôme Glisse, Logan Gunthorpe,
	linux-nvdimm@lists.01.org, linuxppc-dev, X86 ML, Linux MM,
	linux-kernel@vger.kernel.org
In-Reply-To: <20171215140947.26075-1-hch@lst.de>

On Fri, Dec 15, 2017 at 6:09 AM, Christoph Hellwig <hch@lst.de> wrote:
>
> Hi all,
>
> this series started with two patches from Logan that now are in the
> middle of the series to kill the memremap-internal pgmap structure
> and to redo the dev_memreamp_pages interface to be better suitable
> for future PCI P2P uses.  I reviewed them and noticed that there
> isn't really any good reason to keep struct vmem_altmap either,
> and that a lot of these alternative device page map access should
> be better abstracted out instead of being sprinkled all over the
> mm code.  But when we got the RCU warnings in V1 I went for yet
> another approach, and now struct vmem_altmap is kept for now,
> but passed explicitly through the memory hotplug code instead of
> having to do unprotected lookups through the radix tree.  The
> end result is that only the get_user_pages path ever looks up
> struct dev_pagemap, and struct vmem_altmap is now always embedded
> into struct dev_pagemap, and explicitly passed where needed.
>
> Please review carefully, this has only been tested with my legacy
> e820 NVDIMM system.

I hit the following regression in the error path with these patches
applied. I'm working on a bisect and updating the unit tests to
capture this scenario. 4.15-rc2 works as expected.

[   47.102064] ------------[ cut here ]------------
[   47.103099] dax_pmem dax1.0: devm_memremap_pages_release: failed to
free all reserved pages
[   47.104773] WARNING: CPU: 6 PID: 1226 at kernel/memremap.c:306
devm_memremap_pages_release+0x399/0x3e0
[   47.106578] Modules linked in: ip6t_rpfilter ip6t_REJECT
nf_reject_ipv6 xt_conntrack ebtable_nat ebtable_broute bridge stp llc
ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip
6table_mangle ip6table_raw ip6table_security iptable_nat
nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack
iptable_mangle iptable_raw iptable_security ebtable_filter ebtables
ip6table_filter ip6_tables crct10dif_pclmul crc32_pclmul crc32c_intel
ghash_clmulni_intel dax_pmem(O) nd_pmem(O) device_dax(O) nd_btt(O)
nd_e820(O) nfit(O) serio_raw libnvdimm(O) nfit_test_i
omap(O)
[   47.114722] CPU: 6 PID: 1226 Comm: ndctl Tainted: G           O
4.15.0-rc2+ #981
[   47.116082] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
[   47.117993] task: 00000000f9fb534d task.stack: 00000000575f2a25
[   47.119004] RIP: 0010:devm_memremap_pages_release+0x399/0x3e0
[   47.119993] RSP: 0018:ffffc90002f2fd30 EFLAGS: 00010282
[   47.120909] RAX: 0000000000000000 RBX: ffff88043715fa80 RCX: 0000000000000000
[   47.122095] RDX: ffff8801f88d6900 RSI: ffff8801f88ce478 RDI: ffff8801f88ce478
[   47.123284] RBP: ffffc90002f2fd50 R08: 0000000000000000 R09: 0000000000000000
[   47.124466] R10: 0000000000000001 R11: 0000000000000000 R12: ffff8801f1fd2d10
[   47.125648] R13: 0000000440000000 R14: ffff8801f4dc8018 R15: ffffffff81ed6dfe
[   47.126831] FS:  00007fd93f2ba840(0000) GS:ffff8801f88c0000(0000)
knlGS:0000000000000000
[   47.128233] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   47.129216] CR2: 000055fa3e090fc0 CR3: 00000001f3dce000 CR4: 00000000000406e0
[   47.130404] Call Trace:
[   47.130913]  release_nodes+0x160/0x2a0
[   47.131617]  driver_probe_device+0xf9/0x490
[   47.132378]  bind_store+0x109/0x160
[   47.133035]  kernfs_fop_write+0x110/0x1b0
[   47.133775]  __vfs_write+0x33/0x170
[   47.134438]  ? rcu_read_lock_sched_held+0x3f/0x70
[   47.135275]  ? rcu_sync_lockdep_assert+0x2a/0x50
[   47.136091]  ? __sb_start_write+0xd0/0x1b0
[   47.136840]  ? vfs_write+0x18b/0x1b0
[   47.137519]  vfs_write+0xc5/0x1b0
[   47.138151]  SyS_write+0x55/0xc0
[   47.138776]  entry_SYSCALL_64_fastpath+0x1f/0x96
[   47.139600] RIP: 0033:0x7fd93e3a8f84
[   47.140270] RSP: 002b:00007ffca9dc0f68 EFLAGS: 00000246 ORIG_RAX:
0000000000000001
[   47.141593] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fd93e3a8f84
[   47.142778] RDX: 0000000000000007 RSI: 0000000001d2de90 RDI: 0000000000000004
[   47.143962] RBP: 00007ffca9dc0fa0 R08: 0000000001d283d0 R09: 00000000fffffff8
[   47.145147] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000407d50
[   47.146330] R13: 00007ffca9dc15a0 R14: 0000000000000000 R15: 0000000000000000
[   47.147520] Code: f9 57 16 01 01 48 85 db 74 55 4c 89 f7 e8 00 21
44 00 48 c7 c1 80 62 c2 81 48 89 da 48 89 c6 48 c7 c7 08 6a ee 81 e8
c7 9f ea ff <0f> ff e9 ce fe ff ff 48 c7 c2 08 cf ec 81 be ed 02 00 00
48 c7
[   47.150607] ---[ end trace f384c72daa2ac9c5 ]---
[   47.151458] dax_pmem dax1.0: dax_pmem_percpu_exit
[   47.152478] dax_pmem: probe of dax1.0 failed with error -12

^ permalink raw reply

* Re: [-next PATCH 0/4] sysfs and DEVICE_ATTR_<foo>
From: Corey Minyard @ 2017-12-19 19:26 UTC (permalink / raw)
  To: Joe Perches, linux-arm-kernel, linux-acpi, openipmi-developer,
	intel-gfx, linuxppc-dev, netdev, linux-nvme, platform-driver-x86,
	linux-s390, esc.storagedev, linux-scsi, linux-pm, linux-serial,
	linux-usb, linux-kernel, alsa-devel, linux-omap
  Cc: linux-sh, dri-devel, linux-input, linux-media, devel, linux-fbdev
In-Reply-To: <cover.1513706701.git.joe@perches.com>

On 12/19/2017 12:15 PM, Joe Perches wrote:
>   drivers/char/ipmi/ipmi_msghandler.c                | 17 +++---

For ipmi:

Acked-by: Corey Minyard <cminyard@mvista.com>

^ permalink raw reply

* Re: [-next PATCH 0/4] sysfs and DEVICE_ATTR_<foo>
From: Jani Nikula @ 2017-12-19 18:54 UTC (permalink / raw)
  To: Joe Perches, linux-arm-kernel, linux-acpi, openipmi-developer,
	intel-gfx, linuxppc-dev, netdev, linux-nvme, platform-driver-x86,
	linux-s390, esc.storagedev, linux-scsi, linux-pm, linux-serial,
	linux-usb, linux-kernel, alsa-devel, linux-omap
  Cc: devel, linux-fbdev, linux-sh, dri-devel, linux-input, linux-media
In-Reply-To: <cover.1513706701.git.joe@perches.com>

On Tue, 19 Dec 2017, Joe Perches <joe@perches.com> wrote:
>  drivers/gpu/drm/i915/i915_sysfs.c                  | 12 ++--

For i915,

Acked-by: Jani Nikula <jani.nikula@intel.com>


-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply

* Re: [-next PATCH 4/4] treewide: Use DEVICE_ATTR_WO
From: Borislav Petkov @ 2017-12-19 18:54 UTC (permalink / raw)
  To: Joe Perches
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Thomas Falcon, John Allen, Inaky Perez-Gonzalez, linux-wimax,
	James Smart, Dick Kennedy, Zhang Rui, Eduardo Valentin,
	Martin Schwidefsky, Heiko Carstens, Thomas Gleixner,
	H. Peter Anvin, x86, Dmitry Torokhov, James E.J. Bottomley,
	Martin K. Petersen, linux-s390, linux-kernel, linux-input, netdev,
	linuxppc-dev, linux-scsi, linux-pm
In-Reply-To: <1513709474.1234.66.camel@perches.com>

On Tue, Dec 19, 2017 at 10:51:14AM -0800, Joe Perches wrote:
> > The reason for the code churn being?
> 
> Consistency for easier grep by use-type.

Please explain that in the commit message so that we know why it was
changed.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* Re: [-next PATCH 4/4] treewide: Use DEVICE_ATTR_WO
From: Borislav Petkov @ 2017-12-19 18:44 UTC (permalink / raw)
  To: Joe Perches
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Thomas Falcon, John Allen, Inaky Perez-Gonzalez, linux-wimax,
	James Smart, Dick Kennedy, Zhang Rui, Eduardo Valentin,
	Martin Schwidefsky, Heiko Carstens, Thomas Gleixner,
	H. Peter Anvin, x86, Dmitry Torokhov, James E.J. Bottomley,
	Martin K. Petersen, linux-s390, linux-kernel, linux-input, netdev,
	linuxppc-dev, linux-scsi, linux-pm
In-Reply-To: <fa30f1ad73f76dafff816df40cacffe613aa2f48.1513706702.git.joe@perches.com>

On Tue, Dec 19, 2017 at 10:15:09AM -0800, Joe Perches wrote:
> Convert DEVICE_ATTR uses to DEVICE_ATTR_WO where possible.
> 
> Done with perl script:
> 
> $ git grep -w --name-only DEVICE_ATTR | \
>   xargs perl -i -e 'local $/; while (<>) { s/\bDEVICE_ATTR\s*\(\s*(\w+)\s*,\s*\(?(?:\s*S_IWUSR\s*|\s*0200\s*)\)?\s*,\s*NULL\s*,\s*\s_store\s*\)/DEVICE_ATTR_WO(\1)/g; print;}'
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  arch/s390/kernel/smp.c                 | 2 +-
>  arch/x86/kernel/cpu/microcode/core.c   | 2 +-
>  drivers/input/touchscreen/elants_i2c.c | 2 +-
>  drivers/net/ethernet/ibm/ibmvnic.c     | 2 +-
>  drivers/net/wimax/i2400m/sysfs.c       | 3 +--
>  drivers/scsi/lpfc/lpfc_attr.c          | 3 +--
>  drivers/thermal/thermal_sysfs.c        | 2 +-
>  7 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
> index b8c1a85bcf2d..a919b2f0141d 100644
> --- a/arch/s390/kernel/smp.c
> +++ b/arch/s390/kernel/smp.c
> @@ -1151,7 +1151,7 @@ static ssize_t __ref rescan_store(struct device *dev,
>  	rc = smp_rescan_cpus();
>  	return rc ? rc : count;
>  }
> -static DEVICE_ATTR(rescan, 0200, NULL, rescan_store);
> +static DEVICE_ATTR_WO(rescan);
>  #endif /* CONFIG_HOTPLUG_CPU */
>  
>  static int __init s390_smp_init(void)
> diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
> index c4fa4a85d4cb..09c74b0560dd 100644
> --- a/arch/x86/kernel/cpu/microcode/core.c
> +++ b/arch/x86/kernel/cpu/microcode/core.c
> @@ -560,7 +560,7 @@ static ssize_t pf_show(struct device *dev,
>  	return sprintf(buf, "0x%x\n", uci->cpu_sig.pf);
>  }
>  
> -static DEVICE_ATTR(reload, 0200, NULL, reload_store);
> +static DEVICE_ATTR_WO(reload);
>  static DEVICE_ATTR(version, 0400, version_show, NULL);
>  static DEVICE_ATTR(processor_flags, 0400, pf_show, NULL);
>  

# cat /sys/devices/system/cpu/microcode/reload
cat: /sys/devices/system/cpu/microcode/reload: Permission denied

The reason for the code churn being?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* Re: [-next PATCH 4/4] treewide: Use DEVICE_ATTR_WO
From: Joe Perches @ 2017-12-19 18:51 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Thomas Falcon, John Allen, Inaky Perez-Gonzalez, linux-wimax,
	James Smart, Dick Kennedy, Zhang Rui, Eduardo Valentin,
	Martin Schwidefsky, Heiko Carstens, Thomas Gleixner,
	H. Peter Anvin, x86, Dmitry Torokhov, James E.J. Bottomley,
	Martin K. Petersen, linux-s390, linux-kernel, linux-input, netdev,
	linuxppc-dev, linux-scsi, linux-pm
In-Reply-To: <20171219184441.ixxh757tl44qvk5l@pd.tnic>

On Tue, 2017-12-19 at 19:44 +0100, Borislav Petkov wrote:
> On Tue, Dec 19, 2017 at 10:15:09AM -0800, Joe Perches wrote:
> > Convert DEVICE_ATTR uses to DEVICE_ATTR_WO where possible.
> > 
> > Done with perl script:
> > 
> > $ git grep -w --name-only DEVICE_ATTR | \
> >   xargs perl -i -e 'local $/; while (<>) { s/\bDEVICE_ATTR\s*\(\s*(\w+)\s*,\s*\(?(?:\s*S_IWUSR\s*|\s*0200\s*)\)?\s*,\s*NULL\s*,\s*\s_store\s*\)/DEVICE_ATTR_WO(\1)/g; print;}'
[]
> > diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
[]
> > @@ -560,7 +560,7 @@ static ssize_t pf_show(struct device *dev,
> >  	return sprintf(buf, "0x%x\n", uci->cpu_sig.pf);
> >  }
> >  
> > -static DEVICE_ATTR(reload, 0200, NULL, reload_store);
> > +static DEVICE_ATTR_WO(reload);
> >  static DEVICE_ATTR(version, 0400, version_show, NULL);
> >  static DEVICE_ATTR(processor_flags, 0400, pf_show, NULL);
> >  
> 
> # cat /sys/devices/system/cpu/microcode/reload
> cat: /sys/devices/system/cpu/microcode/reload: Permission denied

Not different behavior.  It was and remains write only.

> The reason for the code churn being?

Consistency for easier grep by use-type.

^ permalink raw reply

* [-next PATCH 4/4] treewide: Use DEVICE_ATTR_WO
From: Joe Perches @ 2017-12-19 18:15 UTC (permalink / raw)
  To: Borislav Petkov, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Thomas Falcon, John Allen, Inaky Perez-Gonzalez,
	linux-wimax, James Smart, Dick Kennedy, Zhang Rui,
	Eduardo Valentin
  Cc: Martin Schwidefsky, Heiko Carstens, Thomas Gleixner,
	H. Peter Anvin, x86, Dmitry Torokhov, James E.J. Bottomley,
	Martin K. Petersen, linux-s390, linux-kernel, linux-input, netdev,
	linuxppc-dev, linux-scsi, linux-pm
In-Reply-To: <cover.1513706701.git.joe@perches.com>

Convert DEVICE_ATTR uses to DEVICE_ATTR_WO where possible.

Done with perl script:

$ git grep -w --name-only DEVICE_ATTR | \
  xargs perl -i -e 'local $/; while (<>) { s/\bDEVICE_ATTR\s*\(\s*(\w+)\s*,\s*\(?(?:\s*S_IWUSR\s*|\s*0200\s*)\)?\s*,\s*NULL\s*,\s*\s_store\s*\)/DEVICE_ATTR_WO(\1)/g; print;}'

Signed-off-by: Joe Perches <joe@perches.com>
---
 arch/s390/kernel/smp.c                 | 2 +-
 arch/x86/kernel/cpu/microcode/core.c   | 2 +-
 drivers/input/touchscreen/elants_i2c.c | 2 +-
 drivers/net/ethernet/ibm/ibmvnic.c     | 2 +-
 drivers/net/wimax/i2400m/sysfs.c       | 3 +--
 drivers/scsi/lpfc/lpfc_attr.c          | 3 +--
 drivers/thermal/thermal_sysfs.c        | 2 +-
 7 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
index b8c1a85bcf2d..a919b2f0141d 100644
--- a/arch/s390/kernel/smp.c
+++ b/arch/s390/kernel/smp.c
@@ -1151,7 +1151,7 @@ static ssize_t __ref rescan_store(struct device *dev,
 	rc = smp_rescan_cpus();
 	return rc ? rc : count;
 }
-static DEVICE_ATTR(rescan, 0200, NULL, rescan_store);
+static DEVICE_ATTR_WO(rescan);
 #endif /* CONFIG_HOTPLUG_CPU */
 
 static int __init s390_smp_init(void)
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index c4fa4a85d4cb..09c74b0560dd 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -560,7 +560,7 @@ static ssize_t pf_show(struct device *dev,
 	return sprintf(buf, "0x%x\n", uci->cpu_sig.pf);
 }
 
-static DEVICE_ATTR(reload, 0200, NULL, reload_store);
+static DEVICE_ATTR_WO(reload);
 static DEVICE_ATTR(version, 0400, version_show, NULL);
 static DEVICE_ATTR(processor_flags, 0400, pf_show, NULL);
 
diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
index a458e5ec9e41..819213e88f32 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -1000,7 +1000,7 @@ static ssize_t show_iap_mode(struct device *dev,
 				"Normal" : "Recovery");
 }
 
-static DEVICE_ATTR(calibrate, S_IWUSR, NULL, calibrate_store);
+static DEVICE_ATTR_WO(calibrate);
 static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL);
 static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw);
 
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 1dc4aef37d3a..42b96e1a1b13 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -4411,7 +4411,7 @@ static ssize_t failover_store(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
-static DEVICE_ATTR(failover, 0200, NULL, failover_store);
+static DEVICE_ATTR_WO(failover);
 
 static unsigned long ibmvnic_get_desired_dma(struct vio_dev *vdev)
 {
diff --git a/drivers/net/wimax/i2400m/sysfs.c b/drivers/net/wimax/i2400m/sysfs.c
index 1237109f251a..8c67df11105c 100644
--- a/drivers/net/wimax/i2400m/sysfs.c
+++ b/drivers/net/wimax/i2400m/sysfs.c
@@ -65,8 +65,7 @@ ssize_t i2400m_idle_timeout_store(struct device *dev,
 }
 
 static
-DEVICE_ATTR(i2400m_idle_timeout, S_IWUSR,
-	    NULL, i2400m_idle_timeout_store);
+DEVICE_ATTR_WO(i2400m_idle_timeout);
 
 static
 struct attribute *i2400m_dev_attrs[] = {
diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index 517ff203cfde..6ddaf51a23f6 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -2418,8 +2418,7 @@ lpfc_soft_wwn_enable_store(struct device *dev, struct device_attribute *attr,
 
 	return count;
 }
-static DEVICE_ATTR(lpfc_soft_wwn_enable, S_IWUSR, NULL,
-		   lpfc_soft_wwn_enable_store);
+static DEVICE_ATTR_WO(lpfc_soft_wwn_enable);
 
 /**
  * lpfc_soft_wwpn_show - Return the cfg soft ww port name of the adapter
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index 2bc964392924..ba81c9080f6e 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -317,7 +317,7 @@ emul_temp_store(struct device *dev, struct device_attribute *attr,
 
 	return ret ? ret : count;
 }
-static DEVICE_ATTR(emul_temp, S_IWUSR, NULL, emul_temp_store);
+static DEVICE_ATTR_WO(emul_temp);
 #endif
 
 static ssize_t
-- 
2.15.0

^ permalink raw reply related

* [-next PATCH 0/4] sysfs and DEVICE_ATTR_<foo>
From: Joe Perches @ 2017-12-19 18:15 UTC (permalink / raw)
  To: linux-arm-kernel, linux-acpi, openipmi-developer, intel-gfx,
	linuxppc-dev, netdev, linux-nvme, platform-driver-x86, linux-s390,
	esc.storagedev, linux-scsi, linux-pm, linux-serial, linux-usb,
	linux-kernel, alsa-devel, linux-omap
  Cc: linux-sh, dri-devel, linux-input, linux-media, devel, linux-fbdev

Joe Perches (4):
  sysfs.h: Use octal permissions
  treewide: Use DEVICE_ATTR_RW
  treewide: Use DEVICE_ATTR_RO
  treewide: Use DEVICE_ATTR_WO

 arch/arm/mach-pxa/sharpsl_pm.c                     |  4 +-
 arch/s390/kernel/smp.c                             |  2 +-
 arch/s390/kernel/topology.c                        |  3 +-
 arch/sh/drivers/push-switch.c                      |  2 +-
 arch/tile/kernel/sysfs.c                           | 12 ++--
 arch/x86/kernel/cpu/microcode/core.c               |  2 +-
 drivers/acpi/device_sysfs.c                        |  6 +-
 drivers/char/ipmi/ipmi_msghandler.c                | 17 +++---
 drivers/gpu/drm/i915/i915_sysfs.c                  | 12 ++--
 drivers/input/touchscreen/elants_i2c.c             |  2 +-
 drivers/net/ethernet/ibm/ibmvnic.c                 |  2 +-
 drivers/net/wimax/i2400m/sysfs.c                   |  3 +-
 drivers/nvme/host/core.c                           | 10 ++--
 drivers/platform/x86/compal-laptop.c               | 18 ++----
 drivers/s390/cio/css.c                             |  8 +--
 drivers/s390/cio/device.c                          | 10 ++--
 drivers/s390/crypto/ap_card.c                      |  2 +-
 drivers/scsi/hpsa.c                                | 10 ++--
 drivers/scsi/lpfc/lpfc_attr.c                      | 64 ++++++++--------------
 .../staging/media/atomisp/pci/atomisp2/hmm/hmm.c   |  8 +--
 drivers/thermal/thermal_sysfs.c                    | 17 +++---
 drivers/tty/serial/sh-sci.c                        |  2 +-
 drivers/usb/host/xhci-dbgcap.c                     |  2 +-
 drivers/usb/phy/phy-tahvo.c                        |  2 +-
 drivers/video/fbdev/auo_k190x.c                    |  4 +-
 drivers/video/fbdev/w100fb.c                       |  4 +-
 include/linux/sysfs.h                              | 14 ++---
 lib/test_firmware.c                                | 14 ++---
 lib/test_kmod.c                                    | 14 ++---
 sound/soc/omap/mcbsp.c                             |  4 +-
 sound/soc/soc-core.c                               |  2 +-
 sound/soc/soc-dapm.c                               |  2 +-
 32 files changed, 120 insertions(+), 158 deletions(-)

-- 
2.15.0

^ permalink raw reply

* Re: [PATCH v1 00/15] ASoC: fsl_ssi: Clean up - program flow level
From: Timur Tabi @ 2017-12-19 17:34 UTC (permalink / raw)
  To: Nicolin Chen, broonie
  Cc: linux-kernel, linuxppc-dev, alsa-devel, lgirdwood, fabio.estevam,
	mail, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <1513702819-42310-1-git-send-email-nicoleotsuka@gmail.com>

On 12/19/17 11:00 AM, Nicolin Chen wrote:
> This series of patches is the second set to clean up fsl_ssi driver
> in the program flow level. Any patch here may impact a fundamental
> test case like playback or record.

With Christmas happening over the next two weeks, I don't think I'll be 
able to review these patches until January.

^ permalink raw reply

* [PATCH V4] cxl: Add support for ASB_Notify on POWER9
From: Christophe Lombard @ 2017-12-19 17:27 UTC (permalink / raw)
  To: linuxppc-dev, fbarrat, vaibhav, andrew.donnellan

The POWER9 core supports a new feature: ASB_Notify which requires the
support of the Special Purpose Register: TIDR.

The ASB_Notify command, generated by the AFU, will attempt to
wake-up the host thread identified by the particular LPID:PID:TID.

This patch assign a unique TIDR (thread id) for the current thread which
will be used in the process element entry.

A next patch will handle a new kind of "compatible" property in the
device-tree (PHB DT node) indicating which version of CAPI and which
features are supported, instead of handling PVR values.

Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
Reviewed-by: Philippe Bergheaud <felix@linux.vnet.ibm.com>

---
Changelog[v4]
 - Rebased to latest upstream.
 - Updated the ioctl interface.
 - Removed the field tid in the context structure.

Changelog[v3]
 - Rebased to latest upstream.
 - Updated attr->tid field in cxllib_get_PE_attributes().

Changelog[v2]
 - Rebased to latest upstream.
 - Updated the ioctl interface.
 - Added a checking to allow updating the TIDR if a P9 chip is present.
---
 arch/powerpc/kernel/process.c |  1 +
 drivers/misc/cxl/context.c    | 15 +++++++++++++++
 drivers/misc/cxl/cxl.h        |  3 +++
 drivers/misc/cxl/cxllib.c     |  3 ++-
 drivers/misc/cxl/file.c       |  7 +++++++
 drivers/misc/cxl/native.c     |  2 +-
 include/uapi/misc/cxl.h       |  4 +++-
 7 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 5acb5a1..a6a70e2 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1589,6 +1589,7 @@ int set_thread_tidr(struct task_struct *t)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(set_thread_tidr);
 
 #endif /* CONFIG_PPC64 */
 
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 12a41b2..e309d35 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -22,6 +22,7 @@
 #include <asm/cputable.h>
 #include <asm/current.h>
 #include <asm/copro.h>
+#include <asm/switch_to.h>
 
 #include "cxl.h"
 
@@ -362,3 +363,17 @@ void cxl_context_mm_count_put(struct cxl_context *ctx)
 	if (ctx->mm)
 		mmdrop(ctx->mm);
 }
+
+int cxl_context_thread_tidr(struct cxl_context *ctx)
+{
+	int rc = 0;
+
+	if (!cxl_is_power9())
+		return -ENODEV;
+
+	rc = set_thread_tidr(current);
+	pr_devel("%s: current tidr: %ld\n", __func__,
+		 current->thread.tidr);
+
+	return rc;
+}
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index e46a406..1a5db0b 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -1169,4 +1169,7 @@ void cxl_context_mm_count_get(struct cxl_context *ctx);
 /* Decrements the reference count to "struct mm_struct" */
 void cxl_context_mm_count_put(struct cxl_context *ctx);
 
+/* Handles an unique TIDR (thread id) for the current thread */
+int cxl_context_thread_tidr(struct cxl_context *ctx);
+
 #endif
diff --git a/drivers/misc/cxl/cxllib.c b/drivers/misc/cxl/cxllib.c
index dc9bc18..30ccba4 100644
--- a/drivers/misc/cxl/cxllib.c
+++ b/drivers/misc/cxl/cxllib.c
@@ -199,10 +199,11 @@ int cxllib_get_PE_attributes(struct task_struct *task,
 		 */
 		attr->pid = mm->context.id;
 		mmput(mm);
+		attr->tid = task->thread.tidr;
 	} else {
 		attr->pid = 0;
+		attr->tid = 0;
 	}
-	attr->tid = 0;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(cxllib_get_PE_attributes);
diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
index 76c0b0c..a823c76 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -248,6 +248,13 @@ static long afu_ioctl_start_work(struct cxl_context *ctx,
 	 */
 	smp_mb();
 
+	/* Assign a unique TIDR (thread id) for the current thread */
+	if (work.flags & CXL_START_WORK_TID) {
+		rc = cxl_context_thread_tidr(ctx);
+		if (rc)
+			goto out;
+	}
+
 	trace_cxl_attach(ctx, work.work_element_descriptor, work.num_interrupts, amr);
 
 	if ((rc = cxl_ops->attach_process(ctx, false, work.work_element_descriptor,
diff --git a/drivers/misc/cxl/native.c b/drivers/misc/cxl/native.c
index 02b6b45..036fe5b 100644
--- a/drivers/misc/cxl/native.c
+++ b/drivers/misc/cxl/native.c
@@ -673,7 +673,7 @@ static int process_element_entry_psl9(struct cxl_context *ctx, u64 wed, u64 amr)
 		pid = ctx->mm->context.id;
 	}
 
-	ctx->elem->common.tid = 0;
+	ctx->elem->common.tid = cpu_to_be32(current->thread.tidr);
 	ctx->elem->common.pid = cpu_to_be32(pid);
 
 	ctx->elem->sr = cpu_to_be64(calculate_sr(ctx));
diff --git a/include/uapi/misc/cxl.h b/include/uapi/misc/cxl.h
index 49e8fd0..980ee8f 100644
--- a/include/uapi/misc/cxl.h
+++ b/include/uapi/misc/cxl.h
@@ -31,9 +31,11 @@ struct cxl_ioctl_start_work {
 #define CXL_START_WORK_AMR		0x0000000000000001ULL
 #define CXL_START_WORK_NUM_IRQS		0x0000000000000002ULL
 #define CXL_START_WORK_ERR_FF		0x0000000000000004ULL
+#define CXL_START_WORK_TID		0x0000000000000008ULL
 #define CXL_START_WORK_ALL		(CXL_START_WORK_AMR |\
 					 CXL_START_WORK_NUM_IRQS |\
-					 CXL_START_WORK_ERR_FF)
+					 CXL_START_WORK_ERR_FF |\
+					 CXL_START_WORK_TID)
 
 
 /* Possible modes that an afu can be in */
-- 
2.7.4

^ permalink raw reply related

* [PATCH v1 15/15] ASoC: fsl_ssi: Use ssi->streams instead of reading register
From: Nicolin Chen @ 2017-12-19 17:00 UTC (permalink / raw)
  To: timur, broonie
  Cc: linux-kernel, linuxppc-dev, alsa-devel, lgirdwood, fabio.estevam,
	mail, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <1513702819-42310-1-git-send-email-nicoleotsuka@gmail.com>

Since ssi->streams is being updated along with SCR register and
its SSIEN bit, it's simpler to use it instead.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
 sound/soc/fsl/fsl_ssi.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index a7aaead..9f346bf 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -792,10 +792,6 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream,
 	u32 wl = SSI_SxCCR_WL(sample_size);
 	int ret;
 	u32 scr;
-	int enabled;
-
-	regmap_read(regs, REG_SSI_SCR, &scr);
-	enabled = scr & SSI_SCR_SSIEN;
 
 	/*
 	 * SSI is properly configured if it is enabled and running in
@@ -803,7 +799,7 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream,
 	 * that should set separate configurations for STCCR and SRCCR
 	 * despite running in the synchronous mode.
 	 */
-	if (enabled && ssi->synchronous)
+	if (ssi->streams && ssi->synchronous)
 		return 0;
 
 	if (fsl_ssi_is_i2s_master(ssi)) {
-- 
2.7.4

^ permalink raw reply related

* [PATCH v1 14/15] ASoC: fsl_ssi: Move DT related code to a separate probe()
From: Nicolin Chen @ 2017-12-19 17:00 UTC (permalink / raw)
  To: timur, broonie
  Cc: linux-kernel, linuxppc-dev, alsa-devel, lgirdwood, fabio.estevam,
	mail, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <1513702819-42310-1-git-send-email-nicoleotsuka@gmail.com>

This patch cleans up probe() function by moving all Device Tree
related code into a separate function. It allows the probe() to
be Device Tree independent. This will be very useful for future
integration of imx-ssi driver which has similar functionalities
while exists only because it supports non-DT cases.

This patch also moves symmetric_channels of AC97 from the probe
to the structure snd_soc_dai_driver for simplification.

Additionally, since PowerPC and AC97 use the same pdev pointer
to register a platform device, this patch also unifies related
code.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
 sound/soc/fsl/fsl_ssi.c | 204 +++++++++++++++++++++++++-----------------------
 1 file changed, 108 insertions(+), 96 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 7d6149d..a7aaead 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -234,8 +234,12 @@ struct fsl_ssi_soc_data {
  *
  * @fiq_params: FIQ stream filtering parameters
  *
- * @pdev: Pointer to pdev when using fsl-ssi as sound card (ppc only)
- *        TODO: Should be replaced with simple-sound-card
+ * @card_pdev: Platform_device pointer when using fsl-ssi as sound card
+ *             (PowerPC or AC97 only)
+ * @card_name: Platform_device name when using fsl-ssi as sound card
+ *             (PowerPC or AC97 only)
+ * @card_idx: The index of SSI when registering a sound card
+ *             (PowerPC or AC97 only)
  *
  * @dbg_stats: Debugging statistics
  *
@@ -279,7 +283,9 @@ struct fsl_ssi {
 
 	struct imx_pcm_fiq_params fiq_params;
 
-	struct platform_device *pdev;
+	struct platform_device *card_pdev;
+	char card_name[32];
+	u32 card_idx;
 
 	struct fsl_ssi_dbg dbg_stats;
 
@@ -1155,6 +1161,7 @@ static const struct snd_soc_component_driver fsl_ssi_component = {
 
 static struct snd_soc_dai_driver fsl_ssi_ac97_dai = {
 	.bus_control = true,
+	.symmetric_channels = 1,
 	.probe = fsl_ssi_dai_probe,
 	.remove = fsl_ssi_dai_ac97_remove,
 	.playback = {
@@ -1266,9 +1273,7 @@ static void make_lowercase(char *s)
 static int fsl_ssi_imx_probe(struct platform_device *pdev,
 			     struct fsl_ssi *ssi, void __iomem *iomem)
 {
-	struct device_node *np = pdev->dev.of_node;
 	struct device *dev = &pdev->dev;
-	u32 dmas[4];
 	int ret;
 
 	/* Backward compatible for a DT without ipg clock name assigned */
@@ -1302,14 +1307,8 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev,
 	ssi->dma_params_tx.addr = ssi->ssi_phys + REG_SSI_STX0;
 	ssi->dma_params_rx.addr = ssi->ssi_phys + REG_SSI_SRX0;
 
-	/* Set to dual FIFO mode according to the SDMA sciprt */
-	ret = of_property_read_u32_array(np, "dmas", dmas, 4);
-	if (ssi->use_dma && !ret && dmas[2] == IMX_DMATYPE_SSI_DUAL) {
-		ssi->use_dual_fifo = true;
-		/*
-		 * Use even numbers to avoid channel swap due to SDMA
-		 * script design
-		 */
+	/* Use even numbers to avoid channel swap due to SDMA script design */
+	if (ssi->use_dual_fifo) {
 		ssi->dma_params_tx.maxburst &= ~0x1;
 		ssi->dma_params_rx.maxburst &= ~0x1;
 	}
@@ -1350,42 +1349,103 @@ static void fsl_ssi_imx_clean(struct platform_device *pdev, struct fsl_ssi *ssi)
 		clk_disable_unprepare(ssi->clk);
 }
 
-static int fsl_ssi_probe(struct platform_device *pdev)
+static int fsl_ssi_probe_from_dt(struct fsl_ssi *ssi)
 {
-	struct snd_soc_dai_driver *cpu_dai_drv;
-	struct fsl_ssi *ssi;
-	int ret = 0;
-	struct device_node *np = pdev->dev.of_node;
-	struct device *dev = &pdev->dev;
+	struct device *dev = ssi->dev;
+	struct device_node *np = dev->of_node;
 	const struct of_device_id *of_id;
 	const char *p, *sprop;
 	const uint32_t *iprop;
-	struct resource *res;
-	void __iomem *iomem;
-	char name[64];
-	struct regmap_config regconfig = fsl_ssi_regconfig;
+	u32 dmas[4];
+	int ret;
 
 	of_id = of_match_device(fsl_ssi_ids, dev);
 	if (!of_id || !of_id->data)
 		return -EINVAL;
 
-	ssi = devm_kzalloc(dev, sizeof(*ssi), GFP_KERNEL);
-	if (!ssi)
-		return -ENOMEM;
-
 	ssi->soc = of_id->data;
-	ssi->dev = dev;
+
+	ret = of_property_match_string(np, "clock-names", "ipg");
+	/* Get error code if not found */
+	ssi->has_ipg_clk_name = ret >= 0;
 
 	/* Check if being used in AC97 mode */
 	sprop = of_get_property(np, "fsl,mode", NULL);
-	if (sprop) {
-		if (!strcmp(sprop, "ac97-slave"))
-			ssi->dai_fmt = FSLSSI_AC97_DAIFMT;
+	if (sprop && !strcmp(sprop, "ac97-slave")) {
+		ssi->dai_fmt = FSLSSI_AC97_DAIFMT;
+
+		ret = of_property_read_u32(np, "cell-index", &ssi->card_idx);
+		if (ret) {
+			dev_err(dev, "failed to get SSI index property\n");
+			return -EINVAL;
+		}
+		strcpy(ssi->card_name, "ac97-codec");
 	}
 
 	/* Select DMA or FIQ */
 	ssi->use_dma = !of_property_read_bool(np, "fsl,fiq-stream-filter");
 
+	/* In synchronous mode, STCK and STFS ports are used by RX as well */
+	if (!of_find_property(np, "fsl,ssi-asynchronous", NULL))
+		ssi->synchronous = true;
+
+	/* Fetch FIFO depth; Set to 8 for older DT without this property */
+	iprop = of_get_property(np, "fsl,fifo-depth", NULL);
+	if (iprop)
+		ssi->fifo_depth = be32_to_cpup(iprop);
+	else
+		ssi->fifo_depth = 8;
+
+	/* Use dual FIFO mode depending on the support from SDMA script */
+	ret = of_property_read_u32_array(np, "dmas", dmas, 4);
+	if (ssi->use_dma && !ret && dmas[2] == IMX_DMATYPE_SSI_DUAL)
+		ssi->use_dual_fifo = true;
+
+	/*
+	 * Backward compatible for older bindings by manually triggering the
+	 * machine driver's probe(). Use /compatible property, including the
+	 * address of CPU DAI driver structure, as the name of machine driver
+	 *
+	 * If card_name is set by AC97 earlier, bypass here since it uses a
+	 * different name to register the device.
+	 */
+	if (!ssi->card_name[0] && of_get_property(np, "codec-handle", NULL)) {
+		sprop = of_get_property(of_find_node_by_path("/"),
+					"compatible", NULL);
+		/* Strip "fsl," in the compatible name if applicable */
+		p = strrchr(sprop, ',');
+		if (p)
+			sprop = p + 1;
+		snprintf(ssi->card_name, sizeof(ssi->card_name),
+			 "snd-soc-%s", sprop);
+		make_lowercase(ssi->card_name);
+		ssi->card_idx = 0;
+	}
+
+	return 0;
+}
+
+static int fsl_ssi_probe(struct platform_device *pdev)
+{
+	struct regmap_config regconfig = fsl_ssi_regconfig;
+	struct snd_soc_dai_driver *cpu_dai_drv;
+	struct device *dev = &pdev->dev;
+	struct fsl_ssi *ssi;
+	struct resource *res;
+	void __iomem *iomem;
+	int ret = 0;
+
+	ssi = devm_kzalloc(dev, sizeof(*ssi), GFP_KERNEL);
+	if (!ssi)
+		return -ENOMEM;
+
+	ssi->dev = dev;
+
+	/* Probe from DT */
+	ret = fsl_ssi_probe_from_dt(ssi);
+	if (ret)
+		return ret;
+
 	if (fsl_ssi_is_ac97(ssi)) {
 		cpu_dai_drv = &fsl_ssi_ac97_dai;
 		fsl_ac97_data = ssi;
@@ -1407,15 +1467,11 @@ static int fsl_ssi_probe(struct platform_device *pdev)
 			REG_SSI_SRMSK / sizeof(uint32_t) + 1;
 	}
 
-	ret = of_property_match_string(np, "clock-names", "ipg");
-	if (ret < 0) {
-		ssi->has_ipg_clk_name = false;
-		ssi->regs = devm_regmap_init_mmio(dev, iomem, &regconfig);
-	} else {
-		ssi->has_ipg_clk_name = true;
+	if (ssi->has_ipg_clk_name)
 		ssi->regs = devm_regmap_init_mmio_clk(dev, "ipg", iomem,
 						      &regconfig);
-	}
+	else
+		ssi->regs = devm_regmap_init_mmio(dev, iomem, &regconfig);
 	if (IS_ERR(ssi->regs)) {
 		dev_err(dev, "failed to init register map\n");
 		return PTR_ERR(ssi->regs);
@@ -1427,24 +1483,13 @@ static int fsl_ssi_probe(struct platform_device *pdev)
 		return ssi->irq;
 	}
 
-	/* Set software limitations for synchronous mode */
-	if (!of_find_property(np, "fsl,ssi-asynchronous", NULL)) {
-		if (!fsl_ssi_is_ac97(ssi)) {
-			cpu_dai_drv->symmetric_rates = 1;
-			cpu_dai_drv->symmetric_samplebits = 1;
-			ssi->synchronous = true;
-		}
-
+	/* Set software limitations for synchronous mode except AC97 */
+	if (ssi->synchronous && !fsl_ssi_is_ac97(ssi)) {
+		cpu_dai_drv->symmetric_rates = 1;
 		cpu_dai_drv->symmetric_channels = 1;
+		cpu_dai_drv->symmetric_samplebits = 1;
 	}
 
-	/* Fetch FIFO depth; Set to 8 for older DT without this property */
-	iprop = of_get_property(np, "fsl,fifo-depth", NULL);
-	if (iprop)
-		ssi->fifo_depth = be32_to_cpup(iprop);
-	else
-		ssi->fifo_depth = 8;
-
 	/*
 	 * Configure TX and RX DMA watermarks -- when to send a DMA request
 	 *
@@ -1509,47 +1554,14 @@ static int fsl_ssi_probe(struct platform_device *pdev)
 	if (ret)
 		goto error_asoc_register;
 
-	/* Bypass it if using newer DT bindings of ASoC machine drivers */
-	if (!of_get_property(np, "codec-handle", NULL))
-		goto done;
-
-	/*
-	 * Backward compatible for older bindings by manually triggering the
-	 * machine driver's probe(). Use /compatible property, including the
-	 * address of CPU DAI driver structure, as the name of machine driver.
-	 */
-	sprop = of_get_property(of_find_node_by_path("/"), "compatible", NULL);
-	/* Sometimes the compatible name has a "fsl," prefix, so we strip it. */
-	p = strrchr(sprop, ',');
-	if (p)
-		sprop = p + 1;
-	snprintf(name, sizeof(name), "snd-soc-%s", sprop);
-	make_lowercase(name);
-
-	ssi->pdev = platform_device_register_data(dev, name, 0, NULL, 0);
-	if (IS_ERR(ssi->pdev)) {
-		ret = PTR_ERR(ssi->pdev);
-		dev_err(dev, "failed to register platform: %d\n", ret);
-		goto error_sound_card;
-	}
-
-done:
-	if (fsl_ssi_is_ac97(ssi)) {
-		u32 ssi_idx;
-
-		ret = of_property_read_u32(np, "cell-index", &ssi_idx);
-		if (ret) {
-			dev_err(dev, "failed to get SSI index property\n");
-			goto error_sound_card;
-		}
-
-		ssi->pdev = platform_device_register_data(NULL, "ac97-codec",
-							  ssi_idx, NULL, 0);
-		if (IS_ERR(ssi->pdev)) {
-			ret = PTR_ERR(ssi->pdev);
-			dev_err(dev,
-				"failed to register AC97 codec platform: %d\n",
-				ret);
+	/* Register a platform device for older bindings or AC97 */
+	if (ssi->card_name[0]) {
+		ssi->card_pdev = platform_device_register_data(dev,
+				ssi->card_name, ssi->card_idx, NULL, 0);
+		if (IS_ERR(ssi->card_pdev)) {
+			ret = PTR_ERR(ssi->card_pdev);
+			dev_err(dev, "failed to register %s: %d\n",
+				ssi->card_name, ret);
 			goto error_sound_card;
 		}
 	}
@@ -1577,8 +1589,8 @@ static int fsl_ssi_remove(struct platform_device *pdev)
 
 	fsl_ssi_debugfs_remove(&ssi->dbg_stats);
 
-	if (ssi->pdev)
-		platform_device_unregister(ssi->pdev);
+	if (ssi->card_pdev)
+		platform_device_unregister(ssi->card_pdev);
 
 	if (ssi->soc->imx)
 		fsl_ssi_imx_clean(pdev, ssi);
-- 
2.7.4

^ permalink raw reply related

* [PATCH v1 13/15] ASoC: fsl_ssi: Remove cpu_dai_drv from fsl_ssi structure
From: Nicolin Chen @ 2017-12-19 17:00 UTC (permalink / raw)
  To: timur, broonie
  Cc: linux-kernel, linuxppc-dev, alsa-devel, lgirdwood, fabio.estevam,
	mail, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <1513702819-42310-1-git-send-email-nicoleotsuka@gmail.com>

The cpu_dai_drv is only used for symmetric_rates. So this patch replaces
it with a synchronous boolean flag.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
 sound/soc/fsl/fsl_ssi.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 898f837..7d6149d 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -208,11 +208,11 @@ struct fsl_ssi_soc_data {
  *
  * @regs: Pointer to the regmap registers
  * @irq: IRQ of this SSI
- * @cpu_dai_drv: CPU DAI driver for this device
  *
  * @dai_fmt: DAI configuration this device is currently used with
  * @streams: Mask of current active streams: BIT(TX) and BIT(RX)
  * @i2s_net: I2S and Network mode configurations of SCR register
+ * @synchronous: Use synchronous mode - both of TX and RX use STCK and SFCK
  * @use_dma: DMA is used or FIQ with stream filter
  * @use_dual_fifo: DMA with support for dual FIFO mode
  * @has_ipg_clk_name: If "ipg" is in the clock name list of device tree
@@ -253,11 +253,11 @@ struct fsl_ssi_soc_data {
 struct fsl_ssi {
 	struct regmap *regs;
 	int irq;
-	struct snd_soc_dai_driver cpu_dai_drv;
 
 	unsigned int dai_fmt;
 	u8 streams;
 	u8 i2s_net;
+	bool synchronous;
 	bool use_dma;
 	bool use_dual_fifo;
 	bool has_ipg_clk_name;
@@ -663,7 +663,6 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream,
 	bool tx2, tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
 	struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(dai);
 	struct regmap *regs = ssi->regs;
-	int synchronous = ssi->cpu_dai_drv.symmetric_rates, ret;
 	u32 pm = 999, div2, psr, stccr, mask, afreq, factor, i;
 	unsigned long clkrate, baudrate, tmprate;
 	unsigned int slots = params_channels(hw_params);
@@ -671,6 +670,7 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream,
 	u64 sub, savesub = 100000;
 	unsigned int freq;
 	bool baudclk_is_used;
+	int ret;
 
 	/* Override slots and slot_width if being specifically set... */
 	if (ssi->slots)
@@ -749,7 +749,7 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream,
 	mask = SSI_SxCCR_PM_MASK | SSI_SxCCR_DIV2 | SSI_SxCCR_PSR;
 
 	/* STCCR is used for RX in synchronous mode */
-	tx2 = tx || synchronous;
+	tx2 = tx || ssi->synchronous;
 	regmap_update_bits(regs, REG_SSI_SxCCR(tx2), mask, stccr);
 
 	if (!baudclk_is_used) {
@@ -797,7 +797,7 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream,
 	 * that should set separate configurations for STCCR and SRCCR
 	 * despite running in the synchronous mode.
 	 */
-	if (enabled && ssi->cpu_dai_drv.symmetric_rates)
+	if (enabled && ssi->synchronous)
 		return 0;
 
 	if (fsl_ssi_is_i2s_master(ssi)) {
@@ -829,7 +829,7 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	/* In synchronous mode, the SSI uses STCCR for capture */
-	tx2 = tx || ssi->cpu_dai_drv.symmetric_rates;
+	tx2 = tx || ssi->synchronous;
 	regmap_update_bits(regs, REG_SSI_SxCCR(tx2), SSI_SxCCR_WL_MASK, wl);
 
 	return 0;
@@ -954,7 +954,7 @@ static int _fsl_ssi_set_dai_fmt(struct fsl_ssi *ssi, unsigned int fmt)
 	srcr = strcr;
 
 	/* Set SYN mode and clear RXDIR bit when using SYN or AC97 mode */
-	if (ssi->cpu_dai_drv.symmetric_rates || fsl_ssi_is_ac97(ssi)) {
+	if (ssi->synchronous || fsl_ssi_is_ac97(ssi)) {
 		srcr &= ~SSI_SRCR_RXDIR;
 		scr |= SSI_SCR_SYN;
 	}
@@ -1352,6 +1352,7 @@ static void fsl_ssi_imx_clean(struct platform_device *pdev, struct fsl_ssi *ssi)
 
 static int fsl_ssi_probe(struct platform_device *pdev)
 {
+	struct snd_soc_dai_driver *cpu_dai_drv;
 	struct fsl_ssi *ssi;
 	int ret = 0;
 	struct device_node *np = pdev->dev.of_node;
@@ -1386,14 +1387,12 @@ static int fsl_ssi_probe(struct platform_device *pdev)
 	ssi->use_dma = !of_property_read_bool(np, "fsl,fiq-stream-filter");
 
 	if (fsl_ssi_is_ac97(ssi)) {
-		memcpy(&ssi->cpu_dai_drv, &fsl_ssi_ac97_dai,
-		       sizeof(fsl_ssi_ac97_dai));
+		cpu_dai_drv = &fsl_ssi_ac97_dai;
 		fsl_ac97_data = ssi;
 	} else {
-		memcpy(&ssi->cpu_dai_drv, &fsl_ssi_dai_template,
-		       sizeof(fsl_ssi_dai_template));
+		cpu_dai_drv = &fsl_ssi_dai_template;
 	}
-	ssi->cpu_dai_drv.name = dev_name(dev);
+	cpu_dai_drv->name = dev_name(dev);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	iomem = devm_ioremap_resource(dev, res);
@@ -1431,11 +1430,12 @@ static int fsl_ssi_probe(struct platform_device *pdev)
 	/* Set software limitations for synchronous mode */
 	if (!of_find_property(np, "fsl,ssi-asynchronous", NULL)) {
 		if (!fsl_ssi_is_ac97(ssi)) {
-			ssi->cpu_dai_drv.symmetric_rates = 1;
-			ssi->cpu_dai_drv.symmetric_samplebits = 1;
+			cpu_dai_drv->symmetric_rates = 1;
+			cpu_dai_drv->symmetric_samplebits = 1;
+			ssi->synchronous = true;
 		}
 
-		ssi->cpu_dai_drv.symmetric_channels = 1;
+		cpu_dai_drv->symmetric_channels = 1;
 	}
 
 	/* Fetch FIFO depth; Set to 8 for older DT without this property */
@@ -1490,7 +1490,7 @@ static int fsl_ssi_probe(struct platform_device *pdev)
 	}
 
 	ret = devm_snd_soc_register_component(dev, &fsl_ssi_component,
-					      &ssi->cpu_dai_drv, 1);
+					      cpu_dai_drv, 1);
 	if (ret) {
 		dev_err(dev, "failed to register DAI: %d\n", ret);
 		goto error_asoc_register;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v1 11/15] ASoC: fsl_ssi: Setup AC97 in dai_probe()
From: Nicolin Chen @ 2017-12-19 17:00 UTC (permalink / raw)
  To: timur, broonie
  Cc: linux-kernel, linuxppc-dev, alsa-devel, lgirdwood, fabio.estevam,
	mail, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <1513702819-42310-1-git-send-email-nicoleotsuka@gmail.com>

AC97 configures some registers earlier to start a communication
with CODECs, so this patch moves those register settings to the
dai_probe() as well, along with other register configurations.

It also applies _fsl_ssi_set_dai_fmt() to AC97 only since other
formats would be configured via fsl_ssi_set_dai_fmt() directly.

Meanwhile, this patch adds fsl_ssi_dai_ac97_remove() to cleanup
some control bits for AC97.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
 sound/soc/fsl/fsl_ssi.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 4673709..2fdae78 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -977,9 +977,6 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
 	regmap_write(regs, REG_SSI_SRCR, srcr);
 	regmap_write(regs, REG_SSI_SCR, scr);
 
-	if ((fmt & SND_SOC_DAIFMT_FORMAT_MASK) == SND_SOC_DAIFMT_AC97)
-		fsl_ssi_setup_ac97(ssi);
-
 	return 0;
 }
 
@@ -1108,6 +1105,26 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai)
 		regmap_update_bits(ssi->regs, REG_SSI_SCR,
 				   SSI_SCR_TCH_EN, SSI_SCR_TCH_EN);
 
+	/* AC97 should start earlier to communicate with CODECs */
+	if (fsl_ssi_is_ac97(ssi)) {
+		_fsl_ssi_set_dai_fmt(ssi->dev, ssi, ssi->dai_fmt);
+		fsl_ssi_setup_ac97(ssi);
+	}
+
+	return 0;
+}
+
+/**
+ * Disable register bits for AC97
+ */
+static int fsl_ssi_dai_ac97_remove(struct snd_soc_dai *dai)
+{
+	struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(dai);
+
+	regmap_write(ssi->regs, REG_SSI_SCR, 0);
+	regmap_write(ssi->regs, REG_SSI_SACNT, 0);
+	regmap_write(ssi->regs, REG_SSI_SOR, 0);
+
 	return 0;
 }
 
@@ -1147,6 +1164,7 @@ static const struct snd_soc_component_driver fsl_ssi_component = {
 static struct snd_soc_dai_driver fsl_ssi_ac97_dai = {
 	.bus_control = true,
 	.probe = fsl_ssi_dai_probe,
+	.remove = fsl_ssi_dai_ac97_remove,
 	.playback = {
 		.stream_name = "AC97 Playback",
 		.channels_min = 2,
@@ -1524,9 +1542,6 @@ static int fsl_ssi_probe(struct platform_device *pdev)
 	}
 
 done:
-	if (ssi->dai_fmt)
-		_fsl_ssi_set_dai_fmt(dev, ssi, ssi->dai_fmt);
-
 	if (fsl_ssi_is_ac97(ssi)) {
 		u32 ssi_idx;
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH v1 12/15] ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
From: Nicolin Chen @ 2017-12-19 17:00 UTC (permalink / raw)
  To: timur, broonie
  Cc: linux-kernel, linuxppc-dev, alsa-devel, lgirdwood, fabio.estevam,
	mail, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <1513702819-42310-1-git-send-email-nicoleotsuka@gmail.com>

The _fsl_ssi_set_dai_fmt() is a helper function being called from
fsl_ssi_set_dai_fmt() as an ASoC operation and from dai_probe(),
mainly for AC97 format initialization.

This patch cleans the _fsl_ssi_set_dai_fmt() in following ways:
* Removing *dev pointer in the parameters as it's included in the
  *ssi pointer of struct fsl_ssi.
* Using regmap_update_bits() instead of regmap_read() and masking
  the value manually.
* Removing TXBIT0 configurations since this bit is set to 1 as its
  reset value and there is no use case so far to unset it. And it
  is safe to remove since regmap_update_bits() won't touch it.
* Moving baudclk check to the switch-case routine to skip the I2S
  master check. And moving SxCCR.DC settings after baudclk check.
* Adding format settings for SND_SOC_DAIFMT_AC97 like others.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
 sound/soc/fsl/fsl_ssi.c | 70 ++++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 39 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 2fdae78..898f837 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -850,42 +850,27 @@ static int fsl_ssi_hw_free(struct snd_pcm_substream *substream,
 	return 0;
 }
 
-static int _fsl_ssi_set_dai_fmt(struct device *dev,
-				struct fsl_ssi *ssi, unsigned int fmt)
+static int _fsl_ssi_set_dai_fmt(struct fsl_ssi *ssi, unsigned int fmt)
 {
-	struct regmap *regs = ssi->regs;
-	u32 strcr = 0, stcr, srcr, scr, mask;
+	u32 strcr = 0, scr = 0, stcr, srcr, mask;
 
 	ssi->dai_fmt = fmt;
 
-	if (fsl_ssi_is_i2s_master(ssi) && IS_ERR(ssi->baudclk)) {
-		dev_err(dev, "missing baudclk for master mode\n");
-		return -EINVAL;
-	}
-
-	regmap_read(regs, REG_SSI_SCR, &scr);
-	scr &= ~(SSI_SCR_SYN | SSI_SCR_I2S_MODE_MASK);
 	/* Synchronize frame sync clock for TE to avoid data slipping */
 	scr |= SSI_SCR_SYNC_TX_FS;
 
-	mask = SSI_STCR_TXBIT0 | SSI_STCR_TFDIR | SSI_STCR_TXDIR |
-	       SSI_STCR_TSCKP | SSI_STCR_TFSI | SSI_STCR_TFSL | SSI_STCR_TEFS;
-	regmap_read(regs, REG_SSI_STCR, &stcr);
-	regmap_read(regs, REG_SSI_SRCR, &srcr);
-	stcr &= ~mask;
-	srcr &= ~mask;
-
 	/* Use Network mode as default */
 	ssi->i2s_net = SSI_SCR_NET;
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
-		regmap_update_bits(regs, REG_SSI_STCCR,
-				   SSI_SxCCR_DC_MASK, SSI_SxCCR_DC(2));
-		regmap_update_bits(regs, REG_SSI_SRCCR,
-				   SSI_SxCCR_DC_MASK, SSI_SxCCR_DC(2));
 		switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 		case SND_SOC_DAIFMT_CBM_CFS:
 		case SND_SOC_DAIFMT_CBS_CFS:
+			if (IS_ERR(ssi->baudclk)) {
+				dev_err(ssi->dev,
+					"missing baudclk for master mode\n");
+				return -EINVAL;
+			}
 			ssi->i2s_net |= SSI_SCR_I2S_MODE_MASTER;
 			break;
 		case SND_SOC_DAIFMT_CBM_CFM:
@@ -895,30 +880,34 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
 			return -EINVAL;
 		}
 
+		regmap_update_bits(ssi->regs, REG_SSI_STCCR,
+				   SSI_SxCCR_DC_MASK, SSI_SxCCR_DC(2));
+		regmap_update_bits(ssi->regs, REG_SSI_SRCCR,
+				   SSI_SxCCR_DC_MASK, SSI_SxCCR_DC(2));
+
 		/* Data on rising edge of bclk, frame low, 1clk before data */
-		strcr |= SSI_STCR_TFSI | SSI_STCR_TSCKP |
-			 SSI_STCR_TXBIT0 | SSI_STCR_TEFS;
+		strcr |= SSI_STCR_TFSI | SSI_STCR_TSCKP | SSI_STCR_TEFS;
 		break;
 	case SND_SOC_DAIFMT_LEFT_J:
 		/* Data on rising edge of bclk, frame high */
-		strcr |= SSI_STCR_TXBIT0 | SSI_STCR_TSCKP;
+		strcr |= SSI_STCR_TSCKP;
 		break;
 	case SND_SOC_DAIFMT_DSP_A:
 		/* Data on rising edge of bclk, frame high, 1clk before data */
-		strcr |= SSI_STCR_TFSL | SSI_STCR_TSCKP |
-			 SSI_STCR_TXBIT0 | SSI_STCR_TEFS;
+		strcr |= SSI_STCR_TFSL | SSI_STCR_TSCKP | SSI_STCR_TEFS;
 		break;
 	case SND_SOC_DAIFMT_DSP_B:
 		/* Data on rising edge of bclk, frame high */
-		strcr |= SSI_STCR_TFSL | SSI_STCR_TSCKP | SSI_STCR_TXBIT0;
+		strcr |= SSI_STCR_TFSL | SSI_STCR_TSCKP;
 		break;
 	case SND_SOC_DAIFMT_AC97:
 		/* Data on falling edge of bclk, frame high, 1clk before data */
-		ssi->i2s_net |= SSI_SCR_I2S_MODE_NORMAL;
+		strcr |= SSI_STCR_TEFS;
 		break;
 	default:
 		return -EINVAL;
 	}
+
 	scr |= ssi->i2s_net;
 
 	/* DAI clock inversion */
@@ -952,20 +941,17 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
 		break;
 	case SND_SOC_DAIFMT_CBM_CFM:
 		/* Input bit or frame sync clocks */
-		scr &= ~SSI_SCR_SYS_CLK_EN;
 		break;
 	case SND_SOC_DAIFMT_CBM_CFS:
 		/* Input bit clock but output frame sync clock */
-		strcr &= ~SSI_STCR_TXDIR;
 		strcr |= SSI_STCR_TFDIR;
-		scr &= ~SSI_SCR_SYS_CLK_EN;
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	stcr |= strcr;
-	srcr |= strcr;
+	stcr = strcr;
+	srcr = strcr;
 
 	/* Set SYN mode and clear RXDIR bit when using SYN or AC97 mode */
 	if (ssi->cpu_dai_drv.symmetric_rates || fsl_ssi_is_ac97(ssi)) {
@@ -973,9 +959,15 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
 		scr |= SSI_SCR_SYN;
 	}
 
-	regmap_write(regs, REG_SSI_STCR, stcr);
-	regmap_write(regs, REG_SSI_SRCR, srcr);
-	regmap_write(regs, REG_SSI_SCR, scr);
+	mask = SSI_STCR_TFDIR | SSI_STCR_TXDIR | SSI_STCR_TSCKP |
+	       SSI_STCR_TFSI | SSI_STCR_TFSL | SSI_STCR_TEFS;
+
+	regmap_update_bits(ssi->regs, REG_SSI_STCR, mask, stcr);
+	regmap_update_bits(ssi->regs, REG_SSI_SRCR, mask, srcr);
+
+	mask = SSI_SCR_SYNC_TX_FS | SSI_SCR_I2S_MODE_MASK |
+	       SSI_SCR_SYS_CLK_EN | SSI_SCR_SYN;
+	regmap_update_bits(ssi->regs, REG_SSI_SCR, mask, scr);
 
 	return 0;
 }
@@ -991,7 +983,7 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 	if (fsl_ssi_is_ac97(ssi))
 		return 0;
 
-	return _fsl_ssi_set_dai_fmt(dai->dev, ssi, fmt);
+	return _fsl_ssi_set_dai_fmt(ssi, fmt);
 }
 
 /**
@@ -1107,7 +1099,7 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai)
 
 	/* AC97 should start earlier to communicate with CODECs */
 	if (fsl_ssi_is_ac97(ssi)) {
-		_fsl_ssi_set_dai_fmt(ssi->dev, ssi, ssi->dai_fmt);
+		_fsl_ssi_set_dai_fmt(ssi, ssi->dai_fmt);
 		fsl_ssi_setup_ac97(ssi);
 	}
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH v1 10/15] ASoC: fsl_ssi: Move one-time configurations to dai_probe()
From: Nicolin Chen @ 2017-12-19 17:00 UTC (permalink / raw)
  To: timur, broonie
  Cc: linux-kernel, linuxppc-dev, alsa-devel, lgirdwood, fabio.estevam,
	mail, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <1513702819-42310-1-git-send-email-nicoleotsuka@gmail.com>

The dai_probe() could handle some one-time configurations since
they will not be changed once being configured.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
 sound/soc/fsl/fsl_ssi.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index fd756dd..4673709 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -855,7 +855,6 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
 {
 	struct regmap *regs = ssi->regs;
 	u32 strcr = 0, stcr, srcr, scr, mask;
-	u8 wm;
 
 	ssi->dai_fmt = fmt;
 
@@ -864,8 +863,6 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
 		return -EINVAL;
 	}
 
-	fsl_ssi_setup_regvals(ssi);
-
 	regmap_read(regs, REG_SSI_SCR, &scr);
 	scr &= ~(SSI_SCR_SYN | SSI_SCR_I2S_MODE_MASK);
 	/* Synchronize frame sync clock for TE to avoid data slipping */
@@ -980,16 +977,6 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
 	regmap_write(regs, REG_SSI_SRCR, srcr);
 	regmap_write(regs, REG_SSI_SCR, scr);
 
-	wm = ssi->fifo_watermark;
-
-	regmap_write(regs, REG_SSI_SFCSR,
-		     SSI_SFCSR_TFWM0(wm) | SSI_SFCSR_RFWM0(wm) |
-		     SSI_SFCSR_TFWM1(wm) | SSI_SFCSR_RFWM1(wm));
-
-	if (ssi->use_dual_fifo)
-		regmap_update_bits(regs, REG_SSI_SCR,
-				   SSI_SCR_TCH_EN, SSI_SCR_TCH_EN);
-
 	if ((fmt & SND_SOC_DAIFMT_FORMAT_MASK) == SND_SOC_DAIFMT_AC97)
 		fsl_ssi_setup_ac97(ssi);
 
@@ -1096,14 +1083,31 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd,
 	return 0;
 }
 
+/**
+ * Set DMA data and one-time configurations
+ */
 static int fsl_ssi_dai_probe(struct snd_soc_dai *dai)
 {
 	struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(dai);
+	u32 wm = ssi->fifo_watermark;
 
 	if (ssi->soc->imx && ssi->use_dma)
 		snd_soc_dai_init_dma_data(dai, &ssi->dma_params_tx,
 					  &ssi->dma_params_rx);
 
+	/* Initialize regvals */
+	fsl_ssi_setup_regvals(ssi);
+
+	/* Set watermarks */
+	regmap_write(ssi->regs, REG_SSI_SFCSR,
+		     SSI_SFCSR_TFWM0(wm) | SSI_SFCSR_RFWM0(wm) |
+		     SSI_SFCSR_TFWM1(wm) | SSI_SFCSR_RFWM1(wm));
+
+	/* Enable Dual FIFO mode */
+	if (ssi->use_dual_fifo)
+		regmap_update_bits(ssi->regs, REG_SSI_SCR,
+				   SSI_SCR_TCH_EN, SSI_SCR_TCH_EN);
+
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH v1 09/15] ASoC: fsl_ssi: Use snd_soc_init_dma_data instead
From: Nicolin Chen @ 2017-12-19 17:00 UTC (permalink / raw)
  To: timur, broonie
  Cc: linux-kernel, linuxppc-dev, alsa-devel, lgirdwood, fabio.estevam,
	mail, caleb, arnaud.mouiche, lukma, kernel
In-Reply-To: <1513702819-42310-1-git-send-email-nicoleotsuka@gmail.com>

Since there is a helper function, use it to help readability.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
 sound/soc/fsl/fsl_ssi.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index b51591e..fd756dd 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -1100,10 +1100,9 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai)
 {
 	struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(dai);
 
-	if (ssi->soc->imx && ssi->use_dma) {
-		dai->playback_dma_data = &ssi->dma_params_tx;
-		dai->capture_dma_data = &ssi->dma_params_rx;
-	}
+	if (ssi->soc->imx && ssi->use_dma)
+		snd_soc_dai_init_dma_data(dai, &ssi->dma_params_tx,
+					  &ssi->dma_params_rx);
 
 	return 0;
 }
-- 
2.7.4

^ 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