Subject: Re: [PATCH] Add support for the ESTeem 195E (PPC405EP) SBC

From: Solomon Peachy <[email-masked]>
Date: Thu, 30 Jul 2009 15:45:06 -0400

On Thu, Jul 30, 2009 at 10:06:30AM -0400, Josh Boyer wrote:
> >+		devp = finddevice("/plb/opb/serial@ef600300");
> >+		if (!devp)
> >+			fatal("Can't find node for /plb/opb/serial@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...

(The PPC405EP's serial boards aren't created equally; the first is a 
 dumb tx/rx-only port while the second has a full set of signals.
 The hotfoot is wired such that the second, full-featured port is the 
 primary/console/boot port)

> >+
> >+		devp = finddevice("/plb/opb/ethernet@ef600900");
> >+		if (!devp)
> >+			fatal("Can't find node for /plb/opb/ethernet@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.

> >+			UART0: serial@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...)
 
> >+			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.  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...

 - Solomon
-- 
Solomon Peachy                        [email-masked]
AbsoluteValue Systems                 http://www.linux-wlan.com
721-D North Drive                     +1 (321) 259-0737  (office)
Melbourne, FL 32934                   +1 (321) 259-0286  (fax)