On Tue, Jun 28, 2016 at 10:36:23AM +0530, Nikunj A Dadhania wrote:
> 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"
Yes, that would be ok with me.
> >> 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
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
