Re: Media_build broken by [PATCH RFC v3 5/5] m5mols: Implement .get_frame_desc subdev callback

[Sylwester Nawrocki <s.nawrocki@samsung.com>]

On 10/10/2012 08:27 AM, Hans Verkuil wrote:
>>>> 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. The alternative patch is this:
> 
> http://www.mail-archive.com/linux-media@vger.kernel.org/msg53424.html
> 
> 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.
> 
> So you need to merge one of these two patches to fix this problem. I prefer
> the first, but the second is fine too.

Yes, there is a bug in a driver now, even though the driver compiles on 
ARM, where linux/sizes.h is included indirectly, it won't build on other
architectures where it's not the case. I'm not sure which patch is better,
but one of them needs to be applied. Otherwise we're going to see bug 
reports from people building kernel 3.6+ with allyesconfig, etc..

--

Regards,
Sylwester