[PATCH v1 1/4] sata_dwc_460ex: fix resource leak on error path

[PATCH v1 1/4] sata_dwc_460ex: fix resource leak on error path

[Andy Shevchenko]

DMA mapped IO should be unmapped on the error path in probe() and
unconditionally on remove().

Fixes: 62936009f35a ([libata] Add 460EX on-chip SATA driver, sata_dwc_460ex)
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/ata/sata_dwc_460ex.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
index c7ddef8..8e824817 100644
--- a/drivers/ata/sata_dwc_460ex.c
+++ b/drivers/ata/sata_dwc_460ex.c
@@ -797,7 +797,7 @@ static int dma_dwc_init(struct sata_dwc_device *hsdev, int irq)
 	if (err) {
 		dev_err(host_pvt.dwc_dev, "%s: dma_request_interrupts returns"
 			" %d\n", __func__, err);
-		goto error_out;
+		return err;
 	}
 
 	/* Enabe DMA */
@@ -808,11 +808,6 @@ static int dma_dwc_init(struct sata_dwc_device *hsdev, int irq)
 		sata_dma_regs);
 
 	return 0;
-
-error_out:
-	dma_dwc_exit(hsdev);
-
-	return err;
 }
 
 static int sata_dwc_scr_read(struct ata_link *link, unsigned int scr, u32 *val)
@@ -1662,7 +1657,7 @@ static int sata_dwc_probe(struct platform_device *ofdev)
 	char *ver = (char *)&versionr;
 	u8 *base = NULL;
 	int err = 0;
-	int irq, rc;
+	int irq;
 	struct ata_host *host;
 	struct ata_port_info pi = sata_dwc_port_info[0];
 	const struct ata_port_info *ppi[] = { &pi, NULL };
@@ -1725,7 +1720,7 @@ static int sata_dwc_probe(struct platform_device *ofdev)
 	if (irq == NO_IRQ) {
 		dev_err(&ofdev->dev, "no SATA DMA irq\n");
 		err = -ENODEV;
-		goto error_out;
+		goto error_iomap;
 	}
 
 	/* Get physical SATA DMA register base address */
@@ -1734,14 +1729,16 @@ static int sata_dwc_probe(struct platform_device *ofdev)
 		dev_err(&ofdev->dev, "ioremap failed for AHBDMA register"
 			" address\n");
 		err = -ENODEV;
-		goto error_out;
+		goto error_iomap;
 	}
 
 	/* Save dev for later use in dev_xxx() routines */
 	host_pvt.dwc_dev = &ofdev->dev;
 
 	/* Initialize AHB DMAC */
-	dma_dwc_init(hsdev, irq);
+	err = dma_dwc_init(hsdev, irq);
+	if (err)
+		goto error_dma_iomap;
 
 	/* Enable SATA Interrupts */
 	sata_dwc_enable_interrupts(hsdev);
@@ -1759,9 +1756,8 @@ static int sata_dwc_probe(struct platform_device *ofdev)
 	 * device discovery process, invoking our port_start() handler &
 	 * error_handler() to execute a dummy Softreset EH session
 	 */
-	rc = ata_host_activate(host, irq, sata_dwc_isr, 0, &sata_dwc_sht);
-
-	if (rc != 0)
+	err = ata_host_activate(host, irq, sata_dwc_isr, 0, &sata_dwc_sht);
+	if (err)
 		dev_err(&ofdev->dev, "failed to activate host");
 
 	dev_set_drvdata(&ofdev->dev, host);
@@ -1770,7 +1766,8 @@ static int sata_dwc_probe(struct platform_device *ofdev)
 error_out:
 	/* Free SATA DMA resources */
 	dma_dwc_exit(hsdev);
-
+error_dma_iomap:
+	iounmap((void __iomem *)host_pvt.sata_dma_regs);
 error_iomap:
 	iounmap(base);
 error_kmalloc:
@@ -1791,6 +1788,7 @@ static int sata_dwc_remove(struct platform_device *ofdev)
 	/* Free SATA DMA resources */
 	dma_dwc_exit(hsdev);
 
+	iounmap((void __iomem *)host_pvt.sata_dma_regs);
 	iounmap(hsdev->reg_base);
 	kfree(hsdev);
 	kfree(host);
-- 
2.1.3

[PATCH v1 2/4] sata_dwc_460ex: remove redundant dev_set_drvdata

[Andy Shevchenko]

Driver core sets it to NULL upon probe failure or release.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/ata/sata_dwc_460ex.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
index 8e824817..5c34395 100644
--- a/drivers/ata/sata_dwc_460ex.c
+++ b/drivers/ata/sata_dwc_460ex.c
@@ -1783,7 +1783,6 @@ static int sata_dwc_remove(struct platform_device *ofdev)
 	struct sata_dwc_device *hsdev = host->private_data;
 
 	ata_host_detach(host);
-	dev_set_drvdata(dev, NULL);
 
 	/* Free SATA DMA resources */
 	dma_dwc_exit(hsdev);
-- 
2.1.3

[PATCH v1 3/4] sata_dwc_460ex: enable COMPILE_TEST for the driver

[Andy Shevchenko]

To test how the driver could be compiled in the non-native environment let's
enable COMPILE_TEST for it. It would be useful for further work.

This patch enables COMPILE_TEST for the driver and fixes compilation errors on
at least x86 platforms.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/ata/Kconfig          |  2 +-
 drivers/ata/sata_dwc_460ex.c | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index a3a1360..5ed20e3 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -269,7 +269,7 @@ config ATA_PIIX
 
 config SATA_DWC
 	tristate "DesignWare Cores SATA support"
-	depends on 460EX
+	depends on 460EX || COMPILE_TEST
 	help
 	  This option enables support for the on-chip SATA controller of the
 	  AppliedMicro processor 460EX.
diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
index 5c34395..c1723e0 100644
--- a/drivers/ata/sata_dwc_460ex.c
+++ b/drivers/ata/sata_dwc_460ex.c
@@ -48,6 +48,18 @@
 #define DRV_NAME        "sata-dwc"
 #define DRV_VERSION     "1.3"
 
+#ifndef out_le32
+#define out_le32(a, v)	__raw_writel(__cpu_to_le32(v), (void __iomem *)(a))
+#endif
+
+#ifndef in_le32
+#define in_le32(a)	__le32_to_cpu(__raw_readl((void __iomem *)(a)))
+#endif
+
+#ifndef NO_IRQ
+#define NO_IRQ		0
+#endif
+
 /* SATA DMA driver Globals */
 #define DMA_NUM_CHANS		1
 #define DMA_NUM_CHAN_REGS	8
-- 
2.1.3

[PATCH v1 4/4] sata_dwc_460ex: fix most of the sparse warnings

[Andy Shevchenko]

There are a lot sparse warnings. Most of them related to __iomem keyword which
sometimes absent when it's needed and vise versa.

The patch fixes most of the warnings.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/ata/sata_dwc_460ex.c | 67 ++++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 34 deletions(-)

diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
index c1723e0..6c1b649 100644
--- a/drivers/ata/sata_dwc_460ex.c
+++ b/drivers/ata/sata_dwc_460ex.c
@@ -285,7 +285,7 @@ struct sata_dwc_device {
 	struct device		*dev;		/* generic device struct */
 	struct ata_probe_ent	*pe;		/* ptr to probe-ent */
 	struct ata_host		*host;
-	u8			*reg_base;
+	u8 __iomem		*reg_base;
 	struct sata_dwc_regs	*sata_dwc_regs;	/* DW Synopsys SATA specific */
 	int			irq_dma;
 };
@@ -335,7 +335,9 @@ struct sata_dwc_host_priv {
 	struct	device	*dwc_dev;
 	int	dma_channel;
 };
-struct sata_dwc_host_priv host_pvt;
+
+static struct sata_dwc_host_priv host_pvt;
+
 /*
  * Prototypes
  */
@@ -592,9 +594,9 @@ static int map_sg_to_lli(struct scatterlist *sg, int num_elems,
 
 	sms_val = 0;
 	dms_val = 1 + host_pvt.dma_channel;
-	dev_dbg(host_pvt.dwc_dev, "%s: sg=%p nelem=%d lli=%p dma_lli=0x%08x"
-		" dmadr=0x%08x\n", __func__, sg, num_elems, lli, (u32)dma_lli,
-		(u32)dmadr_addr);
+	dev_dbg(host_pvt.dwc_dev,
+		"%s: sg=%p nelem=%d lli=%p dma_lli=0x%pad dmadr=0x%p\n",
+		__func__, sg, num_elems, lli, &dma_lli, dmadr_addr);
 
 	bl = get_burst_length_encode(AHB_DMA_BRST_DFLT);
 
@@ -785,7 +787,7 @@ static void dma_dwc_exit(struct sata_dwc_device *hsdev)
 {
 	dev_dbg(host_pvt.dwc_dev, "%s:\n", __func__);
 	if (host_pvt.sata_dma_regs) {
-		iounmap(host_pvt.sata_dma_regs);
+		iounmap((void __iomem *)host_pvt.sata_dma_regs);
 		host_pvt.sata_dma_regs = NULL;
 	}
 
@@ -830,7 +832,7 @@ static int sata_dwc_scr_read(struct ata_link *link, unsigned int scr, u32 *val)
 		return -EINVAL;
 	}
 
-	*val = in_le32((void *)link->ap->ioaddr.scr_addr + (scr * 4));
+	*val = in_le32(link->ap->ioaddr.scr_addr + (scr * 4));
 	dev_dbg(link->ap->dev, "%s: id=%d reg=%d val=val=0x%08x\n",
 		__func__, link->ap->print_id, scr, *val);
 
@@ -846,21 +848,19 @@ static int sata_dwc_scr_write(struct ata_link *link, unsigned int scr, u32 val)
 			 __func__, scr);
 		return -EINVAL;
 	}
-	out_le32((void *)link->ap->ioaddr.scr_addr + (scr * 4), val);
+	out_le32(link->ap->ioaddr.scr_addr + (scr * 4), val);
 
 	return 0;
 }
 
 static u32 core_scr_read(unsigned int scr)
 {
-	return in_le32((void __iomem *)(host_pvt.scr_addr_sstatus) +\
-			(scr * 4));
+	return in_le32(host_pvt.scr_addr_sstatus + (scr * 4));
 }
 
 static void core_scr_write(unsigned int scr, u32 val)
 {
-	out_le32((void __iomem *)(host_pvt.scr_addr_sstatus) + (scr * 4),
-		val);
+	out_le32(host_pvt.scr_addr_sstatus + (scr * 4), val);
 }
 
 static void clear_serror(void)
@@ -868,7 +868,6 @@ static void clear_serror(void)
 	u32 val;
 	val = core_scr_read(SCR_ERROR);
 	core_scr_write(SCR_ERROR, val);
-
 }
 
 static void clear_interrupt_bit(struct sata_dwc_device *hsdev, u32 bit)
@@ -1268,24 +1267,24 @@ static void sata_dwc_enable_interrupts(struct sata_dwc_device *hsdev)
 
 static void sata_dwc_setup_port(struct ata_ioports *port, unsigned long base)
 {
-	port->cmd_addr = (void *)base + 0x00;
-	port->data_addr = (void *)base + 0x00;
+	port->cmd_addr = (void __iomem *)base + 0x00;
+	port->data_addr = (void __iomem *)base + 0x00;
 
-	port->error_addr = (void *)base + 0x04;
-	port->feature_addr = (void *)base + 0x04;
+	port->error_addr = (void __iomem *)base + 0x04;
+	port->feature_addr = (void __iomem *)base + 0x04;
 
-	port->nsect_addr = (void *)base + 0x08;
+	port->nsect_addr = (void __iomem *)base + 0x08;
 
-	port->lbal_addr = (void *)base + 0x0c;
-	port->lbam_addr = (void *)base + 0x10;
-	port->lbah_addr = (void *)base + 0x14;
+	port->lbal_addr = (void __iomem *)base + 0x0c;
+	port->lbam_addr = (void __iomem *)base + 0x10;
+	port->lbah_addr = (void __iomem *)base + 0x14;
 
-	port->device_addr = (void *)base + 0x18;
-	port->command_addr = (void *)base + 0x1c;
-	port->status_addr = (void *)base + 0x1c;
+	port->device_addr = (void __iomem *)base + 0x18;
+	port->command_addr = (void __iomem *)base + 0x1c;
+	port->status_addr = (void __iomem *)base + 0x1c;
 
-	port->altstatus_addr = (void *)base + 0x20;
-	port->ctl_addr = (void *)base + 0x20;
+	port->altstatus_addr = (void __iomem *)base + 0x20;
+	port->ctl_addr = (void __iomem *)base + 0x20;
 }
 
 /*
@@ -1326,7 +1325,7 @@ static int sata_dwc_port_start(struct ata_port *ap)
 	for (i = 0; i < SATA_DWC_QCMD_MAX; i++)
 		hsdevp->cmd_issued[i] = SATA_DWC_CMD_ISSUED_NOT;
 
-	ap->bmdma_prd = 0;	/* set these so libata doesn't use them */
+	ap->bmdma_prd = NULL;	/* set these so libata doesn't use them */
 	ap->bmdma_prd_dma = 0;
 
 	/*
@@ -1523,8 +1522,8 @@ static void sata_dwc_qc_prep_by_tag(struct ata_queued_cmd *qc, u8 tag)
 
 	dma_chan = dma_dwc_xfer_setup(sg, qc->n_elem, hsdevp->llit[tag],
 				      hsdevp->llit_dma[tag],
-				      (void *__iomem)(&hsdev->sata_dwc_regs->\
-				      dmadr), qc->dma_dir);
+				      (void __iomem *)&hsdev->sata_dwc_regs->dmadr,
+				      qc->dma_dir);
 	if (dma_chan < 0) {
 		dev_err(ap->dev, "%s: dma_dwc_xfer_setup returns err %d\n",
 			__func__, dma_chan);
@@ -1597,8 +1596,8 @@ static void sata_dwc_error_handler(struct ata_port *ap)
 	ata_sff_error_handler(ap);
 }
 
-int sata_dwc_hardreset(struct ata_link *link, unsigned int *class,
-			unsigned long deadline)
+static int sata_dwc_hardreset(struct ata_link *link, unsigned int *class,
+			      unsigned long deadline)
 {
 	struct sata_dwc_device *hsdev = HSDEV_FROM_AP(link->ap);
 	int ret;
@@ -1630,7 +1629,7 @@ static struct scsi_host_template sata_dwc_sht = {
 	 * max of 1. This will get fixed in in a future release.
 	 */
 	.sg_tablesize		= LIBATA_MAX_PRD,
-	.can_queue		= ATA_DEF_QUEUE,	/* ATA_MAX_QUEUE */
+	/* .can_queue		= ATA_MAX_QUEUE, */
 	.dma_boundary		= ATA_DMA_BOUNDARY,
 };
 
@@ -1667,7 +1666,7 @@ static int sata_dwc_probe(struct platform_device *ofdev)
 	struct sata_dwc_device *hsdev;
 	u32 idr, versionr;
 	char *ver = (char *)&versionr;
-	u8 *base = NULL;
+	u8 __iomem *base;
 	int err = 0;
 	int irq;
 	struct ata_host *host;
@@ -1736,7 +1735,7 @@ static int sata_dwc_probe(struct platform_device *ofdev)
 	}
 
 	/* Get physical SATA DMA register base address */
-	host_pvt.sata_dma_regs = of_iomap(ofdev->dev.of_node, 1);
+	host_pvt.sata_dma_regs = (void *)of_iomap(ofdev->dev.of_node, 1);
 	if (!(host_pvt.sata_dma_regs)) {
 		dev_err(&ofdev->dev, "ioremap failed for AHBDMA register"
 			" address\n");
-- 
2.1.3

Re: [PATCH v1 4/4] sata_dwc_460ex: fix most of the sparse warnings

[Tejun Heo]

On Wed, Jan 07, 2015 at 03:24:22PM +0200, Andy Shevchenko wrote:
> There are a lot sparse warnings. Most of them related to __iomem keyword which
> sometimes absent when it's needed and vise versa.
> 
> The patch fixes most of the warnings.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied 1-4 to libata/for-3.19-fixes.  One nit below.

> @@ -1630,7 +1629,7 @@ static struct scsi_host_template sata_dwc_sht = {
>  	 * max of 1. This will get fixed in in a future release.
>  	 */
>  	.sg_tablesize		= LIBATA_MAX_PRD,
> -	.can_queue		= ATA_DEF_QUEUE,	/* ATA_MAX_QUEUE */
> +	/* .can_queue		= ATA_MAX_QUEUE, */

This part isn't explained by the patch description.  I added it while
applying but please make sure that the patch descriptions are
comprehensive.

Thanks!

-- 
tejun

Re: [PATCH v1 4/4] sata_dwc_460ex: fix most of the sparse warnings

[Tejun Heo]

On Wed, Jan 07, 2015 at 10:36:32AM -0500, Tejun Heo wrote:
> On Wed, Jan 07, 2015 at 03:24:22PM +0200, Andy Shevchenko wrote:
> > There are a lot sparse warnings. Most of them related to __iomem keyword which
> > sometimes absent when it's needed and vise versa.
> > 
> > The patch fixes most of the warnings.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Applied 1-4 to libata/for-3.19-fixes.  One nit below.

Moved 2-4 to libata/for-3.20.

Thanks.

-- 
tejun