* [RFC] u-boot-fw-utils: Allow target-specific fw_env.config @ 2017-06-20 20:40 Brad Mouring 2017-06-20 20:43 ` Marek Vasut 0 siblings, 1 reply; 11+ messages in thread From: Brad Mouring @ 2017-06-20 20:40 UTC (permalink / raw) To: Openembedded Core; +Cc: Marek Vasut, Tom Rini, Brad Mouring As implemented currently, the fw-utils recipe does not allow for a board- or distro-specific fw_env.config override, instead opting to include the default (commented, example-filled) fw_env.config from the u-boot source. This change introduces a variable that allows for overriding this, while defaulting to the example config file from the u-boot source. Signed-off-by: Brad Mouring <brad.mouring@ni.com> --- meta/recipes-bsp/u-boot/u-boot-fw-utils_2017.05.bb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/meta/recipes-bsp/u-boot/u-boot-fw-utils_2017.05.bb b/meta/recipes-bsp/u-boot/u-boot-fw-utils_2017.05.bb index c2e8f0fb84..b06282ac03 100644 --- a/meta/recipes-bsp/u-boot/u-boot-fw-utils_2017.05.bb +++ b/meta/recipes-bsp/u-boot/u-boot-fw-utils_2017.05.bb @@ -7,6 +7,11 @@ INSANE_SKIP_${PN} = "already-stripped" EXTRA_OEMAKE_class-target = 'CROSS_COMPILE=${TARGET_PREFIX} CC="${CC} ${CFLAGS} ${LDFLAGS}" HOSTCC="${BUILD_CC} ${BUILD_CFLAGS} ${BUILD_LDFLAGS}" V=1' EXTRA_OEMAKE_class-cross = 'ARCH=${TARGET_ARCH} CC="${CC} ${CFLAGS} ${LDFLAGS}" V=1' +# U-Boot environment configuration file variables. This file is used +# by the U-Boot environment utilities "fw_printenv" and "fw_setenv". +# By default, use the default included in the U-Boot source +UBOOT_FW_ENV_CONFIG ??= "${S}/tools/env/fw_env.config" + inherit uboot-config do_compile () { @@ -19,7 +24,7 @@ do_install () { install -d ${D}${sysconfdir} install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_printenv install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_setenv - install -m 0644 ${S}/tools/env/fw_env.config ${D}${sysconfdir}/fw_env.config + install -m 0644 ${UBOOT_FW_ENV_CONFIG} ${D}${sysconfdir}/fw_env.config } do_install_class-cross () { -- 2.13.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC] u-boot-fw-utils: Allow target-specific fw_env.config 2017-06-20 20:40 [RFC] u-boot-fw-utils: Allow target-specific fw_env.config Brad Mouring @ 2017-06-20 20:43 ` Marek Vasut 2017-06-20 20:53 ` Brad Mouring 0 siblings, 1 reply; 11+ messages in thread From: Marek Vasut @ 2017-06-20 20:43 UTC (permalink / raw) To: Brad Mouring, Openembedded Core; +Cc: Tom Rini On 06/20/2017 10:40 PM, Brad Mouring wrote: > As implemented currently, the fw-utils recipe does not allow for > a board- or distro-specific fw_env.config override, instead opting > to include the default (commented, example-filled) fw_env.config > from the u-boot source. This change introduces a variable that allows > for overriding this, while defaulting to the example config file from > the u-boot source. > > Signed-off-by: Brad Mouring <brad.mouring@ni.com> > --- > meta/recipes-bsp/u-boot/u-boot-fw-utils_2017.05.bb | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/meta/recipes-bsp/u-boot/u-boot-fw-utils_2017.05.bb b/meta/recipes-bsp/u-boot/u-boot-fw-utils_2017.05.bb > index c2e8f0fb84..b06282ac03 100644 > --- a/meta/recipes-bsp/u-boot/u-boot-fw-utils_2017.05.bb > +++ b/meta/recipes-bsp/u-boot/u-boot-fw-utils_2017.05.bb > @@ -7,6 +7,11 @@ INSANE_SKIP_${PN} = "already-stripped" > EXTRA_OEMAKE_class-target = 'CROSS_COMPILE=${TARGET_PREFIX} CC="${CC} ${CFLAGS} ${LDFLAGS}" HOSTCC="${BUILD_CC} ${BUILD_CFLAGS} ${BUILD_LDFLAGS}" V=1' > EXTRA_OEMAKE_class-cross = 'ARCH=${TARGET_ARCH} CC="${CC} ${CFLAGS} ${LDFLAGS}" V=1' > > +# U-Boot environment configuration file variables. This file is used > +# by the U-Boot environment utilities "fw_printenv" and "fw_setenv". > +# By default, use the default included in the U-Boot source > +UBOOT_FW_ENV_CONFIG ??= "${S}/tools/env/fw_env.config" > + > inherit uboot-config > > do_compile () { > @@ -19,7 +24,7 @@ do_install () { > install -d ${D}${sysconfdir} > install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_printenv > install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_setenv > - install -m 0644 ${S}/tools/env/fw_env.config ${D}${sysconfdir}/fw_env.config > + install -m 0644 ${UBOOT_FW_ENV_CONFIG} ${D}${sysconfdir}/fw_env.config Do we really need yet another variable ? Wouldn't it make more sense to add do_install_append_yourmachine() {} in your meta-whatever to u-boot-fw-utils_%.bbappend and install whatever additional files you need ? > } > > do_install_class-cross () { > -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] u-boot-fw-utils: Allow target-specific fw_env.config 2017-06-20 20:43 ` Marek Vasut @ 2017-06-20 20:53 ` Brad Mouring 2017-06-20 20:57 ` Otavio Salvador 2017-06-20 20:59 ` Marek Vasut 0 siblings, 2 replies; 11+ messages in thread From: Brad Mouring @ 2017-06-20 20:53 UTC (permalink / raw) To: Marek Vasut; +Cc: Tom Rini, Openembedded Core On Tue, Jun 20, 2017 at 10:43:51PM +0200, Marek Vasut wrote: > On 06/20/2017 10:40 PM, Brad Mouring wrote: > > As implemented currently, the fw-utils recipe does not allow for > > ... > > +# by the U-Boot environment utilities "fw_printenv" and "fw_setenv". > > +# By default, use the default included in the U-Boot source > > +UBOOT_FW_ENV_CONFIG ??= "${S}/tools/env/fw_env.config" > > + > > inherit uboot-config > > > > do_compile () { > > @@ -19,7 +24,7 @@ do_install () { > > install -d ${D}${sysconfdir} > > install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_printenv > > install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_setenv > > - install -m 0644 ${S}/tools/env/fw_env.config ${D}${sysconfdir}/fw_env.config > > + install -m 0644 ${UBOOT_FW_ENV_CONFIG} ${D}${sysconfdir}/fw_env.config > > Do we really need yet another variable ? Wouldn't it make more sense to > add do_install_append_yourmachine() {} in your meta-whatever to > u-boot-fw-utils_%.bbappend and install whatever additional files you need ? This is (kinda) what we were doing, there was some discussion as to whether or not this made sense upstream. I was unsure of the acceptability of a do_install_append.*() clobbering a file of the original do_install(). Thanks for the input. > > } > > > > do_install_class-cross () { > > > > > -- > Best regards, > Marek Vasut - Brad ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] u-boot-fw-utils: Allow target-specific fw_env.config 2017-06-20 20:53 ` Brad Mouring @ 2017-06-20 20:57 ` Otavio Salvador 2017-06-20 21:16 ` Marek Vasut 2017-06-20 20:59 ` Marek Vasut 1 sibling, 1 reply; 11+ messages in thread From: Otavio Salvador @ 2017-06-20 20:57 UTC (permalink / raw) To: Brad Mouring; +Cc: Marek Vasut, Tom Rini, Openembedded Core On Tue, Jun 20, 2017 at 5:53 PM, Brad Mouring <brad.mouring@ni.com> wrote: > On Tue, Jun 20, 2017 at 10:43:51PM +0200, Marek Vasut wrote: >> On 06/20/2017 10:40 PM, Brad Mouring wrote: >> > As implemented currently, the fw-utils recipe does not allow for >> > ... >> > +# by the U-Boot environment utilities "fw_printenv" and "fw_setenv". >> > +# By default, use the default included in the U-Boot source >> > +UBOOT_FW_ENV_CONFIG ??= "${S}/tools/env/fw_env.config" >> > + >> > inherit uboot-config >> > >> > do_compile () { >> > @@ -19,7 +24,7 @@ do_install () { >> > install -d ${D}${sysconfdir} >> > install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_printenv >> > install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_setenv >> > - install -m 0644 ${S}/tools/env/fw_env.config ${D}${sysconfdir}/fw_env.config >> > + install -m 0644 ${UBOOT_FW_ENV_CONFIG} ${D}${sysconfdir}/fw_env.config >> >> Do we really need yet another variable ? Wouldn't it make more sense to >> add do_install_append_yourmachine() {} in your meta-whatever to >> u-boot-fw-utils_%.bbappend and install whatever additional files you need ? > > This is (kinda) what we were doing, there was some discussion as to > whether or not this made sense upstream. I was unsure of the > acceptability of a do_install_append.*() clobbering a file of the > original do_install(). > > Thanks for the input. Or split up the configuration to u-boot-fw-utils-config package and make u-boot-fw-utils generic? So we can have the configuration in a specific package. -- Otavio Salvador O.S. Systems http://www.ossystems.com.br http://code.ossystems.com.br Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] u-boot-fw-utils: Allow target-specific fw_env.config 2017-06-20 20:57 ` Otavio Salvador @ 2017-06-20 21:16 ` Marek Vasut 2017-06-20 22:08 ` Otavio Salvador 0 siblings, 1 reply; 11+ messages in thread From: Marek Vasut @ 2017-06-20 21:16 UTC (permalink / raw) To: Otavio Salvador, Brad Mouring; +Cc: Tom Rini, Openembedded Core On 06/20/2017 10:57 PM, Otavio Salvador wrote: > On Tue, Jun 20, 2017 at 5:53 PM, Brad Mouring <brad.mouring@ni.com> wrote: >> On Tue, Jun 20, 2017 at 10:43:51PM +0200, Marek Vasut wrote: >>> On 06/20/2017 10:40 PM, Brad Mouring wrote: >>>> As implemented currently, the fw-utils recipe does not allow for >>>> ... >>>> +# by the U-Boot environment utilities "fw_printenv" and "fw_setenv". >>>> +# By default, use the default included in the U-Boot source >>>> +UBOOT_FW_ENV_CONFIG ??= "${S}/tools/env/fw_env.config" >>>> + >>>> inherit uboot-config >>>> >>>> do_compile () { >>>> @@ -19,7 +24,7 @@ do_install () { >>>> install -d ${D}${sysconfdir} >>>> install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_printenv >>>> install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_setenv >>>> - install -m 0644 ${S}/tools/env/fw_env.config ${D}${sysconfdir}/fw_env.config >>>> + install -m 0644 ${UBOOT_FW_ENV_CONFIG} ${D}${sysconfdir}/fw_env.config >>> >>> Do we really need yet another variable ? Wouldn't it make more sense to >>> add do_install_append_yourmachine() {} in your meta-whatever to >>> u-boot-fw-utils_%.bbappend and install whatever additional files you need ? >> >> This is (kinda) what we were doing, there was some discussion as to >> whether or not this made sense upstream. I was unsure of the >> acceptability of a do_install_append.*() clobbering a file of the >> original do_install(). >> >> Thanks for the input. > > Or split up the configuration to u-boot-fw-utils-config package and > make u-boot-fw-utils generic? So we can have the configuration in a > specific package. Keep in mind that u-boot-fw-utils is actually built for particular machine (it contains ie. the default env for that machine) so bundling the config file with it is IMO OK. No need to introduce additional packages for every simple file. -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] u-boot-fw-utils: Allow target-specific fw_env.config 2017-06-20 21:16 ` Marek Vasut @ 2017-06-20 22:08 ` Otavio Salvador 0 siblings, 0 replies; 11+ messages in thread From: Otavio Salvador @ 2017-06-20 22:08 UTC (permalink / raw) To: Marek Vasut; +Cc: Tom Rini, Openembedded Core, Brad Mouring On Tue, Jun 20, 2017 at 6:16 PM, Marek Vasut <marex@denx.de> wrote: > On 06/20/2017 10:57 PM, Otavio Salvador wrote: >> On Tue, Jun 20, 2017 at 5:53 PM, Brad Mouring <brad.mouring@ni.com> wrote: >>> On Tue, Jun 20, 2017 at 10:43:51PM +0200, Marek Vasut wrote: >>>> On 06/20/2017 10:40 PM, Brad Mouring wrote: >>>>> As implemented currently, the fw-utils recipe does not allow for >>>>> ... >>>>> +# by the U-Boot environment utilities "fw_printenv" and "fw_setenv". >>>>> +# By default, use the default included in the U-Boot source >>>>> +UBOOT_FW_ENV_CONFIG ??= "${S}/tools/env/fw_env.config" >>>>> + >>>>> inherit uboot-config >>>>> >>>>> do_compile () { >>>>> @@ -19,7 +24,7 @@ do_install () { >>>>> install -d ${D}${sysconfdir} >>>>> install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_printenv >>>>> install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_setenv >>>>> - install -m 0644 ${S}/tools/env/fw_env.config ${D}${sysconfdir}/fw_env.config >>>>> + install -m 0644 ${UBOOT_FW_ENV_CONFIG} ${D}${sysconfdir}/fw_env.config >>>> >>>> Do we really need yet another variable ? Wouldn't it make more sense to >>>> add do_install_append_yourmachine() {} in your meta-whatever to >>>> u-boot-fw-utils_%.bbappend and install whatever additional files you need ? >>> >>> This is (kinda) what we were doing, there was some discussion as to >>> whether or not this made sense upstream. I was unsure of the >>> acceptability of a do_install_append.*() clobbering a file of the >>> original do_install(). >>> >>> Thanks for the input. >> >> Or split up the configuration to u-boot-fw-utils-config package and >> make u-boot-fw-utils generic? So we can have the configuration in a >> specific package. > > Keep in mind that u-boot-fw-utils is actually built for particular > machine (it contains ie. the default env for that machine) so bundling > the config file with it is IMO OK. No need to introduce additional > packages for every simple file. I will propose one patch. See what you think ... -- Otavio Salvador O.S. Systems http://www.ossystems.com.br http://code.ossystems.com.br Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] u-boot-fw-utils: Allow target-specific fw_env.config 2017-06-20 20:53 ` Brad Mouring 2017-06-20 20:57 ` Otavio Salvador @ 2017-06-20 20:59 ` Marek Vasut 2017-06-21 2:33 ` Brad Mouring 1 sibling, 1 reply; 11+ messages in thread From: Marek Vasut @ 2017-06-20 20:59 UTC (permalink / raw) To: Brad Mouring; +Cc: Tom Rini, Openembedded Core On 06/20/2017 10:53 PM, Brad Mouring wrote: > On Tue, Jun 20, 2017 at 10:43:51PM +0200, Marek Vasut wrote: >> On 06/20/2017 10:40 PM, Brad Mouring wrote: >>> As implemented currently, the fw-utils recipe does not allow for >>> ... >>> +# by the U-Boot environment utilities "fw_printenv" and "fw_setenv". >>> +# By default, use the default included in the U-Boot source >>> +UBOOT_FW_ENV_CONFIG ??= "${S}/tools/env/fw_env.config" >>> + >>> inherit uboot-config >>> >>> do_compile () { >>> @@ -19,7 +24,7 @@ do_install () { >>> install -d ${D}${sysconfdir} >>> install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_printenv >>> install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_setenv >>> - install -m 0644 ${S}/tools/env/fw_env.config ${D}${sysconfdir}/fw_env.config >>> + install -m 0644 ${UBOOT_FW_ENV_CONFIG} ${D}${sysconfdir}/fw_env.config >> >> Do we really need yet another variable ? Wouldn't it make more sense to >> add do_install_append_yourmachine() {} in your meta-whatever to >> u-boot-fw-utils_%.bbappend and install whatever additional files you need ? > > This is (kinda) what we were doing, there was some discussion as to > whether or not this made sense upstream. Link? > I was unsure of the > acceptability of a do_install_append.*() clobbering a file of the > original do_install(). That's probably what really needs to be discussed. We can probably add some task which by default installs the fw_env.config example and can be overridden in meta-whatever . Maybe the others can jump into here and explain how to handle overriding the default config file best. -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] u-boot-fw-utils: Allow target-specific fw_env.config 2017-06-20 20:59 ` Marek Vasut @ 2017-06-21 2:33 ` Brad Mouring 2017-06-21 5:22 ` Denys Dmytriyenko 0 siblings, 1 reply; 11+ messages in thread From: Brad Mouring @ 2017-06-21 2:33 UTC (permalink / raw) To: Marek Vasut; +Cc: Tom Rini, Openembedded Core On Tue, Jun 20, 2017 at 10:59:55PM +0200, Marek Vasut wrote: > On 06/20/2017 10:53 PM, Brad Mouring wrote: > > On Tue, Jun 20, 2017 at 10:43:51PM +0200, Marek Vasut wrote: > >> On 06/20/2017 10:40 PM, Brad Mouring wrote: > >>> As implemented currently, the fw-utils recipe does not allow for > >>> ... > >>> +# by the U-Boot environment utilities "fw_printenv" and "fw_setenv". > >>> +# By default, use the default included in the U-Boot source > >>> +UBOOT_FW_ENV_CONFIG ??= "${S}/tools/env/fw_env.config" > >>> + > >>> inherit uboot-config > >>> > >>> do_compile () { > >>> @@ -19,7 +24,7 @@ do_install () { > >>> install -d ${D}${sysconfdir} > >>> install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_printenv > >>> install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_setenv > >>> - install -m 0644 ${S}/tools/env/fw_env.config ${D}${sysconfdir}/fw_env.config > >>> + install -m 0644 ${UBOOT_FW_ENV_CONFIG} ${D}${sysconfdir}/fw_env.config > >> > >> Do we really need yet another variable ? Wouldn't it make more sense to > >> add do_install_append_yourmachine() {} in your meta-whatever to > >> u-boot-fw-utils_%.bbappend and install whatever additional files you need ? > > > > This is (kinda) what we were doing, there was some discussion as to > > whether or not this made sense upstream. > > Link? I know it's not a great answer, but we've not pushed the version of the branch where these changes are going in. Eventually, they'll end up in this repo: https://github.com/ni/meta-nilrt > > I was unsure of the > > acceptability of a do_install_append.*() clobbering a file of the > > original do_install(). > > That's probably what really needs to be discussed. > > We can probably add some task which by default installs the > fw_env.config example and can be overridden in meta-whatever . Maybe the > others can jump into here and explain how to handle overriding the > default config file best. That sounds like a solution that would certainly work for this use-case, if no one pipes up with objections or a currently-unseen silver bullet solution, I'll try to whip something together tomorrow and post. Thanks for the idea. Denys, I know you keep pushing the "shove it in a do_install_append()", but to me and my under-informed sensibilities, this seems weird and unclean to clobber a file in a _append(), would it cause some QA failure? > -- > Best regards, > Marek Vasut Regards, Brad Mouring ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] u-boot-fw-utils: Allow target-specific fw_env.config 2017-06-21 2:33 ` Brad Mouring @ 2017-06-21 5:22 ` Denys Dmytriyenko 2017-06-21 6:07 ` Gary Thomas 2017-06-21 12:56 ` Brad Mouring 0 siblings, 2 replies; 11+ messages in thread From: Denys Dmytriyenko @ 2017-06-21 5:22 UTC (permalink / raw) To: Brad Mouring; +Cc: Marek Vasut, Tom Rini, Openembedded Core On Tue, Jun 20, 2017 at 09:33:24PM -0500, Brad Mouring wrote: > On Tue, Jun 20, 2017 at 10:59:55PM +0200, Marek Vasut wrote: > > On 06/20/2017 10:53 PM, Brad Mouring wrote: > > > On Tue, Jun 20, 2017 at 10:43:51PM +0200, Marek Vasut wrote: > > >> On 06/20/2017 10:40 PM, Brad Mouring wrote: > > >>> As implemented currently, the fw-utils recipe does not allow for > > >>> ... > > >>> +# by the U-Boot environment utilities "fw_printenv" and "fw_setenv". > > >>> +# By default, use the default included in the U-Boot source > > >>> +UBOOT_FW_ENV_CONFIG ??= "${S}/tools/env/fw_env.config" > > >>> + > > >>> inherit uboot-config > > >>> > > >>> do_compile () { > > >>> @@ -19,7 +24,7 @@ do_install () { > > >>> install -d ${D}${sysconfdir} > > >>> install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_printenv > > >>> install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_setenv > > >>> - install -m 0644 ${S}/tools/env/fw_env.config ${D}${sysconfdir}/fw_env.config > > >>> + install -m 0644 ${UBOOT_FW_ENV_CONFIG} ${D}${sysconfdir}/fw_env.config > > >> > > >> Do we really need yet another variable ? Wouldn't it make more sense to > > >> add do_install_append_yourmachine() {} in your meta-whatever to > > >> u-boot-fw-utils_%.bbappend and install whatever additional files you need ? > > > > > > This is (kinda) what we were doing, there was some discussion as to > > > whether or not this made sense upstream. > > > > Link? > > I know it's not a great answer, but we've not pushed the version of the > branch where these changes are going in. Eventually, they'll end up in > this repo: > > https://github.com/ni/meta-nilrt > > > > I was unsure of the > > > acceptability of a do_install_append.*() clobbering a file of the > > > original do_install(). > > > > That's probably what really needs to be discussed. > > > > We can probably add some task which by default installs the > > fw_env.config example and can be overridden in meta-whatever . Maybe the > > others can jump into here and explain how to handle overriding the > > default config file best. > > That sounds like a solution that would certainly work for this > use-case, if no one pipes up with objections or a currently-unseen > silver bullet solution, I'll try to whip something together tomorrow > and post. Thanks for the idea. > > Denys, I know you keep pushing the "shove it in a do_install_append()", > but to me and my under-informed sensibilities, this seems weird and > unclean to clobber a file in a _append(), would it cause some QA failure? Hmm, I mentioned it only once... To a patch that does already mention appending stuff... -- Denys ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] u-boot-fw-utils: Allow target-specific fw_env.config 2017-06-21 5:22 ` Denys Dmytriyenko @ 2017-06-21 6:07 ` Gary Thomas 2017-06-21 12:56 ` Brad Mouring 1 sibling, 0 replies; 11+ messages in thread From: Gary Thomas @ 2017-06-21 6:07 UTC (permalink / raw) To: openembedded-core [-- Attachment #1: Type: text/plain, Size: 3081 bytes --] On 2017-06-21 07:22, Denys Dmytriyenko wrote: > On Tue, Jun 20, 2017 at 09:33:24PM -0500, Brad Mouring wrote: >> On Tue, Jun 20, 2017 at 10:59:55PM +0200, Marek Vasut wrote: >>> On 06/20/2017 10:53 PM, Brad Mouring wrote: >>>> On Tue, Jun 20, 2017 at 10:43:51PM +0200, Marek Vasut wrote: >>>>> On 06/20/2017 10:40 PM, Brad Mouring wrote: >>>>>> As implemented currently, the fw-utils recipe does not allow for >>>>>> ... >>>>>> +# by the U-Boot environment utilities "fw_printenv" and "fw_setenv". >>>>>> +# By default, use the default included in the U-Boot source >>>>>> +UBOOT_FW_ENV_CONFIG ??= "${S}/tools/env/fw_env.config" >>>>>> + >>>>>> inherit uboot-config >>>>>> >>>>>> do_compile () { >>>>>> @@ -19,7 +24,7 @@ do_install () { >>>>>> install -d ${D}${sysconfdir} >>>>>> install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_printenv >>>>>> install -m 755 ${S}/tools/env/fw_printenv ${D}${base_sbindir}/fw_setenv >>>>>> - install -m 0644 ${S}/tools/env/fw_env.config ${D}${sysconfdir}/fw_env.config >>>>>> + install -m 0644 ${UBOOT_FW_ENV_CONFIG} ${D}${sysconfdir}/fw_env.config >>>>> >>>>> Do we really need yet another variable ? Wouldn't it make more sense to >>>>> add do_install_append_yourmachine() {} in your meta-whatever to >>>>> u-boot-fw-utils_%.bbappend and install whatever additional files you need ? >>>> >>>> This is (kinda) what we were doing, there was some discussion as to >>>> whether or not this made sense upstream. >>> >>> Link? >> >> I know it's not a great answer, but we've not pushed the version of the >> branch where these changes are going in. Eventually, they'll end up in >> this repo: >> >> https://github.com/ni/meta-nilrt >> >>>> I was unsure of the >>>> acceptability of a do_install_append.*() clobbering a file of the >>>> original do_install(). >>> >>> That's probably what really needs to be discussed. >>> >>> We can probably add some task which by default installs the >>> fw_env.config example and can be overridden in meta-whatever . Maybe the >>> others can jump into here and explain how to handle overriding the >>> default config file best. >> >> That sounds like a solution that would certainly work for this >> use-case, if no one pipes up with objections or a currently-unseen >> silver bullet solution, I'll try to whip something together tomorrow >> and post. Thanks for the idea. >> >> Denys, I know you keep pushing the "shove it in a do_install_append()", >> but to me and my under-informed sensibilities, this seems weird and >> unclean to clobber a file in a _append(), would it cause some QA failure? > > Hmm, I mentioned it only once... To a patch that does already mention > appending stuff... > We do this all the time for this recipe. It just takes a simple .bbappend like this one. -- ------------------------------------------------------------ Gary Thomas | Consulting for the MLB Associates | Embedded world ------------------------------------------------------------ [-- Attachment #2: u-boot-fw-utils_2017.05.bbappend --] [-- Type: text/plain, Size: 290 bytes --] FILESEXTRAPATHS_prepend := "${THISDIR}/${PN}:" SRC_URI += "file://fw_env.config \ " do_install_append() { install -d ${D}${sysconfdir} install -m 0644 ${WORKDIR}/fw_env.config ${D}${sysconfdir}/fw_env.config } PACKAGE_ARCH = "${MACHINE_ARCH}" COMPATIBLE_MACHINE = "${MACHINE}" ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] u-boot-fw-utils: Allow target-specific fw_env.config 2017-06-21 5:22 ` Denys Dmytriyenko 2017-06-21 6:07 ` Gary Thomas @ 2017-06-21 12:56 ` Brad Mouring 1 sibling, 0 replies; 11+ messages in thread From: Brad Mouring @ 2017-06-21 12:56 UTC (permalink / raw) To: Denys Dmytriyenko; +Cc: Marek Vasut, Tom Rini, Openembedded Core On Wed, Jun 21, 2017 at 01:22:58AM -0400, Denys Dmytriyenko wrote: > On Tue, Jun 20, 2017 at 09:33:24PM -0500, Brad Mouring wrote: > > On Tue, Jun 20, 2017 at 10:59:55PM +0200, Marek Vasut wrote: > > > On 06/20/2017 10:53 PM, Brad Mouring wrote: > > > > On Tue, Jun 20, 2017 at 10:43:51PM +0200, Marek Vasut wrote: > > > >> On 06/20/2017 10:40 PM, Brad Mouring wrote: > > > >>> As implemented currently, the fw-utils recipe does not allow for > > > >>> ... > > > >> > > > >> Do we really need yet another variable ? Wouldn't it make more sense to > > > >> add do_install_append_yourmachine() {} in your meta-whatever to > > > >> u-boot-fw-utils_%.bbappend and install whatever additional files you need ? > > > > > > > > This is (kinda) what we were doing, there was some discussion as to > > > > whether or not this made sense upstream. > > > > > > Link? > > > > I know it's not a great answer, but we've not pushed the version of the > > branch where these changes are going in. Eventually, they'll end up in > > this repo: > > > > https://github.com/ni/meta-nilrt > > > > > > I was unsure of the > > > > acceptability of a do_install_append.*() clobbering a file of the > > > > original do_install(). > > > > > > That's probably what really needs to be discussed. > > > > > > We can probably add some task which by default installs the > > > fw_env.config example and can be overridden in meta-whatever . Maybe the > > > others can jump into here and explain how to handle overriding the > > > default config file best. > > > > That sounds like a solution that would certainly work for this > > use-case, if no one pipes up with objections or a currently-unseen > > silver bullet solution, I'll try to whip something together tomorrow > > and post. Thanks for the idea. > > > > Denys, I know you keep pushing the "shove it in a do_install_append()", > > but to me and my under-informed sensibilities, this seems weird and > > unclean to clobber a file in a _append(), would it cause some QA failure? > > Hmm, I mentioned it only once... To a patch that does already mention > appending stuff... Fair. I thought you'd also mentioned it on irc, but it doesn't really matter. After sleeping on it, I think I've gotten over my distaste for overwriting a file when we're already glomming changes atop the original recipe. I'll just take that approach. Thanks, Brad Mouring ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-06-21 12:57 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-20 20:40 [RFC] u-boot-fw-utils: Allow target-specific fw_env.config Brad Mouring 2017-06-20 20:43 ` Marek Vasut 2017-06-20 20:53 ` Brad Mouring 2017-06-20 20:57 ` Otavio Salvador 2017-06-20 21:16 ` Marek Vasut 2017-06-20 22:08 ` Otavio Salvador 2017-06-20 20:59 ` Marek Vasut 2017-06-21 2:33 ` Brad Mouring 2017-06-21 5:22 ` Denys Dmytriyenko 2017-06-21 6:07 ` Gary Thomas 2017-06-21 12:56 ` Brad Mouring
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.