lkml.org 
[lkml]   [2013]   [Apr]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [RFC PATCH arm: initial TI-Nspire support]
Date
On Sunday 07 April 2013, Daniel Tang wrote:
> On 07/04/2013, at 12:24 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >
> >> @@ -313,7 +314,7 @@ define archhelp
> >> echo ' Image - Uncompressed kernel image (arch/$(ARCH)/boot/Image)'
> >> echo '* xipImage - XIP kernel image, if configured (arch/$(ARCH)/boot/xipImage)'
> >> echo ' uImage - U-Boot wrapped zImage'
> >> - echo ' bootpImage - Combined zImage and initial RAM disk'
> >> + echo ' bootpImage - Combined zImage and initial RAM disk'
> >> echo ' (supply initrd image via make variable INITRD=<path>)'
> >> echo '* dtbs - Build device tree blobs for enabled boards'
> >> echo ' install - Install uncompressed kernel'
> >
> > This looks like it wasn't meant to be in the patch.
>
> It probably isn't. I think there was trailing whitespace on that and my editor happened to remove it automatically.
>
> Should this be a separate patch to fix up formatting or should I leave it in as a drive-by fix?

Any cleanups like this should be separate patches, and ideally even
part of a different patch series.

> >
> >> + uart: uart@90020000 {
> >> + reg = <0x90020000 0x1000>;
> >> + interrupts = <1>;
> >> +
> >> + clocks = <&uart_clk>;
> >> + clock-names = "uart_clk";
> >> + };
> >
> > The name for a uart should be "serial". Since this is a pl01x, please add
> > the required properties for the device, e.g.
> >
> > compatible = "arm,pl011", "arm,primecell";
> >
> > You will need the "arm,primecell" bit to make the device appear on the
> > amba bus rather than the platform bus.
>
> That was actually deliberate because different models of the TI-NSPIRE have different
> serial hardware. On the newer CX models, it is a PL01x and on the older models, it has
> a 8250-like interface. They all reside at the same address with the same IRQ though.
>
> I thought it might be cleaner to specify the interrupts and registers in the common file
> and leave it to the board specific ones to implement the "compatible" property.

I see. It seems a little confusing to the reader though. I don't have a good answer,
but there are two other options how this can be handled:

* Put both devices in the devicetree and mark them as status="disabled" in the main file,
but enable one of them in the version specific files.

* leave them out of the .dtsi file and only define them in the specific .dts files.
> >> + err = of_property_read_string(of_aliases, "timer0", &path);
> >> + if (WARN_ON(err))
> >> + return;
> >> +
> >> + timer = of_find_node_by_path(path);
> >> + base = of_iomap(timer, 0);
> >> + if (WARN_ON(!base))
> >> + return;
> >> +
> >> + clk = of_clk_get_by_name(timer, NULL);
> >> + clk_register_clkdev(clk, timer->name, "sp804");
> >> +
> >> + sp804_clocksource_init(base, timer->name);
> >> +
> >> + err = of_property_read_string(of_aliases, "timer1", &path);
> >> + if (WARN_ON(err))
> >> + return;
> >
> > In particular, I think the method of using aliases to pick the right sp804
> > instance is being deprecated now. If both timers are identical, the kernel
> > will now just pick one of them.
>
> Sorry, I don't quite understand.
>
> Out of the timers, I want to add one as a clocksource and one as a clockevent.
> If they're identical (i.e. without using aliases), how should I tell the kernel,
> "Take the first timer you see and make it a clocksource, take the next one you
> see and make it a clockevent"?

The modified sp804 driver will have logic to do that. I think in the end we
decided that the driver should first look for any device that can be used as
a clocksource and use it that way. If it finds a second device, that can be
used as clockevent.

Arnd


\
 
 \ /
  Last update: 2013-04-07 17:01    [W:0.114 / U:0.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site