* [PATCH v4 0/2] Add R8A7790/Lager board PCI USB DT support @ 2014-06-20 20:34 ` Sergei Shtylyov 0 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 20:34 UTC (permalink / raw) To: linux-arm-kernel Hello. Here's the set of 2 patches against Simon Horman's 'renesas.git' repo's 'renesas-devel-v3.16-rc1-20140618' tag. Here we add PCI USB device tree support for the R8A7790-based Lager board. As the 'pci-rcar-gen2' driver device tree support has been merged for 3.16, this patchset can now be merged too... [1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes [2/2] ARM: shmobile: lager: enable internal PCI WBR, Sergei ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 0/2] Add R8A7790/Lager board PCI USB DT support @ 2014-06-20 20:34 ` Sergei Shtylyov 0 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 20:34 UTC (permalink / raw) To: horms, linux-sh, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, devicetree Cc: magnus.damm, linux, linux-arm-kernel, ben.dooks Hello. Here's the set of 2 patches against Simon Horman's 'renesas.git' repo's 'renesas-devel-v3.16-rc1-20140618' tag. Here we add PCI USB device tree support for the R8A7790-based Lager board. As the 'pci-rcar-gen2' driver device tree support has been merged for 3.16, this patchset can now be merged too... [1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes [2/2] ARM: shmobile: lager: enable internal PCI WBR, Sergei ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 0/2] Add R8A7790/Lager board PCI USB DT support @ 2014-06-20 20:34 ` Sergei Shtylyov 0 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 20:34 UTC (permalink / raw) To: linux-arm-kernel Hello. Here's the set of 2 patches against Simon Horman's 'renesas.git' repo's 'renesas-devel-v3.16-rc1-20140618' tag. Here we add PCI USB device tree support for the R8A7790-based Lager board. As the 'pci-rcar-gen2' driver device tree support has been merged for 3.16, this patchset can now be merged too... [1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes [2/2] ARM: shmobile: lager: enable internal PCI WBR, Sergei ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes 2014-06-20 20:34 ` Sergei Shtylyov (?) @ 2014-06-20 20:36 ` Sergei Shtylyov -1 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 20:36 UTC (permalink / raw) To: linux-arm-kernel From: Ben Dooks <ben.dooks@codethink.co.uk> Add device nodes for the R8A7790 internal PCI bridge devices. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> Reviewed-by: Ian Molton <ian.molton@codethink.co.uk> [Sergei: added interrupt-related properties to the PCI bridge nodes] Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- Changes in version 4: - refreshed the patch. Changes in version 3: - added interrupt-related properties to the PCI bridge nodes; - refreshed the patch. Changes in version 2: - reworded summary (fixing typo) and changelog; - removed extra spaces before {; - refreshed the patch. arch/arm/boot/dts/r8a7790.dtsi | 54 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) Index: renesas/arch/arm/boot/dts/r8a7790.dtsi =================================--- renesas.orig/arch/arm/boot/dts/r8a7790.dtsi +++ renesas/arch/arm/boot/dts/r8a7790.dtsi @@ -930,6 +930,60 @@ status = "disabled"; }; + pci0: pci@ee090000 { + compatible = "renesas,pci-r8a7790"; + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; + reg = <0x0 0xee090000 0x0 0xc00>, + <0x0 0xee080000 0x0 0x1100>; + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + + bus-range = <0 0>; + #address-cells = <3>; + #size-cells = <2>; + #interrupt-cells = <1>; + interrupt-map-mask = <0xff00 0 0 0x7>; + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; + }; + + pci1: pci@ee0b0000 { + compatible = "renesas,pci-r8a7790"; + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; + reg = <0x0 0xee0b0000 0x0 0xc00>, + <0x0 0xee0a0000 0x0 0x1100>; + interrupts = <0 112 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + + bus-range = <1 1>; + #address-cells = <3>; + #size-cells = <2>; + #interrupt-cells = <1>; + interrupt-map-mask = <0xff00 0 0 0x7>; + interrupt-map = <0x0000 0 0 1 &gic 0 112 IRQ_TYPE_LEVEL_HIGH + 0x0800 0 0 1 &gic 0 112 IRQ_TYPE_LEVEL_HIGH + 0x1000 0 0 2 &gic 0 112 IRQ_TYPE_LEVEL_HIGH>; + }; + + pci2: pci@ee0d0000 { + compatible = "renesas,pci-r8a7790"; + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; + reg = <0x0 0xee0d0000 0x0 0xc00>, + <0x0 0xee0c0000 0x0 0x1100>; + interrupts = <0 113 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + + bus-range = <2 2>; + #address-cells = <3>; + #size-cells = <2>; + #interrupt-cells = <1>; + interrupt-map-mask = <0xff00 0 0 0x7>; + interrupt-map = <0x0000 0 0 1 &gic 0 113 IRQ_TYPE_LEVEL_HIGH + 0x0800 0 0 1 &gic 0 113 IRQ_TYPE_LEVEL_HIGH + 0x1000 0 0 2 &gic 0 113 IRQ_TYPE_LEVEL_HIGH>; + }; + pciec: pcie@fe000000 { compatible = "renesas,pcie-r8a7790"; reg = <0 0xfe000000 0 0x80000>; ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-20 20:36 ` Sergei Shtylyov 0 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 20:36 UTC (permalink / raw) To: horms, linux-sh, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, devicetree Cc: magnus.damm, linux, linux-arm-kernel, ben.dooks From: Ben Dooks <ben.dooks@codethink.co.uk> Add device nodes for the R8A7790 internal PCI bridge devices. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> Reviewed-by: Ian Molton <ian.molton@codethink.co.uk> [Sergei: added interrupt-related properties to the PCI bridge nodes] Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- Changes in version 4: - refreshed the patch. Changes in version 3: - added interrupt-related properties to the PCI bridge nodes; - refreshed the patch. Changes in version 2: - reworded summary (fixing typo) and changelog; - removed extra spaces before {; - refreshed the patch. arch/arm/boot/dts/r8a7790.dtsi | 54 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) Index: renesas/arch/arm/boot/dts/r8a7790.dtsi =================================================================== --- renesas.orig/arch/arm/boot/dts/r8a7790.dtsi +++ renesas/arch/arm/boot/dts/r8a7790.dtsi @@ -930,6 +930,60 @@ status = "disabled"; }; + pci0: pci@ee090000 { + compatible = "renesas,pci-r8a7790"; + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; + reg = <0x0 0xee090000 0x0 0xc00>, + <0x0 0xee080000 0x0 0x1100>; + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + + bus-range = <0 0>; + #address-cells = <3>; + #size-cells = <2>; + #interrupt-cells = <1>; + interrupt-map-mask = <0xff00 0 0 0x7>; + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; + }; + + pci1: pci@ee0b0000 { + compatible = "renesas,pci-r8a7790"; + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; + reg = <0x0 0xee0b0000 0x0 0xc00>, + <0x0 0xee0a0000 0x0 0x1100>; + interrupts = <0 112 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + + bus-range = <1 1>; + #address-cells = <3>; + #size-cells = <2>; + #interrupt-cells = <1>; + interrupt-map-mask = <0xff00 0 0 0x7>; + interrupt-map = <0x0000 0 0 1 &gic 0 112 IRQ_TYPE_LEVEL_HIGH + 0x0800 0 0 1 &gic 0 112 IRQ_TYPE_LEVEL_HIGH + 0x1000 0 0 2 &gic 0 112 IRQ_TYPE_LEVEL_HIGH>; + }; + + pci2: pci@ee0d0000 { + compatible = "renesas,pci-r8a7790"; + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; + reg = <0x0 0xee0d0000 0x0 0xc00>, + <0x0 0xee0c0000 0x0 0x1100>; + interrupts = <0 113 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + + bus-range = <2 2>; + #address-cells = <3>; + #size-cells = <2>; + #interrupt-cells = <1>; + interrupt-map-mask = <0xff00 0 0 0x7>; + interrupt-map = <0x0000 0 0 1 &gic 0 113 IRQ_TYPE_LEVEL_HIGH + 0x0800 0 0 1 &gic 0 113 IRQ_TYPE_LEVEL_HIGH + 0x1000 0 0 2 &gic 0 113 IRQ_TYPE_LEVEL_HIGH>; + }; + pciec: pcie@fe000000 { compatible = "renesas,pcie-r8a7790"; reg = <0 0xfe000000 0 0x80000>; ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-20 20:36 ` Sergei Shtylyov 0 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 20:36 UTC (permalink / raw) To: linux-arm-kernel From: Ben Dooks <ben.dooks@codethink.co.uk> Add device nodes for the R8A7790 internal PCI bridge devices. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> Reviewed-by: Ian Molton <ian.molton@codethink.co.uk> [Sergei: added interrupt-related properties to the PCI bridge nodes] Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- Changes in version 4: - refreshed the patch. Changes in version 3: - added interrupt-related properties to the PCI bridge nodes; - refreshed the patch. Changes in version 2: - reworded summary (fixing typo) and changelog; - removed extra spaces before {; - refreshed the patch. arch/arm/boot/dts/r8a7790.dtsi | 54 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) Index: renesas/arch/arm/boot/dts/r8a7790.dtsi =================================================================== --- renesas.orig/arch/arm/boot/dts/r8a7790.dtsi +++ renesas/arch/arm/boot/dts/r8a7790.dtsi @@ -930,6 +930,60 @@ status = "disabled"; }; + pci0: pci at ee090000 { + compatible = "renesas,pci-r8a7790"; + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; + reg = <0x0 0xee090000 0x0 0xc00>, + <0x0 0xee080000 0x0 0x1100>; + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + + bus-range = <0 0>; + #address-cells = <3>; + #size-cells = <2>; + #interrupt-cells = <1>; + interrupt-map-mask = <0xff00 0 0 0x7>; + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; + }; + + pci1: pci at ee0b0000 { + compatible = "renesas,pci-r8a7790"; + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; + reg = <0x0 0xee0b0000 0x0 0xc00>, + <0x0 0xee0a0000 0x0 0x1100>; + interrupts = <0 112 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + + bus-range = <1 1>; + #address-cells = <3>; + #size-cells = <2>; + #interrupt-cells = <1>; + interrupt-map-mask = <0xff00 0 0 0x7>; + interrupt-map = <0x0000 0 0 1 &gic 0 112 IRQ_TYPE_LEVEL_HIGH + 0x0800 0 0 1 &gic 0 112 IRQ_TYPE_LEVEL_HIGH + 0x1000 0 0 2 &gic 0 112 IRQ_TYPE_LEVEL_HIGH>; + }; + + pci2: pci at ee0d0000 { + compatible = "renesas,pci-r8a7790"; + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; + reg = <0x0 0xee0d0000 0x0 0xc00>, + <0x0 0xee0c0000 0x0 0x1100>; + interrupts = <0 113 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + + bus-range = <2 2>; + #address-cells = <3>; + #size-cells = <2>; + #interrupt-cells = <1>; + interrupt-map-mask = <0xff00 0 0 0x7>; + interrupt-map = <0x0000 0 0 1 &gic 0 113 IRQ_TYPE_LEVEL_HIGH + 0x0800 0 0 1 &gic 0 113 IRQ_TYPE_LEVEL_HIGH + 0x1000 0 0 2 &gic 0 113 IRQ_TYPE_LEVEL_HIGH>; + }; + pciec: pcie at fe000000 { compatible = "renesas,pcie-r8a7790"; reg = <0 0xfe000000 0 0x80000>; ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes 2014-06-20 20:36 ` Sergei Shtylyov (?) @ 2014-06-20 20:51 ` Arnd Bergmann -1 siblings, 0 replies; 30+ messages in thread From: Arnd Bergmann @ 2014-06-20 20:51 UTC (permalink / raw) To: linux-arm-kernel On Saturday 21 June 2014 00:36:58 Sergei Shtylyov wrote: > + pci0: pci@ee090000 { > + compatible = "renesas,pci-r8a7790"; > + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; > + reg = <0x0 0xee090000 0x0 0xc00>, > + <0x0 0xee080000 0x0 0x1100>; > + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; > + status = "disabled"; > + > + bus-range = <0 0>; > + #address-cells = >; > + #size-cells = <2>; > + #interrupt-cells = <1>; > + interrupt-map-mask = <0xff00 0 0 0x7>; > + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH > + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH > + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; > + }; Hmm, this device node is not actually compliant to the PCI binding, it needs a "ranges" property that can be used to look up the memory and I/O space windows. It also needs a device_type property. I realize that the driver doesn't currently use them, but you should adhere to the binding anyway, so we can fix the driver at some point. Arnd ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-20 20:51 ` Arnd Bergmann 0 siblings, 0 replies; 30+ messages in thread From: Arnd Bergmann @ 2014-06-20 20:51 UTC (permalink / raw) To: Sergei Shtylyov Cc: horms, linux-sh, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, devicetree, magnus.damm, linux, linux-arm-kernel, ben.dooks On Saturday 21 June 2014 00:36:58 Sergei Shtylyov wrote: > + pci0: pci@ee090000 { > + compatible = "renesas,pci-r8a7790"; > + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; > + reg = <0x0 0xee090000 0x0 0xc00>, > + <0x0 0xee080000 0x0 0x1100>; > + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; > + status = "disabled"; > + > + bus-range = <0 0>; > + #address-cells = >; > + #size-cells = <2>; > + #interrupt-cells = <1>; > + interrupt-map-mask = <0xff00 0 0 0x7>; > + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH > + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH > + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; > + }; Hmm, this device node is not actually compliant to the PCI binding, it needs a "ranges" property that can be used to look up the memory and I/O space windows. It also needs a device_type property. I realize that the driver doesn't currently use them, but you should adhere to the binding anyway, so we can fix the driver at some point. Arnd ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-20 20:51 ` Arnd Bergmann 0 siblings, 0 replies; 30+ messages in thread From: Arnd Bergmann @ 2014-06-20 20:51 UTC (permalink / raw) To: linux-arm-kernel On Saturday 21 June 2014 00:36:58 Sergei Shtylyov wrote: > + pci0: pci at ee090000 { > + compatible = "renesas,pci-r8a7790"; > + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; > + reg = <0x0 0xee090000 0x0 0xc00>, > + <0x0 0xee080000 0x0 0x1100>; > + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; > + status = "disabled"; > + > + bus-range = <0 0>; > + #address-cells = >; > + #size-cells = <2>; > + #interrupt-cells = <1>; > + interrupt-map-mask = <0xff00 0 0 0x7>; > + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH > + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH > + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; > + }; Hmm, this device node is not actually compliant to the PCI binding, it needs a "ranges" property that can be used to look up the memory and I/O space windows. It also needs a device_type property. I realize that the driver doesn't currently use them, but you should adhere to the binding anyway, so we can fix the driver at some point. Arnd ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes 2014-06-20 20:51 ` Arnd Bergmann (?) @ 2014-06-20 21:00 ` Sergei Shtylyov -1 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 21:00 UTC (permalink / raw) To: linux-arm-kernel Hello. On 06/21/2014 12:51 AM, Arnd Bergmann wrote: >> + pci0: pci@ee090000 { >> + compatible = "renesas,pci-r8a7790"; >> + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; >> + reg = <0x0 0xee090000 0x0 0xc00>, >> + <0x0 0xee080000 0x0 0x1100>; >> + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; >> + status = "disabled"; >> + >> + bus-range = <0 0>; >> + #address-cells = >; >> + #size-cells = <2>; >> + #interrupt-cells = <1>; >> + interrupt-map-mask = <0xff00 0 0 0x7>; >> + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >> + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >> + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; >> + }; > Hmm, this device node is not actually compliant to the PCI binding, > it needs a "ranges" property that can be used to look up the memory > and I/O space windows. The PCI driver doesn't support I/O space. > It also needs a device_type property. Hm, are you sure about that? I thought only PCI devices should have it... > I realize that the driver doesn't currently use them, but you should > adhere to the binding anyway, so we can fix the driver at some point. Sigh, agreed about the need to fix the driver. Too bad you've spoken up only now though. And you've ACKed the DT bindings without those properties documented or even present in an example... :-( > Arnd WBR, Sergei ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-20 21:00 ` Sergei Shtylyov 0 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 21:00 UTC (permalink / raw) To: Arnd Bergmann Cc: horms, linux-sh, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, devicetree, magnus.damm, linux, linux-arm-kernel, ben.dooks Hello. On 06/21/2014 12:51 AM, Arnd Bergmann wrote: >> + pci0: pci@ee090000 { >> + compatible = "renesas,pci-r8a7790"; >> + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; >> + reg = <0x0 0xee090000 0x0 0xc00>, >> + <0x0 0xee080000 0x0 0x1100>; >> + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; >> + status = "disabled"; >> + >> + bus-range = <0 0>; >> + #address-cells = >; >> + #size-cells = <2>; >> + #interrupt-cells = <1>; >> + interrupt-map-mask = <0xff00 0 0 0x7>; >> + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >> + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >> + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; >> + }; > Hmm, this device node is not actually compliant to the PCI binding, > it needs a "ranges" property that can be used to look up the memory > and I/O space windows. The PCI driver doesn't support I/O space. > It also needs a device_type property. Hm, are you sure about that? I thought only PCI devices should have it... > I realize that the driver doesn't currently use them, but you should > adhere to the binding anyway, so we can fix the driver at some point. Sigh, agreed about the need to fix the driver. Too bad you've spoken up only now though. And you've ACKed the DT bindings without those properties documented or even present in an example... :-( > Arnd WBR, Sergei ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-20 21:00 ` Sergei Shtylyov 0 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 21:00 UTC (permalink / raw) To: linux-arm-kernel Hello. On 06/21/2014 12:51 AM, Arnd Bergmann wrote: >> + pci0: pci at ee090000 { >> + compatible = "renesas,pci-r8a7790"; >> + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; >> + reg = <0x0 0xee090000 0x0 0xc00>, >> + <0x0 0xee080000 0x0 0x1100>; >> + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; >> + status = "disabled"; >> + >> + bus-range = <0 0>; >> + #address-cells = >; >> + #size-cells = <2>; >> + #interrupt-cells = <1>; >> + interrupt-map-mask = <0xff00 0 0 0x7>; >> + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >> + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >> + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; >> + }; > Hmm, this device node is not actually compliant to the PCI binding, > it needs a "ranges" property that can be used to look up the memory > and I/O space windows. The PCI driver doesn't support I/O space. > It also needs a device_type property. Hm, are you sure about that? I thought only PCI devices should have it... > I realize that the driver doesn't currently use them, but you should > adhere to the binding anyway, so we can fix the driver at some point. Sigh, agreed about the need to fix the driver. Too bad you've spoken up only now though. And you've ACKed the DT bindings without those properties documented or even present in an example... :-( > Arnd WBR, Sergei ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes 2014-06-20 21:00 ` Sergei Shtylyov (?) @ 2014-06-20 21:10 ` Arnd Bergmann -1 siblings, 0 replies; 30+ messages in thread From: Arnd Bergmann @ 2014-06-20 21:10 UTC (permalink / raw) To: linux-arm-kernel On Saturday 21 June 2014 01:00:21 Sergei Shtylyov wrote: > > On 06/21/2014 12:51 AM, Arnd Bergmann wrote: > > >> + pci0: pci@ee090000 { > >> + compatible = "renesas,pci-r8a7790"; > >> + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; > >> + reg = <0x0 0xee090000 0x0 0xc00>, > >> + <0x0 0xee080000 0x0 0x1100>; > >> + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; > >> + status = "disabled"; > >> + > >> + bus-range = <0 0>; > >> + #address-cells = >; > >> + #size-cells = <2>; > >> + #interrupt-cells = <1>; > >> + interrupt-map-mask = <0xff00 0 0 0x7>; > >> + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH > >> + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH > >> + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; > >> + }; > > > Hmm, this device node is not actually compliant to the PCI binding, > > it needs a "ranges" property that can be used to look up the memory > > and I/O space windows. > > The PCI driver doesn't support I/O space. Well, whichever windows are registered by the driver should come from the ranges property. > > It also needs a device_type property. > > Hm, are you sure about that? I thought only PCI devices should have it... Yes, pretty sure it's needed in both the host controller and the devices. > > I realize that the driver doesn't currently use them, but you should > > adhere to the binding anyway, so we can fix the driver at some point. > > Sigh, agreed about the need to fix the driver. Too bad you've spoken up > only now though. And you've ACKed the DT bindings without those properties > documented or even present in an example... Yes, I realized that too late as well, sorry about it. For some reason I only looked at the interrupt-map being done right and forgot to check the ranges. We definitely need to move the code handling the ranges into a common location to avoid this mistake in the future. Arnd ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-20 21:10 ` Arnd Bergmann 0 siblings, 0 replies; 30+ messages in thread From: Arnd Bergmann @ 2014-06-20 21:10 UTC (permalink / raw) To: Sergei Shtylyov Cc: horms, linux-sh, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, devicetree, magnus.damm, linux, linux-arm-kernel, ben.dooks On Saturday 21 June 2014 01:00:21 Sergei Shtylyov wrote: > > On 06/21/2014 12:51 AM, Arnd Bergmann wrote: > > >> + pci0: pci@ee090000 { > >> + compatible = "renesas,pci-r8a7790"; > >> + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; > >> + reg = <0x0 0xee090000 0x0 0xc00>, > >> + <0x0 0xee080000 0x0 0x1100>; > >> + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; > >> + status = "disabled"; > >> + > >> + bus-range = <0 0>; > >> + #address-cells = >; > >> + #size-cells = <2>; > >> + #interrupt-cells = <1>; > >> + interrupt-map-mask = <0xff00 0 0 0x7>; > >> + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH > >> + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH > >> + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; > >> + }; > > > Hmm, this device node is not actually compliant to the PCI binding, > > it needs a "ranges" property that can be used to look up the memory > > and I/O space windows. > > The PCI driver doesn't support I/O space. Well, whichever windows are registered by the driver should come from the ranges property. > > It also needs a device_type property. > > Hm, are you sure about that? I thought only PCI devices should have it... Yes, pretty sure it's needed in both the host controller and the devices. > > I realize that the driver doesn't currently use them, but you should > > adhere to the binding anyway, so we can fix the driver at some point. > > Sigh, agreed about the need to fix the driver. Too bad you've spoken up > only now though. And you've ACKed the DT bindings without those properties > documented or even present in an example... Yes, I realized that too late as well, sorry about it. For some reason I only looked at the interrupt-map being done right and forgot to check the ranges. We definitely need to move the code handling the ranges into a common location to avoid this mistake in the future. Arnd ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-20 21:10 ` Arnd Bergmann 0 siblings, 0 replies; 30+ messages in thread From: Arnd Bergmann @ 2014-06-20 21:10 UTC (permalink / raw) To: linux-arm-kernel On Saturday 21 June 2014 01:00:21 Sergei Shtylyov wrote: > > On 06/21/2014 12:51 AM, Arnd Bergmann wrote: > > >> + pci0: pci at ee090000 { > >> + compatible = "renesas,pci-r8a7790"; > >> + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; > >> + reg = <0x0 0xee090000 0x0 0xc00>, > >> + <0x0 0xee080000 0x0 0x1100>; > >> + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; > >> + status = "disabled"; > >> + > >> + bus-range = <0 0>; > >> + #address-cells = >; > >> + #size-cells = <2>; > >> + #interrupt-cells = <1>; > >> + interrupt-map-mask = <0xff00 0 0 0x7>; > >> + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH > >> + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH > >> + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; > >> + }; > > > Hmm, this device node is not actually compliant to the PCI binding, > > it needs a "ranges" property that can be used to look up the memory > > and I/O space windows. > > The PCI driver doesn't support I/O space. Well, whichever windows are registered by the driver should come from the ranges property. > > It also needs a device_type property. > > Hm, are you sure about that? I thought only PCI devices should have it... Yes, pretty sure it's needed in both the host controller and the devices. > > I realize that the driver doesn't currently use them, but you should > > adhere to the binding anyway, so we can fix the driver at some point. > > Sigh, agreed about the need to fix the driver. Too bad you've spoken up > only now though. And you've ACKed the DT bindings without those properties > documented or even present in an example... Yes, I realized that too late as well, sorry about it. For some reason I only looked at the interrupt-map being done right and forgot to check the ranges. We definitely need to move the code handling the ranges into a common location to avoid this mistake in the future. Arnd ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes 2014-06-20 21:10 ` Arnd Bergmann (?) @ 2014-06-20 21:25 ` Sergei Shtylyov -1 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 21:25 UTC (permalink / raw) To: linux-arm-kernel On 06/21/2014 01:10 AM, Arnd Bergmann wrote: >>>> + pci0: pci@ee090000 { >>>> + compatible = "renesas,pci-r8a7790"; >>>> + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; >>>> + reg = <0x0 0xee090000 0x0 0xc00>, >>>> + <0x0 0xee080000 0x0 0x1100>; >>>> + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; >>>> + status = "disabled"; >>>> + >>>> + bus-range = <0 0>; >>>> + #address-cells = >; >>>> + #size-cells = <2>; >>>> + #interrupt-cells = <1>; >>>> + interrupt-map-mask = <0xff00 0 0 0x7>; >>>> + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >>>> + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >>>> + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; >>>> + }; >>> Hmm, this device node is not actually compliant to the PCI binding, >>> it needs a "ranges" property that can be used to look up the memory >>> and I/O space windows. >> The PCI driver doesn't support I/O space. > Well, whichever windows are registered by the driver should come > from the ranges property. I know. Too bad the driver's original author managed to misunderstand that... >>> It also needs a device_type property. >> Hm, are you sure about that? I thought only PCI devices should have it... > Yes, pretty sure it's needed in both the host controller and the > devices. I don't understand the case of the PCI devices, honestly. Shouldn't the "device_type" prop reflect the device's functionality rather than the bus where it's located? >>> I realize that the driver doesn't currently use them, but you should >>> adhere to the binding anyway, so we can fix the driver at some point. >> Sigh, agreed about the need to fix the driver. Too bad you've spoken up >> only now though. And you've ACKed the DT bindings without those properties >> documented or even present in an example... > Yes, I realized that too late as well, sorry about it. For some reason > I only looked at the interrupt-map being done right and forgot to > check the ranges. > We definitely need to move the code handling the ranges into a common > location to avoid this mistake in the future. It is already in the common location, AFAIK; what's lacking there is the code that parses "dma-ranges" as well but that should be pretty easy to add... > Arnd WBR, Sergei ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-20 21:25 ` Sergei Shtylyov 0 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 21:25 UTC (permalink / raw) To: Arnd Bergmann Cc: horms, linux-sh, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, devicetree, magnus.damm, linux, linux-arm-kernel, ben.dooks On 06/21/2014 01:10 AM, Arnd Bergmann wrote: >>>> + pci0: pci@ee090000 { >>>> + compatible = "renesas,pci-r8a7790"; >>>> + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; >>>> + reg = <0x0 0xee090000 0x0 0xc00>, >>>> + <0x0 0xee080000 0x0 0x1100>; >>>> + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; >>>> + status = "disabled"; >>>> + >>>> + bus-range = <0 0>; >>>> + #address-cells = >; >>>> + #size-cells = <2>; >>>> + #interrupt-cells = <1>; >>>> + interrupt-map-mask = <0xff00 0 0 0x7>; >>>> + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >>>> + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >>>> + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; >>>> + }; >>> Hmm, this device node is not actually compliant to the PCI binding, >>> it needs a "ranges" property that can be used to look up the memory >>> and I/O space windows. >> The PCI driver doesn't support I/O space. > Well, whichever windows are registered by the driver should come > from the ranges property. I know. Too bad the driver's original author managed to misunderstand that... >>> It also needs a device_type property. >> Hm, are you sure about that? I thought only PCI devices should have it... > Yes, pretty sure it's needed in both the host controller and the > devices. I don't understand the case of the PCI devices, honestly. Shouldn't the "device_type" prop reflect the device's functionality rather than the bus where it's located? >>> I realize that the driver doesn't currently use them, but you should >>> adhere to the binding anyway, so we can fix the driver at some point. >> Sigh, agreed about the need to fix the driver. Too bad you've spoken up >> only now though. And you've ACKed the DT bindings without those properties >> documented or even present in an example... > Yes, I realized that too late as well, sorry about it. For some reason > I only looked at the interrupt-map being done right and forgot to > check the ranges. > We definitely need to move the code handling the ranges into a common > location to avoid this mistake in the future. It is already in the common location, AFAIK; what's lacking there is the code that parses "dma-ranges" as well but that should be pretty easy to add... > Arnd WBR, Sergei ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-20 21:25 ` Sergei Shtylyov 0 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 21:25 UTC (permalink / raw) To: linux-arm-kernel On 06/21/2014 01:10 AM, Arnd Bergmann wrote: >>>> + pci0: pci at ee090000 { >>>> + compatible = "renesas,pci-r8a7790"; >>>> + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; >>>> + reg = <0x0 0xee090000 0x0 0xc00>, >>>> + <0x0 0xee080000 0x0 0x1100>; >>>> + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; >>>> + status = "disabled"; >>>> + >>>> + bus-range = <0 0>; >>>> + #address-cells = >; >>>> + #size-cells = <2>; >>>> + #interrupt-cells = <1>; >>>> + interrupt-map-mask = <0xff00 0 0 0x7>; >>>> + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >>>> + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >>>> + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; >>>> + }; >>> Hmm, this device node is not actually compliant to the PCI binding, >>> it needs a "ranges" property that can be used to look up the memory >>> and I/O space windows. >> The PCI driver doesn't support I/O space. > Well, whichever windows are registered by the driver should come > from the ranges property. I know. Too bad the driver's original author managed to misunderstand that... >>> It also needs a device_type property. >> Hm, are you sure about that? I thought only PCI devices should have it... > Yes, pretty sure it's needed in both the host controller and the > devices. I don't understand the case of the PCI devices, honestly. Shouldn't the "device_type" prop reflect the device's functionality rather than the bus where it's located? >>> I realize that the driver doesn't currently use them, but you should >>> adhere to the binding anyway, so we can fix the driver at some point. >> Sigh, agreed about the need to fix the driver. Too bad you've spoken up >> only now though. And you've ACKed the DT bindings without those properties >> documented or even present in an example... > Yes, I realized that too late as well, sorry about it. For some reason > I only looked at the interrupt-map being done right and forgot to > check the ranges. > We definitely need to move the code handling the ranges into a common > location to avoid this mistake in the future. It is already in the common location, AFAIK; what's lacking there is the code that parses "dma-ranges" as well but that should be pretty easy to add... > Arnd WBR, Sergei ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes 2014-06-20 21:25 ` Sergei Shtylyov (?) @ 2014-06-21 9:15 ` Arnd Bergmann -1 siblings, 0 replies; 30+ messages in thread From: Arnd Bergmann @ 2014-06-21 9:15 UTC (permalink / raw) To: linux-arm-kernel On Saturday 21 June 2014 01:25:26 Sergei Shtylyov wrote: > On 06/21/2014 01:10 AM, Arnd Bergmann wrote: > >>> It also needs a device_type property. > > >> Hm, are you sure about that? I thought only PCI devices should have it... > > > Yes, pretty sure it's needed in both the host controller and the > > devices. > > I don't understand the case of the PCI devices, honestly. Shouldn't the > "device_type" prop reflect the device's functionality rather than the bus > where it's located? It probably made more sense on real Open Firmware than it does on FDT, but we're trying to follow the bindings anyway. I believe the "device_type=pci" originally indicated a device that has a readable config space (through the OF client interface), which is true for both the host and the devices. > >>> I realize that the driver doesn't currently use them, but you should > >>> adhere to the binding anyway, so we can fix the driver at some point. > > >> Sigh, agreed about the need to fix the driver. Too bad you've spoken up > >> only now though. And you've ACKed the DT bindings without those properties > >> documented or even present in an example... > > > Yes, I realized that too late as well, sorry about it. For some reason > > I only looked at the interrupt-map being done right and forgot to > > check the ranges. > > > We definitely need to move the code handling the ranges into a common > > location to avoid this mistake in the future. > > It is already in the common location, AFAIK; what's lacking there is the > code that parses "dma-ranges" as well but that should be pretty easy to add... We have some common helpers for "ranges" parsing already, but we should improve them so a driver has to do much less in the future. For "dma-ranges", we still to write need most of the code. For platform devices, we can now calculate the offset for the simple (no IOMMU) case, but we still need to calculate the dma mask correctly (that part should be simple, maybe you can help there as you have an immediate need), and we need to handle the IOMMU and swiotlb cases better. For PCI devices, we need to do all the same things we do for platform devices, and we can probably share most of the code. Arnd ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-21 9:15 ` Arnd Bergmann 0 siblings, 0 replies; 30+ messages in thread From: Arnd Bergmann @ 2014-06-21 9:15 UTC (permalink / raw) To: linux-arm-kernel Cc: Sergei Shtylyov, mark.rutland, devicetree, linux, pawel.moll, ijc+devicetree, linux-sh, magnus.damm, robh+dt, horms, galak, ben.dooks On Saturday 21 June 2014 01:25:26 Sergei Shtylyov wrote: > On 06/21/2014 01:10 AM, Arnd Bergmann wrote: > >>> It also needs a device_type property. > > >> Hm, are you sure about that? I thought only PCI devices should have it... > > > Yes, pretty sure it's needed in both the host controller and the > > devices. > > I don't understand the case of the PCI devices, honestly. Shouldn't the > "device_type" prop reflect the device's functionality rather than the bus > where it's located? It probably made more sense on real Open Firmware than it does on FDT, but we're trying to follow the bindings anyway. I believe the "device_type=pci" originally indicated a device that has a readable config space (through the OF client interface), which is true for both the host and the devices. > >>> I realize that the driver doesn't currently use them, but you should > >>> adhere to the binding anyway, so we can fix the driver at some point. > > >> Sigh, agreed about the need to fix the driver. Too bad you've spoken up > >> only now though. And you've ACKed the DT bindings without those properties > >> documented or even present in an example... > > > Yes, I realized that too late as well, sorry about it. For some reason > > I only looked at the interrupt-map being done right and forgot to > > check the ranges. > > > We definitely need to move the code handling the ranges into a common > > location to avoid this mistake in the future. > > It is already in the common location, AFAIK; what's lacking there is the > code that parses "dma-ranges" as well but that should be pretty easy to add... We have some common helpers for "ranges" parsing already, but we should improve them so a driver has to do much less in the future. For "dma-ranges", we still to write need most of the code. For platform devices, we can now calculate the offset for the simple (no IOMMU) case, but we still need to calculate the dma mask correctly (that part should be simple, maybe you can help there as you have an immediate need), and we need to handle the IOMMU and swiotlb cases better. For PCI devices, we need to do all the same things we do for platform devices, and we can probably share most of the code. Arnd ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-21 9:15 ` Arnd Bergmann 0 siblings, 0 replies; 30+ messages in thread From: Arnd Bergmann @ 2014-06-21 9:15 UTC (permalink / raw) To: linux-arm-kernel On Saturday 21 June 2014 01:25:26 Sergei Shtylyov wrote: > On 06/21/2014 01:10 AM, Arnd Bergmann wrote: > >>> It also needs a device_type property. > > >> Hm, are you sure about that? I thought only PCI devices should have it... > > > Yes, pretty sure it's needed in both the host controller and the > > devices. > > I don't understand the case of the PCI devices, honestly. Shouldn't the > "device_type" prop reflect the device's functionality rather than the bus > where it's located? It probably made more sense on real Open Firmware than it does on FDT, but we're trying to follow the bindings anyway. I believe the "device_type=pci" originally indicated a device that has a readable config space (through the OF client interface), which is true for both the host and the devices. > >>> I realize that the driver doesn't currently use them, but you should > >>> adhere to the binding anyway, so we can fix the driver at some point. > > >> Sigh, agreed about the need to fix the driver. Too bad you've spoken up > >> only now though. And you've ACKed the DT bindings without those properties > >> documented or even present in an example... > > > Yes, I realized that too late as well, sorry about it. For some reason > > I only looked at the interrupt-map being done right and forgot to > > check the ranges. > > > We definitely need to move the code handling the ranges into a common > > location to avoid this mistake in the future. > > It is already in the common location, AFAIK; what's lacking there is the > code that parses "dma-ranges" as well but that should be pretty easy to add... We have some common helpers for "ranges" parsing already, but we should improve them so a driver has to do much less in the future. For "dma-ranges", we still to write need most of the code. For platform devices, we can now calculate the offset for the simple (no IOMMU) case, but we still need to calculate the dma mask correctly (that part should be simple, maybe you can help there as you have an immediate need), and we need to handle the IOMMU and swiotlb cases better. For PCI devices, we need to do all the same things we do for platform devices, and we can probably share most of the code. Arnd ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes 2014-06-20 21:25 ` Sergei Shtylyov (?) @ 2014-06-23 20:40 ` Jason Gunthorpe -1 siblings, 0 replies; 30+ messages in thread From: Jason Gunthorpe @ 2014-06-23 20:40 UTC (permalink / raw) To: linux-arm-kernel > >> Hm, are you sure about that? I thought only PCI devices should have it... > > >Yes, pretty sure it's needed in both the host controller and the > >devices. > > I don't understand the case of the PCI devices, honestly. > Shouldn't the "device_type" prop reflect the device's functionality > rather than the bus where it's located? It is confusingly named, but it is required on the host bridge OF node. The spec says ' .. corresponding to a device that implements a PCI bus', which includes the host bridge. The key element is that it must be present on the node that introduces the PCI 3 dword address encoding scheme, and then on all nodes below it that use the 3 dword scheme. Otherwise Linux common OF PCI code does not work properly. Jason ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-23 20:40 ` Jason Gunthorpe 0 siblings, 0 replies; 30+ messages in thread From: Jason Gunthorpe @ 2014-06-23 20:40 UTC (permalink / raw) To: Sergei Shtylyov Cc: Arnd Bergmann, mark.rutland, devicetree, linux, pawel.moll, ijc+devicetree, linux-sh, magnus.damm, robh+dt, horms, galak, ben.dooks, linux-arm-kernel > >> Hm, are you sure about that? I thought only PCI devices should have it... > > >Yes, pretty sure it's needed in both the host controller and the > >devices. > > I don't understand the case of the PCI devices, honestly. > Shouldn't the "device_type" prop reflect the device's functionality > rather than the bus where it's located? It is confusingly named, but it is required on the host bridge OF node. The spec says ' .. corresponding to a device that implements a PCI bus', which includes the host bridge. The key element is that it must be present on the node that introduces the PCI 3 dword address encoding scheme, and then on all nodes below it that use the 3 dword scheme. Otherwise Linux common OF PCI code does not work properly. Jason ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-23 20:40 ` Jason Gunthorpe 0 siblings, 0 replies; 30+ messages in thread From: Jason Gunthorpe @ 2014-06-23 20:40 UTC (permalink / raw) To: linux-arm-kernel > >> Hm, are you sure about that? I thought only PCI devices should have it... > > >Yes, pretty sure it's needed in both the host controller and the > >devices. > > I don't understand the case of the PCI devices, honestly. > Shouldn't the "device_type" prop reflect the device's functionality > rather than the bus where it's located? It is confusingly named, but it is required on the host bridge OF node. The spec says ' .. corresponding to a device that implements a PCI bus', which includes the host bridge. The key element is that it must be present on the node that introduces the PCI 3 dword address encoding scheme, and then on all nodes below it that use the 3 dword scheme. Otherwise Linux common OF PCI code does not work properly. Jason ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes 2014-06-20 21:00 ` Sergei Shtylyov (?) @ 2014-06-20 21:16 ` Sergei Shtylyov -1 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 21:16 UTC (permalink / raw) To: linux-arm-kernel Hello. On 06/21/2014 01:00 AM, Sergei Shtylyov wrote: >>> + pci0: pci@ee090000 { >>> + compatible = "renesas,pci-r8a7790"; >>> + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; >>> + reg = <0x0 0xee090000 0x0 0xc00>, >>> + <0x0 0xee080000 0x0 0x1100>; >>> + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; >>> + status = "disabled"; >>> + >>> + bus-range = <0 0>; >>> + #address-cells = >; >>> + #size-cells = <2>; >>> + #interrupt-cells = <1>; >>> + interrupt-map-mask = <0xff00 0 0 0x7>; >>> + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >>> + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >>> + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; >>> + }; >> Hmm, this device node is not actually compliant to the PCI binding, >> it needs a "ranges" property that can be used to look up the memory >> and I/O space windows. > The PCI driver doesn't support I/O space. Meaning that the hardware doesn't support it... >> It also needs a device_type property. > Hm, are you sure about that? I thought only PCI devices should have it... Seeing it now in 'bus.pci.pdf'. However, it's strange that the PCI devices also seem to need that... >> Arnd WBR, Sergei ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-20 21:16 ` Sergei Shtylyov 0 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 21:16 UTC (permalink / raw) To: Arnd Bergmann Cc: horms, linux-sh, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, devicetree, magnus.damm, linux, linux-arm-kernel, ben.dooks Hello. On 06/21/2014 01:00 AM, Sergei Shtylyov wrote: >>> + pci0: pci@ee090000 { >>> + compatible = "renesas,pci-r8a7790"; >>> + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; >>> + reg = <0x0 0xee090000 0x0 0xc00>, >>> + <0x0 0xee080000 0x0 0x1100>; >>> + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; >>> + status = "disabled"; >>> + >>> + bus-range = <0 0>; >>> + #address-cells = >; >>> + #size-cells = <2>; >>> + #interrupt-cells = <1>; >>> + interrupt-map-mask = <0xff00 0 0 0x7>; >>> + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >>> + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >>> + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; >>> + }; >> Hmm, this device node is not actually compliant to the PCI binding, >> it needs a "ranges" property that can be used to look up the memory >> and I/O space windows. > The PCI driver doesn't support I/O space. Meaning that the hardware doesn't support it... >> It also needs a device_type property. > Hm, are you sure about that? I thought only PCI devices should have it... Seeing it now in 'bus.pci.pdf'. However, it's strange that the PCI devices also seem to need that... >> Arnd WBR, Sergei ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes @ 2014-06-20 21:16 ` Sergei Shtylyov 0 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 21:16 UTC (permalink / raw) To: linux-arm-kernel Hello. On 06/21/2014 01:00 AM, Sergei Shtylyov wrote: >>> + pci0: pci at ee090000 { >>> + compatible = "renesas,pci-r8a7790"; >>> + clocks = <&mstp7_clks R8A7790_CLK_EHCI>; >>> + reg = <0x0 0xee090000 0x0 0xc00>, >>> + <0x0 0xee080000 0x0 0x1100>; >>> + interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>; >>> + status = "disabled"; >>> + >>> + bus-range = <0 0>; >>> + #address-cells = >; >>> + #size-cells = <2>; >>> + #interrupt-cells = <1>; >>> + interrupt-map-mask = <0xff00 0 0 0x7>; >>> + interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >>> + 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH >>> + 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>; >>> + }; >> Hmm, this device node is not actually compliant to the PCI binding, >> it needs a "ranges" property that can be used to look up the memory >> and I/O space windows. > The PCI driver doesn't support I/O space. Meaning that the hardware doesn't support it... >> It also needs a device_type property. > Hm, are you sure about that? I thought only PCI devices should have it... Seeing it now in 'bus.pci.pdf'. However, it's strange that the PCI devices also seem to need that... >> Arnd WBR, Sergei ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 2/2] ARM: shmobile: lager: enable internal PCI 2014-06-20 20:34 ` Sergei Shtylyov (?) @ 2014-06-20 20:38 ` Sergei Shtylyov -1 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 20:38 UTC (permalink / raw) To: linux-arm-kernel From: Ben Dooks <ben.dooks@codethink.co.uk> Enable internal AHB-PCI bridges for the USB EHCI/OHCI controllers attached to them. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> Reviewed-by: Ian Molton <ian.molton@codethink.co.uk> [Sergei: enabled PCI0] Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- Changes in version 4: - refreshed the patch. Changes in version 3: - refreshed the patch. Changes in version 2: - enabled PCI0 device; - reworded summary and changelog; - refreshed the patch. arch/arm/boot/dts/r8a7790-lager.dts | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) Index: renesas/arch/arm/boot/dts/r8a7790-lager.dts =================================--- renesas.orig/arch/arm/boot/dts/r8a7790-lager.dts +++ renesas/arch/arm/boot/dts/r8a7790-lager.dts @@ -219,6 +219,21 @@ renesas,groups = "i2c3"; renesas,function = "i2c3"; }; + + usb0_pins: usb0 { + renesas,groups = "usb0"; + renesas,function = "usb0"; + }; + + usb1_pins: usb1 { + renesas,groups = "usb1"; + renesas,function = "usb1"; + }; + + usb2_pins: usb2 { + renesas,groups = "usb2"; + renesas,function = "usb2"; + }; }; ðer { @@ -368,3 +383,21 @@ regulator-always-on; }; }; + +&pci0 { + status = "okay"; + pinctrl-0 = <&usb0_pins>; + pinctrl-names = "default"; +}; + +&pci1 { + status = "okay"; + pinctrl-0 = <&usb1_pins>; + pinctrl-names = "default"; +}; + +&pci2 { + status = "okay"; + pinctrl-0 = <&usb2_pins>; + pinctrl-names = "default"; +}; ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 2/2] ARM: shmobile: lager: enable internal PCI @ 2014-06-20 20:38 ` Sergei Shtylyov 0 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 20:38 UTC (permalink / raw) To: horms, linux-sh, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, devicetree Cc: magnus.damm, linux, linux-arm-kernel, ben.dooks From: Ben Dooks <ben.dooks@codethink.co.uk> Enable internal AHB-PCI bridges for the USB EHCI/OHCI controllers attached to them. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> Reviewed-by: Ian Molton <ian.molton@codethink.co.uk> [Sergei: enabled PCI0] Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- Changes in version 4: - refreshed the patch. Changes in version 3: - refreshed the patch. Changes in version 2: - enabled PCI0 device; - reworded summary and changelog; - refreshed the patch. arch/arm/boot/dts/r8a7790-lager.dts | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) Index: renesas/arch/arm/boot/dts/r8a7790-lager.dts =================================================================== --- renesas.orig/arch/arm/boot/dts/r8a7790-lager.dts +++ renesas/arch/arm/boot/dts/r8a7790-lager.dts @@ -219,6 +219,21 @@ renesas,groups = "i2c3"; renesas,function = "i2c3"; }; + + usb0_pins: usb0 { + renesas,groups = "usb0"; + renesas,function = "usb0"; + }; + + usb1_pins: usb1 { + renesas,groups = "usb1"; + renesas,function = "usb1"; + }; + + usb2_pins: usb2 { + renesas,groups = "usb2"; + renesas,function = "usb2"; + }; }; ðer { @@ -368,3 +383,21 @@ regulator-always-on; }; }; + +&pci0 { + status = "okay"; + pinctrl-0 = <&usb0_pins>; + pinctrl-names = "default"; +}; + +&pci1 { + status = "okay"; + pinctrl-0 = <&usb1_pins>; + pinctrl-names = "default"; +}; + +&pci2 { + status = "okay"; + pinctrl-0 = <&usb2_pins>; + pinctrl-names = "default"; +}; ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 2/2] ARM: shmobile: lager: enable internal PCI @ 2014-06-20 20:38 ` Sergei Shtylyov 0 siblings, 0 replies; 30+ messages in thread From: Sergei Shtylyov @ 2014-06-20 20:38 UTC (permalink / raw) To: linux-arm-kernel From: Ben Dooks <ben.dooks@codethink.co.uk> Enable internal AHB-PCI bridges for the USB EHCI/OHCI controllers attached to them. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> Reviewed-by: Ian Molton <ian.molton@codethink.co.uk> [Sergei: enabled PCI0] Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- Changes in version 4: - refreshed the patch. Changes in version 3: - refreshed the patch. Changes in version 2: - enabled PCI0 device; - reworded summary and changelog; - refreshed the patch. arch/arm/boot/dts/r8a7790-lager.dts | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) Index: renesas/arch/arm/boot/dts/r8a7790-lager.dts =================================================================== --- renesas.orig/arch/arm/boot/dts/r8a7790-lager.dts +++ renesas/arch/arm/boot/dts/r8a7790-lager.dts @@ -219,6 +219,21 @@ renesas,groups = "i2c3"; renesas,function = "i2c3"; }; + + usb0_pins: usb0 { + renesas,groups = "usb0"; + renesas,function = "usb0"; + }; + + usb1_pins: usb1 { + renesas,groups = "usb1"; + renesas,function = "usb1"; + }; + + usb2_pins: usb2 { + renesas,groups = "usb2"; + renesas,function = "usb2"; + }; }; ðer { @@ -368,3 +383,21 @@ regulator-always-on; }; }; + +&pci0 { + status = "okay"; + pinctrl-0 = <&usb0_pins>; + pinctrl-names = "default"; +}; + +&pci1 { + status = "okay"; + pinctrl-0 = <&usb1_pins>; + pinctrl-names = "default"; +}; + +&pci2 { + status = "okay"; + pinctrl-0 = <&usb2_pins>; + pinctrl-names = "default"; +}; ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2014-06-23 20:40 UTC | newest] Thread overview: 30+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-06-20 20:34 [PATCH v4 0/2] Add R8A7790/Lager board PCI USB DT support Sergei Shtylyov 2014-06-20 20:34 ` Sergei Shtylyov 2014-06-20 20:34 ` Sergei Shtylyov 2014-06-20 20:36 ` [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes Sergei Shtylyov 2014-06-20 20:36 ` Sergei Shtylyov 2014-06-20 20:36 ` Sergei Shtylyov 2014-06-20 20:51 ` Arnd Bergmann 2014-06-20 20:51 ` Arnd Bergmann 2014-06-20 20:51 ` Arnd Bergmann 2014-06-20 21:00 ` Sergei Shtylyov 2014-06-20 21:00 ` Sergei Shtylyov 2014-06-20 21:00 ` Sergei Shtylyov 2014-06-20 21:10 ` Arnd Bergmann 2014-06-20 21:10 ` Arnd Bergmann 2014-06-20 21:10 ` Arnd Bergmann 2014-06-20 21:25 ` Sergei Shtylyov 2014-06-20 21:25 ` Sergei Shtylyov 2014-06-20 21:25 ` Sergei Shtylyov 2014-06-21 9:15 ` Arnd Bergmann 2014-06-21 9:15 ` Arnd Bergmann 2014-06-21 9:15 ` Arnd Bergmann 2014-06-23 20:40 ` Jason Gunthorpe 2014-06-23 20:40 ` Jason Gunthorpe 2014-06-23 20:40 ` Jason Gunthorpe 2014-06-20 21:16 ` Sergei Shtylyov 2014-06-20 21:16 ` Sergei Shtylyov 2014-06-20 21:16 ` Sergei Shtylyov 2014-06-20 20:38 ` [PATCH v4 2/2] ARM: shmobile: lager: enable internal PCI Sergei Shtylyov 2014-06-20 20:38 ` Sergei Shtylyov 2014-06-20 20:38 ` Sergei Shtylyov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.