Messages in this thread | | | From | Arnd Bergmann <> | Subject | Re: [PATCH 05/17] Blackfin: convert to generic checksum code | Date | Wed, 24 Jun 2009 00:06:09 +0200 |
| |
On Tuesday 23 June 2009, Mike Frysinger wrote: > and this is an internal tester ... if we're going to let arches > override it, then its function signature are pretty much set in stone. > i agree we should test every checksum function, but i think only > testing do_csum indirectly would be detrimental to the people who > would want to use this -- arch maintainers looking to implement > do_csum() themselves. otherwise they need to step through the > surrounding functions a bit to find the exact values given to > do_csum() and the exact value expected back such that the calling > function still works. and every arch guy is going to do this same > thing.
It depends on how we want to use it. If it's only for testing architecture that have moved to the generic checksum code, that's fine. I was thinking we could also use it for architectures that want to keep their own code, but we don't have to.
> i'm a bit of a nub here wrt checksuming. the test module i came up > with by simply flooding my board and copying & pasting mismatched > buffers ;). are you saying the following should be OK ? > - if (tret != do_csum_data[i].ret) { > + if (tret != le16_to_cpu(do_csum_data[i].ret)) {
yes, that should work.
> unless there is a macro i could use that'd do the expansion at CPP > time so i'd write the test data as: > static struct do_csum_data __initdata do_csum_data[] = { > DO_CSUM_DATA(1, le16_to_cpu(0x0020)), > ...
Ah, right that won't. I tested only on x86 (little-endian), so le16_to_cpu() is an identity function that can be evaluated for constants, but on big-endian that would break.
Arnd <><
| |