[U-Boot] [PATCH V2 07/12] board: LaCie: Move common headers to board-common directory

[Simon Guinot]

On Fri, Nov 13, 2015 at 09:06:45AM -0500, Tom Rini wrote:
> On Fri, Nov 13, 2015 at 11:30:43AM +0100, Simon Guinot wrote:
> > Hi Nishanth,
> > 
> > On Thu, Nov 12, 2015 at 11:43:37PM -0600, Nishanth Menon wrote:
> > > Header files can be located in a generic location without
> > > needing to reference them with ../common/
> > > 
> > > Generated with the following script
> > > 
> > >  #!/bin/bash
> > > vendor=board/LaCie
> > > common=$vendor/common
> > > 
> > > cfiles=`git grep "../common" $vendor|grep "#include"|cut -d '"' -f2|sort -u|grep c$`
> > > headers=`git grep "../common" $vendor|grep "#include"|cut -d '"' -f2|sort -u|grep h$`
> > > 
> > > mkdir -p $common/include/board-common
> > > set -x
> > > for header in $headers
> > > do
> > > 	echo "processing $header in $common"
> > > 	hbase=`basename $header`
> > > 	git mv $common/$hbase $common/include/board-common
> > > 	sed -i -e "s/\"..\/common\/$hbase\"/<board-common\/$hbase>/g" $vendor/*/*.[chS]
> > > 	sed -i -e "s/\"$hbase\"/<board-common\/$hbase>/g" $vendor/common/*.[chS]
> > > done
> > > 
> > > Cc: Simon Guinot <simon.guinot at sequanux.org>
> > > Cc: Albert ARIBAUD <albert.u.boot at aribaud.net>
> > > 
> > > Signed-off-by: Nishanth Menon <nm at ti.com>
> > > ---
> > >  board/LaCie/common/cpld-gpio-bus.c                            | 2 +-
> > >  board/LaCie/common/{ => include/board-common}/common.h        | 0
> > 
> > Is that really a good idea to move a LaCie-specific file named common.h
> > to a place shared with other boards ?
> > 
> > >  board/LaCie/common/{ => include/board-common}/cpld-gpio-bus.h | 0
> > 
> > IMO, this headers are specific to LaCie boards and it don't make much
> > sense to move them to a shared place. Moreover it is quite convenient to
> > have them close from the board setup files.
> > 
> > Please don't move them.
> 
> Please read and then suggest changes in the "Makefile: Include vendor
> common library in include search path" thread.  Thanks!

Hi Tom,

Do you mean I answered without even really looking at the suggested
change ? Well, it is true :)

Sorry about that. I have been confused by the "git move file" notation
which I am not familiar with. So, I withdraw my previous objections.

Now, I have to say that I am still not convinced by the change.

After applying this patch, I can see that:

#include "../common/cpld-gpio-bus.h"

have been replaced with:

#include <board-common/cpld-gpio-bus.h>

While this change is fine, I can also see that the header file has been
moved from:

board/LaCie/common/cpld-gpio-bus.h

to:

board/LaCie/common/include/board-common/cpld-gpio-bus.h

With both the strings "board" and "common" duplicated, I am definitively
not a big fan of this new path. Moreover I think that moving the headers
away from the board setup files will harm a little bit the code reading.

Maybe it would be better to have a shorter path like:

board/LaCie/include/board-common/cpld-gpio-bus.h

Anyway, IMHO things are fine as they are. But if anyone thinks this
change is needed or valuable, then I am OK with it.

Thanks,

Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151113/2bb814f0/attachment.sig>