| Message ID | 1395397968-6242-7-git-send-email-phil.edworthy@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sergei, On: 21/03/2014 14:30, Sergei wrote: > Subject: Re: [PATCH v4 6/9] ARM: shmobile: Add PCIe device tree nodes for R8A7790 > > Hello. > > On 03/21/2014 01:32 PM, Phil Edworthy wrote: > > > This patch adds the device tree nodes for R8A7790 > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> > > --- > > arch/arm/boot/dts/r8a7790.dtsi | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > > diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi > > index df9ec61..e7d498a 100644 > > --- a/arch/arm/boot/dts/r8a7790.dtsi > > +++ b/arch/arm/boot/dts/r8a7790.dtsi > > @@ -821,4 +821,23 @@ > > #size-cells = <0>; > > status = "disabled"; > > }; > > + > > + pcie: pcie@fe000000 { > > + compatible = "renesas,r8a7790-pcie"; > > The convention adopted on this list is to put the "pcie" first, and SoC > name the last. Ok, shame that all the other PCI hosts use vendor,device-pcie > > + reg = <0 0xfe000000 0 0x80000>; > > + #address-cells = <3>; > > + #size-cells = <2>; > > + device_type = "pci"; > > This legacy property is only meaningful for the true OF firmware and thus > have been phased out (it's only actively used for the memory nodes). Ok, thanks for pointing this out. > WBR, Sergei > Thanks Phil
On Friday 21 March 2014 14:53:48 Phil.Edworthy@renesas.com wrote: > > > > + reg = <0 0xfe000000 0 0x80000>; > > > + #address-cells = >; > > > + #size-cells = <2>; > > > + device_type = "pci"; > > > > This legacy property is only meaningful for the true OF firmware and > thus > > have been phased out (it's only actively used for the memory nodes). > Ok, thanks for pointing this out. > I think the code still relies on it for PCI though. Arnd
Hi Sergei, On: 21/03/2014 14:32, Sergei wrote: > Subject: Re: [PATCH v4 6/9] ARM: shmobile: Add PCIe device tree nodes for R8A7790 > > Hello. > > On 03/21/2014 01:32 PM, Phil Edworthy wrote: > > > This patch adds the device tree nodes for R8A7790 > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> > > --- > > arch/arm/boot/dts/r8a7790.dtsi | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > Why are you not enabling PCIe for Lager, only on Koelsch? The Lager board doesn't have PCIe hardware. Note that it's possible to modify the Lager board and convert the SATA signals to PCIe, and we've done that to check that it works on R-Car H2. > WBR, Sergei > Thanks Phil
Hello. On 03/21/2014 01:32 PM, Phil Edworthy wrote: > This patch adds the device tree nodes for R8A7790 > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> > --- > arch/arm/boot/dts/r8a7790.dtsi | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi > index df9ec61..e7d498a 100644 > --- a/arch/arm/boot/dts/r8a7790.dtsi > +++ b/arch/arm/boot/dts/r8a7790.dtsi > @@ -821,4 +821,23 @@ > #size-cells = <0>; > status = "disabled"; > }; > + > + pcie: pcie@fe000000 { > + compatible = "renesas,r8a7790-pcie"; The convention adopted on this list is to put the "pcie" first, and SoC name the last. > + reg = <0 0xfe000000 0 0x80000>; > + #address-cells = <3>; > + #size-cells = <2>; > + device_type = "pci"; This legacy property is only meaningful for the true OF firmware and thus have been phased out (it's only actively used for the memory nodes). WBR, Sergei
Hello. On 03/21/2014 01:32 PM, Phil Edworthy wrote: > This patch adds the device tree nodes for R8A7790 > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> > --- > arch/arm/boot/dts/r8a7790.dtsi | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) Why are you not enabling PCIe for Lager, only on Koelsch? WBR, Sergei
Hi Sergei, On: 21/03/2014 15:42, Sergei wrote: > Subject: Re: [PATCH v4 6/9] ARM: shmobile: Add PCIe device tree nodes for R8A7790 > > Hello. > > On 03/21/2014 05:57 PM, Arnd Bergmann wrote: > > >>>> + reg = <0 0xfe000000 0 0x80000>; > >>>> + #address-cells = >; > >>>> + #size-cells = <2>; > >>>> + device_type = "pci"; > > >>> This legacy property is only meaningful for the true OF firmware and thus > >>> have been phased out (it's only actively used for the memory nodes). > > >> Ok, thanks for pointing this out. > > > I think the code still relies on it for PCI though. > > OK, I have found where it does that but the recommended value seems to be > "pciex" for PCI-Express case. Ok, but why then do all the other PCIe hosts use "pci"? Thanks Phil
On Fri, Mar 21, 2014 at 03:57:25PM +0100, Arnd Bergmann wrote: > On Friday 21 March 2014 14:53:48 Phil.Edworthy@renesas.com wrote: > > > > > > + reg = <0 0xfe000000 0 0x80000>; > > > > + #address-cells = >; > > > > + #size-cells = <2>; > > > > + device_type = "pci"; > > > > > > This legacy property is only meaningful for the true OF firmware and > > thus > > > have been phased out (it's only actively used for the memory nodes). > > Ok, thanks for pointing this out. > > > > I think the code still relies on it for PCI though. Right, it must be present for PCI nodes, it triggers special behavior in the address parsing routines. What Phil has looks correct to me. Regards, Jason
Hello. On 03/21/2014 05:57 PM, Arnd Bergmann wrote: >>>> + reg = <0 0xfe000000 0 0x80000>; >>>> + #address-cells = >; >>>> + #size-cells = <2>; >>>> + device_type = "pci"; >>> This legacy property is only meaningful for the true OF firmware and thus >>> have been phased out (it's only actively used for the memory nodes). >> Ok, thanks for pointing this out. > I think the code still relies on it for PCI though. OK, I have found where it does that but the recommended value seems to be "pciex" for PCI-Express case. > Arnd WBR, Sergei
On Fri, Mar 21, 2014 at 04:31:24PM +0000, Phil.Edworthy@renesas.com wrote: > > OK, I have found where it does that but the recommended value > > seems to be "pciex" for PCI-Express case. > > Ok, but why then do all the other PCIe hosts use "pci"? Because that is what the PCI bus binding standard says to do, and new drivers have been pushed toward conforming to that document. I'm not sure if "pciex" was ever standardized, I haven't see a spec that reflects that at least. Commit 14e2abb732e485ee57d9d5b2cb8884652238e5c1 introduced support for the "pciex" word, but it looks driven by proprietary choices by IBM.. In any event there isn't a single 'device_type = "pciex";' pre-exsting in the kernel bindings today, so please don't add one :) Regards, Jason
On Fri, Mar 21, 2014 at 4:29 PM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: >> + >> + pcie: pcie@fe000000 { >> + compatible = "renesas,r8a7790-pcie"; > > The convention adopted on this list is to put the "pcie" first, and SoC > name the last. Except for clock bindings ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi index df9ec61..e7d498a 100644 --- a/arch/arm/boot/dts/r8a7790.dtsi +++ b/arch/arm/boot/dts/r8a7790.dtsi @@ -821,4 +821,23 @@ #size-cells = <0>; status = "disabled"; }; + + pcie: pcie@fe000000 { + compatible = "renesas,r8a7790-pcie"; + 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>; + interrupts = <0 116 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>; + clock-names = "pcie"; + status = "disabled"; + }; };
This patch adds the device tree nodes for R8A7790 Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> --- arch/arm/boot/dts/r8a7790.dtsi | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)