On Tue, Mar 26, 2013 at 05:18:32PM +0100, Thomas Petazzoni wrote:

> +             pcie-controller {
> +                     compatible = "marvell,armada-370-pcie";
> +                     status = "disabled";
> +                     device_type = "pci";
> +
> +                     #address-cells = <3>;
> +                     #size-cells = <2>;
> +
> +                     bus-range = <0x00 0xff>;
> +
> +                     reg = <0xd0040000 0x2000>, <0xd0080000 0x2000>;
> +
> +                     reg-names = "pcie0.0", "pcie1.0";
> +
> +                     ranges = <0x82000000 0 0xe0000000 0xe0000000 0 
> 0x08000000   /* non-prefetchable memory */
> +                               0x81000000 0 0          0xe8000000 0 
> 0x00100000>; /* downstream I/O */
> +
> +                     pcie@1,0 {
> +                             device_type = "pci";
> +                             reg = <0x0800 0 0 0 0>;
> +                             #address-cells = <3>;
> +                             #size-cells = <2>;

Very Minor Nit: These two # fields are not strictly necessary

> +                             #interrupt-cells = <1>;
> +                             interrupt-map-mask = <0 0 0 0>;
> +                             interrupt-map = <0 0 0 0 &mpic 58>;
> +                             marvell,pcie-port = <0>;
> +                             marvell,pcie-lane = <0>;
> +                             clocks = <&gateclk 5>;
> +                             status = "disabled";
> +                     };
> +
> +                     pcie@2,0 {
> +                             device_type = "pci";
> +                             reg = <0x1000 0 0 0 0>;
> +                             #address-cells = <3>;
> +                             #size-cells = <2>;
> +                             #interrupt-cells = <1>;
> +                             interrupt-map-mask = <0 0 0 0>;
> +                             interrupt-map = <0 0 0 0 &mpic 62>;
> +                             marvell,pcie-port = <1>;
> +                             marvell,pcie-lane = <0>;
> +                             clocks = <&gateclk 9>;
> +                             status = "disabled";
> +                     };
> +             };
>       };
>  };


This basically looks fine to me, however, I think it is valuable if
you and Thierry could use the same method to pass per-port registers. I
expect others are going to reference these bindings for future work,
and one standard method is more clear than two.

Thierry: Did you settle on using assigned-addresses? Can you share the
final binding for your driver?

Jingoo Han's driver for Exynos uses lots of per-port registers, so I'm
inclined to think that assigned-addresses is the clearer way forward.

This is a fairly minor comment. Would people be comfortable going in as-is
with a small follow-up revision to the DT?

Regards,
Jason
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Reply via email to