On 11/13/2015 09:32 AM, Simon Guinot wrote:
> 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.gui...@sequanux.org>
>>>> Cc: Albert ARIBAUD <albert.u.b...@aribaud.net>
>>>>
>>>> Signed-off-by: Nishanth Menon <n...@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.
>
Thanks.
> 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
That can easily be done as well. for me, it is just regenerating the
scripts..
I strongly suggest to comment on the original patch
https://patchwork.ozlabs.org/patch/544030/ and suggest this so that
other platform folks have the discussion context as well.
>
> Anyway, IMHO things are fine as they are. But if anyone thinks this
> change is needed or valuable, then I am OK with it.
IMHO, I started on this story, because we have a need to have a
board/ti/common directory and adding more cruft on top of what is
already a bunch of crufty includes is just painful.
If you are interested in the complete history:
https://patchwork.ozlabs.org/patch/540280/
https://patchwork.ozlabs.org/patch/541068/
https://patchwork.ozlabs.org/patch/542424/
Tom, Simon,
Are you guys ok with board/$(VENDOR)/include?
--
Regards,
Nishanth Menon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
