From: Greg Ungerer <gerg@snapgear.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-kernel@vger.kernel.org, gerg@uclinux.org,
linux-m68k@vger.kernel.org
Subject: Re: [PATCH] m68k: merge the mmu and non-mmu versions of checksum.h
Date: Fri, 19 Jun 2009 16:54:43 +1000 [thread overview]
Message-ID: <4A3B3633.3000507@snapgear.com> (raw)
In-Reply-To: <20090618194521.GA7464@infradead.org>
Hi Christoph,
Christoph Hellwig wrote:
> On Wed, Jun 17, 2009 at 05:11:15PM +1000, Greg Ungerer wrote:
>> +#ifdef CONFIG_MMU
>> /*
>> * This is a version of ip_compute_csum() optimized for IP headers,
>> * which always checksum on 4 octet boundaries.
>> @@ -59,6 +61,9 @@ static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
>> : "memory");
>> return (__force __sum16)~sum;
>> }
>> +#else
>> +__sum16 ip_fast_csum(const void *iph, unsigned int ihl);
>> +#endif
>
> Any good reason this is inline for all mmu processors and out of line
> for nommu, independent of the actual cpu variant?
I don't recall of the simple (and thus non-mmu) m68k variants
support all the instructions used in this optimized version.
I will check that. It might be that this is mis-placed and
is actually conditional on the CPU type.
The C code version is significantly bigger, I think that is why
it was not inlined here (see arch/m68knommu/lib/checksum.c)
>> static inline __sum16 csum_fold(__wsum sum)
>> {
>> unsigned int tmp = (__force u32)sum;
>> +#ifdef CONFIG_COLDFIRE
>> + tmp = (tmp & 0xffff) + (tmp >> 16);
>> + tmp = (tmp & 0xffff) + (tmp >> 16);
>> + return (__force __sum16)~tmp;
>> +#else
>> __asm__("swap %1\n\t"
>> "addw %1, %0\n\t"
>> "clrw %1\n\t"
>> @@ -74,6 +84,7 @@ static inline __sum16 csum_fold(__wsum sum)
>> : "=&d" (sum), "=&d" (tmp)
>> : "0" (sum), "1" (tmp));
>> return (__force __sum16)~sum;
>> +#endif
>> }
>
> I think this would be cleaner by having totally separate functions
> for both cases, e.g.
>
> #ifdef CONFIG_COLDFIRE
> static inline __sum16 csum_fold(__wsum sum)
> {
> unsigned int tmp = (__force u32)sum;
>
> tmp = (tmp & 0xffff) + (tmp >> 16);
> tmp = (tmp & 0xffff) + (tmp >> 16);
>
> return (__force __sum16)~tmp;
> }
> #else
> ...
> #endif
Ok, I will change that.
Thanks
Greg
------------------------------------------------------------------------
Greg Ungerer -- Principal Engineer EMAIL: gerg@snapgear.com
SnapGear Group, McAfee PHONE: +61 7 3435 2888
825 Stanley St, FAX: +61 7 3891 3630
Woolloongabba, QLD, 4102, Australia WEB: http://www.SnapGear.com
prev parent reply other threads:[~2009-06-19 6:54 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-06-17 7:11 [PATCH] m68k: merge the mmu and non-mmu versions of checksum.h Greg Ungerer
2009-06-18 19:45 ` Christoph Hellwig
2009-06-19 6:54 ` Greg Ungerer [this message]
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=4A3B3633.3000507@snapgear.com \
--to=gerg@snapgear.com \
--cc=gerg@uclinux.org \
--cc=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-m68k@vger.kernel.org \
/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