| Message ID | f521d51b217517a3d19b37f3e8fe611063453979.1348104012.git.peter.crosthwaite@petalogix.com |
|---|---|
| State | New |
| Headers | show |
On 24 September 2012 10:18, Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com> wrote: > Allow multiple qdev_init_gpio_in() calls for the one device. The first call will > define GPIOs 0-N-1, the next GPIOs N- ... . Allows different GPIOs to be handled > with different handlers. Needed when two levels of the QOM class heirachy both > define GPIO functionality, as a single GPIO handler with an index selecter is > not possible. > > Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com> > --- > changed since v5: > moved implementation to irq.c as per PMM review Thanks for this change -- I think it looks much cleaner this way. > hw/irq.c | 17 ++++++++++++++--- > hw/irq.h | 11 ++++++++++- > hw/qdev.c | 6 +++--- > 3 files changed, 27 insertions(+), 7 deletions(-) > > diff --git a/hw/irq.c b/hw/irq.c > index d413a0b..cd17551 100644 > --- a/hw/irq.c > +++ b/hw/irq.c > @@ -38,15 +38,20 @@ void qemu_set_irq(qemu_irq irq, int level) > irq->handler(irq->opaque, irq->n, level); > } > > -qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n) > +qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler, > + void *opaque, int n) Your indent is a little odd here (and below) -- usually the parameters in the second line are aligned with the first (so the 'v' is in the same column as the 'q'). > { > qemu_irq *s; > struct IRQState *p; > int i; > > - s = (qemu_irq *)g_malloc0(sizeof(qemu_irq) * n); > + if (!old) { > + n_old = 0; > + } > + s = old ? g_renew(qemu_irq, old, n + n_old) : > + (qemu_irq *)g_new0(qemu_irq, n); You don't need to cast the return value from g_new0(). Also, this isn't consistent -- either we don't need the memory zeroed (in which case use g_new rather than g_new0) or we do (in which case we need to do something about the new memory g_renew() returns). > p = (struct IRQState *)g_malloc0(sizeof(struct IRQState) * n); qemu_free_irqs() relies on all the struct IRQState *s being freeable with a single g_free(), so you need to g_renew the existing array here. > - for (i = 0; i < n; i++) { > + for (i = n_old; i < n + n_old; i++) { > p->handler = handler; > p->opaque = opaque; > p->n = i; > @@ -56,6 +61,12 @@ qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n) > return s; > } > > +qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n) > +{ > + return qemu_extend_irqs(NULL, 0, handler, opaque, n); > +} > + > + > void qemu_free_irqs(qemu_irq *s) > { > g_free(s[0]); > diff --git a/hw/irq.h b/hw/irq.h > index 56c55f0..e640c10 100644 > --- a/hw/irq.h > +++ b/hw/irq.h > @@ -23,8 +23,17 @@ static inline void qemu_irq_pulse(qemu_irq irq) > qemu_set_irq(irq, 0); > } > > -/* Returns an array of N IRQs. */ > +/* Returns an array of N IRQs. Each IRQ is assigned the argument handler and > + * opaque data. > + */ > qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n); > + > +/* Extends an Array of IRQs. Old IRQs have their handlers and opaque data > + * preserved. New IRQs are assigned the argument handler and opaque data. > + */ > +qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler, > + void *opaque, int n); > + > void qemu_free_irqs(qemu_irq *s); > > /* Returns a new IRQ with opposite polarity. */ > diff --git a/hw/qdev.c b/hw/qdev.c > index b5a52ac..eea9eae 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -291,9 +291,9 @@ BusState *qdev_get_parent_bus(DeviceState *dev) > > void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n) > { > - assert(dev->num_gpio_in == 0); > - dev->num_gpio_in = n; > - dev->gpio_in = qemu_allocate_irqs(handler, dev, n); > + dev->gpio_in = qemu_extend_irqs(dev->gpio_in, dev->num_gpio_in, handler, > + dev, n); > + dev->num_gpio_in += n; > } > > void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n) > -- > 1.7.0.4 > -- PMM
All changes made. Thanks Peter On Mon, Sep 24, 2012 at 7:53 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 24 September 2012 10:18, Peter A. G. Crosthwaite > <peter.crosthwaite@petalogix.com> wrote: >> Allow multiple qdev_init_gpio_in() calls for the one device. The first call will >> define GPIOs 0-N-1, the next GPIOs N- ... . Allows different GPIOs to be handled >> with different handlers. Needed when two levels of the QOM class heirachy both >> define GPIO functionality, as a single GPIO handler with an index selecter is >> not possible. >> >> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com> >> --- >> changed since v5: >> moved implementation to irq.c as per PMM review > > Thanks for this change -- I think it looks much cleaner this way. > >> hw/irq.c | 17 ++++++++++++++--- >> hw/irq.h | 11 ++++++++++- >> hw/qdev.c | 6 +++--- >> 3 files changed, 27 insertions(+), 7 deletions(-) >> >> diff --git a/hw/irq.c b/hw/irq.c >> index d413a0b..cd17551 100644 >> --- a/hw/irq.c >> +++ b/hw/irq.c >> @@ -38,15 +38,20 @@ void qemu_set_irq(qemu_irq irq, int level) >> irq->handler(irq->opaque, irq->n, level); >> } >> >> -qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n) >> +qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler, >> + void *opaque, int n) > > Your indent is a little odd here (and below) -- usually the parameters > in the second line are aligned with the first (so the 'v' is in the > same column as the 'q'). > >> { >> qemu_irq *s; >> struct IRQState *p; >> int i; >> >> - s = (qemu_irq *)g_malloc0(sizeof(qemu_irq) * n); >> + if (!old) { >> + n_old = 0; >> + } >> + s = old ? g_renew(qemu_irq, old, n + n_old) : >> + (qemu_irq *)g_new0(qemu_irq, n); > > You don't need to cast the return value from g_new0(). > Also, this isn't consistent -- either we don't need the > memory zeroed (in which case use g_new rather than g_new0) > or we do (in which case we need to do something about > the new memory g_renew() returns). > >> p = (struct IRQState *)g_malloc0(sizeof(struct IRQState) * n); > > qemu_free_irqs() relies on all the struct IRQState *s being > freeable with a single g_free(), so you need to g_renew the > existing array here. > >> - for (i = 0; i < n; i++) { >> + for (i = n_old; i < n + n_old; i++) { >> p->handler = handler; >> p->opaque = opaque; >> p->n = i; >> @@ -56,6 +61,12 @@ qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n) >> return s; >> } >> >> +qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n) >> +{ >> + return qemu_extend_irqs(NULL, 0, handler, opaque, n); >> +} >> + >> + >> void qemu_free_irqs(qemu_irq *s) >> { >> g_free(s[0]); >> diff --git a/hw/irq.h b/hw/irq.h >> index 56c55f0..e640c10 100644 >> --- a/hw/irq.h >> +++ b/hw/irq.h >> @@ -23,8 +23,17 @@ static inline void qemu_irq_pulse(qemu_irq irq) >> qemu_set_irq(irq, 0); >> } >> >> -/* Returns an array of N IRQs. */ >> +/* Returns an array of N IRQs. Each IRQ is assigned the argument handler and >> + * opaque data. >> + */ >> qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n); >> + >> +/* Extends an Array of IRQs. Old IRQs have their handlers and opaque data >> + * preserved. New IRQs are assigned the argument handler and opaque data. >> + */ >> +qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler, >> + void *opaque, int n); >> + >> void qemu_free_irqs(qemu_irq *s); >> >> /* Returns a new IRQ with opposite polarity. */ >> diff --git a/hw/qdev.c b/hw/qdev.c >> index b5a52ac..eea9eae 100644 >> --- a/hw/qdev.c >> +++ b/hw/qdev.c >> @@ -291,9 +291,9 @@ BusState *qdev_get_parent_bus(DeviceState *dev) >> >> void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n) >> { >> - assert(dev->num_gpio_in == 0); >> - dev->num_gpio_in = n; >> - dev->gpio_in = qemu_allocate_irqs(handler, dev, n); >> + dev->gpio_in = qemu_extend_irqs(dev->gpio_in, dev->num_gpio_in, handler, >> + dev, n); >> + dev->num_gpio_in += n; >> } >> >> void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n) >> -- >> 1.7.0.4 >> > > > > -- PMM
diff --git a/hw/irq.c b/hw/irq.c index d413a0b..cd17551 100644 --- a/hw/irq.c +++ b/hw/irq.c @@ -38,15 +38,20 @@ void qemu_set_irq(qemu_irq irq, int level) irq->handler(irq->opaque, irq->n, level); } -qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n) +qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler, + void *opaque, int n) { qemu_irq *s; struct IRQState *p; int i; - s = (qemu_irq *)g_malloc0(sizeof(qemu_irq) * n); + if (!old) { + n_old = 0; + } + s = old ? g_renew(qemu_irq, old, n + n_old) : + (qemu_irq *)g_new0(qemu_irq, n); p = (struct IRQState *)g_malloc0(sizeof(struct IRQState) * n); - for (i = 0; i < n; i++) { + for (i = n_old; i < n + n_old; i++) { p->handler = handler; p->opaque = opaque; p->n = i; @@ -56,6 +61,12 @@ qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n) return s; } +qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n) +{ + return qemu_extend_irqs(NULL, 0, handler, opaque, n); +} + + void qemu_free_irqs(qemu_irq *s) { g_free(s[0]); diff --git a/hw/irq.h b/hw/irq.h index 56c55f0..e640c10 100644 --- a/hw/irq.h +++ b/hw/irq.h @@ -23,8 +23,17 @@ static inline void qemu_irq_pulse(qemu_irq irq) qemu_set_irq(irq, 0); } -/* Returns an array of N IRQs. */ +/* Returns an array of N IRQs. Each IRQ is assigned the argument handler and + * opaque data. + */ qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n); + +/* Extends an Array of IRQs. Old IRQs have their handlers and opaque data + * preserved. New IRQs are assigned the argument handler and opaque data. + */ +qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler, + void *opaque, int n); + void qemu_free_irqs(qemu_irq *s); /* Returns a new IRQ with opposite polarity. */ diff --git a/hw/qdev.c b/hw/qdev.c index b5a52ac..eea9eae 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -291,9 +291,9 @@ BusState *qdev_get_parent_bus(DeviceState *dev) void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n) { - assert(dev->num_gpio_in == 0); - dev->num_gpio_in = n; - dev->gpio_in = qemu_allocate_irqs(handler, dev, n); + dev->gpio_in = qemu_extend_irqs(dev->gpio_in, dev->num_gpio_in, handler, + dev, n); + dev->num_gpio_in += n; } void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
Allow multiple qdev_init_gpio_in() calls for the one device. The first call will define GPIOs 0-N-1, the next GPIOs N- ... . Allows different GPIOs to be handled with different handlers. Needed when two levels of the QOM class heirachy both define GPIO functionality, as a single GPIO handler with an index selecter is not possible. Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com> --- changed since v5: moved implementation to irq.c as per PMM review hw/irq.c | 17 ++++++++++++++--- hw/irq.h | 11 ++++++++++- hw/qdev.c | 6 +++--- 3 files changed, 27 insertions(+), 7 deletions(-)