ata: Add APM X-Gene SoC AHCI SATA host controller support
Subsystems (3)
v16 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.
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 | 70 +++ arch/arm64/boot/dts/apm-storm.dtsi | 150 ++++++ drivers/ata/Kconfig | 7 + drivers/ata/Makefile | 1 + drivers/ata/ahci_xgene.c | 490 ++++++++++++++++++++ 5 files changed, 718 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/ata/apm-xgene.txt create mode 100644 drivers/ata/ahci_xgene.c
PATCH v16 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 | 75 ++++++++++++++++++++++++++++++++++++ 1 files changed, 75 insertions(+), 0 deletions(-)
arch/arm64/boot/dts/apm-storm.dtsi +75 -0
@@ -176,6 +176,51 @@ reg-names = "csr-reg"; clock-output-names = "eth8clk"; }; + + sataphy1clk: sataphy1clk at 1f21c000 { + compatible = "apm,xgene-device-clock"; + #clock-cells = <1>; + clocks = <&socplldiv2 0>; + clock-names = "socplldiv2"; + 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>; + clock-names = "socplldiv2"; + 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>; + clock-names = "socplldiv2"; + 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 +232,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
PATCH v16 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 | 70 ++++++++++++++++++++ 1 files changed, 70 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/ata/apm-xgene.txt
Documentation/devicetree/bindings/ata/apm-xgene.txt new file +70 -0
@@ -0,0 +1,70 @@ +* 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-sgmii" if mux'ed with SGMII + * "apm,xgene-ahci-pcie" if mux'ed with PCIe +- reg : First memory resource shall be the AHCI memory + resource. + Second memory resource shall be the host controller + memory resource. +- 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-6g" for the SATA 6.0Gbps PHY + +Optional properties: +- status : Shall be "ok" if enabled or "disabled" if disabled. + Default is "ok". +- interrupt-parent : Interrupt controller. + +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>, + <0x0 0x1f22c000 0x0 0x100>; + #phy-cells = <1>; + }; + + phy3: phy at 1f23a000 { + compatible = "apm,xgene-phy"; + reg = <0x0 0x1f23a000 0x0 0x100>, + <0x0 0x1f23c000 0x0 0x100>; + #phy-cells = <1>; + }; + + sata2: sata at 1a400000 { + compatible = "apm,xgene-ahci-sgmii"; + reg = <0x0 0x1a400000 0x0 0x1000>, + <0x0 0x1f220000 0x0 0x10000>; + interrupt-parent = <&gic>; + interrupts = <0x0 0x87 0x4>; + status = "ok"; + clocks = <&sataclk 0>; + phys = <&phy2 0>; + phy-names = "sata-6g"; + }; + + sata3: sata at 1a800000 { + compatible = "apm,xgene-ahci-pcie"; + reg = <0x0 0x1a800000 0x0 0x1000>, + <0x0 0x1f230000 0x0 0x10000>; + interrupt-parent = <&gic>; + interrupts = <0x0 0x88 0x4>; + status = "ok"; + clocks = <&sataclk 0>; + phys = <&phy3 0>; + phy-names = "sata-6g"; + };
Signature · 1 line
1.5.5
PATCH v16 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 | 490 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 498 insertions(+), 0 deletions(-) create mode 100644 drivers/ata/ahci_xgene.c
drivers/ata/Kconfig +7 -0
@@ -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
@@ -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 +490 -0
490 lines not on this page · raw message
Signature · 1 line
1.5.5
PATCH v16 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 | 75 ++++++++++++++++++++++++++++++++++++ 1 files changed, 75 insertions(+), 0 deletions(-)
arch/arm64/boot/dts/apm-storm.dtsi +75 -0
@@ -221,6 +221,48 @@ enable-offset = <0x0>; enable-mask = <0x06>; }; + + sata01clk: sata01clk at 1f21c000 { + compatible = "apm,xgene-device-clock"; + #clock-cells = <1>; + clocks = <&socplldiv2 0>; + clock-names = "socplldiv2"; + 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>; + clock-names = "socplldiv2"; + 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>; + clock-names = "socplldiv2"; + 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 {
@@ -262,5 +304,38 @@ 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-sgmii"; + reg = <0x0 0x1a000000 0x0 0x1000>, + <0x0 0x1f210000 0x0 0x10000>; + interrupts = <0x0 0x86 0x4>; + status = "disabled"; + clocks = <&sata01clk 0>; + phys = <&phy1 0>; + phy-names = "sata-phy"; + }; + + sata2: sata at 1a400000 { + compatible = "apm,xgene-ahci-sgmii"; + reg = <0x0 0x1a400000 0x0 0x1000>, + <0x0 0x1f220000 0x0 0x10000>; + interrupts = <0x0 0x87 0x4>; + status = "ok"; + clocks = <&sata23clk 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 0x10000>; + interrupts = <0x0 0x88 0x4>; + status = "ok"; + clocks = <&sata45clk 0>; + phys = <&phy3 0>; + phy-names = "sata-phy"; + }; }; };
Signature · 1 line
1.5.5
PATCH v16 3/4 ata: Add APM X-Gene SoC AHCI SATA host controller driver
In reply to Loc HoHi,
Thanks for updating this patch, there are still some issue left though (please see below).
Quoted 49 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.
Signed-off-by Loc Ho <[email protected]>Signed-off-by Tuan Phan <[email protected]>Signed-off-by Suman Tripathi <[email protected]>--- drivers/ata/Kconfig | 7 + drivers/ata/Makefile | 1 + drivers/ata/ahci_xgene.c | 490 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 498 insertions(+), 0 deletions(-) create mode 100644 drivers/ata/ahci_xgene.cdrivers/ata/Kconfig +7
-0@@ -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_SOCdrivers/ata/Makefile +1
-0@@ -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.odrivers/ata/ahci_xgene.c new file +0
-0
[...]
Quoted 5 lines
drivers/ata/ahci_xgene.c new file +5
-0+static struct ata_port_operations xgene_ahci_ops = { + .inherits = &ahci_ops, + .hardreset = xgene_ahci_hardreset, + .read_id = xgene_ahci_read_id, +};
[...]
Quoted 63 lines
drivers/ata/ahci_xgene.c new file +63
-0+static int xgene_ahci_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct ahci_host_priv *hpriv; +struct xgene_ahci_context *ctx; +struct resource *res; +int rc; + + hpriv =ahci_platform_get_resources (pdev); +if (IS_ERR (hpriv)) +return PTR_ERR (hpriv); + + ctx =devm_kzalloc (dev,sizeof (*ctx),GFP_KERNEL ); +if (!ctx) { +dev_err (dev,"can't allocate host context\n" ); +return -ENOMEM ; + } + hpriv->plat_data = ctx; + ctx->hpriv = hpriv; + ctx->dev = dev; + + res =platform_get_resource (pdev,IORESOURCE_MEM ,1 ); +if (!res) { +dev_err (dev,"no csr space\n" ); +return -EINVAL ; + } + +/* +* Can't use devm_ioremap_resource due to overlapping region. +* 0xYYYY.0000 - host core +* 0xYYYY.7000 - Mux (if applicable) +* 0xYYYY.A000 - PHY indirect access +* 0xYYYY.C000 - Clock +* 0xYYYY.D000 - RAM shutdown removal +* As we map the entire region as one, it overlaps with the PHY driver. +*/ + ctx->csr_base =devm_ioremap (dev, res->start,resource_size (res)); +if (!ctx->csr_base) { +dev_err (dev,"can't map %pR\n" , res); +return -ENOMEM ; + } + +dev_dbg (dev,"VAddr 0x%p Mmio VAddr 0x%p\n" , ctx->csr_base, + hpriv->mmio); + +/* Select ATA */ +if (of_device_is_compatible (pdev->dev.of_node, +XGENE_AHCI_SGMII_DTS )) { +if (xgene_ahci_mux_select (ctx)) { +dev_err (dev,"SATA mux selection failed\n" ); +return -ENODEV ; + } + } + +/* Due to errata, HW requires full toggle transition */ + rc =ahci_platform_enable_clks (hpriv); +if (rc) + goto disable_resources; +ahci_platform_disable_clks (hpriv); + + rc =ahci_platform_enable_resources (hpriv); +if (rc) + goto disable_resources;
drivers/ata/ahci_xgene.c line 451
Quoted 10 lines
drivers/ata/ahci_xgene.c new file +10
-0+ +/* Configure the host controller */ +xgene_ahci_hw_init (hpriv); + +/* Setup DMA mask - 32 for 32-bit system and 64 for 64-bit system */ + rc =dma_set_mask_and_coherent (dev,DMA_BIT_MASK (8 *sizeof (void *))); +if (rc) { +dev_err (dev,"Unable to set dma mask\n" ); +phy_exit (hpriv->phy); + goto disable_resources;
Since the code has been switched to use generic ahci_platform ->phy support phy_exit() is now handled by ahci_platform_disable_resources() and the direct call should be removed.
drivers/ata/ahci_xgene.c line 456
Quoted 5 lines
drivers/ata/ahci_xgene.c new file +5
-0+ } + + rc =ahci_platform_init_host (pdev, hpriv, &xgene_ahci_port_info,0 ,0 ); +if (rc) { +phy_exit (hpriv->phy);
ditto
Quoted 11 lines
drivers/ata/ahci_xgene.c new file +11
-0+ goto disable_resources; + } + +dev_dbg (dev,"X-Gene SATA host controller initialized\n" ); +return 0 ; + +disable_resources: +ahci_platform_disable_resources (hpriv); +return rc; +} +
drivers/ata/ahci_xgene.c line 477
Quoted 10 lines
drivers/ata/ahci_xgene.c new file +10
-0+static const struct of_device_id xgene_ahci_of_match[] = { + {.compatible =XGENE_AHCI_SGMII_DTS ,}, + {.compatible =XGENE_AHCI_PCIE_DTS ,}, + {}, +}; +MODULE_DEVICE_TABLE (of, xgene_ahci_of_match); + +static struct platform_driver xgene_ahci_driver = { + .probe = xgene_ahci_probe, + .remove = ata_platform_remove_one,
It is good to use ata_platform_remove_one() here but some code still needs to callback ahci_platform_disable_resources() before the driver is removed completely. The way other drivers do it is to define custom ->host_stop method and add it to port ops (xgene_ahci_ops in this case). For X-Gene AHCI driver ->host_stop function can be quite simple and look like this:
static void xgene_ahci_host_stop(struct ata_host *host)
{
struct ahci_host_priv *hpriv = host->private_data;
ahci_platform_disable_resources(hpriv);
}drivers/ata/ahci_xgene.c line 482
Quoted 5 lines
drivers/ata/ahci_xgene.c new file +5
-0+ .driver = { + .name = "xgene-ahci", + .owner = THIS_MODULE, + .of_match_table = xgene_ahci_of_match, + },
Power Management support is missing for this driver. If your platform doesn't support Suspend to RAM yet it would be good to add a comment about this explaining the lack of PM. Otherwise it should be fixed (you can take a look at ahci_imx.c and ahci_sunxi.c for examples).
Otherwise the driver looks good to me and (FWIW) you can add:
Once the aforementioned issues are fixed.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
PATCH v16 3/4 ata: Add APM X-Gene SoC AHCI SATA host controller driver
Hi,
Quoted 12 lines
drivers/ata/ahci_xgene.c line 477
Quoted 5 lines
drivers/ata/ahci_xgene.c new file +5
-0+MODULE_DEVICE_TABLE (of, xgene_ahci_of_match); + +static struct platform_driver xgene_ahci_driver = { + .probe = xgene_ahci_probe, + .remove = ata_platform_remove_one,It is good to use
ata_platform_remove_one()here but some code still needs to callbackahci_platform_disable_resources()before the driver is removed completely. The way other drivers do it is to define custom ->host_stop method and add it to port ops (xgene_ahci_opsin this case). For X-Gene AHCI driver ->host_stop function can be quite simple and look like this:
Quoted 6 lines
static void
xgene_ahci_host_stop(struct ata_host *host){ struct ahci_host_priv *hpriv = host->private_data; ahci_platform_disable_resources(hpriv); }
Will do.
Quoted 11 lines
drivers/ata/ahci_xgene.c line 482
Quoted 6 lines
drivers/ata/ahci_xgene.c new file +5
-0+ .driver = { + .name = "xgene-ahci", + .owner = THIS_MODULE, + .of_match_table = xgene_ahci_of_match, + },Power Management support is missing for this driver. If your platform doesn't support Suspend to RAM yet it would be good to add a comment about this explaining the lack of PM. Otherwise it should be fixed (you can take a look at
ahci_imx.candahci_sunxi.cfor examples).
In order to support PM, I will need an other errata fix in libachi.c. For now PM, will NOT be support.
Quoted 6 lines
Otherwise the driver looks good to me and (FWIW) you can add:
Reviewed-by Bartlomiej Zolnierkiewicz <[email protected]>Once the aforementioned issues are fixed.
I will post another version. If you ack'ed that version, Tejun can add you to the Reviewed-by list.
-Loc