Re: [Qemu-devel] [PATCH 08/17] lm32: pic and juart helper functions

[Blue Swirl]

On Fri, Feb 11, 2011 at 1:12 AM, Michael Walle <address@hidden> wrote:
> This patch adds init functions for the PIC and JTAG UART commonly used
> in the board initialization.
>
> Signed-off-by: Michael Walle <address@hidden>
> ---
>  hw/lm32.h |   31 +++++++++++++++++++++++++++++++
>  1 files changed, 31 insertions(+), 0 deletions(-)
>  create mode 100644 hw/lm32.h
>
> diff --git a/hw/lm32.h b/hw/lm32.h
> new file mode 100644
> index 0000000..9369499
> --- /dev/null
> +++ b/hw/lm32.h
> @@ -0,0 +1,31 @@
> +
> +#include "qemu-common.h"
> +
> +static inline DeviceState *lm32_pic_init(CPUState *env, qemu_irq cpu_irq)
> +{
> +    DeviceState *dev;
> +    SysBusDevice *d;
> +
> +    dev = qdev_create(NULL, "lm32-pic");
> +    qdev_init_nofail(dev);
> +    d = sysbus_from_qdev(dev);
> +    sysbus_connect_irq(d, 0, cpu_irq);
> +
> +    env->pic_env = (struct LM32PicState *)dev;

The assignment belongs to the caller, then the device does not need to
know about CPUState. Please also change the field type in CPUState
from LM32PicState to DeviceState so that it is more opaque and the
ugly cast can be removed (btw, there would be a macro for that).
Probably then LM32PicState can be hidden from outside of the device.

> +
> +    return dev;
> +}
> +
> +static inline DeviceState *lm32_juart_init(CPUState *env)
> +{
> +    DeviceState *dev;
> +    SysBusDevice *d;
> +
> +    dev = qdev_create(NULL, "lm32-juart");
> +    qdev_init_nofail(dev);
> +    d = sysbus_from_qdev(dev);
> +
> +    env->juart_env = (struct LM32JuartState *)dev;

Ditto.