Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
On 02/12/2014 01:21 AM, Stephen N Chivers wrote:
Sebastian Hesselbarth sebastian.hesselba...@gmail.com wrote on
02/12/2014 10:46:36 AM:
From: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
To: Scott Wood scottw...@freescale.com
Cc: Kumar Gala ga...@kernel.crashing.org, Stephen N Chivers
schiv...@csc.com.au, Chris Proctor cproc...@csc.com.au,
linuxppc-dev@lists.ozlabs.org, Arnd Bergmann a...@arndb.de,
devicetree devicet...@vger.kernel.org
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:
Regardless of whether .type = serial gets removed, it seems wrong for
of_match_node() to accept a .type-only match (or .name, or anything else
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@4 {
interrupt-controller;
#address-cells = 0;
#interrupt-cells = 2;
reg = 0x4 0x4;
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.
Hmm, I did a mistake in the patch:
- while (m-name[0] || m-type[0]) {
+ while (m-compatible[0] || m-name[0] || m-type[0]) {
for the second added match. Otherwise, the matches are not
evaluated down to the sentinel but the loop quits on the first
match table entry without .name and .type set.
Sebastian
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
On 02/12/2014 06:28 AM, Kevin Hao wrote:
On Wed, Feb 12, 2014 at 10:21:58AM +1000, Stephen N Chivers wrote:
But, the Interrupt Controller (MPIC)
goes AWOL and it is down hill from there.
The MPIC is specified in the DTS as:
mpic: pic@4 {
interrupt-controller;
#address-cells = 0;
#interrupt-cells = 2;
reg = 0x4 0x4;
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.
How about the following fix?
diff --git a/drivers/of/base.c b/drivers/of/base.c
index ff85450d5683..ca91984d3c4b 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -730,32 +730,40 @@ out:
}
EXPORT_SYMBOL(of_find_node_with_property);
+static int of_match_type_name(const struct device_node *node,
+ const struct of_device_id *m)
I am fine with having a sub-function here, but it should rather be
named of_match_type_or_name.
+{
+ int match = 1;
+
+ if (m-name[0])
+ match = node-name !strcmp(m-name, node-name);
+
+ if (m-type[0])
+ match = node-type !strcmp(m-type, node-type);
+
+ return match;
+}
[...]
+ /* Check against matches without compatible string */
+ m = matches;
+ while (!m-compatible[0] (m-name[0] || m-type[0])) {
We shouldn't check for anything else than the sentinel here.
Although I guess yours will not quit early as mine did but that
way we don't have to worry about it.
Sebastian
+ match = of_match_type_name(node, m);
+ if (match)
+ return m;
+ m++;
+ }
+
return NULL;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
On Wed, Feb 12, 2014 at 09:30:00AM +0100, Sebastian Hesselbarth wrote:
On 02/12/2014 06:28 AM, Kevin Hao wrote:
On Wed, Feb 12, 2014 at 10:21:58AM +1000, Stephen N Chivers wrote:
But, the Interrupt Controller (MPIC)
goes AWOL and it is down hill from there.
The MPIC is specified in the DTS as:
mpic: pic@4 {
interrupt-controller;
#address-cells = 0;
#interrupt-cells = 2;
reg = 0x4 0x4;
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.
How about the following fix?
diff --git a/drivers/of/base.c b/drivers/of/base.c
index ff85450d5683..ca91984d3c4b 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -730,32 +730,40 @@ out:
}
EXPORT_SYMBOL(of_find_node_with_property);
+static int of_match_type_name(const struct device_node *node,
+const struct of_device_id *m)
I am fine with having a sub-function here, but it should rather be
named of_match_type_or_name.
OK.
+{
+int match = 1;
+
+if (m-name[0])
+match = node-name !strcmp(m-name, node-name);
+
+if (m-type[0])
+match = node-type !strcmp(m-type, node-type);
+
+return match;
+}
[...]
+/* Check against matches without compatible string */
+m = matches;
+while (!m-compatible[0] (m-name[0] || m-type[0])) {
We shouldn't check for anything else than the sentinel here.
Although I guess yours will not quit early as mine did but that
way we don't have to worry about it.
Yes, this is still buggy. I will change something like this:
m = matches;
/* Check against matches without compatible string */
while (m-name[0] || m-type[0] || m-compatible[0]) {
if (m-compatible[0]) {
m++;
continue;
}
match = of_match_type_name(node, m);
if (match)
return m;
m++;
}
Thanks,
Kevin
pgps8ftEWKrhl.pgp
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
On Wed, Feb 12, 2014 at 09:25:24AM +0100, Sebastian Hesselbarth wrote:
Hmm, I did a mistake in the patch:
- while (m-name[0] || m-type[0]) {
+ while (m-compatible[0] || m-name[0] || m-type[0]) {
for the second added match. Otherwise, the matches are not
evaluated down to the sentinel but the loop quits on the first
match table entry without .name and .type set.
But this is still not right. We also need to skip the matches with
.compatible here.
Thanks,
Kevin
Sebastian
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
pgpwFO4K17beE.pgp
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
On Wednesday 12 February 2014, Sebastian Hesselbarth wrote: On 02/12/2014 12:38 AM, Stephen N Chivers wrote: Sebastian Hesselbarth sebastian.hesselba...@gmail.com wrote on 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. Deleting the serial line from the match table fixes the problem. I tested it for both orderings of compatible. I revert my statement about removing anything from of_serial.c. Instead we should try to prefer matches with compatibles over type/name without compatibles. Something like the patch below (compile tested only) That would probably be a good idea. However, I think in this case we also want to remove the line from the driver, as it clearly never works on any hardware and the driver just errors out for the device_type match. Arnd ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
On 02/12/14 11:31, Kevin Hao wrote:
On Wed, Feb 12, 2014 at 09:30:00AM +0100, Sebastian Hesselbarth wrote:
On 02/12/2014 06:28 AM, Kevin Hao wrote:
On Wed, Feb 12, 2014 at 10:21:58AM +1000, Stephen N Chivers wrote:
But, the Interrupt Controller (MPIC)
goes AWOL and it is down hill from there.
The MPIC is specified in the DTS as:
mpic: pic@4 {
interrupt-controller;
#address-cells = 0;
#interrupt-cells = 2;
reg = 0x4 0x4;
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.
How about the following fix?
diff --git a/drivers/of/base.c b/drivers/of/base.c
index ff85450d5683..ca91984d3c4b 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -730,32 +730,40 @@ out:
}
EXPORT_SYMBOL(of_find_node_with_property);
+static int of_match_type_name(const struct device_node *node,
+ const struct of_device_id *m)
I am fine with having a sub-function here, but it should rather be
named of_match_type_or_name.
OK.
+{
+ int match = 1;
+
+ if (m-name[0])
+ match = node-name !strcmp(m-name, node-name);
+
+ if (m-type[0])
+ match = node-type !strcmp(m-type, node-type);
+
+ return match;
+}
[...]
+ /* Check against matches without compatible string */
+ m = matches;
+ while (!m-compatible[0] (m-name[0] || m-type[0])) {
We shouldn't check for anything else than the sentinel here.
Although I guess yours will not quit early as mine did but that
way we don't have to worry about it.
Yes, this is still buggy. I will change something like this:
m = matches;
/* Check against matches without compatible string */
while (m-name[0] || m-type[0] || m-compatible[0]) {
if (m-compatible[0]) {
m++;
continue;
}
match = of_match_type_name(node, m);
if (match)
return m;
m++;
}
You can cook it down to:
m = matches;
/* Check against matches without compatible string */
while (m-name[0] || m-type[0] || m-compatible[0]) {
if (!m-compatible[0] of_match_type_or_name(node, m)
return m;
m++;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
On Wed, Feb 12, 2014 at 12:26:14PM +0100, Sebastian Hesselbarth wrote:
You can cook it down to:
m = matches;
/* Check against matches without compatible string */
while (m-name[0] || m-type[0] || m-compatible[0]) {
if (!m-compatible[0] of_match_type_or_name(node, m)
return m;
m++;
}
Will do.
Thanks,
Kevin
pgpEyfZ_1f1QZ.pgp
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Linux-3.14-rc2: Order of serial node compatibles in DTS files.
I have been trial booting a 3.14-rc2 kernel for a 85xx platform
(dtbImage).
After mounting the root filesystem there are no messages from the init
scripts
and the serial console is not available for login.
In the kernel log messages there is:
of_serial f1004500.serial: Unknown serial port found, ignored.
The serial nodes in boards dts file are specified as:
serial0: serial@4500 {
cell-index = 0;
device_type = serial;
compatible = fsl,ns16550, ns16550;
reg = 0x4500 0x100;
clock-frequency = 0;
interrupts = 0x2a 0x2;
interrupt-parent = mpic;
};
Reversing the order of the compatible:
compatible = ns16550, fsl,ns16550;
restores the serial console.
Linux-3.13 does not have this behaviour.
There are 49 dts files in Linux-3.14-rc2 that have the fsl,ns16550
compatible first.
Stephen Chivers,
CSC Australia Pty. Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
On Feb 11, 2014, at 2:57 PM, Stephen N Chivers schiv...@csc.com.au wrote:
I have been trial booting a 3.14-rc2 kernel for a 85xx platform
(dtbImage).
After mounting the root filesystem there are no messages from the init
scripts
and the serial console is not available for login.
In the kernel log messages there is:
of_serial f1004500.serial: Unknown serial port found, ignored.
The serial nodes in boards dts file are specified as:
serial0: serial@4500 {
cell-index = 0;
device_type = serial;
compatible = fsl,ns16550, ns16550;
reg = 0x4500 0x100;
clock-frequency = 0;
interrupts = 0x2a 0x2;
interrupt-parent = mpic;
};
Reversing the order of the compatible:
compatible = ns16550, fsl,ns16550;
restores the serial console.
Linux-3.13 does not have this behaviour.
There are 49 dts files in Linux-3.14-rc2 that have the fsl,ns16550
compatible first.
Hmm,
Wondering if this caused the issue:
commit 105353145eafb3ea919f5cdeb652a9d8f270228e
Author: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
Date: Tue Dec 3 14:52:00 2013 +0100
OF: base: match each node compatible against all given matches first
- k
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
On 02/11/2014 11:33 PM, Kumar Gala wrote:
On Feb 11, 2014, at 2:57 PM, Stephen N Chivers schiv...@csc.com.au wrote:
I have been trial booting a 3.14-rc2 kernel for a 85xx platform
(dtbImage).
After mounting the root filesystem there are no messages from the init
scripts
and the serial console is not available for login.
In the kernel log messages there is:
of_serial f1004500.serial: Unknown serial port found, ignored.
The serial nodes in boards dts file are specified as:
serial0: serial@4500 {
cell-index = 0;
device_type = serial;
compatible = fsl,ns16550, ns16550;
reg = 0x4500 0x100;
clock-frequency = 0;
interrupts = 0x2a 0x2;
interrupt-parent = mpic;
};
Reversing the order of the compatible:
compatible = ns16550, fsl,ns16550;
restores the serial console.
Linux-3.13 does not have this behaviour.
There are 49 dts files in Linux-3.14-rc2 that have the fsl,ns16550
compatible first.
Hmm,
Wondering if this caused the issue:
commit 105353145eafb3ea919f5cdeb652a9d8f270228e
Author: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
Date: Tue Dec 3 14:52:00 2013 +0100
OF: base: match each node compatible against all given matches first
[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.
Sebastian
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
Sebastian Hesselbarth sebastian.hesselba...@gmail.com wrote on
02/12/2014 09:51:43 AM:
From: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
To: Kumar Gala ga...@kernel.crashing.org, Stephen N Chivers
schiv...@csc.com.au
Cc: linuxppc-dev@lists.ozlabs.org, Chris Proctor
cproc...@csc.com.au, devicetree devicet...@vger.kernel.org, Arnd
Bergmann a...@arndb.de
Date: 02/12/2014 09:51 AM
Subject: Re: Linux-3.14-rc2: Order of serial node compatibles in DTS
files.
On 02/11/2014 11:33 PM, Kumar Gala wrote:
On Feb 11, 2014, at 2:57 PM, Stephen N Chivers schiv...@csc.com.au
wrote:
I have been trial booting a 3.14-rc2 kernel for a 85xx platform
(dtbImage).
After mounting the root filesystem there are no messages from the
init
scripts
and the serial console is not available for login.
In the kernel log messages there is:
of_serial f1004500.serial: Unknown serial port found, ignored.
The serial nodes in boards dts file are specified as:
serial0: serial@4500 {
cell-index = 0;
device_type = serial;
compatible = fsl,ns16550, ns16550;
reg = 0x4500 0x100;
clock-frequency = 0;
interrupts = 0x2a 0x2;
interrupt-parent = mpic;
};
Reversing the order of the compatible:
compatible = ns16550, fsl,ns16550;
restores the serial console.
Linux-3.13 does not have this behaviour.
There are 49 dts files in Linux-3.14-rc2 that have the fsl,ns16550
compatible first.
Hmm,
Wondering if this caused the issue:
commit 105353145eafb3ea919f5cdeb652a9d8f270228e
Author: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
Date: Tue Dec 3 14:52:00 2013 +0100
OF: base: match each node compatible against all given matches
first
[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.
Deleting the serial line from the match table fixes the problem.
I tested it for both orderings of compatible.
Sebastian
Thanks,
Stephen Chivers,
CSC Australia Pty. Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
On Tue, 2014-02-11 at 23:51 +0100, Sebastian Hesselbarth wrote: On 02/11/2014 11:33 PM, Kumar Gala wrote: Hmm, Wondering if this caused the issue: commit 105353145eafb3ea919f5cdeb652a9d8f270228e Author: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Date: Tue Dec 3 14:52:00 2013 +0100 OF: base: match each node compatible against all given matches first [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 of_match_node() to accept a .type-only match (or .name, or anything else that doesn't involve .compatible) before it accepts a compatible match other than the first in the compatible property. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
On 02/12/2014 12:38 AM, Stephen N Chivers wrote:
Sebastian Hesselbarth sebastian.hesselba...@gmail.com wrote on
On 02/11/2014 11:33 PM, Kumar Gala wrote:
On Feb 11, 2014, at 2:57 PM, Stephen N Chivers schiv...@csc.com.au wrote:
I have been trial booting a 3.14-rc2 kernel for a 85xx platform
(dtbImage).
[...]
of_serial f1004500.serial: Unknown serial port found, ignored.
The serial nodes in boards dts file are specified as:
serial0: serial@4500 {
cell-index = 0;
device_type = serial;
compatible = fsl,ns16550, ns16550;
reg = 0x4500 0x100;
clock-frequency = 0;
interrupts = 0x2a 0x2;
interrupt-parent = mpic;
};
Wondering if this caused the issue:
commit 105353145eafb3ea919f5cdeb652a9d8f270228e
Author: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
Date: Tue Dec 3 14:52:00 2013 +0100
OF: base: match each node compatible against all given matches first
[...]
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.
Deleting the serial line from the match table fixes the problem.
I tested it for both orderings of compatible.
I revert my statement about removing anything from of_serial.c. Instead
we should try to prefer matches with compatibles over type/name without
compatibles. Something like the patch below (compile tested only)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index ff85450d5683..60da53b385ff 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -734,6 +734,7 @@ static
const struct of_device_id *__of_match_node(const struct of_device_id *matches,
const struct device_node *node)
{
+ const struct of_device_id *m;
const char *cp;
int cplen, l;
@@ -742,15 +743,15 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches,
cp = __of_get_property(node, compatible, cplen);
do {
- const struct of_device_id *m = matches;
+ m = matches;
/* Check against matches with current compatible string */
while (m-name[0] || m-type[0] || m-compatible[0]) {
int match = 1;
- if (m-name[0])
+ if (m-name[0] m-compatible[0])
match = node-name
!strcmp(m-name, node-name);
- if (m-type[0])
+ if (m-type[0] m-compatible[0])
match = node-type
!strcmp(m-type, node-type);
if (m-compatible[0])
@@ -770,6 +771,21 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches,
}
} while (cp (cplen 0));
+ /* Check against matches without compatible string */
+ m = matches;
+ while (m-name[0] || m-type[0]) {
+ int match = 1;
+ if (m-name[0])
+ match = node-name
+ !strcmp(m-name, node-name);
+ if (m-type[0])
+ match = node-type
+ !strcmp(m-type, node-type);
+ if (match)
+ return m;
+ m++;
+ }
+
return NULL;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
On 02/12/2014 12:41 AM, Scott Wood wrote: On Tue, 2014-02-11 at 23:51 +0100, Sebastian Hesselbarth wrote: On 02/11/2014 11:33 PM, Kumar Gala wrote: Hmm, Wondering if this caused the issue: commit 105353145eafb3ea919f5cdeb652a9d8f270228e Author: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Date: Tue Dec 3 14:52:00 2013 +0100 OF: base: match each node compatible against all given matches first [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 of_match_node() to accept a .type-only match (or .name, or anything else 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. Sebastian ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
Sebastian Hesselbarth sebastian.hesselba...@gmail.com wrote on
02/12/2014 10:46:36 AM:
From: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
To: Scott Wood scottw...@freescale.com
Cc: Kumar Gala ga...@kernel.crashing.org, Stephen N Chivers
schiv...@csc.com.au, Chris Proctor cproc...@csc.com.au,
linuxppc-dev@lists.ozlabs.org, Arnd Bergmann a...@arndb.de,
devicetree devicet...@vger.kernel.org
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:
On Tue, 2014-02-11 at 23:51 +0100, Sebastian Hesselbarth wrote:
On 02/11/2014 11:33 PM, Kumar Gala wrote:
Hmm,
Wondering if this caused the issue:
commit 105353145eafb3ea919f5cdeb652a9d8f270228e
Author: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
Date: Tue Dec 3 14:52:00 2013 +0100
OF: base: match each node compatible against all given matches
first
[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
of_match_node() to accept a .type-only match (or .name, or anything
else
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@4 {
interrupt-controller;
#address-cells = 0;
#interrupt-cells = 2;
reg = 0x4 0x4;
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.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
On Wed, Feb 12, 2014 at 10:21:58AM +1000, Stephen N Chivers wrote:
But, the Interrupt Controller (MPIC)
goes AWOL and it is down hill from there.
The MPIC is specified in the DTS as:
mpic: pic@4 {
interrupt-controller;
#address-cells = 0;
#interrupt-cells = 2;
reg = 0x4 0x4;
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.
How about the following fix?
diff --git a/drivers/of/base.c b/drivers/of/base.c
index ff85450d5683..ca91984d3c4b 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -730,32 +730,40 @@ out:
}
EXPORT_SYMBOL(of_find_node_with_property);
+static int of_match_type_name(const struct device_node *node,
+ const struct of_device_id *m)
+{
+ int match = 1;
+
+ if (m-name[0])
+ match = node-name !strcmp(m-name, node-name);
+
+ if (m-type[0])
+ match = node-type !strcmp(m-type, node-type);
+
+ return match;
+}
+
static
const struct of_device_id *__of_match_node(const struct of_device_id *matches,
const struct device_node *node)
{
const char *cp;
int cplen, l;
+ const struct of_device_id *m;
+ int match;
if (!matches)
return NULL;
cp = __of_get_property(node, compatible, cplen);
do {
- const struct of_device_id *m = matches;
+ m = matches;
/* Check against matches with current compatible string */
- while (m-name[0] || m-type[0] || m-compatible[0]) {
- int match = 1;
- if (m-name[0])
- match = node-name
-!strcmp(m-name, node-name);
- if (m-type[0])
- match = node-type
-!strcmp(m-type, node-type);
- if (m-compatible[0])
- match = cp
-!of_compat_cmp(m-compatible, cp,
+ while (m-compatible[0]) {
+ match = of_match_type_name(node, m);
+ match = cp !of_compat_cmp(m-compatible, cp,
strlen(m-compatible));
if (match)
return m;
@@ -770,6 +778,15 @@ const struct of_device_id *__of_match_node(const struct
of_device_id *matches,
}
} while (cp (cplen 0));
+ /* Check against matches without compatible string */
+ m = matches;
+ while (!m-compatible[0] (m-name[0] || m-type[0])) {
+ match = of_match_type_name(node, m);
+ if (match)
+ return m;
+ m++;
+ }
+
return NULL;
}
Thanks,
Kevin
Stephen Chivers,
CSC Australia Pty. Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
pgp8VuWp0SQuQ.pgp
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
