* [powerpc:merge] BUILD SUCCESS a3dcfbc2456df1a2d416b7d0b627d9cededa1432
From: kernel test robot @ 2020-08-04 5:17 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge
branch HEAD: a3dcfbc2456df1a2d416b7d0b627d9cededa1432 Automatic merge of 'master', 'next' and 'fixes' (2020-08-03 22:40)
elapsed time: 987m
configs tested: 82
configs skipped: 7
The following configs have been built successfully.
More configs may be tested in the coming days.
arm defconfig
arm allyesconfig
arm allmodconfig
arm64 allyesconfig
arm64 defconfig
ia64 generic_defconfig
m68k bvme6000_defconfig
arm pxa168_defconfig
powerpc maple_defconfig
mips malta_qemu_32r6_defconfig
arm64 alldefconfig
arm vt8500_v6_v7_defconfig
arm lart_defconfig
mips omega2p_defconfig
mips loongson1c_defconfig
xtensa alldefconfig
nds32 allnoconfig
powerpc alldefconfig
mips bmips_stb_defconfig
arm magician_defconfig
parisc alldefconfig
mips ip32_defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
nios2 defconfig
arc allyesconfig
c6x allyesconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
powerpc defconfig
i386 randconfig-a004-20200803
i386 randconfig-a005-20200803
i386 randconfig-a001-20200803
i386 randconfig-a002-20200803
i386 randconfig-a003-20200803
i386 randconfig-a006-20200803
x86_64 randconfig-a013-20200803
x86_64 randconfig-a011-20200803
x86_64 randconfig-a012-20200803
x86_64 randconfig-a016-20200803
x86_64 randconfig-a015-20200803
x86_64 randconfig-a014-20200803
i386 randconfig-a011-20200803
i386 randconfig-a012-20200803
i386 randconfig-a015-20200803
i386 randconfig-a014-20200803
i386 randconfig-a013-20200803
i386 randconfig-a016-20200803
riscv allyesconfig
riscv allnoconfig
riscv defconfig
riscv allmodconfig
x86_64 rhel
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_sai: Clean code for synchronize mode
From: Shengjiu Wang @ 2020-08-04 4:22 UTC (permalink / raw)
To: Nicolin Chen
Cc: Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, Mark Brown, linuxppc-dev,
linux-kernel
In-Reply-To: <CAA+D8ANuLQuUO+VsABjt2t1ccK+LGayq13d6EEcV18=4KNaC+w@mail.gmail.com>
On Tue, Aug 4, 2020 at 11:23 AM Shengjiu Wang <shengjiu.wang@gmail.com> wrote:
>
> On Tue, Aug 4, 2020 at 11:00 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> >
> > On Tue, Aug 04, 2020 at 10:35:12AM +0800, Shengjiu Wang wrote:
> > > On Tue, Aug 4, 2020 at 10:11 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> > > >
> > > > On Tue, Aug 04, 2020 at 09:39:44AM +0800, Shengjiu Wang wrote:
> > > > > On Tue, Aug 4, 2020 at 5:57 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> > > > > >
> > > > > > On Mon, Aug 03, 2020 at 04:04:23PM +0800, Shengjiu Wang wrote:
> > > > > >
> > > > > > > > > clock generation. The TCSR.TE is no need to enabled when only RX
> > > > > > > > > is enabled.
> > > > > > > >
> > > > > > > > You are correct if there's only RX running without TX joining.
> > > > > > > > However, that's something we can't guarantee. Then we'd enable
> > > > > > > > TE after RE is enabled, which is against what RM recommends:
> > > > > > > >
> > > > > > > > # From 54.3.3.1 Synchronous mode in IMX6SXRM
> > > > > > > > # If the receiver bit clock and frame sync are to be used by
> > > > > > > > # both the transmitter and receiver, it is recommended that
> > > > > > > > # the receiver is the last enabled and the first disabled.
> > > > > > > >
> > > > > > > > I remember I did this "ugly" design by strictly following what
> > > > > > > > RM says. If hardware team has updated the RM or removed this
> > > > > > > > limitation, please quote in the commit logs.
> > > > > > >
> > > > > > > There is no change in RM and same recommandation.
> > > > > > >
> > > > > > > My change does not violate the RM. The direction which generates
> > > > > > > the clock is still last enabled.
> > > > > >
> > > > > > Using Tx syncing with Rx clock for example,
> > > > > > T1: arecord (non-stop) => set RE
> > > > > > T2: aplay => set TE then RE (but RE is already set at T1)
> > > > > >
> > > > > > Anything that I am missing?
> > > > >
> > > > > This is a good example.
> > > > > We have used this change locally for a long time, so I think it is
> > > > > safe to do this change, a little different with the recommandation.
> > > >
> > > > Any reason for we have to go against the recommendation?
> > >
> > > Previous code will enable TE and RE together even for asynchronous
> > > mode.
> > > And for recommendation, previous code just consider the RX sync with
> > > TX, but still violates the recommendation for TX sync with RX case.
> > > So at least this new change is some kind of improvement.
> >
> > Okay. Let's change it then. Please make sure to update/remove
> > those old comments in the trigger(). And it's probably better
> > to mention that what we do now is a bit different from RM:
> > /*
> > * Enable the opposite direction for synchronous mode
> > * 1. Tx sync with Rx: only set RE for Rx; set TE & RE for Tx
> > * 2. Rx sync with Tx: only set TE for Tx; set RE & TE for Rx
> > *
> > * RM recommends to enable RE after TE for case 1 and to enable
> > * TE after RE for case 2, but we here may not always guarantee
> > * that happens: "arecord 1.wav; aplay 2.wav" in case 1 enables
> > * TE after RE, which is against what RM recommends but should
> > * be safe to do, judging by years of testing results.
> > */
>
> Thank you for the agreement.
>
> >
> > Btw, do we need similar change for TRIGGER_STOP?
>
> This is a good question. It is better to do change for STOP,
> but I am afraid that there is no much test to guarantee the result.
>
> best regards
> wang shengjiu
Maybe we can do this change for STOP.
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 1c0e06bb3783..6e4be398eaee 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -517,6 +517,37 @@ static int fsl_sai_hw_free(struct
snd_pcm_substream *substream,
return 0;
}
+static void fsl_sai_config_disable(struct fsl_sai *sai, bool tx)
+{
+ unsigned int ofs = sai->soc_data->reg_offset;
+ u32 xcsr, count = 100;
+
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
+ FSL_SAI_CSR_TERE, 0);
+
+ /* TERE will remain set till the end of current frame */
+ do {
+ udelay(10);
+ regmap_read(sai->regmap, FSL_SAI_xCSR(tx, ofs), &xcsr);
+ } while (--count && xcsr & FSL_SAI_CSR_TERE);
+
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
+ FSL_SAI_CSR_FR, FSL_SAI_CSR_FR);
+
+ /*
+ * For sai master mode, after several open/close sai,
+ * there will be no frame clock, and can't recover
+ * anymore. Add software reset to fix this issue.
+ * This is a hardware bug, and will be fix in the
+ * next sai version.
+ */
+ if (!sai->is_slave_mode) {
+ /* Software Reset for both Tx and Rx */
+ regmap_write(sai->regmap, FSL_SAI_xCSR(tx, ofs),
FSL_SAI_CSR_SR);
+ /* Clear SR bit to finish the reset */
+ regmap_write(sai->regmap, FSL_SAI_xCSR(tx, ofs), 0);
+ }
+}
static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
struct snd_soc_dai *cpu_dai)
@@ -525,7 +556,7 @@ static int fsl_sai_trigger(struct
snd_pcm_substream *substream, int cmd,
unsigned int ofs = sai->soc_data->reg_offset;
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
- u32 xcsr, count = 100;
+ u32 xcsr;
/*
* Asynchronous mode: Clear SYNC for both Tx and Rx.
@@ -579,43 +610,12 @@ static int fsl_sai_trigger(struct
snd_pcm_substream *substream, int cmd,
/* Check if the opposite FRDE is also disabled */
regmap_read(sai->regmap, FSL_SAI_xCSR(!tx, ofs), &xcsr);
- if (!(xcsr & FSL_SAI_CSR_FRDE)) {
- /* Disable both directions and reset their FIFOs */
- regmap_update_bits(sai->regmap, FSL_SAI_TCSR(ofs),
- FSL_SAI_CSR_TERE, 0);
- regmap_update_bits(sai->regmap, FSL_SAI_RCSR(ofs),
- FSL_SAI_CSR_TERE, 0);
-
- /* TERE will remain set till the end of current frame */
- do {
- udelay(10);
- regmap_read(sai->regmap,
- FSL_SAI_xCSR(tx, ofs), &xcsr);
- } while (--count && xcsr & FSL_SAI_CSR_TERE);
-
- regmap_update_bits(sai->regmap, FSL_SAI_TCSR(ofs),
- FSL_SAI_CSR_FR, FSL_SAI_CSR_FR);
- regmap_update_bits(sai->regmap, FSL_SAI_RCSR(ofs),
- FSL_SAI_CSR_FR, FSL_SAI_CSR_FR);
-
- /*
- * For sai master mode, after several open/close sai,
- * there will be no frame clock, and can't recover
- * anymore. Add software reset to fix this issue.
- * This is a hardware bug, and will be fix in the
- * next sai version.
- */
- if (!sai->is_slave_mode) {
- /* Software Reset for both Tx and Rx */
- regmap_write(sai->regmap, FSL_SAI_TCSR(ofs),
- FSL_SAI_CSR_SR);
- regmap_write(sai->regmap, FSL_SAI_RCSR(ofs),
- FSL_SAI_CSR_SR);
- /* Clear SR bit to finish the reset */
- regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), 0);
- regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), 0);
- }
- }
+ if (sai->synchronous[tx] && !sai->synchronous[!tx] &&
!(xcsr & FSL_SAI_CSR_FRDE))
+ fsl_sai_config_disable(sai, !tx);
+
+ if (sai->synchronous[tx] || !sai->synchronous[!tx] ||
!(xcsr & FSL_SAI_CSR_FRDE))
+ fsl_sai_config_disable(sai, tx);
+
^ permalink raw reply related
* [PATCH 2/2] powerpc/topology: Override cpu_smt_mask
From: Srikar Dronamraju @ 2020-08-04 3:33 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra
Cc: Gautham R Shenoy, Michael Neuling, Vincent Guittot,
Srikar Dronamraju, Rik van Riel, linuxppc-dev, LKML,
Dietmar Eggemann, Thomas Gleixner, Mel Gorman, Valentin Schneider
In-Reply-To: <20200804033307.76111-1-srikar@linux.vnet.ibm.com>
On Power9 a pair of cores can be presented by the firmware as a big-core
for backward compatibility reasons, with 4 threads per (small) core and 8
threads per big-core. cpu_smt_mask() should generally point to the cpu mask
of the (small)core.
In order to maintain userspace backward compatibility (with Power8 chips in
case of Power9) in enterprise Linux systems, the topology_sibling_cpumask
has to be set to big-core. Hence override the default cpu_smt_mask() to be
powerpc specific allowing for better scheduling behaviour on Power.
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/cputhreads.h | 1 -
arch/powerpc/include/asm/smp.h | 13 +++++++++++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/cputhreads.h b/arch/powerpc/include/asm/cputhreads.h
index deb99fd6e060..98c8bd155bf9 100644
--- a/arch/powerpc/include/asm/cputhreads.h
+++ b/arch/powerpc/include/asm/cputhreads.h
@@ -23,7 +23,6 @@
extern int threads_per_core;
extern int threads_per_subcore;
extern int threads_shift;
-extern bool has_big_cores;
extern cpumask_t threads_core_mask;
#else
#define threads_per_core 1
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index 9cd0765633c5..d4bc28accb28 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -131,6 +131,19 @@ static inline struct cpumask *cpu_smallcore_mask(int cpu)
extern int cpu_to_core_id(int cpu);
+extern bool has_big_cores;
+
+#define cpu_smt_mask cpu_smt_mask
+#ifdef CONFIG_SCHED_SMT
+static inline const struct cpumask *cpu_smt_mask(int cpu)
+{
+ if (has_big_cores)
+ return per_cpu(cpu_smallcore_map, cpu);
+
+ return per_cpu(cpu_sibling_map, cpu);
+}
+#endif /* CONFIG_SCHED_SMT */
+
/* Since OpenPIC has only 4 IPIs, we use slightly different message numbers.
*
* Make sure this matches openpic_request_IPIs in open_pic.c, or what shows up
--
2.18.2
^ permalink raw reply related
* [PATCH 1/2] sched/topology: Allow archs to override cpu_smt_mask
From: Srikar Dronamraju @ 2020-08-04 3:33 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra
Cc: Gautham R Shenoy, Michael Neuling, Vincent Guittot,
Srikar Dronamraju, Rik van Riel, linuxppc-dev, LKML,
Dietmar Eggemann, Thomas Gleixner, Mel Gorman, Valentin Schneider
cpu_smt_mask tracks topology_sibling_cpumask. This would be good for
most architectures. One of the users of cpu_smt_mask(), would be to
identify idle-cores. On Power9, a pair of cores can be presented by the
firmware as a big-core for backward compatibility reasons.
In order to maintain userspace backward compatibility with previous
versions of processor, (since Power8 had SMT8 cores), Power9 onwards there
is option to the firmware to advertise a pair of SMT4 cores as a fused
cores (referred to as the big_core mode in the Linux Kernel). On Power9
this pair shares the L2 cache as well. However, from the scheduler's point
of view, a core should be determined by SMT4. The load-balancer already
does this. Hence allow PowerPc architecture to override the default
cpu_smt_mask() to point to the SMT4 cores in a big_core mode.
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
include/linux/topology.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/topology.h b/include/linux/topology.h
index 608fa4aadf0e..ad03df1cc266 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -198,7 +198,7 @@ static inline int cpu_to_mem(int cpu)
#define topology_die_cpumask(cpu) cpumask_of(cpu)
#endif
-#ifdef CONFIG_SCHED_SMT
+#if defined(CONFIG_SCHED_SMT) && !defined(cpu_smt_mask)
static inline const struct cpumask *cpu_smt_mask(int cpu)
{
return topology_sibling_cpumask(cpu);
--
2.18.2
^ permalink raw reply related
* [PATCH] powerpc/pseries/hotplug-cpu: increase wait time for vCPU death
From: Michael Roth @ 2020-08-04 3:29 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nathan Lynch, Cedric Le Goater, Greg Kurz
For a power9 KVM guest with XIVE enabled, running a test loop
where we hotplug 384 vcpus and then unplug them, the following traces
can be seen (generally within a few loops) either from the unplugged
vcpu:
[ 1767.353447] cpu 65 (hwid 65) Ready to die...
[ 1767.952096] Querying DEAD? cpu 66 (66) shows 2
[ 1767.952311] list_del corruption. next->prev should be c00a000002470208, but was c00a000002470048
[ 1767.952322] ------------[ cut here ]------------
[ 1767.952323] kernel BUG at lib/list_debug.c:56!
[ 1767.952325] Oops: Exception in kernel mode, sig: 5 [#1]
[ 1767.952326] LE SMP NR_CPUS=2048 NUMA pSeries
[ 1767.952328] Modules linked in: fuse nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nf_tables_set nft_chain_nat_ipv6 nf_nat_ipv6 nft_chain_route_ipv6 nft_chain_nat_ipv4 nf_nat_ipv4 nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nft_chain_route_ipv4 ip6_tables nft_compat ip_set nf_tables nfnetlink uio_pdrv_genirq ip_tables xfs libcrc32c sd_mod sg xts vmx_crypto virtio_net net_failover failover virtio_scsi dm_multipath dm_mirror dm_region_hash dm_log dm_mod be2iscsi bnx2i cnic uio cxgb4i cxgb4 libcxgbi libcxgb qla4xxx iscsi_boot_sysfs iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi
[ 1767.952352] CPU: 66 PID: 0 Comm: swapper/66 Kdump: loaded Not tainted 4.18.0-221.el8.ppc64le #1
[ 1767.952354] NIP: c0000000007ab50c LR: c0000000007ab508 CTR: 00000000000003ac
[ 1767.952355] REGS: c0000009e5a17840 TRAP: 0700 Not tainted (4.18.0-221.el8.ppc64le)
[ 1767.952355] MSR: 800000000282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 28000842 XER: 20040000
[ 1767.952360] CFAR: c0000000001ffe64 IRQMASK: 1
[ 1767.952360] GPR00: c0000000007ab508 c0000009e5a17ac0 c000000001ac0700 0000000000000054
[ 1767.952360] GPR04: c0000009f056cf90 c0000009f05f5628 c0000009ed40d654 c000000001c90700
[ 1767.952360] GPR08: 0000000000000007 c0000009f0573980 00000009ef2b0000 7562202c38303230
[ 1767.952360] GPR12: 0000000000000000 c0000007fff6ce80 c00a000002470208 0000000000000000
[ 1767.952360] GPR16: 0000000000000001 c0000009f05fbb00 0000000000000800 c0000009ff3d4980
[ 1767.952360] GPR20: c0000009f05fbb10 5deadbeef0000100 5deadbeef0000200 0000000000187961
[ 1767.952360] GPR24: c0000009e5a17b78 0000000000000000 0000000000000001 ffffffffffffffff
[ 1767.952360] GPR28: c00a000002470200 c0000009f05fbb10 c0000009f05fbb10 0000000000000000
[ 1767.952375] NIP [c0000000007ab50c] __list_del_entry_valid+0xac/0x100
[ 1767.952376] LR [c0000000007ab508] __list_del_entry_valid+0xa8/0x100
[ 1767.952377] Call Trace:
[ 1767.952378] [c0000009e5a17ac0] [c0000000007ab508] __list_del_entry_valid+0xa8/0x100 (unreliable)
[ 1767.952381] [c0000009e5a17b20] [c000000000476e18] free_pcppages_bulk+0x1f8/0x940
[ 1767.952383] [c0000009e5a17c20] [c00000000047a9a0] free_unref_page+0xd0/0x100
[ 1767.952386] [c0000009e5a17c50] [c0000000000bc2a8] xive_spapr_cleanup_queue+0x148/0x1b0
[ 1767.952388] [c0000009e5a17cf0] [c0000000000b96dc] xive_teardown_cpu+0x1bc/0x240
[ 1767.952391] [c0000009e5a17d30] [c00000000010bf28] pseries_mach_cpu_die+0x78/0x2f0
[ 1767.952393] [c0000009e5a17de0] [c00000000005c8d8] cpu_die+0x48/0x70
[ 1767.952394] [c0000009e5a17e00] [c000000000021cf0] arch_cpu_idle_dead+0x20/0x40
[ 1767.952397] [c0000009e5a17e20] [c0000000001b4294] do_idle+0x2f4/0x4c0
[ 1767.952399] [c0000009e5a17ea0] [c0000000001b46a8] cpu_startup_entry+0x38/0x40
[ 1767.952400] [c0000009e5a17ed0] [c00000000005c43c] start_secondary+0x7bc/0x8f0
[ 1767.952403] [c0000009e5a17f90] [c00000000000ac70] start_secondary_prolog+0x10/0x14
or on the worker thread handling the unplug:
[ 1538.253044] pseries-hotplug-cpu: Attempting to remove CPU <NULL>, drc index: 1000013a
[ 1538.360259] Querying DEAD? cpu 314 (314) shows 2
[ 1538.360736] BUG: Bad page state in process kworker/u768:3 pfn:95de1
[ 1538.360746] cpu 314 (hwid 314) Ready to die...
[ 1538.360784] page:c00a000002577840 refcount:0 mapcount:-128 mapping:0000000000000000 index:0x0
[ 1538.361881] flags: 0x5ffffc00000000()
[ 1538.361908] raw: 005ffffc00000000 5deadbeef0000100 5deadbeef0000200 0000000000000000
[ 1538.361955] raw: 0000000000000000 0000000000000000 00000000ffffff7f 0000000000000000
[ 1538.362002] page dumped because: nonzero mapcount
[ 1538.362033] Modules linked in: kvm xt_CHECKSUM ipt_MASQUERADE xt_conntrack ipt_REJECT nft_counter nf_nat_tftp nft_objref nf_conntrack_tftp tun bridge stp llc nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nf_tables_set nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip6_tables nft_compat ip_set nf_tables nfnetlink sunrpc xts vmx_crypto ip_tables xfs libcrc32c sd_mod sg virtio_net net_failover virtio_scsi failover dm_mirror dm_region_hash dm_log dm_mod
[ 1538.362613] CPU: 0 PID: 548 Comm: kworker/u768:3 Kdump: loaded Not tainted 4.18.0-224.el8.bz1856588.ppc64le #1
[ 1538.362687] Workqueue: pseries hotplug workque pseries_hp_work_fn
[ 1538.362725] Call Trace:
[ 1538.362743] [c0000009d4adf590] [c000000000e0e0fc] dump_stack+0xb0/0xf4 (unreliable)
[ 1538.362789] [c0000009d4adf5d0] [c000000000475dfc] bad_page+0x12c/0x1b0
[ 1538.362827] [c0000009d4adf660] [c0000000004784bc] free_pcppages_bulk+0x5bc/0x940
[ 1538.362871] [c0000009d4adf760] [c000000000478c38] page_alloc_cpu_dead+0x118/0x120
[ 1538.362918] [c0000009d4adf7b0] [c00000000015b898] cpuhp_invoke_callback.constprop.5+0xb8/0x760
[ 1538.362969] [c0000009d4adf820] [c00000000015eee8] _cpu_down+0x188/0x340
[ 1538.363007] [c0000009d4adf890] [c00000000015d75c] cpu_down+0x5c/0xa0
[ 1538.363045] [c0000009d4adf8c0] [c00000000092c544] cpu_subsys_offline+0x24/0x40
[ 1538.363091] [c0000009d4adf8e0] [c0000000009212f0] device_offline+0xf0/0x130
[ 1538.363129] [c0000009d4adf920] [c00000000010aee4] dlpar_offline_cpu+0x1c4/0x2a0
[ 1538.363174] [c0000009d4adf9e0] [c00000000010b2f8] dlpar_cpu_remove+0xb8/0x190
[ 1538.363219] [c0000009d4adfa60] [c00000000010b4fc] dlpar_cpu_remove_by_index+0x12c/0x150
[ 1538.363264] [c0000009d4adfaf0] [c00000000010ca24] dlpar_cpu+0x94/0x800
[ 1538.363302] [c0000009d4adfc00] [c000000000102cc8] pseries_hp_work_fn+0x128/0x1e0
[ 1538.363347] [c0000009d4adfc70] [c00000000018aa84] process_one_work+0x304/0x5d0
[ 1538.363394] [c0000009d4adfd10] [c00000000018b5cc] worker_thread+0xcc/0x7a0
[ 1538.363433] [c0000009d4adfdc0] [c00000000019567c] kthread+0x1ac/0x1c0
[ 1538.363469] [c0000009d4adfe30] [c00000000000b7dc] ret_from_kernel_thread+0x5c/0x80
The latter trace is due to the following sequence:
page_alloc_cpu_dead
drain_pages
drain_pages_zone
free_pcppages_bulk
where drain_pages() in this case is called under the assumption that
the unplugged cpu is no longer executing. To ensure that is the case,
and early call is made to __cpu_die()->pseries_cpu_die(), which runs
a loop that waits for the cpu to reach a halted state by polling its
status via query-cpu-stopped-state RTAS calls. It only polls for
25 iterations before giving up, however, and in the trace above this
results in the following being printed only .1 seconds after the
hotplug worker thread begins processing the unplug request:
[ 1538.253044] pseries-hotplug-cpu: Attempting to remove CPU <NULL>, drc index: 1000013a
[ 1538.360259] Querying DEAD? cpu 314 (314) shows 2
At that point the worker thread assumes the unplugged CPU is in some
unknown/dead state and procedes with the cleanup, causing the race with
the XIVE cleanup code executed by the unplugged CPU.
Fix this by inserting an msleep() after each RTAS call to avoid
pseries_cpu_die() returning prematurely, and double the number of
attempts so we wait at least a total of 5 seconds. While this isn't an
ideal solution, it is similar to how we dealt with a similar issue for
cede_offline mode in the past (940ce422a3).
Fixes: eac1e731b59ee ("powerpc/xive: guest exploitation of the XIVE interrupt controller")
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1856588
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Cedric Le Goater <clg@kaod.org>
Cc: Greg Kurz <groug@kaod.org>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/hotplug-cpu.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index c6e0d8abf75e..3cb172758052 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -111,13 +111,12 @@ static void pseries_cpu_die(unsigned int cpu)
int cpu_status = 1;
unsigned int pcpu = get_hard_smp_processor_id(cpu);
- for (tries = 0; tries < 25; tries++) {
+ for (tries = 0; tries < 50; tries++) {
cpu_status = smp_query_cpu_stopped(pcpu);
if (cpu_status == QCSS_STOPPED ||
cpu_status == QCSS_HARDWARE_ERROR)
break;
- cpu_relax();
-
+ msleep(100);
}
if (cpu_status != 0) {
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] ASoC: fsl_sai: Clean code for synchronize mode
From: Shengjiu Wang @ 2020-08-04 3:23 UTC (permalink / raw)
To: Nicolin Chen
Cc: Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, Mark Brown, linuxppc-dev,
linux-kernel
In-Reply-To: <20200804030004.GA27028@Asurada-Nvidia>
On Tue, Aug 4, 2020 at 11:00 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Tue, Aug 04, 2020 at 10:35:12AM +0800, Shengjiu Wang wrote:
> > On Tue, Aug 4, 2020 at 10:11 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> > >
> > > On Tue, Aug 04, 2020 at 09:39:44AM +0800, Shengjiu Wang wrote:
> > > > On Tue, Aug 4, 2020 at 5:57 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> > > > >
> > > > > On Mon, Aug 03, 2020 at 04:04:23PM +0800, Shengjiu Wang wrote:
> > > > >
> > > > > > > > clock generation. The TCSR.TE is no need to enabled when only RX
> > > > > > > > is enabled.
> > > > > > >
> > > > > > > You are correct if there's only RX running without TX joining.
> > > > > > > However, that's something we can't guarantee. Then we'd enable
> > > > > > > TE after RE is enabled, which is against what RM recommends:
> > > > > > >
> > > > > > > # From 54.3.3.1 Synchronous mode in IMX6SXRM
> > > > > > > # If the receiver bit clock and frame sync are to be used by
> > > > > > > # both the transmitter and receiver, it is recommended that
> > > > > > > # the receiver is the last enabled and the first disabled.
> > > > > > >
> > > > > > > I remember I did this "ugly" design by strictly following what
> > > > > > > RM says. If hardware team has updated the RM or removed this
> > > > > > > limitation, please quote in the commit logs.
> > > > > >
> > > > > > There is no change in RM and same recommandation.
> > > > > >
> > > > > > My change does not violate the RM. The direction which generates
> > > > > > the clock is still last enabled.
> > > > >
> > > > > Using Tx syncing with Rx clock for example,
> > > > > T1: arecord (non-stop) => set RE
> > > > > T2: aplay => set TE then RE (but RE is already set at T1)
> > > > >
> > > > > Anything that I am missing?
> > > >
> > > > This is a good example.
> > > > We have used this change locally for a long time, so I think it is
> > > > safe to do this change, a little different with the recommandation.
> > >
> > > Any reason for we have to go against the recommendation?
> >
> > Previous code will enable TE and RE together even for asynchronous
> > mode.
> > And for recommendation, previous code just consider the RX sync with
> > TX, but still violates the recommendation for TX sync with RX case.
> > So at least this new change is some kind of improvement.
>
> Okay. Let's change it then. Please make sure to update/remove
> those old comments in the trigger(). And it's probably better
> to mention that what we do now is a bit different from RM:
> /*
> * Enable the opposite direction for synchronous mode
> * 1. Tx sync with Rx: only set RE for Rx; set TE & RE for Tx
> * 2. Rx sync with Tx: only set TE for Tx; set RE & TE for Rx
> *
> * RM recommends to enable RE after TE for case 1 and to enable
> * TE after RE for case 2, but we here may not always guarantee
> * that happens: "arecord 1.wav; aplay 2.wav" in case 1 enables
> * TE after RE, which is against what RM recommends but should
> * be safe to do, judging by years of testing results.
> */
Thank you for the agreement.
>
> Btw, do we need similar change for TRIGGER_STOP?
This is a good question. It is better to do change for STOP,
but I am afraid that there is no much test to guarantee the result.
best regards
wang shengjiu
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_sai: Clean code for synchronize mode
From: Nicolin Chen @ 2020-08-04 3:00 UTC (permalink / raw)
To: Shengjiu Wang
Cc: Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, Mark Brown, linuxppc-dev,
linux-kernel
In-Reply-To: <CAA+D8ANagfMXPAkK4-vBDY9rZMukVUXs8FfBCHS0avXt57pekA@mail.gmail.com>
On Tue, Aug 04, 2020 at 10:35:12AM +0800, Shengjiu Wang wrote:
> On Tue, Aug 4, 2020 at 10:11 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> >
> > On Tue, Aug 04, 2020 at 09:39:44AM +0800, Shengjiu Wang wrote:
> > > On Tue, Aug 4, 2020 at 5:57 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> > > >
> > > > On Mon, Aug 03, 2020 at 04:04:23PM +0800, Shengjiu Wang wrote:
> > > >
> > > > > > > clock generation. The TCSR.TE is no need to enabled when only RX
> > > > > > > is enabled.
> > > > > >
> > > > > > You are correct if there's only RX running without TX joining.
> > > > > > However, that's something we can't guarantee. Then we'd enable
> > > > > > TE after RE is enabled, which is against what RM recommends:
> > > > > >
> > > > > > # From 54.3.3.1 Synchronous mode in IMX6SXRM
> > > > > > # If the receiver bit clock and frame sync are to be used by
> > > > > > # both the transmitter and receiver, it is recommended that
> > > > > > # the receiver is the last enabled and the first disabled.
> > > > > >
> > > > > > I remember I did this "ugly" design by strictly following what
> > > > > > RM says. If hardware team has updated the RM or removed this
> > > > > > limitation, please quote in the commit logs.
> > > > >
> > > > > There is no change in RM and same recommandation.
> > > > >
> > > > > My change does not violate the RM. The direction which generates
> > > > > the clock is still last enabled.
> > > >
> > > > Using Tx syncing with Rx clock for example,
> > > > T1: arecord (non-stop) => set RE
> > > > T2: aplay => set TE then RE (but RE is already set at T1)
> > > >
> > > > Anything that I am missing?
> > >
> > > This is a good example.
> > > We have used this change locally for a long time, so I think it is
> > > safe to do this change, a little different with the recommandation.
> >
> > Any reason for we have to go against the recommendation?
>
> Previous code will enable TE and RE together even for asynchronous
> mode.
> And for recommendation, previous code just consider the RX sync with
> TX, but still violates the recommendation for TX sync with RX case.
> So at least this new change is some kind of improvement.
Okay. Let's change it then. Please make sure to update/remove
those old comments in the trigger(). And it's probably better
to mention that what we do now is a bit different from RM:
/*
* Enable the opposite direction for synchronous mode
* 1. Tx sync with Rx: only set RE for Rx; set TE & RE for Tx
* 2. Rx sync with Tx: only set TE for Tx; set RE & TE for Rx
*
* RM recommends to enable RE after TE for case 1 and to enable
* TE after RE for case 2, but we here may not always guarantee
* that happens: "arecord 1.wav; aplay 2.wav" in case 1 enables
* TE after RE, which is against what RM recommends but should
* be safe to do, judging by years of testing results.
*/
Btw, do we need similar change for TRIGGER_STOP?
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_sai: Clean code for synchronize mode
From: Shengjiu Wang @ 2020-08-04 2:35 UTC (permalink / raw)
To: Nicolin Chen
Cc: Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, Mark Brown, linuxppc-dev,
linux-kernel
In-Reply-To: <20200804021114.GA15390@Asurada-Nvidia>
On Tue, Aug 4, 2020 at 10:11 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Tue, Aug 04, 2020 at 09:39:44AM +0800, Shengjiu Wang wrote:
> > On Tue, Aug 4, 2020 at 5:57 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> > >
> > > On Mon, Aug 03, 2020 at 04:04:23PM +0800, Shengjiu Wang wrote:
> > >
> > > > > > clock generation. The TCSR.TE is no need to enabled when only RX
> > > > > > is enabled.
> > > > >
> > > > > You are correct if there's only RX running without TX joining.
> > > > > However, that's something we can't guarantee. Then we'd enable
> > > > > TE after RE is enabled, which is against what RM recommends:
> > > > >
> > > > > # From 54.3.3.1 Synchronous mode in IMX6SXRM
> > > > > # If the receiver bit clock and frame sync are to be used by
> > > > > # both the transmitter and receiver, it is recommended that
> > > > > # the receiver is the last enabled and the first disabled.
> > > > >
> > > > > I remember I did this "ugly" design by strictly following what
> > > > > RM says. If hardware team has updated the RM or removed this
> > > > > limitation, please quote in the commit logs.
> > > >
> > > > There is no change in RM and same recommandation.
> > > >
> > > > My change does not violate the RM. The direction which generates
> > > > the clock is still last enabled.
> > >
> > > Using Tx syncing with Rx clock for example,
> > > T1: arecord (non-stop) => set RE
> > > T2: aplay => set TE then RE (but RE is already set at T1)
> > >
> > > Anything that I am missing?
> >
> > This is a good example.
> > We have used this change locally for a long time, so I think it is
> > safe to do this change, a little different with the recommandation.
>
> Any reason for we have to go against the recommendation?
Previous code will enable TE and RE together even for asynchronous
mode.
And for recommendation, previous code just consider the RX sync with
TX, but still violates the recommendation for TX sync with RX case.
So at least this new change is some kind of improvement.
best regards
wang shengjiu
^ permalink raw reply
* Re: [PATCH 5/6] powerpc/powernv/pci: Drop unused parent variable
From: Joel Stanley @ 2020-08-04 2:19 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linuxppc-dev
In-Reply-To: <20200804005410.146094-6-oohall@gmail.com>
On Tue, 4 Aug 2020 at 01:06, Oliver O'Halloran <oohall@gmail.com> wrote:
>
> The "parent" variable in pnv_pci_ioda_configure_pe() isn't used for
> anything anymore and can be dropped.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply
* Re: [PATCH 4/6] powerpc/powernv: Fix spurious kerneldoc warnings in opal-prd.c
From: Joel Stanley @ 2020-08-04 2:18 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linuxppc-dev
In-Reply-To: <20200804005410.146094-5-oohall@gmail.com>
On Tue, 4 Aug 2020 at 01:03, Oliver O'Halloran <oohall@gmail.com> wrote:
>
> Comments opening with /** are parsed by kerneldoc and this causes the
> following warning to be printed:
>
> arch/powerpc/platforms/powernv/opal-prd.c:31: warning: cannot understand
> function prototype: 'struct opal_prd_msg_queue_item '
>
> opal_prd_mesg_queue_item is an internal data structure so there's no real
> need for it to be documented at all. Fix up the comment to squash the
> warning.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply
* Re: [PATCH 3/6] powerpc/powernv: Staticify functions without prototypes
From: Joel Stanley @ 2020-08-04 2:17 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linuxppc-dev
In-Reply-To: <20200804005410.146094-4-oohall@gmail.com>
On Tue, 4 Aug 2020 at 01:01, Oliver O'Halloran <oohall@gmail.com> wrote:
>
> There's a few scattered in the powernv platform.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -38,7 +38,7 @@
>
> static int eeh_event_irq = -EINVAL;
>
> -void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
> +static void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
> {
> dev_dbg(&pdev->dev, "EEH: Setting up device\n");
> eeh_probe_device(pdev);
This one could even be deleted as eeh_probe_device has it's own dev_dbg.
Reviewed-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_sai: Clean code for synchronize mode
From: Nicolin Chen @ 2020-08-04 2:11 UTC (permalink / raw)
To: Shengjiu Wang
Cc: Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, Mark Brown, linuxppc-dev,
linux-kernel
In-Reply-To: <CAA+D8ANQxnvR2bOyHVRs5h2NJhMeVh4gjLPknaz7aQ86MtL0sQ@mail.gmail.com>
On Tue, Aug 04, 2020 at 09:39:44AM +0800, Shengjiu Wang wrote:
> On Tue, Aug 4, 2020 at 5:57 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> >
> > On Mon, Aug 03, 2020 at 04:04:23PM +0800, Shengjiu Wang wrote:
> >
> > > > > clock generation. The TCSR.TE is no need to enabled when only RX
> > > > > is enabled.
> > > >
> > > > You are correct if there's only RX running without TX joining.
> > > > However, that's something we can't guarantee. Then we'd enable
> > > > TE after RE is enabled, which is against what RM recommends:
> > > >
> > > > # From 54.3.3.1 Synchronous mode in IMX6SXRM
> > > > # If the receiver bit clock and frame sync are to be used by
> > > > # both the transmitter and receiver, it is recommended that
> > > > # the receiver is the last enabled and the first disabled.
> > > >
> > > > I remember I did this "ugly" design by strictly following what
> > > > RM says. If hardware team has updated the RM or removed this
> > > > limitation, please quote in the commit logs.
> > >
> > > There is no change in RM and same recommandation.
> > >
> > > My change does not violate the RM. The direction which generates
> > > the clock is still last enabled.
> >
> > Using Tx syncing with Rx clock for example,
> > T1: arecord (non-stop) => set RE
> > T2: aplay => set TE then RE (but RE is already set at T1)
> >
> > Anything that I am missing?
>
> This is a good example.
> We have used this change locally for a long time, so I think it is
> safe to do this change, a little different with the recommandation.
Any reason for we have to go against the recommendation?
^ permalink raw reply
* Re: [PATCH 1/6] powerpc/powernv/smp: Fix spurious DBG() warning
From: Joel Stanley @ 2020-08-04 2:07 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linuxppc-dev
In-Reply-To: <20200804005410.146094-2-oohall@gmail.com>
On Tue, 4 Aug 2020 at 00:57, Oliver O'Halloran <oohall@gmail.com> wrote:
>
> When building with W=1 we get the following warning:
>
> arch/powerpc/platforms/powernv/smp.c: In function ‘pnv_smp_cpu_kill_self’:
> arch/powerpc/platforms/powernv/smp.c:276:16: error: suggest braces around
> empty body in an ‘if’ statement [-Werror=empty-body]
> 276 | cpu, srr1);
> | ^
> cc1: all warnings being treated as errors
>
> The full context is this block:
>
> if (srr1 && !generic_check_cpu_restart(cpu))
> DBG("CPU%d Unexpected exit while offline srr1=%lx!\n",
> cpu, srr1);
>
> When building with DEBUG undefined DBG() expands to nothing and GCC emits
> the warning due to the lack of braces around an empty statement.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> We could add the braces too. That might even be better since it's a multi-line
> if block even though it's only a single statement.
Or you could put it all on one line, now that our 120 line overlords
have taken over.
Reviewed-by: Joel Stanley <joel@jms.id.au>
Messy:
$ git grep "define DBG(" arch/powerpc/ |grep -v print
arch/powerpc/kernel/crash_dump.c:#define DBG(fmt...)
arch/powerpc/kernel/iommu.c:#define DBG(...)
arch/powerpc/kernel/legacy_serial.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/kernel/prom.c:#define DBG(fmt...)
arch/powerpc/kernel/setup-common.c:#define DBG(fmt...)
arch/powerpc/kernel/setup_32.c:#define DBG(fmt...)
arch/powerpc/kernel/smp.c:#define DBG(fmt...)
arch/powerpc/kernel/vdso.c:#define DBG(fmt...)
arch/powerpc/kvm/book3s_hv_rm_xive.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/mm/book3s64/hash_utils.c:#define DBG(fmt...)
arch/powerpc/platforms/83xx/mpc832x_mds.c:#define DBG(fmt...)
arch/powerpc/platforms/83xx/mpc832x_rdb.c:#define DBG(fmt...)
arch/powerpc/platforms/83xx/mpc836x_mds.c:#define DBG(fmt...)
arch/powerpc/platforms/85xx/mpc85xx_ds.c:#define DBG(fmt, args...)
arch/powerpc/platforms/85xx/mpc85xx_mds.c:#define DBG(fmt...)
arch/powerpc/platforms/85xx/mpc85xx_rdb.c:#define DBG(fmt, args...)
arch/powerpc/platforms/86xx/mpc86xx_hpcn.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/platforms/cell/setup.c:#define DBG(fmt...)
arch/powerpc/platforms/cell/smp.c:#define DBG(fmt...)
arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c:#define DBG(fmt...)
do { } while(0)
arch/powerpc/platforms/maple/pci.c:#define DBG(x...)
arch/powerpc/platforms/maple/setup.c:#define DBG(fmt...)
arch/powerpc/platforms/maple/time.c:#define DBG(x...)
arch/powerpc/platforms/powermac/bootx_init.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/platforms/powermac/feature.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/low_i2c.c:#define DBG(x...) do {\
arch/powerpc/platforms/powermac/low_i2c.c:#define DBG(x...)
arch/powerpc/platforms/powermac/nvram.c:#define DBG(x...)
arch/powerpc/platforms/powermac/pci.c:#define DBG(x...)
arch/powerpc/platforms/powermac/pfunc_base.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/pfunc_core.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/smp.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/time.c:#define DBG(x...)
arch/powerpc/platforms/powernv/smp.c:#define DBG(fmt...)
arch/powerpc/sysdev/dart_iommu.c:#define DBG(...)
arch/powerpc/sysdev/ge/ge_pic.c:#define DBG(fmt...) do { } while (0)
arch/powerpc/sysdev/mpic.c:#define DBG(fmt...)
arch/powerpc/sysdev/tsi108_dev.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/sysdev/tsi108_pci.c:#define DBG(x...)
> ---
> arch/powerpc/platforms/powernv/smp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
> index b2ba3e95bda7..bbf361f23ae8 100644
> --- a/arch/powerpc/platforms/powernv/smp.c
> +++ b/arch/powerpc/platforms/powernv/smp.c
> @@ -43,7 +43,7 @@
> #include <asm/udbg.h>
> #define DBG(fmt...) udbg_printf(fmt)
> #else
> -#define DBG(fmt...)
> +#define DBG(fmt...) do { } while (0)
> #endif
>
> static void pnv_smp_setup_cpu(int cpu)
> --
> 2.26.2
>
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_sai: Clean code for synchronize mode
From: Shengjiu Wang @ 2020-08-04 1:39 UTC (permalink / raw)
To: Nicolin Chen
Cc: Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, Mark Brown, linuxppc-dev,
linux-kernel
In-Reply-To: <20200803215735.GA5461@Asurada-Nvidia>
On Tue, Aug 4, 2020 at 5:57 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Mon, Aug 03, 2020 at 04:04:23PM +0800, Shengjiu Wang wrote:
>
> > > > clock generation. The TCSR.TE is no need to enabled when only RX
> > > > is enabled.
> > >
> > > You are correct if there's only RX running without TX joining.
> > > However, that's something we can't guarantee. Then we'd enable
> > > TE after RE is enabled, which is against what RM recommends:
> > >
> > > # From 54.3.3.1 Synchronous mode in IMX6SXRM
> > > # If the receiver bit clock and frame sync are to be used by
> > > # both the transmitter and receiver, it is recommended that
> > > # the receiver is the last enabled and the first disabled.
> > >
> > > I remember I did this "ugly" design by strictly following what
> > > RM says. If hardware team has updated the RM or removed this
> > > limitation, please quote in the commit logs.
> >
> > There is no change in RM and same recommandation.
> >
> > My change does not violate the RM. The direction which generates
> > the clock is still last enabled.
>
> Using Tx syncing with Rx clock for example,
> T1: arecord (non-stop) => set RE
> T2: aplay => set TE then RE (but RE is already set at T1)
>
> Anything that I am missing?
This is a good example.
We have used this change locally for a long time, so I think it is
safe to do this change, a little different with the recommandation.
>
> > > > + if (!sai->synchronous[TX] && sai->synchronous[RX] && !tx) {
> > > > + regmap_update_bits(sai->regmap, FSL_SAI_xCSR((!tx), ofs),
> > > > + FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
> > > > + } else if (!sai->synchronous[RX] && sai->synchronous[TX] && tx) {
> > > > + regmap_update_bits(sai->regmap, FSL_SAI_xCSR((!tx), ofs),
> > > > + FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
> > >
> > > Two identical regmap_update_bits calls -- both on !tx (RX?)
> > The content for regmap_update_bits is the same, but the precondition
> > is different.
> > The first one is for tx=false and enable TCSR.TE. (TX generate clock)
> > The second one is for tx=true and enable RSCR.RE (RX generate clock)
>
> Why not merge them?
>
> + if ((!sai->synchronous[TX] && sai->synchronous[RX] && !tx) ||
> + ((!sai->synchronous[RX] && sai->synchronous[TX] && tx) {
oh, yes, good point!
best regards
wang shengjiu
^ permalink raw reply
* Re: [merge] Build failure selftest/powerpc/mm/pkey_exec_prot
From: Michael Ellerman @ 2020-08-04 1:08 UTC (permalink / raw)
To: Sandipan Das; +Cc: Sachin Sant, linuxppc-dev
In-Reply-To: <3ada0268-9474-5ee6-b1aa-82e8d245615d@linux.ibm.com>
Sandipan Das <sandipan@linux.ibm.com> writes:
> On 03/08/20 4:32 pm, Michael Ellerman wrote:
>> Sachin Sant <sachinp@linux.vnet.ibm.com> writes:
>>>> On 02-Aug-2020, at 10:58 PM, Sandipan Das <sandipan@linux.ibm.com> wrote:
>>>> On 02/08/20 4:45 pm, Sachin Sant wrote:
>>>>> pkey_exec_prot test from linuxppc merge branch (3f68564f1f5a) fails to
>>>>> build due to following error:
>>>>>
>>>>> gcc -std=gnu99 -O2 -Wall -Werror -DGIT_VERSION='"v5.8-rc7-1276-g3f68564f1f5a"' -I/home/sachin/linux/tools/testing/selftests/powerpc/include -m64 pkey_exec_prot.c /home/sachin/linux/tools/testing/selftests/kselftest_harness.h /home/sachin/linux/tools/testing/selftests/kselftest.h ../harness.c ../utils.c -o /home/sachin/linux/tools/testing/selftests/powerpc/mm/pkey_exec_prot
>>>>> In file included from pkey_exec_prot.c:18:
>>>>> /home/sachin/linux/tools/testing/selftests/powerpc/include/pkeys.h:34: error: "SYS_pkey_mprotect" redefined [-Werror]
>>>>> #define SYS_pkey_mprotect 386
>>>>>
>>>>> In file included from /usr/include/sys/syscall.h:31,
>>>>> from /home/sachin/linux/tools/testing/selftests/powerpc/include/utils.h:47,
>>>>> from /home/sachin/linux/tools/testing/selftests/powerpc/include/pkeys.h:12,
>>>>> from pkey_exec_prot.c:18:
>>>>> /usr/include/bits/syscall.h:1583: note: this is the location of the previous definition
>>>>> # define SYS_pkey_mprotect __NR_pkey_mprotect
>>>>>
>>>>> commit 128d3d021007 introduced this error.
>>>>> selftests/powerpc: Move pkey helpers to headers
>>>>>
>>>>> Possibly the # defines for sys calls can be retained in pkey_exec_prot.c or
>>>>>
>>>>
>>>> I am unable to reproduce this on the latest merge branch (HEAD at f59195f7faa4).
>>>> I don't see any redefinitions in pkey_exec_prot.c either.
>>>>
>>>
>>> I can still see this problem on latest merge branch.
>>> I have following gcc version
>>>
>>> gcc version 8.3.1 20191121
>>
>> What libc version? Or just the distro & version?
>
> Sachin observed this on RHEL 8.2 with glibc-2.28.
> I couldn't reproduce it on Ubuntu 20.04 and Fedora 32 and both these distros
> are using glibc-2.31.
OK odd. Usually it's newer glibc that hits this problem.
I guess on RHEL 8.2 we're getting the asm-generic version? But that
would be quite wrong if that's what's happening.
cheers
^ permalink raw reply
* [PATCH 6/6] powerpc/nx: Don't pack struct coprocessor_request_block
From: Oliver O'Halloran @ 2020-08-04 0:54 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran, Haren Myneni, Dan Streetman
In-Reply-To: <20200804005410.146094-1-oohall@gmail.com>
Building with W=1 results in the following warning:
In file included from arch/powerpc/platforms/powernv/vas-fault.c:16:
./arch/powerpc/include/asm/icswx.h:159:1: error: alignment 1 of ‘struct
coprocessor_request_block’ is less than 16 [-Werror=packed-not-aligned]
159 | } __packed;
| ^
./arch/powerpc/include/asm/icswx.h:159:1: error: alignment 1 of ‘struct
coprocessor_request_block’ is less than 16 [-Werror=packed-not-aligned]
./arch/powerpc/include/asm/icswx.h:159:1: error: alignment 1 of ‘struct
coprocessor_request_block’ is less than 16 [-Werror=packed-not-aligned]
./arch/powerpc/include/asm/icswx.h:159:1: error: alignment 1 of ‘struct
coprocessor_request_block’ is less than 16 [-Werror=packed-not-aligned]
cc1: all warnings being treated as errors
This happens because coprocessor_request_block includes several
sub-structures with an alignment specified using the __aligned(XX)
attribute. The problem comes from coprocessor_request_block having the
__packed attribute. Packing the structure causes the preferred alignment of
the nested structures to be ignored and we get the warnings as a result.
This isn't a problem in practice since the struct is defined with explicit
padding in the form of reserved fields, but we'd like to get rid of the
spurious warnings. The simplest solution is to remove the packed attribute
and use a BUILD_BUG_ON() to ensure the struct is the correct (expected by
HW) size compile time.
Also add a __aligned(128) to the request block structure since Book4 for P8
suggests the HW requires it to be aligned to a 128 byte boundary. There's a
similar requirement for P9 since the COPY and PASTE instructions used to
invoke VAS/NX accelerators operates on a cache line boundary.
Cc: Dan Streetman <ddstreet@ieee.org>
Cc: Haren Myneni <haren@linux.vnet.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
arch/powerpc/include/asm/icswx.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/icswx.h b/arch/powerpc/include/asm/icswx.h
index b0c70a35fd0e..f6599ccb3012 100644
--- a/arch/powerpc/include/asm/icswx.h
+++ b/arch/powerpc/include/asm/icswx.h
@@ -156,8 +156,7 @@ struct coprocessor_request_block {
u8 reserved[32];
struct coprocessor_status_block csb;
-} __packed;
-
+} __aligned(128);
/* RFC02167 Initiate Coprocessor Instructions document
* Chapter 8.2.1.1.1 RS
@@ -188,6 +187,9 @@ static inline int icswx(__be32 ccw, struct coprocessor_request_block *crb)
__be64 ccw_reg = ccw;
u32 cr;
+ /* NB: the same structures are used by VAS-NX */
+ BUILD_BUG_ON(sizeof(*crb) != 128);
+
__asm__ __volatile__(
PPC_ICSWX(%1,0,%2) "\n"
"mfcr %0\n"
--
2.26.2
^ permalink raw reply related
* [PATCH 5/6] powerpc/powernv/pci: Drop unused parent variable
From: Oliver O'Halloran @ 2020-08-04 0:54 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20200804005410.146094-1-oohall@gmail.com>
The "parent" variable in pnv_pci_ioda_configure_pe() isn't used for
anything anymore and can be dropped.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index c9c25fb0783c..6d48155bd885 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -894,7 +894,6 @@ int pnv_ioda_deconfigure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
{
- struct pci_dev *parent;
uint8_t bcomp, dcomp, fcomp;
long rc, rid_end, rid;
@@ -904,7 +903,6 @@ int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
dcomp = OPAL_IGNORE_RID_DEVICE_NUMBER;
fcomp = OPAL_IGNORE_RID_FUNCTION_NUMBER;
- parent = pe->pbus->self;
if (pe->flags & PNV_IODA_PE_BUS_ALL)
count = resource_size(&pe->pbus->busn_res);
else
@@ -925,12 +923,6 @@ int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
}
rid_end = pe->rid + (count << 8);
} else {
-#ifdef CONFIG_PCI_IOV
- if (pe->flags & PNV_IODA_PE_VF)
- parent = pe->parent_dev;
- else
-#endif /* CONFIG_PCI_IOV */
- parent = pe->pdev->bus->self;
bcomp = OpalPciBusAll;
dcomp = OPAL_COMPARE_RID_DEVICE_NUMBER;
fcomp = OPAL_COMPARE_RID_FUNCTION_NUMBER;
--
2.26.2
^ permalink raw reply related
* [PATCH 4/6] powerpc/powernv: Fix spurious kerneldoc warnings in opal-prd.c
From: Oliver O'Halloran @ 2020-08-04 0:54 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20200804005410.146094-1-oohall@gmail.com>
Comments opening with /** are parsed by kerneldoc and this causes the
following warning to be printed:
arch/powerpc/platforms/powernv/opal-prd.c:31: warning: cannot understand
function prototype: 'struct opal_prd_msg_queue_item '
opal_prd_mesg_queue_item is an internal data structure so there's no real
need for it to be documented at all. Fix up the comment to squash the
warning.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
arch/powerpc/platforms/powernv/opal-prd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/opal-prd.c b/arch/powerpc/platforms/powernv/opal-prd.c
index 45f4223a790f..deddaebf8c14 100644
--- a/arch/powerpc/platforms/powernv/opal-prd.c
+++ b/arch/powerpc/platforms/powernv/opal-prd.c
@@ -24,7 +24,7 @@
#include <linux/uaccess.h>
-/**
+/*
* The msg member must be at the end of the struct, as it's followed by the
* message data.
*/
--
2.26.2
^ permalink raw reply related
* [PATCH 3/6] powerpc/powernv: Staticify functions without prototypes
From: Oliver O'Halloran @ 2020-08-04 0:54 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20200804005410.146094-1-oohall@gmail.com>
There's a few scattered in the powernv platform.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
arch/powerpc/platforms/powernv/eeh-powernv.c | 4 ++--
arch/powerpc/platforms/powernv/rng.c | 2 +-
arch/powerpc/platforms/powernv/vas-window.c | 9 ++++-----
3 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 9af8c3b98853..663bd69ac51b 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -38,7 +38,7 @@
static int eeh_event_irq = -EINVAL;
-void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
+static void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
{
dev_dbg(&pdev->dev, "EEH: Setting up device\n");
eeh_probe_device(pdev);
@@ -190,7 +190,7 @@ PNV_EEH_DBGFS_ENTRY(inbB, 0xE10);
#endif /* CONFIG_DEBUG_FS */
-void pnv_eeh_enable_phbs(void)
+static void pnv_eeh_enable_phbs(void)
{
struct pci_controller *hose;
struct pnv_phb *phb;
diff --git a/arch/powerpc/platforms/powernv/rng.c b/arch/powerpc/platforms/powernv/rng.c
index 8035caf6e297..72c25295c1c2 100644
--- a/arch/powerpc/platforms/powernv/rng.c
+++ b/arch/powerpc/platforms/powernv/rng.c
@@ -65,7 +65,7 @@ int powernv_get_random_real_mode(unsigned long *v)
return 1;
}
-int powernv_get_random_darn(unsigned long *v)
+static int powernv_get_random_darn(unsigned long *v)
{
unsigned long val;
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 6434f9cb5aed..5f5fe63a3d1c 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -186,7 +186,7 @@ static void unmap_winctx_mmio_bars(struct vas_window *window)
* OS/User Window Context (UWC) MMIO Base Address Region for the given window.
* Map these bus addresses and save the mapped kernel addresses in @window.
*/
-int map_winctx_mmio_bars(struct vas_window *window)
+static int map_winctx_mmio_bars(struct vas_window *window)
{
int len;
u64 start;
@@ -214,7 +214,7 @@ int map_winctx_mmio_bars(struct vas_window *window)
* registers are not sequential. And, we can only write to offsets
* with valid registers.
*/
-void reset_window_regs(struct vas_window *window)
+static void reset_window_regs(struct vas_window *window)
{
write_hvwc_reg(window, VREG(LPID), 0ULL);
write_hvwc_reg(window, VREG(PID), 0ULL);
@@ -357,7 +357,8 @@ static void init_rsvd_tx_buf_count(struct vas_window *txwin,
* as a one-time task? That could work for NX but what about other
* receivers? Let the receivers tell us the rx-fifo buffers for now.
*/
-int init_winctx_regs(struct vas_window *window, struct vas_winctx *winctx)
+static void init_winctx_regs(struct vas_window *window,
+ struct vas_winctx *winctx)
{
u64 val;
int fifo_size;
@@ -499,8 +500,6 @@ int init_winctx_regs(struct vas_window *window, struct vas_winctx *winctx)
val = SET_FIELD(VAS_WINCTL_NX_WIN, val, winctx->nx_win);
val = SET_FIELD(VAS_WINCTL_OPEN, val, 1);
write_hvwc_reg(window, VREG(WINCTL), val);
-
- return 0;
}
static void vas_release_window_id(struct ida *ida, int winid)
--
2.26.2
^ permalink raw reply related
* [PATCH 2/6] powerpc/powernv: Include asm/powernv.h from the local powernv.h
From: Oliver O'Halloran @ 2020-08-04 0:54 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20200804005410.146094-1-oohall@gmail.com>
The asm/powernv.h header provides prototypes for functions which need to be
called by non-powernv platform code. Also include it in the powernv.h
that's local to the platform directory to squash some warnings about
non-static functions missing prototypes.
Also include powernv.h since from opal-memcons.c since it has the
prototypes for the memcons wrangling functions which are used for the opal
and ultravisor msglog.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
arch/powerpc/platforms/powernv/opal-msglog.c | 2 ++
arch/powerpc/platforms/powernv/powernv.h | 7 +++++++
2 files changed, 9 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/opal-msglog.c b/arch/powerpc/platforms/powernv/opal-msglog.c
index d26da19a611f..d3b6e135c18b 100644
--- a/arch/powerpc/platforms/powernv/opal-msglog.c
+++ b/arch/powerpc/platforms/powernv/opal-msglog.c
@@ -12,6 +12,8 @@
#include <linux/types.h>
#include <asm/barrier.h>
+#include "powernv.h"
+
/* OPAL in-memory console. Defined in OPAL source at core/console.c */
struct memcons {
__be64 magic;
diff --git a/arch/powerpc/platforms/powernv/powernv.h b/arch/powerpc/platforms/powernv/powernv.h
index 1aa51c4fa904..11df4e16a1cc 100644
--- a/arch/powerpc/platforms/powernv/powernv.h
+++ b/arch/powerpc/platforms/powernv/powernv.h
@@ -2,6 +2,13 @@
#ifndef _POWERNV_H
#define _POWERNV_H
+/*
+ * There's various hacks scattered throughout the generic powerpc arch code
+ * that needs to call into powernv platform stuff. The prototypes for those
+ * functions are in asm/powernv.h
+ */
+#include <asm/powernv.h>
+
#ifdef CONFIG_SMP
extern void pnv_smp_init(void);
#else
--
2.26.2
^ permalink raw reply related
* [PATCH 1/6] powerpc/powernv/smp: Fix spurious DBG() warning
From: Oliver O'Halloran @ 2020-08-04 0:54 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20200804005410.146094-1-oohall@gmail.com>
When building with W=1 we get the following warning:
arch/powerpc/platforms/powernv/smp.c: In function ‘pnv_smp_cpu_kill_self’:
arch/powerpc/platforms/powernv/smp.c:276:16: error: suggest braces around
empty body in an ‘if’ statement [-Werror=empty-body]
276 | cpu, srr1);
| ^
cc1: all warnings being treated as errors
The full context is this block:
if (srr1 && !generic_check_cpu_restart(cpu))
DBG("CPU%d Unexpected exit while offline srr1=%lx!\n",
cpu, srr1);
When building with DEBUG undefined DBG() expands to nothing and GCC emits
the warning due to the lack of braces around an empty statement.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
We could add the braces too. That might even be better since it's a multi-line
if block even though it's only a single statement.
---
arch/powerpc/platforms/powernv/smp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
index b2ba3e95bda7..bbf361f23ae8 100644
--- a/arch/powerpc/platforms/powernv/smp.c
+++ b/arch/powerpc/platforms/powernv/smp.c
@@ -43,7 +43,7 @@
#include <asm/udbg.h>
#define DBG(fmt...) udbg_printf(fmt)
#else
-#define DBG(fmt...)
+#define DBG(fmt...) do { } while (0)
#endif
static void pnv_smp_setup_cpu(int cpu)
--
2.26.2
^ permalink raw reply related
* Clean up W=1 for the powernv platform
From: Oliver O'Halloran @ 2020-08-04 0:54 UTC (permalink / raw)
To: linuxppc-dev
Fixes the (mostly) suprious errors we get when building powernv with
W=1. More work is required to build all of powerpc with W=1, let alone
W=2.
^ permalink raw reply
* Re: powerpc: build failures in Linus' tree
From: Michael Ellerman @ 2020-08-04 0:49 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Linux-kernel Mailing List, Linus Torvalds, Paul Mackerras,
PowerPC, Willy Tarreau
In-Reply-To: <20200803223147.28adac79@canb.auug.org.au>
Stephen Rothwell <sfr@canb.auug.org.au> writes:
> On Mon, 03 Aug 2020 21:18:00 +1000 Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> If we just move the include of asm/paca.h below asm-generic/percpu.h
>> then it avoids the bad circular dependency and we still have paca.h
>> included from percpu.h as before.
>>
>> eg:
>>
>> diff --git a/arch/powerpc/include/asm/percpu.h b/arch/powerpc/include/asm/percpu.h
>> index dce863a7635c..8e5b7d0b851c 100644
>> --- a/arch/powerpc/include/asm/percpu.h
>> +++ b/arch/powerpc/include/asm/percpu.h
>> @@ -10,8 +10,6 @@
>>
>> #ifdef CONFIG_SMP
>>
>> -#include <asm/paca.h>
>> -
>> #define __my_cpu_offset local_paca->data_offset
>>
>> #endif /* CONFIG_SMP */
>> @@ -19,4 +17,6 @@
>>
>> #include <asm-generic/percpu.h>
>>
>> +#include <asm/paca.h>
>> +
>> #endif /* _ASM_POWERPC_PERCPU_H_ */
>>
>>
>> So I think I'm inclined to merge that as a minimal fix that's easy to
>> backport.
>>
>> cheers
>
> Looks ok, except does it matter that the include used to be only done
> if __powerpc64__ and CONFIG_SMP are defined?
Basically all of paca.h is inside #ifdef CONFIG_PPC64.
SMP "shouldn't matter", but I tested a SMP=n build and it's clean, so I
think it's good. Of course there's really no guarantees with these
header tangles.
cheers
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_sai: Clean code for synchronize mode
From: Nicolin Chen @ 2020-08-03 21:57 UTC (permalink / raw)
To: Shengjiu Wang
Cc: Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, Mark Brown, linuxppc-dev,
linux-kernel
In-Reply-To: <CAA+D8AOGF44UUq=P1S-M5TUwDUaOnqVmHJKPDBM9DAzt1nVzmQ@mail.gmail.com>
On Mon, Aug 03, 2020 at 04:04:23PM +0800, Shengjiu Wang wrote:
> > > clock generation. The TCSR.TE is no need to enabled when only RX
> > > is enabled.
> >
> > You are correct if there's only RX running without TX joining.
> > However, that's something we can't guarantee. Then we'd enable
> > TE after RE is enabled, which is against what RM recommends:
> >
> > # From 54.3.3.1 Synchronous mode in IMX6SXRM
> > # If the receiver bit clock and frame sync are to be used by
> > # both the transmitter and receiver, it is recommended that
> > # the receiver is the last enabled and the first disabled.
> >
> > I remember I did this "ugly" design by strictly following what
> > RM says. If hardware team has updated the RM or removed this
> > limitation, please quote in the commit logs.
>
> There is no change in RM and same recommandation.
>
> My change does not violate the RM. The direction which generates
> the clock is still last enabled.
Using Tx syncing with Rx clock for example,
T1: arecord (non-stop) => set RE
T2: aplay => set TE then RE (but RE is already set at T1)
Anything that I am missing?
> > > + if (!sai->synchronous[TX] && sai->synchronous[RX] && !tx) {
> > > + regmap_update_bits(sai->regmap, FSL_SAI_xCSR((!tx), ofs),
> > > + FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
> > > + } else if (!sai->synchronous[RX] && sai->synchronous[TX] && tx) {
> > > + regmap_update_bits(sai->regmap, FSL_SAI_xCSR((!tx), ofs),
> > > + FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
> >
> > Two identical regmap_update_bits calls -- both on !tx (RX?)
> The content for regmap_update_bits is the same, but the precondition
> is different.
> The first one is for tx=false and enable TCSR.TE. (TX generate clock)
> The second one is for tx=true and enable RSCR.RE (RX generate clock)
Why not merge them?
+ if ((!sai->synchronous[TX] && sai->synchronous[RX] && !tx) ||
+ ((!sai->synchronous[RX] && sai->synchronous[TX] && tx) {
^ permalink raw reply
* Re: [PATCH v2 15/16] powerpc/powernv/sriov: Make single PE mode a per-BAR setting
From: Segher Boessenkool @ 2020-08-03 21:00 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev, Nathan Chancellor, Oliver O'Halloran,
clang-built-linux
In-Reply-To: <87k0yg1dc8.fsf@mpe.ellerman.id.au>
On Mon, Aug 03, 2020 at 03:57:11PM +1000, Michael Ellerman wrote:
> > I would assume the function should still be generated since those checks
> > are relevant, just the return value is bogus.
>
> Yeah, just sometimes missing warnings boil down to the compiler eliding
> whole sections of code, if it can convince itself they're unreachable.
Including when the compiler considers they must be unreachable because
they would perform undefined behaviour, like, act on uninitialised
values. Such code is removed by cddce ("control dependence dead code
elimination", enabled by -ftree-dce at -O2 or above).
> AFAICS there's nothing weird going on here that should confuse GCC, it's
> about as straight forward as it gets.
Yes. Please open a bug?
> Actually I can reproduce it with:
>
> $ cat > test.c <<EOF
> int foo(void *p)
> {
> unsigned align;
>
> if (!p)
> return align;
>
> return 0;
> }
> EOF
>
> $ gcc -Wall -Wno-maybe-uninitialized -c test.c
> $
>
> No warning.
The whole if() is deleted pretty early.
===
static int foo(void *p)
{
unsigned align;
if (!p)
return align;
return 42;
}
int bork(void) { return foo(0); }
===
doesn't warn either, although that always uses the uninitialised var
(actually, that code is *removed*, and it always does that "return 42").
> But I guess that's behaving as documented. The compiler can't prove that
> foo() will be called with p == NULL, so it doesn't warn.
-Wmaybe-uninitialized doesn't warn for this either, btw.
Segher
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox