Re: [PATCH] isa_{read,write}{w,l} fixed

[Jeff Garzik]

Russell King wrote:
> The only truely portable way of doing this type of thing is to use either
> integer or char arrays.

Good point.  I always use offsets (base_addr + REG_CONSTANT) but other
code I've seen, including acpi IIRC, use readb(&my_struct->foo_field);

> > I much prefer to avoid structs, and use macros to access registers
> > instead:
> > 
> >       my_reg = RTL_R8 (FooRegisterConstant);

> From reading IO-mapping.txt, it appears that the correct way is to:
> 
>         char *my_ptr;
>         my_ptr = ioremap(...);
>         my_reg = readb(my_ptr + reg_offset);

Yes.  I hide base addresses under the hood, to make using the macros
easier.

#define RTL_R8(reg) (readb(ioaddr + (reg)))

so one only writes

	reg = RTL_R8 (Config1);

instead of 

	reg = RTL_R8 (ioaddr + Config1);

Since the driver code always creates a temporary named ioaddr, this
works.

> BTW, isn't it about time this document was updated wrt the isa_* macros?
> It currently implies that it's ok to do:
> 
>         reg = readb(0xc0000)
> 
> to read ISA address 0xc0000, which seems it should now be:
> 
>         reg = isa_readb(0xc0000)

Since isa_xxx is [hopefully] temporary until all drivers are converted,
IMHO IO-mapping.txt should be changed to simply say that
ioremap(0xc0000) is required just like I/O to any other mmio address.

	Jeff

-- 
Jeff Garzik         | "Vegetarian" is the Indian word
Building 1024       | for 'lousy hunter.'
MandrakeSoft, Inc.  |