Re: [Qemu-devel] [PATCH v2] spapr: fix LSI interrupt specifiers in the device tree

[David Gibson]

On Mon, Dec 04, 2017 at 03:41:46PM +0100, Greg Kurz wrote:
> PAPR 2.7 C.6.9.1.2 describes the "#interrupt-cells" property of the
> PowerPC External Interrupt Source Controller node as follows:
> 
> “#interrupt-cells”
> 
>   Standard property name to define the number of cells in an interrupt-
>   specifier within an interrupt domain.
> 
>   prop-encoded-array: An integer, encoded as with encode-int, that denotes
>   the number of cells required to represent an interrupt specifier in its
>   child nodes.
> 
>   The value of this property for the PowerPC External Interrupt option shall
>   be 2. Thus all interrupt specifiers (as used in the standard “interrupts”
>   property) shall consist of two cells, each containing an integer encoded
>   as with encode-int. The first integer represents the interrupt number the
>   second integer is the trigger code: 0 for edge triggered, 1 for level
>   triggered.
> 
> This patch fixes the interrupt specifiers in the "interrupt-map" property
> of the PHB node, that were setting the second cell to 8 (confusion with
> IRQ_TYPE_LEVEL_LOW ?) instead of 1.
> 
> VIO devices and RTAS event sources use the same format for interrupt
> specifiers: while here, we introduce a common helper to handle the
> encoding details.
> 
> Signed-off-by: Greg Kurz <address@hidden>
> Reviewed-by: Cédric Le Goater <address@hidden>
> Tested-by: Cédric Le Goater <address@hidden>

Looks good, but I'd request two minor changes.

1) Please reference LoPAPR instead of the internal PAPR, since other
people can't see it.

2) Change the helper's name to spapr_dt_xics_irq(), for both brevity
and specificity.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature