LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] powerpc/e500: rework compiler flags
From: Scott Wood @ 2013-08-21  2:12 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Catalin Udma

This replaces the reverted "powerpc/e500: Update compilation flags with
core specific options" with a more multiplatform friendly approach
(on 64-bit) and hopefully without breaking anything this time. :-)

This patchset touches parts of code that are also touched by
http://patchwork.ozlabs.org/patch/261248/, so that patch is a
prerequisite.

Scott Wood (4):
  powerpc: Convert some mftb/mftbu into mfspr
  powerpc/85xx: Remove -Wa,-me500
  powerpc/booke64: Use appropriate -mcpu
  powerpc/e500: Set -mcpu flag for 32-bit e500

 arch/powerpc/Makefile                     | 18 +++++++++++++++++-
 arch/powerpc/boot/ppc_asm.h               |  3 +++
 arch/powerpc/boot/util.S                  | 10 +++++-----
 arch/powerpc/include/asm/ppc_asm.h        |  4 ++--
 arch/powerpc/include/asm/reg.h            | 15 ++++++++++-----
 arch/powerpc/include/asm/timex.h          |  4 ++--
 arch/powerpc/kernel/vdso32/gettimeofday.S |  6 +++---
 arch/powerpc/platforms/85xx/smp.c         |  6 ++++--
 arch/powerpc/platforms/Kconfig.cputype    | 13 +++++++++++++
 9 files changed, 59 insertions(+), 20 deletions(-)

-- 
1.8.1.2

^ permalink raw reply

* [PATCH 1/4] powerpc: Convert some mftb/mftbu into mfspr
From: Scott Wood @ 2013-08-21  2:12 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Scott Wood, Catalin Udma
In-Reply-To: <1377051162-2588-1-git-send-email-scottwood@freescale.com>

Some CPUs (such as e500v1/v2) don't implement mftb and will take a
trap.  mfspr should work on everything that has a timebase, and is the
preferred instruction according to ISA v2.06.

Currently we get away with mftb on 85xx because the assembler converts
it to mfspr due to -Wa,-me500.  However, that flag has other effects
that are undesireable for certain targets (e.g.  lwsync is converted to
sync), and is hostile to multiplatform kernels.  Thus we would like to
stop setting it for all e500-family builds.

mftb/mftbu instances which are in 85xx code or common code are
converted.  Instances which will never run on 85xx are left alone.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 arch/powerpc/boot/ppc_asm.h               |  3 +++
 arch/powerpc/boot/util.S                  | 10 +++++-----
 arch/powerpc/include/asm/ppc_asm.h        |  4 ++--
 arch/powerpc/include/asm/reg.h            | 15 ++++++++++-----
 arch/powerpc/include/asm/timex.h          |  4 ++--
 arch/powerpc/kernel/vdso32/gettimeofday.S |  6 +++---
 arch/powerpc/platforms/85xx/smp.c         |  6 ++++--
 7 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/boot/ppc_asm.h b/arch/powerpc/boot/ppc_asm.h
index 1c2c281..eb0e98b 100644
--- a/arch/powerpc/boot/ppc_asm.h
+++ b/arch/powerpc/boot/ppc_asm.h
@@ -59,4 +59,7 @@
 #define	r30	30
 #define	r31	31
 
+#define SPRN_TBRL	268
+#define SPRN_TBRU	269
+
 #endif /* _PPC64_PPC_ASM_H */
diff --git a/arch/powerpc/boot/util.S b/arch/powerpc/boot/util.S
index 427ddfc..5143228 100644
--- a/arch/powerpc/boot/util.S
+++ b/arch/powerpc/boot/util.S
@@ -71,18 +71,18 @@ udelay:
 	add	r4,r4,r5
 	addi	r4,r4,-1
 	divw	r4,r4,r5	/* BUS ticks */
-1:	mftbu	r5
-	mftb	r6
-	mftbu	r7
+1:	mfspr	r5, SPRN_TBRU
+	mfspr	r6, SPRN_TBRL
+	mfspr	r7, SPRN_TBRU
 	cmpw	0,r5,r7
 	bne	1b		/* Get [synced] base time */
 	addc	r9,r6,r4	/* Compute end time */
 	addze	r8,r5
-2:	mftbu	r5
+2:	mfspr	r5, SPRN_TBRU
 	cmpw	0,r5,r8
 	blt	2b
 	bgt	3f
-	mftb	r6
+	mfspr	r6, SPRN_TBRL
 	cmpw	0,r6,r9
 	blt	2b
 3:	blr
diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index 8fdd3da..5995457 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -433,13 +433,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_601)
 
 #if defined(CONFIG_PPC_CELL) || defined(CONFIG_PPC_FSL_BOOK3E)
 #define MFTB(dest)			\
-90:	mftb  dest;			\
+90:	mfspr dest, SPRN_TBRL;		\
 BEGIN_FTR_SECTION_NESTED(96);		\
 	cmpwi dest,0;			\
 	beq-  90b;			\
 END_FTR_SECTION_NESTED(CPU_FTR_CELL_TB_BUG, CPU_FTR_CELL_TB_BUG, 96)
 #else
-#define MFTB(dest)			mftb dest
+#define MFTB(dest)			mfspr dest, SPRN_TBRL
 #endif
 
 #ifndef CONFIG_SMP
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 55b0307..64264bf 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -1120,7 +1120,7 @@
 #if defined(CONFIG_PPC_CELL) || defined(CONFIG_PPC_FSL_BOOK3E)
 #define mftb()		({unsigned long rval;				\
 			asm volatile(					\
-				"90:	mftb %0;\n"			\
+				"90:	mfspr %0, %2;\n"		\
 				"97:	cmpwi %0,0;\n"			\
 				"	beq- 90b;\n"			\
 				"99:\n"					\
@@ -1134,18 +1134,23 @@
 				"	.llong 0\n"			\
 				"	.llong 0\n"			\
 				".previous"				\
-			: "=r" (rval) : "i" (CPU_FTR_CELL_TB_BUG)); rval;})
+			: "=r" (rval) \
+			: "i" (CPU_FTR_CELL_TB_BUG), "i" (SPRN_TBRL)); \
+			rval;})
 #else
 #define mftb()		({unsigned long rval;	\
-			asm volatile("mftb %0" : "=r" (rval)); rval;})
+			asm volatile("mfspr %0, %1" : \
+				     "=r" (rval) : "i" (SPRN_TBRL)); rval;})
 #endif /* !CONFIG_PPC_CELL */
 
 #else /* __powerpc64__ */
 
 #define mftbl()		({unsigned long rval;	\
-			asm volatile("mftbl %0" : "=r" (rval)); rval;})
+			asm volatile("mfspr %0, %1" : "=r" (rval) : \
+				"i" (SPRN_TBRL)); rval;})
 #define mftbu()		({unsigned long rval;	\
-			asm volatile("mftbu %0" : "=r" (rval)); rval;})
+			asm volatile("mfspr %0, %1" : "=r" (rval) : \
+				"i" (SPRN_TBRU)); rval;})
 #endif /* !__powerpc64__ */
 
 #define mttbl(v)	asm volatile("mttbl %0":: "r"(v))
diff --git a/arch/powerpc/include/asm/timex.h b/arch/powerpc/include/asm/timex.h
index c55e14f..18908ca 100644
--- a/arch/powerpc/include/asm/timex.h
+++ b/arch/powerpc/include/asm/timex.h
@@ -29,7 +29,7 @@ static inline cycles_t get_cycles(void)
 	ret = 0;
 
 	__asm__ __volatile__(
-		"97:	mftb %0\n"
+		"97:	mfspr %0, %2\n"
 		"99:\n"
 		".section __ftr_fixup,\"a\"\n"
 		".align 2\n"
@@ -41,7 +41,7 @@ static inline cycles_t get_cycles(void)
 		"	.long 0\n"
 		"	.long 0\n"
 		".previous"
-		: "=r" (ret) : "i" (CPU_FTR_601));
+		: "=r" (ret) : "i" (CPU_FTR_601), "i" (SPRN_TBRL));
 	return ret;
 #endif
 }
diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
index 27e2f62..6b1f2a6 100644
--- a/arch/powerpc/kernel/vdso32/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
@@ -232,9 +232,9 @@ __do_get_tspec:
 	lwz	r6,(CFG_TB_ORIG_STAMP+4)(r9)
 
 	/* Get a stable TB value */
-2:	mftbu	r3
-	mftbl	r4
-	mftbu	r0
+2:	mfspr	r3, SPRN_TBRU
+	mfspr	r4, SPRN_TBRL
+	mfspr	r0, SPRN_TBRU
 	cmplw	cr0,r3,r0
 	bne-	2b
 
diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
index ea7e629..281b7f0 100644
--- a/arch/powerpc/platforms/85xx/smp.c
+++ b/arch/powerpc/platforms/85xx/smp.c
@@ -83,11 +83,13 @@ static void mpc85xx_give_timebase(void)
 	{
 		u64 prev;
 
-		asm volatile("mftb %0" : "=r" (timebase));
+		asm volatile("mfspr %0, %1" : "=r" (timebase) :
+			     "i" (SPRN_TBRL));
 
 		do {
 			prev = timebase;
-			asm volatile("mftb %0" : "=r" (timebase));
+			asm volatile("mfspr %0, %1" : "=r" (timebase) :
+				     "i" (SPRN_TBRL));
 		} while (prev != timebase);
 	}
 #else
-- 
1.8.1.2

^ permalink raw reply related

* Re: [PATCH] Kconfig: Remove hotplug enable hints in CONFIG_KEXEC help texts
From: Stephen Rothwell @ 2013-08-21  1:13 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linux-mips, linux-ia64, linux-sh, Greg Kroah-Hartman, x86,
	linux-kernel, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1377027483-17025-1-git-send-email-geert@linux-m68k.org>

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]

Hi Geert,

On Tue, 20 Aug 2013 21:38:03 +0200 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> commit 40b313608ad4ea655addd2ec6cdd106477ae8e15 ("Finally eradicate
> CONFIG_HOTPLUG") removed remaining references to CONFIG_HOTPLUG, but missed
> a few plain English references in the CONFIG_KEXEC help texts.
> 
> Remove them, too.
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

Looks good to me.  Thanks.

Acked-by: Stephen Rothwell <sfr@canb.auug.org.au>

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: linux-next: build failure after merge of the final tree
From: Stephen Rothwell @ 2013-08-21  0:22 UTC (permalink / raw)
  To: Ben Myers
  Cc: cbe-oss-dev, Arnd Bergmann, Dwight Engen, linux-kernel, xfs,
	linux-next, Jeremy Kerr, linuxppc-dev, Gao feng
In-Reply-To: <20130820192844.GC5262@sgi.com>

[-- Attachment #1: Type: text/plain, Size: 689 bytes --]

Hi Ben,

On Tue, 20 Aug 2013 14:28:44 -0500 Ben Myers <bpm@sgi.com> wrote:
>
> I'd prefer not to break Stephen's tree two days in a row.  We could just revert
> d6970d4b726c in the xfs tree for the time being as Stephen has done, but given
> the choice would prefer the fix.  Do you have a preference between the two
> approaches that Dwight has posted?  The first seems more conservative...

I will automatically revert that commit when I merge the xfs tree until
some other solution is forthcoming (so you don't have to do the revert in
the xfs tree).  This does need to be fixed fairly soon, though.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v4 0/7] net: use platform_{get,set}_drvdata()
From: David Miller @ 2013-08-21  0:19 UTC (permalink / raw)
  To: clbchenlibo.chen
  Cc: sergei.shtylyov, gregkh, netdev, jg1.han, lizefan, vbordug,
	linuxppc-dev
In-Reply-To: <52120859.7060200@huawei.com>

From: Libo Chen <clbchenlibo.chen@huawei.com>
Date: Mon, 19 Aug 2013 19:58:17 +0800

> 
> Use the wrapper functions for getting and setting the driver data using
> platform_device instead of using dev_{get,set}_drvdata() with &pdev->dev,
> so we can directly pass a struct platform_device.
> 
> changelog v4:
> 	remove modify about happy_meal_pci_probe && happy_meal_pci_remove
> changelog v3:
> 	remove modify about dev_set_drvdata()
> changelog v2:
> 	this version add modify record about dev_set_drvdata().

Series applied.

^ permalink raw reply

* Re: [RFC PATCH 1/1] powerpc/embedded6xx: Add support for Motorola/Emerson MVME5100.
From: Scott Wood @ 2013-08-20 23:20 UTC (permalink / raw)
  To: Stephen N Chivers; +Cc: linuxppc-dev, paulus, Chris Proctor
In-Reply-To: <OFE172B06C.BE11BDC5-ONCA257BCC.007C0097-CA257BCD.000D984C@csc.com>

On Tue, 2013-08-20 at 13:28 +1100, Stephen N Chivers wrote:
> Scott Wood <scottwood@freescale.com> wrote on 08/09/2013 11:35:20 AM:
> 
> > From: Scott Wood <scottwood@freescale.com>
> > To: Stephen N Chivers <schivers@csc.com.au>
> > Cc: <benh@kernel.crashing.org>, <paulus@samba.org>, Chris Proctor 
> > <cproctor@csc.com.au>, <linuxppc-dev@lists.ozlabs.org>
> > Date: 08/09/2013 11:36 AM
> > Subject: Re: [RFC PATCH 1/1] powerpc/embedded6xx: Add support for 
> > Motorola/Emerson MVME5100.
> > 
> > simple-bus may be applicable here (in addition to a specific
> > compatible).
> >
> The HAWK ASIC is a difficult beast. I still cannot get a positive
> identification as to what it is (Motorola/Freescale part number
> unknown, not even the part number on the chip on the board helps....).
> The best I can come up with is that it is a tsi108 without
> the ethenets.
> So device_type will be tsi-bridge and compatible will be
> tsi108-bridge.

Don't use device_type.  compatible should include "hawk" in the name
(especially if you're not sure what's really in it), and/or the part
number on the chip.  If you're convinced it's fully compatible with
tsi108-bridge you can add that as a second compatible value, though
given the uncertainty it's probably better to just teach Linux to look
for the new compatible.

If devices on the bus can be used without any special bus setup or
knowledge, then you can add a compatible of "simple-bus" to the end.

> > Why not just look for a chrp,iic node directly?
> >
> I was following the model used in other places, like chrp/setup.c.

Not all examples are good examples. :-)

> > > +       if ((np = of_find_compatible_node(NULL, "pci", "mpc10x-pci"))) 
> {
> > 
> > Why insist on the device_type?
> >
> Following the model in the linkstation (kurobox) platform support. 

Drop the device_type check.

> > > +static void
> > > +mvme5100_restart(char *cmd)
> > > +{
> > > +       volatile ulong          i = 10000000;
> > > +
> > > +
> > > +       local_irq_disable();
> > > +       _nmask_and_or_msr(0, MSR_IP);
> > 
> > Does "mtmsr(mfmsr() | MSR_IP)" not work?
> >
> Don't know. Is from the original code by Matt Porter.

It actually appears that there are no callers remaining that use the
"and" portion of the functionality.  In fact there are no callers that
use it for anything other than setting MSR_IP. :-P

> > > +       out_8((u_char *) BOARD_MODRST_REG, 0x01);
> > > +
> > > +       while (i-- > 0);
> > 
> > Do not use a loop to implement a delay.
> >
> Taken from the original code. But at this point the board
> is going to reset and reboot via firmware, as /sbin/reboot
> or /sbin/halt has been invoked.

Still, it's just a bad idea.  What's wrong with udelay()?

Or just use an infinite loop.  How much value is there really in timing
out here?

> > > +static void __init
> > > +mvme5100_set_bat(void)
> > > +{
> > > +
> > > +
> > > +       mb();
> > > +       mtspr(SPRN_DBAT1U, 0xf0001ffe);
> > > +       mtspr(SPRN_DBAT1L, 0xf000002a);
> > > +       mb();
> > > +       setbat(1, 0xfe000000, 0xfe000000, 0x02000000, 
> PAGE_KERNEL_NCG);
> > > +}
> > 
> > It is no longer allowed to squat on random virtual address space like
> > this.  If you really need a BAT you'll have to allocate the virtual
> > address properly.
> >
> Yes. I found that this was an anathema when researching the port in
> 2010 but I couldn't find any practical solution at the time.
> The code is called early to ensure that the hawk registers are available.
> sysdev/cpm_common.c does the same thing.

> What is the correct solution?

ioremap() has special code to function early (using ioremap_bot).

If you still need to use a BAT that early, reserve the space with
asm/fixmap.h or by adding a function to the early ioremap code to just
reserve the space.  Or better, improve the ioremap code to be capable of
creating a BAT (or equivalent) when requested.

-Scott

^ permalink raw reply

* Re: Critical Interrupt Input
From: Benjamin Herrenschmidt @ 2013-08-20 23:08 UTC (permalink / raw)
  To: Henry Bausley; +Cc: linuxppc-dev
In-Reply-To: <1377038913.25385.194.camel@lx-henry>

On Tue, 2013-08-20 at 15:48 -0700, Henry Bausley wrote:
> Ben,
> 
> 
> After your hints I suspected the read of a real world i/o variable *piom
> which came from ioremap_nocache in the 3 line critical interrupt handler
> 
> void critintr_handler(void *dev)
> {
>   critintrcount++;          // increment a variable
>   iodata = *piom;           // read an I/O location 
>   mtdcr(0x0c0, 0x00002000); // clear critical interrupt 
> } 
> 
> is what caused the problem. Commenting it out seems to make the system stable.  

Right, definitely would do that. BTW. You may want to use proper IO
accessors while at it, to get the right memory barriers etc...

> This led us to disable the critical interrupt when in the
> DataTLBError44x and InstructionTLBError44x exceptions.  Now the critical
> interrupt handler seems to make things more stable when reading real
> world i/o for our application.
>
> 
>   /* Data TLB Error Interrupt */
>   START_EXCEPTION(DataTLBError44x)
>   mtspr	SPRN_SPRG_WSCRATCH0, r10  /* Save some working */
> +  mfmsr r10                      /*  Disable the */
> +  rlwinm r10,r10,0,15,13         /*  MSR's CE bit */
> +  mtmsr r10                     
> 
> 
> Do you see any potential problems with this approach?
> 
> If so can you advise us on how to better take care of this.

 - You potentially still have an exposure ... between the mtspr to
scratch and the mfmsr, a CRIC can occur, causing a re-entrancy which
would than clobber the scratch register. That can be handled by saving
that scratc SPRG into the stack frame on entry/exit from the crit
interrupt. Look at crit_transfer_to_handler, how it already handles
MMUCR:

	mfspr	r0,SPRN_MMUCR
	stw	r0,MMUCR(r11)

Probably add saving of the SPRG_WSCRATCH0 in there (need to add a frame
slot for it) and do the restore in RESTORE_MMU_REGS

 - You need to handle Instructions TLB miss as well

 - You add overhead to the TLB miss handlers which are fairly
performance critical pieces of code. You might be able to alleviate
that by making the whole thing support re-entrancy properly but that's
harder. To do that you would have to:

    * Save *all* the SPRGs used by the TLB miss during crit entry/exit

    * Detect in crit_transfer_to_handler (check the CSRR0 bounds) that 
      the crit code interrupted finish_tlb_load_44x before or at the
      last tlbwe instruction. In that case, immediately clear the 
      partially written TLB entry (index in r13) and change the
      return address to skip right past the last tlbwe.

Cheers,
Ben.


> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> On Tue, 2013-08-20 at 06:56 +1000, Benjamin Herrenschmidt wrote:
> > On Mon, 2013-08-19 at 12:00 -0700, Henry Bausley wrote:
> > > 
> > > Support does appear to be present but there is a problem returning
> > > back to user space I suspect.
> > 
> > Probably a problem with TLB misses vs. crit interrupts.
> > 
> > A critical interrupt can re-enter a TLB miss.
> > 
> > I can see two potential issues there:
> > 
> >  - A bug where we don't properly restore "something" (I thought we did
> > save and restore MMUCR though, but that's worth dbl checking if it works
> > properly) accross the crit entry/exit
> > 
> >  - Something in your crit code causing a TLB miss (the
> > kernel .text/.data/.bss should be bolted but anything else can). We
> > don't currently support re-entering the TLB miss that way.
> > 
> > If we were to support the latter, we'd need to detect on entering a crit
> > that the PC is within the TLB miss handler, and setup a return context
> > to the original instruction (replay the miss) rather than trying to
> > resume it..
> > 
> > Cheers,
> > Ben.
> > 
> > > What fails is it causes Linux user space programs to get Segmentation
> > > errors.
> > > Issuing a simple ls causes a segmentation fault sometimes.  The shell
> > > gets terminated 
> > > and you cannot log back in.  INIT: Id "T0" respawning too fast:
> > > disabled for 5 minutes pops up.
> > > 
> > > However, the critical interrupt handler keeps running.  I know this by
> > > adding the reading 
> > > of a physical I/O location in the handler and can see it is being read
> > > on the scope.
> > > 
> > > 
> > > The only code in the handler is below.
> > > 
> > > void critintr_handler(void *dev)
> > > {
> > >   critintrcount++;          // increment a variable
> > >   iodata = *piom;           // read an I/O location 
> > >   mtdcr(0x0c0, 0x00002000); // clear critical interrupt
> > > }
> > > 
> > > 
> > > Below is a log of the type of crashes that occur:
> > > 
> > > root@10.34.9.213:/opt/ppmac/ktest# ls
> > > Segmentation fault
> > > root@10.34.9.213:/opt/ppmac/ktest# ls
> > > Segmentation fault
> > > root@10.34.9.213:/opt/ppmac/ktest# ls
> > > Makefile        ktest.c    ktest.ko     ktest.mod.o  modules.order
> > > Module.symvers  ktest.cbp  ktest.mod.c  ktest.o
> > > root@10.34.9.213:/opt/ppmac/ktest# ls
> > > 
> > > Debian GNU/Linux 7 powerpmac ttyS0
> > > 
> > > powerpmac login: root
> > > 
> > > Debian GNU/Linux 7 powerpmac ttyS0
> > > 
> > > powerpmac login: root
> > > 
> > > Debian GNU/Linux 7 powerpmac ttyS0
> > > 
> > > powerpmac login: root
> > > 
> > > Debian GNU/Linux 7 powerpmac ttyS0
> > > 
> > > powerpmac login: root
> > > Password: 
> > > Last login: Thu Nov 30 20:42:16 UTC 1933 on ttyS0
> > > Linux powerpmac 3.2.21-aspen_2.01.09 #10 Mon Aug 19 08:49:12 PDT 2013
> > > ppc
> > > 
> > > The programs included with the Debian GNU/Linux system are free
> > > software;
> > > the exact distribution terms for each program are described in the
> > > individual files in /usr/share/doc/*/copyright.
> > > 
> > > Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
> > > permitted by applicable law.
> > > INIT: Id "T0" respawning too fast: disabled for 5 minutes
> > > 
> > > 
> > > ______________________________________________________________________
> > > From: "Benjamin Herrenschmidt" <benh@kernel.crashing.org>
> > > Sent: Saturday, August 17, 2013 3:05 PM
> > > To: "Kumar Gala" <galak@kernel.crashing.org>
> > > Cc: linuxppc-dev@lists.ozlabs.org, hbausley@deltatau.com
> > > Subject: Re: Critical Interrupt Input
> > > 
> > > On Fri, 2013-08-16 at 06:04 -0500, Kumar Gala wrote:
> > > > The 44x low level code needs to handle exception stacks properly for
> > > > this to work. Since its possible to have a critical exception occur
> > > > while in a normal exception level, you have to have proper saving of
> > > > additional register state and a stack frame for the critical
> > > > exception, etc. I'm not sure if that was ever done for 44x.
> > > 
> > > Don't 44x and FSL BookE share the same macros ? I would think 44x does
> > > indeed implement the same crit support as e500...
> > > 
> > > What does the crash look like ?
> > > 
> > > Ben.
> > > 
> > > 
> > > _______________________________________________
> > > Linuxppc-dev mailing list
> > > Linuxppc-dev@lists.ozlabs.org
> > > https://lists.ozlabs.org/listinfo/linuxppc-dev
> > > 
> > > 
> > >   ­­  
> > 
> > 
> 
> 
> 
> 
> 
> Outbound scan for Spam or Virus by Barracuda at Delta Tau

^ permalink raw reply

* Re: Critical Interrupt Input
From: Henry Bausley @ 2013-08-20 22:48 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1376945799.25016.77.camel@pasglop>

Ben,


After your hints I suspected the read of a real world i/o variable *piom
which came from ioremap_nocache in the 3 line critical interrupt handler

void critintr_handler(void *dev)
{
  critintrcount++;          // increment a variable
  iodata =3D *piom;           // read an I/O location=20
  mtdcr(0x0c0, 0x00002000); // clear critical interrupt=20
}=20

is what caused the problem. Commenting it out seems to make the system stab=
le.=20=20

This led us to disable the critical interrupt when in the
DataTLBError44x and InstructionTLBError44x exceptions.  Now the critical
interrupt handler seems to make things more stable when reading real
world i/o for our application.


  /* Data TLB Error Interrupt */
  START_EXCEPTION(DataTLBError44x)
  mtspr	SPRN_SPRG_WSCRATCH0, r10  /* Save some working */
+  mfmsr r10                      /*  Disable the */
+  rlwinm r10,r10,0,15,13         /*  MSR's CE bit */
+  mtmsr r10=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20


Do you see any potential problems with this approach?

If so can you advise us on how to better take care of this.















On Tue, 2013-08-20 at 06:56 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2013-08-19 at 12:00 -0700, Henry Bausley wrote:
> >=20
> > Support does appear to be present but there is a problem returning
> > back to user space I suspect.
>=20
> Probably a problem with TLB misses vs. crit interrupts.
>=20
> A critical interrupt can re-enter a TLB miss.
>=20
> I can see two potential issues there:
>=20
>  - A bug where we don't properly restore "something" (I thought we did
> save and restore MMUCR though, but that's worth dbl checking if it works
> properly) accross the crit entry/exit
>=20
>  - Something in your crit code causing a TLB miss (the
> kernel .text/.data/.bss should be bolted but anything else can). We
> don't currently support re-entering the TLB miss that way.
>=20
> If we were to support the latter, we'd need to detect on entering a crit
> that the PC is within the TLB miss handler, and setup a return context
> to the original instruction (replay the miss) rather than trying to
> resume it..
>=20
> Cheers,
> Ben.
>=20
> > What fails is it causes Linux user space programs to get Segmentation
> > errors.
> > Issuing a simple ls causes a segmentation fault sometimes.  The shell
> > gets terminated=20
> > and you cannot log back in.  INIT: Id "T0" respawning too fast:
> > disabled for 5 minutes pops up.
> >=20
> > However, the critical interrupt handler keeps running.  I know this by
> > adding the reading=20
> > of a physical I/O location in the handler and can see it is being read
> > on the scope.
> >=20
> >=20
> > The only code in the handler is below.
> >=20
> > void critintr_handler(void *dev)
> > {
> >   critintrcount++;          // increment a variable
> >   iodata =3D *piom;           // read an I/O location=20
> >   mtdcr(0x0c0, 0x00002000); // clear critical interrupt
> > }
> >=20
> >=20
> > Below is a log of the type of crashes that occur:
> >=20
> > root@10.34.9.213:/opt/ppmac/ktest# ls
> > Segmentation fault
> > root@10.34.9.213:/opt/ppmac/ktest# ls
> > Segmentation fault
> > root@10.34.9.213:/opt/ppmac/ktest# ls
> > Makefile        ktest.c    ktest.ko     ktest.mod.o  modules.order
> > Module.symvers  ktest.cbp  ktest.mod.c  ktest.o
> > root@10.34.9.213:/opt/ppmac/ktest# ls
> >=20
> > Debian GNU/Linux 7 powerpmac ttyS0
> >=20
> > powerpmac login: root
> >=20
> > Debian GNU/Linux 7 powerpmac ttyS0
> >=20
> > powerpmac login: root
> >=20
> > Debian GNU/Linux 7 powerpmac ttyS0
> >=20
> > powerpmac login: root
> >=20
> > Debian GNU/Linux 7 powerpmac ttyS0
> >=20
> > powerpmac login: root
> > Password:=20
> > Last login: Thu Nov 30 20:42:16 UTC 1933 on ttyS0
> > Linux powerpmac 3.2.21-aspen_2.01.09 #10 Mon Aug 19 08:49:12 PDT 2013
> > ppc
> >=20
> > The programs included with the Debian GNU/Linux system are free
> > software;
> > the exact distribution terms for each program are described in the
> > individual files in /usr/share/doc/*/copyright.
> >=20
> > Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
> > permitted by applicable law.
> > INIT: Id "T0" respawning too fast: disabled for 5 minutes
> >=20
> >=20
> > ______________________________________________________________________
> > From: "Benjamin Herrenschmidt" <benh@kernel.crashing.org>
> > Sent: Saturday, August 17, 2013 3:05 PM
> > To: "Kumar Gala" <galak@kernel.crashing.org>
> > Cc: linuxppc-dev@lists.ozlabs.org, hbausley@deltatau.com
> > Subject: Re: Critical Interrupt Input
> >=20
> > On Fri, 2013-08-16 at 06:04 -0500, Kumar Gala wrote:
> > > The 44x low level code needs to handle exception stacks properly for
> > > this to work. Since its possible to have a critical exception occur
> > > while in a normal exception level, you have to have proper saving of
> > > additional register state and a stack frame for the critical
> > > exception, etc. I'm not sure if that was ever done for 44x.
> >=20
> > Don't 44x and FSL BookE share the same macros ? I would think 44x does
> > indeed implement the same crit support as e500...
> >=20
> > What does the crash look like ?
> >=20
> > Ben.
> >=20
> >=20
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/linuxppc-dev
> >=20
> >=20
> >   =C2=AD=C2=AD=20=20
>=20
>=20




=0D
Outbound scan for Spam or Virus by Barracuda at Delta Tau=0D

^ permalink raw reply

* Re: [PATCH v8 2/2] ASoC: fsl: Add S/PDIF machine driver
From: Mark Brown @ 2013-08-20 22:28 UTC (permalink / raw)
  To: Stephen Warren
  Cc: mark.rutland, devicetree, alsa-devel, lars, festevam, s.hauer,
	Nicolin Chen, timur, rob.herring, tomasz.figa, p.zabel, R65777,
	shawn.guo, linuxppc-dev
In-Reply-To: <5213C94D.7050304@wwwdotorg.org>

[-- Attachment #1: Type: text/plain, Size: 419 bytes --]

On Tue, Aug 20, 2013 at 01:53:49PM -0600, Stephen Warren wrote:
> On 08/20/2013 01:07 PM, Mark Brown wrote:

> > The point is that it might turn into a more correct binding
> > depending on what the S/PDIF device actually is.

> There's *never* an object on the board called a "dummy codec".

Oh, is that what you're talking about?  Yes, that makes sense.  I had
been responding to the comments about the transceivers.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v4 03/19] powerpc: refactor of_get_cpu_node to support other architectures
From: Benjamin Herrenschmidt @ 2013-08-20 21:48 UTC (permalink / raw)
  To: Sudeep KarkadaNagesha
  Cc: Jonas Bonn, devicetree@vger.kernel.org, Michal Simek,
	linux-pm@vger.kernel.org, Viresh Kumar,
	linux-kernel@vger.kernel.org, rob.herring@calxeda.com,
	Rafael J. Wysocki, Greg Kroah-Hartman, grant.likely@linaro.org,
	linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <52135F86.9020401@arm.com>

On Tue, 2013-08-20 at 13:22 +0100, Sudeep KarkadaNagesha wrote:
> Correct, he has reviewed but I am waiting for his ACK before I could
> sent you pull request. Hopefully I should be able to send pull request
> tomorrow with his ACK.
> 
> Hi Ben,
> 
> If this is patch is fine, can I have your ACK ?

I just want to test it (just in case...), will give an Ack then.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH] powerpc: add the missing required isync for the coherent icache flush
From: Benjamin Herrenschmidt @ 2013-08-20 21:28 UTC (permalink / raw)
  To: Kevin Hao; +Cc: Wang Dongsheng-B40534, linuxppc
In-Reply-To: <20130820121627.GB18634@pek-khao-d1.corp.ad.wrs.com>

On Tue, 2013-08-20 at 20:16 +0800, Kevin Hao wrote:

> Dummy question: What does the ifetch buffers mean? The instruction fetch
> pipeline or instruction dispatch pipeline? Shouldn't all the prefetched
> instructions in these buffers be discarded by isync?

Architecturally isync doesn't have to toss prefetch completely. It only
needs to make sense that context changes performed by previous
instructions (and interrupts/traps) happen at the point of the isync,
for example, it will ensure that a trap conditional is fully evaluated
before subsequent instruction execution, etc....

So in this case, it makes sure the icbi has been executed and it's the
icbi that invalidates the prefetched instructions.

> 
> >, sync orders the icbi and isync ensures its execution has been
> > synchronized. At least I *think* that's the required sequence, I have to
> > dbl check the arch, maybe tomorrow. I wouldn't be surprise if we also
> > need a sync before the icbi to order the actual stores to memory that
> > might have modified instructions with the icbi.
> 
> Doesn't the coherence between icache and dcache be maintained by the snooping?

The icache yes, but not the ifetch buffers (basically think of them as
buffering stages between icache and dispatch). At least that's my
understanding of the implementation. 

Cheers,
Ben.

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

^ permalink raw reply

* Re: linux-next: build failure after merge of the final tree
From: Arnd Bergmann @ 2013-08-20 20:46 UTC (permalink / raw)
  To: Dwight Engen
  Cc: cbe-oss-dev, Stephen Rothwell, David Chinner, linux-kernel, xfs,
	Ben Myers, linux-next, Gao feng, linuxppc-dev, Jeremy Kerr
In-Reply-To: <20130820120702.000b044e@oracle.com>

On Tuesday 20 August 2013, Dwight Engen wrote:
> diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
> index f390042..90fb308 100644
> --- a/arch/powerpc/platforms/cell/spufs/inode.c
> +++ b/arch/powerpc/platforms/cell/spufs/inode.c
> @@ -620,12 +620,12 @@ spufs_parse_options(struct super_block *sb, char *options, struct inode *root)
>                 case Opt_uid:
>                         if (match_int(&args[0], &option))
>                                 return 0;
> -                       root->i_uid = option;
> +                       root->i_uid = make_kuid(&init_user_ns, option);
>                         break;
>                 case Opt_gid:
>                         if (match_int(&args[0], &option))
>                                 return 0;
> -                       root->i_gid = option;
> +                       root->i_gid = make_kgid(&init_user_ns, option);
>                         break;
>                 case Opt_mode:
>                         if (match_octal(&args[0], &option))

Doesn't this mean the uid/gid is taken from the initial namespace rather than
from the namespace of the 'mount' process calling this? I think the logical
choice would be to have the UID be the one that gets passed here in the caller's
namespace.

	Arnd

^ permalink raw reply

* Re: [PATCH v8 2/2] ASoC: fsl: Add S/PDIF machine driver
From: Stephen Warren @ 2013-08-20 19:53 UTC (permalink / raw)
  To: Mark Brown
  Cc: mark.rutland, devicetree, alsa-devel, lars, festevam, s.hauer,
	Nicolin Chen, timur, rob.herring, tomasz.figa, p.zabel, R65777,
	shawn.guo, linuxppc-dev
In-Reply-To: <20130820190701.GR30073@sirena.org.uk>

On 08/20/2013 01:07 PM, Mark Brown wrote:
> On Tue, Aug 20, 2013 at 09:48:46AM -0600, Stephen Warren wrote:
>> On 08/19/2013 06:18 PM, Mark Brown wrote:
> 
>>> S/PDIF is also sometimes used as an interconnect between
>>> devices - some CODECs have S/PDIF I/O (more normally used as an
>>> external connector on the box).  This is most frequently seen
>>> as a way to
> 
>> That's not a good argument for an incorrect binding.
> 
> The point is that it might turn into a more correct binding
> depending on what the S/PDIF device actually is.

There's *never* an object on the board called a "dummy codec".

^ permalink raw reply

* [PATCH] Kconfig: Remove hotplug enable hints in CONFIG_KEXEC help texts
From: Geert Uytterhoeven @ 2013-08-20 19:38 UTC (permalink / raw)
  To: Stephen Rothwell, Greg Kroah-Hartman
  Cc: linux-mips, linux-ia64, linux-sh, x86, linux-kernel,
	Geert Uytterhoeven, linuxppc-dev, linux-arm-kernel

commit 40b313608ad4ea655addd2ec6cdd106477ae8e15 ("Finally eradicate
CONFIG_HOTPLUG") removed remaining references to CONFIG_HOTPLUG, but missed
a few plain English references in the CONFIG_KEXEC help texts.

Remove them, too.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 arch/arm/Kconfig     |    3 +--
 arch/ia64/Kconfig    |    6 +++---
 arch/mips/Kconfig    |    6 +++---
 arch/powerpc/Kconfig |    6 +++---
 arch/sh/Kconfig      |    6 +++---
 arch/x86/Kconfig     |    6 +++---
 6 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 43594d5..cd5c1c9 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -2064,8 +2064,7 @@ config KEXEC
 
 	  It is an ongoing process to be certain the hardware in a machine
 	  is properly shutdown, so do not be surprised if this code does not
-	  initially work for you.  It may help to enable device hotplugging
-	  support.
+	  initially work for you.
 
 config ATAGS_PROC
 	bool "Export atags in procfs"
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 5a768ad..b36370d 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -565,9 +565,9 @@ config KEXEC
 
 	  It is an ongoing process to be certain the hardware in a machine
 	  is properly shutdown, so do not be surprised if this code does not
-	  initially work for you.  It may help to enable device hotplugging
-	  support.  As of this writing the exact hardware interface is
-	  strongly in flux, so no good recommendation can be made.
+	  initially work for you.  As of this writing the exact hardware
+	  interface is strongly in flux, so no good recommendation can be
+	  made.
 
 config CRASH_DUMP
 	  bool "kernel crash dumps"
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index e12764c..dccd7ce 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2305,9 +2305,9 @@ config KEXEC
 
 	  It is an ongoing process to be certain the hardware in a machine
 	  is properly shutdown, so do not be surprised if this code does not
-	  initially work for you.  It may help to enable device hotplugging
-	  support.  As of this writing the exact hardware interface is
-	  strongly in flux, so no good recommendation can be made.
+	  initially work for you.  As of this writing the exact hardware
+	  interface is strongly in flux, so no good recommendation can be
+	  made.
 
 config CRASH_DUMP
 	  bool "Kernel crash dumps"
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index dbd9d3c..cc61418 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -369,9 +369,9 @@ config KEXEC
 
 	  It is an ongoing process to be certain the hardware in a machine
 	  is properly shutdown, so do not be surprised if this code does not
-	  initially work for you.  It may help to enable device hotplugging
-	  support.  As of this writing the exact hardware interface is
-	  strongly in flux, so no good recommendation can be made.
+	  initially work for you.  As of this writing the exact hardware
+	  interface is strongly in flux, so no good recommendation can be
+	  made.
 
 config CRASH_DUMP
 	bool "Build a kdump crash kernel"
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 1020dd8..1018ed3 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -643,9 +643,9 @@ config KEXEC
 
 	  It is an ongoing process to be certain the hardware in a machine
 	  is properly shutdown, so do not be surprised if this code does not
-	  initially work for you.  It may help to enable device hotplugging
-	  support.  As of this writing the exact hardware interface is
-	  strongly in flux, so no good recommendation can be made.
+	  initially work for you.  As of this writing the exact hardware
+	  interface is strongly in flux, so no good recommendation can be
+	  made.
 
 config CRASH_DUMP
 	bool "kernel crash dumps (EXPERIMENTAL)"
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b32ebf9..6ace5de 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1627,9 +1627,9 @@ config KEXEC
 
 	  It is an ongoing process to be certain the hardware in a machine
 	  is properly shutdown, so do not be surprised if this code does not
-	  initially work for you.  It may help to enable device hotplugging
-	  support.  As of this writing the exact hardware interface is
-	  strongly in flux, so no good recommendation can be made.
+	  initially work for you.  As of this writing the exact hardware
+	  interface is strongly in flux, so no good recommendation can be
+	  made.
 
 config CRASH_DUMP
 	bool "kernel crash dumps"
-- 
1.7.9.5

^ permalink raw reply related

* Re: linux-next: build failure after merge of the final tree
From: Ben Myers @ 2013-08-20 19:28 UTC (permalink / raw)
  To: Dwight Engen, Jeremy Kerr
  Cc: cbe-oss-dev, Stephen Rothwell, Arnd Bergmann, linux-kernel, xfs,
	linux-next, Gao feng, linuxppc-dev
In-Reply-To: <20130820120702.000b044e@oracle.com>

Hi Jeremy,

Apologies for breaking your build...

On Tue, Aug 20, 2013 at 12:07:02PM -0400, Dwight Engen wrote:
> On Tue, 20 Aug 2013 17:20:52 +1000
> Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > After merging the final tree, today's linux-next build (powerpc
> > allyesconfig) failed like this:
> > 
> > arch/powerpc/platforms/cell/spufs/inode.c: In function
> > 'spufs_parse_options':
> > arch/powerpc/platforms/cell/spufs/inode.c:623:16: error: incompatible
> > types when assigning to type 'kuid_t' from type 'int' root->i_uid =
> > option; ^ arch/powerpc/platforms/cell/spufs/inode.c:628:16: error:
> > incompatible types when assigning to type 'kgid_t' from type 'int'
> > root->i_gid = option; ^
> > 
> > Caused by commit d6970d4b726c ("enable building user namespace with
> > xfs") from the xfs tree (that was fun to find :-)).
> > 
> > I have reverted that commit for today.  It could probably be replaced
> > with a patch that just changed XFS_FS to SPU_FS in the
> > UIDGID_CONVERTED config dependency - or someone could fix up SPU_FS.
> 
> Hi, (already sent this email based on Intel's kbuild robot this
> morning, sorry for the dup to those who already got it). 
> 
> Yep this looks to me like SPU_FS should have been in the list of
> stuff that had not been UIDGID_CONVERTED, but reviving
> UIDGID_CONVERTED and adding SPU_FS to it won't work for
> non powerpc arch because SPU_FS = n won't be defined. The following can
> be used to mark it as incompatible with USER_NS:
> 
> diff --git a/arch/powerpc/platforms/cell/Kconfig
> b/arch/powerpc/platforms/cell/Kconfig index 9978f59..fcf8336 100644
> --- a/arch/powerpc/platforms/cell/Kconfig
> +++ b/arch/powerpc/platforms/cell/Kconfig
> @@ -61,6 +61,7 @@ config SPU_FS
>         tristate "SPU file system"
>         default m
>         depends on PPC_CELL
> +       depends on USER_NS=n
>         select SPU_BASE
>         select MEMORY_HOTPLUG
>         help
> 
> Or if the rest of spufs is already okay for user namespace (I have not
> checked it, but this seems to be the only place it is dealing with
> uid/gid), then the following will fix these particular errors
> (cross-compile tested, but I don't have a powerpc to run test on):
> 
> diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
> index f390042..90fb308 100644
> --- a/arch/powerpc/platforms/cell/spufs/inode.c
> +++ b/arch/powerpc/platforms/cell/spufs/inode.c
> @@ -620,12 +620,12 @@ spufs_parse_options(struct super_block *sb, char *options, struct inode *root)
>                 case Opt_uid:
>                         if (match_int(&args[0], &option))
>                                 return 0;
> -                       root->i_uid = option;
> +                       root->i_uid = make_kuid(&init_user_ns, option);
>                         break;
>                 case Opt_gid:
>                         if (match_int(&args[0], &option))
>                                 return 0;
> -                       root->i_gid = option;
> +                       root->i_gid = make_kgid(&init_user_ns, option);
>                         break;
>                 case Opt_mode:
>                         if (match_octal(&args[0], &option))

I'd prefer not to break Stephen's tree two days in a row.  We could just revert
d6970d4b726c in the xfs tree for the time being as Stephen has done, but given
the choice would prefer the fix.  Do you have a preference between the two
approaches that Dwight has posted?  The first seems more conservative...

Thanks,
	Ben

^ permalink raw reply

* Re: [PATCH v9 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
From: Mark Brown @ 2013-08-20 19:11 UTC (permalink / raw)
  To: Stephen Warren
  Cc: mark.rutland, devicetree, alsa-devel, lars, festevam, s.hauer,
	Nicolin Chen, timur, rob.herring, tomasz.figa, p.zabel, R65777,
	shawn.guo, linuxppc-dev
In-Reply-To: <5213965C.6010908@wwwdotorg.org>

[-- Attachment #1: Type: text/plain, Size: 393 bytes --]

On Tue, Aug 20, 2013 at 10:16:28AM -0600, Stephen Warren wrote:

> What changed between v8 and v9?

There was a changelog in the cover mail for the series:

| Changelog:
| v8->v9:
|  * Use bool instead of atomic_t.
|  * Use clk_prepare_enable() instead.
|  * Dropped dumpregs().
|  * Dropped unnecessary clk_round_rate().
|  * Dropped unused clock source enum.
|  * Revised dev_err() message.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v8 2/2] ASoC: fsl: Add S/PDIF machine driver
From: Mark Brown @ 2013-08-20 19:07 UTC (permalink / raw)
  To: Stephen Warren
  Cc: mark.rutland, devicetree, alsa-devel, lars, festevam, s.hauer,
	Nicolin Chen, timur, rob.herring, tomasz.figa, p.zabel, R65777,
	shawn.guo, linuxppc-dev
In-Reply-To: <52138FDE.3080007@wwwdotorg.org>

[-- Attachment #1: Type: text/plain, Size: 482 bytes --]

On Tue, Aug 20, 2013 at 09:48:46AM -0600, Stephen Warren wrote:
> On 08/19/2013 06:18 PM, Mark Brown wrote:

> > S/PDIF is also sometimes used as an interconnect between devices -
> > some CODECs have S/PDIF I/O (more normally used as an external
> > connector on the box).  This is most frequently seen as a way to

> That's not a good argument for an incorrect binding.

The point is that it might turn into a more correct binding depending on
what the S/PDIF device actually is.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [alsa-devel] [PATCH v9 2/2] ASoC: fsl: Add S/PDIF machine driver
From: Stephen Warren @ 2013-08-20 19:00 UTC (permalink / raw)
  To: Nicole Otsuka
  Cc: mark.rutland, devicetree, alsa-devel, lars, linuxppc-dev, s.hauer,
	Nicolin Chen, timur, rob.herring, tomasz.figa, broonie, p.zabel,
	R65777, shawn.guo, festevam
In-Reply-To: <CAGoOwPQ_7F1-3AetKBF35pwO_-LNY6yUtx_r8Mn+HZia_WqyoA@mail.gmail.com>

On 08/20/2013 10:54 AM, Nicole Otsuka wrote:
> Hi Stephen,
> 
> I have a whole changelist in the cover letter (PATCH 0/2). Please take a
> look at it.
> 
> And about the codec dummy things.. I do that only because the codec
> driver's doc say so.

The DT binding document is not meant to be influenced by internal driver
architecture.

^ permalink raw reply

* Re: [alsa-devel] [PATCH v9 2/2] ASoC: fsl: Add S/PDIF machine driver
From: Nicole Otsuka @ 2013-08-20 16:54 UTC (permalink / raw)
  To: Stephen Warren
  Cc: mark.rutland, devicetree, alsa-devel, lars, linuxppc-dev, s.hauer,
	Nicolin Chen, timur, rob.herring, tomasz.figa, broonie, p.zabel,
	R65777, shawn.guo, festevam
In-Reply-To: <52139680.1010800@wwwdotorg.org>

[-- Attachment #1: Type: text/plain, Size: 1350 bytes --]

Hi Stephen,

I have a whole changelist in the cover letter (PATCH 0/2). Please take a
look at it.

And about the codec dummy things.. I do that only because the codec
driver's doc say so.

Thank you,
Nicolin


On Wed, Aug 21, 2013 at 12:17 AM, Stephen Warren <swarren@wwwdotorg.org>wrote:

> On 08/19/2013 10:32 PM, Nicolin Chen wrote:
> > This patch implements a device-tree-only machine driver for Freescale
> > i.MX series Soc. It works with spdif_transmitter/spdif_receiver and
> > fsl_spdif.c drivers.
> >
> > Signed-off-by: Nicolin Chen <b42378@freescale.com>
> > ---
> >  .../devicetree/bindings/sound/imx-audio-spdif.txt  |   29 +++++
>
> What changed between v8 and v9?
>
> > diff --git a/Documentation/devicetree/bindings/sound/imx-audio-spdif.txt
> b/Documentation/devicetree/bindings/sound/imx-audio-spdif.txt
>
> > +Optional properties:
> > +
> > +  - spdif-transmitter : The phandle of the spdif-transmitter dummy codec
> > +
> > +  - spdif-receiver : The phandle of the spdif-receiver dummy codec
> > +
> > +* Note: At least one of these two properties should be set in the DT
> binding.
>
> I still don't think "dummy CODEC" is an appropriate thing to have in DT.
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>

[-- Attachment #2: Type: text/html, Size: 2158 bytes --]

^ permalink raw reply

* Re: [PATCH v7 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
From: Stephen Warren @ 2013-08-20 16:28 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	alsa-devel@alsa-project.org, lars@metafoo.de, festevam@gmail.com,
	s.hauer@pengutronix.de, Nicolin Chen, timur@tabi.org,
	rob.herring@calxeda.com, tomasz.figa@gmail.com,
	broonie@kernel.org, p.zabel@pengutronix.de, R65777@freescale.com,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20130820051947.GB29114@S2101-09.ap.freescale.net>

On 08/19/2013 11:19 PM, Shawn Guo wrote:
> On Mon, Aug 19, 2013 at 10:54:33AM +0100, Mark Rutland wrote:
>>> I guess it's better to drop the 'imx6q-spdif' here?
>>
>> That depends:
>>
>> * If the two IP blocks are identical, only the "imx35-spdif" name is
>>   necessary, and we can forget about "fsl,imx6q-spdif".
>>
>> * If "fsl,imx6q-spdif" is a strict superset of "fsl,imx35-spdif", having
>>   both names documented and in a compatible list for a "fsl,imx6q-spdif"
>>   device makes sense.
> 
> Practically, I found it's very useful to have "fsl,<soc>-<ip>" in the
> device compatible property in <soc>.dtsi, even when device driver does
> not match it right now.  For this example, I still prefer to have the
> following line for spdif device in imx6q.dtsi.
> 
> 	compatible = "fsl,imx6q-spdif", "fsl,imx35-spdif";
> 
> The reason for that is we usually do not see all the differences of an
> IP block from one SoC to another when we firstly define the bindings
> for the device by looking at hardware reference manual.  Some
> programming model differences are only identified when we're actually
> programming.  That said, if some day we find there is difference between
> imx6q-spdif and imx35-spdif to be handled when we add something new to
> the driver, we only need to add "fsl,imx6q-spdif" as a new compatible
> into device driver and bindings document.  The existing device tree
> would need no update to work with the new kernel driver.

Yes, we should definitely include the specific SoC model in the
compatible property even in the case where we've done exhaustive
research and validated that the two IP blocks are indeed 100% identical.

This enables us to retro-actively enable and bug workarounds or quirks
which are required for one SoC but not the other. If the compatible
value is already there in DT, we just need a code-change. If the
compatible value isn't already there in DT, we need to change the DT as
well, which means only new DTs will work correctly with new kernels.

^ permalink raw reply

* Re: [PATCH v9 2/2] ASoC: fsl: Add S/PDIF machine driver
From: Stephen Warren @ 2013-08-20 16:17 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: mark.rutland, devicetree, alsa-devel, lars, festevam, s.hauer,
	timur, rob.herring, tomasz.figa, broonie, p.zabel, R65777,
	shawn.guo, linuxppc-dev
In-Reply-To: <ede222d17047e863da34a78069ceead2668bd5d2.1376972170.git.b42378@freescale.com>

On 08/19/2013 10:32 PM, Nicolin Chen wrote:
> This patch implements a device-tree-only machine driver for Freescale
> i.MX series Soc. It works with spdif_transmitter/spdif_receiver and
> fsl_spdif.c drivers.
> 
> Signed-off-by: Nicolin Chen <b42378@freescale.com>
> ---
>  .../devicetree/bindings/sound/imx-audio-spdif.txt  |   29 +++++

What changed between v8 and v9?

> diff --git a/Documentation/devicetree/bindings/sound/imx-audio-spdif.txt b/Documentation/devicetree/bindings/sound/imx-audio-spdif.txt

> +Optional properties:
> +
> +  - spdif-transmitter : The phandle of the spdif-transmitter dummy codec
> +
> +  - spdif-receiver : The phandle of the spdif-receiver dummy codec
> +
> +* Note: At least one of these two properties should be set in the DT binding.

I still don't think "dummy CODEC" is an appropriate thing to have in DT.

^ permalink raw reply

* Re: [PATCH v9 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
From: Stephen Warren @ 2013-08-20 16:16 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: mark.rutland, devicetree, alsa-devel, lars, festevam, s.hauer,
	timur, rob.herring, tomasz.figa, broonie, p.zabel, R65777,
	shawn.guo, linuxppc-dev
In-Reply-To: <03182ed7464c156ec6345194d3f7ec920f753c2f.1376972170.git.b42378@freescale.com>

On 08/19/2013 10:32 PM, Nicolin Chen wrote:
> This patch implements a device-tree-only CPU DAI driver for Freescale
> S/PDIF controller that supports stereo playback and record feature.
> 
> Signed-off-by: Nicolin Chen <b42378@freescale.com>
> ---
>  .../devicetree/bindings/sound/fsl,spdif.txt        |   54 +

What changed between v8 and v9?

^ permalink raw reply

* Re: linux-next: build failure after merge of the final tree
From: Dwight Engen @ 2013-08-20 16:07 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: cbe-oss-dev, Arnd Bergmann, David Chinner, linux-kernel, xfs,
	Ben Myers, linux-next, Gao feng, linuxppc-dev, Jeremy Kerr
In-Reply-To: <20130820172052.1f0d89ddf6a1a40ef70333fd@canb.auug.org.au>

On Tue, 20 Aug 2013 17:20:52 +1000
Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> After merging the final tree, today's linux-next build (powerpc
> allyesconfig) failed like this:
> 
> arch/powerpc/platforms/cell/spufs/inode.c: In function
> 'spufs_parse_options':
> arch/powerpc/platforms/cell/spufs/inode.c:623:16: error: incompatible
> types when assigning to type 'kuid_t' from type 'int' root->i_uid =
> option; ^ arch/powerpc/platforms/cell/spufs/inode.c:628:16: error:
> incompatible types when assigning to type 'kgid_t' from type 'int'
> root->i_gid = option; ^
> 
> Caused by commit d6970d4b726c ("enable building user namespace with
> xfs") from the xfs tree (that was fun to find :-)).
> 
> I have reverted that commit for today.  It could probably be replaced
> with a patch that just changed XFS_FS to SPU_FS in the
> UIDGID_CONVERTED config dependency - or someone could fix up SPU_FS.

Hi, (already sent this email based on Intel's kbuild robot this
morning, sorry for the dup to those who already got it). 

Yep this looks to me like SPU_FS should have been in the list of
stuff that had not been UIDGID_CONVERTED, but reviving
UIDGID_CONVERTED and adding SPU_FS to it won't work for
non powerpc arch because SPU_FS = n won't be defined. The following can
be used to mark it as incompatible with USER_NS:

diff --git a/arch/powerpc/platforms/cell/Kconfig
b/arch/powerpc/platforms/cell/Kconfig index 9978f59..fcf8336 100644
--- a/arch/powerpc/platforms/cell/Kconfig
+++ b/arch/powerpc/platforms/cell/Kconfig
@@ -61,6 +61,7 @@ config SPU_FS
        tristate "SPU file system"
        default m
        depends on PPC_CELL
+       depends on USER_NS=n
        select SPU_BASE
        select MEMORY_HOTPLUG
        help

Or if the rest of spufs is already okay for user namespace (I have not
checked it, but this seems to be the only place it is dealing with
uid/gid), then the following will fix these particular errors
(cross-compile tested, but I don't have a powerpc to run test on):

diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
index f390042..90fb308 100644
--- a/arch/powerpc/platforms/cell/spufs/inode.c
+++ b/arch/powerpc/platforms/cell/spufs/inode.c
@@ -620,12 +620,12 @@ spufs_parse_options(struct super_block *sb, char *options, struct inode *root)
                case Opt_uid:
                        if (match_int(&args[0], &option))
                                return 0;
-                       root->i_uid = option;
+                       root->i_uid = make_kuid(&init_user_ns, option);
                        break;
                case Opt_gid:
                        if (match_int(&args[0], &option))
                                return 0;
-                       root->i_gid = option;
+                       root->i_gid = make_kgid(&init_user_ns, option);
                        break;
                case Opt_mode:
                        if (match_octal(&args[0], &option))

^ permalink raw reply related

* Re: [PATCH v8 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
From: Stephen Warren @ 2013-08-20 15:49 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: mark.rutland, devicetree, alsa-devel, lars, Pawel Moll, festevam,
	s.hauer, Kumar Gala, timur, rob.herring, tomasz.figa, broonie,
	p.zabel, R65777, shawn.guo, linuxppc-dev
In-Reply-To: <20130820022858.GB13169@MrMyself>

On 08/19/2013 08:28 PM, Nicolin Chen wrote:
> On Mon, Aug 19, 2013 at 03:35:58PM -0600, Stephen Warren wrote:
>>> +	"core"		The core clock of spdif controller
>>> +	"rxtx<0-7>"	Clock source list for tx and rx clock.
>>> +			This clock list should be identical to
>>> +			the source list connecting to the spdif
>>> +			clock mux in "SPDIF Transceiver Clock
>>> +			Diagram" of SoC reference manual. It
>>> +			can also be referred to TxClk_Source
>>> +			bit of register SPDIF_STC.
>>
>> So, the HW block has 1 clock input, yet there's a mux somewhere else in
>> the SoC which has 8 inputs?
>>
>> If so, I'm not completely sure it's correct to reference anything other
>> than the "core" clock in this binding. I think the other clocks would be
>> more suitably represented in the system-level "sound card" binding that
>> I guess patch 2/2 (which I haven't read yet) adds, since I assume those
>> clock are more to do with system-level clock tree setup decisions, and
>> might not even exist in some other SoC that included this IP block.
>>
>> What do others think, assuming I'm correct about my HW design assumptions?
> 
> The core clock is being only needed when accessing registers of this IP.
> Thus, in the driver, I let regmap handle it.
> 
> While the other 8 clocks are actual reference clocks for Tx. Tx clock needs
> to select one of them that can easily derive a child clock matching the tx
> sample rate. This is essential for the IP, so I don't think it's nicer to 
> put into machine driver.

So just to be clear, the S/PDIF IP block truly has 8 rxtx clock input
signals, and the mux between them is internal to the S/PDIF block? If
so, then this aspect of the binding is fine.

^ permalink raw reply

* Re: [PATCH v8 2/2] ASoC: fsl: Add S/PDIF machine driver
From: Stephen Warren @ 2013-08-20 15:48 UTC (permalink / raw)
  To: Mark Brown
  Cc: mark.rutland, devicetree, alsa-devel, lars, festevam, s.hauer,
	Nicolin Chen, timur, rob.herring, tomasz.figa, p.zabel, R65777,
	shawn.guo, linuxppc-dev
In-Reply-To: <20130820001858.GF30073@sirena.org.uk>

On 08/19/2013 06:18 PM, Mark Brown wrote:
> On Mon, Aug 19, 2013 at 03:39:26PM -0600, Stephen Warren wrote:
>> On 08/19/2013 06:08 AM, Nicolin Chen wrote:
>>> This patch implements a device-tree-only machine driver for
>>> Freescale i.MX series Soc. It works with
>>> spdif_transmitter/spdif_receiver and fsl_spdif.c drivers.
>> 
>>> diff --git
>>> a/Documentation/devicetree/bindings/sound/imx-audio-spdif.txt
>>> b/Documentation/devicetree/bindings/sound/imx-audio-spdif.txt
> 
>>> +Optional properties:
> 
>>> +  - spdif-transmitter : The phandle of the spdif-transmitter
>>> dummy codec +  - spdif-receiver : The phandle of the
>>> spdif-receiver dummy codec
> 
>>> +* Note: At least one of these two properties should be set in
>>> the DT binding.
> 
>> Those object truly don't exist in HW and only exist due to
>> internal details of Linux's ASoC subsystem.
> 
> They will physically exist if they're usefully present on the board
> (in the sense that they're there and can be pointed at) - there
> will be either a TOSLINK optical connector or (less commonly) an
> electrical connector breaking the signal out to go elsewhere though
> they don't have any interaction with software usually which is more
> what you mean here.

Sure, the S/PDIF signal is connected to something, but it is not a
"dummy CODEC"; a "dummy CODEC" is purely something internal to ASoC
and absolutely nothing to do with HW.

>> Or, to map the properties more directly to HW, perhaps name the
>> property "spdif-tx-jack-exists", or even "spdif-tx-jack" and make
>> it a phandle to a node that represents the actual S/PDIF
>> connector?
> 
> S/PDIF is also sometimes used as an interconnect between devices -
> some CODECs have S/PDIF I/O (more normally used as an external
> connector on the box).  This is most frequently seen as a way to
> plumb HDMI in since some HDMI devices seem to provide this as a
> legacy interconnect, though it can get used just for regular CODECs
> as well.  Using this machine driver would probably be a bit of an
> abuse for some applications, though with things like the HDMI one
> the goal of the hardware is to be dropped into a driver like this
> so perhaps it makes sense and is useful anyway.
> 
> Equally well it'd be good to get this stuff actually merged, it
> seems to have been surprisingly time consuming thus far...

That's not a good argument for an incorrect binding.

^ permalink raw reply


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