[PATCH 3/4] ARM: Xilinx: Adding timer support to the platform

[arnd@arndb.de (Arnd Bergmann)]

On Tuesday 08 February 2011, John Linn wrote:
> > > +#define PERIPH_BASE            0xF8000000
> > 
> > should be:
> > 
> > #define PERIPH_BASE            IOMEM(0xF8000000)
> > 
> > But then in a different patch you do:
> > +               .virtual        = TTC0_BASE,
> > +               .pfn            = __phys_to_pfn(TTC0_BASE),
> > 
> > So it's also used as a physical address.  Yuck.
> 
> I have been mapping everything flat (virt = phys) and I should have been
> clearer on that.
> 
> It seems like there are two sets of #defines expected, virtual and
> physical, even if they are the same.

Yes, it quickly gets messy otherwise. Normally, you only need to specify
very few physical addresses, e.g. the start of each register window.
When you have separate defines for your virtual bases, you can also
make them type safe right away, like:

#define PERIPH_BASE_PHYS 0xf800000
#define PERIPH_BASE     IOMEM(PERIPH_BASE_PHYS)

#define TCC0_BASE 	(PERIPH_BASE + 0xA0000)
#define TCC1_BASE	(PERIPH_BASE + 0xB0000)

This way, you don't need to convert the types in driver code.

	Arnd