Re: [Qemu-devel] [PATCH v2 2/9] softmmu: provide softmmu access type enum

[Leon Alrae]

Hi Peter,

On 08/07/2014 14:00, Peter Maydell wrote:
> On 8 July 2014 08:57, Leon Alrae <address@hidden> wrote:
>> New MIPS features depend on the access type and enum is more convenient than
>> using the numbers directly.
>>
> Mmm, I've thought for a while it would be better to have this
> be an enum, but never got round to it.
> 
>> Signed-off-by: Leon Alrae <address@hidden>
>> ---
>>  include/exec/cpu-common.h |    6 ++++++
>>  softmmu_template.h        |   26 ++++++++++++++++----------
>>  2 files changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
>> index e3ec4c8..1c3171a 100644
>> --- a/include/exec/cpu-common.h
>> +++ b/include/exec/cpu-common.h
>> @@ -26,6 +26,12 @@ typedef struct CPUListState {
>>      FILE *file;
>>  } CPUListState;
>>
>> +enum mmu_access_type {
> 
> CODING_STYLE says enum names should be CamelCase.
> I think you also want this to be a typedef.

The style was copied from "enum device_endian {" which is few lines
below, I assumed it was correct :) I'll fix this.

>> +    MMU_DATA_LOAD  = 0,
>> +    MMU_DATA_STORE = 1,
>> +    MMU_INST_FETCH = 2
>> +};
>> +
> 
> We should probably also chase through and update the
> prototypes of functions like tlb_fill() and cpu_unaligned_access()
> and so on to take this enum type rather than int. (I
> suspect there's a lot of those running into different
> targets so it might need doing over multiple patches.)
> 
> thanks
> -- PMM
> 

I intentionally skipped this here to focus this patchset on the new
features only. I also think that eventually we will have to update the
prototypes and go through all the targets.

Thanks,
Leon