Hi Thomas, On Dec 5, 2007 11:25 AM, Thomas Bogendoerfer <tsbogend@xxxxxxxxxxxxxxxx> wrote: > > These: > > > > > +#define READ_SC(p, r) readb((p)->membase + RD_##r) > > > +#define WRITE_SC(p, r, v) writeb((v), (p)->membase + WR_##r) > > > > and these: > > > > > +#define READ_SC_PORT(p, r) read_sc_port(p, RD_PORT_##r) > > > +#define WRITE_SC_PORT(p, r, v) write_sc_port(p, WR_PORT_##r, v) > > > > really don't need to exist. All they do is make the code harder to read. > > but they make the code safer. The chip has common register and port > registers, which are randomly splattered over the address range. And > some of them are read only, some write only. Read only and Write > only register live at the same register offset and their function > usually doesn't have anything in common. By using these macros I'll > get compile errors when doing a READ_SC from a write only register > and vice versa. I will also get compile errors, if I try to access a > common register via READ_SC_PORT/WRITE_SC_PORT. You can use grep to make sure there are no reads to a write-only register. What you have there is not safety but macro obfuscation at its best. It makes the code harder to read for anyone not intimately familiar with the driver.