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

[Bhushan Bharat-R65777 <R65777@xxxxxxxxxxxxx>]

> >>>>> +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 ?

Thanks
-Bharat

>    r.dbcr0 = 0;
> }
> 
> vs
> 
> struct kvmppc_debug_reg {
> #ifdef CONFIG_BOOKE
>    <lots of defines>
> #else
>    <lots of other defines>
> #endif
> };
> 
> void booke_foo() {
>    struct kvmppc_debug_reg r;
>    r.dbcr0 = 0;
> }
> 
> Which one has less #ifdefs? Keep in mind that the less #ifdefs you have, the
> more readable and maintainable your code becomes, because config options have
> less effect on your syntax.
> 
> 
> Alex
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body
> of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at
> http://vger.kernel.org/majordomo-info.html

��.n��������+%������w��{.n�����o��^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�