| Message ID | 20180822000648.23314-3-giulio.benetti@micronovasrl.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | sunxi-mali-mainline*: add arm64 build support and | expand |
Hello Giulio, On Wed, 22 Aug 2018 02:06:48 +0200, Giulio Benetti wrote: > Blobs for arm64(aarch64) are available at Bootlin Github and > sunxi-mali-mainline package has been updated to support them. > > Add support for building on arm64(aarch64): > - Bump version to latest commit: > For support H3 and H5 SoCs and r8p1 driver version. > > git shortlog --invert-grep --grep travis --no-merges 52ef1c5e133cc5fd791ca636239dc5e7b19c26d5.. > Maxime Ripard (6): > Add r8p1 release > sunxi: Move the reset test to a function > sunxi: Add H3 support > sunxi: Add H5 support > sunxi: Remove generic compatible > sunxi: Set clock rate in the driver > > - Add ARCH=arm or ARCH=arm64 to SUNXI_MALI_MAINLINE_DRIVER_MAKE_OPTS > to build for the right architecture according to BR2_arm or BR2_aarch64. > > Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com> So actually your PATCH 1/2 without PATCH 2/2 is broken, because PATCH 1/2 now allows to enable sunxi-mali-mainline on AArch64, but sunxi-mali-mainline-driver does not support AArch64. It seems like you have some issue understanding the concept of bisectability :-) Anyway, now that PATCH 1/2 is applied and pushed, it's too late to back out. > @@ -13,6 +13,12 @@ SUNXI_MALI_MAINLINE_DRIVER_MAKE_OPTS = \ > CROSS_COMPILE=$(TARGET_CROSS) \ > INSTALL_MOD_PATH=$(TARGET_DIR) > > +ifeq ($(BR2_arm),y) > +SUNXI_MALI_MAINLINE_DRIVER_MAKE_OPTS += ARCH=arm > +else ifeq ($(BR2_aarch64),y) > +SUNXI_MALI_MAINLINE_DRIVER_MAKE_OPTS += ARCH=arm64 > +endif Could you instead change the .mk to do this: SUNXI_MALI_MAINLINE_DRIVER_MAKE_OPTS = \ $(LINUX_MAKE_FLAGS) \ KDIR=$(LINUX_DIR) Indeed, LINUX_MAKE_FLAGS already include CROSS_COMPILE, INSTALL_MOD_PATH and ARCH. As separate commits, please add hash files for both sunxi-mali-mainline and sunxi-mainline-driver. Also, it would be good to have patches adding license information for both of those packages. For the mali-blobs, there's a PDF containing the EULA. For the kernel driver, I didn't see a license file, but I didn't look everywhere. If there is none, could you check with Maxime to make sure a license file gets added ? Thanks, Thomas
Hello, Il 22/08/2018 13:29, Thomas Petazzoni ha scritto: > Hello Giulio, > > On Wed, 22 Aug 2018 02:06:48 +0200, Giulio Benetti wrote: >> Blobs for arm64(aarch64) are available at Bootlin Github and >> sunxi-mali-mainline package has been updated to support them. >> >> Add support for building on arm64(aarch64): >> - Bump version to latest commit: >> For support H3 and H5 SoCs and r8p1 driver version. >> >> git shortlog --invert-grep --grep travis --no-merges 52ef1c5e133cc5fd791ca636239dc5e7b19c26d5.. >> Maxime Ripard (6): >> Add r8p1 release >> sunxi: Move the reset test to a function >> sunxi: Add H3 support >> sunxi: Add H5 support >> sunxi: Remove generic compatible >> sunxi: Set clock rate in the driver >> >> - Add ARCH=arm or ARCH=arm64 to SUNXI_MALI_MAINLINE_DRIVER_MAKE_OPTS >> to build for the right architecture according to BR2_arm or BR2_aarch64. >> >> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com> > > So actually your PATCH 1/2 without PATCH 2/2 is broken, because PATCH > 1/2 now allows to enable sunxi-mali-mainline on AArch64, but > sunxi-mali-mainline-driver does not support AArch64. It seems like you > have some issue understanding the concept of bisectability :-) Yes, fortunately you've helped me out to understand it well now. Thanks > > Anyway, now that PATCH 1/2 is applied and pushed, it's too late to back > out. > >> @@ -13,6 +13,12 @@ SUNXI_MALI_MAINLINE_DRIVER_MAKE_OPTS = \ >> CROSS_COMPILE=$(TARGET_CROSS) \ >> INSTALL_MOD_PATH=$(TARGET_DIR) >> >> +ifeq ($(BR2_arm),y) >> +SUNXI_MALI_MAINLINE_DRIVER_MAKE_OPTS += ARCH=arm >> +else ifeq ($(BR2_aarch64),y) >> +SUNXI_MALI_MAINLINE_DRIVER_MAKE_OPTS += ARCH=arm64 >> +endif > > Could you instead change the .mk to do this: > > SUNXI_MALI_MAINLINE_DRIVER_MAKE_OPTS = \ > $(LINUX_MAKE_FLAGS) \ > KDIR=$(LINUX_DIR) > > Indeed, LINUX_MAKE_FLAGS already include CROSS_COMPILE, > INSTALL_MOD_PATH and ARCH. Sure, this is very clean. > > As separate commits, please add hash files for both sunxi-mali-mainline > and sunxi-mainline-driver. Ok. Also, it would be good to have patches > adding license information for both of those packages. For the > mali-blobs, there's a PDF containing the EULA. Do you mean like odroid-mali package .mk? I mean reporting: SUNXI_MALI_MAINLINE_LICENSE = EULA SUNXI_MALI_MAINLINE_LICENSE_FILES = EULA for Mali 400MP _AW.pdf Right? For the kernel driver, I > didn't see a license file, but I didn't look everywhere. If there is > none, could you check with Maxime to make sure a license file gets > added ? I'm not an expert of licensing but as stated here: https://developer.arm.com/products/software/mali-drivers/utgard-kernel it is distributed under GPLv2 licence the base code, so the added code should be too. What do you all think? Kind regards Giulio Benetti
Hello, On Wed, 22 Aug 2018 20:37:13 +0200, Giulio Benetti wrote: > Also, it would be good to have patches > > adding license information for both of those packages. For the > > mali-blobs, there's a PDF containing the EULA. > > Do you mean like odroid-mali package .mk? > I mean reporting: > SUNXI_MALI_MAINLINE_LICENSE = EULA > SUNXI_MALI_MAINLINE_LICENSE_FILES = EULA for Mali 400MP _AW.pdf > Right? Yes. Make sure "make legal-info" is working after this. > For the kernel driver, I > > didn't see a license file, but I didn't look everywhere. If there is > > none, could you check with Maxime to make sure a license file gets > > added ? > > I'm not an expert of licensing but as stated here: > https://developer.arm.com/products/software/mali-drivers/utgard-kernel > it is distributed under GPLv2 licence the base code, > so the added code should be too. > What do you all think? Yes, of course, it should be under GPLv2, but it would be nice to have a license file in the source code. Best regards, Thomas
Hello, Il 22/08/2018 21:26, Thomas Petazzoni ha scritto: > Hello, > > On Wed, 22 Aug 2018 20:37:13 +0200, Giulio Benetti wrote: > >> Also, it would be good to have patches >>> adding license information for both of those packages. For the >>> mali-blobs, there's a PDF containing the EULA. >> >> Do you mean like odroid-mali package .mk? >> I mean reporting: >> SUNXI_MALI_MAINLINE_LICENSE = EULA >> SUNXI_MALI_MAINLINE_LICENSE_FILES = EULA for Mali 400MP _AW.pdf >> Right? > > Yes. Make sure "make legal-info" is working after this. I'm getting problems with whitespaces in pdf filename. I couldn't find in Buildroot Documentation if it's allowed or not having filenames containing spaces. I've tried using: 'EULA for Mali 400MP _AW.pdf' "EULA for Mali 400MP _AW.pdf" etc. but with no luck. If it's not possible to have whitespaces, should I workaround it in package changing filename before legal-info recipe? Like: EULA for Mali 400MP _AW.pdf becoming: EULA_for_Mali_400MP_AW.pdf ? Thanks Giulio
Hello, On Fri, 24 Aug 2018 21:10:56 +0200, Giulio Benetti wrote: > I'm getting problems with whitespaces in pdf filename. > I couldn't find in Buildroot Documentation if it's allowed or not having > filenames containing spaces. > I've tried using: > 'EULA for Mali 400MP _AW.pdf' > "EULA for Mali 400MP _AW.pdf" > etc. > but with no luck. > > If it's not possible to have whitespaces, should I workaround it in > package changing filename before legal-info recipe? > Like: > EULA for Mali 400MP _AW.pdf > becoming: > EULA_for_Mali_400MP_AW.pdf > ? Yes, I think it makes sense to have a POST_PATCH_HOOKS that renames the file so that it doesn't have spaces. Thomas
Giulio, All, On 2018-08-24 21:10 +0200, Giulio Benetti spake thusly: [--SNIP--] > I'm getting problems with whitespaces in pdf filename. > I couldn't find in Buildroot Documentation if it's allowed or not having > filenames containing spaces. > I've tried using: > 'EULA for Mali 400MP _AW.pdf' > "EULA for Mali 400MP _AW.pdf" > etc. > but with no luck. Could you please also try with a \-escape: SUNXI_MALI_MAINLINE_LICENSE_FILES = EULA\ for\ Mali\ 400MP\ _AW.pdf If that still fails, please try with two \\. I would prefer if we were to be able to keep the original filenames in license files, in case something else in the package would reference them. > If it's not possible to have whitespaces, should I workaround it in package > changing filename before legal-info recipe? > Like: > EULA for Mali 400MP _AW.pdf > becoming: > EULA_for_Mali_400MP_AW.pdf > ? As Thomas already replied, yes. But I'd prefer if that were used as a last resort. Thanks! Regards, Yann E. MORIN.
Hi, Il 24/08/2018 23:14, Yann E. MORIN ha scritto: > Giulio, All, > > On 2018-08-24 21:10 +0200, Giulio Benetti spake thusly: > [--SNIP--] >> I'm getting problems with whitespaces in pdf filename. >> I couldn't find in Buildroot Documentation if it's allowed or not having >> filenames containing spaces. >> I've tried using: >> 'EULA for Mali 400MP _AW.pdf' >> "EULA for Mali 400MP _AW.pdf" >> etc. >> but with no luck. > > Could you please also try with a \-escape: > > SUNXI_MALI_MAINLINE_LICENSE_FILES = EULA\ for\ Mali\ 400MP\ _AW.pdf > > If that still fails, please try with two \\. > > I would prefer if we were to be able to keep the original filenames in > license files, in case something else in the package would reference > them. > >> If it's not possible to have whitespaces, should I workaround it in package >> changing filename before legal-info recipe? >> Like: >> EULA for Mali 400MP _AW.pdf >> becoming: >> EULA_for_Mali_400MP_AW.pdf >> ? > > As Thomas already replied, yes. But I'd prefer if that were used as a > last resort. Just tried with both \ and \\ but nothing to do, it doesn't work. It always tries to find 'EULA' or 'EULA\' So do I proceed with renaming on POST_PATCH_HOOKS? Thanks Giulio
Giulio, All, On 2018-08-24 23:28 +0200, Giulio Benetti spake thusly: > Il 24/08/2018 23:14, Yann E. MORIN ha scritto: > >On 2018-08-24 21:10 +0200, Giulio Benetti spake thusly: > >[--SNIP--] > >>I'm getting problems with whitespaces in pdf filename. > >>I couldn't find in Buildroot Documentation if it's allowed or not having > >>filenames containing spaces. > >>I've tried using: > >>'EULA for Mali 400MP _AW.pdf' > >>"EULA for Mali 400MP _AW.pdf" > >>etc. > >>but with no luck. > >Could you please also try with a \-escape: > > SUNXI_MALI_MAINLINE_LICENSE_FILES = EULA\ for\ Mali\ 400MP\ _AW.pdf [--SNIP--] > Just tried with both \ and \\ but nothing to do, it doesn't work. > It always tries to find 'EULA' or 'EULA\' Damn... Yeah, I now had a look at the code, and it indeed can not work. I had hoped ot was a shell command that would do the splitting, but no; it is done by a make foreach loop, so we're doomed... > So do I proceed with renaming on POST_PATCH_HOOKS? Yes, please. Thanks! :-) Regards, Yann E. MORIN.
Hello Maxime, Il 22/08/2018 21:26, Thomas Petazzoni ha scritto: > Hello, > > On Wed, 22 Aug 2018 20:37:13 +0200, Giulio Benetti wrote: > >> Also, it would be good to have patches >>> adding license information for both of those packages. For the >>> mali-blobs, there's a PDF containing the EULA. >> >> Do you mean like odroid-mali package .mk? >> I mean reporting: >> SUNXI_MALI_MAINLINE_LICENSE = EULA >> SUNXI_MALI_MAINLINE_LICENSE_FILES = EULA for Mali 400MP _AW.pdf >> Right? > > Yes. Make sure "make legal-info" is working after this. > >> For the kernel driver, I >>> didn't see a license file, but I didn't look everywhere. If there is >>> none, could you check with Maxime to make sure a license file gets >>> added ? >> >> I'm not an expert of licensing but as stated here: >> https://developer.arm.com/products/software/mali-drivers/utgard-kernel >> it is distributed under GPLv2 licence the base code, >> so the added code should be too. >> What do you all think? > > Yes, of course, it should be under GPLv2, but it would be nice to have > a license file in the source code. Is there a way to add a license into sunxi-mali repository? Is it enough using a classic GPLv2 COPYING file taken from elsewhere and put it in sunxi-mali root folder? If yes I can contribute to your repo and open a PR. Thanks Giulio Benetti
diff --git a/package/sunxi-mali-mainline-driver/sunxi-mali-mainline-driver.mk b/package/sunxi-mali-mainline-driver/sunxi-mali-mainline-driver.mk index 24817b07c7..e7a5ed2079 100644 --- a/package/sunxi-mali-mainline-driver/sunxi-mali-mainline-driver.mk +++ b/package/sunxi-mali-mainline-driver/sunxi-mali-mainline-driver.mk @@ -4,7 +4,7 @@ # ################################################################################ -SUNXI_MALI_MAINLINE_DRIVER_VERSION = 52ef1c5e133cc5fd791ca636239dc5e7b19c26d5 +SUNXI_MALI_MAINLINE_DRIVER_VERSION = 8e6b7d25b13089d53dbfc1ebd9e8737b0dc809cb SUNXI_MALI_MAINLINE_DRIVER_SITE = $(call github,mripard,sunxi-mali,$(SUNXI_MALI_MAINLINE_DRIVER_VERSION)) SUNXI_MALI_MAINLINE_DRIVER_DEPENDENCIES = linux @@ -13,6 +13,12 @@ SUNXI_MALI_MAINLINE_DRIVER_MAKE_OPTS = \ CROSS_COMPILE=$(TARGET_CROSS) \ INSTALL_MOD_PATH=$(TARGET_DIR) +ifeq ($(BR2_arm),y) +SUNXI_MALI_MAINLINE_DRIVER_MAKE_OPTS += ARCH=arm +else ifeq ($(BR2_aarch64),y) +SUNXI_MALI_MAINLINE_DRIVER_MAKE_OPTS += ARCH=arm64 +endif + define SUNXI_MALI_MAINLINE_DRIVER_USE_APPLY_PATCHES ln -sf $(SUNXI_MALI_MAINLINE_REV)/series $(@D)/patches $(SED) 's|quilt push -a|$(TOPDIR)/support/scripts/apply-patches.sh . ../patches|' \
Blobs for arm64(aarch64) are available at Bootlin Github and sunxi-mali-mainline package has been updated to support them. Add support for building on arm64(aarch64): - Bump version to latest commit: For support H3 and H5 SoCs and r8p1 driver version. git shortlog --invert-grep --grep travis --no-merges 52ef1c5e133cc5fd791ca636239dc5e7b19c26d5.. Maxime Ripard (6): Add r8p1 release sunxi: Move the reset test to a function sunxi: Add H3 support sunxi: Add H5 support sunxi: Remove generic compatible sunxi: Set clock rate in the driver - Add ARCH=arm or ARCH=arm64 to SUNXI_MALI_MAINLINE_DRIVER_MAKE_OPTS to build for the right architecture according to BR2_arm or BR2_aarch64. Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com> --- .../sunxi-mali-mainline-driver.mk | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)