Re: [PATCH for-7.1 16/18] hw/arm/exynos4210: Fold combiner splits into exynos4210_init_board_irqs()

[Richard Henderson <richard.henderson@linaro.org>]

On 4/4/22 10:46, Peter Maydell wrote:
> At this point, the function exynos4210_init_board_irqs() splits input
> IRQ lines to connect them to the input combiner, output combiner and
> external GIC.  The function exynos4210_combiner_get_gpioin() splits
> some of the combiner input lines further to connect them to multiple
> different inputs on the combiner.
> 
> Because (unlike qemu_irq_split()) the TYPE_SPLIT_IRQ device has a
> configurable number of outputs, we can do all this in one place, by
> making exynos4210_init_board_irqs() add extra outputs to the splitter
> device when it must be connected to more than one input on each
> combiner.
> 
> We do this with a new data structure, the combinermap, which is an
> array each of whose elements is a list of the interrupt IDs on the
> combiner which must be tied together.  As we loop through each
> interrupt ID, if we find that it is the first one in one of these
> lists, we configure the splitter device with eonugh extra outputs and
> wire them up to the other interrupt IDs in the list.
> 
> Conveniently, for all the cases where this is necessary, the
> lowest-numbered interrupt ID in each group is in the range of the
> external combiner, so we only need to code for this in the first of
> the two loops in exynos4210_init_board_irqs().
> 
> The old code in exynos4210_combiner_get_gpioin() which is being
> deleted here had several problems which don't exist in the new code
> in its handling of the multi-core timer interrupts:
>   (1) the case labels specified bits 4 ... 8, but bit '8' doesn't
>       exist; these should have been 4 ... 7
>   (2) it used the input irq[EXYNOS4210_COMBINER_GET_IRQ_NUM(1, bit + 4)]
>       multiple times as the input of several different splitters,
>       which isn't allowed
>   (3) in an apparent cut-and-paste error, the cases for all the
>       multi-core timer inputs used "bit + 4" even though the
>       bit range for the case was (intended to be) 4 ... 7, which
>       meant it was looking at non-existent bits 8 ... 11.
> None of these exist in the new code.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/arm/exynos4210.h |   6 +-
>   hw/arm/exynos4210.c         | 178 +++++++++++++++++++++++-------------
>   2 files changed, 119 insertions(+), 65 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~