This affects arch/powerpc/boot/dts/virtex440-ml510.dts but I think it is
actually a more general issue:
$ make ARCH=powerpc CROSS_COMPILE=powerpc-linux- virtex440-ml510.dtb
CC scripts/mod/devicetable-offsets.s
GEN scripts/mod/devicetable-offsets.h
HOSTCC scripts/mod/file2alias.o
HOSTLD scripts/mod/modpost
DTC arch/powerpc/boot/virtex440-ml510.dtb
Error: arch/powerpc/boot/dts/virtex440-ml510.dts:374.6-7 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [arch/powerpc/boot/virtex440-ml510.dtb] Error 1
make: *** [virtex440-ml510.dtb] Error 2
Line 374 is the "IDSEL 0x16..." line here:
interrupt-map = <
/* IRQ mapping for pci slots and ALI M1533
...
* management core also isn't used.
*/
/* IDSEL 0x16 / dev=6, bus=0 / PCI slot 3 */
0x3000 0 0 1 &xps_intc_0 3 2
0x3000 0 0 2 &xps_intc_0 2 2
0x3000 0 0 3 &xps_intc_0 5 2
0x3000 0 0 4 &xps_intc_0 4 2
Which gets preprocessed into:
interrupt-map = <
# 375 "arch/powerpc/boot/dts/virtex440-ml510.dts"
0x3000 0 0 1 &xps_intc_0 3 2
0x3000 0 0 2 &xps_intc_0 2 2
0x3000 0 0 3 &xps_intc_0 5 2
0x3000 0 0 4 &xps_intc_0 4 2
If I manually remove the "# 375 " line then that fixes the error
(although there is then a subsequent one of the same type).
I suppose this is a bug in dtc? It appears to have at least some
awareness of these preprocessor line number comments since it manages to
report the original source line number.
Ian.
On Fri, 31 May 2013 11:29:30 +0100, Ian Campbell <[email protected]> wrote:
> This affects arch/powerpc/boot/dts/virtex440-ml510.dts but I think it is
> actually a more general issue:
>
> $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux- virtex440-ml510.dtb
> CC scripts/mod/devicetable-offsets.s
> GEN scripts/mod/devicetable-offsets.h
> HOSTCC scripts/mod/file2alias.o
> HOSTLD scripts/mod/modpost
> DTC arch/powerpc/boot/virtex440-ml510.dtb
> Error: arch/powerpc/boot/dts/virtex440-ml510.dts:374.6-7 syntax error
> FATAL ERROR: Unable to parse input tree
> make[1]: *** [arch/powerpc/boot/virtex440-ml510.dtb] Error 1
> make: *** [virtex440-ml510.dtb] Error 2
>
> Line 374 is the "IDSEL 0x16..." line here:
> interrupt-map = <
> /* IRQ mapping for pci slots and ALI M1533
> ...
> * management core also isn't used.
> */
>
> /* IDSEL 0x16 / dev=6, bus=0 / PCI slot 3 */
> 0x3000 0 0 1 &xps_intc_0 3 2
> 0x3000 0 0 2 &xps_intc_0 2 2
> 0x3000 0 0 3 &xps_intc_0 5 2
> 0x3000 0 0 4 &xps_intc_0 4 2
>
> Which gets preprocessed into:
> interrupt-map = <
> # 375 "arch/powerpc/boot/dts/virtex440-ml510.dts"
> 0x3000 0 0 1 &xps_intc_0 3 2
> 0x3000 0 0 2 &xps_intc_0 2 2
> 0x3000 0 0 3 &xps_intc_0 5 2
> 0x3000 0 0 4 &xps_intc_0 4 2
>
> If I manually remove the "# 375 " line then that fixes the error
> (although there is then a subsequent one of the same type).
>
> I suppose this is a bug in dtc? It appears to have at least some
> awareness of these preprocessor line number comments since it manages to
> report the original source line number.
dtc is only able to track line numbers when the native /include/
directive is used. The #include directive doesn't help it. It should be
added, but until it is the following patch solves the problem:
I've got this patch in my tree. Either Rob or I will push it to Linus in
the next few days.
g.
---
commit d01dccdcb3ea8233b09efb9c24db9f057fbd3b37
Author: Grant Likely <[email protected]>
Date: Fri May 31 12:45:18 2013 +0100
dtc: Suppress cpp linemarker annotations
DTC isn't able to parse cpp linemarker annotations, so suppress them in
the cpp output by adding the -P flag to the cpp options.
Signed-off-by: Grant Likely <[email protected]>
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 51bb3de..fc08a2b 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -149,7 +149,7 @@ cpp_flags = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \
ld_flags = $(LDFLAGS) $(ldflags-y)
-dtc_cpp_flags = -Wp,-MD,$(depfile).pre -nostdinc \
+dtc_cpp_flags = -Wp,-MD,$(depfile).pre -nostdinc -P \
-I$(srctree)/arch/$(SRCARCH)/boot/dts \
-I$(srctree)/arch/$(SRCARCH)/boot/dts/include \
-undef -D__DTS__
On Fri, 2013-05-31 at 12:48 +0100, Grant Likely wrote:
> On Fri, 31 May 2013 11:29:30 +0100, Ian Campbell <[email protected]> wrote:
> > This affects arch/powerpc/boot/dts/virtex440-ml510.dts but I think it is
> > actually a more general issue:
> >
> > $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux- virtex440-ml510.dtb
> > CC scripts/mod/devicetable-offsets.s
> > GEN scripts/mod/devicetable-offsets.h
> > HOSTCC scripts/mod/file2alias.o
> > HOSTLD scripts/mod/modpost
> > DTC arch/powerpc/boot/virtex440-ml510.dtb
> > Error: arch/powerpc/boot/dts/virtex440-ml510.dts:374.6-7 syntax error
> > FATAL ERROR: Unable to parse input tree
> > make[1]: *** [arch/powerpc/boot/virtex440-ml510.dtb] Error 1
> > make: *** [virtex440-ml510.dtb] Error 2
> >
> > Line 374 is the "IDSEL 0x16..." line here:
> > interrupt-map = <
> > /* IRQ mapping for pci slots and ALI M1533
> > ...
> > * management core also isn't used.
> > */
> >
> > /* IDSEL 0x16 / dev=6, bus=0 / PCI slot 3 */
> > 0x3000 0 0 1 &xps_intc_0 3 2
> > 0x3000 0 0 2 &xps_intc_0 2 2
> > 0x3000 0 0 3 &xps_intc_0 5 2
> > 0x3000 0 0 4 &xps_intc_0 4 2
> >
> > Which gets preprocessed into:
> > interrupt-map = <
> > # 375 "arch/powerpc/boot/dts/virtex440-ml510.dts"
> > 0x3000 0 0 1 &xps_intc_0 3 2
> > 0x3000 0 0 2 &xps_intc_0 2 2
> > 0x3000 0 0 3 &xps_intc_0 5 2
> > 0x3000 0 0 4 &xps_intc_0 4 2
> >
> > If I manually remove the "# 375 " line then that fixes the error
> > (although there is then a subsequent one of the same type).
> >
> > I suppose this is a bug in dtc? It appears to have at least some
> > awareness of these preprocessor line number comments since it manages to
> > report the original source line number.
>
> dtc is only able to track line numbers when the native /include/
> directive is used. The #include directive doesn't help it. It should be
> added, but until it is the following patch solves the problem:
>
> I've got this patch in my tree. Either Rob or I will push it to Linus in
> the next few days.
Thanks, I'll do something similar in my device-tree.git tree.
Ian
On Fri, 2013-05-31 at 08:01 -0500, Jon Loeliger wrote:
> > >
> > > Line 374 is the "IDSEL 0x16..." line here:
> > > interrupt-map = <
> > > /* IRQ mapping for pci slots and ALI M1533
> > > ...
> > > * management core also isn't used.
> > > */
> > >
> > > /* IDSEL 0x16 / dev=6, bus=0 / PCI slot 3 */
> > > 0x3000 0 0 1 &xps_intc_0 3 2
> > > 0x3000 0 0 2 &xps_intc_0 2 2
> > > 0x3000 0 0 3 &xps_intc_0 5 2
> > > 0x3000 0 0 4 &xps_intc_0 4 2
>
> Can you show me the original source without mods here, please?
This is Linux v3.10-rc3 arch/powerpc/boot/dts/virtex440-ml510.dts also
attached.
> Or is the "..." purely elided comments?
Yes.
> > > Which gets preprocessed into:
> > > interrupt-map = <
> > > # 375 "arch/powerpc/boot/dts/virtex440-ml510.dts"
> > > 0x3000 0 0 1 &xps_intc_0 3 2
> > > 0x3000 0 0 2 &xps_intc_0 2 2
> > > 0x3000 0 0 3 &xps_intc_0 5 2
> > > 0x3000 0 0 4 &xps_intc_0 4 2
>
>
> > dtc is only able to track line numbers when the native /include/
> > directive is used. The #include directive doesn't help it. It should be
> > added, but until it is the following patch solves the problem:
>
> It's supposed to do better than that, I think.
> This, from dtc-lexer.l
>
> <*>^"#"(line)?{WS}+[0-9]+{WS}+{STRING}({WS}+[0-9]+)? {
> char *line, *tmp, *fn;
> /* skip text before line # */
> line = yytext;
> while (!isdigit(*line))
> line++;
> /* skip digits in line # */
> tmp = line;
> while (!isspace(*tmp))
> tmp++;
> /* "NULL"-terminate line # */
> *tmp = '\0';
> /* start of filename */
> fn = strchr(tmp + 1, '"') + 1;
> /* strip trailing " from filename */
> tmp = strchr(fn, '"');
> *tmp = 0;
> /* -1 since #line is the number of the next line */
> srcpos_set_line(xstrdup(fn), atoi(line) - 1);
> }
>
> Hrm. Is this a "that's not in the kernel's copy yet" problem?
> Or did this fail to match the offending '# <line> <file>' somehow?
> (Like, is that '# 375' really in column 1?)
The "#" is in the first column.
I've attached the actual file.
Ian.
On Fri, 2013-05-31 at 08:01 -0500, Jon Loeliger wrote:
> Hrm. Is this a "that's not in the kernel's copy yet" problem?
BTW I'm using dtc.git 4e76ec796c90d44d417f82d9db2d67cfe575f8ed and not
the kernel copy.
dtc-lexer.l in my HEAD is identical to the current master
(2e3fc7e9b3a4722a5500afaa9faf7874c61b2e6a) according to git diff.
Ian.
> >
> > Line 374 is the "IDSEL 0x16..." line here:
> > interrupt-map = <
> > /* IRQ mapping for pci slots and ALI M1533
> > ...
> > * management core also isn't used.
> > */
> >
> > /* IDSEL 0x16 / dev=6, bus=0 / PCI slot 3 */
> > 0x3000 0 0 1 &xps_intc_0 3 2
> > 0x3000 0 0 2 &xps_intc_0 2 2
> > 0x3000 0 0 3 &xps_intc_0 5 2
> > 0x3000 0 0 4 &xps_intc_0 4 2
Can you show me the original source without mods here, please?
Or is the "..." purely elided comments?
> > Which gets preprocessed into:
> > interrupt-map = <
> > # 375 "arch/powerpc/boot/dts/virtex440-ml510.dts"
> > 0x3000 0 0 1 &xps_intc_0 3 2
> > 0x3000 0 0 2 &xps_intc_0 2 2
> > 0x3000 0 0 3 &xps_intc_0 5 2
> > 0x3000 0 0 4 &xps_intc_0 4 2
> dtc is only able to track line numbers when the native /include/
> directive is used. The #include directive doesn't help it. It should be
> added, but until it is the following patch solves the problem:
It's supposed to do better than that, I think.
This, from dtc-lexer.l
<*>^"#"(line)?{WS}+[0-9]+{WS}+{STRING}({WS}+[0-9]+)? {
char *line, *tmp, *fn;
/* skip text before line # */
line = yytext;
while (!isdigit(*line))
line++;
/* skip digits in line # */
tmp = line;
while (!isspace(*tmp))
tmp++;
/* "NULL"-terminate line # */
*tmp = '\0';
/* start of filename */
fn = strchr(tmp + 1, '"') + 1;
/* strip trailing " from filename */
tmp = strchr(fn, '"');
*tmp = 0;
/* -1 since #line is the number of the next line */
srcpos_set_line(xstrdup(fn), atoi(line) - 1);
}
Hrm. Is this a "that's not in the kernel's copy yet" problem?
Or did this fail to match the offending '# <line> <file>' somehow?
(Like, is that '# 375' really in column 1?)
Thanks,
jdl
On 05/31/2013 05:48 AM, Grant Likely wrote:
> On Fri, 31 May 2013 11:29:30 +0100, Ian Campbell <[email protected]> wrote:
>> This affects arch/powerpc/boot/dts/virtex440-ml510.dts but I think it is
>> actually a more general issue:
>>
>> $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux- virtex440-ml510.dtb
>> CC scripts/mod/devicetable-offsets.s
>> GEN scripts/mod/devicetable-offsets.h
>> HOSTCC scripts/mod/file2alias.o
>> HOSTLD scripts/mod/modpost
>> DTC arch/powerpc/boot/virtex440-ml510.dtb
>> Error: arch/powerpc/boot/dts/virtex440-ml510.dts:374.6-7 syntax error
>> FATAL ERROR: Unable to parse input tree
>> make[1]: *** [arch/powerpc/boot/virtex440-ml510.dtb] Error 1
>> make: *** [virtex440-ml510.dtb] Error 2
>>
>> Line 374 is the "IDSEL 0x16..." line here:
>> interrupt-map = <
>> /* IRQ mapping for pci slots and ALI M1533
>> ...
>> * management core also isn't used.
>> */
>>
>> /* IDSEL 0x16 / dev=6, bus=0 / PCI slot 3 */
>> 0x3000 0 0 1 &xps_intc_0 3 2
>> 0x3000 0 0 2 &xps_intc_0 2 2
>> 0x3000 0 0 3 &xps_intc_0 5 2
>> 0x3000 0 0 4 &xps_intc_0 4 2
>>
>> Which gets preprocessed into:
>> interrupt-map = <
>> # 375 "arch/powerpc/boot/dts/virtex440-ml510.dts"
>> 0x3000 0 0 1 &xps_intc_0 3 2
>> 0x3000 0 0 2 &xps_intc_0 2 2
>> 0x3000 0 0 3 &xps_intc_0 5 2
>> 0x3000 0 0 4 &xps_intc_0 4 2
>>
>> If I manually remove the "# 375 " line then that fixes the error
>> (although there is then a subsequent one of the same type).
>>
>> I suppose this is a bug in dtc? It appears to have at least some
>> awareness of these preprocessor line number comments since it manages to
>> report the original source line number.
>
> dtc is only able to track line numbers when the native /include/
> directive is used. The #include directive doesn't help it. It should be
> added, but until it is the following patch solves the problem:
>
> I've got this patch in my tree. Either Rob or I will push it to Linus in
> the next few days.
>
> g.
>
> ---
> commit d01dccdcb3ea8233b09efb9c24db9f057fbd3b37
> Author: Grant Likely <[email protected]>
> Date: Fri May 31 12:45:18 2013 +0100
>
> dtc: Suppress cpp linemarker annotations
>
> DTC isn't able to parse cpp linemarker annotations, so suppress them in
> the cpp output by adding the -P flag to the cpp options.
That's not true; it explicitly does have code to parse the line markers.
I'll have to investigate why it isn't working in this case.
If you apply this patch, then anyone who has switched to #include rther
than /include/ will get incorrect line numbers in dtc error messages.
Admittedly that's a smaller population right now though. Perhaps we
should just do a kernel-wide conversion though.
On Fri, May 31, 2013 at 5:04 PM, Stephen Warren <[email protected]> wrote:
> On 05/31/2013 05:48 AM, Grant Likely wrote:
>> On Fri, 31 May 2013 11:29:30 +0100, Ian Campbell <[email protected]> wrote:
>>> This affects arch/powerpc/boot/dts/virtex440-ml510.dts but I think it is
>>> actually a more general issue:
>>>
>>> $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux- virtex440-ml510.dtb
>>> CC scripts/mod/devicetable-offsets.s
>>> GEN scripts/mod/devicetable-offsets.h
>>> HOSTCC scripts/mod/file2alias.o
>>> HOSTLD scripts/mod/modpost
>>> DTC arch/powerpc/boot/virtex440-ml510.dtb
>>> Error: arch/powerpc/boot/dts/virtex440-ml510.dts:374.6-7 syntax error
>>> FATAL ERROR: Unable to parse input tree
>>> make[1]: *** [arch/powerpc/boot/virtex440-ml510.dtb] Error 1
>>> make: *** [virtex440-ml510.dtb] Error 2
>>>
>>> Line 374 is the "IDSEL 0x16..." line here:
>>> interrupt-map = <
>>> /* IRQ mapping for pci slots and ALI M1533
>>> ...
>>> * management core also isn't used.
>>> */
>>>
>>> /* IDSEL 0x16 / dev=6, bus=0 / PCI slot 3 */
>>> 0x3000 0 0 1 &xps_intc_0 3 2
>>> 0x3000 0 0 2 &xps_intc_0 2 2
>>> 0x3000 0 0 3 &xps_intc_0 5 2
>>> 0x3000 0 0 4 &xps_intc_0 4 2
>>>
>>> Which gets preprocessed into:
>>> interrupt-map = <
>>> # 375 "arch/powerpc/boot/dts/virtex440-ml510.dts"
>>> 0x3000 0 0 1 &xps_intc_0 3 2
>>> 0x3000 0 0 2 &xps_intc_0 2 2
>>> 0x3000 0 0 3 &xps_intc_0 5 2
>>> 0x3000 0 0 4 &xps_intc_0 4 2
>>>
>>> If I manually remove the "# 375 " line then that fixes the error
>>> (although there is then a subsequent one of the same type).
>>>
>>> I suppose this is a bug in dtc? It appears to have at least some
>>> awareness of these preprocessor line number comments since it manages to
>>> report the original source line number.
>>
>> dtc is only able to track line numbers when the native /include/
>> directive is used. The #include directive doesn't help it. It should be
>> added, but until it is the following patch solves the problem:
>>
>> I've got this patch in my tree. Either Rob or I will push it to Linus in
>> the next few days.
>>
>> g.
>>
>> ---
>> commit d01dccdcb3ea8233b09efb9c24db9f057fbd3b37
>> Author: Grant Likely <[email protected]>
>> Date: Fri May 31 12:45:18 2013 +0100
>>
>> dtc: Suppress cpp linemarker annotations
>>
>> DTC isn't able to parse cpp linemarker annotations, so suppress them in
>> the cpp output by adding the -P flag to the cpp options.
>
> That's not true; it explicitly does have code to parse the line markers.
> I'll have to investigate why it isn't working in this case.
>
> If you apply this patch, then anyone who has switched to #include rther
> than /include/ will get incorrect line numbers in dtc error messages.
> Admittedly that's a smaller population right now though. Perhaps we
> should just do a kernel-wide conversion though.
My mistake. I tested the wrong thing. I've dropped the patch.
g.
On 05/31/2013 04:29 AM, Ian Campbell wrote:
> This affects arch/powerpc/boot/dts/virtex440-ml510.dts but I think it is
> actually a more general issue:
>
> $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux- virtex440-ml510.dtb
> CC scripts/mod/devicetable-offsets.s
> GEN scripts/mod/devicetable-offsets.h
> HOSTCC scripts/mod/file2alias.o
> HOSTLD scripts/mod/modpost
> DTC arch/powerpc/boot/virtex440-ml510.dtb
> Error: arch/powerpc/boot/dts/virtex440-ml510.dts:374.6-7 syntax error
> FATAL ERROR: Unable to parse input tree
> make[1]: *** [arch/powerpc/boot/virtex440-ml510.dtb] Error 1
> make: *** [virtex440-ml510.dtb] Error 2
...
> interrupt-map = <
> # 375 "arch/powerpc/boot/dts/virtex440-ml510.dts"
> 0x3000 0 0 1 &xps_intc_0 3 2
> 0x3000 0 0 2 &xps_intc_0 2 2
> 0x3000 0 0 3 &xps_intc_0 5 2
> 0x3000 0 0 4 &xps_intc_0 4 2
I /think/ what's happening here is that dtc's rule for #line parsing
allows the formats:
# LINE FILENAME
or:
#LINE FILENAME FLAGS
where FLAGS is an integer
The lexer rule that optionally consumes flags requires some WS
(white-space) between the filename and flags. It looks like this
whitespace can actually cross a line boundary, so it ends up consuming
the first "0" of the hex constant on the next line, which then leaves
"x3000" to be parsed as cell data, which is a syntax error.
You can hack around this for testing by doing one of a few things:
a) Add an extra integer on to the end of the problematic #line directives.
b) Remove the leading "0x" from the constants following the #line
directives. I think this will still mean dtc eats the 3000 as part of
the #line directive, but hides the syntax error.
The solution here is to make dtc's #line matching regex:
<*>^"#"(line)?{WS}+[0-9]+{WS}+{STRING}({WS}+[0-9]+)? {
... use someting other than {WS} in the final instance; some kind of
{WS} that won't match/cross line boundaries. I'll see if I can cook up a
patch for this.
On Fri, 2013-05-31 at 17:31 +0100, Grant Likely wrote:
> On Fri, May 31, 2013 at 5:04 PM, Stephen Warren <[email protected]> wrote:
> > On 05/31/2013 05:48 AM, Grant Likely wrote:
> >> ---
> >> commit d01dccdcb3ea8233b09efb9c24db9f057fbd3b37
> >> Author: Grant Likely <[email protected]>
> >> Date: Fri May 31 12:45:18 2013 +0100
> >>
> >> dtc: Suppress cpp linemarker annotations
> >>
> >> DTC isn't able to parse cpp linemarker annotations, so suppress them in
> >> the cpp output by adding the -P flag to the cpp options.
> >
> > That's not true; it explicitly does have code to parse the line markers.
> > I'll have to investigate why it isn't working in this case.
> >
> > If you apply this patch, then anyone who has switched to #include rther
> > than /include/ will get incorrect line numbers in dtc error messages.
> > Admittedly that's a smaller population right now though. Perhaps we
> > should just do a kernel-wide conversion though.
>
> My mistake. I tested the wrong thing. I've dropped the patch.
FWIW I've also dropped it from my device-tree.git tree.
Ian.