diff mbox

[v5,6/9] ARM: shmobile: Add PCIe device tree nodes for R8A7790

Message ID 1395766604-30926-7-git-send-email-phil.edworthy@renesas.com (mailing list archive)
State Awaiting Upstream
Delegated to: Paul Mundt
Headers show

Commit Message

Phil Edworthy March 25, 2014, 4:56 p.m. UTC
This patch adds the device tree nodes for R8A7790

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
v5:
 - Renesas SoCs compatible string has peripheral before device name
 - Add PCIe bus clock reference
 - Add additional interrupt bindings
 - Use dma-ranges property to specify inbound memory regions
---
 arch/arm/boot/dts/r8a7790-lager.dts | 10 ++++++++++
 arch/arm/boot/dts/r8a7790.dtsi      | 24 ++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

Comments

Arnd Bergmann March 25, 2014, 6:42 p.m. UTC | #1
On Tuesday 25 March 2014 16:56:41 Phil Edworthy wrote:
> +               /* Map all possible DDR as inbound ranges */
> +               dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x80000000
> +                             0x43000000 1 0x80000000 1 0x80000000 0 0x80000000>;

Typo: 0x43000000 should be 0x42000000 I guess.

Since you control the mapping, I wonder if you could also do this as

               dma-ranges = <0x42000000 0 0x00000000 0 0x40000000 0 0x80000000
                             0x43000000 0 0x80000000 1 0x80000000 0 0x80000000>;

i.e. map all the RAM into PCI bus addresses below the 32-bit boundary.
This would be really nice from the perspective that now all PCI
devices could access all of RAM without using an IOMMU or the
relying on 64-bit master capability.

The downside of this is that you'd probably need a custom dma_map_ops
wrapper.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simon Horman March 25, 2014, 9:03 p.m. UTC | #2
On Tue, Mar 25, 2014 at 04:56:41PM +0000, Phil Edworthy wrote:
> This patch adds the device tree nodes for R8A7790
> 
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>

Please split this patch into two patches.

1. A patch that updates arch/arm/boot/dts/r8a7790.dtsi

   I suggest calling that patch
   "ARM: shmobile: r8a7791: Add PCI device nodes"

2. A patch that updates arch/arm/boot/dts/r8a7790-lager.dts

   I suggest calling that patch
   "ARM: shmobile: lager: Add PCI device nodes"

> ---
> v5:
>  - Renesas SoCs compatible string has peripheral before device name
>  - Add PCIe bus clock reference
>  - Add additional interrupt bindings
>  - Use dma-ranges property to specify inbound memory regions
> ---
>  arch/arm/boot/dts/r8a7790-lager.dts | 10 ++++++++++
>  arch/arm/boot/dts/r8a7790.dtsi      | 24 ++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index a55c5f8..bbbcb63 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -139,6 +139,16 @@
>  		states = <3300000 1
>  			  1800000 0>;
>  	};
> +
> +	clocks {
> +		/* External PCIe bus clock - not used */
> +		pcie_bus_clk: pcie_bus_clk {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <0>;
> +			clock-output-names = "pcie_bus";
> +		};
> +	};
>  };
>  
>  &extal_clk {
> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index df9ec61..dbcea57 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -821,4 +821,28 @@
>  		#size-cells = <0>;
>  		status = "disabled";
>  	};
> +
> +	pcie: pcie@fe000000 {
> +		compatible = "renesas,pcie-r8a7790";
> +		reg = <0 0xfe000000 0 0x80000>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		ranges = <0x01000000 0 0x00000000 0 0xfe100000 0 0x00100000
> +			  0x02000000 0 0xfe200000 0 0xfe200000 0 0x00200000
> +			  0x02000000 0 0x30000000 0 0x30000000 0 0x08000000
> +			  0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>;
> +		/* Map all possible DDR as inbound ranges */
> +		dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x80000000
> +			      0x43000000 1 0x80000000 1 0x80000000 0 0x80000000>;
> +		interrupts = <0 116 IRQ_TYPE_LEVEL_HIGH
> +			      0 117 IRQ_TYPE_LEVEL_HIGH
> +			      0 118 IRQ_TYPE_LEVEL_HIGH>;
> +		#interrupt-cells = <1>;
> +		interrupt-map-mask = <0 0 0 0>;
> +		interrupt-map = <0 0 0 0 &gic 0 116 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&mstp3_clks R8A7790_CLK_PCIE>, <&pcie_bus_clk>;
> +		clock-names = "pcie", "pcie_bus";
> +		status = "disabled";
> +	};
>  };
> -- 
> 1.9.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Phil Edworthy March 26, 2014, 8:54 a.m. UTC | #3
Hi Simon,

On: 25/03/2014 21:03, Simon wrote:
> Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes 
for R8A7790
> 
> On Tue, Mar 25, 2014 at 04:56:41PM +0000, Phil Edworthy wrote:
> > This patch adds the device tree nodes for R8A7790
> > 
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> 
> Please split this patch into two patches.
> 
> 1. A patch that updates arch/arm/boot/dts/r8a7790.dtsi
> 
>    I suggest calling that patch
>    "ARM: shmobile: r8a7791: Add PCI device nodes"
> 
> 2. A patch that updates arch/arm/boot/dts/r8a7790-lager.dts
> 
>    I suggest calling that patch
>    "ARM: shmobile: lager: Add PCI device nodes"
> 

For this, and your other comments, will do!

Thanks
Phil

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Phil Edworthy March 26, 2014, 9:55 a.m. UTC | #4
Hi Arnd,

On: 25/03/2014 18:42, Arnd wrote:
> Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes 
for R8A7790
> 
> On Tuesday 25 March 2014 16:56:41 Phil Edworthy wrote:
> > +               /* Map all possible DDR as inbound ranges */
> > +               dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 
0x80000000
> > +                             0x43000000 1 0x80000000 1 0x80000000 0 
0x80000000>;
> 
> Typo: 0x43000000 should be 0x42000000 I guess.
I used 0x43000000 as this is a 64-bit type. The OF PCI range code 
currently treats both 32 and 64-bit types the same way, but I thought it 
would be good to set this in case we ever need to use it.
 
> Since you control the mapping, I wonder if you could also do this as
> 
>                dma-ranges = <0x42000000 0 0x00000000 0 0x40000000 0 
0x80000000
>                              0x43000000 0 0x80000000 1 0x80000000 0 
0x80000000>;
> 
> i.e. map all the RAM into PCI bus addresses below the 32-bit boundary.
> This would be really nice from the perspective that now all PCI
> devices could access all of RAM without using an IOMMU or the
> relying on 64-bit master capability.
> 
> The downside of this is that you'd probably need a custom dma_map_ops
> wrapper.
Since the OF PCi range code treats both 32 and 64-bit types the same way, 
my PCIe driver only creates 64-bit mappings. In addition, the PCIe 
controller has to use a 64-bit mapping for anything over 2GiB. Based on 
this, I think it's sensible to leave the mappings as 1-to-1.

Thanks
Phil
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann March 26, 2014, 10:34 a.m. UTC | #5
On Wednesday 26 March 2014 09:55:04 Phil.Edworthy@renesas.com wrote:
> Hi Arnd,
> 
> On: 25/03/2014 18:42, Arnd wrote:
> > Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes 
> for R8A7790
> > 
> > On Tuesday 25 March 2014 16:56:41 Phil Edworthy wrote:
> > > +               /* Map all possible DDR as inbound ranges */
> > > +               dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 
> 0x80000000
> > > +                             0x43000000 1 0x80000000 1 0x80000000 0 
> 0x80000000>;
> > 
> > Typo: 0x43000000 should be 0x42000000 I guess.
> I used 0x43000000 as this is a 64-bit type. The OF PCI range code 
> currently treats both 32 and 64-bit types the same way, but I thought it 
> would be good to set this in case we ever need to use it.

Ah, I forgot about the space identifier. It looks correct then, but
it seems  a little strange to use a 32-bit identifier in one case
and a 64-bit one in the other.

> > Since you control the mapping, I wonder if you could also do this as
> > 
> >                dma-ranges = <0x42000000 0 0x00000000 0 0x40000000 0 
> 0x80000000
> >                              0x43000000 0 0x80000000 1 0x80000000 0 
> 0x80000000>;
> > 
> > i.e. map all the RAM into PCI bus addresses below the 32-bit boundary.
> > This would be really nice from the perspective that now all PCI
> > devices could access all of RAM without using an IOMMU or the
> > relying on 64-bit master capability.
> > 
> > The downside of this is that you'd probably need a custom dma_map_ops
> > wrapper.
> Since the OF PCi range code treats both 32 and 64-bit types the same way, 
> my PCIe driver only creates 64-bit mappings. In addition, the PCIe 
> controller has to use a 64-bit mapping for anything over 2GiB. Based on 
> this, I think it's sensible to leave the mappings as 1-to-1.

I'm not following, sorry. What is the hardware requirement in the
controller?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Phil Edworthy March 26, 2014, 11:01 a.m. UTC | #6
Hi Arnd,

On: 26/03/2014 10:34, Arnd wrote:
> Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes 
for R8A7790
> 
> On Wednesday 26 March 2014 09:55:04 Phil.Edworthy@renesas.com wrote:
> > Hi Arnd,
> > 
> > On: 25/03/2014 18:42, Arnd wrote:
> > > Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree 
nodes 
> > for R8A7790
> > > 
> > > On Tuesday 25 March 2014 16:56:41 Phil Edworthy wrote:
> > > > +               /* Map all possible DDR as inbound ranges */
> > > > +               dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 
0 
> > 0x80000000
> > > > +                             0x43000000 1 0x80000000 1 0x80000000 
0 
> > 0x80000000>;
> > > 
> > > Typo: 0x43000000 should be 0x42000000 I guess.
> > I used 0x43000000 as this is a 64-bit type. The OF PCI range code 
> > currently treats both 32 and 64-bit types the same way, but I thought 
it 
> > would be good to set this in case we ever need to use it.
> 
> Ah, I forgot about the space identifier. It looks correct then, but
> it seems  a little strange to use a 32-bit identifier in one case
> and a 64-bit one in the other.

If the OF PCI range code allowed the PCIe host driver to determine if it's 
a 32-bit mapping, we could use that and get a small performance 
improvement with PCIe throughput.

> > > Since you control the mapping, I wonder if you could also do this as
> > > 
> > >                dma-ranges = <0x42000000 0 0x00000000 0 0x40000000 0 
> > 0x80000000
> > >                              0x43000000 0 0x80000000 1 0x80000000 0 
> > 0x80000000>;
> > > 
> > > i.e. map all the RAM into PCI bus addresses below the 32-bit 
boundary.
> > > This would be really nice from the perspective that now all PCI
> > > devices could access all of RAM without using an IOMMU or the
> > > relying on 64-bit master capability.
> > > 
> > > The downside of this is that you'd probably need a custom 
dma_map_ops
> > > wrapper.
> > Since the OF PCi range code treats both 32 and 64-bit types the same 
way, 
> > my PCIe driver only creates 64-bit mappings. In addition, the PCIe 
> > controller has to use a 64-bit mapping for anything over 2GiB. Based 
on 
> > this, I think it's sensible to leave the mappings as 1-to-1.
> 
> I'm not following, sorry. What is the hardware requirement in the
> controller?
With this controller, you can only specify maps whose size are a power of 
two, and the size must be less than or equal to the cpu address alignment. 
Further, when the size is 4GiB, you have to use a 64-bit mapping. Thinking 
about it, the 4GiB case is not relevant to our discussion about 32-bit vs 
64-bit mappings.

Still, my comment about the OF PCI range code treating both 32 and 64-bit 
types the same way means that PCIe host driver has to assume it's a 64-bit 
mapping.

Thanks
Phil
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann March 26, 2014, 11:14 a.m. UTC | #7
On Wednesday 26 March 2014 11:01:46 Phil.Edworthy@renesas.com wrote:
> On: 26/03/2014 10:34, Arnd wrote:
> > Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes 
> for R8A7790
> > 
> > On Wednesday 26 March 2014 09:55:04 Phil.Edworthy@renesas.com wrote:
> > > Hi Arnd,
> > > 
> > > On: 25/03/2014 18:42, Arnd wrote:
> > > > Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree 
> nodes 
> > > for R8A7790
> > > > 
> > > > On Tuesday 25 March 2014 16:56:41 Phil Edworthy wrote:
> > > > > +               /* Map all possible DDR as inbound ranges */
> > > > > +               dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 
> 0 
> > > 0x80000000
> > > > > +                             0x43000000 1 0x80000000 1 0x80000000 
> 0 
> > > 0x80000000>;
> > > > 
> > > > Typo: 0x43000000 should be 0x42000000 I guess.
> > > I used 0x43000000 as this is a 64-bit type. The OF PCI range code 
> > > currently treats both 32 and 64-bit types the same way, but I thought 
> it 
> > > would be good to set this in case we ever need to use it.
> > 
> > Ah, I forgot about the space identifier. It looks correct then, but
> > it seems  a little strange to use a 32-bit identifier in one case
> > and a 64-bit one in the other.
> 
> If the OF PCI range code allowed the PCIe host driver to determine if it's 
> a 32-bit mapping, we could use that and get a small performance 
> improvement with PCIe throughput.

I don't think it's supposed to care. Soem of the upper bits of the ranges
only really make sense of PCI device registers, not for the top-level
ranges property. The driver can however still look at the address itself
to get that information.

> > > Since the OF PCi range code treats both 32 and 64-bit types the same 
> way, 
> > > my PCIe driver only creates 64-bit mappings. In addition, the PCIe 
> > > controller has to use a 64-bit mapping for anything over 2GiB. Based 
> on 
> > > this, I think it's sensible to leave the mappings as 1-to-1.
> > 
> > I'm not following, sorry. What is the hardware requirement in the
> > controller?
> With this controller, you can only specify maps whose size are a power of 
> two, and the size must be less than or equal to the cpu address alignment. 
> Further, when the size is 4GiB, you have to use a 64-bit mapping. Thinking 
> about it, the 4GiB case is not relevant to our discussion about 32-bit vs 
> 64-bit mappings.

But the ranges you specified in the property don't actually fit in those
constraints: you have a range with size 0x8000000 and start 0x40000000,
which you say can't be programmed into the hardware.

> Still, my comment about the OF PCI range code treating both 32 and 64-bit 
> types the same way means that PCIe host driver has to assume it's a 64-bit 
> mapping.

I was thinking more of PCI devices than the host itself. If the host
driver can verify that all mappings are in the first 4GB and cover all of
RAM, we won't have to use an swiotlb for devices that don't support 64-bit
DMA, which is a very significant performance difference.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Phil Edworthy March 26, 2014, 11:34 a.m. UTC | #8
Hi Arnd,

On: 26/03/2014 11:14, Arnd wrote:
> Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes 
for R8A7790
> 
> On Wednesday 26 March 2014 11:01:46 Phil.Edworthy@renesas.com wrote:
> > On: 26/03/2014 10:34, Arnd wrote:
> > > Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree 
nodes 
> > for R8A7790
> > > 
> > > On Wednesday 26 March 2014 09:55:04 Phil.Edworthy@renesas.com wrote:
> > > > Hi Arnd,
> > > > 
> > > > On: 25/03/2014 18:42, Arnd wrote:
> > > > > Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree 
> > nodes 
> > > > for R8A7790
> > > > > 
> > > > > On Tuesday 25 March 2014 16:56:41 Phil Edworthy wrote:
> > > > > > +               /* Map all possible DDR as inbound ranges */
> > > > > > +               dma-ranges = <0x42000000 0 0x40000000 0 
0x40000000 
> > 0 
> > > > 0x80000000
> > > > > > +                             0x43000000 1 0x80000000 1 
0x80000000 
> > 0 
> > > > 0x80000000>;
> > > > > 
> > > > > Typo: 0x43000000 should be 0x42000000 I guess.
> > > > I used 0x43000000 as this is a 64-bit type. The OF PCI range code 
> > > > currently treats both 32 and 64-bit types the same way, but I 
thought 
> > it 
> > > > would be good to set this in case we ever need to use it.
> > > 
> > > Ah, I forgot about the space identifier. It looks correct then, but
> > > it seems  a little strange to use a 32-bit identifier in one case
> > > and a 64-bit one in the other.
> > 
> > If the OF PCI range code allowed the PCIe host driver to determine if 
it's 
> > a 32-bit mapping, we could use that and get a small performance 
> > improvement with PCIe throughput.
> 
> I don't think it's supposed to care. Soem of the upper bits of the 
ranges
> only really make sense of PCI device registers, not for the top-level
> ranges property. The driver can however still look at the address itself
> to get that information.

Ah, yes that is a possibility.


> > > > Since the OF PCi range code treats both 32 and 64-bit types the 
same 
> > way, 
> > > > my PCIe driver only creates 64-bit mappings. In addition, the PCIe 

> > > > controller has to use a 64-bit mapping for anything over 2GiB. 
Based 
> > on 
> > > > this, I think it's sensible to leave the mappings as 1-to-1.
> > > 
> > > I'm not following, sorry. What is the hardware requirement in the
> > > controller?
> > With this controller, you can only specify maps whose size are a power 
of 
> > two, and the size must be less than or equal to the cpu address 
alignment. 
> > Further, when the size is 4GiB, you have to use a 64-bit mapping. 
Thinking 
> > about it, the 4GiB case is not relevant to our discussion about 32-bit 
vs 
> > 64-bit mappings.
> 
> But the ranges you specified in the property don't actually fit in those
> constraints: you have a range with size 0x8000000 and start 0x40000000,
> which you say can't be programmed into the hardware.

Actually, the driver checks the dma-ranges against these constraints, and 
if necessary will create multiple mappings to fulfil the requested 
dma-ranges.


> > Still, my comment about the OF PCI range code treating both 32 and 
64-bit 
> > types the same way means that PCIe host driver has to assume it's a 
64-bit 
> > mapping.
> 
> I was thinking more of PCI devices than the host itself. If the host
> driver can verify that all mappings are in the first 4GB and cover all 
of
> RAM, we won't have to use an swiotlb for devices that don't support 
64-bit
> DMA, which is a very significant performance difference.
Ok, I think I understand. However, all the other PCI host drivers just do 
1-to-1 mapping between PCI and CPU addresses, right? Whilst it might be 
nice be able to support mapping CPU addresses > 4GiB to PCI addresses 
under 4GiB, can that be something to consider later on?

Thanks
Phil
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann March 26, 2014, 11:52 a.m. UTC | #9
On Wednesday 26 March 2014 11:34:41 Phil.Edworthy@renesas.com wrote:
> > But the ranges you specified in the property don't actually fit in those
> > constraints: you have a range with size 0x8000000 and start 0x40000000,
> > which you say can't be programmed into the hardware.
> 
> Actually, the driver checks the dma-ranges against these constraints, and 
> if necessary will create multiple mappings to fulfil the requested 
> dma-ranges.

Ok, I didn't notice. My initial suggestion was to not put that logic
into the driver but instead specify in the host bridge binding that each
entry in the dma-ranges property has to meet the hardware constraints.

As long as you don't have too much complexity to detect this case, I'm
fine with it either way.
 
> > > Still, my comment about the OF PCI range code treating both 32 and 
> 64-bit 
> > > types the same way means that PCIe host driver has to assume it's a 
> 64-bit 
> > > mapping.
> > 
> > I was thinking more of PCI devices than the host itself. If the host
> > driver can verify that all mappings are in the first 4GB and cover all 
> of
> > RAM, we won't have to use an swiotlb for devices that don't support 
> 64-bit
> > DMA, which is a very significant performance difference.
> Ok, I think I understand. However, all the other PCI host drivers just do 
> 1-to-1 mapping between PCI and CPU addresses, right? Whilst it might be 
> nice be able to support mapping CPU addresses > 4GiB to PCI addresses 
> under 4GiB, can that be something to consider later on?

Yes, fair enough. The current version is much simpler, so that's ok.
Just keep it in mind if you run into performance problems. Also, note
that we don't actually support swiotlb on arm32 yet, so your current
code is broken for an PCI DMA master that is not 64-bit capable.
We need swiotlb on arm32 anyway, and that will fix this problem, but
adding the hack I described would also fix it.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Phil Edworthy March 26, 2014, 11:56 a.m. UTC | #10
Hi Arnd,

On: 26/03/2014 11:53, Arnd wrote:
> Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes 
for R8A7790
> 
> On Wednesday 26 March 2014 11:34:41 Phil.Edworthy@renesas.com wrote:
> > > But the ranges you specified in the property don't actually fit in 
those
> > > constraints: you have a range with size 0x8000000 and start 
0x40000000,
> > > which you say can't be programmed into the hardware.
> > 
> > Actually, the driver checks the dma-ranges against these constraints, 
and 
> > if necessary will create multiple mappings to fulfil the requested 
> > dma-ranges.
> 
> Ok, I didn't notice. My initial suggestion was to not put that logic
> into the driver but instead specify in the host bridge binding that each
> entry in the dma-ranges property has to meet the hardware constraints.
> 
> As long as you don't have too much complexity to detect this case, I'm
> fine with it either way.
Ok, good!

> > > > Still, my comment about the OF PCI range code treating both 32 and 

> > 64-bit 
> > > > types the same way means that PCIe host driver has to assume it's 
a 
> > 64-bit 
> > > > mapping.
> > > 
> > > I was thinking more of PCI devices than the host itself. If the host
> > > driver can verify that all mappings are in the first 4GB and cover 
all 
> > of
> > > RAM, we won't have to use an swiotlb for devices that don't support 
> > 64-bit
> > > DMA, which is a very significant performance difference.
> > Ok, I think I understand. However, all the other PCI host drivers just 
do 
> > 1-to-1 mapping between PCI and CPU addresses, right? Whilst it might 
be 
> > nice be able to support mapping CPU addresses > 4GiB to PCI addresses 
> > under 4GiB, can that be something to consider later on?
> 
> Yes, fair enough. The current version is much simpler, so that's ok.
> Just keep it in mind if you run into performance problems. Also, note
> that we don't actually support swiotlb on arm32 yet, so your current
> code is broken for an PCI DMA master that is not 64-bit capable.
> We need swiotlb on arm32 anyway, and that will fix this problem, but
> adding the hack I described would also fix it.
Ok, thanks for all your comments!

Phil

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index a55c5f8..bbbcb63 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -139,6 +139,16 @@ 
 		states = <3300000 1
 			  1800000 0>;
 	};
+
+	clocks {
+		/* External PCIe bus clock - not used */
+		pcie_bus_clk: pcie_bus_clk {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <0>;
+			clock-output-names = "pcie_bus";
+		};
+	};
 };
 
 &extal_clk {
diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index df9ec61..dbcea57 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -821,4 +821,28 @@ 
 		#size-cells = <0>;
 		status = "disabled";
 	};
+
+	pcie: pcie@fe000000 {
+		compatible = "renesas,pcie-r8a7790";
+		reg = <0 0xfe000000 0 0x80000>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		ranges = <0x01000000 0 0x00000000 0 0xfe100000 0 0x00100000
+			  0x02000000 0 0xfe200000 0 0xfe200000 0 0x00200000
+			  0x02000000 0 0x30000000 0 0x30000000 0 0x08000000
+			  0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>;
+		/* Map all possible DDR as inbound ranges */
+		dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x80000000
+			      0x43000000 1 0x80000000 1 0x80000000 0 0x80000000>;
+		interrupts = <0 116 IRQ_TYPE_LEVEL_HIGH
+			      0 117 IRQ_TYPE_LEVEL_HIGH
+			      0 118 IRQ_TYPE_LEVEL_HIGH>;
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0>;
+		interrupt-map = <0 0 0 0 &gic 0 116 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp3_clks R8A7790_CLK_PCIE>, <&pcie_bus_clk>;
+		clock-names = "pcie", "pcie_bus";
+		status = "disabled";
+	};
 };