MPC8349ea Random Device Generator driver

[Olof Johansson]

On Wed, Jun 06, 2007 at 05:07:05PM -0500, Timur Tabi wrote:
> Olof Johansson wrote:
> 
> >There's nothing wrong with the way he coded that up. Lots of drivers
> >are written that way (all of mine are). It's at least as clear as any
> >structure, and it doesn't cause temptation to do...
> 
> That vast majority of Freescale SOC device register maps are handled via a 
> structure. He's doing everything via 32-bit operations, even though the 
> registers are 64 bits, and therefore he has twice as much macros as he 
> needs.

I see only one double access back to back, not a big deal. I do however
fail to see how a couple of constructs are working, separate email with
those questions later though.

> >>>+
> >>>+
> >>>+	/* check for things like FIFO underflow */
> >>>+	
> >>>+	u32 v;
> >>>+
> >>>+	v = in_be32(rng_regs + TALITOS_RNGISR_HI);
> >>	u64 v;
> >>	v = rng->rngisr;
> >>
> >>or something like that.  Try to use the built-in support for 64-bit data 
> >>types when possible.
> >
> >...this. NO! Don't reference ioremapped memory from regular code like
> >that. The way he's doing it is the preferred way.
> 
> Can you explain that better?  What is "regular code"?

>From http://kerneltrap.org/node/3848:

---
In the byte-order case, what we have is:

	typedef __u16 __bitwise __le16;
	typedef __u16 __bitwise __be16;
	typedef __u32 __bitwise __le32;
	typedef __u32 __bitwise __be32;
	typedef __u64 __bitwise __le64;
	typedef __u64 __bitwise __be64;

and if you think about the above rules about what is acceptable for 
bitwise types, you'll likely immediately notivce that it automatically 
means

 - you can never assign a __le16 type to any other integer type or any 
   other bitwise type. You'd get a warnign about incompatible types. Makes 
   sense, no?
...
---

Note that rngisr is __be64.  Sparse should complain loudly over your code.


-Olof