Skip to contentSkip to conversation

ata: Add APM X-Gene SoC AHCI SATA host controller support

PATCH New Loc Ho 4 participants 13 messages Mar 14, 2014 – Apr 28, 2014

Subsystems (3)
lore
Share & tools
lore

Loc Ho

author
#0 lore raw

v18 cover ata: Add APM X-Gene SoC AHCI SATA host controller support

This patch adds support for the APM X-Gene SoC AHCI SATA host controller. In order for the host controller to work, the corresponding PHY driver musts also be available. Currently, only Gen3 disk is supported with this initial version.

v18:
* Remove clock-name properity from dts nodes
* Update binding document
* Clean up dt compatible string
* Remove IP module offset defines
* Spilt resource region into individual modules - core, diagnostic, AXI, and MUX
* Update function xgene_ahci_init_memram, xgene_ahci_hw_init, xgene_ahci_mux_select, and xgene_ahci_probe due to resource spilt
* Update function comment in function xgene_ahci_do_hardreset
* Change DMA coherent mask back to 64

v17:
* Add comment on no support for PM currently
* Add xgene_ahci_host_stop function to support stopping the controller
* Remove call to phy_exit as not necessary with new ahci_platform code

v16:
* Rebase to libata-for-3.15
* Pull in the PHY DTS patch as the host controller DTS patch depends on it

v15:
* Rebase to libata next branch
* Remove field plat_data and PHY from context structure
* Fix comment on function xgene_ahci_read_id as well as using bit mask to clear DEVSLP bit
* Remove function xgene_ahci_force_phy_rdy and xgene_ahci_phy_restart as not required since Gen1/Gen2 support remove for this initial version
* Update function xgene_ahci_do_hardreset comment
* Remove Gen1/Gen2 support from function xgene_ahci_do_hardreset
* Change int to u32 for variable portcmd_saved in function xgene_ahci_hardreset
* Change variable hplat_data to ctx in function xgene_ahci_probe
* Remove PHY call and using ahci_platform_enable_resource instead
* Add ahci_patlform_remove_one to driver function .remove
* Change phy-name to "sata-phy"

v14:
* Remove the shutdown already check and replace the while loop check with msleep in function xgene_ahci_init_memram

v13:
* Add fully-winged style comment for function xgene_ahci_read_id and xgene_ahci_do_hardrest
* Minor comments update for function xgene_ahci_read_id, xgene_ahci_do_hardrest, and xgene_ahci_hardreset
* NOTE: There is no functional code change.

v12:
* Remove function xgene_ahci_get_channel and use the ata_port field port_no
* Update comment for function xgene_ahci_read_id to function comment style '/**'
* Update comment for multiple lines to fully-winged style

v11:
* Drop the export functions requirement with libachi
* Change CONFIG_SATA_XGENE to CONFIG_AHCI_XGENE
* Rename file sata_xgene.c to ahci_xgene.c
* Convert to use Hans De Geode version 5 ahci_platform code re-factor changes to reduce code duplication. For extra context, use plat_data to store our context. The probe function follows the ahci_sunxi implementation. A number of code fragments update to reflect this change.
* Update comment for function xgene_ahci_read_id
* Minor code move around in function xgene_ahci_do_hardreset and use ATA_BUSY instead 0x80
* Fix hardreset to use start_engine function pointer as required due to newer kernel rebased
* Fix the set DMA mask for 32-bit as well

v10:
* Update binding documentation

v9:
* Remove ACPI/EFI include files
* Remove the IO flush support, interrupt routine, and DTS resources
* Remove function xgene_rd, xgene_wr, and xgene_wr_flush
* Remove PMP support (function xgene_ahci_qc_issue, xgene_ahci_qc_prep, xgene_ahci_qc_fill_rtf, xgene_ahci_softreset, and xgene_ahci_do_softreset)
* Rename function xgene_ahci_enable_phy to xgene_ahci_force_phy_rdy
* Clean up hardreset functions
* Require v7 of the PHY driver

v8:
* Remove _ADDR from defines
* Remove define MSTAWAUX_COHERENT_BYPASS_SET and STARAUX_COHERENT_BYPASS_SET and use direct coding
* Remove the un-necessary check for DTS boot with built in ACPI table
* Switch to use dma_set_mask_and_coherent for setting DMA mask
* Remove ACPI table matching code
* Update clock-names for sata01clk, sata23clk, and sata45clk

v7:
* Update the clock code by toggle the clock
* Update the DTS clock mask values due to the clock spilt between host and v5 of the PHY drivers

v6:
* Update binding documentation
* Change select PHY_XGENE_SATA to PHY_XGENE
* Add ULL to constants
* Change indentation and comments
* Clean up the probe functions a bit more
* Remove xgene_ahci_remove function
* Add the flush register to DTS
* Remove the interrupt-parent from DTS

v5:
* Sync up to v3 of the PHY driver
* Remove MSLIM wrapper functions
* Change the memory shutdown loop to use usleep_range
* Use devm_ioremap_resource instead devm_ioremap
* Remove suspend/resume functions as not needed

v4:
* Remove the ID property in DT
* Remove the temporary PHY direct function call and use PHY function
* Change printk to pr_debug
* Move the IOB flush addresses into the DT
* Remove the parameters retrieval function as no longer needed
* Remove the header file as no longer needed
* Require v2 patch of the SATA PHY driver. Require slightly modification in the Kconfig as it is moved to folder driver/phy and use Kconfig PHY_XGENE_SATA instead SATA_XGENE_PHY.

v3:
* Move out the SATA PHY to another driver
* Remove the clock-cells entry from DTS
* Remove debug wrapper
* Remove delay functions wrapper
* Clean up resource and IRQ query
* Remove query clock name
* Switch to use dma_set_mask/dma_coherent_mask
* Remove un-necessary devm_kfree
* Update GPL license header to v2
* Spilt up function xgene_ahci_hardreset
* Spilt up function xgene_ahci_probe
* Remove all reference of CONFIG_ARCH_MSLIM
* Clean up chip revision code

v2:
* Clean up file sata_xgene.c with Lindent and etc
* Clean up file sata_xgene_serdes.c with Lindent and etc
* Add description to each patch

v1:
* inital version

---
Loc Ho (4):
arm64: Add APM X-Gene SoC 15Gbps Multi-purpose PHY DTS entries
Documentation: Add documentation for the APM X-Gene SoC SATA host
controller DTS binding
ata: Add APM X-Gene SoC AHCI SATA host controller driver
arm64: Add APM X-Gene SoC AHCI SATA host controller DTS entries

 .../devicetree/bindings/ata/apm-xgene.txt          |   76 +++
 arch/arm64/boot/dts/apm-storm.dtsi                 |  152 ++++++
 drivers/ata/Kconfig                                |    7 +
 drivers/ata/Makefile                               |    1 +
 drivers/ata/ahci_xgene.c                           |  486 ++++++++++++++++++++
 5 files changed, 722 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ata/apm-xgene.txt
 create mode 100644 drivers/ata/ahci_xgene.c
author
#1 lore raw

PATCH v18 1/4 arm64: Add APM X-Gene SoC 15Gbps Multi-purpose PHY DTS entries

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

---
 arch/arm64/boot/dts/apm-storm.dtsi |   72 ++++++++++++++++++++++++++++++++++++
 1 files changed, 72 insertions(+), 0 deletions(-)
arch/arm64/boot/dts/apm-storm.dtsi +72 -0
index d37d736..6d4f493 100644
@@ -176,6 +176,48 @@
 				reg-names = "csr-reg";
 				clock-output-names = "eth8clk";
 			};
+
+			sataphy1clk: sataphy1clk at 1f21c000 {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				reg = <0x0 0x1f21c000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "sataphy1clk";
+				status = "disabled";
+				csr-offset = <0x4>;
+				csr-mask = <0x00>;
+				enable-offset = <0x0>;
+				enable-mask = <0x06>;
+			};
+
+			sataphy2clk: sataphy1clk at 1f22c000 {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				reg = <0x0 0x1f22c000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "sataphy2clk";
+				status = "ok";
+				csr-offset = <0x4>;
+				csr-mask = <0x3a>;
+				enable-offset = <0x0>;
+				enable-mask = <0x06>;
+			};
+
+			sataphy3clk: sataphy1clk at 1f23c000 {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				reg = <0x0 0x1f23c000 0x0 0x1000>;
+				reg-names = "csr-reg";
+				clock-output-names = "sataphy3clk";
+				status = "ok";
+				csr-offset = <0x4>;
+				csr-mask = <0x3a>;
+				enable-offset = <0x0>;
+				enable-mask = <0x06>;
+			};
 		};
 
 		serial0: serial at 1c020000 {
@@ -187,5 +229,35 @@
 			interrupt-parent = <&gic>;
 			interrupts = <0x0 0x4c 0x4>;
 		};
+
+		phy1: phy at 1f21a000 {
+			compatible = "apm,xgene-phy";
+			reg = <0x0 0x1f21a000 0x0 0x100>;
+			#phy-cells = <1>;
+			clocks = <&sataphy1clk 0>;
+			status = "disabled";
+			apm,tx-boost-gain = <30 30 30 30 30 30>;
+			apm,tx-eye-tuning = <2 10 10 2 10 10>;
+		};
+
+		phy2: phy at 1f22a000 {
+			compatible = "apm,xgene-phy";
+			reg = <0x0 0x1f22a000 0x0 0x100>;
+			#phy-cells = <1>;
+			clocks = <&sataphy2clk 0>;
+			status = "ok";
+			apm,tx-boost-gain = <30 30 30 30 30 30>;
+			apm,tx-eye-tuning = <1 10 10 2 10 10>;
+		};
+
+		phy3: phy at 1f23a000 {
+			compatible = "apm,xgene-phy";
+			reg = <0x0 0x1f23a000 0x0 0x100>;
+			#phy-cells = <1>;
+			clocks = <&sataphy3clk 0>;
+			status = "ok";
+			apm,tx-boost-gain = <31 31 31 31 31 31>;
+			apm,tx-eye-tuning = <2 10 10 2 10 10>;
+		};
 	};
 };
Signature · 1 line
1.5.5
author
#2 lore raw

PATCH v18 2/4 Documentation: Add documentation for the APM X-Gene SoC SATA host controller DTS binding

This patch adds documentation for the APM X-Gene SoC SATA host controller DTS binding.

---
 .../devicetree/bindings/ata/apm-xgene.txt          |   76 ++++++++++++++++++++
 1 files changed, 76 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ata/apm-xgene.txt
Documentation/devicetree/bindings/ata/apm-xgene.txt new file +76 -0
new file mode 100644
index 0000000..7bcfbf5
@@ -0,0 +1,76 @@
+* APM X-Gene 6.0 Gb/s SATA host controller nodes
+
+SATA host controller nodes are defined to describe on-chip Serial ATA
+controllers. Each SATA controller (pair of ports) have its own node.
+
+Required properties:
+- compatible		: Shall contain:
+  * "apm,xgene-ahci"
+- reg			: First memory resource shall be the AHCI memory
+			  resource.
+			  Second memory resource shall be the host controller
+			  core memory resource.
+			  Third memory resource shall be the host controller
+			  diagnostic memory resource.
+			  4th memory resource shall be the host controller
+			  AXI memory resource.
+			  5th optional memory resource shall be the host
+			  controller MUX memory resource if required.
+- interrupts		: Interrupt-specifier for SATA host controller IRQ.
+- clocks		: Reference to the clock entry.
+- phys			: A list of phandles + phy-specifiers, one for each
+			  entry in phy-names.
+- phy-names		: Should contain:
+  * "sata-phy" for the SATA 6.0Gbps PHY
+
+Optional properties:
+- status		: Shall be "ok" if enabled or "disabled" if disabled.
+			  Default is "ok".
+
+Example:
+		sataclk: sataclk {
+			compatible = "fixed-clock";
+			#clock-cells = <1>;
+			clock-frequency = <100000000>;
+			clock-output-names = "sataclk";
+		};
+
+		phy2: phy at 1f22a000 {
+			compatible = "apm,xgene-phy";
+			reg = <0x0 0x1f22a000 0x0 0x100>;
+			#phy-cells = <1>;
+		};
+
+		phy3: phy at 1f23a000 {
+			compatible = "apm,xgene-phy";
+			reg = <0x0 0x1f23a000 0x0 0x100>;
+			#phy-cells = <1>;
+		};
+
+		sata2: sata at 1a400000 {
+			compatible = "apm,xgene-ahci";
+			reg = <0x0 0x1a400000 0x0 0x1000>,
+			      <0x0 0x1f220000 0x0 0x1000>,
+			      <0x0 0x1f22d000 0x0 0x1000>,
+			      <0x0 0x1f22e000 0x0 0x1000>,
+			      <0x0 0x1f227000 0x0 0x1000>;
+			interrupts = <0x0 0x87 0x4>;
+			status = "ok";
+			clocks = <&sataclk 0>;
+			phys = <&phy2 0>;
+			phy-names = "sata-phy";
+		};
+
+		sata3: sata at 1a800000 {
+			compatible = "apm,xgene-ahci-pcie";
+			reg = <0x0 0x1a800000 0x0 0x1000>,
+			      <0x0 0x1f230000 0x0 0x1000>,
+			      <0x0 0x1f23d000 0x0 0x1000>,
+			      <0x0 0x1f23e000 0x0 0x1000>,
+			      <0x0 0x1f237000 0x0 0x1000>;
+			interrupts = <0x0 0x88 0x4>;
+			status = "ok";
+			clocks = <&sataclk 0>;
+			phys = <&phy3 0>;
+			phy-names = "sata-phy";
+		};
Signature · 1 line
1.5.5
author
#3 lore raw

PATCH v18 3/4 ata: Add APM X-Gene SoC AHCI SATA host controller driver

This patch adds support for the APM X-Gene SoC AHCI SATA host controller driver. It requires the corresponding APM X-Gene SoC PHY driver. This initial version only supports Gen3 speed.

---
 drivers/ata/Kconfig      |    7 +
 drivers/ata/Makefile     |    1 +
 drivers/ata/ahci_xgene.c |  486 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 494 insertions(+), 0 deletions(-)
 create mode 100644 drivers/ata/ahci_xgene.c
drivers/ata/Kconfig +7 -0
index 93fc2f0..9de4ca5 100644
@@ -115,6 +115,13 @@ config AHCI_SUNXI
 
 	  If unsure, say N.
 
+config AHCI_XGENE
+	tristate "APM X-Gene 6.0Gbps AHCI SATA host controller support"
+	depends on SATA_AHCI_PLATFORM && (ARM64 || COMPILE_TEST)
+	select PHY_XGENE
+	help
+	 This option enables support for APM X-Gene SoC SATA host controller.
+
 config SATA_FSL
 	tristate "Freescale 3.0Gbps SATA support"
 	depends on FSL_SOC
drivers/ata/Makefile +1 -0
index 246050b..72b423b 100644
@@ -12,6 +12,7 @@ obj-$(CONFIG_SATA_DWC)		+= sata_dwc_460ex.o
 obj-$(CONFIG_SATA_HIGHBANK)	+= sata_highbank.o libahci.o
 obj-$(CONFIG_AHCI_IMX)		+= ahci_imx.o
 obj-$(CONFIG_AHCI_SUNXI)	+= ahci_sunxi.o
+obj-$(CONFIG_AHCI_XGENE)	+= ahci_xgene.o
 
 # SFF w/ custom DMA
 obj-$(CONFIG_PDC_ADMA)		+= pdc_adma.o
drivers/ata/ahci_xgene.c new file +486 -0

486 lines not on this page · raw message

Signature · 1 line
1.5.5
author
#4 lore raw

PATCH v18 4/4 arm64: Add APM X-Gene SoC AHCI SATA host controller DTS entries

This patch adds APM X-Gene SoC AHCI SATA host controller DTS entries.

---
 arch/arm64/boot/dts/apm-storm.dtsi |   80 ++++++++++++++++++++++++++++++++++++
 1 files changed, 80 insertions(+), 0 deletions(-)
arch/arm64/boot/dts/apm-storm.dtsi +80 -0
index 6d4f493..93f4b2d 100644
@@ -218,6 +218,45 @@
 				enable-offset = <0x0>;
 				enable-mask = <0x06>;
 			};
+
+			sata01clk: sata01clk at 1f21c000 {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				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 at 1f22c000 {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				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 at 1f23c000 {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				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 at 1c020000 {
@@ -259,5 +298,46 @@
 			apm,tx-boost-gain = <31 31 31 31 31 31>;
 			apm,tx-eye-tuning = <2 10 10 2 10 10>;
 		};
+
+		sata1: sata at 1a000000 {
+			compatible = "apm,xgene-ahci";
+			reg = <0x0 0x1a000000 0x0 0x1000>,
+			      <0x0 0x1f210000 0x0 0x1000>,
+			      <0x0 0x1f21d000 0x0 0x1000>,
+			      <0x0 0x1f21e000 0x0 0x1000>,
+			      <0x0 0x1f217000 0x0 0x1000>;
+			interrupts = <0x0 0x86 0x4>;
+			status = "disabled";
+			clocks = <&sata01clk 0>;
+			phys = <&phy1 0>;
+			phy-names = "sata-phy";
+		};
+
+		sata2: sata at 1a400000 {
+			compatible = "apm,xgene-ahci";
+			reg = <0x0 0x1a400000 0x0 0x1000>,
+			      <0x0 0x1f220000 0x0 0x1000>,
+			      <0x0 0x1f22d000 0x0 0x1000>,
+			      <0x0 0x1f22e000 0x0 0x1000>,
+			      <0x0 0x1f227000 0x0 0x1000>;
+			interrupts = <0x0 0x87 0x4>;
+			status = "ok";
+			clocks = <&sata23clk 0>;
+			phys = <&phy2 0>;
+			phy-names = "sata-phy";
+		};
+
+		sata3: sata at 1a800000 {
+			compatible = "apm,xgene-ahci";
+			reg = <0x0 0x1a800000 0x0 0x1000>,
+			      <0x0 0x1f230000 0x0 0x1000>,
+			      <0x0 0x1f23d000 0x0 0x1000>,
+			      <0x0 0x1f23e000 0x0 0x1000>;
+			interrupts = <0x0 0x88 0x4>;
+			status = "ok";
+			clocks = <&sata45clk 0>;
+			phys = <&phy3 0>;
+			phy-names = "sata-phy";
+		};
 	};
 };
Signature · 1 line
1.5.5
#6 lore raw

PATCH v18 3/4 ata: Add APM X-Gene SoC AHCI SATA host controller driver

In reply to Loc Ho
Quoted 3 lines · Loc Ho

This patch adds support for the APM X-Gene SoC AHCI SATA host controller driver. It requires the corresponding APM X-Gene SoC PHY driver. This initial version only supports Gen3 speed.

This version seems workable, thanks for the quick follow-up.

The comment about Gen3 speed above reminds me that you took some shortcuts to get here and you removed support for some features as well as some bug workarounds in the process. I'm guessing some of them won't be necessary because they are only for prototype hardware or for early boot loader versions that don't yet set up the hardware right, but others actually need to come back.

That is usually a good approach, but I'd also like to make sure we can deal with them nicely when you have to add them back later, and don't have to add ugly extensions to the DT binding to support the old dtb files.

Can you list (also in the changelog) the parts of the driver that you have taken out for now and that you expect to add back at later stage? I think that would be helpful for perspective.

Regarding the support for multiple link speeds, how do you think it will be done? Can you have a driver-side link speed autoconfiguration, or do you have to add DT properties and let the driver know about the attached device?

Arnd

author
#7 lore raw

PATCH v18 3/4 ata: Add APM X-Gene SoC AHCI SATA host controller driver

Hi,

Quoted 13 lines
Quoted 3 lines

This patch adds support for the APM X-Gene SoC AHCI SATA host controller driver. It requires the corresponding APM X-Gene SoC PHY driver. This initial version only supports Gen3 speed.

This version seems workable, thanks for the quick follow-up.

The comment about Gen3 speed above reminds me that you took some shortcuts to get here and you removed support for some features as well as some bug workarounds in the process. I'm guessing some of them won't be necessary because they are only for prototype hardware or for early boot loader versions that don't yet set up the hardware right, but others actually need to come back.

Quoted 9 lines

That is usually a good approach, but I'd also like to make sure we can deal with them nicely when you have to add them back later, and don't have to add ugly extensions to the DT binding to support the old dtb files.

Can you list (also in the changelog) the parts of the driver that you have taken out for now and that you expect to add back at later stage? I think that would be helpful for perspective.

Here is an list of patches that we will be preparing for once the basic driver is completed. Do you want me to re-generate the patch change log with this info?

1. Support for Gen1/Gen2/Gen3
Solution: The simplest solution is to have the PHY framework support setting the desire speed. I had argued with the TI folks but they are reluctant to add this function to the framework. They argued that this is still not generic enough. I will start the discussion again later on. The other possibility (but not sure if this is doable) is to have the PHY init function to be smarter and do the necessary operation assuming the underlying PHY is capable in detecting the link up speed. I will need to check the spec of this.

2. Retry COMRESET (hardreset) if failed first time
Solution: This code is purely in the host driver hardreset function. It isn't for sure that we will need this. Given our current initial board design, this seems to required 1 out of many, many power cycle boot.

3. Cleaning the receiver line when SER_DISPARITY/SER_10B_8B error during COMRESET (hardreset function call)
Solution: It is observed that during COMRESET, the receiver line can report these errors and an clean up is required. For this, the code will be with in the host driver and an call into the PHY layer. The solution is the similar to #1 above.

4. IO flush before servicing completed operations (mostly read operations)
Solution: The read IO operations require an flush from the host controller hardware side. For this, we will need to change the libahci to call an function that is provided by the host driver if non-NULL.

5. PM (power management) support
Solution: Due to an errata in the host controller, stopping the controller requires an slightly modified version in the way it detects the host port completed the requested operation. For this we will have to introduce an AHCI_HFLAG_XXX to detect the completion of FIS_RX stopped by the CI register instead the PORT_CMD register or just delay for 500ms.

Quoted 6 lines

Regarding the support for multiple link speeds, how do you think it will be done?
Can you have a driver-side link speed autoconfiguration, or do you have to add DT properties and let the driver know about the attached device?

See above #1.

-Loc

author
#8 lore raw

PATCH v18 3/4 ata: Add APM X-Gene SoC AHCI SATA host controller driver

Hi Kishon/Felipe,

Quoted 27 lines
Quoted 13 lines
Quoted 3 lines

This patch adds support for the APM X-Gene SoC AHCI SATA host controller driver. It requires the corresponding APM X-Gene SoC PHY driver. This initial version only supports Gen3 speed.

This version seems workable, thanks for the quick follow-up.

The comment about Gen3 speed above reminds me that you took some shortcuts to get here and you removed support for some features as well as some bug workarounds in the process. I'm guessing some of them won't be necessary because they are only for prototype hardware or for early boot loader versions that don't yet set up the hardware right, but others actually need to come back.

Quoted 9 lines

That is usually a good approach, but I'd also like to make sure we can deal with them nicely when you have to add them back later, and don't have to add ugly extensions to the DT binding to support the old dtb files.

Can you list (also in the changelog) the parts of the driver that you have taken out for now and that you expect to add back at later stage? I think that would be helpful for perspective.

Here is an list of patches that we will be preparing for once the basic driver is completed. Do you want me to re-generate the patch change log with this info?

Quoted 9 lines

1. Support for Gen1/Gen2/Gen3
Solution: The simplest solution is to have the PHY framework support setting the desire speed. I had argued with the TI folks but they are reluctant to add this function to the framework. They argued that this is still not generic enough. I will start the discussion again later on. The other possibility (but not sure if this is doable) is to have the PHY init function to be smarter and do the necessary operation assuming the underlying PHY is capable in detecting the link up speed. I will need to check the spec of this.

In order for the X-Gene SATA PHY to support SATA Gen1 and Gen2 speed, we need an method to instruct the underlying PHY driver to switch to an specified setting after link up. For this errata, Suman Tripathi had submitted the patch [1]. It is not possible to hide this within the PHY driver. Each instance of the PHY driver controls two ports. By calling the exit function and then init function, it will affect the other ports - which is not the correct behavior. At this point, we don't see any solution besides introduce an PHY framework function set_rate. Can you let us know if this solution is acceptable?

[1] https://lkml.org/lkml/2014/4/18/491

-Loc

#9 lore raw

PATCH v18 3/4 ata: Add APM X-Gene SoC AHCI SATA host controller driver

Quoted 50 lines · Loc Ho

Hi Kishon/Felipe,

Quoted 27 lines
Quoted 13 lines
Quoted 3 lines

This patch adds support for the APM X-Gene SoC AHCI SATA host controller driver. It requires the corresponding APM X-Gene SoC PHY driver. This initial version only supports Gen3 speed.

This version seems workable, thanks for the quick follow-up.

The comment about Gen3 speed above reminds me that you took some shortcuts to get here and you removed support for some features as well as some bug workarounds in the process. I'm guessing some of them won't be necessary because they are only for prototype hardware or for early boot loader versions that don't yet set up the hardware right, but others actually need to come back.

Quoted 9 lines

That is usually a good approach, but I'd also like to make sure we can deal with them nicely when you have to add them back later, and don't have to add ugly extensions to the DT binding to support the old dtb files.

Can you list (also in the changelog) the parts of the driver that you have taken out for now and that you expect to add back at later stage? I think that would be helpful for perspective.

Here is an list of patches that we will be preparing for once the basic driver is completed. Do you want me to re-generate the patch change log with this info?

Quoted 9 lines

1. Support for Gen1/Gen2/Gen3
Solution: The simplest solution is to have the PHY framework support setting the desire speed. I had argued with the TI folks but they are reluctant to add this function to the framework. They argued that this is still not generic enough. I will start the discussion again later on. The other possibility (but not sure if this is doable) is to have the PHY init function to be smarter and do the necessary operation assuming the underlying PHY is capable in detecting the link up speed. I will need to check the spec of this.

In order for the X-Gene SATA PHY to support SATA Gen1 and Gen2 speed, we need an method to instruct the underlying PHY driver to switch to an specified setting after link up. For this errata, Suman Tripathi had submitted the patch [1]. It is not possible to hide this within the PHY driver. Each instance of the PHY driver controls two ports. By calling the exit function and then init function, it will affect the other ports - which is not the correct behavior. At this point, we don't see any solution besides introduce an PHY framework function set_rate. Can you let us know if this solution is acceptable?

Quoted 1 line

[1] https://lkml.org/lkml/2014/4/18/491

that 'lane' argument isn't acceptable. If one PHY talks to two Links, your PHY driver should register two providers, then the lane argument can be ignored.

Rate can be reused for different things depending on the underlying Bus; in case of USB, it could be for switching among Superspeed10/Superspeed5Highspeed; or 1Gbit/100Mbit switch on Networking interfaces; or Gen1/2/3 selection for PCI controllers and so on. The only thing I can't find a way to abstract is that 'lane' argument which isn't even specific to SATA, it's particular to how you guys wrote your driver. Should you have one PHY per SATA link, you wouldn't have added that 'lane' argument at all.

cheers

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140428/cf8a8180/attachment.sig>
#12 lore raw

v18 cover ata: Add APM X-Gene SoC AHCI SATA host controller support

In reply to Loc Ho
Quoted 4 lines · Loc Ho

This patch adds support for the APM X-Gene SoC AHCI SATA host controller. In order for the host controller to work, the corresponding PHY driver musts also be available. Currently, only Gen3 disk is supported with this initial version.

Applied 1-4 to libata/for-3.15.

Thanks.

Signature · 1 line
tejun