Re: [PATCH v2] booke: Added ONE_REG interface for IAC/DAC debug registers

[Alexander Graf <agraf@suse.de>]

On 07/24/2012 03:26 PM, Bhushan Bharat-R65777 wrote:
>>>>>>> +struct kvmppc_debug_reg {
>>>>>>> +#ifdef CONFIG_BOOKE
>>>>>>> +	u32 dbcr0;
>>>>>>> +	u32 dbcr1;
>>>>>>> +	u32 dbcr2;
>>>>>>> +#ifdef CONFIG_KVM_E500MC
>>>>>>> +	u32 dbcr4;
>>>>>>> +#endif
>>>>>>> +	u64 iac[KVMPPC_MAX_IAC];
>>>>>>> +	u64 dac[KVMPPC_MAX_DAC];
>>>>>>> +#endif
>>>>>>> +};
>>>>>> Is there any reason for this to be a separate struct?
>>>>> No specific reason, The rest of debug ( which will follow sometime
>>>>> soon) uses
>>>> this structure and looks to make code look easy.
>>>>
>>>> So why not use an fsl / booke specific struct for the debug patches
>>>> then? Debug registers are really nothing common between book3s and
>>>> booke, so we shouldn't treat them as such by using the same struct
>> definition.
>>> All elements of struct are under #ifdef CONFIG_BOOKE? So for book3s it is as
>> good as void, only struct type if declared. Do you want the struct type also
>> under config_booke ?
>>
>> struct kvmppc_booke_debug_reg {
>>      <lots of defines>
>> };
>>
>> struct kvmppc_book3s_debug_reg {
>>      <lots of other defines>
>> };
>>
>> void booke_foo() {
>>     struct kvmppc_booke_debug_reg r;
> kvmppc_booke_debug_reg or kvmppc_book3s_debug_reg ?

In booke_foo() you certainly will never want to use struct 
kvmppc_book3s_debug_reg, no?


Alex