Re: [PATCH v3 4/4] arm64: Add APM X-Gene SoC 15Gbps Multi-purpose PHY DTS entries

[Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>]

Hello.

On 26-11-2013 9:46, Loc Ho wrote:

> arm64: Add APM X-Gene SoC 15Gbps Multi-purpose PHY DTS entries

    As I already said, there's no need to repeat the patch summary line (the 
mail subject) in the changelog verbatim.

> This patch adds the DTS entries for the APM X-Gene SoC 15Gbps Multi-purpose
> PHY driver. The PHY for SATA controller 0 and 1 are enabled by default.

> Signed-off-by: Loc Ho <lho@apm.com>
> Signed-off-by: Tuan Phan <tphan@apm.com>
> Signed-off-by: Suman Tripathi <stripathi@apm.com>
> ---
>   arch/arm64/boot/dts/apm-storm.dtsi |   31 +++++++++++++++++++++++++++++++
>   1 files changed, 31 insertions(+), 0 deletions(-)

> diff --git a/arch/arm64/boot/dts/apm-storm.dtsi b/arch/arm64/boot/dts/apm-storm.dtsi
> index 359d7b6..f74c26a 100644
> --- a/arch/arm64/boot/dts/apm-storm.dtsi
> +++ b/arch/arm64/boot/dts/apm-storm.dtsi
> @@ -193,5 +193,36 @@
>   			reg = <0x0 0x17000014 0x0 0x100>;
>   			mask = <0x1>;
>   		};
> +
> +		phy1: phy@1f210000 {

    Shouldn't the address part of the node name correspond to the first 
address specifier in the "reg" prop?

> +			compatible = "apm,xgene-phy";
> +			reg = <0x0 0x1f21a000 0x0 0x100>,
> +			      <0x0 0x1f21c000 0x0 0x100>;
> +			#phy-cells = <1>;
> +			status = "disabled";
> +			apm-tx-boost-gain = <0x2 0x2 0x2 0x2 0x2 0x2>;
> +			apm-tx-eye-tuning = <0xa 0xa 0xa 0xa 0xa 0xa>;

    The vendor name should be separated by comma in the vendor specific prop 
names.

> +		};

    Same comments about the other 2 PHY nodes.

WBR, Sergei