linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Pekka Enberg" <penberg@cs.helsinki.fi>
To: "Thomas Bogendoerfer" <tsbogend@alpha.franken.de>
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-mips@linux-mips.org,
	"Andy Whitcroft" <apw@shadowen.org>,
	"Alan Cox" <alan@lxorguk.ukuu.org.uk>
Subject: Re: [PATCH] SC26XX: New serial driver for SC2681 uarts
Date: Sat, 22 Dec 2007 11:47:14 +0200	[thread overview]
Message-ID: <84144f020712220147q2e291fa1t4ee03e4d64be95b@mail.gmail.com> (raw)
In-Reply-To: <20071205092506.GA6691@alpha.franken.de>

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.

  reply	other threads:[~2007-12-22  9:47 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-02 19:43 [PATCH] SC26XX: New serial driver for SC2681 uarts Thomas Bogendoerfer
2007-12-03 23:53 ` Andrew Morton
2007-12-03 23:57   ` Arjan van de Ven
2007-12-04  8:25     ` Thomas Bogendoerfer
2007-12-04 11:01       ` Geert Uytterhoeven
2007-12-04 12:45       ` Alan Cox
2007-12-04 23:57         ` Thomas Bogendoerfer
2007-12-04 23:41   ` Thomas Bogendoerfer
2007-12-05  3:27     ` Andrew Morton
2007-12-05  9:25       ` Thomas Bogendoerfer
2007-12-22  9:47         ` Pekka Enberg [this message]
2007-12-21 17:36   ` Andy Whitcroft

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=84144f020712220147q2e291fa1t4ee03e4d64be95b@mail.gmail.com \
    --to=penberg@cs.helsinki.fi \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=apw@shadowen.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=tsbogend@alpha.franken.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).