* Re: Apparent kernel bug with GDB on ppc405
From: Benjamin Herrenschmidt @ 2007-10-26 3:23 UTC (permalink / raw)
To: Grant Likely; +Cc: gdb, linuxppc-embedded, Matt Mackall
In-Reply-To: <fa686aa40710251945l51604c5vdf0b49b03b9a1a57@mail.gmail.com>
> This is actually 405. Does that have the same issue?
hrm... I don't remember :-) There -is- something fishy about the icache
on 405 but I don't remember for sure. Try sticking an iccci in there and
tell us if that helps.
Ben.
^ permalink raw reply
* [0/2] A counter-proposal for the literals transition
From: David Gibson @ 2007-10-26 6:17 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev
So I've been doing some thinking and coding today on the dts-versioning
/ new-style literals issue. Here's a set of two patches which need a
little polish, but I think demonstrate a better way to handle the
transition; and a start on some not-directly-related improvements to the
parser design. Some rationales:
- As I've been saying, I think *any* passing of data from the
parser back to the lexer to control its behaviour is asking for (at the
least) very confusing flow of control.
- property/node names suck. To cover the various existing
things in Apple/IBM firmwares, they have to allow all sorts of nasty
punctuation (e.g. +*?-) which could cause great trouble with expressions
later on.
- therefore, I think I was thinking arse-backwards when I came
up with the lexer states for CELLDATA and MEMRESERVE. Instead
recognizing sensible literals and sensible identifiers should be the
normal lexer behaviour, and recognizing the weird the prop/node names
should be the special lexer state.
- I was always a bit dubious about the very-missible visual
difference between
/memreserve/ 0x80000 0xf0000;
and
/memreserve/ 0x80000-0xf0000;
Plus the "range" form doesn't seem to have been used by any in-kernel
dts files. Therefore, rather than changing the range symbol, simply
drop support for the range form in the new version.
- Rather than making lexer rules be narrow, so that only valid
symbols will be lexed, it's actually better to make the lexer rules
broad (not conflicting with other valid things, obviously), then verify
that the returned tokens are really valid further up the stack. This
way we can generate a meaningful "bad symbol/identifier/name/literal"
message, instead of simply getting "syntax error" - or worse, splitting
the bogus token into multiple valid tokens which we manage to parse far
enough to get a thoroughly confusing state. (Imagine if a mistype like
<0xdeadgeef>, parsed as a 2-cell value, <0xdead> then <geef>, with geef
assumed to be a variable/identifier. Or if <077877> was parsed as
octal 077 then 877 decimal.)
Some things that I know need polish in these patches:
- Currently if we get badly formatted literals, we print an
error message with yyerror(), but parsing continues using I'm not sure
quite what crazy assumed value for the literal.
- -Odts output is still v0.
--
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
* [1/2] dtc: Simplify lexing/parsing of literals vs. node/property names
From: David Gibson @ 2007-10-26 6:19 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev
In-Reply-To: <20071026061718.GA6136@localhost.localdomain>
The current scheme of having CELLDATA and MEMRESERVE states to
recognize hex literals instead of node or property names is
arse-backwards. The patch switches things around so that literals are
lexed in normal states, and property/node names are only recognized in
the special PROPNODENAME state, which is only entered after a { or a
;, and is left as soon as we scan a property/node name or a keyword.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
dtc-lexer.l | 105 +++++++++++---------------------------------------
dtc-parser.y | 123 ++++++++++++++++++++++++++---------------------------------
2 files changed, 80 insertions(+), 148 deletions(-)
Index: dtc/dtc-lexer.l
===================================================================
--- dtc.orig/dtc-lexer.l 2007-10-26 12:54:30.000000000 +1000
+++ dtc/dtc-lexer.l 2007-10-26 15:11:02.000000000 +1000
@@ -21,9 +21,8 @@
%option noyywrap nounput yylineno
%x INCLUDE
-%x CELLDATA
%x BYTESTRING
-%x MEMRESERVE
+%x PROPNODENAME
PROPCHAR [a-zA-Z0-9,._+*#?-]
UNITCHAR [0-9a-f,]
@@ -51,7 +50,7 @@
%%
-"/include/" BEGIN(INCLUDE);
+<*>"/include/" BEGIN(INCLUDE);
<INCLUDE>\"[^"\n]*\" {
yytext[strlen(yytext) - 1] = 0;
@@ -63,13 +62,13 @@
}
-<<EOF>> {
+<*><<EOF>> {
if (!pop_input_file()) {
yyterminate();
}
}
-\"([^\\"]|\\.)*\" {
+<*>\"([^\\"]|\\.)*\" {
yylloc.filenum = srcpos_filenum;
yylloc.first_line = yylineno;
DPRINT("String: %s\n", yytext);
@@ -79,45 +78,24 @@
return DT_STRING;
}
-"/memreserve/" {
+<*>"/memreserve/" {
yylloc.filenum = srcpos_filenum;
yylloc.first_line = yylineno;
DPRINT("Keyword: /memreserve/\n");
- BEGIN(MEMRESERVE);
- return DT_MEMRESERVE;
- }
-
-<MEMRESERVE>[0-9a-fA-F]+ {
- yylloc.filenum = srcpos_filenum;
- yylloc.first_line = yylineno;
- if (yyleng > 2*sizeof(yylval.addr)) {
- fprintf(stderr, "Address value %s too large\n",
- yytext);
- }
- yylval.addr = (u64) strtoull(yytext, NULL, 16);
- DPRINT("Addr: %llx\n",
- (unsigned long long)yylval.addr);
- return DT_ADDR;
- }
-
-<MEMRESERVE>";" {
- yylloc.filenum = srcpos_filenum;
- yylloc.first_line = yylineno;
- DPRINT("/MEMRESERVE\n");
BEGIN(INITIAL);
- return ';';
+ return DT_MEMRESERVE;
}
<*>[a-zA-Z_][a-zA-Z0-9_]*: {
yylloc.filenum = srcpos_filenum;
yylloc.first_line = yylineno;
DPRINT("Label: %s\n", yytext);
- yylval.str = strdup(yytext);
- yylval.str[yyleng-1] = '\0';
+ yylval.labelref = strdup(yytext);
+ yylval.labelref[yyleng-1] = '\0';
return DT_LABEL;
}
-<CELLDATA>[bodh]# {
+[bodh]# {
yylloc.filenum = srcpos_filenum;
yylloc.first_line = yylineno;
if (*yytext == 'b')
@@ -132,27 +110,19 @@
return DT_BASE;
}
-<CELLDATA>[0-9a-fA-F]+ {
- yylloc.filenum = srcpos_filenum;
- yylloc.first_line = yylineno;
- yylval.str = strdup(yytext);
- DPRINT("Cell: '%s'\n", yylval.str);
- return DT_CELL;
- }
-
-<CELLDATA>">" {
+[0-9a-fA-F]+ {
yylloc.filenum = srcpos_filenum;
yylloc.first_line = yylineno;
- DPRINT("/CELLDATA\n");
- BEGIN(INITIAL);
- return '>';
+ yylval.literal = strdup(yytext);
+ DPRINT("Literal: '%s'\n", yylval.literal);
+ return DT_LITERAL;
}
-<CELLDATA>\&{REFCHAR}* {
+\&{REFCHAR}* {
yylloc.filenum = srcpos_filenum;
yylloc.first_line = yylineno;
DPRINT("Ref: %s\n", yytext+1);
- yylval.str = strdup(yytext+1);
+ yylval.labelref = strdup(yytext+1);
return DT_REF;
}
@@ -172,30 +142,13 @@
return ']';
}
-, { /* Technically this is a valid property name,
- but we'd rather use it as punctuation, so detect it
- here in preference */
- yylloc.filenum = srcpos_filenum;
- yylloc.first_line = yylineno;
- DPRINT("Char (propname like): %c (\\x%02x)\n", yytext[0],
- (unsigned)yytext[0]);
- return yytext[0];
- }
-
-{PROPCHAR}+ {
+<PROPNODENAME>{PROPCHAR}+(@{UNITCHAR}+)? {
yylloc.filenum = srcpos_filenum;
yylloc.first_line = yylineno;
- DPRINT("PropName: %s\n", yytext);
- yylval.str = strdup(yytext);
- return DT_PROPNAME;
- }
-
-{PROPCHAR}+(@{UNITCHAR}+)? {
- yylloc.filenum = srcpos_filenum;
- yylloc.first_line = yylineno;
- DPRINT("NodeName: %s\n", yytext);
- yylval.str = strdup(yytext);
- return DT_NODENAME;
+ DPRINT("PropNodeName: %s\n", yytext);
+ yylval.propnodename = strdup(yytext);
+ BEGIN(INITIAL);
+ return DT_PROPNODENAME;
}
@@ -213,21 +166,13 @@
<*>. {
yylloc.filenum = srcpos_filenum;
yylloc.first_line = yylineno;
- switch (yytext[0]) {
- case '<':
- DPRINT("CELLDATA\n");
- BEGIN(CELLDATA);
- break;
- case '[':
- DPRINT("BYTESTRING\n");
- BEGIN(BYTESTRING);
- break;
- default:
-
+ if ((yytext[0] == '{')
+ || (yytext[0] == ';')) {
+ DPRINT("<PROPNODENAME>\n");
+ BEGIN(PROPNODENAME);
+ }
DPRINT("Char: %c (\\x%02x)\n", yytext[0],
(unsigned)yytext[0]);
- break;
- }
return yytext[0];
}
Index: dtc/dtc-parser.y
===================================================================
--- dtc.orig/dtc-parser.y 2007-10-26 13:00:21.000000000 +1000
+++ dtc/dtc-parser.y 2007-10-26 15:11:02.000000000 +1000
@@ -25,45 +25,46 @@
#include "srcpos.h"
int yylex(void);
-cell_t cell_from_string(char *s, unsigned int base);
+unsigned long long eval_literal(const char *s, int base, int bits);
extern struct boot_info *the_boot_info;
%}
%union {
- cell_t cval;
+ char *propnodename;
+ char *literal;
+ char *labelref;
unsigned int cbase;
u8 byte;
- char *str;
struct data data;
+
+ u64 addr;
+ cell_t cell;
struct property *prop;
struct property *proplist;
struct node *node;
struct node *nodelist;
- int datalen;
- int hexlen;
- u64 addr;
struct reserve_info *re;
}
%token DT_MEMRESERVE
-%token <addr> DT_ADDR
-%token <str> DT_PROPNAME
-%token <str> DT_NODENAME
+%token <propnodename> DT_PROPNODENAME
+%token <literal> DT_LITERAL
%token <cbase> DT_BASE
-%token <str> DT_CELL
%token <byte> DT_BYTE
%token <data> DT_STRING
-%token <str> DT_LABEL
-%token <str> DT_REF
+%token <labelref> DT_LABEL
+%token <labelref> DT_REF
%type <data> propdata
%type <data> propdataprefix
%type <re> memreserve
%type <re> memreserves
-%type <cbase> opt_cell_base
+%type <addr> addr
%type <data> celllist
+%type <cbase> cellbase
+%type <cell> cellval
%type <data> bytestring
%type <prop> propdef
%type <proplist> proplist
@@ -72,8 +73,7 @@
%type <node> nodedef
%type <node> subnode
%type <nodelist> subnodes
-%type <str> label
-%type <str> nodename
+%type <labelref> label
%%
@@ -96,16 +96,23 @@
;
memreserve:
- label DT_MEMRESERVE DT_ADDR DT_ADDR ';'
+ label DT_MEMRESERVE addr addr ';'
{
$$ = build_reserve_entry($3, $4, $1);
}
- | label DT_MEMRESERVE DT_ADDR '-' DT_ADDR ';'
+ | label DT_MEMRESERVE addr '-' addr ';'
{
$$ = build_reserve_entry($3, $5 - $3 + 1, $1);
}
;
+addr:
+ DT_LITERAL
+ {
+ $$ = eval_literal($1, 16, 64);
+ }
+ ;
+
devicetree:
'/' nodedef
{
@@ -132,11 +139,11 @@
;
propdef:
- label DT_PROPNAME '=' propdata ';'
+ label DT_PROPNODENAME '=' propdata ';'
{
$$ = build_property($2, $4, $1);
}
- | label DT_PROPNAME ';'
+ | label DT_PROPNODENAME ';'
{
$$ = build_property($2, empty_data, $1);
}
@@ -177,23 +184,14 @@
}
;
-opt_cell_base:
- /* empty */
- {
- $$ = 16;
- }
- | DT_BASE
- ;
-
celllist:
/* empty */
{
$$ = empty_data;
}
- | celllist opt_cell_base DT_CELL
+ | celllist cellval
{
- $$ = data_append_cell($1,
- cell_from_string($3, $2));
+ $$ = data_append_cell($1, $2);
}
| celllist DT_REF
{
@@ -205,6 +203,21 @@
}
;
+cellbase:
+ /* empty */
+ {
+ $$ = 16;
+ }
+ | DT_BASE
+ ;
+
+cellval:
+ cellbase DT_LITERAL
+ {
+ $$ = eval_literal($2, $1, 32);
+ }
+ ;
+
bytestring:
/* empty */
{
@@ -232,23 +245,12 @@
;
subnode:
- label nodename nodedef
+ label DT_PROPNODENAME nodedef
{
$$ = name_node($3, $2, $1);
}
;
-nodename:
- DT_NODENAME
- {
- $$ = $1;
- }
- | DT_PROPNAME
- {
- $$ = $1;
- }
- ;
-
label:
/* empty */
{
@@ -273,33 +275,18 @@
fname, yylloc.first_line, s);
}
-
-/*
- * Convert a string representation of a numeric cell
- * in the given base into a cell.
- *
- * FIXME: should these specification errors be fatal instead?
- */
-
-cell_t cell_from_string(char *s, unsigned int base)
+unsigned long long eval_literal(const char *s, int base, int bits)
{
- cell_t c;
+ unsigned long long val;
char *e;
- c = strtoul(s, &e, base);
- if (*e) {
- fprintf(stderr,
- "Line %d: Invalid cell value '%s' : "
- "%c is not a base %d digit; %d assumed\n",
- yylloc.first_line, s, *e, base, c);
- }
-
- if (errno == EINVAL || errno == ERANGE) {
- fprintf(stderr,
- "Line %d: Invalid cell value '%s'; %d assumed\n",
- yylloc.first_line, s, c);
- errno = 0;
- }
-
- return c;
+ errno = 0;
+ val = strtoull(s, &e, base);
+ if (*e)
+ yyerror("bad characters in literal");
+ else if ((errno == ERANGE) || (val > ((1ULL << bits)-1)))
+ yyerror("literal out of range");
+ else if (errno != 0)
+ yyerror("bad literal");
+ return val;
}
--
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
* [2/2] dtc: Switch dtc to C-style literals
From: David Gibson @ 2007-10-26 6:20 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev
In-Reply-To: <20071026061718.GA6136@localhost.localdomain>
This patch introduces a new version of dts file, distinguished from
older files by starting with the special token /dts-v1/. dts files in
the new version take C-style literals instead of the old bare hex or
OF-style base notation. In addition, the "range" for of memreserve entries
(/memreserve/ f0000-fffff) is no longer recognized in the new format.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
dtc-lexer.l | 40 ++++++++++++++++++++++++++++++++--------
dtc-parser.y | 38 ++++++++++++++++++++++++++++++++++++--
tests/run_tests.sh | 4 ++++
tests/test_tree1.dts | 16 +++++++++-------
tests/test_tree1_dts0.dts | 27 +++++++++++++++++++++++++++
5 files changed, 108 insertions(+), 17 deletions(-)
Index: dtc/dtc-lexer.l
===================================================================
--- dtc.orig/dtc-lexer.l 2007-10-26 15:11:02.000000000 +1000
+++ dtc/dtc-lexer.l 2007-10-26 15:36:46.000000000 +1000
@@ -23,6 +23,7 @@
%x INCLUDE
%x BYTESTRING
%x PROPNODENAME
+%s V1
PROPCHAR [a-zA-Z0-9,._+*#?-]
UNITCHAR [0-9a-f,]
@@ -44,12 +45,18 @@
#define DPRINT(fmt, ...) do { } while (0)
#endif
+static int dts_version; /* = 0 */
-
+#define BEGIN_DEFAULT() if (dts_version == 0) { \
+ DPRINT("<INITIAL>\n"); \
+ BEGIN(INITIAL); \
+ } else { \
+ DPRINT("<V1>\n"); \
+ BEGIN(V1); \
+ }
%}
%%
-
<*>"/include/" BEGIN(INCLUDE);
<INCLUDE>\"[^"\n]*\" {
@@ -58,7 +65,7 @@
/* Some unrecoverable error.*/
exit(1);
}
- BEGIN(INITIAL);
+ BEGIN_DEFAULT();
}
@@ -78,11 +85,20 @@
return DT_STRING;
}
+<*>"/dts-v1/" {
+ yylloc.filenum = srcpos_filenum;
+ yylloc.first_line = yylineno;
+ DPRINT("Keyword: /dts-v1/\n");
+ dts_version = 1;
+ BEGIN_DEFAULT();
+ return DT_V1;
+ }
+
<*>"/memreserve/" {
yylloc.filenum = srcpos_filenum;
yylloc.first_line = yylineno;
DPRINT("Keyword: /memreserve/\n");
- BEGIN(INITIAL);
+ BEGIN_DEFAULT();
return DT_MEMRESERVE;
}
@@ -95,7 +111,7 @@
return DT_LABEL;
}
-[bodh]# {
+<INITIAL>[bodh]# {
yylloc.filenum = srcpos_filenum;
yylloc.first_line = yylineno;
if (*yytext == 'b')
@@ -110,7 +126,15 @@
return DT_BASE;
}
-[0-9a-fA-F]+ {
+<INITIAL>[0-9a-fA-F]+ {
+ yylloc.filenum = srcpos_filenum;
+ yylloc.first_line = yylineno;
+ yylval.literal = strdup(yytext);
+ DPRINT("Literal: '%s'\n", yylval.literal);
+ return DT_LEGACYLITERAL;
+ }
+
+<V1>[0-9]+|0[xX][0-9a-fA-F]+ {
yylloc.filenum = srcpos_filenum;
yylloc.first_line = yylineno;
yylval.literal = strdup(yytext);
@@ -138,7 +162,7 @@
yylloc.filenum = srcpos_filenum;
yylloc.first_line = yylineno;
DPRINT("/BYTESTRING\n");
- BEGIN(INITIAL);
+ BEGIN_DEFAULT();
return ']';
}
@@ -147,7 +171,7 @@
yylloc.first_line = yylineno;
DPRINT("PropNodeName: %s\n", yytext);
yylval.propnodename = strdup(yytext);
- BEGIN(INITIAL);
+ BEGIN_DEFAULT();
return DT_PROPNODENAME;
}
Index: dtc/dtc-parser.y
===================================================================
--- dtc.orig/dtc-parser.y 2007-10-26 15:11:02.000000000 +1000
+++ dtc/dtc-parser.y 2007-10-26 15:49:05.000000000 +1000
@@ -48,9 +48,11 @@
struct reserve_info *re;
}
+%token DT_V1
%token DT_MEMRESERVE
%token <propnodename> DT_PROPNODENAME
%token <literal> DT_LITERAL
+%token <literal> DT_LEGACYLITERAL
%token <cbase> DT_BASE
%token <byte> DT_BYTE
%token <data> DT_STRING
@@ -61,6 +63,8 @@
%type <data> propdataprefix
%type <re> memreserve
%type <re> memreserves
+%type <re> v0_memreserve
+%type <re> v0_memreserves
%type <addr> addr
%type <data> celllist
%type <cbase> cellbase
@@ -78,7 +82,11 @@
%%
sourcefile:
- memreserves devicetree
+ DT_V1 ';' memreserves devicetree
+ {
+ the_boot_info = build_boot_info($3, $4);
+ }
+ | v0_memreserves devicetree
{
the_boot_info = build_boot_info($1, $2);
}
@@ -100,6 +108,24 @@
{
$$ = build_reserve_entry($3, $4, $1);
}
+ ;
+
+v0_memreserves:
+ /* empty */
+ {
+ $$ = NULL;
+ }
+ | v0_memreserve v0_memreserves
+ {
+ $$ = chain_reserve_entry($1, $2);
+ };
+ ;
+
+v0_memreserve:
+ memreserve
+ {
+ $$ = $1;
+ }
| label DT_MEMRESERVE addr '-' addr ';'
{
$$ = build_reserve_entry($3, $5 - $3 + 1, $1);
@@ -108,6 +134,10 @@
addr:
DT_LITERAL
+ {
+ $$ = eval_literal($1, 0, 64);
+ }
+ | DT_LEGACYLITERAL
{
$$ = eval_literal($1, 16, 64);
}
@@ -212,7 +242,11 @@
;
cellval:
- cellbase DT_LITERAL
+ DT_LITERAL
+ {
+ $$ = eval_literal($1, 0, 32);
+ }
+ | cellbase DT_LEGACYLITERAL
{
$$ = eval_literal($2, $1, 32);
}
Index: dtc/tests/run_tests.sh
===================================================================
--- dtc.orig/tests/run_tests.sh 2007-10-26 15:11:02.000000000 +1000
+++ dtc/tests/run_tests.sh 2007-10-26 15:18:43.000000000 +1000
@@ -118,6 +118,10 @@
tree1_tests_rw dtc_tree1.test.dtb
run_test dtbs_equal_ordered dtc_tree1.test.dtb test_tree1.dtb
+ run_test dtc.sh -I dts -O dtb -o dtc_tree1_dts0.test.dtb test_tree1_dts0.dts
+ tree1_tests dtc_tree1_dts0.test.dtb
+ tree1_tests_rw dtc_tree1_dts0.test.dtb
+
run_test dtc.sh -I dts -O dtb -o dtc_escapes.test.dtb escapes.dts
run_test string_escapes dtc_escapes.test.dtb
}
Index: dtc/tests/test_tree1.dts
===================================================================
--- dtc.orig/tests/test_tree1.dts 2007-10-26 15:11:02.000000000 +1000
+++ dtc/tests/test_tree1.dts 2007-10-26 15:18:43.000000000 +1000
@@ -1,27 +1,29 @@
-/memreserve/ deadbeef00000000-deadbeef000fffff;
-/memreserve/ abcd1234 00001234;
+/dts-v1/;
+
+/memreserve/ 0xdeadbeef00000000 0x100000;
+/memreserve/ 0xabcd1234 0x00001234;
/ {
compatible = "test_tree1";
- prop-int = <deadbeef>;
+ prop-int = <0xdeadbeef>;
prop-str = "hello world";
subnode@1 {
compatible = "subnode1";
- prop-int = <deadbeef>;
+ prop-int = <0xdeadbeef>;
subsubnode {
compatible = "subsubnode1", "subsubnode";
- prop-int = <deadbeef>;
+ prop-int = <0xdeadbeef>;
};
};
subnode@2 {
- prop-int = <abcd1234>;
+ prop-int = <0xabcd1234>;
subsubnode@0 {
compatible = "subsubnode2", "subsubnode";
- prop-int = <abcd1234>;
+ prop-int = <0xabcd1234>;
};
};
};
Index: dtc/tests/test_tree1_dts0.dts
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ dtc/tests/test_tree1_dts0.dts 2007-10-26 15:18:43.000000000 +1000
@@ -0,0 +1,27 @@
+/memreserve/ deadbeef00000000-deadbeef000fffff;
+/memreserve/ abcd1234 00001234;
+
+/ {
+ compatible = "test_tree1";
+ prop-int = <deadbeef>;
+ prop-str = "hello world";
+
+ subnode@1 {
+ compatible = "subnode1";
+ prop-int = <deadbeef>;
+
+ subsubnode {
+ compatible = "subsubnode1", "subsubnode";
+ prop-int = <deadbeef>;
+ };
+ };
+
+ subnode@2 {
+ prop-int = <abcd1234>;
+
+ subsubnode@0 {
+ compatible = "subsubnode2", "subsubnode";
+ prop-int = <abcd1234>;
+ };
+ };
+};
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* [PATCH 01/16] Add of_get_next_parent()
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev; +Cc: davem
Iterating through a device node's parents is simple enough, but dealing
with the refcounts properly is a little ugly, and replicating that logic
is asking for someone to get it wrong or forget it all together, eg:
while (dn != NULL) {
/* loop body */
tmp = of_get_parent(dn);
of_node_put(dn);
dn = tmp;
}
So add of_get_next_parent(), inspired by of_get_next_child(). The contract
is that it returns the parent and drops the reference on the current node,
this makes the loop look like:
while (dn != NULL) {
/* loop body */
dn = of_get_next_parent(dn);
}
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
drivers/of/base.c | 25 +++++++++++++++++++++++++
include/linux/of.h | 1 +
2 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 9377f3b..e0db2b5 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -138,6 +138,31 @@ struct device_node *of_get_parent(const struct device_node *node)
EXPORT_SYMBOL(of_get_parent);
/**
+ * of_get_next_parent - Iterate to a node's parent
+ * @node: Node to get parent of
+ *
+ * This is like of_get_parent() except that it drops the
+ * refcount on the passed node, making it suitable for iterating
+ * through a node's parents.
+ *
+ * Returns a node pointer with refcount incremented, use
+ * of_node_put() on it when done.
+ */
+struct device_node *of_get_next_parent(struct device_node *node)
+{
+ struct device_node *parent;
+
+ if (!node)
+ return NULL;
+
+ read_lock(&devtree_lock);
+ parent = of_node_get(node->parent);
+ of_node_put(node);
+ read_unlock(&devtree_lock);
+ return parent;
+}
+
+/**
* of_get_next_child - Iterate a node childs
* @node: parent node
* @prev: previous child of the parent node, or NULL to get first
diff --git a/include/linux/of.h b/include/linux/of.h
index 5c39b92..3557d1e 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -44,6 +44,7 @@ extern struct device_node *of_find_compatible_node(struct device_node *from,
extern struct device_node *of_find_node_by_path(const char *path);
extern struct device_node *of_find_node_by_phandle(phandle handle);
extern struct device_node *of_get_parent(const struct device_node *node);
+extern struct device_node *of_get_next_parent(struct device_node *node);
extern struct device_node *of_get_next_child(const struct device_node *node,
struct device_node *prev);
extern struct property *of_find_property(const struct device_node *np,
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* [PATCH 02/16] Use of_get_next_parent() in pci_dma_bus_setup_pSeries()
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <80449d4682309dbf8cf80816be4f381fe875f3d1.1193381582.git.michael@ellerman.id.au>
pci_dma_bus_setup_pSeries() should use of_get_next_parent() to safely
iterate through the parent nodes.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/pseries/iommu.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index be17d23..83c0e0f 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -314,7 +314,7 @@ static void pci_dma_bus_setup_pSeries(struct pci_bus *bus)
{
struct device_node *dn;
struct iommu_table *tbl;
- struct device_node *isa_dn, *isa_dn_orig;
+ struct device_node *isa_dn;
struct device_node *tmp;
struct pci_dn *pci;
int children;
@@ -334,13 +334,13 @@ static void pci_dma_bus_setup_pSeries(struct pci_bus *bus)
/* Check if the ISA bus on the system is under
* this PHB.
*/
- isa_dn = isa_dn_orig = of_find_node_by_type(NULL, "isa");
+ isa_dn = of_find_node_by_type(NULL, "isa");
while (isa_dn && isa_dn != dn)
- isa_dn = isa_dn->parent;
+ isa_dn = of_get_next_parent(isa_dn);
- if (isa_dn_orig)
- of_node_put(isa_dn_orig);
+ /* Drop our reference, it's still safe to check the pointer below */
+ of_node_put(isa_dn);
/* Count number of direct PCI children of the PHB. */
for (children = 0, tmp = dn->child; tmp; tmp = tmp->sibling)
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* [PATCH 03/16] Use of_get_next_parent() in pci_dma_bus_setup_pSeriesLP()
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <80449d4682309dbf8cf80816be4f381fe875f3d1.1193381582.git.michael@ellerman.id.au>
pci_dma_bus_setup_pSeriesLP() should use of_get_next_parent() to safely
iterate through the parent nodes.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/pseries/iommu.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 83c0e0f..e2b325d 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -403,7 +403,7 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus *bus)
DBG("pci_dma_bus_setup_pSeriesLP: setting up bus %s\n", dn->full_name);
/* Find nearest ibm,dma-window, walking up the device tree */
- for (pdn = dn; pdn != NULL; pdn = pdn->parent) {
+ for (pdn = of_node_get(dn); pdn; pdn = of_get_next_parent(pdn)) {
dma_window = of_get_property(pdn, "ibm,dma-window", NULL);
if (dma_window != NULL)
break;
@@ -411,6 +411,7 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus *bus)
if (dma_window == NULL) {
DBG(" no ibm,dma-window property !\n");
+ of_node_put(pdn);
return;
}
@@ -437,6 +438,8 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus *bus)
if (pdn != dn)
PCI_DN(dn)->iommu_table = ppci->iommu_table;
+
+ of_node_put(pdn);
}
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* [PATCH 04/16] Use of_get_next_parent() in pci_dma_dev_setup_pSeries()
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <80449d4682309dbf8cf80816be4f381fe875f3d1.1193381582.git.michael@ellerman.id.au>
pci_dma_dev_setup_pSeries() should use of_get_next_parent() to safely
iterate through the parent nodes.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/pseries/iommu.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index e2b325d..5e9430e 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -472,14 +472,17 @@ static void pci_dma_dev_setup_pSeries(struct pci_dev *dev)
* an already allocated iommu table is found and use that.
*/
+ dn = of_node_get(dn);
while (dn && PCI_DN(dn) && PCI_DN(dn)->iommu_table == NULL)
- dn = dn->parent;
+ dn = of_get_next_parent(dn);
if (dn && PCI_DN(dn))
dev->dev.archdata.dma_data = PCI_DN(dn)->iommu_table;
else
printk(KERN_WARNING "iommu: Device %s has no iommu table\n",
pci_name(dev));
+
+ of_node_put(dn);
}
static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* [PATCH 05/16] Use of_get_next_parent() in pci_dma_dev_setup_pSeriesLP()
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <80449d4682309dbf8cf80816be4f381fe875f3d1.1193381582.git.michael@ellerman.id.au>
pci_dma_dev_setup_pSeriesLP() should use of_get_next_parent() to safely
iterate through the parent nodes.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/pseries/iommu.c | 12 ++++++++----
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 5e9430e..ef1aa8d 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -503,8 +503,9 @@ static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
dn = pci_device_to_OF_node(dev);
DBG(" node is %s\n", dn->full_name);
- for (pdn = dn; pdn && PCI_DN(pdn) && !PCI_DN(pdn)->iommu_table;
- pdn = pdn->parent) {
+ for (pdn = of_node_get(dn);
+ pdn && PCI_DN(pdn) && !PCI_DN(pdn)->iommu_table;
+ pdn = of_get_next_parent(pdn)) {
dma_window = of_get_property(pdn, "ibm,dma-window", NULL);
if (dma_window)
break;
@@ -514,7 +515,7 @@ static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
printk(KERN_WARNING "pci_dma_dev_setup_pSeriesLP: "
"no DMA window found for pci dev=%s dn=%s\n",
pci_name(dev), dn? dn->full_name : "<null>");
- return;
+ goto out_put;
}
DBG(" parent is %s\n", pdn->full_name);
@@ -524,7 +525,7 @@ static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
if (dma_window == NULL || pdn->parent == NULL) {
DBG(" no dma window for device, linking to parent\n");
dev->dev.archdata.dma_data = PCI_DN(pdn)->iommu_table;
- return;
+ goto out_put;
}
pci = PCI_DN(pdn);
@@ -544,6 +545,9 @@ static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
}
dev->dev.archdata.dma_data = pci->iommu_table;
+
+out_put:
+ of_node_put(pdn);
}
#else /* CONFIG_PCI */
#define pci_dma_bus_setup_pSeries NULL
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* [PATCH 06/16] Use of_get_next_child() in pci_dma_bus_setup_pSeries()
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <80449d4682309dbf8cf80816be4f381fe875f3d1.1193381582.git.michael@ellerman.id.au>
pci_dma_bus_setup_pSeries() should use of_get_next_child() to safely
iterate through the nodes children.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/pseries/iommu.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index ef1aa8d..1a9e14f 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -343,7 +343,7 @@ static void pci_dma_bus_setup_pSeries(struct pci_bus *bus)
of_node_put(isa_dn);
/* Count number of direct PCI children of the PHB. */
- for (children = 0, tmp = dn->child; tmp; tmp = tmp->sibling)
+ for (children = 0, tmp = NULL; (tmp = of_get_next_child(dn, tmp));)
children++;
DBG("Children: %d\n", children);
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* [PATCH 07/16] Use of_get_next_parent() in xics_setup_8259_cascade()
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <80449d4682309dbf8cf80816be4f381fe875f3d1.1193381582.git.michael@ellerman.id.au>
Use of_get_next_parent() in xics_setup_8259_cascade() to simplify
the loop logic.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/pseries/xics.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/xics.c b/arch/powerpc/platforms/pseries/xics.c
index 66e7d68..58a3cc7 100644
--- a/arch/powerpc/platforms/pseries/xics.c
+++ b/arch/powerpc/platforms/pseries/xics.c
@@ -617,7 +617,7 @@ static void __init xics_init_one_node(struct device_node *np,
static void __init xics_setup_8259_cascade(void)
{
- struct device_node *np, *old, *found = NULL;
+ struct device_node *np, *found = NULL;
int cascade, naddr;
const u32 *addrp;
unsigned long intack = 0;
@@ -638,11 +638,8 @@ static void __init xics_setup_8259_cascade(void)
}
pr_debug("xics: cascade mapped to irq %d\n", cascade);
- for (old = of_node_get(found); old != NULL ; old = np) {
- np = of_get_parent(old);
- of_node_put(old);
- if (np == NULL)
- break;
+ np = of_node_get(found);
+ while ((np = of_get_next_parent(np))) {
if (strcmp(np->name, "pci") != 0)
continue;
addrp = of_get_property(np, "8259-interrupt-acknowledge", NULL);
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* [PATCH 08/16] Use of_get_next_parent() in pseries_mpic_init_IRQ()
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <80449d4682309dbf8cf80816be4f381fe875f3d1.1193381582.git.michael@ellerman.id.au>
Use of_get_next_parent() in pseries_mpic_init_IRQ() to simplify
the loop logic.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/pseries/setup.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index fdb9b1c..f08dfaf 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -129,7 +129,7 @@ void pseries_8259_cascade(unsigned int irq, struct irq_desc *desc)
static void __init pseries_mpic_init_IRQ(void)
{
- struct device_node *np, *old, *cascade = NULL;
+ struct device_node *np, *cascade = NULL;
const unsigned int *addrp;
unsigned long intack = 0;
const unsigned int *opprop;
@@ -182,11 +182,8 @@ static void __init pseries_mpic_init_IRQ(void)
}
/* Check ACK type */
- for (old = of_node_get(cascade); old != NULL ; old = np) {
- np = of_get_parent(old);
- of_node_put(old);
- if (np == NULL)
- break;
+ np = of_node_get(cascade);
+ while ((np = of_get_next_parent(np))) {
if (strcmp(np->name, "pci") != 0)
continue;
addrp = of_get_property(np, "8259-interrupt-acknowledge",
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* [PATCH 09/16] Use of_get_next_parent() in axon_msi.c
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <80449d4682309dbf8cf80816be4f381fe875f3d1.1193381582.git.michael@ellerman.id.au>
We can use of_get_next_parent() in two places in axon_msi.c to
simplify the looping logic.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/cell/axon_msi.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
index 095988f..76870fe 100644
--- a/arch/powerpc/platforms/cell/axon_msi.c
+++ b/arch/powerpc/platforms/cell/axon_msi.c
@@ -125,7 +125,7 @@ static struct axon_msic *find_msi_translator(struct pci_dev *dev)
return NULL;
}
- for (; dn; tmp = of_get_parent(dn), of_node_put(dn), dn = tmp) {
+ for (; dn; dn = of_get_next_parent(dn)) {
ph = of_get_property(dn, "msi-translator", NULL);
if (ph)
break;
@@ -171,7 +171,7 @@ static int axon_msi_check_device(struct pci_dev *dev, int nvec, int type)
static int setup_msi_msg_address(struct pci_dev *dev, struct msi_msg *msg)
{
- struct device_node *dn, *tmp;
+ struct device_node *dn;
struct msi_desc *entry;
int len;
const u32 *prop;
@@ -184,7 +184,7 @@ static int setup_msi_msg_address(struct pci_dev *dev, struct msi_msg *msg)
entry = list_first_entry(&dev->msi_list, struct msi_desc, list);
- for (; dn; tmp = of_get_parent(dn), of_node_put(dn), dn = tmp) {
+ for (; dn; dn = of_get_next_parent(dn)) {
if (entry->msi_attrib.is_64) {
prop = of_get_property(dn, "msi-address-64", &len);
if (prop)
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* [PATCH 10/16] Use of_get_next_child() in EEH gather_pci_data()
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <80449d4682309dbf8cf80816be4f381fe875f3d1.1193381582.git.michael@ellerman.id.au>
We should use of_get_next_child() in the EEH gather_pci_data()
routine to safely traverse the node's children.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/pseries/eeh.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
index 22322b3..7309caa 100644
--- a/arch/powerpc/platforms/pseries/eeh.c
+++ b/arch/powerpc/platforms/pseries/eeh.c
@@ -238,12 +238,10 @@ static size_t gather_pci_data(struct pci_dn *pdn, char * buf, size_t len)
/* Gather status on devices under the bridge */
if (dev->class >> 16 == PCI_BASE_CLASS_BRIDGE) {
- dn = pdn->node->child;
- while (dn) {
+ for (dn = NULL; (dn = of_get_next_child(pdn->node, dn));) {
pdn = PCI_DN(dn);
if (pdn)
n += gather_pci_data(pdn, buf+n, len-n);
- dn = dn->sibling;
}
}
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* [PATCH 11/16] Use of_get_next_child() in eeh_restore_bars()
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <80449d4682309dbf8cf80816be4f381fe875f3d1.1193381582.git.michael@ellerman.id.au>
We should use of_get_next_child() in the eeh_restore_bars()
routine to safely traverse the node's children.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/pseries/eeh.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
index 7309caa..abe3de1 100644
--- a/arch/powerpc/platforms/pseries/eeh.c
+++ b/arch/powerpc/platforms/pseries/eeh.c
@@ -841,11 +841,8 @@ void eeh_restore_bars(struct pci_dn *pdn)
if ((pdn->eeh_mode & EEH_MODE_SUPPORTED) && !IS_BRIDGE(pdn->class_code))
__restore_bars (pdn);
- dn = pdn->node->child;
- while (dn) {
+ for (dn = NULL; (dn = of_get_next_child(pdn->node, dn));)
eeh_restore_bars (PCI_DN(dn));
- dn = dn->sibling;
- }
}
/**
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* [PATCH 12/16] Use of_get_next_child() in eeh_add_device_tree_early()
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <80449d4682309dbf8cf80816be4f381fe875f3d1.1193381582.git.michael@ellerman.id.au>
We should use of_get_next_child() in the eeh_add_device_tree_early()
routine to safely traverse the node's children.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/pseries/eeh.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
index abe3de1..d1d6d55 100644
--- a/arch/powerpc/platforms/pseries/eeh.c
+++ b/arch/powerpc/platforms/pseries/eeh.c
@@ -1120,7 +1120,7 @@ static void eeh_add_device_early(struct device_node *dn)
void eeh_add_device_tree_early(struct device_node *dn)
{
struct device_node *sib;
- for (sib = dn->child; sib; sib = sib->sibling)
+ for (sib = NULL; (sib = of_get_next_child(dn, sib));)
eeh_add_device_tree_early(sib);
eeh_add_device_early(dn);
}
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* [PATCH 13/16] Use of_get_next_child() in __eeh_mark_slot()
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <80449d4682309dbf8cf80816be4f381fe875f3d1.1193381582.git.michael@ellerman.id.au>
We should use of_get_next_child() in __eeh_mark_slot() to safely
traverse the node's children.
To achieve this we need to change __eeh_mark_slot() to take the parent
node, not the child. This is also safer, as passing anything other than
node->child to the existing routine will not traverse all peers, only
those deeper in the sibling list.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/pseries/eeh.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
index d1d6d55..d06ab36 100644
--- a/arch/powerpc/platforms/pseries/eeh.c
+++ b/arch/powerpc/platforms/pseries/eeh.c
@@ -373,9 +373,11 @@ struct device_node * find_device_pe(struct device_node *dn)
* an interrupt context, which is bad.
*/
-static void __eeh_mark_slot (struct device_node *dn, int mode_flag)
+static void __eeh_mark_slot(struct device_node *parent, int mode_flag)
{
- while (dn) {
+ struct device_node *dn;
+
+ for (dn = NULL; (dn = of_get_next_child(parent, dn));) {
if (PCI_DN(dn)) {
/* Mark the pci device driver too */
struct pci_dev *dev = PCI_DN(dn)->pcidev;
@@ -386,9 +388,8 @@ static void __eeh_mark_slot (struct device_node *dn, int mode_flag)
dev->error_state = pci_channel_io_frozen;
if (dn->child)
- __eeh_mark_slot (dn->child, mode_flag);
+ __eeh_mark_slot(dn, mode_flag);
}
- dn = dn->sibling;
}
}
@@ -408,7 +409,7 @@ void eeh_mark_slot (struct device_node *dn, int mode_flag)
if (dev)
dev->error_state = pci_channel_io_frozen;
- __eeh_mark_slot (dn->child, mode_flag);
+ __eeh_mark_slot(dn, mode_flag);
}
static void __eeh_clear_slot (struct device_node *dn, int mode_flag)
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* [PATCH 14/16] Use of_get_next_child() in __eeh_clear_slot()
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <80449d4682309dbf8cf80816be4f381fe875f3d1.1193381582.git.michael@ellerman.id.au>
We should use of_get_next_child() in __eeh_clear_slot() to safely
traverse the node's children.
To achieve this we need to change __eeh_clear_slot() to take the parent
node, not the child. This is also safer, as passing anything other than
node->child to the existing routine will not traverse all peers, only
those deeper in the sibling list.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/pseries/eeh.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
index d06ab36..1537597 100644
--- a/arch/powerpc/platforms/pseries/eeh.c
+++ b/arch/powerpc/platforms/pseries/eeh.c
@@ -412,16 +412,17 @@ void eeh_mark_slot (struct device_node *dn, int mode_flag)
__eeh_mark_slot(dn, mode_flag);
}
-static void __eeh_clear_slot (struct device_node *dn, int mode_flag)
+static void __eeh_clear_slot(struct device_node *parent, int mode_flag)
{
- while (dn) {
+ struct device_node *dn;
+
+ for (dn = NULL; (dn = of_get_next_child(parent, dn));) {
if (PCI_DN(dn)) {
PCI_DN(dn)->eeh_mode &= ~mode_flag;
PCI_DN(dn)->eeh_check_count = 0;
if (dn->child)
- __eeh_clear_slot (dn->child, mode_flag);
+ __eeh_clear_slot(dn, mode_flag);
}
- dn = dn->sibling;
}
}
@@ -438,7 +439,7 @@ void eeh_clear_slot (struct device_node *dn, int mode_flag)
PCI_DN(dn)->eeh_mode &= ~mode_flag;
PCI_DN(dn)->eeh_check_count = 0;
- __eeh_clear_slot (dn->child, mode_flag);
+ __eeh_clear_slot(dn, mode_flag);
spin_unlock_irqrestore(&confirm_error_lock, flags);
}
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* [PATCH 15/16] Use of_get_next_child() in eeh_reset_device()
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <80449d4682309dbf8cf80816be4f381fe875f3d1.1193381582.git.michael@ellerman.id.au>
We should use of_get_next_child() in eeh_reset_device() to safely
traverse the node's children.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
Linas, I don't grok the logic in here, can you check it's OK. The old code
would potentially not walk through all siblings if pe_dn->node was not
equal to pe_dn->node->parent->child, but now it will regardless.
arch/powerpc/platforms/pseries/eeh_driver.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/eeh_driver.c b/arch/powerpc/platforms/pseries/eeh_driver.c
index 15e015e..abf1850 100644
--- a/arch/powerpc/platforms/pseries/eeh_driver.c
+++ b/arch/powerpc/platforms/pseries/eeh_driver.c
@@ -249,7 +249,7 @@ static void eeh_report_failure(struct pci_dev *dev, void *userdata)
static int eeh_reset_device (struct pci_dn *pe_dn, struct pci_bus *bus)
{
- struct device_node *dn;
+ struct device_node *dn, *parent;
int cnt, rc;
/* pcibios will clear the counter; save the value */
@@ -270,15 +270,16 @@ static int eeh_reset_device (struct pci_dn *pe_dn, struct pci_bus *bus)
if (!pcibios_find_pci_bus(dn) && PCI_DN(dn->parent))
dn = dn->parent->child;
- while (dn) {
+ parent = of_node_get(dn->parent);
+ for (dn = NULL; (dn = of_get_next_child(parent, dn));) {
struct pci_dn *ppe = PCI_DN(dn);
/* On Power4, always true because eeh_pe_config_addr=0 */
if (pe_dn->eeh_pe_config_addr == ppe->eeh_pe_config_addr) {
rtas_configure_bridge(ppe);
eeh_restore_bars(ppe);
}
- dn = dn->sibling;
}
+ of_node_put(parent);
/* Give the system 5 seconds to finish running the user-space
* hotplug shutdown scripts, e.g. ifdown for ethernet. Yes,
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* [PATCH 16/16] Use of_get_next_child() in EEH print_device_node_tree()
From: Michael Ellerman @ 2007-10-26 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <80449d4682309dbf8cf80816be4f381fe875f3d1.1193381582.git.michael@ellerman.id.au>
We should use of_get_next_child() in print_device_node_tree() to safely
traverse the node's children.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/pseries/eeh_driver.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/eeh_driver.c b/arch/powerpc/platforms/pseries/eeh_driver.c
index abf1850..0a23bc4 100644
--- a/arch/powerpc/platforms/pseries/eeh_driver.c
+++ b/arch/powerpc/platforms/pseries/eeh_driver.c
@@ -44,6 +44,7 @@ static inline const char * pcid_name (struct pci_dev *pdev)
#ifdef DEBUG
static void print_device_node_tree (struct pci_dn *pdn, int dent)
{
+ struct device_node *pc;
int i;
if (!pdn) return;
for (i=0;i<dent; i++)
@@ -52,11 +53,8 @@ static void print_device_node_tree (struct pci_dn *pdn, int dent)
pdn->node->name, pdn->eeh_mode, pdn->eeh_config_addr,
pdn->eeh_pe_config_addr, pdn->node->full_name);
dent += 3;
- struct device_node *pc = pdn->node->child;
- while (pc) {
+ for (pc = NULL; (pc = of_get_next_child(pdn->node, pc));)
print_device_node_tree(PCI_DN(pc), dent);
- pc = pc->sibling;
- }
}
#endif
--
1.5.2.rc1.1884.g59b20
^ permalink raw reply related
* Re: [PATCH 09/16] Use of_get_next_parent() in axon_msi.c
From: Stephen Rothwell @ 2007-10-26 7:24 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <4e749fe7078e2c006cf40921cea904833c75263c.1193381582.git.michael@ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 763 bytes --]
On Fri, 26 Oct 2007 16:54:41 +1000 (EST) Michael Ellerman <michael@ellerman.id.au> wrote:
>
> +++ b/arch/powerpc/platforms/cell/axon_msi.c
> @@ -125,7 +125,7 @@ static struct axon_msic *find_msi_translator(struct pci_dev *dev)
> return NULL;
> }
>
> - for (; dn; tmp = of_get_parent(dn), of_node_put(dn), dn = tmp) {
> + for (; dn; dn = of_get_next_parent(dn)) {
> ph = of_get_property(dn, "msi-translator", NULL);
> if (ph)
> break;
You no longer assign anything to tmp, but just below here, you may jump
to out_error: which will do an of_node_put(tmp). So you need to
initialise tmp or have another error goto label.
--
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 11/16] Use of_get_next_child() in eeh_restore_bars()
From: Stephen Rothwell @ 2007-10-26 7:29 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <ef7facd658919117aa7c693b4eeb12241ca859d8.1193381582.git.michael@ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 699 bytes --]
On Fri, 26 Oct 2007 16:54:43 +1000 (EST) Michael Ellerman <michael@ellerman.id.au> wrote:
>
> +++ b/arch/powerpc/platforms/pseries/eeh.c
> @@ -841,11 +841,8 @@ void eeh_restore_bars(struct pci_dn *pdn)
> if ((pdn->eeh_mode & EEH_MODE_SUPPORTED) && !IS_BRIDGE(pdn->class_code))
> __restore_bars (pdn);
>
> - dn = pdn->node->child;
> - while (dn) {
> + for (dn = NULL; (dn = of_get_next_child(pdn->node, dn));)
Just wondering if we need
#define for_each_child_node(dn, parent) \
for (dn = of_get_next_child(parent, NULL); dn; \
dn = of_get_next_child(parent, dn))
--
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
* mpc8xx, i2c read DB busy issue
From: Ankur Maheshwari @ 2007-10-26 9:30 UTC (permalink / raw)
To: linuxppc-embedded
Hi all,
I am trying to use mpc860 as i2c-SLAVE. Kernel, I am using is
Linux-2.4.4, which I can't change due to some reasons.
I am able to do I2C read/write operations on mpc8xx-i2c slave only after
re-insmoding my driver module (i2c-algo-8xx.c, and adapter layer drive,
code I took form i2c-rpx/r360.c ).
The issue I am facing is when I insmod i2c-driver on fresh Linux boot,
any i2c-mpc860 as slave read/write operation gives me busy status (which
is for Rx BD not available) in Controller's i2c Event Registers. When I
get busy status I have to do force_close() and re-init of bd, then only
busy status goes off.
I tried calling cpm_iic_init(); twice while __init but with no change in
busy status.
After some initial try, i2c slave write works but i2c read gives no data
to the master. After re-insmoding driver, read/write works with some
times (1 out of 15 times) i2c-read failing.
But still I can't understand why busy is coming....
Any help or suggestions are highly appreciated.
thanks,
Ankur Maheshwari
^ permalink raw reply
* Re: ppc manual paging question
From: Wang, Baojun @ 2007-10-26 9:50 UTC (permalink / raw)
To: benh, linuxppc-dev
In-Reply-To: <393040796.08064@lzu.edu.cn>
[-- Attachment #1: Type: text/plain, Size: 3456 bytes --]
On Monday 22 October 2007 16:04:14, Benjamin Herrenschmidt wrote：
> > Yup, I've found how does the kernel handle tlbs, I think the most
> > important thing is I forgot read/write the SPRN_SPRG3 register as _switch
> > does.
>
> SPRG3 is for use by the operating system for whatever you want... if you
> are copying linux code, then you probably indeed want to get that right
> but you don't have to use SPRG3.
>
> > I've add the _PAGE_PRESENT flag to the related PTE
>
> Hrm.. that has nothing ot do with the PTE. Bolting is more a property of
> your replacement algorithm in the TLB miss handler.
>
> Ben.
Hi,
First thanks a lot for your help I've finish the tlb code, now I can
manually translate the virtual address correctly, I verified this by printing
out the data within the virtual address and it's fine. now the only thing
left is jump to that address (the address is point to _start function), But I
got an error about unable to access the stack (0xd100fc60 ...), but it is
valid before the instruction:
/**
* XXX: should not defined here
*/
#define EVENTS_USER_ADDR_OFFSET 36
_GLOBAL(jump_xm_dom)
stwu r1,-INT_FRAME_SIZE(r1)
mflr r0
stw r0,INT_FRAME_SIZE+4(r1)
stw r31,INT_FRAME_SIZE+128(r1)
lwz r5,EVENTS_USER_ADDR_OFFSET(r4)
mr r31,r5 /* new_domain->events_user_addr */
cmpwi r3,0
beq 1f
mtctr r3 /* jump to entry_point */
bctrl
li r3,0
1:
lwz r31,INT_FRAME_SIZE+128(r1)
lwz r0,INT_FRAME_SIZE+4(r1)
addi r1,r1,INT_FRAME_SIZE
mtlr r0
blr
the SP is valid before `bctrl', while exec bctrl, I got the error said unable
to access address SP ($r1) from bdigdb, without bdigbd (running directly), an
error is print out while the system is dead:
insn: 94 21 ff 40 7c 08 02 a6 90 01 00 c4 7f e3 fb 78 3d 20 10 01 90 69 07 a0
48 00 02 55 80 01 00 c4
$T0440:10000094;01:d1072e60;#ee
address d1072e60 is the address of SP ($r1) before bctrl.
NOTE entry_point($r3) is address like 0x100000a0 which is loaded from the
userspace by a loader program (it loads all section marked as PT_LOAD, such
as .text, the above insn is the entry of .text section, which is _start), but
the above code is from the kernel space. and here is the _start function:
#define INT_FRAME_SIZE 192
.globl _start
_start:
stwu 1, -INT_FRAME_SIZE(1)
mflr 0
stw 0, INT_FRAME_SIZE+4(1)
mr 3,31 /* new_domain->events_user_addr */
lis 9, event_handling@ha
stw 3, event_handling@l(9)
bl kmain
lwz 0, INT_FRAME_SIZE+4(1);
mtlr 0
addi 1, 1, INT_FRAME_SIZE
blr
.size _start, .-_start
I'm sorry I'm not very familiar with the ppc assembly, is there something
fundamentally wrong? Thank you very much!
Regards,
Wang
--
Wang, Baojun Lanzhou University
Distributed & Embedded System Lab http://dslab.lzu.edu.cn
School of Information Science and Engeneering wangbj@lzu.edu.cn
Tianshui South Road 222. Lanzhou 730000 .P.R.China
Tel:+86-931-8912025 Fax:+86-931-8912022
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [i2c] i2c-mpc.c driver issues
From: Jean Delvare @ 2007-10-26 9:53 UTC (permalink / raw)
To: Tjernlund; +Cc: linuxppc-dev, i2c
In-Reply-To: <019001c81681$b5d449c0$5267a8c0@Jocke>
Hi Jocke,
On Wed, 24 Oct 2007 23:06:13 +0200, Tjernlund wrote:
> While browsing the i2c-mpc.c driver I noticed some things that look odd
> to me so I figured I report them. Could not find a maintainer in the MAINTANERS file
> so I sent here, cc:ed linuxppc-dev as well.
>
> 1) There are a lot of return -1 error code that is propagated back to
> userspace. Should be changed to proper -Exxx codes.
This is true of many Linux i2c bus drivers, unfortunately. While nothing
actually prevents drivers from returning -1 to userspace on error,
meaningful error codes would of course be preferred.
> 2) mpc_read(), according to the comment below it sends a STOP condition here but
> this function does not known if this is the last read or not. mpc_xfer is
> the one that knows when the transaction is over and should send the stop, which it already
> does.
>
> /* Generate stop on last byte */
> if (i == length - 1)
> writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_TXAK);
Probably correct, although I am not familiar with this specific
hardware. I guess that the same is true of mpc_write as well, which is
even worse because write + read combined transactions are very common
(while read + write are not.)
I'm not completely sure that mpc_xfer sends the stop. mpc_i2c_stop
doesn't seem to do much.
Now that you've identified these bugs, what about sending patches
to fix them?
--
Jean Delvare
^ 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