All of lore.kernel.org
 help / color / mirror / Atom feed
* [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: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: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: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.