diff mbox

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

Message ID 52FAB5A7.7080208@gmail.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Sebastian Hesselbarth Feb. 11, 2014, 11:43 p.m. UTC 
On 02/12/2014 12:38 AM, Stephen N Chivers wrote:
> Sebastian Hesselbarth <sebastian.hesselbarth@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 <schivers@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.hesselbarth@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)

Comments

Arnd Bergmann Feb. 12, 2014, 11 a.m. UTC | #1 
On Wednesday 12 February 2014, Sebastian Hesselbarth wrote:
> On 02/12/2014 12:38 AM, Stephen N Chivers wrote:
> > Sebastian Hesselbarth <sebastian.hesselbarth@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
diff mbox

Patch

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;
 }