[coreboot] [PATCH] superiotool: libsuperiodetect

Stefan Reinauer stefan.reinauer at coresystems.de
Wed Jun 30 14:51:43 CEST 2010


On 6/30/10 2:35 PM, Carl-Daniel Hailfinger wrote:
> Using the superiotool code to detect SuperI/O chips can be really useful
> even for applications besides superiotool, e.g. flashrom.
> The biggest hurdle right now is the excessive size of the superiotool
> binary (>2.2 MB) which is still ~15 kB after lzma compression. That's
> simply unacceptable for a library.
>   
Why? There is no size constraints on libraries in Unix like systems.

Assuming we're talking about a static library
>From my system:

20M /usr/lib64/libbfd.a
20M /usr/lib64/libc.a
11M /usr/lib64/libcrypto.a
19M /usr/lib64/libdb-4.5.a
3.9M /usr/lib64/libfreetype.a
5.1M /usr/lib64/libgio-2.0.a
3.6M /usr/lib64/libglib-2.0.a
3.3M /usr/lib64/libgmp.a
2.8M /usr/lib64/libm.a
2.8M /usr/lib64/libncurses.a
3.2M /usr/lib64/libncursesw.a
4.9M /usr/lib64/libopcodes.a
4.2M /usr/lib64/libruby-static.a
3.0M /usr/lib64/libssl.a
6.9M /usr/lib64/libxml2.a

Also, keep in mind that the size of the library in the filesystem is not
the size increase when linking it.

15kb lzma compressed is nothing.
> This minimally invasive patch makes all dumping code optional and
> reduces binary size for the probe-only case (libsuperiodetect) a lot.
> Binary sizes:
> 2268258 (before)
>   31600 (after) 98.6% reduction
> LZMA compressed and stripped binary sizes:
> 14778 (before)
>  6709 (after) 54.6% reduction
>   
Not worth the additional unreadability of the code in my opinion

> To test compilation of superiotool with probe-only functionality, run
> make CONFIG_LIB=yes
>
> This patch is the first step towards libsuperiodetect, and more will
> have to follow.
>   
I think the idea for libsuperio is great.

I'll gladly ack this without the very intrusive "we save 6700 bytes"
part of the patch.

> IMHO the {EOT} markers should be killed in libsuperiodetect, at least
> for the LDNs. Such an operation would have increased the size of the
> patch and would have reduced readability as well. Due to that, I skipped
> it, but I plan to do that in a later operation.
>   
What would you use instead? The array members all have different sizes
as far as I can tell, so some kind of end marker will be needed.

> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>   

>  		/* Init: 0x55, 0x55. Exit: 0xaa. Port: 0x3f0. */
> @@ -765,7 +803,9 @@
>  				     uint8_t revreg)
>  {
>  	uint8_t id, rev;
> +#ifndef LIBSUPERIODETECT
>  	uint16_t runtime_base;
> +#endif /* ! LIBSUPERIODETECT */
>   
Maybe this could be moved downwards to where it is used, to save an ifndef?

> Index: superiotool_libsuperiodetect/superiotool.h
> ===================================================================
> --- superiotool_libsuperiodetect/superiotool.h	(Revision 5651)
> +++ superiotool_libsuperiodetect/superiotool.h	(Arbeitskopie)
> @@ -98,12 +98,19 @@
>  struct superio_registers {
>  	int32_t superio_id;		/* Signed, as we need EOT. */
>  	const char *name;		/* Super I/O name */
> +#ifndef LIBSUPERIODETECT
>  	struct {
>  		int8_t ldn;
>  		const char *name;	/* LDN name */
>  		int16_t idx[IDXSIZE];
>  		int16_t def[IDXSIZE];
>  	} ldn[LDNSIZE];
> +#else /* LIBSUPERIODETECT */
> +	/* Ugly hack to avoid messing with EOT markers. */
> +	struct {
> +		int8_t dummy;
> +	} dummy[1];
> +#endif /* LIBSUPERIODETECT */
>  }; 
>   
hm ... the comment is very terse.



-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.dehttp://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866





More information about the coreboot mailing list