NACK: pull request for raspi2 eoan kernel (set 2 falvours for armhf and change flavour name to raspi4 for arm64)

Fri Nov 22 09:07:03 UTC 2019

The email subject is invalid. It should be something in the form of:

[SRU][Eoan/raspi2][PULL] <a sensible subject>

> BugLink: https://bugs.launchpad.net/bugs/1853446
> 
> [Impact]
> It is risky to use single armhf kernel to support rpi2/3/4, since
> upstream has a specific config for rpi2/3 and another config for rpi4,
> the major difference are LPAE, HIGHMEM and VMSPLIT_3G, we already
> met a bug that is intrduced by HIGHMEM and VMSPLIT_3G on rpi2/3,
> so add two falvours for rpi2/3 and rpi4.
> flavour 1: raspi2 (for rpi2/3, disable LPAE, HIGHMEM and VMSPLIT_3G)
> flavour 2: raspi4 (for rpi4, enable LPAE, HIGHMEM and VMSPLIT_3G)
> 
> For arm64 kernel, we change the flavour name to raspi4, it supports
> rpi2/3/4.
>
> [Fix]
> Change the related configuration files in the debian.raspi2.
> 
> 
> [Test Case]
> After apply the patches, and I built two flavours of armhf and
> arm64 kernels, Then check the .config, they all meet the expectation.
> And boot those kernels on the rpi2/3/4 boards. worked well.

How did you build the kernels? Did you build source packages and upload them to
a test builder PPA?

> [Regression Risk]
> Low, just change the flavour name for arm64, and for armhf, the
> flavour raspi4 is basically same as bofre (raspi2), the new flavour
> raspi2 just disable LPAE/HIGHMEM/VMSPLIT_3G, it is same as the config
> for rpi2/3 in the upstream.
> 
> The following changes since commit c1a0780b4dd5a9bfdf828d024194156d0ea45774:
> 
>    UBUNTU: [Packaging] raspi2: Fix snapcraft.yaml (2019-11-13 10:37:29 
> +0100)
> 
> are available in the Git repository at:
> 
> https://github.com/jason77-wang/eoan-rpi4.git:Ubuntu-raspi2-lp1853446

IMO this is not a valid git repo+refspec. It should probably be:
https://github.com/jason77-wang/eoan-rpi4 Ubuntu-raspi2-lp1853446
but I assume you created this PR using 'git request-pull' so it must be
correct and I simply don't understand how to consume it :-)

> for you to fetch changes up to b557f61befe948366fde6f25befd34fdb6af0d89:
> 
>    UBUNTU: SAUCE: abi: change the abi name to raspi4 for arm64 kernel 
> (2019-11-21 20:59:39 +0800)
> 
> ----------------------------------------------------------------
> Hui Wang (4):
>        UBUNTU: SAUCE: add one more flavour to armhf kernel
>        UBUNTU: SAUCE: ABI: add abi for the flavour raspi4 on armhf kernel
>        UBUNTU: SAUCE: change the flavour name to raspi4 on arm64 kernel
>        UBUNTU: SAUCE: abi: change the abi name to raspi4 for arm64 kernel

These changes are not SAUCE patches. A SAUCE patch is a change of the kernel
code itself that is not in upstream. But you're modifying Ubuntu-specific files
under debian.raspi2. So the convention for the commit subject is:

For config changes (under debian.raspi2/config/)
UBUNTU: [Config] raspi2: ....

And for packaging changes (like adding a new flavor):
UBUNTU: [Packaging] raspi2: ....

Also, your commit messages are missing correct BugLink tags. You have:
https://bugs.launchpad.net/bugs/1853446
instead of:
BugLink: https://bugs.launchpad.net/bugs/1853446

Regarding that new flavor name 'raspi4': Is that really the name we want? Can't
we finally create a more generic flavor like 'raspi', so that in two years from
now people are not confused when they need to use a 'raspi4' kernel on their Pi
5 or 6 or something?

...Juerg

>   debian.raspi2/abi/5.3.0-1011.12/arm64/{raspi2 => 
> raspi4}                     |     0
>   debian.raspi2/abi/5.3.0-1011.12/arm64/{raspi2.compiler => 
> raspi4.compiler}   |     0
>   debian.raspi2/abi/5.3.0-1011.12/arm64/{raspi2.modules => 
> raspi4.modules}     |     0
>   debian.raspi2/abi/5.3.0-1011.12/arm64/{raspi2.retpoline => 
> raspi4.retpoline} |     0
>   debian.raspi2/abi/5.3.0-1011.12/armhf/raspi4 | 19968 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   debian.raspi2/abi/5.3.0-1011.12/armhf/raspi4.compiler |     1 +
>   debian.raspi2/abi/5.3.0-1011.12/armhf/raspi4.modules |  4292 
> +++++++++++++++++++++++++
>   debian.raspi2/abi/5.3.0-1011.12/armhf/raspi4.retpoline |     1 +
>   debian.raspi2/config/arm64/config.flavour.raspi2 |     3 -
>   debian.raspi2/config/arm64/config.flavour.raspi4 |     3 +
>   debian.raspi2/config/armhf/config.common.armhf |     2 -
>   debian.raspi2/config/armhf/config.flavour.raspi2 |     7 +
>   debian.raspi2/config/armhf/config.flavour.raspi4 |    10 +
>   debian.raspi2/config/config.common.ubuntu |     9 +-
>   debian.raspi2/control.d/vars.raspi4 |     6 +
>   debian.raspi2/etc/getabis |     4 +-
>   debian.raspi2/rules.d/arm64.mk |     2 +-
>   debian.raspi2/rules.d/armhf.mk |     2 +-
>   18 files changed, 24296 insertions(+), 14 deletions(-)
>   rename debian.raspi2/abi/5.3.0-1011.12/arm64/{raspi2 => raspi4} (100%)
>   rename debian.raspi2/abi/5.3.0-1011.12/arm64/{raspi2.compiler => 
> raspi4.compiler} (100%)
>   rename debian.raspi2/abi/5.3.0-1011.12/arm64/{raspi2.modules => 
> raspi4.modules} (100%)
>   rename debian.raspi2/abi/5.3.0-1011.12/arm64/{raspi2.retpoline => 
> raspi4.retpoline} (100%)
>   create mode 100644 debian.raspi2/abi/5.3.0-1011.12/armhf/raspi4
>   create mode 100644 debian.raspi2/abi/5.3.0-1011.12/armhf/raspi4.compiler
>   create mode 100644 debian.raspi2/abi/5.3.0-1011.12/armhf/raspi4.modules
>   create mode 100644 debian.raspi2/abi/5.3.0-1011.12/armhf/raspi4.retpoline
>   delete mode 100644 debian.raspi2/config/arm64/config.flavour.raspi2
>   create mode 100644 debian.raspi2/config/arm64/config.flavour.raspi4
>   create mode 100644 debian.raspi2/config/armhf/config.flavour.raspi4
>   create mode 100644 debian.raspi2/control.d/vars.raspi4
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20191122/12f22f11/attachment-0001.sig>