Re: [PATCH] [POWERPC][RFC] MPC8360E-RDK: Device tree and board file

[Scott Wood <scottwood@freescale.com>]

On Mon, Dec 17, 2007 at 04:14:03PM +1100, David Gibson wrote:
> > +		crypto@30000 {
> > +			device_type = "crypto";
> > +			model = "SEC2";
> > +			compatible = "talitos";
> 
> This device_type/compatible/model stuff is also crap, although I
> suspect it needs to be fixed in the driver, as gianfar (finally) was.

The driver doesn't seem to be in-tree... Kim, what do(es) the external
driver(s) look like?  Do they use OF at all yet?

> > +		ranges = <0 0xe0100000 0x00100000>;
> > +		reg = <0xe0100000 0x480>;
> > +		/* filled by u-boot */
> > +		brg-frequency = <0>;
> > +		bus-frequency = <0>;
> 
> This should probably be clock-frequency, not bus-frequency.  After
> all, it's a bus node, what other sort of frequency would it be.

Actually, it should probably be dropped altogether.

> > +		muram@10000 {
> > +			device_type = "muram";
> 
> And this device_type needs to go, too.

Yes, replace it with compatible = "fsl,cpm-muram".

> > +			ranges = <0 0x00010000 0x0000c000>;
> > +
> > +			data-only@0 {
> > +				reg = <0 0xc000>;

compatible = "fsl,cpm-muram-data".

> > +			phy1: ethernet-phy@1 {
> > +				reg = <1>;
> > +				device_type = "ethernet-phy";
> > +			};
> 
> These phy nodes have basically no information in them.  PHY nodes are
> optional -

If they are truly optional, then several Linux drivers (including ucc_geth,
which this board uses) are broken, as they'll error out if there's no
phy-handle (gianfar is even worse -- it looks like the fsl_soc code will
crash in that case).  But what do you propose they do in the absence of a
phy-handle?  Hope that probing only finds one phy?

> only include them if they actually have something useful to say (which
> would mean at least a compatible property).

They *do* have useful information -- reg and phandle.  The type of phy can
be probed, but which phy corresponds to which ethernet can't.

-Scott