Hi Nishanth,
On 13 November 2015 at 16:57, Nishanth Menon nm@ti.com wrote:
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.guinot@sequanux.org Cc: Albert ARIBAUD albert.u.boot@aribaud.net
Signed-off-by: Nishanth Menon nm@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?
Yes.
- Simon