[PATCH] imd: model and compatible missing in metadata

[PATCH] imd: model and compatible missing in metadata

[Thorsten K. Scherer]

fixes 97e81f2

Signed-off-by: Thorsten K. Scherer <t.scherer@eckelmann.de>
---
 include/asm-generic/barebox.lds.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/asm-generic/barebox.lds.h b/include/asm-generic/barebox.lds.h
index 74d3ca4a9..decaab79a 100644
--- a/include/asm-generic/barebox.lds.h
+++ b/include/asm-generic/barebox.lds.h
@@ -66,7 +66,7 @@
 #define BAREBOX_IMD				\
 	KEEP(*(.barebox_imd_start))		\
 	KEEP(*(.barebox_imd_1*))		\
-	*(.barebox_imd_0*)			\
+	KEEP(*(.barebox_imd_0*))		\
 	KEEP(*(.barebox_imd_end))
 
 #if defined(CONFIG_ARCH_BAREBOX_MAX_BARE_INIT_SIZE) && \
-- 
2.19.1


-- 
Eckelmann AG
Vorstand: Dipl.-Ing. Peter Frankenbach (Sprecher) Dipl.-Wi.-Ing. Philipp Eckelmann
Dr.-Ing. Marco Münchhof Dr.-Ing. Frank Uhlemann
Vorsitzender des Aufsichtsrats: Hubertus G. Krossa
Stv. Vorsitzender des Aufsichtsrats: Dr.-Ing. Gerd Eckelmann
Sitz der Gesellschaft: Berliner Str. 161, 65205 Wiesbaden, Amtsgericht Wiesbaden HRB 12636
http://www.eckelmann.de

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imd: model and compatible missing in metadata

[Sascha Hauer]

On Thu, Dec 13, 2018 at 07:33:56AM +0100, Thorsten K. Scherer wrote:
> fixes 97e81f2
> 
> Signed-off-by: Thorsten K. Scherer <t.scherer@eckelmann.de>
> ---
>  include/asm-generic/barebox.lds.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/asm-generic/barebox.lds.h b/include/asm-generic/barebox.lds.h
> index 74d3ca4a9..decaab79a 100644
> --- a/include/asm-generic/barebox.lds.h
> +++ b/include/asm-generic/barebox.lds.h
> @@ -66,7 +66,7 @@
>  #define BAREBOX_IMD				\
>  	KEEP(*(.barebox_imd_start))		\
>  	KEEP(*(.barebox_imd_1*))		\
> -	*(.barebox_imd_0*)			\
> +	KEEP(*(.barebox_imd_0*))		\

Applied, thanks. I wonder why I didn't add that in the first place,
given that all other lines have the KEEP().

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imd: model and compatible missing in metadata

[Antony Pavlov]

On Thu, 13 Dec 2018 07:33:56 +0100
"Thorsten K. Scherer" <t.scherer@eckelmann.de> wrote:

Hi!


> fixes 97e81f2
> 
> Signed-off-by: Thorsten K. Scherer <t.scherer@eckelmann.de>
> ---
>  include/asm-generic/barebox.lds.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/asm-generic/barebox.lds.h b/include/asm-generic/barebox.lds.h
> index 74d3ca4a9..decaab79a 100644
> --- a/include/asm-generic/barebox.lds.h
> +++ b/include/asm-generic/barebox.lds.h
> @@ -66,7 +66,7 @@
>  #define BAREBOX_IMD				\
>  	KEEP(*(.barebox_imd_start))		\
>  	KEEP(*(.barebox_imd_1*))		\
> -	*(.barebox_imd_0*)			\
> +	KEEP(*(.barebox_imd_0*))		\
>  	KEEP(*(.barebox_imd_end))
>  
>  #if defined(CONFIG_ARCH_BAREBOX_MAX_BARE_INIT_SIZE) && \
> -- 
> 2.19.1

It looks like this patch has unobvious drawback.
The bareboximd utility sees too many 'model' and 'compatible' strings in a single image, e.g.:

  $ git clone -b next git://git.pengutronix.de/git/barebox.git
  $ cd barebox
  barebox$ git log -1 --oneline
  5407887ff (HEAD -> next, origin/next) Merge branch 'for-next/mtd' into next
  barebox$ export ARCH=arm
  barebox$ export CROSS_COMPILE=arm-linux-gnueabi-
  barebox$ make tegra_v7_defconfig
  barebox$ echo "CONFIG_IMD=y" >> .config
  barebox$ echo "CONFIG_IMD_TARGET=y" >> .config
  barebox$ make oldconfig
  barebox$ make
  barebox$ ./scripts/bareboximd images/barebox-tegra20-toshiba-ac100-usbloader.img
  build: #1 Mon Dec 17 13:59:41 MSK 2018
  release: 2018.12.0-00273-g5407887ff
  of_compatible: nvidia,jetson-tk1 nvidia,tegra124
  model: NVIDIA Tegra124 Jetson TK1
  of_compatible: toradex,iris toradex,colibri_t20 nvidia,tegra20
  model: Toradex Colibri T20 on Iris
  of_compatible: compal,paz00 nvidia,tegra20
  model: Toshiba AC100 / Dynabook AZ
  of_compatible: nvidia,beaver nvidia,tegra30
  model: NVIDIA Tegra30 Beaver evaluation board

-- 
Best regards,
  Antony Pavlov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imd: model and compatible missing in metadata

[Sascha Hauer]

On Mon, Dec 17, 2018 at 02:18:52PM +0300, Antony Pavlov wrote:
> On Thu, 13 Dec 2018 07:33:56 +0100
> "Thorsten K. Scherer" <t.scherer@eckelmann.de> wrote:
> 
> Hi!
> 
> 
> > fixes 97e81f2
> > 
> > Signed-off-by: Thorsten K. Scherer <t.scherer@eckelmann.de>
> > ---
> >  include/asm-generic/barebox.lds.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/include/asm-generic/barebox.lds.h b/include/asm-generic/barebox.lds.h
> > index 74d3ca4a9..decaab79a 100644
> > --- a/include/asm-generic/barebox.lds.h
> > +++ b/include/asm-generic/barebox.lds.h
> > @@ -66,7 +66,7 @@
> >  #define BAREBOX_IMD				\
> >  	KEEP(*(.barebox_imd_start))		\
> >  	KEEP(*(.barebox_imd_1*))		\
> > -	*(.barebox_imd_0*)			\
> > +	KEEP(*(.barebox_imd_0*))		\
> >  	KEEP(*(.barebox_imd_end))
> >  
> >  #if defined(CONFIG_ARCH_BAREBOX_MAX_BARE_INIT_SIZE) && \
> > -- 
> > 2.19.1
> 
> It looks like this patch has unobvious drawback.
> The bareboximd utility sees too many 'model' and 'compatible' strings in a single image, e.g.:

Now I remember what the intention was that this line looks so apparently
different from the other ones ;)

To get the compatible strings into the binary I have created imd_used().
This should be called with the symbol that should be kept in the
resulting image.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imd: model and compatible missing in metadata

[Scherer, Thorsten]

Hallo Sascha,

On Mon, 2018-12-17 at 12:31 +0100, Sascha Hauer wrote:
> On Mon, Dec 17, 2018 at 02:18:52PM +0300, Antony Pavlov wrote:
> > On Thu, 13 Dec 2018 07:33:56 +0100
> > "Thorsten K. Scherer" <t.scherer@eckelmann.de> wrote:
> > 
> > Hi!
> > 
> > 
> > > fixes 97e81f2
> > > 
> > > Signed-off-by: Thorsten K. Scherer <t.scherer@eckelmann.de>
> > > ---
> > >  include/asm-generic/barebox.lds.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/include/asm-generic/barebox.lds.h b/include/asm-
> > > generic/barebox.lds.h
> > > index 74d3ca4a9..decaab79a 100644
> > > --- a/include/asm-generic/barebox.lds.h
> > > +++ b/include/asm-generic/barebox.lds.h
> > > @@ -66,7 +66,7 @@
> > >  #define BAREBOX_IMD				\
> > >  	KEEP(*(.barebox_imd_start))		\
> > >  	KEEP(*(.barebox_imd_1*))		\
> > > -	*(.barebox_imd_0*)			\
> > > +	KEEP(*(.barebox_imd_0*))		\
> > >  	KEEP(*(.barebox_imd_end))
> > >  
> > >  #if defined(CONFIG_ARCH_BAREBOX_MAX_BARE_INIT_SIZE) && \
> > > -- 
> > > 2.19.1
> > 
> > It looks like this patch has unobvious drawback.
> > The bareboximd utility sees too many 'model' and 'compatible'
> > strings in a single image, e.g.:
> 
> Now I remember what the intention was that this line looks so
> apparently
> different from the other ones ;)
> 
> To get the compatible strings into the binary I have created
> imd_used().
> This should be called with the symbol that should be kept in the
> resulting image.
> 
> Sascha
> 

sorry for introducing confusion.   I misread the documenation on IMD as
"of_compatible and model will be figured out and put into the
proper  location by barebox".  What's your opinion on adding some
documentation for "imd_used" in (user/imd.rst)?  So I may try to
produce a meaningful  a patch for that.

Or is this out of scope (at least in this context)?


Best regards,
Thorsten
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imd: model and compatible missing in metadata

[s.hauer]

On Thu, Dec 20, 2018 at 12:54:08PM +0000, Scherer, Thorsten wrote:
> Hallo Sascha,
> 
> On Mon, 2018-12-17 at 12:31 +0100, Sascha Hauer wrote:
> > On Mon, Dec 17, 2018 at 02:18:52PM +0300, Antony Pavlov wrote:
> > > On Thu, 13 Dec 2018 07:33:56 +0100
> > > "Thorsten K. Scherer" <t.scherer@eckelmann.de> wrote:
> > > 
> > > Hi!
> > > 
> > > 
> > > > fixes 97e81f2
> > > > 
> > > > Signed-off-by: Thorsten K. Scherer <t.scherer@eckelmann.de>
> > > > ---
> > > >  include/asm-generic/barebox.lds.h | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/include/asm-generic/barebox.lds.h b/include/asm-
> > > > generic/barebox.lds.h
> > > > index 74d3ca4a9..decaab79a 100644
> > > > --- a/include/asm-generic/barebox.lds.h
> > > > +++ b/include/asm-generic/barebox.lds.h
> > > > @@ -66,7 +66,7 @@
> > > >  #define BAREBOX_IMD				\
> > > >  	KEEP(*(.barebox_imd_start))		\
> > > >  	KEEP(*(.barebox_imd_1*))		\
> > > > -	*(.barebox_imd_0*)			\
> > > > +	KEEP(*(.barebox_imd_0*))		\
> > > >  	KEEP(*(.barebox_imd_end))
> > > >  
> > > >  #if defined(CONFIG_ARCH_BAREBOX_MAX_BARE_INIT_SIZE) && \
> > > > -- 
> > > > 2.19.1
> > > 
> > > It looks like this patch has unobvious drawback.
> > > The bareboximd utility sees too many 'model' and 'compatible'
> > > strings in a single image, e.g.:
> > 
> > Now I remember what the intention was that this line looks so
> > apparently
> > different from the other ones ;)
> > 
> > To get the compatible strings into the binary I have created
> > imd_used().
> > This should be called with the symbol that should be kept in the
> > resulting image.
> > 
> > Sascha
> > 
> 
> sorry for introducing confusion.   I misread the documenation on IMD as
> "of_compatible and model will be figured out and put into the
> proper  location by barebox".  What's your opinion on adding some
> documentation for "imd_used" in (user/imd.rst)?  So I may try to
> produce a meaningful  a patch for that.

Sure, that would be appreciated.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imd: model and compatible missing in metadata

[Scherer, Thorsten]

Hello,

i spent some time trying to understand the usage of imd_used etc. 

In another reply I stated:

'I misread the documenation on IMD as "of_compatible and model will be
figured out and put into the proper location by barebox".'

I am not so sure, if that is really the case.


As far as i understand things:

1. 	of_compatible and model are (should be) automagically added to imd from the
	entries in the dts.

git show 97e81f2d7 | grep -A 1 'For.*model.*of_compatible'
cat scripts/gen-dtb-s | grep -A 23 -B 3 'barebox_imd_0'

2.	the IMD_USED macro puts arbitrary tags in the metadata.  Which may
	also be a tag named model.


Now I wonder, if the lack of the patch just covers up an issue at another
place or if there is something (in the context of multi image support?) which
I do not understand.  Maybe there is something messed up with the entries in
the device tree?

https://www.spinics.net/lists/arm-kernel/msg699453.html

Maybe someone can point me in the right direction?

Best regards,
Thorsten

On Mon, Dec 17, 2018 at 12:31:02PM +0100, Sascha Hauer wrote:
> On Mon, Dec 17, 2018 at 02:18:52PM +0300, Antony Pavlov wrote:
> > On Thu, 13 Dec 2018 07:33:56 +0100
> > "Thorsten K. Scherer" <t.scherer@eckelmann.de> wrote:
> > 
> > Hi!
> > 
> > 
> > > fixes 97e81f2
> > > 
> > > Signed-off-by: Thorsten K. Scherer <t.scherer@eckelmann.de>
> > > ---
> > >  include/asm-generic/barebox.lds.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/include/asm-generic/barebox.lds.h b/include/asm-generic/barebox.lds.h
> > > index 74d3ca4a9..decaab79a 100644
> > > --- a/include/asm-generic/barebox.lds.h
> > > +++ b/include/asm-generic/barebox.lds.h
> > > @@ -66,7 +66,7 @@
> > >  #define BAREBOX_IMD				\
> > >  	KEEP(*(.barebox_imd_start))		\
> > >  	KEEP(*(.barebox_imd_1*))		\
> > > -	*(.barebox_imd_0*)			\
> > > +	KEEP(*(.barebox_imd_0*))		\
> > >  	KEEP(*(.barebox_imd_end))
> > >  
> > >  #if defined(CONFIG_ARCH_BAREBOX_MAX_BARE_INIT_SIZE) && \
> > > -- 
> > > 2.19.1
> > 
> > It looks like this patch has unobvious drawback.
> > The bareboximd utility sees too many 'model' and 'compatible' strings in a single image, e.g.:
> 
> Now I remember what the intention was that this line looks so apparently
> different from the other ones ;)
> 
> To get the compatible strings into the binary I have created imd_used().
> This should be called with the symbol that should be kept in the
> resulting image.
> 
> Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

-- 
Mit freundlichen Grüßen
Thorsten K. Scherer
Diplom-Ingenieur (FH)
Kälte- und Gebäudeleittechnik
Datentechnik


Eckelmann AG
Berliner Str. 161, 65205 Wiesbaden
Telefon +49 611 7103-329     
Fax +49 611 7103-133
thorsten.scherer@eckelmann.de
https://www.eckelmann.de

Eckelmann Group - Source of inspiration

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imd: model and compatible missing in metadata

[Uwe Kleine-König]

Hello,

On Tue, Jan 22, 2019 at 09:10:40AM +0100, Scherer, Thorsten wrote:
> i spent some time trying to understand the usage of imd_used etc. 
> 
> In another reply I stated:
> 
> 'I misread the documenation on IMD as "of_compatible and model will be
> figured out and put into the proper location by barebox".'

That is wrong. The current code doesn't ensure that and I don't see a
way how this could be done comfortably without per-board changes.
 
> I am not so sure, if that is really the case.
> 
> As far as i understand things:
> 
> 1. 	of_compatible and model are (should be) automagically added to imd from the
> 	entries in the dts.
> 
> git show 97e81f2d7 | grep -A 1 'For.*model.*of_compatible'
> cat scripts/gen-dtb-s | grep -A 23 -B 3 'barebox_imd_0'

The imd tags for compatible and model are generated, but not
automatically included with this change. This is AFAICT the current
state.

The problem is that the needed information is included in the "barebox"
image (i.e. the file with the name "barebox" that is build in the top
build dir) but then there is an additional step that extracts "all
needed" parts from that image to create a binary that then is able to be
started on your target. So if "barebox" has three entry points for
different machines, there are three images generated and each only has a
single entry point. Unused functions and data structures are stripped.
The linker detects that the imd section that contains the compatible and
model is not used and so this goes away in all three generated binaries.

> 2.	the IMD_USED macro puts arbitrary tags in the metadata.  Which may
> 	also be a tag named model.

The IMD_USED claims a certain data structure is used such that the
linker doesn't throw it away.
 
> Now I wonder, if the lack of the patch just covers up an issue at another
> place or if there is something (in the context of multi image support?) which
> I do not understand. 

Did you look at the patch I sent you?

> Maybe there is something messed up with the entries in
> the device tree?
> 
> https://www.spinics.net/lists/arm-kernel/msg699453.html

This is unrelated, this is a patch for Linux to add a device tree.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imd: model and compatible missing in metadata

[Scherer, Thorsten]

Hello,

On Tue, Jan 22, 2019 at 09:53:37AM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> On Tue, Jan 22, 2019 at 09:10:40AM +0100, Scherer, Thorsten wrote:
> > i spent some time trying to understand the usage of imd_used etc. 
> > 
> > In another reply I stated:
> > 
> > 'I misread the documenation on IMD as "of_compatible and model will be
> > figured out and put into the proper location by barebox".'
> 
> That is wrong. The current code doesn't ensure that and I don't see a
> way how this could be done comfortably without per-board changes.
>  
> > I am not so sure, if that is really the case.
> > 
> > As far as i understand things:
> > 
> > 1. 	of_compatible and model are (should be) automagically added to imd from the
> > 	entries in the dts.
> > 
> > git show 97e81f2d7 | grep -A 1 'For.*model.*of_compatible'
> > cat scripts/gen-dtb-s | grep -A 23 -B 3 'barebox_imd_0'
> 
> The imd tags for compatible and model are generated, but not
> automatically included with this change. This is AFAICT the current
> state.
> 
> The problem is that the needed information is included in the "barebox"
> image (i.e. the file with the name "barebox" that is build in the top
> build dir) but then there is an additional step that extracts "all
> needed" parts from that image to create a binary that then is able to be
> started on your target. So if "barebox" has three entry points for
> different machines, there are three images generated and each only has a
> single entry point. Unused functions and data structures are stripped.
> The linker detects that the imd section that contains the compatible and
> model is not used and so this goes away in all three generated binaries.
> 

Thank you for the explanation.  Now things start to clear up a bit.  

> > 2.	the IMD_USED macro puts arbitrary tags in the metadata.  Which may
> > 	also be a tag named model.
> 
> The IMD_USED claims a certain data structure is used such that the
> linker doesn't throw it away.
>  

Ok.

> > Now I wonder, if the lack of the patch just covers up an issue at another
> > place or if there is something (in the context of multi image support?) which
> > I do not understand. 
> 
> Did you look at the patch I sent you?
> 

I did.  But not closely enough.  On the invokation of imd, model and
of_compatible were not displayed.  Somehow I did not get, that your
patch was made to apply for the imx25 boards and not for the imx6
board (the one which I was checking).  After adjusting the lowlevel.c
file of the imx6 board, everything went fine.

> > Maybe there is something messed up with the entries in
> > the device tree?
> > 
> > https://www.spinics.net/lists/arm-kernel/msg699453.html
> 
> This is unrelated, this is a patch for Linux to add a device tree.

Right.  It didn't make sense to add this.

> 
> Best regards
> Uwe
> 

Again, thank you for feedback.

Best regards
Thorsten

> -- 
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |

-- 
Mit freundlichen Grüßen
Thorsten K. Scherer
Diplom-Ingenieur (FH)
Kälte- und Gebäudeleittechnik
Datentechnik


Eckelmann AG
Berliner Str. 161, 65205 Wiesbaden
Telefon +49 611 7103-329     
Fax +49 611 7103-133
thorsten.scherer@eckelmann.de
https://www.eckelmann.de

Eckelmann Group - Source of inspiration

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox