* Re: [PATCH 0/4] PowerPC: implement GPIO API
From: Segher Boessenkool @ 2007-12-23 2:47 UTC (permalink / raw)
To: avorontsov; +Cc: linuxppc-dev
In-Reply-To: <20071221202824.GA4607@localhost.localdomain>
> OF device tree GPIOs bindings are similar to IRQs:
But GPIOs are a very different thing. Most importantly, the "number"
of a GPIO is completely local to the GPIO controller.
> pario0: gpio-controller@0 {
> #gpio-cells = <2>;
Your Linux code doesn't actually use this. Why is it needed, anyway?
You should be able to encode a GPIO identifier in a single 32-bit word,
for any possible GPIO controller.
> num-ports = <7>;
What is this? What is a "port"? This doesn't belong in a generic GPIO
binding.
> device@0 {
> gpios = <bank pin bank pin bank pin>;
> gpio-parent = <&pario0>;
Not every GPIO controller has banks. Not every device uses GPIOs
on a single GPIO controller. It is inconvenient to force all bindings
to use the same name ("gpios") for its property that shows the GPIOs
(and for it to have only one such property).
So I recommend:
-- Advise (in the generic GPIO binding) people to use
< phandle-of-gpio-controller gpio-id-on-that-controller >
to refer to a GPIO from some device node;
-- And either:
-- Define (in the generic GPIO binding) that a "gpio-id" is a single
32-bit cell;
or
-- Define (in the generic GPIO binding) that a "gpio-id" is a number
of 32-bit cells, and that that number of cells is encoded as a
32-bit
integer in the "#gpio-cells" property in the device node of the
respective GPIO controller.
(I like the first option better, unless someone can think of some
reasonable
situation where some specific GPIO controller binding needs more than
32 bits
to encode GPIO #).
Segher
^ permalink raw reply
* Re: [PATCH 1/4] [POWERPC] Implement GPIO API embryo
From: Segher Boessenkool @ 2007-12-23 2:49 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: linuxppc-dev
In-Reply-To: <20071221203115.GA4633@localhost.localdomain>
> +int __of_parse_gpio_bank_pin(struct device_node *np, int index,
> + int bank_width, int max_bank)
> +{
> + int bank;
> + int pin;
> + const u32 *gpios;
> +
> + /*
> + * We can get there only if of_get_gpio() succeeded, thus
> + * no need checking for "gpios" existence.
> + */
> + gpios = of_get_property(np, "gpios", NULL);
> + bank = gpios[index * 2];
> + pin = gpios[index * 2 + 1];
If you stick with the #gpio-cells plan, here is where you should use it.
Segher
^ permalink raw reply
* Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC
From: Timur Tabi @ 2007-12-23 2:58 UTC (permalink / raw)
To: Scott Wood; +Cc: Olof Johansson, linuxppc-dev, alsa-devel
In-Reply-To: <476AF015.5040600@freescale.com>
Scott Wood wrote:
>> None of the SOC nodes in any DTS have a "compatible" entry.
>
> Not quite true; ep88xc, mpc8272ads, and pq2fads have them.
Ah ok. So what should the compatible entry for 8641 be?
compatible = "fsl,mpc8641"
That looks a lot like what a compatible entry for the CPU should be.
How are we differentiating between the compatible cores and compatible SOCs?
^ permalink raw reply
* Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC
From: Timur Tabi @ 2007-12-23 3:23 UTC (permalink / raw)
To: Lee Revell; +Cc: Takashi Iwai, Olof Johansson, alsa-devel, linuxppc-dev
In-Reply-To: <75b66ecd0712202128n2f13adf2ie9a344e0c19d69ad@mail.gmail.com>
Lee Revell wrote:
> Please use DMA_32BIT_MASK (see include/linux/dma-mapping.h) instead of
> 0xffffffff.
No prob. But did you see this comment:
/*
* NOTE: do not use the below macros in new code and do not add new
definitions
* here.
*
* Instead, just open-code DMA_BIT_MASK(n) within your driver
*/
So I guess I should use DMA_BIT_MASK(32) instead.
> I've personally fixed a heisenbug in an ALSA driver
> caused by incorrectly typed DMA mask...
Can you explain to me what all of this does? Is it okay to use a static
u64 variable? Why do so many drivers do it that way? I don't even know
if 0xFFFFFFFF is the right number for my platform.
^ permalink raw reply
* Re: [PATCH] [POWERPC] MPC8360E-RDK: Device tree and board file
From: Timur Tabi @ 2007-12-23 3:25 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: linuxppc-dev
In-Reply-To: <20071221202309.GA4557@localhost.localdomain>
Anton Vorontsov wrote:
> 2. QE SCCs (slow UCCs, used as an UARTs)
I a posted a driver that provides this support, I'm just waiting for
Kumar to apply it.
What revision of the 8360 does this board use?
^ permalink raw reply
* Re: [PATCH] [POWERPC] MPC8360E-RDK: Device tree and board file
From: Timur Tabi @ 2007-12-23 3:28 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: linuxppc-dev
In-Reply-To: <20071221202309.GA4557@localhost.localdomain>
Anton Vorontsov wrote:
> + qe@e0100000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "fsl,qe";
> + ranges = <0 0xe0100000 0x00100000>;
> + reg = <0xe0100000 0x480>;
> + /* filled by u-boot */
> + brg-frequency = <0>;
You need "bus-frequency" here too
^ permalink raw reply
* [PATCH] Add information on enabling sound on the MPC8641 HPCN
From: Timur Tabi @ 2007-12-23 3:35 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Timur Tabi
Add a comment to the DTS file for the MPC8641 HPCN describing a wiring change
needed to get sound working on this board.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
For a two-line comment, I thought the DTS would be the best place to put this
information.
arch/powerpc/boot/dts/mpc8641_hpcn.dts | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/boot/dts/mpc8641_hpcn.dts b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
index 86fc228..08f78a6 100644
--- a/arch/powerpc/boot/dts/mpc8641_hpcn.dts
+++ b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
@@ -7,6 +7,9 @@
* under the terms of the GNU General Public License as published by the
* Free Software Foundation; either version 2 of the License, or (at your
* option) any later version.
+ *
+ * On some revisions of this board, C29 of the Uli M1575 must be connected
+ * to ground (e.g. ground of resistor R353) in order to enable sound.
*/
--
1.5.2.4
^ permalink raw reply related
* Re: [PATCH 0/4] PowerPC: implement GPIO API
From: David Gibson @ 2007-12-23 3:37 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <593200f4887c1fb73d9c9fb9b1db0b8c@kernel.crashing.org>
On Sun, Dec 23, 2007 at 03:47:50AM +0100, Segher Boessenkool wrote:
> > OF device tree GPIOs bindings are similar to IRQs:
>
> But GPIOs are a very different thing. Most importantly, the "number"
> of a GPIO is completely local to the GPIO controller.
Yes... just as interrupt specifiers are local to their interrupt
domain.
> > pario0: gpio-controller@0 {
> > #gpio-cells = <2>;
>
> Your Linux code doesn't actually use this. Why is it needed, anyway?
> You should be able to encode a GPIO identifier in a single 32-bit word,
> for any possible GPIO controller.
>
> > num-ports = <7>;
>
> What is this? What is a "port"? This doesn't belong in a generic GPIO
> binding.
>
> > device@0 {
> > gpios = <bank pin bank pin bank pin>;
> > gpio-parent = <&pario0>;
>
> Not every GPIO controller has banks.
That's just bad terminology in the example. "bank pin" means an
arbitrary format gpio specifier.
> Not every device uses GPIOs
> on a single GPIO controller. It is inconvenient to force all bindings
> to use the same name ("gpios") for its property that shows the GPIOs
> (and for it to have only one such property).
>
> So I recommend:
>
> -- Advise (in the generic GPIO binding) people to use
> < phandle-of-gpio-controller gpio-id-on-that-controller >
> to refer to a GPIO from some device node;
Ah, yes, that's a good point. Given the ugly workarounds we need to
deal with devices which have interrupts from multiple domains, we
don't want to copy that limitation to the GPIO scheme.
> -- And either:
> -- Define (in the generic GPIO binding) that a "gpio-id" is a single
> 32-bit cell;
> or
> -- Define (in the generic GPIO binding) that a "gpio-id" is a number
> of 32-bit cells, and that that number of cells is encoded as a
> 32-bit
> integer in the "#gpio-cells" property in the device node of the
> respective GPIO controller.
This option was the idea; the "bank pin" information has a format
local to the gpio controller. I agree the terminology needs to change
to "gpio specifier" by analogy with the interrupt tree, though.
> (I like the first option better, unless someone can think of some
> reasonable
> situation where some specific GPIO controller binding needs more than
> 32 bits
> to encode GPIO #).
I can't think of a situation where it would be strictly speaking
necessary, but I can think of several where it would be more
convenient. GPIO controllers that do have a bank/pin arrangement is
one. GPIO controllers than have a pin number, plus some sort of
direction or level information is another.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [PATCH 1/4] [POWERPC] Implement GPIO API embryo
From: David Gibson @ 2007-12-23 3:40 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <b632798b4f5523653160b6a7156266e1@kernel.crashing.org>
On Sun, Dec 23, 2007 at 03:49:30AM +0100, Segher Boessenkool wrote:
> > +int __of_parse_gpio_bank_pin(struct device_node *np, int index,
> > + int bank_width, int max_bank)
> > +{
> > + int bank;
> > + int pin;
> > + const u32 *gpios;
> > +
> > + /*
> > + * We can get there only if of_get_gpio() succeeded, thus
> > + * no need checking for "gpios" existence.
> > + */
> > + gpios = of_get_property(np, "gpios", NULL);
> > + bank = gpios[index * 2];
> > + pin = gpios[index * 2 + 1];
>
> If you stick with the #gpio-cells plan, here is where you should use it.
I think part of what's happening here is due to the patch's history.
The "bank pin" information was always of a format local to the
controller, but originally the size wasn't explicitly stated in the
tree - it was just implicit in the type of gpio controller. I
suggested that #gpio-cells be added, which it has been, but it looks
like the code hasn't been updated to use it everywhere. Obviously a
driver for a particular gpio controller would generally need to assert
that the controller's #gpio-cells has the correct value for this
controller type, after which code like the above would be acceptable.
--
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 17/21] [POWERPC] Base support for 440SPe "Katmai" eval board
From: Stephen Rothwell @ 2007-12-23 3:42 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev
In-Reply-To: <20071221194828.3a6e6299@vader.jdub.homelinux.org>
[-- Attachment #1: Type: text/plain, Size: 732 bytes --]
On Fri, 21 Dec 2007 19:48:28 -0600 Josh Boyer <jwboyer@gmail.com> wrote:
>
> On Sat, 22 Dec 2007 11:21:05 +1100
> Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> > On Fri, 21 Dec 2007 15:39:34 +1100 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> > >
> > > +++ linux-merge/arch/powerpc/platforms/44x/katmai.c 2007-12-21 14:34:33.000000000 +1100
> > > +
> > > +static struct of_device_id katmai_of_bus[] = {
> >
> > __initdata (preferably) or const, please.
>
> I'll fix this with a separate commit. The same comment applies for all
> the 4xx platforms already in the kernel.
Fine by me.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [Cbe-oss-dev] Time for cell code reshuffle?
From: Jeremy Kerr @ 2007-12-23 3:47 UTC (permalink / raw)
To: cbe-oss-dev; +Cc: linuxppc-dev, Arnd Bergmann
In-Reply-To: <200712211522.03841.arnd@arndb.de>
Hi all,
> To the question, where what it should go, I'd leave the decision to
> Jeremy, but my current idea would be:
>
> arch/powerpc/platforms/cell/spufs -> arch/powerpc/spufs
I'd suggest arch/powerpc/sysdev/spufs to keep arch/powerpc clean.
However, this may also depend on the (intended) structure of SPURSEngine
support, which may mean that fs/spufs might be a better place. It would
suck to have to move things twice, so maybe someone from Tosihba could
provide some input? Would the powerpc spufs code be suitable for
SPURSEngine?
As thers have posted earlier, i gthink that arch/powerpc/sysdev/cell
would be good for the generic cell support, then we could have
arch/powerpc/platforms/{cell,ps3,beat} containing only the individual
platform code.
Cheers,
Jeremy
^ permalink raw reply
* Re: [patch v2] PS3: Fix printing of os-area magic numbers
From: Geoff Levand @ 2007-12-23 4:54 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Geert Uytterhoeven, linuxppc-dev@ozlabs.org
In-Reply-To: <18283.17563.790043.79740@cargo.ozlabs.ibm.com>
On 12/20/2007 08:44 PM, Paul Mackerras wrote:
> Geoff Levand writes:
>
>> Fix a bug in the printing of the os-area magic numbers which assumed that
>> magic numbers were zero terminated strings. The magic numbers are represented
>> in memory as integers. If the os-area sections are not initialized correctly
>> they could contained random data that would be printed to the display.
>
>> + u8 str[sizeof(h->magic_num) + 1];
>> + u8 *s, *d;
>> +
>> + for(s = h->magic_num, d = str; s < h->magic_num + sizeof(h->magic_num);
>> + s++, d++) {
>> + *d = isprint(*s) ? *s : '.';
>> + }
>> + d[sizeof(h->magic_num)] = 0;
>
> This last statement is wrong, because d has been incremented to point
> to the last byte of str already by this stage.
>
> It would be nicer if you pulled out the two instances of the for loop
> into a little helper function.
OK.
>> + for(s = (u8*)&db->magic_num, d = str;
>
> Why do you need the (u8*) cast in this case but not the other?
The types are different. The header magic is an array of u8, the db magic
is a u32.
-Geoff
^ permalink raw reply
* [patch v3] PS3: Fix printing of os-area magic numbers
From: Geoff Levand @ 2007-12-23 5:09 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Geert Uytterhoeven, linuxppc-dev@ozlabs.org
In-Reply-To: <473F6A28.5000309@am.sony.com>
Fix a bug in the printing of the os-area magic numbers which assumed that
magic numbers were zero terminated strings. The magic numbers are represented
in memory as integers. If the os-area sections are not initialized correctly
they could contained random data that would be printed to the display.
Also unify the handling of header and db magic numbers and make both
of type array of u8.
CC: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
---
v2: o handle unprintable chars.
v3: o breakout string dump into helper dump_field()
o unify handling of header and db magic numbers
arch/powerpc/platforms/ps3/os-area.c | 40 ++++++++++++++++++++++++++---------
1 file changed, 30 insertions(+), 10 deletions(-)
--- a/arch/powerpc/platforms/ps3/os-area.c
+++ b/arch/powerpc/platforms/ps3/os-area.c
@@ -23,6 +23,7 @@
#include <linux/workqueue.h>
#include <linux/fs.h>
#include <linux/syscalls.h>
+#include <linux/ctype.h>
#include <asm/lmb.h>
@@ -37,6 +38,8 @@ enum os_area_ldr_format {
HEADER_LDR_FORMAT_GZIP = 1,
};
+#define OS_AREA_HEADER_MAGIC_NUM "cell_ext_os_area"
+
/**
* struct os_area_header - os area header segment.
* @magic_num: Always 'cell_ext_os_area'.
@@ -114,13 +117,11 @@ struct os_area_params {
u8 _reserved_5[8];
};
-enum {
- OS_AREA_DB_MAGIC_NUM = 0x2d64622dU,
-};
+#define OS_AREA_DB_MAGIC_NUM "-db-"
/**
* struct os_area_db - Shared flash memory database.
- * @magic_num: Always '-db-' = 0x2d64622d.
+ * @magic_num: Always '-db-'.
* @version: os_area_db format version number.
* @index_64: byte offset of the database id index for 64 bit variables.
* @count_64: number of usable 64 bit index entries
@@ -135,7 +136,7 @@ enum {
*/
struct os_area_db {
- u32 magic_num;
+ u8 magic_num[4];
u16 version;
u16 _reserved_1;
u16 index_64;
@@ -265,12 +266,26 @@ static void __init os_area_get_property(
prop->name);
}
+static void dump_field(char *s, const u8 *f, unsigned int size)
+{
+#if defined(DEBUG)
+ unsigned int i;
+
+ for (i = 0; i < size; i++)
+ s[i] = isprint(f[i]) ? f[i] : '.';
+ s[i] = 0;
+#endif
+}
+
#define dump_header(_a) _dump_header(_a, __func__, __LINE__)
static void _dump_header(const struct os_area_header *h, const char *func,
int line)
{
+ char str[sizeof(h->magic_num) + 1];
+
+ dump_field(str, h->magic_num, sizeof(h->magic_num));
pr_debug("%s:%d: h.magic_num: '%s'\n", func, line,
- h->magic_num);
+ str);
pr_debug("%s:%d: h.hdr_version: %u\n", func, line,
h->hdr_version);
pr_debug("%s:%d: h.db_area_offset: %u\n", func, line,
@@ -311,7 +326,8 @@ static void _dump_params(const struct os
static int verify_header(const struct os_area_header *header)
{
- if (memcmp(header->magic_num, "cell_ext_os_area", 16)) {
+ if (memcmp(header->magic_num, OS_AREA_HEADER_MAGIC_NUM,
+ sizeof(header->magic_num))) {
pr_debug("%s:%d magic_num failed\n", __func__, __LINE__);
return -1;
}
@@ -331,7 +347,8 @@ static int verify_header(const struct os
static int db_verify(const struct os_area_db *db)
{
- if (db->magic_num != OS_AREA_DB_MAGIC_NUM) {
+ if (memcmp(db->magic_num, OS_AREA_DB_MAGIC_NUM,
+ sizeof(db->magic_num))) {
pr_debug("%s:%d magic_num failed\n", __func__, __LINE__);
return -1;
}
@@ -484,8 +501,11 @@ static int db_get_rtc_diff(const struct
static void _dump_db(const struct os_area_db *db, const char *func,
int line)
{
+ char str[sizeof(db->magic_num) + 1];
+
+ dump_field(str, db->magic_num, sizeof(db->magic_num));
pr_debug("%s:%d: db.magic_num: '%s'\n", func, line,
- (const char*)&db->magic_num);
+ str);
pr_debug("%s:%d: db.version: %u\n", func, line,
db->version);
pr_debug("%s:%d: db.index_64: %u\n", func, line,
@@ -516,7 +536,7 @@ static void os_area_db_init(struct os_ar
memset(db, 0, sizeof(struct os_area_db));
- db->magic_num = OS_AREA_DB_MAGIC_NUM;
+ memcpy(db->magic_num, OS_AREA_DB_MAGIC_NUM, sizeof(db->magic_num));
db->version = 1;
db->index_64 = HEADER_SIZE;
db->count_64 = VALUES_64_COUNT;
^ permalink raw reply
* Re: Yet more patches added to for-2.6.25/master branches
From: Michael Neuling @ 2007-12-23 7:23 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <18283.44454.618519.155696@cargo.ozlabs.ibm.com>
Paulus,
Can you take the crash shutdown hook patches as well for 2.6.25?
http://patchwork.ozlabs.org/linuxppc/patch?person=414&id=15546
http://patchwork.ozlabs.org/linuxppc/patch?person=414&id=15547
Mikey
In message <18283.44454.618519.155696@cargo.ozlabs.ibm.com> you wrote:
> Aegis Lin (1):
> [POWERPC] spufs: Use separate timer for /proc/spu_loadavg calculation
>
> Andre Detsch (1):
> [POWERPC] spufs: DMA Restart after SIGSEGV
>
> Arnd Bergmann (1):
> [POWERPC] spufs: block fault handlers in spu_acquire_runnable
>
> Benjamin Herrenschmidt (2):
> [POWERPC] Fix for via-pmu based backlight control
> [POWERPC] Fix possible NULL deref in ppc32 PCI
>
> Christoph Hellwig (2):
> [POWERPC] spufs: add enchanced simple attr macros
> [POWERPC] spufs: make state_mutex interruptible
>
> Emil Medve (1):
> [POWERPC] Optimize counting distinct entries in the relocation sections
>
> Jeremy Kerr (5):
> [POWERPC] spufs: move fault, lscsa_alloc and switch code to spufs modul
e
> [POWERPC] spufs: fix incorrect interrupt status clearing in backing mbo
x stat poll
> [POWERPC] spufs: use #defines for SPU class [012] exception status
> [POWERPC] spufs: rework class 0 and 1 interrupt handling
> [POWERPC] spufs: Don't leak kernel stack through an empty {i,m}box_info
read
>
> Julio M. Merino Vidal (1):
> [POWERPC] spufs: fix typos in sched.c comments
>
> Luke Browning (4):
> [POWERPC] spufs: add backing ops for privcntl register
> [POWERPC] spufs: reorganize spu_run_init
> [POWERPC] spufs: spu_find_victim may choose wrong victim
> [POWERPC] spufs: decouple spu scheduler from spufs_spu_run (asynchronou
s scheduling)
>
> Masato Noguchi (2):
> [POWERPC] cell: wrap master run control bit
> [POWERPC] spufs: don't set reserved bits in spu interrupt status
>
> Scott Wood (1):
> [POWERPC] Implement arch disable/enable irq hooks.
>
> Stephen Rothwell (5):
> [POWERPC] Add EHEA and EHCA as modules in the ppc64_defconfig
> [POWERPC] The builtin matches for ibmebus.c can be __initdata
> [POWERPC] Constify the of_device_id passed to of_platform_bus_probe
> [POWERPC] Pointers marked as __iomem do not need to be volatile
> [POWERPC] Make non-PCI build work again
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
^ permalink raw reply
* Re: [PATCH/RFC] powerpc: DBox2 Board Support
From: Stephen Rothwell @ 2007-12-23 8:18 UTC (permalink / raw)
To: Jochen Friedrich; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <476D61DB.2090201@scram.de>
[-- Attachment #1: Type: text/plain, Size: 992 bytes --]
On Sat, 22 Dec 2007 20:13:31 +0100 Jochen Friedrich <jochen@scram.de> wrote:
>
> +++ b/arch/powerpc/platforms/8xx/dbox2.h
> +
> +#include <sysdev/fsl_soc.h>
Nothing in this header files uses anything in that one, so don't include
it.
> +++ b/arch/powerpc/platforms/8xx/dbox2_setup.c
> +
> +char *dbox2_manuf_name[3] = {
static const.
> + "Nokia",
> + "Philips",
> + "Sagem",
> +};
> +
> +static enum dbox2_mid dbox2_manuf_id;
> +
> +struct cpm_pin {
> + int port, pin, flags;
> +};
> +
> +static struct cpm_pin dbox2_pins[] = {
const?
> +static int __init dbox2_probe(void)
> +{
> + unsigned long root = of_get_flat_dt_root();
> + return of_flat_dt_is_compatible(root, "betaresearch,dbox2");
You should include asm/prom.h to use the flattened device tree function.
> +static struct of_device_id __initdata of_bus_ids[] = {
__initdata
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [patch v3] PS3: Fix printing of os-area magic numbers
From: Geert Uytterhoeven @ 2007-12-23 9:51 UTC (permalink / raw)
To: Geoff Levand; +Cc: linuxppc-dev@ozlabs.org, Paul Mackerras
In-Reply-To: <476DED97.8070709@am.sony.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1062 bytes --]
On Sat, 22 Dec 2007, Geoff Levand wrote:
> +static void dump_field(char *s, const u8 *f, unsigned int size)
> +{
> +#if defined(DEBUG)
> + unsigned int i;
> +
> + for (i = 0; i < size; i++)
> + s[i] = isprint(f[i]) ? f[i] : '.';
> + s[i] = 0;
> +#endif
> +}
Sorry for nitpicking again.
Usually the _length_ of a C-string is the number of characters, while the
_size_ of a C-string includes the zero-terminator.
In dump_field() it writes size+1 bytes to s.
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply
* Re: [PATCH 0/4] PowerPC: implement GPIO API
From: Segher Boessenkool @ 2007-12-23 9:53 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20071223033724.GB10699@localhost.localdomain>
>>> OF device tree GPIOs bindings are similar to IRQs:
>>
>> But GPIOs are a very different thing. Most importantly, the "number"
>> of a GPIO is completely local to the GPIO controller.
>
> Yes... just as interrupt specifiers are local to their interrupt
> domain.
Sure, but an interrupt domain can be (and usually is) more than
one device node. A GPIO controller is just one node.
>>> device@0 {
>>> gpios = <bank pin bank pin bank pin>;
>>> gpio-parent = <&pario0>;
>>
>> Not every GPIO controller has banks.
>
> That's just bad terminology in the example. "bank pin" means an
> arbitrary format gpio specifier.
Okay. Don't split it into two things then, just say "gpio-spec"
or such.
>> Not every device uses GPIOs
>> on a single GPIO controller. It is inconvenient to force all bindings
>> to use the same name ("gpios") for its property that shows the GPIOs
>> (and for it to have only one such property).
>>
>> So I recommend:
>>
>> -- Advise (in the generic GPIO binding) people to use
>> < phandle-of-gpio-controller gpio-id-on-that-controller >
>> to refer to a GPIO from some device node;
>
> Ah, yes, that's a good point. Given the ugly workarounds we need to
> deal with devices which have interrupts from multiple domains, we
> don't want to copy that limitation to the GPIO scheme.
Yeah, and we even know for a fact that devices exist that do GPIOs
on multiple GPIO controllers. In the interrupt case, no one thought
anyone would be crazy enough to route their IRQs like that :-)
>> -- Define (in the generic GPIO binding) that a "gpio-id" is a number
>> of 32-bit cells, and that that number of cells is encoded as a
>> 32-bit
>> integer in the "#gpio-cells" property in the device node of the
>> respective GPIO controller.
>
> This option was the idea; the "bank pin" information has a format
> local to the gpio controller. I agree the terminology needs to change
> to "gpio specifier" by analogy with the interrupt tree, though.
Right, with that cleared up, and the binding doc expanded a bit,
you won't hear complaints from me :-)
>> (I like the first option better, unless someone can think of some
>> reasonable
>> situation where some specific GPIO controller binding needs more than
>> 32 bits
>> to encode GPIO #).
>
> I can't think of a situation where it would be strictly speaking
> necessary, but I can think of several where it would be more
> convenient. GPIO controllers that do have a bank/pin arrangement is
> one. GPIO controllers than have a pin number, plus some sort of
> direction or level information is another.
Ah yes, a second word for pin "type" information makes a lot of sense.
#gpio-cells it is, then. Let's please make sure that we put that "type"
thing in the documentation (as an example), and that the first
controller
bindings we put in use it.
Segher
^ permalink raw reply
* Re: [PATCH 1/4] [POWERPC] Implement GPIO API embryo
From: Segher Boessenkool @ 2007-12-23 10:00 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20071223034046.GC10699@localhost.localdomain>
> I think part of what's happening here is due to the patch's history.
That doesn't make it right though ;-)
> Obviously a
> driver for a particular gpio controller would generally need to assert
> that the controller's #gpio-cells has the correct value for this
> controller type, after which code like the above would be acceptable.
Well to nitpick... _a_ correct value. But sure, if this is device-
specific code, it is okay. OTOH, this really should be a helper
function that each GPIO controller driver can use.
Segher
^ permalink raw reply
* Re: [PATCH 0/4] PowerPC: implement GPIO API
From: Anton Vorontsov @ 2007-12-23 11:47 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, David Gibson
In-Reply-To: <77aab6d2233de8687320e1827ff695c2@kernel.crashing.org>
On Sun, Dec 23, 2007 at 10:53:05AM +0100, Segher Boessenkool wrote:
[...]
> >>> device@0 {
> >>> gpios = <bank pin bank pin bank pin>;
> >>> gpio-parent = <&pario0>;
> >>
> >> Not every GPIO controller has banks.
> >
> > That's just bad terminology in the example. "bank pin" means an
> > arbitrary format gpio specifier.
>
> Okay. Don't split it into two things then, just say "gpio-spec"
> or such.
Will do. "bank pin" was just an example of QE/CPMs gpio specifiers, they
could be arbitrary in general.
> >> Not every device uses GPIOs
> >> on a single GPIO controller. It is inconvenient to force all bindings
> >> to use the same name ("gpios") for its property that shows the GPIOs
> >> (and for it to have only one such property).
> >>
> >> So I recommend:
> >>
> >> -- Advise (in the generic GPIO binding) people to use
> >> < phandle-of-gpio-controller gpio-id-on-that-controller >
> >> to refer to a GPIO from some device node;
> >
> > Ah, yes, that's a good point. Given the ugly workarounds we need to
> > deal with devices which have interrupts from multiple domains, we
> > don't want to copy that limitation to the GPIO scheme.
>
> Yeah, and we even know for a fact that devices exist that do GPIOs
> on multiple GPIO controllers. In the interrupt case, no one thought
> anyone would be crazy enough to route their IRQs like that :-)
Oh, <&gpio-controller-phandle gpio-spec> is indeed better
scheme. Well, parsing it would be a bit more complicated, as gpio-spec
is variable length: next placement of phandle depends on previous
gpio-specs... But this is doable of course.
> >> -- Define (in the generic GPIO binding) that a "gpio-id" is a number
> >> of 32-bit cells, and that that number of cells is encoded as a
> >> 32-bit
> >> integer in the "#gpio-cells" property in the device node of the
> >> respective GPIO controller.
> >
> > This option was the idea; the "bank pin" information has a format
> > local to the gpio controller. I agree the terminology needs to change
> > to "gpio specifier" by analogy with the interrupt tree, though.
>
> Right, with that cleared up, and the binding doc expanded a bit,
> you won't hear complaints from me :-)
Ok, I should write documentation indeed. ;-)
> >> (I like the first option better, unless someone can think of some
> >> reasonable
> >> situation where some specific GPIO controller binding needs more than
> >> 32 bits
> >> to encode GPIO #).
> >
> > I can't think of a situation where it would be strictly speaking
> > necessary, but I can think of several where it would be more
> > convenient. GPIO controllers that do have a bank/pin arrangement is
> > one. GPIO controllers than have a pin number, plus some sort of
> > direction or level information is another.
>
> Ah yes, a second word for pin "type" information makes a lot of sense.
> #gpio-cells it is, then. Let's please make sure that we put that "type"
> thing in the documentation (as an example), and that the first
> controller
> bindings we put in use it.
There is no limitation to define gpio direction in the gpio-spec, but
the thing is: we're passing gpios to the drivers which are already
know in what direction gpio should be set up, and we have an API to
set up GPIOs.
Example: fsl_nand, we're passing gpio, and driver is doing
gpio_direction_output() call on it. So we don't have to pass
gpio direction information in the gpio specifier.
As for level, yes this is important information, and encoding it
into gpio-spec seems reasonable (in fsl_nand example, ready-not-busy
(rnb) gpio. GPIO could be wired to be !rnb or just rnb).
Thanks!
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH 1/3] [POWERPC] FSL UPM: routines to manage FSL UPMs
From: Anton Vorontsov @ 2007-12-23 11:59 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: linuxppc-dev
In-Reply-To: <20071223132457.a40e8100.sfr@canb.auug.org.au>
On Sun, Dec 23, 2007 at 01:24:57PM +1100, Stephen Rothwell wrote:
> On Fri, 21 Dec 2007 23:39:25 +0300 Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> >
> > +int fsl_upm_get_for(struct device_node *node, const char *name,
> > + struct fsl_upm *upm)
> > +{
> > + int ret;
> > + struct device_node *lbus;
> > + struct resource lbc_res;
> > + ptrdiff_t mxmr_offs;
> > +
> > + lbus = of_get_parent(node);
> > + if (!lbus) {
> > + pr_err("FSL UPM: can't get parent local bus node\n");
> > + return -ENOENT;
> > + }
> > +
> > + ret = of_address_to_resource(lbus, 0, &lbc_res);
>
> of_node_put(lbus) as of_get_parent() gets a reference.
Thanks, will fix.
> > +static inline void fsl_upm_start_pattern(struct fsl_upm *upm, u32 pat_offset)
> > +{
> > + spin_lock_irqsave(&upm_lock, upm_lock_flags);
>
> I may be wrong, but don't we need the "flags" argument to
> spin_lock_irqsave to be on the stack? And the save and restore to be in
> the same function?
In general case, yes. Here, not exactly. We have to grab a lock at the
start(), do runs(), and release a lock at the end():
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH 2/3] [POWERPC][NAND] FSL UPM NAND driver
From: Anton Vorontsov @ 2007-12-23 12:02 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: linuxppc-dev, linux-mtd
In-Reply-To: <20071223133329.9d2c44e4.sfr@canb.auug.org.au>
On Sun, Dec 23, 2007 at 01:33:29PM +1100, Stephen Rothwell wrote:
> On Fri, 21 Dec 2007 23:41:30 +0300 Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> >
> > +static int __devinit upm_chip_probe(struct of_device *ofdev,
> > + const struct of_device_id *ofid)
> > +{
> > + struct upm_data *ud;
> > + struct resource io_res;
> > + const u32 *prop;
> > + int ret;
> > + int size;
> > +
> > + ud = kzalloc(sizeof(*ud), GFP_KERNEL);
> > + if (!ud)
> > + return -ENOMEM;
> > +
> > + ret = of_address_to_resource(ofdev->node, 0, &io_res);
> > + if (ret) {
> > + dev_err(&ofdev->dev, "can't get IO base\n");
> > + goto err;
> > + }
> > +
> > + prop = of_get_property(ofdev->node, "width", &size);
> > + if (!prop || size != sizeof(u32)) {
> > + dev_err(&ofdev->dev, "can't get chip width\n");
> > + goto err;
>
> Here ret is 0, is that the correct return code?
Nope.
> > + ud->rnb_gpio = of_get_gpio(ofdev->node, 0);
> > + if (ud->rnb_gpio >= 0) {
> > + ret = gpio_request(ud->rnb_gpio, ofdev->dev.bus_id);
> > + if (ret) {
> > + dev_err(&ofdev->dev, "can't request RNB gpio\n");
> > + goto err;
> > + }
> > + gpio_direction_input(ud->rnb_gpio);
> > + } else if (ud->rnb_gpio == -EINVAL) {
> > + dev_err(&ofdev->dev, "specified RNB gpio is invalid\n");
> > + goto err;
>
> Again ret is 0 here.
Much thanks for catching that!
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH 1/3] [POWERPC] FSL UPM: routines to manage FSL UPMs
From: Anton Vorontsov @ 2007-12-23 12:17 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: linuxppc-dev
In-Reply-To: <20071223115935.GB5643@zarina>
On Sun, Dec 23, 2007 at 02:59:35PM +0300, Anton Vorontsov wrote:
[..]
> > > +static inline void fsl_upm_start_pattern(struct fsl_upm *upm, u32 pat_offset)
> > > +{
> > > + spin_lock_irqsave(&upm_lock, upm_lock_flags);
> >
> > I may be wrong, but don't we need the "flags" argument to
> > spin_lock_irqsave to be on the stack? And the save and restore to be in
> > the same function?
>
> In general case, yes. Here, not exactly. We have to grab a lock at the
> start(), do runs(), and release a lock at the end():
Ugh, that's stupid of course. flags are indeed should be on the stack.
So, what I can use here is a mutex, and thus forbid using these
routines from the isrs. Another option would be disabling interrupts
and getting plain lock, but that is ugly. So will use a mutex.
Much thanks,
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [Cbe-oss-dev] Time for cell code reshuffle?
From: Christoph Hellwig @ 2007-12-23 12:35 UTC (permalink / raw)
To: Jeremy Kerr; +Cc: linuxppc-dev, cbe-oss-dev, Arnd Bergmann
In-Reply-To: <200712231247.44154.jk@ozlabs.org>
On Sun, Dec 23, 2007 at 12:47:42PM +0900, Jeremy Kerr wrote:
> Hi all,
>
> > To the question, where what it should go, I'd leave the decision to
> > Jeremy, but my current idea would be:
> >
> > arch/powerpc/platforms/cell/spufs -> arch/powerpc/spufs
>
> I'd suggest arch/powerpc/sysdev/spufs to keep arch/powerpc clean.
>
> However, this may also depend on the (intended) structure of SPURSEngine
> support, which may mean that fs/spufs might be a better place. It would
> suck to have to move things twice, so maybe someone from Tosihba could
> provide some input? Would the powerpc spufs code be suitable for
> SPURSEngine?
Strong NACK for fs/spufs/ fs code is the smallest part of the spu
support, and all this nasty lowlevel code doesn't belong into fs/ at
all.
^ permalink raw reply
* Re: [PATCH 1/3] [POWERPC] FSL UPM: routines to manage FSL UPMs
From: Anton Vorontsov @ 2007-12-23 12:25 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: linuxppc-dev
In-Reply-To: <20071223121735.GA5831@zarina>
On Sun, Dec 23, 2007 at 03:17:35PM +0300, Anton Vorontsov wrote:
> On Sun, Dec 23, 2007 at 02:59:35PM +0300, Anton Vorontsov wrote:
> [..]
> > > > +static inline void fsl_upm_start_pattern(struct fsl_upm *upm, u32 pat_offset)
> > > > +{
> > > > + spin_lock_irqsave(&upm_lock, upm_lock_flags);
> > >
> > > I may be wrong, but don't we need the "flags" argument to
> > > spin_lock_irqsave to be on the stack? And the save and restore to be in
> > > the same function?
> >
> > In general case, yes. Here, not exactly. We have to grab a lock at the
> > start(), do runs(), and release a lock at the end():
>
> Ugh, that's stupid of course. flags are indeed should be on the stack.
> So, what I can use here is a mutex, and thus forbid using these
> routines from the isrs. Another option would be disabling interrupts
> and getting plain lock, but that is ugly. So will use a mutex.
Ignore me please, I should had more sleep today. For God's sake,
why I've just said a "mutex"?.. I should just get a plain lock and
forget about isrs.
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [Cbe-oss-dev] Time for cell code reshuffle?
From: Geert Uytterhoeven @ 2007-12-23 14:06 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linuxppc-dev, Jeremy Kerr, Arnd Bergmann, cbe-oss-dev
In-Reply-To: <20071223123501.GA19823@lst.de>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1460 bytes --]
On Sun, 23 Dec 2007, Christoph Hellwig wrote:
> On Sun, Dec 23, 2007 at 12:47:42PM +0900, Jeremy Kerr wrote:
> > > To the question, where what it should go, I'd leave the decision to
> > > Jeremy, but my current idea would be:
> > >
> > > arch/powerpc/platforms/cell/spufs -> arch/powerpc/spufs
> >
> > I'd suggest arch/powerpc/sysdev/spufs to keep arch/powerpc clean.
> >
> > However, this may also depend on the (intended) structure of SPURSEngine
> > support, which may mean that fs/spufs might be a better place. It would
> > suck to have to move things twice, so maybe someone from Tosihba could
> > provide some input? Would the powerpc spufs code be suitable for
> > SPURSEngine?
>
> Strong NACK for fs/spufs/ fs code is the smallest part of the spu
> support, and all this nasty lowlevel code doesn't belong into fs/ at
> all.
drivers/spu/?
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ 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