LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 3/4] hotplug, x86: Disable ARCH_CPU_PROBE_RELEASE on x86
From: Toshi Kani @ 2013-08-17 19:46 UTC (permalink / raw)
  To: mingo, hpa, tglx, gregkh, benh
  Cc: fenghua.yu, Toshi Kani, x86, linux-kernel, rjw, isimatu.yasuaki,
	srivatsa.bhat, bp, linuxppc-dev
In-Reply-To: <1376768819-28975-1-git-send-email-toshi.kani@hp.com>

Commit d7c53c9e enabled ARCH_CPU_PROBE_RELEASE on x86 in order to
serialize CPU online/offline operations.  Although it is the config
option to enable CPU hotplug test interfaces, probe & release, it is
also the option to enable cpu_hotplug_driver_lock() as well.  Therefore,
this option had to be enabled on x86 with dummy arch_cpu_probe() and
arch_cpu_release().

Since then, lock_device_hotplug() was introduced to serialize CPU
online/offline & hotplug operations.  Therefore, this config option
is no longer required for the serialization.  This patch disables
this config option on x86 and revert the changes made by commit
d7c53c9e.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 arch/x86/Kconfig          |    4 ----
 arch/x86/kernel/smpboot.c |   21 ---------------------
 2 files changed, 25 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b32ebf9..c87e49a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -255,10 +255,6 @@ config ARCH_HWEIGHT_CFLAGS
 	default "-fcall-saved-ecx -fcall-saved-edx" if X86_32
 	default "-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx -fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 -fcall-saved-r11" if X86_64
 
-config ARCH_CPU_PROBE_RELEASE
-	def_bool y
-	depends on HOTPLUG_CPU
-
 config ARCH_SUPPORTS_UPROBES
 	def_bool y
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index aecc98a..5b24a9d 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -82,27 +82,6 @@
 /* State of each CPU */
 DEFINE_PER_CPU(int, cpu_state) = { 0 };
 
-#ifdef CONFIG_HOTPLUG_CPU
-/*
- * We need this for trampoline_base protection from concurrent accesses when
- * off- and onlining cores wildly.
- */
-static DEFINE_MUTEX(x86_cpu_hotplug_driver_mutex);
-
-void cpu_hotplug_driver_lock(void)
-{
-	mutex_lock(&x86_cpu_hotplug_driver_mutex);
-}
-
-void cpu_hotplug_driver_unlock(void)
-{
-	mutex_unlock(&x86_cpu_hotplug_driver_mutex);
-}
-
-ssize_t arch_cpu_probe(const char *buf, size_t count) { return -1; }
-ssize_t arch_cpu_release(const char *buf, size_t count) { return -1; }
-#endif
-
 /* Number of siblings per CPU package */
 int smp_num_siblings = 1;
 EXPORT_SYMBOL(smp_num_siblings);

^ permalink raw reply related

* [PATCH 0/4] Unify CPU hotplug lock interface
From: Toshi Kani @ 2013-08-17 19:46 UTC (permalink / raw)
  To: mingo, hpa, tglx, gregkh, benh
  Cc: fenghua.yu, Toshi Kani, x86, linux-kernel, rjw, isimatu.yasuaki,
	srivatsa.bhat, bp, linuxppc-dev

lock_device_hotplug() was recently introduced to serialize CPU & Memory
online/offline and hotplug operations, along with sysfs online interface
restructure (commit 4f3549d7).  With this new locking scheme,
cpu_hotplug_driver_lock() is redundant and is no longer necessary.

This patchset makes sure that lock_device_hotplug() covers all CPU online/
offline interfaces, and then removes cpu_hotplug_driver_lock().

The patchset is based on Linus's tree, 3.11.0-rc5.

---
Toshi Kani (4):
  hotplug, x86: Fix online state in cpu0 debug interface
  hotplug, x86: Add hotplug lock to missing places
  hotplug, x86: Disable ARCH_CPU_PROBE_RELEASE on x86
  hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()

---
 arch/powerpc/kernel/smp.c              | 12 ----------
 arch/powerpc/platforms/pseries/dlpar.c | 40 +++++++++++++---------------------
 arch/x86/Kconfig                       |  4 ----
 arch/x86/kernel/smpboot.c              | 21 ------------------
 arch/x86/kernel/topology.c             | 11 ++++++----
 drivers/base/cpu.c                     | 26 ++++++++++++----------
 include/linux/cpu.h                    | 13 -----------
 7 files changed, 37 insertions(+), 90 deletions(-)

^ permalink raw reply

* Re: Critical Interrupt Input
From: Benjamin Herrenschmidt @ 2013-08-17 22:05 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev, hbausley
In-Reply-To: <DA804F98-B7E5-42C1-9D5C-8A8A63681EC0@kernel.crashing.org>

On Fri, 2013-08-16 at 06:04 -0500, Kumar Gala wrote:
> The 44x low level code needs to handle exception stacks properly for
> this to work.  Since its possible to have a critical exception occur
> while in a normal exception level, you have to have proper saving of
> additional register state and a stack frame for the critical
> exception, etc.  I'm not sure if that was ever done for 44x.

Don't 44x and FSL BookE share the same macros ? I would think 44x does
indeed implement the same crit support as e500...

What does the crash look like ?

Ben.

^ permalink raw reply

* Re: [RFC PATCH v2 3/4] powerpc: refactor of_get_cpu_node to support other architectures
From: Benjamin Herrenschmidt @ 2013-08-17 22:09 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Jonas Bonn, devicetree, Michal Simek, linux-pm,
	Sudeep KarkadaNagesha, linux-kernel, Rob Herring,
	Rafael J. Wysocki, Grant Likely, linuxppc-dev, linux-arm-kernel
In-Reply-To: <2032060.4bgTKOdEX2@flatron>

On Sat, 2013-08-17 at 12:50 +0200, Tomasz Figa wrote:
> I wonder how would this handle uniprocessor ARM (pre-v7) cores, for
> which 
> the updated bindings[1] define #address-cells = <0> and so no reg 
> property.
> 
> [1] - http://thread.gmane.org/gmane.linux.ports.arm.kernel/260795

Why did you do that in the binding ? That sounds like looking to create
problems ... 

Traditionally, UP setups just used "0" as the "reg" property on other
architectures, why do differently ?

Ben.

^ permalink raw reply

* Re: [RFC PATCH v2 3/4] powerpc: refactor of_get_cpu_node to support other architectures
From: Tomasz Figa @ 2013-08-17 22:22 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Jonas Bonn, devicetree, Michal Simek, Lorenzo Pieralisi, linux-pm,
	Sudeep KarkadaNagesha, linux-kernel, Rob Herring,
	Rafael J. Wysocki, Grant Likely, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1376777376.25016.11.camel@pasglop>

On Sunday 18 of August 2013 08:09:36 Benjamin Herrenschmidt wrote:
> On Sat, 2013-08-17 at 12:50 +0200, Tomasz Figa wrote:
> > I wonder how would this handle uniprocessor ARM (pre-v7) cores, for
> > which
> > the updated bindings[1] define #address-cells = <0> and so no reg
> > property.
> > 
> > [1] - http://thread.gmane.org/gmane.linux.ports.arm.kernel/260795
> 
> Why did you do that in the binding ? That sounds like looking to create
> problems ...

[Copying Lorenzo...]

I'm not the author of the change. I was just passing by, while the 
question showed up in my mind. ;)

> Traditionally, UP setups just used "0" as the "reg" property on other
> architectures, why do differently ?

Right, especially since the ARM DT topology parsing code still considers a 
device tree without reg property in cpu node invalid.

Best regards,
Tomasz

^ permalink raw reply

* Re: [PATCH 2/4] hotplug, x86: Add hotplug lock to missing places
From: Greg KH @ 2013-08-17 23:15 UTC (permalink / raw)
  To: Toshi Kani
  Cc: fenghua.yu, bp, x86, linux-kernel, rjw, isimatu.yasuaki, mingo,
	srivatsa.bhat, tglx, hpa, linuxppc-dev
In-Reply-To: <1376768819-28975-3-git-send-email-toshi.kani@hp.com>

On Sat, Aug 17, 2013 at 01:46:57PM -0600, Toshi Kani wrote:
> lock_device_hotplug() serializes CPU & Memory online/offline and hotplug
> operations.  However, this lock is not held in the debug interfaces below
> that initiate CPU online/offline operations.
> 
>  - _debug_hotplug_cpu(), cpu0 hotplug test interface enabled by
>    CONFIG_DEBUG_HOTPLUG_CPU0.
>  - cpu_probe_store() and cpu_release_store(), cpu hotplug test interface
>    enabled by CONFIG_ARCH_CPU_PROBE_RELEASE.
> 
> This patch changes the above interfaces to hold lock_device_hotplug().
> 
> Signed-off-by: Toshi Kani <toshi.kani@hp.com>
> ---
>  arch/x86/kernel/topology.c |    2 ++
>  drivers/base/cpu.c         |   16 ++++++++++++++--
>  2 files changed, 16 insertions(+), 2 deletions(-)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply

* Re: MPC8315 reboot failure, lockdep splat possibly related?
From: Anthony Foiani @ 2013-08-18  0:07 UTC (permalink / raw)
  To: Bhushan Bharat-R65777; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D07114F7C@039-SN2MPN1-013.039d.mgd.msft.net>

Bhushan Bharat-R65777 <R65777@freescale.com> writes:

> You should get rid of this by changing spin_lock/unlock() in
> fsl_sata_set_irq_coalescing() to spin_lock_irqsave/restore()

I can verify that the suggested change removes the lockdep warning.
The below patch is against 3.9.7 and has been tested on hardware with
that release.

It applies with slight fuzz to linux-next; I've compile-tested that
version, but I have not booted that build on the hardware.  The
linux-next patch can be found here:

  http://scrye.com/~tkil/linux/fsl-sata-lockdep-201308/next-sata-fsl-save-irqs-while-coalescing.patch
  (or: http://preview.tinyurl.com/mpd4e9h )

Unfortunately, the hang on reboot was not easily repeatable; I'll
report whether it happens in the next few days or not.

Thanks again,
Anthony Foiani

-- >8 --

>From 2abb6df770c95eb4103476c70847a78f816fe5e3 Mon Sep 17 00:00:00 2001
From: Anthony Foiani <anthony.foiani@gmail.com>
Date: Sat, 17 Aug 2013 13:28:17 -0600
Subject: [PATCH] sata: fsl: save irqs while coalescing

Before this patch, I was seeing the following lockdep splat on my
MPC8315 (PPC32) target:

  [    9.086051] =================================
  [    9.090393] [ INFO: inconsistent lock state ]
  [    9.094744] 3.9.7-ajf-gc39503d #1 Not tainted
  [    9.099087] ---------------------------------
  [    9.103432] inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
  [    9.109431] scsi_eh_1/39 [HC1[1]:SC0[0]:HE0:SE1] takes:
  [    9.114642]  (&(&host->lock)->rlock){?.+...}, at: [<c02f4168>] sata_fsl_interrupt+0x50/0x250
  [    9.123137] {HARDIRQ-ON-W} state was registered at:
  [    9.128004]   [<c006cdb8>] lock_acquire+0x90/0xf4
  [    9.132737]   [<c043ef04>] _raw_spin_lock+0x34/0x4c
  [    9.137645]   [<c02f3560>] fsl_sata_set_irq_coalescing+0x68/0x100
  [    9.143750]   [<c02f36a0>] sata_fsl_init_controller+0xa8/0xc0
  [    9.149505]   [<c02f3f10>] sata_fsl_probe+0x17c/0x2e8
  [    9.154568]   [<c02acc90>] driver_probe_device+0x90/0x248
  [    9.159987]   [<c02acf0c>] __driver_attach+0xc4/0xc8
  [    9.164964]   [<c02aae74>] bus_for_each_dev+0x5c/0xa8
  [    9.170028]   [<c02ac218>] bus_add_driver+0x100/0x26c
  [    9.175091]   [<c02ad638>] driver_register+0x88/0x198
  [    9.180155]   [<c0003a24>] do_one_initcall+0x58/0x1b4
  [    9.185226]   [<c05aeeac>] kernel_init_freeable+0x118/0x1c0
  [    9.190823]   [<c0004110>] kernel_init+0x18/0x108
  [    9.195542]   [<c000f6b8>] ret_from_kernel_thread+0x64/0x6c
  [    9.201142] irq event stamp: 160
  [    9.204366] hardirqs last  enabled at (159): [<c043f778>] _raw_spin_unlock_irq+0x30/0x50
  [    9.212469] hardirqs last disabled at (160): [<c000f414>] reenable_mmu+0x30/0x88
  [    9.219867] softirqs last  enabled at (144): [<c002ae5c>] __do_softirq+0x168/0x218
  [    9.227435] softirqs last disabled at (137): [<c002b0d4>] irq_exit+0xa8/0xb4
  [    9.234481]
  [    9.234481] other info that might help us debug this:
  [    9.240995]  Possible unsafe locking scenario:
  [    9.240995]
  [    9.246898]        CPU0
  [    9.249337]        ----
  [    9.251776]   lock(&(&host->lock)->rlock);
  [    9.255878]   <Interrupt>
  [    9.258492]     lock(&(&host->lock)->rlock);
  [    9.262765]
  [    9.262765]  *** DEADLOCK ***
  [    9.262765]
  [    9.268684] no locks held by scsi_eh_1/39.
  [    9.272767]
  [    9.272767] stack backtrace:
  [    9.277117] Call Trace:
  [    9.279589] [cfff9da0] [c0008504] show_stack+0x48/0x150 (unreliable)
  [    9.285972] [cfff9de0] [c0447d5c] print_usage_bug.part.35+0x268/0x27c
  [    9.292425] [cfff9e10] [c006ace4] mark_lock+0x2ac/0x658
  [    9.297660] [cfff9e40] [c006b7e4] __lock_acquire+0x754/0x1840
  [    9.303414] [cfff9ee0] [c006cdb8] lock_acquire+0x90/0xf4
  [    9.308745] [cfff9f20] [c043ef04] _raw_spin_lock+0x34/0x4c
  [    9.314250] [cfff9f30] [c02f4168] sata_fsl_interrupt+0x50/0x250
  [    9.320187] [cfff9f70] [c0079ff0] handle_irq_event_percpu+0x90/0x254
  [    9.326547] [cfff9fc0] [c007a1fc] handle_irq_event+0x48/0x78
  [    9.332220] [cfff9fe0] [c007c95c] handle_level_irq+0x9c/0x104
  [    9.337981] [cfff9ff0] [c000d978] call_handle_irq+0x18/0x28
  [    9.343568] [cc7139f0] [c000608c] do_IRQ+0xf0/0x1a8
  [    9.348464] [cc713a20] [c000fc8c] ret_from_except+0x0/0x14
  [    9.353983] --- Exception: 501 at _raw_spin_unlock_irq+0x40/0x50
  [    9.353983]     LR = _raw_spin_unlock_irq+0x30/0x50
  [    9.364839] [cc713af0] [c043db10] wait_for_common+0xac/0x188
  [    9.370513] [cc713b30] [c02ddee4] ata_exec_internal_sg+0x2b0/0x4f0
  [    9.376699] [cc713be0] [c02de18c] ata_exec_internal+0x68/0xa8
  [    9.382454] [cc713c20] [c02de4b8] ata_dev_read_id+0x158/0x594
  [    9.388205] [cc713ca0] [c02ec244] ata_eh_recover+0xd88/0x13d0
  [    9.393962] [cc713d20] [c02f2520] sata_pmp_error_handler+0xc0/0x8ac
  [    9.400234] [cc713dd0] [c02ecdc8] ata_scsi_port_error_handler+0x464/0x5e8
  [    9.407023] [cc713e10] [c02ecfd0] ata_scsi_error+0x84/0xb8
  [    9.412528] [cc713e40] [c02c4974] scsi_error_handler+0xd8/0x47c
  [    9.418457] [cc713eb0] [c004737c] kthread+0xa8/0xac
  [    9.423355] [cc713f40] [c000f6b8] ret_from_kernel_thread+0x64/0x6c

This fix was suggested by Bhushan Bharat <R65777@freescale.com>, and
was discussed in email at:

  http://linuxppc.10917.n7.nabble.com/MPC8315-reboot-failure-lockdep-splat-possibly-related-tp75162.html

Signed-off-by: Anthony Foiani <anthony.foiani@gmail.com>
---
 drivers/ata/sata_fsl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index 0e93ba9..c240b63 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -292,6 +292,7 @@ static void fsl_sata_set_irq_coalescing(struct ata_host *host,
 {
 	struct sata_fsl_host_priv *host_priv = host->private_data;
 	void __iomem *hcr_base = host_priv->hcr_base;
+	unsigned long flags;
 
 	if (count > ICC_MAX_INT_COUNT_THRESHOLD)
 		count = ICC_MAX_INT_COUNT_THRESHOLD;
@@ -304,12 +305,12 @@ static void fsl_sata_set_irq_coalescing(struct ata_host *host,
 			(count > ICC_MIN_INT_COUNT_THRESHOLD))
 		ticks = ICC_SAFE_INT_TICKS;
 
-	spin_lock(&host->lock);
+	spin_lock_irqsave(&host->lock, flags);
 	iowrite32((count << 24 | ticks), hcr_base + ICC);
 
 	intr_coalescing_count = count;
 	intr_coalescing_ticks = ticks;
-	spin_unlock(&host->lock);
+	spin_unlock_irqrestore(&host->lock, flags);
 
 	DPRINTK("intrrupt coalescing, count = 0x%x, ticks = %x\n",
 			intr_coalescing_count, intr_coalescing_ticks);
-- 
1.8.1.4

^ permalink raw reply related

* Re: [PATCH 1/4] hotplug, x86: Fix online state in cpu0 debug interface
From: Rafael J. Wysocki @ 2013-08-18  0:59 UTC (permalink / raw)
  To: Toshi Kani
  Cc: fenghua.yu, bp, gregkh, x86, linux-kernel, isimatu.yasuaki, mingo,
	srivatsa.bhat, tglx, hpa, linuxppc-dev
In-Reply-To: <1376768819-28975-2-git-send-email-toshi.kani@hp.com>

On Saturday, August 17, 2013 01:46:56 PM Toshi Kani wrote:
> _debug_hotplug_cpu() is a debug interface that puts cpu0 offline during
> boot-up when CONFIG_DEBUG_HOTPLUG_CPU0 is set.  After cpu0 is put offline
> in this interface, however, /sys/devices/system/cpu/cpu0/online still
> shows 1 (online).
> 
> This patch fixes _debug_hotplug_cpu() to update dev->offline when CPU
> online/offline operation succeeded.
> 
> Signed-off-by: Toshi Kani <toshi.kani@hp.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  arch/x86/kernel/topology.c |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
> index 6e60b5f..5823bbd 100644
> --- a/arch/x86/kernel/topology.c
> +++ b/arch/x86/kernel/topology.c
> @@ -72,16 +72,19 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
>  		ret = cpu_down(cpu);
>  		if (!ret) {
>  			pr_info("CPU %u is now offline\n", cpu);
> +			dev->offline = true;
>  			kobject_uevent(&dev->kobj, KOBJ_OFFLINE);
>  		} else
>  			pr_debug("Can't offline CPU%d.\n", cpu);
>  		break;
>  	case 1:
>  		ret = cpu_up(cpu);
> -		if (!ret)
> +		if (!ret) {
> +			dev->offline = false;
>  			kobject_uevent(&dev->kobj, KOBJ_ONLINE);
> -		else
> +		} else {
>  			pr_debug("Can't online CPU%d.\n", cpu);
> +		}
>  		break;
>  	default:
>  		ret = -EINVAL;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [PATCH 2/4] hotplug, x86: Add hotplug lock to missing places
From: Rafael J. Wysocki @ 2013-08-18  0:59 UTC (permalink / raw)
  To: Toshi Kani
  Cc: fenghua.yu, bp, gregkh, x86, linux-kernel, isimatu.yasuaki, mingo,
	srivatsa.bhat, tglx, hpa, linuxppc-dev
In-Reply-To: <1376768819-28975-3-git-send-email-toshi.kani@hp.com>

On Saturday, August 17, 2013 01:46:57 PM Toshi Kani wrote:
> lock_device_hotplug() serializes CPU & Memory online/offline and hotplug
> operations.  However, this lock is not held in the debug interfaces below
> that initiate CPU online/offline operations.
> 
>  - _debug_hotplug_cpu(), cpu0 hotplug test interface enabled by
>    CONFIG_DEBUG_HOTPLUG_CPU0.
>  - cpu_probe_store() and cpu_release_store(), cpu hotplug test interface
>    enabled by CONFIG_ARCH_CPU_PROBE_RELEASE.
> 
> This patch changes the above interfaces to hold lock_device_hotplug().
> 
> Signed-off-by: Toshi Kani <toshi.kani@hp.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  arch/x86/kernel/topology.c |    2 ++
>  drivers/base/cpu.c         |   16 ++++++++++++++--
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
> index 5823bbd..a3f35eb 100644
> --- a/arch/x86/kernel/topology.c
> +++ b/arch/x86/kernel/topology.c
> @@ -65,6 +65,7 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
>  	if (!cpu_is_hotpluggable(cpu))
>  		return -EINVAL;
>  
> +	lock_device_hotplug();
>  	cpu_hotplug_driver_lock();
>  
>  	switch (action) {
> @@ -91,6 +92,7 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
>  	}
>  
>  	cpu_hotplug_driver_unlock();
> +	unlock_device_hotplug();
>  
>  	return ret;
>  }
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index 4c358bc..4cc6928 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -88,7 +88,13 @@ static ssize_t cpu_probe_store(struct device *dev,
>  			       const char *buf,
>  			       size_t count)
>  {
> -	return arch_cpu_probe(buf, count);
> +	ssize_t ret;
> +
> +	lock_device_hotplug();
> +	ret = arch_cpu_probe(buf, count);
> +	unlock_device_hotplug();
> +
> +	return ret;
>  }
>  
>  static ssize_t cpu_release_store(struct device *dev,
> @@ -96,7 +102,13 @@ static ssize_t cpu_release_store(struct device *dev,
>  				 const char *buf,
>  				 size_t count)
>  {
> -	return arch_cpu_release(buf, count);
> +	ssize_t ret;
> +
> +	lock_device_hotplug();
> +	ret = arch_cpu_release(buf, count);
> +	unlock_device_hotplug();
> +
> +	return ret;
>  }
>  
>  static DEVICE_ATTR(probe, S_IWUSR, NULL, cpu_probe_store);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [PATCH 3/4] hotplug, x86: Disable ARCH_CPU_PROBE_RELEASE on x86
From: Rafael J. Wysocki @ 2013-08-18  1:00 UTC (permalink / raw)
  To: Toshi Kani
  Cc: fenghua.yu, bp, gregkh, x86, linux-kernel, isimatu.yasuaki, mingo,
	srivatsa.bhat, tglx, hpa, linuxppc-dev
In-Reply-To: <1376768819-28975-4-git-send-email-toshi.kani@hp.com>

On Saturday, August 17, 2013 01:46:58 PM Toshi Kani wrote:
> Commit d7c53c9e enabled ARCH_CPU_PROBE_RELEASE on x86 in order to
> serialize CPU online/offline operations.  Although it is the config
> option to enable CPU hotplug test interfaces, probe & release, it is
> also the option to enable cpu_hotplug_driver_lock() as well.  Therefore,
> this option had to be enabled on x86 with dummy arch_cpu_probe() and
> arch_cpu_release().
> 
> Since then, lock_device_hotplug() was introduced to serialize CPU
> online/offline & hotplug operations.  Therefore, this config option
> is no longer required for the serialization.  This patch disables
> this config option on x86 and revert the changes made by commit
> d7c53c9e.
> 
> Signed-off-by: Toshi Kani <toshi.kani@hp.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  arch/x86/Kconfig          |    4 ----
>  arch/x86/kernel/smpboot.c |   21 ---------------------
>  2 files changed, 25 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index b32ebf9..c87e49a 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -255,10 +255,6 @@ config ARCH_HWEIGHT_CFLAGS
>  	default "-fcall-saved-ecx -fcall-saved-edx" if X86_32
>  	default "-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx -fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 -fcall-saved-r11" if X86_64
>  
> -config ARCH_CPU_PROBE_RELEASE
> -	def_bool y
> -	depends on HOTPLUG_CPU
> -
>  config ARCH_SUPPORTS_UPROBES
>  	def_bool y
>  
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index aecc98a..5b24a9d 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -82,27 +82,6 @@
>  /* State of each CPU */
>  DEFINE_PER_CPU(int, cpu_state) = { 0 };
>  
> -#ifdef CONFIG_HOTPLUG_CPU
> -/*
> - * We need this for trampoline_base protection from concurrent accesses when
> - * off- and onlining cores wildly.
> - */
> -static DEFINE_MUTEX(x86_cpu_hotplug_driver_mutex);
> -
> -void cpu_hotplug_driver_lock(void)
> -{
> -	mutex_lock(&x86_cpu_hotplug_driver_mutex);
> -}
> -
> -void cpu_hotplug_driver_unlock(void)
> -{
> -	mutex_unlock(&x86_cpu_hotplug_driver_mutex);
> -}
> -
> -ssize_t arch_cpu_probe(const char *buf, size_t count) { return -1; }
> -ssize_t arch_cpu_release(const char *buf, size_t count) { return -1; }
> -#endif
> -
>  /* Number of siblings per CPU package */
>  int smp_num_siblings = 1;
>  EXPORT_SYMBOL(smp_num_siblings);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [PATCH 4/4] hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()
From: Rafael J. Wysocki @ 2013-08-18  1:01 UTC (permalink / raw)
  To: Toshi Kani
  Cc: fenghua.yu, bp, gregkh, x86, linux-kernel, isimatu.yasuaki, mingo,
	srivatsa.bhat, tglx, hpa, linuxppc-dev
In-Reply-To: <1376768819-28975-5-git-send-email-toshi.kani@hp.com>

On Saturday, August 17, 2013 01:46:59 PM Toshi Kani wrote:
> cpu_hotplug_driver_lock() serializes CPU online/offline operations
> when ARCH_CPU_PROBE_RELEASE is set.  This lock interface is no longer
> necessary with the following reason:
> 
>  - lock_device_hotplug() now protects CPU online/offline operations,
>    including the probe & release interfaces enabled by
>    ARCH_CPU_PROBE_RELEASE.  The use of cpu_hotplug_driver_lock() is
>    redundant.
>  - cpu_hotplug_driver_lock() is only valid when ARCH_CPU_PROBE_RELEASE
>    is defined, which is misleading and is only enabled on powerpc.
> 
> This patch removes the cpu_hotplug_driver_lock() interface.  As
> a result, ARCH_CPU_PROBE_RELEASE only enables / disables the cpu
> probe & release interface as intended.  There is no functional change
> in this patch.
> 
> Signed-off-by: Toshi Kani <toshi.kani@hp.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
> Performed build test only on powerpc.
> ---
>  arch/powerpc/kernel/smp.c              |   12 ----------
>  arch/powerpc/platforms/pseries/dlpar.c |   40 ++++++++++++--------------------
>  arch/x86/kernel/topology.c             |    2 --
>  drivers/base/cpu.c                     |   10 +-------
>  include/linux/cpu.h                    |   13 ----------
>  5 files changed, 16 insertions(+), 61 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 38b0ba6..1667269 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -763,18 +763,6 @@ void __cpu_die(unsigned int cpu)
>  		smp_ops->cpu_die(cpu);
>  }
>  
> -static DEFINE_MUTEX(powerpc_cpu_hotplug_driver_mutex);
> -
> -void cpu_hotplug_driver_lock()
> -{
> -	mutex_lock(&powerpc_cpu_hotplug_driver_mutex);
> -}
> -
> -void cpu_hotplug_driver_unlock()
> -{
> -	mutex_unlock(&powerpc_cpu_hotplug_driver_mutex);
> -}
> -
>  void cpu_die(void)
>  {
>  	if (ppc_md.cpu_die)
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
> index a1a7b9a..e39325d 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -387,18 +387,13 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
>  	char *cpu_name;
>  	int rc;
>  
> -	cpu_hotplug_driver_lock();
>  	rc = strict_strtoul(buf, 0, &drc_index);
> -	if (rc) {
> -		rc = -EINVAL;
> -		goto out;
> -	}
> +	if (rc)
> +		return -EINVAL;
>  
>  	dn = dlpar_configure_connector(drc_index);
> -	if (!dn) {
> -		rc = -EINVAL;
> -		goto out;
> -	}
> +	if (!dn)
> +		return -EINVAL;
>  
>  	/* configure-connector reports cpus as living in the base
>  	 * directory of the device tree.  CPUs actually live in the
> @@ -407,8 +402,7 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
>  	cpu_name = kasprintf(GFP_KERNEL, "/cpus%s", dn->full_name);
>  	if (!cpu_name) {
>  		dlpar_free_cc_nodes(dn);
> -		rc = -ENOMEM;
> -		goto out;
> +		return -ENOMEM;
>  	}
>  
>  	kfree(dn->full_name);
> @@ -417,22 +411,21 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
>  	rc = dlpar_acquire_drc(drc_index);
>  	if (rc) {
>  		dlpar_free_cc_nodes(dn);
> -		rc = -EINVAL;
> -		goto out;
> +		return -EINVAL;
>  	}
>  
>  	rc = dlpar_attach_node(dn);
>  	if (rc) {
>  		dlpar_release_drc(drc_index);
>  		dlpar_free_cc_nodes(dn);
> -		goto out;
> +		return rc;
>  	}
>  
>  	rc = dlpar_online_cpu(dn);
> -out:
> -	cpu_hotplug_driver_unlock();
> +	if (rc)
> +		return rc;
>  
> -	return rc ? rc : count;
> +	return count;
>  }
>  
>  static int dlpar_offline_cpu(struct device_node *dn)
> @@ -505,30 +498,27 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
>  		return -EINVAL;
>  	}
>  
> -	cpu_hotplug_driver_lock();
>  	rc = dlpar_offline_cpu(dn);
>  	if (rc) {
>  		of_node_put(dn);
> -		rc = -EINVAL;
> -		goto out;
> +		return -EINVAL;
>  	}
>  
>  	rc = dlpar_release_drc(*drc_index);
>  	if (rc) {
>  		of_node_put(dn);
> -		goto out;
> +		return rc;
>  	}
>  
>  	rc = dlpar_detach_node(dn);
>  	if (rc) {
>  		dlpar_acquire_drc(*drc_index);
> -		goto out;
> +		return rc;
>  	}
>  
>  	of_node_put(dn);
> -out:
> -	cpu_hotplug_driver_unlock();
> -	return rc ? rc : count;
> +
> +	return count;
>  }
>  
>  static int __init pseries_dlpar_init(void)
> diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
> index a3f35eb..649b010 100644
> --- a/arch/x86/kernel/topology.c
> +++ b/arch/x86/kernel/topology.c
> @@ -66,7 +66,6 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
>  		return -EINVAL;
>  
>  	lock_device_hotplug();
> -	cpu_hotplug_driver_lock();
>  
>  	switch (action) {
>  	case 0:
> @@ -91,7 +90,6 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
>  		ret = -EINVAL;
>  	}
>  
> -	cpu_hotplug_driver_unlock();
>  	unlock_device_hotplug();
>  
>  	return ret;
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index 4cc6928..b4ebd7b 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -45,8 +45,6 @@ static int __ref cpu_subsys_online(struct device *dev)
>  	int from_nid, to_nid;
>  	int ret;
>  
> -	cpu_hotplug_driver_lock();
> -
>  	from_nid = cpu_to_node(cpuid);
>  	ret = cpu_up(cpuid);
>  	/*
> @@ -57,18 +55,12 @@ static int __ref cpu_subsys_online(struct device *dev)
>  	if (from_nid != to_nid)
>  		change_cpu_under_node(cpu, from_nid, to_nid);
>  
> -	cpu_hotplug_driver_unlock();
>  	return ret;
>  }
>  
>  static int cpu_subsys_offline(struct device *dev)
>  {
> -	int ret;
> -
> -	cpu_hotplug_driver_lock();
> -	ret = cpu_down(dev->id);
> -	cpu_hotplug_driver_unlock();
> -	return ret;
> +	return cpu_down(dev->id);
>  }
>  
>  void unregister_cpu(struct cpu *cpu)
> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index ab0eade..e847ef8 100644
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -182,19 +182,6 @@ extern void cpu_hotplug_enable(void);
>  void clear_tasks_mm_cpumask(int cpu);
>  int cpu_down(unsigned int cpu);
>  
> -#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
> -extern void cpu_hotplug_driver_lock(void);
> -extern void cpu_hotplug_driver_unlock(void);
> -#else
> -static inline void cpu_hotplug_driver_lock(void)
> -{
> -}
> -
> -static inline void cpu_hotplug_driver_unlock(void)
> -{
> -}
> -#endif
> -
>  #else		/* CONFIG_HOTPLUG_CPU */
>  
>  #define get_online_cpus()	do { } while (0)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [PATCH 0/4] Unify CPU hotplug lock interface
From: Rafael J. Wysocki @ 2013-08-18  1:02 UTC (permalink / raw)
  To: Toshi Kani
  Cc: fenghua.yu, bp, gregkh, x86, linux-kernel, isimatu.yasuaki, mingo,
	srivatsa.bhat, tglx, hpa, linuxppc-dev
In-Reply-To: <1376768819-28975-1-git-send-email-toshi.kani@hp.com>

On Saturday, August 17, 2013 01:46:55 PM Toshi Kani wrote:
> lock_device_hotplug() was recently introduced to serialize CPU & Memory
> online/offline and hotplug operations, along with sysfs online interface
> restructure (commit 4f3549d7).  With this new locking scheme,
> cpu_hotplug_driver_lock() is redundant and is no longer necessary.
> 
> This patchset makes sure that lock_device_hotplug() covers all CPU online/
> offline interfaces, and then removes cpu_hotplug_driver_lock().
> 
> The patchset is based on Linus's tree, 3.11.0-rc5.

Nice series, thanks a lot for taking care of this!

Rafael


> ---
> Toshi Kani (4):
>   hotplug, x86: Fix online state in cpu0 debug interface
>   hotplug, x86: Add hotplug lock to missing places
>   hotplug, x86: Disable ARCH_CPU_PROBE_RELEASE on x86
>   hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()
> 
> ---
>  arch/powerpc/kernel/smp.c              | 12 ----------
>  arch/powerpc/platforms/pseries/dlpar.c | 40 +++++++++++++---------------------
>  arch/x86/Kconfig                       |  4 ----
>  arch/x86/kernel/smpboot.c              | 21 ------------------
>  arch/x86/kernel/topology.c             | 11 ++++++----
>  drivers/base/cpu.c                     | 26 ++++++++++++----------
>  include/linux/cpu.h                    | 13 -----------
>  7 files changed, 37 insertions(+), 90 deletions(-)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: PCIE device errors after linux kernel upgrade
From: ravich @ 2013-08-18 11:30 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <CAErSpo6zGZkL7hWmBOVzd2=MWvY18v5xLO1qg7jVfSgqa8wHzw@mail.gmail.com>

Hi

>>Is this still a problem, or have you found a solution in the meantime? 
I did not found any solution yet
>>Where are the "PCIE ERR_CAP_STAT" and similar messages from? 
PCIE ERR_CAP_STAT is coming from:
drivers/edac/mpc85xx_edac.c

>>Can you post the complete logs somewhere, maybe in a bugzilla or similar? 
cant I post here??

Thanks for your time 



--
View this message in context: http://linuxppc.10917.n7.nabble.com/Re-PCIE-device-errors-after-linux-kernel-upgrade-tp74563p75193.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.

^ permalink raw reply

* RE: MPC8315 reboot failure, lockdep splat possibly related?
From: Bhushan Bharat-R65777 @ 2013-08-18 17:00 UTC (permalink / raw)
  To: Anthony Foiani; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <gwqnjoo9g.fsf@dworkin.scrye.com>



> -----Original Message-----
> From: Anthony Foiani [mailto:tkil@scrye.com]
> Sent: Sunday, August 18, 2013 5:37 AM
> To: Bhushan Bharat-R65777
> Cc: linuxppc-dev@lists.ozlabs.org
> Subject: Re: MPC8315 reboot failure, lockdep splat possibly related?
>=20
> Bhushan Bharat-R65777 <R65777@freescale.com> writes:
>=20
> > You should get rid of this by changing spin_lock/unlock() in
> > fsl_sata_set_irq_coalescing() to spin_lock_irqsave/restore()
>=20
> I can verify that the suggested change removes the lockdep warning.
> The below patch is against 3.9.7 and has been tested on hardware with tha=
t
> release.
>=20
> It applies with slight fuzz to linux-next; I've compile-tested that versi=
on, but
> I have not booted that build on the hardware.  The linux-next patch can b=
e found
> here:
>=20
>   http://scrye.com/~tkil/linux/fsl-sata-lockdep-201308/next-sata-fsl-save=
-irqs-
> while-coalescing.patch
>   (or: http://preview.tinyurl.com/mpd4e9h )

Anthony, I would prefer if you can send the patch (In case not then let me =
know)

Thanks
-Bharat

>=20
> Unfortunately, the hang on reboot was not easily repeatable; I'll report =
whether
> it happens in the next few days or not.


>=20
> Thanks again,
> Anthony Foiani
>=20
> -- >8 --
>=20
> From 2abb6df770c95eb4103476c70847a78f816fe5e3 Mon Sep 17 00:00:00 2001
> From: Anthony Foiani <anthony.foiani@gmail.com>
> Date: Sat, 17 Aug 2013 13:28:17 -0600
> Subject: [PATCH] sata: fsl: save irqs while coalescing
>=20
> Before this patch, I was seeing the following lockdep splat on my
> MPC8315 (PPC32) target:
>=20
>   [    9.086051] =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>   [    9.090393] [ INFO: inconsistent lock state ]
>   [    9.094744] 3.9.7-ajf-gc39503d #1 Not tainted
>   [    9.099087] ---------------------------------
>   [    9.103432] inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
>   [    9.109431] scsi_eh_1/39 [HC1[1]:SC0[0]:HE0:SE1] takes:
>   [    9.114642]  (&(&host->lock)->rlock){?.+...}, at: [<c02f4168>]
> sata_fsl_interrupt+0x50/0x250
>   [    9.123137] {HARDIRQ-ON-W} state was registered at:
>   [    9.128004]   [<c006cdb8>] lock_acquire+0x90/0xf4
>   [    9.132737]   [<c043ef04>] _raw_spin_lock+0x34/0x4c
>   [    9.137645]   [<c02f3560>] fsl_sata_set_irq_coalescing+0x68/0x100
>   [    9.143750]   [<c02f36a0>] sata_fsl_init_controller+0xa8/0xc0
>   [    9.149505]   [<c02f3f10>] sata_fsl_probe+0x17c/0x2e8
>   [    9.154568]   [<c02acc90>] driver_probe_device+0x90/0x248
>   [    9.159987]   [<c02acf0c>] __driver_attach+0xc4/0xc8
>   [    9.164964]   [<c02aae74>] bus_for_each_dev+0x5c/0xa8
>   [    9.170028]   [<c02ac218>] bus_add_driver+0x100/0x26c
>   [    9.175091]   [<c02ad638>] driver_register+0x88/0x198
>   [    9.180155]   [<c0003a24>] do_one_initcall+0x58/0x1b4
>   [    9.185226]   [<c05aeeac>] kernel_init_freeable+0x118/0x1c0
>   [    9.190823]   [<c0004110>] kernel_init+0x18/0x108
>   [    9.195542]   [<c000f6b8>] ret_from_kernel_thread+0x64/0x6c
>   [    9.201142] irq event stamp: 160
>   [    9.204366] hardirqs last  enabled at (159): [<c043f778>]
> _raw_spin_unlock_irq+0x30/0x50
>   [    9.212469] hardirqs last disabled at (160): [<c000f414>]
> reenable_mmu+0x30/0x88
>   [    9.219867] softirqs last  enabled at (144): [<c002ae5c>]
> __do_softirq+0x168/0x218
>   [    9.227435] softirqs last disabled at (137): [<c002b0d4>]
> irq_exit+0xa8/0xb4
>   [    9.234481]
>   [    9.234481] other info that might help us debug this:
>   [    9.240995]  Possible unsafe locking scenario:
>   [    9.240995]
>   [    9.246898]        CPU0
>   [    9.249337]        ----
>   [    9.251776]   lock(&(&host->lock)->rlock);
>   [    9.255878]   <Interrupt>
>   [    9.258492]     lock(&(&host->lock)->rlock);
>   [    9.262765]
>   [    9.262765]  *** DEADLOCK ***
>   [    9.262765]
>   [    9.268684] no locks held by scsi_eh_1/39.
>   [    9.272767]
>   [    9.272767] stack backtrace:
>   [    9.277117] Call Trace:
>   [    9.279589] [cfff9da0] [c0008504] show_stack+0x48/0x150 (unreliable)
>   [    9.285972] [cfff9de0] [c0447d5c] print_usage_bug.part.35+0x268/0x27=
c
>   [    9.292425] [cfff9e10] [c006ace4] mark_lock+0x2ac/0x658
>   [    9.297660] [cfff9e40] [c006b7e4] __lock_acquire+0x754/0x1840
>   [    9.303414] [cfff9ee0] [c006cdb8] lock_acquire+0x90/0xf4
>   [    9.308745] [cfff9f20] [c043ef04] _raw_spin_lock+0x34/0x4c
>   [    9.314250] [cfff9f30] [c02f4168] sata_fsl_interrupt+0x50/0x250
>   [    9.320187] [cfff9f70] [c0079ff0] handle_irq_event_percpu+0x90/0x254
>   [    9.326547] [cfff9fc0] [c007a1fc] handle_irq_event+0x48/0x78
>   [    9.332220] [cfff9fe0] [c007c95c] handle_level_irq+0x9c/0x104
>   [    9.337981] [cfff9ff0] [c000d978] call_handle_irq+0x18/0x28
>   [    9.343568] [cc7139f0] [c000608c] do_IRQ+0xf0/0x1a8
>   [    9.348464] [cc713a20] [c000fc8c] ret_from_except+0x0/0x14
>   [    9.353983] --- Exception: 501 at _raw_spin_unlock_irq+0x40/0x50
>   [    9.353983]     LR =3D _raw_spin_unlock_irq+0x30/0x50
>   [    9.364839] [cc713af0] [c043db10] wait_for_common+0xac/0x188
>   [    9.370513] [cc713b30] [c02ddee4] ata_exec_internal_sg+0x2b0/0x4f0
>   [    9.376699] [cc713be0] [c02de18c] ata_exec_internal+0x68/0xa8
>   [    9.382454] [cc713c20] [c02de4b8] ata_dev_read_id+0x158/0x594
>   [    9.388205] [cc713ca0] [c02ec244] ata_eh_recover+0xd88/0x13d0
>   [    9.393962] [cc713d20] [c02f2520] sata_pmp_error_handler+0xc0/0x8ac
>   [    9.400234] [cc713dd0] [c02ecdc8] ata_scsi_port_error_handler+0x464/=
0x5e8
>   [    9.407023] [cc713e10] [c02ecfd0] ata_scsi_error+0x84/0xb8
>   [    9.412528] [cc713e40] [c02c4974] scsi_error_handler+0xd8/0x47c
>   [    9.418457] [cc713eb0] [c004737c] kthread+0xa8/0xac
>   [    9.423355] [cc713f40] [c000f6b8] ret_from_kernel_thread+0x64/0x6c
>=20
> This fix was suggested by Bhushan Bharat <R65777@freescale.com>, and was
> discussed in email at:
>=20
>   http://linuxppc.10917.n7.nabble.com/MPC8315-reboot-failure-lockdep-spla=
t-
> possibly-related-tp75162.html
>=20
> Signed-off-by: Anthony Foiani <anthony.foiani@gmail.com>
> ---
>  drivers/ata/sata_fsl.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>=20
> diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c index
> 0e93ba9..c240b63 100644
> --- a/drivers/ata/sata_fsl.c
> +++ b/drivers/ata/sata_fsl.c
> @@ -292,6 +292,7 @@ static void fsl_sata_set_irq_coalescing(struct ata_ho=
st
> *host,  {
>  	struct sata_fsl_host_priv *host_priv =3D host->private_data;
>  	void __iomem *hcr_base =3D host_priv->hcr_base;
> +	unsigned long flags;
>=20
>  	if (count > ICC_MAX_INT_COUNT_THRESHOLD)
>  		count =3D ICC_MAX_INT_COUNT_THRESHOLD;
> @@ -304,12 +305,12 @@ static void fsl_sata_set_irq_coalescing(struct ata_=
host
> *host,
>  			(count > ICC_MIN_INT_COUNT_THRESHOLD))
>  		ticks =3D ICC_SAFE_INT_TICKS;
>=20
> -	spin_lock(&host->lock);
> +	spin_lock_irqsave(&host->lock, flags);
>  	iowrite32((count << 24 | ticks), hcr_base + ICC);
>=20
>  	intr_coalescing_count =3D count;
>  	intr_coalescing_ticks =3D ticks;
> -	spin_unlock(&host->lock);
> +	spin_unlock_irqrestore(&host->lock, flags);
>=20
>  	DPRINTK("intrrupt coalescing, count =3D 0x%x, ticks =3D %x\n",
>  			intr_coalescing_count, intr_coalescing_ticks);
> --
> 1.8.1.4
>=20

^ permalink raw reply

* Re: MPC8315 reboot failure, lockdep splat possibly related?
From: Anthony Foiani @ 2013-08-18 19:19 UTC (permalink / raw)
  To: Bhushan Bharat-R65777; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D07115911@039-SN2MPN1-013.039d.mgd.msft.net>

Bhushan Bharat-R65777 <R65777@freescale.com> writes:

> Anthony, I would prefer if you can send the patch (In case not then
> let me know)

Sure, I'll get it sent in a few minutes.

Would you like me to put an Acked-by on it for you?

Best regards,
Anthony Foiani

^ permalink raw reply

* Re: [RFC PATCH 1/1] powerpc/embedded6xx: Add support for Motorola/Emerson MVME5100.
From: Stephen N Chivers @ 2013-08-18 20:58 UTC (permalink / raw)
  To: Scott Wood; +Cc: Chris Proctor, linuxppc-dev, paulus, Stephen N Chivers
In-Reply-To: <1376345070.20487.41.camel@snotra.buserror.net>

Scott Wood <scottwood@freescale.com> wrote on 08/13/2013 08:04:30 AM:

> From: Scott Wood <scottwood@freescale.com>
> To: Stephen N Chivers <schivers@csc.com.au>
> Cc: Chris Proctor <cproctor@csc.com.au>, Kumar Gala 
> <galak@kernel.crashing.org>, <linuxppc-dev@lists.ozlabs.org>, 
> <paulus@samba.org>
> Date: 08/13/2013 08:04 AM
> Subject: Re: [RFC PATCH 1/1] powerpc/embedded6xx: Add support for 
> Motorola/Emerson MVME5100.
> 
> On Tue, 2013-08-13 at 08:57 +1100, Stephen N Chivers wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 08/09/2013 04:07:24 AM:
> > 
> > > From: Scott Wood <scottwood@freescale.com>
> > > To: Kumar Gala <galak@kernel.crashing.org>
> > > Cc: Stephen N Chivers <schivers@csc.com.au>, <paulus@samba.org>, 
> > > <linuxppc-dev@lists.ozlabs.org>, Chris Proctor <cproctor@csc.com.au>
> > > Date: 08/09/2013 04:08 AM
> > > Subject: Re: [RFC PATCH 1/1] powerpc/embedded6xx: Add support for 
> > > Motorola/Emerson MVME5100.
> > > 
> > > On Thu, 2013-08-08 at 10:30 -0500, Kumar Gala wrote:
> > > > Also, we don't take full defconfigs in the tree, look at 'make 
> > > savedefconfig'
> > > 
> > > Why does this board need its own defconfig at all?  Just add it to
> > > ppc6xx_defconfig.
> > 
> > The boards firmware as stated above is PPCBug. PPCBug is not Open 
Firmware
> > compatible.
> > 
> > There is no U-Boot support for the board.
> 
> I'm not sure why that precludes the use of ppc6xx_defconfig...
> 
> > In ppc6xx_defconfig the e100 network device and NFS support is modular 
and
> > that forces the use of an initrd.
> 
> OK, then let's enable those in ppc6xx_defconfig.  We do in a lot of the
> other defconfigs (such as 85xx and derivatives).
> 
> > PPCBugs network boot command takes only one file argument and so makes
> > it difficult to use an initrd.
> > 
> > So the choices are:
> >         - Providing a defconfig for the board,
> >         - Building the kernel with the initrd embedded in it,
> >         - Finding an alternative boot loader,
> >         - Doing the board support for U-Boot.
> > The first choice is simple and documents the configuration of the
> > board without mixing it into ppc6xx_defconfig.
> 
> The first choice means your board would get much less build-testing, and
> would increase maintenance costs of defconfig changes (and/or lead to
> divergence from other defconfigs that doesn't actually have anything to
> do with this board or its use cases).
> 
> The fifth choice is "add what you need to ppc6xx_defconfig".  Within
> reason of course, but I think a common PCI network device and NFS root
> support is reasonable.
Thanks for the suggestion.

I have tried it and ppc6xx_defconfig does generate a viable kernel for
the MVME5100 but there is a total lack of serial console messages.

The reason is due to the requirement for a register shift to access the
UART register(s) on (via) the HAWK ASIC. The HAWK ASIC is very likely to
be a MPC107/TSI107, making it compatible with "tsi-bridge". The TSI
bridges also require register shifts to access their "built-in" UARTS.

I will check the MPC107 data sheet against the TSI107 datasheet
and the MVME5100 programmers manual later today.

The serial console setup in 'legacy_serial' does not support reg-shift
or reg-offset properties and so a preferred_console is not added by it.
The DTS file for the board does specify the register shift so that
'of_serial' will correctly setup the UARTS. But that is too late, the
preferred console will be tty0 as the .config derived from 
ppc6xx_defconfig
has CONF_VT_CONSOLE set. In the test kernels I have built 'con_init' in
the VT support is always called before serial8250_console_init.

I did try modifying legacy_serial to correctly support the tsi-bridge
UARTS and add the preferred console but encountered even more problems
in 8250_core when registering the serial console (to the point of
having a silently panic'ing kernel).

So I think for the moment, the board will need its own default config.

I think similar problems would occur for the Holly and MPC7448_HPC2 boards
as these also have tsi-bridges, but their dts files do not specify the
reg-shift properties. This begs the question do the serial consoles
for Holly and MPC7448_HPC2 actually work?
> 
> -Scott
> 
> 
> 

^ permalink raw reply

* Re: [PATCH 2/2] powerpc/iommu: check dev->iommu_group before remove a device from iommu_group
From: Wei Yang @ 2013-08-19  1:29 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: paulus, benh, linuxppc-dev, linux-kernel
In-Reply-To: <520DFBC8.4040509@ozlabs.ru>

On Fri, Aug 16, 2013 at 08:15:36PM +1000, Alexey Kardashevskiy wrote:
>On 08/16/2013 08:08 PM, Wei Yang wrote:
>> ---
>>  arch/powerpc/kernel/iommu.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>> 
>> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
>> index b20ff17..5abf7c3 100644
>> --- a/arch/powerpc/kernel/iommu.c
>> +++ b/arch/powerpc/kernel/iommu.c
>> @@ -1149,7 +1149,8 @@ static int iommu_bus_notifier(struct notifier_block *nb,
>>  	case BUS_NOTIFY_ADD_DEVICE:
>>  		return iommu_add_device(dev);
>>  	case BUS_NOTIFY_DEL_DEVICE:
>> -		iommu_del_device(dev);
>> +		if (dev->iommu_group)
>> +			iommu_del_device(dev);
>>  		return 0;
>>  	default:
>>  		return 0;
>> 
>
>This one seems redundant, no?

Sorry for the late.

Yes, these two patches have the same purpose to guard the system, while in two
different places.  One is in powernv platform, the other is in the generic iommu 
driver.

The one in powernv platform is used to correct the original logic.

The one in generic iommu driver is to keep system safe in case other platform to
call iommu_group_remove_device() without the check.

>
>
>-- 
>Alexey

-- 
Richard Yang
Help you, Help me

^ permalink raw reply

* Re: [PATCH 2/2] powerpc/iommu: check dev->iommu_group before remove a device from iommu_group
From: Alexey Kardashevskiy @ 2013-08-19  1:39 UTC (permalink / raw)
  To: Wei Yang; +Cc: paulus, benh, linuxppc-dev, linux-kernel
In-Reply-To: <20130819012945.GA8342@weiyang.vnet.ibm.com>

On 08/19/2013 11:29 AM, Wei Yang wrote:
> On Fri, Aug 16, 2013 at 08:15:36PM +1000, Alexey Kardashevskiy wrote:
>> On 08/16/2013 08:08 PM, Wei Yang wrote:
>>> ---
>>>  arch/powerpc/kernel/iommu.c |    3 ++-
>>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
>>> index b20ff17..5abf7c3 100644
>>> --- a/arch/powerpc/kernel/iommu.c
>>> +++ b/arch/powerpc/kernel/iommu.c
>>> @@ -1149,7 +1149,8 @@ static int iommu_bus_notifier(struct notifier_block *nb,
>>>  	case BUS_NOTIFY_ADD_DEVICE:
>>>  		return iommu_add_device(dev);
>>>  	case BUS_NOTIFY_DEL_DEVICE:
>>> -		iommu_del_device(dev);
>>> +		if (dev->iommu_group)
>>> +			iommu_del_device(dev);
>>>  		return 0;
>>>  	default:
>>>  		return 0;
>>>
>>
>> This one seems redundant, no?
> 
> Sorry for the late.
> 
> Yes, these two patches have the same purpose to guard the system, while in two
> different places.  One is in powernv platform, the other is in the generic iommu 
> driver.
> 
> The one in powernv platform is used to correct the original logic.
> 
> The one in generic iommu driver is to keep system safe in case other platform to
> call iommu_group_remove_device() without the check.


But I am moving bus notifier to powernv code (posted a patch last week,
otherwise Freescale's IOMMU conflicted) so this won't be the case.



-- 
Alexey

^ permalink raw reply

* Re: [PATCH 2/2] powerpc/iommu: check dev->iommu_group before remove a device from iommu_group
From: Wei Yang @ 2013-08-19  1:55 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: paulus, benh, linuxppc-dev, linux-kernel
In-Reply-To: <52117765.7010205@ozlabs.ru>

On Mon, Aug 19, 2013 at 11:39:49AM +1000, Alexey Kardashevskiy wrote:
>On 08/19/2013 11:29 AM, Wei Yang wrote:
>> On Fri, Aug 16, 2013 at 08:15:36PM +1000, Alexey Kardashevskiy wrote:
>>> On 08/16/2013 08:08 PM, Wei Yang wrote:
>>>> ---
>>>>  arch/powerpc/kernel/iommu.c |    3 ++-
>>>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
>>>> index b20ff17..5abf7c3 100644
>>>> --- a/arch/powerpc/kernel/iommu.c
>>>> +++ b/arch/powerpc/kernel/iommu.c
>>>> @@ -1149,7 +1149,8 @@ static int iommu_bus_notifier(struct notifier_block *nb,
>>>>  	case BUS_NOTIFY_ADD_DEVICE:
>>>>  		return iommu_add_device(dev);
>>>>  	case BUS_NOTIFY_DEL_DEVICE:
>>>> -		iommu_del_device(dev);
>>>> +		if (dev->iommu_group)
>>>> +			iommu_del_device(dev);
>>>>  		return 0;
>>>>  	default:
>>>>  		return 0;
>>>>
>>>
>>> This one seems redundant, no?
>> 
>> Sorry for the late.
>> 
>> Yes, these two patches have the same purpose to guard the system, while in two
>> different places.  One is in powernv platform, the other is in the generic iommu 
>> driver.
>> 
>> The one in powernv platform is used to correct the original logic.
>> 
>> The one in generic iommu driver is to keep system safe in case other platform to
>> call iommu_group_remove_device() without the check.
>
>
>But I am moving bus notifier to powernv code (posted a patch last week,
>otherwise Freescale's IOMMU conflicted) so this won't be the case.

Yes, I see the patch.

This means other platforms, besides powernv, will check the dev->iommu_group
before remove the device? This would be a convention?

If this is the case, the second patch is enough. We don't need to check it in
generic iommu driver.

Since I am not very familiar with the code convention, I post these two
patches together. This doesn't mean I need to push both of them. Your comments
are welcome, lets me understand which one is more suitable in this case.

>
>
>
>-- 
>Alexey

-- 
Richard Yang
Help you, Help me

^ permalink raw reply

* RE: [PATCH 1/2] powerpc/85xx: add hardware automatically enter altivec idle state
From: Wang Dongsheng-B40534 @ 2013-08-19  2:53 UTC (permalink / raw)
  To: Wood Scott-B07421, Kumar Gala; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1376671853.31636.252.camel@snotra.buserror.net>

VGhhbmtzIGZvciB5b3VyIGZlZWRiYWNrLg0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0t
DQo+IEZyb206IFdvb2QgU2NvdHQtQjA3NDIxDQo+IFNlbnQ6IFNhdHVyZGF5LCBBdWd1c3QgMTcs
IDIwMTMgMTI6NTEgQU0NCj4gVG86IEt1bWFyIEdhbGENCj4gQ2M6IFdhbmcgRG9uZ3NoZW5nLUI0
MDUzNDsgbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRD
SCAxLzJdIHBvd2VycGMvODV4eDogYWRkIGhhcmR3YXJlIGF1dG9tYXRpY2FsbHkgZW50ZXINCj4g
YWx0aXZlYyBpZGxlIHN0YXRlDQo+IA0KPiBPbiBGcmksIDIwMTMtMDgtMTYgYXQgMDY6MDIgLTA1
MDAsIEt1bWFyIEdhbGEgd3JvdGU6DQo+ID4gT24gQXVnIDE2LCAyMDEzLCBhdCAyOjIzIEFNLCBE
b25nc2hlbmcgV2FuZyB3cm90ZToNCj4gPg0KPiA+ID4gRnJvbTogV2FuZyBEb25nc2hlbmcgPGRv
bmdzaGVuZy53YW5nQGZyZWVzY2FsZS5jb20+DQo+ID4gPg0KPiA+ID4gRWFjaCBjb3JlJ3MgQWx0
aVZlYyB1bml0IG1heSBiZSBwbGFjZWQgaW50byBhIHBvd2VyIHNhdmluZ3MgbW9kZQ0KPiA+ID4g
YnkgdHVybmluZyBvZmYgcG93ZXIgdG8gdGhlIHVuaXQuIENvcmUgaGFyZHdhcmUgd2lsbCBhdXRv
bWF0aWNhbGx5DQo+ID4gPiBwb3dlciBkb3duIHRoZSBBbHRpVmVjIHVuaXQgYWZ0ZXIgbm8gQWx0
aVZlYyBpbnN0cnVjdGlvbnMgaGF2ZQ0KPiA+ID4gZXhlY3V0ZWQgaW4gTiBjeWNsZXMuIFRoZSBB
bHRpVmVjIHBvd2VyLWNvbnRyb2wgaXMgdHJpZ2dlcmVkIGJ5DQo+IGhhcmR3YXJlLg0KPiA+ID4N
Cj4gPiA+IFNpZ25lZC1vZmYtYnk6IFdhbmcgRG9uZ3NoZW5nIDxkb25nc2hlbmcud2FuZ0BmcmVl
c2NhbGUuY29tPg0KPiA+DQo+ID4gV2h5IHRyZWF0IHRoaXMgYXMgYSBpZGxlIEhXIGdvdmVybm9y
IHZzIGp1c3Qgc29tZSBvbmUgdGltZSBzZXR1cCBhdA0KPiBib290IG9mIHRoZSB0aW1lIGRlbGF5
Pw0KPiANCj4gSXQgaXMgYmVpbmcgZG9uZSBhcyBvbmUtdGltZSBzZXR1cCwgZGVzcGl0ZSB0aGUg
ZnVuY3Rpb24gbmFtZS4NCj4gDQo+IE1heWJlIGl0IHNob3VsZCBiZSBtb3ZlZCBpbnRvIF9fc2V0
dXAvcmVzdG9yZV9jcHVfZTY1MDAgKEJUVywgd2UgcmVhbGx5DQo+IHNob3VsZCByZWZhY3RvciB0
aG9zZSB0byByZWR1Y2UgZHVwbGljYXRpb24pIHdpdGggdGhlIHRpbWViYXNlIGJpdA0KPiBudW1i
ZXIgaGFyZGNvZGVkIHJhdGhlciB0aGFuIGEgdGltZSBpbiB1cy4NCj4gDQpUaGUgZnJlcXVlbmN5
IG9mIHRoZSBkaWZmZXJlbnQgcGxhdGZvcm1zIHRpbWViYXNlIGlzIG5vdCB0aGUgc2FtZS4NCklm
IHdlIHVzZSBpdCwgd2UgbmVlZCB0byBzZXQgZGlmZmVyZW50IHRpbWViYXNlIGJpdCBvbiBlYWNo
IHBsYXRmb3JtLg0KVGhhdCBpcyB3aHkgSSBkaWQgbm90IHB1dCB0aGUgY29kZSBpbiBfX3NldHVw
L3Jlc3RvcmVfY3B1X2U2NTAwLCBJIG5lZWQNCnVzZSB0Yl90aWNrc19wZXJfdXNlYyB0byBnZXQg
dGltZWJhc2UgYml0LiBTbyB3ZSBvbmx5IG5lZWQgdG8gc2V0IGEgdGltZQ0KZm9yIGVhY2ggcGxh
dGZvcm0sIGFuZCBub3Qgc2V0IGRpZmZlcmVudCB0aW1lYmFzZSBiaXQuDQoNCj4gQXMgZm9yIHRo
ZSBQVlIgY2hlY2ssIHRoZSB1cHN0cmVhbSBrZXJuZWwgZG9lc24ndCBuZWVkIHRvIGNhcmUgYWJv
dXQNCj4gcmV2MSwgc28ga25vd2luZyBpdCdzIGFuIGU2NTAwIGlzIGdvb2QgZW5vdWdoLg0KPiAN
CkJ1dCBBbHRpVmVjIGlkbGUgJiBQVzIwIGNhbm5vdCB3b3JrIG9uIHJldjEgcGxhdGZvcm0uDQpX
ZSBkaWRuJ3QgaGF2ZSB0byBkZWFsIHdpdGggaXQ/DQoNCi1kb25nc2hlbmcNCg==

^ permalink raw reply

* Re: [PATCH v6 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
From: Nicolin Chen @ 2013-08-19  3:07 UTC (permalink / raw)
  To: Bhushan Bharat-R65777
  Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	alsa-devel@alsa-project.org, lars@metafoo.de,
	swarren@wwwdotorg.org, linuxppc-dev@lists.ozlabs.org,
	s.hauer@pengutronix.de, timur@tabi.org, rob.herring@calxeda.com,
	tomasz.figa@gmail.com, broonie@kernel.org, p.zabel@pengutronix.de,
	shawn.guo@linaro.org, festevam@gmail.com
In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D07114BC3@039-SN2MPN1-013.039d.mgd.msft.net>

Hi Bhushan,

   Thank you for the comments :)
   I'll fix some in v7.

   Here is my some replies to you.

On Sat, Aug 17, 2013 at 02:24:19AM +0800, Bhushan Bharat-R65777 wrote:
> > This patch add S/PDIF controller driver for Freescale SoC.
> 
> Please give some more description of the driver?

I've referred some ASoC drivers, all of them seem to be brief as mine.
So I'm not sure what else information I should provide here. It's already
kinda okay to me.


> > +struct spdif_mixer_control {
> > +	/* buffer ptrs for writer */
> > +	u32 upos;
> > +	u32 qpos;
> 
> They does not look like pointer?

They are more like offsets to get the correspond pointer.
But I'll change the confusing comments.


> > +/* U/Q Channel receive register full */
> > +static void spdif_irq_uqrx_full(struct fsl_spdif_priv *spdif_priv, char name)
> > +{
> > +	struct spdif_mixer_control *ctrl = &spdif_priv->fsl_spdif_control;
> > +	struct regmap *regmap = spdif_priv->regmap;
> > +	struct platform_device *pdev = spdif_priv->pdev;
> > +	u32 *pos, size, val, reg;
> > +
> > +	switch (name) {
> > +	case 'U':
> > +		pos = &ctrl->upos;
> > +		size = SPDIF_UBITS_SIZE;
> > +		reg = REG_SPDIF_SRU;
> > +		break;
> > +	case 'Q':
> > +		pos = &ctrl->qpos;
> > +		size = SPDIF_QSUB_SIZE;
> > +		reg = REG_SPDIF_SRQ;
> > +		break;
> > +	default:
> > +		return;
> 
> Should return error.

IMHO, this should be fine. It's a void type function and being used
in the isr(). The params 'name' is totally controlled by driver itself,
so basically we don't need to worry about the default path.

> > +	if (*pos >= size * 2) {
> > +		*pos = 0;
> > +	} else if (unlikely((*pos % size) + 3 > size)) {
> > +		dev_err(&pdev->dev, "User bit receivce buffer overflow\n");
> > +		return;
> 
> Should return error.

Ditto, it's being used in isr(), we don't need to detect the return value,
just use dev_err() to warn users and let the driver clear the irq.


> > +/* U/Q Channel framing error */
> > +static void spdif_irq_uq_err(struct fsl_spdif_priv *spdif_priv)
> > +{
> > +	struct spdif_mixer_control *ctrl = &spdif_priv->fsl_spdif_control;
> > +	struct regmap *regmap = spdif_priv->regmap;
> > +	struct platform_device *pdev = spdif_priv->pdev;
> > +	u32 val;
> > +
> > +	dev_dbg(&pdev->dev, "isr: U/Q Channel framing error\n");
> > +
> > +	/* read U/Q data and do buffer reset */
> > +	regmap_read(regmap, REG_SPDIF_SRU, &val);
> > +	regmap_read(regmap, REG_SPDIF_SRQ, &val);
> 
> Above prints says read u/q data and buffer reset, what is buffer reset? Is that read on clear?

That's the behavior needed by IP, according to the reference manual:
"U Channel receive register full, can't be cleared with reg. IntClear.
To clear it, read from U Rx reg." and "Q Channel receive register full,
can't be cleared with reg. IntClear. To clear it, read from Q Rx reg."


> > +static void spdif_softreset(struct fsl_spdif_priv *spdif_priv)
> > +{
> > +	struct regmap *regmap = spdif_priv->regmap;
> > +	u32 val, cycle = 1000;
> > +
> > +	regmap_write(regmap, REG_SPDIF_SCR, SCR_SOFT_RESET);
> > +	regcache_sync(regmap);
> > +
> > +	/* RESET bit would be cleared after finishing its reset procedure */
> > +	do {
> > +		regmap_read(regmap, REG_SPDIF_SCR, &val);
> > +	} while ((val & SCR_SOFT_RESET) && cycle--);
> 
> What if reset is not cleared and timeout happen? 

We here suppose the reset bit would be cleared -- "The software reset
will last 8 cycles." from RM, so if this happened to be a failure, the
whole IP module won't be normally working as well.

Well, but I don't mind to put here an extra failed return to make it
clear. 


> > +static u8 reverse_bits(u8 input)
> > +{
> > +	u8 tmp = input;
> > +
> > +	tmp = ((tmp & 0b10101010) >> 1) | ((tmp << 1) & 0b10101010);
> > +	tmp = ((tmp & 0b11001100) >> 2) | ((tmp << 2) & 0b11001100);
> > +	tmp = ((tmp & 0b11110000) >> 4) | ((tmp << 4) & 0b11110000);
> 
> What is this logic, can the hardcoding be removed and some description on above calculation?

This was provided by Philipp Zabel in his review at patch v3.
It's pretty clear to me that it just reverses the bits for u8.
I don't think this logic has any problem and the mask here 
doesn't look like any hardcode to me.


> > +static bool fsl_spdif_volatile_reg(struct device *dev, unsigned int reg)
> > +{
> > +	/* Sync all registers after reset */
> 
> Where us sync :) ?

The "return true" would do that. For volatile registers, if no "return true"
here, the whole regmap would use the value in cache, while for some bits
we need to trace its true value from the physical registers not from cache.


Best regards,
Nicolin Chen

^ permalink raw reply

* RE: [PATCH] powerpc: add the missing required isync for the coherent icache flush
From: Wang Dongsheng-B40534 @ 2013-08-19  3:24 UTC (permalink / raw)
  To: Kevin Hao, Benjamin Herrenschmidt; +Cc: linuxppc
In-Reply-To: <1376567767-20620-1-git-send-email-haokexin@gmail.com>



> -----Original Message-----
> From: Linuxppc-dev [mailto:linuxppc-dev-
> bounces+b40534=3Dfreescale.com@lists.ozlabs.org] On Behalf Of Kevin Hao
> Sent: Thursday, August 15, 2013 7:56 PM
> To: Benjamin Herrenschmidt
> Cc: linuxppc
> Subject: [PATCH] powerpc: add the missing required isync for the coherent
> icache flush
>=20
> Even we don't need to flush the dcache and invalidate the icache
> on the CPU which has coherent icache. But we do need an isync to
> discard the prefetched instructions in this case.
>=20
> Signed-off-by: Kevin Hao <haokexin@gmail.com>
> ---
>  arch/powerpc/kernel/misc_32.S | 2 ++
>  arch/powerpc/kernel/misc_64.S | 1 +
>  2 files changed, 3 insertions(+)
>=20
> diff --git a/arch/powerpc/kernel/misc_32.S
> b/arch/powerpc/kernel/misc_32.S
> index 777d999..0b84c8d 100644
> --- a/arch/powerpc/kernel/misc_32.S
> +++ b/arch/powerpc/kernel/misc_32.S
> @@ -433,6 +433,7 @@ _GLOBAL(invalidate_dcache_range)
>   */
>  _GLOBAL(__flush_dcache_icache)
>  BEGIN_FTR_SECTION
> +	isync
There is not touch the icache, why need we to do this?

>  	blr
>  END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
>  	rlwinm	r3,r3,0,0,31-PAGE_SHIFT		/* Get page base address
> */
> @@ -474,6 +475,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_44x)
>   */
>  _GLOBAL(__flush_dcache_icache_phys)
>  BEGIN_FTR_SECTION
> +	isync
>  	blr					/* for 601, do nothing */
>  END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
>  	mfmsr	r10
> diff --git a/arch/powerpc/kernel/misc_64.S
> b/arch/powerpc/kernel/misc_64.S
> index 992a78e..d74fefb 100644
> --- a/arch/powerpc/kernel/misc_64.S
> +++ b/arch/powerpc/kernel/misc_64.S
> @@ -69,6 +69,7 @@ PPC64_CACHES:
>=20
>  _KPROBE(flush_icache_range)
>  BEGIN_FTR_SECTION
> +	isync
>  	blr
>  END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
>  /*
> --
> 1.8.3.1
>=20
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH] powerpc: add the missing required isync for the coherent icache flush
From: Benjamin Herrenschmidt @ 2013-08-19  3:36 UTC (permalink / raw)
  To: Wang Dongsheng-B40534; +Cc: linuxppc, Kevin Hao
In-Reply-To: <ABB05CD9C9F68C46A5CEDC7F1543925901020606@039-SN2MPN1-021.039d.mgd.msft.net>

On Mon, 2013-08-19 at 03:24 +0000, Wang Dongsheng-B40534 wrote:
> 
> > -----Original Message-----
> > From: Linuxppc-dev [mailto:linuxppc-dev-
> > bounces+b40534=freescale.com@lists.ozlabs.org] On Behalf Of Kevin Hao
> > Sent: Thursday, August 15, 2013 7:56 PM
> > To: Benjamin Herrenschmidt
> > Cc: linuxppc
> > Subject: [PATCH] powerpc: add the missing required isync for the coherent
> > icache flush
> > 
> > Even we don't need to flush the dcache and invalidate the icache
> > on the CPU which has coherent icache. But we do need an isync to
> > discard the prefetched instructions in this case.
> > 
> > Signed-off-by: Kevin Hao <haokexin@gmail.com>
> > ---
> >  arch/powerpc/kernel/misc_32.S | 2 ++
> >  arch/powerpc/kernel/misc_64.S | 1 +
> >  2 files changed, 3 insertions(+)
> > 
> > diff --git a/arch/powerpc/kernel/misc_32.S
> > b/arch/powerpc/kernel/misc_32.S
> > index 777d999..0b84c8d 100644
> > --- a/arch/powerpc/kernel/misc_32.S
> > +++ b/arch/powerpc/kernel/misc_32.S
> > @@ -433,6 +433,7 @@ _GLOBAL(invalidate_dcache_range)
> >   */
> >  _GLOBAL(__flush_dcache_icache)
> >  BEGIN_FTR_SECTION
> > +	isync
> There is not touch the icache, why need we to do this?

The semantic of this function is to make data executable. Even if the
implementation has a snooping icache, it *still* needs to make sure
prefetched code is tossed out of the pipeline which is what isync
should provide.

The architecture actually specifies that.

Cheers,
Ben.

> >  	blr
> >  END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
> >  	rlwinm	r3,r3,0,0,31-PAGE_SHIFT		/* Get page base address
> > */
> > @@ -474,6 +475,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_44x)
> >   */
> >  _GLOBAL(__flush_dcache_icache_phys)
> >  BEGIN_FTR_SECTION
> > +	isync
> >  	blr					/* for 601, do nothing */
> >  END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
> >  	mfmsr	r10
> > diff --git a/arch/powerpc/kernel/misc_64.S
> > b/arch/powerpc/kernel/misc_64.S
> > index 992a78e..d74fefb 100644
> > --- a/arch/powerpc/kernel/misc_64.S
> > +++ b/arch/powerpc/kernel/misc_64.S
> > @@ -69,6 +69,7 @@ PPC64_CACHES:
> > 
> >  _KPROBE(flush_icache_range)
> >  BEGIN_FTR_SECTION
> > +	isync
> >  	blr
> >  END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
> >  /*
> > --
> > 1.8.3.1
> > 
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/linuxppc-dev
> 

^ permalink raw reply

* Re: [PATCH] powerpc: add the missing required isync for the coherent icache flush
From: Benjamin Herrenschmidt @ 2013-08-19  3:37 UTC (permalink / raw)
  To: Wang Dongsheng-B40534; +Cc: linuxppc, Kevin Hao
In-Reply-To: <1376883397.25016.60.camel@pasglop>

On Mon, 2013-08-19 at 13:36 +1000, Benjamin Herrenschmidt wrote:

> The semantic of this function is to make data executable. Even if the
> implementation has a snooping icache, it *still* needs to make sure
> prefetched code is tossed out of the pipeline which is what isync
> should provide.
> 
> The architecture actually specifies that.

In fact, on P5 and later, I think we are supposed to do a single dummy
icbi followed by sync and isync.

Cheers,
Ben.

> Cheers,
> Ben.
> 
> > >  	blr
> > >  END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
> > >  	rlwinm	r3,r3,0,0,31-PAGE_SHIFT		/* Get page base address
> > > */
> > > @@ -474,6 +475,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_44x)
> > >   */
> > >  _GLOBAL(__flush_dcache_icache_phys)
> > >  BEGIN_FTR_SECTION
> > > +	isync
> > >  	blr					/* for 601, do nothing */
> > >  END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
> > >  	mfmsr	r10
> > > diff --git a/arch/powerpc/kernel/misc_64.S
> > > b/arch/powerpc/kernel/misc_64.S
> > > index 992a78e..d74fefb 100644
> > > --- a/arch/powerpc/kernel/misc_64.S
> > > +++ b/arch/powerpc/kernel/misc_64.S
> > > @@ -69,6 +69,7 @@ PPC64_CACHES:
> > > 
> > >  _KPROBE(flush_icache_range)
> > >  BEGIN_FTR_SECTION
> > > +	isync
> > >  	blr
> > >  END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
> > >  /*
> > > --
> > > 1.8.3.1
> > > 
> > > _______________________________________________
> > > Linuxppc-dev mailing list
> > > Linuxppc-dev@lists.ozlabs.org
> > > https://lists.ozlabs.org/listinfo/linuxppc-dev
> > 
> 

^ permalink raw reply

* [RFC PATCH V3 0/5] powerpc/cpuidle: Generic POWERPC cpuidle driver enabled for POWER and POWERNV platforms
From: Deepthi Dharwar @ 2013-08-19  4:27 UTC (permalink / raw)
  To: benh, daniel.lezcano, kernel, scottwood, linux-pm, linuxppc-dev
  Cc: preeti, dongsheng.wang

This patch series consolidates the backend cpuidle driver for pseries
and powernv platforms and also enables the new drivers/cpuidle/cpuidle-powerpc.c 
to include other powerpc drivers with minimal code duplication.

Current existing backend driver for pseries has been moved to drivers/cpuidle 
and has been extended to accommodate powernv idle power mgmt states. 
As seen in V1 of this patch series, having a separate powernv backend driver 
results in too much code duplication, which is less elegant and can pose 
maintenance problems going further.

Using the cpuidle framework to exploit platform low power idle states
management can take advantage of advanced heuristics, tunables and features 
provided by framework. The statistics and tracing infrastructure provided 
by the cpuidle framework also helps in enabling power management 
related tools and help tune the system and applications.

Earlier in 3.3 kernel, pseries idle state management was modified to 
exploit the cpuidle framework and the end goal of this patch is to have powernv
platform also to hook its idle states into cpuidle framework with minimal 
code duplication between both platforms. This result is a generic powerpc 
backend driver currently enabled for pseries and powernv platforms and which
can be extended to accommodate other powerpc archs in the future.

This series aims to maintain compatibility and functionality to existing pseries 
and powernv idle cpu management code. There are no new functions or idle 
states added as part of this series. This can be extended by adding more 
states to this existing framework.

With this patch series, the powernv cpuidle functionalities are on-par with 
pSeries idle management. Other POWERPC platforms can use this patch series 
to exploit the CPUIDLE framework.

This patch mainly focus on an integrated CPUIDLE backend driver for POWERPC. 
Minor cpuidle clean-ups including a generic hotplug cpuidle notifier, 
using cpuidle_register calls will be taken up going further.

V1 -> http://lkml.org/lkml/2013/7/23/143
V2 -> https://lkml.org/lkml/2013/7/30/872

Changes from V2:
===============

* This patch series does not include smt-snooze-delay fixes. 
  This will be taken up later on.

* Integrated POWERPC driver in drivers/cpuidle.
  Enabled for all of POWERPC platform. 
  Currently has PSERIES and POWERNV support.
  No compile time flags in .c file. This  will be 
  one consolidated binary that does a run time 
  detection based on platform and take decisions 
  accordingly.

 Deepthi Dharwar (5):
      pseries/cpuidle: Remove dependency of pseries.h file
      pseries: Move plpar_wrapper.h to powerpc common include/asm location.
      powerpc/cpuidle: Generic powerpc backend cpuidle driver.
      powerpc/cpuidle: Enable powernv cpuidle support.
      powernv/cpuidle: Enable idle powernv cpu to call into the cpuidle framework.


 arch/powerpc/include/asm/paca.h                 |   23 +
 arch/powerpc/include/asm/plpar_wrappers.h       |  325 +++++++++++++++++++
 arch/powerpc/include/asm/processor.h            |    2 
 arch/powerpc/platforms/powernv/setup.c          |   14 +
 arch/powerpc/platforms/pseries/Kconfig          |    9 -
 arch/powerpc/platforms/pseries/Makefile         |    1 
 arch/powerpc/platforms/pseries/cmm.c            |    3 
 arch/powerpc/platforms/pseries/dtl.c            |    3 
 arch/powerpc/platforms/pseries/hotplug-cpu.c    |    3 
 arch/powerpc/platforms/pseries/hvconsole.c      |    2 
 arch/powerpc/platforms/pseries/iommu.c          |    3 
 arch/powerpc/platforms/pseries/kexec.c          |    2 
 arch/powerpc/platforms/pseries/lpar.c           |    2 
 arch/powerpc/platforms/pseries/plpar_wrappers.h |  324 -------------------
 arch/powerpc/platforms/pseries/processor_idle.c |  362 ---------------------
 arch/powerpc/platforms/pseries/pseries.h        |    3 
 arch/powerpc/platforms/pseries/setup.c          |    2 
 arch/powerpc/platforms/pseries/smp.c            |    2 
 drivers/cpuidle/Kconfig                         |    7 
 drivers/cpuidle/Makefile                        |    2 
 drivers/cpuidle/cpuidle-powerpc.c               |  391 +++++++++++++++++++++++
 21 files changed, 772 insertions(+), 713 deletions(-)
 create mode 100644 arch/powerpc/include/asm/plpar_wrappers.h
 delete mode 100644 arch/powerpc/platforms/pseries/plpar_wrappers.h
 delete mode 100644 arch/powerpc/platforms/pseries/processor_idle.c
 create mode 100644 drivers/cpuidle/cpuidle-powerpc.c


-- Deepthi

^ permalink raw reply


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