arm/mv: Don't rely on firmware MSI mapping in ICU

[kd]

Summary

On Armada8k boards various peripherals (e.g. USB) have interrupt lines connected to on of the ICU interrupt controllers.
After an interrupt is detected it triggers MSI to a given address, with a programmed value. This in turn triggers an SPI interrupt.
Normally MSI vector should be allocated by ICUs parent and set during interrupt allocation.
Instead of doing that we relied on the ICU being pre-configured in firmware.
This worked with EDK2 and older versions of u-boot, but in the newer ones that is no longer the case.
Extend ICU msi-parents - GICP and SEI to support MSI interface and use it during interrupt allocation.
This allows us to boot on armada 7k/8k SoCs that run modern uboot.

For SATA interrupts we need to apply a WA previously done in firmware.
We have two SATA ports connected to one controller.
Each ports gets its own interrupt, but only one of them is described in dts, also ahci_generic driver expects only one irq too.
Fix it by mapping both interrupts to the same MSI when one of them is allocated, which allows us to use both SATA ports.

Test Plan

Boot macchiatobin, verify that all basic components that use ICU interrupts work. (MMC/USB/SATA)

[mmel]

I'm sorry but I don't like this approach.
The PIC_MAP_INT function is designed to map external data of exact source to given irqsrc. It should not be misused to transfer any additional, platform specific data back to requester. Moreover if these external data (pure synthetic, without any relation to FDT) are passed as INTR_MAP_DATA_FDT to mapping layer.
Imho, we should convert gicp to standard MSI based interrupt controller. The communication between ICU and GICP should be splitted to normal MSI request and standard method for getting MSI mapping should be used (msi_map_msi()).
It's a little hard for me to express all the nuances so I can prepare skeleton for this solution, if you want.

[mw]

@mmel Thanks for the review. I'd also prefer the msi approach, in fact this is what GICP-ICU really is. I thought the the MSI code in FreeBSD is PCI-dependent, but after another look I can see it may work in our case. Let us update the code to this approach.

[mmel]

Thanks, perfect. Please let me know if you need help. I think that there will be more and more similar interrupt controllers/systems, so it's important to create a clean and flexible interface.

[manu]

sys/arm/mv/mv_ap806_gicp.c

138

Note that OF_getencprop_alloc doesn't returns the number of elements but the size in bytes.
So here we end up reading out of bound and in my case allocating a 1.8GB bitmap (yup you read right :P )

[mw]

sys/arm/mv/mv_ap806_gicp.c

138

1. The upstream version of the allocation uses OF_getencprop_alloc_multi.

2. With below diff:

--- a/sys/arm/mv/mv_ap806_gicp.c
+++ b/sys/arm/mv/mv_ap806_gicp.c
@@ -130,6 +130,8 @@ mv_ap806_gicp_attach(device_t dev)
        sc->spi_ranges_cnt = OF_getencprop_alloc_multi(node, "marvell,spi-ranges",
            sizeof(*sc->spi_ranges), (void **)&sc->spi_ranges);
 
+       printf("%s: sc->spi_ranges_cnt = %zd\n\n", __func__, sc->spi_ranges_cnt);
+
        sc->spi_bitmap_size = 0;
        for (i = 0; i < sc->spi_ranges_cnt; i += 2)
                sc->spi_bitmap_size += sc->spi_ranges[i + 1];

I see a sane value, matching the DT contents. Boot log snippet:

mv_ap806_gicp0: <Marvell GICP> mem 0x3f0040-0x3f004f on simplebus1
mv_ap806_gicp_attach: sc->spi_ranges_cnt = 4

[manu]

sys/arm/mv/mv_ap806_gicp.c

138

sys/contrib/device-tree/src/arm64/marvell/armada-ap80x.dtsi have marvell,spi-ranges = <64 64>, <288 64>
That is translated to 4 uint32_t in the dtb, so OF_getencprop_alloc will return 16.

[mw]

sys/arm/mv/mv_ap806_gicp.c

138

Yes, but there was a division (/ sizeof(*sc->spi_ranges)), so the result was 4 in the old code too :) Anyway, with the new code we also get '4' so I think we can agree, there is no issue here, can't we?

[manu]

sys/arm/mv/mv_ap806_gicp.c

138

Damn, somehow I don't have it locally, I'll make sure that I have the full correct patch applied (wasn't applied by me ...)
Sorry for the noise ;)

[kd]

Hi,

I've finally managed to find some time to rewrite this patch.
Sorry for taking so long, I'd appreciate if you could take a look.

[mw]

I tested with DT boot from EDK2 and U-Boot 2019.10 on both MacchiatoBin and CN913x-DB.

[mmel]

This looks perfect to me, many thanks.