On Thu, Feb 13, 2014 at 5:32 AM, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 31 January 2014 04:44, Alistair Francis <alistair.fran...@xilinx.com>
> wrote:
>> This patch implements the ARM PMCCNTR register including
>> the disable and reset components of the PMCR register.
>>
>> Signed-off-by: Alistair Francis <alistair.fran...@xilinx.com>
>
> I'm afraid you'll need to respin this against my target-arm.next
> branch, because I've landed the patches which update the
> API for read/write functions.
> git://git.linaro.org/people/peter.maydell/qemu-arm.git target-arm.next
>
>> +
>> static int ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri,
>> uint64_t *value)
>> {
>> @@ -644,9 +734,10 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>> */
>> { .name = "PMSELR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 5,
>> .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
>> - /* Unimplemented, RAZ/WI. XXX PMUSERENR */
>
> Your patch has dropped this XXX comment without fixing the
> issue. Luckily you'll find that in target-arm.next it's been fixed,
> by providing a suitable accessfn for the register.
>
>> { .name = "PMCCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 =
>> 0,
>> - .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
>> + .access = PL1_RW, .resetvalue = 0, .type = ARM_CP_IO,
>
> This changes the access rights, incorrectly.
>
>> + .readfn = pmccntr_read, .writefn = pmccntr_write,
>> + .raw_readfn = raw_read, .raw_writefn = raw_write },
>
> This is wrong, because raw_read/write need to return the same
> value as the actual register, just without odd side effects.
>
My understanding is that the _read and _write functions as you have
implemented are this exact semantic. So you should be able to just
drop the raw_'s.
Regards,
Peter
>> { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2
>> = 1,
>> .access = PL0_RW,
>> .fieldoffset = offsetof(CPUARMState, cp15.c9_pmxevtyper),
>
> Thanks, and sorry for the inconvenience resulting from
> the clashing patchset.
>
> -- PMM
>
