Messages in this thread |  | | | Subject | Re: [PATCH 1 of 3] Introduce __memcpy_toio32 | | From | Bryan O'Sullivan <> | | Date | Wed, 28 Dec 2005 06:47:42 -0800 |
| |
On Tue, 2005-12-27 at 21:52 -0600, Matt Mackall wrote: > On Tue, Dec 27, 2005 at 03:41:55PM -0800, Bryan O'Sullivan wrote: > > +/* > > + * MMIO copy routines. These are guaranteed to operate in units denoted > > + * by their names. This style of operation is required by some devices. > > + */ > > Using kdoc style for new code is nice.
OK, will do.
> > +extern void fastcall __memcpy_toio32(volatile void __iomem *to, const void *from, size_t count); > > + > > Minor rant: extern is always redundant for function prototypes in C.
I know. My intent was to keep the prototype consistent with the prevailing style of other declarations in that same routine. If you think that cleanliness is more important, I'll be happy to change it.
> Suspicious use of volatile - writel is doing the actual write, this > function never does a dereference.
Yeah. I lost the plot there a bit. I'll remove the volatiles.
> As you've already got private > copies of the pointers already in s and d, it's perfectily reasonable > and idiomatic to do: > > while (--count >= 0) > __raw_writel(*s++, d++);
But pointer arithmetic is undefined on void pointers. gcc lets you do it, but it treats sizeof(void) as 1, which gives entirely the wrong results in this case.
> And as you appear to be using the __raw.. version to avoid repeated > mb()s, you probably ought to tack one on at the end.
Well spotted, thanks.
<b
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |