David Gibson <da...@gibson.dropbear.id.au> writes:
> [ Unknown signature status ]
> On Mon, Jun 27, 2016 at 03:41:06PM +0530, Nikunj A Dadhania wrote:
>> Nikunj A Dadhania <nik...@linux.vnet.ibm.com> writes:
>>
>> > David Gibson <da...@gibson.dropbear.id.au> writes:
>> >
>> >> [ Unknown signature status ]
>> >> On Thu, Jun 23, 2016 at 11:17:28PM +0530, Nikunj A Dadhania wrote:
>> >>> From: Benjamin Herrenschmidt <b...@kernel.crashing.org>
>> >>>
>> >>> The existing implementation remains same and ics-base is introduced.
>> >>>
>> >>> This will allow different implementations for the source controllers
>> >>> such as the MSI support of PHB3 on Power8 which uses in-memory state
>> >>> tables for example.
>> >>>
>> >>> Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
>> >>> Signed-off-by: Nikunj A Dadhania <nik...@linux.vnet.ibm.com>
>> >>> ---
>> >>> hw/intc/xics.c | 101
>> >>> +++++++++++++++++++++++++++++++++-----------------
>> >>> hw/intc/xics_spapr.c | 36 ++++++++++--------
>> >>> include/hw/ppc/xics.h | 11 +++++-
>> >>> 3 files changed, 97 insertions(+), 51 deletions(-)
>> >>>
>> >>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>> >>> index 326d21f..e2aa48d 100644
>> >>> --- a/hw/intc/xics.c
>> >>> +++ b/hw/intc/xics.c
>> >>> @@ -220,9 +220,32 @@ static const TypeInfo xics_common_info = {
>> >>> #define XISR(ss) (((ss)->xirr) & XISR_MASK)
>> >>> #define CPPR(ss) (((ss)->xirr) >> 24)
>> >>>
>> >>> -static void ics_reject(ICSState *ics, int nr);
>> >>> -static void ics_resend(ICSState *ics);
>> >>> -static void ics_eoi(ICSState *ics, int nr);
>> >>> +static void ics_base_reject(ICSState *ics, uint32_t nr)
>> >>
>> >> AFICT these will actually work for any of the derived classes, since
>> >> they call the function pointer. So I thin the original name was
>> >> better than ics_base_*().
>> >
>> > Sure, will change.
>>
>> I had a look at this again, we will need to use ics_base_*(), same file
>> has the implementation of ics_reject() for TYPE_ICS.
>
> No, the ics_reject() plain names still work best for the generic
> versions which call via the function pointers. Instead we should find
> a new name for the TYPE_ICE implementations.
Should we go back to call it as TYPE_ICS_SIMPLE retaining the "ics" for
migration compatibility. Rename all the related functions as
ics_simple_*
Similar to what we did for XICS
#define TYPE_XICS_COMMON "xics-common"
#define TYPE_XICS_SPAPR "xics"
#define TYPE_XICS_SPAPR_KVM "xics-spapr-kvm"
#define TYPE_XICS_NATIVE "xics-native"
Like this
#define TYPE_ICS_BASE "ics-base"
#define TYPE_ICS_SIMPLE "ics"
#define TYPE_KVM_ICS "icskvm"
>
>> BenH's patches had renamed the class implementation as ics_simple_*().
>> Since we moved to using ICS_BASE, ICS and KVM_ICS, IMHO this seems to
>> the appropriate names.
>
> No. Using ics_base_*() for the generic versions is actively
> misleading. Using good names for those is more important than what
> would usually be consistent naming practice for the TYPE_ICS
> implementation.
Regards
Nikunj
