On Thu, Oct 11, 2018 at 03:04:08PM +0200, Greg Kurz wrote: > On Thu, 11 Oct 2018 13:02:50 +0200 > Philippe Mathieu-Daudé <phi...@redhat.com> wrote: > > > Hi Greg, > > > > On 11/10/2018 09:00, Greg Kurz wrote: > > > According to CODING_STYLE, structured types names are expected to be > > > in CamelCase. PCI is dropped from the name for better > > readability.
I'd prefer you didn't drop PCI from the name. These specifically *are* PCI MSIs, whereas lots of PAPR documentation (particularly for the interrupt controller) uses MSIs to indicate general message interrupts, such as those from VIO and firmware event reporting. > > > > > > While here, this also converts a call to g_malloc(n * sizeofi(foo)) to > > > > Oops finger slipped on its way to the left parenthesis :) > > > sizeof(foo) > > > > > g_new(n, foo), which is a recommended good practice. > > > > Clearly 2 different patches... > > > > You're right in theory. I added the g_new conversion in the same patch > for simplicity mostly. I'll let David decide :) I'd prefer them separate. > > > > > > > Signed-off-by: Greg Kurz <gr...@kaod.org> > > > --- > > > hw/ppc/spapr_pci.c | 22 +++++++++++----------- > > > include/hw/pci-host/spapr.h | 12 ++++++------ > > > 2 files changed, 17 insertions(+), 17 deletions(-) > > > > > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > > > index c2271e6ed462..bfb959f2b065 100644 > > > --- a/hw/ppc/spapr_pci.c > > > +++ b/hw/ppc/spapr_pci.c > > > @@ -277,7 +277,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, > > > sPAPRMachineState *spapr, > > > unsigned int irq, max_irqs = 0; > > > sPAPRPHBState *phb = NULL; > > > PCIDevice *pdev = NULL; > > > - spapr_pci_msi *msi; > > > + sPAPRMSI *msi; > > > > In CamelCase this should be "SpaprMsi"... > > I however agree sPAPRMSI is nicer, the code is also easier to > > review. Yeah.. it's kind of a mangled combination of CamelCase with what's been done elsewhere in the spapr code. > > > > > int *config_addr_key; > > > Error *err = NULL; > > > int i; > > > @@ -325,7 +325,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, > > > sPAPRMachineState *spapr, > > > return; > > > } > > > > > > - msi = (spapr_pci_msi *) g_hash_table_lookup(phb->msi, &config_addr); > > > + msi = (sPAPRMSI *) g_hash_table_lookup(phb->msi, &config_addr); > > > > > > /* Releasing MSIs */ > > > if (!req_num) { > > > @@ -414,7 +414,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, > > > sPAPRMachineState *spapr, > > > irq, req_num); > > > > > > /* Add MSI device to cache */ > > > - msi = g_new(spapr_pci_msi, 1); > > > + msi = g_new(sPAPRMSI, 1); > > > msi->first_irq = irq; > > > msi->num = req_num; > > > config_addr_key = g_new(int, 1); > > > @@ -445,7 +445,7 @@ static void > > > rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu, > > > unsigned int intr_src_num = -1, ioa_intr_num = rtas_ld(args, 3); > > > sPAPRPHBState *phb = NULL; > > > PCIDevice *pdev = NULL; > > > - spapr_pci_msi *msi; > > > + sPAPRMSI *msi; > > > > > > /* Find sPAPRPHBState */ > > > phb = spapr_pci_find_phb(spapr, buid); > > > @@ -458,7 +458,7 @@ static void > > > rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu, > > > } > > > > > > /* Find device descriptor and start IRQ */ > > > - msi = (spapr_pci_msi *) g_hash_table_lookup(phb->msi, &config_addr); > > > + msi = (sPAPRMSI *) g_hash_table_lookup(phb->msi, &config_addr); > > > if (!msi || !msi->first_irq || !msi->num || (ioa_intr_num >= > > > msi->num)) { > > > trace_spapr_pci_msi("Failed to return vector", config_addr); > > > rtas_st(rets, 0, RTAS_OUT_HW_ERROR); > > > @@ -1849,9 +1849,9 @@ static const VMStateDescription > > > vmstate_spapr_pci_msi = { > > > .version_id = 1, > > > .minimum_version_id = 1, > > > .fields = (VMStateField []) { > > > - VMSTATE_UINT32(key, spapr_pci_msi_mig), > > > - VMSTATE_UINT32(value.first_irq, spapr_pci_msi_mig), > > > - VMSTATE_UINT32(value.num, spapr_pci_msi_mig), > > > + VMSTATE_UINT32(key, sPAPRMSIMig), > > > + VMSTATE_UINT32(value.first_irq, sPAPRMSIMig), > > > + VMSTATE_UINT32(value.num, sPAPRMSIMig), > > > VMSTATE_END_OF_LIST() > > > }, > > > }; > > > @@ -1883,12 +1883,12 @@ static int spapr_pci_pre_save(void *opaque) > > > if (!sphb->msi_devs_num) { > > > return 0; > > > } > > > - sphb->msi_devs = g_malloc(sphb->msi_devs_num * > > > sizeof(spapr_pci_msi_mig)); > > > + sphb->msi_devs = g_new(sPAPRMSIMig, sphb->msi_devs_num); > > > > > > g_hash_table_iter_init(&iter, sphb->msi); > > > for (i = 0; g_hash_table_iter_next(&iter, &key, &value); ++i) { > > > sphb->msi_devs[i].key = *(uint32_t *) key; > > > - sphb->msi_devs[i].value = *(spapr_pci_msi *) value; > > > + sphb->msi_devs[i].value = *(sPAPRMSI *) value; > > > } > > > > > > return 0; > > > @@ -1938,7 +1938,7 @@ static const VMStateDescription vmstate_spapr_pci = > > > { > > > vmstate_spapr_pci_lsi, struct > > > spapr_pci_lsi), > > > VMSTATE_INT32(msi_devs_num, sPAPRPHBState), > > > VMSTATE_STRUCT_VARRAY_ALLOC(msi_devs, sPAPRPHBState, > > > msi_devs_num, 0, > > > - vmstate_spapr_pci_msi, > > > spapr_pci_msi_mig), > > > + vmstate_spapr_pci_msi, sPAPRMSIMig), > > > VMSTATE_END_OF_LIST() > > > }, > > > }; > > > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h > > > index 7c66c3872f96..041126da573d 100644 > > > --- a/include/hw/pci-host/spapr.h > > > +++ b/include/hw/pci-host/spapr.h > > > @@ -34,15 +34,15 @@ > > > > > > typedef struct sPAPRPHBState sPAPRPHBState; > > > > > > -typedef struct spapr_pci_msi { > > > +typedef struct sPAPRMSI { > > > uint32_t first_irq; > > > uint32_t num; > > > -} spapr_pci_msi; > > > +} sPAPRMSI; > > > > > > -typedef struct spapr_pci_msi_mig { > > > +typedef struct sPAPRMSIMig { > > > uint32_t key; > > > - spapr_pci_msi value; > > > -} spapr_pci_msi_mig; > > > + sPAPRMSI value; > > > +} sPAPRMSIMig; > > > > > > struct sPAPRPHBState { > > > PCIHostState parent_obj; > > > @@ -70,7 +70,7 @@ struct sPAPRPHBState { > > > GHashTable *msi; > > > /* Temporary cache for migration purposes */ > > > int32_t msi_devs_num; > > > - spapr_pci_msi_mig *msi_devs; > > > + sPAPRMSIMig *msi_devs; > > > > > > QLIST_ENTRY(sPAPRPHBState) list; > > > > > > > > > > -- 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