On Wed, Sep 03, 2008 at 11:33:51AM -0400, Eduardo Valentin wrote:
> > @@ -159,7 +159,7 @@ static struct omap_mcbsp_platform_data 
> > omap730_mcbsp_pdata[] = {
> >  #ifdef CONFIG_ARCH_OMAP15XX
> >  static struct omap_mcbsp_platform_data omap15xx_mcbsp_pdata[] = {
> >        {
> > -               .virt_base      = OMAP1510_MCBSP1_BASE,
> > +               .virt_base      = OMAP1510_MCBSP1_BASE, /* FIXME: virtual 
> > or physical */
> AFAIK, OMAP1510_MCBSP1_BASE is physical. So, I'd say:
> +               .virt_base      = IO_ADDRESS(OMAP1510_MCBSP1_BASE),
> 
> Because, plat-omap/mcbsp.c expect .virt_base to be a virtual address.

And today, the story is completely different, having looked through more
of the code and some documentation.

- OMAPxxxx_MCBSPx_BASE are all physical addresses.  Fine.
- physical addresses > 0xfffb0000 are subject to an offset (IO_OFFSET)
  but others for the DSP located and DSP shared peripherals aren't.

So, applying the IO_OFFSET via IO_ADDRESS() or io_p2v() to all addresses
breaks.  Meaning it's completely random whether something should be put
through IO_ADDRESS() and similar.

This isn't obvious.  It isn't readable.  It isn't maintainable.  It doesn't
lend itself to compile time checking.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to