public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Detlev Zundel <dzu@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] display_buffer: fix misaligned buffer
Date: Wed, 01 Sep 2010 17:01:51 +0200	[thread overview]
Message-ID: <m262ypecdc.fsf@ohwell.denx.de> (raw)
In-Reply-To: <4C7C9B85.6080202@emk-elektronik.de> (Reinhard Meyer's message of "Tue, 31 Aug 2010 08:04:53 +0200")

Hi Reinhard,

> making the change to the union, I also realized that
>
> /* Copy from memory into linebuf and print hex values */
> for (i = 0; i < linelen; i++) {
> 	uint32_t x;
> 	if (width == 4)
> 		x = lb.u32[i] = *(volatile uint32_t *)data;
> 	else if (width == 2)
> 		x = lb.u16[i] = *(volatile uint16_t *)data;
> 	else
> 		x = lb.u8[i] = *(volatile uint8_t *)data;
> 	printf(" %0*x", width * 2, x);
> 	data += width;
> }
>
> is still a bit "ugly". What about:
>
> union data {
> 	u_int32_t *u32;
> 	u_int16_t *u16;
> 	u_int8_t *u8;
> 	void *v;
> } dp;
> dp.v = data;
>
> then:
>
> /* Copy from memory into linebuf and print hex values */
> for (i = 0; i < linelen; i++) {
> 	if (width == 4)
> 		x = lb.u32[i] = *(dp.u32)++;
> 	else if (width == 2)
> 		x = lb.u16[i] = *(dp.u16)++;
> 	else
> 		x = lb.u8[i] = *(dp.u8)++;
> 	printf(" %0*x", width * 2, x);
> }

If this works then I have to admit, I like what I see :)

> optionally calling printf inside the if:
>
> /* Copy from memory into linebuf and print hex values */
> for (i = 0; i < linelen; i++) {
> 	if (width == 4)
> 		printf(" %08x", lb.u32[i] = *(dp.u32)++);
> 	else if (width == 2)
> 		printf(" %04x", lb.u16[i] = *(dp.u16)++);
> 	else
> 		printf(" %02x", lb.u8[i] = *(dp.u8)++);
> }

Yes, this may speedup the printf also (ok, not much, but hey).

> maybe it would even be more effective to swap for and if:
>
> /* Copy from memory into linebuf and print hex values */
> if (width == 4) {
> 	for (i = 0; i < linelen; i++)
> 		printf(" %08x", lb.u32[i] = *(dp.u32)++);
> } else if (width == 2) {
> 	for (i = 0; i < linelen; i++)
> 		printf(" %04x", lb.u16[i] = *(dp.u16)++);
> } else {
> 	for (i = 0; i < linelen; i++)
> 		printf(" %02x", lb.u8[i] = *(dp.u8)++);
> }

No, please don't, but why not do a switch on width?

> Of course, all is purely cosmetic ;)

Well this time my signature is in order :)

Cheers
  Detlev

-- 
The  mathematician's patterns,  like the  painter's or  the poet's,  must be
beautiful;  the ideas, like the colours or the words, must fit together in a
harmonious way. Beauty is the first test: there is no permanent place in the
world for ugly mathematics.                       -- G. H. Hardy
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

  reply	other threads:[~2010-09-01 15:01 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-27 20:23 [U-Boot] [PATCH] display_buffer: fix misaligned buffer Reinhard Meyer
2010-08-30  8:59 ` Detlev Zundel
2010-08-30  9:22   ` Reinhard Meyer
2010-08-30  9:39     ` Reinhard Meyer
2010-08-30 10:02       ` Detlev Zundel
2010-08-30 10:31         ` Stefano Babic
2010-08-30 10:46           ` Albert ARIBAUD
2010-08-30 11:04             ` Reinhard Meyer
2010-08-30 11:05           ` Detlev Zundel
2010-08-30 13:37             ` Reinhard Meyer
2010-08-30 16:47               ` Detlev Zundel
2010-08-30 18:03                 ` Albert ARIBAUD
2010-08-30 18:25                   ` Reinhard Meyer
2010-08-30 22:32                     ` Detlev Zundel
2010-08-30 22:29                   ` Detlev Zundel
2010-08-31  5:38                     ` Albert ARIBAUD
2010-08-31  6:04                       ` Reinhard Meyer
2010-09-01 15:01                         ` Detlev Zundel [this message]
2010-09-02  7:39                         ` Wolfgang Denk
2010-09-02 17:42                         ` Mike Frysinger
2010-09-07 23:23       ` Wolfgang Denk
2010-08-30  9:49     ` Detlev Zundel
2010-09-07 23:22     ` Wolfgang Denk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m262ypecdc.fsf@ohwell.denx.de \
    --to=dzu@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox