* [Buildroot] [PATCH] package/pflash: new package
@ 2016-07-25 6:01 Joel Stanley
2016-07-25 6:14 ` Bernd Kuhls
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Joel Stanley @ 2016-07-25 6:01 UTC (permalink / raw)
To: buildroot
pflash is a tool for reading and writing to the PNOR on OpenPower
servers, for firmware upgrades, and extracting diagnostic information.
This includes a fix so that pflash can build with musl, which has been submitted
upstream[1].
[1] http://patchwork.ozlabs.org/patch/652168/
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
package/Config.in | 1 +
...-pflash-use-atexit-for-musl-compatibility.patch | 42 ++++++++++++++++++++++
package/pflash/Config.in | 6 ++++
package/pflash/pflash.mk | 32 +++++++++++++++++
4 files changed, 81 insertions(+)
create mode 100644 package/pflash/0001-pflash-use-atexit-for-musl-compatibility.patch
create mode 100644 package/pflash/Config.in
create mode 100644 package/pflash/pflash.mk
diff --git a/package/Config.in b/package/Config.in
index 814141f18784..9f873c31fa58 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -426,6 +426,7 @@ endmenu
source "package/owl-linux/Config.in"
source "package/parted/Config.in"
source "package/pciutils/Config.in"
+ source "package/pflash/Config.in"
source "package/picocom/Config.in"
source "package/pifmrds/Config.in"
source "package/powertop/Config.in"
diff --git a/package/pflash/0001-pflash-use-atexit-for-musl-compatibility.patch b/package/pflash/0001-pflash-use-atexit-for-musl-compatibility.patch
new file mode 100644
index 000000000000..f8ac58eba329
--- /dev/null
+++ b/package/pflash/0001-pflash-use-atexit-for-musl-compatibility.patch
@@ -0,0 +1,42 @@
+From 7f6b017b8dc3b5f276b30353e0a7186e4e433e7a Mon Sep 17 00:00:00 2001
+From: Joel Stanley <joel@jms.id.au>
+Date: Mon, 25 Jul 2016 13:24:37 +0930
+Subject: [PATCH] pflash: use atexit for musl compatibility
+To: skiboot at lists.ozlabs.org
+
+I accidentally built myself a cross-toolchain with the musl libc. It does
+not support on_exit which we use to clean up in pflash.
+
+Instead use atexit with is supported by both uclibc, musl and glibc.
+
+Signed-off-by: Joel Stanley <joel@jms.id.au>
+---
+ external/pflash/pflash.c | 5 ++---
+ 1 file changed, 2 insertions(+), 3 deletions(-)
+
+diff --git a/external/pflash/pflash.c b/external/pflash/pflash.c
+index c124356fd3d9..27000469052e 100644
+--- a/external/pflash/pflash.c
++++ b/external/pflash/pflash.c
+@@ -509,7 +509,7 @@ static void print_help(const char *pname)
+ printf("\t\tThis message.\n\n");
+ }
+
+-void exiting(int d, void *p)
++void exiting(void)
+ {
+ if (need_relock)
+ arch_flash_set_wrprotect(bl, 1);
+@@ -775,8 +775,7 @@ int main(int argc, char *argv[])
+ exit(1);
+ }
+
+- on_exit(exiting, NULL);
+-
++ atexit(exiting);
+
+ rc = blocklevel_get_info(bl, &fl_name,
+ &fl_total_size, &fl_erase_granule);
+--
+2.8.1
+
diff --git a/package/pflash/Config.in b/package/pflash/Config.in
new file mode 100644
index 000000000000..315989724088
--- /dev/null
+++ b/package/pflash/Config.in
@@ -0,0 +1,6 @@
+config BR2_PACKAGE_PFLASH
+ bool "pflash"
+ depends on BR2_arm || BR2_powerpc64 || BR2_powerpc64le || BR2_x86_64
+ help
+ PNOR Flash reading and writing tool for OpenPower servers. Used for
+ firmware upgrades and extracting diagnostic infromation from flash.
diff --git a/package/pflash/pflash.mk b/package/pflash/pflash.mk
new file mode 100644
index 000000000000..ab19ad699866
--- /dev/null
+++ b/package/pflash/pflash.mk
@@ -0,0 +1,32 @@
+################################################################################
+#
+# pflash
+#
+################################################################################
+
+PFLASH_VERSION = skiboot-5.2.4
+
+PFLASH_SITE = $(call github,open-power,skiboot,$(PFLASH_VERSION))
+PFLASH_INSTALL_STAGING = YES
+PFLASH_LICENSE = Apache 2.0
+PFLASH_LICENSE_FILE = LICENCE
+
+PFLASH_MAKE_OPTS += CROSS_COMPILE="$(TARGET_CROSS)" \
+ PFLASH_VERSION=$(PFLASH_VERSION) \
+ DESTDIR=$(TARGET_DIR) \
+ -C $(@D)/external/pflash \
+
+# A makefile bug causes recent versions of pflash to fail setting CC and LD
+# based on CROSS_COMPILE. Set CC and LD to remain compatible with those
+# versions.
+PFLASH_MAKE_OPTS += CC=$(TARGET_CC) LD=$(TARGET_LD)
+
+define PFLASH_BUILD_CMDS
+ $(TARGET_MAKE_ENV) $(MAKE) $(PFLASH_MAKE_OPTS) all
+endef
+
+define PFLASH_INSTALL_TARGET_CMDS
+ $(TARGET_MAKE_ENV) $(MAKE) $(PFLASH_MAKE_OPTS) install
+endef
+
+$(eval $(generic-package))
--
2.8.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH] package/pflash: new package
2016-07-25 6:01 [Buildroot] [PATCH] package/pflash: new package Joel Stanley
@ 2016-07-25 6:14 ` Bernd Kuhls
2016-07-25 6:20 ` Joel Stanley
2016-07-25 6:38 ` [Buildroot] [PATCH v2] " Joel Stanley
2016-07-25 22:07 ` [Buildroot] [PATCH] " Thomas Petazzoni
2 siblings, 1 reply; 7+ messages in thread
From: Bernd Kuhls @ 2016-07-25 6:14 UTC (permalink / raw)
To: buildroot
Hi,
Am Mon, 25 Jul 2016 15:31:27 +0930 schrieb Joel Stanley:
> diff --git a/package/pflash/Config.in b/package/pflash/Config.in
> new file mode 100644
> index 000000000000..315989724088
> --- /dev/null
> +++ b/package/pflash/Config.in
> @@ -0,0 +1,6 @@
> +config BR2_PACKAGE_PFLASH
> + bool "pflash"
> + depends on BR2_arm || BR2_powerpc64 || BR2_powerpc64le || BR2_x86_64
> + help
> + PNOR Flash reading and writing tool for OpenPower servers. Used for
> + firmware upgrades and extracting diagnostic infromation from flash.
please add
https://github.com/open-power/skiboot
to the help text.
> diff --git a/package/pflash/pflash.mk b/package/pflash/pflash.mk
> new file mode 100644
> index 000000000000..ab19ad699866
> --- /dev/null
> +++ b/package/pflash/pflash.mk
> @@ -0,0 +1,32 @@
> +################################################################################
> +#
> +# pflash
> +#
> +################################################################################
> +
> +PFLASH_VERSION = skiboot-5.2.4
> +
no newline here.
> +PFLASH_SITE = $(call github,open-power,skiboot,$(PFLASH_VERSION))
> +PFLASH_INSTALL_STAGING = YES
Do other packages need header files from pflash? If not, it is
not necessary to install the package to staging.
Regards, Bernd
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH] package/pflash: new package
2016-07-25 6:14 ` Bernd Kuhls
@ 2016-07-25 6:20 ` Joel Stanley
0 siblings, 0 replies; 7+ messages in thread
From: Joel Stanley @ 2016-07-25 6:20 UTC (permalink / raw)
To: buildroot
On Mon, Jul 25, 2016 at 3:44 PM, Bernd Kuhls <bernd.kuhls@t-online.de> wrote:
> Hi,
>
> Am Mon, 25 Jul 2016 15:31:27 +0930 schrieb Joel Stanley:
>
>> diff --git a/package/pflash/Config.in b/package/pflash/Config.in
>> new file mode 100644
>> index 000000000000..315989724088
>> --- /dev/null
>> +++ b/package/pflash/Config.in
>> @@ -0,0 +1,6 @@
>> +config BR2_PACKAGE_PFLASH
>> + bool "pflash"
>> + depends on BR2_arm || BR2_powerpc64 || BR2_powerpc64le || BR2_x86_64
>> + help
>> + PNOR Flash reading and writing tool for OpenPower servers. Used for
>> + firmware upgrades and extracting diagnostic infromation from flash.
>
> please add
>
> https://github.com/open-power/skiboot
>
> to the help text.
Good idea.
>
>> diff --git a/package/pflash/pflash.mk b/package/pflash/pflash.mk
>> new file mode 100644
>> index 000000000000..ab19ad699866
>> --- /dev/null
>> +++ b/package/pflash/pflash.mk
>> @@ -0,0 +1,32 @@
>> +################################################################################
>> +#
>> +# pflash
>> +#
>> +################################################################################
>> +
>> +PFLASH_VERSION = skiboot-5.2.4
>> +
>
> no newline here.
>
>> +PFLASH_SITE = $(call github,open-power,skiboot,$(PFLASH_VERSION))
>> +PFLASH_INSTALL_STAGING = YES
>
> Do other packages need header files from pflash? If not, it is
> not necessary to install the package to staging.
Thanks for the explanation. We don't install any headers so I can drop this.
Thanks for the review!
Cheers,
Joel
> Regards, Bernd
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH v2] package/pflash: new package
2016-07-25 6:01 [Buildroot] [PATCH] package/pflash: new package Joel Stanley
2016-07-25 6:14 ` Bernd Kuhls
@ 2016-07-25 6:38 ` Joel Stanley
2016-07-25 22:07 ` [Buildroot] [PATCH] " Thomas Petazzoni
2 siblings, 0 replies; 7+ messages in thread
From: Joel Stanley @ 2016-07-25 6:38 UTC (permalink / raw)
To: buildroot
pflash is a tool for reading and writing to the PNOR on OpenPower
servers, for firmware upgrades, and extracting diagnostic information.
This includes a fix so that pflash can build with musl, which has been submitted
upstream[1].
[1] http://patchwork.ozlabs.org/patch/652168/
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v2:
- fix whitespcae in pflash.mk
- drop PFLASH_INSTALL_STAGING; we do not install headers and therefore do not
need to install the package to staging
- add url to help text
package/Config.in | 1 +
...-pflash-use-atexit-for-musl-compatibility.patch | 42 ++++++++++++++++++++++
package/pflash/Config.in | 8 +++++
package/pflash/pflash.mk | 30 ++++++++++++++++
4 files changed, 81 insertions(+)
create mode 100644 package/pflash/0001-pflash-use-atexit-for-musl-compatibility.patch
create mode 100644 package/pflash/Config.in
create mode 100644 package/pflash/pflash.mk
diff --git a/package/Config.in b/package/Config.in
index 814141f18784..9f873c31fa58 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -426,6 +426,7 @@ endmenu
source "package/owl-linux/Config.in"
source "package/parted/Config.in"
source "package/pciutils/Config.in"
+ source "package/pflash/Config.in"
source "package/picocom/Config.in"
source "package/pifmrds/Config.in"
source "package/powertop/Config.in"
diff --git a/package/pflash/0001-pflash-use-atexit-for-musl-compatibility.patch b/package/pflash/0001-pflash-use-atexit-for-musl-compatibility.patch
new file mode 100644
index 000000000000..f8ac58eba329
--- /dev/null
+++ b/package/pflash/0001-pflash-use-atexit-for-musl-compatibility.patch
@@ -0,0 +1,42 @@
+From 7f6b017b8dc3b5f276b30353e0a7186e4e433e7a Mon Sep 17 00:00:00 2001
+From: Joel Stanley <joel@jms.id.au>
+Date: Mon, 25 Jul 2016 13:24:37 +0930
+Subject: [PATCH] pflash: use atexit for musl compatibility
+To: skiboot at lists.ozlabs.org
+
+I accidentally built myself a cross-toolchain with the musl libc. It does
+not support on_exit which we use to clean up in pflash.
+
+Instead use atexit with is supported by both uclibc, musl and glibc.
+
+Signed-off-by: Joel Stanley <joel@jms.id.au>
+---
+ external/pflash/pflash.c | 5 ++---
+ 1 file changed, 2 insertions(+), 3 deletions(-)
+
+diff --git a/external/pflash/pflash.c b/external/pflash/pflash.c
+index c124356fd3d9..27000469052e 100644
+--- a/external/pflash/pflash.c
++++ b/external/pflash/pflash.c
+@@ -509,7 +509,7 @@ static void print_help(const char *pname)
+ printf("\t\tThis message.\n\n");
+ }
+
+-void exiting(int d, void *p)
++void exiting(void)
+ {
+ if (need_relock)
+ arch_flash_set_wrprotect(bl, 1);
+@@ -775,8 +775,7 @@ int main(int argc, char *argv[])
+ exit(1);
+ }
+
+- on_exit(exiting, NULL);
+-
++ atexit(exiting);
+
+ rc = blocklevel_get_info(bl, &fl_name,
+ &fl_total_size, &fl_erase_granule);
+--
+2.8.1
+
diff --git a/package/pflash/Config.in b/package/pflash/Config.in
new file mode 100644
index 000000000000..c1937f2cea32
--- /dev/null
+++ b/package/pflash/Config.in
@@ -0,0 +1,8 @@
+config BR2_PACKAGE_PFLASH
+ bool "pflash"
+ depends on BR2_arm || BR2_powerpc64 || BR2_powerpc64le || BR2_x86_64
+ help
+ PNOR Flash reading and writing tool for OpenPower servers. Used for
+ firmware upgrades and extracting diagnostic infromation from flash.
+
+ https://github.com/open-power/skiboot
diff --git a/package/pflash/pflash.mk b/package/pflash/pflash.mk
new file mode 100644
index 000000000000..ce5c546fc043
--- /dev/null
+++ b/package/pflash/pflash.mk
@@ -0,0 +1,30 @@
+################################################################################
+#
+# pflash
+#
+################################################################################
+
+PFLASH_VERSION = skiboot-5.2.4
+PFLASH_SITE = $(call github,open-power,skiboot,$(PFLASH_VERSION))
+PFLASH_LICENSE = Apache 2.0
+PFLASH_LICENSE_FILE = LICENCE
+
+PFLASH_MAKE_OPTS += CROSS_COMPILE="$(TARGET_CROSS)" \
+ PFLASH_VERSION=$(PFLASH_VERSION) \
+ DESTDIR=$(TARGET_DIR) \
+ -C $(@D)/external/pflash \
+
+# A makefile bug causes recent versions of pflash to fail setting CC and LD
+# based on CROSS_COMPILE. Set CC and LD to remain compatible with those
+# versions.
+PFLASH_MAKE_OPTS += CC=$(TARGET_CC) LD=$(TARGET_LD)
+
+define PFLASH_BUILD_CMDS
+ $(TARGET_MAKE_ENV) $(MAKE) $(PFLASH_MAKE_OPTS) all
+endef
+
+define PFLASH_INSTALL_TARGET_CMDS
+ $(TARGET_MAKE_ENV) $(MAKE) $(PFLASH_MAKE_OPTS) install
+endef
+
+$(eval $(generic-package))
--
2.8.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH] package/pflash: new package
2016-07-25 6:01 [Buildroot] [PATCH] package/pflash: new package Joel Stanley
2016-07-25 6:14 ` Bernd Kuhls
2016-07-25 6:38 ` [Buildroot] [PATCH v2] " Joel Stanley
@ 2016-07-25 22:07 ` Thomas Petazzoni
2016-07-26 2:27 ` Joel Stanley
2 siblings, 1 reply; 7+ messages in thread
From: Thomas Petazzoni @ 2016-07-25 22:07 UTC (permalink / raw)
To: buildroot
Hello,
On Mon, 25 Jul 2016 15:31:27 +0930, Joel Stanley wrote:
> diff --git a/package/pflash/Config.in b/package/pflash/Config.in
> new file mode 100644
> index 000000000000..315989724088
> --- /dev/null
> +++ b/package/pflash/Config.in
> @@ -0,0 +1,6 @@
> +config BR2_PACKAGE_PFLASH
> + bool "pflash"
> + depends on BR2_arm || BR2_powerpc64 || BR2_powerpc64le || BR2_x86_64
Is there something that makes it inherently usable only on those
architectures?
> diff --git a/package/pflash/pflash.mk b/package/pflash/pflash.mk
> new file mode 100644
> index 000000000000..ab19ad699866
> --- /dev/null
> +++ b/package/pflash/pflash.mk
> @@ -0,0 +1,32 @@
> +################################################################################
> +#
> +# pflash
> +#
> +################################################################################
> +
> +PFLASH_VERSION = skiboot-5.2.4
> +
> +PFLASH_SITE = $(call github,open-power,skiboot,$(PFLASH_VERSION))
So skiboot is a much much larger repository, with lots of other things,
and you're building just the external/pflash subdirectory of it.
Are you sure we'll never want to have a package for other things in the
skiboot repository?
> +PFLASH_INSTALL_STAGING = YES
You're not specifying any PFLASH_INSTALL_STAGING_CMDS, so this is
useless.
> +PFLASH_LICENSE = Apache 2.0
> +PFLASH_LICENSE_FILE = LICENCE
> +
> +PFLASH_MAKE_OPTS += CROSS_COMPILE="$(TARGET_CROSS)" \
> + PFLASH_VERSION=$(PFLASH_VERSION) \
> + DESTDIR=$(TARGET_DIR) \
DESTDIR should only be passed at install time.
> + -C $(@D)/external/pflash \
Unneeded trailing \
Also, we prefer the -C option and its argument to be part of the build
and install commands themselves.
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH] package/pflash: new package
2016-07-25 22:07 ` [Buildroot] [PATCH] " Thomas Petazzoni
@ 2016-07-26 2:27 ` Joel Stanley
2016-07-26 7:32 ` Thomas Petazzoni
0 siblings, 1 reply; 7+ messages in thread
From: Joel Stanley @ 2016-07-26 2:27 UTC (permalink / raw)
To: buildroot
On Tue, Jul 26, 2016 at 7:37 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Mon, 25 Jul 2016 15:31:27 +0930, Joel Stanley wrote:
>
>> diff --git a/package/pflash/Config.in b/package/pflash/Config.in
>> new file mode 100644
>> index 000000000000..315989724088
>> --- /dev/null
>> +++ b/package/pflash/Config.in
>> @@ -0,0 +1,6 @@
>> +config BR2_PACKAGE_PFLASH
>> + bool "pflash"
>> + depends on BR2_arm || BR2_powerpc64 || BR2_powerpc64le || BR2_x86_64
>
> Is there something that makes it inherently usable only on those
> architectures?
Yeah, the 'libflash' library selects different backends depending on it's
>
>> diff --git a/package/pflash/pflash.mk b/package/pflash/pflash.mk
>> new file mode 100644
>> index 000000000000..ab19ad699866
>> --- /dev/null
>> +++ b/package/pflash/pflash.mk
>> @@ -0,0 +1,32 @@
>> +################################################################################
>> +#
>> +# pflash
>> +#
>> +################################################################################
>> +
>> +PFLASH_VERSION = skiboot-5.2.4
>> +
>> +PFLASH_SITE = $(call github,open-power,skiboot,$(PFLASH_VERSION))
>
> So skiboot is a much much larger repository, with lots of other things,
> and you're building just the external/pflash subdirectory of it.
>
> Are you sure we'll never want to have a package for other things in the
> skiboot repository?
We might want to package other tools in the future.
How should I change the packaging of pflash?
>
>> +PFLASH_INSTALL_STAGING = YES
>
> You're not specifying any PFLASH_INSTALL_STAGING_CMDS, so this is
> useless.
Thanks for clearing that up. I've fixed it in v2.
>
>> +PFLASH_LICENSE = Apache 2.0
>> +PFLASH_LICENSE_FILE = LICENCE
>> +
>> +PFLASH_MAKE_OPTS += CROSS_COMPILE="$(TARGET_CROSS)" \
>> + PFLASH_VERSION=$(PFLASH_VERSION) \
>> + DESTDIR=$(TARGET_DIR) \
>
> DESTDIR should only be passed at install time.
Okay.
>
>> + -C $(@D)/external/pflash \
>
> Unneeded trailing \
Okay.
>
> Also, we prefer the -C option and its argument to be part of the build
> and install commands themselves.
Even though we would be replicating them for every invocation of make?
I think it's better placed in a variable so changes can be made in one
place.
Cheers,
Joel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH] package/pflash: new package
2016-07-26 2:27 ` Joel Stanley
@ 2016-07-26 7:32 ` Thomas Petazzoni
0 siblings, 0 replies; 7+ messages in thread
From: Thomas Petazzoni @ 2016-07-26 7:32 UTC (permalink / raw)
To: buildroot
Hello,
On Tue, 26 Jul 2016 11:57:17 +0930, Joel Stanley wrote:
> >> diff --git a/package/pflash/Config.in b/package/pflash/Config.in
> >> new file mode 100644
> >> index 000000000000..315989724088
> >> --- /dev/null
> >> +++ b/package/pflash/Config.in
> >> @@ -0,0 +1,6 @@
> >> +config BR2_PACKAGE_PFLASH
> >> + bool "pflash"
> >> + depends on BR2_arm || BR2_powerpc64 || BR2_powerpc64le || BR2_x86_64
> >
> > Is there something that makes it inherently usable only on those
> > architectures?
>
> Yeah, the 'libflash' library selects different backends depending on it's
OK. But then it makes sense only for pflash itself, and not skitool as
the whole I suppose?
> > So skiboot is a much much larger repository, with lots of other things,
> > and you're building just the external/pflash subdirectory of it.
> >
> > Are you sure we'll never want to have a package for other things in the
> > skiboot repository?
>
> We might want to package other tools in the future.
>
> How should I change the packaging of pflash?
A top-level BR2_PACKAGE_SKITOOL option, with a package called skitool.
And then a sub-option BR2_PACKAGE_SKITOOL_PFLASH, which enables the
build/installation of pflash.
BR2_PACKAGE_SKITOOL can "select" BR2_PACKAGE_SKITOOL_PFLASH to make
sure the package at least installs one thing.
> > Also, we prefer the -C option and its argument to be part of the build
> > and install commands themselves.
>
> Even though we would be replicating them for every invocation of make?
> I think it's better placed in a variable so changes can be made in one
> place.
We generally repeat the -C $(@D) in the different invocations, yes. Not
sure it's ideal, but that's how we typically do it in most packages.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-07-26 7:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-25 6:01 [Buildroot] [PATCH] package/pflash: new package Joel Stanley
2016-07-25 6:14 ` Bernd Kuhls
2016-07-25 6:20 ` Joel Stanley
2016-07-25 6:38 ` [Buildroot] [PATCH v2] " Joel Stanley
2016-07-25 22:07 ` [Buildroot] [PATCH] " Thomas Petazzoni
2016-07-26 2:27 ` Joel Stanley
2016-07-26 7:32 ` Thomas Petazzoni
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.