From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers Date: Fri, 17 Apr 2015 11:34:51 +0100 Message-ID: <1429266891.25195.260.camel@citrix.com> References: <1427796446.2115.34.camel@citrix.com> <1427796462-24376-5-git-send-email-ian.campbell@citrix.com> <551E8CD5.3080407@citrix.com> <1429201350.25195.175.camel@citrix.com> <5530A5BC.1030202@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5530A5BC.1030202@gmail.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall Cc: Julien Grall , xen-devel@lists.xen.org, julien.grall@linaro.org, tim@xen.org, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On Fri, 2015-04-17 at 07:18 +0100, Julien Grall wrote: > >>> +/* Read only + read as zero */ > >> > >> This comment may confuse developer who wants to implement RO register > >> which another value than 0. > >> > >> I got confuse too. It would be nice to expand the comment for the RO case. > > > > I'm afraid I don't understand the confusion so I'm not sure how to > > clarify. What did you think this comment was saying? > > When I read the comment I though you were implemented two distinct part: > RO and RAZ. Well, in a sense I am ;-) > As the value return by RO may not always be 0 (we have a handful of > cases in traps.c), I didn't understand why you were implementing both > within the same helper. > > Although this helper choose to implement RO as RAZ. So I think it would > be good to mention it in order to avoid confusion later. Perhaps rather than: /* Read as zero + write ignore */ /* Write only + write ignore */ /* Read only + read as zero */ this would be clearer: /* Read as zero and write ignore */ /* Write only as write ignore */ /* Read only as read as zero */ ? IOW I suspect it is the + which has confused you, especially since it is used differently in the first vs the second two. Ian.