LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] [POWERPC] powerpc: Add -mno-spe for ARCH=powerpc builds
From: Kumar Gala @ 2007-10-19  3:20 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <18199.59909.763111.810973@cargo.ozlabs.ibm.com>


On Oct 18, 2007, at 6:19 PM, Paul Mackerras wrote:

> Kumar Gala writes:
>
>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>
> Your commit message doesn't give any reason why you are doing this, or
> any explanation of what goes wrong without it.  In fact, the commit
> message is completely empty. :)  Please resubmit with a decent commit
> message.

I will, just as an FYI I based this on your commit for -mno-altivec  
(which has no rationale for the commit) :)

- k

^ permalink raw reply

* Re: [PATCH] synchronize_irq needs a barrier
From: Nick Piggin @ 2007-10-19  2:52 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Linux Kernel Mailing List, linuxppc-dev, Thomas Gleixner, akpm,
	Linus Torvalds, Ingo Molnar
In-Reply-To: <20071019023219.GB8453@gondor.apana.org.au>

On Friday 19 October 2007 12:32, Herbert Xu wrote:

> First of all let's agree on some basic assumptions:
>
> * A pair of spin lock/unlock subsumes the effect of a full mb.

Not unless you mean a pair of spin lock/unlock as in
2 spin lock/unlock pairs (4 operations).

*X = 10;
spin_lock(&lock);
/* *Y speculatively loaded here */
/* store to *X leaves CPU store queue here */
spin_unlock(&lock);
y = *Y;

> * A spin lock in general only equates to (SS/SL/LL).
> * A spin unlock in general only equates to (SS/LS).

I don't use the sparc barriers, so they don't come naturally to
me ;)

I think both loads and stores can pass into the critical section
by having the spin_lock pass earlier ops, or by having spin_unlock
be passed by later ones.


> In particular, a load after a spin unlock may pass before the
> spin unlock.
>
> Here is the race (with tg3 as the only example that I know of).
> The driver attempts to quiesce interrupts such that after the
> call to synchronize_irq it is intended that no further IRQ
> handler calls for that device will do any work besides acking
> the IRQ.
>
> Here is how it does it:
>
> CPU0				CPU1
> 				spin lock
> 					load irq_sync
> irq_sync = 1
> smp_mb
> synchronize_irq()
> 	while (IRQ_INPROGRESS)
> 		wait
> 	return
> 				set IRQ_INPROGRESS
> 				spin unlock
> 				tg3_msi
> 					ack IRQ
> 					if (irq_sync)
> 						return
> 					do work
>
> The problem here is that load of irq_sync in the handler has
> passed above the setting of IRQ_INPROGRESS.
>
> Linus's patch fixes it because this becomes:
>
> CPU0				CPU1
> 				spin lock
> 					load irq_sync
> irq_sync = 1
> smp_mb
> synchronize_irq
> 				set IRQ_INPROGRESS
> 				spin unlock
> 	spin lock
> 	spin unlock
> 				tg3_msi
> 					ack IRQ
> 					if (irq_sync)
> 						return
> 					do work
> 	while (IRQ_INPROGRESS)
> 		wait
> 				spin lock
> 				clear IRQ_INPROGRESS
> 				spin unlock
> 	return
>
> Even though we still do the work on the right we will now notice
> the INPROGRESS flag on the left and wait.
>
> It's hard to fix this in the drivers because they'd either have
> to access the desc lock or add a full mb to the fast path on the
> right.
>
> Once this goes in we can also remove the smp_mb from tg3.c.  BTW,
> a lot of drivers (including the fusion example Ben quoted) call
> synchronize_irq before free_irq.  This is unnecessary because
> the latter already calls it anyway.
>
> Cheers,

^ permalink raw reply

* Re: [PATCH] synchronize_irq needs a barrier
From: Linus Torvalds @ 2007-10-19  2:55 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Linux Kernel Mailing List, linuxppc-dev, Thomas Gleixner, akpm,
	Ingo Molnar
In-Reply-To: <20071019023219.GB8453@gondor.apana.org.au>



On Fri, 19 Oct 2007, Herbert Xu wrote:
>
> In other words I think this patch is great :)

Hey, I appreciate it, but I do think that the "spinlock only to 
immediately unlock it again" is pretty horrid.

I'm convinced that there should be some way to do this without actually 
taking the lock. I *think* it should work with something like

	for (;;) {
		smp_rmb();
		if (!spin_is_locked(&desc->lock)) {
			smp_rmb();
			if (!(desc->status & IRQ_INPROGRESS)
				break;
		}
		cpu_relax();
	}

instead. Which basically requires that we see the descriptor lock being 
not held, before we see the IRQ_INPROGRESS bit being clear. Put another 
way: it loops until it sees the lock having been released, and the 
IRQ_INPROGRESS bit being clear after that.

The above requires no serializing instructions on x86, which is a good 
goal (now that smp_rmb() is just a compiler barrier). And it doesn't 
actually have to bounce any cachelines.

And it doesn't have that ugly "get lock only to release it", which just 
makes me go "Eww!".

But it's a bit subtler. It basically depends on the fact that 
spin_unlock() obviously has to make sure that there is a release barrier 
in the unlock, so any writes done (to the IRQ_INPROGRESS bit) within the 
locked region *must* be visible before the spinlock itself has been 
released.

So somebody should:
 - use another pair of eyes and brains to back me up on this.
 - write up some coherent changelog entry, using the emails that have 
   passed back and forth.
 - actually turn the above into a tested patch with a comment.

And I'm pushing for that "somebody" being somebody else than me ;)

			Linus

^ permalink raw reply

* Re: [PATCH] synchronize_irq needs a barrier
From: Herbert Xu @ 2007-10-19  2:32 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Linux Kernel Mailing List, linuxppc-dev, Thomas Gleixner, akpm,
	Ingo Molnar
In-Reply-To: <alpine.LFD.0.999.0710181627091.26902@woody.linux-foundation.org>

On Thu, Oct 18, 2007 at 04:39:59PM -0700, Linus Torvalds wrote:
>
> Basic notion: the only thing that serializes the IRQ_INPROGRESS flag is 
> the descriptor lock. And we don't have to (or even want to!) hold it while 
> waiting for the thing, but we want to *have*held*it* in between whatever 
> we're synchronizing with.

Thinking about this again and checking code I have come to the
conclusion that

1) There is a race (in some drivers) even with the full mb.
2) Linus's patch fixes it.
3) It's difficult to fix it elegantly in the driver.

In other words I think this patch is great :)

First of all let's agree on some basic assumptions:

* A pair of spin lock/unlock subsumes the effect of a full mb.
* A spin lock in general only equates to (SS/SL/LL).
* A spin unlock in general only equates to (SS/LS).

In particular, a load after a spin unlock may pass before the
spin unlock.

Here is the race (with tg3 as the only example that I know of).
The driver attempts to quiesce interrupts such that after the
call to synchronize_irq it is intended that no further IRQ
handler calls for that device will do any work besides acking
the IRQ.

Here is how it does it:

CPU0				CPU1
				spin lock
					load irq_sync
irq_sync = 1
smp_mb
synchronize_irq()
	while (IRQ_INPROGRESS)
		wait
	return
				set IRQ_INPROGRESS
				spin unlock
				tg3_msi
					ack IRQ
					if (irq_sync)
						return
					do work

The problem here is that load of irq_sync in the handler has
passed above the setting of IRQ_INPROGRESS.

Linus's patch fixes it because this becomes:

CPU0				CPU1
				spin lock
					load irq_sync
irq_sync = 1
smp_mb
synchronize_irq
				set IRQ_INPROGRESS
				spin unlock
	spin lock
	spin unlock
				tg3_msi
					ack IRQ
					if (irq_sync)
						return
					do work
	while (IRQ_INPROGRESS)
		wait
				spin lock
				clear IRQ_INPROGRESS
				spin unlock
	return

Even though we still do the work on the right we will now notice
the INPROGRESS flag on the left and wait.

It's hard to fix this in the drivers because they'd either have
to access the desc lock or add a full mb to the fast path on the
right.

Once this goes in we can also remove the smp_mb from tg3.c.  BTW,
a lot of drivers (including the fusion example Ben quoted) call
synchronize_irq before free_irq.  This is unnecessary because
the latter already calls it anyway.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH v2] [2/2] powerpc: Switch to generic WARN_ON()/BUG_ON()
From: Olof Johansson @ 2007-10-19  2:03 UTC (permalink / raw)
  To: linux-kernel; +Cc: grundler, kyle, linuxppc-dev, lethal, akpm
In-Reply-To: <20071011171413.GC10877@lixom.net>

[POWERPC] Switch to generic WARN_ON()/BUG_ON()

Not using the ppc-specific WARN_ON/BUG_ON constructs actually saves about
4K text on a ppc64_defconfig. The main reason seems to be that prepping
the arguments to the conditional trap instructions is more work than
just doing a compare and branch.

Signed-off-by: Olof Johansson <olof@lixom.net>

Index: k.org/include/asm-powerpc/bug.h
===================================================================
--- k.org.orig/include/asm-powerpc/bug.h
+++ k.org/include/asm-powerpc/bug.h
@@ -54,12 +54,6 @@
 	".previous\n"
 #endif
 
-/*
- * BUG_ON() and WARN_ON() do their best to cooperate with compile-time
- * optimisations. However depending on the complexity of the condition
- * some compiler versions may not produce optimal results.
- */
-
 #define BUG() do {						\
 	__asm__ __volatile__(					\
 		"1:	twi 31,0,0\n"				\
@@ -69,20 +63,6 @@
 	for(;;) ;						\
 } while (0)
 
-#define BUG_ON(x) do {						\
-	if (__builtin_constant_p(x)) {				\
-		if (x)						\
-			BUG();					\
-	} else {						\
-		__asm__ __volatile__(				\
-		"1:	"PPC_TLNEI"	%4,0\n"			\
-		_EMIT_BUG_ENTRY					\
-		: : "i" (__FILE__), "i" (__LINE__), "i" (0),	\
-		  "i" (sizeof(struct bug_entry)),		\
-		  "r" ((__force long)(x)));			\
-	}							\
-} while (0)
-
 #define __WARN() do {						\
 	__asm__ __volatile__(					\
 		"1:	twi 31,0,0\n"				\
@@ -92,23 +72,6 @@
 		  "i" (sizeof(struct bug_entry)));		\
 } while (0)
 
-#define WARN_ON(x) ({						\
-	int __ret_warn_on = !!(x);				\
-	if (__builtin_constant_p(__ret_warn_on)) {		\
-		if (__ret_warn_on)				\
-			__WARN();				\
-	} else {						\
-		__asm__ __volatile__(				\
-		"1:	"PPC_TLNEI"	%4,0\n"			\
-		_EMIT_BUG_ENTRY					\
-		: : "i" (__FILE__), "i" (__LINE__),		\
-		  "i" (BUGFLAG_WARNING),			\
-		  "i" (sizeof(struct bug_entry)),		\
-		  "r" (__ret_warn_on));				\
-	}							\
-	unlikely(__ret_warn_on);				\
-})
-
 #endif /* __ASSEMBLY __ */
 #endif /* CONFIG_BUG */
 

^ permalink raw reply

* Re: [PATCH v2 3/4] Implement clockevents driver for powerpc
From: Paul Mackerras @ 2007-10-19  1:53 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linuxppc-dev, Thomas Gleixner, Realtime Kernel
In-Reply-To: <471777BD.8090800@ru.mvista.com>

Sergei Shtylyov writes:

>     And now you have incomplete read_persistent_clock() implementation for 

I don't see anything incomplete about it.  If you do, feel free to
post a patch.

> example, god knows why it was preferred to mine -- well, it also implemented 

Your most recent post of your patch to implement read_persistent_clock
was in May -- five months ago -- and you said this about it: "This
patch hasn't received a good testing though".

You don't have to be a god to figure out why I preferred a patch that
had been tested, where the author was responding to comments and
posting updated versions of his patch in the period leading up to the
merge window, over that.

>     Well, that's up to you.  I take it you wouldn't accept a patch 
> implementing auto-reload mode?

I already told you.  Show me numbers (real measurements showing that
it's better) and I'll consider it.

>     There are. I'll have to send patches (it's not that I have time for this) 
> but this is surely the fastest way to get things fixed (if I don't get ignored 
> that is).

All of us only get stuff in by spending the time to develop patches
and posting them for comment.  Stop whinging.

>     I just wanted the reasons clarified and got what I wanted -- as I thought, 
> the decision behind preferring patches was somewhat biased, nobody really 
> cared about code quality or just wasn't familiar with hrtimers enough to judge 
> on the code quality...

You really know how to persuade people to cooperate, don't you...  :P

Paul.

^ permalink raw reply

* Re: Freescale Interrupt enabling
From: David Gibson @ 2007-10-19  1:46 UTC (permalink / raw)
  To: Alan Bennett; +Cc: linuxppc-dev
In-Reply-To: <bfa0697f0710180741i5063ad94ga13086bf3c43a511@mail.gmail.com>

On Thu, Oct 18, 2007 at 08:41:35AM -0600, Alan Bennett wrote:
> We were developing with Linux 2.6.10 and a Planetcore boot loader,
> however, recent work has us up and running with 2.6.23+ and U-boot
> 1.2.0.  However, we are now running into a few challenges regarding
> the differences.
> 
> Our driver writer's code isn't functioning, but it was with 2.6.10 and
> planet core.  The best I can tell is that the default interrupt
> controller configuration isn't where it was in the planetcore/2.6.10
> version.
> 
> for example, let's look at enabling timer1 / interrupt number 12
> 
> simple description.
> 	       timer1 {
> 	 	       name = "timer1";
>  		       compatible = "fsl,mpc8248_timer";
> 		       interrupts = <c 8>;
> 		       interrupt-parent = <&PIC>;

Device tree nodes should really describe an actual device, not just
some random floating interrupt.  You need to work out what device this
interrupt actually belongs to, and describe that.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: Merge dtc
From: David Gibson @ 2007-10-19  1:45 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: ppcdev, Paul Mackerras, Milton Miller
In-Reply-To: <20071018195926.GA22887@uranus.ravnborg.org>

On Thu, Oct 18, 2007 at 09:59:26PM +0200, Sam Ravnborg wrote:
> On Thu, Oct 18, 2007 at 12:49:54PM -0500, Milton Miller wrote:
> > On Tue Oct 16 15:02:17 EST 2007, David Gibson wrote:
> > 
> > >This very large patch incorporates a copy of dtc into the kernel
> > >source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> > >longer an external dependency to build kernels with configurations
> > >which need a dtb file.
> > >
> > >Signed-off-by: David Gibson <david at gibson.dropbear.id.au>
> > >
> > >Too big for the list, full patch at
> > >http://ozlabs.org/~dgibson/home/merge-dtc.patch+
> > 
> > So split it up.   The obvious one is "here is the unique content, then 
> > copy these files from dtc git" would have been better.
> One obvious split is a patch solely containing the _shipped files.
> And next patch the rest.

Um.. why?  No way is that a logical split.

> As Milton already pointed out you should build dtc in the
> dtc directory (why the -src prefix??).

The -src suffix is only there because I'm not building in the
directory - we can't have both a dtc binary and a dtc directory in
arch/powerpc/boot.  

Ok, so how do I build in the subdirectory?  I was going to do that,
but couldn't for the life of me figure out how.

> And the dtc specific Makefile looks like something from
> the late 80'. Please drop all these ALLUPPERCASE variables
> and accept a little bit of redundancy.

Hrm... I'm pretty dubious about this.  Practically every Makefile in
the universe, *except* Kbuild uses uppercase for most variables.
Makefile.dtc is imported verbatim from the standalone dtc package, and
is supposed to have the minimal information about what needs to be
built to import into Makefiles that actually know how to build things.

> Then mere humans may be able to read the Makefile.

Says a maintainer of Kbuild, about my tiny and not-very-complex
Makefile fragment... um, ok...

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: Merge dtc
From: David Gibson @ 2007-10-19  1:30 UTC (permalink / raw)
  To: Milton Miller; +Cc: ppcdev, Paul Mackerras, Sam Ravnborg
In-Reply-To: <9a3f3582990ad94feb76770e82c50bdd@bga.com>

On Thu, Oct 18, 2007 at 12:49:54PM -0500, Milton Miller wrote:
> On Tue Oct 16 15:02:17 EST 2007, David Gibson wrote:
> 
> > This very large patch incorporates a copy of dtc into the kernel
> > source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> > longer an external dependency to build kernels with configurations
> > which need a dtb file.
> >
> > Signed-off-by: David Gibson <david at gibson.dropbear.id.au>
> >
> > Too big for the list, full patch at
> > http://ozlabs.org/~dgibson/home/merge-dtc.patch+
> 
> So split it up.   The obvious one is "here is the unique content, then 
> copy these files from dtc git" would have been better.

*facepalm*

The point of the small patches thing is not to glom together logically
separate patches.  It's *not* to split patches up simply for the sake
of making them smaller.

You've suggested the closest thing to a logical split here, but even
then - the dtc files are dead without the Makefile changes, and the
Makefile changes won't build without the dtc files (so the patches
would have to go in reversed order to what you suggest).

Not to mention that the Makefile patch will be tiny and the raw import
will still be way too big for the list.

> I finally went and looked at the url.  The Kbuild integration is wrong. 

Wrong?  Not how you'd prefer them perhaps...

>   It should build dtc in dtc-src and run the binary from there, and the 
> rules should be in a Kconfig as a normal host-target in that directory. 

Why?  This approach has the advantage that the subdirectory contains
*only* verbatim imported files, which could well make updates simpler.

>    (I don't have a problem with that Kbuild including Makefie.dtc).
> 
> things like the dependancy on scripts_basic in the top level
> Makefile.

I'm not even sure what you mean by this.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH v5 9/9] add MPC837x MDS board default device tree
From: David Gibson @ 2007-10-19  0:56 UTC (permalink / raw)
  To: Li Yang; +Cc: linuxppc-dev, paulus
In-Reply-To: <1192702580-6353-1-git-send-email-leoli@freescale.com>

On Thu, Oct 18, 2007 at 06:16:20PM +0800, Li Yang wrote:
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
> diff --git a/arch/powerpc/boot/dts/mpc8377_mds.dts b/arch/powerpc/boot/dts/mpc8377_mds.dts
> new file mode 100644
> index 0000000..8530de6
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/mpc8377_mds.dts
> @@ -0,0 +1,282 @@
[snip]
> +		crypto@30000 {
> +			model = "SEC3";
> +			compatible = "talitos";

That compatible doesn't look specific enough.  It should at least have
a vendor portion.  In general it's best to have all the information
you need to pick a driver and options in compatible, rather than
splitting that info into model.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH v6 4/9] add platform support for MPC837x MDS board
From: Stephen Rothwell @ 2007-10-19  0:43 UTC (permalink / raw)
  To: Li Yang; +Cc: linuxppc-dev, paulus
In-Reply-To: <1192719847-25045-4-git-send-email-leoli@freescale.com>

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

On Thu, 18 Oct 2007 23:04:02 +0800 Li Yang <leoli@freescale.com> wrote:
>
> +++ b/arch/powerpc/platforms/83xx/mpc837x_mds.c
> +static void __init mpc837x_mds_setup_arch(void)
> +{
> +#ifdef CONFIG_PCI
> +	struct device_node *np;
> +#endif
> +
> +	if (ppc_md.progress)
> +		ppc_md.progress("mpc837x_mds_setup_arch()", 0);
> +
> +#ifdef CONFIG_PCI
> +	for (np = NULL; (np = of_find_node_by_type(np, "pci")) != NULL;)

	for_each_node_by_type(np, "pci")

> +static int __init mpc837x_mds_probe(void)
> +{
> +        unsigned long root = of_get_flat_dt_root();
> +
> +        return of_flat_dt_is_compatible(root, "fsl,mpc837xmds");

To call these two routines, you should include <asm/prom.h> directly.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

^ permalink raw reply

* tiny login: useradd command
From: pjmaiya @ 2007-10-18 10:24 UTC (permalink / raw)
  To: linuxppc-embedded

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

Hi,
 I am using montavista linux. Using TCT tool i have added package for user creation. I am having following problem
  a.. If I use tiny login package, I will be getting useradd binary but number of parameter are few like
 Usage: adduser [OPTIONS]... <USER>
Options:

  -h <directory>    specify home directory
  -s <shell>        specify shell
  -g <gecos>        specify GECOS stringa.. If I don't use tiny login package, I will be selecting useradd package from admin menu. But I am unable to execute this command since it gives follwing error
      /usr/sbin/groupadd missing..
 
  a.. Actually I want useradd command similar to linux where more argument are taken, especially I wanted 'user' to be part of more than one group.
can anyone help me out..

thanx in advance,
pjmaiya

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

^ permalink raw reply

* Re: [PATCH] [POWERPC] ucc_geth: Eliminate compile warnings
From: David Miller @ 2007-10-19  0:30 UTC (permalink / raw)
  To: Emilian.Medve; +Cc: netdev, leoli, jgarzik, linuxppc-dev
In-Reply-To: <1192745713-20829-1-git-send-email-Emilian.Medve@Freescale.com>

From: Emil Medve <Emilian.Medve@freescale.com>
Date: Thu, 18 Oct 2007 17:15:13 -0500

> drivers/net/ucc_geth.c: In function 'ucc_geth_startup':
> drivers/net/ucc_geth.c:2614: warning: assignment makes integer from pointer without a cast
> drivers/net/ucc_geth.c:2651: warning: assignment makes integer from pointer without a cast
> 
> Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>

It only kills the warning on 32-bit systems, the cast is wrong
either way.

>  			ugeth->tx_bd_ring_offset[j] =
> -				kmalloc((u32) (length + align), GFP_KERNEL);
> +				(u32)kmalloc(length + align, GFP_KERNEL);
>  
>  			if (ugeth->tx_bd_ring_offset[j] != 0)
>  				ugeth->p_tx_bd_ring[j] =

Pointers can be up to "unsigned long" in size, therefore that
is the minimal amount of storage you need to store them into
if they are needed in integer form for some reason.

Any cast from pointer to integer like this is a huge red flag.

^ permalink raw reply

* Re: [PATCH v3 2/2] [POWERPC] MPC8568E-MDS: add support for flash
From: Anton Vorontsov @ 2007-10-18 22:29 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <C2A61B74-FDB4-4571-A2F4-F0B7317FD7C7@kernel.crashing.org>

On Thu, Oct 18, 2007 at 02:58:25PM -0500, Kumar Gala wrote:
[...]
> > +
> > +		flash@0,0 {
> > +			#address-cells = <1>;
> > +			#size-cells = <1>;
> > +			compatible = "Spansion,S29GL256N11TFIV2O", "cfi-flash";
> > +			reg = <0 0 2000000>;
> > +			bank-width = <2>;
> > +			device-width = <1>;
> > +
> 
> Are you basing the partition map on something or making it up?   
> Clearly hrcw & u-boot are at fixed offsets, wondering about kernel &  
> rootfs?

I'm making it up. From the brief look at the u-boot* git sources,
there is no `flashboot` yet, thus I'm free to make this up...

I've partitioned this flash based on these thoughts:

1. HRCW - whole sector, to not wear out it, plus it's impossible to
   create just 64 bytes partition;
2. Kernel - 2MB, should be enough for bootup kernels?
3. Rootfs - the rest up to...
4. U-Boot at the end.

I'm not saying that it's best map ever, I'm open to suggestions. ;-)

* Unfortunately I didn't look at the stock Freescale u-boot, maybe
  there was flashboot, most probably.. I'll find the CD to look this
  up.

Much thanks for noticing this,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

^ permalink raw reply

* Re: [PATCH v2 3/4] Implement clockevents driver for powerpc
From: Paul Mackerras @ 2007-10-19  0:14 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linuxppc-dev, Thomas Gleixner, Realtime Kernel
In-Reply-To: <47177229.2030200@ru.mvista.com>

Sergei Shtylyov writes:

> > What problem do you see arising from this?
> 
>     Timers firing too early.

Only if the minimum interrupt latency is less than 1 decrementer
tick.  That seems pretty unlikely to me unless you have a very slow
timebase frequency.

In fact what we should program the decrementer to is:

	timeout - (is_booke? 0: 1) - min_interrupt_latency

I was assuming that min_interrupt_latency (measured in timebase ticks)
would be at least 1, but apparently some systems can have a timebase
frequency as low as 1kHz, so we'll have to have an ifdef or something.

Paul.

^ permalink raw reply

* [PATCH] [POWERPC] ucc_geth: Eliminate compile warnings
From: Emil Medve @ 2007-10-18 22:15 UTC (permalink / raw)
  To: jgarzik, leoli, netdev, linuxppc-dev

drivers/net/ucc_geth.c: In function 'ucc_geth_startup':
drivers/net/ucc_geth.c:2614: warning: assignment makes integer from pointer without a cast
drivers/net/ucc_geth.c:2651: warning: assignment makes integer from pointer without a cast

Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
---

This patch is against Jeff's tree: d85714d81cc0408daddb68c10f7fd69eafe7c213 

netdev-2.6> scripts/checkpatch.pl 0001-POWERPC-ucc_geth-Eliminate-compile-warnings.patch 
Your patch has no obvious style problems and is ready for submission.

 drivers/net/ucc_geth.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index bec413b..d427da8 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -2611,7 +2611,7 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth)
 			if (UCC_GETH_TX_BD_RING_ALIGNMENT > 4)
 				align = UCC_GETH_TX_BD_RING_ALIGNMENT;
 			ugeth->tx_bd_ring_offset[j] =
-				kmalloc((u32) (length + align), GFP_KERNEL);
+				(u32)kmalloc(length + align, GFP_KERNEL);
 
 			if (ugeth->tx_bd_ring_offset[j] != 0)
 				ugeth->p_tx_bd_ring[j] =
@@ -2648,7 +2648,7 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth)
 			if (UCC_GETH_RX_BD_RING_ALIGNMENT > 4)
 				align = UCC_GETH_RX_BD_RING_ALIGNMENT;
 			ugeth->rx_bd_ring_offset[j] =
-				kmalloc((u32) (length + align), GFP_KERNEL);
+				(u32)kmalloc(length + align, GFP_KERNEL);
 			if (ugeth->rx_bd_ring_offset[j] != 0)
 				ugeth->p_rx_bd_ring[j] =
 					(void*)((ugeth->rx_bd_ring_offset[j] +
-- 
1.5.3.GIT

^ permalink raw reply related

* Re: [PATCH] synchronize_irq needs a barrier
From: Benjamin Herrenschmidt @ 2007-10-18 23:52 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Herbert Xu, Linux Kernel Mailing List, linuxppc-dev,
	Thomas Gleixner, akpm, Ingo Molnar
In-Reply-To: <alpine.LFD.0.999.0710181627091.26902@woody.linux-foundation.org>


> So how about something like this (untested! not necessarily very well 
> thought through either!)
> 
> Basic notion: the only thing that serializes the IRQ_INPROGRESS flag is 
> the descriptor lock. And we don't have to (or even want to!) hold it while 
> waiting for the thing, but we want to *have*held*it* in between whatever 
> we're synchronizing with.
> 
> The internal irq handler functions already held the lock when they did 
> whatever they need to serialize - and they are possibly performance 
> critical too - so they use the "internal" function that doesn't get the 
> lock unnecessarily again.

That may do the trick as the read can't cross the spin_lock (it can
cross spin_unlock but not lock). Advantage over adding a barrier to
handle_IRQ_event() is that it keeps the overhead to the slow path
(synchronize_irq).

Note that I didn't actually experience a problem here. I just came upon
that by accident while thinking about a similar issue I have with
napi_synchronize().

Looks good to me on a first glance (unfortunately a bit ugly but heh).

Ben.

^ permalink raw reply

* Re: [PATCH] synchronize_irq needs a barrier
From: Linus Torvalds @ 2007-10-18 23:39 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Herbert Xu, Linux Kernel Mailing List, linuxppc-dev,
	Thomas Gleixner, akpm, Ingo Molnar
In-Reply-To: <1192749449.7367.51.camel@pasglop>



On Fri, 19 Oct 2007, Benjamin Herrenschmidt wrote:
> 
> I agree and you can see that in fact, we don't have enough barrier on
> the other side since spin_unlock doesn't prevent subsequent loads from
> crossing a previous store...
> 
> I wonder if that's worth trying to address, adding a barrier in
> handle_IRQ_event for example, or we can continue ignoring the barrier
> and let some drivers do their own fixes in fancy ways.

So how about something like this (untested! not necessarily very well 
thought through either!)

Basic notion: the only thing that serializes the IRQ_INPROGRESS flag is 
the descriptor lock. And we don't have to (or even want to!) hold it while 
waiting for the thing, but we want to *have*held*it* in between whatever 
we're synchronizing with.

The internal irq handler functions already held the lock when they did 
whatever they need to serialize - and they are possibly performance 
critical too - so they use the "internal" function that doesn't get the 
lock unnecessarily again.

Hmm? 

		Linus

---
 kernel/irq/manage.c |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 80eab7a..f3e9575 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -14,6 +14,18 @@
 
 #include "internals.h"
 
+/*
+ * Internally wait for IRQ_INPROGRESS to go away on other CPU's,
+ * after having serialized with the descriptor lock.
+ */
+static inline void do_synchronize_irq(struct irq_desc *desc)
+{
+#ifdef CONFIG_SMP
+	while (desc->status & IRQ_INPROGRESS)
+		cpu_relax();
+#endif
+}
+
 #ifdef CONFIG_SMP
 
 /**
@@ -28,13 +40,15 @@
  */
 void synchronize_irq(unsigned int irq)
 {
+	unsigned long flags;
 	struct irq_desc *desc = irq_desc + irq;
 
 	if (irq >= NR_IRQS)
 		return;
 
-	while (desc->status & IRQ_INPROGRESS)
-		cpu_relax();
+	spin_lock_irqsave(&desc->lock, flags);
+	spin_unlock_irqrestore(&desc->lock, flags);
+	do_synchronize_irq(desc);
 }
 EXPORT_SYMBOL(synchronize_irq);
 
@@ -129,7 +143,7 @@ void disable_irq(unsigned int irq)
 
 	disable_irq_nosync(irq);
 	if (desc->action)
-		synchronize_irq(irq);
+		do_synchronize_irq(desc);
 }
 EXPORT_SYMBOL(disable_irq);
 
@@ -443,7 +457,7 @@ void free_irq(unsigned int irq, void *dev_id)
 			unregister_handler_proc(irq, action);
 
 			/* Make sure it's not being used on another CPU */
-			synchronize_irq(irq);
+			do_synchronize_irq(desc);
 #ifdef CONFIG_DEBUG_SHIRQ
 			/*
 			 * It's a shared IRQ -- the driver ought to be

^ permalink raw reply related

* Building zImage
From: Siva Prasad @ 2007-10-18 23:22 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <mailman.1363.1192734606.3099.linuxppc-dev@ozlabs.org>

Hi,

When I tried to build zImage (make zImage V=3D1) based on the 2.6.19
kernel for 8641HPCN board, I got the following error...

make -f scripts/Makefile.build obj=3Dlib
make -f
/export/beavis/work/sprasad/2.6.19/linux-2.6.19/scripts/Makefile.modpost
vmlinux
  scripts/mod/modpost -m  -o
/export/beavis/work/sprasad/2.6.19/linux-2.6.19/Module.symvers
vmlinux=20
rm -f .old_version
make ARCH=3Dppc64 -f scripts/Makefile.build obj=3Darch/powerpc/boot
arch/powerpc/boot/zImage
ln: accessing `arch/powerpc/boot/zImage': No such file or directory
make[1]: *** [arch/powerpc/boot/zImage] Error 1
make: *** [zImage] Error 2

When I looked into the Makefile, "image-y" was having no value. So, "ln"
was failing. Do I have to select one of the below options of zImage. I
am not really sure which one to use for 8641 supported by FreeScale.

image-$(CONFIG_PPC_PSERIES)             +=3D zImage.pseries
image-$(CONFIG_PPC_MAPLE)               +=3D zImage.pseries
image-$(CONFIG_PPC_IBM_CELL_BLADE)      +=3D zImage.pseries
image-$(CONFIG_PPC_CHRP)                +=3D zImage.chrp
image-$(CONFIG_PPC_PMAC)                +=3D zImage.pmac
image-$(CONFIG_DEFAULT_UIMAGE)          +=3D uImage

Well!... we are not using U-Boot, so uImage is not an option, and that
builds fine.

Any way, I tried to make uImage, but I am not sure if it is including
the dts file in the final uImage built, as the wrapper was passed the
value of platform as "uboot". I am confused.

Thanks for your help in advance.

- siva

^ permalink raw reply

* 转发: Re: Linux booting problem on Xilinx ppc
From: keng_629 @ 2007-10-18 23:27 UTC (permalink / raw)
  To: linuxppc-embedded
In-Reply-To: <fa686aa40710171123uefac227q4fecd64516794129@mail.gmail.com>

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


On  10/17/07,  aauer1   <aauer1@gmx.at >  wrote:
>
>
>  Grant  Likely-2  wrote:
>   >
>   >  On  10/17/07,  aauer1   <aauer1@gmx.at >  wrote:
>   > >  Now,  I  know  that  the  kernel  boots  but  I  don't  get  an  output  with  my
>   > >  Xilinx
>   > >  UartLite  module.  Are  there  some  kernel  modules  which  must  be  activated??
>   > >  Btw.  I  have  used  the  Linux-2.6-virtex  kernel  from
>   > >  http://git.secretlab.ca/git/  and  another  Kernel  (2.6.23  from  kernel.org)
>   > >  -
>   > >  both  with  the  same  result.
>   >
>   >  Post  the  output  of  __log_buf  here  please.
>   >
>   >
>
>  Here  is  my  dump  of  the  __log_buf:
>  ======================
>   <5 >[        0.000000]  Linux  version  2.6.23-rc2
>  (aauer@servitus.student.iaik.tugraz.at)  (gcc  version  4.0.0  (DENX  ELDK  4.1
>  4.0
>  .0))  #8  Wed  Oct  17  20:05:28  CEST  2007
>   <6 >[        0.000000]  Xilinx  ML403  Reference  System  (Virtex-4  FX)
>   <7 >[        0.000000]  Entering  add_active_range(0,  0,  16384)  0  entries  of  256
>  used
>   <4 >[        0.000000]  Zone  PFN  ranges:
>   <4 >[        0.000000]      DMA                          0  - >        16384
>   <4 >[        0.000000]      Normal            16384  - >        16384
>   <4 >[        0.000000]  Movable  zone  start  PFN  for  each  node
>   <4 >[        0.000000]  early_node_map[1]  active  PFN  ranges
>   <4 >[        0.000000]          0:                0  - >        16384
>   <7 >[        0.000000]  On  node  0  totalpages:  16384
>   <7 >[        0.000000]      DMA  zone:  128  pages  used  for  memmap
>   <7 >[        0.000000]      DMA  zone:  0  pages  reserved
>   <7 >[        0.000000]      DMA  zone:  16256  pages,  LIFO  batch:3
>   <7 >[        0.000000]      Normal  zone:  0  pages  used  for  memmap
>   <7 >[        0.000000]      Movable  zone:  0  pages  used  for  memmap
>   <4 >[        0.000000]  Built  1  zonelists  in  Zone  order.    Total  pages:  16256
>   <5 >[        0.000000]  Kernel  command  line:  console=ttUL0  root=/dev/xsysace2  rw

try the command line bootargs console=ttyUL0,boardrate root=/dev/xsysace2 rw 
you must make sure that you have the xsysace2 device, or you can take a test using ramdisk.i think it will run well

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

^ permalink raw reply

* Re: [PATCH] ppc44x: support for 256K PAGE_SIZE
From: Paul Mackerras @ 2007-10-18 23:21 UTC (permalink / raw)
  To: Yuri Tikhonov; +Cc: linuxppc-dev
In-Reply-To: <200710181108.19413.yur@emcraft.com>

Yuri Tikhonov writes:

> The following patch adds support for 256KB PAGE_SIZE on ppc44x-based boards. 
> The applications to be run on the kernel with 256KB PAGE_SIZE have to be 
> built using the modified version of binutils, where the MAXPAGESIZE 
> definition is set to 0x40000 (as opposite to standard 0x10000).

Have you measured the performance using a 64kB page size?  If so, how
does it compare with the 256kB page size?

The 64kB page size has the attraction that no binutils changes are
required.

Paul.

^ permalink raw reply

* Re: [PATCH] [POWERPC] powerpc: Add -mno-spe for ARCH=powerpc builds
From: Paul Mackerras @ 2007-10-18 23:19 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <Pine.LNX.4.64.0710181655270.28684@blarg.am.freescale.net>

Kumar Gala writes:

> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>

Your commit message doesn't give any reason why you are doing this, or
any explanation of what goes wrong without it.  In fact, the commit
message is completely empty. :)  Please resubmit with a decent commit
message.

Paul.

^ permalink raw reply

* Re: [PATCH] synchronize_irq needs a barrier
From: Benjamin Herrenschmidt @ 2007-10-18 23:17 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev, akpm, Herbert Xu, linux-kernel
In-Reply-To: <alpine.LFD.0.999.0710181549330.26902@woody.linux-foundation.org>


On Thu, 2007-10-18 at 15:52 -0700, Linus Torvalds wrote:
> 
> On Fri, 19 Oct 2007, Benjamin Herrenschmidt wrote:
> > 
> > The barrier would guarantee that ioc->active (and in fact the write to
> > the chip too above) are globally visible
> 
> No, it doesn't really guarantee that.
> 
> The thing is, there is no such thing as "globally visible".
> 
> There is a "ordering of visibility wrt CPU's", but it's not global, it's 
> quite potentially per-CPU. So a barrier on one CPU doesn't guarantee 
> anything at all without a barrier on the *other* CPU.
> 
> That said, the interrupt handling itself contains various barriers on the 
> CPU's that receive interrupts, thanks to the spinlocking. But I do agree 
> with Herbert that adding a "smb_mb()" is certainly in no way "obviously 
> correct", because it doesn't talk about what the other side does wrt 
> barriers and that word in memory.

I agree and you can see that in fact, we don't have enough barrier on
the other side since spin_unlock doesn't prevent subsequent loads from
crossing a previous store...

I wonder if that's worth trying to address, adding a barrier in
handle_IRQ_event for example, or we can continue ignoring the barrier
and let some drivers do their own fixes in fancy ways.

Ben.

^ permalink raw reply

* Re: [PATCH] synchronize_irq needs a barrier
From: Linus Torvalds @ 2007-10-18 22:52 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, akpm, Herbert Xu, linux-kernel
In-Reply-To: <1192745137.7367.40.camel@pasglop>



On Fri, 19 Oct 2007, Benjamin Herrenschmidt wrote:
> 
> The barrier would guarantee that ioc->active (and in fact the write to
> the chip too above) are globally visible

No, it doesn't really guarantee that.

The thing is, there is no such thing as "globally visible".

There is a "ordering of visibility wrt CPU's", but it's not global, it's 
quite potentially per-CPU. So a barrier on one CPU doesn't guarantee 
anything at all without a barrier on the *other* CPU.

That said, the interrupt handling itself contains various barriers on the 
CPU's that receive interrupts, thanks to the spinlocking. But I do agree 
with Herbert that adding a "smb_mb()" is certainly in no way "obviously 
correct", because it doesn't talk about what the other side does wrt 
barriers and that word in memory.

		Linus

^ permalink raw reply

* Re: qe: add ability to upload QE firmware
From: Timur Tabi @ 2007-10-18 22:53 UTC (permalink / raw)
  To: Jerry Van Baren; +Cc: linuxppc-dev
In-Reply-To: <4717DEE8.9010508@gmail.com>

Jerry Van Baren wrote:

> We seem to be a little conflicted over whether it is an upload or a 
> download.  ;-)

Not me.

To the host == download
 From the host == upload

^ 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