[U-Boot-Users] [PATCH 2/3] 83xx: serdes setup routines

[Jean-Christophe PLAGNIOL-VILLARD]

On 16:44 Thu 13 Mar     , Anton Vorontsov wrote:
> On Wed, Mar 12, 2008 at 11:54:39PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 18:07 Fri 07 Mar     , Anton Vorontsov wrote:
> > > This patch adds few routines to configure serdes on 837x targets.
> > > 
> > > Signed-off-by: Anton Vorontsov <avorontsov at ru.mvista.com>
> > > ---
> > >  cpu/mpc83xx/Makefile |    2 +-
> > >  cpu/mpc83xx/serdes.c |  161 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  include/fsl_serdes.h |   25 ++++++++
> > >  3 files changed, 187 insertions(+), 1 deletions(-)
> > >  create mode 100644 cpu/mpc83xx/serdes.c
> > >  create mode 100644 include/fsl_serdes.h
> > > 
> > > diff --git a/cpu/mpc83xx/Makefile b/cpu/mpc83xx/Makefile
> > > index 94a3cb8..678be29 100644
> > > --- a/cpu/mpc83xx/Makefile
> > > +++ b/cpu/mpc83xx/Makefile
> > > @@ -29,7 +29,7 @@ LIB	= $(obj)lib$(CPU).a
> > >  
> > >  START	= start.o
> > >  COBJS	= traps.o cpu.o cpu_init.o speed.o interrupts.o \
> > > -	  spd_sdram.o ecc.o qe_io.o pci.o fdt.o
> > > +	  spd_sdram.o ecc.o qe_io.o pci.o fdt.o serdes.o
> > Please split it with on line for one file
> 
> Not sure if just splitting COBJS in separate lines is any better.
> In long term we should start using $(CONFIG_ symbols. I can start
> doing it for serdes...
> 
> > >  
> [...]
> > > +#ifdef CONFIG_FSL_SERDES
> > Please move it to the Makefile
> 
> - - - -
> From: Anton Vorontsov <avorontsov at ru.mvista.com>
> Subject: 83xx: serdes setup routines
> 
> This patch adds few routines to configure serdes on 837x targets.
> 
> Signed-off-by: Anton Vorontsov <avorontsov at ru.mvista.com>
> ---
>  cpu/mpc83xx/Makefile |    6 +-
>  cpu/mpc83xx/serdes.c |  156 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/fsl_serdes.h |   25 ++++++++
>  3 files changed, 185 insertions(+), 2 deletions(-)
>  create mode 100644 cpu/mpc83xx/serdes.c
>  create mode 100644 include/fsl_serdes.h
> 
> diff --git a/cpu/mpc83xx/Makefile b/cpu/mpc83xx/Makefile
> index 94a3cb8..27e1567 100644
> --- a/cpu/mpc83xx/Makefile
> +++ b/cpu/mpc83xx/Makefile
> @@ -28,8 +28,10 @@ include $(TOPDIR)/config.mk
>  LIB	= $(obj)lib$(CPU).a
>  
>  START	= start.o
> -COBJS	= traps.o cpu.o cpu_init.o speed.o interrupts.o \
> -	  spd_sdram.o ecc.o qe_io.o pci.o fdt.o
> +COBJS-y	+= traps.o cpu.o cpu_init.o speed.o interrupts.o \
> +	   spd_sdram.o ecc.o qe_io.o pci.o
> +COBJS-$(CONFIG_FSL_SERDES) += serdes.o
> +COBJS = $(COBJS-y)
I really insist please split it on line for one file.

See the anwsers on Stefano Patch

> > > It will be nice to split one line for each file
> >
> > Why? IMHO the could will not become more readble that way, on
> > contrary...

Right. The readability will suffer a little by changing this into the
one-object-per-line version.

> When you have mutltiple patch for a makefile, ex : add 2 new file in 2
> patch, it could be applied without rebase it the second patch

Correct. Even though the likelyhood of multiple patches in this specific
directory is very low. But nevertheless I'm tempted to change it to the
one-object-per-line version. This makes it also easier to add one object
in
alphabetical order and not having to reorder the lines. Here an example
of
adding "ccccccccccccc.o":

For the multiple-objects-per-line:

-COBJS = aaaaaaaaaaaa.o bbbbbbbbbbbb.o eeeeeeeeeeee.o
-COBJS += ffffffffffff.o hhhhhhhhhhhh.o xxxxxxxxxxxx.o
+COBJS = aaaaaaaaaaaa.o bbbbbbbbbbbb.o cccccccccccc.0
+COBJS += eeeeeeeeeeee.o ffffffffffff.o hhhhhhhhhhhh.o
+COBJS += xxxxxxxxxxxx.o

For the one-object-per-line:

COBJS = aaaaaaaaaaaa.o
COBJS += bbbbbbbbbbbb.o
+COBJS += cccccccccccc.o
COBJS += eeeeeeeeeeee.o
COBJS += ffffffffffff.o

Best regards,
Stefan 

----

Best Regards,
J.