Re: [PATCH v2 2/4] ARM: sunxi: Add ahci-sunxi driver for the Allwinner SUNXi SoCs sata
From: Arnd Bergmann <hidden>
Date: 2014-01-04 21:39:50
Also in:
linux-arm-kernel, linux-ide
On Saturday 04 January 2014 10:14:36 Hans de Goede wrote:
quoted hunk
diff --git a/Documentation/devicetree/bindings/ata/ahci-sunxi.txt b/Documentation/devicetree/bindings/ata/ahci-sunxi.txt new file mode 100644 index 0000000..0792fa5 --- /dev/null +++ b/Documentation/devicetree/bindings/ata/ahci-sunxi.txt@@ -0,0 +1,24 @@ +Allwinner SUNXI AHCI SATA Controller + +SATA nodes are defined to describe on-chip Serial ATA controllers. +Each SATA controller should have its own node. + +Required properties: +- compatible : compatible list, contains "allwinner,sun4i-a10-ahci" +- reg : <registers mapping> +- interrupts : <interrupt mapping for AHCI IRQ> +- clocks : clocks for ACHI +- clock-names : clock names for AHCI
The binding needs to specify the required names for the clocks.
+Optional properties:
+- pwr-supply : regulator to control the power supply GPIO
+
+Example:
+ ahci@01c18000 {
+ compatible = "allwinner,sun4i-a10-ahci";
+ reg = <0x01c18000 0x1000>;
+ interrupts = <0 56 1>;
+ clocks = <&ahb_gates 25>, <&pll6 0>;
+ clock-names = "ahb_sata", "pll6_sata";"pll6_sata" doesn't sound particularly generic. The name should reflect what the clock is used for, not what drives it. Also, please send the binding as a separate patch with Cc to the devicetree-discuss mailing list.
+
+static int sunxi_ahci_phy_init(struct device *dev, void __iomem *reg_base)
+{
+ u32 reg_val;
+ int timeout;
+
+ /* This magic is from the original code */
+ writel(0, reg_base + AHCI_RWCR);
+ mdelay(5);This function should probably be in a separate phy driver. I would very much hope that we can minimize the required code in an AHCI driver and move code from this new file into the ahci-platform driver. The clock, regulator and phy setup can all be optional properties of the generic driver, and then there shouldn't be much left that is sunxi specific.
+static int sunxi_ahci_susp(struct device *dev)
+{
+ struct ata_host *host = dev_get_drvdata(dev);
+ struct ahci_host_priv *hpriv = host->private_data;
+ struct sunxi_ahci *ahci = hpriv->plat_data;
+ int ret;
+
+ /*
+ * AHCI spec rev1.1 section 8.3.3:
+ * Software must disable interrupts prior to requesting a
+ * transition of the HBA to D3 state.
+ */
+ sunxi_clrbits(hpriv->mmio + HOST_CTL, HOST_IRQ_EN);
+
+ ret = ata_host_suspend(host, PMSG_SUSPEND);
+ if (ret)
+ return ret;
+
+ sunxi_ahci_disable_clks(ahci);
+
+ return 0;
+}The only thing in here that seems sunxi-specific is the irq disabling part. Can't you do this instead by calling disable_irq() and make the function completely generic? Arnd