On Thursday 13 March 2014 09:50:04 Phil Edworthy wrote:
> This patch adds the bindings for the rcar PCIE driver. The driver
> resides under drivers/pci/host/pcie-rcar.c
> 
> Signed-off-by: Phil Edworthy <[email protected]>
> ---
>  Documentation/devicetree/bindings/pci/rcar-pci.txt | 40 
> ++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci.txt
> 
> diff --git a/Documentation/devicetree/bindings/pci/rcar-pci.txt 
> b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> new file mode 100644
> index 0000000..0e219b0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> @@ -0,0 +1,40 @@
> +* Renesas RCar PCIe interface
> +
> +Required properties:
> +- compatible: should contain one of the following
> +     "renesas,r8a7779-pcie", "renesas,r8a7790-pcie", "renesas,r8a7791-pcie"
> +- reg: base addresses and lengths of the pcie controller.
> +- #address-cells: set to <3>
> +- #size-cells: set to <2>
> +- device_type: set to "pci"
> +- ranges: ranges for the PCI memory and I/O regions

I see your description and example includes I/O regions, but the driver
claims in a comment that these don't work. Do you also have a patch to
make them work?

In case that patch is part of the 9-patch series, do you mind adding the
arm kernel mailing list (and maybe me personally) to Cc the next time?

> +- interrupts: interrupt values for MSI interrupt
> +- #interrupt-cells: set to <1>
> +- interrupt-map-mask and interrupt-map: standard PCI properties
> +     to define the mapping of the PCIe interface to interrupt
> +     numbers.
> +- clocks: from common clock binding: handle to pci clock.
> +- clock-names: from common clock binding: should be "pcie"
> +
> +Example:
> +
> +SoC specific DT Entry:
> +
> +     pcie: pcie@0x01000000 {
> +             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>;

I'm curious about why there are two non-prefetchable regions. What is
the significance of this?

> +             interrupts = <0 116 4>;
> +             #interrupt-cells = <1>;
> +             interrupt-map-mask = <0 0 0 0>;
> +             interrupt-map = <0 0 0 0 &gic 0 116 4>;
> +             clocks = <&mstp3_clks R8A7790_CLK_PCIE>;
> +             clock-names = "pcie";
> +             status = "disabled";
> +     };

Are you sure that there is only one legacy interrupt?  Most host
controllers wire up IntA through IntD to different output pins.

Otherwise looks good.

        Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to