On 09/22/2016 01:40 AM, David Gibson wrote:
> On Mon, Sep 19, 2016 at 11:59:33AM +0530, Nikunj A Dadhania wrote:
>> From: Benjamin Herrenschmidt <b...@kernel.crashing.org>
>>
>> Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
>> [Move object allocation and adding child to the helper]
>> Signed-off-by: Nikunj A Dadhania <nik...@linux.vnet.ibm.com>
>> Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>
>> ---
>> hw/intc/xics.c | 10 ++++++++++
>> hw/intc/xics_spapr.c | 6 +-----
>> include/hw/ppc/xics.h | 1 +
>> 3 files changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>> index 05e938f..c7901c4 100644
>> --- a/hw/intc/xics.c
>> +++ b/hw/intc/xics.c
>> @@ -109,6 +109,16 @@ static void xics_common_reset(DeviceState *d)
>> }
>> }
>>
>> +void xics_add_ics(XICSState *xics)
>> +{
>> + ICSState *ics;
>> +
>> + ics = ICS(object_new(TYPE_ICS));
>> + object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL);
>
> You'll need to construct a name here so you don't have all the ics
> objects called an indistinguishable "ics".
Yes, exactly, and so PowerNV does not use it because at least three ics
are needed :
qemu) info qom-tree
/machine (powernv-machine)
/unattached (container)
/sysbus (System)
/ipmi-bt[0] (qemu:memory-region)
/device[0] (pnv-phb3)
/ics-phb-lsi (ics)
/ics-phb-msi (phb3-msi)
...
/psi (pnv-psi)
/xscom-psi[0] (qemu:memory-region)
/psihb[0] (qemu:memory-region)
/ics-psi (ics)
I think we can drop that patch.
However some routine like this one :
+void xics_insert_ics(XICSState *xics, ICSState *ics)
+{
+ ics->xics = xics;
+ QLIST_INSERT_HEAD(&xics->ics, ics, list);
+}
+
would be useful to hide the list details below xics :
/* link in the PSI ICS */
xics_insert_ics(XICS_COMMON(&chip->xics), &chip->psi.ics);
....
/* insert the ICS in XICS */
xics_insert_ics(xics, phb->lsi_ics);
xics_insert_ics(xics, ICS_BASE(phb->msis));
Cheers,
C.
>> + ics->xics = xics;
>> + QLIST_INSERT_HEAD(&xics->ics, ics, list);
>> +}
>> +
>> static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char *name,
>> void *opaque, Error **errp)
>> {
>> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
>> index 0b0845d..270f20e 100644
>> --- a/hw/intc/xics_spapr.c
>> +++ b/hw/intc/xics_spapr.c
>> @@ -305,12 +305,8 @@ static void xics_spapr_realize(DeviceState *dev, Error
>> **errp)
>> static void xics_spapr_initfn(Object *obj)
>> {
>> XICSState *xics = XICS_SPAPR(obj);
>> - ICSState *ics;
>>
>> - ics = ICS(object_new(TYPE_ICS));
>> - object_property_add_child(obj, "ics", OBJECT(ics), NULL);
>> - ics->xics = xics;
>> - QLIST_INSERT_HEAD(&xics->ics, ics, list);
>> + xics_add_ics(xics);
>> }
>>
>> static void xics_spapr_class_init(ObjectClass *oc, void *data)
>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
>> index e49a2da..a7a1e54 100644
>> --- a/include/hw/ppc/xics.h
>> +++ b/include/hw/ppc/xics.h
>> @@ -197,5 +197,6 @@ void ics_write_xive(ICSState *ics, int nr, int server,
>> void ics_set_irq_type(ICSState *ics, int srcno, bool lsi);
>>
>> ICSState *xics_find_source(XICSState *icp, int irq);
>> +void xics_add_ics(XICSState *xics);
>>
>> #endif /* XICS_H */
>
