Re: [PATCH v2 2/5] kvmtool: replace GIC specific IRQ type #defines

[Will Deacon <will.deacon@arm.com>]

On Wed, Dec 17, 2014 at 11:14:44AM +0000, Andre Przywara wrote:
> We had GIC specific defines for the IRQ type identifiers in kvmtool.
> But In fact the specification of being a level or edge interrupt
> is quite generic, with the GIC binding using the generic Linux
> defines.
> So lets replace the GIC specific names in favour of the general
> defines used in Linux.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  tools/kvm/arm/fdt.c                    |    2 +-
>  tools/kvm/arm/include/arm-common/gic.h |    5 -----
>  tools/kvm/arm/pci.c                    |    2 +-
>  tools/kvm/arm/timer.c                  |    8 ++++----
>  tools/kvm/include/kvm/fdt.h            |    9 +++++++++
>  5 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/kvm/arm/fdt.c b/tools/kvm/arm/fdt.c
> index 4a33846..24f030f 100644
> --- a/tools/kvm/arm/fdt.c
> +++ b/tools/kvm/arm/fdt.c
> @@ -79,7 +79,7 @@ static void generate_irq_prop(void *fdt, u8 irq)
>  	u32 irq_prop[] = {
>  		cpu_to_fdt32(GIC_FDT_IRQ_TYPE_SPI),
>  		cpu_to_fdt32(irq - GIC_SPI_IRQ_BASE),
> -		cpu_to_fdt32(GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
> +		cpu_to_fdt32(IRQ_TYPE_EDGE_RISING),
>  	};
>  
>  	_FDT(fdt_property(fdt, "interrupts", irq_prop, sizeof(irq_prop)));
> diff --git a/tools/kvm/arm/include/arm-common/gic.h b/tools/kvm/arm/include/arm-common/gic.h
> index 850edc7..5a36f2c 100644
> --- a/tools/kvm/arm/include/arm-common/gic.h
> +++ b/tools/kvm/arm/include/arm-common/gic.h
> @@ -10,11 +10,6 @@
>  #define GIC_FDT_IRQ_TYPE_SPI		0
>  #define GIC_FDT_IRQ_TYPE_PPI		1
>  
> -#define GIC_FDT_IRQ_FLAGS_EDGE_LO_HI	1
> -#define GIC_FDT_IRQ_FLAGS_EDGE_HI_LO	2
> -#define GIC_FDT_IRQ_FLAGS_LEVEL_HI	4
> -#define GIC_FDT_IRQ_FLAGS_LEVEL_LO	8
> -
>  #define GIC_FDT_IRQ_PPI_CPU_SHIFT	8
>  #define GIC_FDT_IRQ_PPI_CPU_MASK	(0xff << GIC_FDT_IRQ_PPI_CPU_SHIFT)
>  
> diff --git a/tools/kvm/arm/pci.c b/tools/kvm/arm/pci.c
> index 9f4dabc..99a8130 100644
> --- a/tools/kvm/arm/pci.c
> +++ b/tools/kvm/arm/pci.c
> @@ -87,7 +87,7 @@ void pci__generate_fdt_nodes(void *fdt, u32 gic_phandle)
>  			.gic_irq = {
>  				.type	= cpu_to_fdt32(GIC_FDT_IRQ_TYPE_SPI),
>  				.num	= cpu_to_fdt32(irq - GIC_SPI_IRQ_BASE),
> -				.flags	= cpu_to_fdt32(GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
> +				.flags	= cpu_to_fdt32(IRQ_TYPE_EDGE_RISING),
>  			},
>  		};
>  
> diff --git a/tools/kvm/arm/timer.c b/tools/kvm/arm/timer.c
> index 209251e..29991da 100644
> --- a/tools/kvm/arm/timer.c
> +++ b/tools/kvm/arm/timer.c
> @@ -15,19 +15,19 @@ void timer__generate_fdt_nodes(void *fdt, struct kvm *kvm, int *irqs)
>  	u32 irq_prop[] = {
>  		cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
>  		cpu_to_fdt32(irqs[0]),
> -		cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
> +		cpu_to_fdt32(cpu_mask | IRQ_TYPE_EDGE_RISING),
>  
>  		cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
>  		cpu_to_fdt32(irqs[1]),
> -		cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
> +		cpu_to_fdt32(cpu_mask | IRQ_TYPE_EDGE_RISING),
>  
>  		cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
>  		cpu_to_fdt32(irqs[2]),
> -		cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
> +		cpu_to_fdt32(cpu_mask | IRQ_TYPE_EDGE_RISING),
>  
>  		cpu_to_fdt32(GIC_FDT_IRQ_TYPE_PPI),
>  		cpu_to_fdt32(irqs[3]),
> -		cpu_to_fdt32(cpu_mask | GIC_FDT_IRQ_FLAGS_EDGE_LO_HI),
> +		cpu_to_fdt32(cpu_mask | IRQ_TYPE_EDGE_RISING),
>  	};
>  
>  	_FDT(fdt_begin_node(fdt, "timer"));
> diff --git a/tools/kvm/include/kvm/fdt.h b/tools/kvm/include/kvm/fdt.h
> index 19f95ac..dee9a71 100644
> --- a/tools/kvm/include/kvm/fdt.h
> +++ b/tools/kvm/include/kvm/fdt.h
> @@ -7,6 +7,15 @@
>  
>  #define FDT_MAX_SIZE	0x10000
>  
> +/* Those definitions are generic FDT values for specifying IRQ
> + * types and are used in the Linux kernel internally as well as in
> + * the dts files and their documentation.
> + */
> +#define IRQ_TYPE_EDGE_RISING	1
> +#define IRQ_TYPE_EDGE_FALLING	2
> +#define IRQ_TYPE_LEVEL_HIGH	4
> +#define IRQ_TYPE_LEVEL_LOW	8

Any chance we can keep this as an enum, please? That matches that the kernel
uses internally, and allows you to take a specific type instead of a u32
for the device-tree generating functions.

Will