* [PATCH v7 14/15] ARM: sun4i: dt: Add ahci / sata support
From: Hans de Goede @ 2014-02-22 15:53 UTC (permalink / raw)
To: Tejun Heo, Maxime Ripard
Cc: Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede
In-Reply-To: <1393084424-31099-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
From: Oliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
This patch adds sunxi sata support to A10 boards that have such a connector.
Some boards also feature a regulator via a GPIO and support for this is also
added.
Signed-off-by: Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
arch/arm/boot/dts/sun4i-a10-a1000.dts | 4 ++++
arch/arm/boot/dts/sun4i-a10-cubieboard.dts | 6 +++++
arch/arm/boot/dts/sun4i-a10.dtsi | 8 +++++++
arch/arm/boot/dts/sunxi-ahci-reg.dtsi | 36 ++++++++++++++++++++++++++++++
4 files changed, 54 insertions(+)
create mode 100644 arch/arm/boot/dts/sunxi-ahci-reg.dtsi
diff --git a/arch/arm/boot/dts/sun4i-a10-a1000.dts b/arch/arm/boot/dts/sun4i-a10-a1000.dts
index cbd2e13..d6ec839 100644
--- a/arch/arm/boot/dts/sun4i-a10-a1000.dts
+++ b/arch/arm/boot/dts/sun4i-a10-a1000.dts
@@ -35,6 +35,10 @@
};
};
+ ahci: sata@01c18000 {
+ status = "okay";
+ };
+
pinctrl@01c20800 {
emac_power_pin_a1000: emac_power_pin@0 {
allwinner,pins = "PH15";
diff --git a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
index b139ee6..6df237d8 100644
--- a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
+++ b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
@@ -12,6 +12,7 @@
/dts-v1/;
/include/ "sun4i-a10.dtsi"
+/include/ "sunxi-ahci-reg.dtsi"
/ {
model = "Cubietech Cubieboard";
@@ -33,6 +34,11 @@
};
};
+ ahci: sata@01c18000 {
+ target-supply = <®_ahci_5v>;
+ status = "okay";
+ };
+
pinctrl@01c20800 {
led_pins_cubieboard: led_pins@0 {
allwinner,pins = "PH20", "PH21";
diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 336dbec..454077a 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -338,6 +338,14 @@
#size-cells = <0>;
};
+ ahci: sata@01c18000 {
+ compatible = "allwinner,sun4i-a10-ahci";
+ reg = <0x01c18000 0x1000>;
+ interrupts = <56>;
+ clocks = <&pll6 0>, <&ahb_gates 25>;
+ status = "disabled";
+ };
+
intc: interrupt-controller@01c20400 {
compatible = "allwinner,sun4i-ic";
reg = <0x01c20400 0x400>;
diff --git a/arch/arm/boot/dts/sunxi-ahci-reg.dtsi b/arch/arm/boot/dts/sunxi-ahci-reg.dtsi
new file mode 100644
index 0000000..7072af1
--- /dev/null
+++ b/arch/arm/boot/dts/sunxi-ahci-reg.dtsi
@@ -0,0 +1,36 @@
+/*
+ * sunxi boards sata target power supply common code
+ *
+ * Copyright 2014 - Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+/ {
+ soc@01c00000 {
+ pio: pinctrl@01c20800 {
+ ahci_pwr_pin_a: ahci_pwr_pin@0 {
+ allwinner,pins = "PB8";
+ allwinner,function = "gpio_out";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
+ };
+ };
+
+ reg_ahci_5v: ahci-5v {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <&ahci_pwr_pin_a>;
+ regulator-name = "ahci-5v";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ enable-active-high;
+ gpio = <&pio 1 8 0>;
+ };
+};
--
1.9.0
^ permalink raw reply related
* [PATCH v7 13/15] ARM: sun4i: dt: Remove grouping + simple-bus compatible for regulators
From: Hans de Goede @ 2014-02-22 15:53 UTC (permalink / raw)
To: Tejun Heo, Maxime Ripard
Cc: Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede
In-Reply-To: <1393084424-31099-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
According to Documentation/devicetree/bindings/regulator/regulator.txt
regulator nodes should not be placed under 'simple-bus'.
Mark Rutland also explains about it at:
http://www.spinics.net/lists/linux-usb/msg101497.html
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
arch/arm/boot/dts/sun4i-a10-a1000.dts | 22 +++++++++-------------
arch/arm/boot/dts/sun4i-a10-hackberry.dts | 18 +++++++-----------
2 files changed, 16 insertions(+), 24 deletions(-)
diff --git a/arch/arm/boot/dts/sun4i-a10-a1000.dts b/arch/arm/boot/dts/sun4i-a10-a1000.dts
index d4b081d..cbd2e13 100644
--- a/arch/arm/boot/dts/sun4i-a10-a1000.dts
+++ b/arch/arm/boot/dts/sun4i-a10-a1000.dts
@@ -80,18 +80,14 @@
};
};
- regulators {
- compatible = "simple-bus";
-
- reg_emac_3v3: emac-3v3 {
- compatible = "regulator-fixed";
- pinctrl-names = "default";
- pinctrl-0 = <&emac_power_pin_a1000>;
- regulator-name = "emac-3v3";
- regulator-min-microvolt = <3300000>;
- regulator-max-microvolt = <3300000>;
- enable-active-high;
- gpio = <&pio 7 15 0>;
- };
+ reg_emac_3v3: emac-3v3 {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <&emac_power_pin_a1000>;
+ regulator-name = "emac-3v3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ enable-active-high;
+ gpio = <&pio 7 15 0>;
};
};
diff --git a/arch/arm/boot/dts/sun4i-a10-hackberry.dts b/arch/arm/boot/dts/sun4i-a10-hackberry.dts
index 3a1595f..6692d336 100644
--- a/arch/arm/boot/dts/sun4i-a10-hackberry.dts
+++ b/arch/arm/boot/dts/sun4i-a10-hackberry.dts
@@ -54,16 +54,12 @@
};
};
- regulators {
- compatible = "simple-bus";
-
- reg_emac_3v3: emac-3v3 {
- compatible = "regulator-fixed";
- regulator-name = "emac-3v3";
- regulator-min-microvolt = <3300000>;
- regulator-max-microvolt = <3300000>;
- enable-active-high;
- gpio = <&pio 7 19 0>;
- };
+ reg_emac_3v3: emac-3v3 {
+ compatible = "regulator-fixed";
+ regulator-name = "emac-3v3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ enable-active-high;
+ gpio = <&pio 7 19 0>;
};
};
--
1.9.0
^ permalink raw reply related
* [PATCH v7 12/15] ata: ahci_platform: runtime resume the device before use
From: Hans de Goede @ 2014-02-22 15:53 UTC (permalink / raw)
To: Tejun Heo, Maxime Ripard
Cc: Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Balaji T K, Hans de Goede
In-Reply-To: <1393084424-31099-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
From: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
On OMAP platforms the device needs to be runtime resumed before
it can be accessed. The OMAP HWMOD framework takes care of
enabling the module and its resources based on the
device's runtime PM state.
In this patch we runtime resume during .probe() and runtime suspend
after .remove().
We also update the runtime PM state during .resume().
CC: Balaji T K <balajitk-l0cyMroinI0@public.gmane.org>
Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/ata/ahci.h | 1 +
drivers/ata/ahci_platform.c | 15 +++++++++++++++
2 files changed, 16 insertions(+)
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 3ab7ac9..51af275 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -324,6 +324,7 @@ struct ahci_host_priv {
u32 em_loc; /* enclosure management location */
u32 em_buf_sz; /* EM buffer size in byte */
u32 em_msg_type; /* EM message type */
+ bool got_runtime_pm; /* Did we do pm_runtime_get? */
struct clk *clks[AHCI_MAX_CLKS]; /* Optional */
struct regulator *target_pwr; /* Optional */
struct phy *phy; /* If platform uses phy */
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 99d38c1..75698a4 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -24,6 +24,7 @@
#include <linux/libata.h>
#include <linux/ahci_platform.h>
#include <linux/phy/phy.h>
+#include <linux/pm_runtime.h>
#include "ahci.h"
static void ahci_host_stop(struct ata_host *host);
@@ -229,6 +230,11 @@ static void ahci_platform_put_resources(struct device *dev, void *res)
struct ahci_host_priv *hpriv = res;
int c;
+ if (hpriv->got_runtime_pm) {
+ pm_runtime_put_sync(dev);
+ pm_runtime_disable(dev);
+ }
+
for (c = 0; c < AHCI_MAX_CLKS && hpriv->clks[c]; c++)
clk_put(hpriv->clks[c]);
}
@@ -326,6 +332,10 @@ struct ahci_host_priv *ahci_platform_get_resources(
}
}
+ pm_runtime_enable(dev);
+ pm_runtime_get_sync(dev);
+ hpriv->got_runtime_pm = true;
+
devres_remove_group(dev, NULL);
return hpriv;
@@ -635,6 +645,11 @@ int ahci_platform_resume(struct device *dev)
if (rc)
goto disable_resources;
+ /* We resumed so update PM runtime state */
+ pm_runtime_disable(dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+
return 0;
disable_resources:
--
1.9.0
^ permalink raw reply related
* [PATCH v7 11/15] ata: ahci_platform: Manage SATA PHY
From: Hans de Goede @ 2014-02-22 15:53 UTC (permalink / raw)
To: Tejun Heo, Maxime Ripard
Cc: Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Balaji T K, Hans de Goede
In-Reply-To: <1393084424-31099-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
From: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
Some platforms have a PHY hooked up to the
SATA controller. The PHY needs to be initialized
and powered up for SATA to work. We do that
using the PHY framework.
CC: Balaji T K <balajitk-l0cyMroinI0@public.gmane.org>
Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/ata/ahci.h | 2 ++
drivers/ata/ahci_platform.c | 47 +++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index bf8100c..3ab7ac9 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -37,6 +37,7 @@
#include <linux/clk.h>
#include <linux/libata.h>
+#include <linux/phy/phy.h>
#include <linux/regulator/consumer.h>
/* Enclosure Management Control */
@@ -325,6 +326,7 @@ struct ahci_host_priv {
u32 em_msg_type; /* EM message type */
struct clk *clks[AHCI_MAX_CLKS]; /* Optional */
struct regulator *target_pwr; /* Optional */
+ struct phy *phy; /* If platform uses phy */
void *plat_data; /* Other platform data */
/*
* Optional ahci_start_engine override, if not set this gets set to the
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index d7e55ba..99d38c1 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -23,6 +23,7 @@
#include <linux/platform_device.h>
#include <linux/libata.h>
#include <linux/ahci_platform.h>
+#include <linux/phy/phy.h>
#include "ahci.h"
static void ahci_host_stop(struct ata_host *host);
@@ -147,6 +148,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_disable_clks);
* the following order:
* 1) Regulator
* 2) Clocks (through ahci_platform_enable_clks)
+ * 3) Phy
*
* If resource enabling fails at any point the previous enabled
* resources are disabled in reverse order.
@@ -171,8 +173,23 @@ int ahci_platform_enable_resources(struct ahci_host_priv *hpriv)
if (rc)
goto disable_regulator;
+ if (hpriv->phy) {
+ rc = phy_init(hpriv->phy);
+ if (rc)
+ goto disable_clks;
+
+ rc = phy_power_on(hpriv->phy);
+ if (rc) {
+ phy_exit(hpriv->phy);
+ goto disable_clks;
+ }
+ }
+
return 0;
+disable_clks:
+ ahci_platform_disable_clks(hpriv);
+
disable_regulator:
if (hpriv->target_pwr)
regulator_disable(hpriv->target_pwr);
@@ -186,14 +203,20 @@ EXPORT_SYMBOL_GPL(ahci_platform_enable_resources);
*
* This function disables all ahci_platform managed resources in
* the following order:
- * 1) Clocks (through ahci_platform_disable_clks)
- * 2) Regulator
+ * 1) Phy
+ * 2) Clocks (through ahci_platform_disable_clks)
+ * 3) Regulator
*
* LOCKING:
* None.
*/
void ahci_platform_disable_resources(struct ahci_host_priv *hpriv)
{
+ if (hpriv->phy) {
+ phy_power_off(hpriv->phy);
+ phy_exit(hpriv->phy);
+ }
+
ahci_platform_disable_clks(hpriv);
if (hpriv->target_pwr)
@@ -222,6 +245,7 @@ static void ahci_platform_put_resources(struct device *dev, void *res)
* 2) regulator for controlling the targets power (optional)
* 3) 0 - AHCI_MAX_CLKS clocks, as specified in the devs devicetree node,
* or for non devicetree enabled platforms a single clock
+ * 4) phy (optional)
*
* LOCKING:
* None.
@@ -283,6 +307,25 @@ struct ahci_host_priv *ahci_platform_get_resources(
hpriv->clks[i] = clk;
}
+ hpriv->phy = devm_phy_get(dev, "sata-phy");
+ if (IS_ERR(hpriv->phy)) {
+ rc = PTR_ERR(hpriv->phy);
+ switch (rc) {
+ case -ENODEV:
+ case -ENOSYS:
+ /* continue normally */
+ hpriv->phy = NULL;
+ break;
+
+ case -EPROBE_DEFER:
+ goto err_out;
+
+ default:
+ dev_err(dev, "couldn't get sata-phy\n");
+ goto err_out;
+ }
+ }
+
devres_remove_group(dev, NULL);
return hpriv;
--
1.9.0
^ permalink raw reply related
* [PATCH v7 10/15] ata: ahci_platform: Update DT compatible list
From: Hans de Goede @ 2014-02-22 15:53 UTC (permalink / raw)
To: Tejun Heo, Maxime Ripard
Cc: Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede
In-Reply-To: <1393084424-31099-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
From: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
The ahci_platform driver supports "snps,dwc-ahci".
Add this to the DT binding information.
Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
Documentation/devicetree/bindings/ata/ahci-platform.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index d86e854..48b285f 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -6,8 +6,8 @@ Each SATA controller should have its own node.
Required properties:
- compatible : compatible list, one of "snps,spear-ahci",
"snps,exynos5440-ahci", "ibm,476gtr-ahci",
- "allwinner,sun4i-a10-ahci", "fsl,imx53-ahci" or
- "fsl,imx6q-ahci"
+ "allwinner,sun4i-a10-ahci", "fsl,imx53-ahci"
+ "fsl,imx6q-ahci" or "snps,dwc-ahci"
- interrupts : <interrupt mapping for SATA IRQ>
- reg : <registers mapping>
--
1.9.0
^ permalink raw reply related
* [PATCH v7 09/15] ata: ahci_platform: Add DT compatible for Synopsis DWC AHCI controller
From: Hans de Goede @ 2014-02-22 15:53 UTC (permalink / raw)
To: Tejun Heo, Maxime Ripard
Cc: Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede
In-Reply-To: <1393084424-31099-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
From: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
Add compatible string "snps,dwc-ahci", which should be used
for Synopsis Designware SATA cores. e.g. on TI OMAP5 and DRA7 platforms.
Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/ata/ahci_platform.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index bdadec1..d7e55ba 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -609,6 +609,7 @@ static const struct of_device_id ahci_of_match[] = {
{ .compatible = "snps,spear-ahci", },
{ .compatible = "snps,exynos5440-ahci", },
{ .compatible = "ibm,476gtr-ahci", },
+ { .compatible = "snps,dwc-ahci", },
{},
};
MODULE_DEVICE_TABLE(of, ahci_of_match);
--
1.9.0
^ permalink raw reply related
* [PATCH v7 08/15] ahci-imx: Port to library-ised ahci_platform
From: Hans de Goede @ 2014-02-22 15:53 UTC (permalink / raw)
To: Tejun Heo, Maxime Ripard
Cc: Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede
In-Reply-To: <1393084424-31099-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
This avoids the ugliness of creating a nested platform device from probe.
While moving it around anyways, move the mk6q phy init code from probe
to imx_sata_enable, as the phy needs to be re-initialized on resume too,
otherwise the drive won't be recognized after resume.
Tested on a wandboard i.mx6 quad.
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
.../devicetree/bindings/ata/ahci-platform.txt | 9 +-
drivers/ata/ahci_imx.c | 331 ++++++++-------------
2 files changed, 134 insertions(+), 206 deletions(-)
diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index 499bfed..d86e854 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -5,8 +5,9 @@ Each SATA controller should have its own node.
Required properties:
- compatible : compatible list, one of "snps,spear-ahci",
- "snps,exynos5440-ahci", "ibm,476gtr-ahci", or
- "allwinner,sun4i-a10-ahci"
+ "snps,exynos5440-ahci", "ibm,476gtr-ahci",
+ "allwinner,sun4i-a10-ahci", "fsl,imx53-ahci" or
+ "fsl,imx6q-ahci"
- interrupts : <interrupt mapping for SATA IRQ>
- reg : <registers mapping>
@@ -15,6 +16,10 @@ Optional properties:
- clocks : a list of phandle + clock specifier pairs
- target-supply : regulator for SATA target power
+"fsl,imx53-ahci", "fsl,imx6q-ahci" required properties:
+- clocks : must contain the sata, sata_ref and ahb clocks
+- clock-names : must contain "ahb" for the ahb clock
+
Examples:
sata@ffe08000 {
compatible = "snps,spear-ahci";
diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index dd4d6f7..3cb5d69 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -42,13 +42,7 @@ enum ahci_imx_type {
struct imx_ahci_priv {
struct platform_device *ahci_pdev;
enum ahci_imx_type type;
-
- /* i.MX53 clock */
- struct clk *sata_gate_clk;
- /* Common clock */
- struct clk *sata_ref_clk;
struct clk *ahb_clk;
-
struct regmap *gpr;
bool no_device;
bool first_time;
@@ -58,28 +52,52 @@ static int ahci_imx_hotplug;
module_param_named(hotplug, ahci_imx_hotplug, int, 0644);
MODULE_PARM_DESC(hotplug, "AHCI IMX hot-plug support (0=Don't support, 1=support)");
-static int imx_sata_clock_enable(struct device *dev)
+static void ahci_imx_host_stop(struct ata_host *host);
+
+static int imx_sata_enable(struct ahci_host_priv *hpriv)
{
- struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
+ struct imx_ahci_priv *imxpriv = hpriv->plat_data;
int ret;
- if (imxpriv->type == AHCI_IMX53) {
- ret = clk_prepare_enable(imxpriv->sata_gate_clk);
- if (ret < 0) {
- dev_err(dev, "prepare-enable sata_gate clock err:%d\n",
- ret);
+ if (imxpriv->no_device)
+ return 0;
+
+ if (hpriv->target_pwr) {
+ ret = regulator_enable(hpriv->target_pwr);
+ if (ret)
return ret;
- }
}
- ret = clk_prepare_enable(imxpriv->sata_ref_clk);
- if (ret < 0) {
- dev_err(dev, "prepare-enable sata_ref clock err:%d\n",
- ret);
- goto clk_err;
- }
+ ret = ahci_platform_enable_clks(hpriv);
+ if (ret < 0)
+ goto disable_regulator;
if (imxpriv->type == AHCI_IMX6Q) {
+ /*
+ * set PHY Paremeters, two steps to configure the GPR13,
+ * one write for rest of parameters, mask of first write
+ * is 0x07ffffff, and the other one write for setting
+ * the mpll_clk_en.
+ */
+ regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
+ IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK |
+ IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK |
+ IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK |
+ IMX6Q_GPR13_SATA_SPD_MODE_MASK |
+ IMX6Q_GPR13_SATA_MPLL_SS_EN |
+ IMX6Q_GPR13_SATA_TX_ATTEN_MASK |
+ IMX6Q_GPR13_SATA_TX_BOOST_MASK |
+ IMX6Q_GPR13_SATA_TX_LVL_MASK |
+ IMX6Q_GPR13_SATA_MPLL_CLK_EN |
+ IMX6Q_GPR13_SATA_TX_EDGE_RATE,
+ IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB |
+ IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M |
+ IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F |
+ IMX6Q_GPR13_SATA_SPD_MODE_3P0G |
+ IMX6Q_GPR13_SATA_MPLL_SS_EN |
+ IMX6Q_GPR13_SATA_TX_ATTEN_9_16 |
+ IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB |
+ IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
IMX6Q_GPR13_SATA_MPLL_CLK_EN,
IMX6Q_GPR13_SATA_MPLL_CLK_EN);
@@ -89,15 +107,19 @@ static int imx_sata_clock_enable(struct device *dev)
return 0;
-clk_err:
- if (imxpriv->type == AHCI_IMX53)
- clk_disable_unprepare(imxpriv->sata_gate_clk);
+disable_regulator:
+ if (hpriv->target_pwr)
+ regulator_disable(hpriv->target_pwr);
+
return ret;
}
-static void imx_sata_clock_disable(struct device *dev)
+static void imx_sata_disable(struct ahci_host_priv *hpriv)
{
- struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
+ struct imx_ahci_priv *imxpriv = hpriv->plat_data;
+
+ if (imxpriv->no_device)
+ return;
if (imxpriv->type == AHCI_IMX6Q) {
regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
@@ -105,10 +127,10 @@ static void imx_sata_clock_disable(struct device *dev)
!IMX6Q_GPR13_SATA_MPLL_CLK_EN);
}
- clk_disable_unprepare(imxpriv->sata_ref_clk);
+ ahci_platform_disable_clks(hpriv);
- if (imxpriv->type == AHCI_IMX53)
- clk_disable_unprepare(imxpriv->sata_gate_clk);
+ if (hpriv->target_pwr)
+ regulator_disable(hpriv->target_pwr);
}
static void ahci_imx_error_handler(struct ata_port *ap)
@@ -118,7 +140,7 @@ static void ahci_imx_error_handler(struct ata_port *ap)
struct ata_host *host = dev_get_drvdata(ap->dev);
struct ahci_host_priv *hpriv = host->private_data;
void __iomem *mmio = hpriv->mmio;
- struct imx_ahci_priv *imxpriv = dev_get_drvdata(ap->dev->parent);
+ struct imx_ahci_priv *imxpriv = hpriv->plat_data;
ahci_error_handler(ap);
@@ -136,7 +158,7 @@ static void ahci_imx_error_handler(struct ata_port *ap)
*/
reg_val = readl(mmio + PORT_PHY_CTL);
writel(reg_val | PORT_PHY_CTL_PDDQ_LOC, mmio + PORT_PHY_CTL);
- imx_sata_clock_disable(ap->dev);
+ imx_sata_disable(hpriv);
imxpriv->no_device = true;
}
@@ -144,7 +166,9 @@ static int ahci_imx_softreset(struct ata_link *link, unsigned int *class,
unsigned long deadline)
{
struct ata_port *ap = link->ap;
- struct imx_ahci_priv *imxpriv = dev_get_drvdata(ap->dev->parent);
+ struct ata_host *host = dev_get_drvdata(ap->dev);
+ struct ahci_host_priv *hpriv = host->private_data;
+ struct imx_ahci_priv *imxpriv = hpriv->plat_data;
int ret = -EIO;
if (imxpriv->type == AHCI_IMX53)
@@ -156,7 +180,8 @@ static int ahci_imx_softreset(struct ata_link *link, unsigned int *class,
}
static struct ata_port_operations ahci_imx_ops = {
- .inherits = &ahci_platform_ops,
+ .inherits = &ahci_ops,
+ .host_stop = ahci_imx_host_stop,
.error_handler = ahci_imx_error_handler,
.softreset = ahci_imx_softreset,
};
@@ -168,79 +193,6 @@ static const struct ata_port_info ahci_imx_port_info = {
.port_ops = &ahci_imx_ops,
};
-static int imx_sata_init(struct device *dev, void __iomem *mmio)
-{
- int ret = 0;
- unsigned int reg_val;
- struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
-
- ret = imx_sata_clock_enable(dev);
- if (ret < 0)
- return ret;
-
- /*
- * Configure the HWINIT bits of the HOST_CAP and HOST_PORTS_IMPL,
- * and IP vendor specific register HOST_TIMER1MS.
- * Configure CAP_SSS (support stagered spin up).
- * Implement the port0.
- * Get the ahb clock rate, and configure the TIMER1MS register.
- */
- reg_val = readl(mmio + HOST_CAP);
- if (!(reg_val & HOST_CAP_SSS)) {
- reg_val |= HOST_CAP_SSS;
- writel(reg_val, mmio + HOST_CAP);
- }
- reg_val = readl(mmio + HOST_PORTS_IMPL);
- if (!(reg_val & 0x1)) {
- reg_val |= 0x1;
- writel(reg_val, mmio + HOST_PORTS_IMPL);
- }
-
- reg_val = clk_get_rate(imxpriv->ahb_clk) / 1000;
- writel(reg_val, mmio + HOST_TIMER1MS);
-
- return 0;
-}
-
-static void imx_sata_exit(struct device *dev)
-{
- imx_sata_clock_disable(dev);
-}
-
-static int imx_ahci_suspend(struct device *dev)
-{
- struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
-
- /*
- * If no_device is set, The CLKs had been gated off in the
- * initialization so don't do it again here.
- */
- if (!imxpriv->no_device)
- imx_sata_clock_disable(dev);
-
- return 0;
-}
-
-static int imx_ahci_resume(struct device *dev)
-{
- struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
- int ret = 0;
-
- if (!imxpriv->no_device)
- ret = imx_sata_clock_enable(dev);
-
- return ret;
-}
-
-static struct ahci_platform_data imx_sata_pdata = {
- .init = imx_sata_init,
- .exit = imx_sata_exit,
- .ata_port_info = &ahci_imx_port_info,
- .suspend = imx_ahci_suspend,
- .resume = imx_ahci_resume,
-
-};
-
static const struct of_device_id imx_ahci_of_match[] = {
{ .compatible = "fsl,imx53-ahci", .data = (void *)AHCI_IMX53 },
{ .compatible = "fsl,imx6q-ahci", .data = (void *)AHCI_IMX6Q },
@@ -251,151 +203,122 @@ MODULE_DEVICE_TABLE(of, imx_ahci_of_match);
static int imx_ahci_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- struct resource *mem, *irq, res[2];
const struct of_device_id *of_id;
- enum ahci_imx_type type;
- const struct ahci_platform_data *pdata = NULL;
+ struct ahci_host_priv *hpriv;
struct imx_ahci_priv *imxpriv;
- struct device *ahci_dev;
- struct platform_device *ahci_pdev;
+ unsigned int reg_val;
int ret;
of_id = of_match_device(imx_ahci_of_match, dev);
if (!of_id)
return -EINVAL;
- type = (enum ahci_imx_type)of_id->data;
- pdata = &imx_sata_pdata;
-
imxpriv = devm_kzalloc(dev, sizeof(*imxpriv), GFP_KERNEL);
- if (!imxpriv) {
- dev_err(dev, "can't alloc ahci_host_priv\n");
+ if (!imxpriv)
return -ENOMEM;
- }
-
- ahci_pdev = platform_device_alloc("ahci", -1);
- if (!ahci_pdev)
- return -ENODEV;
-
- ahci_dev = &ahci_pdev->dev;
- ahci_dev->parent = dev;
imxpriv->no_device = false;
imxpriv->first_time = true;
- imxpriv->type = type;
-
+ imxpriv->type = (enum ahci_imx_type)of_id->data;
imxpriv->ahb_clk = devm_clk_get(dev, "ahb");
if (IS_ERR(imxpriv->ahb_clk)) {
dev_err(dev, "can't get ahb clock.\n");
- ret = PTR_ERR(imxpriv->ahb_clk);
- goto err_out;
+ return PTR_ERR(imxpriv->ahb_clk);
}
- if (type == AHCI_IMX53) {
- imxpriv->sata_gate_clk = devm_clk_get(dev, "sata_gate");
- if (IS_ERR(imxpriv->sata_gate_clk)) {
- dev_err(dev, "can't get sata_gate clock.\n");
- ret = PTR_ERR(imxpriv->sata_gate_clk);
- goto err_out;
+ if (imxpriv->type == AHCI_IMX6Q) {
+ imxpriv->gpr = syscon_regmap_lookup_by_compatible(
+ "fsl,imx6q-iomuxc-gpr");
+ if (IS_ERR(imxpriv->gpr)) {
+ dev_err(dev,
+ "failed to find fsl,imx6q-iomux-gpr regmap\n");
+ return PTR_ERR(imxpriv->gpr);
}
}
- imxpriv->sata_ref_clk = devm_clk_get(dev, "sata_ref");
- if (IS_ERR(imxpriv->sata_ref_clk)) {
- dev_err(dev, "can't get sata_ref clock.\n");
- ret = PTR_ERR(imxpriv->sata_ref_clk);
- goto err_out;
- }
+ hpriv = ahci_platform_get_resources(pdev);
+ if (IS_ERR(hpriv))
+ return PTR_ERR(hpriv);
+
+ hpriv->plat_data = imxpriv;
- imxpriv->ahci_pdev = ahci_pdev;
- platform_set_drvdata(pdev, imxpriv);
+ ret = imx_sata_enable(hpriv);
+ if (ret)
+ return ret;
- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (!mem || !irq) {
- dev_err(dev, "no mmio/irq resource\n");
- ret = -ENOMEM;
- goto err_out;
+ /*
+ * Configure the HWINIT bits of the HOST_CAP and HOST_PORTS_IMPL,
+ * and IP vendor specific register HOST_TIMER1MS.
+ * Configure CAP_SSS (support stagered spin up).
+ * Implement the port0.
+ * Get the ahb clock rate, and configure the TIMER1MS register.
+ */
+ reg_val = readl(hpriv->mmio + HOST_CAP);
+ if (!(reg_val & HOST_CAP_SSS)) {
+ reg_val |= HOST_CAP_SSS;
+ writel(reg_val, hpriv->mmio + HOST_CAP);
+ }
+ reg_val = readl(hpriv->mmio + HOST_PORTS_IMPL);
+ if (!(reg_val & 0x1)) {
+ reg_val |= 0x1;
+ writel(reg_val, hpriv->mmio + HOST_PORTS_IMPL);
}
- res[0] = *mem;
- res[1] = *irq;
+ reg_val = clk_get_rate(imxpriv->ahb_clk) / 1000;
+ writel(reg_val, hpriv->mmio + HOST_TIMER1MS);
- ahci_dev->coherent_dma_mask = DMA_BIT_MASK(32);
- ahci_dev->dma_mask = &ahci_dev->coherent_dma_mask;
- ahci_dev->of_node = dev->of_node;
+ ret = ahci_platform_init_host(pdev, hpriv, &ahci_imx_port_info, 0, 0);
+ if (ret)
+ imx_sata_disable(hpriv);
- if (type == AHCI_IMX6Q) {
- imxpriv->gpr = syscon_regmap_lookup_by_compatible(
- "fsl,imx6q-iomuxc-gpr");
- if (IS_ERR(imxpriv->gpr)) {
- dev_err(dev,
- "failed to find fsl,imx6q-iomux-gpr regmap\n");
- ret = PTR_ERR(imxpriv->gpr);
- goto err_out;
- }
+ return ret;
+}
- /*
- * Set PHY Paremeters, two steps to configure the GPR13,
- * one write for rest of parameters, mask of first write
- * is 0x07fffffe, and the other one write for setting
- * the mpll_clk_en happens in imx_sata_clock_enable().
- */
- regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
- IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK |
- IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK |
- IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK |
- IMX6Q_GPR13_SATA_SPD_MODE_MASK |
- IMX6Q_GPR13_SATA_MPLL_SS_EN |
- IMX6Q_GPR13_SATA_TX_ATTEN_MASK |
- IMX6Q_GPR13_SATA_TX_BOOST_MASK |
- IMX6Q_GPR13_SATA_TX_LVL_MASK |
- IMX6Q_GPR13_SATA_MPLL_CLK_EN |
- IMX6Q_GPR13_SATA_TX_EDGE_RATE,
- IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB |
- IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M |
- IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F |
- IMX6Q_GPR13_SATA_SPD_MODE_3P0G |
- IMX6Q_GPR13_SATA_MPLL_SS_EN |
- IMX6Q_GPR13_SATA_TX_ATTEN_9_16 |
- IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB |
- IMX6Q_GPR13_SATA_TX_LVL_1_025_V);
- }
+static void ahci_imx_host_stop(struct ata_host *host)
+{
+ struct ahci_host_priv *hpriv = host->private_data;
- ret = platform_device_add_resources(ahci_pdev, res, 2);
- if (ret)
- goto err_out;
+ imx_sata_disable(hpriv);
+}
- ret = platform_device_add_data(ahci_pdev, pdata, sizeof(*pdata));
- if (ret)
- goto err_out;
+static int imx_ahci_suspend(struct device *dev)
+{
+ struct ata_host *host = dev_get_drvdata(dev);
+ struct ahci_host_priv *hpriv = host->private_data;
+ int ret;
- ret = platform_device_add(ahci_pdev);
- if (ret) {
-err_out:
- platform_device_put(ahci_pdev);
+ ret = ahci_platform_suspend_host(dev);
+ if (ret)
return ret;
- }
+
+ imx_sata_disable(hpriv);
return 0;
}
-static int imx_ahci_remove(struct platform_device *pdev)
+static int imx_ahci_resume(struct device *dev)
{
- struct imx_ahci_priv *imxpriv = platform_get_drvdata(pdev);
- struct platform_device *ahci_pdev = imxpriv->ahci_pdev;
+ struct ata_host *host = dev_get_drvdata(dev);
+ struct ahci_host_priv *hpriv = host->private_data;
+ int ret;
- platform_device_unregister(ahci_pdev);
- return 0;
+ ret = imx_sata_enable(hpriv);
+ if (ret)
+ return ret;
+
+ return ahci_platform_resume_host(dev);
}
+static SIMPLE_DEV_PM_OPS(ahci_imx_pm_ops, imx_ahci_suspend, imx_ahci_resume);
+
static struct platform_driver imx_ahci_driver = {
.probe = imx_ahci_probe,
- .remove = imx_ahci_remove,
+ .remove = ata_platform_remove_one,
.driver = {
.name = "ahci-imx",
.owner = THIS_MODULE,
.of_match_table = imx_ahci_of_match,
+ .pm = &ahci_imx_pm_ops,
},
};
module_platform_driver(imx_ahci_driver);
--
1.9.0
^ permalink raw reply related
* [PATCH v7 07/15] ARM: sunxi: Add support for Allwinner SUNXi SoCs sata to ahci_platform
From: Hans de Goede @ 2014-02-22 15:53 UTC (permalink / raw)
To: Tejun Heo, Maxime Ripard
Cc: Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede
In-Reply-To: <1393084424-31099-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
From: Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
This patch adds support for the ahci sata controler found on Allwinner A10
and A20 SoCs to the ahci_platform driver.
Orignally written by Olliver Schinagl using the approach of having a platform
device which probe method creates a new child platform device which gets
driven by ahci_platform.c, as done by ahci_imx.c .
Refactored by Hans de Goede to add most of the non sunxi specific functionality
to ahci_platform.c and use a platform_data pointer from of_device_id for the
sunxi specific bits.
Signed-off-by: Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
.../devicetree/bindings/ata/ahci-platform.txt | 15 +-
drivers/ata/Kconfig | 9 +
drivers/ata/Makefile | 1 +
drivers/ata/ahci_sunxi.c | 249 +++++++++++++++++++++
4 files changed, 271 insertions(+), 3 deletions(-)
create mode 100644 drivers/ata/ahci_sunxi.c
diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index 1ac807f..499bfed 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -4,7 +4,9 @@ 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 "snps,spear-ahci"
+- compatible : compatible list, one of "snps,spear-ahci",
+ "snps,exynos5440-ahci", "ibm,476gtr-ahci", or
+ "allwinner,sun4i-a10-ahci"
- interrupts : <interrupt mapping for SATA IRQ>
- reg : <registers mapping>
@@ -13,10 +15,17 @@ Optional properties:
- clocks : a list of phandle + clock specifier pairs
- target-supply : regulator for SATA target power
-Example:
+Examples:
sata@ffe08000 {
compatible = "snps,spear-ahci";
reg = <0xffe08000 0x1000>;
interrupts = <115>;
-
};
+
+ ahci: sata@01c18000 {
+ compatible = "allwinner,sun4i-a10-ahci";
+ reg = <0x01c18000 0x1000>;
+ interrupts = <56>;
+ clocks = <&pll6 0>, <&ahb_gates 25>;
+ target-supply = <®_ahci_5v>;
+ };
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 4e73772..cc67cc0 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -106,6 +106,15 @@ config AHCI_IMX
If unsure, say N.
+config AHCI_SUNXI
+ tristate "Allwinner sunxi AHCI SATA support"
+ depends on ARCH_SUNXI && SATA_AHCI_PLATFORM
+ help
+ This option enables support for the Allwinner sunxi SoC's
+ onboard AHCI SATA.
+
+ If unsure, say N.
+
config SATA_FSL
tristate "Freescale 3.0Gbps SATA support"
depends on FSL_SOC
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 46518c6..246050b 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_SATA_SIL24) += sata_sil24.o
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
# SFF w/ custom DMA
obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
diff --git a/drivers/ata/ahci_sunxi.c b/drivers/ata/ahci_sunxi.c
new file mode 100644
index 0000000..001f7dfc
--- /dev/null
+++ b/drivers/ata/ahci_sunxi.c
@@ -0,0 +1,249 @@
+/*
+ * Allwinner sunxi AHCI SATA platform driver
+ * Copyright 2013 Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
+ * Copyright 2014 Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
+ *
+ * based on the AHCI SATA platform driver by Jeff Garzik and Anton Vorontsov
+ * Based on code from Allwinner Technology Co., Ltd. <www.allwinnertech.com>,
+ * Daniel Wang <danielwang-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/ahci_platform.h>
+#include <linux/clk.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include "ahci.h"
+
+#define AHCI_BISTAFR 0x00a0
+#define AHCI_BISTCR 0x00a4
+#define AHCI_BISTFCTR 0x00a8
+#define AHCI_BISTSR 0x00ac
+#define AHCI_BISTDECR 0x00b0
+#define AHCI_DIAGNR0 0x00b4
+#define AHCI_DIAGNR1 0x00b8
+#define AHCI_OOBR 0x00bc
+#define AHCI_PHYCS0R 0x00c0
+#define AHCI_PHYCS1R 0x00c4
+#define AHCI_PHYCS2R 0x00c8
+#define AHCI_TIMER1MS 0x00e0
+#define AHCI_GPARAM1R 0x00e8
+#define AHCI_GPARAM2R 0x00ec
+#define AHCI_PPARAMR 0x00f0
+#define AHCI_TESTR 0x00f4
+#define AHCI_VERSIONR 0x00f8
+#define AHCI_IDR 0x00fc
+#define AHCI_RWCR 0x00fc
+#define AHCI_P0DMACR 0x0170
+#define AHCI_P0PHYCR 0x0178
+#define AHCI_P0PHYSR 0x017c
+
+static void sunxi_clrbits(void __iomem *reg, u32 clr_val)
+{
+ u32 reg_val;
+
+ reg_val = readl(reg);
+ reg_val &= ~(clr_val);
+ writel(reg_val, reg);
+}
+
+static void sunxi_setbits(void __iomem *reg, u32 set_val)
+{
+ u32 reg_val;
+
+ reg_val = readl(reg);
+ reg_val |= set_val;
+ writel(reg_val, reg);
+}
+
+static void sunxi_clrsetbits(void __iomem *reg, u32 clr_val, u32 set_val)
+{
+ u32 reg_val;
+
+ reg_val = readl(reg);
+ reg_val &= ~(clr_val);
+ reg_val |= set_val;
+ writel(reg_val, reg);
+}
+
+static u32 sunxi_getbits(void __iomem *reg, u8 mask, u8 shift)
+{
+ return (readl(reg) >> shift) & mask;
+}
+
+static int ahci_sunxi_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);
+
+ sunxi_setbits(reg_base + AHCI_PHYCS1R, BIT(19));
+ sunxi_clrsetbits(reg_base + AHCI_PHYCS0R,
+ (0x7 << 24),
+ (0x5 << 24) | BIT(23) | BIT(18));
+ sunxi_clrsetbits(reg_base + AHCI_PHYCS1R,
+ (0x3 << 16) | (0x1f << 8) | (0x3 << 6),
+ (0x2 << 16) | (0x6 << 8) | (0x2 << 6));
+ sunxi_setbits(reg_base + AHCI_PHYCS1R, BIT(28) | BIT(15));
+ sunxi_clrbits(reg_base + AHCI_PHYCS1R, BIT(19));
+ sunxi_clrsetbits(reg_base + AHCI_PHYCS0R,
+ (0x7 << 20), (0x3 << 20));
+ sunxi_clrsetbits(reg_base + AHCI_PHYCS2R,
+ (0x1f << 5), (0x19 << 5));
+ mdelay(5);
+
+ sunxi_setbits(reg_base + AHCI_PHYCS0R, (0x1 << 19));
+
+ timeout = 250; /* Power up takes aprox 50 us */
+ do {
+ reg_val = sunxi_getbits(reg_base + AHCI_PHYCS0R, 0x7, 28);
+ if (reg_val == 0x02)
+ break;
+
+ if (--timeout == 0) {
+ dev_err(dev, "PHY power up failed.\n");
+ return -EIO;
+ }
+ udelay(1);
+ } while (1);
+
+ sunxi_setbits(reg_base + AHCI_PHYCS2R, (0x1 << 24));
+
+ timeout = 100; /* Calibration takes aprox 10 us */
+ do {
+ reg_val = sunxi_getbits(reg_base + AHCI_PHYCS2R, 0x1, 24);
+ if (reg_val == 0x00)
+ break;
+
+ if (--timeout == 0) {
+ dev_err(dev, "PHY calibration failed.\n");
+ return -EIO;
+ }
+ udelay(1);
+ } while (1);
+
+ mdelay(15);
+
+ writel(0x7, reg_base + AHCI_RWCR);
+
+ return 0;
+}
+
+static void ahci_sunxi_start_engine(struct ata_port *ap)
+{
+ void __iomem *port_mmio = ahci_port_base(ap);
+ struct ahci_host_priv *hpriv = ap->host->private_data;
+
+ /* Setup DMA before DMA start */
+ sunxi_clrsetbits(hpriv->mmio + AHCI_P0DMACR, 0x0000ff00, 0x00004400);
+
+ /* Start DMA */
+ sunxi_setbits(port_mmio + PORT_CMD, PORT_CMD_START);
+}
+
+static const struct ata_port_info ahci_sunxi_port_info = {
+ AHCI_HFLAGS(AHCI_HFLAG_32BIT_ONLY | AHCI_HFLAG_NO_MSI |
+ AHCI_HFLAG_NO_PMP | AHCI_HFLAG_YES_NCQ),
+ .flags = AHCI_FLAG_COMMON | ATA_FLAG_NCQ,
+ .pio_mask = ATA_PIO4,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &ahci_platform_ops,
+};
+
+static int ahci_sunxi_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ahci_host_priv *hpriv;
+ int rc;
+
+ hpriv = ahci_platform_get_resources(pdev);
+ if (IS_ERR(hpriv))
+ return PTR_ERR(hpriv);
+
+ hpriv->start_engine = ahci_sunxi_start_engine;
+
+ rc = ahci_platform_enable_resources(hpriv);
+ if (rc)
+ return rc;
+
+ rc = ahci_sunxi_phy_init(dev, hpriv->mmio);
+ if (rc)
+ goto disable_resources;
+
+ rc = ahci_platform_init_host(pdev, hpriv, &ahci_sunxi_port_info, 0, 0);
+ if (rc)
+ goto disable_resources;
+
+ return 0;
+
+disable_resources:
+ ahci_platform_disable_resources(hpriv);
+ return rc;
+}
+
+#ifdef CONFIG_PM_SLEEP
+int ahci_sunxi_resume(struct device *dev)
+{
+ struct ata_host *host = dev_get_drvdata(dev);
+ struct ahci_host_priv *hpriv = host->private_data;
+ int rc;
+
+ rc = ahci_platform_enable_resources(hpriv);
+ if (rc)
+ return rc;
+
+ rc = ahci_sunxi_phy_init(dev, hpriv->mmio);
+ if (rc)
+ goto disable_resources;
+
+ rc = ahci_platform_resume_host(dev);
+ if (rc)
+ goto disable_resources;
+
+ return 0;
+
+disable_resources:
+ ahci_platform_disable_resources(hpriv);
+ return rc;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(ahci_sunxi_pm_ops, ahci_platform_suspend,
+ ahci_sunxi_resume);
+
+static const struct of_device_id ahci_sunxi_of_match[] = {
+ { .compatible = "allwinner,sun4i-a10-ahci", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, ahci_sunxi_of_match);
+
+static struct platform_driver ahci_sunxi_driver = {
+ .probe = ahci_sunxi_probe,
+ .remove = ata_platform_remove_one,
+ .driver = {
+ .name = "ahci-sunxi",
+ .owner = THIS_MODULE,
+ .of_match_table = ahci_sunxi_of_match,
+ .pm = &ahci_sunxi_pm_ops,
+ },
+};
+module_platform_driver(ahci_sunxi_driver);
+
+MODULE_DESCRIPTION("Allwinner sunxi AHCI SATA driver");
+MODULE_AUTHOR("Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>");
+MODULE_LICENSE("GPL");
--
1.9.0
^ permalink raw reply related
* [PATCH v7 06/15] ahci-platform: "Library-ise" suspend / resume functionality
From: Hans de Goede @ 2014-02-22 15:53 UTC (permalink / raw)
To: Tejun Heo, Maxime Ripard
Cc: Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede
In-Reply-To: <1393084424-31099-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Split suspend / resume code into host suspend / resume functionality and
resource enable / disabling phases, and export the new suspend_ / resume_host
functions.
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/ata/ahci_platform.c | 109 ++++++++++++++++++++++++++++++++++++------
include/linux/ahci_platform.h | 5 ++
2 files changed, 99 insertions(+), 15 deletions(-)
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 7f3f2ac..bdadec1 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -452,14 +452,26 @@ static void ahci_host_stop(struct ata_host *host)
}
#ifdef CONFIG_PM_SLEEP
-static int ahci_suspend(struct device *dev)
+/**
+ * ahci_platform_suspend_host - Suspend an ahci-platform host
+ * @dev: device pointer for the host
+ *
+ * This function does all the usual steps needed to suspend an
+ * ahci-platform host, note any necessary resources (ie clks, phy, etc.)
+ * must be disabled after calling this.
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * 0 on success otherwise a negative error code
+ */
+int ahci_platform_suspend_host(struct device *dev)
{
- struct ahci_platform_data *pdata = dev_get_platdata(dev);
struct ata_host *host = dev_get_drvdata(dev);
struct ahci_host_priv *hpriv = host->private_data;
void __iomem *mmio = hpriv->mmio;
u32 ctl;
- int rc;
if (hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
dev_err(dev, "firmware update required for suspend/resume\n");
@@ -476,7 +488,64 @@ static int ahci_suspend(struct device *dev)
writel(ctl, mmio + HOST_CTL);
readl(mmio + HOST_CTL); /* flush */
- rc = ata_host_suspend(host, PMSG_SUSPEND);
+ return ata_host_suspend(host, PMSG_SUSPEND);
+}
+EXPORT_SYMBOL_GPL(ahci_platform_suspend_host);
+
+/**
+ * ahci_platform_resume_host - Resume an ahci-platform host
+ * @dev: device pointer for the host
+ *
+ * This function does all the usual steps needed to resume an
+ * ahci-platform host, note any necessary resources (ie clks, phy, etc.)
+ * must be initialized / enabled before calling this.
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * 0 on success otherwise a negative error code
+ */
+int ahci_platform_resume_host(struct device *dev)
+{
+ struct ata_host *host = dev_get_drvdata(dev);
+ int rc;
+
+ if (dev->power.power_state.event == PM_EVENT_SUSPEND) {
+ rc = ahci_reset_controller(host);
+ if (rc)
+ return rc;
+
+ ahci_init_controller(host);
+ }
+
+ ata_host_resume(host);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ahci_platform_resume_host);
+
+/**
+ * ahci_platform_suspend - Suspend an ahci-platform device
+ * @dev: the platform device to suspend
+ *
+ * This function suspends the host associated with the device, followed
+ * by disabling all the resources of the device.
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * 0 on success otherwise a negative error code
+ */
+int ahci_platform_suspend(struct device *dev)
+{
+ struct ahci_platform_data *pdata = dev_get_platdata(dev);
+ struct ata_host *host = dev_get_drvdata(dev);
+ struct ahci_host_priv *hpriv = host->private_data;
+ int rc;
+
+ rc = ahci_platform_suspend_host(dev);
if (rc)
return rc;
@@ -487,8 +556,22 @@ static int ahci_suspend(struct device *dev)
return 0;
}
+EXPORT_SYMBOL_GPL(ahci_platform_suspend);
-static int ahci_resume(struct device *dev)
+/**
+ * ahci_platform_resume - Resume an ahci-platform device
+ * @dev: the platform device to resume
+ *
+ * This function enables all the resources of the device followed by
+ * resuming the host associated with the device.
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * 0 on success otherwise a negative error code
+ */
+int ahci_platform_resume(struct device *dev)
{
struct ahci_platform_data *pdata = dev_get_platdata(dev);
struct ata_host *host = dev_get_drvdata(dev);
@@ -505,15 +588,9 @@ static int ahci_resume(struct device *dev)
goto disable_resources;
}
- if (dev->power.power_state.event == PM_EVENT_SUSPEND) {
- rc = ahci_reset_controller(host);
- if (rc)
- goto disable_resources;
-
- ahci_init_controller(host);
- }
-
- ata_host_resume(host);
+ rc = ahci_platform_resume_host(dev);
+ if (rc)
+ goto disable_resources;
return 0;
@@ -522,9 +599,11 @@ disable_resources:
return rc;
}
+EXPORT_SYMBOL_GPL(ahci_platform_resume);
#endif
-static SIMPLE_DEV_PM_OPS(ahci_pm_ops, ahci_suspend, ahci_resume);
+static SIMPLE_DEV_PM_OPS(ahci_pm_ops, ahci_platform_suspend,
+ ahci_platform_resume);
static const struct of_device_id ahci_of_match[] = {
{ .compatible = "snps,spear-ahci", },
diff --git a/include/linux/ahci_platform.h b/include/linux/ahci_platform.h
index b80c51c..542f268 100644
--- a/include/linux/ahci_platform.h
+++ b/include/linux/ahci_platform.h
@@ -50,4 +50,9 @@ int ahci_platform_init_host(struct platform_device *pdev,
unsigned int force_port_map,
unsigned int mask_port_map);
+int ahci_platform_suspend_host(struct device *dev);
+int ahci_platform_resume_host(struct device *dev);
+int ahci_platform_suspend(struct device *dev);
+int ahci_platform_resume(struct device *dev);
+
#endif /* _AHCI_PLATFORM_H */
--
1.9.0
^ permalink raw reply related
* [PATCH v7 05/15] ahci-platform: "Library-ise" ahci_probe functionality
From: Hans de Goede @ 2014-02-22 15:53 UTC (permalink / raw)
To: Tejun Heo, Maxime Ripard
Cc: Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede
In-Reply-To: <1393084424-31099-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
ahci_probe consists of 3 steps:
1) Get resources (get mmio, clks, regulator)
2) Enable resources, handled by ahci_platform_enable_resouces
3) The more or less standard ahci-host controller init sequence
This commit refactors step 1 and 3 into separate functions, so the platform
drivers for AHCI implementations which need a specific order in step 2,
and / or need to do some custom register poking at some time, can re-use
ahci-platform.c code without needing to copy and paste it.
Note that ahci_platform_init_host's prototype takes the 3 non function
members of ahci_platform_data as arguments, the idea is that drivers using
the new exported utility functions will not use ahci_platform_data at all,
and hopefully in the future ahci_platform_data can go away entirely.
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/ata/ahci_platform.c | 195 ++++++++++++++++++++++++++++--------------
include/linux/ahci_platform.h | 14 +++
2 files changed, 144 insertions(+), 65 deletions(-)
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 6ebbc17..7f3f2ac 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -201,64 +201,64 @@ void ahci_platform_disable_resources(struct ahci_host_priv *hpriv)
}
EXPORT_SYMBOL_GPL(ahci_platform_disable_resources);
-static void ahci_put_clks(struct ahci_host_priv *hpriv)
+static void ahci_platform_put_resources(struct device *dev, void *res)
{
+ struct ahci_host_priv *hpriv = res;
int c;
for (c = 0; c < AHCI_MAX_CLKS && hpriv->clks[c]; c++)
clk_put(hpriv->clks[c]);
}
-static int ahci_probe(struct platform_device *pdev)
+/**
+ * ahci_platform_get_resources - Get platform resources
+ * @pdev: platform device to get resources for
+ *
+ * This function allocates an ahci_host_priv struct, and gets the
+ * following resources, storing a reference to them inside the returned
+ * struct:
+ *
+ * 1) mmio registers (IORESOURCE_MEM 0, mandatory)
+ * 2) regulator for controlling the targets power (optional)
+ * 3) 0 - AHCI_MAX_CLKS clocks, as specified in the devs devicetree node,
+ * or for non devicetree enabled platforms a single clock
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * The allocated ahci_host_priv on success, otherwise an ERR_PTR value
+ */
+struct ahci_host_priv *ahci_platform_get_resources(
+ struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- struct ahci_platform_data *pdata = dev_get_platdata(dev);
- const struct platform_device_id *id = platform_get_device_id(pdev);
- struct ata_port_info pi = ahci_port_info[id ? id->driver_data : 0];
- const struct ata_port_info *ppi[] = { &pi, NULL };
struct ahci_host_priv *hpriv;
- struct ata_host *host;
- struct resource *mem;
struct clk *clk;
- int irq;
- int n_ports;
- int i;
- int rc;
+ int i, rc = -ENOMEM;
- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!mem) {
- dev_err(dev, "no mmio space\n");
- return -EINVAL;
- }
+ if (!devres_open_group(dev, NULL, GFP_KERNEL))
+ return ERR_PTR(-ENOMEM);
- irq = platform_get_irq(pdev, 0);
- if (irq <= 0) {
- dev_err(dev, "no irq\n");
- return -EINVAL;
- }
+ hpriv = devres_alloc(ahci_platform_put_resources, sizeof(*hpriv),
+ GFP_KERNEL);
+ if (!hpriv)
+ goto err_out;
- if (pdata && pdata->ata_port_info)
- pi = *pdata->ata_port_info;
+ devres_add(dev, hpriv);
- hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
- if (!hpriv) {
- dev_err(dev, "can't alloc ahci_host_priv\n");
- return -ENOMEM;
- }
-
- hpriv->flags |= (unsigned long)pi.private_data;
-
- hpriv->mmio = devm_ioremap(dev, mem->start, resource_size(mem));
+ hpriv->mmio = devm_ioremap_resource(dev,
+ platform_get_resource(pdev, IORESOURCE_MEM, 0));
if (!hpriv->mmio) {
- dev_err(dev, "can't map %pR\n", mem);
- return -ENOMEM;
+ dev_err(dev, "no mmio space\n");
+ goto err_out;
}
hpriv->target_pwr = devm_regulator_get_optional(dev, "target");
if (IS_ERR(hpriv->target_pwr)) {
rc = PTR_ERR(hpriv->target_pwr);
if (rc == -EPROBE_DEFER)
- return -EPROBE_DEFER;
+ goto err_out;
hpriv->target_pwr = NULL;
}
@@ -277,33 +277,62 @@ static int ahci_probe(struct platform_device *pdev)
if (IS_ERR(clk)) {
rc = PTR_ERR(clk);
if (rc == -EPROBE_DEFER)
- goto free_clk;
+ goto err_out;
break;
}
hpriv->clks[i] = clk;
}
- rc = ahci_platform_enable_resources(hpriv);
- if (rc)
- goto free_clk;
+ devres_remove_group(dev, NULL);
+ return hpriv;
- /*
- * Some platforms might need to prepare for mmio region access,
- * which could be done in the following init call. So, the mmio
- * region shouldn't be accessed before init (if provided) has
- * returned successfully.
- */
- if (pdata && pdata->init) {
- rc = pdata->init(dev, hpriv->mmio);
- if (rc)
- goto disable_resources;
- }
+err_out:
+ devres_release_group(dev, NULL);
+ return ERR_PTR(rc);
+}
+EXPORT_SYMBOL_GPL(ahci_platform_get_resources);
+
+/**
+ * ahci_platform_init_host - Bring up an ahci-platform host
+ * @pdev: platform device pointer for the host
+ * @hpriv: ahci-host private data for the host
+ * @pi_template: template for the ata_port_info to use
+ * @force_port_map: param passed to ahci_save_initial_config
+ * @mask_port_map: param passed to ahci_save_initial_config
+ *
+ * This function does all the usual steps needed to bring up an
+ * ahci-platform host, note any necessary resources (ie clks, phy, etc.)
+ * must be initialized / enabled before calling this.
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * 0 on success otherwise a negative error code
+ */
+int ahci_platform_init_host(struct platform_device *pdev,
+ struct ahci_host_priv *hpriv,
+ const struct ata_port_info *pi_template,
+ unsigned int force_port_map,
+ unsigned int mask_port_map)
+{
+ struct device *dev = &pdev->dev;
+ struct ata_port_info pi = *pi_template;
+ const struct ata_port_info *ppi[] = { &pi, NULL };
+ struct ata_host *host;
+ int i, irq, n_ports, rc;
- ahci_save_initial_config(dev, hpriv,
- pdata ? pdata->force_port_map : 0,
- pdata ? pdata->mask_port_map : 0);
+ irq = platform_get_irq(pdev, 0);
+ if (irq <= 0) {
+ dev_err(dev, "no irq\n");
+ return -EINVAL;
+ }
/* prepare host */
+ hpriv->flags |= (unsigned long)pi.private_data;
+
+ ahci_save_initial_config(dev, hpriv, force_port_map, mask_port_map);
+
if (hpriv->cap & HOST_CAP_NCQ)
pi.flags |= ATA_FLAG_NCQ;
@@ -320,10 +349,8 @@ static int ahci_probe(struct platform_device *pdev)
n_ports = max(ahci_nr_ports(hpriv->cap), fls(hpriv->port_map));
host = ata_host_alloc_pinfo(dev, ppi, n_ports);
- if (!host) {
- rc = -ENOMEM;
- goto pdata_exit;
- }
+ if (!host)
+ return -ENOMEM;
host->private_data = hpriv;
@@ -338,7 +365,8 @@ static int ahci_probe(struct platform_device *pdev)
for (i = 0; i < host->n_ports; i++) {
struct ata_port *ap = host->ports[i];
- ata_port_desc(ap, "mmio %pR", mem);
+ ata_port_desc(ap, "mmio %pR",
+ platform_get_resource(pdev, IORESOURCE_MEM, 0));
ata_port_desc(ap, "port 0x%x", 0x100 + ap->port_no * 0x80);
/* set enclosure management message type */
@@ -352,13 +380,53 @@ static int ahci_probe(struct platform_device *pdev)
rc = ahci_reset_controller(host);
if (rc)
- goto pdata_exit;
+ return rc;
ahci_init_controller(host);
ahci_print_info(host, "platform");
- rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
- &ahci_platform_sht);
+ return ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
+ &ahci_platform_sht);
+}
+EXPORT_SYMBOL_GPL(ahci_platform_init_host);
+
+static int ahci_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ahci_platform_data *pdata = dev_get_platdata(dev);
+ const struct platform_device_id *id = platform_get_device_id(pdev);
+ const struct ata_port_info *pi_template;
+ struct ahci_host_priv *hpriv;
+ int rc;
+
+ hpriv = ahci_platform_get_resources(pdev);
+ if (IS_ERR(hpriv))
+ return PTR_ERR(hpriv);
+
+ rc = ahci_platform_enable_resources(hpriv);
+ if (rc)
+ return rc;
+
+ /*
+ * Some platforms might need to prepare for mmio region access,
+ * which could be done in the following init call. So, the mmio
+ * region shouldn't be accessed before init (if provided) has
+ * returned successfully.
+ */
+ if (pdata && pdata->init) {
+ rc = pdata->init(dev, hpriv->mmio);
+ if (rc)
+ goto disable_resources;
+ }
+
+ if (pdata && pdata->ata_port_info)
+ pi_template = pdata->ata_port_info;
+ else
+ pi_template = &ahci_port_info[id ? id->driver_data : 0];
+
+ rc = ahci_platform_init_host(pdev, hpriv, pi_template,
+ pdata ? pdata->force_port_map : 0,
+ pdata ? pdata->mask_port_map : 0);
if (rc)
goto pdata_exit;
@@ -368,8 +436,6 @@ pdata_exit:
pdata->exit(dev);
disable_resources:
ahci_platform_disable_resources(hpriv);
-free_clk:
- ahci_put_clks(hpriv);
return rc;
}
@@ -383,7 +449,6 @@ static void ahci_host_stop(struct ata_host *host)
pdata->exit(dev);
ahci_platform_disable_resources(hpriv);
- ahci_put_clks(hpriv);
}
#ifdef CONFIG_PM_SLEEP
diff --git a/include/linux/ahci_platform.h b/include/linux/ahci_platform.h
index b674b01..b80c51c 100644
--- a/include/linux/ahci_platform.h
+++ b/include/linux/ahci_platform.h
@@ -20,7 +20,14 @@
struct device;
struct ata_port_info;
struct ahci_host_priv;
+struct platform_device;
+/*
+ * Note ahci_platform_data is deprecated, it is only kept around for use
+ * by the old da850 and spear13xx ahci code.
+ * New drivers should instead declare their own platform_driver struct, and
+ * use ahci_platform* functions in their own probe, suspend and resume methods.
+ */
struct ahci_platform_data {
int (*init)(struct device *dev, void __iomem *addr);
void (*exit)(struct device *dev);
@@ -35,5 +42,12 @@ int ahci_platform_enable_clks(struct ahci_host_priv *hpriv);
void ahci_platform_disable_clks(struct ahci_host_priv *hpriv);
int ahci_platform_enable_resources(struct ahci_host_priv *hpriv);
void ahci_platform_disable_resources(struct ahci_host_priv *hpriv);
+struct ahci_host_priv *ahci_platform_get_resources(
+ struct platform_device *pdev);
+int ahci_platform_init_host(struct platform_device *pdev,
+ struct ahci_host_priv *hpriv,
+ const struct ata_port_info *pi_template,
+ unsigned int force_port_map,
+ unsigned int mask_port_map);
#endif /* _AHCI_PLATFORM_H */
--
1.9.0
^ permalink raw reply related
* [PATCH v7 04/15] ahci-platform: Add enable_ / disable_resources helper functions
From: Hans de Goede @ 2014-02-22 15:53 UTC (permalink / raw)
To: Tejun Heo, Maxime Ripard
Cc: Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede
In-Reply-To: <1393084424-31099-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/ata/ahci_platform.c | 112 ++++++++++++++++++++++++++++--------------
include/linux/ahci_platform.h | 2 +
2 files changed, 77 insertions(+), 37 deletions(-)
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 907c076..6ebbc17 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -139,6 +139,68 @@ void ahci_platform_disable_clks(struct ahci_host_priv *hpriv)
}
EXPORT_SYMBOL_GPL(ahci_platform_disable_clks);
+/**
+ * ahci_platform_enable_resources - Enable platform resources
+ * @hpriv: host private area to store config values
+ *
+ * This function enables all ahci_platform managed resources in
+ * the following order:
+ * 1) Regulator
+ * 2) Clocks (through ahci_platform_enable_clks)
+ *
+ * If resource enabling fails at any point the previous enabled
+ * resources are disabled in reverse order.
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * 0 on success otherwise a negative error code
+ */
+int ahci_platform_enable_resources(struct ahci_host_priv *hpriv)
+{
+ int rc;
+
+ if (hpriv->target_pwr) {
+ rc = regulator_enable(hpriv->target_pwr);
+ if (rc)
+ return rc;
+ }
+
+ rc = ahci_platform_enable_clks(hpriv);
+ if (rc)
+ goto disable_regulator;
+
+ return 0;
+
+disable_regulator:
+ if (hpriv->target_pwr)
+ regulator_disable(hpriv->target_pwr);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(ahci_platform_enable_resources);
+
+/**
+ * ahci_platform_disable_resources - Disable platform resources
+ * @hpriv: host private area to store config values
+ *
+ * This function disables all ahci_platform managed resources in
+ * the following order:
+ * 1) Clocks (through ahci_platform_disable_clks)
+ * 2) Regulator
+ *
+ * LOCKING:
+ * None.
+ */
+void ahci_platform_disable_resources(struct ahci_host_priv *hpriv)
+{
+ ahci_platform_disable_clks(hpriv);
+
+ if (hpriv->target_pwr)
+ regulator_disable(hpriv->target_pwr);
+}
+EXPORT_SYMBOL_GPL(ahci_platform_disable_resources);
+
static void ahci_put_clks(struct ahci_host_priv *hpriv)
{
int c;
@@ -221,15 +283,9 @@ static int ahci_probe(struct platform_device *pdev)
hpriv->clks[i] = clk;
}
- if (hpriv->target_pwr) {
- rc = regulator_enable(hpriv->target_pwr);
- if (rc)
- goto free_clk;
- }
-
- rc = ahci_enable_clks(dev, hpriv);
+ rc = ahci_platform_enable_resources(hpriv);
if (rc)
- goto disable_regulator;
+ goto free_clk;
/*
* Some platforms might need to prepare for mmio region access,
@@ -240,7 +296,7 @@ static int ahci_probe(struct platform_device *pdev)
if (pdata && pdata->init) {
rc = pdata->init(dev, hpriv->mmio);
if (rc)
- goto disable_unprepare_clk;
+ goto disable_resources;
}
ahci_save_initial_config(dev, hpriv,
@@ -310,11 +366,8 @@ static int ahci_probe(struct platform_device *pdev)
pdata_exit:
if (pdata && pdata->exit)
pdata->exit(dev);
-disable_unprepare_clk:
- ahci_disable_clks(hpriv);
-disable_regulator:
- if (hpriv->target_pwr)
- regulator_disable(hpriv->target_pwr);
+disable_resources:
+ ahci_platform_disable_resources(hpriv);
free_clk:
ahci_put_clks(hpriv);
return rc;
@@ -329,11 +382,8 @@ static void ahci_host_stop(struct ata_host *host)
if (pdata && pdata->exit)
pdata->exit(dev);
- ahci_disable_clks(hpriv);
+ ahci_platform_disable_resources(hpriv);
ahci_put_clks(hpriv);
-
- if (hpriv->target_pwr)
- regulator_disable(hpriv->target_pwr);
}
#ifdef CONFIG_PM_SLEEP
@@ -368,10 +418,7 @@ static int ahci_suspend(struct device *dev)
if (pdata && pdata->suspend)
return pdata->suspend(dev);
- ahci_disable_clks(hpriv);
-
- if (hpriv->target_pwr)
- regulator_disable(hpriv->target_pwr);
+ ahci_platform_disable_resources(hpriv);
return 0;
}
@@ -383,26 +430,20 @@ static int ahci_resume(struct device *dev)
struct ahci_host_priv *hpriv = host->private_data;
int rc;
- if (hpriv->target_pwr) {
- rc = regulator_enable(hpriv->target_pwr);
- if (rc)
- return rc;
- }
-
- rc = ahci_enable_clks(dev, hpriv);
+ rc = ahci_platform_enable_resources(hpriv);
if (rc)
- goto disable_regulator;
+ return rc;
if (pdata && pdata->resume) {
rc = pdata->resume(dev);
if (rc)
- goto disable_unprepare_clk;
+ goto disable_resources;
}
if (dev->power.power_state.event == PM_EVENT_SUSPEND) {
rc = ahci_reset_controller(host);
if (rc)
- goto disable_unprepare_clk;
+ goto disable_resources;
ahci_init_controller(host);
}
@@ -411,11 +452,8 @@ static int ahci_resume(struct device *dev)
return 0;
-disable_unprepare_clk:
- ahci_disable_clks(hpriv);
-disable_regulator:
- if (hpriv->target_pwr)
- regulator_disable(hpriv->target_pwr);
+disable_resources:
+ ahci_platform_disable_resources(hpriv);
return rc;
}
diff --git a/include/linux/ahci_platform.h b/include/linux/ahci_platform.h
index 769d065..b674b01 100644
--- a/include/linux/ahci_platform.h
+++ b/include/linux/ahci_platform.h
@@ -33,5 +33,7 @@ struct ahci_platform_data {
int ahci_platform_enable_clks(struct ahci_host_priv *hpriv);
void ahci_platform_disable_clks(struct ahci_host_priv *hpriv);
+int ahci_platform_enable_resources(struct ahci_host_priv *hpriv);
+void ahci_platform_disable_resources(struct ahci_host_priv *hpriv);
#endif /* _AHCI_PLATFORM_H */
--
1.9.0
^ permalink raw reply related
* [PATCH v7 03/15] ahci-platform: Add support for an optional regulator for sata-target power
From: Hans de Goede @ 2014-02-22 15:53 UTC (permalink / raw)
To: Tejun Heo, Maxime Ripard
Cc: Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede
In-Reply-To: <1393084424-31099-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
.../devicetree/bindings/ata/ahci-platform.txt | 1 +
drivers/ata/ahci.h | 2 ++
drivers/ata/ahci_platform.c | 36 ++++++++++++++++++++--
3 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index 3ced07d..1ac807f 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -11,6 +11,7 @@ Required properties:
Optional properties:
- dma-coherent : Present if dma operations are coherent
- clocks : a list of phandle + clock specifier pairs
+- target-supply : regulator for SATA target power
Example:
sata@ffe08000 {
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index c12862b..bf8100c 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -37,6 +37,7 @@
#include <linux/clk.h>
#include <linux/libata.h>
+#include <linux/regulator/consumer.h>
/* Enclosure Management Control */
#define EM_CTRL_MSG_TYPE 0x000f0000
@@ -323,6 +324,7 @@ struct ahci_host_priv {
u32 em_buf_sz; /* EM buffer size in byte */
u32 em_msg_type; /* EM message type */
struct clk *clks[AHCI_MAX_CLKS]; /* Optional */
+ struct regulator *target_pwr; /* Optional */
void *plat_data; /* Other platform data */
/*
* Optional ahci_start_engine override, if not set this gets set to the
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 609975d..907c076 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -192,6 +192,14 @@ static int ahci_probe(struct platform_device *pdev)
return -ENOMEM;
}
+ hpriv->target_pwr = devm_regulator_get_optional(dev, "target");
+ if (IS_ERR(hpriv->target_pwr)) {
+ rc = PTR_ERR(hpriv->target_pwr);
+ if (rc == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ hpriv->target_pwr = NULL;
+ }
+
for (i = 0; i < AHCI_MAX_CLKS; i++) {
/*
* For now we must use clk_get(dev, NULL) for the first clock,
@@ -213,9 +221,15 @@ static int ahci_probe(struct platform_device *pdev)
hpriv->clks[i] = clk;
}
+ if (hpriv->target_pwr) {
+ rc = regulator_enable(hpriv->target_pwr);
+ if (rc)
+ goto free_clk;
+ }
+
rc = ahci_enable_clks(dev, hpriv);
if (rc)
- goto free_clk;
+ goto disable_regulator;
/*
* Some platforms might need to prepare for mmio region access,
@@ -298,6 +312,9 @@ pdata_exit:
pdata->exit(dev);
disable_unprepare_clk:
ahci_disable_clks(hpriv);
+disable_regulator:
+ if (hpriv->target_pwr)
+ regulator_disable(hpriv->target_pwr);
free_clk:
ahci_put_clks(hpriv);
return rc;
@@ -314,6 +331,9 @@ static void ahci_host_stop(struct ata_host *host)
ahci_disable_clks(hpriv);
ahci_put_clks(hpriv);
+
+ if (hpriv->target_pwr)
+ regulator_disable(hpriv->target_pwr);
}
#ifdef CONFIG_PM_SLEEP
@@ -350,6 +370,9 @@ static int ahci_suspend(struct device *dev)
ahci_disable_clks(hpriv);
+ if (hpriv->target_pwr)
+ regulator_disable(hpriv->target_pwr);
+
return 0;
}
@@ -360,9 +383,15 @@ static int ahci_resume(struct device *dev)
struct ahci_host_priv *hpriv = host->private_data;
int rc;
+ if (hpriv->target_pwr) {
+ rc = regulator_enable(hpriv->target_pwr);
+ if (rc)
+ return rc;
+ }
+
rc = ahci_enable_clks(dev, hpriv);
if (rc)
- return rc;
+ goto disable_regulator;
if (pdata && pdata->resume) {
rc = pdata->resume(dev);
@@ -384,6 +413,9 @@ static int ahci_resume(struct device *dev)
disable_unprepare_clk:
ahci_disable_clks(hpriv);
+disable_regulator:
+ if (hpriv->target_pwr)
+ regulator_disable(hpriv->target_pwr);
return rc;
}
--
1.9.0
^ permalink raw reply related
* [PATCH v7 02/15] ahci-platform: Add support for devices with more then 1 clock
From: Hans de Goede @ 2014-02-22 15:53 UTC (permalink / raw)
To: Tejun Heo, Maxime Ripard
Cc: Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede
In-Reply-To: <1393084424-31099-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
The allwinner-sun4i AHCI controller needs 2 clocks to be enabled and the
imx AHCI controller needs 3 clocks to be enabled.
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
.../devicetree/bindings/ata/ahci-platform.txt | 1 +
drivers/ata/ahci.h | 3 +-
drivers/ata/ahci_platform.c | 119 ++++++++++++++++-----
include/linux/ahci_platform.h | 4 +
4 files changed, 99 insertions(+), 28 deletions(-)
diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index 89de156..3ced07d 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -10,6 +10,7 @@ Required properties:
Optional properties:
- dma-coherent : Present if dma operations are coherent
+- clocks : a list of phandle + clock specifier pairs
Example:
sata@ffe08000 {
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 64d1a99..c12862b 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -51,6 +51,7 @@
enum {
AHCI_MAX_PORTS = 32,
+ AHCI_MAX_CLKS = 3,
AHCI_MAX_SG = 168, /* hardware max is 64K */
AHCI_DMA_BOUNDARY = 0xffffffff,
AHCI_MAX_CMDS = 32,
@@ -321,7 +322,7 @@ struct ahci_host_priv {
u32 em_loc; /* enclosure management location */
u32 em_buf_sz; /* EM buffer size in byte */
u32 em_msg_type; /* EM message type */
- struct clk *clk; /* Only for platforms supporting clk */
+ struct clk *clks[AHCI_MAX_CLKS]; /* Optional */
void *plat_data; /* Other platform data */
/*
* Optional ahci_start_engine override, if not set this gets set to the
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 4b231ba..609975d 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -87,6 +87,66 @@ static struct scsi_host_template ahci_platform_sht = {
AHCI_SHT("ahci_platform"),
};
+/**
+ * ahci_platform_enable_clks - Enable platform clocks
+ * @hpriv: host private area to store config values
+ *
+ * This function enables all the clks found in hpriv->clks, starting
+ * at index 0. If any clk fails to enable it disables all the clks
+ * already enabled in reverse order, and then returns an error.
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * 0 on success otherwise a negative error code
+ */
+int ahci_platform_enable_clks(struct ahci_host_priv *hpriv)
+{
+ int c, rc;
+
+ for (c = 0; c < AHCI_MAX_CLKS && hpriv->clks[c]; c++) {
+ rc = clk_prepare_enable(hpriv->clks[c]);
+ if (rc)
+ goto disable_unprepare_clk;
+ }
+ return 0;
+
+disable_unprepare_clk:
+ while (--c >= 0)
+ clk_disable_unprepare(hpriv->clks[c]);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(ahci_platform_enable_clks);
+
+/**
+ * ahci_platform_disable_clks - Disable platform clocks
+ * @hpriv: host private area to store config values
+ *
+ * This function disables all the clks found in hpriv->clks, in reverse
+ * order of ahci_platform_enable_clks (starting at the end of the array).
+ *
+ * LOCKING:
+ * None.
+ */
+void ahci_platform_disable_clks(struct ahci_host_priv *hpriv)
+{
+ int c;
+
+ for (c = AHCI_MAX_CLKS - 1; c >= 0; c--)
+ if (hpriv->clks[c])
+ clk_disable_unprepare(hpriv->clks[c]);
+}
+EXPORT_SYMBOL_GPL(ahci_platform_disable_clks);
+
+static void ahci_put_clks(struct ahci_host_priv *hpriv)
+{
+ int c;
+
+ for (c = 0; c < AHCI_MAX_CLKS && hpriv->clks[c]; c++)
+ clk_put(hpriv->clks[c]);
+}
+
static int ahci_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -97,6 +157,7 @@ static int ahci_probe(struct platform_device *pdev)
struct ahci_host_priv *hpriv;
struct ata_host *host;
struct resource *mem;
+ struct clk *clk;
int irq;
int n_ports;
int i;
@@ -131,17 +192,31 @@ static int ahci_probe(struct platform_device *pdev)
return -ENOMEM;
}
- hpriv->clk = clk_get(dev, NULL);
- if (IS_ERR(hpriv->clk)) {
- dev_err(dev, "can't get clock\n");
- } else {
- rc = clk_prepare_enable(hpriv->clk);
- if (rc) {
- dev_err(dev, "clock prepare enable failed");
- goto free_clk;
+ for (i = 0; i < AHCI_MAX_CLKS; i++) {
+ /*
+ * For now we must use clk_get(dev, NULL) for the first clock,
+ * because some platforms (da850, spear13xx) are not yet
+ * converted to use devicetree for clocks. For new platforms
+ * this is equivalent to of_clk_get(dev->of_node, 0).
+ */
+ if (i == 0)
+ clk = clk_get(dev, NULL);
+ else
+ clk = of_clk_get(dev->of_node, i);
+
+ if (IS_ERR(clk)) {
+ rc = PTR_ERR(clk);
+ if (rc == -EPROBE_DEFER)
+ goto free_clk;
+ break;
}
+ hpriv->clks[i] = clk;
}
+ rc = ahci_enable_clks(dev, hpriv);
+ if (rc)
+ goto free_clk;
+
/*
* Some platforms might need to prepare for mmio region access,
* which could be done in the following init call. So, the mmio
@@ -222,11 +297,9 @@ pdata_exit:
if (pdata && pdata->exit)
pdata->exit(dev);
disable_unprepare_clk:
- if (!IS_ERR(hpriv->clk))
- clk_disable_unprepare(hpriv->clk);
+ ahci_disable_clks(hpriv);
free_clk:
- if (!IS_ERR(hpriv->clk))
- clk_put(hpriv->clk);
+ ahci_put_clks(hpriv);
return rc;
}
@@ -239,10 +312,8 @@ static void ahci_host_stop(struct ata_host *host)
if (pdata && pdata->exit)
pdata->exit(dev);
- if (!IS_ERR(hpriv->clk)) {
- clk_disable_unprepare(hpriv->clk);
- clk_put(hpriv->clk);
- }
+ ahci_disable_clks(hpriv);
+ ahci_put_clks(hpriv);
}
#ifdef CONFIG_PM_SLEEP
@@ -277,8 +348,7 @@ static int ahci_suspend(struct device *dev)
if (pdata && pdata->suspend)
return pdata->suspend(dev);
- if (!IS_ERR(hpriv->clk))
- clk_disable_unprepare(hpriv->clk);
+ ahci_disable_clks(hpriv);
return 0;
}
@@ -290,13 +360,9 @@ static int ahci_resume(struct device *dev)
struct ahci_host_priv *hpriv = host->private_data;
int rc;
- if (!IS_ERR(hpriv->clk)) {
- rc = clk_prepare_enable(hpriv->clk);
- if (rc) {
- dev_err(dev, "clock prepare enable failed");
- return rc;
- }
- }
+ rc = ahci_enable_clks(dev, hpriv);
+ if (rc)
+ return rc;
if (pdata && pdata->resume) {
rc = pdata->resume(dev);
@@ -317,8 +383,7 @@ static int ahci_resume(struct device *dev)
return 0;
disable_unprepare_clk:
- if (!IS_ERR(hpriv->clk))
- clk_disable_unprepare(hpriv->clk);
+ ahci_disable_clks(hpriv);
return rc;
}
diff --git a/include/linux/ahci_platform.h b/include/linux/ahci_platform.h
index 73a2500..769d065 100644
--- a/include/linux/ahci_platform.h
+++ b/include/linux/ahci_platform.h
@@ -19,6 +19,7 @@
struct device;
struct ata_port_info;
+struct ahci_host_priv;
struct ahci_platform_data {
int (*init)(struct device *dev, void __iomem *addr);
@@ -30,4 +31,7 @@ struct ahci_platform_data {
unsigned int mask_port_map;
};
+int ahci_platform_enable_clks(struct ahci_host_priv *hpriv);
+void ahci_platform_disable_clks(struct ahci_host_priv *hpriv);
+
#endif /* _AHCI_PLATFORM_H */
--
1.9.0
^ permalink raw reply related
* [PATCH v7 01/15] libahci: Allow drivers to override start_engine
From: Hans de Goede @ 2014-02-22 15:53 UTC (permalink / raw)
To: Tejun Heo, Maxime Ripard
Cc: Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede
In-Reply-To: <1393084424-31099-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Allwinner A10 and A20 ARM SoCs have an AHCI sata controller which needs a
special register to be poked before starting the DMA engine.
This register gets reset on an ahci_stop_engine call, so there is no other
place then ahci_start_engine where this poking can be done.
This commit allows drivers to override ahci_start_engine behavior for use by
the Allwinner AHCI driver (and potentially other drivers in the future).
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/ata/ahci.c | 6 ++++--
drivers/ata/ahci.h | 6 ++++++
drivers/ata/libahci.c | 26 +++++++++++++++++++-------
drivers/ata/sata_highbank.c | 3 ++-
4 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index dc2756f..eda68b4 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -564,6 +564,7 @@ static int ahci_vt8251_hardreset(struct ata_link *link, unsigned int *class,
unsigned long deadline)
{
struct ata_port *ap = link->ap;
+ struct ahci_host_priv *hpriv = ap->host->private_data;
bool online;
int rc;
@@ -574,7 +575,7 @@ static int ahci_vt8251_hardreset(struct ata_link *link, unsigned int *class,
rc = sata_link_hardreset(link, sata_ehc_deb_timing(&link->eh_context),
deadline, &online, NULL);
- ahci_start_engine(ap);
+ hpriv->start_engine(ap);
DPRINTK("EXIT, rc=%d, class=%u\n", rc, *class);
@@ -589,6 +590,7 @@ static int ahci_p5wdh_hardreset(struct ata_link *link, unsigned int *class,
{
struct ata_port *ap = link->ap;
struct ahci_port_priv *pp = ap->private_data;
+ struct ahci_host_priv *hpriv = ap->host->private_data;
u8 *d2h_fis = pp->rx_fis + RX_FIS_D2H_REG;
struct ata_taskfile tf;
bool online;
@@ -604,7 +606,7 @@ static int ahci_p5wdh_hardreset(struct ata_link *link, unsigned int *class,
rc = sata_link_hardreset(link, sata_ehc_deb_timing(&link->eh_context),
deadline, &online, NULL);
- ahci_start_engine(ap);
+ hpriv->start_engine(ap);
/* The pseudo configuration device on SIMG4726 attached to
* ASUS P5W-DH Deluxe doesn't send signature FIS after
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 2289efd..64d1a99 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -323,6 +323,12 @@ struct ahci_host_priv {
u32 em_msg_type; /* EM message type */
struct clk *clk; /* Only for platforms supporting clk */
void *plat_data; /* Other platform data */
+ /*
+ * Optional ahci_start_engine override, if not set this gets set to the
+ * default ahci_start_engine during ahci_save_initial_config, this can
+ * be overridden anytime before the host is activated.
+ */
+ void (*start_engine)(struct ata_port *ap);
};
extern int ahci_ignore_sss;
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 36605ab..f839bb3 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -394,6 +394,9 @@ static ssize_t ahci_show_em_supported(struct device *dev,
*
* If inconsistent, config values are fixed up by this function.
*
+ * If it is not set already this function sets hpriv->start_engine to
+ * ahci_start_engine.
+ *
* LOCKING:
* None.
*/
@@ -500,6 +503,9 @@ void ahci_save_initial_config(struct device *dev,
hpriv->cap = cap;
hpriv->cap2 = cap2;
hpriv->port_map = port_map;
+
+ if (!hpriv->start_engine)
+ hpriv->start_engine = ahci_start_engine;
}
EXPORT_SYMBOL_GPL(ahci_save_initial_config);
@@ -766,7 +772,7 @@ static void ahci_start_port(struct ata_port *ap)
/* enable DMA */
if (!(hpriv->flags & AHCI_HFLAG_DELAY_ENGINE))
- ahci_start_engine(ap);
+ hpriv->start_engine(ap);
/* turn on LEDs */
if (ap->flags & ATA_FLAG_EM) {
@@ -1234,7 +1240,7 @@ int ahci_kick_engine(struct ata_port *ap)
/* restart engine */
out_restart:
- ahci_start_engine(ap);
+ hpriv->start_engine(ap);
return rc;
}
EXPORT_SYMBOL_GPL(ahci_kick_engine);
@@ -1426,6 +1432,7 @@ static int ahci_hardreset(struct ata_link *link, unsigned int *class,
const unsigned long *timing = sata_ehc_deb_timing(&link->eh_context);
struct ata_port *ap = link->ap;
struct ahci_port_priv *pp = ap->private_data;
+ struct ahci_host_priv *hpriv = ap->host->private_data;
u8 *d2h_fis = pp->rx_fis + RX_FIS_D2H_REG;
struct ata_taskfile tf;
bool online;
@@ -1443,7 +1450,7 @@ static int ahci_hardreset(struct ata_link *link, unsigned int *class,
rc = sata_link_hardreset(link, timing, deadline, &online,
ahci_check_ready);
- ahci_start_engine(ap);
+ hpriv->start_engine(ap);
if (online)
*class = ahci_dev_classify(ap);
@@ -2007,10 +2014,12 @@ static void ahci_thaw(struct ata_port *ap)
void ahci_error_handler(struct ata_port *ap)
{
+ struct ahci_host_priv *hpriv = ap->host->private_data;
+
if (!(ap->pflags & ATA_PFLAG_FROZEN)) {
/* restart engine */
ahci_stop_engine(ap);
- ahci_start_engine(ap);
+ hpriv->start_engine(ap);
}
sata_pmp_error_handler(ap);
@@ -2031,6 +2040,7 @@ static void ahci_post_internal_cmd(struct ata_queued_cmd *qc)
static void ahci_set_aggressive_devslp(struct ata_port *ap, bool sleep)
{
+ struct ahci_host_priv *hpriv = ap->host->private_data;
void __iomem *port_mmio = ahci_port_base(ap);
struct ata_device *dev = ap->link.device;
u32 devslp, dm, dito, mdat, deto;
@@ -2094,7 +2104,7 @@ static void ahci_set_aggressive_devslp(struct ata_port *ap, bool sleep)
PORT_DEVSLP_ADSE);
writel(devslp, port_mmio + PORT_DEVSLP);
- ahci_start_engine(ap);
+ hpriv->start_engine(ap);
/* enable device sleep feature for the drive */
err_mask = ata_dev_set_feature(dev,
@@ -2106,6 +2116,7 @@ static void ahci_set_aggressive_devslp(struct ata_port *ap, bool sleep)
static void ahci_enable_fbs(struct ata_port *ap)
{
+ struct ahci_host_priv *hpriv = ap->host->private_data;
struct ahci_port_priv *pp = ap->private_data;
void __iomem *port_mmio = ahci_port_base(ap);
u32 fbs;
@@ -2134,11 +2145,12 @@ static void ahci_enable_fbs(struct ata_port *ap)
} else
dev_err(ap->host->dev, "Failed to enable FBS\n");
- ahci_start_engine(ap);
+ hpriv->start_engine(ap);
}
static void ahci_disable_fbs(struct ata_port *ap)
{
+ struct ahci_host_priv *hpriv = ap->host->private_data;
struct ahci_port_priv *pp = ap->private_data;
void __iomem *port_mmio = ahci_port_base(ap);
u32 fbs;
@@ -2166,7 +2178,7 @@ static void ahci_disable_fbs(struct ata_port *ap)
pp->fbs_enabled = false;
}
- ahci_start_engine(ap);
+ hpriv->start_engine(ap);
}
static void ahci_pmp_attach(struct ata_port *ap)
diff --git a/drivers/ata/sata_highbank.c b/drivers/ata/sata_highbank.c
index 870b11e..b3b18d1 100644
--- a/drivers/ata/sata_highbank.c
+++ b/drivers/ata/sata_highbank.c
@@ -403,6 +403,7 @@ static int ahci_highbank_hardreset(struct ata_link *link, unsigned int *class,
static const unsigned long timing[] = { 5, 100, 500};
struct ata_port *ap = link->ap;
struct ahci_port_priv *pp = ap->private_data;
+ struct ahci_host_priv *hpriv = ap->host->private_data;
u8 *d2h_fis = pp->rx_fis + RX_FIS_D2H_REG;
struct ata_taskfile tf;
bool online;
@@ -431,7 +432,7 @@ static int ahci_highbank_hardreset(struct ata_link *link, unsigned int *class,
break;
} while (!online && retry--);
- ahci_start_engine(ap);
+ hpriv->start_engine(ap);
if (online)
*class = ahci_dev_classify(ap);
--
1.9.0
^ permalink raw reply related
* (unknown),
From: Hans de Goede @ 2014-02-22 15:53 UTC (permalink / raw)
To: Tejun Heo, Maxime Ripard
Cc: Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
Hi all,
Here is v7 of my patchset for adding ahci-sunxi support. This is hopefully
the final final version of this set :)
Note I'm going on vacation for a week starting Monday, so if I'm not responding
that is why. Tejun if you feel some small cleanups are still necessary and
you don't want to wait for me to get back feel free to squash in any cleanups
you deem necessary.
This has been tested with Allwinner A10, Allwinner A20 and Freeware imx6x SoCs,
including suspend / resume. Note that the ahci_imx driver now also has imx53
sata support, it would be good if someone could test that with this series.
History:
v1, by Olliver Schinagl:
This was using the approach of having a platform device which probe method
creates a new child platform device which gets driven by ahci_platform.c,
as done by ahci_imx.c .
v2, by Hans de Goede:
Stand-alone platform driver based on Olliver's work
v3, by Hans de Goede:
patch-series, with 4 different parts
a) Make ahci_platform.c more generic, handle more then 1 clk, target pwr
regulator
b) New ahci-sunxi code only populating ahci_platform_data, passed to
ahci_platform.c to of_device_id matching.
c) Refactor ahci-imx code to work the same as the new ahci-sunxi code, this
is the reason why v3 is an RFC, I'm waiting for the wandboard I ordered to
arrive so that I can actually test this.
d) dts bindings for the sunxi ahci parts
v4, by Hans de Goede:
patch-series, with 5 different parts:
a) Make ahci_platform.c more generic, handle more then 1 clk, target pwr
regulator
b) Turn parts of ahci_platform.c into a library for use by other drivers
c) New ahci-sunxi driver using the ahci_platform.c library functionality
d) Refactor ahci-imx code to work the same as the new ahci-sunxi code
e) dts bindings for the sunxi ahci parts
v5:
v4 + the following changes:
1) fsl,imx6q driver is now tested
2) fixed suspend / resume on fsl,imx6q
3) Modifed devicetree node naming to match dt spec
4) Reworked the busy waiting code in the sunxi-phy handling as suggested by
Russell King
v6:
v5 rebased on top of 3.14-rc3 + the following changes
1) Added Roger Quadros' generic phy support series
2) Added a "ARM: sun4i: dt: Remove grouping + simple-bus for regulators" dts
patch
v7:
v6 + the following changes:
1) Addressed all Tejun's review remarks:
* Added function header comments to all exported ahci_platform functions
* Added comments in some other places
* Removed use of 2 empty lines to separate functions in some cases
* Use devres to automatically call ahci_platform_put_resources on
get_resource failure, probe failure and regular device remove
2) Dropped patches to move ahci_host_priv struct declaration to include/linux,
this was a left-over from v3 and is no longer necessary
3) Updated Roger's "ata: ahci_platform: Manage SATA PHY" patch:
* Update function header comments for the changes this makes
* Drop the Kconfig PHY requires hack, my patch for the phy-core to always be
built-in has been queued in Greg KH's tree, so this is no longer necessary.
4) Dropped Roger's "ata: ahci_platform: Add 'struct device' argument to ahci_platform_put_resources()"
patch, ahci_platform_put_resources already has a device argument as result
of it being changed into a devres release function
Tejun, can you please add patches 1-12 to your ata tree for 3.15 ?
Maxime, can you please add patch 13-15 to your dts tree for 3.15 ?
Thanks & Regards,
Hans
^ permalink raw reply
* Re: [PATCH v7 0/2] AS3935 lightning sensor support
From: Jonathan Cameron @ 2014-02-22 12:50 UTC (permalink / raw)
To: Matt Ranostay, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: matt.porter-QSEj5FYQhm4dnm+yROfE0A,
pantelis.antoniou-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1392179475-2611-1-git-send-email-mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Just a quick note to say I'm hold off on this in the hope for some
feedback on my suggested interface for position devices that I sent
in response to the previous version. I think lumping these under
proximity devices is the wrong approach as that doesn't generalize well.
On 12/02/14 04:31, Matt Ranostay wrote:
> This series adds support for the AMS AS3935 lightning sensor that allows
> reporting back estimated storm distance and strike events.
>
> Chagges from v6
>
> * Revised binding documents to not use the term "interrupts mapping"
> * Renamed tune-cap property to a more clear tuning-capacitor-pf
>
> Changes from v5
>
> * SPI write cache-aligned issues fixed
> * Fixed mutex_unlock's being missed
> * Reports distance in meters instead of kilometers (1km steps)
> * tune_cap is now in picofarads and not a register value
>
> Matt Ranostay (2):
> iio:as3935: Add DT binding docs for AS3935 driver
> iio: Add AS3935 lightning sensor support
>
> .../ABI/testing/sysfs-bus-iio-proximity-as3935 | 18 +
> .../devicetree/bindings/iio/proximity/as3935.txt | 28 ++
> .../devicetree/bindings/vendor-prefixes.txt | 1 +
> drivers/iio/Kconfig | 1 +
> drivers/iio/Makefile | 1 +
> drivers/iio/proximity/Kconfig | 19 +
> drivers/iio/proximity/Makefile | 6 +
> drivers/iio/proximity/as3935.c | 446 +++++++++++++++++++++
> 8 files changed, 520 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
> create mode 100644 Documentation/devicetree/bindings/iio/proximity/as3935.txt
> create mode 100644 drivers/iio/proximity/Kconfig
> create mode 100644 drivers/iio/proximity/Makefile
> create mode 100644 drivers/iio/proximity/as3935.c
>
^ permalink raw reply
* Re: [RFCv1 4/4] mfd: twl4030-madc: Move driver to drivers/iio/adc
From: Jonathan Cameron @ 2014-02-22 12:47 UTC (permalink / raw)
To: Sebastian Reichel, Sebastian Reichel, Marek Belisko
Cc: Lee Jones, Samuel Ortiz, Lars-Peter Clausen, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Grant Likely,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1392403586-30540-4-git-send-email-sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
On 14/02/14 18:46, Sebastian Reichel wrote:
> This is a driver for an A/D converter, which belongs into
> drivers/iio/adc.
>
> Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
Being inherently lazy I'm going to review this patch as it gives the complete
driver rather than taking on the conversion patch directly!
It's a long shot, but are there any docs freely available for this part?
Obviously that may well mean that some of my comments apply to the driver
in general rather than the changes you've made. Please feel free to
pick and choose!
I'd like ideally to see a little more generic tidying up in this driver.
As you'll notice inline I get irritated at having lots and lots of error
messages. Still that's personal preference but I felt a lot more justified
in moaning after finding one that was incorrect ;)
There is also a bit of functionality in here that I'm not sure is ever
used (the request callback functions). It complicates the code so if no
using it I'd be tempted to drop it.
It's nice in a way that the driver before your conversion provided only
a very limited and in kernel interface given we don't have to hang on to
any old ABI elements.
> ---
> drivers/iio/adc/Kconfig | 10 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/twl4030-madc.c | 922 +++++++++++++++++++++++++++++++++++++++++
> drivers/mfd/Kconfig | 10 -
> drivers/mfd/Makefile | 1 -
> drivers/mfd/twl4030-madc.c | 922 -----------------------------------------
> 6 files changed, 933 insertions(+), 933 deletions(-)
> create mode 100644 drivers/iio/adc/twl4030-madc.c
> delete mode 100644 drivers/mfd/twl4030-madc.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 2209f28..427f75c 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -183,6 +183,16 @@ config TI_AM335X_ADC
> Say yes here to build support for Texas Instruments ADC
> driver which is also a MFD client.
>
> +config TWL4030_MADC
> + tristate "TWL4030 MADC (Monitoring A/D Converter)"
> + depends on TWL4030_CORE
> + help
> + This driver provides support for Triton TWL4030-MADC. The
> + driver supports both RT and SW conversion methods.
> +
> + This driver can also be built as a module. If so, the module will be
> + called twl4030-madc.
> +
> config TWL6030_GPADC
> tristate "TWL6030 GPADC (General Purpose A/D Converter) Support"
> depends on TWL4030_CORE
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index ba9a10a..9acf2df 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -20,5 +20,6 @@ obj-$(CONFIG_MCP3422) += mcp3422.o
> obj-$(CONFIG_NAU7802) += nau7802.o
> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
> +obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
> obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
> obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
> diff --git a/drivers/iio/adc/twl4030-madc.c b/drivers/iio/adc/twl4030-madc.c
> new file mode 100644
> index 0000000..4da61c4
> --- /dev/null
> +++ b/drivers/iio/adc/twl4030-madc.c
> @@ -0,0 +1,922 @@
> +/*
> + *
> + * TWL4030 MADC module driver-This driver monitors the real time
> + * conversion of analog signals like battery temperature,
> + * battery type, battery level etc.
> + *
> + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/
> + * J Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
> + *
> + * Based on twl4030-madc.c
> + * Copyright (C) 2008 Nokia Corporation
> + * Mikko Ylinen <mikko.k.ylinen-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> + *
> + * Amit Kucheria <amit.kucheria-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110-1301 USA
> + *
> + */
> +
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/delay.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/i2c/twl.h>
> +#include <linux/i2c/twl4030-madc.h>
> +#include <linux/module.h>
> +#include <linux/stddef.h>
> +#include <linux/mutex.h>
> +#include <linux/bitops.h>
> +#include <linux/jiffies.h>
> +#include <linux/types.h>
> +#include <linux/gfp.h>
> +#include <linux/err.h>
> +
> +#include <linux/iio/iio.h>
Why include machine.h or driver.h. You aren't currently using the
mapping coding anyway that these provide.
> +#include <linux/iio/machine.h>
> +#include <linux/iio/driver.h>
> +
> +/*
> + * struct twl4030_madc_data - a container for madc info
> + * @dev - pointer to device structure for madc
> + * @lock - mutex protecting this data structure
> + * @requests - Array of request struct corresponding to SW1, SW2 and RT
> + * @imr - Interrupt mask register of MADC
> + * @isr - Interrupt status register of MADC
> + */
> +struct twl4030_madc_data {
> + struct device *dev;
> + struct mutex lock; /* mutex protecting this data structure */
> + struct twl4030_madc_request requests[TWL4030_MADC_NUM_METHODS];
> + bool use_second_irq;
These are 32 bit signed values?
> + int imr;
> + int isr;
> +};
> +
> +static int twl4030_madc_read(struct iio_dev *iio_dev,
> + const struct iio_chan_spec *chan,
> + int *val, int *val2, long mask)
> +{
> + struct twl4030_madc_data *madc = iio_priv(iio_dev);
> + struct twl4030_madc_request req;
> + int channel = chan->channel;
> + int ret;
> +
> + req.method = madc->use_second_irq ? TWL4030_MADC_SW2 : TWL4030_MADC_SW1;
> +
> + req.channels = BIT(channel);
> + req.active = 0;
> + req.func_cb = NULL;
req.raw = !(mask & IIO_CHAN_INFO_PROCESSED);
req.do_avg = !!(mask & IIO_CHAN_INFO_AVERAGE_RAW);
> + req.raw = (mask & IIO_CHAN_INFO_PROCESSED) ? false : true;
> + req.do_avg = (mask & IIO_CHAN_INFO_AVERAGE_RAW) ? true : false;
> +
> + ret = twl4030_madc_conversion(&req);
> + if (ret < 0)
> + return ret;
> +
> + *val = req.rbuf[channel];
> +
> + return IIO_VAL_INT;
> +}
> +
> +static const struct iio_info twl4030_madc_iio_info = {
> + .read_raw = &twl4030_madc_read,
> + .driver_module = THIS_MODULE,
> +};
> +
Please give this a prefixed name. Chances of ADC_CHANNEL
turning up in an IIO header is a little too high to do this
without! e.g. #define TWL4030_ADC_CHANNEL
> +#define ADC_CHANNEL(_channel, _type, _name, _mask) { \
> + .type = _type, \
I don't think you use scan_type anywhere so don't bother specifying it.
here.
> + .scan_type = IIO_ST('u', 10, 16, 0), \
> + .channel = _channel, \
> + .info_mask_separate = _mask, \
> + .datasheet_name = _name, \
> + .indexed = 1, \
> +}
> +
Is the average have an adjustable number of samples?
If not I'd be tempted to use the more common option of IIO_CHAN_INFO_RAW
rather than the average version (tends only to be used in fairly obscure
cases where both a raw access and an averaged one are available).
> +static const struct iio_chan_spec twl4030_madc_iio_channels[] = {
> + ADC_CHANNEL(0, IIO_VOLTAGE, "ADCIN0", BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> + ADC_CHANNEL(1, IIO_TEMP, "ADCIN1", BIT(IIO_CHAN_INFO_PROCESSED) |
> + BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> + ADC_CHANNEL(2, IIO_VOLTAGE, "ADCIN2", BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> + ADC_CHANNEL(3, IIO_VOLTAGE, "ADCIN3", BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> + ADC_CHANNEL(4, IIO_VOLTAGE, "ADCIN4", BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> + ADC_CHANNEL(5, IIO_VOLTAGE, "ADCIN5", BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> + ADC_CHANNEL(6, IIO_VOLTAGE, "ADCIN6", BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> + ADC_CHANNEL(7, IIO_VOLTAGE, "ADCIN7", BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> + ADC_CHANNEL(8, IIO_VOLTAGE, "ADCIN8", BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> + ADC_CHANNEL(9, IIO_VOLTAGE, "ADCIN9", BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> + ADC_CHANNEL(10, IIO_CURRENT, "ADCIN10", BIT(IIO_CHAN_INFO_PROCESSED) |
> + BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> + ADC_CHANNEL(11, IIO_VOLTAGE, "ADCIN11", BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> + ADC_CHANNEL(12, IIO_VOLTAGE, "ADCIN12", BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> + ADC_CHANNEL(13, IIO_VOLTAGE, "ADCIN13", BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> + ADC_CHANNEL(14, IIO_VOLTAGE, "ADCIN14", BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> + ADC_CHANNEL(15, IIO_VOLTAGE, "ADCIN15", BIT(IIO_CHAN_INFO_AVERAGE_RAW)),
> +};
> +
Why the artificial limitation of one of these devices? I guess this is to
allow the exported function to work without needing to be associated with
any particular device... Hmm.
> +static struct twl4030_madc_data *twl4030_madc;
> +
> +struct twl4030_prescale_divider_ratios {
> + s16 numerator;
> + s16 denominator;
> +};
> +
> +static const struct twl4030_prescale_divider_ratios
> +twl4030_divider_ratios[16] = {
> + {1, 1}, /* CHANNEL 0 No Prescaler */
> + {1, 1}, /* CHANNEL 1 No Prescaler */
> + {6, 10}, /* CHANNEL 2 */
> + {6, 10}, /* CHANNEL 3 */
> + {6, 10}, /* CHANNEL 4 */
> + {6, 10}, /* CHANNEL 5 */
> + {6, 10}, /* CHANNEL 6 */
> + {6, 10}, /* CHANNEL 7 */
> + {3, 14}, /* CHANNEL 8 */
> + {1, 3}, /* CHANNEL 9 */
> + {1, 1}, /* CHANNEL 10 No Prescaler */
> + {15, 100}, /* CHANNEL 11 */
> + {1, 4}, /* CHANNEL 12 */
> + {1, 1}, /* CHANNEL 13 Reserved channels */
> + {1, 1}, /* CHANNEL 14 Reseved channels */
> + {5, 11}, /* CHANNEL 15 */
> +};
> +
> +
> +/*
> + * Conversion table from -3 to 55 degree Celcius
> + */
> +static int therm_tbl[] = {
> +30800, 29500, 28300, 27100,
> +26000, 24900, 23900, 22900, 22000, 21100, 20300, 19400, 18700, 17900,
> +17200, 16500, 15900, 15300, 14700, 14100, 13600, 13100, 12600, 12100,
> +11600, 11200, 10800, 10400, 10000, 9630, 9280, 8950, 8620, 8310,
> +8020, 7730, 7460, 7200, 6950, 6710, 6470, 6250, 6040, 5830,
> +5640, 5450, 5260, 5090, 4920, 4760, 4600, 4450, 4310, 4170,
> +4040, 3910, 3790, 3670, 3550
> +};
> +
> +/*
> + * Structure containing the registers
> + * of different conversion methods supported by MADC.
> + * Hardware or RT real time conversion request initiated by external host
> + * processor for RT Signal conversions.
> + * External host processors can also request for non RT conversions
> + * SW1 and SW2 software conversions also called asynchronous or GPC request.
> + */
> +static
> +const struct twl4030_madc_conversion_method twl4030_conversion_methods[] = {
> + [TWL4030_MADC_RT] = {
> + .sel = TWL4030_MADC_RTSELECT_LSB,
> + .avg = TWL4030_MADC_RTAVERAGE_LSB,
> + .rbase = TWL4030_MADC_RTCH0_LSB,
> + },
> + [TWL4030_MADC_SW1] = {
> + .sel = TWL4030_MADC_SW1SELECT_LSB,
> + .avg = TWL4030_MADC_SW1AVERAGE_LSB,
> + .rbase = TWL4030_MADC_GPCH0_LSB,
> + .ctrl = TWL4030_MADC_CTRL_SW1,
> + },
> + [TWL4030_MADC_SW2] = {
> + .sel = TWL4030_MADC_SW2SELECT_LSB,
> + .avg = TWL4030_MADC_SW2AVERAGE_LSB,
> + .rbase = TWL4030_MADC_GPCH0_LSB,
> + .ctrl = TWL4030_MADC_CTRL_SW2,
> + },
> +};
> +
> +/*
> + * Function to read a particular channel value.
> + * @madc - pointer to struct twl4030_madc_data
> + * @reg - lsb of ADC Channel
> + * If the i2c read fails it returns an error else returns 0.
> + */
> +static int twl4030_madc_channel_raw_read(struct twl4030_madc_data *madc, u8 reg)
> +{
> + u8 msb, lsb;
> + int ret;
> + /*
> + * For each ADC channel, we have MSB and LSB register pair. MSB address
> + * is always LSB address+1. reg parameter is the address of LSB register
> + */
> + ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, &msb, reg + 1);
> + if (ret) {
> + dev_err(madc->dev, "unable to read MSB register 0x%X\n",
> + reg + 1);
> + return ret;
> + }
> + ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, &lsb, reg);
> + if (ret) {
> + dev_err(madc->dev, "unable to read LSB register 0x%X\n", reg);
> + return ret;
> + }
> +
> + return (int)(((msb << 8) | lsb) >> 6);
> +}
> +
> +/*
> + * Return battery temperature
> + * Or < 0 on failure.
> + */
> +static int twl4030battery_temperature(int raw_volt)
> +{
> + u8 val;
> + int temp, curr, volt, res, ret;
> +
> + volt = (raw_volt * TEMP_STEP_SIZE) / TEMP_PSR_R;
> + /* Getting and calculating the supply current in micro ampers */
> + ret = twl_i2c_read_u8(TWL_MODULE_MAIN_CHARGE, &val,
> + REG_BCICTL2);
> + if (ret < 0)
> + return ret;
blank line here and after similar error cases makes it a little easier to
read.
> + curr = ((val & TWL4030_BCI_ITHEN) + 1) * 10;
> + /* Getting and calculating the thermistor resistance in ohms */
> + res = volt * 1000 / curr;
> + /* calculating temperature */
> + for (temp = 58; temp >= 0; temp--) {
> + int actual = therm_tbl[temp];
> +
No blank line here.
> + if ((actual - res) >= 0)
> + break;
> + }
> +
> + return temp + 1;
> +}
> +
> +static int twl4030battery_current(int raw_volt)
> +{
> + int ret;
> + u8 val;
> +
> + ret = twl_i2c_read_u8(TWL_MODULE_MAIN_CHARGE, &val,
> + TWL4030_BCI_BCICTL1);
> + if (ret)
> + return ret;
> + if (val & TWL4030_BCI_CGAIN) /* slope of 0.44 mV/mA */
> + return (raw_volt * CURR_STEP_SIZE) / CURR_PSR_R1;
> + else /* slope of 0.88 mV/mA */
> + return (raw_volt * CURR_STEP_SIZE) / CURR_PSR_R2;
> +}
blank line here please.
Various comments on this in the next function:
This would be much simpler if any error immediately resulted in an
exit with error code. If it's gone wrong enough to issue a dev_err
then don't try to muddle through. Given there is nothing to clear
up in here, just error out directly on the first problem.
> +/*
> + * Function to read channel values
> + * @madc - pointer to twl4030_madc_data struct
> + * @reg_base - Base address of the first channel
> + * @Channels - 16 bit bitmap. If the bit is set, channel value is read
> + * @buf - The channel values are stored here. if read fails error
> + * @raw - Return raw values without conversion
> + * value is stored
> + * Returns the number of successfully read channels.
> + */
> +static int twl4030_madc_read_channels(struct twl4030_madc_data *madc,
> + u8 reg_base, unsigned
> + long channels, int *buf,
> + bool raw)
> +{
> + int count = 0, count_req = 0, i;
> + u8 reg;
> +
> + for_each_set_bit(i, &channels, TWL4030_MADC_MAX_CHANNELS) {
Skip the temporary for reg. Yes, it gets used twice, but easier to read
inline.
> + reg = reg_base + 2 * i;
> + buf[i] = twl4030_madc_channel_raw_read(madc, reg);
> + if (buf[i] < 0) {
> + dev_err(madc->dev,
> + "Unable to read register 0x%X\n", reg);
> + count_req++;
> + continue;
> + }
I'd prefer this as
if (raw) {
count ++;
} else {
switch (i) {
..
}
Also near as I can tell count gets incremented unless there is an error
anyway. So just increment it outside and allow this function to return the
error code.
> + if (raw) {
> + count++;
> + continue;
> + }
> + switch (i) {
These cases could do with a suitable #define to give them a name.
> + case 10:
> + buf[i] = twl4030battery_current(buf[i]);
> + if (buf[i] < 0) {
> + dev_err(madc->dev, "err reading current\n");
> + count_req++;
> + } else {
> + count++;
> + buf[i] = buf[i] - 750;
> + }
> + break;
> + case 1:
> + buf[i] = twl4030battery_temperature(buf[i]);
> + if (buf[i] < 0) {
> + dev_err(madc->dev, "err reading temperature\n");
> + count_req++;
> + } else {
> + buf[i] -= 3;
> + count++;
> + }
> + break;
> + default:
> + count++;
> + /* Analog Input (V) = conv_result * step_size / R
> + * conv_result = decimal value of 10-bit conversion
> + * result
> + * step size = 1.5 / (2 ^ 10 -1)
> + * R = Prescaler ratio for input channels.
> + * Result given in mV hence multiplied by 1000.
> + */
> + buf[i] = (buf[i] * 3 * 1000 *
> + twl4030_divider_ratios[i].denominator)
> + / (2 * 1023 *
> + twl4030_divider_ratios[i].numerator);
> + }
> + }
This is already apparant from the errors emmited earlier. I'd drop this
and the count_req counter in general.
Personally I'd error out in those cases anyway rather than carrying on
with the rest of the channels. If it's worth of a dev_err it's worthy
of getting out as fast as possible rather than trying to muddle on.
> + if (count_req)
> + dev_err(madc->dev, "%d channel conversion failed\n", count_req);
> +
> + return count;
> +}
> +
> +/*
> + * Enables irq.
> + * @madc - pointer to twl4030_madc_data struct
> + * @id - irq number to be enabled
> + * can take one of TWL4030_MADC_RT, TWL4030_MADC_SW1, TWL4030_MADC_SW2
> + * corresponding to RT, SW1, SW2 conversion requests.
> + * If the i2c read fails it returns an error else returns 0.
> + */
> +static int twl4030_madc_enable_irq(struct twl4030_madc_data *madc, u8 id)
> +{
> + u8 val;
> + int ret;
> +
> + ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, &val, madc->imr);
> + if (ret) {
> + dev_err(madc->dev, "unable to read imr register 0x%X\n",
> + madc->imr);
> + return ret;
> + }
A blank line here and in similar cases would slight aid readability.
> + val &= ~(1 << id);
> + ret = twl_i2c_write_u8(TWL4030_MODULE_MADC, val, madc->imr);
> + if (ret) {
> + dev_err(madc->dev,
> + "unable to write imr register 0x%X\n", madc->imr);
> + return ret;
> +
Loose blank line here.
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Disables irq.
> + * @madc - pointer to twl4030_madc_data struct
> + * @id - irq number to be disabled
> + * can take one of TWL4030_MADC_RT, TWL4030_MADC_SW1, TWL4030_MADC_SW2
> + * corresponding to RT, SW1, SW2 conversion requests.
> + * Returns error if i2c read/write fails.
> + */
> +static int twl4030_madc_disable_irq(struct twl4030_madc_data *madc, u8 id)
> +{
> + u8 val;
> + int ret;
> +
> + ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, &val, madc->imr);
> + if (ret) {
> + dev_err(madc->dev, "unable to read imr register 0x%X\n",
> + madc->imr);
> + return ret;
> + }
> + val |= (1 << id);
> + ret = twl_i2c_write_u8(TWL4030_MODULE_MADC, val, madc->imr);
> + if (ret) {
> + dev_err(madc->dev,
> + "unable to write imr register 0x%X\n", madc->imr);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static irqreturn_t twl4030_madc_threaded_irq_handler(int irq, void *_madc)
> +{
> + struct twl4030_madc_data *madc = _madc;
> + const struct twl4030_madc_conversion_method *method;
> + u8 isr_val, imr_val;
> + int i, len, ret;
> + struct twl4030_madc_request *r;
> +
> + mutex_lock(&madc->lock);
> + ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, &isr_val, madc->isr);
> + if (ret) {
> + dev_err(madc->dev, "unable to read isr register 0x%X\n",
> + madc->isr);
> + goto err_i2c;
> + }
> + ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, &imr_val, madc->imr);
> + if (ret) {
> + dev_err(madc->dev, "unable to read imr register 0x%X\n",
> + madc->imr);
> + goto err_i2c;
> + }
> + isr_val &= ~imr_val;
> + for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
> + if (!(isr_val & (1 << i)))
> + continue;
> + ret = twl4030_madc_disable_irq(madc, i);
> + if (ret < 0)
> + dev_dbg(madc->dev, "Disable interrupt failed%d\n", i);
> + madc->requests[i].result_pending = 1;
> + }
Can this and the previous loop not be combined thus simplifying some tests?
> + for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
> + r = &madc->requests[i];
> + /* No pending results for this method, move to next one */
> + if (!r->result_pending)
> + continue;
> + method = &twl4030_conversion_methods[r->method];
> + /* Read results */
> + len = twl4030_madc_read_channels(madc, method->rbase,
> + r->channels, r->rbuf, r->raw);
> + /* Return results to caller */
> + if (r->func_cb != NULL) {
> + r->func_cb(len, r->channels, r->rbuf);
> + r->func_cb = NULL;
> + }
> + /* Free request */
> + r->result_pending = 0;
> + r->active = 0;
> + }
> + mutex_unlock(&madc->lock);
> +
> + return IRQ_HANDLED;
> +
> +err_i2c:
> + /*
> + * In case of error check whichever request is active
> + * and service the same.
> + */
> + for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
> + r = &madc->requests[i];
> + if (r->active == 0)
> + continue;
> + method = &twl4030_conversion_methods[r->method];
> + /* Read results */
> + len = twl4030_madc_read_channels(madc, method->rbase,
> + r->channels, r->rbuf, r->raw);
> + /* Return results to caller */
> + if (r->func_cb != NULL) {
> + r->func_cb(len, r->channels, r->rbuf);
> + r->func_cb = NULL;
> + }
> + /* Free request */
> + r->result_pending = 0;
> + r->active = 0;
> + }
> + mutex_unlock(&madc->lock);
> +
> + return IRQ_HANDLED;
> +}
> +
This structure does seem a little complex. I'd have been more tempted
by using a completion and having the calling function wait on it. How
often does an actual callback make sense?
> +static int twl4030_madc_set_irq(struct twl4030_madc_data *madc,
> + struct twl4030_madc_request *req)
> +{
> + struct twl4030_madc_request *p;
> + int ret;
> +
> + p = &madc->requests[req->method];
> + memcpy(p, req, sizeof(*req));
> + ret = twl4030_madc_enable_irq(madc, req->method);
> + if (ret < 0) {
> + dev_err(madc->dev, "enable irq failed!!\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Function which enables the madc conversion
> + * by writing to the control register.
> + * @madc - pointer to twl4030_madc_data struct
> + * @conv_method - can be TWL4030_MADC_RT, TWL4030_MADC_SW2, TWL4030_MADC_SW1
> + * corresponding to RT SW1 or SW2 conversion methods.
> + * Returns 0 if succeeds else a negative error value
> + */
> +static int twl4030_madc_start_conversion(struct twl4030_madc_data *madc,
> + int conv_method)
> +{
> + const struct twl4030_madc_conversion_method *method;
> + int ret = 0;
> + method = &twl4030_conversion_methods[conv_method];
> + switch (conv_method) {
Can we get here via any paths where these aren't the methods set?
> + case TWL4030_MADC_SW1:
> + case TWL4030_MADC_SW2:
> + ret = twl_i2c_write_u8(TWL4030_MODULE_MADC,
> + TWL4030_MADC_SW_START, method->ctrl);
> + if (ret) {
> + dev_err(madc->dev,
> + "unable to write ctrl register 0x%X\n",
> + method->ctrl);
> + return ret;
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
Could we fix up the kernel doc in this file in general. It's so nearly
there but not quite!
> +/*
> + * Function that waits for conversion to be ready
> + * @madc - pointer to twl4030_madc_data struct
> + * @timeout_ms - timeout value in milliseconds
> + * @status_reg - ctrl register
> + * returns 0 if succeeds else a negative error value
> + */
> +static int twl4030_madc_wait_conversion_ready(struct twl4030_madc_data *madc,
> + unsigned int timeout_ms,
> + u8 status_reg)
> +{
> + unsigned long timeout;
> + int ret;
> +
> + timeout = jiffies + msecs_to_jiffies(timeout_ms);
> + do {
> + u8 reg;
> +
> + ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, ®, status_reg);
> + if (ret) {
> + dev_err(madc->dev,
> + "unable to read status register 0x%X\n",
> + status_reg);
> + return ret;
> + }
> + if (!(reg & TWL4030_MADC_BUSY) && (reg & TWL4030_MADC_EOC_SW))
> + return 0;
> + usleep_range(500, 2000);
> + } while (!time_after(jiffies, timeout));
> + dev_err(madc->dev, "conversion timeout!\n");
> +
> + return -EAGAIN;
> +}
> +
> +/*
> + * An exported function which can be called from other kernel drivers.
What uses this currently? Ideally we'd do this through the standard IIO paths
for in kernel users. Right now the device tree mappings for that are less
than ideal (ask Mark Rutland if you want some choice comments ;) Perhaps
we take the view this can be cleaned up later. There are some elements in here
we haven't yet looked at supporting for client drivers. Will have to think
about that.
It would be particularly nice if possible to use a generic battery driver
for the two cases that seem to be using this functionality at the moment.
> + * @req twl4030_madc_request structure
> + * req->rbuf will be filled with read values of channels based on the
> + * channel index. If a particular channel reading fails there will
> + * be a negative error value in the corresponding array element.
> + * returns 0 if succeeds else error value
> + */
> +int twl4030_madc_conversion(struct twl4030_madc_request *req)
> +{
> + const struct twl4030_madc_conversion_method *method;
> + u8 ch_msb, ch_lsb;
> + int ret;
> +
> + if (!req || !twl4030_madc)
> + return -EINVAL;
> +
> + mutex_lock(&twl4030_madc->lock);
> + if (req->method < TWL4030_MADC_RT || req->method > TWL4030_MADC_SW2) {
> + ret = -EINVAL;
> + goto out;
> + }
> + /* Do we have a conversion request ongoing */
> + if (twl4030_madc->requests[req->method].active) {
> + ret = -EBUSY;
> + goto out;
> + }
> + ch_msb = (req->channels >> 8) & 0xff;
> + ch_lsb = req->channels & 0xff;
> + method = &twl4030_conversion_methods[req->method];
> + /* Select channels to be converted */
> + ret = twl_i2c_write_u8(TWL4030_MODULE_MADC, ch_msb, method->sel + 1);
> + if (ret) {
> + dev_err(twl4030_madc->dev,
> + "unable to write sel register 0x%X\n", method->sel + 1);
> + goto out;
> + }
> + ret = twl_i2c_write_u8(TWL4030_MODULE_MADC, ch_lsb, method->sel);
> + if (ret) {
> + dev_err(twl4030_madc->dev,
> + "unable to write sel register 0x%X\n", method->sel + 1);
> + goto out;
> + }
> + /* Select averaging for all channels if do_avg is set */
> + if (req->do_avg) {
> + ret = twl_i2c_write_u8(TWL4030_MODULE_MADC,
> + ch_msb, method->avg + 1);
> + if (ret) {
> + dev_err(twl4030_madc->dev,
> + "unable to write avg register 0x%X\n",
> + method->avg + 1);
> + goto out;
> + }
> + ret = twl_i2c_write_u8(TWL4030_MODULE_MADC,
> + ch_lsb, method->avg);
> + if (ret) {
> + dev_err(twl4030_madc->dev,
Excesive error messages are sometimes irritating as they break up code
flow. They are really irritating when incorrect ;)
> + "unable to write sel reg 0x%X\n",
> + method->sel + 1);
> + goto out;
> + }
> + }
> + if (req->type == TWL4030_MADC_IRQ_ONESHOT && req->func_cb != NULL) {
> + ret = twl4030_madc_set_irq(twl4030_madc, req);
> + if (ret < 0)
> + goto out;
> + ret = twl4030_madc_start_conversion(twl4030_madc, req->method);
> + if (ret < 0)
> + goto out;
> + twl4030_madc->requests[req->method].active = 1;
> + ret = 0;
> + goto out;
> + }
Is it possible to get here? This comment suggests not. If so, why have
the test?
> + /* With RT method we should not be here anymore */
> + if (req->method == TWL4030_MADC_RT) {
> + ret = -EINVAL;
> + goto out;
> + }
> + ret = twl4030_madc_start_conversion(twl4030_madc, req->method);
> + if (ret < 0)
> + goto out;
> + twl4030_madc->requests[req->method].active = 1;
> + /* Wait until conversion is ready (ctrl register returns EOC) */
So no interrupts in this case?
> + ret = twl4030_madc_wait_conversion_ready(twl4030_madc, 5, method->ctrl);
> + if (ret) {
> + twl4030_madc->requests[req->method].active = 0;
> + goto out;
> + }
> + ret = twl4030_madc_read_channels(twl4030_madc, method->rbase,
> + req->channels, req->rbuf, req->raw);
> + twl4030_madc->requests[req->method].active = 0;
> +
> +out:
> + mutex_unlock(&twl4030_madc->lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(twl4030_madc_conversion);
> +
> +/*
Proper kerneldoc would be nice here. Otherwise the comment doesn't
really add anything so I'd be tempted to drop it ;)
> + * Return channel value
> + * Or < 0 on failure.
> + */
> +int twl4030_get_madc_conversion(int channel_no)
> +{
> + struct twl4030_madc_request req;
> + int temp = 0;
> + int ret;
> +
> + req.channels = (1 << channel_no);
> + req.method = TWL4030_MADC_SW2;
> + req.active = 0;
> + req.func_cb = NULL;
> + ret = twl4030_madc_conversion(&req);
> + if (ret < 0)
> + return ret;
> + if (req.rbuf[channel_no] > 0)
> + temp = req.rbuf[channel_no];
> +
> + return temp;
> +}
> +EXPORT_SYMBOL_GPL(twl4030_get_madc_conversion);
> +
> +/*
> + * Function to enable or disable bias current for
> + * main battery type reading or temperature sensing
> + * @madc - pointer to twl4030_madc_data struct
> + * @chan - can be one of the two values
What is a battery type reading? Voltage? Current? Charge?
> + * TWL4030_BCI_ITHEN - Enables bias current for main battery type reading
> + * TWL4030_BCI_TYPEN - Enables bias current for main battery temperature
These constants have rather incomprehensible names. Whilst I gues they
are straight off the data sheet, seems to me that we could make them a little
longer and easier to follow! The comment here helps, but sensible names
would be better!
> + * sensing
> + * @on - enable or disable chan.
> + */
> +static int twl4030_madc_set_current_generator(struct twl4030_madc_data *madc,
> + int chan, int on)
> +{
> + int ret;
> + u8 regval;
> +
> + ret = twl_i2c_read_u8(TWL_MODULE_MAIN_CHARGE,
> + ®val, TWL4030_BCI_BCICTL1);
> + if (ret) {
> + dev_err(madc->dev, "unable to read BCICTL1 reg 0x%X",
> + TWL4030_BCI_BCICTL1);
> + return ret;
> + }
Introduce an intermediate variable and this could be nice and easy to read
Say
int regmask;
if (chan == 0)
regmask = TWL4030_BCI_TYPEN;
else
regmask = TWL4030_BFI_ITHEN;
if (on)
regval |= regmask;
else
regval &= ~regmask;
It's longer but doesn't give me a headache.
> + if (on)
> + regval |= chan ? TWL4030_BCI_ITHEN : TWL4030_BCI_TYPEN;
> + else
> + regval &= chan ? ~TWL4030_BCI_ITHEN : ~TWL4030_BCI_TYPEN;
> + ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE,
> + regval, TWL4030_BCI_BCICTL1);
> + if (ret) {
> + dev_err(madc->dev, "unable to write BCICTL1 reg 0x%X\n",
> + TWL4030_BCI_BCICTL1);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Function that sets MADC software power on bit to enable MADC
> + * @madc - pointer to twl4030_madc_data struct
> + * @on - Enable or disable MADC software powen on bit.
> + * returns error if i2c read/write fails else 0
> + */
> +static int twl4030_madc_set_power(struct twl4030_madc_data *madc, int on)
> +{
> + u8 regval;
> + int ret;
> +
> + ret = twl_i2c_read_u8(TWL_MODULE_MAIN_CHARGE,
> + ®val, TWL4030_MADC_CTRL1);
> + if (ret) {
> + dev_err(madc->dev, "unable to read madc ctrl1 reg 0x%X\n",
> + TWL4030_MADC_CTRL1);
> + return ret;
> + }
> + if (on)
> + regval |= TWL4030_MADC_MADCON;
> + else
> + regval &= ~TWL4030_MADC_MADCON;
> + ret = twl_i2c_write_u8(TWL4030_MODULE_MADC, regval, TWL4030_MADC_CTRL1);
> + if (ret) {
> + dev_err(madc->dev, "unable to write madc ctrl1 reg 0x%X\n",
> + TWL4030_MADC_CTRL1);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Initialize MADC and request for threaded irq
> + */
> +static int twl4030_madc_probe(struct platform_device *pdev)
> +{
> + struct twl4030_madc_data *madc;
> + struct twl4030_madc_platform_data *pdata = dev_get_platdata(&pdev->dev);
> + struct device_node *np = pdev->dev.of_node;
> + int irq, ret;
> + u8 regval;
> + struct iio_dev *iio_dev = NULL;
> +
> + if (!pdata && !np) {
> + dev_err(&pdev->dev, "platform_data not available\n");
> + return -EINVAL;
> + }
> +
> + iio_dev = devm_iio_device_alloc(&pdev->dev,
> + sizeof(struct twl4030_madc_data));
> + if (!iio_dev) {
> + dev_err(&pdev->dev, "failed allocating iio device\n");
> + return -ENOMEM;
> + }
> +
> + madc = iio_priv(iio_dev);
> + madc->dev = &pdev->dev;
> +
> + iio_dev->name = dev_name(&pdev->dev);
> + iio_dev->dev.parent = &pdev->dev;
> + iio_dev->dev.of_node = pdev->dev.of_node;
> + iio_dev->info = &twl4030_madc_iio_info;
> + iio_dev->modes = INDIO_DIRECT_MODE;
> + iio_dev->channels = twl4030_madc_iio_channels;
> + iio_dev->num_channels = 16;
> +
> + /*
> + * Phoenix provides 2 interrupt lines. The first one is connected to
> + * the OMAP. The other one can be connected to the other processor such
> + * as modem. Hence two separate ISR and IMR registers.
> + */
> + if (pdata)
> + madc->use_second_irq = pdata->irq_line != 1;
> + else
> + madc->use_second_irq = false;
> +
> + madc->imr = (madc->use_second_irq == 1) ?
> + TWL4030_MADC_IMR1 : TWL4030_MADC_IMR2;
> + madc->isr = (madc->use_second_irq == 1) ?
> + TWL4030_MADC_ISR1 : TWL4030_MADC_ISR2;
> +
> + ret = twl4030_madc_set_power(madc, 1);
> + if (ret < 0)
> + return ret;
> + ret = twl4030_madc_set_current_generator(madc, 0, 1);
> + if (ret < 0)
> + goto err_current_generator;
> +
> + ret = twl_i2c_read_u8(TWL_MODULE_MAIN_CHARGE,
> + ®val, TWL4030_BCI_BCICTL1);
> + if (ret) {
> + dev_err(&pdev->dev, "unable to read reg BCI CTL1 0x%X\n",
> + TWL4030_BCI_BCICTL1);
> + goto err_i2c;
> + }
> + regval |= TWL4030_BCI_MESBAT;
> + ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE,
> + regval, TWL4030_BCI_BCICTL1);
> + if (ret) {
> + dev_err(&pdev->dev, "unable to write reg BCI Ctl1 0x%X\n",
> + TWL4030_BCI_BCICTL1);
> + goto err_i2c;
> + }
> +
> + /* Check that MADC clock is on */
> + ret = twl_i2c_read_u8(TWL4030_MODULE_INTBR, ®val, TWL4030_REG_GPBR1);
> + if (ret) {
Personally I'd rank the driver as rather to vebose on read error messages
given they are pretty unusual. Ah well I guess each to their own.
> + dev_err(&pdev->dev, "unable to read reg GPBR1 0x%X\n",
> + TWL4030_REG_GPBR1);
> + goto err_i2c;
> + }
> +
> + /* If MADC clk is not on, turn it on */
> + if (!(regval & TWL4030_GPBR1_MADC_HFCLK_EN)) {
> + dev_info(&pdev->dev, "clk disabled, enabling\n");
> + regval |= TWL4030_GPBR1_MADC_HFCLK_EN;
> + ret = twl_i2c_write_u8(TWL4030_MODULE_INTBR, regval,
> + TWL4030_REG_GPBR1);
> + if (ret) {
> + dev_err(&pdev->dev, "unable to write reg GPBR1 0x%X\n",
> + TWL4030_REG_GPBR1);
> + goto err_i2c;
> + }
> + }
> +
> + platform_set_drvdata(pdev, iio_dev);
> + mutex_init(&madc->lock);
> +
> + irq = platform_get_irq(pdev, 0);
As ever using devm for a irq request makes for a bit of a review
nightmare as we have to be very careful that nothing has been removed
that this might use before the devm cleanup occurs (at end of remove).
In this case the fact that the current generator is off and the adc unit
is disabled sounds to me like it might cause interesting results in the
interrupt handler?
It's one of those things that will probably never happen but I find it
hard to convince myself 'can't happen'
> + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> + twl4030_madc_threaded_irq_handler,
> + IRQF_TRIGGER_RISING, "twl4030_madc", madc);
> + if (ret) {
> + dev_dbg(&pdev->dev, "could not request irq\n");
> + goto err_i2c;
> + }
Err, where is this defined? Ah, found it above. I'm not keen on
preventing multiple instances of a device like this. It is so easy
to not do this I'd prefer to see the driver fixed to remove this.
> + twl4030_madc = madc;
> +
> + ret = iio_device_register(iio_dev);
> + if (ret) {
> + dev_dbg(&pdev->dev, "could not register iio device\n");
> + goto err_i2c;
> + }
> +
> + return 0;
> +
> +err_i2c:
> + twl4030_madc_set_current_generator(madc, 0, 0);
> +err_current_generator:
> + twl4030_madc_set_power(madc, 0);
> + return ret;
> +}
> +
> +static int twl4030_madc_remove(struct platform_device *pdev)
> +{
> + struct iio_dev *iio_dev = platform_get_drvdata(pdev);
> + struct twl4030_madc_data *madc = iio_priv(iio_dev);
> +
> + twl4030_madc_set_current_generator(madc, 0, 0);
> + twl4030_madc_set_power(madc, 0);
> +
> + iio_device_unregister(iio_dev);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id twl_madc_of_match[] = {
> + {.compatible = "ti,twl4030-madc", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, twl_madc_of_match);
> +#endif
> +
> +static struct platform_driver twl4030_madc_driver = {
> + .probe = twl4030_madc_probe,
> + .remove = twl4030_madc_remove,
> + .driver = {
> + .name = "twl4030_madc",
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(twl_madc_of_match),
> + },
> +};
> +
> +module_platform_driver(twl4030_madc_driver);
> +
> +MODULE_DESCRIPTION("TWL4030 ADC driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("J Keerthy");
> +MODULE_ALIAS("platform:twl4030_madc");
Rest of patch was removal of code so no comments!
^ permalink raw reply
* Re: [PATCH v7 4/8] ARM: sunxi: Add driver for SD/MMC hosts found on Allwinner sunxi SoCs
From: Hans de Goede @ 2014-02-22 11:19 UTC (permalink / raw)
To: Maxime Ripard
Cc: David Lanzendörfer, devicetree-u79uwXL29TY76Z2rM5mHXA,
Ulf Hansson, Laurent Pinchart, Mike Turquette, Simon Baatz,
Emilio López, linux-mmc-u79uwXL29TY76Z2rM5mHXA, Chris Ball,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, H Hartley Sweeten,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Tejun Heo,
Guennadi Liakhovetski,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20140222083128.GH3931@lukather>
Hi,
On 02/22/2014 09:31 AM, Maxime Ripard wrote:
<snip>
>>>>> This should be before the registration. Otherwise, you're racy.
>>>>
>>>> Nope, we only need this to get the data on sunxi_mmc_remove,
>>>> everywhere else the data is found through the mmc-host struct.
>>>
>>> Still, if anyone makes a following patch using the platform_device
>>> for some reason, we will have a race condition, without any way to
>>> notice it.
>>>
>>> Plus, you're doing all the other bits of initialization of your
>>> structures much earlier, why not be consistent and having all of
>>> them at the same place?
>>
>> Most platform drivers I've worked on do platform_set_drvdata as late
>> as possible, so that the drvdata does not get set and never cleared
>> in error paths.
>
> You don't actually have to clear it,
Erm, yes and no, you used to have to manually clear it, so as to not
leave a dangling pointer in there, which may confuse things later.
But since a few kernel releases, the driver core will explicitly
clear it on probe failure, since many many drivers got this wrong.
I know this because I wrote the driver core patch doing the clearing :)
So in drivers which used to get it right, you will see the
platform_set_drvdata call quite late and it being so late
in the sunxi-mmc.c driver is old habits dying hard.
But you're right that this is no longer needed, still I see little
reason to move it up, but if you really want it to be moved up,
I'm fine with that.
> and some frameworks actually require you to call dev_set_drvdata before registration, so that
> statement looks quite odd to me.
And the proper thing to do when using those frameworks was to
manually clear drvdata again on probe failure. Anyways this is
all handled in the driver core now, so this whole discussion
is moot anyways. I was merely trying to explain where my
preference for doing the dev_set_drvdata call as late as
possible comes from.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v6 17/18] ARM: sun4i: dt: Add ahci / sata support
From: Hans de Goede @ 2014-02-22 10:09 UTC (permalink / raw)
To: Maxime Ripard
Cc: Tejun Heo, Oliver Schinagl, Richard Zhu, Roger Quadros, Lee Jones,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20140221181519.GC3931@lukather>
Hi Maxime,
On 02/21/2014 07:15 PM, Maxime Ripard wrote:
> Hi Hans,
>
> On Wed, Feb 19, 2014 at 01:01:59PM +0100, Hans de Goede wrote:
>> From: Oliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
>>
>> This patch adds sunxi sata support to A10 boards that have such a connector.
>> Some boards also feature a regulator via a GPIO and support for this is also
>> added.
>>
>> Signed-off-by: Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>> arch/arm/boot/dts/sun4i-a10-a1000.dts | 4 ++++
>> arch/arm/boot/dts/sun4i-a10-cubieboard.dts | 6 +++++
>> arch/arm/boot/dts/sun4i-a10.dtsi | 8 +++++++
>> arch/arm/boot/dts/sunxi-ahci-reg.dtsi | 36 ++++++++++++++++++++++++++++++
>> 4 files changed, 54 insertions(+)
>> create mode 100644 arch/arm/boot/dts/sunxi-ahci-reg.dtsi
>>
>> diff --git a/arch/arm/boot/dts/sun4i-a10-a1000.dts b/arch/arm/boot/dts/sun4i-a10-a1000.dts
>> index cbd2e13..d6ec839 100644
>> --- a/arch/arm/boot/dts/sun4i-a10-a1000.dts
>> +++ b/arch/arm/boot/dts/sun4i-a10-a1000.dts
>> @@ -35,6 +35,10 @@
>> };
>> };
>>
>> + ahci: sata@01c18000 {
>> + status = "okay";
>> + };
>> +
>> pinctrl@01c20800 {
>> emac_power_pin_a1000: emac_power_pin@0 {
>> allwinner,pins = "PH15";
>> diff --git a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
>> index b139ee6..6df237d8 100644
>> --- a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
>> +++ b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
>> @@ -12,6 +12,7 @@
>>
>> /dts-v1/;
>> /include/ "sun4i-a10.dtsi"
>> +/include/ "sunxi-ahci-reg.dtsi"
>>
>> / {
>> model = "Cubietech Cubieboard";
>> @@ -33,6 +34,11 @@
>> };
>> };
>>
>> + ahci: sata@01c18000 {
>> + target-supply = <®_ahci_5v>;
>> + status = "okay";
>> + };
>> +
>> pinctrl@01c20800 {
>> led_pins_cubieboard: led_pins@0 {
>> allwinner,pins = "PH20", "PH21";
>> diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
>> index 336dbec..454077a 100644
>> --- a/arch/arm/boot/dts/sun4i-a10.dtsi
>> +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
>> @@ -338,6 +338,14 @@
>> #size-cells = <0>;
>> };
>>
>> + ahci: sata@01c18000 {
>> + compatible = "allwinner,sun4i-a10-ahci";
>> + reg = <0x01c18000 0x1000>;
>> + interrupts = <56>;
>> + clocks = <&pll6 0>, <&ahb_gates 25>;
>> + status = "disabled";
>> + };
>> +
>> intc: interrupt-controller@01c20400 {
>> compatible = "allwinner,sun4i-ic";
>> reg = <0x01c20400 0x400>;
>> diff --git a/arch/arm/boot/dts/sunxi-ahci-reg.dtsi b/arch/arm/boot/dts/sunxi-ahci-reg.dtsi
>> new file mode 100644
>> index 0000000..7072af1
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/sunxi-ahci-reg.dtsi
>> @@ -0,0 +1,36 @@
>> +/*
>> + * sunxi boards sata target power supply common code
>
>
> Since IIRC we have pretty much the same needs for the USB, can't we
> just drop the SATA specific mention and use it as the common DTSI for
> the usual regulators?
On most boards with sata, there will also be 1 or 2 usb regulators,
so we need differently named regulator nodes for all 3 of ahci,
usb1 and usb2 vbus. On some boards how ever we may only need the
usb regulators. So if you look in my current personal sunxi-devel
tree you will see separate dtsi files for both ahci and usb regulators,
another advantage of having these separate is that the gpio controlling
the regulator can be pre-populated with the reference design gpio which
is used in most boards, so that the ahci specific code in the dts
becomes only the ahci: sata@... node.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v7 8/8] ARM: sunxi: Add documentation for driver for SD/MMC hosts found on Allwinner sunxi SoCs
From: Maxime Ripard @ 2014-02-22 8:37 UTC (permalink / raw)
To: David Lanzendörfer
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Ulf Hansson, Laurent Pinchart,
Mike Turquette, Simon Baatz, Hans de Goede, Emilio López,
linux-mmc-u79uwXL29TY76Z2rM5mHXA, Chris Ball,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, H Hartley Sweeten,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Tejun Heo,
Guennadi Liakhovetski,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1559221.mEv19UvCzF-pgFh0Jf6HD9Xzn/AsuzBOg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]
Hi David,
On Sat, Feb 22, 2014 at 08:32:03AM +0100, David Lanzendörfer wrote:
> > Ditto. Plus, this is not a mod0 clock.
> Yes it is! But maybe the formulation hasn't been clear enough...
Technically, it's not, it has this phase controls features a mod0
clock doesn't have.
> > You never talked about the clock-names property, and which clocks
> > were supposed to be provided.
>
> Yes I did? But I expanded the text a little bit further...
I can't see any reference to the fact that clock-names should be set,
and what values should it hold.
Something like that:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt#n11
> > > + interrupts = <0 32 4>;
> > > + bus-width = <4>;
> > And you never talked about bus-width either.
> I can throw in a line for refering to the mmc slot gpio lib docs.
Yes, that would be great :)
> > Isn't the cd-gpios property requested too?
> I can refer to the docs there as well if you like... :-)
That would be great too :)
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v7 4/8] ARM: sunxi: Add driver for SD/MMC hosts found on Allwinner sunxi SoCs
From: Maxime Ripard @ 2014-02-22 8:31 UTC (permalink / raw)
To: Hans de Goede
Cc: David Lanzendörfer, devicetree-u79uwXL29TY76Z2rM5mHXA,
Ulf Hansson, Laurent Pinchart, Mike Turquette, Simon Baatz,
Emilio López, linux-mmc-u79uwXL29TY76Z2rM5mHXA, Chris Ball,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, H Hartley Sweeten,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Tejun Heo,
Guennadi Liakhovetski,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <5304A042.7090903-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 5035 bytes --]
Hi Hans,
(As a side note, your mailer just did something nasty with the
wrapping which made the code snippets totally unreadable. I'm going to
drop them.)
On Wed, Feb 19, 2014 at 01:14:58PM +0100, Hans de Goede wrote:
> >>>> + wmb(); /* Ensure idma_des hit main mem before we start the idmac */
> >>>
> >>> wmb ensure the proper ordering of the instructions, not flushing
> >>> the caches like what your comment implies.
> >>
> >> Since I put that comment there, allow me to explain. A modern ARM
> >> cpu core has 2 or more units handling stores. One for regular
> >> memory stores, and one for io-mem stores. Regular mem stores can
> >> be re-ordered, io stores cannot. Normally there is no "syncing"
> >> between the 2 store units. Cache flushing is not an issue here
> >> since the memory holding the descriptors for the idma controller
> >> is allocated cache coherent, which on arm means it is not cached.
> >>
> >> What is an issue here is the io-store starting the idmac hitting
> >> the io-mem before the descriptors hit the main-mem, the wmb()
> >> ensures this does not happen.
> >
> > To expand a bit, my point was not that it was functionnally
> > wrong. Since you put a barrier in there, and that it resides in a
> > cache coherent section, we're fine.
> >
> > My point was that the comment itself was misleading.
>
> Well as explained above, the purpose of the wmb is to ensure that
> the descriptors hit main memory, before the following writel (in the
> caller of this function) starts the controller. So I don't see
> exactly how the comment is wrong.
>
> If you've a better wording for the comment, suggestions are welcome.
Your first reply was great :)
But if you feel like it enough, fine.
[ codeless snip..]
> >>> I'd rather put it at a debug loglevel.
> >>
> >> Erm, this only happens if something is seriously wrong.
> >
> > Still. Something would be seriously wrong in the MMC
> > driver/controller. You don't want to bloat the whole kernel logs
> > with the dump of your registers just because the MMC is
> > failing. This is of no interest to anyone but someone that would
> > actually try to debug what's wrong.
>
> This is not a complete register dump, this writes a single line to
> the kernel log saying that an io error happened, and printing the
> error flags set in the status register. We cannot be much shorter
> then this without simply not notifying the user that an io error has
> happened, and not notifying the user is wrong IMHO.
Ok.
> >>>> + /* And put it back in reset */
> >>>> + sunxi_mmc_exit_host(host);
> >>>
> >>> Hu? If it's in reset, how can it generate some IRQs?
> >>
> >> Yes, that is why we do the whole dance of init controller, get
> >> irq, disable irq, drop it back in reset (until the mmc subsys
> >> does a power on of the mmc card / sdio dev).
> >>
> >> Sometime the controller asserts the irq in reset for some reason,
> >> so without the dance as soon as we do the devm_request_irq we get
> >> an irq, and worse, not only do we get an irq, we cannot clear it
> >> since writing to the interrupt status register does not work when
> >> the controller is in reset, so we get stuck re-entering the irq
> >> handler.
> >
> > Hmmm, I see. It probably deserves some commenting here too then.
>
> This call is the mirror of the sunxi_mmc_init_host a few lines
> higher, which has this comment:
>
> /* Make sure the controller is in a sane state before enabling irqs */
>
> Which attempts to explain why we do the init controller, claim irq,
> disable irq, put controller back in reset sequence. Again suggestions
> for a better comment are welcome.
And again, the second part of your first reply was great :)
> >>>> + ret = mmc_add_host(mmc);
> >>>> + if (ret)
> >>>> + goto error_free_dma;
> >>>> +
> >>>> + dev_info(&pdev->dev, "base:0x%p irq:%u\n", host->reg_base, host->irq);
> >>>> + platform_set_drvdata(pdev, mmc);
> >>>
> >>> This should be before the registration. Otherwise, you're racy.
> >>
> >> Nope, we only need this to get the data on sunxi_mmc_remove,
> >> everywhere else the data is found through the mmc-host struct.
> >
> > Still, if anyone makes a following patch using the platform_device
> > for some reason, we will have a race condition, without any way to
> > notice it.
> >
> > Plus, you're doing all the other bits of initialization of your
> > structures much earlier, why not be consistent and having all of
> > them at the same place?
>
> Most platform drivers I've worked on do platform_set_drvdata as late
> as possible, so that the drvdata does not get set and never cleared
> in error paths.
You don't actually have to clear it, and some frameworks actually
require you to call dev_set_drvdata before registration, so that
statement looks quite odd to me.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v7 8/8] ARM: sunxi: Add documentation for driver for SD/MMC hosts found on Allwinner sunxi SoCs
From: David Lanzendörfer @ 2014-02-22 7:32 UTC (permalink / raw)
To: Maxime Ripard
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Ulf Hansson, Laurent Pinchart,
Mike Turquette, Simon Baatz, Hans de Goede, Emilio López,
linux-mmc-u79uwXL29TY76Z2rM5mHXA, Chris Ball,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, H Hartley Sweeten,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Tejun Heo,
Guennadi Liakhovetski,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20140218154212.GL3142@lukather>
[-- Attachment #1: Type: text/plain, Size: 838 bytes --]
Hi
> Please use the real compatibles here. It's much easier to search
> for. Plus, your driver doesn't support all the SoCs you're mentionning here.
> [...]
> Please provide the real property name to use. No need for an example
> here, you have a full-fledged one in a few lines.
Fixed that.
> Ditto. Plus, this is not a mod0 clock.
Yes it is! But maybe the formulation hasn't been clear enough...
> You never talked about the clock-names property, and which clocks were
> supposed to be provided.
Yes I did? But I expanded the text a little bit further...
> > + interrupts = <0 32 4>;
> > + bus-width = <4>;
> And you never talked about bus-width either.
I can throw in a line for refering to the mmc slot gpio lib docs.
> Isn't the cd-gpios property requested too?
I can refer to the docs there as well if you like... :-)
cheers
david
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 07/10] spi: sh-msiof: Add support for R-Car H2 and M2
From: Mark Brown @ 2014-02-22 3:14 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Magnus Damm, Takashi Yoshii, linux-spi, SH-Linux, linux-kernel,
Geert Uytterhoeven,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAMuHMdUByWPcYM8M_m5NVM4QP5cQE_tOLGQg=2fTNF7gqkUhig-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 462 bytes --]
On Thu, Feb 20, 2014 at 05:13:16PM +0100, Geert Uytterhoeven wrote:
> "renesas,tx-fifo-size" and "renesas,rx-fifo-size" are part of the existing
> bindings, so I'm a bit reluctant to change these.
> Hmm, since the original bindings didn't specify the default values,
> I could make them chip-specific, though.
That sounds like a good idea, you could also change the properties to be
deprecatedn and recommend that people just don't bother setting them at
all.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 01/10] spi: sh-msiof: Fix SPI bus population from DT
From: Mark Brown @ 2014-02-22 3:10 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Takashi Yoshii, Magnus Damm, linux-spi, linux-sh, linux-kernel,
Geert Uytterhoeven, devicetree
In-Reply-To: <1392907389-21798-2-git-send-email-geert@linux-m68k.org>
[-- Attachment #1: Type: text/plain, Size: 244 bytes --]
On Thu, Feb 20, 2014 at 03:43:00PM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>
> DT doesn't instantiate SPI children if spi_master.dev.of_node is not set up
> properly.
Applied, thanks.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 4/7] ECHI Platform: Merge ppc-of EHCI driver into the ehci-platform driver
From: Tony Prisk @ 2014-02-22 2:32 UTC (permalink / raw)
To: Mark Rutland, Alistair Popple
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <20140221114803.GB8783-NuALmloUBlrZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org>
On 22/02/14 00:48, Mark Rutland wrote:
> [Adding Tony Prisk to Cc]
>
> On Fri, Feb 21, 2014 at 06:31:30AM +0000, Alistair Popple wrote:
>> Currently the ppc-of driver uses the compatibility string
>> "usb-ehci". This means platforms that use device-tree and implement an
>> EHCI compatible interface have to either use the ppc-of driver or add
>> a compatible line to the ehci-platform driver. It would be more
>> appropriate for the platform driver to be compatible with "usb-ehci"
>> as non-powerpc platforms are also beginning to utilise device-tree.
>>
>> This patch merges the device tree property parsing from ehci-ppc-of
>> into the platform driver and adds a "usb-ehci" compatibility
>> string. The existing ehci-ppc-of driver is removed and the 440EPX
>> specific quirks are added to the ehci-platform driver.
>>
>> Signed-off-by: Alistair Popple <alistair-Y4h6yKqj69EXC2x5gXVKYQ@public.gmane.org>
>> ---
>> drivers/usb/host/Kconfig | 7 +-
>> drivers/usb/host/ehci-hcd.c | 5 -
>> drivers/usb/host/ehci-platform.c | 87 +++++++++++++-
>> drivers/usb/host/ehci-ppc-of.c | 238 --------------------------------------
>> 4 files changed, 89 insertions(+), 248 deletions(-)
>> delete mode 100644 drivers/usb/host/ehci-ppc-of.c
> Please use of_property_read_bool for these.
>
> This driver already handles "via,vt8500-ehci" and "wm,prizm-ehci" which
> aren't documented to handle these properties, but now gain support for
> them. It might be worth unifying the binding documents if there's
> nothing special about those two host controllers.
>
> We seem to have two binding documents for "via,vt8500-ehci", so some
> cleanup is definitely in order.
>
> Tony, you seem to have written both documents judging by 95e9fd10f06c
> and 8ad551d150e3. Do you have any issue with merging both of these into
> a common usb-ehci document?
......
> Cheers,
> Mark.
I'm not sure how we ended up with two bindings for the driver anyway. I
think this was an error on my part somewhere.
None of the in-tree dts files use "wm,prizm-ehci".
I have no issue with merging all the documentation into a single
usb-ehci binding.
Regards
Tony Prisk
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox