Skip to contentSkip to conversation

Improve powerpc-utils with librtas

PATCH Superseded Sam Bobroff 4 participants 14 messages May 8, 2015 – May 11, 2015

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Sam Bobroff

author
#0 lore raw

cover Improve powerpc-utils with librtas

Many useful functions are missing from powerpc-utils when built by buildroot, because they depend on librtas which is not included.

This set adds the librtas package, a small patch to fix a problem with it's make install target, and tweaks the powerpc-utils config to include it. This makes the following additional programs available from powerpc-utils:

* uesensor
* set_poweron_time
* drmgr
* lsslot
* activate_firmware
* rtas_ibm_get_vpd
* serv_config
* rtas_event_decode
* sys_ident

Sam Bobroff (2):
librtas: new package
powerpc-utils: remove --without-librtas

 package/Config.in                          |    1 +
 package/librtas/Config.in                  |    9 +++++++++
 package/librtas/librtas.mk                 |   27 +++++++++++++++++++++++++++
 package/librtas/missing-librtas.so.1.patch |   11 +++++++++++
 package/powerpc-utils/Config.in            |    1 +
 package/powerpc-utils/powerpc-utils.mk     |    3 +--
 6 files changed, 50 insertions(+), 2 deletions(-)
 create mode 100644 package/librtas/Config.in
 create mode 100644 package/librtas/librtas.mk
 create mode 100644 package/librtas/missing-librtas.so.1.patch
Signature · 1 line
1.7.10.4
author
#1 lore raw

PATCH 1/2 librtas: new package

Signed-off-by Sam Bobroff <[email protected]>
---
 package/Config.in                          |    1 +
 package/librtas/Config.in                  |    9 +++++++++
 package/librtas/librtas.mk                 |   27 +++++++++++++++++++++++++++
 package/librtas/missing-librtas.so.1.patch |   11 +++++++++++
 4 files changed, 48 insertions(+)
 create mode 100644 package/librtas/Config.in
 create mode 100644 package/librtas/librtas.mk
 create mode 100644 package/librtas/missing-librtas.so.1.patch
package/Config.in +1 -0
index af4d2b7..51de1c0 100644
@@ -832,6 +832,7 @@ menu "Hardware handling"
 	source "package/libphidget/Config.in"
 	source "package/libqmi/Config.in"
 	source "package/libraw1394/Config.in"
+	source "package/librtas/Config.in"
 	source "package/librtlsdr/Config.in"
 	source "package/libserial/Config.in"
 	source "package/libserialport/Config.in"
package/librtas/Config.in new file +9 -0
new file mode 100644
index 0000000..bf0a705
@@ -0,0 +1,9 @@
+config BR2_PACKAGE_LIBRTAS
+	bool "librtas"
+	depends on BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le
+	help
+	  The librtas package provides an interface for Run-Time Abstraction
+	  Services (RTAS) calls on PAPR platforms. The libraries allow users to
+	  examine and manipulate hardware, and parse RTAS events.
+
+	  http://librtas.sourceforge.net/
package/librtas/librtas.mk new file +27 -0
new file mode 100644
index 0000000..d13c876
@@ -0,0 +1,27 @@
+################################################################################
+#
+# librtas
+#
+################################################################################
+
+LIBRTAS_VERSION = 1.3.13
+LIBRTAS_SITE = http://downloads.sourceforge.net/project/librtas
+LIBRTAS_LICENSE = Common Public License Version 1.0
+LIBRTAS_LICENSE_FILES = COPYRIGHT
+LIBRTAS_INSTALL_STAGING = YES
+LIBRTAS_INSTALL_TARGET = YES
+
+define LIBRTAS_BUILD_CMDS
+	$(MAKE) -C $(@D) CC="$(TARGET_CC)" LD="$(TARGET_LD)"
+endef
+
+define LIBRTAS_INSTALL_STAGING_CMDS
+	$(MAKE) -C $(@D) DESTDIR=$(STAGING_DIR) install
+endef
+
+define LIBRTAS_INSTALL_TARGET_CMDS
+	$(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) install
+endef
+
+
+$(eval $(generic-package))
package/librtas/missing-librtas.so.1.patch new file +11 -0
new file mode 100644
index 0000000..de21d66
@@ -0,0 +1,11 @@
+diff -purN librtas-1.3.12.orig/librtas_src/Makefile librtas-1.3.12/librtas_src/Makefile
+--- librtas-1.3.12.orig/librtas_src/Makefile	2015-04-09 10:45:19.772381633 +1000
++++ librtas-1.3.12/librtas_src/Makefile	2015-04-09 10:45:57.968604762 +1000
+@@ -33,6 +33,7 @@ install:
+ 	@$(call install_lib,$(LIBRTAS),$(DESTDIR))
+ 	@$(call install_inc,$(HEADERS),$(DESTDIR))
+ 	@ln -sf $(LIBRTAS) $(DESTDIR)/$(LIB_DIR)$(call is_lib64,$(LIBRTAS))/$(LIBRTAS_SONAME)
++	@ln -sf $(LIBRTAS) $(DESTDIR)/$(LIB_DIR)$(call is_lib64,$(LIBRTAS))/$(LIBRTAS_SONAME).1
+ 
+ uninstall:
+ 	@$(call uninstall_lib,$(LIBRTAS),$(DESTDIR))
Signature · 1 line
1.7.10.4
#2 lore raw

PATCH 1/2 librtas: new package

Hi Sam,

Quoted 8 lines · Sam Bobroff
package/librtas/librtas.mk new file +2 -0
new file mode 100644
index 0000000..d13c876
@@ -0,0 +1,27 @@
+################################################################################
+#

package/librtas/librtas.mk line 12

Quoted 10 lines
package/librtas/librtas.mk new file +10 -0
+# librtas
+#
+################################################################################
+
+LIBRTAS_VERSION = 1.3.13
+LIBRTAS_SITE = http://downloads.sourceforge.net/project/librtas
+LIBRTAS_LICENSE = Common Public License Version 1.0
+LIBRTAS_LICENSE_FILES = COPYRIGHT
+LIBRTAS_INSTALL_STAGING = YES
+LIBRTAS_INSTALL_TARGET = YES

Not needed, this is the default.

package/librtas/missing-librtas.so.1.patch

Quoted 21 lines
package/librtas/librtas.mk new file +15 -0
+
+define LIBRTAS_BUILD_CMDS
+	$(MAKE) -C $(@D) CC="$(TARGET_CC)" LD="$(TARGET_LD)"
+endef
+
+define LIBRTAS_INSTALL_STAGING_CMDS
+	$(MAKE) -C $(@D) DESTDIR=$(STAGING_DIR) install
+endef
+
+define LIBRTAS_INSTALL_TARGET_CMDS
+	$(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) install
+endef
+
+
+$(eval $(generic-package))
package/librtas/missing-librtas.so.1.patch new file +0 -0
new file mode 100644
index 0000000..de21d66
@@ -0,0 +1,11 @@

Please add your Sign-off here.

Quoted 11 lines
package/librtas/missing-librtas.so.1.patch new file +11 -0
+diff -purN librtas-1.3.12.orig/librtas_src/Makefile librtas-1.3.12/librtas_src/Makefile
+--- librtas-1.3.12.orig/librtas_src/Makefile	2015-04-09 10:45:19.772381633 +1000
++++ librtas-1.3.12/librtas_src/Makefile	2015-04-09 10:45:57.968604762 +1000
+@@ -33,6 +33,7 @@ install:
+ 	@$(call install_lib,$(LIBRTAS),$(DESTDIR))
+ 	@$(call install_inc,$(HEADERS),$(DESTDIR))
+ 	@ln -sf $(LIBRTAS) $(DESTDIR)/$(LIB_DIR)$(call is_lib64,$(LIBRTAS))/$(LIBRTAS_SONAME)
++	@ln -sf $(LIBRTAS) $(DESTDIR)/$(LIB_DIR)$(call is_lib64,$(LIBRTAS))/$(LIBRTAS_SONAME).1
+ 
+ uninstall:
+ 	@$(call uninstall_lib,$(LIBRTAS),$(DESTDIR))
Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
author
#3 lore raw

PATCH 1/2 librtas: new package

Quoted 15 lines · Baruch Siach

Hi Sam,

Quoted 12 lines · Sam Bobroff
package/librtas/librtas.mk new file +6 -0
new file mode 100644
index 0000000..d13c876
@@ -0,0 +1,27 @@
+################################################################################
+#
+# librtas
+#
+################################################################################
+
Quoted 8 lines

package/librtas/librtas.mk line 12

Quoted 6 lines
package/librtas/librtas.mk new file +6 -0
+LIBRTAS_VERSION = 1.3.13
+LIBRTAS_SITE = http://downloads.sourceforge.net/project/librtas
+LIBRTAS_LICENSE = Common Public License Version 1.0
+LIBRTAS_LICENSE_FILES = COPYRIGHT
+LIBRTAS_INSTALL_STAGING = YES
+LIBRTAS_INSTALL_TARGET = YES

Not needed, this is the default.

OK.

Quoted 16 lines
Quoted 16 lines
package/librtas/librtas.mk new file +15 -0
+
+define LIBRTAS_BUILD_CMDS
+	$(MAKE) -C $(@D) CC="$(TARGET_CC)" LD="$(TARGET_LD)"
+endef
+
+define LIBRTAS_INSTALL_STAGING_CMDS
+	$(MAKE) -C $(@D) DESTDIR=$(STAGING_DIR) install
+endef
+
+define LIBRTAS_INSTALL_TARGET_CMDS
+	$(MAKE) -C $(@D) DESTDIR=$(TARGET_DIR) install
+endef
+
+
+$(eval $(generic-package))
Quoted 8 lines

package/librtas/missing-librtas.so.1.patch

Quoted 6 lines
package/librtas/missing-librtas.so.1.patch new file +0 -0
new file mode 100644
index 0000000..de21d66
@@ -0,0 +1,11 @@

Please add your Sign-off here.

OK. Will do.

Quoted 13 lines

package/librtas/missing-librtas.so.1.patch line 11

Quoted 12 lines
package/librtas/missing-librtas.so.1.patch new file +11 -0
+diff -purN librtas-1.3.12.orig/librtas_src/Makefile librtas-1.3.12/librtas_src/Makefile
+--- librtas-1.3.12.orig/librtas_src/Makefile	2015-04-09 10:45:19.772381633 +1000
++++ librtas-1.3.12/librtas_src/Makefile	2015-04-09 10:45:57.968604762 +1000
+@@ -33,6 +33,7 @@ install:
+ 	@$(call install_lib,$(LIBRTAS),$(DESTDIR))
+ 	@$(call install_inc,$(HEADERS),$(DESTDIR))
+ 	@ln -sf $(LIBRTAS) $(DESTDIR)/$(LIB_DIR)$(call is_lib64,$(LIBRTAS))/$(LIBRTAS_SONAME)
++	@ln -sf $(LIBRTAS) $(DESTDIR)/$(LIB_DIR)$(call is_lib64,$(LIBRTAS))/$(LIBRTAS_SONAME).1
+ 
+ uninstall:
+ 	@$(call uninstall_lib,$(LIBRTAS),$(DESTDIR))

Thanks for the comments.

Sam.

#4 lore raw

PATCH 1/2 librtas: new package

In reply to Baruch Siach
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Baruch" == Baruch Siach <[email protected]> writes:

Hi,

Quoted 8 lines

package/librtas/missing-librtas.so.1.patch

Quoted 6 lines
package/librtas/missing-librtas.so.1.patch new file +0 -0
new file mode 100644
index 0000000..de21d66
@@ -0,0 +1,11 @@

Please add your Sign-off here.

And a description. Why is it needed, has the patch been sent upstream? What is the status?

Quoted 11 lines
Quoted 11 lines
package/librtas/missing-librtas.so.1.patch new file +11 -0
+diff -purN librtas-1.3.12.orig/librtas_src/Makefile librtas-1.3.12/librtas_src/Makefile
+--- librtas-1.3.12.orig/librtas_src/Makefile	2015-04-09 10:45:19.772381633 +1000
++++ librtas-1.3.12/librtas_src/Makefile	2015-04-09 10:45:57.968604762 +1000
+@@ -33,6 +33,7 @@ install:
+ 	@$(call install_lib,$(LIBRTAS),$(DESTDIR))
+ 	@$(call install_inc,$(HEADERS),$(DESTDIR))
+ 	@ln -sf $(LIBRTAS) $(DESTDIR)/$(LIB_DIR)$(call is_lib64,$(LIBRTAS))/$(LIBRTAS_SONAME)
++	@ln -sf $(LIBRTAS) $(DESTDIR)/$(LIB_DIR)$(call is_lib64,$(LIBRTAS))/$(LIBRTAS_SONAME).1
+ 
+ uninstall:
+ 	@$(call uninstall_lib,$(LIBRTAS),$(DESTDIR))
Signature · 1 line
Bye, Peter Korsgaard
author
#5 lore raw

PATCH 1/2 librtas: new package

Quoted 13 lines · Peter Korsgaard
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Baruch" == Baruch Siach <[email protected]> writes:

Hi,

Quoted 8 lines

package/librtas/missing-librtas.so.1.patch

Quoted 6 lines
package/librtas/missing-librtas.so.1.patch new file +0 -0
new file mode 100644
index 0000000..de21d66
@@ -0,0 +1,11 @@

Please add your Sign-off here.

Quoted 2 lines

And a description. Why is it needed, has the patch been sent upstream? What is the status?

OK, I'll add all that to the patch.

Thanks,

Sam.

Quoted 13 lines
Quoted 11 lines
Quoted 11 lines
package/librtas/missing-librtas.so.1.patch new file +11 -0
+diff -purN librtas-1.3.12.orig/librtas_src/Makefile librtas-1.3.12/librtas_src/Makefile
+--- librtas-1.3.12.orig/librtas_src/Makefile	2015-04-09 10:45:19.772381633 +1000
++++ librtas-1.3.12/librtas_src/Makefile	2015-04-09 10:45:57.968604762 +1000
+@@ -33,6 +33,7 @@ install:
+ 	@$(call install_lib,$(LIBRTAS),$(DESTDIR))
+ 	@$(call install_inc,$(HEADERS),$(DESTDIR))
+ 	@ln -sf $(LIBRTAS) $(DESTDIR)/$(LIB_DIR)$(call is_lib64,$(LIBRTAS))/$(LIBRTAS_SONAME)
++	@ln -sf $(LIBRTAS) $(DESTDIR)/$(LIB_DIR)$(call is_lib64,$(LIBRTAS))/$(LIBRTAS_SONAME).1
+ 
+ uninstall:
+ 	@$(call uninstall_lib,$(LIBRTAS),$(DESTDIR))
#6 lore raw

PATCH 1/2 librtas: new package

In reply to Peter Korsgaard

Dear Peter Korsgaard,

Quoted 11 lines · Peter Korsgaard
Quoted 8 lines

package/librtas/missing-librtas.so.1.patch

Quoted 6 lines
package/librtas/missing-librtas.so.1.patch new file +0 -0
new file mode 100644
index 0000000..de21d66
@@ -0,0 +1,11 @@

Please add your Sign-off here.

And a description. Why is it needed, has the patch been sent upstream? What is the status?

And patch should be named 0001-<foo>.patch.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#7 lore raw

PATCH 2/2 powerpc-utils: remove --without-librtas

In reply to Sam Bobroff

Now that librtas is available, remove --without-librtas and make the rtas-dependant functions available.

Signed-off-by Sam Bobroff <[email protected]>
---
 package/powerpc-utils/Config.in        |    1 +
 package/powerpc-utils/powerpc-utils.mk |    3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)
package/powerpc-utils/Config.in +1 -0
index eded41e..a754899 100644
@@ -2,6 +2,7 @@ config BR2_PACKAGE_POWERPC_UTILS
 	bool "powerpc-utils"
 	depends on BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le
 	select BR2_PACKAGE_ZLIB
+	select BR2_PACKAGE_LIBRTAS
 	help
 	  System utilities for PowerPC machines.
package/powerpc-utils/powerpc-utils.mk +1 -2
index 3b30458..3fb9e24 100644
@@ -6,11 +6,10 @@
 
 POWERPC_UTILS_VERSION = 1.2.24
 POWERPC_UTILS_SITE = http://downloads.sourceforge.net/project/powerpc-utils/powerpc-utils
-POWERPC_UTILS_DEPENDENCIES = zlib
+POWERPC_UTILS_DEPENDENCIES = zlib librtas
 POWERPC_UTILS_LICENSE = Common Public License Version 1.0
 POWERPC_UTILS_LICENSE_FILES = COPYRIGHT
 
-POWERPC_UTILS_CONF_OPTS = --without-librtas
 POWERPC_UTILS_CONF_ENV = \
 	ax_cv_check_cflags___fstack_protector_all=$(if $(BR2_TOOLCHAIN_HAS_SSP),yes,no)
Signature · 1 line
1.7.10.4
#8 lore raw

PATCH 2/2 powerpc-utils: remove --without-librtas

Hi Sam,

Quoted 26 lines · Sam Bobroff

Now that librtas is available, remove --without-librtas and make the rtas-dependant functions available.

Signed-off-by Sam Bobroff <[email protected]>
---
 package/powerpc-utils/Config.in        |    1 +
 package/powerpc-utils/powerpc-utils.mk |    3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)
package/powerpc-utils/Config.in +1 -0
index eded41e..a754899 100644
@@ -2,6 +2,7 @@ config BR2_PACKAGE_POWERPC_UTILS
 	bool "powerpc-utils"
 	depends on BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le
 	select BR2_PACKAGE_ZLIB
+	select BR2_PACKAGE_LIBRTAS
 	help
 	  System utilities for PowerPC machines.
package/powerpc-utils/powerpc-utils.mk +0 -0
index 3b30458..3fb9e24 100644

package/powerpc-utils/powerpc-utils.mk line 9

Quoted 5 lines
package/powerpc-utils/powerpc-utils.mk +1 -1
@@ -6,11 +6,10 @@
 
 POWERPC_UTILS_VERSION = 1.2.24
 POWERPC_UTILS_SITE = http://downloads.sourceforge.net/project/powerpc-utils/powerpc-utils
-POWERPC_UTILS_DEPENDENCIES = zlib
+POWERPC_UTILS_DEPENDENCIES = zlib librtas

Why not make librtas an optional dependency? Something like:

ifeq ($(BR2_PACKAGE_LIBRTAS),y)
POWERPC_UTILS_DEPENDENCIES += librtas
POWERPC_UTILS_CONF_OPTS = --with-librtas
else
POWERPC_UTILS_CONF_OPTS = --without-librtas
endif

package/powerpc-utils/powerpc-utils.mk line 14

Quoted 6 lines
package/powerpc-utils/powerpc-utils.mk +0 -1
 POWERPC_UTILS_LICENSE = Common Public License Version 1.0
 POWERPC_UTILS_LICENSE_FILES = COPYRIGHT
 
-POWERPC_UTILS_CONF_OPTS = --without-librtas
 POWERPC_UTILS_CONF_ENV = \
 	ax_cv_check_cflags___fstack_protector_all=$(if $(BR2_TOOLCHAIN_HAS_SSP),yes,no)

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
author
#9 lore raw

PATCH 2/2 powerpc-utils: remove --without-librtas

Quoted 44 lines · Baruch Siach

Hi Sam,

Quoted 26 lines · Sam Bobroff

Now that librtas is available, remove --without-librtas and make the rtas-dependant functions available.

Signed-off-by Sam Bobroff <[email protected]>
---
 package/powerpc-utils/Config.in        |    1 +
 package/powerpc-utils/powerpc-utils.mk |    3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)
package/powerpc-utils/Config.in +1 -0
index eded41e..a754899 100644
@@ -2,6 +2,7 @@ config BR2_PACKAGE_POWERPC_UTILS
 	bool "powerpc-utils"
 	depends on BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le
 	select BR2_PACKAGE_ZLIB
+	select BR2_PACKAGE_LIBRTAS
 	help
 	  System utilities for PowerPC machines.
package/powerpc-utils/powerpc-utils.mk +0 -0
index 3b30458..3fb9e24 100644

package/powerpc-utils/powerpc-utils.mk line 9

Quoted 5 lines
package/powerpc-utils/powerpc-utils.mk +1 -1
@@ -6,11 +6,10 @@
 
 POWERPC_UTILS_VERSION = 1.2.24
 POWERPC_UTILS_SITE = http://downloads.sourceforge.net/project/powerpc-utils/powerpc-utils
-POWERPC_UTILS_DEPENDENCIES = zlib
+POWERPC_UTILS_DEPENDENCIES = zlib librtas

Why not make librtas an optional dependency? Something like:

ifeq ($(BR2_PACKAGE_LIBRTAS),y)
POWERPC_UTILS_DEPENDENCIES += librtas
POWERPC_UTILS_CONF_OPTS = --with-librtas
else
POWERPC_UTILS_CONF_OPTS = --without-librtas
endif

Quoted 9 lines

package/powerpc-utils/powerpc-utils.mk line 14

Quoted 6 lines
package/powerpc-utils/powerpc-utils.mk +0 -1
 POWERPC_UTILS_LICENSE = Common Public License Version 1.0
 POWERPC_UTILS_LICENSE_FILES = COPYRIGHT
 
-POWERPC_UTILS_CONF_OPTS = --without-librtas
 POWERPC_UTILS_CONF_ENV = \
 	ax_cv_check_cflags___fstack_protector_all=$(if $(BR2_TOOLCHAIN_HAS_SSP),yes,no)

baruch

Hi Baruch,

I thought about making it optional, but there seemed to be two reasons to always include it:

* librtas is a small library and it provides a lot of functionality to powerpc-utils. Why leave it out?

* Distros (AFAIK) always include librtas when providing powerpc-utils, so it would surprise users to provide a powerpc-utils without it by default. So if I did make it optional, I think it should be using some method that caused librtas to be included by default or at least provided some indication in the menu that they may want to go and enable librtas. Then I thought this was a bit complicated for a use case that I couldn't find a reason for.

That said, if you have a reason to make it optional I'll be happy to make it so. Maybe using a sub-option to add the dependency, and that could default to yes?

e.g.

[*] powerpc-utils
[*] RTAS functionality

Cheers,
Sam.

#10 lore raw

PATCH 2/2 powerpc-utils: remove --without-librtas

Hi Sam,

Quoted 20 lines · Sam Bobroff
Quoted 6 lines · Baruch Siach

package/powerpc-utils/powerpc-utils.mk line 9

Quoted 2 lines · Sam Bobroff
package/powerpc-utils/powerpc-utils.mk +1 -1
-POWERPC_UTILS_DEPENDENCIES = zlib
+POWERPC_UTILS_DEPENDENCIES = zlib librtas

Why not make librtas an optional dependency? Something like:

Quoted 6 lines

ifeq ($(BR2_PACKAGE_LIBRTAS),y)
POWERPC_UTILS_DEPENDENCIES += librtas
POWERPC_UTILS_CONF_OPTS = --with-librtas
else
POWERPC_UTILS_CONF_OPTS = --without-librtas
endif

I thought about making it optional, but there seemed to be two reasons to always include it:

* librtas is a small library and it provides a lot of functionality to powerpc-utils. Why leave it out?

Quoted 7 lines

* Distros (AFAIK) always include librtas when providing powerpc-utils, so it would surprise users to provide a powerpc-utils without it by default. So if I did make it optional, I think it should be using some method that caused librtas to be included by default or at least provided some indication in the menu that they may want to go and enable librtas. Then I thought this was a bit complicated for a use case that I couldn't find a reason for.

These are valid reasons. What is the target installed size of librtas compared to powerpc-utils?

Quoted 8 lines

That said, if you have a reason to make it optional I'll be happy to make it so. Maybe using a sub-option to add the dependency, and that could default to yes?

e.g.

[*] powerpc-utils
[*] RTAS functionality

Sounds reasonable to me. Let's see what others think.

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
author
#11 lore raw

PATCH 2/2 powerpc-utils: remove --without-librtas

Quoted 31 lines · Baruch Siach

Hi Sam,

Quoted 20 lines · Sam Bobroff
Quoted 6 lines · Baruch Siach

package/powerpc-utils/powerpc-utils.mk line 9

Quoted 2 lines · Sam Bobroff
package/powerpc-utils/powerpc-utils.mk +1 -1
-POWERPC_UTILS_DEPENDENCIES = zlib
+POWERPC_UTILS_DEPENDENCIES = zlib librtas

Why not make librtas an optional dependency? Something like:

Quoted 6 lines

ifeq ($(BR2_PACKAGE_LIBRTAS),y)
POWERPC_UTILS_DEPENDENCIES += librtas
POWERPC_UTILS_CONF_OPTS = --with-librtas
else
POWERPC_UTILS_CONF_OPTS = --without-librtas
endif

I thought about making it optional, but there seemed to be two reasons to always include it:

* librtas is a small library and it provides a lot of functionality to powerpc-utils. Why leave it out?

Quoted 7 lines

* Distros (AFAIK) always include librtas when providing powerpc-utils, so it would surprise users to provide a powerpc-utils without it by default. So if I did make it optional, I think it should be using some method that caused librtas to be included by default or at least provided some indication in the menu that they may want to go and enable librtas. Then I thought this was a bit complicated for a use case that I couldn't find a reason for.

Quoted 2 lines

These are valid reasons. What is the target installed size of librtas compared to powerpc-utils?

This might be a bit rough but it looks like:

* librtas: 360Kb
* powerpc-utils with librtas: 1,660Kb
.. for a total of 2020Kb

* powerpc-utils without librtas: 484Kb

So it looks like the increase is about 1.5Mb (or 3x, depending on how you look at it).

Quoted 17 lines
Quoted 8 lines

That said, if you have a reason to make it optional I'll be happy to make it so. Maybe using a sub-option to add the dependency, and that could default to yes?

e.g.

[*] powerpc-utils
[*] RTAS functionality

Sounds reasonable to me. Let's see what others think.

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
#12 lore raw

PATCH 2/2 powerpc-utils: remove --without-librtas

Hi Sam,

Quoted 24 lines · Sam Bobroff
Quoted 18 lines · Baruch Siach
Quoted 9 lines · Sam Bobroff
Quoted 1 line · Baruch Siach

Why not make librtas an optional dependency?

I thought about making it optional, but there seemed to be two reasons to always include it:

* librtas is a small library and it provides a lot of functionality to powerpc-utils. Why leave it out?

Quoted 7 lines

* Distros (AFAIK) always include librtas when providing powerpc-utils, so it would surprise users to provide a powerpc-utils without it by default. So if I did make it optional, I think it should be using some method that caused librtas to be included by default or at least provided some indication in the menu that they may want to go and enable librtas. Then I thought this was a bit complicated for a use case that I couldn't find a reason for.

Quoted 2 lines

These are valid reasons. What is the target installed size of librtas compared to powerpc-utils?

This might be a bit rough but it looks like:

Quoted 8 lines

* librtas: 360Kb
* powerpc-utils with librtas: 1,660Kb
.. for a total of 2020Kb

* powerpc-utils without librtas: 484Kb

So it looks like the increase is about 1.5Mb (or 3x, depending on how you look at it).

This is a significant increase. In this case I think it is best to give the user an explicit option like you suggested. Mentioning these numbers in the option help text should also be useful, I guess.

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
#13 lore raw

PATCH 2/2 powerpc-utils: remove --without-librtas

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Baruch" == Baruch Siach <[email protected]> writes:

Hi,

Quoted 9 lines
Quoted 9 lines
Quoted 2 lines

These are valid reasons. What is the target installed size of librtas compared to powerpc-utils?

This might be a bit rough but it looks like:

* librtas: 360Kb
* powerpc-utils with librtas: 1,660Kb
.. for a total of 2020Kb

Quoted 8 lines
Quoted 4 lines

* powerpc-utils without librtas: 484Kb

So it looks like the increase is about 1.5Mb (or 3x, depending on how you look at it).

This is a significant increase. In this case I think it is best to give the user an explicit option like you suggested. Mentioning these numbers in the option help text should also be useful, I guess.

Yes, I agree for such a big increase in size.

Signature · 1 line
Bye, Peter Korsgaard