Message ID | 20190925064534.19155-18-david@gibson.dropbear.id.au (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | spapr: IRQ subsystem cleanups | expand |
On 25/09/2019 08:45, David Gibson wrote: > spapr_irq_claim() and the hooks it is based on return an integer error code > as well as taking an Error ** parameter. But none of the callers check the > integer, so we can remove it and just use the richer Error **. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Cédric Le Goater <clg@kaod.org> > --- > hw/ppc/spapr_irq.c | 32 +++++++++++++------------------- > include/hw/ppc/spapr_irq.h | 4 ++-- > 2 files changed, 15 insertions(+), 21 deletions(-) > > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > index 261d66ba17..2673a90604 100644 > --- a/hw/ppc/spapr_irq.c > +++ b/hw/ppc/spapr_irq.c > @@ -112,8 +112,8 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, Error **errp) > spapr->ics = ICS_SPAPR(obj); > } > > -static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > - Error **errp) > +static void spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > + Error **errp) > { > ICSState *ics = spapr->ics; > > @@ -122,11 +122,10 @@ static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > > if (!ics_irq_free(ics, irq - ics->offset)) { > error_setg(errp, "IRQ %d is not free", irq); > - return -1; > + return; > } > > ics_set_irq_type(ics, irq - ics->offset, lsi); > - return 0; > } > > static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq) > @@ -252,14 +251,12 @@ static void spapr_irq_init_xive(SpaprMachineState *spapr, Error **errp) > spapr_xive_hcall_init(spapr); > } > > -static int spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi, > - Error **errp) > +static void spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi, > + Error **errp) > { > if (!spapr_xive_irq_claim(spapr->xive, irq, lsi)) { > error_setg(errp, "IRQ %d is invalid", irq); > - return -1; > } > - return 0; > } > > static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq) > @@ -406,25 +403,22 @@ static void spapr_irq_init_dual(SpaprMachineState *spapr, Error **errp) > } > } > > -static int spapr_irq_claim_dual(SpaprMachineState *spapr, int irq, bool lsi, > - Error **errp) > +static void spapr_irq_claim_dual(SpaprMachineState *spapr, int irq, bool lsi, > + Error **errp) > { > Error *local_err = NULL; > - int ret; > > - ret = spapr_irq_xics.claim(spapr, irq, lsi, &local_err); > + spapr_irq_xics.claim(spapr, irq, lsi, &local_err); > if (local_err) { > error_propagate(errp, local_err); > - return ret; > + return; > } > > - ret = spapr_irq_xive.claim(spapr, irq, lsi, &local_err); > + spapr_irq_xive.claim(spapr, irq, lsi, &local_err); > if (local_err) { > error_propagate(errp, local_err); > - return ret; > + return; > } > - > - return ret; > } > > static void spapr_irq_free_dual(SpaprMachineState *spapr, int irq) > @@ -622,12 +616,12 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) > spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE); > } > > -int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) > +void spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) > { > assert(irq >= SPAPR_XIRQ_BASE); > assert(irq < (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE)); > > - return spapr->irq->claim(spapr, irq, lsi, errp); > + spapr->irq->claim(spapr, irq, lsi, errp); > } > > void spapr_irq_free(SpaprMachineState *spapr, int irq, int num) > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h > index ed88b4599a..75279ca137 100644 > --- a/include/hw/ppc/spapr_irq.h > +++ b/include/hw/ppc/spapr_irq.h > @@ -42,7 +42,7 @@ typedef struct SpaprIrq { > uint8_t ov5; > > void (*init)(SpaprMachineState *spapr, Error **errp); > - int (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > + void (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > void (*free)(SpaprMachineState *spapr, int irq); > void (*print_info)(SpaprMachineState *spapr, Monitor *mon); > void (*dt_populate)(SpaprMachineState *spapr, uint32_t nr_servers, > @@ -61,7 +61,7 @@ extern SpaprIrq spapr_irq_xive; > extern SpaprIrq spapr_irq_dual; > > void spapr_irq_init(SpaprMachineState *spapr, Error **errp); > -int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > +void spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > void spapr_irq_free(SpaprMachineState *spapr, int irq, int num); > qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq); > int spapr_irq_post_load(SpaprMachineState *spapr, int version_id); >
On 9/25/19 8:45 AM, David Gibson wrote: > spapr_irq_claim() and the hooks it is based on return an integer error code > as well as taking an Error ** parameter. But none of the callers check the > integer, so we can remove it and just use the richer Error **. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- > hw/ppc/spapr_irq.c | 32 +++++++++++++------------------- > include/hw/ppc/spapr_irq.h | 4 ++-- > 2 files changed, 15 insertions(+), 21 deletions(-) > > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > index 261d66ba17..2673a90604 100644 > --- a/hw/ppc/spapr_irq.c > +++ b/hw/ppc/spapr_irq.c > @@ -112,8 +112,8 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, Error **errp) > spapr->ics = ICS_SPAPR(obj); > } > > -static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > - Error **errp) > +static void spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > + Error **errp) > { > ICSState *ics = spapr->ics; > > @@ -122,11 +122,10 @@ static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > > if (!ics_irq_free(ics, irq - ics->offset)) { > error_setg(errp, "IRQ %d is not free", irq); > - return -1; > + return; > } > > ics_set_irq_type(ics, irq - ics->offset, lsi); > - return 0; > } > > static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq) > @@ -252,14 +251,12 @@ static void spapr_irq_init_xive(SpaprMachineState *spapr, Error **errp) > spapr_xive_hcall_init(spapr); > } > > -static int spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi, > - Error **errp) > +static void spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi, > + Error **errp) > { > if (!spapr_xive_irq_claim(spapr->xive, irq, lsi)) { > error_setg(errp, "IRQ %d is invalid", irq); > - return -1; > } > - return 0; > } > > static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq) > @@ -406,25 +403,22 @@ static void spapr_irq_init_dual(SpaprMachineState *spapr, Error **errp) > } > } > > -static int spapr_irq_claim_dual(SpaprMachineState *spapr, int irq, bool lsi, > - Error **errp) > +static void spapr_irq_claim_dual(SpaprMachineState *spapr, int irq, bool lsi, > + Error **errp) > { > Error *local_err = NULL; > - int ret; > > - ret = spapr_irq_xics.claim(spapr, irq, lsi, &local_err); > + spapr_irq_xics.claim(spapr, irq, lsi, &local_err); > if (local_err) { > error_propagate(errp, local_err); > - return ret; > + return; > } > > - ret = spapr_irq_xive.claim(spapr, irq, lsi, &local_err); > + spapr_irq_xive.claim(spapr, irq, lsi, &local_err); > if (local_err) { > error_propagate(errp, local_err); > - return ret; > + return; > } > - > - return ret; > } > > static void spapr_irq_free_dual(SpaprMachineState *spapr, int irq) > @@ -622,12 +616,12 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) > spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE); > } > > -int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) > +void spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) > { > assert(irq >= SPAPR_XIRQ_BASE); > assert(irq < (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE)); > > - return spapr->irq->claim(spapr, irq, lsi, errp); > + spapr->irq->claim(spapr, irq, lsi, errp); > } > > void spapr_irq_free(SpaprMachineState *spapr, int irq, int num) > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h > index ed88b4599a..75279ca137 100644 > --- a/include/hw/ppc/spapr_irq.h > +++ b/include/hw/ppc/spapr_irq.h > @@ -42,7 +42,7 @@ typedef struct SpaprIrq { > uint8_t ov5; > > void (*init)(SpaprMachineState *spapr, Error **errp); > - int (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > + void (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > void (*free)(SpaprMachineState *spapr, int irq); > void (*print_info)(SpaprMachineState *spapr, Monitor *mon); > void (*dt_populate)(SpaprMachineState *spapr, uint32_t nr_servers, > @@ -61,7 +61,7 @@ extern SpaprIrq spapr_irq_xive; > extern SpaprIrq spapr_irq_dual; > > void spapr_irq_init(SpaprMachineState *spapr, Error **errp); > -int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > +void spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > void spapr_irq_free(SpaprMachineState *spapr, int irq, int num); > qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq); > int spapr_irq_post_load(SpaprMachineState *spapr, int version_id); > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On Wed, 25 Sep 2019 16:45:31 +1000 David Gibson <david@gibson.dropbear.id.au> wrote: > spapr_irq_claim() and the hooks it is based on return an integer error code > as well as taking an Error ** parameter. But none of the callers check the > integer, so we can remove it and just use the richer Error **. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- FYI, the very same topic of using the return value versus Error ** to check errors was discussed recently in this thread: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg04197.html I tend to agree with Daniel (Cc'd) at first thought, but I don't have a strong opinion about this yet. It is mostly to inform you that people are currently discussing/working on it. > hw/ppc/spapr_irq.c | 32 +++++++++++++------------------- > include/hw/ppc/spapr_irq.h | 4 ++-- > 2 files changed, 15 insertions(+), 21 deletions(-) > > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > index 261d66ba17..2673a90604 100644 > --- a/hw/ppc/spapr_irq.c > +++ b/hw/ppc/spapr_irq.c > @@ -112,8 +112,8 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, Error **errp) > spapr->ics = ICS_SPAPR(obj); > } > > -static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > - Error **errp) > +static void spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > + Error **errp) > { > ICSState *ics = spapr->ics; > > @@ -122,11 +122,10 @@ static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > > if (!ics_irq_free(ics, irq - ics->offset)) { > error_setg(errp, "IRQ %d is not free", irq); > - return -1; > + return; > } > > ics_set_irq_type(ics, irq - ics->offset, lsi); > - return 0; > } > > static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq) > @@ -252,14 +251,12 @@ static void spapr_irq_init_xive(SpaprMachineState *spapr, Error **errp) > spapr_xive_hcall_init(spapr); > } > > -static int spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi, > - Error **errp) > +static void spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi, > + Error **errp) > { > if (!spapr_xive_irq_claim(spapr->xive, irq, lsi)) { > error_setg(errp, "IRQ %d is invalid", irq); > - return -1; > } > - return 0; > } > > static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq) > @@ -406,25 +403,22 @@ static void spapr_irq_init_dual(SpaprMachineState *spapr, Error **errp) > } > } > > -static int spapr_irq_claim_dual(SpaprMachineState *spapr, int irq, bool lsi, > - Error **errp) > +static void spapr_irq_claim_dual(SpaprMachineState *spapr, int irq, bool lsi, > + Error **errp) > { > Error *local_err = NULL; > - int ret; > > - ret = spapr_irq_xics.claim(spapr, irq, lsi, &local_err); > + spapr_irq_xics.claim(spapr, irq, lsi, &local_err); > if (local_err) { > error_propagate(errp, local_err); > - return ret; > + return; > } > > - ret = spapr_irq_xive.claim(spapr, irq, lsi, &local_err); > + spapr_irq_xive.claim(spapr, irq, lsi, &local_err); > if (local_err) { > error_propagate(errp, local_err); > - return ret; > + return; > } > - > - return ret; > } > > static void spapr_irq_free_dual(SpaprMachineState *spapr, int irq) > @@ -622,12 +616,12 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) > spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE); > } > > -int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) > +void spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) > { > assert(irq >= SPAPR_XIRQ_BASE); > assert(irq < (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE)); > > - return spapr->irq->claim(spapr, irq, lsi, errp); > + spapr->irq->claim(spapr, irq, lsi, errp); > } > > void spapr_irq_free(SpaprMachineState *spapr, int irq, int num) > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h > index ed88b4599a..75279ca137 100644 > --- a/include/hw/ppc/spapr_irq.h > +++ b/include/hw/ppc/spapr_irq.h > @@ -42,7 +42,7 @@ typedef struct SpaprIrq { > uint8_t ov5; > > void (*init)(SpaprMachineState *spapr, Error **errp); > - int (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > + void (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > void (*free)(SpaprMachineState *spapr, int irq); > void (*print_info)(SpaprMachineState *spapr, Monitor *mon); > void (*dt_populate)(SpaprMachineState *spapr, uint32_t nr_servers, > @@ -61,7 +61,7 @@ extern SpaprIrq spapr_irq_xive; > extern SpaprIrq spapr_irq_dual; > > void spapr_irq_init(SpaprMachineState *spapr, Error **errp); > -int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > +void spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > void spapr_irq_free(SpaprMachineState *spapr, int irq, int num); > qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq); > int spapr_irq_post_load(SpaprMachineState *spapr, int version_id);
On Thu, Sep 26, 2019 at 10:36:02AM +0200, Greg Kurz wrote: > On Wed, 25 Sep 2019 16:45:31 +1000 > David Gibson <david@gibson.dropbear.id.au> wrote: > > > spapr_irq_claim() and the hooks it is based on return an integer error code > > as well as taking an Error ** parameter. But none of the callers check the > > integer, so we can remove it and just use the richer Error **. > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > --- > > FYI, the very same topic of using the return value versus Error ** to > check errors was discussed recently in this thread: > > https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg04197.html > > I tend to agree with Daniel (Cc'd) at first thought, but I don't have a strong > opinion about this yet. It is mostly to inform you that people are currently > discussing/working on it. So, I made this change mostly because redundant encodings give me an itch. But having read that post and then experimented a bit, I found the approach he proposes does cut down a noticeable amount of boilerplate. So I think I'll move towards that approach and drop this patch. > > > hw/ppc/spapr_irq.c | 32 +++++++++++++------------------- > > include/hw/ppc/spapr_irq.h | 4 ++-- > > 2 files changed, 15 insertions(+), 21 deletions(-) > > > > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > > index 261d66ba17..2673a90604 100644 > > --- a/hw/ppc/spapr_irq.c > > +++ b/hw/ppc/spapr_irq.c > > @@ -112,8 +112,8 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, Error **errp) > > spapr->ics = ICS_SPAPR(obj); > > } > > > > -static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > > - Error **errp) > > +static void spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > > + Error **errp) > > { > > ICSState *ics = spapr->ics; > > > > @@ -122,11 +122,10 @@ static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > > > > if (!ics_irq_free(ics, irq - ics->offset)) { > > error_setg(errp, "IRQ %d is not free", irq); > > - return -1; > > + return; > > } > > > > ics_set_irq_type(ics, irq - ics->offset, lsi); > > - return 0; > > } > > > > static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq) > > @@ -252,14 +251,12 @@ static void spapr_irq_init_xive(SpaprMachineState *spapr, Error **errp) > > spapr_xive_hcall_init(spapr); > > } > > > > -static int spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi, > > - Error **errp) > > +static void spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi, > > + Error **errp) > > { > > if (!spapr_xive_irq_claim(spapr->xive, irq, lsi)) { > > error_setg(errp, "IRQ %d is invalid", irq); > > - return -1; > > } > > - return 0; > > } > > > > static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq) > > @@ -406,25 +403,22 @@ static void spapr_irq_init_dual(SpaprMachineState *spapr, Error **errp) > > } > > } > > > > -static int spapr_irq_claim_dual(SpaprMachineState *spapr, int irq, bool lsi, > > - Error **errp) > > +static void spapr_irq_claim_dual(SpaprMachineState *spapr, int irq, bool lsi, > > + Error **errp) > > { > > Error *local_err = NULL; > > - int ret; > > > > - ret = spapr_irq_xics.claim(spapr, irq, lsi, &local_err); > > + spapr_irq_xics.claim(spapr, irq, lsi, &local_err); > > if (local_err) { > > error_propagate(errp, local_err); > > - return ret; > > + return; > > } > > > > - ret = spapr_irq_xive.claim(spapr, irq, lsi, &local_err); > > + spapr_irq_xive.claim(spapr, irq, lsi, &local_err); > > if (local_err) { > > error_propagate(errp, local_err); > > - return ret; > > + return; > > } > > - > > - return ret; > > } > > > > static void spapr_irq_free_dual(SpaprMachineState *spapr, int irq) > > @@ -622,12 +616,12 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) > > spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE); > > } > > > > -int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) > > +void spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) > > { > > assert(irq >= SPAPR_XIRQ_BASE); > > assert(irq < (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE)); > > > > - return spapr->irq->claim(spapr, irq, lsi, errp); > > + spapr->irq->claim(spapr, irq, lsi, errp); > > } > > > > void spapr_irq_free(SpaprMachineState *spapr, int irq, int num) > > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h > > index ed88b4599a..75279ca137 100644 > > --- a/include/hw/ppc/spapr_irq.h > > +++ b/include/hw/ppc/spapr_irq.h > > @@ -42,7 +42,7 @@ typedef struct SpaprIrq { > > uint8_t ov5; > > > > void (*init)(SpaprMachineState *spapr, Error **errp); > > - int (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > > + void (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > > void (*free)(SpaprMachineState *spapr, int irq); > > void (*print_info)(SpaprMachineState *spapr, Monitor *mon); > > void (*dt_populate)(SpaprMachineState *spapr, uint32_t nr_servers, > > @@ -61,7 +61,7 @@ extern SpaprIrq spapr_irq_xive; > > extern SpaprIrq spapr_irq_dual; > > > > void spapr_irq_init(SpaprMachineState *spapr, Error **errp); > > -int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > > +void spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); > > void spapr_irq_free(SpaprMachineState *spapr, int irq, int num); > > qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq); > > int spapr_irq_post_load(SpaprMachineState *spapr, int version_id); >
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 261d66ba17..2673a90604 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -112,8 +112,8 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, Error **errp) spapr->ics = ICS_SPAPR(obj); } -static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, - Error **errp) +static void spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, + Error **errp) { ICSState *ics = spapr->ics; @@ -122,11 +122,10 @@ static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, if (!ics_irq_free(ics, irq - ics->offset)) { error_setg(errp, "IRQ %d is not free", irq); - return -1; + return; } ics_set_irq_type(ics, irq - ics->offset, lsi); - return 0; } static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq) @@ -252,14 +251,12 @@ static void spapr_irq_init_xive(SpaprMachineState *spapr, Error **errp) spapr_xive_hcall_init(spapr); } -static int spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi, - Error **errp) +static void spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi, + Error **errp) { if (!spapr_xive_irq_claim(spapr->xive, irq, lsi)) { error_setg(errp, "IRQ %d is invalid", irq); - return -1; } - return 0; } static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq) @@ -406,25 +403,22 @@ static void spapr_irq_init_dual(SpaprMachineState *spapr, Error **errp) } } -static int spapr_irq_claim_dual(SpaprMachineState *spapr, int irq, bool lsi, - Error **errp) +static void spapr_irq_claim_dual(SpaprMachineState *spapr, int irq, bool lsi, + Error **errp) { Error *local_err = NULL; - int ret; - ret = spapr_irq_xics.claim(spapr, irq, lsi, &local_err); + spapr_irq_xics.claim(spapr, irq, lsi, &local_err); if (local_err) { error_propagate(errp, local_err); - return ret; + return; } - ret = spapr_irq_xive.claim(spapr, irq, lsi, &local_err); + spapr_irq_xive.claim(spapr, irq, lsi, &local_err); if (local_err) { error_propagate(errp, local_err); - return ret; + return; } - - return ret; } static void spapr_irq_free_dual(SpaprMachineState *spapr, int irq) @@ -622,12 +616,12 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE); } -int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) +void spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) { assert(irq >= SPAPR_XIRQ_BASE); assert(irq < (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE)); - return spapr->irq->claim(spapr, irq, lsi, errp); + spapr->irq->claim(spapr, irq, lsi, errp); } void spapr_irq_free(SpaprMachineState *spapr, int irq, int num) diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index ed88b4599a..75279ca137 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -42,7 +42,7 @@ typedef struct SpaprIrq { uint8_t ov5; void (*init)(SpaprMachineState *spapr, Error **errp); - int (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); + void (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); void (*free)(SpaprMachineState *spapr, int irq); void (*print_info)(SpaprMachineState *spapr, Monitor *mon); void (*dt_populate)(SpaprMachineState *spapr, uint32_t nr_servers, @@ -61,7 +61,7 @@ extern SpaprIrq spapr_irq_xive; extern SpaprIrq spapr_irq_dual; void spapr_irq_init(SpaprMachineState *spapr, Error **errp); -int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); +void spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); void spapr_irq_free(SpaprMachineState *spapr, int irq, int num); qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq); int spapr_irq_post_load(SpaprMachineState *spapr, int version_id);
spapr_irq_claim() and the hooks it is based on return an integer error code as well as taking an Error ** parameter. But none of the callers check the integer, so we can remove it and just use the richer Error **. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- hw/ppc/spapr_irq.c | 32 +++++++++++++------------------- include/hw/ppc/spapr_irq.h | 4 ++-- 2 files changed, 15 insertions(+), 21 deletions(-)