On Tue, Dec 04, 2007 at 07:27:38PM -0800, Andrew Morton wrote: > grumble. > > 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. If there is a better way to get more readable code for you and the safety I'd like, just tell me. > Think of the poor reader who sees this: > > status = READ_SC_PORT(port, SR); > > and then goes madly searching for "SR". which he then finds by this name in the data sheet. All the register names are named as close to the datasheet as possible. And I consider consulting datasheets when looking at drivers a pretty good idea. > Code is written once and is read a thousand times. Please optimise for > reading. it's no big deal to change that, but is getting bitten by stupid chips worth it ? I'd prefer to get a compile error than debugging a runtime error. Thomas. -- Crap can work. Given enough thrust pigs will fly, but it's not necessary a good idea. [ RFC1925, 2.3 ]
