Hi Phil,

On Tue, Apr 5, 2016 at 10:56 AM, Phil Edworthy
<[email protected]> wrote:
> Signed-off-by: Phil Edworthy <[email protected]>

Thanks for your patch!

Unfortunately you seem to have missed my earlier review comments.

> ---
>  arch/arm64/boot/dts/renesas/r8a7795.dtsi | 58 
> ++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi 
> b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> index 868c10e..6cfc248 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> @@ -131,6 +131,15 @@
>                 status = "disabled";
>         };
>
> +       /* External PCIe clock - can be overridden by the board */
> +       pcie_bus_clk: pcie_bus_clk {

Please drop the "_clk" suffix from the device node's name.

> +               compatible = "fixed-clock";
> +               #clock-cells = <0>;
> +               clock-frequency = <100000000>;
> +               clock-output-names = "pcie_bus";

The "clock-output-names" property is optional and not needed. Please drop it,
the clock will be named after the device node's name.

> +               status = "disabled";
> +       };
> +
>         soc {
>                 compatible = "simple-bus";
>                 interrupt-parent = <&gic>;
> @@ -1156,5 +1165,54 @@
>                         power-domains = <&cpg>;
>                         status = "disabled";
>                 };
> +               pciec0: pcie@fe000000 {
> +                       compatible = "renesas,pcie-r8a7795";
> +                       reg = <0 0xfe000000 0 0x80000>;
> +                       #address-cells = <3>;
> +                       #size-cells = <2>;
> +                       bus-range = <0x00 0xff>;
> +                       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 
> 0x40000000>;
> +                       interrupts = <0 116 IRQ_TYPE_LEVEL_HIGH>,
> +                               <0 117 IRQ_TYPE_LEVEL_HIGH>,
> +                               <0 118 IRQ_TYPE_LEVEL_HIGH>;

<GIC_SPI ...> (x3)

> +                       #interrupt-cells = <1>;
> +                       interrupt-map-mask = <0 0 0 0>;
> +                       interrupt-map = <0 0 0 0 &gic 0 116 
> IRQ_TYPE_LEVEL_HIGH>;

... &gic GIC_SPI ...

> +                       clocks = <&cpg CPG_MOD 319>, <&pcie_bus_clk>;
> +                       clock-names = "pcie", "pcie_bus";
> +                       power-domains = <&cpg>;
> +                       status = "disabled";
> +               };
> +
> +               pciec1: pcie@ee800000 {
> +                       compatible = "renesas,pcie-r8a7795";
> +                       reg = <0 0xee800000 0 0x80000>;
> +                       #address-cells = <3>;
> +                       #size-cells = <2>;
> +                       bus-range = <0x00 0xff>;
> +                       device_type = "pci";
> +                       ranges = <0x01000000 0 0x00000000 0 0xee900000 0 
> 0x00100000
> +                               0x02000000 0 0xeea00000 0 0xeea00000 0 
> 0x00200000
> +                               0x02000000 0 0xc0000000 0 0xc0000000 0 
> 0x08000000
> +                               0x42000000 0 0xc8000000 0 0xc8000000 0 
> 0x08000000>;
> +                       /* Map all possible DDR as inbound ranges */
> +                       dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 
> 0x40000000>;
> +                       interrupts = <0 148 IRQ_TYPE_LEVEL_HIGH>,
> +                               <0 149 IRQ_TYPE_LEVEL_HIGH>,
> +                               <0 150 IRQ_TYPE_LEVEL_HIGH>;

<GIC_SPI ...> (x3)

> +                       #interrupt-cells = <1>;
> +                       interrupt-map-mask = <0 0 0 0>;
> +                       interrupt-map = <0 0 0 0 &gic 0 148 
> IRQ_TYPE_LEVEL_HIGH>;

... &gic GIC_SPI ...

> +                       clocks = <&cpg CPG_MOD 318>, <&pcie_bus_clk>;
> +                       clock-names = "pcie", "pcie_bus";
> +                       power-domains = <&cpg>;
> +                       status = "disabled";
> +               };
>         };
>  };

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

Reply via email to