Re: [PATCH] SC26XX: New serial driver for SC2681 uarts

["Pekka Enberg" <penberg@cs.helsinki.fi>]

Hi Thomas,

On Dec 5, 2007 11:25 AM, Thomas Bogendoerfer <tsbogend@alpha.franken.de> 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.