[Buildroot] [PATCH 2/2] odroid-gl: reorganize structure
Arnout Vandecappelle
arnout at mind.be
Tue Apr 18 20:14:19 UTC 2017
Hi Dagg,
On 17-04-17 08:56, Dagg Stompler wrote:
> - remove support for 32 bit libs as it is impossible to build a 32 bit odroid image.
I think this part should be a separate patch.
> - reorganize structure to support other boards from odroid.
This would better be combined in a series that actually adds such a different
board, so we can see where this is going.
>
> Signed-off-by: Dagg Stompler <daggs at gmx.com>
> ---
> package/odroid-gl/Config.in | 13 +++++++++++--
> package/odroid-gl/odroid-gl.hash | 2 +-
> package/odroid-gl/odroid-gl.mk | 20 ++++++++++++--------
> 3 files changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/package/odroid-gl/Config.in b/package/odroid-gl/Config.in
> index 9c605f6de..72727f41e 100644
> --- a/package/odroid-gl/Config.in
> +++ b/package/odroid-gl/Config.in
> @@ -6,9 +6,10 @@ config BR2_PACKAGE_ODROID_GL
> depends on BR2_TOOLCHAIN_USES_GLIBC
> depends on BR2_aarch64 || BR2_ARM_EABIHF
> help
> - Install the ARM Mali drivers for odroidc2 based systems.
> + Install the ARM Mali drivers for odorid boards.
^^^^^^ odroid?
>
> - https://github.com/mdrjr/c2_mali
> + support list:
> + c2: https://github.com/mdrjr/c2_mali
I guess the idea here is to add more boards?
>
> if BR2_PACKAGE_ODROID_GL
>
> @@ -18,6 +19,14 @@ config BR2_PACKAGE_PROVIDES_LIBEGL
> config BR2_PACKAGE_PROVIDES_LIBGLES
> default "odroid-gl"
>
> +choice
> + prompt "odroid board selection"
> +
> + config BR2_PACKAGE_ODROID_GL_C2
> + bool "odroid c2 gl"
With just one option, this choice really doesn't make any sense. It could make
sense as an in-between patch in a series that adds 4 or more additional boards.
But if it is just one or two more, you can keep it in a single patch adding
those two options.
Also, in this patch you're introducing this new symbol, but you're not actually
using it anywhere. That is really completely pointless: even if you gradually
want to add several boards, the first patch should contain both the new config
symbol and how it is used in the .mk file.
[sidetrack: it is already crazy that Mali needs a different binary blob for
different chips, but it also needs a different binary for a different *board*?
Really? What are these guys smoking?]
> +
> +endchoice
> +
> endif
>
> config BR2_PACKAGE_ODROID_GL_X11
> diff --git a/package/odroid-gl/odroid-gl.hash b/package/odroid-gl/odroid-gl.hash
> index b71ebab13..456944651 100644
> --- a/package/odroid-gl/odroid-gl.hash
> +++ b/package/odroid-gl/odroid-gl.hash
> @@ -1,2 +1,2 @@
> # Locally computed hash
> -sha256 6c85e96f3372c24c6497f2a1cbea867a8dbf3a7b3edd736802c762176006aec6 odroid-mali-4f8a541693fee5fdcaa162a7fd8922861a4ba0a9.tar.gz
> +sha256 6c85e96f3372c24c6497f2a1cbea867a8dbf3a7b3edd736802c762176006aec6 odroid-gl-c2-4f8a541693fee5fdcaa162a7fd8922861a4ba0a9.tar.gz
> diff --git a/package/odroid-gl/odroid-gl.mk b/package/odroid-gl/odroid-gl.mk
> index 0591497ca..40be77f01 100644
> --- a/package/odroid-gl/odroid-gl.mk
> +++ b/package/odroid-gl/odroid-gl.mk
> @@ -4,8 +4,20 @@
> #
> ################################################################################
>
> +ifeq ($(BR2_PACKAGE_ODROID_GL_C2),y)
> ODROID_GL_VERSION = 4f8a541693fee5fdcaa162a7fd8922861a4ba0a9
> ODROID_GL_SITE = $(call github,mdrjr,c2_mali,$(ODROID_GL_VERSION))
> +ODROID_GL_LIBS_SUBDIR = fbdev/mali_libs/
> +ODROID_GL_PREFIX = c2
> +ifeq ($(BR2_PACKAGE_ODROID_GL_X11),y)
> +ODROID_GL_HEADERS_SUBDIR = x11/mali_headers/
> +ODROID_GL_LIBS_SUBDIR = x11/mali_libs/
What is the reason to move this part?
> +else
> +ODROID_GL_HEADERS_SUBDIR = fbdev/mali_headers/
> +endif
> +endif
> +
> +ODROID_GL_SOURCE := odroid-gl-$(ODROID_GL_PREFIX)-$(ODROID_GL_VERSION).tar.gz
Use = instead of :=, except if there is a *really* good reason not to. And such
a reason would typically warrant a comment explaining it.
But I wonder where this is going... With the github helper, the _SOURCE can
actually be set to *anything* (as long as it ends with .tar.gz). github will
only look at the directory path leading up to it. So this ODROID_GL_PREFIX can't
possibly have any function, as far as I can see...
> ODROID_GL_LICENSE = Hardkernel EULA
> ODROID_GL_LICENSE_FILES = README.md
>
> @@ -13,8 +25,6 @@ ODROID_GL_INSTALL_STAGING = YES
> ODROID_GL_PROVIDES = libegl libgles
>
> ifeq ($(BR2_PACKAGE_ODROID_GL_X11),y)
> -ODROID_GL_HEADERS_SUBDIR = x11/mali_headers/
> -ODROID_GL_LIBS_SUBDIR = x11/mali_libs/
> # The X11 blobs are linked against those libraries, and the headers
> # include headers from those libraries
> ODROID_GL_DEPENDENCIES += \
> @@ -25,12 +35,6 @@ define ODROID_GL_FIX_EGL_PC
> $(SED) "s/Cflags: /Cflags: -DMESA_EGL_NO_X11_HEADERS /" \
> $(STAGING_DIR)/usr/lib/pkgconfig/egl.pc
> endef
> -ODROID_GL_HEADERS_SUBDIR = fbdev/mali_headers/
> -ifeq ($(BR2_aarch64),y)
> -ODROID_GL_LIBS_SUBDIR = fbdev/mali_libs/
> -else
> -ODROID_GL_LIBS_SUBDIR = fbdev/32bit_libs/
If it can never work on 32 bits, why do they even have this directory?
Regards,
Arnout
> -endif
> endif
>
> define ODROID_GL_INSTALL_LIBS
>
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
More information about the buildroot
mailing list