Re: [PATCH v1] arm64: dts: qcom: sa8775p: Update iommu-map entry

[Bjorn Andersson]

On Thu, Oct 03, 2024 at 03:57:11PM GMT, Subramanian Ananthanarayanan wrote:
> 
> On 10/2/2024 7:48 AM, Bjorn Andersson wrote:
> > On Tue, Oct 01, 2024 at 05:16:01PM GMT, Subramanian Ananthanarayanan wrote:
> > > SA8775P has only support for SMMU v2, due to this PCIe has limited
> > > SID entries to enable dynamic IOMMU mapping in the driver, hence
> > > we are updating static entries.
> > > 
> > > iommu-map entries are added to support more PCIe device like switch
> > > attach, SRIOV capable devices.
> > > 
> > Is there a reason for this to be specific to sa8775p-ride? Will other
> > boards have different iommu-maps?
> > 
> > Regards,
> > Bjorn
> 
> These settings are specific to ride board which has SRIOV usecase, in all
> other
> cases we expect only direct attach where the main DT is sufficent.
> 

What would the drawback be of adding this in the dtsi? I understand that
you currently don't have any other boards with this setup today, but I
hope that will change.

Anyway, the problem you're describing above is that SA8775P only
support SMMU v2, that it has limited SID entries etc. These might be
facts (well, "support" seems like the wrong word), but they don't
explain why you add the iommu-map on this device.

Please update the commit message to explain to future readers why
iommu-map is populated like this, and why it's not done in the platform
dtsi. It's useful for the review, but it's going to be very useful for
future dts authors (who will turn to git log to figure out why
sa8775p-ride.dtsi looks like this).

Regards,
Bjorn

> - Subramanian
> 
> > 
> > > Signed-off-by: Subramanian Ananthanarayanan <quic_skananth@xxxxxxxxxxx>
> > > ---
> > >   arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 62 ++++++++++++++++++++++
> > >   1 file changed, 62 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> > > index 0c1b21def4b6..05c9f572ae42 100644
> > > --- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> > > @@ -675,6 +675,37 @@ &pcie0 {
> > >   	pinctrl-names = "default";
> > >   	pinctrl-0 = <&pcie0_default_state>;
> > > +	iommu-map = <0x0 &pcie_smmu 0x0000 0x1>,
> > > +		    <0x100 &pcie_smmu 0x0001 0x1>,
> > > +		    <0x101 &pcie_smmu 0x0002 0x1>,
> > > +		    <0x208 &pcie_smmu 0x0003 0x1>,
> > > +		    <0x210 &pcie_smmu 0x0004 0x1>,
> > > +		    <0x218 &pcie_smmu 0x0005 0x1>,
> > > +		    <0x280 &pcie_smmu 0x0006 0x1>,
> > > +		    <0x281 &pcie_smmu 0x0007 0x1>,
> > > +		    <0x282 &pcie_smmu 0x0008 0x1>,
> > > +		    <0x283 &pcie_smmu 0x0009 0x1>,
> > > +		    <0x284 &pcie_smmu 0x000a 0x1>,
> > > +		    <0x285 &pcie_smmu 0x000b 0x1>,
> > > +		    <0x286 &pcie_smmu 0x000c 0x1>,
> > > +		    <0x287 &pcie_smmu 0x000d 0x1>,
> > > +		    <0x288 &pcie_smmu 0x000e 0x1>,
> > > +		    <0x289 &pcie_smmu 0x000f 0x1>,
> > > +		    <0x28a &pcie_smmu 0x0010 0x1>,
> > > +		    <0x28b &pcie_smmu 0x0011 0x1>,
> > > +		    <0x28c &pcie_smmu 0x0012 0x1>,
> > > +		    <0x28d &pcie_smmu 0x0013 0x1>,
> > > +		    <0x28e &pcie_smmu 0x0014 0x1>,
> > > +		    <0x28f &pcie_smmu 0x0015 0x1>,
> > > +		    <0x290 &pcie_smmu 0x0016 0x1>,
> > > +		    <0x291 &pcie_smmu 0x0017 0x1>,
> > > +		    <0x292 &pcie_smmu 0x0018 0x1>,
> > > +		    <0x293 &pcie_smmu 0x0019 0x1>,
> > > +		    <0x300 &pcie_smmu 0x001a 0x1>,
> > > +		    <0x400 &pcie_smmu 0x001b 0x1>,
> > > +		    <0x500 &pcie_smmu 0x001c 0x1>,
> > > +		    <0x501 &pcie_smmu 0x001d 0x1>;
> > > +
> > >   	status = "okay";
> > >   };
> > > @@ -685,6 +716,37 @@ &pcie1 {
> > >   	pinctrl-names = "default";
> > >   	pinctrl-0 = <&pcie1_default_state>;
> > > +	iommu-map = <0x0 &pcie_smmu 0x0080 0x1>,
> > > +		    <0x100 &pcie_smmu 0x0081 0x1>,
> > > +		    <0x101 &pcie_smmu 0x0082 0x1>,
> > > +		    <0x208 &pcie_smmu 0x0083 0x1>,
> > > +		    <0x210 &pcie_smmu 0x0084 0x1>,
> > > +		    <0x218 &pcie_smmu 0x0085 0x1>,
> > > +		    <0x280 &pcie_smmu 0x0086 0x1>,
> > > +		    <0x281 &pcie_smmu 0x0087 0x1>,
> > > +		    <0x282 &pcie_smmu 0x0088 0x1>,
> > > +		    <0x283 &pcie_smmu 0x0089 0x1>,
> > > +		    <0x284 &pcie_smmu 0x008a 0x1>,
> > > +		    <0x285 &pcie_smmu 0x008b 0x1>,
> > > +		    <0x286 &pcie_smmu 0x008c 0x1>,
> > > +		    <0x287 &pcie_smmu 0x008d 0x1>,
> > > +		    <0x288 &pcie_smmu 0x008e 0x1>,
> > > +		    <0x289 &pcie_smmu 0x008f 0x1>,
> > > +		    <0x28a &pcie_smmu 0x0090 0x1>,
> > > +		    <0x28b &pcie_smmu 0x0091 0x1>,
> > > +		    <0x28c &pcie_smmu 0x0092 0x1>,
> > > +		    <0x28d &pcie_smmu 0x0093 0x1>,
> > > +		    <0x28e &pcie_smmu 0x0094 0x1>,
> > > +		    <0x28f &pcie_smmu 0x0095 0x1>,
> > > +		    <0x290 &pcie_smmu 0x0096 0x1>,
> > > +		    <0x291 &pcie_smmu 0x0097 0x1>,
> > > +		    <0x292 &pcie_smmu 0x0098 0x1>,
> > > +		    <0x29d &pcie_smmu 0x0099 0x1>,
> > > +		    <0x300 &pcie_smmu 0x009a 0x1>,
> > > +		    <0x400 &pcie_smmu 0x009b 0x1>,
> > > +		    <0x500 &pcie_smmu 0x009c 0x1>,
> > > +		    <0x501 &pcie_smmu 0x009d 0x1>;
> > > +
> > >   	status = "okay";
> > >   };
> > > -- 
> > > 2.34.1
> > >