On Mon, Jan 19, 2015 at 1:03 PM, Peter Maydell <peter.mayd...@linaro.org>
wrote:
> On 19 January 2015 at 18:05, Greg Bellows <greg.bell...@linaro.org> wrote:
> >
> >
> > On Mon, Jan 19, 2015 at 9:17 AM, Peter Maydell <peter.mayd...@linaro.org
> >
> > wrote:
> >> + if (ri->type & ARM_CP_CONST) {
> >> + return true;
> >> + }
> >
> >
> > Had to refresh my memory on this. It appears we changed the name
> (polarity)
> > of the function based on our previous discussion. However, according to
> the
> > above description, we should return 'true' if read/write would cause an
> > assertion. but in the case of CONST we would not assert, but still return
> > 'true'?
>
> Doh. I inverted the name and polarity but forgot to change the function
> body. (I have no idea why that didn't blow up). Will fix (and test a
> bit more thoroughly...)
>
>
FYI, I actually ran the changes and they do unwantedly assert.
> >>
> >> + return (ri->raw_writefn || ri->writefn || ri->fieldoffset) &&
> >> + (ri->raw_readfn || ri->readfn || ri->fieldoffset);
> >
> >
> > This case appears to need inverting as it will resolve to 'true' but
> should
> > be valid.
> >
> > NIT: It would be cleaner to pull the ri->fieldoffset check above this
> check
> > to simplify the conditional.
>
> That's deliberately matching the checks in the read/write_raw_cp_reg
> functions, but then I didn't do that for the CP_CONST check I guess.
>
> -- PMM
>
