Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1320564 > unrolled thread

Re: [PATCH 11/13] dtb: amd: Add PCIe SMMU device tree node

Started byWill Deacon <will.deacon@arm.com>
First post2016-01-28 12:20 +0100
Last post2016-01-28 12:30 +0100
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 11/13] dtb: amd: Add PCIe SMMU device tree node Will Deacon <will.deacon@arm.com> - 2016-01-28 12:20 +0100
    Re: [PATCH 11/13] dtb: amd: Add PCIe SMMU device tree node Mark Rutland <mark.rutland@arm.com> - 2016-01-28 12:20 +0100
      Re: [PATCH 11/13] dtb: amd: Add PCIe SMMU device tree node Will Deacon <will.deacon@arm.com> - 2016-01-28 12:30 +0100

#1320564 — Re: [PATCH 11/13] dtb: amd: Add PCIe SMMU device tree node

FromWill Deacon <will.deacon@arm.com>
Date2016-01-28 12:20 +0100
SubjectRe: [PATCH 11/13] dtb: amd: Add PCIe SMMU device tree node
Message-ID<qVSMh-2B1-3@gated-at.bofh.it>
On Thu, Jan 28, 2016 at 11:14:53AM +0000, Mark Rutland wrote:
> On Wed, Jan 27, 2016 at 03:11:59PM -0600, Suravee Suthikulpanit wrote:
> > From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > 
> > Add PCIe SMMU device tree node for AMD Seattle SOC.
> > 
> > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > ---
> >  arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
> > index a7fc059..bfccfea 100644
> > --- a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
> > +++ b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
> > @@ -210,6 +210,7 @@
> >  			device_type = "pci";
> >  			bus-range = <0 0x7f>;
> >  			msi-parent = <&v2m0>;
> > +			#stream-id-cells = <16>;
> >  			reg = <0 0xf0000000 0 0x10000000>;
> >  
> >  			interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
> > @@ -230,6 +231,28 @@
> >  				<0x03000000 0x01 0x00000000 0x01 0x00000000 0x7f 0x00000000>;
> >  		};
> >  
> > +		pcie0_smmu: smmu@e0a00000 {
> > +			 compatible = "arm,mmu-401";
> > +			 reg = <0 0xe0a00000 0 0x10000>;
> > +			 #global-interrupts = <1>;
> > +			 interrupts = /* Uses combined intr for both
> > +				       * global and context
> > +				       */
> > +				      <0 333 4>,
> > +				      <0 333 4>;
> > +			/* Note:
> > +			 * SID[2:0]  = PCIe function number
> > +			 * SID[7:3]  = PCIe device number
> > +			 * SID[14:8] = PCIe bus number
> > +			 */
> > +			 mmu-masters = <&pcie0
> > +				/* 1:00:[0,3] */ 256 257 258 259
> > +				/* 2:00:[0,3] */ 512 513 514 515
> > +				/* 3:00:[0,3] */ 768 769 770 771
> > +				/* 4:00:[0,3] */ 1024 1025 1026 1027
> > +			 >;
> > +		 };
> 
> This doesn't look right to me.
> 
> I didn't think that RID->SID mapping was actually defined by any
> binding, so (how) are these numbers used?
> 
> I'm uncomfortable with this, given we should be moving towards the
> generic IOMMU binding (and then we'd use the iommu-map binding [1] for
> this).
> 
> Will, Robin, thoughts?

The driver currently assumes a 1:1 RID:SID mapping when it sees a PCI
device, so those numbers should be ignored.

Will

[toc] | [next] | [standalone]

#1320572

FromMark Rutland <mark.rutland@arm.com>
Date2016-01-28 12:20 +0100
Message-ID<qVSMi-2B1-25@gated-at.bofh.it>
In reply to#1320564
On Thu, Jan 28, 2016 at 11:17:39AM +0000, Will Deacon wrote:
> On Thu, Jan 28, 2016 at 11:14:53AM +0000, Mark Rutland wrote:
> > On Wed, Jan 27, 2016 at 03:11:59PM -0600, Suravee Suthikulpanit wrote:
> > > From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > > 
> > > Add PCIe SMMU device tree node for AMD Seattle SOC.
> > > 
> > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > > ---
> > >  arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 23 +++++++++++++++++++++++
> > >  1 file changed, 23 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
> > > index a7fc059..bfccfea 100644
> > > --- a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
> > > +++ b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
> > > @@ -210,6 +210,7 @@
> > >  			device_type = "pci";
> > >  			bus-range = <0 0x7f>;
> > >  			msi-parent = <&v2m0>;
> > > +			#stream-id-cells = <16>;
> > >  			reg = <0 0xf0000000 0 0x10000000>;
> > >  
> > >  			interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
> > > @@ -230,6 +231,28 @@
> > >  				<0x03000000 0x01 0x00000000 0x01 0x00000000 0x7f 0x00000000>;
> > >  		};
> > >  
> > > +		pcie0_smmu: smmu@e0a00000 {
> > > +			 compatible = "arm,mmu-401";
> > > +			 reg = <0 0xe0a00000 0 0x10000>;
> > > +			 #global-interrupts = <1>;
> > > +			 interrupts = /* Uses combined intr for both
> > > +				       * global and context
> > > +				       */
> > > +				      <0 333 4>,
> > > +				      <0 333 4>;
> > > +			/* Note:
> > > +			 * SID[2:0]  = PCIe function number
> > > +			 * SID[7:3]  = PCIe device number
> > > +			 * SID[14:8] = PCIe bus number
> > > +			 */
> > > +			 mmu-masters = <&pcie0
> > > +				/* 1:00:[0,3] */ 256 257 258 259
> > > +				/* 2:00:[0,3] */ 512 513 514 515
> > > +				/* 3:00:[0,3] */ 768 769 770 771
> > > +				/* 4:00:[0,3] */ 1024 1025 1026 1027
> > > +			 >;
> > > +		 };
> > 
> > This doesn't look right to me.
> > 
> > I didn't think that RID->SID mapping was actually defined by any
> > binding, so (how) are these numbers used?
> > 
> > I'm uncomfortable with this, given we should be moving towards the
> > generic IOMMU binding (and then we'd use the iommu-map binding [1] for
> > this).
> > 
> > Will, Robin, thoughts?
> 
> The driver currently assumes a 1:1 RID:SID mapping when it sees a PCI
> device, so those numbers should be ignored.

Given that, they shouldn't be in the DT, then?

Mark.

[toc] | [prev] | [next] | [standalone]

#1320580

FromWill Deacon <will.deacon@arm.com>
Date2016-01-28 12:30 +0100
Message-ID<qVSVY-2EG-27@gated-at.bofh.it>
In reply to#1320572
On Thu, Jan 28, 2016 at 11:18:19AM +0000, Mark Rutland wrote:
> On Thu, Jan 28, 2016 at 11:17:39AM +0000, Will Deacon wrote:
> > On Thu, Jan 28, 2016 at 11:14:53AM +0000, Mark Rutland wrote:
> > > On Wed, Jan 27, 2016 at 03:11:59PM -0600, Suravee Suthikulpanit wrote:
> > > > From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > > > 
> > > > Add PCIe SMMU device tree node for AMD Seattle SOC.
> > > > 
> > > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > > > ---
> > > >  arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 23 +++++++++++++++++++++++
> > > >  1 file changed, 23 insertions(+)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
> > > > index a7fc059..bfccfea 100644
> > > > --- a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
> > > > +++ b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
> > > > @@ -210,6 +210,7 @@
> > > >  			device_type = "pci";
> > > >  			bus-range = <0 0x7f>;
> > > >  			msi-parent = <&v2m0>;
> > > > +			#stream-id-cells = <16>;
> > > >  			reg = <0 0xf0000000 0 0x10000000>;
> > > >  
> > > >  			interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
> > > > @@ -230,6 +231,28 @@
> > > >  				<0x03000000 0x01 0x00000000 0x01 0x00000000 0x7f 0x00000000>;
> > > >  		};
> > > >  
> > > > +		pcie0_smmu: smmu@e0a00000 {
> > > > +			 compatible = "arm,mmu-401";
> > > > +			 reg = <0 0xe0a00000 0 0x10000>;
> > > > +			 #global-interrupts = <1>;
> > > > +			 interrupts = /* Uses combined intr for both
> > > > +				       * global and context
> > > > +				       */
> > > > +				      <0 333 4>,
> > > > +				      <0 333 4>;
> > > > +			/* Note:
> > > > +			 * SID[2:0]  = PCIe function number
> > > > +			 * SID[7:3]  = PCIe device number
> > > > +			 * SID[14:8] = PCIe bus number
> > > > +			 */
> > > > +			 mmu-masters = <&pcie0
> > > > +				/* 1:00:[0,3] */ 256 257 258 259
> > > > +				/* 2:00:[0,3] */ 512 513 514 515
> > > > +				/* 3:00:[0,3] */ 768 769 770 771
> > > > +				/* 4:00:[0,3] */ 1024 1025 1026 1027
> > > > +			 >;
> > > > +		 };
> > > 
> > > This doesn't look right to me.
> > > 
> > > I didn't think that RID->SID mapping was actually defined by any
> > > binding, so (how) are these numbers used?
> > > 
> > > I'm uncomfortable with this, given we should be moving towards the
> > > generic IOMMU binding (and then we'd use the iommu-map binding [1] for
> > > this).
> > > 
> > > Will, Robin, thoughts?
> > 
> > The driver currently assumes a 1:1 RID:SID mapping when it sees a PCI
> > device, so those numbers should be ignored.
> 
> Given that, they shouldn't be in the DT, then?

Not unless there's a patch extending the driver/binding so that they do
something useful, no.

Will

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web