Re: [PATCH v3 14/14] omap: mailbox: reorganize headers
Hello, On Tue, May 25, 2010 at 8:49 AM, Felipe Balbi wrote: > On Mon, May 24, 2010 at 09:14:24PM +0200, ext Felipe Contreras wrote: >> Now, I am pretty sure that platform_device.h will always depend on >> device.h that's why I removed them, but I can add them again if >> somebody wants to, but again, the situation is already bad. > > so there's platform_device.h on that file, a little comment on the changelog > would suffice to avoid this kind of mis-understanding. Sure, will do. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 14/14] omap: mailbox: reorganize headers
Hi, On Mon, May 24, 2010 at 09:14:24PM +0200, ext Felipe Contreras wrote: Now, regarding device.h: which was my only concern Now, I am pretty sure that platform_device.h will always depend on device.h that's why I removed them, but I can add them again if somebody wants to, but again, the situation is already bad. so there's platform_device.h on that file, a little comment on the changelog would suffice to avoid this kind of mis-understanding. -- balbi DefectiveByDesign.org -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 14/14] omap: mailbox: reorganize headers
On Mon, May 24, 2010 at 8:16 PM, Felipe Balbi wrote: > personally I don't like this patch since it makes the C-sources rely on > indirect header inclusion by another header. That's really error prone and > will cause build failures if someone decides to remove the include > from plat/mailbox.h. First of all, you cannot expect each and every source file to include each and every header it's using directly. It would be nice, but it's not happening now, and probably will not happen ever. Now, with a few exceptions, I didn't assume somebody else would be loading anything, in fact, the patch is mostly _adding_ headers that were missing. If you take a look at the modified files regarding kernel.h and module.h: * arch/arm/mach-omap1/mailbox.c: Not really using kernel.h, missing module.h * arch/arm/mach-omap2/mailbox.c: Missing module.h * arch/arm/plat-omap/mailbox.c Missing kernel.h So it was clear to me that these were not carefully added, and probably were redundant, so I choose to remove them completely. If you have something against that, I can add them where it makes sense but the current patch is not really making the situation much worst. Now, regarding device.h: * arch/arm/mach-omap2/mailbox.c Using it, but missing * arch/arm/plat-omap/include/plat/mailbox.h Was using it, but a 'struct device' would suffice * arch/arm/plat-omap/mailbox.c Using it correctly Now, I am pretty sure that platform_device.h will always depend on device.h that's why I removed them, but I can add them again if somebody wants to, but again, the situation is already bad. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 14/14] omap: mailbox: reorganize headers
Hi, On Sat, May 22, 2010 at 07:14:25PM +0200, ext Felipe Contreras wrote: Signed-off-by: Felipe Contreras --- arch/arm/mach-omap1/mailbox.c |3 --- arch/arm/mach-omap2/mailbox.c |1 - arch/arm/plat-omap/include/plat/mailbox.h |3 ++- arch/arm/plat-omap/mailbox.c |5 ++--- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/arch/arm/mach-omap1/mailbox.c b/arch/arm/mach-omap1/mailbox.c index e50b3c2..fdd6cc9 100644 --- a/arch/arm/mach-omap1/mailbox.c +++ b/arch/arm/mach-omap1/mailbox.c @@ -9,13 +9,10 @@ * for more details. */ -#include -#include #include #include #include #include -#include #define MAILBOX_ARM2DSP10x00 #define MAILBOX_ARM2DSP1b 0x04 diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c index 66d366d..d46e439 100644 --- a/arch/arm/mach-omap2/mailbox.c +++ b/arch/arm/mach-omap2/mailbox.c @@ -10,7 +10,6 @@ * for more details. */ -#include #include #include #include diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h index c44fde3..9976565 100644 --- a/arch/arm/plat-omap/include/plat/mailbox.h +++ b/arch/arm/plat-omap/include/plat/mailbox.h @@ -3,9 +3,10 @@ #ifndef MAILBOX_H #define MAILBOX_H -#include +#include #include #include +#include #include typedef u32 mbox_msg_t; diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c index a8e22e1..8d4c7be 100644 --- a/arch/arm/plat-omap/mailbox.c +++ b/arch/arm/plat-omap/mailbox.c @@ -21,10 +21,9 @@ * */ -#include -#include #include -#include +#include +#include #include #include #include personally I don't like this patch since it makes the C-sources rely on indirect header inclusion by another header. That's really error prone and will cause build failures if someone decides to remove the include from plat/mailbox.h. Those headers are safe against multiple inclusions and I think they should be included directly in all source files (header or not) that need any symbols from it. -- balbi DefectiveByDesign.org -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
