Hi,
On 01/21/2014 02:42 AM, Ma Haijun wrote:
Hi
-----Original Message-----
From: linux-sunxi@googlegroups.com [mailto:linux-sunxi@googlegroups.com]
On Behalf Of Hans de Goede
Sent: Tuesday, January 21, 2014 12:45 AM
To: Tejun Heo
Cc: Oliver Schinagl; Maxime Ripard; Richard Zhu; Roger Quadros; linux-
i...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; devicetree;
linux-
su...@googlegroups.com; Hans de Goede
Subject: [linux-sunxi] [PATCH RFC v4 10/10] ahci_imx: Port to library-ised
ahci_platform
This avoids the ugliness of creating a nested platform device from probe.
Note untested, I've ordered a wandboard to be able to test these changes.
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
.../devicetree/bindings/ata/ahci-platform.txt | 8 +-
drivers/ata/ahci_imx.c | 199
++++++++-------------
2 files changed, 80 insertions(+), 127 deletions(-)
diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt
b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index f036e786..ee3a127 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -5,8 +5,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", or
- "allwinner,sun4i-a10-ahci"
+ "snps,exynos5440-ahci", "ibm,476gtr-ahci",
+ "allwinner,sun4i-a10-ahci" or "fsl,imx6q-ahci"
- interrupts : <interrupt mapping for SATA IRQ>
- reg : <registers mapping>
@@ -18,6 +18,10 @@ Optional properties:
allwinner,sun4i-a10-ahci required properties:
- clocks : index 0 must point to the sata_ref clk, 1 to the
ahb clk
+fsl,imx6q-ahci required properties:
+- clocks : index 0 must point to the sataf clk, 1 to the
sata_ref
+ clk and 2 to the ahb clk
+
Examples:
sata@ffe08000 {
compatible = "snps,spear-ahci";
diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c index
49fa0ca..3454b18 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -34,10 +34,14 @@ enum {
HOST_TIMER1MS = 0xe0, /* Timer 1-ms */
};
+enum {
+ CLK_SATA,
+ CLK_SATA_REF,
+ CLK_AHB
+};
+
struct imx_ahci_priv {
struct platform_device *ahci_pdev;
- struct clk *sata_ref_clk;
- struct clk *ahb_clk;
struct regmap *gpr;
bool no_device;
bool first_time;
@@ -47,6 +51,8 @@ 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 void ahci_imx_host_stop(struct ata_host *host);
+
static void ahci_imx_error_handler(struct ata_port *ap) {
u32 reg_val;
@@ -54,7 +60,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);
@@ -75,12 +81,13 @@ static void ahci_imx_error_handler(struct ata_port
*ap)
regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
IMX6Q_GPR13_SATA_MPLL_CLK_EN,
!IMX6Q_GPR13_SATA_MPLL_CLK_EN);
- clk_disable_unprepare(imxpriv->sata_ref_clk);
+ ahci_platform_disable_clks(hpriv);
imxpriv->no_device = true;
}
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,
};
@@ -91,23 +98,38 @@ static const struct ata_port_info ahci_imx_port_info =
{
.port_ops = &ahci_imx_ops,
};
-static int imx6q_sata_init(struct device *dev, struct ahci_host_priv
*hpriv)
+static int imx_ahci_probe(struct platform_device *pdev)
{
- int ret = 0;
+ struct device *dev = &pdev->dev;
+ struct ahci_host_priv *hpriv;
+ struct imx_ahci_priv *imxpriv;
unsigned int reg_val;
- struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
+ int rc;
+
+ imxpriv = devm_kzalloc(dev, sizeof(*imxpriv), GFP_KERNEL);
+ if (!imxpriv)
+ return -ENOMEM;
+
+ hpriv = ahci_platform_get_resources(pdev);
I think we need a better encapsulation or a better name, the private data is
not so private here.
It is private from the pov of libata, also look at the data type:
ahci_host_priv and how we get it in other functions, through host->private_data
+ if (IS_ERR(hpriv))
+ return PTR_ERR(hpriv);
+ if (!hpriv->clks[CLK_AHB]) {
+ dev_err(dev, "no ahb clk, need sata, sata_ref and ahb
clks\n");
+ rc = -ENOENT;
+ goto put_resources;
+ }
+ hpriv->plat_data = imxpriv;
+
+ rc = ahci_platform_enable_resources(hpriv);
+ if (rc)
+ goto put_resources;
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);
- }
-
- ret = clk_prepare_enable(imxpriv->sata_ref_clk);
- if (ret < 0) {
- dev_err(dev, "prepare-enable sata_ref clock err:%d\n", ret);
- return ret;
+ rc = PTR_ERR(imxpriv->gpr);
+ goto disable_resources;
}
/*
@@ -156,24 +178,45 @@ static int imx6q_sata_init(struct device *dev,
struct
ahci_host_priv *hpriv)
writel(reg_val, hpriv->mmio + HOST_PORTS_IMPL);
}
- reg_val = clk_get_rate(imxpriv->ahb_clk) / 1000;
+ reg_val = clk_get_rate(hpriv->clks[CLK_AHB]) / 1000;
This makes the private data even less private.
Huh? Of course ahci_imx itself does touch it, it is the owner of it, and
the owner touching private data is sort of the whole idea.
I think here we should forget that there is already a clk pointer somewhere,
instead, fetch it by ourselves with clk_get/devm_clk_get.
And the fact that the generic platform driver does not use clock-names
property,
does not prevent imx driver to make use of it.
That would come at the cost of duplicating all the clk related code, which
is exactly what we're trying to avoid.
writel(reg_val, hpriv->mmio + HOST_TIMER1MS);
+ rc = ahci_platform_init_host(pdev, hpriv, &ahci_imx_port_info, 0,
0);
+ if (rc)
+ goto disable_resources;
+
return 0;
+
+disable_resources:
+ ahci_platform_disable_resources(hpriv);
+put_resources:
+ ahci_platform_put_resources(hpriv);
+ return rc;
}
-static void imx6q_sata_exit(struct device *dev)
+static void ahci_imx_host_stop(struct ata_host *host)
{
- struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
+ struct ahci_host_priv *hpriv = host->private_data;
+ struct imx_ahci_priv *imxpriv = hpriv->plat_data;
- regmap_update_bits(imxpriv->gpr, 0x34,
IMX6Q_GPR13_SATA_MPLL_CLK_EN,
+ if (!imxpriv->no_device) {
+ regmap_update_bits(imxpriv->gpr, 0x34,
+ IMX6Q_GPR13_SATA_MPLL_CLK_EN,
!IMX6Q_GPR13_SATA_MPLL_CLK_EN);
- clk_disable_unprepare(imxpriv->sata_ref_clk);
+ ahci_platform_disable_clks(hpriv);
+ }
}
static int imx_ahci_suspend(struct device *dev) {
- struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
+ struct ata_host *host = dev_get_drvdata(dev);
+ struct ahci_host_priv *hpriv = host->private_data;
+ struct imx_ahci_priv *imxpriv = hpriv->plat_data;
+ int rc;
+
+ rc = ahci_platform_suspend_host(dev);
+ if (rc)
+ return rc;
/*
* If no_device is set, The CLKs had been gated off in the @@ -183,7
+226,7 @@ static int imx_ahci_suspend(struct device *dev)
regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
IMX6Q_GPR13_SATA_MPLL_CLK_EN,
!IMX6Q_GPR13_SATA_MPLL_CLK_EN);
- clk_disable_unprepare(imxpriv->sata_ref_clk);
+ ahci_platform_disable_clks(hpriv);
}
return 0;
@@ -191,11 +234,13 @@ static int imx_ahci_suspend(struct device *dev)
static int imx_ahci_resume(struct device *dev) {
- struct imx_ahci_priv *imxpriv = dev_get_drvdata(dev->parent);
+ struct ata_host *host = dev_get_drvdata(dev);
+ struct ahci_host_priv *hpriv = host->private_data;
+ struct imx_ahci_priv *imxpriv = hpriv->plat_data;
int ret;
if (!imxpriv->no_device) {
- ret = clk_prepare_enable(imxpriv->sata_ref_clk);
+ ret = ahci_platform_enable_clks(hpriv);
if (ret < 0) {
dev_err(dev, "pre-enable sata_ref clock err:%d\n",
ret);
return ret;
@@ -207,123 +252,27 @@ static int imx_ahci_resume(struct device *dev)
usleep_range(1000, 2000);
}
- return 0;
+ return ahci_platform_resume_host(dev);
}
-static struct ahci_platform_data imx6q_sata_pdata = {
- .init = imx6q_sata_init,
- .exit = imx6q_sata_exit,
- .ata_port_info = &ahci_imx_port_info,
- .suspend = imx_ahci_suspend,
- .resume = imx_ahci_resume,
-};
+static SIMPLE_DEV_PM_OPS(ahci_imx_pm_ops, imx_ahci_suspend,
+imx_ahci_resume);
static const struct of_device_id imx_ahci_of_match[] = {
- { .compatible = "fsl,imx6q-ahci", .data = &imx6q_sata_pdata},
+ { .compatible = "fsl,imx6q-ahci", },
{},
};
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;
- const struct ahci_platform_data *pdata = NULL;
- struct imx_ahci_priv *imxpriv;
- struct device *ahci_dev;
- struct platform_device *ahci_pdev;
- int ret;
-
- imxpriv = devm_kzalloc(dev, sizeof(*imxpriv), GFP_KERNEL);
- if (!imxpriv) {
- dev_err(dev, "can't alloc ahci_host_priv\n");
- 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->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;
- }
-
- 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;
- }
-
- imxpriv->ahci_pdev = ahci_pdev;
- platform_set_drvdata(pdev, imxpriv);
-
- of_id = of_match_device(imx_ahci_of_match, dev);
- if (of_id) {
- pdata = of_id->data;
- } else {
- ret = -EINVAL;
- goto err_out;
- }
- 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;
- }
-
- res[0] = *mem;
- res[1] = *irq;
-
- 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 = platform_device_add_resources(ahci_pdev, res, 2);
- if (ret)
- goto err_out;
-
- ret = platform_device_add_data(ahci_pdev, pdata, sizeof(*pdata));
- if (ret)
- goto err_out;
-
- ret = platform_device_add(ahci_pdev);
- if (ret) {
-err_out:
- platform_device_put(ahci_pdev);
- return ret;
- }
-
- return 0;
-}
-
-static int imx_ahci_remove(struct platform_device *pdev) -{
- struct imx_ahci_priv *imxpriv = platform_get_drvdata(pdev);
- struct platform_device *ahci_pdev = imxpriv->ahci_pdev;
-
- platform_device_unregister(ahci_pdev);
- return 0;
-}
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.8.5.3
--
Regards,
Ma
Regards,
Hans
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
