[PATCH 1/6] sata_mv: increase PIO IORDY timeout

[PATCH 1/6] sata_mv: increase PIO IORDY timeout

[Saeed Bishara]

The old value (0xbc) in cycles of the IORDY timeout is suitable for
devices with core clock of 166 MHz, but some SoC controllers have
faster core clocks. The new value will make the IORDY timeout large
enough also for all SoC devices.

Signed-off-by: Saeed Bishara <saeed@marvell.com>
---
 drivers/ata/sata_mv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 6f5093b..312c3e0 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -3393,7 +3393,7 @@ static void mv_soc_reset_hc_port(struct mv_host_priv *hpriv,
 	ZERO(0x024);		/* respq outp */
 	ZERO(0x020);		/* respq inp */
 	ZERO(0x02c);		/* test control */
-	writel(0xbc, port_mmio + EDMA_IORDY_TMOUT);
+	writel(0x800, port_mmio + EDMA_IORDY_TMOUT);
 }
 
 #undef ZERO
-- 
1.6.0.4

[PATCH 2/6] sata_mv: support clkdev framework

[Saeed Bishara]

Signed-off-by: Saeed Bishara <saeed@marvell.com>
---
 drivers/ata/sata_mv.c |   37 ++++++++++++++++++++++++++++++++++---
 1 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 312c3e0..0906df4 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -59,6 +59,7 @@
 #include <linux/dmapool.h>
 #include <linux/dma-mapping.h>
 #include <linux/device.h>
+#include <linux/clk.h>
 #include <linux/platform_device.h>
 #include <linux/ata_platform.h>
 #include <linux/mbus.h>
@@ -548,6 +549,10 @@ struct mv_host_priv {
 	u32			irq_cause_offset;
 	u32			irq_mask_offset;
 	u32			unmask_all_irqs;
+
+#if defined(CONFIG_HAVE_CLK)
+	struct clk		*clk;
+#endif
 	/*
 	 * These consistent DMA memory pools give us guaranteed
 	 * alignment for hardware-accessed data structures,
@@ -4041,6 +4046,14 @@ static int mv_platform_probe(struct platform_device *pdev)
 				   resource_size(res));
 	hpriv->base -= SATAHC0_REG_BASE;
 
+#if defined(CONFIG_HAVE_CLK)
+	hpriv->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(hpriv->clk))
+		dev_notice(&pdev->dev, "cannot get clkdev\n");
+	else
+		clk_enable(hpriv->clk);
+#endif
+
 	/*
 	 * (Re-)program MBUS remapping windows if we are asked to.
 	 */
@@ -4049,12 +4062,12 @@ static int mv_platform_probe(struct platform_device *pdev)
 
 	rc = mv_create_dma_pools(hpriv, &pdev->dev);
 	if (rc)
-		return rc;
+		goto err;
 
 	/* initialize adapter */
 	rc = mv_init_host(host, chip_soc);
 	if (rc)
-		return rc;
+		goto err;
 
 	dev_printk(KERN_INFO, &pdev->dev,
 		   "slots %u ports %d\n", (unsigned)MV_MAX_Q_DEPTH,
@@ -4062,6 +4075,15 @@ static int mv_platform_probe(struct platform_device *pdev)
 
 	return ata_host_activate(host, platform_get_irq(pdev, 0), mv_interrupt,
 				 IRQF_SHARED, &mv6_sht);
+err:
+#if defined(CONFIG_HAVE_CLK)
+	if (!IS_ERR(hpriv->clk)) {
+		clk_disable(hpriv->clk);
+		clk_put(hpriv->clk);
+	}
+#endif
+
+	return rc;
 }
 
 /*
@@ -4076,8 +4098,17 @@ static int __devexit mv_platform_remove(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct ata_host *host = dev_get_drvdata(dev);
-
+#if defined(CONFIG_HAVE_CLK)
+	struct mv_host_priv *hpriv = host->private_data;
+#endif
 	ata_host_detach(host);
+
+#if defined(CONFIG_HAVE_CLK)
+	if (!IS_ERR(hpriv->clk)) {
+		clk_disable(hpriv->clk);
+		clk_put(hpriv->clk);
+	}
+#endif
 	return 0;
 }
 
-- 
1.6.0.4

[PATCH 3/6] sata_mv: add power management support for the platform driver

[Saeed Bishara]

Signed-off-by: Saeed Bishara <saeed@marvell.com>
---
 drivers/ata/sata_mv.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 43 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 drivers/ata/sata_mv.c

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
old mode 100644
new mode 100755
index 0906df4..17a138b
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -4112,9 +4112,52 @@ static int __devexit mv_platform_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int mv_platform_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	struct ata_host *host = dev_get_drvdata(&pdev->dev);
+	if (host)
+		return ata_host_suspend(host, state);
+	else
+		return 0;
+}
+
+static int mv_platform_resume(struct platform_device *pdev)
+{
+	struct ata_host *host = dev_get_drvdata(&pdev->dev);
+	int ret;
+
+	if (host) {
+		struct mv_host_priv *hpriv = host->private_data;
+		const struct mv_sata_platform_data *mv_platform_data = \
+			pdev->dev.platform_data;
+		/*
+		 * (Re-)program MBUS remapping windows if we are asked to.
+		 */
+		if (mv_platform_data->dram != NULL)
+			mv_conf_mbus_windows(hpriv, mv_platform_data->dram);
+
+		/* initialize adapter */
+		ret = mv_init_host(host, chip_soc);
+		if (ret) {
+			printk(KERN_ERR DRV_NAME ": Error during HW init\n");
+			return ret;
+		}
+		ata_host_resume(host);
+	}
+
+	return 0;
+}
+#else
+#define mv_platform_suspend NULL
+#define mv_platform_resume NULL
+#endif
+
 static struct platform_driver mv_platform_driver = {
 	.probe			= mv_platform_probe,
 	.remove			= __devexit_p(mv_platform_remove),
+	.suspend		= mv_platform_suspend,
+	.resume			= mv_platform_resume,
 	.driver			= {
 				   .name = DRV_NAME,
 				   .owner = THIS_MODULE,
-- 
1.6.0.4

[PATCH 4/6] sata_mv: move the PCI bar description initialization code from mv_init_host to mv_pci_init_one

[Saeed Bishara]

the mv_init_host will be used to initialize the host hw on resume. the PCI bar description need to be initialized only once when the device probed.

Signed-off-by: Saeed Bishara <saeed@marvell.com>
---
 drivers/ata/sata_mv.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 17a138b..3eadbd5 100755
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -3910,14 +3910,6 @@ static int mv_init_host(struct ata_host *host, unsigned int board_idx)
 		void __iomem *port_mmio = mv_port_base(mmio, port);
 
 		mv_port_init(&ap->ioaddr, port_mmio);
-
-#ifdef CONFIG_PCI
-		if (!IS_SOC(hpriv)) {
-			unsigned int offset = port_mmio - mmio;
-			ata_port_pbar_desc(ap, MV_PRIMARY_BAR, -1, "mmio");
-			ata_port_pbar_desc(ap, MV_PRIMARY_BAR, offset, "port");
-		}
-#endif
 	}
 
 	for (hc = 0; hc < n_hc; hc++) {
@@ -4268,7 +4260,7 @@ static int mv_pci_init_one(struct pci_dev *pdev,
 	const struct ata_port_info *ppi[] = { &mv_port_info[board_idx], NULL };
 	struct ata_host *host;
 	struct mv_host_priv *hpriv;
-	int n_ports, rc;
+	int n_ports, port, rc;
 
 	if (!printed_version++)
 		dev_printk(KERN_INFO, &pdev->dev, "version " DRV_VERSION "\n");
@@ -4304,6 +4296,15 @@ static int mv_pci_init_one(struct pci_dev *pdev,
 	if (rc)
 		return rc;
 
+	for (port = 0; port < host->n_ports; port++) {
+		struct ata_port *ap = host->ports[port];
+		void __iomem *port_mmio = mv_port_base(hpriv->base, port);
+		unsigned int offset = port_mmio - hpriv->base;
+
+		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, -1, "mmio");
+		ata_port_pbar_desc(ap, MV_PRIMARY_BAR, offset, "port");
+	}
+
 	/* initialize adapter */
 	rc = mv_init_host(host, board_idx);
 	if (rc)
-- 
1.6.0.4

[PATCH 5/6] sata_mv: store the board_idx into the host private data

[Saeed Bishara]

This information will be used int the resume function.

Signed-off-by: Saeed Bishara <saeed@marvell.com>
---
 drivers/ata/sata_mv.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 3eadbd5..71ac62e 100755
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -539,6 +539,7 @@ struct mv_port_signal {
 
 struct mv_host_priv {
 	u32			hp_flags;
+	unsigned int 		board_idx;
 	u32			main_irq_mask;
 	struct mv_port_signal	signal[8];
 	const struct mv_hw_ops	*ops;
@@ -3859,7 +3860,6 @@ static int mv_chip_id(struct ata_host *host, unsigned int board_idx)
 /**
  *      mv_init_host - Perform some early initialization of the host.
  *	@host: ATA host to initialize
- *      @board_idx: controller index
  *
  *      If possible, do an early global reset of the host.  Then do
  *      our port init and clear/unmask all/relevant host interrupts.
@@ -3867,13 +3867,13 @@ static int mv_chip_id(struct ata_host *host, unsigned int board_idx)
  *      LOCKING:
  *      Inherited from caller.
  */
-static int mv_init_host(struct ata_host *host, unsigned int board_idx)
+static int mv_init_host(struct ata_host *host)
 {
 	int rc = 0, n_hc, port, hc;
 	struct mv_host_priv *hpriv = host->private_data;
 	void __iomem *mmio = hpriv->base;
 
-	rc = mv_chip_id(host, board_idx);
+	rc = mv_chip_id(host, hpriv->board_idx);
 	if (rc)
 		goto done;
 
@@ -4032,6 +4032,7 @@ static int mv_platform_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	host->private_data = hpriv;
 	hpriv->n_ports = n_ports;
+	hpriv->board_idx = chip_soc;
 
 	host->iomap = NULL;
 	hpriv->base = devm_ioremap(&pdev->dev, res->start,
@@ -4057,7 +4058,7 @@ static int mv_platform_probe(struct platform_device *pdev)
 		goto err;
 
 	/* initialize adapter */
-	rc = mv_init_host(host, chip_soc);
+	rc = mv_init_host(host);
 	if (rc)
 		goto err;
 
@@ -4274,6 +4275,7 @@ static int mv_pci_init_one(struct pci_dev *pdev,
 		return -ENOMEM;
 	host->private_data = hpriv;
 	hpriv->n_ports = n_ports;
+	hpriv->board_idx = board_idx;
 
 	/* acquire resources */
 	rc = pcim_enable_device(pdev);
@@ -4306,7 +4308,7 @@ static int mv_pci_init_one(struct pci_dev *pdev,
 	}
 
 	/* initialize adapter */
-	rc = mv_init_host(host, board_idx);
+	rc = mv_init_host(host);
 	if (rc)
 		return rc;
 
-- 
1.6.0.4

[PATCH 6/6] sata_mv: add power management support for the PCI controllers.

[Saeed Bishara]

Signed-off-by: Saeed Bishara <saeed@marvell.com>
---
 drivers/ata/sata_mv.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 71ac62e..704d86f 100755
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -4161,6 +4161,9 @@ static struct platform_driver mv_platform_driver = {
 #ifdef CONFIG_PCI
 static int mv_pci_init_one(struct pci_dev *pdev,
 			   const struct pci_device_id *ent);
+#ifdef CONFIG_PM
+static int mv_pci_device_resume(struct pci_dev *pdev);
+#endif
 
 
 static struct pci_driver mv_pci_driver = {
@@ -4168,6 +4171,11 @@ static struct pci_driver mv_pci_driver = {
 	.id_table		= mv_pci_tbl,
 	.probe			= mv_pci_init_one,
 	.remove			= ata_pci_remove_one,
+#ifdef CONFIG_PM
+	.suspend		= ata_pci_device_suspend,
+	.resume			= mv_pci_device_resume,
+#endif
+
 };
 
 /* move to PCI layer or libata core? */
@@ -4324,6 +4332,27 @@ static int mv_pci_init_one(struct pci_dev *pdev,
 	return ata_host_activate(host, pdev->irq, mv_interrupt, IRQF_SHARED,
 				 IS_GEN_I(hpriv) ? &mv5_sht : &mv6_sht);
 }
+
+#ifdef CONFIG_PM
+static int mv_pci_device_resume(struct pci_dev *pdev)
+{
+	struct ata_host *host = dev_get_drvdata(&pdev->dev);
+	int rc;
+
+	rc = ata_pci_device_do_resume(pdev);
+	if (rc)
+		return rc;
+
+	/* initialize adapter */
+	rc = mv_init_host(host);
+	if (rc)
+		return rc;
+
+	ata_host_resume(host);
+
+	return 0;
+}
+#endif
 #endif
 
 static int mv_platform_probe(struct platform_device *pdev);
-- 
1.6.0.4

Re: [PATCH 3/6] sata_mv: add power management support for the platform driver

[Grant Grundler]

On Sun, Dec 6, 2009 at 8:26 AM, Saeed Bishara <saeed@marvell.com> wrote:
> Signed-off-by: Saeed Bishara <saeed@marvell.com>
> ---
>  drivers/ata/sata_mv.c |   43 +++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 43 insertions(+), 0 deletions(-)
>  mode change 100644 => 100755 drivers/ata/sata_mv.c
>
> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> old mode 100644
> new mode 100755
> index 0906df4..17a138b
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -4112,9 +4112,52 @@ static int __devexit mv_platform_remove(struct platform_device *pdev)
>        return 0;
>  }
>
> +#ifdef CONFIG_PM
> +static int mv_platform_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> +       struct ata_host *host = dev_get_drvdata(&pdev->dev);
> +       if (host)
> +               return ata_host_suspend(host, state);
> +       else
> +               return 0;

Just curious: Is this really enough?

Most drivers in drivers/ata/ are setting ata_pci_device_suspend() as
their .suspend function. This calls ata_host_suspend().

ata_piix seems to be the only driver that calls  ata_pci_device_do_suspend().

Please add "Reviewed-by: Grant Grundler <grundler@google.com>" once
the above is clarified.

thanks,
grant

> +}
> +
> +static int mv_platform_resume(struct platform_device *pdev)
> +{
> +       struct ata_host *host = dev_get_drvdata(&pdev->dev);
> +       int ret;
> +
> +       if (host) {
> +               struct mv_host_priv *hpriv = host->private_data;
> +               const struct mv_sata_platform_data *mv_platform_data = \
> +                       pdev->dev.platform_data;
> +               /*
> +                * (Re-)program MBUS remapping windows if we are asked to.
> +                */
> +               if (mv_platform_data->dram != NULL)
> +                       mv_conf_mbus_windows(hpriv, mv_platform_data->dram);
> +
> +               /* initialize adapter */
> +               ret = mv_init_host(host, chip_soc);
> +               if (ret) {
> +                       printk(KERN_ERR DRV_NAME ": Error during HW init\n");
> +                       return ret;
> +               }
> +               ata_host_resume(host);
> +       }
> +
> +       return 0;
> +}
> +#else
> +#define mv_platform_suspend NULL
> +#define mv_platform_resume NULL
> +#endif
> +
>  static struct platform_driver mv_platform_driver = {
>        .probe                  = mv_platform_probe,
>        .remove                 = __devexit_p(mv_platform_remove),
> +       .suspend                = mv_platform_suspend,
> +       .resume                 = mv_platform_resume,
>        .driver                 = {
>                                   .name = DRV_NAME,
>                                   .owner = THIS_MODULE,
> --
> 1.6.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 6/6] sata_mv: add power management support for the PCI controllers.

[Grant Grundler]

On Sun, Dec 6, 2009 at 8:26 AM, Saeed Bishara <saeed@marvell.com> wrote:
> Signed-off-by: Saeed Bishara <saeed@marvell.com>
> ---
>  drivers/ata/sata_mv.c |   29 +++++++++++++++++++++++++++++
>  1 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index 71ac62e..704d86f 100755
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -4161,6 +4161,9 @@ static struct platform_driver mv_platform_driver = {
>  #ifdef CONFIG_PCI
>  static int mv_pci_init_one(struct pci_dev *pdev,
>                           const struct pci_device_id *ent);
> +#ifdef CONFIG_PM
> +static int mv_pci_device_resume(struct pci_dev *pdev);
> +#endif
>
>
>  static struct pci_driver mv_pci_driver = {
> @@ -4168,6 +4171,11 @@ static struct pci_driver mv_pci_driver = {
>        .id_table               = mv_pci_tbl,
>        .probe                  = mv_pci_init_one,
>        .remove                 = ata_pci_remove_one,
> +#ifdef CONFIG_PM
> +       .suspend                = ata_pci_device_suspend,
> +       .resume                 = mv_pci_device_resume,
> +#endif

Saeed,
Apologies - Ignore my previous comment on the mv_platform_driver() patch.
I should have been looking at this patch.

And interesting to see that sata_via is the only SATA driver setting
ata_pci_device_resume() for it's .resume call. The rest of the callers
are PATA drivers.

Reviewed-by: Grant Grundler <grundler@google.com>

thanks,
grant

> +
>  };
>
>  /* move to PCI layer or libata core? */
> @@ -4324,6 +4332,27 @@ static int mv_pci_init_one(struct pci_dev *pdev,
>        return ata_host_activate(host, pdev->irq, mv_interrupt, IRQF_SHARED,
>                                 IS_GEN_I(hpriv) ? &mv5_sht : &mv6_sht);
>  }
> +
> +#ifdef CONFIG_PM
> +static int mv_pci_device_resume(struct pci_dev *pdev)
> +{
> +       struct ata_host *host = dev_get_drvdata(&pdev->dev);
> +       int rc;
> +
> +       rc = ata_pci_device_do_resume(pdev);
> +       if (rc)
> +               return rc;
> +
> +       /* initialize adapter */
> +       rc = mv_init_host(host);
> +       if (rc)
> +               return rc;
> +
> +       ata_host_resume(host);
> +
> +       return 0;
> +}
> +#endif
>  #endif
>
>  static int mv_platform_probe(struct platform_device *pdev);
> --
> 1.6.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 1/6] sata_mv: increase PIO IORDY timeout

[Jeff Garzik]

On 12/06/2009 11:26 AM, Saeed Bishara wrote:
> The old value (0xbc) in cycles of the IORDY timeout is suitable for
> devices with core clock of 166 MHz, but some SoC controllers have
> faster core clocks. The new value will make the IORDY timeout large
> enough also for all SoC devices.
>
> Signed-off-by: Saeed Bishara<saeed@marvell.com>
> ---
>   drivers/ata/sata_mv.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)

applied 1-4

Re: [PATCH 5/6] sata_mv: store the board_idx into the host private data

[Jeff Garzik]

On 12/06/2009 11:26 AM, Saeed Bishara wrote:
> This information will be used int the resume function.
>
> Signed-off-by: Saeed Bishara<saeed@marvell.com>

applied, after fixing build breakage:  the platform resume function was 
not updated by this patch, for the new mv_init_host() signature

Re: [PATCH 6/6] sata_mv: add power management support for the PCI controllers.

[Jeff Garzik]

On 12/06/2009 11:26 AM, Saeed Bishara wrote:
> Signed-off-by: Saeed Bishara<saeed@marvell.com>
> ---
>   drivers/ata/sata_mv.c |   29 +++++++++++++++++++++++++++++
>   1 files changed, 29 insertions(+), 0 deletions(-)

applied