Re: unsigned long ioremap()?

From: Abramo Bagnara (abramo@alsa-project.org)
Date: Fri May 04 2001 - 02:15:12 EST


"David S. Miller" wrote:
>
> Abramo Bagnara writes:
> > IMO this is a far less effective debugging strategy.
>
> I agree with you.
>
> But guess what driver authors are going to do? They are going to cast
> the thing left and right. And sure you can then search for that, but
> it isn't likely to make people fix this from the start.

We can easily find now the current misuses (I've already posted a
complete list some time ago) and ensure that authors will do the right
thing.

True benefits will come in future from to have a correct API (the only
point to remember is that ioremap returned value and readl arguments is
*not* a pointer, this is not questionable).

>
> I suppose the point is that there is a fine line wrt. using APIs to
> influence people to "do the right thing", and this has been
> exemplified in several threads I've been involved in wrt. PCI dma
> and other topics. :-)
>
> One final point, I want to reiterate that I believe:
>
> foo = readl(&regs->bar);
>
> is perfectly legal and should not be discouraged and in particular,
> not made painful to do.

I disagree: regs it's not a dereferenceable thing and I think it's an
abuse of pointer type. You're keeping a pointer that need a big sign on
it saying "Don't dereference me", it's a mess.

However you might like to substitute this with:

#define fld_readl(cookie, str, fld) readl(cookie + (unsigned
long)&((struct str *) 0)->fld)

or without that, it's perfectly fine to have:

regs = (struct reg *) ioremap(addr, size);
foo = readl((unsigned long)&regs->bar);

It's a driver author matter of preference, that don't touch the fact
that API is correct.

However I've verified that often this lead to unexpected errors due to
different alignment on different architecture and this is why I
personally prefer constant offsets over structures fields.

-- 
Abramo Bagnara                       mailto:abramo@alsa-project.org

Opera Unica Phone: +39.546.656023 Via Emilia Interna, 140 48014 Castel Bolognese (RA) - Italy

ALSA project http://www.alsa-project.org It sounds good! - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Mon May 07 2001 - 21:00:19 EST