Re: [PATCH 1/2] ARM: Tegra: dt: Split out separate Tegra SoC DT

[Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>]

On Mon, May 2, 2011 at 12:00 PM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> Olof Johansson wrote at Sunday, May 01, 2011 8:56 AM:
>> On Fri, Apr 29, 2011 at 10:12:30PM -0600, Stephen Warren wrote:
>> > tegra250.dts describes everything within the Tegra SoC, but leaves all
>> > optional devices disabled.
>> >
>> > tegra-harmony.dts is now the configuration of the SoC for the board,
>> > i.e. it enables the optional devices that are required for the board,
>> > and provides any required platform data for those devics.
>> >
>> > The prevents every Tegra board having to repeat all the basic internal
>> > SoC wiring.
>> >
>> > Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>
>> Acked-by: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
>>
>> A mostly unrelated question below.
>>
>> [...]
>>
>> > +   serial@70006000 {
>> > +           compatible = "nvidia,tegra250-uart";
>>
>> I know this is how Grant specified it, but shouldn't these also have
>> a compat for ns16550?
>
> At present, I'm not sure it is technically compatible. If you look at
> drivers/tty/serial/of_serial.c, you'll see:
>
> static struct of_device_id __devinitdata of_platform_serial_table[] = {
> ...
>        { .compatible = "ns16550",  .data = (void *)PORT_16550, },
> ...
>        { .compatible = "nvidia,tegra250-uart",  .data = (void *)PORT_XSCALE, },
>
> That PORT_XSCALE is different to the ns16550 entry. Arguably, that
> field should be something that comes from the device tree, just
> like e.g. reg-shift, but it certainly doesn't right now.
>
> Grant, what are your thoughts on this?

Heh, when I added that line I just mirrored the 'type' of 16550 as
detected by the 8250.c driver when it probes and made a mental note
that it should be revisited.  I don't know the hardware very well, so
I cannot easily say what the right thing to do is, but making it
ns16550 compatible does seem to make sense.

g.