[PATCH 02/11] ata: fix sparse warning in sata_promise.c

[PATCH 02/11] ata: fix sparse warning in sata_promise.c

[Harvey Harrison]

drivers/ata/sata_promise.c:546:15: warning: symbol 'len' shadows an earlier one
drivers/ata/sata_promise.c:538:6: originally declared here

len is set again immediately after the loop, so this is safe.

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 drivers/ata/sata_promise.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/ata/sata_promise.c b/drivers/ata/sata_promise.c
index a07d319..f251a5f 100644
--- a/drivers/ata/sata_promise.c
+++ b/drivers/ata/sata_promise.c
@@ -543,7 +543,7 @@ static void pdc_fill_sg(struct ata_queued_cmd *qc)
 	idx = 0;
 	for_each_sg(qc->sg, sg, qc->n_elem, si) {
 		u32 addr, offset;
-		u32 sg_len, len;
+		u32 sg_len;
 
 		/* determine if physical DMA addr spans 64K boundary.
 		 * Note h/w doesn't support 64-bit, so we unconditionally
-- 
1.5.4.1.1278.gc75be

[PATCH 03/11] ata: fix sparse warning in sata_via.c

[Harvey Harrison]

drivers/ata/sata_via.c:336:2: warning: returning void-valued expression

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 drivers/ata/sata_via.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/sata_via.c b/drivers/ata/sata_via.c
index 30caa03..0d03f44 100644
--- a/drivers/ata/sata_via.c
+++ b/drivers/ata/sata_via.c
@@ -333,8 +333,8 @@ static int vt6420_prereset(struct ata_link *link, unsigned long deadline)
 
 static void vt6420_error_handler(struct ata_port *ap)
 {
-	return ata_bmdma_drive_eh(ap, vt6420_prereset, ata_std_softreset,
-				  NULL, ata_std_postreset);
+	ata_bmdma_drive_eh(ap, vt6420_prereset, ata_std_softreset, NULL,
+			   ata_std_postreset);
 }
 
 static int vt6421_pata_cable_detect(struct ata_port *ap)
-- 
1.5.4.1.1278.gc75be

[PATCH 04/11] ata: fix sparse warnings in sata_mv.c

[Harvey Harrison]

pp is never used again in this function, no need to declare a
new one.

drivers/ata/sata_mv.c:1545:24: warning: symbol 'pp' shadows an earlier one
drivers/ata/sata_mv.c:1501:22: originally declared here
drivers/ata/sata_mv.c:1553:24: warning: symbol 'pp' shadows an earlier one
drivers/ata/sata_mv.c:1501:22: originally declared here

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 drivers/ata/sata_mv.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 04b5717..2ecd44d 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -1542,7 +1542,7 @@ static void mv_err_intr(struct ata_port *ap, struct ata_queued_cmd *qc)
 		eh_freeze_mask = EDMA_EH_FREEZE_5;
 
 		if (edma_err_cause & EDMA_ERR_SELF_DIS_5) {
-			struct mv_port_priv *pp	= ap->private_data;
+			pp = ap->private_data;
 			pp->pp_flags &= ~MV_PP_FLAG_EDMA_EN;
 			ata_ehi_push_desc(ehi, "EDMA self-disable");
 		}
@@ -1550,7 +1550,7 @@ static void mv_err_intr(struct ata_port *ap, struct ata_queued_cmd *qc)
 		eh_freeze_mask = EDMA_EH_FREEZE;
 
 		if (edma_err_cause & EDMA_ERR_SELF_DIS) {
-			struct mv_port_priv *pp	= ap->private_data;
+			pp = ap->private_data;
 			pp->pp_flags &= ~MV_PP_FLAG_EDMA_EN;
 			ata_ehi_push_desc(ehi, "EDMA self-disable");
 		}
-- 
1.5.4.1.1278.gc75be

[PATCH 05/11] ata: replace macro with static inline in libata.h

[Harvey Harrison]

Avoid a metric ton of sparse warnings like:
drivers/ata/pata_ali.c:176:14: warning: symbol '__x' shadows an earlier one
drivers/ata/pata_ali.c:176:14: originally declared here

Due to nesting min_t macro inside max_t macro which both use a __x
identifier internally.

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 include/linux/libata.h |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/include/linux/libata.h b/include/linux/libata.h
index bc5a8d0..2845983 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -764,7 +764,11 @@ struct ata_timing {
 	unsigned short udma;		/* t2CYCTYP/2 */
 };
 
-#define FIT(v, vmin, vmax)	max_t(short, min_t(short, v, vmax), vmin)
+static inline short FIT(short v, short vmin, short vmax)
+{
+	short tmp = min_t(short, v, vmax);
+	return max_t(short, tmp, vmin);
+}
 
 extern const unsigned long sata_deb_timing_normal[];
 extern const unsigned long sata_deb_timing_hotplug[];
-- 
1.5.4.1.1278.gc75be

[PATCH 06/11] ata: sparse fixes for pata_amd.c

[Harvey Harrison]

drop return statement.
drivers/ata/pata_amd.c:149:2: warning: returning void-valued expression

Commit ce54d1616302117fa98513ae916bb3333e1c02ea pata_amd: update mode selection for NV PATAs

added the initializer for nv_mode_filter but missed deleting the previously
set mode_filter

drivers/ata/pata_amd.c:509:3: warning: Initializer entry defined twice
drivers/ata/pata_amd.c:521:3:   also defined here
drivers/ata/pata_amd.c:544:3: warning: Initializer entry defined twice
drivers/ata/pata_amd.c:556:3:   also defined here

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 drivers/ata/pata_amd.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/pata_amd.c b/drivers/ata/pata_amd.c
index ea567e2..4b8d9b5 100644
--- a/drivers/ata/pata_amd.c
+++ b/drivers/ata/pata_amd.c
@@ -146,9 +146,8 @@ static int amd_pre_reset(struct ata_link *link, unsigned long deadline)
 
 static void amd_error_handler(struct ata_port *ap)
 {
-	return ata_bmdma_drive_eh(ap, amd_pre_reset,
-				      ata_std_softreset, NULL,
-				      ata_std_postreset);
+	ata_bmdma_drive_eh(ap, amd_pre_reset, ata_std_softreset, NULL,
+			   ata_std_postreset);
 }
 
 static int amd_cable_detect(struct ata_port *ap)
@@ -506,7 +505,6 @@ static struct ata_port_operations amd133_port_ops = {
 static struct ata_port_operations nv100_port_ops = {
 	.set_piomode	= nv100_set_piomode,
 	.set_dmamode	= nv100_set_dmamode,
-	.mode_filter	= ata_pci_default_filter,
 	.tf_load	= ata_tf_load,
 	.tf_read	= ata_tf_read,
 	.check_status 	= ata_check_status,
@@ -541,7 +539,6 @@ static struct ata_port_operations nv100_port_ops = {
 static struct ata_port_operations nv133_port_ops = {
 	.set_piomode	= nv133_set_piomode,
 	.set_dmamode	= nv133_set_dmamode,
-	.mode_filter	= ata_pci_default_filter,
 	.tf_load	= ata_tf_load,
 	.tf_read	= ata_tf_read,
 	.check_status 	= ata_check_status,
-- 
1.5.4.1.1278.gc75be

[PATCH 07/11] ata: fix sparse warning in pata_cs5536.c

[Harvey Harrison]

Everybody passes in a u32...why fight it.

drivers/ata/pata_cs5536.c:124:26: warning: incorrect type in argument 3 (different signedness)
drivers/ata/pata_cs5536.c:124:26:    expected int *val
drivers/ata/pata_cs5536.c:124:26:    got unsigned int *<noident>

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 drivers/ata/pata_cs5536.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/ata/pata_cs5536.c b/drivers/ata/pata_cs5536.c
index d753e56..c71505e 100644
--- a/drivers/ata/pata_cs5536.c
+++ b/drivers/ata/pata_cs5536.c
@@ -85,7 +85,7 @@ static const u8 pci_reg[4] = {
 	PCI_IDE_CFG, PCI_IDE_DTC, PCI_IDE_CAST, PCI_IDE_ETC,
 };
 
-static inline int cs5536_read(struct pci_dev *pdev, int reg, int *val)
+static inline int cs5536_read(struct pci_dev *pdev, int reg, u32 *val)
 {
 	if (unlikely(use_msr)) {
 		u32 dummy;
-- 
1.5.4.1.1278.gc75be

[PATCH 08/11] ata: fix sparse warning in pata_jmicron.c

[Harvey Harrison]

drivers/ata/pata_jmicron.c:118:2: warning: returning void-valued expression

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 drivers/ata/pata_jmicron.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/ata/pata_jmicron.c b/drivers/ata/pata_jmicron.c
index 5b8174d..00bbbbd 100644
--- a/drivers/ata/pata_jmicron.c
+++ b/drivers/ata/pata_jmicron.c
@@ -115,7 +115,8 @@ static int jmicron_pre_reset(struct ata_link *link, unsigned long deadline)
 
 static void jmicron_error_handler(struct ata_port *ap)
 {
-	return ata_bmdma_drive_eh(ap, jmicron_pre_reset, ata_std_softreset, NULL, ata_std_postreset);
+	ata_bmdma_drive_eh(ap, jmicron_pre_reset, ata_std_softreset, NULL,
+			   ata_std_postreset);
 }
 
 /* No PIO or DMA methods needed for this device */
-- 
1.5.4.1.1278.gc75be

[PATCH 09/11] ata: fix sparse warning in pata_marvell.c

[Harvey Harrison]

drivers/ata/pata_marvell.c:88:2: warning: returning void-valued expression

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 drivers/ata/pata_marvell.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/pata_marvell.c b/drivers/ata/pata_marvell.c
index 9afc8a3..a81f25d 100644
--- a/drivers/ata/pata_marvell.c
+++ b/drivers/ata/pata_marvell.c
@@ -85,8 +85,8 @@ static int marvell_cable_detect(struct ata_port *ap)
 
 static void marvell_error_handler(struct ata_port *ap)
 {
-	return ata_bmdma_drive_eh(ap, marvell_pre_reset, ata_std_softreset,
-				  NULL, ata_std_postreset);
+	ata_bmdma_drive_eh(ap, marvell_pre_reset, ata_std_softreset, NULL,
+			   ata_std_postreset);
 }
 
 /* No PIO or DMA methods needed for this device */
-- 
1.5.4.1.1278.gc75be

[PATCH 10/11] ata: fix sparse warning in pata_acpi.c

[Harvey Harrison]

drivers/ata/pata_acpi.c:80:2: warning: returning void-valued expression

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 drivers/ata/pata_acpi.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/pata_acpi.c b/drivers/ata/pata_acpi.c
index 244098a..bdc3b9d 100644
--- a/drivers/ata/pata_acpi.c
+++ b/drivers/ata/pata_acpi.c
@@ -77,8 +77,8 @@ static int pacpi_cable_detect(struct ata_port *ap)
 
 static void pacpi_error_handler(struct ata_port *ap)
 {
-	return ata_bmdma_drive_eh(ap, pacpi_pre_reset, ata_std_softreset,
-				  NULL, ata_std_postreset);
+	ata_bmdma_drive_eh(ap, pacpi_pre_reset, ata_std_softreset, NULL,
+			   ata_std_postreset);
 }
 
 /**
-- 
1.5.4.1.1278.gc75be

[PATCH 11/11] ata: fix sparse warnings in pata_legacy.c

[Harvey Harrison]

Let's use ld for legacy_data instead of shadowing these static
int variables.

drivers/ata/pata_legacy.c:777:21: warning: symbol 'qdi' shadows an earlier one
drivers/ata/pata_legacy.c:128:12: originally declared here
drivers/ata/pata_legacy.c:811:21: warning: symbol 'qdi' shadows an earlier one
drivers/ata/pata_legacy.c:128:12: originally declared here
drivers/ata/pata_legacy.c:848:21: warning: symbol 'qdi' shadows an earlier one
drivers/ata/pata_legacy.c:128:12: originally declared here
drivers/ata/pata_legacy.c:882:21: warning: symbol 'qdi' shadows an earlier one
drivers/ata/pata_legacy.c:128:12: originally declared here
drivers/ata/pata_legacy.c:1040:21: warning: symbol 'winbond' shadows an earlier one
drivers/ata/pata_legacy.c:129:12: originally declared here

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 drivers/ata/pata_legacy.c |   44 ++++++++++++++++++++++----------------------
 1 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
index 6c59969..1c598e5 100644
--- a/drivers/ata/pata_legacy.c
+++ b/drivers/ata/pata_legacy.c
@@ -774,14 +774,14 @@ static struct ata_port_operations opti82c46x_port_ops = {
 static void qdi6500_set_piomode(struct ata_port *ap, struct ata_device *adev)
 {
 	struct ata_timing t;
-	struct legacy_data *qdi = ap->host->private_data;
+	struct legacy_data *ld = ap->host->private_data;
 	int active, recovery;
 	u8 timing;
 
 	/* Get the timing data in cycles */
 	ata_timing_compute(adev, adev->pio_mode, &t, 30303, 1000);
 
-	if (qdi->fast) {
+	if (ld->fast) {
 		active = 8 - FIT(t.active, 1, 8);
 		recovery = 18 - FIT(t.recover, 3, 18);
 	} else {
@@ -790,9 +790,9 @@ static void qdi6500_set_piomode(struct ata_port *ap, struct ata_device *adev)
 	}
 	timing = (recovery << 4) | active | 0x08;
 
-	qdi->clock[adev->devno] = timing;
+	ld->clock[adev->devno] = timing;
 
-	outb(timing, qdi->timing);
+	outb(timing, ld->timing);
 }
 
 /**
@@ -808,14 +808,14 @@ static void qdi6500_set_piomode(struct ata_port *ap, struct ata_device *adev)
 static void qdi6580dp_set_piomode(struct ata_port *ap, struct ata_device *adev)
 {
 	struct ata_timing t;
-	struct legacy_data *qdi = ap->host->private_data;
+	struct legacy_data *ld = ap->host->private_data;
 	int active, recovery;
 	u8 timing;
 
 	/* Get the timing data in cycles */
 	ata_timing_compute(adev, adev->pio_mode, &t, 30303, 1000);
 
-	if (qdi->fast) {
+	if (ld->fast) {
 		active = 8 - FIT(t.active, 1, 8);
 		recovery = 18 - FIT(t.recover, 3, 18);
 	} else {
@@ -824,12 +824,12 @@ static void qdi6580dp_set_piomode(struct ata_port *ap, struct ata_device *adev)
 	}
 	timing = (recovery << 4) | active | 0x08;
 
-	qdi->clock[adev->devno] = timing;
+	ld->clock[adev->devno] = timing;
 
-	outb(timing, qdi->timing + 2 * ap->port_no);
+	outb(timing, ld->timing + 2 * ap->port_no);
 	/* Clear the FIFO */
 	if (adev->class != ATA_DEV_ATA)
-		outb(0x5F, qdi->timing + 3);
+		outb(0x5F, ld->timing + 3);
 }
 
 /**
@@ -845,14 +845,14 @@ static void qdi6580dp_set_piomode(struct ata_port *ap, struct ata_device *adev)
 static void qdi6580_set_piomode(struct ata_port *ap, struct ata_device *adev)
 {
 	struct ata_timing t;
-	struct legacy_data *qdi = ap->host->private_data;
+	struct legacy_data *ld = ap->host->private_data;
 	int active, recovery;
 	u8 timing;
 
 	/* Get the timing data in cycles */
 	ata_timing_compute(adev, adev->pio_mode, &t, 30303, 1000);
 
-	if (qdi->fast) {
+	if (ld->fast) {
 		active = 8 - FIT(t.active, 1, 8);
 		recovery = 18 - FIT(t.recover, 3, 18);
 	} else {
@@ -860,11 +860,11 @@ static void qdi6580_set_piomode(struct ata_port *ap, struct ata_device *adev)
 		recovery = 15 - FIT(t.recover, 0, 15);
 	}
 	timing = (recovery << 4) | active | 0x08;
-	qdi->clock[adev->devno] = timing;
-	outb(timing, qdi->timing + 2 * adev->devno);
+	ld->clock[adev->devno] = timing;
+	outb(timing, ld->timing + 2 * adev->devno);
 	/* Clear the FIFO */
 	if (adev->class != ATA_DEV_ATA)
-		outb(0x5F, qdi->timing + 3);
+		outb(0x5F, ld->timing + 3);
 }
 
 /**
@@ -879,12 +879,12 @@ static unsigned int qdi_qc_issue_prot(struct ata_queued_cmd *qc)
 {
 	struct ata_port *ap = qc->ap;
 	struct ata_device *adev = qc->dev;
-	struct legacy_data *qdi = ap->host->private_data;
+	struct legacy_data *ld = ap->host->private_data;
 
-	if (qdi->clock[adev->devno] != qdi->last) {
+	if (ld->clock[adev->devno] != ld->last) {
 		if (adev->pio_mode) {
-			qdi->last = qdi->clock[adev->devno];
-			outb(qdi->clock[adev->devno], qdi->timing +
+			ld->last = ld->clock[adev->devno];
+			outb(ld->clock[adev->devno], ld->timing +
 							2 * ap->port_no);
 		}
 	}
@@ -1037,12 +1037,12 @@ static u8 winbond_readcfg(unsigned long port, u8 reg)
 static void winbond_set_piomode(struct ata_port *ap, struct ata_device *adev)
 {
 	struct ata_timing t;
-	struct legacy_data *winbond = ap->host->private_data;
+	struct legacy_data *ld = ap->host->private_data;
 	int active, recovery;
 	u8 reg;
 	int timing = 0x88 + (ap->port_no * 4) + (adev->devno * 2);
 
-	reg = winbond_readcfg(winbond->timing, 0x81);
+	reg = winbond_readcfg(ld->timing, 0x81);
 
 	/* Get the timing data in cycles */
 	if (reg & 0x40)		/* Fast VLB bus, assume 50MHz */
@@ -1053,7 +1053,7 @@ static void winbond_set_piomode(struct ata_port *ap, struct ata_device *adev)
 	active = (FIT(t.active, 3, 17) - 1) & 0x0F;
 	recovery = (FIT(t.recover, 1, 15) + 1) & 0x0F;
 	timing = (active << 4) | recovery;
-	winbond_writecfg(winbond->timing, timing, reg);
+	winbond_writecfg(ld->timing, timing, reg);
 
 	/* Load the setup timing */
 
@@ -1063,7 +1063,7 @@ static void winbond_set_piomode(struct ata_port *ap, struct ata_device *adev)
 	if (!ata_pio_need_iordy(adev))
 		reg |= 0x02;	/* IORDY off */
 	reg |= (FIT(t.setup, 0, 3) << 6);
-	winbond_writecfg(winbond->timing, timing + 1, reg);
+	winbond_writecfg(ld->timing, timing + 1, reg);
 }
 
 static int winbond_port(struct platform_device *dev,
-- 
1.5.4.1.1278.gc75be

Re: [PATCH 05/11] ata: replace macro with static inline in libata.h

[Alan Cox]

On Wed, 13 Feb 2008 21:14:18 -0800
Harvey Harrison <harvey.harrison@gmail.com> wrote:

> Avoid a metric ton of sparse warnings like:
> drivers/ata/pata_ali.c:176:14: warning: symbol '__x' shadows an earlier one
> drivers/ata/pata_ali.c:176:14: originally declared here

NAK that appears to be a sparse bug.

1-4/6-10 Acked-by: Alan Cox <alan@redhat.com>

Alan

Re: [PATCH 11/11] ata: fix sparse warnings in pata_legacy.c

[Alan Cox]

On Wed, 13 Feb 2008 21:14:32 -0800
Harvey Harrison <harvey.harrison@gmail.com> wrote:

> Let's use ld for legacy_data instead of shadowing these static
> int variables.

NAK - I purposefully used names that indicate what device the private
data is being used for in order to make it clear. Changing qdi and
winbond to ld_qdi and ld_winbond would avoid that and fix the shadowing
of the static value.

Alan

[PATCH 11/11v2] ata: fix sparse warnings in pata_legacy.c

[Harvey Harrison]

Use ld_qdi and ld_winbond to avoid shadowing static int
variables qdi and winbond.  The ld_ prefix refers to
legacy_data.

drivers/ata/pata_legacy.c:777:21: warning: symbol 'qdi' shadows an earlier one
drivers/ata/pata_legacy.c:128:12: originally declared here
drivers/ata/pata_legacy.c:811:21: warning: symbol 'qdi' shadows an earlier one
drivers/ata/pata_legacy.c:128:12: originally declared here
drivers/ata/pata_legacy.c:848:21: warning: symbol 'qdi' shadows an earlier one
drivers/ata/pata_legacy.c:128:12: originally declared here
drivers/ata/pata_legacy.c:882:21: warning: symbol 'qdi' shadows an earlier one
drivers/ata/pata_legacy.c:128:12: originally declared here
drivers/ata/pata_legacy.c:1040:21: warning: symbol 'winbond' shadows an earlier one
drivers/ata/pata_legacy.c:129:12: originally declared here

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
Alan Cox's suggestion of using a ld_ prefix incorporated.

 drivers/ata/pata_legacy.c |   44 ++++++++++++++++++++++----------------------
 1 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
index 6c59969..68077f2 100644
--- a/drivers/ata/pata_legacy.c
+++ b/drivers/ata/pata_legacy.c
@@ -774,14 +774,14 @@ static struct ata_port_operations opti82c46x_port_ops = {
 static void qdi6500_set_piomode(struct ata_port *ap, struct ata_device *adev)
 {
 	struct ata_timing t;
-	struct legacy_data *qdi = ap->host->private_data;
+	struct legacy_data *ld_qdi = ap->host->private_data;
 	int active, recovery;
 	u8 timing;
 
 	/* Get the timing data in cycles */
 	ata_timing_compute(adev, adev->pio_mode, &t, 30303, 1000);
 
-	if (qdi->fast) {
+	if (ld_qdi->fast) {
 		active = 8 - FIT(t.active, 1, 8);
 		recovery = 18 - FIT(t.recover, 3, 18);
 	} else {
@@ -790,9 +790,9 @@ static void qdi6500_set_piomode(struct ata_port *ap, struct ata_device *adev)
 	}
 	timing = (recovery << 4) | active | 0x08;
 
-	qdi->clock[adev->devno] = timing;
+	ld_qdi->clock[adev->devno] = timing;
 
-	outb(timing, qdi->timing);
+	outb(timing, ld_qdi->timing);
 }
 
 /**
@@ -808,14 +808,14 @@ static void qdi6500_set_piomode(struct ata_port *ap, struct ata_device *adev)
 static void qdi6580dp_set_piomode(struct ata_port *ap, struct ata_device *adev)
 {
 	struct ata_timing t;
-	struct legacy_data *qdi = ap->host->private_data;
+	struct legacy_data *ld_qdi = ap->host->private_data;
 	int active, recovery;
 	u8 timing;
 
 	/* Get the timing data in cycles */
 	ata_timing_compute(adev, adev->pio_mode, &t, 30303, 1000);
 
-	if (qdi->fast) {
+	if (ld_qdi->fast) {
 		active = 8 - FIT(t.active, 1, 8);
 		recovery = 18 - FIT(t.recover, 3, 18);
 	} else {
@@ -824,12 +824,12 @@ static void qdi6580dp_set_piomode(struct ata_port *ap, struct ata_device *adev)
 	}
 	timing = (recovery << 4) | active | 0x08;
 
-	qdi->clock[adev->devno] = timing;
+	ld_qdi->clock[adev->devno] = timing;
 
-	outb(timing, qdi->timing + 2 * ap->port_no);
+	outb(timing, ld_qdi->timing + 2 * ap->port_no);
 	/* Clear the FIFO */
 	if (adev->class != ATA_DEV_ATA)
-		outb(0x5F, qdi->timing + 3);
+		outb(0x5F, ld_qdi->timing + 3);
 }
 
 /**
@@ -845,14 +845,14 @@ static void qdi6580dp_set_piomode(struct ata_port *ap, struct ata_device *adev)
 static void qdi6580_set_piomode(struct ata_port *ap, struct ata_device *adev)
 {
 	struct ata_timing t;
-	struct legacy_data *qdi = ap->host->private_data;
+	struct legacy_data *ld_qdi = ap->host->private_data;
 	int active, recovery;
 	u8 timing;
 
 	/* Get the timing data in cycles */
 	ata_timing_compute(adev, adev->pio_mode, &t, 30303, 1000);
 
-	if (qdi->fast) {
+	if (ld_qdi->fast) {
 		active = 8 - FIT(t.active, 1, 8);
 		recovery = 18 - FIT(t.recover, 3, 18);
 	} else {
@@ -860,11 +860,11 @@ static void qdi6580_set_piomode(struct ata_port *ap, struct ata_device *adev)
 		recovery = 15 - FIT(t.recover, 0, 15);
 	}
 	timing = (recovery << 4) | active | 0x08;
-	qdi->clock[adev->devno] = timing;
-	outb(timing, qdi->timing + 2 * adev->devno);
+	ld_qdi->clock[adev->devno] = timing;
+	outb(timing, ld_qdi->timing + 2 * adev->devno);
 	/* Clear the FIFO */
 	if (adev->class != ATA_DEV_ATA)
-		outb(0x5F, qdi->timing + 3);
+		outb(0x5F, ld_qdi->timing + 3);
 }
 
 /**
@@ -879,12 +879,12 @@ static unsigned int qdi_qc_issue_prot(struct ata_queued_cmd *qc)
 {
 	struct ata_port *ap = qc->ap;
 	struct ata_device *adev = qc->dev;
-	struct legacy_data *qdi = ap->host->private_data;
+	struct legacy_data *ld_qdi = ap->host->private_data;
 
-	if (qdi->clock[adev->devno] != qdi->last) {
+	if (ld_qdi->clock[adev->devno] != ld_qdi->last) {
 		if (adev->pio_mode) {
-			qdi->last = qdi->clock[adev->devno];
-			outb(qdi->clock[adev->devno], qdi->timing +
+			ld_qdi->last = ld_qdi->clock[adev->devno];
+			outb(ld_qdi->clock[adev->devno], ld_qdi->timing +
 							2 * ap->port_no);
 		}
 	}
@@ -1037,12 +1037,12 @@ static u8 winbond_readcfg(unsigned long port, u8 reg)
 static void winbond_set_piomode(struct ata_port *ap, struct ata_device *adev)
 {
 	struct ata_timing t;
-	struct legacy_data *winbond = ap->host->private_data;
+	struct legacy_data *ld_winbond = ap->host->private_data;
 	int active, recovery;
 	u8 reg;
 	int timing = 0x88 + (ap->port_no * 4) + (adev->devno * 2);
 
-	reg = winbond_readcfg(winbond->timing, 0x81);
+	reg = winbond_readcfg(ld_winbond->timing, 0x81);
 
 	/* Get the timing data in cycles */
 	if (reg & 0x40)		/* Fast VLB bus, assume 50MHz */
@@ -1053,7 +1053,7 @@ static void winbond_set_piomode(struct ata_port *ap, struct ata_device *adev)
 	active = (FIT(t.active, 3, 17) - 1) & 0x0F;
 	recovery = (FIT(t.recover, 1, 15) + 1) & 0x0F;
 	timing = (active << 4) | recovery;
-	winbond_writecfg(winbond->timing, timing, reg);
+	winbond_writecfg(ld_winbond->timing, timing, reg);
 
 	/* Load the setup timing */
 
@@ -1063,7 +1063,7 @@ static void winbond_set_piomode(struct ata_port *ap, struct ata_device *adev)
 	if (!ata_pio_need_iordy(adev))
 		reg |= 0x02;	/* IORDY off */
 	reg |= (FIT(t.setup, 0, 3) << 6);
-	winbond_writecfg(winbond->timing, timing + 1, reg);
+	winbond_writecfg(ld_winbond->timing, timing + 1, reg);
 }
 
 static int winbond_port(struct platform_device *dev,
-- 
1.5.4.1.1278.gc75be

Re: [PATCH 05/11] ata: replace macro with static inline in libata.h

[Harvey Harrison]

On Thu, 2008-02-14 at 12:01 +0000, Alan Cox wrote:
> On Wed, 13 Feb 2008 21:14:18 -0800
> Harvey Harrison <harvey.harrison@gmail.com> wrote:
> 
> > Avoid a metric ton of sparse warnings like:
> > drivers/ata/pata_ali.c:176:14: warning: symbol '__x' shadows an earlier one
> > drivers/ata/pata_ali.c:176:14: originally declared here
> 
> NAK that appears to be a sparse bug.
> 
> 1-4/6-10 Acked-by: Alan Cox <alan@redhat.com>
> 

No worries, for something like this it had better be obviously correct
or not applied.  I will try to come up with something more palatable.

Harvey

Re: [PATCH 11/11v2] ata: fix sparse warnings in pata_legacy.c

[Alan Cox]

On Thu, 14 Feb 2008 09:36:32 -0800
Harvey Harrison <harvey.harrison@gmail.com> wrote:

> Use ld_qdi and ld_winbond to avoid shadowing static int
> variables qdi and winbond.  The ld_ prefix refers to
> legacy_data.
> 
> drivers/ata/pata_legacy.c:777:21: warning: symbol 'qdi' shadows an earlier one
> drivers/ata/pata_legacy.c:128:12: originally declared here
> drivers/ata/pata_legacy.c:811:21: warning: symbol 'qdi' shadows an earlier one
> drivers/ata/pata_legacy.c:128:12: originally declared here
> drivers/ata/pata_legacy.c:848:21: warning: symbol 'qdi' shadows an earlier one
> drivers/ata/pata_legacy.c:128:12: originally declared here
> drivers/ata/pata_legacy.c:882:21: warning: symbol 'qdi' shadows an earlier one
> drivers/ata/pata_legacy.c:128:12: originally declared here
> drivers/ata/pata_legacy.c:1040:21: warning: symbol 'winbond' shadows an earlier one
> drivers/ata/pata_legacy.c:129:12: originally declared here
> 
> Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>

Acked-by: Alan Cox <alan@redhat.com>

[PATCH 05/11v2] ata: replace macro with static inline in libata.h

[Harvey Harrison]

Instead of using min/max_t(short... use a static inline to get
typechecking.

As a bonus, avoid a metric ton of sparse warnings like:
drivers/ata/pata_ali.c:176:14: warning: symbol '__x' shadows an earlier one
drivers/ata/pata_ali.c:176:14: originally declared here

Due to nesting min_t macro inside max_t macro which both use a __x
identifier internally.

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
Alan, is this more to your liking?

 include/linux/libata.h |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/include/linux/libata.h b/include/linux/libata.h
index bc5a8d0..b5590fb 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -764,7 +764,14 @@ struct ata_timing {
 	unsigned short udma;		/* t2CYCTYP/2 */
 };
 
-#define FIT(v, vmin, vmax)	max_t(short, min_t(short, v, vmax), vmin)
+static inline short FIT(short v, short vmin, short vmax)
+{
+	if (v >= vmax)
+		return vmax;
+	if (v <= vmin)
+		return vmin;
+	return v;
+}
 
 extern const unsigned long sata_deb_timing_normal[];
 extern const unsigned long sata_deb_timing_hotplug[];
-- 
1.5.4.1.1278.gc75be

Re: [PATCH 09/11] ata: fix sparse warning in pata_marvell.c

[Jan Engelhardt]

On Feb 13 2008 21:14, Harvey Harrison wrote:
>drivers/ata/pata_marvell.c:88:2: warning: returning void-valued expression

This is interesting. Why did not gcc warn us about this?

(foo.c:3: warning: "return" with a value, in function returning void)

Re: [PATCH 06/11] ata: sparse fixes for pata_amd.c

[Jeff Garzik]

Harvey Harrison wrote:
> drop return statement.
> drivers/ata/pata_amd.c:149:2: warning: returning void-valued expression
> 
> Commit ce54d1616302117fa98513ae916bb3333e1c02ea pata_amd: update mode selection for NV PATAs
> 
> added the initializer for nv_mode_filter but missed deleting the previously
> set mode_filter
> 
> drivers/ata/pata_amd.c:509:3: warning: Initializer entry defined twice
> drivers/ata/pata_amd.c:521:3:   also defined here
> drivers/ata/pata_amd.c:544:3: warning: Initializer entry defined twice
> drivers/ata/pata_amd.c:556:3:   also defined here
> 
> Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
> ---
>  drivers/ata/pata_amd.c |    7 ++-----
>  1 files changed, 2 insertions(+), 5 deletions(-)

applied 6-10

Re: [PATCH 11/11v2] ata: fix sparse warnings in pata_legacy.c

[Jeff Garzik]

Harvey Harrison wrote:
> Use ld_qdi and ld_winbond to avoid shadowing static int
> variables qdi and winbond.  The ld_ prefix refers to
> legacy_data.
> 
> drivers/ata/pata_legacy.c:777:21: warning: symbol 'qdi' shadows an earlier one
> drivers/ata/pata_legacy.c:128:12: originally declared here
> drivers/ata/pata_legacy.c:811:21: warning: symbol 'qdi' shadows an earlier one
> drivers/ata/pata_legacy.c:128:12: originally declared here
> drivers/ata/pata_legacy.c:848:21: warning: symbol 'qdi' shadows an earlier one
> drivers/ata/pata_legacy.c:128:12: originally declared here
> drivers/ata/pata_legacy.c:882:21: warning: symbol 'qdi' shadows an earlier one
> drivers/ata/pata_legacy.c:128:12: originally declared here
> drivers/ata/pata_legacy.c:1040:21: warning: symbol 'winbond' shadows an earlier one
> drivers/ata/pata_legacy.c:129:12: originally declared here
> 
> Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
> ---
> Alan Cox's suggestion of using a ld_ prefix incorporated.

applied