[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 02/13] omap: Don't use hw_error() in device i
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH v2 02/13] omap: Don't use hw_error() in device init() methods |
Date: |
Thu, 17 Dec 2015 15:30:56 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 17/12/15 13:19, Markus Armbruster wrote:
> Device init() methods aren't supposed to call hw_error(), they should
> report the error and fail cleanly. Do that.
>
> The errors are all device misconfiguration. All callers use
> qdev_init_nofail(), so this patch merely converts hw_error() crashes
> into &error_abort crashes. Improvement, because now it crashes closer
> to where the misconfiguration bug would be, and a few more bad
> examples of hw_error() use are gone.
>
> Cc: Peter Maydell <address@hidden>
> Signed-off-by: Markus Armbruster <address@hidden>
> Reviewed-by: Peter Maydell <address@hidden>
> ---
> hw/gpio/omap_gpio.c | 19 +++++++++++++++----
> hw/i2c/omap_i2c.c | 8 ++++++--
> hw/intc/omap_intc.c | 10 +++++++---
> 3 files changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/hw/gpio/omap_gpio.c b/hw/gpio/omap_gpio.c
> index 3c53898..1e0654b 100644
> --- a/hw/gpio/omap_gpio.c
> +++ b/hw/gpio/omap_gpio.c
> @@ -21,6 +21,7 @@
> #include "hw/hw.h"
> #include "hw/arm/omap.h"
> #include "hw/sysbus.h"
> +#include "qemu/error-report.h"
>
> struct omap_gpio_s {
> qemu_irq irq;
> @@ -700,8 +701,17 @@ static int omap2_gpio_init(SysBusDevice *sbd)
What about omap_gpio_init() ? There is also a hw_error in there!
> int i;
>
> if (!s->iclk) {
> - hw_error("omap2-gpio: iclk not connected\n");
> + error_report("omap2-gpio: iclk not connected");
> + return -1;
> }
> +
> + for (i = 0; i < s->modulecount; i++) {
You're now using modulecount here -^
> + if (!s->fclk[i]) {
> + error_report("omap2-gpio: fclk%d not connected", i);
> + return -1;
> + }
> + }
> +
> if (s->mpu_model < omap3430) {
> s->modulecount = (s->mpu_model < omap2430) ? 4 : 5;
... but it is initialized just here. So I think it is wrong that you
moved the "!s->fclk[i]" before that.
> memory_region_init_io(&s->iomem, OBJECT(s), &omap2_gpif_top_ops, s,
> @@ -710,15 +720,15 @@ static int omap2_gpio_init(SysBusDevice *sbd)
> } else {
> s->modulecount = 6;
> }
> +
> s->modules = g_new0(struct omap2_gpio_s, s->modulecount);
> s->handler = g_new0(qemu_irq, s->modulecount * 32);
> qdev_init_gpio_in(dev, omap2_gpio_set, s->modulecount * 32);
> qdev_init_gpio_out(dev, s->handler, s->modulecount * 32);
> +
Unrelated whitespace changes?
> for (i = 0; i < s->modulecount; i++) {
> struct omap2_gpio_s *m = &s->modules[i];
> - if (!s->fclk[i]) {
> - hw_error("omap2-gpio: fclk%d not connected\n", i);
> - }
> +
> m->revision = (s->mpu_model < omap3430) ? 0x18 : 0x25;
> m->handler = &s->handler[i * 32];
> sysbus_init_irq(sbd, &m->irq[0]); /* mpu irq */
> @@ -728,6 +738,7 @@ static int omap2_gpio_init(SysBusDevice *sbd)
> "omap.gpio-module", 0x1000);
> sysbus_init_mmio(sbd, &m->iomem);
> }
> +
dito.
Thomas
- [Qemu-devel] [PATCH v2 00/13] Clean up some hw_error() misuse, Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 06/13] error: Don't append a newline when printing the error hint, Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 05/13] raven: Mark use of hw_error() in realize() FIXME, Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 09/13] isa: Trivially convert remaining PCI-ISA bridges to realize(), Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 12/13] audio: Clean up inappropriate and unreachable use of hw_error(), Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 02/13] omap: Don't use hw_error() in device init() methods, Markus Armbruster, 2015/12/17
- Re: [Qemu-devel] [PATCH v2 02/13] omap: Don't use hw_error() in device init() methods,
Thomas Huth <=
- [Qemu-devel] [PATCH v2 04/13] etraxfs_eth: Don't use hw_error() in init() method, Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 11/13] isa: Clean up inappropriate hw_error(), Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 10/13] isa: Clean up error handling around isa_bus_new(), Markus Armbruster, 2015/12/17