Re: [PATCH] ARM: OMAP: Add minimal OMAP2430 support

Juha Yrjölä
2006-06-11T10:08:02Z

Hi,

Komal Shah wrote:

> After looking at the files which are needed to change for
> prcm_read/write[l], clock.c needs the major update. I hope Richard is
> OK with going from __REG32(...) to __raw_read/write[l] based prcm_rw
> wrappers?

read_reg() and write_reg() are definitely the way to go. The __REG()
macros are unportable, and usually even lead to inefficient assembly code.

As for which API is the most coherent, I've found that this approach
makes the most sense:

static void __iomem *prcm_base;

static u32 prcm_read_reg(int reg);
static void prcm_write_reg(int reg, u32 value);

If you go with the readl/writel argument ordering, you'll end up with
oddness like:

static u32 mmc_writel(struct mmc *mmc, u32 value, int reg);

You should also not expose the raw register read/write functions, but
rather implement wrapper functions to modify the relevant registers.
Otherwise you might run into race issues with a preemptible kernel.

Cheers,
Juha