Bug in drivers/serial/of_serial.c?

[Grant Likely]

On Fri, Nov 20, 2009 at 3:11 PM, John Linn <John.Linn at xilinx.com> wrote:
>
>> -----Original Message-----
>> From: glikely at secretlab.ca [mailto:glikely at secretlab.ca] On Behalf Of Grant Likely
>> Sent: Friday, November 20, 2009 2:58 PM
>> To: Stephen Neuendorffer
>> Cc: Arnd Bergmann; John Linn; Alon Ziv; linuxppc-dev at lists.ozlabs.org
>> Subject: Re: Bug in drivers/serial/of_serial.c?
>>
>> On Thu, Nov 19, 2009 at 10:42 AM, Stephen Neuendorffer
>> <stephen.neuendorffer at xilinx.com> wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: linuxppc-dev-bounces+stephen=neuendorffer.name at lists.ozlabs.org
>> > [mailto:linuxppc-dev-
>> >> bounces+stephen=neuendorffer.name at lists.ozlabs.org] On Behalf Of Arnd
>> > Bergmann
>> >> Sent: Thursday, November 19, 2009 9:33 AM
>> >> To: Stephen Neuendorffer
>> >> Cc: John Linn; Alon Ziv; linuxppc-dev at lists.ozlabs.org
>> >> Subject: Re: Bug in drivers/serial/of_serial.c?
>> >>
>> >> On Thursday 19 November 2009, Stephen Neuendorffer wrote:
>> >> > 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.
>> >>
>> >> No, the problem that Alon has is that the firmware currently has no
>> >> way whatsoever to give a correct device tree, because of-serial.c
>> >> does not even know about ns16550a.
>> >>
>> >> The patch adds both a special-case for the specific uart he
>> >> is using so that one is grandfathered in and a new compatible
>> >> value so future boards can specify both ns16550a and ns16550.
>> >
>> > That's true...  The 16550a line still needs to get added, but not the
>> > xlnx-
>> > specific line.
>>
>> The xlnx- line should be added and is entirely appropriate for exactly
>> the reason Arnd stated.
>
> I'm just wanting to make sure I'm on the same page...
>
> It seems like you're saying that adding this line fixes the system before any device tree generator fix, for existing systems, true?
>
> If that's true, how do you know that the 16550 in the xilinx system is not a 16450 as that's the default?
>
> Right now we're not generating the ns16450 as we should be when there are no FIFOs.  Since we've been using 16550 and not 16550A it probably hasn't been a problem.

Hmmm, right.  I forgot that we talked about that on the phone.  Yes,
in that case it is not a good idea to add the
xlnx,xps-uart16550-2.00.b because we don't know if it is configured as
a 16450 or a 16550.  At least, if the xlnx,... match is added, then
the driver *also* needs to look for the value of the "xlnx,is-a-16550"
property.

BTW, while looking at that file... Arnd, does the .type = "serial"
stuff really need to be there?

Cheers,
g.

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