Re: [PATCH v5 7/7] hw/arm/tosa: Replace fprintf() calls by LED devices

[Luc Michel]

On 9/10/20 10:54 PM, Philippe Mathieu-Daudé wrote:
> The recently added LED device reports LED status changes with
> the 'led_set_intensity' trace event. It is less invasive than
> the fprintf() calls. We need however to have a binary built
> with tracing support.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Luc Michel <luc.michel@greensocs.com>

> ---
>   hw/arm/tosa.c  | 40 +++++++++++++++-------------------------
>   hw/arm/Kconfig |  1 +
>   2 files changed, 16 insertions(+), 25 deletions(-)
>
> diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
> index 90eef1f14dd..f23651fd775 100644
> --- a/hw/arm/tosa.c
> +++ b/hw/arm/tosa.c
> @@ -24,6 +24,7 @@
>   #include "hw/irq.h"
>   #include "hw/ssi/ssi.h"
>   #include "hw/sysbus.h"
> +#include "hw/misc/led.h"
>   #include "exec/address-spaces.h"
>   
>   #define TOSA_RAM 0x04000000
> @@ -81,26 +82,6 @@ typedef struct TosaMiscGPIOState {
>       SysBusDevice parent_obj;
>   } TosaMiscGPIOState;
>   
> -static void tosa_gpio_leds(void *opaque, int line, int level)
> -{
> -    switch (line) {
> -    case 0:
> -        fprintf(stderr, "blue LED %s.\n", level ? "on" : "off");
> -        break;
> -    case 1:
> -        fprintf(stderr, "green LED %s.\n", level ? "on" : "off");
> -        break;
> -    case 2:
> -        fprintf(stderr, "amber LED %s.\n", level ? "on" : "off");
> -        break;
> -    case 3:
> -        fprintf(stderr, "wlan LED %s.\n", level ? "on" : "off");
> -        break;
> -    default:
> -        g_assert_not_reached();
> -    }
> -}
> -
>   static void tosa_reset(void *opaque, int line, int level)
>   {
>       if (level) {
> @@ -112,7 +93,6 @@ static void tosa_misc_gpio_init(Object *obj)
>   {
>       DeviceState *dev = DEVICE(obj);
>   
> -    qdev_init_gpio_in_named(dev, tosa_gpio_leds, "leds", 4);
>       qdev_init_gpio_in_named(dev, tosa_reset, "reset", 1);
>   }
>   
> @@ -122,6 +102,7 @@ static void tosa_gpio_setup(PXA2xxState *cpu,
>                   TC6393xbState *tmio)
>   {
>       DeviceState *misc_gpio;
> +    LEDState *led[4];
>   
>       misc_gpio = sysbus_create_simple(TYPE_TOSA_MISC_GPIO, -1, NULL);
>   
> @@ -143,14 +124,23 @@ static void tosa_gpio_setup(PXA2xxState *cpu,
>                           qdev_get_gpio_in(cpu->gpio, TOSA_GPIO_JC_CF_IRQ),
>                           NULL);
>   
> +    led[0] = led_create_simple(OBJECT(misc_gpio), GPIO_POLARITY_ACTIVE_HIGH,
> +                               LED_COLOR_BLUE, "bluetooth");
> +    led[1] = led_create_simple(OBJECT(misc_gpio), GPIO_POLARITY_ACTIVE_HIGH,
> +                               LED_COLOR_GREEN, "note");
> +    led[2] = led_create_simple(OBJECT(misc_gpio), GPIO_POLARITY_ACTIVE_HIGH,
> +                               LED_COLOR_AMBER, "charger-error");
> +    led[3] = led_create_simple(OBJECT(misc_gpio), GPIO_POLARITY_ACTIVE_HIGH,
> +                               LED_COLOR_GREEN, "wlan");
> +
>       qdev_connect_gpio_out(scp1, TOSA_GPIO_BT_LED,
> -                          qdev_get_gpio_in_named(misc_gpio, "leds", 0));
> +                          qdev_get_gpio_in(DEVICE(led[0]), 0));
>       qdev_connect_gpio_out(scp1, TOSA_GPIO_NOTE_LED,
> -                          qdev_get_gpio_in_named(misc_gpio, "leds", 1));
> +                          qdev_get_gpio_in(DEVICE(led[1]), 0));
>       qdev_connect_gpio_out(scp1, TOSA_GPIO_CHRG_ERR_LED,
> -                          qdev_get_gpio_in_named(misc_gpio, "leds", 2));
> +                          qdev_get_gpio_in(DEVICE(led[2]), 0));
>       qdev_connect_gpio_out(scp1, TOSA_GPIO_WLAN_LED,
> -                          qdev_get_gpio_in_named(misc_gpio, "leds", 3));
> +                          qdev_get_gpio_in(DEVICE(led[3]), 0));
>   
>       qdev_connect_gpio_out(scp1, TOSA_GPIO_TC6393XB_L3V_ON, tc6393xb_l3v_get(tmio));
>   
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 06ba1c355b1..bbcfa098ae2 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -150,6 +150,7 @@ config TOSA
>       select ZAURUS  # scoop
>       select MICRODRIVE
>       select PXA2XX
> +    select LED
>   
>   config SPITZ
>       bool