* Re: dtc: Use stdint.h types throughout dtc
From: Jon Loeliger @ 2008-07-14 18:49 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20080625035307.GB8284@yookeroo.seuss>
> Currently, dtc defines Linux-like names for various fixed-size integer
> types. There's no good reason to do this; even Linux itself doesn't
> use these names for externally visible things any more. This patch
> replaces these with the C99 standardized type names from stdint.h.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Applied.
jdl
^ permalink raw reply
* Re: dtc: Use the same endian-conversion functions as libfdt
From: Jon Loeliger @ 2008-07-14 18:50 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20080625042753.GC8284@yookeroo.seuss>
> Currently both libfdt and dtc define a set of endian conversion macros
> for accessing the device tree blob which is always big-endian. libfdt
> uses names like cpu_to_fdt32() and dtc uses names like cpu_to_be32 (as
> the Linux kernel). This patch switches dtc over to using the libfdt
> macros (including libfdt_env.h to supply them). This has a couple of
> small advantages:
> - Removes some code duplication
> - Will make conversion a bit easier if we ever need to produce
> little-endian device tree blobs.
> - dtc no longer needs to pull in netinet/in.h simply for the
> ntohs() and ntohl() functions
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Applied.
jdl
^ permalink raw reply
* Re: dtc: Use libfdt endian conversion functions in libfdt
From: Jon Loeliger @ 2008-07-14 18:50 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20080626004306.GG308@yookeroo.seuss>
> Following on from the last patch, which made dtc use the same endian
> conversion functions as libfdt, this patch makes ftdump use these
> functions as well. This brings us down to a single set of endian
> handling functions in all of dtc and libfdt, so just one place to fix
> things.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Applied.
jdl
^ permalink raw reply
* Re: [alsa-devel] [PATCH v2 1/3] ALSA SoC: Add OpenFirmware helper for matching bus and codec drivers
From: Timur Tabi @ 2008-07-14 18:53 UTC (permalink / raw)
To: Timur Tabi, Grant Likely, Jon Smirl, linuxppc-dev, alsa-devel,
liam.girdwood
In-Reply-To: <20080714184929.GJ25448@sirena.org.uk>
Mark Brown wrote:
>> The only problem with this is that the OF probing code in the kernel binds
>> drivers to device tree nodes. So when a driver claims a node, no other driver
>> will be probed with it.
>
>> So we can't have generic nodes that classify the motherboard and just let
>> everyone get probed on it.
>
> My suggestion is that you change this for the root node.
That's an interesting idea.
> It's already
> got the information required in there, it's just there's no way to use
> it to load modules at the minute.
Correct.
> You could presumably read the
> information out of the device tree using existing APIs to check you're
> running on the right board once code is loaded?
Yes. The driver <-> node binding is only for probing. Any driver can scan the
entire tree at any time.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: dtc: Address an assortment of portability problems
From: Jon Loeliger @ 2008-07-14 18:54 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev, Benno Rice
In-Reply-To: <20080626010349.GH308@yookeroo.seuss>
> I've recently worked with a FreeBSD developer, getting dtc and libfdt
> working on FreeBSD. This showed up a number of portability problems
> in the dtc package which this patch addresses. Changes are as
> follows:
>
> - the parent_offset and supernode_atdepth_offset testcases
> used the glibc extension functions strchrnul() and strndupa(). Those
> are removed, using slightly longer coding with standard C functions
> instead.
>
> - some other testcases had a #define _GNU_SOURCE for no
> particular reason. This is removed.
>
> - run_tests.sh has bash specific constructs removed, and the
> interpreter changed to /bin/sh. This apparently now runs fine on
> FreeBSD's /bin/sh, and I've also tested it with both ash and dash.
>
> - convert-dtsv0-lexer.l has some extra #includes added. These
> must have been included indirectly with Linux and glibc, but aren't on
> FreeBSD.
>
> - the endian handling functions in libfdt_env.h, based on
> endian.h and byteswap.h are replaced with some portable open-coded
> versions. Unfortunately, these result in fairly crappy code when
> compiled, but as far as I can determine there doesn't seem to be any
> POSIX, SUS or de facto standard way of determining endianness at
> compile time, nor standard names for byteswapping functions.
>
> - some more endian handling, from testdata.h using the
> problematic endian.h is simply removed, since it wasn't actually being
> used anyway.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This patch didn't apply directly.
The problem appeared to be the mysterious and suspect "exit 99"
in the middle of the following context. I whacked on the patch
directly by-hand, and managed to get it to apply.
Obligatory "Not using git" grumble.
Applied after all.
jdl
> Index: dtc/tests/run_tests.sh
> ===================================================================
> --- dtc.orig/tests/run_tests.sh 2008-06-26 10:40:59.000000000 +1000
> +++ dtc/tests/run_tests.sh 2008-06-26 10:43:21.000000000 +1000
> @@ -15,19 +15,19 @@
> tot_strange=0
>
> base_run_test() {
> - tot_tests=$[tot_tests + 1]
> + tot_tests=$((tot_tests + 1))
> if VALGRIND="$VALGRIND" "$@"; then
> - tot_pass=$[tot_pass + 1]
> + tot_pass=$((tot_pass + 1))
> else
> ret="$?"
> if [ "$ret" == "1" ]; then
> - tot_config=$[tot_config + 1]
> + tot_config=$((tot_config + 1))
> elif [ "$ret" == "2" ]; then
> - tot_fail=$[tot_fail + 1]
> + tot_fail=$((tot_fail + 1))
> elif [ "$ret" == "$VGCODE" ]; then
> - tot_vg=$[tot_vg + 1]
> + tot_vg=$((tot_vg + 1))
> else
> - tot_strange=$[tot_strange + 1]
> + tot_strange=$((tot_strange + 1))
> fi
> exit 99
> fi
^ permalink raw reply
* Re: dtc: Clean up lexing of include files
From: Jon Loeliger @ 2008-07-14 18:55 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20080626070857.GA1869@yookeroo.seuss>
> Currently we scan the /include/ directive as two tokens, the
> "/include/" keyword itself, then the string giving the file name to
> include. We use a special scanner state to keep the two linked
> together, and use the scanner state stack to keep track of the
> original state while we're parsing the two /include/ tokens.
>
> This does mean that we need to enable the 'stack' option in flex,
> which results in a not-easily-suppressed warning from the flex
> boilerplate code. This is mildly irritating.
>
> However, this two-token scanning of the /include/ directive also has
> some extremely strange edge cases, because there are a variety of
> tokens recognized in all scanner states, including INCLUDE. For
> example the following strange dts file:
>
> /include/ /dts-v1/;
> / {
> /* ... */
> };
>
> Will be processed successfully with the /include/ being effectively
> ignored: the '/dts-v1/' and ';' are recognized even in INCLUDE state,
> then the ';' transitions us to PROPNODENAME state, throwing away
> INCLUDE, and the previous state is never popped off the stack. Or
> for another example this construct:
> foo /include/ = "somefile.dts"
> will be parsed as though it were:
> foo = /include/ "somefile.dts"
> Again, the '=' is scanned without leaving INCLUDE state, then the next
> string triggers the include logic.
>
> And finally, we use a different regexp for the string with the
> included filename than the normal string regexpt, which is also
> potentially weird.
>
> This patch, therefore, cleans up the lexical handling of the /include/
> directive. Instead of the INCLUDE state, we instead scan the whole
> include directive, both keyword and filename as a single token. This
> does mean a bit more complexity in extracting the filename out of
> yytext, but I think it's worth it to avoid the strageness described
> above. It also means it's no longer possible to put a comment between
> the /include/ and the filename, but I'm really not very worried about
> breaking files using such a strange construct.
Applied.
jdl
^ permalink raw reply
* Re: dtc: Enable and fix -Wpointer-arith warnings
From: Jon Loeliger @ 2008-07-14 18:59 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20080707001048.GC6267@yookeroo.seuss>
> This patch turns on the -Wpointer-arith option in the dtc Makefile,
> and fixes the resulting warnings due to using (void *) in pointer
> arithmetic. While convenient, pointer arithmetic on void * is not
> portable, so it's better that we avoid it, particularly in libfdt.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This patch didn't apply. I tracked it down to bad context
in the Makefile where this appeared:
> CPPFLAGS += -std=c99 -D_XOPEN_SOURCE -D_BSD_SOURCE
> CFLAGS += -Werror
but the file had this instead:
> BISON = bison
> LEX = flex
in this part:
> Index: dtc/Makefile
> ===================================================================
> --- dtc.orig/Makefile 2008-07-04 11:48:05.000000000 +1000
> +++ dtc/Makefile 2008-07-04 16:54:42.000000000 +1000
> @@ -16,7 +16,7 @@
> CONFIG_LOCALVERSION =
>
> CPPFLAGS = -I libfdt
> -CFLAGS = -Wall -g -Os
> +CFLAGS = -Wall -g -Os -Wpointer-arith
>
> CPPFLAGS += -std=c99 -D_XOPEN_SOURCE -D_BSD_SOURCE
> CFLAGS += -Werror
Again, I hand whacked-the patch mail and applied it.
For the love-of-Pete, please use git.
jdl
^ permalink raw reply
* Re: dtc: Enable and fix -Wcast-qual warnings
From: Jon Loeliger @ 2008-07-14 19:01 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20080707001415.GD6267@yookeroo.seuss>
> Enabling -Wcast-qual warnings in dtc shows up a number of places where
> we are incorrectly discarding a const qualification. There are also
> some places where we are intentionally discarding the 'const', and we
> need an ugly cast through uintptr_t to suppress the warning. However,
> most of these are pretty well isolated with the *_w() functions. So
> in the interests of maximum safety with const qualifications, this
> patch enables the warnings and fixes the existing complaints.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Guess which follow-on patch didn't apply for the same reason?
Needed to look like this hand-modified hunk...
> Index: dtc/Makefile
> ===================================================================
> --- dtc.orig/Makefile 2008-07-04 16:54:38.000000000 +1000
> +++ dtc/Makefile 2008-07-04 16:54:38.000000000 +1000
> @@ -16,7 +16,7 @@
> CONFIG_LOCALVERSION =
>
> CPPFLAGS = -I libfdt
> -CFLAGS = -Wall -g -Os -Wpointer-arith
> +CFLAGS = -Wall -g -Os -Wpointer-arith -Wcast-qual
>
> BISON = bison
> LEX = flex
Applied.
jdl
^ permalink raw reply
* Re: dtc: Run relevant checks on dtb input as well as dts
From: Jon Loeliger @ 2008-07-14 19:01 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20080707011913.GE6267@yookeroo.seuss>
> This patch adjusts the testsuite to run most of the tests for the tree
> checking code on input in dtb form as well as dts form. Some checks
> which only make sense for dts input (like reference handling) are
> excluded, as are those which currently take dtb input because they
> rely on things which cannot be lexically constructed in a dts file.
>
> This shows up two small bugs in dtc, which are also corrected.
>
> First, the name_properties test which was is supposed to remove
> correctly formed 'name' properties (because they can be reconstructed
> from tne node name) was instead removing 'name' properties even if
> they weren't correct.
>
> Secondly, when using dtb or fs input, the runtime tree in dtc did not
> have the parent pointer initialized propertly because.built
> internally. The appropriate initialization is added to the
> add_child() function.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Applied.
jdl
^ permalink raw reply
* Re: libfdt: Increase namespace-pollution paranoia
From: Jon Loeliger @ 2008-07-14 19:02 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20080709041024.GB29648@yookeroo.seuss>
> libfdt is supposed to easy to embed in projects all and sundry.
> Often, it won't be practical to separate the embedded libfdt's
> namespace from that of the surrounding project. Which means there can
> be namespace conflicts between even libfdt's internal/static functions
> and functions or macros coming from the surrounding project's headers
> via libfdt_env.h.
>
> This patch, therefore, renames a bunch of libfdt internal functions
> and macros and makes a few other chances to reduce the chances of
> namespace collisions with embedding projects. Specifically:
> - Internal functions (even static ones) are now named _fdt_*()
>
> - The type and (static) global for the error table in
> fdt_strerror() gain an fdt_ prefix
>
> - The unused macro PALIGN is removed
>
> - The memeq and streq macros are removed and open-coded in the
> users (they were only used once each)
>
> - Other macros gain an FDT_ prefix
>
> - To save some of the bulk from the previous change, an
> FDT_TAGALIGN() macro is introduced, where FDT_TAGALIGN(x) ==
> FDT_ALIGN(x, FDT_TAGSIZE)
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Applied.
jdl
^ permalink raw reply
* Re: [PATCH] libfdt: Improve documentation in libfdt.h
From: Jon Loeliger @ 2008-07-14 19:02 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linuxppc-dev
In-Reply-To: <20080709092243.10615.84477.stgit@leda.ptxnet.pengutronix.de>
> Fix a few typos and mistakes.
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Applied.
jdl
^ permalink raw reply
* Re: libfdt: Increase namespace-pollution paranoia
From: Jon Loeliger @ 2008-07-14 19:08 UTC (permalink / raw)
To: Kumar Gala, jwboyer, ppc-dev list, monstr, David Gibson
>
> My (DTC) plan is to apply the single patch with some
> include file fixes, and release that.
>
> I'll line the slew of patches up for the following release.
>
> jdl
And that's not at all what happened.
One of David's patches (BSD portability) was a superset of the
include-file fixes supplied as a point-solution in a different
patch. So I applied David's more-general patch.
In fact, I applied all of David's outstanding patches and
have tagged it v1.2.0-rc2 and pushed it out now.
Please let me know if you have other fixes before I make
and release a non -rc version.
Thanks,
jdl
^ permalink raw reply
* Re: 82xx performance
From: Arnd Bergmann @ 2008-07-14 20:44 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <DCEAAC0833DD314AB0B58112AD99B93B04AE61BC@ismail.innsys.innovsys.com>
On Monday 14 July 2008, Rune Torgersen wrote:
> Context switching - times in microseconds - smaller is better
> ------------------------------------------------------------------------
> Host OS 2p/0K 2p/16K 2p/64K 8p/16K 8p/64K 16p/16K 16p/64K
> ctxsw ctxsw ctxsw ctxsw ctxsw ctxsw ctxsw
> --------- ------------- ------ ------ ------ ------ ------ ------- -------
> 9919_unit Linux 2.6.25 20.6 86.2 28.5 103.8 38.7 111.8 57.4
> 9919_unit Linux 2.6.18 5.3300 63.2 17.9 73.4 23.1 74.9 26.2
This is certainly significant, but a lot has happened between the two
versions. I few ideas:
* compare some of the key configuration options:
# CONFIG_DEBUG_*
# CONFIG_PREEMPT*
# CONFIG_NO_HZ
# CONFIG_HZ
* Try looking at where the time is spent, using oprofile or readprofile
* Try setting /proc/sys/kernel/compat/sched_yield to 1, to get the legacy
behaviour of the scheduler.
* Maybe there is a kernel version that supports your hardware in both
arch/ppc/ and arch/powerpc. In that case, you could see if the platform
change had an impact.
Arnd <><
^ permalink raw reply
* Re: [PATCH 08/16 v3] powerpc: Do not probe PCI buses or eBus devices if CMO is enabled
From: Brian King @ 2008-07-14 21:35 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, paulus, David Darrington
In-Reply-To: <20080704125418.GK1310@linux.vnet.ibm.com>
Ben,
Please drop this patch from the series. After further discussion, this patch
is not required and has actually been causing problems.
Thanks,
Brian
Robert Jennings wrote:
> From: Brian King <brking@linux.vnet.ibm.com>
>
> The Cooperative Memory Overcommit (CMO) on System p does not currently
> support native PCI devices or eBus devices when enabled. Prevent
> PCI bus probe and eBus device probe if the feature is enabled.
>
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> Signed-off-by: Robert Jennings <rcj@linux.vnet.ibm.com>
>
> ---
>
> arch/powerpc/kernel/ibmebus.c | 6 ++++++
> arch/powerpc/platforms/pseries/setup.c | 4 ++++
> 2 files changed, 10 insertions(+)
>
> Index: b/arch/powerpc/kernel/ibmebus.c
> ===================================================================
> --- a/arch/powerpc/kernel/ibmebus.c
> +++ b/arch/powerpc/kernel/ibmebus.c
> @@ -45,6 +45,7 @@
> #include <linux/of_platform.h>
> #include <asm/ibmebus.h>
> #include <asm/abs_addr.h>
> +#include <asm/firmware.h>
>
> static struct device ibmebus_bus_device = { /* fake "parent" device */
> .bus_id = "ibmebus",
> @@ -332,6 +333,11 @@ static int __init ibmebus_bus_init(void)
> {
> int err;
>
> + if (firmware_has_feature(FW_FEATURE_CMO)) {
> + printk(KERN_WARNING "Not probing eBus since CMO is enabled\n");
> + return 0;
> + }
> +
> printk(KERN_INFO "IBM eBus Device Driver\n");
>
> err = of_bus_type_init(&ibmebus_bus_type, "ibmebus");
> Index: b/arch/powerpc/platforms/pseries/setup.c
> ===================================================================
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -539,6 +539,10 @@ static void pseries_shared_idle_sleep(vo
>
> static int pSeries_pci_probe_mode(struct pci_bus *bus)
> {
> + if (firmware_has_feature(FW_FEATURE_CMO)) {
> + dev_warn(&bus->dev, "Not probing PCI bus since CMO is enabled\n");
> + return PCI_PROBE_NONE;
> + }
> if (firmware_has_feature(FW_FEATURE_LPAR))
> return PCI_PROBE_DEVTREE;
> return PCI_PROBE_NORMAL;
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
--
Brian King
Linux on Power Virtualization
IBM Linux Technology Center
^ permalink raw reply
* Re: [alsa-devel] [PATCH v2 1/3] ALSA SoC: Add OpenFirmware helper for matching bus and codec drivers
From: Grant Likely @ 2008-07-14 22:28 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel, liam.girdwood
In-Reply-To: <487B9D98.8010008@freescale.com>
On Mon, Jul 14, 2008 at 01:40:24PM -0500, Timur Tabi wrote:
> Mark Brown wrote:
>
> > Desktop Management Interface, a standard BIOS interface for getting
> > system data on x86 class hardware. Of particular interest here is the
> > fact that it contains various ID strings for things like motherboard and
> > chassis - on Linux drivers can be automatically loaded based on these
> > strings. See drivers/misc/thinkpad_acpi.c for an example of a driver
> > that does this.
>
> The only problem with this is that the OF probing code in the kernel binds
> drivers to device tree nodes. So when a driver claims a node, no other driver
> will be probed with it.
Yes, but that is only for the of_platform bus which is just one way to
setup device drivers from the device tree. Setting it up from the
platform code is just as valid an option. Or, each ASoC machine driver could
read the device tree itself in the module_init hook to decide whether or not
to instantiate itself.
Cheers,
g.
^ permalink raw reply
* Re: [alsa-devel] [PATCH v2 1/3] ALSA SoC: Add OpenFirmware helper for matching bus and codec drivers
From: Jon Smirl @ 2008-07-14 23:45 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev, alsa-devel, liam.girdwood
In-Reply-To: <487B9D98.8010008@freescale.com>
On 7/14/08, Timur Tabi <timur@freescale.com> wrote:
> Mark Brown wrote:
>
> > Desktop Management Interface, a standard BIOS interface for getting
> > system data on x86 class hardware. Of particular interest here is the
> > fact that it contains various ID strings for things like motherboard and
> > chassis - on Linux drivers can be automatically loaded based on these
> > strings. See drivers/misc/thinkpad_acpi.c for an example of a driver
> > that does this.
>
>
> The only problem with this is that the OF probing code in the kernel binds
> drivers to device tree nodes. So when a driver claims a node, no other driver
> will be probed with it.
>
> So we can't have generic nodes that classify the motherboard and just let
> everyone get probed on it.
Allowing multiple binds at the root causes the problem of something
like compatible="lite5200b,mpc5200-simple". Both platforms would bind
and that's not what you want.
I'm not a fan of each platform creating a platform device in
arch/powerpc/platforms/*. That implies that each platform would cause
another source file to be added each containing pretty much identical
code. It also makes the mpc5200-simple platform pointless. Of course
this scheme works and I'm doing it right now, but it's clearly not an
optimal solution.
Another scheme would be to add kernel code to always create virtual OF
devices like "lite5200b-fabric" that are derived off from the machine
name that achieved a bind.
A third scheme would be to convert the powerpc machine drivers
themselves to device drivers and move them out of
arch/powerpc/platforms/* into drivers/whatever. Then add the ASoC
fabric code to them. I don't know if you can load device drivers early
enough to load a powerpc machine driver from initrd.
A fourth scheme is to do it at compile time. But that means no
universal firmware images that support multiple hardware revisions. We
have this one today too.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [PATCH v3 3/5] of-bindings: Add binding documentation for SPI busses and devices
From: Segher Boessenkool @ 2008-07-15 0:12 UTC (permalink / raw)
To: Grant Likely; +Cc: spi-devel-general, dbrownell, linuxppc-dev
In-Reply-To: <20080712083447.14782.25031.stgit@trillian.secretlab.ca>
> From: Grant Likely <grant.likely@secretlab.ca>
>
> Add documentation about how to describe SPI busses in the device tree.
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Very nice! I wish all bindings were like this (namely, not overly
terse).
Acked-by: Segher Boessenkool <segher@kernel.crashing.org>
Segher
^ permalink raw reply
* Re: Mikrotik RouterBoard 333
From: Segher Boessenkool @ 2008-07-15 0:17 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20080708042631.GB7328@yookeroo.seuss>
> Its firmware apparently provides a flattened device tree to the OS.
> And while this step towards world domination is flattering, it's an
> example of what I feared when people first got enthusiastic about the
> idea of including flattened device trees in firmwares. The tree has
> not, AFAIK, been past this list, and has apparently not been reviewed
> by someone knowledgeable about device trees. In short, it's crap, and
> now that it's embedded in the firware we can't really fix it.
Can't you build a kernel with a blob that overrides the
firmware-provided
blob?
Segher
^ permalink raw reply
* Re: dtc: Address an assortment of portability problems
From: David Gibson @ 2008-07-15 0:44 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev, Benno Rice
In-Reply-To: <E1KITC5-0006rG-EI@jdl.com>
On Mon, Jul 14, 2008 at 01:54:41PM -0500, Jon Loeliger wrote:
> > I've recently worked with a FreeBSD developer, getting dtc and libfdt
> > working on FreeBSD. This showed up a number of portability problems
> > in the dtc package which this patch addresses. Changes are as
> > follows:
> >
> > - the parent_offset and supernode_atdepth_offset testcases
> > used the glibc extension functions strchrnul() and strndupa(). Those
> > are removed, using slightly longer coding with standard C functions
> > instead.
> >
> > - some other testcases had a #define _GNU_SOURCE for no
> > particular reason. This is removed.
> >
> > - run_tests.sh has bash specific constructs removed, and the
> > interpreter changed to /bin/sh. This apparently now runs fine on
> > FreeBSD's /bin/sh, and I've also tested it with both ash and dash.
> >
> > - convert-dtsv0-lexer.l has some extra #includes added. These
> > must have been included indirectly with Linux and glibc, but aren't on
> > FreeBSD.
> >
> > - the endian handling functions in libfdt_env.h, based on
> > endian.h and byteswap.h are replaced with some portable open-coded
> > versions. Unfortunately, these result in fairly crappy code when
> > compiled, but as far as I can determine there doesn't seem to be any
> > POSIX, SUS or de facto standard way of determining endianness at
> > compile time, nor standard names for byteswapping functions.
> >
> > - some more endian handling, from testdata.h using the
> > problematic endian.h is simply removed, since it wasn't actually being
> > used anyway.
> >
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>
> This patch didn't apply directly.
>
> The problem appeared to be the mysterious and suspect "exit 99"
> in the middle of the following context. I whacked on the patch
> directly by-hand, and managed to get it to apply.
Sod. When working I usually have an extra patch in my stack that adds
that exit 99 - it's easier to find and fix testsuite failures when it
bombs out after the first one.
Obviously, I forgot to double check that it didn't leak into the
context :(. Sorry.
--
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][RT][PPC64] Fix preempt unsafe paths accessing per_cpu variables
From: Benjamin Herrenschmidt @ 2008-07-15 1:32 UTC (permalink / raw)
To: Chirag Jog
Cc: linux-rt-users, Josh Triplett, Steven Rostedt, linuxppc-dev,
Nivedita Singhvi, Timothy R. Chavez, paulmck, linux.kernel
In-Reply-To: <20080709160543.GG7101@linux.vnet.ibm.com>
On Wed, 2008-07-09 at 21:35 +0530, Chirag Jog wrote:
> Hi,
> This patch fixes various paths in the -rt kernel on powerpc64 where per_cpu
> variables are accessed in a preempt unsafe way.
> When a power box with -rt kernel is booted, multiple BUG messages are
> generated "BUG: init:1 task might have lost a preemption check!".
> After booting a kernel with these patches applied, these messages
> don't appear.
>
> Also I ran the realtime tests from ltp to ensure the stability.
That sounds bad tho...
IE. You are changing the code to lock/unlock on all those TLB batching
operations, but seem to miss the core reason why it was done that way:
ie, the code assumes that it will not change CPU -between- those calls,
since the whole stuff should be already have been within a per-cpu
locked section at the caller level.
As for the TCE code, well, it lived on the assumption that the upper
level spinlock did the job of preventing preempt, I suppose that's not
the case anymore. So that part of the patch sounds ok.
Ben.
>
> Signed-Off-By: Chirag <chirag@linux.vnet.ibm.com>
> arch/powerpc/mm/tlb_64.c | 31 ++++++++++++++++---------------
> arch/powerpc/platforms/pseries/iommu.c | 14 ++++++++++----
> include/asm-powerpc/tlb.h | 5 ++---
> 3 files changed, 28 insertions(+), 22 deletions(-)
>
>
> Index: linux-2.6.25.8-rt7/arch/powerpc/mm/tlb_64.c
> ===================================================================
> --- linux-2.6.25.8-rt7.orig/arch/powerpc/mm/tlb_64.c 2008-07-09 21:29:21.000000000 +0530
> +++ linux-2.6.25.8-rt7/arch/powerpc/mm/tlb_64.c 2008-07-09 21:30:37.000000000 +0530
> @@ -38,7 +38,6 @@
> * include/asm-powerpc/tlb.h file -- tgall
> */
> DEFINE_PER_CPU_LOCKED(struct mmu_gather, mmu_gathers);
> -DEFINE_PER_CPU(struct pte_freelist_batch *, pte_freelist_cur);
> unsigned long pte_freelist_forced_free;
>
> struct pte_freelist_batch
> @@ -48,7 +47,7 @@
> pgtable_free_t tables[0];
> };
>
> -DEFINE_PER_CPU(struct pte_freelist_batch *, pte_freelist_cur);
> +DEFINE_PER_CPU_LOCKED(struct pte_freelist_batch *, pte_freelist_cur);
> unsigned long pte_freelist_forced_free;
>
> #define PTE_FREELIST_SIZE \
> @@ -92,24 +91,21 @@
>
> void pgtable_free_tlb(struct mmu_gather *tlb, pgtable_free_t pgf)
> {
> - /*
> - * This is safe since tlb_gather_mmu has disabled preemption.
> - * tlb->cpu is set by tlb_gather_mmu as well.
> - */
> + int cpu;
> cpumask_t local_cpumask = cpumask_of_cpu(tlb->cpu);
> - struct pte_freelist_batch **batchp = &__get_cpu_var(pte_freelist_cur);
> + struct pte_freelist_batch **batchp = &get_cpu_var_locked(pte_freelist_cur, &cpu);
>
> if (atomic_read(&tlb->mm->mm_users) < 2 ||
> cpus_equal(tlb->mm->cpu_vm_mask, local_cpumask)) {
> pgtable_free(pgf);
> - return;
> + goto cleanup;
> }
>
> if (*batchp == NULL) {
> *batchp = (struct pte_freelist_batch *)__get_free_page(GFP_ATOMIC);
> if (*batchp == NULL) {
> pgtable_free_now(pgf);
> - return;
> + goto cleanup;
> }
> (*batchp)->index = 0;
> }
> @@ -118,6 +114,9 @@
> pte_free_submit(*batchp);
> *batchp = NULL;
> }
> +
> + cleanup:
> + put_cpu_var_locked(pte_freelist_cur, cpu);
> }
>
> /*
> @@ -253,13 +252,15 @@
>
> void pte_free_finish(void)
> {
> - /* This is safe since tlb_gather_mmu has disabled preemption */
> - struct pte_freelist_batch **batchp = &__get_cpu_var(pte_freelist_cur);
> + int cpu;
> + struct pte_freelist_batch **batchp = &get_cpu_var_locked(pte_freelist_cur, &cpu);
>
> - if (*batchp == NULL)
> - return;
> - pte_free_submit(*batchp);
> - *batchp = NULL;
> + if (*batchp) {
> + pte_free_submit(*batchp);
> + *batchp = NULL;
> + }
> +
> + put_cpu_var_locked(pte_freelist_cur, cpu);
> }
>
> /**
> Index: linux-2.6.25.8-rt7/include/asm-powerpc/tlb.h
> ===================================================================
> --- linux-2.6.25.8-rt7.orig/include/asm-powerpc/tlb.h 2008-07-09 21:29:21.000000000 +0530
> +++ linux-2.6.25.8-rt7/include/asm-powerpc/tlb.h 2008-07-09 21:29:41.000000000 +0530
> @@ -40,18 +40,17 @@
>
> static inline void tlb_flush(struct mmu_gather *tlb)
> {
> - struct ppc64_tlb_batch *tlbbatch = &__get_cpu_var(ppc64_tlb_batch);
> + struct ppc64_tlb_batch *tlbbatch = &get_cpu_var(ppc64_tlb_batch);
>
> /* If there's a TLB batch pending, then we must flush it because the
> * pages are going to be freed and we really don't want to have a CPU
> * access a freed page because it has a stale TLB
> */
> if (tlbbatch->index) {
> - preempt_disable();
> __flush_tlb_pending(tlbbatch);
> - preempt_enable();
> }
>
> + put_cpu_var(ppc64_tlb_batch);
> pte_free_finish();
> }
>
> Index: linux-2.6.25.8-rt7/arch/powerpc/platforms/pseries/iommu.c
> ===================================================================
> --- linux-2.6.25.8-rt7.orig/arch/powerpc/platforms/pseries/iommu.c 2008-07-09 21:29:21.000000000 +0530
> +++ linux-2.6.25.8-rt7/arch/powerpc/platforms/pseries/iommu.c 2008-07-09 21:29:41.000000000 +0530
> @@ -124,7 +124,7 @@
> }
> }
>
> -static DEFINE_PER_CPU(u64 *, tce_page) = NULL;
> +static DEFINE_PER_CPU_LOCKED(u64 *, tce_page) = NULL;
>
> static void tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum,
> long npages, unsigned long uaddr,
> @@ -135,12 +135,13 @@
> u64 *tcep;
> u64 rpn;
> long l, limit;
> + int cpu;
>
> if (npages == 1)
> return tce_build_pSeriesLP(tbl, tcenum, npages, uaddr,
> direction);
>
> - tcep = __get_cpu_var(tce_page);
> + tcep = get_cpu_var_locked(tce_page, &cpu);
>
> /* This is safe to do since interrupts are off when we're called
> * from iommu_alloc{,_sg}()
> @@ -148,10 +149,13 @@
> if (!tcep) {
> tcep = (u64 *)__get_free_page(GFP_ATOMIC);
> /* If allocation fails, fall back to the loop implementation */
> - if (!tcep)
> + if (!tcep) {
> + put_cpu_var_locked(tce_page, cpu);
> return tce_build_pSeriesLP(tbl, tcenum, npages,
> uaddr, direction);
> - __get_cpu_var(tce_page) = tcep;
> + }
> +
> + per_cpu_var_locked(tce_page, cpu) = tcep;
> }
>
> rpn = (virt_to_abs(uaddr)) >> TCE_SHIFT;
> @@ -188,6 +192,8 @@
> printk("\ttce[0] val = 0x%lx\n", tcep[0]);
> show_stack(current, (unsigned long *)__get_SP());
> }
> +
> + put_cpu_var_locked(tce_page, cpu);
> }
>
> static void tce_free_pSeriesLP(struct iommu_table *tbl, long tcenum, long npages)
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply
* Re: linux-next: manual merge of the powerpc tree
From: Steven Rostedt @ 2008-07-15 1:24 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Mark Nelson, linuxppc-dev, linux-next, Paul Mackerras,
Ingo Molnar, Thomas Gleixner
In-Reply-To: <20080714154459.0ed9da62.sfr@canb.auug.org.au>
On Mon, 14 Jul 2008, Stephen Rothwell wrote:
> Hi Paul, Ben,
>
> Today's linux-next merge of the powerpc tree got a conflict in
> arch/powerpc/Kconfig between commit
> 4e491d14f2506b218d678935c25a7027b79178b1 ("ftrace: support for PowerPC")
> from the ftrace tree and commit 3affedc4e1ce837033b6c5e9289d2ce2f5a62d31
> ("powerpc/dma: implement new dma_*map*_attrs() interfaces") from the
> powerpc tree.
>
> The former commit moved the "select HAVE_OPROFILE" to the bottom of the
> "config PPC" list (for no reason that I can fathom) while the latter
> added another select. Simple fixup. I can carry it.
I heard someone mention that they try to keep the configs in alphabetical
order. I did the move of OPROFILE for just that reason. I should have said
so in the change log. Oh well.
-- Steve
^ permalink raw reply
* Re: Mikrotik RouterBoard 333
From: David Gibson @ 2008-07-15 1:41 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <4ca5bd814ebc5d76508d40503f4884db@kernel.crashing.org>
On Tue, Jul 15, 2008 at 02:17:36AM +0200, Segher Boessenkool wrote:
>> Its firmware apparently provides a flattened device tree to the OS.
>> And while this step towards world domination is flattering, it's an
>> example of what I feared when people first got enthusiastic about the
>> idea of including flattened device trees in firmwares. The tree has
>> not, AFAIK, been past this list, and has apparently not been reviewed
>> by someone knowledgeable about device trees. In short, it's crap, and
>> now that it's embedded in the firware we can't really fix it.
>
> Can't you build a kernel with a blob that overrides the
> firmware-provided blob?
Sorry, my phrasing was slightly unclear. Certainly we can work around
a firmware with a crap device tree by replacing it, if necessary.
Basically that's just treating the firmware as though it's one of
these old-style jobs which provides its tiny handful of necessary bits
of information (memory size, maybe a few others) in a format that
happens to resemble a device tree.
But it seems kind of silly for firmware to go to the trouble of
providing a device tree just for us to ignore it and substitute our
own.
--
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] kill useless SMT code in prom_hold_cpus
From: Benjamin Herrenschmidt @ 2008-07-15 2:05 UTC (permalink / raw)
To: Nathan Lynch; +Cc: linuxppc-dev
In-Reply-To: <20080708223631.GP9594@localdomain>
On Tue, 2008-07-08 at 17:36 -0500, Nathan Lynch wrote:
> I think this code that counts SMT threads and compares against NR_CPUS
> is an artifact of pre-powerpc-merge ppc64. We care about starting
> only primary threads in the OF client code.
>
> Signed-off-by: Nathan Lynch <ntl@pobox.com>
That looks good. I'm not merging it right now because I want to dbl
check that it's allright on all SMT machines. IE. We compare reg[0]
against _prom->cpu now instead of interrupt_server[0] and I thus
want to ensure it's the same everywhere.
Cheers,
Ben.
> arch/powerpc/kernel/prom_init.c | 39 +++------------------------------------
> 1 files changed, 3 insertions(+), 36 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 1ea8c8d..b1dd86c 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -205,8 +205,6 @@ static int __initdata mem_reserve_cnt;
> static cell_t __initdata regbuf[1024];
>
>
> -#define MAX_CPU_THREADS 2
> -
> /*
> * Error results ... some OF calls will return "-1" on error, some
> * will return 0, some will return either. To simplify, here are
> @@ -1332,10 +1330,6 @@ static void __init prom_hold_cpus(void)
> unsigned int reg;
> phandle node;
> char type[64];
> - int cpuid = 0;
> - unsigned int interrupt_server[MAX_CPU_THREADS];
> - unsigned int cpu_threads, hw_cpu_num;
> - int propsize;
> struct prom_t *_prom = &RELOC(prom);
> unsigned long *spinloop
> = (void *) LOW_ADDR(__secondary_hold_spinloop);
> @@ -1379,7 +1373,6 @@ static void __init prom_hold_cpus(void)
> reg = -1;
> prom_getprop(node, "reg", ®, sizeof(reg));
>
> - prom_debug("\ncpuid = 0x%x\n", cpuid);
> prom_debug("cpu hw idx = 0x%x\n", reg);
>
> /* Init the acknowledge var which will be reset by
> @@ -1388,28 +1381,9 @@ static void __init prom_hold_cpus(void)
> */
> *acknowledge = (unsigned long)-1;
>
> - propsize = prom_getprop(node, "ibm,ppc-interrupt-server#s",
> - &interrupt_server,
> - sizeof(interrupt_server));
> - if (propsize < 0) {
> - /* no property. old hardware has no SMT */
> - cpu_threads = 1;
> - interrupt_server[0] = reg; /* fake it with phys id */
> - } else {
> - /* We have a threaded processor */
> - cpu_threads = propsize / sizeof(u32);
> - if (cpu_threads > MAX_CPU_THREADS) {
> - prom_printf("SMT: too many threads!\n"
> - "SMT: found %x, max is %x\n",
> - cpu_threads, MAX_CPU_THREADS);
> - cpu_threads = 1; /* ToDo: panic? */
> - }
> - }
> -
> - hw_cpu_num = interrupt_server[0];
> - if (hw_cpu_num != _prom->cpu) {
> + if (reg != _prom->cpu) {
> /* Primary Thread of non-boot cpu */
> - prom_printf("%x : starting cpu hw idx %x... ", cpuid, reg);
> + prom_printf("starting cpu hw idx %x... ", reg);
> call_prom("start-cpu", 3, 0, node,
> secondary_hold, reg);
>
> @@ -1424,17 +1398,10 @@ static void __init prom_hold_cpus(void)
> }
> #ifdef CONFIG_SMP
> else
> - prom_printf("%x : boot cpu %x\n", cpuid, reg);
> + prom_printf("boot cpu hw idx %x\n", reg);
> #endif /* CONFIG_SMP */
> -
> - /* Reserve cpu #s for secondary threads. They start later. */
> - cpuid += cpu_threads;
> }
>
> - if (cpuid > NR_CPUS)
> - prom_printf("WARNING: maximum CPUs (" __stringify(NR_CPUS)
> - ") exceeded: ignoring extras\n");
> -
> prom_debug("prom_hold_cpus: end...\n");
> }
>
^ permalink raw reply
* Re: [PATCH] kill useless SMT code in prom_hold_cpus
From: Tony Breeds @ 2008-07-15 2:22 UTC (permalink / raw)
To: Nathan Lynch; +Cc: linuxppc-dev
In-Reply-To: <20080708223631.GP9594@localdomain>
On Tue, Jul 08, 2008 at 05:36:31PM -0500, Nathan Lynch wrote:
> I think this code that counts SMT threads and compares against NR_CPUS
> is an artifact of pre-powerpc-merge ppc64. We care about starting
> only primary threads in the OF client code.
<snip>
> - prom_printf("%x : starting cpu hw idx %x... ", cpuid, reg);
> + prom_printf("starting cpu hw idx %x... ", reg);
If we remove this, where else can we see the mapping of hardware IDs
to logical cpu IDs? This is useful on POWER4 (at least where they can be
different).
<snip>
> - if (cpuid > NR_CPUS)
> - prom_printf("WARNING: maximum CPUs (" __stringify(NR_CPUS)
> - ") exceeded: ignoring extras\n");
> -
I think this printf() is valuable, if your boot a 128 thread machine on
a kernel with NR_CPUS=64, this is the only messaage you get to indicate
that you're wasting 64 threads, and how to resolve it.
Yours Tony
linux.conf.au http://www.marchsouth.org/
Jan 19 - 24 2009 The Australian Linux Technical Conference!
^ permalink raw reply
* Re: [PATCH] kill useless SMT code in prom_hold_cpus
From: Nathan Lynch @ 2008-07-15 2:24 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1216087555.7740.42.camel@pasglop>
Benjamin Herrenschmidt wrote:
> On Tue, 2008-07-08 at 17:36 -0500, Nathan Lynch wrote:
> > I think this code that counts SMT threads and compares against NR_CPUS
> > is an artifact of pre-powerpc-merge ppc64. We care about starting
> > only primary threads in the OF client code.
> >
> > Signed-off-by: Nathan Lynch <ntl@pobox.com>
>
> That looks good. I'm not merging it right now because I want to dbl
> check that it's allright on all SMT machines. IE. We compare reg[0]
> against _prom->cpu now instead of interrupt_server[0] and I thus
> want to ensure it's the same everywhere.
Thanks. Looks like prom_find_boot_cpu is setting _prom->cpu to reg
(or 0), so I think it should be fine... a system where reg differed
from interrupt_server[0] would have been broken before this patch
anyway, I think?
^ 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