Re: [Qemu-devel] [PATCH] Realview/Versatile: Export LED state

[Andreas Färber]

Hello,

Am 11.01.2013 14:42, schrieb Wolfgang Mauerer:
> The configuration register for the onboard LEDs is
> emulated, but the state is not exported, which makes
> the feature not particularly useful. Create a character
> device to make status changes accessible to the host.
> 
> For example, use the command line argument
> 
> -chardev socket,id=leds,host=localhost,port=12345,server,nowait
> 
> to observe status changes via a socket.
> 
> Signed-off-by: Wolfgang Mauerer <address@hidden>
> ---
>  hw/arm_sysctl.c |   24 ++++++++++++++++++++++++
>  1 files changed, 24 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/arm_sysctl.c b/hw/arm_sysctl.c
> index b733617..7cb8559 100644
> --- a/hw/arm_sysctl.c
> +++ b/hw/arm_sysctl.c
> @@ -19,6 +19,7 @@ typedef struct {
>      SysBusDevice busdev;
>      MemoryRegion iomem;
>      qemu_irq pl110_mux_ctrl;
> +    CharDriverState *display;
>  
>      uint32_t sys_id;
>      uint32_t leds;
> @@ -92,6 +93,24 @@ static void arm_sysctl_reset(DeviceState *d)
>      }
>  }
>  
> +static void notify_led_change(CharDriverState *chr, uint32_t old, uint32_t 
> new)
> +{
> +    uint32_t changed;
> +    unsigned int i;
> +
> +    if (chr == NULL) {
> +        return;
> +    }
> +
> +    changed = old ^ new;
> +    for (i = 0; i < 8; i++) {
> +        if (changed & (1 << i)) {
> +            qemu_chr_fe_printf(chr, "%u:%s\r\n", i,
> +                               new & (1 << i) ? "on" : "off");

Instead of inventing a custom text-based protocol, would exposing a bool
QOM property per LED be an option? What exactly are you trying to do
with the state?

> +        }
> +    }
> +}
> +
>  static uint64_t arm_sysctl_read(void *opaque, hwaddr offset,
>                                  unsigned size)
>  {
> @@ -198,6 +217,7 @@ static void arm_sysctl_write(void *opaque, hwaddr offset,
>  
>      switch (offset) {
>      case 0x08: /* LED */
> +        notify_led_change(s->display, s->leds, (uint32_t)val);
>          s->leds = val;
>      case 0x0c: /* OSC0 */
>      case 0x10: /* OSC1 */
> @@ -386,6 +406,10 @@ static int arm_sysctl_init(SysBusDevice *dev)
>  {
>      arm_sysctl_state *s = FROM_SYSBUS(arm_sysctl_state, dev);
>  
> +    s->display = qemu_chr_new("leds0", "chardev:leds", NULL);
> +    if (s->display) {

Can this ever be NULL?

> +        qemu_chr_fe_printf(s->display, "8 LEDs available\r\n");
> +    }
>      memory_region_init_io(&s->iomem, &arm_sysctl_ops, s, "arm-sysctl", 
> 0x1000);
>      sysbus_init_mmio(dev, &s->iomem);
>      qdev_init_gpio_in(&s->busdev.qdev, arm_sysctl_gpio_set, 2);

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg