[U-Boot] [PATCH 2/2] XPedite5200 board support

Peter Tyser ptyser at xes-inc.com
Wed Nov 19 01:03:30 CET 2008


On Wed, 2008-11-19 at 00:36 +0100, Wolfgang Denk wrote:
> Dear Peter Tyser,
> 
> In message <1225213372-15439-3-git-send-email-ptyser at xes-inc.com> you wrote:
> > Initial support for Extreme Engineering Solutions XPedite5200 -
> > a MPC8548-based PMC single board computer.
> > 
> > Signed-off-by: Peter Tyser <ptyser at xes-inc.com>
> 
> There are some coding style violations: identation not by TAB, bad
> multiline comments, way too long lines,

The long line violations I see are in tlb.c and XPEDITE5200.h.  Every
other 8548 board I see has the same violations, but I'm assuming it that
doesn't make a difference and should be fixed...

> 
> > diff --git a/board/xes/common/fsl_85xx_pci.c b/board/xes/common/fsl_85xx_pci.c
> > index ac91534..b86a044 100644
> > --- a/board/xes/common/fsl_85xx_pci.c
> > +++ b/board/xes/common/fsl_85xx_pci.c
> ...
> >  void pci_init_board(void)
> >  {
> > @@ -96,6 +132,66 @@ void pci_init_board(void)
> >  	debug("   pci_init_board: devdisr=%x, io_sel=%x, host_agent=%x\n",
> >  			devdisr, io_sel, host_agent);
> >  
> > +#ifdef CONFIG_PCI1
> > +{
> 
> Wrong indentation ?

Leftover unused { }, will remove.

> > +	width = 0; /* Silence compiler warning... */
> > +	io_sel &= 0xf; /* Silence compiler warning... */
> > +	pci = (ccsr_fsl_pci_t *) CONFIG_SYS_PCI1_ADDR;
> > +	hose = &pci1_hose;
> > +	host = host_agent_cfg[host_agent].pci_host[0];
> > +	r = hose->regions;
> > +
> > +	uint pci_spd_norm = (gur->pordevsr & MPC85xx_PORDEVSR_PCI1_SPD);
> > +	uint pci_32 = gur->pordevsr & MPC85xx_PORDEVSR_PCI1_PCI32;
> > +	uint pci_arb = gur->pordevsr & MPC85xx_PORDEVSR_PCI1_ARB;
> > +	uint pcix = gur->pordevsr & MPC85xx_PORDEVSR_PCI1;
> > +	uint freq = CONFIG_SYS_CLK_FREQ / 1000 / 1000;
> 
> Please never mix declarations with code.

Will fix.

> 
> > +		first_free_busno = hose->last_busno+1;
> > +		printf("    PCI1 on bus %02x - %02x\n",
> > +		       hose->first_busno, hose->last_busno);
> > +	} else {
> > +		printf("    PCI1: disabled\n");
> > +	}
> > +}
> > +#elif defined CONFIG_MPC8548
> > +	/* PCI1 not present on MPC8572 */
> > +	gur->devdisr |= MPC85xx_DEVDISR_PCI1; /* disable */
> > +#endif
> 
> Yes, wrong indentation.
> 
> > diff --git a/board/xes/xpedite5200/ddr.c b/board/xes/xpedite5200/ddr.c
> > new file mode 100644
> > index 0000000..c5616d5
> > --- /dev/null
> > +++ b/board/xes/xpedite5200/ddr.c
> 
> I think one day we will have to merge all these 20+ ddr.c files into
> one or a few common ones :-(  Much to much copy & paste around here.

I'll fix the multiline comments as well.

Thanks,
Peter



More information about the U-Boot mailing list