From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Thu, 27 Mar 2014 23:00:54 +0100
Subject: [U-Boot] [PATCH v2 1/9] sunxi: initial sun7i clocks and timer
	support.
In-Reply-To: <1395955796.4541.20.camel@hastur.hellion.org.uk>
References: <1395438845.2234.95.camel@hastur.hellion.org.uk>
	<201403242152.38910.marex@denx.de>
	<1395955796.4541.20.camel@hastur.hellion.org.uk>
Message-ID: <201403272300.54825.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Thursday, March 27, 2014 at 10:29:56 PM, Ian Campbell wrote:
> On Mon, 2014-03-24 at 21:52 +0100, Marek Vasut wrote:
> > > +static struct sunxi_timer *timer_base =
> > > +     &((struct sunxi_timer_reg *)SUNXI_TIMER_BASE)->timer[TIMER_NUM];
> > > +
> > > +/* macro to read the 32 bit timer: since it decrements, we invert read
> > > value */ +#define READ_TIMER() (~readl(&timer_base->val))
> > 
> > This macro has to go, just use ~readl() in place. But still, why do you
> > use that negation in "~readl()" anyway ?
> 
> The comment right above it explains why: the timer counts backwards and
> inverting it accounts for that.
> 
> This is subtle enough that I don't think using ~readl() in place in the
> 3 callers would be an improvement.

Please do it, we don't want any implementers down the line using this 
"READ_TIMER()" call and getting hit by "timer_base undefined" . That macro hides 
the dependency on this symbol, while if you expanded it in-place, the dependency 
would be explicit. I really do want to see that macro gone, sorry.

Best regards,
Marek Vasut