[SRU F][PATCH 01/02] UBUNTU: [Packaging] Add support for ODM drivers

[Marcelo Henrique Cerri]

Can you give us an explanation regarding the purpose of this and what
ODM stands for?

I have other comments bellow:

On Fri, Jan 22, 2021 at 05:28:07PM +0100, Stefan Bader wrote:
> BugLink: https://bugs.launchpad.net/bugs/1912789
> 
> We want to be able to selectively turn on ODM driver support for those
> kernels/arches we have to but otherwise not inherit this to other
> derivatives. This is done by a new config option which we will have to
> depend on in the new drivers config options. Support is toggled by
> changing a makefile rule variable.
> The logic is a bit reversed. The common config has it enabled but it
> will be turn off for builds unless one asks for it to be on. Example
> of that will be in a follow-up patch.
> 
> Signed-off-by: Stefan Bader <stefan.bader at canonical.com>
> ---
>  debian.master/config/config.common.ubuntu | 1 +
>  debian/rules.d/0-common-vars.mk           | 4 ++++
>  debian/rules.d/1-maintainer.mk            | 1 +
>  debian/rules.d/2-binary-arch.mk           | 3 +++
>  ubuntu/Kconfig                            | 6 ++++++
>  5 files changed, 15 insertions(+)
> 
> diff --git a/debian.master/config/config.common.ubuntu b/debian.master/config/config.common.ubuntu
> index 75cc55c2d321..00fae58b1ade 100644
> --- a/debian.master/config/config.common.ubuntu
> +++ b/debian.master/config/config.common.ubuntu
> @@ -10362,6 +10362,7 @@ CONFIG_UBIFS_FS_ZLIB=y
>  CONFIG_UBIFS_FS_ZSTD=y
>  # CONFIG_UBSAN is not set
>  CONFIG_UBSAN_ALIGNMENT=y
> +CONFIG_UBUNTU_ODM_DRIVERS=y
>  CONFIG_UCB1400_CORE=m
>  CONFIG_UCLAMP_BUCKETS_COUNT=5
>  CONFIG_UCLAMP_TASK=y
> diff --git a/debian/rules.d/0-common-vars.mk b/debian/rules.d/0-common-vars.mk
> index 5394b7c96f82..b71546ab03a0 100644
> --- a/debian/rules.d/0-common-vars.mk
> +++ b/debian/rules.d/0-common-vars.mk
> @@ -187,6 +187,10 @@ do_common_headers_indep=true
>  # add a 'full source' mode
>  do_full_source=false
>  
> +# Add an option to enable special drivers which should only be build when
> +# explicitly enabled.
> +do_odm_drivers=false
> +
>  # build tools
>  ifneq ($(wildcard $(CURDIR)/tools),)
>  	ifeq ($(do_tools),)
> diff --git a/debian/rules.d/1-maintainer.mk b/debian/rules.d/1-maintainer.mk
> index 956829027e0f..fabdf0888fce 100644
> --- a/debian/rules.d/1-maintainer.mk
> +++ b/debian/rules.d/1-maintainer.mk
> @@ -86,6 +86,7 @@ printenv:
>  	@echo "do_flavour_header_package = $(do_flavour_header_package)"
>  	@echo "do_common_headers_indep   = $(do_common_headers_indep)"
>  	@echo "do_full_source            = $(do_full_source)"
> +	@echo "do_odm_drivers            = $(do_odm_drivers)"
>  	@echo "do_tools                  = $(do_tools)"
>  	@echo "do_any_tools              = $(do_any_tools)"
>  	@echo "do_linux_tools            = $(do_linux_tools)"
> diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk
> index 703fd2ade338..6311c9a74e05 100644
> --- a/debian/rules.d/2-binary-arch.mk
> +++ b/debian/rules.d/2-binary-arch.mk
> @@ -31,6 +31,9 @@ $(stampdir)/stamp-prepare-tree-%: $(commonconfdir)/config.common.$(family) $(arc
>  	[ "$(do_full_source)" != 'true' ] && true || \
>  		rsync -a --exclude debian --exclude debian.master --exclude $(DEBIAN) * $(builddir)/build-$*
>  	cat $(wordlist 1,3,$^) | sed -e 's/.*CONFIG_VERSION_SIGNATURE.*/CONFIG_VERSION_SIGNATURE="Ubuntu $(release)-$(revision)-$* $(raw_kernelversion)"/' > $(builddir)/build-$*/.config
> +	[ "$(do_odm_drivers)" = 'true' ] && true || \
> +		sed -ie 's/.*CONFIG_UBUNTU_ODM_DRIVERS.*/# CONFIG_UBUNTU_ODM_DRIVERS is not set/' \
> +		    $(builddir)/build-$*/.config


So if I understood that correctly, derivatives will have to set the
config option to "y" *and* do_odm_drivers=true if they want to opt
in. Is that really necessary?

I'm probably missing something, but why is do_odm_drivers even
necessary?


>  	find $(builddir)/build-$* -name "*.ko" | xargs rm -f
>  	$(build_cd) $(kmake) $(build_O) -j1 syncconfig prepare scripts
>  	touch $@
> diff --git a/ubuntu/Kconfig b/ubuntu/Kconfig
> index 8cea998f29a3..d969907b0bfd 100644
> --- a/ubuntu/Kconfig
> +++ b/ubuntu/Kconfig
> @@ -1,5 +1,11 @@
>  menu "Ubuntu Supplied Third-Party Device Drivers"
>  
> +
> +config UBUNTU_ODM_DRIVERS
> +bool "Enable support for ODM drivers"


That will cause all derivatives to be prompted about this config. Is
that what you are planning?


> +help
> +	Turn on support for Ubuntu ODM supplied drivers


A better description here would be welcomed. What's ODM?


> +
>  #
>  # NOTE: to allow drivers to be added and removed without causing merge
>  # collisions you should add new entries in the middle of the six lines
> -- 
> 2.25.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

-- 
Regards,
Marcelo

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20210122/dee31af4/attachment.sig>