* Re: wmb vs mmiowb
From: Nick Piggin @ 2007-08-23 3:54 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-ia64, Jesse Barnes, linuxppc-dev
In-Reply-To: <alpine.LFD.0.999.0708221953360.30176@woody.linux-foundation.org>
On Wed, Aug 22, 2007 at 07:57:56PM -0700, Linus Torvalds wrote:
>
>
> On Thu, 23 Aug 2007, Nick Piggin wrote:
> >
> > > Irix actually had an io_unlock() routine that did this
> > > implicitly, but iirc that was shot down for Linux...
> >
> > Why was it shot down? Seems like a pretty good idea to me ;)
>
> It's horrible. We'd need it for *every* single spinlock type. We have lots
> of them.
>
> So the choice is between:
>
> - sane:
>
> mmiowb()
>
> followed by any of the existing "spin_unlock()" variants (plain,
> _irq(), _bh(), _irqrestore())
>
> - insane: multiply our current set of unlock primitives by two, by making
> "io" versions for them all:
>
> spin_unlock_io[_irq|_irqrestore|_bh]()
>
> but there's actually an EVEN WORSE problem with the stupid Irix approach,
> namely that it requires that the unlocker be aware of the exact details of
> what happens inside the lock. If the locking is done at an outer layer,
> that's not at all obvious!
OK, but we'd have some kind of functions that are called not to
serialise the CPUs, but to serialise the IO. It would be up to
the calling code to already provide CPU synchronisation.
serialize_io(); / unserialize_io(); / a nicer name
If we could pass in some kind of relevant resoure (eg. the IO
memory or device or something), then we might even be able to
put debug checks there to ensure two CPUs are never inside the
same critical IO section at once.
> In other words, Irix (once again) made a horrible and idiotic choice.
We could make a better one. I don't think mmiowb is really insane, but
I'd worry it being confused with a regular type of barrier and that CPU
synchronisation needs to be provided for it to work or make sense.
> Big surprise. Irix was probably the flakiest and worst of all the
> commercial proprietary unixes. No taste.
Is it? I've never used it ;)
^ permalink raw reply
* Re: How to port PReP to arch/powerpc?
From: David Gibson @ 2007-08-23 3:37 UTC (permalink / raw)
To: Josh Boyer; +Cc: Meelis Roos, linuxppc-dev
In-Reply-To: <20070822072956.56b7df46@vader.jdub.homelinux.org>
On Wed, Aug 22, 2007 at 07:29:56AM -0500, Josh Boyer wrote:
> On Wed, 22 Aug 2007 14:50:07 +0300 (EEST)
> Meelis Roos <mroos@linux.ee> wrote:
>
> > It seems no one cares for arch/ppc any more but the move to powerpc was
> > not finished completely (PReP subarch was not migrated but sees users, I
> > do not know about the user base of the other subarches). So I have been
> > thinking about migrating PReP support to arch/powerpc myself since I use
> > it.
>
> The merge is still on-going. So it's not "finished" at all.
>
> > Any general guidelines or specific hints about that? I have looked
> > around in the code and some pieces are clearly PReP specific (setup,
> > pci, boot) but others seem wuite a bit mixed up with other subarches.
>
> David Gibson and Rob Landley had a quite interesting discussion about
> PReP last night on IRC. You might want to contact them and see if they
> came to any conclusions.
Yes, my draft PReP support patch, tweaked this morning to build again
(changes roughly identical to Rob Landley's patch, I believe) is at:
http://ozlabs.org/~dgibson/home/tmp/prep-support
--
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: bogus vscsi dt node on iseries
From: Stephen Rothwell @ 2007-08-23 3:36 UTC (permalink / raw)
To: Olaf Hering; +Cc: linuxppc-dev
In-Reply-To: <20070822175121.GA21233@aepfle.de>
[-- Attachment #1: Type: text/plain, Size: 403 bytes --]
On Wed, 22 Aug 2007 19:51:21 +0200 Olaf Hering <olaf@aepfle.de> wrote:
>
>
> Does anyone know why dt_vdevices() creates a vscsi node on legacy iseries?
> I'm sure only viodasd and viocd is required as blockdevice type.
Because the IBM vscsi client (in theory) works on legacy iSeries.
--
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
* Re: asm-ppc header issues when building ARCH=powerpc
From: Kumar Gala @ 2007-08-23 3:33 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev@ozlabs.org list, Paul Mackerras, David Gibson
In-Reply-To: <75EDD900-3B5E-4D08-9311-13B6510EDE6E@kernel.crashing.org>
On Aug 22, 2007, at 10:19 AM, Kumar Gala wrote:
> Guys,
>
> I was wondering if I could get your help with looking at the
> following lists and determining if we have an issue or not related
> the following files:
>
> Getting some classification on these would be good. Possibly
> classifications, doesn't build in ARCH=powerpc, remove include, real
> issue, etc.
>
> - k
My analysis of <asm/bootinfo.h> usage:
./drivers/macintosh/adb-iop.c:#include <asm/bootinfo.h> remove
./drivers/char/vme_scc.c:#include <asm/bootinfo.h> 68k only
./drivers/char/serial167.c:#include <asm/bootinfo.h> 68k only
./drivers/serial/dz.c:#include <asm/bootinfo.h> decstation
./drivers/mtd/devices/ms02-nv.c:#include <asm/bootinfo.h> decstation
./drivers/net/macsonic.c:#include <asm/bootinfo.h> 68k
./drivers/net/jazzsonic.c:#include <asm/bootinfo.h> mips
./drivers/video/pmag-aa-fb.c:#include <asm/bootinfo.h> mips
./drivers/video/maxinefb.c:#include <asm/bootinfo.h> mips
./drivers/video/logo/logo.c:#include <asm/bootinfo.h> mips
./drivers/video/macfb.c:#include <asm/bootinfo.h> 68k
./drivers/video/valkyriefb.c:#include <asm/bootinfo.h> 68k
- k
^ permalink raw reply
* Re: asm-ppc header issues when building ARCH=powerpc
From: David Gibson @ 2007-08-23 3:33 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev@ozlabs.org list, Paul Mackerras
In-Reply-To: <2F7E4C06-9AC3-458A-B1E3-CA6E0BDD189A@kernel.crashing.org>
On Wed, Aug 22, 2007 at 10:33:55PM -0500, Kumar Gala wrote:
>
> On Aug 22, 2007, at 10:19 AM, Kumar Gala wrote:
>
> >Guys,
> >
> >I was wondering if I could get your help with looking at the
> >following lists and determining if we have an issue or not related
> >the following files:
> >
> >Getting some classification on these would be good. Possibly
> >classifications, doesn't build in ARCH=powerpc, remove include, real
> >issue, etc.
> >
> >- k
>
> My analysis of <asm/bootinfo.h> usage:
>
> ./drivers/macintosh/adb-iop.c:#include <asm/bootinfo.h> remove
> ./drivers/char/vme_scc.c:#include <asm/bootinfo.h> 68k only
> ./drivers/char/serial167.c:#include <asm/bootinfo.h> 68k only
> ./drivers/serial/dz.c:#include <asm/bootinfo.h> decstation
> ./drivers/mtd/devices/ms02-nv.c:#include <asm/bootinfo.h> decstation
> ./drivers/net/macsonic.c:#include <asm/bootinfo.h> 68k
> ./drivers/net/jazzsonic.c:#include <asm/bootinfo.h> mips
> ./drivers/video/pmag-aa-fb.c:#include <asm/bootinfo.h> mips
> ./drivers/video/maxinefb.c:#include <asm/bootinfo.h> mips
> ./drivers/video/logo/logo.c:#include <asm/bootinfo.h> mips
> ./drivers/video/macfb.c:#include <asm/bootinfo.h> 68k
> ./drivers/video/valkyriefb.c:#include <asm/bootinfo.h> 68k
Uh.. I'm pretty sure valkyriefb.c is for (old) PowerMacs, not 68k.
--
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 1/2] [POWERPC] Remove cmd_line from head*.S
From: Kumar Gala @ 2007-08-23 3:27 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: ppc-dev, paulus
In-Reply-To: <20070823130913.2e9db6eb.sfr@canb.auug.org.au>
On Aug 22, 2007, at 10:09 PM, Stephen Rothwell wrote:
> It is just a C char array, so declare it thusly.
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> arch/powerpc/kernel/head_32.S | 8 --------
> arch/powerpc/kernel/head_44x.S | 8 --------
> arch/powerpc/kernel/head_4xx.S | 7 -------
> arch/powerpc/kernel/head_64.S | 8 --------
> arch/powerpc/kernel/head_8xx.S | 8 --------
> arch/powerpc/kernel/head_fsl_booke.S | 8 --------
> arch/powerpc/kernel/prom.c | 2 ++
> 7 files changed, 2 insertions(+), 47 deletions(-)
>
> --
> Cheers,
> Stephen Rothwell sfr@canb.auug.org.au
[snip]
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 0028fe6..dc85005 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -60,6 +60,8 @@
> #endif
>
>
> +char cmd_line[COMMAND_LINE_SIZE];
> +
would this be better in setup-common.c?
> static int __initdata dt_root_addr_cells;
> static int __initdata dt_root_size_cells;
>
- k
^ permalink raw reply
* Re: [Kgdb-bugreport] 2.6.23-rc3-mm1: kgdb build failure on powerpc
From: Jason Wessel @ 2007-08-23 3:25 UTC (permalink / raw)
To: Andrew Morton
Cc: kgdb-bugreport, amitkale, linux-kernel, linuxppc-dev,
Mariusz Kozlowski, Paul Mackerras
In-Reply-To: <20070822165318.b82da13c.akpm@linux-foundation.org>
Andrew Morton wrote:
> On Wed, 22 Aug 2007 17:44:12 -0500
> Jason Wessel <jason.wessel@windriver.com> wrote:
>
>
>> + while (!atomic_read(&debugger_active));
>>
>
> eek. We're in the process of hunting down and eliminating exactly this
> construct. There have been cases where the compiler cached the
> atomic_read() result in a register, turning the above into an infinite
> loop.
>
> Plus we should never add power-burners like that into the kernel anyway.
> That loop should have a cpu_relax() in it. Which will also fix the
> compiler problem described above.
>
>
Agreed, and fixed with a cpu_relax.
> Thirdly, please always add a newline when coding statements like that:
>
> while (expr())
> ;
>
The other instances I found of the same problem in the kgdb core are
fixed too.
I merged all the changes into the for_mm branch in the kgdb git tree.
Thanks,
Jason.
^ permalink raw reply
* Re: [PATCH 2/2] [POWERPC] Size swapper_pg_dir correctly
From: Kumar Gala @ 2007-08-23 3:26 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: ppc-dev, paulus
In-Reply-To: <20070823131032.04d929e7.sfr@canb.auug.org.au>
On Aug 22, 2007, at 10:10 PM, Stephen Rothwell wrote:
> David Gibson pointed out that swapper_pg_dir actually need to be
> PGD_TABLE_SIZE bytes long not PAGE_SIZE. This actually saves 64k in
> the bss for a ppc64_defconfig kernel built with CONFIG_PPC_64K_PAGES.
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> arch/powerpc/kernel/asm-offsets.c | 4 ++++
> arch/powerpc/kernel/head_64.S | 2 +-
> include/asm-powerpc/pgtable-4k.h | 2 ++
> include/asm-powerpc/pgtable-64k.h | 2 ++
> 4 files changed, 9 insertions(+), 1 deletions(-)
>
> --
> Cheers,
> Stephen Rothwell sfr@canb.auug.org.au
>
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/
> kernel/asm-offsets.c
> index a408053..0ae5d57 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -320,5 +320,9 @@ int main(void)
> DEFINE(VMALLOC_START_ESID, GET_ESID(VMALLOC_START));
> DEFINE(VMALLOC_START_VSID, KERNEL_VSID(VMALLOC_START));
> #endif
> +
> +#ifdef CONFIG_PPC64
> + DEFINE(PGD_TABLE_SIZE, PGD_TABLE_SIZE);
> +#endif
Why limit this to ppc64? The cleanup should be reasonable for all ppc.
- k
^ permalink raw reply
* Re: [PATCH v6 1/2] [POWERPC] fsl_soc: add support for fsl_spi
From: Stephen Rothwell @ 2007-08-23 3:24 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: linuxppc-dev
In-Reply-To: <20070822145732.GA22686@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 466 bytes --]
Hi Anton,
On Wed, 22 Aug 2007 18:57:32 +0400 Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
>
> + sysclk = *(u32 *)of_get_property(np, "bus-frequency", NULL);
I just cringe everytime I see someone dereference a pointer they got from
somewhere (effectively) external without checking for NULL. I realise
that sometimes "that can't happen" ...
--
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
* Re: asm-ppc header issues when building ARCH=powerpc
From: Kumar Gala @ 2007-08-23 3:22 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev@ozlabs.org list, Paul Mackerras
In-Reply-To: <20070823024757.GC7042@localhost.localdomain>
On Aug 22, 2007, at 9:47 PM, David Gibson wrote:
> On Wed, Aug 22, 2007 at 10:19:21AM -0500, Kumar Gala wrote:
>> Guys,
>>
>> I was wondering if I could get your help with looking at the
>> following lists and determining if we have an issue or not related
>> the following files:
>>
>> Getting some classification on these would be good. Possibly
>> classifications, doesn't build in ARCH=powerpc, remove include, real
>> issue, etc.
>>
>> - k
>>
>> ./include/asm-powerpc/irq.h:#include <asm/mpc83xx.h> - protected by !
>> CONFIG_PPC_MERGE
>
> irq.h seems to have a big slab of CONFIG_PPC_MERGE dependent stuff.
> Looks like a good candidate for splitting into asm-ppc/irq.h and
> asm-powerpc/irq.h
Yeah was going to look at a bit of that since the list of
CONFIG_PPC_MERGE headers is pretty short:
dcr.h, i8259.h, ipic.h, irq.h
>> ./drivers/ide/ppc/mpc8xx.c:#include <asm/residual.h>
>
> Since mpc8xx is certainly not PReP, I don't think there's any way it
> ought to be including residual.h
Yeah, not sure what's up with that. It needs a !MERGE in Kconfig for
now.
>> ./drivers/mtd/maps/tqm834x.c:#include <asm/ppcboot.h>
>> ./drivers/mtd/maps/pq2fads.c:#include <asm/ppcboot.h>
>
> Although these both have an extern of type bd_t (defined in
> ppcboot.h), afaict they don't actually use it, so these should be
> removable. Longer term, all these ugly hardcoded map files should be
> relegated to arch/ppc only, replaced with physmap_of and suitable
> information in the device tree for arch/powerpc. However, being able
> to build them on arch/powerpc may be useful during transition.
Yeah was going to kill tqm834x.c since there is no way to Kconfig it
into existence, and !MERGE pq2fads.c.
>> ./drivers/i2c/busses/i2c-ibm_iic.c:#include <asm/ibm4xx.h>
>
> This driver will need significant reworking to port to arch/powerpc
> (similar to the treatment ibm_emac has received). Such rework will
> remove the dependency on ibm4xx.h and ocp.h
>
>> ./drivers/mtd/maps/walnut.c:#include <asm/ibm4xx.h>
>
> Suspect it's not needed. But in any case, this too should be replaced
> by physmap_of for arch/powerpc.
>
>> ./include/asm-powerpc/irq.h:#include <asm/ibm4xx.h> - protected by !
>> CONFIG_PPC_MERGE
>>
>> ./drivers/mtd/maps/ebony.c:#include <asm/ibm44x.h>
>
> Ebony flash now works with physmap_of, so this is only used on
> arch/ppc.
>
>> ./drivers/mtd/maps/ocotea.c:#include <asm/ibm44x.h>
>
> As for drivers/mtd/maps/walnut.c.
>
>> ./drivers/mtd/nand/ndfc.c:#include <asm/ibm44x.h>
>
> This probably also wants rework to make a device-tree-aware
> arch/powerpc version.
>
>> ./include/asm-powerpc/irq.h:#include <asm/ibm44x.h> - protected by !
>> CONFIG_PPC_MERGE
>>
>> ./drivers/i2c/busses/i2c-ibm_iic.c:#include <asm/ocp.h>
>
> See above.
>
>> ./drivers/net/ucc_geth_mii.c:#include <asm/ocp.h> - just bogus, needs
>> removal
>> ./drivers/net/ibm_emac/ibm_emac_core.c:#include <asm/ocp.h>
>> ./drivers/net/ibm_emac/ibm_emac_core.h:#include <asm/ocp.h>
>> ./drivers/net/ibm_emac/ibm_emac_tah.h:#include <asm/ocp.h>
>> ./drivers/net/ibm_emac/ibm_emac_phy.c:#include <asm/ocp.h>
>> ./drivers/net/ibm_emac/ibm_emac_mal.c:#include <asm/ocp.h>
>> ./drivers/net/ibm_emac/ibm_emac_zmii.h:#include <asm/ocp.h>
>
> ibm_emac is arch/ppc only. BenH and my 'ibm_newemac' patch adds a
> new, device-tree aware arch/powerpc port of this driver.
>
>>
>> ./drivers/macintosh/adb-iop.c:#include <asm/bootinfo.h>
>
> I think this is just unnecessary, maybe also for a bunch of the ones
> below.
most of the below are 68k or mips uses
>> ./drivers/char/vme_scc.c:#include <asm/bootinfo.h>
>> ./drivers/char/serial167.c:#include <asm/bootinfo.h>
>> ./drivers/serial/dz.c:#include <asm/bootinfo.h>
>> ./drivers/mtd/devices/ms02-nv.c:#include <asm/bootinfo.h>
>> ./drivers/net/macsonic.c:#include <asm/bootinfo.h>
>> ./drivers/net/jazzsonic.c:#include <asm/bootinfo.h>
>> ./drivers/video/pmag-aa-fb.c:#include <asm/bootinfo.h>
>> ./drivers/video/maxinefb.c:#include <asm/bootinfo.h>
>> ./drivers/video/logo/logo.c:#include <asm/bootinfo.h>
>> ./drivers/video/valkyriefb.c:#include <asm/bootinfo.h>
>> ./drivers/video/macfb.c:#include <asm/bootinfo.h>
- k
^ permalink raw reply
* Re: [PATCH 2/2] [POWERPC] Size swapper_pg_dir correctly
From: David Gibson @ 2007-08-23 3:13 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: ppc-dev, paulus
In-Reply-To: <20070823131032.04d929e7.sfr@canb.auug.org.au>
On Thu, Aug 23, 2007 at 01:10:32PM +1000, Stephen Rothwell wrote:
> David Gibson pointed out that swapper_pg_dir actually need to be
> PGD_TABLE_SIZE bytes long not PAGE_SIZE. This actually saves 64k in
> the bss for a ppc64_defconfig kernel built with CONFIG_PPC_64K_PAGES.
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
--
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 1/2] [POWERPC] Remove cmd_line from head*.S
From: David Gibson @ 2007-08-23 3:13 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: ppc-dev, paulus
In-Reply-To: <20070823130913.2e9db6eb.sfr@canb.auug.org.au>
On Thu, Aug 23, 2007 at 01:09:13PM +1000, Stephen Rothwell wrote:
> It is just a C char array, so declare it thusly.
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
--
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
* [PATCH 2/2] [POWERPC] Size swapper_pg_dir correctly
From: Stephen Rothwell @ 2007-08-23 3:10 UTC (permalink / raw)
To: paulus; +Cc: ppc-dev
In-Reply-To: <20070823130913.2e9db6eb.sfr@canb.auug.org.au>
David Gibson pointed out that swapper_pg_dir actually need to be
PGD_TABLE_SIZE bytes long not PAGE_SIZE. This actually saves 64k in
the bss for a ppc64_defconfig kernel built with CONFIG_PPC_64K_PAGES.
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
arch/powerpc/kernel/asm-offsets.c | 4 ++++
arch/powerpc/kernel/head_64.S | 2 +-
include/asm-powerpc/pgtable-4k.h | 2 ++
include/asm-powerpc/pgtable-64k.h | 2 ++
4 files changed, 9 insertions(+), 1 deletions(-)
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index a408053..0ae5d57 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -320,5 +320,9 @@ int main(void)
DEFINE(VMALLOC_START_ESID, GET_ESID(VMALLOC_START));
DEFINE(VMALLOC_START_VSID, KERNEL_VSID(VMALLOC_START));
#endif
+
+#ifdef CONFIG_PPC64
+ DEFINE(PGD_TABLE_SIZE, PGD_TABLE_SIZE);
+#endif
return 0;
}
diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index 381d07a..a8e2348 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -1537,4 +1537,4 @@ empty_zero_page:
.globl swapper_pg_dir
swapper_pg_dir:
- .space PAGE_SIZE
+ .space PGD_TABLE_SIZE
diff --git a/include/asm-powerpc/pgtable-4k.h b/include/asm-powerpc/pgtable-4k.h
index add5481..818e2ab 100644
--- a/include/asm-powerpc/pgtable-4k.h
+++ b/include/asm-powerpc/pgtable-4k.h
@@ -10,10 +10,12 @@
#define PUD_INDEX_SIZE 7
#define PGD_INDEX_SIZE 9
+#ifndef __ASSEMBLY__
#define PTE_TABLE_SIZE (sizeof(pte_t) << PTE_INDEX_SIZE)
#define PMD_TABLE_SIZE (sizeof(pmd_t) << PMD_INDEX_SIZE)
#define PUD_TABLE_SIZE (sizeof(pud_t) << PUD_INDEX_SIZE)
#define PGD_TABLE_SIZE (sizeof(pgd_t) << PGD_INDEX_SIZE)
+#endif /* __ASSEMBLY__ */
#define PTRS_PER_PTE (1 << PTE_INDEX_SIZE)
#define PTRS_PER_PMD (1 << PMD_INDEX_SIZE)
diff --git a/include/asm-powerpc/pgtable-64k.h b/include/asm-powerpc/pgtable-64k.h
index 33ae901..bd54b77 100644
--- a/include/asm-powerpc/pgtable-64k.h
+++ b/include/asm-powerpc/pgtable-64k.h
@@ -9,9 +9,11 @@
#define PUD_INDEX_SIZE 0
#define PGD_INDEX_SIZE 4
+#ifndef __ASSEMBLY__
#define PTE_TABLE_SIZE (sizeof(real_pte_t) << PTE_INDEX_SIZE)
#define PMD_TABLE_SIZE (sizeof(pmd_t) << PMD_INDEX_SIZE)
#define PGD_TABLE_SIZE (sizeof(pgd_t) << PGD_INDEX_SIZE)
+#endif /* __ASSEMBLY__ */
#define PTRS_PER_PTE (1 << PTE_INDEX_SIZE)
#define PTRS_PER_PMD (1 << PMD_INDEX_SIZE)
--
1.5.2.4
^ permalink raw reply related
* [PATCH 1/2] [POWERPC] Remove cmd_line from head*.S
From: Stephen Rothwell @ 2007-08-23 3:09 UTC (permalink / raw)
To: paulus; +Cc: ppc-dev
It is just a C char array, so declare it thusly.
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
arch/powerpc/kernel/head_32.S | 8 --------
arch/powerpc/kernel/head_44x.S | 8 --------
arch/powerpc/kernel/head_4xx.S | 7 -------
arch/powerpc/kernel/head_64.S | 8 --------
arch/powerpc/kernel/head_8xx.S | 8 --------
arch/powerpc/kernel/head_fsl_booke.S | 8 --------
arch/powerpc/kernel/prom.c | 2 ++
7 files changed, 2 insertions(+), 47 deletions(-)
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index 7d73a13..de2fa61 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -1300,14 +1300,6 @@ empty_zero_page:
swapper_pg_dir:
.space 4096
-/*
- * This space gets a copy of optional info passed to us by the bootstrap
- * Used to pass parameters into the kernel like root=/dev/sda1, etc.
- */
- .globl cmd_line
-cmd_line:
- .space 512
-
.globl intercept_table
intercept_table:
.long 0, 0, i0x200, i0x300, i0x400, 0, i0x600, i0x700
diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
index 8869596..e4068ff 100644
--- a/arch/powerpc/kernel/head_44x.S
+++ b/arch/powerpc/kernel/head_44x.S
@@ -744,14 +744,6 @@ exception_stack_bottom:
exception_stack_top:
/*
- * This space gets a copy of optional info passed to us by the bootstrap
- * which is used to pass parameters into the kernel like root=/dev/sda1, etc.
- */
- .globl cmd_line
-cmd_line:
- .space 512
-
-/*
* Room for two PTE pointers, usually the kernel and current user pointers
* to their respective root page table.
*/
diff --git a/arch/powerpc/kernel/head_4xx.S b/arch/powerpc/kernel/head_4xx.S
index adc7f80..958805b 100644
--- a/arch/powerpc/kernel/head_4xx.S
+++ b/arch/powerpc/kernel/head_4xx.S
@@ -1007,13 +1007,6 @@ critical_stack_top:
.globl exception_stack_top
exception_stack_top:
-/* This space gets a copy of optional info passed to us by the bootstrap
- * which is used to pass parameters into the kernel like root=/dev/sda1, etc.
- */
- .globl cmd_line
-cmd_line:
- .space 512
-
/* Room for two PTE pointers, usually the kernel and current user pointers
* to their respective root page table.
*/
diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index 33c4e8c..381d07a 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -1538,11 +1538,3 @@ empty_zero_page:
.globl swapper_pg_dir
swapper_pg_dir:
.space PAGE_SIZE
-
-/*
- * This space gets a copy of optional info passed to us by the bootstrap
- * Used to pass parameters into the kernel like root=/dev/sda1, etc.
- */
- .globl cmd_line
-cmd_line:
- .space COMMAND_LINE_SIZE
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 901be47..22e1a3c 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -835,14 +835,6 @@ empty_zero_page:
swapper_pg_dir:
.space 4096
-/*
- * This space gets a copy of optional info passed to us by the bootstrap
- * Used to pass parameters into the kernel like root=/dev/sda1, etc.
- */
- .globl cmd_line
-cmd_line:
- .space 512
-
/* Room for two PTE table poiners, usually the kernel and current user
* pointer to their respective root page table (pgdir).
*/
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 1f155d3..5f47adb 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -1050,14 +1050,6 @@ exception_stack_bottom:
exception_stack_top:
/*
- * This space gets a copy of optional info passed to us by the bootstrap
- * which is used to pass parameters into the kernel like root=/dev/sda1, etc.
- */
- .globl cmd_line
-cmd_line:
- .space 512
-
-/*
* Room for two PTE pointers, usually the kernel and current user pointers
* to their respective root page table.
*/
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 0028fe6..dc85005 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -60,6 +60,8 @@
#endif
+char cmd_line[COMMAND_LINE_SIZE];
+
static int __initdata dt_root_addr_cells;
static int __initdata dt_root_size_cells;
--
1.5.2.4
^ permalink raw reply related
* Re: asm-ppc header issues when building ARCH=powerpc
From: Kumar Gala @ 2007-08-23 3:09 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev@ozlabs.org list, Paul Mackerras
In-Reply-To: <20070823024937.GD7042@localhost.localdomain>
On Aug 22, 2007, at 9:49 PM, David Gibson wrote:
> On Wed, Aug 22, 2007 at 02:30:47PM -0500, Josh Boyer wrote:
>> On Wed, 22 Aug 2007 10:19:21 -0500
>> Kumar Gala <galak@kernel.crashing.org> wrote:
>>
>>> Guys,
>>>
>>> I was wondering if I could get your help with looking at the
>>> following lists and determining if we have an issue or not related
>>> the following files:
>>>
>>> Getting some classification on these would be good. Possibly
>>> classifications, doesn't build in ARCH=powerpc, remove include, real
>>> issue, etc.
>>
>> Sure.
>>
>>> ./drivers/i2c/busses/i2c-ibm_iic.c:#include <asm/ocp.h>
>>> ./drivers/i2c/busses/i2c-ibm_iic.c:#include <asm/ibm4xx.h>
>>
>> These one depends on IBM_OCP in Kconfig. We don't select/enable
>> that on
>> any existing arch/powerpc 4xx stuff so it won't be built anyway.
>
> Nor will we ever enable IBM_OCP in arch/powerpc: the device tree
> entirely obsoletes the OCP crap.
Agreed. We shouldn't taint arch/powerpc with OCP :)
- k
^ permalink raw reply
* Re: wmb vs mmiowb
From: Linus Torvalds @ 2007-08-23 2:57 UTC (permalink / raw)
To: Nick Piggin; +Cc: linux-ia64, Jesse Barnes, linuxppc-dev
In-Reply-To: <20070823022043.GB18788@wotan.suse.de>
On Thu, 23 Aug 2007, Nick Piggin wrote:
>
> > Irix actually had an io_unlock() routine that did this
> > implicitly, but iirc that was shot down for Linux...
>
> Why was it shot down? Seems like a pretty good idea to me ;)
It's horrible. We'd need it for *every* single spinlock type. We have lots
of them.
So the choice is between:
- sane:
mmiowb()
followed by any of the existing "spin_unlock()" variants (plain,
_irq(), _bh(), _irqrestore())
- insane: multiply our current set of unlock primitives by two, by making
"io" versions for them all:
spin_unlock_io[_irq|_irqrestore|_bh]()
but there's actually an EVEN WORSE problem with the stupid Irix approach,
namely that it requires that the unlocker be aware of the exact details of
what happens inside the lock. If the locking is done at an outer layer,
that's not at all obvious!
In other words, Irix (once again) made a horrible and idiotic choice.
Big surprise. Irix was probably the flakiest and worst of all the
commercial proprietary unixes. No taste.
Linus
^ permalink raw reply
* Re: [PATCH] ucc_geth: kill unused include
From: David Gibson @ 2007-08-23 2:54 UTC (permalink / raw)
To: Kumar Gala; +Cc: netdev, Li Yang, Paul Mackerras, Jeff Garzik, linuxppc-dev
In-Reply-To: <Pine.LNX.4.64.0708222144440.6464@blarg.am.freescale.net>
On Wed, Aug 22, 2007 at 09:51:03PM -0500, Kumar Gala wrote:
> The ucc_geth_mii code is based on the gianfar_mii code that use to include
> ocp.h. ucc never need this and it causes issues when we want to kill
> arch/ppc includes from arch/powerpc.
>
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
--
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: asm-ppc header issues when building ARCH=powerpc
From: David Gibson @ 2007-08-23 2:49 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev@ozlabs.org list, Paul Mackerras
In-Reply-To: <20070822143047.6c8a0d4e@weaponx.rchland.ibm.com>
On Wed, Aug 22, 2007 at 02:30:47PM -0500, Josh Boyer wrote:
> On Wed, 22 Aug 2007 10:19:21 -0500
> Kumar Gala <galak@kernel.crashing.org> wrote:
>
> > Guys,
> >
> > I was wondering if I could get your help with looking at the
> > following lists and determining if we have an issue or not related
> > the following files:
> >
> > Getting some classification on these would be good. Possibly
> > classifications, doesn't build in ARCH=powerpc, remove include, real
> > issue, etc.
>
> Sure.
>
> > ./drivers/i2c/busses/i2c-ibm_iic.c:#include <asm/ocp.h>
> > ./drivers/i2c/busses/i2c-ibm_iic.c:#include <asm/ibm4xx.h>
>
> These one depends on IBM_OCP in Kconfig. We don't select/enable that on
> any existing arch/powerpc 4xx stuff so it won't be built anyway.
Nor will we ever enable IBM_OCP in arch/powerpc: the device tree
entirely obsoletes the OCP crap.
--
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
* [PATCH] ucc_geth: kill unused include
From: Kumar Gala @ 2007-08-23 2:51 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, Li Yang, Paul Mackerras, linuxppc-dev
The ucc_geth_mii code is based on the gianfar_mii code that use to include
ocp.h. ucc never need this and it causes issues when we want to kill
arch/ppc includes from arch/powerpc.
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
Jeff, if you issue with this for 2.6.23, I'd prefer to push this via
the powerpc.git trees in 2.6.24 as part of a larger cleanup. Let me know
one way or the other.
- k
drivers/net/ucc_geth_mii.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ucc_geth_mii.c b/drivers/net/ucc_geth_mii.c
index 6c257b8..df884f0 100644
--- a/drivers/net/ucc_geth_mii.c
+++ b/drivers/net/ucc_geth_mii.c
@@ -32,7 +32,6 @@
#include <linux/mm.h>
#include <linux/module.h>
#include <linux/platform_device.h>
-#include <asm/ocp.h>
#include <linux/crc32.h>
#include <linux/mii.h>
#include <linux/phy.h>
--
1.5.2.4
^ permalink raw reply related
* Re: asm-ppc header issues when building ARCH=powerpc
From: David Gibson @ 2007-08-23 2:47 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev@ozlabs.org list, Paul Mackerras
In-Reply-To: <75EDD900-3B5E-4D08-9311-13B6510EDE6E@kernel.crashing.org>
On Wed, Aug 22, 2007 at 10:19:21AM -0500, Kumar Gala wrote:
> Guys,
>
> I was wondering if I could get your help with looking at the
> following lists and determining if we have an issue or not related
> the following files:
>
> Getting some classification on these would be good. Possibly
> classifications, doesn't build in ARCH=powerpc, remove include, real
> issue, etc.
>
> - k
>
> ./include/asm-powerpc/irq.h:#include <asm/mpc83xx.h> - protected by !
> CONFIG_PPC_MERGE
irq.h seems to have a big slab of CONFIG_PPC_MERGE dependent stuff.
Looks like a good candidate for splitting into asm-ppc/irq.h and
asm-powerpc/irq.h
> ./drivers/ide/ppc/mpc8xx.c:#include <asm/residual.h>
Since mpc8xx is certainly not PReP, I don't think there's any way it
ought to be including residual.h
> ./drivers/mtd/maps/tqm834x.c:#include <asm/ppcboot.h>
> ./drivers/mtd/maps/pq2fads.c:#include <asm/ppcboot.h>
Although these both have an extern of type bd_t (defined in
ppcboot.h), afaict they don't actually use it, so these should be
removable. Longer term, all these ugly hardcoded map files should be
relegated to arch/ppc only, replaced with physmap_of and suitable
information in the device tree for arch/powerpc. However, being able
to build them on arch/powerpc may be useful during transition.
> ./drivers/i2c/busses/i2c-ibm_iic.c:#include <asm/ibm4xx.h>
This driver will need significant reworking to port to arch/powerpc
(similar to the treatment ibm_emac has received). Such rework will
remove the dependency on ibm4xx.h and ocp.h
> ./drivers/mtd/maps/walnut.c:#include <asm/ibm4xx.h>
Suspect it's not needed. But in any case, this too should be replaced
by physmap_of for arch/powerpc.
> ./include/asm-powerpc/irq.h:#include <asm/ibm4xx.h> - protected by !
> CONFIG_PPC_MERGE
>
> ./drivers/mtd/maps/ebony.c:#include <asm/ibm44x.h>
Ebony flash now works with physmap_of, so this is only used on
arch/ppc.
> ./drivers/mtd/maps/ocotea.c:#include <asm/ibm44x.h>
As for drivers/mtd/maps/walnut.c.
> ./drivers/mtd/nand/ndfc.c:#include <asm/ibm44x.h>
This probably also wants rework to make a device-tree-aware
arch/powerpc version.
> ./include/asm-powerpc/irq.h:#include <asm/ibm44x.h> - protected by !
> CONFIG_PPC_MERGE
>
> ./drivers/i2c/busses/i2c-ibm_iic.c:#include <asm/ocp.h>
See above.
> ./drivers/net/ucc_geth_mii.c:#include <asm/ocp.h> - just bogus, needs
> removal
> ./drivers/net/ibm_emac/ibm_emac_core.c:#include <asm/ocp.h>
> ./drivers/net/ibm_emac/ibm_emac_core.h:#include <asm/ocp.h>
> ./drivers/net/ibm_emac/ibm_emac_tah.h:#include <asm/ocp.h>
> ./drivers/net/ibm_emac/ibm_emac_phy.c:#include <asm/ocp.h>
> ./drivers/net/ibm_emac/ibm_emac_mal.c:#include <asm/ocp.h>
> ./drivers/net/ibm_emac/ibm_emac_zmii.h:#include <asm/ocp.h>
ibm_emac is arch/ppc only. BenH and my 'ibm_newemac' patch adds a
new, device-tree aware arch/powerpc port of this driver.
>
> ./drivers/macintosh/adb-iop.c:#include <asm/bootinfo.h>
I think this is just unnecessary, maybe also for a bunch of the ones
below.
> ./drivers/char/vme_scc.c:#include <asm/bootinfo.h>
> ./drivers/char/serial167.c:#include <asm/bootinfo.h>
> ./drivers/serial/dz.c:#include <asm/bootinfo.h>
> ./drivers/mtd/devices/ms02-nv.c:#include <asm/bootinfo.h>
> ./drivers/net/macsonic.c:#include <asm/bootinfo.h>
> ./drivers/net/jazzsonic.c:#include <asm/bootinfo.h>
> ./drivers/video/pmag-aa-fb.c:#include <asm/bootinfo.h>
> ./drivers/video/maxinefb.c:#include <asm/bootinfo.h>
> ./drivers/video/logo/logo.c:#include <asm/bootinfo.h>
> ./drivers/video/valkyriefb.c:#include <asm/bootinfo.h>
> ./drivers/video/macfb.c:#include <asm/bootinfo.h>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
--
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: Section mismatch warning
From: Kumar Gala @ 2007-08-23 2:36 UTC (permalink / raw)
To: sivaji; +Cc: linuxppc-dev
In-Reply-To: <12286518.post@talk.nabble.com>
On Aug 22, 2007, at 9:30 PM, sivaji wrote:
>
> Hi,
> When compiling the 2.6.23-rc3 kernel, i got following warning
> messages.
>
> WARNING: vmlinux.o(.text+0x18): Section mismatch: reference to
> .init.text:early_init (between '__start' and '__after_mmu_off')
> WARNING: vmlinux.o(.text+0x3834): Section mismatch: reference to
> .init.text:machine_init (between 'start_here' and 'set_context')
> WARNING: vmlinux.o(.text+0x383c): Section mismatch: reference to
> .init.text:MMU_init (between 'start_here' and 'set_context')
> WARNING: vmlinux.o(.text+0x3866): Section mismatch: reference to
> .init.text:start_kernel (between 'start_here' and 'set_context')
> WARNING: vmlinux.o(.text+0x386a): Section mismatch: reference to
> .init.text:start_kernel (between 'start_here' and 'set_context')
>
> Processor : 8641D
> Give some idea how to overcome this warning.
There is a patch posted on the list for this. I doubt will fix these
for 2.6.23 as they are just warnings.
http://patchwork.ozlabs.org/linuxppc/patch?id=13066
- k
^ permalink raw reply
* Section mismatch warning
From: sivaji @ 2007-08-23 2:30 UTC (permalink / raw)
To: linuxppc-dev
Hi,
When compiling the 2.6.23-rc3 kernel, i got following warning
messages.
WARNING: vmlinux.o(.text+0x18): Section mismatch: reference to
.init.text:early_init (between '__start' and '__after_mmu_off')
WARNING: vmlinux.o(.text+0x3834): Section mismatch: reference to
.init.text:machine_init (between 'start_here' and 'set_context')
WARNING: vmlinux.o(.text+0x383c): Section mismatch: reference to
.init.text:MMU_init (between 'start_here' and 'set_context')
WARNING: vmlinux.o(.text+0x3866): Section mismatch: reference to
.init.text:start_kernel (between 'start_here' and 'set_context')
WARNING: vmlinux.o(.text+0x386a): Section mismatch: reference to
.init.text:start_kernel (between 'start_here' and 'set_context')
Processor : 8641D
Give some idea how to overcome this warning.
--
View this message in context: http://www.nabble.com/Section-mismatch-warning-tf4315101.html#a12286518
Sent from the linuxppc-dev mailing list archive at Nabble.com.
^ permalink raw reply
* Re: wmb vs mmiowb
From: Nick Piggin @ 2007-08-23 2:20 UTC (permalink / raw)
To: Jesse Barnes; +Cc: linux-ia64, Linus Torvalds, linuxppc-dev
In-Reply-To: <200708221202.12403.jesse.barnes@intel.com>
On Wed, Aug 22, 2007 at 12:02:11PM -0700, Jesse Barnes wrote:
> On Wednesday, August 22, 2007 11:07 am Linus Torvalds wrote:
> > > It really seems like it is some completely different concept from a
> > > barrier. And it shows, on the platform where it really matters
> > > (sn2), where the thing actually spins.
> >
> > I agree that it probably isn't a "write barrier" per se. Think of it
> > as a "tie two subsystems together" thing.
>
> Right, maybe it's not the best name, but as long as you separate your
> memory access types, you can think of it as a real write barrier, just
> for mmio accesses (well uncached access really).
If we have the following situation (all vars start at 0)
CPU0 CPU1 CPU2
spin_lock(&lock); ~
A = 1; ~
wmb(); ~
B = 2; ~
spin_unlock(&lock); X = B;
spin_lock(&lock); rmb();
A = 10; Y = A;
wmb(); ~
B = 11; ~
spin_unlock(&lock); ~
(I use the '~' just to show CPU2 is not specifically temporally
related to CPU0 or CPU1).
Then CPU2 could have X==11 and Y==1, according to the Linux abstract
memory consistency model, couldn't it? I think so, and I think this
is what your mmiowb is trying to protect. In the above situation,
CPU2 would just use the spinlock -- I don't think we have a simple
primitive that CPU0 and 1 can call to prevent this reordering at
CPU2. An IO device obviously can't use a spinlock :).
> > (And it doesn't just matter on sn2. It also matters on powerpc64,
> > although I think they just set a flag and do the *real* sync in the
> > spin_unlock() path).
>
> Yeah, they keep threatening to use this instead, but I'm not sure how
> easy it would be. Also they may have more devices/drivers to worry
> about than sn2, so maybe changing over would mean too much driver
> debugging (well auditing really since it's not that hard to know where
> to put them). Irix actually had an io_unlock() routine that did this
> implicitly, but iirc that was shot down for Linux...
Why was it shot down? Seems like a pretty good idea to me ;)
I'm clueless when it comes to drivers, but I see a lot of mmiowb()
that are not paired with spin_unlock. How are these obvious? (ie.
what is the pattern?) It looks like some might be lockless FIFOs (or
maybe I'm just not aware of where the locks are). Can you just quickly
illustrate the problem being solved?
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 05/20] bootwrapper: flatdevtree fixes
From: David Gibson @ 2007-08-23 2:01 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, paulus
In-Reply-To: <20070822172456.GA22932@ld0162-tx32.am.freescale.net>
On Wed, Aug 22, 2007 at 12:24:56PM -0500, Scott Wood wrote:
> On Wed, Aug 22, 2007 at 11:09:07AM +1000, David Gibson wrote:
> > On Tue, Aug 21, 2007 at 11:09:58AM -0500, Scott Wood wrote:
> > > The point of #2 was as part of the fix to #1 -- otherwise, the same
> > > check for NULL would have to be moved into ft_create_node to
> > > conditionally call ft_find_device or ft_find_device_rel.
> >
> > Um... oh, ok, I hadn't spotted that (1) made ft_create() use
> > find_device_rel(). That sounds doubly wrong: you have the internal
> > offset pointer, you should be able to create a phandle using the
> > phandle allocation stuff, rather than having to refind the node you've
> > just created from the parent.
>
> Yeah, that'd make more sense.
>
> > > > (3) I really dislike; I just don't see the point.
> > >
> > > It's needed by dt_get_path().
> >
> > No, it isn't. dt_get_path() needs *some* way of getting the name of a
> > node, but it could be a separate function, which I think would be
> > preferable rather than folding it into getprop - you don't need to
> > search for the name, so a getname() function would have quite a
> > different structure to getprop().
>
> I'd rather not add a new entry in ops just for that; it's more of an
> attribute of the dtb format that name is handled specially. IIUC, on
> real OF you'd use the same code for both.
Actually, no - sorry, that's the other problem with this, which I
forgot to mention. On real OF, the "name" property contains the
node's name *without the unit address*; that is, only the portion
before the '@'. So your getprop change does not match real OF
behaviour - and real OF behaviour will not do what you want for
dt_get_path().
Actually, in any case, I don't think we want to implement get_path()
this way for real OF. Better to have get_path() itself as a callback:
on real OF I believe we can directly ask for the full path to a given
phandle, the get name based implementation can then be made specific
to the flat device tree.
Or actually, I think we might be able to come up with a get_path()
implementation for flat tree that's less hideous than repeatedly
calling get_parent() which is an ugly, ugly operation on the flat tree
(and will get worse with libfdt).
> Plus, something might come along that needs to dynamically look for
> either name or something else. It's more flexible this way.
Hrm... "something might come along" just seems contrived to me.
--
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: wmb vs mmiowb
From: Nick Piggin @ 2007-08-23 1:59 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-ia64, Jesse Barnes, linuxppc-dev
In-Reply-To: <alpine.LFD.0.999.0708221049560.30176@woody.linux-foundation.org>
On Wed, Aug 22, 2007 at 11:07:32AM -0700, Linus Torvalds wrote:
>
>
> On Wed, 22 Aug 2007, Nick Piggin wrote:
> >
> > It took me more than a glance to see what the difference is supposed to be
> > between wmb() and mmiowb(). I think especially because mmiowb isn't really
> > like a write barrier.
>
> Well, it is, but it isn't. Not on its own - but together with a "normal"
> barrier it is.
But it is stronger (or different) to write barrier semantics, because it
enforces the order in which a 3rd party (the IO device) sees writes from
multiple CPUs. The rest of our barrier concept is based purely on the
POV of the single entity executing the barrier.
Now it's needed because the IO device is not participating in the same
synchronisation logic that the CPUs are, which is why I say it is more
like a synchronisation primitive than a barrier primitive.
> > wmb is supposed to order all writes coming out of a single CPU, so that's
> > pretty simple.
>
> No. wmb orders all *normal* writes coming out of a single CPU.
I'm pretty sure wmb() should order *all* writes, and smp_wmb() is what
you're thinking of for ordering regular writes to cacheable memory.
> It may not do anything at all for "uncached" IO writes that aren't part of
> the cache coherency, and that are handled using totally different queues
> (both inside and outside of the CPU)!
>
> Now, on x86, the CPU actually tends to order IO writes *more* than it
> orders any other writes (they are mostly entirely synchronous, unless the
> area has been marked as write merging), but at least on PPC, it's the
> other way around: without the cache as a serialization entry, you end up
> having a totally separate queueu to serialize, and a regular-memory write
> barrier does nothing at all to the IO queue.
Well PPC AFAIKS doesn't need the special synchronisation semantics of
this mmiowb primitive -- the reason it is not a noop is because the API
seems to also imply a wmb() (which is fine, and you'd normally want that
eg. uncacheable stores must be ordered with the spin_unlock store).
It is just implemented with the PPC sync instruction, which just orders
all stores coming out of _this_ CPU. Their IO fabric must prevent IOs
from being reordered between CPUs if they're executed in a known order
(which is what Altix does not prevent).
> So think of the IO write queue as something totally asynchronous that has
> zero connection to the normal write ordering - and then think of mmiowb()
> as a way to *insert* a synchronization point.
If wmb (the non _smp one) orders all stores including IO stores, then it
should be sufficient to prevent IO writes from leaking out of a critical
section. The problem is that the "reader" (the IO device) itself is not
coherent. So _synchronisation_ point is right; it is not really a barrier.
Basically it says all IO writes issued by this CPU at this point will be
seen before any other IO writes issued by any other CPUs subsequently.
make_mmio_coherent()? queue_mmio_writes()? (I'd still prefer some kind of
acquire/release API that shows why CPU/CPU order matters too, and how it
is taken care of).
> > It really seems like it is some completely different concept from a
> > barrier. And it shows, on the platform where it really matters (sn2), where
> > the thing actually spins.
>
> I agree that it probably isn't a "write barrier" per se. Think of it as a
> "tie two subsystems together" thing.
Yes, in a way it is more like that. Which does fit with my suggestions
for a name.
> (And it doesn't just matter on sn2. It also matters on powerpc64, although
> I think they just set a flag and do the *real* sync in the spin_unlock()
> path).
>
> Side note: the thing that makes "mmiowb()" even more exciting is that it's
> not just the CPU, it's the fabric outside the CPU that matters too. That's
> why the sn2 needs this - but the powerpc example shows a case where the
> ordering requirement actually comes from the CPU itself.
Well I think sn2 is the *only* reason it matters. When the ordering
requirement is coming from the CPU itself, that *is* just a traditional
write barrier (one which orders normal and io writes).
The funny things powerpc are doing in spin_unlock/etc. are a different
issue. Basically they are just helping along device drivers who get this
wrong and assume spinlocks order IOs; our lack of an acquire/release API
for IOs... they're just trying to get through this sorry state of affairs
without going insane ;) Powerpc is special here because their ordering
instructions distinguish between normal and IO, wheras most others don't
(including ia64, alpha, etc), so _most_ others do get their IOs ordered by
critical sections. This is a different issue to the mmiowb one (but still
shows that our APIs could be improved).
Why don't we get a nice easy spin_lock_io/spin_unlock_io, which takes
care of all the mmiowb and iowrite vs spin unlock problems? (individual
IOs within the lock would still need to be ordered as approprite).
Then we could also have a serialize_io()/unserialize_io() that takes
care of the same things but can be used when we have something other
than a spinlock for ordering CPUs (serialize_io may be a noop, but it
is good to ensure people are thinking about how they're excluding
other CPUs here -- if other CPUs are not excluded, then any code calling
mmiowb is buggy, right?).
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox