[U-Boot] [PATCH 3/3] MAKEALL: fix boards_by_field function

Albert ARIBAUD albert.u.boot at aribaud.net
Thu Oct 17 13:41:56 CEST 2013


Hi Masahiro,

On Thu, 17 Oct 2013 19:32:31 +0900, Masahiro Yamada
<yamada.m at jp.panasonic.com> wrote:

> Hello Albert
> 
> 
> 
> 
> > >                     -v field="$1" \
> > >                     -v select="$2" \
> > >                     -F "$FS" \
> > >     -               '($1 !~ /^#/ && $field == select) { print $1 }' \
> > >     +               '($1 !~ /^#/ && $field == select) { print $7 }' \
> > >                     boards.cfg
> > >      }
> > >      boards_by_arch() { boards_by_field 2 "$@" ; }
> > >      boards_by_cpu()  { boards_by_field 3 "$@" "[: \t]+" ; }
> > >     -boards_by_soc()  { boards_by_field 6 "$@" ; }
> > >     +boards_by_soc()  { boards_by_field 4 "$@" ; }
> > > 
> > > 
> > > TAB is also treated as a field speparator, so
> > > we should have taken the 8th field for Tegra
> > > whereas the 7th field for the other cpus.
> > 
> > (As per our discussion, 'tab' here should be 'colon')
> 
> Yes, I answerd so in my previous reply.
> 
> 
> > Not sure I am getting the logic here. Colon is *not* a field separator,
> > precisely because it is not present on all lines; it is a sub-field
> > separator. At this low level, the only field separator should be spaces.
> > 
> > Therefore, I would prefer boards_by_field() and board_by_cpu() to *not*
> > handle colons and thus consider the CPU field as a whole even when it
> > consists in a cpu:splcpu pair.
> 
> Yes. I think I already did this in my v1 patch.
> 
> 
>   +		-v cut="$3" \
>   +		'{sub(cut,"",$field)}
> 
> These lines split the pair into cpu and spl_cpu.
> I simply cut down  spl_cpu because it is the same behaviour
> as before Commit 27af930e.
> 
> 
> 
> > Splitting that pair and using either cpu or splcpu depending on
> > whether building SPL or not should only happen when the CPU field is
> > actually used, not fetched.
> > 
> > Can you try and provide a v2 patch (set) along these lines?
> 
> 
> Sorry, I cannot understand what you mean.

I do understand that your patch wants to restore the behavior prior to
27af930e.

My problem is, while things worked prior to 27af930e, they were not
done the right way, because boards_by_<field>() functions should not
depend on the fact that a field contains a colon or not; they should
only depend on the fact that fields are space-separated.

IOW, target integratorcp_cm1136 on line 46 in boards.cfg has 9 fields,
and its CPU field is "arm1136"; and line 348, dalmore, *also* has 9
fields even though its CPU field is "armv7:arm720t".

The way the code is written now, board_by_field() has to do the job of 
board_by_cpu() and has to know the CPU field has colon-separated
subfields. What should be done is, board_by_field should not even worry
about colons at all, and it is board_by_cpu() which should know about
CPU dubfields and treat them properly.

Is this clearer?

> Best Regards
> Masahiro Yamada
> 


Amicalement,
-- 
Albert.


More information about the U-Boot mailing list