[PATCH 0/7] media_build: various kernel version fixes

[PATCH 0/7] media_build: various kernel version fixes

[Brad Love]

This first four patches in this set disables drivers which cannot
be compiled before a specific kernel revision.

To fix of_find_i2c_device_by_node|of_find_i2c_adapter_by_node in
kernels 3.5 to 3.11.x the correct header is included.

The frame_vector.c wildcard check also appears to be broken for me.
The check is changed from using relative patch to absolute path,
and verifying frame_vector.c is in the build dir (v4l/) instead
of in the linux/ directory.

Lastly, I have one new addition to contemplate. I maintain driver
packages for a lot of different kernel revisions on a lot of different
architectures and some times make_config_compat.pl incorrectly
enables backport options which cause build failure. Instead of
making the config check more complicated I propose creation and
inclusion of an empty config-mycompat.h, after config-compat.h is included
in compat.h, which would allow for overriding any options necessary
due to symbols/macros/etc already existing in the target kernel.
config-mycompat.h would be touched before make_config_compat.h is
called and deleted during distclean, allowing a builder to copy any
overrides into the header before starting the compilation process.
This would allow usage of the media_build system without having to
supply out of tree patches to correct the 'bad' options. If I
somehow missed this functionality a pointer to it would be lovely.


Brad Love (7):
  Disable VIDEO_ADV748X for kernels older than 4.8
  Disable additional drivers requiring gpio/consumer.h
  Disable DVBC8SECTPFE for kernels older than 3.5
  Disable SOC_CAMERA for kernels older than 3.5
  Header location fix for 3.5.0 to 3.11.x
  Fix frame vector wildcard file check
  Add config-compat.h override config-mycompat.h

 v4l/Makefile     |  5 +++--
 v4l/compat.h     | 14 ++++++++++++++
 v4l/versions.txt | 18 ++++++++++++++++--
 3 files changed, 33 insertions(+), 4 deletions(-)

-- 
2.7.4

[PATCH 1/7] Disable VIDEO_ADV748X for kernels older than 4.8

[Brad Love]

Needs i2c_new_secondary_device

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 v4l/versions.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/v4l/versions.txt b/v4l/versions.txt
index 6220485..ae0731d 100644
--- a/v4l/versions.txt
+++ b/v4l/versions.txt
@@ -13,6 +13,10 @@ RADIO_WL128X
 # needs *probe_new in struct i2c_driver
 VIDEO_OV5670
 
+[4.8.0]
+# needs i2c_new_secondary_device
+VIDEO_ADV748X
+
 [4.7.0]
 # needs i2c_mux_alloc
 DVB_RTL2830
-- 
2.7.4

[PATCH 2/7] Disable additional drivers requiring gpio/consumer.h

[Brad Love]

One driver migrated to 3.13 from 3.5

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 v4l/versions.txt | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/v4l/versions.txt b/v4l/versions.txt
index ae0731d..2306830 100644
--- a/v4l/versions.txt
+++ b/v4l/versions.txt
@@ -107,6 +107,15 @@ VIDEO_VIM2M
 [3.13.0]
 # needs gpio/consumer.h
 RADIO_SI4713
+VIDEO_OV2685
+VIDEO_OV5695
+VIDEO_OV9650
+VIDEO_MT9T112
+SOC_CAMERA_MT9T112
+VIDEO_OV772X
+SOC_CAMERA_OV772X
+VIDEO_TW9910
+SOC_CAMERA_TW9910
 
 [3.12.0]
 # BIN_ATTR_RW was changed
@@ -221,7 +230,6 @@ SOC_CAMERA
 SOC_CAMERA_MT9V022
 SOC_CAMERA_MT9M001
 SOC_CAMERA_MT9T031
-SOC_CAMERA_OV772X
 SOC_CAMERA_PLATFORM
 # Needs of_match_ptr
 VIDEO_THS8200
-- 
2.7.4

Re: [PATCH 2/7] Disable additional drivers requiring gpio/consumer.h

[Jasmin J.]

Hello Brad!

Tonight build broke due to patch 95ee4c285022!
You enabled VIDEO_OV2685 for 3.13., which doesn't
compile for Kernels older than 3.17. When you look
to the Kernel 3.17 section a lot of the drivers you
enabled for 3.13 with your patch should be enabled
for 3.17 only.

So please test this and provide a follow up patch.
I will not revert 95ee4c285022 now, except you can't
fix it in a reasonable time frame.

If you like and you have time you can improve
scripts/make_kconfig.pl to detect such an issue to
avoid future problems like this. I also had such a
situation with enabling a driver twice in the past.

BR,
   Jasmin

Re: [PATCH 2/7] Disable additional drivers requiring gpio/consumer.h

[Brad Love]

Hi Jasmin,


On 2018-05-12 04:31, Jasmin J. wrote:
> Hello Brad!
>
> Tonight build broke due to patch 95ee4c285022!
> You enabled VIDEO_OV2685 for 3.13., which doesn't
> compile for Kernels older than 3.17. When you look
> to the Kernel 3.17 section a lot of the drivers you
> enabled for 3.13 with your patch should be enabled
> for 3.17 only.
>
> So please test this and provide a follow up patch.
> I will not revert 95ee4c285022 now, except you can't
> fix it in a reasonable time frame.
>
> If you like and you have time you can improve
> scripts/make_kconfig.pl to detect such an issue to
> avoid future problems like this. I also had such a
> situation with enabling a driver twice in the past.
>
> BR,
>    Jasmin

Apologies. Interesting though, as I was working against 3.10 while
submitting this. I will verify and submit a correction today.

Cheers,

Brad

Re: [PATCH 2/7] Disable additional drivers requiring gpio/consumer.h

[Brad Love]

Hi Jasmin,


On 2018-05-12 08:20, Brad Love wrote:
> Hi Jasmin,
>
>
> On 2018-05-12 04:31, Jasmin J. wrote:
>> Hello Brad!
>>
>> Tonight build broke due to patch 95ee4c285022!
>> You enabled VIDEO_OV2685 for 3.13., which doesn't
>> compile for Kernels older than 3.17. When you look
>> to the Kernel 3.17 section a lot of the drivers you
>> enabled for 3.13 with your patch should be enabled
>> for 3.17 only.
>>
>> So please test this and provide a follow up patch.
>> I will not revert 95ee4c285022 now, except you can't
>> fix it in a reasonable time frame.
>>
>> If you like and you have time you can improve
>> scripts/make_kconfig.pl to detect such an issue to
>> avoid future problems like this. I also had such a
>> situation with enabling a driver twice in the past.
>>
>> BR,
>>    Jasmin
> Apologies. Interesting though, as I was working against 3.10 while
> submitting this. I will verify and submit a correction today.
>
> Cheers,
>
> Brad

Looks like the media_build environment on the system I made this patch
for was a little older, pretty much from right before you added all the
affected drivers to 3.17.0. Bad on me for not noticing. I submitted a
fixup for that and also moved the other drivers from the patch to 3.17.0
as well. Should compile again for you.

Cheers,

Brad

Re: [PATCH 2/7] Disable additional drivers requiring gpio/consumer.h

[Jasmin J.]

Hello Brad!

THX for the quick fix!

Hans did already apply it and Kernel 3.16 builds now.
I hope we will see a complete build tomorrow.

BR,
   Jasmin

[PATCH 3/7] Disable DVBC8SECTPFE for kernels older than 3.5

[Brad Love]

Needs of_find_i2c_adapter_by_node

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 v4l/versions.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/v4l/versions.txt b/v4l/versions.txt
index 2306830..f5e9a42 100644
--- a/v4l/versions.txt
+++ b/v4l/versions.txt
@@ -182,6 +182,8 @@ VIDEO_S5K6AA
 MEDIA_TUNER_E4000
 # needs regmap_init with 4 arguments
 DVB_USB_AF9015
+# needs of_find_i2c_adapter_by_node
+DVB_C8SECTPFE
 
 [3.4.0]
 # needs devm_regulator_bulk_get
-- 
2.7.4

[PATCH 4/7] Disable SOC_CAMERA for kernels older than 3.5

[Brad Love]

Needs of_find_i2c_adapter_by_node

Migrated from 3.5 to 3.12.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 v4l/versions.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/v4l/versions.txt b/v4l/versions.txt
index f5e9a42..fcd6949 100644
--- a/v4l/versions.txt
+++ b/v4l/versions.txt
@@ -184,6 +184,7 @@ MEDIA_TUNER_E4000
 DVB_USB_AF9015
 # needs of_find_i2c_adapter_by_node
 DVB_C8SECTPFE
+SOC_CAMERA
 
 [3.4.0]
 # needs devm_regulator_bulk_get
@@ -228,7 +229,6 @@ VIDEO_GS1662
 
 [3.2.0]
 # due to the rename at include/linux from "pm_qos_params.h" to "pm_qos.h"
-SOC_CAMERA
 SOC_CAMERA_MT9V022
 SOC_CAMERA_MT9M001
 SOC_CAMERA_MT9T031
-- 
2.7.4

[PATCH 5/7] Header location fix for 3.5.0 to 3.11.x

[Brad Love]

Header does not exist before 3.5.0 and is merged into linux/i2c.h
in 3.12.0.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 v4l/compat.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/v4l/compat.h b/v4l/compat.h
index d52c602..87ce401 100644
--- a/v4l/compat.h
+++ b/v4l/compat.h
@@ -2414,4 +2414,11 @@ static inline void *memdup_user_nul(const void __user *src, size_t len)
 #include <linux/frame.h>
 #endif
 
+/* header location for of_find_i2c_[device,adapter]_by_node */
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 12, 0)
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 5, 0)
+#include <linux/of_i2c.h>
+#endif
+#endif
+
 #endif /*  _COMPAT_H */
-- 
2.7.4

Re: [PATCH 5/7] Header location fix for 3.5.0 to 3.11.x

[Jasmin J.]

Hello Brad!

This patch added the inclusion of "linux/of_i2c.h".
This gave the warnings in the last nightly build for Kernel
3.6 - 3.9.

I just pushed a fix for that, so we should have an OK build this
night.

BR,
   Jasmin

Re: [PATCH 5/7] Header location fix for 3.5.0 to 3.11.x

[Brad Love]

On 2018-05-12 14:42, Jasmin J. wrote:
> Hello Brad!
>
> This patch added the inclusion of "linux/of_i2c.h".
> This gave the warnings in the last nightly build for Kernel
> 3.6 - 3.9.
>
> I just pushed a fix for that, so we should have an OK build this
> night.
>
> BR,
>    Jasmin

Thanks for the fix Jasmin. Are the build logs public?

Cheers,

Brad

Re: [PATCH 5/7] Header location fix for 3.5.0 to 3.11.x

[Jasmin J.]

Hello Brad!

> Are the build logs public?
Look for the eMail
   cron job: media_tree daily build: ?????

The ????? is OK, WARNINGS or ERRORS
In this eMail there are links to the short and long logfile.


Hans uses a complete build system which can download the Kernel sources, build
media-build against all of them and does also Sparse and Smatch tests. It even
builds gcc 7.x and all of the required tools for you. You can find it here:
   https://git.linuxtv.org/hverkuil/build-scripts.git

BR,
   Jasmin

[PATCH 6/7] Fix frame vector wildcard file check

[Brad Love]

This check was consistently failing on all systems tested.
The path to object directory is used here to explicitly override
CWD. The thought is, if frame_vector.c exists in the build
directory then the build system has determined it is required,
and the source therefore should be compiled. The module will
not be built unless the build system has enabled it's config
option anyways, so this change should be safe in all circumstances.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 v4l/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/v4l/Makefile b/v4l/Makefile
index b512600..270a624 100644
--- a/v4l/Makefile
+++ b/v4l/Makefile
@@ -88,7 +88,7 @@ ifneq ($(filter $(no-makefile-media-targets), $(MAKECMDGOALS)),)
 endif
 
 makefile-mm := 1
-ifeq ($(wildcard ../linux/mm/frame_vector.c),)
+ifeq ("$(wildcard $(obj)/frame_vector.c)","")
 	makefile-mm := 0
 endif
 
-- 
2.7.4

Re: [PATCH 6/7] Fix frame vector wildcard file check

[Hans Verkuil]

On 04/26/18 19:19, Brad Love wrote:
> This check was consistently failing on all systems tested.
> The path to object directory is used here to explicitly override
> CWD. The thought is, if frame_vector.c exists in the build
> directory then the build system has determined it is required,
> and the source therefore should be compiled. The module will
> not be built unless the build system has enabled it's config
> option anyways, so this change should be safe in all circumstances.
> 
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
>  v4l/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/v4l/Makefile b/v4l/Makefile
> index b512600..270a624 100644
> --- a/v4l/Makefile
> +++ b/v4l/Makefile
> @@ -88,7 +88,7 @@ ifneq ($(filter $(no-makefile-media-targets), $(MAKECMDGOALS)),)
>  endif
>  
>  makefile-mm := 1
> -ifeq ($(wildcard ../linux/mm/frame_vector.c),)
> +ifeq ("$(wildcard $(obj)/frame_vector.c)","")
>  	makefile-mm := 0
>  endif
>  
> 

Ah, nice. Hopefully this fixes this issue. I never could figure out
why it failed for some people.

Regards,

	Hans

[PATCH 7/7] Add config-compat.h override config-mycompat.h

[Brad Love]

config-mycompat.h is for overriding macros which are incorrectly
enabled on certain kernels by the build system. The file should be
left empty, unless build errors are encountered for a kernel. The
file is removed by distclean, therefore should be externally
sourced, before the build process starts, when required.

In standard operation the file is empty, but if a particular kernel has
incorrectly enabled options defined this allows them to be undefined.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 v4l/Makefile | 3 ++-
 v4l/compat.h | 7 +++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/v4l/Makefile b/v4l/Makefile
index 270a624..ee18d11 100644
--- a/v4l/Makefile
+++ b/v4l/Makefile
@@ -273,6 +273,7 @@ links::
 	@find ../linux/drivers/misc -name '*.[ch]' -type f -print0 | xargs -0n 255 ln -sf --target-directory=.
 
 config-compat.h:: $(obj)/.version .myconfig scripts/make_config_compat.pl
+	-touch $(obj)/config-mycompat.h
 	perl scripts/make_config_compat.pl $(SRCDIR) $(obj)/.myconfig $(obj)/config-compat.h
 
 kernel-links makelinks::
@@ -298,7 +299,7 @@ clean::
 distclean:: clean
 	-rm -f .version .*.o.flags .*.o.d *.mod.gcno Makefile.media \
 		Kconfig Kconfig.kern .config .config.cmd .myconfig \
-		.kconfig.dep
+		.kconfig.dep config-mycompat.h
 	-rm -rf .tmp_versions .tmp*.ver .tmp*.o .*.gcno .cache.mk
 	-rm -f scripts/lxdialog scripts/kconfig
 	@find .. -name '*.orig' -exec rm '{}' \;
diff --git a/v4l/compat.h b/v4l/compat.h
index 87ce401..db48fdf 100644
--- a/v4l/compat.h
+++ b/v4l/compat.h
@@ -8,6 +8,13 @@
 #include <linux/version.h>
 
 #include "config-compat.h"
+/* config-mycompat.h is for overriding #defines which
+ * are incorrectly enabled on certain kernels. The file
+ * should be left empty, unless build errors are encountered
+ * for a kernel. The file is removed by distclean, therefore
+ * should be externally sourced, before compilation, when required.
+ */
+#include "config-mycompat.h"
 
 #ifndef SZ_512
 #define SZ_512				0x00000200
-- 
2.7.4

Re: [PATCH 7/7] Add config-compat.h override config-mycompat.h

[Hans Verkuil]

Hi Brad,

On 04/26/18 19:19, Brad Love wrote:
> config-mycompat.h is for overriding macros which are incorrectly
> enabled on certain kernels by the build system. The file should be
> left empty, unless build errors are encountered for a kernel. The
> file is removed by distclean, therefore should be externally
> sourced, before the build process starts, when required.
> 
> In standard operation the file is empty, but if a particular kernel has
> incorrectly enabled options defined this allows them to be undefined.

Can you give an example where this will be used?

FYI: I've committed patches 1-6, but I don't quite understand when this patch
is needed.

With "for overriding macros which are incorrectly enabled on certain kernels"
do you mean when distros do backports of features from later kernels?

Regards,

	Hans

> 
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
>  v4l/Makefile | 3 ++-
>  v4l/compat.h | 7 +++++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/v4l/Makefile b/v4l/Makefile
> index 270a624..ee18d11 100644
> --- a/v4l/Makefile
> +++ b/v4l/Makefile
> @@ -273,6 +273,7 @@ links::
>  	@find ../linux/drivers/misc -name '*.[ch]' -type f -print0 | xargs -0n 255 ln -sf --target-directory=.
>  
>  config-compat.h:: $(obj)/.version .myconfig scripts/make_config_compat.pl
> +	-touch $(obj)/config-mycompat.h
>  	perl scripts/make_config_compat.pl $(SRCDIR) $(obj)/.myconfig $(obj)/config-compat.h
>  
>  kernel-links makelinks::
> @@ -298,7 +299,7 @@ clean::
>  distclean:: clean
>  	-rm -f .version .*.o.flags .*.o.d *.mod.gcno Makefile.media \
>  		Kconfig Kconfig.kern .config .config.cmd .myconfig \
> -		.kconfig.dep
> +		.kconfig.dep config-mycompat.h
>  	-rm -rf .tmp_versions .tmp*.ver .tmp*.o .*.gcno .cache.mk
>  	-rm -f scripts/lxdialog scripts/kconfig
>  	@find .. -name '*.orig' -exec rm '{}' \;
> diff --git a/v4l/compat.h b/v4l/compat.h
> index 87ce401..db48fdf 100644
> --- a/v4l/compat.h
> +++ b/v4l/compat.h
> @@ -8,6 +8,13 @@
>  #include <linux/version.h>
>  
>  #include "config-compat.h"
> +/* config-mycompat.h is for overriding #defines which
> + * are incorrectly enabled on certain kernels. The file
> + * should be left empty, unless build errors are encountered
> + * for a kernel. The file is removed by distclean, therefore
> + * should be externally sourced, before compilation, when required.
> + */
> +#include "config-mycompat.h"
>  
>  #ifndef SZ_512
>  #define SZ_512				0x00000200
> 

Re: [PATCH 7/7] Add config-compat.h override config-mycompat.h

[Brad Love]

Hi Hans,


On 2018-05-11 09:41, Hans Verkuil wrote:
> Hi Brad,
>
> On 04/26/18 19:19, Brad Love wrote:
>> config-mycompat.h is for overriding macros which are incorrectly
>> enabled on certain kernels by the build system. The file should be
>> left empty, unless build errors are encountered for a kernel. The
>> file is removed by distclean, therefore should be externally
>> sourced, before the build process starts, when required.
>>
>> In standard operation the file is empty, but if a particular kernel has
>> incorrectly enabled options defined this allows them to be undefined.
> Can you give an example where this will be used?
>
> FYI: I've committed patches 1-6, but I don't quite understand when this patch
> is needed.
>
> With "for overriding macros which are incorrectly enabled on certain kernels"
> do you mean when distros do backports of features from later kernels?
>
> Regards,
>
> 	Hans


Apologies if I was not very clear. Yes, this is for use in
kernels/distros whose maintainers have integrated various backports, and
which the media_build system does not pick up on for whatever reason. At
that point there are options defined in config-compat.h, which enable
backports in compat.h, but which already exist in the target kernel.

For example, on the device I'm working on right now, in kernel 3.10, I
have to supply the following three options in config-mycompat.h or
modify the tree and stuff them right into the top of compat.h:

#undef NEED_WRITEL_RELAXED
#undef NEED_PM_RUNTIME_GET
#undef NEED_PFN_TO_PHYS


The above disables those three media_build backports and allows
everything to build. It seems there is quite often at least one backport
I must disable, and some target kernels require multiple backports disabled.

Cheers,

Brad



>
>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>> ---
>>  v4l/Makefile | 3 ++-
>>  v4l/compat.h | 7 +++++++
>>  2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/v4l/Makefile b/v4l/Makefile
>> index 270a624..ee18d11 100644
>> --- a/v4l/Makefile
>> +++ b/v4l/Makefile
>> @@ -273,6 +273,7 @@ links::
>>  	@find ../linux/drivers/misc -name '*.[ch]' -type f -print0 | xargs -0n 255 ln -sf --target-directory=.
>>  
>>  config-compat.h:: $(obj)/.version .myconfig scripts/make_config_compat.pl
>> +	-touch $(obj)/config-mycompat.h
>>  	perl scripts/make_config_compat.pl $(SRCDIR) $(obj)/.myconfig $(obj)/config-compat.h
>>  
>>  kernel-links makelinks::
>> @@ -298,7 +299,7 @@ clean::
>>  distclean:: clean
>>  	-rm -f .version .*.o.flags .*.o.d *.mod.gcno Makefile.media \
>>  		Kconfig Kconfig.kern .config .config.cmd .myconfig \
>> -		.kconfig.dep
>> +		.kconfig.dep config-mycompat.h
>>  	-rm -rf .tmp_versions .tmp*.ver .tmp*.o .*.gcno .cache.mk
>>  	-rm -f scripts/lxdialog scripts/kconfig
>>  	@find .. -name '*.orig' -exec rm '{}' \;
>> diff --git a/v4l/compat.h b/v4l/compat.h
>> index 87ce401..db48fdf 100644
>> --- a/v4l/compat.h
>> +++ b/v4l/compat.h
>> @@ -8,6 +8,13 @@
>>  #include <linux/version.h>
>>  
>>  #include "config-compat.h"
>> +/* config-mycompat.h is for overriding #defines which
>> + * are incorrectly enabled on certain kernels. The file
>> + * should be left empty, unless build errors are encountered
>> + * for a kernel. The file is removed by distclean, therefore
>> + * should be externally sourced, before compilation, when required.
>> + */
>> +#include "config-mycompat.h"
>>  
>>  #ifndef SZ_512
>>  #define SZ_512				0x00000200
>>

Re: [PATCH 7/7] Add config-compat.h override config-mycompat.h

[Hans Verkuil]

On 05/11/18 17:08, Brad Love wrote:
> Hi Hans,
> 
> 
> On 2018-05-11 09:41, Hans Verkuil wrote:
>> Hi Brad,
>>
>> On 04/26/18 19:19, Brad Love wrote:
>>> config-mycompat.h is for overriding macros which are incorrectly
>>> enabled on certain kernels by the build system. The file should be
>>> left empty, unless build errors are encountered for a kernel. The
>>> file is removed by distclean, therefore should be externally
>>> sourced, before the build process starts, when required.
>>>
>>> In standard operation the file is empty, but if a particular kernel has
>>> incorrectly enabled options defined this allows them to be undefined.
>> Can you give an example where this will be used?
>>
>> FYI: I've committed patches 1-6, but I don't quite understand when this patch
>> is needed.
>>
>> With "for overriding macros which are incorrectly enabled on certain kernels"
>> do you mean when distros do backports of features from later kernels?
>>
>> Regards,
>>
>> 	Hans
> 
> 
> Apologies if I was not very clear. Yes, this is for use in
> kernels/distros whose maintainers have integrated various backports, and
> which the media_build system does not pick up on for whatever reason. At
> that point there are options defined in config-compat.h, which enable
> backports in compat.h, but which already exist in the target kernel.
> 
> For example, on the device I'm working on right now, in kernel 3.10, I
> have to supply the following three options in config-mycompat.h or
> modify the tree and stuff them right into the top of compat.h:
> 
> #undef NEED_WRITEL_RELAXED
> #undef NEED_PM_RUNTIME_GET
> #undef NEED_PFN_TO_PHYS
> 
> 
> The above disables those three media_build backports and allows
> everything to build. It seems there is quite often at least one backport
> I must disable, and some target kernels require multiple backports disabled.

OK, that's what I thought. Can you post a v2 of this patch with this explanation
included? Both in the commit log and in the compat.h comment.

That should make it clear what the purpose of this file is.

Regards,

	Hans

> 
> Cheers,
> 
> Brad
> 
> 
> 
>>
>>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>>> ---
>>>  v4l/Makefile | 3 ++-
>>>  v4l/compat.h | 7 +++++++
>>>  2 files changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/v4l/Makefile b/v4l/Makefile
>>> index 270a624..ee18d11 100644
>>> --- a/v4l/Makefile
>>> +++ b/v4l/Makefile
>>> @@ -273,6 +273,7 @@ links::
>>>  	@find ../linux/drivers/misc -name '*.[ch]' -type f -print0 | xargs -0n 255 ln -sf --target-directory=.
>>>  
>>>  config-compat.h:: $(obj)/.version .myconfig scripts/make_config_compat.pl
>>> +	-touch $(obj)/config-mycompat.h
>>>  	perl scripts/make_config_compat.pl $(SRCDIR) $(obj)/.myconfig $(obj)/config-compat.h
>>>  
>>>  kernel-links makelinks::
>>> @@ -298,7 +299,7 @@ clean::
>>>  distclean:: clean
>>>  	-rm -f .version .*.o.flags .*.o.d *.mod.gcno Makefile.media \
>>>  		Kconfig Kconfig.kern .config .config.cmd .myconfig \
>>> -		.kconfig.dep
>>> +		.kconfig.dep config-mycompat.h
>>>  	-rm -rf .tmp_versions .tmp*.ver .tmp*.o .*.gcno .cache.mk
>>>  	-rm -f scripts/lxdialog scripts/kconfig
>>>  	@find .. -name '*.orig' -exec rm '{}' \;
>>> diff --git a/v4l/compat.h b/v4l/compat.h
>>> index 87ce401..db48fdf 100644
>>> --- a/v4l/compat.h
>>> +++ b/v4l/compat.h
>>> @@ -8,6 +8,13 @@
>>>  #include <linux/version.h>
>>>  
>>>  #include "config-compat.h"
>>> +/* config-mycompat.h is for overriding #defines which
>>> + * are incorrectly enabled on certain kernels. The file
>>> + * should be left empty, unless build errors are encountered
>>> + * for a kernel. The file is removed by distclean, therefore
>>> + * should be externally sourced, before compilation, when required.
>>> + */
>>> +#include "config-mycompat.h"
>>>  
>>>  #ifndef SZ_512
>>>  #define SZ_512				0x00000200
>>>
> 
> 

Re: [PATCH 7/7] Add config-compat.h override config-mycompat.h

[Brad Love]

Hi Hans,


On 2018-05-11 10:11, Hans Verkuil wrote:
> On 05/11/18 17:08, Brad Love wrote:
>> Hi Hans,
>>
>>
>> On 2018-05-11 09:41, Hans Verkuil wrote:
>>> Hi Brad,
>>>
>>> On 04/26/18 19:19, Brad Love wrote:
>>>> config-mycompat.h is for overriding macros which are incorrectly
>>>> enabled on certain kernels by the build system. The file should be
>>>> left empty, unless build errors are encountered for a kernel. The
>>>> file is removed by distclean, therefore should be externally
>>>> sourced, before the build process starts, when required.
>>>>
>>>> In standard operation the file is empty, but if a particular kernel has
>>>> incorrectly enabled options defined this allows them to be undefined.
>>> Can you give an example where this will be used?
>>>
>>> FYI: I've committed patches 1-6, but I don't quite understand when this patch
>>> is needed.
>>>
>>> With "for overriding macros which are incorrectly enabled on certain kernels"
>>> do you mean when distros do backports of features from later kernels?
>>>
>>> Regards,
>>>
>>> 	Hans
>>
>> Apologies if I was not very clear. Yes, this is for use in
>> kernels/distros whose maintainers have integrated various backports, and
>> which the media_build system does not pick up on for whatever reason. At
>> that point there are options defined in config-compat.h, which enable
>> backports in compat.h, but which already exist in the target kernel.
>>
>> For example, on the device I'm working on right now, in kernel 3.10, I
>> have to supply the following three options in config-mycompat.h or
>> modify the tree and stuff them right into the top of compat.h:
>>
>> #undef NEED_WRITEL_RELAXED
>> #undef NEED_PM_RUNTIME_GET
>> #undef NEED_PFN_TO_PHYS
>>
>>
>> The above disables those three media_build backports and allows
>> everything to build. It seems there is quite often at least one backport
>> I must disable, and some target kernels require multiple backports disabled.
> OK, that's what I thought. Can you post a v2 of this patch with this explanation
> included? Both in the commit log and in the compat.h comment.
>
> That should make it clear what the purpose of this file is.
>
> Regards,
>
> 	Hans


Will do now.



>
>> Cheers,
>>
>> Brad
>>
>>
>>
>>>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>>>> ---
>>>>  v4l/Makefile | 3 ++-
>>>>  v4l/compat.h | 7 +++++++
>>>>  2 files changed, 9 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/v4l/Makefile b/v4l/Makefile
>>>> index 270a624..ee18d11 100644
>>>> --- a/v4l/Makefile
>>>> +++ b/v4l/Makefile
>>>> @@ -273,6 +273,7 @@ links::
>>>>  	@find ../linux/drivers/misc -name '*.[ch]' -type f -print0 | xargs -0n 255 ln -sf --target-directory=.
>>>>  
>>>>  config-compat.h:: $(obj)/.version .myconfig scripts/make_config_compat.pl
>>>> +	-touch $(obj)/config-mycompat.h
>>>>  	perl scripts/make_config_compat.pl $(SRCDIR) $(obj)/.myconfig $(obj)/config-compat.h
>>>>  
>>>>  kernel-links makelinks::
>>>> @@ -298,7 +299,7 @@ clean::
>>>>  distclean:: clean
>>>>  	-rm -f .version .*.o.flags .*.o.d *.mod.gcno Makefile.media \
>>>>  		Kconfig Kconfig.kern .config .config.cmd .myconfig \
>>>> -		.kconfig.dep
>>>> +		.kconfig.dep config-mycompat.h
>>>>  	-rm -rf .tmp_versions .tmp*.ver .tmp*.o .*.gcno .cache.mk
>>>>  	-rm -f scripts/lxdialog scripts/kconfig
>>>>  	@find .. -name '*.orig' -exec rm '{}' \;
>>>> diff --git a/v4l/compat.h b/v4l/compat.h
>>>> index 87ce401..db48fdf 100644
>>>> --- a/v4l/compat.h
>>>> +++ b/v4l/compat.h
>>>> @@ -8,6 +8,13 @@
>>>>  #include <linux/version.h>
>>>>  
>>>>  #include "config-compat.h"
>>>> +/* config-mycompat.h is for overriding #defines which
>>>> + * are incorrectly enabled on certain kernels. The file
>>>> + * should be left empty, unless build errors are encountered
>>>> + * for a kernel. The file is removed by distclean, therefore
>>>> + * should be externally sourced, before compilation, when required.
>>>> + */
>>>> +#include "config-mycompat.h"
>>>>  
>>>>  #ifndef SZ_512
>>>>  #define SZ_512				0x00000200
>>>>
>>

Re: [PATCH 7/7] Add config-compat.h override config-mycompat.h

[Jasmin J.]

Hello Brad!

> and which the media_build system does not pick up on for whatever
> reason.
Maybe it would be better to analyse why "make_config_compat.pl" selects
wrongly the compatibility code.

> It seems there is quite often at least one backport I must disable,
> and some target kernels require multiple backports disabled.
This sounds strange. media-build should handle those cases correctly
in my opinion and should be fixed.
At least we should check why this happens.

Patch 7/7 sounds like a workaround for me.
If there is really no other solution, than we need to implement this
possibility for distro maintainers.

On the other hand, why is media-build used by distro maintainers at all?
I thought distro Kernels are built with a full tree and thus doesn't
need media-build.

BR,
   Jasmin

Re: [PATCH 7/7] Add config-compat.h override config-mycompat.h

[Brad Love]

Hi Jasmin,


On 2018-05-11 15:02, Jasmin J. wrote:
> Hello Brad!
>
>> and which the media_build system does not pick up on for whatever
>> reason.
> Maybe it would be better to analyse why "make_config_compat.pl" selects
> wrongly the compatibility code.

I've found several reasons, but the one I seem to encounter often
is the symbols are located in files that the config_compat script
does not check for. Some symbols have moved between revisions,
so a check that works in 4.2 will fail in 3.10 and will also fail in 3.2.
I've also seen not-found symbols defined in arch/ code, which makes
it a little difficult to add additional paths to the search. There is also
the case of a maintainer who puts their backports wherever they
felt like and just glued things together in their build.

I'm not averse to handling this other ways, but while I was looking at
fixing make_config_compat.pl it just seemed that the script would
have to be made more complex. The search strategy would have to
change to include additional search paths, possibly depending on
kernel version as well as support to find symbols in the arch code,
while making sure the symbol was found in the correct target arch.

I know this is a workaround, but I have to do this 'workaround' in
some form for almost every package I maintain.


>
>> It seems there is quite often at least one backport I must disable,
>> and some target kernels require multiple backports disabled.
> This sounds strange. media-build should handle those cases correctly
> in my opinion and should be fixed.
> At least we should check why this happens.
>
> Patch 7/7 sounds like a workaround for me.
> If there is really no other solution, than we need to implement this
> possibility for distro maintainers.

This is not for distro maintainers. I am the lead engineer at Hauppauge
and and a responsibility of mine is the support our hardware on the
largest amount of systems and architectures possible. I work with
kernels anywhere from 3.2 to 4.15, all provided by either manufacturers
or distributions. Some are close to mainline, others not so much.

>
> On the other hand, why is media-build used by distro maintainers at all?
> I thought distro Kernels are built with a full tree and thus doesn't
> need media-build.
>
> BR,
>    Jasmin

I am not a distro maintainer. What I do is maintain and provide out of
tree driver packages for a large variety of systems, as well as in tree
integrations of the entire media tree for an assortment of Ubuntu kernels.

I have no influence over how a maintainer or distro publisher organizes
source code. I just take their tree and either compile the media_build
system out of tree, or integrate it completely. It is not very often I can
get *random_arch* kernel tree from *random_manufacturer* and
media_build 'just works'.

Like I said in the cover-letter, I'm totally open to better ways of handling
this. I am just honestly tired of having to fudge with things before every
new build, dirtying up the media_build tree. I like things reproducible and
clean and this seems to be the only thing left preventing that.

Cheers,

Brad

Re: [PATCH 7/7] Add config-compat.h override config-mycompat.h

[Jasmin J.]

Hello Brad!

THX for this clarification!

So you tried already to fix the config_compat script and I agree with you that
it is difficult for you because of the various Kernels and distributions you
have to maintain.

Then your workaround is indeed a possibility to use media-build to build your
modules out of tree for all the combinations you have. Maybe in the future
other people may use this also.

BR,
   Jasmin

Re: [PATCH 7/7] Add config-compat.h override config-mycompat.h

[Brad Love]

Hi Jasmin,


On 2018-05-11 16:43, Jasmin J. wrote:
> Hello Brad!
>
> THX for this clarification!
>
> So you tried already to fix the config_compat script and I agree with you that
> it is difficult for you because of the various Kernels and distributions you
> have to maintain.
>
> Then your workaround is indeed a possibility to use media-build to build your
> modules out of tree for all the combinations you have. Maybe in the future
> other people may use this also.
>
> BR,
>    Jasmin

Please do check out my v2. Hans also had questions and concerns over the
intended usage, so I did my best to explain it thoroughly in the v2 commit
message along with inside of compat.h. I hope it is clear enough now to
understand for anyone who might need the feature in the future.

Cheers,

Brad