Re: [PATCH 10/17] hw/misc/max111x: Use GPIO lines rather than max111x_set_input()

[Alistair Francis]

On Sun, Jun 28, 2020 at 7:28 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The max111x ADC device model allows other code to set the level on
> the 8 ADC inputs using the max111x_set_input() function.  Replace
> this with generic qdev GPIO inputs, which also allow inputs to be set
> to arbitrary values.
>
> Using GPIO lines will make it easier for board code to wire things
> up, so that if device A wants to set the ADC input it doesn't need to
> have a direct pointer to the max111x but can just set that value on
> its output GPIO, which is then wired up by the board to the
> appropriate max111x input.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/hw/ssi/ssi.h |  3 ---
>  hw/arm/spitz.c       |  9 +++++----
>  hw/misc/max111x.c    | 16 +++++++++-------
>  3 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
> index 4be5325e654..5fd411f2e4e 100644
> --- a/include/hw/ssi/ssi.h
> +++ b/include/hw/ssi/ssi.h
> @@ -111,7 +111,4 @@ SSIBus *ssi_create_bus(DeviceState *parent, const char 
> *name);
>
>  uint32_t ssi_transfer(SSIBus *bus, uint32_t val);
>
> -/* max111x.c */
> -void max111x_set_input(DeviceState *dev, int line, uint8_t value);
> -
>  #endif
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index 93a25edcb5b..fa592aad6d6 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -696,13 +696,14 @@ static void corgi_ssp_gpio_cs(void *opaque, int line, 
> int level)
>
>  static void spitz_adc_temp_on(void *opaque, int line, int level)
>  {
> +    int batt_temp;
> +
>      if (!max1111)
>          return;
>
> -    if (level)
> -        max111x_set_input(max1111, MAX1111_BATT_TEMP, SPITZ_BATTERY_TEMP);
> -    else
> -        max111x_set_input(max1111, MAX1111_BATT_TEMP, 0);
> +    batt_temp = level ? SPITZ_BATTERY_TEMP : 0;
> +
> +    qemu_set_irq(qdev_get_gpio_in(max1111, MAX1111_BATT_TEMP), batt_temp);
>  }
>
>  static void corgi_ssp_realize(SSISlave *d, Error **errp)
> diff --git a/hw/misc/max111x.c b/hw/misc/max111x.c
> index abddfa3c660..3a5cb838445 100644
> --- a/hw/misc/max111x.c
> +++ b/hw/misc/max111x.c
> @@ -131,12 +131,21 @@ static const VMStateDescription vmstate_max111x = {
>      }
>  };
>
> +static void max111x_input_set(void *opaque, int line, int value)
> +{
> +    MAX111xState *s = MAX_111X(opaque);
> +
> +    assert(line >= 0 && line < s->inputs);
> +    s->input[line] = value;
> +}
> +
>  static int max111x_init(SSISlave *d, int inputs)
>  {
>      DeviceState *dev = DEVICE(d);
>      MAX111xState *s = MAX_111X(dev);
>
>      qdev_init_gpio_out(dev, &s->interrupt, 1);
> +    qdev_init_gpio_in(dev, max111x_input_set, inputs);
>
>      s->inputs = inputs;
>
> @@ -153,13 +162,6 @@ static void max1111_realize(SSISlave *dev, Error **errp)
>      max111x_init(dev, 4);
>  }
>
> -void max111x_set_input(DeviceState *dev, int line, uint8_t value)
> -{
> -    MAX111xState *s = MAX_111X(dev);
> -    assert(line >= 0 && line < s->inputs);
> -    s->input[line] = value;
> -}
> -
>  static void max111x_reset(DeviceState *dev)
>  {
>      MAX111xState *s = MAX_111X(dev);
> --
> 2.20.1
>
>