Re: [PATCH] arm64: dts: rockchip: rk3588: add msi-parent for pcie3x4_ep

From: Niklas Cassel <cassel@kernel.org>
To: Heiko Stuebner <heiko@sntech.de>
Cc: Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Damien Le Moal <dlemoal@kernel.org>,
	Sebastian Reichel <sebastian.reichel@collabora.com>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org
Subject: Re: [PATCH] arm64: dts: rockchip: rk3588: add msi-parent for pcie3x4_ep
Date: Fri, 20 Dec 2024 09:03:36 +0100	[thread overview]
Message-ID: <Z2Uk2A7-YwkSfAOh@ryzen> (raw)
In-Reply-To: <173318214613.1403925.10026428339576666444.b4-ty@sntech.de>

On Tue, Dec 03, 2024 at 12:29:16AM +0100, Heiko Stuebner wrote:
> 
> On Wed, 20 Nov 2024 18:10:49 +0100, Niklas Cassel wrote:
> > Add msi-parent for the pcie3x4_ep PCI endpoint node.
> > 
> > The pcie3x4_ep node should use the same msi-parent as the pcie3x4 node
> > (which represents the PCIe controller running in Root Complex mode).
> > 
> > The GIC ITS can be used to trigger an IRQ on the endpoint when any of
> > the endpoint's PCI BARs are written to by the host[1].
> > 
> > [...]
> 
> Applied, thanks!
> 
> [1/1] arm64: dts: rockchip: rk3588: add msi-parent for pcie3x4_ep
>       commit: b6f09f497b07008aa65c31341138cecafa78222c
> 

Hello Heiko,

When I sent this patch, I had tested it against Frank's v8 series which adds
support for a PCIe endpoint triggering an interrupt (using GIC ITS) when the
host side writes to the PCI BAR that the endpoint has configured as doorbell:
https://lore.kernel.org/linux-pci/20241116-ep-msi-v8-0-6f1f68ffd1bb@nxp.com/

However, it seems that in v10 of his series, he has changed it so that the
PCIe endpoint node now requires 'msi-map' instead of 'msi-parent'
(just like how it is done in the PCIe root complex node):

See:
https://lore.kernel.org/linux-pci/20241204-ep-msi-v10-0-87c378dbcd6d@nxp.com/

"""
Changes in v10:

[...]

- Use "msi-map" in pci ep controler node, instead of of msi-parent. first
argument is
	(func_no << 8 | vfunc_no)
"""

I didn't realize that DT property required for this feature could change so
drastically, since DT is supposed to describe hardware, and msi-parent made
perfect sense, since this property is usually the one used for devices
connected to a bus.
(msi-map is usually only used on the host bus adapter / root complex.)

Knowing what I now know, I should have waited until this feature had landed
before submitting this patch. I apologize for this.

Could you please drop this patch from your v6.14-armsoc/dts64 branch?

Or should I send a revert?

Once Frank's series has landed, I can resubmit a patch that adds whichever
DT property he finally ends up using.

Kind regards,
Niklas