Re: Bug in drivers/serial/of_serial.c?

[Grant Likely <grant.likely@secretlab.ca>]

[fixed top-posting nonsense  :-) ]

On Thu, Nov 19, 2009 at 10:22 AM, Stephen Neuendorffer
<stephen.neuendorffer@xilinx.com> wrote:
>> diff --git a/drivers/serial/of_serial.c b/drivers/serial/of_serial.c
>> index 02406ba..241be77 100644
>> --- a/drivers/serial/of_serial.c
>> +++ b/drivers/serial/of_serial.c
>> @@ -161,7 +161,9 @@ static int of_platform_serial_remove(struct
>> of_device *ofdev)
>>  static struct of_device_id __devinitdata of_platform_serial_table[] =
> {
>>       { .type = "serial", .compatible = "ns8250",   .data = (void
>> *)PORT_8250, },
>>       { .type = "serial", .compatible = "ns16450",  .data = (void
>> *)PORT_16450, },
>> +     { .type = "serial", .compatible = "xlnx,xps-uart16550-2.00.b",
>> .data = (void *)PORT_16550A, },
>>       { .type = "serial", .compatible = "ns16550",  .data = (void
>> *)PORT_16550, },
>> +     { .type = "serial", .compatible = "ns16550a", .data = (void
>> *)PORT_16550A, },
>>       { .type = "serial", .compatible = "ns16750",  .data = (void
>> *)PORT_16750, },
>>       { .type = "serial", .compatible = "ns16850",  .data = (void
>> *)PORT_16850, },
>>  #ifdef CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL
>
> NAK.

No, I wouldn't NAK this.  It is an appropriate fix to still support
older trees that use the ns16550 value.  Arnd, I've got no problem
with this being merged.

g.

>
> If the problem is in the device trees that are being generated, we
> should fix the issue there.
> We've been trying to avoid putting the fully specified IP versions in
> the kernel like this, since
> the IP changes so often.

Yes, the error is in the device tree generator... which is *exactly*
why the fully specified IP version is in the device tree; so that a
driver can work around bugs like this.  You're right that we avoid
specifying it to eliminate an explosion of xlnx,<device><version>
strings in the kernel; but it is perfectly safe to use in situations
like this.

As you also know, but it is worth stating here for the record, this is
also exactly why newer versions of the IP claim compatibility with
fully specified versions of older IP.  So that the list of strings
doesn't explode, but also so that it claims compatibility with a real
version of the device (not a made up string whose meaning can change
over time).

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.