From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50665) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHman-0007Zr-09 for qemu-devel@nongnu.org; Tue, 28 Jun 2016 02:33:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bHmaj-0000gi-Mq for qemu-devel@nongnu.org; Tue, 28 Jun 2016 02:33:52 -0400 Date: Tue, 28 Jun 2016 16:25:52 +1000 From: David Gibson Message-ID: <20160628062552.GG4242@voom.fritz.box> References: <1466704050-15108-1-git-send-email-nikunj@linux.vnet.ibm.com> <1466704050-15108-10-git-send-email-nikunj@linux.vnet.ibm.com> <20160627042630.GH4242@voom.fritz.box> <87k2hb43bz.fsf@abhimanyu.i-did-not-set--mail-host-address--so-tickle-me> <878txr3ps5.fsf@abhimanyu.i-did-not-set--mail-host-address--so-tickle-me> <20160628030024.GW4242@voom.fritz.box> <874m8d2980.fsf@abhimanyu.i-did-not-set--mail-host-address--so-tickle-me> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="mXWEJCq0x1D2MX0F" Content-Disposition: inline In-Reply-To: <874m8d2980.fsf@abhimanyu.i-did-not-set--mail-host-address--so-tickle-me> Subject: Re: [Qemu-devel] [PATCH v1 09/11] ppc/xics: Split ICS into ics-base and ics class List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikunj A Dadhania Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, clg@kaod.org, Benjamin Herrenschmidt --mXWEJCq0x1D2MX0F Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jun 28, 2016 at 10:36:23AM +0530, Nikunj A Dadhania wrote: > David Gibson writes: >=20 > > [ Unknown signature status ] > > On Mon, Jun 27, 2016 at 03:41:06PM +0530, Nikunj A Dadhania wrote: > >> Nikunj A Dadhania writes: > >>=20 > >> > David Gibson writes: > >> > > >> >> [ Unknown signature status ] > >> >> On Thu, Jun 23, 2016 at 11:17:28PM +0530, Nikunj A Dadhania wrote: > >> >>> From: Benjamin Herrenschmidt > >> >>>=20 > >> >>> The existing implementation remains same and ics-base is introduce= d. > >> >>>=20 > >> >>> This will allow different implementations for the source controlle= rs > >> >>> such as the MSI support of PHB3 on Power8 which uses in-memory sta= te > >> >>> tables for example. > >> >>>=20 > >> >>> Signed-off-by: Benjamin Herrenschmidt > >> >>> Signed-off-by: Nikunj A Dadhania > >> >>> --- > >> >>> hw/intc/xics.c | 101 +++++++++++++++++++++++++++++++++----= ------------- > >> >>> hw/intc/xics_spapr.c | 36 ++++++++++-------- > >> >>> include/hw/ppc/xics.h | 11 +++++- > >> >>> 3 files changed, 97 insertions(+), 51 deletions(-) > >> >>>=20 > >> >>> 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 =3D { > >> >>> #define XISR(ss) (((ss)->xirr) & XISR_MASK) > >> >>> #define CPPR(ss) (((ss)->xirr) >> 24) > >> >>> =20 > >> >>> -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. > >>=20 > >> 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. >=20 > 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_* >=20 > Similar to what we did for XICS >=20 > #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" >=20 >=20 > Like this >=20 >=20 > #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. >=20 > Regards > Nikunj >=20 --=20 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 --mXWEJCq0x1D2MX0F Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXchhwAAoJEGw4ysog2bOS+oQP/A8NCI5kUUdjMRcz7I/HIf/8 zy20i1lF5DZPBgnyxZwAYzUfPR91XCvsJFe/rzgO03aEG3/N5Z2000NM49ie3XX7 YeOdrlGfSav0JoMgZT+ia0GRQoBBlCKMk+mH4Ort/jtR9Ig8jCppEkXzuOw8TQdb Ea2dNlEfDbEawpH7Op64kG5PTF90ADxq37bBNS2DGyDMVx7TzvEAfa8ZmsgMQ2kC jlPqfDVueR+HVSiBdpxsYZZYFpJBDKghLCibCFdif/sCN9ql1VjynRLxXs4EaX+I 5lGooP5JiwFxsLTdJFW5LkRhrMjouhgWI+ZYC8kk0SR7TGEVN1fVCrzvx8fdjdro KToETG4b0fry8AdJfk4+NoRNlp3wMwVv4nquHafhGmgdJU9kBsHPTXHMREgy8Yn5 k9k9JRqDrczrcsm5LE5nI7sOMDxcl/mf/3pHE6sMJHJ0TLhkgwJLVsfUWIo7Bqqa q3nx3IQvTwGPkMwAF4mJcyfUtk7h+4MIhWhlcuuOqhejhaIrXZvMyHDkpGzOHVfW RAKh3xmYx25Wi+g8iK106b169XyDjzCFaOBw/XHpf6xvDRyBZs9cO5aNao+McOO7 ZsKM4XsXMzaRxKLEK8lTZQUgTVaLGUkqodAeSrp3LZo9FMfq8TT6sTfshqseTy1j /isFD0uWd9vR+6E3/ARo =c13D -----END PGP SIGNATURE----- --mXWEJCq0x1D2MX0F--