On Thu, Jul 30, 2009 at 03:45:06PM -0400, Solomon Peachy wrote:
>On Thu, Jul 30, 2009 at 10:06:30AM -0400, Josh Boyer wrote:
>> >+ devp = finddevice("/plb/opb/ser...@ef600300");
>> >+ if (!devp)
>> >+ fatal("Can't find node for /plb/opb/ser...@ef600300");
>> >+ del_node(devp);
>>
>> Slightly confused here. You delete the first serial node in the single eth
>> case?
>
>The board is actually single eth/serial or dual eth/serial. And strictly
>speaking, this serial port is the second one in the devicetree...
Maybe a brief comment in the code explaining that would help. Also, I did
notice the DTS had them in the order you mention, and I forgot to come back
and correct my question there.
>> >+
>> >+ devp = finddevice("/plb/opb/ether...@ef600900");
>> >+ if (!devp)
>> >+ fatal("Can't find node for /plb/opb/ether...@ef600900");
>> >+ del_node(devp);
>> >+ }
>> >+
>> >+ ibm4xx_quiesce_eth((u32 *)0xef600800, (u32 *)0xef600900);
>>
>> Shouldn't you do the quiesce conditionally if the other eth port doesn't
>> exist?
>
>I don't know if this is strictly necessary with the modern ibm_emac
>driver, but it's certainly safe to call because all 405EPs have dual
>ethernet controllers.
>
>From the (pre-dts) driver perspecive, the only way to tell if the
>hotfoot had one ethernet port or two was that the second PHY failed to
>initialize.
>
>Additionally, the production bootloader (u-boot 1.2.0.x) isn't terribly
>smart and tries to drive the second controller if the first one doesn't
>have a cable plugged in, so it's possible the second controller is
>running when control is handed over to Linux, even on single ethernet
>boards.
OK, that makes sense.
>
>> >+ UART0: ser...@ef600400 {
>> >+ device_type = "serial";
>> >+ compatible = "ns16550";
>> >+ reg = <0xef600400 0x00000008>;
>> >+ virtual-reg = <0xef600400>;
>> >+ clock-frequency = <0>; /* Filled in by zImage */
>> >+ current-speed = <0x9600>;
>>
>> Just a question, but is the baud supposed to be 38400 or 9600? At first
>> glance
>> it almost seems like a typo :).
>
>It's supposed to be 38400 baud, hence the explicit 0x in front. (I lost
>count of the number of times I saw '38400' listed in various dts
>files...)
Cool. Just checking.
>> >+ gpio-leds {
>> >+ compatible = "gpio-leds";
>> >+ status {
>> >+ label = "Status";
>> >+ gpios = <&GPIO 1 0>;
>> >+ /* linux,default=trigger = ".."; */
>> >+ };
>>
>> What does that comment mean?
>
>Whoops, it's supposed to read 'linux,default-trigger', but the LEDs are
>manually twiddled for the time being. I'd forgotten to strip that out.
>
>(see linux/Documentation/powerpc/dts-bindings/gpio/led.txt)
OK. If it's not needed just yank it.
>
>> Ok. So I'm not really all that thrilled with changes to ppcboot.h.
>> We try to keep this file as much in-sync with U-Boot as we can. Did
>> your HOTFOOT changes get pulled into upstream U-Boot?
>
>Yeah, I thought this may be a problem, but I didn't know a better way to
>go about this and still maintain compatibility with the many thousands
>of boards already in the field. I mean, I could strip out the ppcboot.h
>changes and maintain that as an out-of-tree patch, but without that
>patch, the kernel won't boot on in-the-field boards, rendering the
>upstreaming of support for this board kinda pointless.
>
>I haven't tried to push anything to upstream u-boot, given how ancient
>the in-production bootloader is. The guy who originally mangled u-boot
>for this board did so before the "standard" 405EP dual ethernet layout
>was added, and never tried to push it upstream. Any upstream uboot work
>will take the form of a native dts/fdt port that probably won't use
>ppcboot.h anyway, which brings us full circle...
There is another way. Perhaps you could just copy ppcboot.h to a new file
called "hotfoot.h" and just use that. It's a duplication of ppcboot.h to
some degree, but it seems to make sense for your board and it helps preserve
the "stock" ppcboot.h for other boards.
josh
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev
