[2/2] hw/sd/allwinner-sdhost: Don't send non-boolean IRQ line levels

QEMU allows qemu_irq lines to transfer arbitrary integers.  However
the convention is that for a simple IRQ line the values transferred
are always 0 and 1.  The A10 SD controller device instead assumes a
0-vs-non-0 convention, which happens to work with the interrupt
controller it is wired up to.

Coerce the value to boolean to follow our usual convention.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sd/allwinner-sdhost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hi Peter,

On 6/6/23 12:46, Peter Maydell wrote:
> QEMU allows qemu_irq lines to transfer arbitrary integers.  However
> the convention is that for a simple IRQ line the values transferred
> are always 0 and 1.  The A10 SD controller device instead assumes a
> 0-vs-non-0 convention, which happens to work with the interrupt
> controller it is wired up to.
> 
> Coerce the value to boolean to follow our usual convention.

I remember once wanting to convert qemu_set_irq() to take a boolean
argument but someone said using integer was more useful because ...?
I searched a bit but can't find that in my mail archives, maybe this
was on IRC. Any clue? (I find simpler to use a boolean rather than
having a convention of using integers restricted to [0, 1] range).

Regards,

Phil.

On Tue, 6 Jun 2023 at 13:39, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Hi Peter,
>
> On 6/6/23 12:46, Peter Maydell wrote:
> > QEMU allows qemu_irq lines to transfer arbitrary integers.  However
> > the convention is that for a simple IRQ line the values transferred
> > are always 0 and 1.  The A10 SD controller device instead assumes a
> > 0-vs-non-0 convention, which happens to work with the interrupt
> > controller it is wired up to.
> >
> > Coerce the value to boolean to follow our usual convention.
>
> I remember once wanting to convert qemu_set_irq() to take a boolean
> argument but someone said using integer was more useful because ...?
> I searched a bit but can't find that in my mail archives, maybe this
> was on IRC. Any clue? (I find simpler to use a boolean rather than
> having a convention of using integers restricted to [0, 1] range).

We have a lot of use cases where we just want to transfer a boolean
value between two devices. We have a few use cases where we want to
transfer an arbitrary integer across the channel between two devices.
(For instance hw/intc/etraxfs_pic.c:pic_update() sends a vector
number to the CPU via a qemu_irq -- see commit f4f643882d9dc467.)

At the moment we use qemu_irq() for both. In theory we could
construct a parallel set of machinery for wiring up and setting
values for the "want an integer" case and restrict qemu_irq() to
bool only, but the lazy path is to use the same function for both.
(If we had machinery that made it easy to construct arbitrary
strongly-typed signal connections, that might perhaps be ideal.
But it's probably not very easy especially in C.)

thanks
-- PMM

On 6/6/23 14:55, Peter Maydell wrote:
> On Tue, 6 Jun 2023 at 13:39, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Hi Peter,
>>
>> On 6/6/23 12:46, Peter Maydell wrote:
>>> QEMU allows qemu_irq lines to transfer arbitrary integers.  However
>>> the convention is that for a simple IRQ line the values transferred
>>> are always 0 and 1.  The A10 SD controller device instead assumes a
>>> 0-vs-non-0 convention, which happens to work with the interrupt
>>> controller it is wired up to.
>>>
>>> Coerce the value to boolean to follow our usual convention.
>>
>> I remember once wanting to convert qemu_set_irq() to take a boolean
>> argument but someone said using integer was more useful because ...?
>> I searched a bit but can't find that in my mail archives, maybe this
>> was on IRC. Any clue? (I find simpler to use a boolean rather than
>> having a convention of using integers restricted to [0, 1] range).
> 
> We have a lot of use cases where we just want to transfer a boolean
> value between two devices. We have a few use cases where we want to
> transfer an arbitrary integer across the channel between two devices.
> (For instance hw/intc/etraxfs_pic.c:pic_update() sends a vector
> number to the CPU via a qemu_irq -- see commit f4f643882d9dc467.)

Thanks for this reference!

> At the moment we use qemu_irq() for both. In theory we could
> construct a parallel set of machinery for wiring up and setting
> values for the "want an integer" case and restrict qemu_irq() to
> bool only, but the lazy path is to use the same function for both.
> (If we had machinery that made it easy to construct arbitrary
> strongly-typed signal connections, that might perhaps be ideal.
> But it's probably not very easy especially in C.)

I see, thanks.

On Tue, Jun 06, 2023 at 11:46:09AM +0100, Peter Maydell wrote:
> QEMU allows qemu_irq lines to transfer arbitrary integers.  However
> the convention is that for a simple IRQ line the values transferred
> are always 0 and 1.  The A10 SD controller device instead assumes a
> 0-vs-non-0 convention, which happens to work with the interrupt
> controller it is wired up to.
> 
> Coerce the value to boolean to follow our usual convention.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Tested-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  hw/sd/allwinner-sdhost.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/sd/allwinner-sdhost.c b/hw/sd/allwinner-sdhost.c
> index 92a0f42708d..62df8f3d396 100644
> --- a/hw/sd/allwinner-sdhost.c
> +++ b/hw/sd/allwinner-sdhost.c
> @@ -191,7 +191,7 @@ static void allwinner_sdhost_update_irq(AwSdHostState *s)
>      }
>  
>      trace_allwinner_sdhost_update_irq(irq);
> -    qemu_set_irq(s->irq, irq);
> +    qemu_set_irq(s->irq, !!irq);
>  }
>  
>  static void allwinner_sdhost_update_transfer_cnt(AwSdHostState *s,
> -- 
> 2.34.1
>