Re: [PATCH v15 04/15] irqchip/gic-v3-its: Add support for device tree msi-map and msi-mask

[Marc Zyngier <maz@kernel.org>]

On Tue, 11 Feb 2025 19:21:57 +0000,
Frank Li <Frank.Li@nxp.com> wrote:
> 
> Some platform devices create child devices dynamically and require the
> parent device's msi-map to map device IDs to actual sideband information.
> 
> A typical use case is using ITS as a PCIe Endpoint Controller(EPC)'s
> doorbell function, where PCI hosts send TLP memory writes to the EP
> controller. The EP controller converts these writes to AXI transactions
> and appends platform-specific sideband information.  See below figure.
> 
>                ┌────────────────────────────────┐
>                │                                │
>                │     PCI Endpoint Controller    │
>                │                                │
>                │  ┌─────┐   ┌─────┐     ┌─────┐ │
>     PCI Bus    │  │     │   │     │     │     │ │
>     ─────────► │  │Func1│   │Func2│ ... │Func │ │
>     TLP Memory │  │     │   │     │     │<n>  │ │
>     Write Push │  │     │   │     │     │     │ │
>     DoorBell   │  └─┬─┬─┘   └──┬──┘     └──┬──┘ │
>                │    │ │        │           │    │
>                └────┼─┼────────┼───────────┼────┘
>         sideband    │ │ Address│           │
>         information ▼ ▼ /Data  ▼           ▼
>                    ┌────────────────────────┐
>                    │    MSI Controller      │
>                    └────────────────────────┘
>

Please stop using these figures in commit messages. I don't think they
help, and they are not in consistent with the way the commit messages
are managed.

> EPC's DTS will provide such information by msi-map and msi-mask. A
> simplified dts as
> 
> pcie-ep@10000000 {
> 	...
> 	msi-map = <0 &its 0xc 8>;
>                           ^^^ 0xc is implement defined sideband information,
> 			      which append to AXI write transaction.
> 	           ^ 0 is function index.

What does this sideband field represent? How is the ITS driver
supposed to use that data? Is it the full devid as presented to the
ITS? Something combined with the "function index"? Is the "function
index" a full RID, as defined in the documentation?

Also, msi-map is so far reserved to a PCIe RC, not this sort of wonky
contraption. This needs to be documented.

> 
> 	msi-mask = <0x7>
> }
> 
> Check msi-map if msi-parent missed to keep compatility with existed system.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change from v14 to v15
> - none
> 
> change from v13 to v14
> new patch
> ---
>  drivers/irqchip/irq-gic-v3-its-msi-parent.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> index e150365fbe892..6c7389bb84a4a 100644
> --- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> +++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
> @@ -118,6 +118,14 @@ static int of_pmsi_get_dev_id(struct irq_domain *domain, struct device *dev,
>  		index++;
>  	} while (!ret);
>  
> +	if (ret) {
> +		struct device_node *np = NULL;
> +
> +		ret = of_map_id(dev->of_node, dev->id, "msi-map", "msi-map-mask", &np, dev_id);
> +		if (np)
> +			of_node_put(np);
> +	}
> +
>  	return ret;
>  }
>  
> 

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.