From: olof@lixom•net (Olof Johansson)
To: Timur Tabi <timur@freescale•com>
Cc: linuxppc-dev@ozlabs•org, sl@powerlinux•fr,
Philippe Lachenal <philippe.lachenal@hotmail•fr>
Subject: Re: MPC8349ea Random Device Generator driver
Date: Wed, 6 Jun 2007 17:09:13 -0500 [thread overview]
Message-ID: <20070606220913.GA27820@lixom.net> (raw)
In-Reply-To: <46672DB4.80504@freescale.com>
On Wed, Jun 06, 2007 at 04:57:08PM -0500, Timur Tabi wrote:
> > +#include <asm/of_platform.h>
> > +#include <asm/io.h>
> > +
> > +#define TALITOS_RNGSR 0x028 /* RNG status register */
> > +#define TALITOS_RNGSR_HI 0x02c /* RNG status register */
> > +#define TALITOS_RNGSR_HI_RD 0x1 /* RNG Reset done */
> > +#define TALITOS_RNGSR_HI_OFL 0xff0000/* number of dwords in RNG output FIFO*/
> > +#define TALITOS_RNGDSR 0x010 /* RNG data size register */
> > +#define TALITOS_RNGDSR_HI 0x014 /* RNG data size register */
> > +#define TALITOS_RNG_FIFO 0x800 /* RNG FIFO - pool of random numbers */
> > +#define TALITOS_RNGISR 0x030 /* RNG Interrupt status register */
> > +#define TALITOS_RNGISR_HI 0x034 /* RNG Interrupt status register */
> > +#define TALITOS_RNGRCR 0x018 /* RNG Reset control register */
> > +#define TALITOS_RNGRCR_HI 0x01c /* RNG Reset control register */
>
> Please create a structure instead of using macros like this. Example:
>
> struct sec_rng {
> __be64 rngmr;
> u8 res1[8];
> __be64 rngdsr;
> __be64 rngrcr;
> ...
> };
>
> and then ...
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...
> > +static int talitos_hwrng_data_present(struct hwrng *rng)
> > +{
> > + void __iomem *rng_regs = (void __iomem *)rng->priv;
>
> struct sec_rng __iomem *rng = (struct sec_rng __iomem *) rng->priv;
>
> > +
> > +
> > + /* 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.
-Olof
next prev parent reply other threads:[~2007-06-06 22:02 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-06-05 15:42 MPC8349ea Random Device Generator driver Philippe Lachenal
2007-06-06 14:15 ` Arnd Bergmann
2007-06-07 11:29 ` MPC8349ea Random Number " Philippe Lachenal
2007-06-07 14:03 ` Arnd Bergmann
2007-06-06 21:57 ` MPC8349ea Random Device " Timur Tabi
2007-06-06 22:09 ` Olof Johansson [this message]
2007-06-06 22:07 ` Timur Tabi
2007-06-06 22:11 ` Arnd Bergmann
2007-06-06 22:19 ` Timur Tabi
2007-06-06 22:35 ` Arnd Bergmann
2007-06-06 22:38 ` Timur Tabi
2007-06-06 22:24 ` Olof Johansson
2007-06-06 22:32 ` Timur Tabi
2007-06-06 23:54 ` Olof Johansson
2007-06-07 14:23 ` Timur Tabi
2007-06-07 15:20 ` Olof Johansson
2007-06-07 15:20 ` Timur Tabi
2007-06-07 15:36 ` Olof Johansson
2007-06-06 22:48 ` Timur Tabi
2007-06-07 0:00 ` Olof Johansson
2007-06-07 2:55 ` Kim Phillips
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=20070606220913.GA27820@lixom.net \
--to=olof@lixom$(echo .)net \
--cc=linuxppc-dev@ozlabs$(echo .)org \
--cc=philippe.lachenal@hotmail$(echo .)fr \
--cc=sl@powerlinux$(echo .)fr \
--cc=timur@freescale$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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