Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module

2014-09-10 Thread Markus Pargmann
On Wed, Sep 10, 2014 at 10:42:04AM -0700, Nicolin Chen wrote:
> On Wed, Sep 10, 2014 at 04:12:53PM +0800, Shengjiu Wang wrote:
> > > Then we can get a patch like:
> > > open() {
> > > + clk_prepare_enable();
> > >   
> > > }
> > > 
> > > close() {
> > >   
> > > + clk_disable_unprepare()
> > > }
> 
> > what is the open() and close()? do you mean the fsl_ssi_startup()
> > and fsl_ssi_shutdown()?
> 
> Yea.
> 
> > > probe() {
> > >   clk_get();
> > >   clk_prepare_enable();
> > >   
> > >   if (xxx)
> > > - goto err_xx;
> > > + return ret;
> > >   
> > > + clk_disable_unprepare();
> > >   return 0;
> > > -err_xx:
> > > - clk_disable_unprepare()
> > > }
> 
> > If this probe() is fsl_ssi_imx_probe(), I think no need to add
> > clk_prepare_enable() or clk_disable_unprepare(), seems there is no 
> > registers accessing in this probe.
> 
> This is trying to be safe, especially for such a driver being used
> by multiple platforms. You can omit this as long as the patch can
> pass the test on old imx, PowerPC, and AC97 platforms.
> 
> >
> 
> And another risk just came to my mind is that there would be a
> possibility that a machine driver would call set_dai_fmt() early,
> after SSI's probe() and before SSI's startup(), if the machine
> driver contains dai_fmt assignment in its probe(). Then, without
> regmap_mmio_clk(), it'll be tough for us over here because we may
> also need to add clock enable/disable for set_dai_fmt/set_sysclk(),
> even if there might be still tiny risk that we missed something.

Thanks, didn't thought about that. As there are no restrictions on when
these functions may be called, it has to be handled.

> 
> Then there could be a selfish approach to circumvent it is to use
> regmap_mmio_clk() with "ipg" at the beginning and call regmap_mmio()
> without "ipg" if getting a failed return value from regmap_mmio_clk,
> and meanwhile to keep the clock always enabled for the regmap_mmio()
> case just like what the current driver is doing. This may result
> those non-ipg-clk platforms can't benefit from this refinement
> unless they update their DT bindings -- use "ipg" for core clock
> This might be the safest and simplest way for us, I'm not sure
> everyone would be comfortable with this idea though.

I like the "selfish" approach. It would save a lot of clock
enabling/disabling and error handling and at the same time it doesn't break
the DT compatibility. The platforms with an old DT would have the old
behaviour, but that could be changed by updating the devicetrees which
should be easy to do for all the imx SoCs.

Best regards,

Markus

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V3 0/3] Add new PowerPC specific ELF core notes

2014-09-10 Thread Anshuman Khandual
On 05/23/2014 08:45 PM, Anshuman Khandual wrote:
>   This patch series adds five new ELF core note sections which can be
> used with existing ptrace request PTRACE_GETREGSET/SETREGSET for accessing
> various transactional memory and miscellaneous register sets on PowerPC
> platform. Please find a test program exploiting these new ELF core note
> types on a POWER8 system.
> 
> RFC: https://lkml.org/lkml/2014/4/1/292
> V1:  https://lkml.org/lkml/2014/4/2/43
> V2:  https://lkml.org/lkml/2014/5/5/88
> 
> Changes in V3
> =
> (1) Added two new error paths in every TM related get/set functions when 
> regset
> support is not present on the system (ENODEV) or when the process does not
> have any transaction active (ENODATA) in the context
> 
> (2) Installed the active hooks for all the newly added regset core note types
> 
> Changes in V2
> =
> (1) Removed all the power specific ptrace requests corresponding to new 
> NT_PPC_*
> elf core note types. Now all the register sets can be accessed from ptrace
> through PTRACE_GETREGSET/PTRACE_SETREGSET using the individual NT_PPC* 
> core
> note type instead
> (2) Fixed couple of attribute values for REGSET_TM_CGPR register set
> (3) Renamed flush_tmreg_to_thread as flush_tmregs_to_thread
> (4) Fixed 32 bit checkpointed GPR support
> (5) Changed commit messages accordingly
> 
> Outstanding Issues
> ==
> (1) Running DSCR register value inside a transaction does not seem to be saved
> at thread.dscr when the process stops for ptrace examination.

Hey Sam and Suka,

Thanks for reviewing this patch series. I was busy with some other work
for last couple of months. Went through your comments, will get back to
this patch series in some time and work on the comments.

Thanks again.

Regards
Anshuman

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH V2] ASoC: fsl_ssi: refine ipg clock usage in this module

2014-09-10 Thread Shengjiu Wang
Move the ipg clock enable and disable operation to startup and shutdown,
that is only enable ipg clock when ssi is working. Keep clock is disabled
when ssi is in idle.
otherwise, _fsl_ssi_set_dai_fmt function need to be called in probe,
so add ipg clock control for it.

Signed-off-by: Shengjiu Wang 
---

change log
v2: update patch according to maintainer's review comments.

 sound/soc/fsl/fsl_ssi.c |   39 +++
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 2fc3e66..4447f95 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -530,6 +530,11 @@ static int fsl_ssi_startup(struct snd_pcm_substream 
*substream,
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct fsl_ssi_private *ssi_private =
snd_soc_dai_get_drvdata(rtd->cpu_dai);
+   int ret;
+
+   ret = clk_prepare_enable(ssi_private->clk);
+   if (ret)
+   return ret;
 
/* When using dual fifo mode, it is safer to ensure an even period
 * size. If appearing to an odd number while DMA always starts its
@@ -544,6 +549,21 @@ static int fsl_ssi_startup(struct snd_pcm_substream 
*substream,
 }
 
 /**
+ * fsl_ssi_shutdown: shutdown the SSI
+ *
+ */
+static void fsl_ssi_shutdown(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+   struct snd_soc_pcm_runtime *rtd = substream->private_data;
+   struct fsl_ssi_private *ssi_private =
+   snd_soc_dai_get_drvdata(rtd->cpu_dai);
+
+   clk_disable_unprepare(ssi_private->clk);
+
+}
+
+/**
  * fsl_ssi_set_bclk - configure Digital Audio Interface bit clock
  *
  * Note: This function can be only called when using SSI as DAI master
@@ -771,6 +791,7 @@ static int _fsl_ssi_set_dai_fmt(struct fsl_ssi_private 
*ssi_private,
struct regmap *regs = ssi_private->regs;
u32 strcr = 0, stcr, srcr, scr, mask;
u8 wm;
+   int ret;
 
ssi_private->dai_fmt = fmt;
 
@@ -779,6 +800,10 @@ static int _fsl_ssi_set_dai_fmt(struct fsl_ssi_private 
*ssi_private,
return -EINVAL;
}
 
+   ret = clk_prepare_enable(ssi_private->clk);
+   if (ret)
+   return ret;
+
fsl_ssi_setup_reg_vals(ssi_private);
 
regmap_read(regs, CCSR_SSI_SCR, &scr);
@@ -811,6 +836,7 @@ static int _fsl_ssi_set_dai_fmt(struct fsl_ssi_private 
*ssi_private,
ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_SLAVE;
break;
default:
+   clk_disable_unprepare(ssi_private->clk);
return -EINVAL;
}
 
@@ -836,6 +862,7 @@ static int _fsl_ssi_set_dai_fmt(struct fsl_ssi_private 
*ssi_private,
ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_NORMAL;
break;
default:
+   clk_disable_unprepare(ssi_private->clk);
return -EINVAL;
}
scr |= ssi_private->i2s_mode;
@@ -859,6 +886,7 @@ static int _fsl_ssi_set_dai_fmt(struct fsl_ssi_private 
*ssi_private,
strcr ^= CCSR_SSI_STCR_TFSI;
break;
default:
+   clk_disable_unprepare(ssi_private->clk);
return -EINVAL;
}
 
@@ -877,6 +905,7 @@ static int _fsl_ssi_set_dai_fmt(struct fsl_ssi_private 
*ssi_private,
scr &= ~CCSR_SSI_SCR_SYS_CLK_EN;
break;
default:
+   clk_disable_unprepare(ssi_private->clk);
return -EINVAL;
}
 
@@ -925,6 +954,7 @@ static int _fsl_ssi_set_dai_fmt(struct fsl_ssi_private 
*ssi_private,
if (fmt & SND_SOC_DAIFMT_AC97)
fsl_ssi_setup_ac97(ssi_private);
 
+   clk_disable_unprepare(ssi_private->clk);
return 0;
 
 }
@@ -1043,6 +1073,7 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai)
 
 static const struct snd_soc_dai_ops fsl_ssi_dai_ops = {
.startup= fsl_ssi_startup,
+   .shutdown   = fsl_ssi_shutdown,
.hw_params  = fsl_ssi_hw_params,
.hw_free= fsl_ssi_hw_free,
.set_fmt= fsl_ssi_set_dai_fmt,
@@ -1175,12 +1206,6 @@ static int fsl_ssi_imx_probe(struct platform_device 
*pdev,
return ret;
}
 
-   ret = clk_prepare_enable(ssi_private->clk);
-   if (ret) {
-   dev_err(&pdev->dev, "clk_prepare_enable failed: %d\n", ret);
-   return ret;
-   }
-
/* For those SLAVE implementations, we ingore non-baudclk cases
 * and, instead, abandon MASTER mode that needs baud clock.
 */
@@ -1236,7 +1261,6 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev,
return 0;
 
 error_pcm:
-   clk_disable_unprepare(ssi_private->clk);
 
return ret;
 }
@@ -1246,7 +1270,6 @@ static void fsl_ssi_imx_clean(struct platform_device 
*pdev,
 {
   

[git pull] Please pull mpe.git for-linus branch (for powerpc)

2014-09-10 Thread Michael Ellerman
Hi Linus,

Hopefully you saw the message from Ben adding me as a co-maintainer for powerpc:

  
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=ea668936b708029a0a11422ff834b651ac434c2d


Ben's travelling so this is my first attempt at a pull request.

There's nothing too exciting. The CONFIG_FHANDLE one is annoying, I know you
love defconfig changes. But we've had a couple of developers waste time
debugging boxes that wouldn't boot, only to realise it's just that systemd
needs CONFIG_FHANDLE and our defconfigs don't have it.

The new syscalls seem to be working, I've run the selftests that exist, and
also let trinity bash on them for a while.


The following changes since commit 2ce7598c9a453e0acd0e07be7be3f5eb39608ebd:

  Linux 3.17-rc4 (2014-09-07 16:09:43 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mpe/linux.git for-linus

for you to fetch changes up to 7d59deb50aafbdc01b52aed209d202d827261cb0:

  powerpc: Wire up sys_seccomp(), sys_getrandom() and sys_memfd_create() 
(2014-09-09 19:02:47 +1000)


Anton Blanchard (1):
  powerpc/perf: Fix ABIv2 kernel backtraces

Cyril Bur (1):
  powerpc: Make CONFIG_FHANDLE=y for all 64 bit powerpc defconfigs

Li Zhong (1):
  powerpc: use machine_subsys_initcall() for opal_hmi_handler_init()

Pranith Kumar (1):
  powerpc: Wire up sys_seccomp(), sys_getrandom() and sys_memfd_create()

Thomas Falcon (1):
  powerpc/pseries: Fix endian issues in memory hotplug

 arch/powerpc/configs/cell_defconfig |  1 +
 arch/powerpc/configs/celleb_defconfig   |  1 +
 arch/powerpc/configs/corenet64_smp_defconfig|  1 +
 arch/powerpc/configs/g5_defconfig   |  1 +
 arch/powerpc/configs/maple_defconfig|  1 +
 arch/powerpc/configs/pasemi_defconfig   |  1 +
 arch/powerpc/configs/ppc64_defconfig|  1 +
 arch/powerpc/configs/ppc64e_defconfig   |  1 +
 arch/powerpc/configs/ps3_defconfig  |  1 +
 arch/powerpc/configs/pseries_defconfig  |  1 +
 arch/powerpc/configs/pseries_le_defconfig   |  1 +
 arch/powerpc/include/asm/ptrace.h   |  7 +++
 arch/powerpc/include/asm/systbl.h   |  3 +++
 arch/powerpc/include/asm/unistd.h   |  2 +-
 arch/powerpc/include/uapi/asm/unistd.h  |  3 +++
 arch/powerpc/perf/callchain.c   |  2 +-
 arch/powerpc/platforms/powernv/opal-hmi.c   |  3 ++-
 arch/powerpc/platforms/pseries/hotplug-memory.c | 36 
+++-
 18 files changed, 47 insertions(+), 20 deletions(-)





signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH V2] powerpc/eeh: Fix kernel crash when passing through VF

2014-09-10 Thread Wei Yang
When doing vfio passthrough a VF, the kernel will crash with following
message:

[  442.656459] Unable to handle kernel paging request for data at address 
0x0060
[  442.656593] Faulting instruction address: 0xc0038b88
[  442.656706] Oops: Kernel access of bad area, sig: 11 [#1]
[  442.656798] SMP NR_CPUS=1024 NUMA PowerNV
[  442.656890] Modules linked in: vfio_pci mlx4_core nf_conntrack_netbios_ns 
nf_conntrack_broadcast ipt_MASQUERADE ip6t_REJECT xt_conntrack bnep bluetooth 
rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables 
ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle 
ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat 
nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle 
iptable_security iptable_raw tg3 nfsd be2net nfs_acl ses lockd ptp enclosure 
pps_core kvm_hv kvm_pr shpchp binfmt_misc kvm sunrpc uinput lpfc 
scsi_transport_fc ipr scsi_tgt [last unloaded: mlx4_core]
[  442.658152] CPU: 40 PID: 14948 Comm: qemu-system-ppc Not tainted 
3.10.42yw-pkvm+ #37
[  442.658219] task: c00f7e2a9a00 ti: c00f6dc3c000 task.ti: 
c00f6dc3c000
[  442.658287] NIP: c0038b88 LR: c04435a8 CTR: c0455bc0
[  442.658352] REGS: c00f6dc3f580 TRAP: 0300   Not tainted  
(3.10.42yw-pkvm+)
[  442.658419] MSR: 90009032   CR: 28004882  XER: 
2000
[  442.658577] CFAR: c000908c DAR: 0060 DSISR: 4000 
SOFTE: 1
GPR00: c04435a8 c00f6dc3f800 c12b1c10 cda24000
GPR04: 0003 1004 15b3 
GPR08: c127f5d8   
GPR12: c0068078 cfdd6800 01003c320c80 01003c3607f0
GPR16: 0001 105480c8 1055aaa8 01003c31ab18
GPR20: 01003c10fb40 01003c360ae8 1063bcf0 1063bdb0
GPR24: 01003c15ed70 10548f40 c01fe5514c88 c01fe5514cb0
GPR28: cda24000  cda24000 0003
[  442.659471] NIP [c0038b88] .pcibios_set_pcie_reset_state+0x28/0x130
[  442.659530] LR [c04435a8] .pci_set_pcie_reset_state+0x28/0x40
[  442.659585] Call Trace:
[  442.659610] [c00f6dc3f800] [000719e0] 0x719e0 (unreliable)
[  442.659677] [c00f6dc3f880] [c04435a8] 
.pci_set_pcie_reset_state+0x28/0x40
[  442.659757] [c00f6dc3f900] [c0455bf8] 
.reset_fundamental+0x38/0x80
[  442.659835] [c00f6dc3f980] [c04562a8] 
.pci_dev_specific_reset+0xa8/0xf0
[  442.659913] [c00f6dc3fa00] [c04448c4] .__pci_dev_reset+0x44/0x430
[  442.659980] [c00f6dc3fab0] [c0444d5c] 
.pci_reset_function+0x7c/0xc0
[  442.660059] [c00f6dc3fb30] [d0001c141ab8] .vfio_pci_open+0xe8/0x2b0 
[vfio_pci]
[  442.660139] [c00f6dc3fbd0] [c0586c30] 
.vfio_group_fops_unl_ioctl+0x3a0/0x630
[  442.660219] [c00f6dc3fc90] [c0255fbc] .do_vfs_ioctl+0x4ec/0x7c0
[  442.660286] [c00f6dc3fd80] [c0256364] .SyS_ioctl+0xd4/0xf0
[  442.660354] [c00f6dc3fe30] [c0009e54] syscall_exit+0x0/0x98
[  442.660420] Instruction dump:
[  442.660454] 4bfffce9 4bfffee4 7c0802a6 fbc1fff0 fbe1fff8 f8010010 f821ff81 
7c7e1b78
[  442.660566] 7c9f2378 6000 6000 e93e02c8  2fa3 41de00c4 
2b9f0002
[  442.660679] ---[ end trace a64ac9546bcf0328 ]---
[  442.660724]

The reason is current VF is not EEH enabled.

This patch is a quick fix for this problem.

Signed-off-by: Wei Yang 
Acked-by: Gavin Shan 

V1 -> V2: 
   1. code style and patch subject adjustment

---
 arch/powerpc/kernel/eeh.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 4a45ba8..403445e 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -625,7 +625,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
 int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state 
state)
 {
struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
-   struct eeh_pe *pe = edev->pe;
+   struct eeh_pe *pe = edev ? edev->pe : NULL;
 
if (!pe) {
pr_err("%s: No PE found on PCI device %s\n",
-- 
1.7.9.5

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 2/2] powerpc: Add smp_mb()s to arch_spin_unlock_wait()

2014-09-10 Thread Michael Ellerman
Backported from 78e05b1421fa upstream, for stable 3.14 and 3.16.

Similar to the previous commit which described why we need to add a
barrier to arch_spin_is_locked(), we have a similar problem with
spin_unlock_wait().

We need a barrier on entry to ensure any spinlock we have previously
taken is visibly locked prior to the load of lock->slock.

It's also not clear if spin_unlock_wait() is intended to have ACQUIRE
semantics. For now be conservative and add a barrier on exit to give it
ACQUIRE semantics.

Signed-off-by: Michael Ellerman 
Signed-off-by: Benjamin Herrenschmidt 
---
 arch/powerpc/lib/locks.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/lib/locks.c b/arch/powerpc/lib/locks.c
index 0c9c8d7d0734..170a0346f756 100644
--- a/arch/powerpc/lib/locks.c
+++ b/arch/powerpc/lib/locks.c
@@ -70,12 +70,16 @@ void __rw_yield(arch_rwlock_t *rw)
 
 void arch_spin_unlock_wait(arch_spinlock_t *lock)
 {
+   smp_mb();
+
while (lock->slock) {
HMT_low();
if (SHARED_PROCESSOR)
__spin_yield(lock);
}
HMT_medium();
+
+   smp_mb();
 }
 
 EXPORT_SYMBOL(arch_spin_unlock_wait);
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 1/2] powerpc: Add smp_mb() to arch_spin_is_locked()

2014-09-10 Thread Michael Ellerman
Backported from 51d7d5205d33 upstream, for stable 3.14 and 3.16.

The kernel defines the function spin_is_locked(), which can be used to
check if a spinlock is currently locked.

Using spin_is_locked() on a lock you don't hold is obviously racy. That
is, even though you may observe that the lock is unlocked, it may become
locked at any time.

There is (at least) one exception to that, which is if two locks are
used as a pair, and the holder of each checks the status of the other
before doing any update.

Assuming *A and *B are two locks, and *COUNTER is a shared non-atomic
value:

The first CPU does:

spin_lock(*A)

if spin_is_locked(*B)
# nothing
else
smp_mb()
LOAD r = *COUNTER
r++
STORE *COUNTER = r

spin_unlock(*A)

And the second CPU does:

spin_lock(*B)

if spin_is_locked(*A)
# nothing
else
smp_mb()
LOAD r = *COUNTER
r++
STORE *COUNTER = r

spin_unlock(*B)

Although this is a strange locking construct, it should work.

It seems to be understood, but not documented, that spin_is_locked() is
not a memory barrier, so in the examples above and below the caller
inserts its own memory barrier before acting on the result of
spin_is_locked().

For now we assume spin_is_locked() is implemented as below, and we break
it out in our examples:

bool spin_is_locked(*LOCK) {
LOAD l = *LOCK
return l.locked
}

Our intuition is that there should be no problem even if the two code
sequences run simultaneously such as:

CPU 0   CPU 1
==
spin_lock(*A)   spin_lock(*B)
LOAD b = *B LOAD a = *A
if b.locked # true  if a.locked # true
# nothing   # nothing
spin_unlock(*A) spin_unlock(*B)

If one CPU gets the lock before the other then it will do the update and
the other CPU will back off:

CPU 0   CPU 1
==
spin_lock(*A)
LOAD b = *B
spin_lock(*B)
if b.locked # false LOAD a = *A
elseif a.locked # true
smp_mb()# nothing
LOAD r1 = *COUNTER  spin_unlock(*B)
r1++
STORE *COUNTER = r1
spin_unlock(*A)

However in reality spin_lock() itself is not indivisible. On powerpc we
implement it as a load-and-reserve and store-conditional.

Ignoring the retry logic for the lost reservation case, it boils down to:
spin_lock(*LOCK) {
LOAD l = *LOCK
l.locked = true
STORE *LOCK = l
ACQUIRE_BARRIER
}

The ACQUIRE_BARRIER is required to give spin_lock() ACQUIRE semantics as
defined in memory-barriers.txt:

 This acts as a one-way permeable barrier.  It guarantees that all
 memory operations after the ACQUIRE operation will appear to happen
 after the ACQUIRE operation with respect to the other components of
 the system.

On modern powerpc systems we use lwsync for ACQUIRE_BARRIER. lwsync is
also know as "lightweight sync", or "sync 1".

As described in Power ISA v2.07 section B.2.1.1, in this scenario the
lwsync is not the barrier itself. It instead causes the LOAD of *LOCK to
act as the barrier, preventing any loads or stores in the locked region
from occurring prior to the load of *LOCK.

Whether this behaviour is in accordance with the definition of ACQUIRE
semantics in memory-barriers.txt is open to discussion, we may switch to
a different barrier in future.

What this means in practice is that the following can occur:

CPU 0   CPU 1
==
LOAD a = *A LOAD b = *B
a.locked = true b.locked = true
LOAD b = *B LOAD a = *A
STORE *A = aSTORE *B = b
if b.locked # false if a.locked # false
elseelse
smp_mb()smp_mb()
LOAD r1 = *COUNTER  LOAD r2 = *COUNTER
r1++r2++
STORE *COUNTER = r1
STORE *COUNTER = r2 # Lost update
spin_unlock(*A) spin_unlock(*B)

That is, the load of *B can occur prior to the store that makes *A
visibly locked. And similarly for CPU 1. The result is both CPUs hold
their lock and believe the other lock is unlocked.

The easiest fix for this is to add a full memory barrier to the start of
spin_is_locked(), so adding to our previous definition would give us:

bool spin_is_locked(*LOCK) {
smp_mb()
LOAD l = *LOCK
return l.locked
   

Re: [Xen-devel] [PATCH v1 08/21] x86/xen/MSI: Use MSI chip framework to configure MSI/MSI-X irq

2014-09-10 Thread Yijing Wang
On 2014/9/10 22:59, Konrad Rzeszutek Wilk wrote:
> On Wed, Sep 10, 2014 at 01:38:25PM +0100, David Vrabel wrote:
>> On 09/09/14 03:06, Yijing Wang wrote:
>>> On 2014/9/5 22:29, David Vrabel wrote:
 On 05/09/14 11:09, Yijing Wang wrote:
> Use MSI chip framework instead of arch MSI functions to configure
> MSI/MSI-X irq. So we can manage MSI/MSI-X irq in a unified framework.
 [...]
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
 [...]
> @@ -418,9 +430,9 @@ int __init pci_xen_init(void)
>  #endif
>  
>  #ifdef CONFIG_PCI_MSI
> - x86_msi.setup_msi_irqs = xen_setup_msi_irqs;
> - x86_msi.teardown_msi_irq = xen_teardown_msi_irq;
> - x86_msi.teardown_msi_irqs = xen_teardown_msi_irqs;
> + xen_msi_chip.setup_irqs = xen_setup_msi_irqs;
> + xen_msi_chip.teardown_irqs = xen_teardown_msi_irqs;
> + x86_msi_chip = &xen_msi_chip;
>   msi_chip.irq_mask = xen_nop_msi_mask;
>   msi_chip.irq_unmask = xen_nop_msi_mask;

 Why have these not been changed to set the x86_msi_chip.mask/unmask
 fields instead?
>>>
>>> Hi David, x86_msi_chip here is struct msi_chip data type, used to configure 
>>> MSI/MSI-X
>>> irq. msi_chip above is struct irq_chip data type, represent the MSI irq 
>>> controller. They are
>>> not the same object. Their name easily confusing people.
>>
>> Ok, it all makes sense now.
>>
>> Acked-by: David Vrabel 
> 
> You can also add 'Tested-by: Konrad Rzeszutek Wilk '
> 
> on the whole series - I ran it through on Xen and on baremetal with a mix of 
> 32/64 builds.
> 
> Oh, and also Reviewed-by: Konrad Rzeszutek Wilk  the 
> Xen parts.

Thanks very much for your test and review!

Thanks!
Yijing.

> 
>>
>> David
>>
>> ___
>> Xen-devel mailing list
>> xen-de...@lists.xen.org
>> http://lists.xen.org/xen-devel
> 
> .
> 


-- 
Thanks!
Yijing

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [Xen-devel] [PATCH v1 08/21] x86/xen/MSI: Use MSI chip framework to configure MSI/MSI-X irq

2014-09-10 Thread Yijing Wang
On 2014/9/10 20:38, David Vrabel wrote:
> On 09/09/14 03:06, Yijing Wang wrote:
>> On 2014/9/5 22:29, David Vrabel wrote:
>>> On 05/09/14 11:09, Yijing Wang wrote:
 Use MSI chip framework instead of arch MSI functions to configure
 MSI/MSI-X irq. So we can manage MSI/MSI-X irq in a unified framework.
>>> [...]
 --- a/arch/x86/pci/xen.c
 +++ b/arch/x86/pci/xen.c
>>> [...]
 @@ -418,9 +430,9 @@ int __init pci_xen_init(void)
  #endif
  
  #ifdef CONFIG_PCI_MSI
 -  x86_msi.setup_msi_irqs = xen_setup_msi_irqs;
 -  x86_msi.teardown_msi_irq = xen_teardown_msi_irq;
 -  x86_msi.teardown_msi_irqs = xen_teardown_msi_irqs;
 +  xen_msi_chip.setup_irqs = xen_setup_msi_irqs;
 +  xen_msi_chip.teardown_irqs = xen_teardown_msi_irqs;
 +  x86_msi_chip = &xen_msi_chip;
msi_chip.irq_mask = xen_nop_msi_mask;
msi_chip.irq_unmask = xen_nop_msi_mask;
>>>
>>> Why have these not been changed to set the x86_msi_chip.mask/unmask
>>> fields instead?
>>
>> Hi David, x86_msi_chip here is struct msi_chip data type, used to configure 
>> MSI/MSI-X
>> irq. msi_chip above is struct irq_chip data type, represent the MSI irq 
>> controller. They are
>> not the same object. Their name easily confusing people.
> 
> Ok, it all makes sense now.
> 
> Acked-by: David Vrabel 

Thanks!

> 
> David
> 
> .
> 


-- 
Thanks!
Yijing

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [Xen-devel] [PATCH v1 04/21] x86/xen/MSI: Eliminate arch_msix_mask_irq() and arch_msi_mask_irq()

2014-09-10 Thread Yijing Wang
On 2014/9/10 20:36, David Vrabel wrote:
> On 05/09/14 11:09, Yijing Wang wrote:
>> Commit 0e4ccb150 added two __weak arch functions arch_msix_mask_irq()
>> and arch_msi_mask_irq() to fix a bug found when running xen in x86.
>> Introduced these two funcntions make MSI code complex. And mask/unmask
>> is the irq actions related to interrupt controller, should not use
>> weak arch functions to override mask/unmask interfaces. This patch
>> reverted commit 0e4ccb150 and export struct irq_chip msi_chip, modify
>> msi_chip->irq_mask/irq_unmask() in xen init functions to fix this
>> bug for simplicity. Also this is preparation for using struct
>> msi_chip instead of weak arch MSI functions in all platforms.
> 
> Acked-by: David Vrabel 
> 
> But I wonder if it would be better the Xen subsystem to provide its own
> struct irq_chip instead of adjusting the fields in the generic x86 one.

Thanks! Currently, Xen and the bare x86 system only have the different 
irq_chip->irq_mask/irq_unmask.
So I chose to override the two ops of bare x86 irq_chip in xen. Konrad 
Rzeszutek Wilk has been tested it
ok in his platform, so I think we could use its own irq_chip for xen later if 
the difference become large.

Thanks!
Yijing.

> 
> David
> 
> .
> 


-- 
Thanks!
Yijing

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: bit fields && data tearing

2014-09-10 Thread Peter Hurley
On 09/10/2014 05:48 PM, James Bottomley wrote:
> On Tue, 2014-09-09 at 06:40 -0400, Peter Hurley wrote:
>> On 09/08/2014 10:56 PM, James Bottomley wrote:
>>> On Mon, 2014-09-08 at 19:30 -0400, Peter Hurley wrote:
 On 09/08/2014 01:50 AM, James Bottomley wrote:
>> But additionally, even if gcc combines adjacent writes _that are part
>> of the program flow_ then I believe the situation is no worse than
>> would otherwise exist.
>>
>> For instance, given the following:
>>
>> struct x {
>>  spinlock_t lock;
>>  long a;
>>  byte b;
>>  byte c;
>> };
>>
>> void locked_store_b(struct x *p)
>> {
>>  spin_lock(&p->lock);
>>  p->b = 1;
>>  spin_unlock(&p->lock);
>>  p->c = 2;
>> }
>>
>> Granted, the author probably expects ordered writes of
>>  STORE B
>>  STORE C
>> but that's not guaranteed because there is no memory barrier
>> ordering B before C.
>
> Yes, there is: loads and stores may not migrate into or out of critical
> sections.

 That's a common misconception.

 The processor is free to re-order this to:

STORE C
STORE B
UNLOCK

 That's because the unlock() only guarantees that:

 Stores before the unlock in program order are guaranteed to complete
 before the unlock completes. Stores after the unlock _may_ complete
 before the unlock completes.

 My point was that even if compiler barriers had the same semantics
 as memory barriers, the situation would be no worse. That is, code
 that is sensitive to memory barriers (like the example I gave above)
 would merely have the same fragility with one-way compiler barriers
 (with respect to the compiler combining writes).

 That's what I meant by "no worse than would otherwise exist".
>>>
>>> Actually, that's not correct.  This is actually deja vu with me on the
>>> other side of the argument.  When we first did spinlocks on PA, I argued
>>> as you did: lock only a barrier for code after and unlock for code
>>> before.  The failing case is that you can have a critical section which
>>> performs an atomically required operation and a following unit which
>>> depends on it being performed.  If you begin the following unit before
>>> the atomic requirement, you may end up losing.  It turns out this kind
>>> of pattern is inherent in a lot of mail box device drivers: you need to
>>> set up the mailbox atomically then poke it.  Setup is usually atomic,
>>> deciding which mailbox to prime and actually poking it is in the
>>> following unit.  Priming often involves an I/O bus transaction and if
>>> you poke before priming, you get a misfire.
>>
>> Take it up with the man because this was discussed extensively last
>> year and it was decided that unlocks would not be full barriers.
>> Thus the changes to memory-barriers.txt that explicitly note this
>> and the addition of smp_mb__after_unlock_lock() (for two different
>> locks; an unlock followed by a lock on the same lock is a full barrier).
>>
>> Code that expects ordered writes after an unlock needs to explicitly
>> add the memory barrier.
> 
> I don't really care what ARM does; spin locks are full barriers on
> architectures that need them.  The driver problem we had that detected
> our semi permeable spinlocks was an LSI 53c875 which is enterprise class
> PCI, so presumably not relevant to ARM anyway.

Almost certainly ia64 arch_spin_unlock() is not a full barrier.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3] topology: add support for node_to_mem_node() to determine the fallback node

2014-09-10 Thread Nishanth Aravamudan
On 10.09.2014 [12:06:16 -0700], Andrew Morton wrote:
> On Tue, 9 Sep 2014 17:47:23 -0700 Nishanth Aravamudan 
>  wrote:
> 
> > On 09.09.2014 [17:11:15 -0700], Andrew Morton wrote:
> > > On Tue, 9 Sep 2014 12:03:27 -0700 Nishanth Aravamudan 
> > >  wrote:
> > > 
> > > > From: Joonsoo Kim 
> > > > 
> > > > We need to determine the fallback node in slub allocator if the
> > > > allocation target node is memoryless node. Without it, the SLUB wrongly
> > > > select the node which has no memory and can't use a partial slab,
> > > > because of node mismatch. Introduced function, node_to_mem_node(X), will
> > > > return a node Y with memory that has the nearest distance. If X is
> > > > memoryless node, it will return nearest distance node, but, if X is
> > > > normal node, it will return itself.
> > > > 
> > > > We will use this function in following patch to determine the fallback
> > > > node.
> > > > 
> > > > ...
> > > >
> > > > --- a/include/linux/topology.h
> > > > +++ b/include/linux/topology.h
> > > > @@ -119,11 +119,20 @@ static inline int numa_node_id(void)
> > > >   * Use the accessor functions set_numa_mem(), numa_mem_id() and 
> > > > cpu_to_mem().
> > > 
> > > This comment could be updated.
> > 
> > Will do, do you prefer a follow-on patch or one that replaces this one?
> 
> Either is OK for me.  I always turn replacement patches into
> incrementals so I (and others) can see what changed.

Ok, I'll probably send you just an incremental then myself.

> > > >   */
> > > >  DECLARE_PER_CPU(int, _numa_mem_);
> > > > +extern int _node_numa_mem_[MAX_NUMNODES];
> > > >  
> > > >  #ifndef set_numa_mem
> > > >  static inline void set_numa_mem(int node)
> > > >  {
> > > > this_cpu_write(_numa_mem_, node);
> > > > +   _node_numa_mem_[numa_node_id()] = node;
> > > > +}
> > > > +#endif
> > > > +
> > > > +#ifndef node_to_mem_node
> > > > +static inline int node_to_mem_node(int node)
> > > > +{
> > > > +   return _node_numa_mem_[node];
> > > >  }
> > > 
> > > A wee bit of documentation wouldn't hurt.
> 
> This?

Yep, I'll make sure it gets added.

> > > > --- a/mm/page_alloc.c
> > > > +++ b/mm/page_alloc.c
> > > > @@ -85,6 +85,7 @@ EXPORT_PER_CPU_SYMBOL(numa_node);
> > > >   */
> > > >  DEFINE_PER_CPU(int, _numa_mem_);   /* Kernel "local 
> > > > memory" node */
> > > >  EXPORT_PER_CPU_SYMBOL(_numa_mem_);
> > > > +int _node_numa_mem_[MAX_NUMNODES];
> > > 
> > > How does this get updated as CPUs, memory and nodes are hot-added and
> > > removed?
> > 
> > As CPUs are added, the architecture code in the CPU bringup will update
> > the NUMA topology. Memory and node hotplug are still open issues, I
> > mentioned the former in the cover letter. I should have mentioned it in
> > this commit message as well.
> 
> Please define "open issue".  The computer will crash and catch fire?
> If not that, then what?

Umm, let's call it "undefined" (untested?). Which is no different than
where we are today, afaict, with memoryless nodes. I think going from
memoryless->memoryful probably works, but the other direction may not
(and may not be possible in the common case).

-Nish

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 1/2] pseries: Fix endian issues in onlining cpu threads

2014-09-10 Thread Thomas Falcon
The ibm,ppc-interrupt-server#s property is in big endian format.
These values need to be converted when used by little endian
architectures.

Signed-off-by: Thomas Falcon 
---
 arch/powerpc/platforms/pseries/dlpar.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index c2806c8..cd425dc 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -363,7 +363,7 @@ static int dlpar_online_cpu(struct device_node *dn)
int rc = 0;
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
 
intserv = of_get_property(dn, "ibm,ppc-interrupt-server#s", &len);
if (!intserv)
@@ -374,7 +374,7 @@ static int dlpar_online_cpu(struct device_node *dn)
cpu_maps_update_begin();
for (i = 0; i < nthreads; i++) {
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != 
be32_to_cpu(intserv[i]))
continue;
BUG_ON(get_cpu_current_state(cpu)
!= CPU_STATE_OFFLINE);
@@ -388,7 +388,7 @@ static int dlpar_online_cpu(struct device_node *dn)
}
if (cpu == num_possible_cpus())
printk(KERN_WARNING "Could not find cpu to online "
-  "with physical id 0x%x\n", intserv[i]);
+  "with physical id 0x%x\n", 
be32_to_cpu(intserv[i]));
}
cpu_maps_update_done();
 
-- 
1.8.5.2

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 2/2] pseries: Fix endian issues in cpu hot-removal

2014-09-10 Thread Thomas Falcon
When removing a cpu, this patch makes sure that values
gotten from or passed to firmware are in the correct
endian format.

Signed-off-by: Thomas Falcon 
---
 arch/powerpc/platforms/pseries/dlpar.c   | 14 +++---
 arch/powerpc/platforms/pseries/hotplug-cpu.c |  8 
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index cd425dc..c5ecfdb 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -442,7 +442,7 @@ static int dlpar_offline_cpu(struct device_node *dn)
int rc = 0;
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
 
intserv = of_get_property(dn, "ibm,ppc-interrupt-server#s", &len);
if (!intserv)
@@ -453,7 +453,7 @@ static int dlpar_offline_cpu(struct device_node *dn)
cpu_maps_update_begin();
for (i = 0; i < nthreads; i++) {
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != 
be32_to_cpu(intserv[i]))
continue;
 
if (get_cpu_current_state(cpu) == CPU_STATE_OFFLINE)
@@ -475,14 +475,14 @@ static int dlpar_offline_cpu(struct device_node *dn)
 * Upgrade it's state to CPU_STATE_OFFLINE.
 */
set_preferred_offline_state(cpu, CPU_STATE_OFFLINE);
-   BUG_ON(plpar_hcall_norets(H_PROD, intserv[i])
+   BUG_ON(plpar_hcall_norets(H_PROD, 
be32_to_cpu(intserv[i]))
!= H_SUCCESS);
__cpu_die(cpu);
break;
}
if (cpu == num_possible_cpus())
printk(KERN_WARNING "Could not find cpu to offline "
-  "with physical id 0x%x\n", intserv[i]);
+  "with physical id 0x%x\n", 
be32_to_cpu(intserv[i]));
}
cpu_maps_update_done();
 
@@ -494,7 +494,7 @@ out:
 static ssize_t dlpar_cpu_release(const char *buf, size_t count)
 {
struct device_node *dn;
-   const u32 *drc_index;
+   const __be32 *drc_index;
int rc;
 
dn = of_find_node_by_path(buf);
@@ -513,7 +513,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
return -EINVAL;
}
 
-   rc = dlpar_release_drc(*drc_index);
+   rc = dlpar_release_drc(be32_to_cpup(drc_index));
if (rc) {
of_node_put(dn);
return rc;
@@ -521,7 +521,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
 
rc = dlpar_detach_node(dn);
if (rc) {
-   dlpar_acquire_drc(*drc_index);
+   dlpar_acquire_drc(be32_to_cpup(drc_index));
return rc;
}
 
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c 
b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 447f8c6..031762d 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -90,7 +90,7 @@ static void rtas_stop_self(void)
 {
static struct rtas_args args = {
.nargs = 0,
-   .nret = 1,
+   .nret = cpu_to_be32(1),
.rets = &args.args[0],
};
 
@@ -312,7 +312,7 @@ static void pseries_remove_processor(struct device_node *np)
 {
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
 
intserv = of_get_property(np, "ibm,ppc-interrupt-server#s", &len);
if (!intserv)
@@ -323,7 +323,7 @@ static void pseries_remove_processor(struct device_node *np)
cpu_maps_update_begin();
for (i = 0; i < nthreads; i++) {
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != 
be32_to_cpu(intserv[i]))
continue;
BUG_ON(cpu_online(cpu));
set_cpu_present(cpu, false);
@@ -332,7 +332,7 @@ static void pseries_remove_processor(struct device_node *np)
}
if (cpu >= nr_cpu_ids)
printk(KERN_WARNING "Could not find cpu to remove "
-  "with physical id 0x%x\n", intserv[i]);
+  "with physical id 0x%x\n", 
be32_to_cpu(intserv[i]));
}
cpu_maps_update_done();
 }
-- 
1.8.5.2

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: bit fields && data tearing

2014-09-10 Thread James Bottomley
On Tue, 2014-09-09 at 06:40 -0400, Peter Hurley wrote:
> On 09/08/2014 10:56 PM, James Bottomley wrote:
> > On Mon, 2014-09-08 at 19:30 -0400, Peter Hurley wrote:
> >> On 09/08/2014 01:50 AM, James Bottomley wrote:
>  But additionally, even if gcc combines adjacent writes _that are part
>  of the program flow_ then I believe the situation is no worse than
>  would otherwise exist.
> 
>  For instance, given the following:
> 
>  struct x {
>   spinlock_t lock;
>   long a;
>   byte b;
>   byte c;
>  };
> 
>  void locked_store_b(struct x *p)
>  {
>   spin_lock(&p->lock);
>   p->b = 1;
>   spin_unlock(&p->lock);
>   p->c = 2;
>  }
> 
>  Granted, the author probably expects ordered writes of
>   STORE B
>   STORE C
>  but that's not guaranteed because there is no memory barrier
>  ordering B before C.
> >>>
> >>> Yes, there is: loads and stores may not migrate into or out of critical
> >>> sections.
> >>
> >> That's a common misconception.
> >>
> >> The processor is free to re-order this to:
> >>
> >>STORE C
> >>STORE B
> >>UNLOCK
> >>
> >> That's because the unlock() only guarantees that:
> >>
> >> Stores before the unlock in program order are guaranteed to complete
> >> before the unlock completes. Stores after the unlock _may_ complete
> >> before the unlock completes.
> >>
> >> My point was that even if compiler barriers had the same semantics
> >> as memory barriers, the situation would be no worse. That is, code
> >> that is sensitive to memory barriers (like the example I gave above)
> >> would merely have the same fragility with one-way compiler barriers
> >> (with respect to the compiler combining writes).
> >>
> >> That's what I meant by "no worse than would otherwise exist".
> > 
> > Actually, that's not correct.  This is actually deja vu with me on the
> > other side of the argument.  When we first did spinlocks on PA, I argued
> > as you did: lock only a barrier for code after and unlock for code
> > before.  The failing case is that you can have a critical section which
> > performs an atomically required operation and a following unit which
> > depends on it being performed.  If you begin the following unit before
> > the atomic requirement, you may end up losing.  It turns out this kind
> > of pattern is inherent in a lot of mail box device drivers: you need to
> > set up the mailbox atomically then poke it.  Setup is usually atomic,
> > deciding which mailbox to prime and actually poking it is in the
> > following unit.  Priming often involves an I/O bus transaction and if
> > you poke before priming, you get a misfire.
> 
> Take it up with the man because this was discussed extensively last
> year and it was decided that unlocks would not be full barriers.
> Thus the changes to memory-barriers.txt that explicitly note this
> and the addition of smp_mb__after_unlock_lock() (for two different
> locks; an unlock followed by a lock on the same lock is a full barrier).
> 
> Code that expects ordered writes after an unlock needs to explicitly
> add the memory barrier.

I don't really care what ARM does; spin locks are full barriers on
architectures that need them.  The driver problem we had that detected
our semi permeable spinlocks was an LSI 53c875 which is enterprise class
PCI, so presumably not relevant to ARM anyway.

James


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: bit fields && data tearing

2014-09-10 Thread Rob Landley
On Wed, Sep 10, 2014 at 3:18 PM, H. Peter Anvin  wrote:
> On 09/08/2014 10:52 AM, One Thousand Gnomes wrote:
>>
>> I think the whole "removing Alpha EV5" support is basically bonkers. Just
>> use set_bit in the tty layer. Alpha will continue to work as well as it
>> always has done and you won't design out support for any future processor
>> that turns out not to do byte aligned stores.
>>
>
> I think it's pretty safe to say such a processor is unlikely to ever be
> created, for the same reason there weren't any 48-bit computers when
> 32-bit computers ran out of steam: it caused more problems than it
> solved, and Alpha pretty much proved that.  The engineering advantages
> would have to be so overwhelmingly in favor for someone to want to walk
> down that road again.
>
> -hpa

I note for the record I'm trying to get basic Alpha support working in
http://landley.net/aboriginal/about.html now that qemu at least offers
a qemu-system-alpha, and if I do get it working I may take a stab at
adding alpha support to musl-libc.

It's not just that I want to get cross platform testing of package
builds working on a convenient/reproducible/portable way on as many
different targets as I can. My project is designed the way it is
because I think losing the ability to reproduce things from first
principles (and thus understand why it was done that way in the first
place) is a _bad_idea_. And an increasingly real-world problem, see
http://wrttn.in/04af1a for example. (And the way NASA lost the plans
for the Saturn V rocket... In fact scientists' general disinclination
to reproduce old experimental results to see if they were actually
true or not comes up on a regular basis, most recent I saw was
http://www.theglobeandmail.com/life/health-and-fitness/health/the-curious-case-of-the-cyclists-unshaved-legs/article20370814/
)

Of course Peter has consistently dismissed my concerns as "academic":

http://lkml.iu.edu/hypermail/linux/kernel/0802.1/4400.html

Personally, I hope the projects I work on _do_ outlive me, an try to
make it easy for newbies to recapitulate phylogeny. That's why I
thought requiring perl dependencies to build was a bad idea, and why I
thought removing 386 support entirely (instead of restricting it to
UP) was a bad idea. The idea that Linux no longer needs to build on
the original machine Linus designed it for, and now it no longer needs
to work on the machine the "arch" directory was created for (and the
first 64 bit machine)... These positions would appear to have a single
consistent proponent, with whom I disagree.

Oh well, just wanted to get that out there,

Rob
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: bit fields && data tearing

2014-09-10 Thread H. Peter Anvin
On 09/08/2014 10:52 AM, One Thousand Gnomes wrote:
> 
> I think the whole "removing Alpha EV5" support is basically bonkers. Just
> use set_bit in the tty layer. Alpha will continue to work as well as it
> always has done and you won't design out support for any future processor
> that turns out not to do byte aligned stores.
> 

I think it's pretty safe to say such a processor is unlikely to ever be
created, for the same reason there weren't any 48-bit computers when
32-bit computers ran out of steam: it caused more problems than it
solved, and Alpha pretty much proved that.  The engineering advantages
would have to be so overwhelmingly in favor for someone to want to walk
down that road again.

-hpa


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc: make of_device_ids const

2014-09-10 Thread Uwe Kleine-König
of_device_ids (i.e. compatible strings and the respective data) are not
supposed to change at runtime. All functions working with of_device_ids
provided by  work with const of_device_ids. This allows to
mark all struct of_device_id const, too.

While touching these line also put the __init annotation at the right
position where necessary.

Signed-off-by: Uwe Kleine-König 
---
Hello,

I don't know how arch/powerpc is maintained. So please tell me if I
should split this patch further.

I manually checked that all const annotations are OK, and the 0day build
bot didn't find a regression.

Best regards
Uwe

 arch/powerpc/kernel/ibmebus.c|  2 +-
 arch/powerpc/kernel/legacy_serial.c  |  2 +-
 arch/powerpc/kernel/of_platform.c|  2 +-
 arch/powerpc/platforms/40x/ep405.c   |  2 +-
 arch/powerpc/platforms/40x/ppc40x_simple.c   |  2 +-
 arch/powerpc/platforms/40x/virtex.c  |  2 +-
 arch/powerpc/platforms/40x/walnut.c  |  2 +-
 arch/powerpc/platforms/44x/canyonlands.c |  2 +-
 arch/powerpc/platforms/44x/ebony.c   |  2 +-
 arch/powerpc/platforms/44x/iss4xx.c  |  2 +-
 arch/powerpc/platforms/44x/ppc44x_simple.c   |  2 +-
 arch/powerpc/platforms/44x/ppc476.c  |  2 +-
 arch/powerpc/platforms/44x/sam440ep.c|  2 +-
 arch/powerpc/platforms/44x/virtex.c  |  2 +-
 arch/powerpc/platforms/44x/warp.c|  2 +-
 arch/powerpc/platforms/512x/mpc512x_shared.c |  2 +-
 arch/powerpc/platforms/52xx/lite5200.c   |  4 ++--
 arch/powerpc/platforms/52xx/media5200.c  |  2 +-
 arch/powerpc/platforms/52xx/mpc52xx_common.c | 12 ++--
 arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c|  2 +-
 arch/powerpc/platforms/52xx/mpc52xx_pic.c|  4 ++--
 arch/powerpc/platforms/82xx/ep8248e.c|  2 +-
 arch/powerpc/platforms/82xx/km82xx.c |  2 +-
 arch/powerpc/platforms/82xx/mpc8272_ads.c|  2 +-
 arch/powerpc/platforms/82xx/pq2fads.c|  2 +-
 arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c   |  2 +-
 arch/powerpc/platforms/83xx/misc.c   |  2 +-
 arch/powerpc/platforms/83xx/mpc834x_itx.c|  2 +-
 arch/powerpc/platforms/83xx/suspend.c|  4 ++--
 arch/powerpc/platforms/85xx/common.c |  2 +-
 arch/powerpc/platforms/85xx/ppa8548.c|  2 +-
 arch/powerpc/platforms/85xx/sgy_cts1000.c|  4 ++--
 arch/powerpc/platforms/86xx/gef_ppc9a.c  |  2 +-
 arch/powerpc/platforms/86xx/gef_sbc310.c |  2 +-
 arch/powerpc/platforms/86xx/gef_sbc610.c |  2 +-
 arch/powerpc/platforms/86xx/mpc8610_hpcd.c   |  2 +-
 arch/powerpc/platforms/86xx/mpc86xx_hpcn.c   |  2 +-
 arch/powerpc/platforms/86xx/sbc8641d.c   |  2 +-
 arch/powerpc/platforms/8xx/adder875.c|  2 +-
 arch/powerpc/platforms/8xx/ep88xc.c  |  2 +-
 arch/powerpc/platforms/8xx/mpc86xads_setup.c |  2 +-
 arch/powerpc/platforms/8xx/mpc885ads_setup.c |  2 +-
 arch/powerpc/platforms/8xx/tqm8xx_setup.c|  2 +-
 arch/powerpc/platforms/cell/celleb_pci.c |  2 +-
 arch/powerpc/platforms/cell/celleb_setup.c   |  2 +-
 arch/powerpc/platforms/embedded6xx/gamecube.c|  2 +-
 arch/powerpc/platforms/embedded6xx/linkstation.c |  2 +-
 arch/powerpc/platforms/embedded6xx/mvme5100.c|  2 +-
 arch/powerpc/platforms/embedded6xx/storcenter.c  |  2 +-
 arch/powerpc/platforms/embedded6xx/wii.c |  2 +-
 arch/powerpc/platforms/pasemi/gpio_mdio.c|  2 +-
 arch/powerpc/platforms/pasemi/setup.c|  2 +-
 arch/powerpc/sysdev/axonram.c|  2 +-
 arch/powerpc/sysdev/fsl_85xx_l2ctlr.c|  2 +-
 arch/powerpc/sysdev/mv64x60_dev.c|  2 +-
 arch/powerpc/sysdev/pmi.c|  2 +-
 arch/powerpc/sysdev/xilinx_intc.c|  2 +-
 arch/powerpc/sysdev/xilinx_pci.c |  2 +-
 58 files changed, 67 insertions(+), 67 deletions(-)

diff --git a/arch/powerpc/kernel/ibmebus.c b/arch/powerpc/kernel/ibmebus.c
index 1114d13ac19f..ac86c53e2542 100644
--- a/arch/powerpc/kernel/ibmebus.c
+++ b/arch/powerpc/kernel/ibmebus.c
@@ -55,7 +55,7 @@ static struct device ibmebus_bus_device = { /* fake "parent" 
device */
 struct bus_type ibmebus_bus_type;
 
 /* These devices will automatically be added to the bus during init */
-static struct of_device_id __initdata ibmebus_matches[] = {
+static const struct of_device_id ibmebus_matches[] __initconst = {
{ .compatible = "IBM,lhca" },
{ .compatible = "IBM,lhea" },
{},
diff --git a/arch/powerpc/kernel/legacy_serial.c 
b/arch/powerpc/kernel/legacy_serial.c
index 936258881c98..7b750c4ed5c7 100644
--- a/arch/powerpc/kernel/legacy_serial.c
+++ b/arch/powerpc/kernel/legacy_serial.c
@@ -35,7 +35,7 @@ static struct legacy_serial_info {
phys_addr_t taddr;
 } legacy_serial_infos[MAX_LEGACY_SER

Re: [PATCH v3] topology: add support for node_to_mem_node() to determine the fallback node

2014-09-10 Thread Andrew Morton
On Tue, 9 Sep 2014 17:47:23 -0700 Nishanth Aravamudan  
wrote:

> On 09.09.2014 [17:11:15 -0700], Andrew Morton wrote:
> > On Tue, 9 Sep 2014 12:03:27 -0700 Nishanth Aravamudan 
> >  wrote:
> > 
> > > From: Joonsoo Kim 
> > > 
> > > We need to determine the fallback node in slub allocator if the
> > > allocation target node is memoryless node. Without it, the SLUB wrongly
> > > select the node which has no memory and can't use a partial slab,
> > > because of node mismatch. Introduced function, node_to_mem_node(X), will
> > > return a node Y with memory that has the nearest distance. If X is
> > > memoryless node, it will return nearest distance node, but, if X is
> > > normal node, it will return itself.
> > > 
> > > We will use this function in following patch to determine the fallback
> > > node.
> > > 
> > > ...
> > >
> > > --- a/include/linux/topology.h
> > > +++ b/include/linux/topology.h
> > > @@ -119,11 +119,20 @@ static inline int numa_node_id(void)
> > >   * Use the accessor functions set_numa_mem(), numa_mem_id() and 
> > > cpu_to_mem().
> > 
> > This comment could be updated.
> 
> Will do, do you prefer a follow-on patch or one that replaces this one?

Either is OK for me.  I always turn replacement patches into
incrementals so I (and others) can see what changed.

> > >   */
> > >  DECLARE_PER_CPU(int, _numa_mem_);
> > > +extern int _node_numa_mem_[MAX_NUMNODES];
> > >  
> > >  #ifndef set_numa_mem
> > >  static inline void set_numa_mem(int node)
> > >  {
> > >   this_cpu_write(_numa_mem_, node);
> > > + _node_numa_mem_[numa_node_id()] = node;
> > > +}
> > > +#endif
> > > +
> > > +#ifndef node_to_mem_node
> > > +static inline int node_to_mem_node(int node)
> > > +{
> > > + return _node_numa_mem_[node];
> > >  }
> > 
> > A wee bit of documentation wouldn't hurt.

This?

> > > --- a/mm/page_alloc.c
> > > +++ b/mm/page_alloc.c
> > > @@ -85,6 +85,7 @@ EXPORT_PER_CPU_SYMBOL(numa_node);
> > >   */
> > >  DEFINE_PER_CPU(int, _numa_mem_); /* Kernel "local memory" node */
> > >  EXPORT_PER_CPU_SYMBOL(_numa_mem_);
> > > +int _node_numa_mem_[MAX_NUMNODES];
> > 
> > How does this get updated as CPUs, memory and nodes are hot-added and
> > removed?
> 
> As CPUs are added, the architecture code in the CPU bringup will update
> the NUMA topology. Memory and node hotplug are still open issues, I
> mentioned the former in the cover letter. I should have mentioned it in
> this commit message as well.

Please define "open issue".  The computer will crash and catch fire?
If not that, then what?

> I do notice that Lee's commit message from 7aac78988551 ("numa:
> introduce numa_mem_id()- effective local memory node id"):
> 
> "Generic initialization of 'numa_mem' occurs in __build_all_zonelists().
> This will initialize the boot cpu at boot time, and all cpus on change
> of numa_zonelist_order, or when node or memory hot-plug requires
> zonelist rebuild.  Archs that support memoryless nodes will need to
> initialize 'numa_mem' for secondary cpus as they're brought on-line."
> 
> And since we update the _node_numa_mem_ value on set_cpu_numa_mem()
> calls, which were already needed for numa_mem_id(), we might be covered.
> Testing these cases (hotplug) is next in my plans.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module

2014-09-10 Thread Nicolin Chen
On Wed, Sep 10, 2014 at 04:12:53PM +0800, Shengjiu Wang wrote:
> > Then we can get a patch like:
> > open() {
> > +   clk_prepare_enable();
> > 
> > }
> > 
> > close() {
> > 
> > +   clk_disable_unprepare()
> > }

> what is the open() and close()? do you mean the fsl_ssi_startup()
> and fsl_ssi_shutdown()?

Yea.

> > probe() {
> > clk_get();
> > clk_prepare_enable();
> > 
> > if (xxx)
> > -   goto err_xx;
> > +   return ret;
> > 
> > +   clk_disable_unprepare();
> > return 0;
> > -err_xx:
> > -   clk_disable_unprepare()
> > }

> If this probe() is fsl_ssi_imx_probe(), I think no need to add
> clk_prepare_enable() or clk_disable_unprepare(), seems there is no 
> registers accessing in this probe.

This is trying to be safe, especially for such a driver being used
by multiple platforms. You can omit this as long as the patch can
pass the test on old imx, PowerPC, and AC97 platforms.

>

And another risk just came to my mind is that there would be a
possibility that a machine driver would call set_dai_fmt() early,
after SSI's probe() and before SSI's startup(), if the machine
driver contains dai_fmt assignment in its probe(). Then, without
regmap_mmio_clk(), it'll be tough for us over here because we may
also need to add clock enable/disable for set_dai_fmt/set_sysclk(),
even if there might be still tiny risk that we missed something.

Then there could be a selfish approach to circumvent it is to use
regmap_mmio_clk() with "ipg" at the beginning and call regmap_mmio()
without "ipg" if getting a failed return value from regmap_mmio_clk,
and meanwhile to keep the clock always enabled for the regmap_mmio()
case just like what the current driver is doing. This may result
those non-ipg-clk platforms can't benefit from this refinement
unless they update their DT bindings -- use "ipg" for core clock
This might be the safest and simplest way for us, I'm not sure
everyone would be comfortable with this idea though.

Best regards,
Nicolin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] sound: aoa: printk replacement

2014-09-10 Thread Sudip Mukherjee
On Wed, Sep 10, 2014 at 05:38:10PM +0200, Takashi Iwai wrote:
> At Wed, 10 Sep 2014 20:37:50 +0530,
> Sudip Mukherjee wrote:
> > 
> > On Wed, Sep 10, 2014 at 04:43:03PM +0200, Takashi Iwai wrote:
> > > At Wed, 10 Sep 2014 20:02:04 +0530,
> > > Sudip Mukherjee wrote:
> > > > 
> > > > On Wed, Sep 10, 2014 at 03:57:04PM +0200, Johannes Berg wrote:
> > > > > On Wed, 2014-09-10 at 19:21 +0530, Sudip Mukherjee wrote:
> > > > > > as pr_* macros are more preffered over printk, so printk replaced
> > > > > > with corresponding pr_* macros.
> > > > > 
> > > > > Are you simply running checkpatch on every file and decided to do
> > > > > something about it? :)
> > > > > 
> > > > i am running checkpatch on the patch generated. if i am doing checkpatch
> > > > cleanups then that i do it only in the staging.
> > > > only exception : printk .. :)
> > > > 
> > > > > I'll let Takashi decide whether to take this or not as I no longer 
> > > > > care
> > > > > about this code, but IMHO this changes is completely pointless since 
> > > > > you
> > > > > don't also clean up the code to have a common prefix with #define 
> > > > > pr_fmt
> > > > > and then clean up the callers etc.
> > > > > 
> > > > i mentioned in the comment that in a future patch we can have pr_fmt,
> > > > it was not done in this patch since the changes for this patch is
> > > > generated by a script and not manually.
> > > > if Takashi accepts this then the next patch will have pr_fmt.
> > > 
> > > If you're going to work on it, please give a patch series and let me
> > > merge once.  There is no good merit to merge a half-baked piece by
> > > piece.
> > > 
> > > Regarding the changes you've made: so far, I've merged two such
> > > patches just because it's a good exercise for newbies.  You've played
> > > it and experienced it enough.  So it's time to go up to a higher
> > > stage, more "real" fixes.
> > can you please give me some hint of fixes that can be attempted by
> > newbies. except printk :)
> 
> You should study the code at first.  There is nothing you can "fix"
> without understanding the code.  So, pick up a driver you're
> interested in.  You may or may not find bugs there.  Or, if you see /
> know any bug, try to join debugging.
> 
> 
> Takashi

one of my patch has alredy changed printk to pr_* in ctxfi. but that can be 
improved
to dev_*. since this has to be done manully and not through script , so should 
i send the
patch for one file at a time or for all of them together in a single patch?

thanks
sudip
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] sound: aoa: printk replacement

2014-09-10 Thread Takashi Iwai
At Wed, 10 Sep 2014 20:37:50 +0530,
Sudip Mukherjee wrote:
> 
> On Wed, Sep 10, 2014 at 04:43:03PM +0200, Takashi Iwai wrote:
> > At Wed, 10 Sep 2014 20:02:04 +0530,
> > Sudip Mukherjee wrote:
> > > 
> > > On Wed, Sep 10, 2014 at 03:57:04PM +0200, Johannes Berg wrote:
> > > > On Wed, 2014-09-10 at 19:21 +0530, Sudip Mukherjee wrote:
> > > > > as pr_* macros are more preffered over printk, so printk replaced
> > > > > with corresponding pr_* macros.
> > > > 
> > > > Are you simply running checkpatch on every file and decided to do
> > > > something about it? :)
> > > > 
> > > i am running checkpatch on the patch generated. if i am doing checkpatch
> > > cleanups then that i do it only in the staging.
> > > only exception : printk .. :)
> > > 
> > > > I'll let Takashi decide whether to take this or not as I no longer care
> > > > about this code, but IMHO this changes is completely pointless since you
> > > > don't also clean up the code to have a common prefix with #define pr_fmt
> > > > and then clean up the callers etc.
> > > > 
> > > i mentioned in the comment that in a future patch we can have pr_fmt,
> > > it was not done in this patch since the changes for this patch is
> > > generated by a script and not manually.
> > > if Takashi accepts this then the next patch will have pr_fmt.
> > 
> > If you're going to work on it, please give a patch series and let me
> > merge once.  There is no good merit to merge a half-baked piece by
> > piece.
> > 
> > Regarding the changes you've made: so far, I've merged two such
> > patches just because it's a good exercise for newbies.  You've played
> > it and experienced it enough.  So it's time to go up to a higher
> > stage, more "real" fixes.
> can you please give me some hint of fixes that can be attempted by
> newbies. except printk :)

You should study the code at first.  There is nothing you can "fix"
without understanding the code.  So, pick up a driver you're
interested in.  You may or may not find bugs there.  Or, if you see /
know any bug, try to join debugging.


Takashi
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] sound: aoa: printk replacement

2014-09-10 Thread Sudip Mukherjee
On Wed, Sep 10, 2014 at 04:43:03PM +0200, Takashi Iwai wrote:
> At Wed, 10 Sep 2014 20:02:04 +0530,
> Sudip Mukherjee wrote:
> > 
> > On Wed, Sep 10, 2014 at 03:57:04PM +0200, Johannes Berg wrote:
> > > On Wed, 2014-09-10 at 19:21 +0530, Sudip Mukherjee wrote:
> > > > as pr_* macros are more preffered over printk, so printk replaced
> > > > with corresponding pr_* macros.
> > > 
> > > Are you simply running checkpatch on every file and decided to do
> > > something about it? :)
> > > 
> > i am running checkpatch on the patch generated. if i am doing checkpatch
> > cleanups then that i do it only in the staging.
> > only exception : printk .. :)
> > 
> > > I'll let Takashi decide whether to take this or not as I no longer care
> > > about this code, but IMHO this changes is completely pointless since you
> > > don't also clean up the code to have a common prefix with #define pr_fmt
> > > and then clean up the callers etc.
> > > 
> > i mentioned in the comment that in a future patch we can have pr_fmt,
> > it was not done in this patch since the changes for this patch is
> > generated by a script and not manually.
> > if Takashi accepts this then the next patch will have pr_fmt.
> 
> If you're going to work on it, please give a patch series and let me
> merge once.  There is no good merit to merge a half-baked piece by
> piece.
> 
> Regarding the changes you've made: so far, I've merged two such
> patches just because it's a good exercise for newbies.  You've played
> it and experienced it enough.  So it's time to go up to a higher
> stage, more "real" fixes.
can you please give me some hint of fixes that can be attempted by
newbies. except printk :)
so far i have started with fixing the sparse warnings in staging.

> 
> For example, if you are still interested in printk stuff, try to
> change the calls to dev_err() and co.  Of course, this needs more
> understanding of the code you'll handle, which object is passed for
> which messages.
> 
sure , i will send you this one.

thanks
sudip


> 
> thanks,
> 
> Takashi
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [Xen-devel] [PATCH v1 08/21] x86/xen/MSI: Use MSI chip framework to configure MSI/MSI-X irq

2014-09-10 Thread Konrad Rzeszutek Wilk
On Wed, Sep 10, 2014 at 01:38:25PM +0100, David Vrabel wrote:
> On 09/09/14 03:06, Yijing Wang wrote:
> > On 2014/9/5 22:29, David Vrabel wrote:
> >> On 05/09/14 11:09, Yijing Wang wrote:
> >>> Use MSI chip framework instead of arch MSI functions to configure
> >>> MSI/MSI-X irq. So we can manage MSI/MSI-X irq in a unified framework.
> >> [...]
> >>> --- a/arch/x86/pci/xen.c
> >>> +++ b/arch/x86/pci/xen.c
> >> [...]
> >>> @@ -418,9 +430,9 @@ int __init pci_xen_init(void)
> >>>  #endif
> >>>  
> >>>  #ifdef CONFIG_PCI_MSI
> >>> - x86_msi.setup_msi_irqs = xen_setup_msi_irqs;
> >>> - x86_msi.teardown_msi_irq = xen_teardown_msi_irq;
> >>> - x86_msi.teardown_msi_irqs = xen_teardown_msi_irqs;
> >>> + xen_msi_chip.setup_irqs = xen_setup_msi_irqs;
> >>> + xen_msi_chip.teardown_irqs = xen_teardown_msi_irqs;
> >>> + x86_msi_chip = &xen_msi_chip;
> >>>   msi_chip.irq_mask = xen_nop_msi_mask;
> >>>   msi_chip.irq_unmask = xen_nop_msi_mask;
> >>
> >> Why have these not been changed to set the x86_msi_chip.mask/unmask
> >> fields instead?
> > 
> > Hi David, x86_msi_chip here is struct msi_chip data type, used to configure 
> > MSI/MSI-X
> > irq. msi_chip above is struct irq_chip data type, represent the MSI irq 
> > controller. They are
> > not the same object. Their name easily confusing people.
> 
> Ok, it all makes sense now.
> 
> Acked-by: David Vrabel 

You can also add 'Tested-by: Konrad Rzeszutek Wilk '

on the whole series - I ran it through on Xen and on baremetal with a mix of 
32/64 builds.

Oh, and also Reviewed-by: Konrad Rzeszutek Wilk  the 
Xen parts.

> 
> David
> 
> ___
> Xen-devel mailing list
> xen-de...@lists.xen.org
> http://lists.xen.org/xen-devel
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] sound: aoa: printk replacement

2014-09-10 Thread Takashi Iwai
At Wed, 10 Sep 2014 20:02:04 +0530,
Sudip Mukherjee wrote:
> 
> On Wed, Sep 10, 2014 at 03:57:04PM +0200, Johannes Berg wrote:
> > On Wed, 2014-09-10 at 19:21 +0530, Sudip Mukherjee wrote:
> > > as pr_* macros are more preffered over printk, so printk replaced
> > > with corresponding pr_* macros.
> > 
> > Are you simply running checkpatch on every file and decided to do
> > something about it? :)
> > 
> i am running checkpatch on the patch generated. if i am doing checkpatch
> cleanups then that i do it only in the staging.
> only exception : printk .. :)
> 
> > I'll let Takashi decide whether to take this or not as I no longer care
> > about this code, but IMHO this changes is completely pointless since you
> > don't also clean up the code to have a common prefix with #define pr_fmt
> > and then clean up the callers etc.
> > 
> i mentioned in the comment that in a future patch we can have pr_fmt,
> it was not done in this patch since the changes for this patch is
> generated by a script and not manually.
> if Takashi accepts this then the next patch will have pr_fmt.

If you're going to work on it, please give a patch series and let me
merge once.  There is no good merit to merge a half-baked piece by
piece.

Regarding the changes you've made: so far, I've merged two such
patches just because it's a good exercise for newbies.  You've played
it and experienced it enough.  So it's time to go up to a higher
stage, more "real" fixes.

For example, if you are still interested in printk stuff, try to
change the calls to dev_err() and co.  Of course, this needs more
understanding of the code you'll handle, which object is passed for
which messages.


thanks,

Takashi
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] sound: aoa: printk replacement

2014-09-10 Thread Sudip Mukherjee
On Wed, Sep 10, 2014 at 03:57:04PM +0200, Johannes Berg wrote:
> On Wed, 2014-09-10 at 19:21 +0530, Sudip Mukherjee wrote:
> > as pr_* macros are more preffered over printk, so printk replaced
> > with corresponding pr_* macros.
> 
> Are you simply running checkpatch on every file and decided to do
> something about it? :)
> 
i am running checkpatch on the patch generated. if i am doing checkpatch
cleanups then that i do it only in the staging.
only exception : printk .. :)

> I'll let Takashi decide whether to take this or not as I no longer care
> about this code, but IMHO this changes is completely pointless since you
> don't also clean up the code to have a common prefix with #define pr_fmt
> and then clean up the callers etc.
> 
i mentioned in the comment that in a future patch we can have pr_fmt,
it was not done in this patch since the changes for this patch is
generated by a script and not manually.
if Takashi accepts this then the next patch will have pr_fmt.

thanks
sudip

> There's a reason pr_* is preferred, but random code changes like this
> aren't it, I think.
> 
> johannes
> 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] sound: aoa: printk replacement

2014-09-10 Thread Johannes Berg
On Wed, 2014-09-10 at 19:21 +0530, Sudip Mukherjee wrote:
> as pr_* macros are more preffered over printk, so printk replaced
> with corresponding pr_* macros.

Are you simply running checkpatch on every file and decided to do
something about it? :)

I'll let Takashi decide whether to take this or not as I no longer care
about this code, but IMHO this changes is completely pointless since you
don't also clean up the code to have a common prefix with #define pr_fmt
and then clean up the callers etc.

There's a reason pr_* is preferred, but random code changes like this
aren't it, I think.

johannes

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] sound: aoa: printk replacement

2014-09-10 Thread Sudip Mukherjee
as pr_* macros are more preffered over printk, so printk replaced
with corresponding pr_* macros.
this patch will generate warning from checkpatch as it only did printk
replacement and didnot fixed the other warning of :
Alignment should match open parenthesis and 
Possible unnecessary 'out of memory' message

Signed-off-by: Sudip Mukherjee 
---

The replacement was done by a bash script to avoid copy paste error. The script 
is as follows :

OLD1="printk(KERN_DEBUG \?"
OLD2="printk(KERN_ERR \?"
OLD3="printk(KERN_INFO \?"
OLD4="printk(KERN_WARNING \?"
OLD5="printk(KERN_ALERT \?"
NEW1="pr_debug("
NEW2="pr_err("
NEW3="pr_info("
NEW4="pr_warn("
NEW5="pr_alert("
find . -type d -mindepth 0 -maxdepth 2 | while read DIR
do
for f in "${DIR}"/*.c
do
sed -i -e "s/$OLD1/$NEW1/g" -e "s/$OLD2/$NEW2/g" -e "s/$OLD3/$NEW3/g" -e 
"s/$OLD4/$NEW4/g" -e "s/$OLD5/$NEW5/g" "$f"
done
done

Takashi : thanks for the -i option.

for the codecs section another future patch can define proper pr_fmt
and eliminate the use of PFX while printing.

this patch is not build tested and i understand that you can reject the patch 
without even seeing at it.

 sound/aoa/codecs/onyx.c | 16 
 sound/aoa/codecs/tas.c  | 10 +-
 sound/aoa/codecs/toonie.c   |  6 +++---
 sound/aoa/core/alsa.c   | 10 +-
 sound/aoa/core/core.c   |  6 +++---
 sound/aoa/core/gpio-pmf.c   |  6 +++---
 sound/aoa/fabrics/layout.c  | 16 
 sound/aoa/soundbus/core.c   |  2 +-
 sound/aoa/soundbus/i2sbus/control.c |  4 ++--
 sound/aoa/soundbus/i2sbus/core.c|  8 
 sound/aoa/soundbus/i2sbus/pcm.c | 28 ++--
 11 files changed, 56 insertions(+), 56 deletions(-)

diff --git a/sound/aoa/codecs/onyx.c b/sound/aoa/codecs/onyx.c
index 401107b..55aa69f 100644
--- a/sound/aoa/codecs/onyx.c
+++ b/sound/aoa/codecs/onyx.c
@@ -873,7 +873,7 @@ static int onyx_init_codec(struct aoa_codec *codec)
int err;
 
if (!onyx->codec.gpio || !onyx->codec.gpio->methods) {
-   printk(KERN_ERR PFX "gpios not assigned!!\n");
+   pr_err(PFX "gpios not assigned!!\n");
return -EINVAL;
}
 
@@ -885,12 +885,12 @@ static int onyx_init_codec(struct aoa_codec *codec)
msleep(1);
 
if (onyx_register_init(onyx)) {
-   printk(KERN_ERR PFX "failed to initialise onyx registers\n");
+   pr_err(PFX "failed to initialise onyx registers\n");
return -ENODEV;
}
 
if (aoa_snd_device_new(SNDRV_DEV_CODEC, onyx, &ops)) {
-   printk(KERN_ERR PFX "failed to create onyx snd device!\n");
+   pr_err(PFX "failed to create onyx snd device!\n");
return -ENODEV;
}
 
@@ -925,7 +925,7 @@ static int onyx_init_codec(struct aoa_codec *codec)
if (onyx->codec.soundbus_dev->attach_codec(onyx->codec.soundbus_dev,
   aoa_get_card(),
   ci, onyx)) {
-   printk(KERN_ERR PFX "error creating onyx pcm\n");
+   pr_err(PFX "error creating onyx pcm\n");
return -ENODEV;
}
 #define ADDCTL(n)  \
@@ -977,7 +977,7 @@ static int onyx_init_codec(struct aoa_codec *codec)
}
}
 #undef ADDCTL
-   printk(KERN_INFO PFX "attached to onyx codec via i2c\n");
+   pr_info(PFX "attached to onyx codec via i2c\n");
 
return 0;
  error:
@@ -991,7 +991,7 @@ static void onyx_exit_codec(struct aoa_codec *codec)
struct onyx *onyx = codec_to_onyx(codec);
 
if (!onyx->codec.soundbus_dev) {
-   printk(KERN_ERR PFX "onyx_exit_codec called without 
soundbus_dev!\n");
+   pr_err(PFX "onyx_exit_codec called without soundbus_dev!\n");
return;
}
onyx->codec.soundbus_dev->detach_codec(onyx->codec.soundbus_dev, onyx);
@@ -1016,7 +1016,7 @@ static int onyx_i2c_probe(struct i2c_client *client,
/* we try to read from register ONYX_REG_CONTROL
 * to check if the codec is present */
if (onyx_read_register(onyx, ONYX_REG_CONTROL, &dummy) != 0) {
-   printk(KERN_ERR PFX "failed to read control register\n");
+   pr_err(PFX "failed to read control register\n");
goto fail;
}
 
@@ -1029,7 +1029,7 @@ static int onyx_i2c_probe(struct i2c_client *client,
if (aoa_codec_register(&onyx->codec)) {
goto fail;
}
-   printk(KERN_DEBUG PFX "created and attached onyx instance\n");
+   pr_debug(PFX "created and attached onyx instance\n");
return 0;
  fail:
kfree(onyx);
diff --git a/sound/aoa/codecs/tas.c b/sound/aoa/codecs/tas.c
index cf3c630..abf6e8a 100644
--- a/sound/aoa/codecs/tas.c
+++ b/sound/aoa/codecs/tas.c
@@ -806,13 +806,13 @@ static

Re: [PATCH v2 1/3] init/main.c: Give init_task a canary

2014-09-10 Thread Aaron Tomlin
On Wed, Sep 10, 2014 at 02:26:54AM -0500, Chuck Ebbert wrote:
> On Tue,  9 Sep 2014 10:42:27 +0100
> Aaron Tomlin  wrote:
> 
> > +void task_stack_end_magic(struct task_struct *tsk)
> > +{
> > +   unsigned long *stackend;
> > +
> > +   stackend = end_of_stack(tsk);
> > +   *stackend = STACK_END_MAGIC;/* for overflow detection */
> > +}
> > +
> 
> For clarity this should probably be called set_task_stack_end_magic().

Agreed.

> And has this been tested on parisc and metag, which use STACK_GROWSUP ?
> I can't see how end_of_stack() as it's defined now could work on those archs.

AFAIU, dup_task_struct() has always done this explicitly.
I see no reason why init_task requires special attention.

-- 
Aaron Tomlin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [Xen-devel] [PATCH v1 08/21] x86/xen/MSI: Use MSI chip framework to configure MSI/MSI-X irq

2014-09-10 Thread David Vrabel
On 09/09/14 03:06, Yijing Wang wrote:
> On 2014/9/5 22:29, David Vrabel wrote:
>> On 05/09/14 11:09, Yijing Wang wrote:
>>> Use MSI chip framework instead of arch MSI functions to configure
>>> MSI/MSI-X irq. So we can manage MSI/MSI-X irq in a unified framework.
>> [...]
>>> --- a/arch/x86/pci/xen.c
>>> +++ b/arch/x86/pci/xen.c
>> [...]
>>> @@ -418,9 +430,9 @@ int __init pci_xen_init(void)
>>>  #endif
>>>  
>>>  #ifdef CONFIG_PCI_MSI
>>> -   x86_msi.setup_msi_irqs = xen_setup_msi_irqs;
>>> -   x86_msi.teardown_msi_irq = xen_teardown_msi_irq;
>>> -   x86_msi.teardown_msi_irqs = xen_teardown_msi_irqs;
>>> +   xen_msi_chip.setup_irqs = xen_setup_msi_irqs;
>>> +   xen_msi_chip.teardown_irqs = xen_teardown_msi_irqs;
>>> +   x86_msi_chip = &xen_msi_chip;
>>> msi_chip.irq_mask = xen_nop_msi_mask;
>>> msi_chip.irq_unmask = xen_nop_msi_mask;
>>
>> Why have these not been changed to set the x86_msi_chip.mask/unmask
>> fields instead?
> 
> Hi David, x86_msi_chip here is struct msi_chip data type, used to configure 
> MSI/MSI-X
> irq. msi_chip above is struct irq_chip data type, represent the MSI irq 
> controller. They are
> not the same object. Their name easily confusing people.

Ok, it all makes sense now.

Acked-by: David Vrabel 

David
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [Xen-devel] [PATCH v1 04/21] x86/xen/MSI: Eliminate arch_msix_mask_irq() and arch_msi_mask_irq()

2014-09-10 Thread David Vrabel
On 05/09/14 11:09, Yijing Wang wrote:
> Commit 0e4ccb150 added two __weak arch functions arch_msix_mask_irq()
> and arch_msi_mask_irq() to fix a bug found when running xen in x86.
> Introduced these two funcntions make MSI code complex. And mask/unmask
> is the irq actions related to interrupt controller, should not use
> weak arch functions to override mask/unmask interfaces. This patch
> reverted commit 0e4ccb150 and export struct irq_chip msi_chip, modify
> msi_chip->irq_mask/irq_unmask() in xen init functions to fix this
> bug for simplicity. Also this is preparation for using struct
> msi_chip instead of weak arch MSI functions in all platforms.

Acked-by: David Vrabel 

But I wonder if it would be better the Xen subsystem to provide its own
struct irq_chip instead of adjusting the fields in the generic x86 one.

David
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/2 v6] powerpc/kvm: common sw breakpoint instr across ppc

2014-09-10 Thread Alexander Graf


On 09.09.14 19:07, Madhavan Srinivasan wrote:
> This patch extends the use of illegal instruction as software
> breakpoint instruction across the ppc platform. Patch extends
> booke program interrupt code to support software breakpoint.
> 
> Signed-off-by: Madhavan Srinivasan 
> ---
> Patch is only compile tested. Will really help if
> someone can try it out and let me know the comments.

I've squashed the following patch into this one to make it work on booke hv.

Alex


diff --git a/arch/powerpc/kvm/bookehv_interrupts.S
b/arch/powerpc/kvm/bookehv_interrupts.S
index c8e4da5..81bd8a0 100644
--- a/arch/powerpc/kvm/bookehv_interrupts.S
+++ b/arch/powerpc/kvm/bookehv_interrupts.S
@@ -238,7 +238,7 @@ kvm_handler BOOKE_INTERRUPT_EXTERNAL, EX_PARAMS(GEN), \
 kvm_handler BOOKE_INTERRUPT_ALIGNMENT, EX_PARAMS(GEN), \
SPRN_SRR0, SPRN_SRR1,(NEED_DEAR | NEED_ESR)
 kvm_handler BOOKE_INTERRUPT_PROGRAM, EX_PARAMS(GEN), \
-   SPRN_SRR0, SPRN_SRR1,NEED_ESR
+   SPRN_SRR0, SPRN_SRR1, (NEED_ESR | NEED_EMU)
 kvm_handler BOOKE_INTERRUPT_FP_UNAVAIL, EX_PARAMS(GEN), \
SPRN_SRR0, SPRN_SRR1, 0
 kvm_handler BOOKE_INTERRUPT_AP_UNAVAIL, EX_PARAMS(GEN), \
@@ -348,7 +348,7 @@ kvm_handler BOOKE_INTERRUPT_INST_STORAGE, SPRN_SRR0,
SPRN_SRR1, NEED_ESR
 kvm_handler BOOKE_INTERRUPT_EXTERNAL, SPRN_SRR0, SPRN_SRR1, 0
 kvm_handler BOOKE_INTERRUPT_ALIGNMENT, \
SPRN_SRR0, SPRN_SRR1, (NEED_DEAR | NEED_ESR)
-kvm_handler BOOKE_INTERRUPT_PROGRAM, SPRN_SRR0, SPRN_SRR1, NEED_ESR
+kvm_handler BOOKE_INTERRUPT_PROGRAM, SPRN_SRR0, SPRN_SRR1, (NEED_ESR |
NEED_EMU)
 kvm_handler BOOKE_INTERRUPT_FP_UNAVAIL, SPRN_SRR0, SPRN_SRR1, 0
 kvm_handler BOOKE_INTERRUPT_SYSCALL, SPRN_SRR0, SPRN_SRR1, 0
 kvm_handler BOOKE_INTERRUPT_AP_UNAVAIL, SPRN_SRR0, SPRN_SRR1, 0
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module

2014-09-10 Thread Markus Pargmann
Hi,

On Wed, Sep 10, 2014 at 06:30:06PM +0800, Shengjiu Wang wrote:
> On Wed, Sep 10, 2014 at 08:21:18AM +0200, Markus Pargmann wrote:
> > On Tue, Sep 09, 2014 at 11:38:05AM -0700, Nicolin Chen wrote:
> > > On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote:
> > > > @@ -1321,7 +1333,11 @@ static int fsl_ssi_probe(struct platform_device 
> > > > *pdev)
> > > > return -ENOMEM;
> > > > }
> > > >  
> > > > -   ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > > > +   if (ssi_private->soc->imx)
> > > > +   ssi_private->regs = 
> > > > devm_regmap_init_mmio_clk(&pdev->dev,
> > > > +   "ipg", iomem, &fsl_ssi_regconfig);
> > > > +   else
> > > > +   ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, 
> > > > iomem,
> > > 
> > > As Markus mentioned, the key point here is to be compatible with those
> > > non-clock-name platforms.
> > > 
> > > I think it would be safer to keep the current code while adding an extra
> > > clk_disable_unprepare() at the end of probe() as a common routine. And
> > > meantime, make sure to have the call for imx only because it seems that
> > > the other platforms do not depend on the clock. //a bit guessing here :)
> > > 
> > > Then we can get a patch like:
> > > open() {
> > > + clk_prepare_enable();
> > >   
> > > }
> > > 
> > > close() {
> > >   
> > > + clk_disable_unprepare()
> > > }
> > > 
> > > probe() {
> > >   clk_get();
> > >   clk_prepare_enable();
> > >   
> > >   if (xxx)
> > > - goto err_xx;
> > > + return ret;
> > >   
> > > + clk_disable_unprepare();
> > >   return 0;
> > > -err_xx:
> > > - clk_disable_unprepare()
> > > }
> > > 
> > > remove() {
> > >   
> > > - clk_disable_unprepare()
> > > }
> > 
> > If I remember correctly, there may be AC97 communication with the codec
> > before any substream is created. That's why we enable the SSI unit right
> > at the beginning for AC97 in fsl_ssi_setup_reg_vals(). So we need to
> > check for AC97 before disabling clocks.
> > 
> > Best regards,
> > 
> > Markus
> 
> hi Markus
> 
> I think if clk_prepare_enable() in startup(), and clk_disable_unprepare()
> in shutdown can meet this requirement, right?

Yes that could work.

> 
> done:
> if (ssi_private->dai_fmt)
> _fsl_ssi_set_dai_fmt(ssi_private, ssi_private->dai_fmt);
> 
> I find that in end of probe, there is setting of dai_fmt. Can we remove this?
> because this setting need to enable ipg clock, and if ac97, ipg clock can't be
> disabled.

No you can't remove it. It is necessary for the DT property "fsl,mode".
Most dts do not have this property anymore because the sound cards are
setting the dai-fmt. But there are still some powerpc dts files that
contain that property.

Best regards,

Markus

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module

2014-09-10 Thread Shengjiu Wang
On Wed, Sep 10, 2014 at 08:21:18AM +0200, Markus Pargmann wrote:
> On Tue, Sep 09, 2014 at 11:38:05AM -0700, Nicolin Chen wrote:
> > On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote:
> > > @@ -1321,7 +1333,11 @@ static int fsl_ssi_probe(struct platform_device 
> > > *pdev)
> > >   return -ENOMEM;
> > >   }
> > >  
> > > - ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > > + if (ssi_private->soc->imx)
> > > + ssi_private->regs = devm_regmap_init_mmio_clk(&pdev->dev,
> > > + "ipg", iomem, &fsl_ssi_regconfig);
> > > + else
> > > + ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > 
> > As Markus mentioned, the key point here is to be compatible with those
> > non-clock-name platforms.
> > 
> > I think it would be safer to keep the current code while adding an extra
> > clk_disable_unprepare() at the end of probe() as a common routine. And
> > meantime, make sure to have the call for imx only because it seems that
> > the other platforms do not depend on the clock. //a bit guessing here :)
> > 
> > Then we can get a patch like:
> > open() {
> > +   clk_prepare_enable();
> > 
> > }
> > 
> > close() {
> > 
> > +   clk_disable_unprepare()
> > }
> > 
> > probe() {
> > clk_get();
> > clk_prepare_enable();
> > 
> > if (xxx)
> > -   goto err_xx;
> > +   return ret;
> > 
> > +   clk_disable_unprepare();
> > return 0;
> > -err_xx:
> > -   clk_disable_unprepare()
> > }
> > 
> > remove() {
> > 
> > -   clk_disable_unprepare()
> > }
> 
> If I remember correctly, there may be AC97 communication with the codec
> before any substream is created. That's why we enable the SSI unit right
> at the beginning for AC97 in fsl_ssi_setup_reg_vals(). So we need to
> check for AC97 before disabling clocks.
> 
> Best regards,
> 
> Markus

hi Markus

I think if clk_prepare_enable() in startup(), and clk_disable_unprepare()
in shutdown can meet this requirement, right?


done:
if (ssi_private->dai_fmt)
_fsl_ssi_set_dai_fmt(ssi_private, ssi_private->dai_fmt);

I find that in end of probe, there is setting of dai_fmt. Can we remove this?
because this setting need to enable ipg clock, and if ac97, ipg clock can't be
disabled.

wang shengjiu
> 
> -- 
> Pengutronix e.K.   | |
> Industrial Linux Solutions | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
> Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module

2014-09-10 Thread Shengjiu Wang
On Tue, Sep 09, 2014 at 12:59:29PM -0700, Nicolin Chen wrote:
> On Tue, Sep 09, 2014 at 02:37:42PM -0500, Timur Tabi wrote:
> > On 09/09/2014 01:38 PM, Nicolin Chen wrote:
> > >make sure to have the call for imx only because it seems that
> > >the other platforms do not depend on the clock.
> > 
> > Although I doubt anyone will every add support for clocks to PowerPC "side"
> > of this driver, I would prefer to avoid IMX-specific changes. Instead, the
> > code should check if a clock is available.  That's why I suggested this
> > change:
> > 
> > -   if (ssi_private->soc->imx)
> > +   if (!IS_ERR(ssi_private->clk))
> 
> Hmm I think the following change may be better?
> 
> probe() {
>   
> + /*
> +  * Initially mark the clock to NULL for all platforms so that later
> +  * clk_prepare_enable() will ignore and return 0 for non-clock cases.
> +  */
> + ssi_private->clk = NULL;
>   .
>   fsl_ssi_imx_probe();
> }
ssi_private is initialized to zero in beginning of probe. I think no need to
add this change here.

wang shengjiu
> 
> In this way, all platforms, not confined to imx any more, will be able
> to call clk_prepare_enable(). Then we don't need an extra platform check
> before calling it.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V1] ASoC: fsl_ssi: refine ipg clock usage in this module

2014-09-10 Thread Shengjiu Wang
On Tue, Sep 09, 2014 at 11:38:05AM -0700, Nicolin Chen wrote:
> On Tue, Sep 09, 2014 at 05:18:07PM +0800, Shengjiu Wang wrote:
> > @@ -1321,7 +1333,11 @@ static int fsl_ssi_probe(struct platform_device 
> > *pdev)
> > return -ENOMEM;
> > }
> >  
> > -   ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> > +   if (ssi_private->soc->imx)
> > +   ssi_private->regs = devm_regmap_init_mmio_clk(&pdev->dev,
> > +   "ipg", iomem, &fsl_ssi_regconfig);
> > +   else
> > +   ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
> 
> As Markus mentioned, the key point here is to be compatible with those
> non-clock-name platforms.
> 
> I think it would be safer to keep the current code while adding an extra
> clk_disable_unprepare() at the end of probe() as a common routine. And
> meantime, make sure to have the call for imx only because it seems that
> the other platforms do not depend on the clock. //a bit guessing here :)
> 
> Then we can get a patch like:
> open() {
> + clk_prepare_enable();
>   
> }
> 
> close() {
>   
> + clk_disable_unprepare()
> }
what is the open() and close()? do you mean the fsl_ssi_startup()
and fsl_ssi_shutdown()?
> 
> probe() {
>   clk_get();
>   clk_prepare_enable();
>   
>   if (xxx)
> - goto err_xx;
> + return ret;
>   
> + clk_disable_unprepare();
>   return 0;
> -err_xx:
> - clk_disable_unprepare()
> }
If this probe() is fsl_ssi_imx_probe(), I think no need to add
clk_prepare_enable() or clk_disable_unprepare(), seems there is no 
registers accessing in this probe.
> 
> remove() {
>   
> - clk_disable_unprepare()
> }
> 
> As long as you make the subject clear as 'Don't enable core/ipg clock
> when SSI's idle', I'm sure you can make them within a single patch.
> 
> And an alternative way for open() and close() is to put those code into
> pm_runtime_resume/suspend() instead (since we might have some internal
> code need to be added by using pm_runtime as well), which would make
> the further code neater IMO.
> 
> Thank you
> Nicolin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2 1/3] init/main.c: Give init_task a canary

2014-09-10 Thread Chuck Ebbert
On Tue,  9 Sep 2014 10:42:27 +0100
Aaron Tomlin  wrote:

> +void task_stack_end_magic(struct task_struct *tsk)
> +{
> + unsigned long *stackend;
> +
> + stackend = end_of_stack(tsk);
> + *stackend = STACK_END_MAGIC;/* for overflow detection */
> +}
> +

For clarity this should probably be called set_task_stack_end_magic().

And has this been tested on parisc and metag, which use STACK_GROWSUP ?
I can't see how end_of_stack() as it's defined now could work on those archs.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev