On Tue, Sep 23, 2003 at 09:16:17AM +0100, Christoph Hellwig wrote:
> On Mon, Sep 22, 2003 at 04:30:28PM -0700, Greg KH wrote:
> > for (addr = 0x00; addr <= (is_isa ? 0xffff : 0x7f); addr++) {
> > - /* XXX: WTF is going on here??? */
> > - if ((is_isa && check_region(addr, 1)) ||
> > + void *region_used = request_region(addr, 1, "foo");
> > + release_region(addr, 1);
> > + if ((is_isa && (region_used == NULL)) ||
>
> WTF?? Your papering over bugs again, this doesn't help at all.
Why?
Ok, from my reading of this horrible chunk of code it does the
following:
- if this is a isa based controller, then we check the region
that is to be used.
- If it is already in use by someone else, then we skip it, and
move on to the next address.
- If it is not in use, then we pass the address down to the chip
driver and let it try to find the chip at this address (it
will do the reserving of the address space on its own.)
So basically, check_region is pretty valid here, as we are trying to see
if something else is already at this address, to try to prevent i2c
drivers from stomping on each other. I replaced this with a
request_region()/release_region() pair to get rid of the compiler
warning.
Is this your understanding too? Or do you think we should just get rid
of the request_region() check here all together?
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
