[Buildroot] [PATCH v4 6/7] package/amd-catalyst: Add support AMD CCCLE

[Yann E. MORIN <yann.morin.1998@free.fr>]

Romain, All,

On 2016-08-05 16:53 +0200, Romain Perier spake thusly:
> The AMD Catalyst Linux driver includes a graphical controler center for AMD
> graphics accelerators, called Catalyst Controler Center. This commits
> adds support to install this tool by adding a separated prompt.
> 
> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
> ---
>  package/amd-catalyst/Config.in       | 13 +++++++++++++
>  package/amd-catalyst/amd-catalyst.mk | 10 ++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/package/amd-catalyst/Config.in b/package/amd-catalyst/Config.in
> index 0099e91..99676d3 100644
> --- a/package/amd-catalyst/Config.in
> +++ b/package/amd-catalyst/Config.in
> @@ -43,6 +43,19 @@ config BR2_PACKAGE_AMD_CATALYST_CMDLINE_TOOLS
>  	help
>  	  Build the amd command line tools
>  
> +comment "Catalyst Control Center needs Qt4 with X11 and PNG support"
> +	depends on !BR2_PACKAGE_QT || !BR2_PACKAGE_QT_X11 || BR2_PACKAGE_QT_NOPNG
> +
> +config BR2_PACKAGE_AMD_CATALYST_CCCLE
> +	bool "Catalyst Control Center"
> +	depends on BR2_USE_MMU && BR2_PACKAGE_QT && BR2_PACKAGE_QT_X11

Please, do not mix architecture-related dependencies (MMU) with package
dependencies. Besides, you forgot to depend on !QT_NO_PNG:

    depends on BR2_USE_MMU
    depends on BR2_PACKAGE_QT
    depends on BR2_PACKAGE_QT_X11
    depends on !BR2_PACKAGE_QT_NOPNG

However, for the MMU stuff, I wonder if it is really needed, as the
whole stuff depends on glibc, which is MMU-only anyway.

> +	select BR2_PACKAGE_PROCPS_NG # runtime
> +	select BR2_PACKAGE_BUSYBOX_SHOW_OTHERS

I prefer when dependencies are logically ordered. Here, we need
BB_SHOW_OTHERS becasue we want to selec procps-ng, so:

    select BR2_PACKAGE_BUSYBOX_SHOW_OTHERS # procps-ng
    select BR2_PACKAGE_PROCPS_NG # runtime

(Technically, there is *no* difference; it's just easier to follow for
us mere humans. ;-) )

(Oh, did you add the MMU dependency because of procps-ng? Not needed,
since glibc is MMU-only.)

Except for those minor nits, that looks good! :-)

Regards,
Yann E. MORIN.

> +	select BR2_PACKAGE_QT_GUI_MODULE
> +	select BR2_PACKAGE_QT_ACCESSIBILITY
> +	help
> +	  Installs the Catalyst Control Center, a Qt graphical tool to
> +	  control AMD graphics accelerators.
>  endif
>  
>  config BR2_PACKAGE_AMD_CATALYST_MODULE
> diff --git a/package/amd-catalyst/amd-catalyst.mk b/package/amd-catalyst/amd-catalyst.mk
> index 5e1b90b..ee3c2d6 100644
> --- a/package/amd-catalyst/amd-catalyst.mk
> +++ b/package/amd-catalyst/amd-catalyst.mk
> @@ -122,6 +122,15 @@ define  AMD_CATALYST_INSTALL_CMDLINE_TOOLS
>  endef
>  endif
>  
> +ifeq ($(BR2_PACKAGE_AMD_CATALYST_CCCLE), y)
> +define AMD_CATALYST_INSTALL_CCCLE
> +	$(INSTALL) -m 0755 $(AMD_CATALYST_ARCH_DIR)/usr/X11R6/bin/amdcccle \
> +		$(TARGET_DIR)/usr/bin/amdcccle
> +	$(INSTALL) -m 0755 $(AMD_CATALYST_ARCH_DIR)/usr/sbin/amdnotifyui \
> +		$(TARGET_DIR)/usr/sbin/amdnotifyui
> +endef
> +endif
> +
>  define AMD_CATALYST_INSTALL_STAGING_CMDS
>  	$(call AMD_CATALYST_INSTALL_STAGING_XORG)
>  endef
> @@ -129,6 +138,7 @@ endef
>  define AMD_CATALYST_INSTALL_TARGET_CMDS
>  	$(call AMD_CATALYST_INSTALL_XORG)
>  	$(call AMD_CATALYST_INSTALL_CMDLINE_TOOLS)
> +	$(call AMD_CATALYST_INSTALL_CCCLE)
>  endef
>  
>  $(eval $(generic-package))
> -- 
> 2.8.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'