[PATCH 1/4] Add support for 750CL Holly board

[Josh Boyer]

On Fri, 2007-05-04 at 13:35 -0500, Kumar Gala wrote:
> On May 4, 2007, at 1:14 PM, Josh Boyer wrote:
> > +
> > +config HOLLY
> > +	bool "PPC750GX/CL with TSI10x bridge (Hickory/Holly)"
> > +	select TSI108_BRIDGE
> > +	select PPC_UDBG_16550
> > +	select MPIC
> > +	select MPIC_WEIRD
> > +	help
> > +	  Select HOLLY if configuring for an IBM 750GX/CL Eval
> > +	  Board with TSI108/9 bridge (Hickory/Holly)
> >  endchoice
> >
> >  config TSI108_BRIDGE
> >  	bool
> 
> I wondering if we should have select MPIC and MPIC_WEIRD here instead  
> of on the board config.

Could be done, yes.  I don't have an mpc7448hpc2 to test with, but I can
make that change.

> > +	printk(KERN_INFO "PPC750GX/CL Platform\n");
> 
> Should this be something like "Holly PPC750GX/CL Platform"

Well... no.  It should really be "Hickory/Holly PPC750GX/CL Platform" to
be correct.  But that sucks.  Suppose I could match off of the cpu node
in the DT and just print one or the other... except they share a DT too
at the moment.

I'll think about what to do.

> > +void holly_show_cpuinfo(struct seq_file *m)
> > +{
> > +	seq_printf(m, "vendor\t\t: IBM\n");
> > +	seq_printf(m, "machine\t\t: PPC750 GX/CL\n");
> 
> Should 'Holly' be in the machine name?

Again, the whole "Hickory/Holly" thing.

> 
> > +}
> > +
> > +void holly_restart(char *cmd)
> > +{
> > +	unsigned long *ocn_bar1 = NULL;
> 
> This should probably be __be32 __iomem

Good catch.  I need to run this through sparse.

> > +/*
> > + * Called very early, device-tree isn't unflattened
> > + */
> > +static int __init holly_probe(void)
> > +{
> > +	unsigned long root = of_get_flat_dt_root();
> > +
> > +	if (!of_flat_dt_is_compatible(root, "ppc750"))
> > +		return 0;
> 
> This seems like to generic of a match.

Likely is, yes.

> > --- linux-2.6.orig/drivers/net/tsi108_eth.h
> > +++ linux-2.6/drivers/net/tsi108_eth.h
> > @@ -49,7 +49,11 @@
> >   */
> >  #define PHY_MV88E	1	/* Marvel 88Exxxx PHY */
> >  #define PHY_BCM54XX	2	/* Broardcom BCM54xx PHY */
> > +#if defined(CONFIG_HOLLY)
> > +#define TSI108_PHY_TYPE PHY_BCM54XX
> > +#else
> >  #define TSI108_PHY_TYPE	PHY_MV88E
> > +#endif
> 
> This seems pretty bad.  Can we at least make this some type of kernel  
> config since I'm guessing the tsi108 isn't using the phylib.

Yep.

Thanks for the comments.  I'll fix it up in the next round.

josh