* Re: [PATCH] generic check_legacy_ioport
From: Benjamin Herrenschmidt @ 2007-04-25 0:12 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev, Olaf Hering, Christian Krafft
In-Reply-To: <200704250001.18162.arnd@arndb.de>
On Wed, 2007-04-25 at 00:01 +0200, Arnd Bergmann wrote:
> On Tuesday 24 April 2007, Olaf Hering wrote:
> >
> > > Do you think a device_type fdc, i8042 or ipmi will appear outside an isa
> > > node?
> >
> > Does anyone know where those ipmi devices appear in the device-tree?
>
> There are actually _no_ ipmi devices that we expect to appear here.
> the reason why the check is in the ipmi driver is in order not to crash
> on powerpc machines that load the ipmi driver but have no ipmi nodes
> in the device tree.
To be totally correct, there might well be, but we shouldn't care.
That is, if a device XXX (let's say XXX is ipmi but it could be i8042 or
whatever else) exist in the device-tree outside of the ISA bus, then it
needs it's own platform or of_platform device to be probed and shouldn't
make the check_legacy_io_port() go true.
There are various cases (especially in embedded world) where "standard"
components have been puts in ASICs but at funky/different addresses, and
they shouldn't be hit by the legacy probe.
Thus, the default implementation of check_legacy_ioport() should only
match things that are under an "isa" bus. Anything else needs to be
handled either by a platform specific check_legacy_ioport() override via
ppc_md (if it's really some legacy stuff and under a fucked up
device-tree) or via driver specific alternate probing method (like an
of_platform_device).
Ben.
^ permalink raw reply
* Re: [PATCH] generic check_legacy_ioport
From: Benjamin Herrenschmidt @ 2007-04-25 0:09 UTC (permalink / raw)
To: Olaf Hering; +Cc: linuxppc-dev
In-Reply-To: <20070424185402.GA16077@aepfle.de>
On Tue, 2007-04-24 at 20:54 +0200, Olaf Hering wrote:
> On Tue, Apr 24, Olaf Hering wrote:
>
> > > And also check now if the parent is "isa". Just to be sure. If a
> > > platform won't match that, then it will need it's own
> > > check_legacy_ioport which is fine with me.
> >
> > Do you think a device_type fdc, i8042 or ipmi will appear outside an isa
> > node?
>
> Does anyone know where those ipmi devices appear in the device-tree?
Somebody with an up-to-date qs21 device-tree at hand ?
Ben.
^ permalink raw reply
* Re: [PATCH 1/6] Start split out of common open firmware code
From: Benjamin Herrenschmidt @ 2007-04-25 0:07 UTC (permalink / raw)
To: David Miller; +Cc: sfr, paulus, linuxppc-dev
In-Reply-To: <20070424.110456.124867547.davem@davemloft.net>
> I totally disagree that you should be changing anything
> during these consolidations.
>
> Move the code around in one pass where you can _VERIFY_
> that things are precisely the same and won't break.
>
> Then later you can get fancy and change things however you
> wish.
Oh I think we all agree there... we just use the opportunity to discuss
how we should improve the consolidation further :-)
> You people giving comments like this have your priorities totally
> screwed up and you are doing nothing but unnecessarily slowing down
> this wonderful consolidation. Even worse you might even be giving
> Stephen a disincentive to keep working actively on this.
>
> So please STOP THIS CRAP now, thanks.
>
> Everything so far is pure whining, and has nothing to do with
> the substance of what Stephen is trying to accomplish, a
> _CODE CONSOLIDATION_. So please don't get in the way of that
> effort.
Dave, no need to shout :-) I think it's pretty well agreed that
Stephen's patches are good to go (unless nits I haven't seen), we are
really just discussing what we should do on top of them.
Now regarding the parsing of #address-cells / #size-cells, it's indeed a
bug that we walk up the tree on powerpc, we did that initially, I think,
due to a misunderstanding of the spec, but since the code's been doing
that forever, nobody ever wanted to change it and take the risk to break
it.
At one point, I suppose we'll have to bit the bullet and do a patch
going to the proper behaviour and see what breaks (probably nothing
nowadays, except maybe some rare old apple mac-io stuffs that I need to
double check) but that should definitely be done separately from the
consolidation.
Ben.
^ permalink raw reply
* Re: [PATCH 1/6] Start split out of common open firmware code
From: Benjamin Herrenschmidt @ 2007-04-25 0:03 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Stephen Rothwell, paulus, David S. Miller, ppc-dev
In-Reply-To: <f64705c9250a93dcb7a4015b9a0d5667@kernel.crashing.org>
> While I realise your patch is just moving stuff around,
> can this be fixed please? A missing "#address-cells"
> property means "two cells", not "inherit from parent".
> The few platforms with a broken tree should just be
> fixed (patch the tree in the bootwrapper, or perhaps
> _do_ use this workaround, but only on the affected
> platforms).
If you can get me access to all of the old PowerMac or CHRP crap with
broken device-tree's ...
Note that I don't think there's much cases of such breakage on the
field, but I'd be annoyed to change a behaviour we had from day 1.
> > +int of_n_size_cells(struct device_node *np)
>
> Same here (the default is one cell though).
And same comment.
Ben.
^ permalink raw reply
* Re: [PATCH 6/6] Consolidate of_find_node_by routines
From: Benjamin Herrenschmidt @ 2007-04-25 0:01 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: ppc-dev, paulus, David S. Miller
In-Reply-To: <20070424224349.e760efe2.sfr@canb.auug.org.au>
On Tue, 2007-04-24 at 22:43 +1000, Stephen Rothwell wrote:
> This consolidates the routines of_find_node_by_path, of_find_node_by_name,
> of_find_node_by_type and of_find_compatible_device. Again, the comparison
> of strings are done differently by Sparc and PowerPC and also these add
> readlocks around the iterations.
Same question about how should we consolidate the string comparisons ?
In addition, we should really have a more intelligent path walker than
supports going down one level by name, unitaddress or both.
Ben.
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> arch/powerpc/kernel/prom.c | 113 +-------------------------------------------
> arch/sparc/kernel/prom.c | 61 +-----------------------
> arch/sparc64/kernel/prom.c | 61 +-----------------------
> drivers/openfw/base.c | 114 ++++++++++++++++++++++++++++++++++++++++++++
> include/asm-powerpc/prom.h | 1 +
> include/asm-sparc/prom.h | 1 +
> include/asm-sparc64/prom.h | 1 +
> 7 files changed, 120 insertions(+), 232 deletions(-)
>
> --
> Cheers,
> Stephen Rothwell sfr@canb.auug.org.au
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 90901e4..fc64bcb 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -77,7 +77,7 @@ static struct boot_param_header *initial_boot_params __initdata;
> struct boot_param_header *initial_boot_params;
> #endif
>
> -static struct device_node *allnodes = NULL;
> +extern struct device_node *allnodes; /* temporary while merging */
>
> extern rwlock_t devtree_lock; /* temporary while merging */
>
> @@ -1068,117 +1068,6 @@ EXPORT_SYMBOL(machine_is_compatible);
> *
> *******/
>
> -/**
> - * of_find_node_by_name - Find a node by its "name" property
> - * @from: The node to start searching from or NULL, the node
> - * you pass will not be searched, only the next one
> - * will; typically, you pass what the previous call
> - * returned. of_node_put() will be called on it
> - * @name: The name string to match against
> - *
> - * Returns a node pointer with refcount incremented, use
> - * of_node_put() on it when done.
> - */
> -struct device_node *of_find_node_by_name(struct device_node *from,
> - const char *name)
> -{
> - struct device_node *np;
> -
> - read_lock(&devtree_lock);
> - np = from ? from->allnext : allnodes;
> - for (; np != NULL; np = np->allnext)
> - if (np->name != NULL && strcasecmp(np->name, name) == 0
> - && of_node_get(np))
> - break;
> - of_node_put(from);
> - read_unlock(&devtree_lock);
> - return np;
> -}
> -EXPORT_SYMBOL(of_find_node_by_name);
> -
> -/**
> - * of_find_node_by_type - Find a node by its "device_type" property
> - * @from: The node to start searching from or NULL, the node
> - * you pass will not be searched, only the next one
> - * will; typically, you pass what the previous call
> - * returned. of_node_put() will be called on it
> - * @name: The type string to match against
> - *
> - * Returns a node pointer with refcount incremented, use
> - * of_node_put() on it when done.
> - */
> -struct device_node *of_find_node_by_type(struct device_node *from,
> - const char *type)
> -{
> - struct device_node *np;
> -
> - read_lock(&devtree_lock);
> - np = from ? from->allnext : allnodes;
> - for (; np != 0; np = np->allnext)
> - if (np->type != 0 && strcasecmp(np->type, type) == 0
> - && of_node_get(np))
> - break;
> - of_node_put(from);
> - read_unlock(&devtree_lock);
> - return np;
> -}
> -EXPORT_SYMBOL(of_find_node_by_type);
> -
> -/**
> - * of_find_compatible_node - Find a node based on type and one of the
> - * tokens in its "compatible" property
> - * @from: The node to start searching from or NULL, the node
> - * you pass will not be searched, only the next one
> - * will; typically, you pass what the previous call
> - * returned. of_node_put() will be called on it
> - * @type: The type string to match "device_type" or NULL to ignore
> - * @compatible: The string to match to one of the tokens in the device
> - * "compatible" list.
> - *
> - * Returns a node pointer with refcount incremented, use
> - * of_node_put() on it when done.
> - */
> -struct device_node *of_find_compatible_node(struct device_node *from,
> - const char *type, const char *compatible)
> -{
> - struct device_node *np;
> -
> - read_lock(&devtree_lock);
> - np = from ? from->allnext : allnodes;
> - for (; np != 0; np = np->allnext) {
> - if (type != NULL
> - && !(np->type != 0 && strcasecmp(np->type, type) == 0))
> - continue;
> - if (of_device_is_compatible(np, compatible) && of_node_get(np))
> - break;
> - }
> - of_node_put(from);
> - read_unlock(&devtree_lock);
> - return np;
> -}
> -EXPORT_SYMBOL(of_find_compatible_node);
> -
> -/**
> - * of_find_node_by_path - Find a node matching a full OF path
> - * @path: The full path to match
> - *
> - * Returns a node pointer with refcount incremented, use
> - * of_node_put() on it when done.
> - */
> -struct device_node *of_find_node_by_path(const char *path)
> -{
> - struct device_node *np = allnodes;
> -
> - read_lock(&devtree_lock);
> - for (; np != 0; np = np->allnext) {
> - if (np->full_name != 0 && strcasecmp(np->full_name, path) == 0
> - && of_node_get(np))
> - break;
> - }
> - read_unlock(&devtree_lock);
> - return np;
> -}
> -EXPORT_SYMBOL(of_find_node_by_path);
>
> /**
> * of_find_node_by_phandle - Find a node given a phandle
> diff --git a/arch/sparc/kernel/prom.c b/arch/sparc/kernel/prom.c
> index 3f8ccfa..012f983 100644
> --- a/arch/sparc/kernel/prom.c
> +++ b/arch/sparc/kernel/prom.c
> @@ -25,23 +25,10 @@
> #include <asm/prom.h>
> #include <asm/oplib.h>
>
> -static struct device_node *allnodes;
> +extern struct device_node *allnodes; /* temporary while merging */
>
> extern rwlock_t devtree_lock; /* temporary while merging */
>
> -struct device_node *of_find_node_by_path(const char *path)
> -{
> - struct device_node *np = allnodes;
> -
> - for (; np != 0; np = np->allnext) {
> - if (np->full_name != 0 && strcmp(np->full_name, path) == 0)
> - break;
> - }
> -
> - return np;
> -}
> -EXPORT_SYMBOL(of_find_node_by_path);
> -
> struct device_node *of_find_node_by_phandle(phandle handle)
> {
> struct device_node *np;
> @@ -54,52 +41,6 @@ struct device_node *of_find_node_by_phandle(phandle handle)
> }
> EXPORT_SYMBOL(of_find_node_by_phandle);
>
> -struct device_node *of_find_node_by_name(struct device_node *from,
> - const char *name)
> -{
> - struct device_node *np;
> -
> - np = from ? from->allnext : allnodes;
> - for (; np != NULL; np = np->allnext)
> - if (np->name != NULL && strcmp(np->name, name) == 0)
> - break;
> -
> - return np;
> -}
> -EXPORT_SYMBOL(of_find_node_by_name);
> -
> -struct device_node *of_find_node_by_type(struct device_node *from,
> - const char *type)
> -{
> - struct device_node *np;
> -
> - np = from ? from->allnext : allnodes;
> - for (; np != 0; np = np->allnext)
> - if (np->type != 0 && strcmp(np->type, type) == 0)
> - break;
> -
> - return np;
> -}
> -EXPORT_SYMBOL(of_find_node_by_type);
> -
> -struct device_node *of_find_compatible_node(struct device_node *from,
> - const char *type, const char *compatible)
> -{
> - struct device_node *np;
> -
> - np = from ? from->allnext : allnodes;
> - for (; np != 0; np = np->allnext) {
> - if (type != NULL
> - && !(np->type != 0 && strcmp(np->type, type) == 0))
> - continue;
> - if (of_device_is_compatible(np, compatible))
> - break;
> - }
> -
> - return np;
> -}
> -EXPORT_SYMBOL(of_find_compatible_node);
> -
> int of_getintprop_default(struct device_node *np, const char *name, int def)
> {
> struct property *prop;
> diff --git a/arch/sparc64/kernel/prom.c b/arch/sparc64/kernel/prom.c
> index be3b958..ae2065c 100644
> --- a/arch/sparc64/kernel/prom.c
> +++ b/arch/sparc64/kernel/prom.c
> @@ -29,23 +29,10 @@
> #include <asm/asi.h>
> #include <asm/upa.h>
>
> -static struct device_node *allnodes;
> +extern struct device_node *allnodes; /* temporary while merging */
>
> extern rwlock_t devtree_lock; /* temporary while merging */
>
> -struct device_node *of_find_node_by_path(const char *path)
> -{
> - struct device_node *np = allnodes;
> -
> - for (; np != 0; np = np->allnext) {
> - if (np->full_name != 0 && strcmp(np->full_name, path) == 0)
> - break;
> - }
> -
> - return np;
> -}
> -EXPORT_SYMBOL(of_find_node_by_path);
> -
> struct device_node *of_find_node_by_phandle(phandle handle)
> {
> struct device_node *np;
> @@ -58,52 +45,6 @@ struct device_node *of_find_node_by_phandle(phandle handle)
> }
> EXPORT_SYMBOL(of_find_node_by_phandle);
>
> -struct device_node *of_find_node_by_name(struct device_node *from,
> - const char *name)
> -{
> - struct device_node *np;
> -
> - np = from ? from->allnext : allnodes;
> - for (; np != NULL; np = np->allnext)
> - if (np->name != NULL && strcmp(np->name, name) == 0)
> - break;
> -
> - return np;
> -}
> -EXPORT_SYMBOL(of_find_node_by_name);
> -
> -struct device_node *of_find_node_by_type(struct device_node *from,
> - const char *type)
> -{
> - struct device_node *np;
> -
> - np = from ? from->allnext : allnodes;
> - for (; np != 0; np = np->allnext)
> - if (np->type != 0 && strcmp(np->type, type) == 0)
> - break;
> -
> - return np;
> -}
> -EXPORT_SYMBOL(of_find_node_by_type);
> -
> -struct device_node *of_find_compatible_node(struct device_node *from,
> - const char *type, const char *compatible)
> -{
> - struct device_node *np;
> -
> - np = from ? from->allnext : allnodes;
> - for (; np != 0; np = np->allnext) {
> - if (type != NULL
> - && !(np->type != 0 && strcmp(np->type, type) == 0))
> - continue;
> - if (of_device_is_compatible(np, compatible))
> - break;
> - }
> -
> - return np;
> -}
> -EXPORT_SYMBOL(of_find_compatible_node);
> -
> int of_getintprop_default(struct device_node *np, const char *name, int def)
> {
> struct property *prop;
> diff --git a/drivers/openfw/base.c b/drivers/openfw/base.c
> index 60f7bd4..bb66e48 100644
> --- a/drivers/openfw/base.c
> +++ b/drivers/openfw/base.c
> @@ -20,6 +20,8 @@
> #include <linux/openfw.h>
> #include <linux/spinlock.h>
>
> +struct device_node *allnodes;
> +
> /* use when traversing tree through the allnext, child, sibling,
> * or parent members of struct device_node.
> */
> @@ -158,3 +160,115 @@ struct device_node *of_get_next_child(const struct device_node *node,
> return next;
> }
> EXPORT_SYMBOL(of_get_next_child);
> +
> +/**
> + * of_find_node_by_path - Find a node matching a full OF path
> + * @path: The full path to match
> + *
> + * Returns a node pointer with refcount incremented, use
> + * of_node_put() on it when done.
> + */
> +struct device_node *of_find_node_by_path(const char *path)
> +{
> + struct device_node *np = allnodes;
> +
> + read_lock(&devtree_lock);
> + for (; np; np = np->allnext) {
> + if (np->full_name && (of_node_cmp(np->full_name, path) == 0)
> + && of_node_get(np))
> + break;
> + }
> + read_unlock(&devtree_lock);
> + return np;
> +}
> +EXPORT_SYMBOL(of_find_node_by_path);
> +
> +/**
> + * of_find_node_by_name - Find a node by its "name" property
> + * @from: The node to start searching from or NULL, the node
> + * you pass will not be searched, only the next one
> + * will; typically, you pass what the previous call
> + * returned. of_node_put() will be called on it
> + * @name: The name string to match against
> + *
> + * Returns a node pointer with refcount incremented, use
> + * of_node_put() on it when done.
> + */
> +struct device_node *of_find_node_by_name(struct device_node *from,
> + const char *name)
> +{
> + struct device_node *np;
> +
> + read_lock(&devtree_lock);
> + np = from ? from->allnext : allnodes;
> + for (; np; np = np->allnext)
> + if (np->name && (of_node_cmp(np->name, name) == 0)
> + && of_node_get(np))
> + break;
> + of_node_put(from);
> + read_unlock(&devtree_lock);
> + return np;
> +}
> +EXPORT_SYMBOL(of_find_node_by_name);
> +
> +/**
> + * of_find_node_by_type - Find a node by its "device_type" property
> + * @from: The node to start searching from or NULL, the node
> + * you pass will not be searched, only the next one
> + * will; typically, you pass what the previous call
> + * returned. of_node_put() will be called on it
> + * @name: The type string to match against
> + *
> + * Returns a node pointer with refcount incremented, use
> + * of_node_put() on it when done.
> + */
> +struct device_node *of_find_node_by_type(struct device_node *from,
> + const char *type)
> +{
> + struct device_node *np;
> +
> + read_lock(&devtree_lock);
> + np = from ? from->allnext : allnodes;
> + for (; np; np = np->allnext)
> + if (np->type && (of_node_cmp(np->type, type) == 0)
> + && of_node_get(np))
> + break;
> + of_node_put(from);
> + read_unlock(&devtree_lock);
> + return np;
> +}
> +EXPORT_SYMBOL(of_find_node_by_type);
> +
> +/**
> + * of_find_compatible_node - Find a node based on type and one of the
> + * tokens in its "compatible" property
> + * @from: The node to start searching from or NULL, the node
> + * you pass will not be searched, only the next one
> + * will; typically, you pass what the previous call
> + * returned. of_node_put() will be called on it
> + * @type: The type string to match "device_type" or NULL to ignore
> + * @compatible: The string to match to one of the tokens in the device
> + * "compatible" list.
> + *
> + * Returns a node pointer with refcount incremented, use
> + * of_node_put() on it when done.
> + */
> +struct device_node *of_find_compatible_node(struct device_node *from,
> + const char *type, const char *compatible)
> +{
> + struct device_node *np;
> +
> + read_lock(&devtree_lock);
> + np = from ? from->allnext : allnodes;
> + for (; np; np = np->allnext) {
> + if (type
> + && !(np->type && (of_node_cmp(np->type, type) == 0)))
> + continue;
> + if (of_device_is_compatible(np, compatible) && of_node_get(np))
> + break;
> + }
> + of_node_put(from);
> + read_unlock(&devtree_lock);
> + return np;
> +}
> +EXPORT_SYMBOL(of_find_compatible_node);
> diff --git a/include/asm-powerpc/prom.h b/include/asm-powerpc/prom.h
> index a0679be..d0ff917 100644
> --- a/include/asm-powerpc/prom.h
> +++ b/include/asm-powerpc/prom.h
> @@ -26,6 +26,7 @@
>
> #define of_compat_cmp(s1, s2, l) strncasecmp((s1), (s2), (l))
> #define of_prop_cmp(s1, s2) strcmp((s1), (s2))
> +#define of_node_cmp(s1, s2) strcasecmp((s1), (s2))
>
> /* Definitions used by the flattened device tree */
> #define OF_DT_HEADER 0xd00dfeed /* marker */
> diff --git a/include/asm-sparc/prom.h b/include/asm-sparc/prom.h
> index 4621af6..92919ac 100644
> --- a/include/asm-sparc/prom.h
> +++ b/include/asm-sparc/prom.h
> @@ -25,6 +25,7 @@
>
> #define of_compat_cmp(s1, s2, l) strncmp((s1), (s2), (l))
> #define of_prop_cmp(s1, s2) strcasecmp((s1), (s2))
> +#define of_node_cmp(s1, s2) strcmp((s1), (s2))
>
> typedef u32 phandle;
> typedef u32 ihandle;
> diff --git a/include/asm-sparc64/prom.h b/include/asm-sparc64/prom.h
> index c36b884..d2123c2 100644
> --- a/include/asm-sparc64/prom.h
> +++ b/include/asm-sparc64/prom.h
> @@ -25,6 +25,7 @@
>
> #define of_compat_cmp(s1, s2, l) strncmp((s1), (s2), (l))
> #define of_prop_cmp(s1, s2) strcasecmp((s1), (s2))
> +#define of_node_cmp(s1, s2) strcmp((s1), (s2))
>
> typedef u32 phandle;
> typedef u32 ihandle;
^ permalink raw reply
* Re: [PATCH 3/6] Consolidate of_find_property
From: Benjamin Herrenschmidt @ 2007-04-25 0:00 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: ppc-dev, paulus, David S. Miller
In-Reply-To: <20070424223930.1dab0e28.sfr@canb.auug.org.au>
On Tue, 2007-04-24 at 22:39 +1000, Stephen Rothwell wrote:
> The only change here is that a readlock is taken while the property list
> is being traversed on Sparc where it was not taken previously.
>
> Also, Sparc uses strcasecmp to compare property names while PowerPC
> uses strcmp.
Ok, so that's the opposite as the previous one... property names are
supposed to be case sensitive though, no ? Dave, how do you think we
should converge here ? I don't think we would take much risk on ppc by
using strncasecmp but it shouldn't be necessary...
Ben.
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> arch/powerpc/kernel/prom.c | 24 +-----------------------
> arch/sparc/kernel/prom.c | 22 +---------------------
> arch/sparc64/kernel/prom.c | 22 +---------------------
> drivers/openfw/base.c | 26 ++++++++++++++++++++++++++
> include/asm-powerpc/prom.h | 1 +
> include/asm-sparc/prom.h | 1 +
> include/asm-sparc64/prom.h | 1 +
> 7 files changed, 32 insertions(+), 65 deletions(-)
>
> --
> Cheers,
> Stephen Rothwell sfr@canb.auug.org.au
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 723df55..939d5e4 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -79,10 +79,7 @@ struct boot_param_header *initial_boot_params;
>
> static struct device_node *allnodes = NULL;
>
> -/* use when traversing tree through the allnext, child, sibling,
> - * or parent members of struct device_node.
> - */
> -static DEFINE_RWLOCK(devtree_lock);
> +extern rwlock_t devtree_lock; /* temporary while merging */
>
> /* export that to outside world */
> struct device_node *of_chosen;
> @@ -1454,25 +1451,6 @@ static int __init prom_reconfig_setup(void)
> __initcall(prom_reconfig_setup);
> #endif
>
> -struct property *of_find_property(const struct device_node *np,
> - const char *name,
> - int *lenp)
> -{
> - struct property *pp;
> -
> - read_lock(&devtree_lock);
> - for (pp = np->properties; pp != 0; pp = pp->next)
> - if (strcmp(pp->name, name) == 0) {
> - if (lenp != 0)
> - *lenp = pp->length;
> - break;
> - }
> - read_unlock(&devtree_lock);
> -
> - return pp;
> -}
> -EXPORT_SYMBOL(of_find_property);
> -
> /*
> * Add a property to a node
> */
> diff --git a/arch/sparc/kernel/prom.c b/arch/sparc/kernel/prom.c
> index f2ce0d4..0f5aab4 100644
> --- a/arch/sparc/kernel/prom.c
> +++ b/arch/sparc/kernel/prom.c
> @@ -27,10 +27,7 @@
>
> static struct device_node *allnodes;
>
> -/* use when traversing tree through the allnext, child, sibling,
> - * or parent members of struct device_node.
> - */
> -static DEFINE_RWLOCK(devtree_lock);
> +extern rwlock_t devtree_lock; /* temporary while merging */
>
> struct device_node *of_get_parent(const struct device_node *node)
> {
> @@ -130,23 +127,6 @@ struct device_node *of_find_compatible_node(struct device_node *from,
> }
> EXPORT_SYMBOL(of_find_compatible_node);
>
> -struct property *of_find_property(const struct device_node *np,
> - const char *name,
> - int *lenp)
> -{
> - struct property *pp;
> -
> - for (pp = np->properties; pp != 0; pp = pp->next) {
> - if (strcasecmp(pp->name, name) == 0) {
> - if (lenp != 0)
> - *lenp = pp->length;
> - break;
> - }
> - }
> - return pp;
> -}
> -EXPORT_SYMBOL(of_find_property);
> -
> int of_getintprop_default(struct device_node *np, const char *name, int def)
> {
> struct property *prop;
> diff --git a/arch/sparc64/kernel/prom.c b/arch/sparc64/kernel/prom.c
> index d6dd972..c759660 100644
> --- a/arch/sparc64/kernel/prom.c
> +++ b/arch/sparc64/kernel/prom.c
> @@ -31,10 +31,7 @@
>
> static struct device_node *allnodes;
>
> -/* use when traversing tree through the allnext, child, sibling,
> - * or parent members of struct device_node.
> - */
> -static DEFINE_RWLOCK(devtree_lock);
> +extern rwlock_t devtree_lock; /* temporary while merging */
>
> struct device_node *of_get_parent(const struct device_node *node)
> {
> @@ -134,23 +131,6 @@ struct device_node *of_find_compatible_node(struct device_node *from,
> }
> EXPORT_SYMBOL(of_find_compatible_node);
>
> -struct property *of_find_property(const struct device_node *np,
> - const char *name,
> - int *lenp)
> -{
> - struct property *pp;
> -
> - for (pp = np->properties; pp != 0; pp = pp->next) {
> - if (strcasecmp(pp->name, name) == 0) {
> - if (lenp != 0)
> - *lenp = pp->length;
> - break;
> - }
> - }
> - return pp;
> -}
> -EXPORT_SYMBOL(of_find_property);
> -
> int of_getintprop_default(struct device_node *np, const char *name, int def)
> {
> struct property *prop;
> diff --git a/drivers/openfw/base.c b/drivers/openfw/base.c
> index 0751313..2597d29 100644
> --- a/drivers/openfw/base.c
> +++ b/drivers/openfw/base.c
> @@ -18,6 +18,12 @@
> */
> #include <linux/module.h>
> #include <linux/openfw.h>
> +#include <linux/spinlock.h>
> +
> +/* use when traversing tree through the allnext, child, sibling,
> + * or parent members of struct device_node.
> + */
> +DEFINE_RWLOCK(devtree_lock);
>
> int of_n_addr_cells(struct device_node *np)
> {
> @@ -51,6 +57,26 @@ int of_n_size_cells(struct device_node *np)
> }
> EXPORT_SYMBOL(of_n_size_cells);
>
> +struct property *of_find_property(const struct device_node *np,
> + const char *name,
> + int *lenp)
> +{
> + struct property *pp;
> +
> + read_lock(&devtree_lock);
> + for (pp = np->properties; pp != 0; pp = pp->next) {
> + if (of_prop_cmp(pp->name, name) == 0) {
> + if (lenp != 0)
> + *lenp = pp->length;
> + break;
> + }
> + }
> + read_unlock(&devtree_lock);
> +
> + return pp;
> +}
> +EXPORT_SYMBOL(of_find_property);
> +
> /*
> * Find a property with a given name for a given node
> * and return the value.
> diff --git a/include/asm-powerpc/prom.h b/include/asm-powerpc/prom.h
> index 1f3e8cd..a0679be 100644
> --- a/include/asm-powerpc/prom.h
> +++ b/include/asm-powerpc/prom.h
> @@ -25,6 +25,7 @@
> #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1
>
> #define of_compat_cmp(s1, s2, l) strncasecmp((s1), (s2), (l))
> +#define of_prop_cmp(s1, s2) strcmp((s1), (s2))
>
> /* Definitions used by the flattened device tree */
> #define OF_DT_HEADER 0xd00dfeed /* marker */
> diff --git a/include/asm-sparc/prom.h b/include/asm-sparc/prom.h
> index cc049fd..d3dc05b 100644
> --- a/include/asm-sparc/prom.h
> +++ b/include/asm-sparc/prom.h
> @@ -24,6 +24,7 @@
> #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1
>
> #define of_compat_cmp(s1, s2, l) strncmp((s1), (s2), (l))
> +#define of_prop_cmp(s1, s2) strcasecmp((s1), (s2))
>
> typedef u32 phandle;
> typedef u32 ihandle;
> diff --git a/include/asm-sparc64/prom.h b/include/asm-sparc64/prom.h
> index 843f9e8..a1b64e7 100644
> --- a/include/asm-sparc64/prom.h
> +++ b/include/asm-sparc64/prom.h
> @@ -24,6 +24,7 @@
> #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1
>
> #define of_compat_cmp(s1, s2, l) strncmp((s1), (s2), (l))
> +#define of_prop_cmp(s1, s2) strcasecmp((s1), (s2))
>
> typedef u32 phandle;
> typedef u32 ihandle;
^ permalink raw reply
* Re: [PATCH 2/6] Consolidate of_device_is_compatible
From: Benjamin Herrenschmidt @ 2007-04-24 23:59 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: ppc-dev, paulus, David S. Miller
In-Reply-To: <20070424223812.2842f3f2.sfr@canb.auug.org.au>
On Tue, 2007-04-24 at 22:38 +1000, Stephen Rothwell wrote:
> The only difference here is that Sparc uses strncmp to match compatibility
> names while PowerPC uses strncasecmp.
We should settle for a single implementation. In theory, strncmp would
be the way to go but there have been "bugs" here or there, especially in
Apple DTs, that made me use strncasecmp instead in the past.
Dave, what do you reckon ? I should try to find out the bogus machines
and add workarounds in the various drivers etc... instead or we can just
go common on strncasecmp ?
Ben.
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> arch/powerpc/kernel/prom.c | 25 -------------------------
> arch/sparc/kernel/prom.c | 21 ---------------------
> arch/sparc64/kernel/prom.c | 21 ---------------------
> drivers/openfw/base.c | 24 ++++++++++++++++++++++++
> include/asm-powerpc/prom.h | 2 ++
> include/asm-sparc/prom.h | 2 ++
> include/asm-sparc64/prom.h | 2 ++
> 7 files changed, 30 insertions(+), 67 deletions(-)
>
> --
> Cheers,
> Stephen Rothwell sfr@canb.auug.org.au
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index d701e89..723df55 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -1042,31 +1042,6 @@ void __init early_init_devtree(void *params)
>
> #undef printk
>
> -/** Checks if the given "compat" string matches one of the strings in
> - * the device's "compatible" property
> - */
> -int of_device_is_compatible(const struct device_node *device,
> - const char *compat)
> -{
> - const char* cp;
> - int cplen, l;
> -
> - cp = of_get_property(device, "compatible", &cplen);
> - if (cp == NULL)
> - return 0;
> - while (cplen > 0) {
> - if (strncasecmp(cp, compat, strlen(compat)) == 0)
> - return 1;
> - l = strlen(cp) + 1;
> - cp += l;
> - cplen -= l;
> - }
> -
> - return 0;
> -}
> -EXPORT_SYMBOL(of_device_is_compatible);
> -
> -
> /**
> * Indicates whether the root node has a given value in its
> * compatible property.
> diff --git a/arch/sparc/kernel/prom.c b/arch/sparc/kernel/prom.c
> index ac3f3c2..f2ce0d4 100644
> --- a/arch/sparc/kernel/prom.c
> +++ b/arch/sparc/kernel/prom.c
> @@ -32,27 +32,6 @@ static struct device_node *allnodes;
> */
> static DEFINE_RWLOCK(devtree_lock);
>
> -int of_device_is_compatible(const struct device_node *device,
> - const char *compat)
> -{
> - const char* cp;
> - int cplen, l;
> -
> - cp = of_get_property(device, "compatible", &cplen);
> - if (cp == NULL)
> - return 0;
> - while (cplen > 0) {
> - if (strncmp(cp, compat, strlen(compat)) == 0)
> - return 1;
> - l = strlen(cp) + 1;
> - cp += l;
> - cplen -= l;
> - }
> -
> - return 0;
> -}
> -EXPORT_SYMBOL(of_device_is_compatible);
> -
> struct device_node *of_get_parent(const struct device_node *node)
> {
> struct device_node *np;
> diff --git a/arch/sparc64/kernel/prom.c b/arch/sparc64/kernel/prom.c
> index 7ef5488..d6dd972 100644
> --- a/arch/sparc64/kernel/prom.c
> +++ b/arch/sparc64/kernel/prom.c
> @@ -36,27 +36,6 @@ static struct device_node *allnodes;
> */
> static DEFINE_RWLOCK(devtree_lock);
>
> -int of_device_is_compatible(const struct device_node *device,
> - const char *compat)
> -{
> - const char* cp;
> - int cplen, l;
> -
> - cp = of_get_property(device, "compatible", &cplen);
> - if (cp == NULL)
> - return 0;
> - while (cplen > 0) {
> - if (strncmp(cp, compat, strlen(compat)) == 0)
> - return 1;
> - l = strlen(cp) + 1;
> - cp += l;
> - cplen -= l;
> - }
> -
> - return 0;
> -}
> -EXPORT_SYMBOL(of_device_is_compatible);
> -
> struct device_node *of_get_parent(const struct device_node *node)
> {
> struct device_node *np;
> diff --git a/drivers/openfw/base.c b/drivers/openfw/base.c
> index 04a8cc2..0751313 100644
> --- a/drivers/openfw/base.c
> +++ b/drivers/openfw/base.c
> @@ -63,3 +63,27 @@ const void *of_get_property(const struct device_node *np, const char *name,
> return pp ? pp->value : NULL;
> }
> EXPORT_SYMBOL(of_get_property);
> +
> +/** Checks if the given "compat" string matches one of the strings in
> + * the device's "compatible" property
> + */
> +int of_device_is_compatible(const struct device_node *device,
> + const char *compat)
> +{
> + const char* cp;
> + int cplen, l;
> +
> + cp = of_get_property(device, "compatible", &cplen);
> + if (cp == NULL)
> + return 0;
> + while (cplen > 0) {
> + if (of_compat_cmp(cp, compat, strlen(compat)) == 0)
> + return 1;
> + l = strlen(cp) + 1;
> + cp += l;
> + cplen -= l;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(of_device_is_compatible);
> diff --git a/include/asm-powerpc/prom.h b/include/asm-powerpc/prom.h
> index 30cea54..1f3e8cd 100644
> --- a/include/asm-powerpc/prom.h
> +++ b/include/asm-powerpc/prom.h
> @@ -24,6 +24,8 @@
> #define OF_ROOT_NODE_ADDR_CELLS_DEFAULT 1
> #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1
>
> +#define of_compat_cmp(s1, s2, l) strncasecmp((s1), (s2), (l))
> +
> /* Definitions used by the flattened device tree */
> #define OF_DT_HEADER 0xd00dfeed /* marker */
> #define OF_DT_BEGIN_NODE 0x1 /* Start of node, full name */
> diff --git a/include/asm-sparc/prom.h b/include/asm-sparc/prom.h
> index 411018d..cc049fd 100644
> --- a/include/asm-sparc/prom.h
> +++ b/include/asm-sparc/prom.h
> @@ -23,6 +23,8 @@
> #define OF_ROOT_NODE_ADDR_CELLS_DEFAULT 2
> #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1
>
> +#define of_compat_cmp(s1, s2, l) strncmp((s1), (s2), (l))
> +
> typedef u32 phandle;
> typedef u32 ihandle;
>
> diff --git a/include/asm-sparc64/prom.h b/include/asm-sparc64/prom.h
> index 1097e66..843f9e8 100644
> --- a/include/asm-sparc64/prom.h
> +++ b/include/asm-sparc64/prom.h
> @@ -23,6 +23,8 @@
> #define OF_ROOT_NODE_ADDR_CELLS_DEFAULT 2
> #define OF_ROOT_NODE_SIZE_CELLS_DEFAULT 1
>
> +#define of_compat_cmp(s1, s2, l) strncmp((s1), (s2), (l))
> +
> typedef u32 phandle;
> typedef u32 ihandle;
>
^ permalink raw reply
* Re: [PATCH 1/6] Start split out of common open firmware code
From: David Miller @ 2007-04-24 23:35 UTC (permalink / raw)
To: sfr; +Cc: linuxppc-dev, paulus
In-Reply-To: <20070424223245.78f4fdfb.sfr@canb.auug.org.au>
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 24 Apr 2007 22:32:45 +1000
> This creates drivers/openfw/base.c (depending on CONFIG_OPENFW) and puts
> the first trivially common bits from the prom.c files into it.
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
All 6 patches look great to me. Thanks Stephen.
I've been trying to think about how to ease the merging of
all of this stuff since it touches a bunch of platforms.
I'm happy to take care of all of this stuff in my sparc-2.6.22 GIT
tree at:
master.kernel.org:/pub/scm/linux/kernel/git/davem/sparc-2.6.22.git
which I just rebased, if the PPC folks are fine with this.
If agreed, please just send over the PPC driver et al. patches from
the powerpc tree (ie. the get_property() renaming bits) and I'll suck
them into my sparc-2.6.22 tree.
If not agreed, we do have to come up with some kind of merge plan :-)
^ permalink raw reply
* [PATCH v2] pasemi: Kconfig for gpio_mdio
From: Olof Johansson @ 2007-04-24 23:17 UTC (permalink / raw)
To: paulus; +Cc: linuxppc-dev, arnd
In-Reply-To: <20070424175834.GA7985@lixom.net>
arch/powerpc/platforms/pasemi/gpio_mdio.c really depends on CONFIG_PHYLIB.
Add a config option for it, allow for it to be disabled if needed and fix
the dependency.
Signed-off-by: Olof Johansson <olof@lixom.net>
diff --git a/arch/powerpc/platforms/pasemi/Kconfig b/arch/powerpc/platforms/pasemi/Kconfig
index 64e5525..eb4dbc7 100644
--- a/arch/powerpc/platforms/pasemi/Kconfig
+++ b/arch/powerpc/platforms/pasemi/Kconfig
@@ -19,4 +19,11 @@ config PPC_PASEMI_IOMMU
help
IOMMU support for PA6T-1682M
+config PPC_PASEMI_MDIO
+ depends on PHYLIB
+ tristate "MDIO support via GPIO"
+ default y
+ help
+ Driver for MDIO via GPIO on PWRficient platforms
+
endmenu
diff --git a/arch/powerpc/platforms/pasemi/Makefile b/arch/powerpc/platforms/pasemi/Makefile
index e90265f..7ffd43b 100644
--- a/arch/powerpc/platforms/pasemi/Makefile
+++ b/arch/powerpc/platforms/pasemi/Makefile
@@ -1,2 +1,2 @@
-obj-y += setup.o pci.o time.o idle.o powersave.o iommu.o gpio_mdio.o
-
+obj-y += setup.o pci.o time.o idle.o powersave.o iommu.o
+obj-$(CONFIG_PPC_PASEMI_MDIO) += gpio_mdio.o
^ permalink raw reply related
* Re: [PATCH] 64K page support for kexec
From: Olof Johansson @ 2007-04-24 23:07 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Paul Mackerras, cbe-oss-dev, Arnd Bergmann, linuxppc-dev
In-Reply-To: <1177455014.14873.144.camel@localhost.localdomain>
On Wed, Apr 25, 2007 at 08:50:14AM +1000, Benjamin Herrenschmidt wrote:
>
> > This assumes that the page sizes are ordered. Why not just iterate from
> > 0 to MMU_PAGE_COUNT?
>
> You don't want to hit MMU_PAGE_4K which is guaranteed to be 0 ... maybe
> just having an if (size == MMU_PAGE_4K) continue; statement in the loop
> would be better ?
Either that, or from 1 to MMU_PAGE_COUNT (with a comment as to why we're skipping
0).
> > > + /*
> > > + * FIXME, this could be made more efficient by storing the type
> > > + * of hash algorithm in mmu_psize_defs[]. The code below assumes
> > > + * the number of bits in the va representing the offset in the
> > > + * page is less than 23. This affects the hash algorithm that is
> > > + * used. When 16G pages are supported, a new hash algorithm
> > > + * needs to be provided. See POWER ISA Book III.
> > > + *
> > > + * The code below works for 16M, 64K, and 4K pages.
> > > + */
> >
> > A BUG_ON() when other sizes are hit could be a good idea?
>
> a BUG_ON if the B bit is set would be useful too. (that is 1T segment
> HPTE).
Yep.
> > > @@ -408,8 +453,9 @@ static void native_hpte_clear(void)
> > > * already hold the native_tlbie_lock.
> > > */
> > > if (hpte_v & HPTE_V_VALID) {
> > > + hpte_decode(hptep, slot, &psize, &hpte_v);
> > > hptep->v = 0;
> > > - __tlbie(slot2va(hpte_v, slot), MMU_PAGE_4K);
> > > + __tlbie(hpte_v, psize);
> >
> > Using hpte_v as variable name is a bit misleading. avpn or va would be
> > a better variable name.
>
> No. The variable doesn't contain only the va, it contains the whole "V"
> part of the HPTE which includes other things like the valid bit.
Ok. The previous usage was confusing me (given that they did a translation
from hpte_v to va, or at least that's what the function name implies).
-Olof
^ permalink raw reply
* Re: [PATCH] pasemi: Kconfig for mdio_gpio
From: Olof Johansson @ 2007-04-24 23:04 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev, paulus
In-Reply-To: <200704242353.07286.arnd@arndb.de>
On Tue, Apr 24, 2007 at 11:53:06PM +0200, Arnd Bergmann wrote:
> On Tuesday 24 April 2007, Olof Johansson wrote:
> > +config PPC_PASEMI_MDIO
> > +???????depends on PPC_PASEMI
> > +???????bool "MDIO support via GPIO"
> > +???????default y
> > +???????select PHYLIB
> > +???????help
> > +??????? ?Driver for MDIO via GPIO on PWRficient platforms
> > +
>
> Looks still wrong. I stumbled over the original bug because in fedora,
> CONFIG_PHYLIB=m. When you make PPC_PASEMI_MDIO a bool, it will
> force PHYLIB=y, which is potentially undesired.
>
> Can you make this a modular driver (tristate)?
>
> Instead of making it select PHYLIB directly, it might be more
> straightforward to make PPC_PASEMI_MDIO depend on PHYLIB
> and have the actual network driver select both PPC_PASEMI_MDIO
> and PHYLIB.
Good points. New patch shortly.
-Olof
^ permalink raw reply
* Re: [PATCH] 64K page support for kexec
From: Benjamin Herrenschmidt @ 2007-04-24 22:50 UTC (permalink / raw)
To: Olof Johansson; +Cc: Paul Mackerras, cbe-oss-dev, Arnd Bergmann, linuxppc-dev
In-Reply-To: <20070424194348.GA8371@lixom.net>
> This assumes that the page sizes are ordered. Why not just iterate from
> 0 to MMU_PAGE_COUNT?
You don't want to hit MMU_PAGE_4K which is guaranteed to be 0 ... maybe
just having an if (size == MMU_PAGE_4K) continue; statement in the loop
would be better ?
> > + if (!mmu_psize_defs[size].shift)
> > + continue;
>
> A comment to the effect of "unused entries have a shift value of 0" could be
> useful.
>
> > + if (penc == mmu_psize_defs[size].penc)
> > + break;
> > + }
> > + }
> >
> > - vpi = ((va >> 28) ^ pteg) & htab_hash_mask;
> > + /*
> > + * FIXME, this could be made more efficient by storing the type
> > + * of hash algorithm in mmu_psize_defs[]. The code below assumes
> > + * the number of bits in the va representing the offset in the
> > + * page is less than 23. This affects the hash algorithm that is
> > + * used. When 16G pages are supported, a new hash algorithm
> > + * needs to be provided. See POWER ISA Book III.
> > + *
> > + * The code below works for 16M, 64K, and 4K pages.
> > + */
>
> A BUG_ON() when other sizes are hit could be a good idea?
a BUG_ON if the B bit is set would be useful too. (that is 1T segment
HPTE).
> > + shift = mmu_psize_defs[size].shift;
> > + if (mmu_psize_defs[size].avpnm)
> > + avpnm_bits = __ilog2_u64(mmu_psize_defs[size].avpnm) + 1;
> > + else
> > + avpnm_bits = 0;
> > + if (shift - avpnm_bits <= 23) {
> > + avpn = HPTE_V_AVPN_VAL(hpte_v) << 23;
> > +
> > + if (shift < 23) {
> > + unsigned long vpi, pteg;
> > +
> > + pteg = slot / HPTES_PER_GROUP;
> > + if (hpte_v & HPTE_V_SECONDARY)
> > + pteg = ~pteg;
> > + vpi = ((avpn >> 28) ^ pteg) & htab_hash_mask;
> > + avpn |= (vpi << mmu_psize_defs[size].shift);
> > + }
> > + }
> > +#if 0
> > + /* 16GB page hash, p > 23 */
> > + else {
>
> Same thing here w.r.t. ifdefs
>
> >
> > - va |= vpi << PAGE_SHIFT;
> > }
> > +#endif
> >
> > - return va;
> > + *va = avpn;
> > + *psize = size;
> > }
> >
> > /*
> > @@ -374,8 +420,6 @@ static unsigned long slot2va(unsigned lo
> > *
> > * TODO: add batching support when enabled. remember, no dynamic memory here,
> > * athough there is the control page available...
> > - *
> > - * XXX FIXME: 4k only for now !
> > */
> > static void native_hpte_clear(void)
> > {
> > @@ -383,6 +427,7 @@ static void native_hpte_clear(void)
> > hpte_t *hptep = htab_address;
> > unsigned long hpte_v;
> > unsigned long pteg_count;
> > + int psize;
> >
> > pteg_count = htab_hash_mask + 1;
> >
> > @@ -408,8 +453,9 @@ static void native_hpte_clear(void)
> > * already hold the native_tlbie_lock.
> > */
> > if (hpte_v & HPTE_V_VALID) {
> > + hpte_decode(hptep, slot, &psize, &hpte_v);
> > hptep->v = 0;
> > - __tlbie(slot2va(hpte_v, slot), MMU_PAGE_4K);
> > + __tlbie(hpte_v, psize);
>
> Using hpte_v as variable name is a bit misleading. avpn or va would be
> a better variable name.
No. The variable doesn't contain only the va, it contains the whole "V"
part of the HPTE which includes other things like the valid bit.
Ben.
^ permalink raw reply
* Re: [PATCH] 64K page support for kexec
From: Benjamin Herrenschmidt @ 2007-04-24 22:48 UTC (permalink / raw)
To: Luke Browning; +Cc: linuxppc-dev, Paul Mackerras, cbe-oss-dev, Arnd Bergmann
In-Reply-To: <1177439513.24866.5.camel@luke-laptop>
Getting better :-)
Sorry for the constant nagging, let's say I'm a bit perfectionist...
> -static unsigned long slot2va(unsigned long hpte_v, unsigned long slot)
> -{
> - unsigned long avpn = HPTE_V_AVPN_VAL(hpte_v);
> - unsigned long va;
> -
> - va = avpn << 23;
> -
> - if (! (hpte_v & HPTE_V_LARGE)) {
> - unsigned long vpi, pteg;
> -
> - pteg = slot / HPTES_PER_GROUP;
> - if (hpte_v & HPTE_V_SECONDARY)
> - pteg = ~pteg;
Hrm... hpte_decode ends up being a pretty big function... I suppose
that's ok.
> +#define LP_SHIFT 12
> +#define LP_BITS 8
> +#define LP_MASK(i) ((((1 << LP_BITS) - 1) >> (i)) << LP_SHIFT)
> +
> +static void hpte_decode(hpte_t *hpte, unsigned long slot,
> + int *psize, unsigned long *va)
> +{
> + unsigned long hpte_r = hpte->r;
> + unsigned long hpte_v = hpte->v;
> + unsigned long avpn;
> + int i, size, shift, penc, avpnm_bits;
> +
> + if (!(hpte_v & HPTE_V_LARGE))
> + size = MMU_PAGE_4K;
> +#if 0
> + else if (hpte_v & 0x4000000000000000UL)
> + size = MMU_PAGE_16G;
> +#endif
Remove the above. I don't think it's right anyway. We'll deal with 16G
pages when we start using them.
> + else if (!(hpte_r & LP_MASK(0)))
> + size = MMU_PAGE_16M;
Is that correct ? (The above). I haven't quite noticed in the previous
instances of the patch, sorry about that but I don't think that's the
way to detect the "old style" 16M pages... I -suspect- that the normal
algorithm will work for them, that is, they'll have a penc of 0 which
will match what's in the mmu_psize_defs[MMU_PAGE_16M] but it's worth
actually testing it.
Now that I think about it, it's possible that I lead you on the wrong
track there initially... Sorry about that.
I think your above code will treat anything with a penc of 0 as a 16M
page, which might be true with current implementations, is also, I
think, not mandated by the arch, is it ? (I don't have my 2.03 at hand
as I'm writing this email).
> + else {
> + for (i = 0; i < LP_BITS; i++) {
> + if ((hpte_r & LP_MASK(i+1)) == LP_MASK(i+1))
> + break;
> + }
> + penc = LP_MASK(i+1) >> LP_SHIFT;
> + for (size = MMU_PAGE_64K; size < MMU_PAGE_16M; size++) {
> + if (!mmu_psize_defs[size].shift)
> + continue;
> + if (penc == mmu_psize_defs[size].penc)
> + break;
> + }
> + }
>
> - vpi = ((va >> 28) ^ pteg) & htab_hash_mask;
> + /*
> + * FIXME, this could be made more efficient by storing the type
> + * of hash algorithm in mmu_psize_defs[]. The code below assumes
> + * the number of bits in the va representing the offset in the
> + * page is less than 23. This affects the hash algorithm that is
> + * used. When 16G pages are supported, a new hash algorithm
> + * needs to be provided. See POWER ISA Book III.
> + *
> + * The code below works for 16M, 64K, and 4K pages.
> + */
I'm not 100% certain about your comment. The by type of hash algorithm
you mean the segment size right ? This is not directly related to the
page size. While 1T segments are mandatory for 16G pages, they can also
hold normal page sizes... If we're going to implement support for 1T
segment, we should get the segment size (and thus the hash algorithm)
from the B bit of the PTE.
In fact, when doing 1T segments, we'll have to deal with them regardless
of the page size (the hashing will be different for all page sizes).
> + shift = mmu_psize_defs[size].shift;
> + if (mmu_psize_defs[size].avpnm)
> + avpnm_bits = __ilog2_u64(mmu_psize_defs[size].avpnm) + 1;
> + else
> + avpnm_bits = 0;
> + if (shift - avpnm_bits <= 23) {
> + avpn = HPTE_V_AVPN_VAL(hpte_v) << 23;
> +
> + if (shift < 23) {
> + unsigned long vpi, pteg;
> +
> + pteg = slot / HPTES_PER_GROUP;
> + if (hpte_v & HPTE_V_SECONDARY)
> + pteg = ~pteg;
> + vpi = ((avpn >> 28) ^ pteg) & htab_hash_mask;
> + avpn |= (vpi << mmu_psize_defs[size].shift);
> + }
> + }
> +#if 0
> + /* 16GB page hash, p > 23 */
> + else {
>
> - va |= vpi << PAGE_SHIFT;
> }
> +#endif
Just don't keep the code in #if 0, just a comment about something
needing to be done for 16G ...
> - return va;
> + *va = avpn;
> + *psize = size;
> }
>
> /*
> @@ -374,8 +420,6 @@ static unsigned long slot2va(unsigned lo
> *
> * TODO: add batching support when enabled. remember, no dynamic memory here,
> * athough there is the control page available...
> - *
> - * XXX FIXME: 4k only for now !
> */
> static void native_hpte_clear(void)
> {
> @@ -383,6 +427,7 @@ static void native_hpte_clear(void)
> hpte_t *hptep = htab_address;
> unsigned long hpte_v;
> unsigned long pteg_count;
> + int psize;
>
> pteg_count = htab_hash_mask + 1;
>
> @@ -408,8 +453,9 @@ static void native_hpte_clear(void)
> * already hold the native_tlbie_lock.
> */
> if (hpte_v & HPTE_V_VALID) {
> + hpte_decode(hptep, slot, &psize, &hpte_v);
> hptep->v = 0;
> - __tlbie(slot2va(hpte_v, slot), MMU_PAGE_4K);
> + __tlbie(hpte_v, psize);
> }
> }
>
>
^ permalink raw reply
* Re: [PATCH] powerpc: uninline of_iomap function
From: Benjamin Herrenschmidt @ 2007-04-24 22:35 UTC (permalink / raw)
To: Christian Krafft
Cc: linuxppc-dev, Paul Mackerras, Arnd Bergmann, Arnd Bergmann
In-Reply-To: <20070424173202.15e5ce42@localhost>
On Tue, 2007-04-24 at 17:32 +0200, Christian Krafft wrote:
> From: Christian Krafft <krafft@de.ibm.com>
> There is no big reason to have that function inlined.
>
> Signed-off-by: Christian Krafft <krafft@de.ibm.com>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Index: linux-2.6.21-rc4/arch/powerpc/kernel/prom_parse.c
> ===================================================================
> --- linux-2.6.21-rc4.orig/arch/powerpc/kernel/prom_parse.c
> +++ linux-2.6.21-rc4/arch/powerpc/kernel/prom_parse.c
> @@ -1042,3 +1042,15 @@ const void *of_get_mac_address(struct de
> }
> EXPORT_SYMBOL(of_get_mac_address);
>
> +void __iomem *of_iomap(struct device_node *np, int index)
> +{
> + struct resource res;
> +
> + if (of_address_to_resource(np, index, &res))
> + return NULL;
> +
> + return ioremap(res.start, 1 + res.end - res.start);
> +}
> +EXPORT_SYMBOL(of_iomap);
> +
> +
> Index: linux-2.6.21-rc4/include/asm-powerpc/prom.h
> ===================================================================
> --- linux-2.6.21-rc4.orig/include/asm-powerpc/prom.h
> +++ linux-2.6.21-rc4/include/asm-powerpc/prom.h
> @@ -19,7 +19,6 @@
> #include <linux/proc_fs.h>
> #include <linux/platform_device.h>
> #include <asm/atomic.h>
> -#include <asm/io.h>
>
> /* Definitions used by the flattened device tree */
> #define OF_DT_HEADER 0xd00dfeed /* marker */
> @@ -352,16 +351,14 @@ static inline int of_irq_to_resource(str
> return irq;
> }
>
> -static inline void __iomem *of_iomap(struct device_node *np, int index)
> -{
> - struct resource res;
> -
> - if (of_address_to_resource(np, index, &res))
> - return NULL;
> -
> - return ioremap(res.start, 1 + res.end - res.start);
> -}
> -
> +/**
> + * of_iomap - Maps the memory mapped IO for a given device_node
> + * @device: the device whose io range will be mapped
> + * @index: index of the io range
> + *
> + * Returns a pointer to the mapped memory
> + */
> +extern void __iomem *of_iomap(struct device_node *device, int index);
>
> #endif /* __KERNEL__ */
> #endif /* _POWERPC_PROM_H */
>
>
^ permalink raw reply
* Re: [PATCH] Xilinx framebuffer device driver
From: Arnd Bergmann @ 2007-04-24 22:35 UTC (permalink / raw)
To: linuxppc-embedded; +Cc: Andrei Konovalov, Rick Moleres
In-Reply-To: <462E0D5C.1090105@ru.mvista.com>
PiAroKCgoKCgoC5hY3RpdmF0ZSA9oKCgoKBGQl9BQ1RJVkFURV9OT1csCj4gK6CgoKCgoKAuaGVp
Z2h0ID0goKCgoKCgOTksoKCgoKAvKiBpbiBtbSBvZiBORUMgTkw2NDQ4QkMyMC0wOCBvbiBNTDMw
MCAqLwo+ICugoKCgoKCgLndpZHRoID2goKCgoKCgoDEzMqCgoKCgLyogaW4gbW0gb2YgTkVDIE5M
NjQ0OEJDMjAtMDggb24gTUwzMDAgKi8KPiArfTsKClRoZSBzaXplIGxvb2tzIHZlcnkgc3BlY2lm
aWMgdG8gYSBwYXJ0aWN1bGFyIGRpc3BsYXkgaGFyZHdhcmUuIEkgZ3Vlc3MKd2hlbiB0aGUgZHJp
dmVyIGdldHMgY29udmVydGVkIHRvIGFuIG9mX3BsYXRmb3JtX2RyaXZlciwgdGhpcyBzaG91bGQg
Y29tZQpmcm9tIHRoZSBkZXZpY2UgdHJlZS4KClVudGlsIHRoZW4sIG1heWJlIGEgY29uZmlnIG9w
dGlvbiwgd2l0aCBhIHJlYXNvbmFibGUgZGVmYXVsdCB3b3VsZCBiZQpyaWdodC4KCj4gKwo+ICtz
dHJ1Y3QgeGlsaW54ZmJfZHJ2ZGF0YSB7Cj4gKwo+ICugoKCgoKCgc3RydWN0IGZiX2luZm+goGlu
Zm87oKCgoKCgoKCgoKAvKiBGQiBkcml2ZXIgaW5mbyByZWNvcmQgKi8KPiArCj4gK6CgoKCgoKB1
bnNpZ25lZCBsb25noKCgcmVnc19waHlzO6CgoKCgoC8qIHBoeXMuIGFkZHJlc3Mgb2YgdGhlIGNv
bnRyb2wgcmVnaXN0ZXJzICovCj4gK6CgoKCgoKB1MzIgoKCgoKCgoKCgoKCgKnJlZ3M7oKCgoKCg
oKCgoC8qIHZpcnQuIGFkZHJlc3Mgb2YgdGhlIGNvbnRyb2wgcmVnaXN0ZXJzICovCj4gKwo+ICug
oKCgoKCgdW5zaWduZWQgY2hhcqCgoCpmYl92aXJ0O6CgoKCgoKAvKiB2aXJ0LiBhZGRyZXNzIG9m
IHRoZSBmcmFtZSBidWZmZXIgKi8KClRoZSB2aXJ0dWFsIGFkZHJlc3NlcyBzaG91bGQgYmUgbWFy
a2VkIGFzIF9faW9tZW0sIHNvIHlvdSBjYW4gY2hlY2sgdGhlIGNvcnJlY3QKdXNhZ2Ugd2l0aCBz
cGFyc2UuCgpOb3Qgc3VyZSBhYm91dCByZWdzX3BoeXMuIENhbiB0aGlzIGJlIGJleW9uZCB0aGUg
MzIgYml0IGxpbWl0IG9uIGFueSBtYWNoaW5lPwoKPiArCj4gK6CgoKCgoKBkcnZkYXRhID0ga21h
bGxvYyhzaXplb2Yoc3RydWN0IHhpbGlueGZiX2RydmRhdGEpLCBHRlBfS0VSTkVMKTsKPiAroKCg
oKCgoGlmICghZHJ2ZGF0YSkgewo+ICugoKCgoKCgoKCgoKCgoKBwcmludGsoS0VSTl9FUlIgIkNv
dWxkbid0IGFsbG9jYXRlIGRldmljZSBwcml2YXRlIHJlY29yZFxuIik7Cj4gK6CgoKCgoKCgoKCg
oKCgoHJldHVybiAtRU5PTUVNOwo+ICugoKCgoKCgfQo+ICugoKCgoKCgbWVtc2V0KCh2b2lkKilk
cnZkYXRhLCAwLCBzaXplb2Yoc3RydWN0IHhpbGlueGZiX2RydmRhdGEpKTsKClVzZQoJZHJ2ZGF0
YSA9IGt6YWxsb2Moc2l6ZW9mICgqZHJ2ZGF0YSksIEdGUF9LRVJORUwpOwoKPiAroKCgoKCgoGRy
dmRhdGEtPnJlZ3NfcGh5cyA9IHJlZ3NfcmVzLT5zdGFydDsKPiAroKCgoKCgoGRydmRhdGEtPnJl
Z3MgPSAodTMyICopIGlvcmVtYXAocmVnc19yZXMtPnN0YXJ0LCA4KTsKCnUzMiBfX2lvbWVtKgoK
CglBcm5kIDw+PAo=
^ permalink raw reply
* Re: [PATCH] generic check_legacy_ioport
From: Benjamin Herrenschmidt @ 2007-04-24 22:34 UTC (permalink / raw)
To: Olaf Hering; +Cc: linuxppc-dev
In-Reply-To: <20070424112530.GA11489@aepfle.de>
> Do you think a device_type fdc, i8042 or ipmi will appear outside an isa
> node?
Well, if they do, I don't want that code to recognize them... I'm pretty
sure for example we have IPMI on QS21 blades and it's not on an ISA bus
and shouldn't match the legacy IO ports for example.
> > To be totally correct, we should -also- check if the port number fits in
> > the the actual "reg" property but I'm not sure I can be bothered :-)
>
> Why all this complexity? Its there mainly to match a class of boards,
> not to match a specific device configuration.
>
> Thats how it may look finally, currently only compile tested.
>
> int check_legacy_ioport(unsigned long base_port)
> {
> struct device_node *parent, *np = NULL;
> int ret = -ENODEV;
>
> switch(base_port) {
> case I8042_DATA_REG:
> np = of_find_node_by_type(NULL, "8042");
> break;
> case FDC_BASE: /* FDC1 */
> np = of_find_node_by_type(NULL, "fdc");
> break;
> case 0xca2:
> case 0xca9:
> case 0xe4:
> np = of_find_node_by_type(NULL, "ipmi");
> break;
> #ifdef CONFIG_PPC_PREP
> case _PIDXR:
> case _PNPWRP:
> case PNPBIOS_BASE:
> /* implement me */
> #endif
> default:
> break;
> }
> if (np) {
Why not just
if (np == NULL)
return -ENODEV;
And save some tabs ? :-)
> parent = of_get_parent(np);
> if (parent) {
> ret = strcmp(parent->type, "isa");
> of_node_put(parent);
> }
> of_node_put(np);
> }
> return ret;
> }
Looks good except maybe
ret = strcmp(parent->type, "isa");
should probably be
ret = strcmp(parent->type, "isa") ? -ENODEV : 0;
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] Xilinx framebuffer device driver
From: Grant Likely @ 2007-04-24 22:17 UTC (permalink / raw)
To: Andrei Konovalov; +Cc: Rick Moleres, linuxppc-embedded
In-Reply-To: <462E0D5C.1090105@ru.mvista.com>
On 4/24/07, Andrei Konovalov <akonovalov@ru.mvista.com> wrote:
> Add support for the video controller IP block included into Xilinx ML300 and
> ML403 reference designs.
>
> Signed-off-by: Andrei Konovalov <akonovalov@ru.mvista.com>
> ---
>
> This patch relies on the "Patchset to establish sanity in Xilinx Virtex support" by Gran Likely to have
> the frame buffer device registered on the platform bus. Without this patchset one needs to fill in
> the struct platform_device and make sure platform_device_register() is called elsewhere.
>
> Reviews and comments are welcome.
>
> Would be nice to get this driver into mainline for the 2.6.22.
Quick comment on first perusal: The driver uses the out_be32 macro
directly for accessing registers, which doesn't work if the FB block
is configured for DCR access (like the ML403 reference design). There
will need to be a property in the platform device binding to determine
how to access registers.
Cheers,
g.
--
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH] generic check_legacy_ioport
From: Arnd Bergmann @ 2007-04-24 22:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Christian Krafft, Olaf Hering
In-Reply-To: <20070424185402.GA16077@aepfle.de>
On Tuesday 24 April 2007, Olaf Hering wrote:
>
> > Do you think a device_type fdc, i8042 or ipmi will appear outside an isa
> > node?
>
> Does anyone know where those ipmi devices appear in the device-tree?
There are actually _no_ ipmi devices that we expect to appear here.
the reason why the check is in the ipmi driver is in order not to crash
on powerpc machines that load the ipmi driver but have no ipmi nodes
in the device tree.
The init function of the ipmi module will scan all buses (PCI, ACPI, OF,
...) until it finds any devices using regular probes. If it doesn't,
it will poke at "well-known" io-ports that are used by convention on
legacy i386 machines.
I don't think there are any powerpc machines where it can find something
there, but we decided to leave the code architecture independent in
case there ever are, and just to add the check_legacy_ioport call in there.
Arnd <><
^ permalink raw reply
* Re: [PATCH] pasemi: Kconfig for mdio_gpio
From: Arnd Bergmann @ 2007-04-24 21:53 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Olof Johansson, paulus
In-Reply-To: <20070424175834.GA7985@lixom.net>
On Tuesday 24 April 2007, Olof Johansson wrote:
> +config PPC_PASEMI_MDIO
> +=A0=A0=A0=A0=A0=A0=A0depends on PPC_PASEMI
> +=A0=A0=A0=A0=A0=A0=A0bool "MDIO support via GPIO"
> +=A0=A0=A0=A0=A0=A0=A0default y
> +=A0=A0=A0=A0=A0=A0=A0select PHYLIB
> +=A0=A0=A0=A0=A0=A0=A0help
> +=A0=A0=A0=A0=A0=A0=A0 =A0Driver for MDIO via GPIO on PWRficient platforms
> +
Looks still wrong. I stumbled over the original bug because in fedora,
CONFIG_PHYLIB=3Dm. When you make PPC_PASEMI_MDIO a bool, it will
force PHYLIB=3Dy, which is potentially undesired.
Can you make this a modular driver (tristate)?
Instead of making it select PHYLIB directly, it might be more
straightforward to make PPC_PASEMI_MDIO depend on PHYLIB
and have the actual network driver select both PPC_PASEMI_MDIO
and PHYLIB.
Arnd <><
^ permalink raw reply
* Re: [PATCH 2/2] POWERPC: Remove global CPM mappings
From: Dan Malek @ 2007-04-24 20:24 UTC (permalink / raw)
To: Vitaly Bordug; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20070424091831.1793fa26@localhost.localdomain>
On Apr 24, 2007, at 1:18 AM, Vitaly Bordug wrote:
> At first I was thinking of make_everybody_happy solution,
Just make me happy :-) The problem with these
"works in progress" submissions is no one ever
goes back to properly finish them, so I suspect
this code you are checking in, which I don't
like at all, is going to just stay there and bug
me until _I_ fix it.
> I know it can be more efficient. And I am looking at this way, but
> it just cannot
> be achieved via single step.
Why? Because it's more work than you want to do?
> TODO list includes rehaul of GPIO (with long-time-grown
> feature_call + device tree bindings that were implemented for 8360
> but looks reasonable), muxing, etc.
Nothing you have done so far affects any of this.
The things you are changing are the very basic
support functions. You'll never go back and
fix these because then the excuse will be
"all of the drivers now use it."
> This patch just fixes what already exist in kernel, removing the
> global IMMR pointer
It doesn't "fix" anything, it just changes the model to make it
more heavyweight.
> and bringing all remaining code paths to the same need_stuff-
> >immr_map->use_it->immr_unmap model.
That's not the model. The model is: driver init maps IMMR,
driver uses it's local mapping, driver unloaded unmaps IMMR.
We don't map/unmap on every use. The macros should be
given this mapped immr as a parameter, not always
map and unmap because it takes more than five minutes
to properly change all of the APIs.
> Current code is messy at some parts, and this what I am trying to
> address (with current patch and upcomings).
IMHO this does nothing to clean it up. I don't understand
the big deal about needing to remove the IMMR as a global
pointer. The kernel has lots of other global variables that
no one seems concerned about and admit they must be
present. These crappy macros and hacks in the name of
removing a global IMMR just isn't right.
> Thanks for looking at it!
I'm not in any agreement this is correct nor do I want to
see the crap checked in. You're turning a simple,
couple of clock cycle memory access into huge overhead,
probably context switchable operation. Someone is
going to grab a spin lock, and end up crashing on
what appears to be a trivial memory access.....
This is not progress.
Thanks.
-- Dan
^ permalink raw reply
* Re: [PATCH] 64K page support for kexec
From: Olof Johansson @ 2007-04-24 19:43 UTC (permalink / raw)
To: Luke Browning; +Cc: Paul Mackerras, cbe-oss-dev, Arnd Bergmann, linuxppc-dev
In-Reply-To: <1177439513.24866.5.camel@luke-laptop>
Hi,
Didn't see this posted before, but see some comments below.
Thanks,
Olof
On Tue, Apr 24, 2007 at 03:31:53PM -0300, Luke Browning wrote:
> +#define LP_SHIFT 12
> +#define LP_BITS 8
> +#define LP_MASK(i) ((((1 << LP_BITS) - 1) >> (i)) << LP_SHIFT)
Seems to me that something like:
#define LP_MASK(i) ((0xff >> (i)) << LP_SHIFT)
is considerably easier to read.
> +static void hpte_decode(hpte_t *hpte, unsigned long slot,
> + int *psize, unsigned long *va)
> +{
> + unsigned long hpte_r = hpte->r;
> + unsigned long hpte_v = hpte->v;
> + unsigned long avpn;
> + int i, size, shift, penc, avpnm_bits;
> +
> + if (!(hpte_v & HPTE_V_LARGE))
> + size = MMU_PAGE_4K;
> +#if 0
> + else if (hpte_v & 0x4000000000000000UL)
> + size = MMU_PAGE_16G;
> +#endif
No ifdefs please. Just take it out if it's not needed.
> + else if (!(hpte_r & LP_MASK(0)))
> + size = MMU_PAGE_16M;
> + else {
> + for (i = 0; i < LP_BITS; i++) {
> + if ((hpte_r & LP_MASK(i+1)) == LP_MASK(i+1))
> + break;
> + }
> + penc = LP_MASK(i+1) >> LP_SHIFT;
No need for braces. Also, why iterate from 0 to LP_BITS if you're doing i+1 everywhere?
> + for (size = MMU_PAGE_64K; size < MMU_PAGE_16M; size++) {
This assumes that the page sizes are ordered. Why not just iterate from
0 to MMU_PAGE_COUNT?
> + if (!mmu_psize_defs[size].shift)
> + continue;
A comment to the effect of "unused entries have a shift value of 0" could be
useful.
> + if (penc == mmu_psize_defs[size].penc)
> + break;
> + }
> + }
>
> - vpi = ((va >> 28) ^ pteg) & htab_hash_mask;
> + /*
> + * FIXME, this could be made more efficient by storing the type
> + * of hash algorithm in mmu_psize_defs[]. The code below assumes
> + * the number of bits in the va representing the offset in the
> + * page is less than 23. This affects the hash algorithm that is
> + * used. When 16G pages are supported, a new hash algorithm
> + * needs to be provided. See POWER ISA Book III.
> + *
> + * The code below works for 16M, 64K, and 4K pages.
> + */
A BUG_ON() when other sizes are hit could be a good idea?
> + shift = mmu_psize_defs[size].shift;
> + if (mmu_psize_defs[size].avpnm)
> + avpnm_bits = __ilog2_u64(mmu_psize_defs[size].avpnm) + 1;
> + else
> + avpnm_bits = 0;
> + if (shift - avpnm_bits <= 23) {
> + avpn = HPTE_V_AVPN_VAL(hpte_v) << 23;
> +
> + if (shift < 23) {
> + unsigned long vpi, pteg;
> +
> + pteg = slot / HPTES_PER_GROUP;
> + if (hpte_v & HPTE_V_SECONDARY)
> + pteg = ~pteg;
> + vpi = ((avpn >> 28) ^ pteg) & htab_hash_mask;
> + avpn |= (vpi << mmu_psize_defs[size].shift);
> + }
> + }
> +#if 0
> + /* 16GB page hash, p > 23 */
> + else {
Same thing here w.r.t. ifdefs
>
> - va |= vpi << PAGE_SHIFT;
> }
> +#endif
>
> - return va;
> + *va = avpn;
> + *psize = size;
> }
>
> /*
> @@ -374,8 +420,6 @@ static unsigned long slot2va(unsigned lo
> *
> * TODO: add batching support when enabled. remember, no dynamic memory here,
> * athough there is the control page available...
> - *
> - * XXX FIXME: 4k only for now !
> */
> static void native_hpte_clear(void)
> {
> @@ -383,6 +427,7 @@ static void native_hpte_clear(void)
> hpte_t *hptep = htab_address;
> unsigned long hpte_v;
> unsigned long pteg_count;
> + int psize;
>
> pteg_count = htab_hash_mask + 1;
>
> @@ -408,8 +453,9 @@ static void native_hpte_clear(void)
> * already hold the native_tlbie_lock.
> */
> if (hpte_v & HPTE_V_VALID) {
> + hpte_decode(hptep, slot, &psize, &hpte_v);
> hptep->v = 0;
> - __tlbie(slot2va(hpte_v, slot), MMU_PAGE_4K);
> + __tlbie(hpte_v, psize);
Using hpte_v as variable name is a bit misleading. avpn or va would be
a better variable name.
-Olof
^ permalink raw reply
* Re: [PATCH 1/6] Start split out of common open firmware code
From: Jon Loeliger @ 2007-04-24 19:14 UTC (permalink / raw)
To: Segher Boessenkool
Cc: linuxppc-dev@ozlabs.org, Paul Mackerras, David Miller, sfr
In-Reply-To: <c84b9df8fbe6f6a423cab8c3a4493409@kernel.crashing.org>
On Tue, 2007-04-24 at 13:18, Segher Boessenkool wrote:
> > I totally disagree that you should be changing anything
> > during these consolidations.
>
> And I didn't say so, I actually *said* I don't think
> that should be done.
>
> I was just remarking some really bad workarounds end
> up in more generic code now, and as such need to be
> fixed. Not now, but *later*.
>
> > Move the code around in one pass where you can _VERIFY_
> > that things are precisely the same and won't break.
> >
> > Then later you can get fancy and change things however you
> > wish.
>
> Yes exactly.
>
> > Everything so far is pure whining, and has nothing to do with
> > the substance of what Stephen is trying to accomplish, a
> > _CODE CONSOLIDATION_. So please don't get in the way of that
> > effort.
>
> I'm not standing in the way here, I'm quite happy with
> what Stephen is doing, great job.
>
> I'm just pointing out some bugs/workarounds in the
> merged code (that were there before, sure) -- it would
> be lovely if they at least could be commented as being
> not-quite-right in the new code so no one reading the
> code will assume it is doing the "right thing".
>
>
> Segher
And to be perfectly clear, I agreed with _Segher_
just as he outlines here.
Yes, it needs to be done. No, it does not happen
in this consolidation step. It happens in some
subsequent step that should not be forgotten.
jdl
^ permalink raw reply
* Re: [PATCH] generic check_legacy_ioport
From: Olaf Hering @ 2007-04-24 18:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20070424112530.GA11489@aepfle.de>
On Tue, Apr 24, Olaf Hering wrote:
> > And also check now if the parent is "isa". Just to be sure. If a
> > platform won't match that, then it will need it's own
> > check_legacy_ioport which is fine with me.
>
> Do you think a device_type fdc, i8042 or ipmi will appear outside an isa
> node?
Does anyone know where those ipmi devices appear in the device-tree?
^ permalink raw reply
* [PATCH] 64K page support for kexec
From: Luke Browning @ 2007-04-24 18:31 UTC (permalink / raw)
To: Arnd Bergmann, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, cbe-oss-dev
This patch fixes a couple of kexec problems related to 64K page
support in the kernel. kexec issues a tlbie for each pte. The
parameters for the tlbie are the page size and the virtual address.
Support was missing for the computation of these two parameters
for 64K pages. This patch adds that support.
Patch is updated from previous version to address Ben's comments
and to make it easier to add 16G page support in the future.
Signed-off-by: Luke Browning <lukebrowning@us.ibm.com>
Index: linux-2.6.21-rc4/arch/powerpc/mm/hash_native_64.c
===================================================================
--- linux-2.6.21-rc4.orig/arch/powerpc/mm/hash_native_64.c
+++ linux-2.6.21-rc4/arch/powerpc/mm/hash_native_64.c
@@ -340,31 +340,77 @@ static void native_hpte_invalidate(unsig
local_irq_restore(flags);
}
-/*
- * XXX This need fixing based on page size. It's only used by
- * native_hpte_clear() for now which needs fixing too so they
- * make a good pair...
- */
-static unsigned long slot2va(unsigned long hpte_v, unsigned long slot)
-{
- unsigned long avpn = HPTE_V_AVPN_VAL(hpte_v);
- unsigned long va;
-
- va = avpn << 23;
-
- if (! (hpte_v & HPTE_V_LARGE)) {
- unsigned long vpi, pteg;
-
- pteg = slot / HPTES_PER_GROUP;
- if (hpte_v & HPTE_V_SECONDARY)
- pteg = ~pteg;
+#define LP_SHIFT 12
+#define LP_BITS 8
+#define LP_MASK(i) ((((1 << LP_BITS) - 1) >> (i)) << LP_SHIFT)
+
+static void hpte_decode(hpte_t *hpte, unsigned long slot,
+ int *psize, unsigned long *va)
+{
+ unsigned long hpte_r = hpte->r;
+ unsigned long hpte_v = hpte->v;
+ unsigned long avpn;
+ int i, size, shift, penc, avpnm_bits;
+
+ if (!(hpte_v & HPTE_V_LARGE))
+ size = MMU_PAGE_4K;
+#if 0
+ else if (hpte_v & 0x4000000000000000UL)
+ size = MMU_PAGE_16G;
+#endif
+ else if (!(hpte_r & LP_MASK(0)))
+ size = MMU_PAGE_16M;
+ else {
+ for (i = 0; i < LP_BITS; i++) {
+ if ((hpte_r & LP_MASK(i+1)) == LP_MASK(i+1))
+ break;
+ }
+ penc = LP_MASK(i+1) >> LP_SHIFT;
+ for (size = MMU_PAGE_64K; size < MMU_PAGE_16M; size++) {
+ if (!mmu_psize_defs[size].shift)
+ continue;
+ if (penc == mmu_psize_defs[size].penc)
+ break;
+ }
+ }
- vpi = ((va >> 28) ^ pteg) & htab_hash_mask;
+ /*
+ * FIXME, this could be made more efficient by storing the type
+ * of hash algorithm in mmu_psize_defs[]. The code below assumes
+ * the number of bits in the va representing the offset in the
+ * page is less than 23. This affects the hash algorithm that is
+ * used. When 16G pages are supported, a new hash algorithm
+ * needs to be provided. See POWER ISA Book III.
+ *
+ * The code below works for 16M, 64K, and 4K pages.
+ */
+ shift = mmu_psize_defs[size].shift;
+ if (mmu_psize_defs[size].avpnm)
+ avpnm_bits = __ilog2_u64(mmu_psize_defs[size].avpnm) + 1;
+ else
+ avpnm_bits = 0;
+ if (shift - avpnm_bits <= 23) {
+ avpn = HPTE_V_AVPN_VAL(hpte_v) << 23;
+
+ if (shift < 23) {
+ unsigned long vpi, pteg;
+
+ pteg = slot / HPTES_PER_GROUP;
+ if (hpte_v & HPTE_V_SECONDARY)
+ pteg = ~pteg;
+ vpi = ((avpn >> 28) ^ pteg) & htab_hash_mask;
+ avpn |= (vpi << mmu_psize_defs[size].shift);
+ }
+ }
+#if 0
+ /* 16GB page hash, p > 23 */
+ else {
- va |= vpi << PAGE_SHIFT;
}
+#endif
- return va;
+ *va = avpn;
+ *psize = size;
}
/*
@@ -374,8 +420,6 @@ static unsigned long slot2va(unsigned lo
*
* TODO: add batching support when enabled. remember, no dynamic memory here,
* athough there is the control page available...
- *
- * XXX FIXME: 4k only for now !
*/
static void native_hpte_clear(void)
{
@@ -383,6 +427,7 @@ static void native_hpte_clear(void)
hpte_t *hptep = htab_address;
unsigned long hpte_v;
unsigned long pteg_count;
+ int psize;
pteg_count = htab_hash_mask + 1;
@@ -408,8 +453,9 @@ static void native_hpte_clear(void)
* already hold the native_tlbie_lock.
*/
if (hpte_v & HPTE_V_VALID) {
+ hpte_decode(hptep, slot, &psize, &hpte_v);
hptep->v = 0;
- __tlbie(slot2va(hpte_v, slot), MMU_PAGE_4K);
+ __tlbie(hpte_v, psize);
}
}
^ permalink raw reply
* Re: [PATCH] ib_core: Add missing device link to class device
From: Roland Dreier @ 2007-04-24 18:27 UTC (permalink / raw)
To: Joachim Fenkes; +Cc: LinuxPPC-Dev, LKML, OF-General
In-Reply-To: <OF8C8AA8EC.A66DAC49-ONC12572C7.00416B4B-C12572C7.00428AF7@de.ibm.com>
> I had a look at the kernel code -- currently, all device drivers except
> ehca do this by themselves:
> So I think it makes a lot of sense to put the class_dev.dev assignment
> into generic ib_core code instead of repeating it in all the drivers.
> The respective lines could move out of the drivers in the future but
> won't hurt anyone until then.
Actually I think we should delete the duplicate code now while merging
this. So I queued this up for 2.6.22:
commit f19c8d7cbe3153d68f0a559afd02f66655310238
Author: Joachim Fenkes <fenkes@de.ibm.com>
Date: Mon Apr 23 18:20:27 2007 +0200
IB: Set class_dev->dev in core for nice device symlink
All RDMA drivers except ehca set class_dev->dev to their dma_device
value (ehca leaves this unset). dma_device is the only value that
makes any sense, so move this assignment to core/sysfs.c. This reduce
the duplicated code in the rest of the drivers and gives ehca a nice
/sys/class/infiniband/ehcaX/device symlink.
Signed-off-by: Joachim Fenkes <fenkes@de.ibm.com>
Signed-off-by: Roland Dreier <rolandd@cisco.com>
diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
index 000c086..08c299e 100644
--- a/drivers/infiniband/core/sysfs.c
+++ b/drivers/infiniband/core/sysfs.c
@@ -683,6 +683,7 @@ int ib_device_register_sysfs(struct ib_device *device)
class_dev->class = &ib_class;
class_dev->class_data = device;
+ class_dev->dev = device->dma_device;
strlcpy(class_dev->class_id, device->name, BUS_ID_SIZE);
INIT_LIST_HEAD(&device->port_list);
diff --git a/drivers/infiniband/hw/amso1100/c2_provider.c b/drivers/infiniband/hw/amso1100/c2_provider.c
index fef9727..607c09b 100644
--- a/drivers/infiniband/hw/amso1100/c2_provider.c
+++ b/drivers/infiniband/hw/amso1100/c2_provider.c
@@ -796,7 +796,6 @@ int c2_register_device(struct c2_dev *dev)
memcpy(&dev->ibdev.node_guid, dev->pseudo_netdev->dev_addr, 6);
dev->ibdev.phys_port_cnt = 1;
dev->ibdev.dma_device = &dev->pcidev->dev;
- dev->ibdev.class_dev.dev = &dev->pcidev->dev;
dev->ibdev.query_device = c2_query_device;
dev->ibdev.query_port = c2_query_port;
dev->ibdev.modify_port = c2_modify_port;
diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
index 24e0df0..af28a31 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
@@ -1108,7 +1108,6 @@ int iwch_register_device(struct iwch_dev *dev)
memcpy(dev->ibdev.node_desc, IWCH_NODE_DESC, sizeof(IWCH_NODE_DESC));
dev->ibdev.phys_port_cnt = dev->rdev.port_info.nports;
dev->ibdev.dma_device = &(dev->rdev.rnic_info.pdev->dev);
- dev->ibdev.class_dev.dev = &(dev->rdev.rnic_info.pdev->dev);
dev->ibdev.query_device = iwch_query_device;
dev->ibdev.query_port = iwch_query_port;
dev->ibdev.modify_port = iwch_modify_port;
diff --git a/drivers/infiniband/hw/ipath/ipath_verbs.c b/drivers/infiniband/hw/ipath/ipath_verbs.c
index f5604b8..18c6df2 100644
--- a/drivers/infiniband/hw/ipath/ipath_verbs.c
+++ b/drivers/infiniband/hw/ipath/ipath_verbs.c
@@ -1559,7 +1559,6 @@ int ipath_register_ib_device(struct ipath_devdata *dd)
dev->node_type = RDMA_NODE_IB_CA;
dev->phys_port_cnt = 1;
dev->dma_device = &dd->pcidev->dev;
- dev->class_dev.dev = dev->dma_device;
dev->query_device = ipath_query_device;
dev->modify_device = ipath_modify_device;
dev->query_port = ipath_query_port;
diff --git a/drivers/infiniband/hw/mthca/mthca_provider.c b/drivers/infiniband/hw/mthca/mthca_provider.c
index 0725ad7..47e6fd4 100644
--- a/drivers/infiniband/hw/mthca/mthca_provider.c
+++ b/drivers/infiniband/hw/mthca/mthca_provider.c
@@ -1293,7 +1293,6 @@ int mthca_register_device(struct mthca_dev *dev)
dev->ib_dev.node_type = RDMA_NODE_IB_CA;
dev->ib_dev.phys_port_cnt = dev->limits.num_ports;
dev->ib_dev.dma_device = &dev->pdev->dev;
- dev->ib_dev.class_dev.dev = &dev->pdev->dev;
dev->ib_dev.query_device = mthca_query_device;
dev->ib_dev.query_port = mthca_query_port;
dev->ib_dev.modify_device = mthca_modify_device;
^ permalink raw reply related
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