Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.

Sebastian Hesselbarth [off-list ref] wrote on 
02/12/2014 10:46:36 AM:

From: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
To: Scott Wood <redacted>
Cc: Kumar Gala <redacted>, Stephen N Chivers 
[off-list ref], Chris Proctor [off-list ref], 
linuxppc-dev@lists.ozlabs.org, Arnd Bergmann [off-list ref], 
devicetree [off-list ref]
Date: 02/12/2014 11:04 AM
Subject: Re: Linux-3.14-rc2: Order of serial node compatibles in DTS 

files.

On 02/12/2014 12:41 AM, Scott Wood wrote:

quoted

On Tue, 2014-02-11 at 23:51 +0100, Sebastian Hesselbarth wrote:

quoted

On 02/11/2014 11:33 PM, Kumar Gala wrote:

quoted

Hmm,

Wondering if this caused the issue:

commit 105353145eafb3ea919f5cdeb652a9d8f270228e
Author: Sebastian Hesselbarth [off-list ref]
Date:   Tue Dec 3 14:52:00 2013 +0100

      OF: base: match each node compatible against all given matches 

first

quoted

quoted

[adding Arnd on Cc]

Could be. I checked tty/serial/of_serial.c and it does not provide a
compatible for "fsl,ns16550". Does reverting the patch fix the issue
observed?

I don't think the missing compatible is causing it, but of_serial
provides a DT match for .type = "serial" just to fail later on
with the error seen above.

The commit in question reorders of_match_device in a way that match
table order is not relevant anymore. This can cause it to match
.type = "serial" first here.

Rather than touching the commit, I suggest to remove the problematic
.type = "serial" from the match table. It is of no use anyway.

Regardless of whether .type = "serial" gets removed, it seems wrong 

for

quoted

of_match_node() to accept a .type-only match (or .name, or anything 

else

quoted

that doesn't involve .compatible) before it accepts a compatible match
other than the first in the compatible property.

Right, I thought about it and came to the same conclusion. I sent a
patch a second ago to prefer .compatible != NULL matches over those
with .compatible == NULL.

Would be great if Stephen can re-test that. If it solves the issue, I
can send a patch tomorrow.

Done.

But, the Interrupt Controller (MPIC)
goes AWOL and it is down hill from there.

The MPIC is specified in the DTS as:

        mpic: pic@40000 {
                        interrupt-controller;
                        #address-cells = <0>;
                        #interrupt-cells = <2>;
                        reg = <0x40000 0x40000>;
                        compatible = "chrp,open-pic";
                        device_type = "open-pic";
                        big-endian;
                };

The board support file has the standard mechanism for allocating
the PIC:

        struct mpic *mpic;

        mpic = mpic_alloc(NULL, 0, 0, 0, 256, " OpenPIC  ");
        BUG_ON(mpic == NULL);

        mpic_init(mpic);

I checked for damage in applying the patch and it has applied
correctly.

Stephen Chivers,
CSC Australia Pty. Ltd.