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

Fri Jan 22 21:22:41 UTC 2021

Hi, Stefan.

I'm attaching one alternative that should work in a similar way using
do_odm_drivers=true without forcing every derivative to include the
new config option.

On Fri, Jan 22, 2021 at 02:34:40PM -0300, Marcelo Henrique Cerri wrote:
> 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
> 

-- 
Regards,
Marcelo

-------------- next part --------------
A non-text attachment was scrubbed...
Name: odm.diff
Type: text/x-diff
Size: 1078 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20210122/bac00b01/attachment-0001.diff>
-------------- 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/bac00b01/attachment-0001.sig>