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...)
>>
>> + 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
