public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Wolfgang Denk <wd@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] crc32: Correct endianness of crc32 result
Date: Thu, 18 Apr 2013 16:39:09 +0200	[thread overview]
Message-ID: <20130418143909.192F720019A@gemini.denx.de> (raw)
In-Reply-To: <20130418131810.38c916b8@lilith>

Dear Albert,

In message <20130418131810.38c916b8@lilith> you wrote:
> 
> > OK - and what about the other architectures that suffer from the same
> > issues?
> 
> They should/could provide their own optimized versions, obviously.

Well, if this was a generally needed or even useful service, that
might make sense.  But it is highly exotic stuff...

> > I really dislike introducing such custom functions when we have
> > standard functions available that implement the same purposes.
> 
> I understand the point; this is a classical opposition of generic vs
> optimized.

Not really.  It is more opposition against premature optimization at
the cost of non-standard code.

Is there any justification that we really need hightly optimized
versus generic code here?  Is this used anywhere in a place where
performance is critical?

> > Can we not stick to standard functions?  Instead of htobe32() we
> > should use htonl() which is available both in U-Boot and in the host
> > environment.
> 
> Note that there are two problems here: endianness conversion and
> unaligned storage. We can indeed use htoln() instead of htobe32(), but
> that only affects/solved endianness issues, not alignment ones, for
> which there is no solution that is efficient across all ARM targets.

Correct.  But the htobe32() approach was cobining this with the
generic memcpy(), too, so we would do the same with htoln().

> So how about changing the element type of output in the definition of
> hash_func_ws, adapting the corresponding implementations sha1_csum_wd,
> sha256_csum_wd and crc32_wd_buf, and adapting the output argument
> of the sole call to hash_func_ws, that is, the local "u8
> output[HASH_MAX_DIGEST_SIZE];" in hash.c? Then we'be done with
> alignment.

OK, but that would be a totally different approach (which appears to
be a good one, here).

> Note finally that we *need* unaligned access macros/inline
> functions/whatever, if only for exceptions laid out in
> doc/README.arm-unaligned-accesses. If we avoid calling them too often,
> we can live with generic.

Agreed; the focus should always be on avoidance, though.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
    \|/ ____ \|/                                     \|/ ____ \|/
     @~/ ,. \~@                                       @~/ ,. \~@
    /_( \__/ )_\                                     /_( \__/ )_\
       \__U_/                                           \__U_/

  reply	other threads:[~2013-04-18 14:39 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-05 23:11 [U-Boot] [PATCH] crc32: Correct endianness of crc32 result Simon Glass
2013-04-05 23:32 ` Allen Martin
2013-04-06  7:04 ` Wolfgang Denk
2013-04-16 21:57   ` Simon Glass
2013-04-16 23:00     ` Tom Rini
2013-04-16 23:16       ` Simon Glass
2013-04-17  5:40     ` Wolfgang Denk
2013-04-17 18:28       ` Simon Glass
2013-04-17 19:23         ` Albert ARIBAUD
2013-04-17 20:59           ` Simon Glass
2013-04-18  6:20             ` Albert ARIBAUD
2013-04-18 10:36               ` Wolfgang Denk
2013-04-18 11:18                 ` Albert ARIBAUD
2013-04-18 14:39                   ` Wolfgang Denk [this message]
2013-04-18 16:39                     ` Albert ARIBAUD
2013-04-18 16:58                       ` Tom Rini
2013-04-18 18:43                         ` Albert ARIBAUD
2013-04-18 19:06                           ` Simon Glass
2013-04-18 19:18                             ` Tom Rini

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=20130418143909.192F720019A@gemini.denx.de \
    --to=wd@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