Em Wed, 10 Oct 2012 12:52:48 +0200 Hans Verkuil <hverkuil@xxxxxxxxx> escreveu: > On Wed 10 October 2012 12:39:39 Mauro Carvalho Chehab wrote: > > Em Wed, 10 Oct 2012 08:27:26 +0200 > > Hans Verkuil <hverkuil@xxxxxxxxx> escreveu: > > > > > On Wed October 10 2012 03:05:30 Mauro Carvalho Chehab wrote: > > > > Em Mon, 8 Oct 2012 15:03:36 +0200 > > > > Hans Verkuil <hverkuil@xxxxxxxxx> escreveu: > > > > > > > > > On Sun October 7 2012 13:13:36 Sylwester Nawrocki wrote: > > > > > > On 10/07/2012 03:19 AM, Michael West wrote: > > > > > > > This patch changes versions.txt and disables VIDEO_M5MOLS which > > > > > > > fixed the build for my 3.2 kernel but looking at the logs it looks > > > > > > > like this is not the way to fix it as it's not just a 3.6+ problem > > > > > > > as it does not build on 3.6 as well... So probably best to find > > > > > > > why it doesn't build on the current kernel first. > > > > > > > > > > > > To fix the build on kernels 3.6+ <linux/sizes.h> just needs to be > > > > > > inclcuded in m5mols.h. This is what my patch from previous message > > > > > > in this thread does. But this will break again on kernel versions > > > > > > _3.5 and lower_ where <linux/sizes.h> doesn't exist. I thought > > > > > > originally it could have been simply replaced there with <asm/sizes.h>, > > > > > > but not all architectures have it > > > > > > > > > > > > $ git grep "#define SZ_1M" v2.6.32 > > > > > > v2.6.32:arch/arm/include/asm/sizes.h:#define SZ_1M 0x00100000 > > > > > > v2.6.32:arch/sh/include/asm/sizes.h:#define SZ_1M 0x00100000 > > > > > > > > > > > > $ git grep "#define SZ_1M" v3.6-rc5 > > > > > > v3.6-rc5:drivers/base/dma-contiguous.c:#define SZ_1M (1 << 20) > > > > > > v3.6-rc5:include/linux/sizes.h:#define SZ_1M 0x00100000 > > > > > > > > > > > > > > > > > > Let's just use the below patch to solve this build break, this way > > > > > > there is no need to touch anything at media_build. > > > > > > > > > > > > From 11adc6956f3fe87c897aa6add08f8437422969a8 Mon Sep 17 00:00:00 2001 > > > > > > From: Sylwester Nawrocki <sylvester.nawrocki@xxxxxxxxx> > > > > > > Date: Sun, 7 Oct 2012 13:04:37 +0200 > > > > > > Subject: [PATCH] m5mols: Replace SZ_1M with explicit value > > > > > > > > > > > > SZ_1M macro definition was introduced in commit ab7ef22419927 > > > > > > "[media] m5mols: Implement .get_frame_desc subdev callback" > > > > > > but required <linux/sizes.h> header was not included. To prevent > > > > > > build errors with older kernels where <linux/sizes.h> doesn't exist > > > > > > use explicit value rather than SZ_1M. > > > > > > > > > > > > Reported-by: Jan Hoogenraad <jan-conceptronic@xxxxxxxxxxxxxx> > > > > > > Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki@xxxxxxxxx> > > > > > > > > > > Acked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> > > > > > > > > > > Note: until this patch is merged I am disabling this driver in media_build > > > > > since right now it doesn't compile at all. Please notify me when this is > > > > > fixed in media_tree.git so that I can enable it again. > > > > > > > > > > Regards, > > > > > > > > > > Hans > > > > > > > > > > > --- > > > > > > drivers/media/i2c/m5mols/m5mols.h | 2 +- > > > > > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > > > > > > > > > diff --git a/drivers/media/i2c/m5mols/m5mols.h b/drivers/media/i2c/m5mols/m5mols.h > > > > > > index 4ab8b37..30654f5 100644 > > > > > > --- a/drivers/media/i2c/m5mols/m5mols.h > > > > > > +++ b/drivers/media/i2c/m5mols/m5mols.h > > > > > > @@ -24,7 +24,7 @@ > > > > > > * determined by CAPP_JPEG_SIZE_MAX register. > > > > > > */ > > > > > > #define M5MOLS_JPEG_TAGS_SIZE 0x20000 > > > > > > -#define M5MOLS_MAIN_JPEG_SIZE_MAX (5 * SZ_1M) > > > > > > +#define M5MOLS_MAIN_JPEG_SIZE_MAX (5 * 1024 * 1024) > > > > > > > > Nah! Please don't do that! we shouldn't be patching a driver upstream > > > > just because it broke media_build. Also, fixing it there is as simple as > > > > adding something similar to this at compat.h: > > > > > > > > #ifndef SZ_1M > > > > #define SZ_1m (1024 * 1024) > > > > #endif > > > > > > Actually, I prefer 1024 * 1024 over SZ_1M. > > > > I also think that this obfuscates the code, but the right place to discuss it is > > not here; it is with whomever is proposing those defines, and the ones > > that acked with it: > > > > $ git log include/linux/sizes.h > > commit dccd2304cc907c4b4d2920eeb24b055320fe942e > > Author: Alessandro Rubini <rubini@xxxxxxxxx> > > Date: Sun Jun 24 12:46:05 2012 +0100 > > > > ARM: 7430/1: sizes.h: move from asm-generic to <linux/sizes.h> > > > > sizes.h is used throughout the AMBA code and drivers, so the header > > should be available to everyone in order to driver AMBA/PrimeCell > > peripherals behind a PCI bridge where the host can be any platform > > (I'm doing it under x86). > > > > At this step <asm-generic/sizes.h> includes <linux/sizes.h>, > > to allow a grace period for both in-tree and out-of-tree drivers. > > > > Signed-off-by: Alessandro Rubini <rubini@xxxxxxxxx> > > Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@xxxxxx> > > Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx> > > Cc: Alan Cox <alan@xxxxxxxxxxxxxxx> > > Signed-off-by: Russell King <rmk+kernel@xxxxxxxxxxxxxxxx> > > > > Provided that this is now officially part of the Kernel internal ABI, > > we should not nack or revert changes on codes that would be using it. > > > > > The alternative patch is this: > > > > > > http://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg53424.html > > > > If this patch makes sense upstream (e. g. if is there any scenario where > > linux/sizes.h is not implicitly included), then applying it would actually > > be a fix, and such patch should be included. > > > > Do you have any .config where such compilation breakage happen upstream? > > Just enable the sensor driver for x86. It will fail to compile in the for_v3.7 > git branch. Again, this has nothing to do with the media_build. It's just a > missing include that breaks compilation unless you're compiling for arm. > > > > > If, otherwise, this is not the case, we should just fix it at the media > > build, by either not compiling those drivers or by providing a backward > > compatibility at compat.h. > > > > Btw, just not compiling m5mols is likely the best approach, as I > > seriously doubt that anyone using this driver is using the media-build stuff[1]. > > > > > Note that the arm architecture will pull in linux/sizes.h, but the x86 arch > > > doesn't, so this driver won't compile with x86. It's a real driver bug that > > > has nothing to do with media_build. > > > > Well, linux/sizes.h is not an arch-dependent header. > > linux/sizes.h is included by arch/arm/include/asm/memory.h, which is included > by other headers. So when compiling on arm, this header is pulled in for you, > when compiling on x86 you have to include it manually. > > To fix this you either need to include <linux/sizes.h> explicitly, or stop using > SZ_1M. Forget about media_build, that's a red-herring. It's just a missing header > problem. Ok. Sylwester (or Hans), please send me the "include linux/sizes.h" variant then. I suspect we'll see other "SZ_1M" patches in the future, so it makes no sense to swim against the waves. Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
