diff mbox

[v7,4/4] arm64: Add APM X-Gene SoC SATA host controller DTS entries

Message ID 1388708539-333-5-git-send-email-lho@apm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Loc Ho Jan. 3, 2014, 12:22 a.m. UTC
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 |   81 ++++++++++++++++++++++++++++++++++++
 1 files changed, 81 insertions(+), 0 deletions(-)

Comments

Arnd Bergmann Jan. 3, 2014, 3:33 p.m. UTC | #1
On Friday 03 January 2014, Loc Ho wrote:
> +                       sata23clk: sata23clk@1f22c000 {
> +                               compatible = "apm,xgene-device-clock";
> +                               #clock-cells = <1>;
> +                               clocks = <&socplldiv2 0>;
> +                               clock-names = "sata23clk";

> +                       };
> +
> +                       sata45clk: sata45clk@1f23c000 {
> +                               compatible = "apm,xgene-device-clock";
> +                               #clock-cells = <1>;
> +                               clocks = <&socplldiv2 0>;
> +                               clock-names = "sata45clk";

Something is wrong here: You have two devices with the same "compatible"
string but using different "clock-names" strings. The binding document
lists this as an optional property with the description  "shall be
the name of the device clock. If missing, use the device name", which
doesn't seem to make any sense.

Please fix the binding and the existing users of this, and don't introduce
any more broken instances. Since each device clock is documented to
have only one parent anyway, please just make it an anonymous clock.

	Arnd
Loc Ho Jan. 3, 2014, 5:23 p.m. UTC | #2
Hi,

>> +                       sata23clk: sata23clk@1f22c000 {
>> +                               compatible = "apm,xgene-device-clock";
>> +                               #clock-cells = <1>;
>> +                               clocks = <&socplldiv2 0>;
>> +                               clock-names = "sata23clk";
>
>> +                       };
>> +
>> +                       sata45clk: sata45clk@1f23c000 {
>> +                               compatible = "apm,xgene-device-clock";
>> +                               #clock-cells = <1>;
>> +                               clocks = <&socplldiv2 0>;
>> +                               clock-names = "sata45clk";
>
> Something is wrong here: You have two devices with the same "compatible"
> string but using different "clock-names" strings. The binding document
> lists this as an optional property with the description  "shall be
> the name of the device clock. If missing, use the device name", which
> doesn't seem to make any sense.
>
> Please fix the binding and the existing users of this, and don't introduce
> any more broken instances. Since each device clock is documented to
> have only one parent anyway, please just make it an anonymous clock.

Okay I miss understood this... I will need to fix the X-Gene device
parent clocks as well in an separate patch to the clock driver owner.

-Loc
Arnd Bergmann Jan. 3, 2014, 7:28 p.m. UTC | #3
On Friday 03 January 2014, Loc Ho wrote:
> Okay I miss understood this... I will need to fix the X-Gene device
> parent clocks as well in an separate patch to the clock driver owner.

Ok, thanks!

	Arnd
diff mbox

Patch

diff --git a/arch/arm64/boot/dts/apm-storm.dtsi b/arch/arm64/boot/dts/apm-storm.dtsi
index fd2b662..5b09797 100644
--- a/arch/arm64/boot/dts/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm-storm.dtsi
@@ -218,6 +218,48 @@ 
 				enable-offset = <0x0>;
 				enable-mask = <0x06>;
 			};
+
+			sata01clk: sata01clk@1f21c000 {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				clock-names = "sata01clk";
+				reg = <0x0 0x1f21c000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "sata01clk";
+				csr-offset = <0x4>;
+				csr-mask = <0x05>;
+				enable-offset = <0x0>;
+				enable-mask = <0x39>;
+			};
+
+			sata23clk: sata23clk@1f22c000 {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				clock-names = "sata23clk";
+				reg = <0x0 0x1f22c000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "sata23clk";
+				csr-offset = <0x4>;
+				csr-mask = <0x05>;
+				enable-offset = <0x0>;
+				enable-mask = <0x39>;
+			};
+
+			sata45clk: sata45clk@1f23c000 {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				clock-names = "sata45clk";
+				reg = <0x0 0x1f23c000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "sata45clk";
+				csr-offset = <0x4>;
+				csr-mask = <0x05>;
+				enable-offset = <0x0>;
+				enable-mask = <0x39>;
+			};
 		};
 
 		serial0: serial@1c020000 {
@@ -266,5 +308,44 @@ 
 			apm,tx-boost-gain = <2 3 3 2 3 3>;
 			apm,tx-eye-tuning = <2 10 10 2 10 12>;
 		};
+
+		sata1: sata@1a000000 {
+			compatible = "apm,xgene-ahci-sgmii";
+			reg = <0x0 0x1a000000 0x0 0x1000>,
+			      <0x0 0x1f210000 0x0 0x10000>,
+			      <0x0 0x1f2a0000 0x0 0x10000>,
+			      <0x0 0x1c000200 0x0 0x100>;
+			interrupts = <0x0 0x86 0x4>;
+			status = "disabled";
+			clocks = <&sata01clk 0>;
+			phys = <&phy1 0>;
+			phy-names = "sata-6g";
+		};
+
+		sata2: sata@1a400000 {
+			compatible = "apm,xgene-ahci-sgmii";
+			reg = <0x0 0x1a400000 0x0 0x1000>,
+			      <0x0 0x1f220000 0x0 0x10000>,
+			      <0x0 0x1f2a0000 0x0 0x10000>,
+			      <0x0 0x1c000200 0x0 0x100>;
+			interrupts = <0x0 0x87 0x4>;
+			status = "ok";
+			clocks = <&sata23clk 0>;
+			phys = <&phy2 0>;
+			phy-names = "sata-6g";
+		};
+
+		sata3: sata@1a800000 {
+			compatible = "apm,xgene-ahci-pcie";
+			reg = <0x0 0x1a800000 0x0 0x1000>,
+			      <0x0 0x1f230000 0x0 0x10000>,
+			      <0x0 0x1f2a0000 0x0 0x10000>,
+			      <0x0 0x1c000200 0x0 0x100>;
+			interrupts = <0x0 0x88 0x4>;
+			status = "ok";
+			clocks = <&sata45clk 0>;
+			phys = <&phy3 0>;
+			phy-names = "sata-6g";
+		};
 	};
 };