On Sunday 07 April 2013, Daniel Tang wrote:
> On 07/04/2013, at 12:24 AM, Arnd Bergmann <a...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
