Re: [Xen-devel] [PATCH 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

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.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel