[PATCH] Add cast to __iomem pointer in scsi drivers

[PATCH] Add cast to __iomem pointer in scsi drivers

[Takashi Iwai]

Add the missing cast to __iomem pointer in some scsi drivers.

Signed-off-by: Takashi Iwai <tiwai@suse.de>

---
 drivers/scsi/megaraid.c |    8 ++++----
 drivers/scsi/sata_svw.c |   41 +++++++++++++++++++++++------------------
 drivers/scsi/sata_vsc.c |   43 ++++++++++++++++++++++++-------------------
 3 files changed, 51 insertions(+), 41 deletions(-)

diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index d101a8a..13c9395 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -72,10 +72,10 @@ static unsigned short int max_mbox_busy_
 module_param(max_mbox_busy_wait, ushort, 0);
 MODULE_PARM_DESC(max_mbox_busy_wait, "Maximum wait for mailbox in microseconds if busy (default=MBOX_BUSY_WAIT=10)");
 
-#define RDINDOOR(adapter)		readl((adapter)->base + 0x20)
-#define RDOUTDOOR(adapter)		readl((adapter)->base + 0x2C)
-#define WRINDOOR(adapter,value)		writel(value, (adapter)->base + 0x20)
-#define WROUTDOOR(adapter,value)	writel(value, (adapter)->base + 0x2C)
+#define RDINDOOR(adapter)		readl((void __iomem *)((adapter)->base + 0x20))
+#define RDOUTDOOR(adapter)		readl((void __iomem *)((adapter)->base + 0x2C))
+#define WRINDOOR(adapter,value)		writel(value, (void __iomem *)((adapter)->base + 0x20))
+#define WROUTDOOR(adapter,value)	writel(value, (void __iomem *)((adapter)->base + 0x2C))
 
 /*
  * Global variables
diff --git a/drivers/scsi/sata_svw.c b/drivers/scsi/sata_svw.c
index d847256..862a3ba 100644
--- a/drivers/scsi/sata_svw.c
+++ b/drivers/scsi/sata_svw.c
@@ -110,26 +110,31 @@ static void k2_sata_tf_load(struct ata_p
 	unsigned int is_addr = tf->flags & ATA_TFLAG_ISADDR;
 
 	if (tf->ctl != ap->last_ctl) {
-		writeb(tf->ctl, ioaddr->ctl_addr);
+		writeb(tf->ctl, (void __iomem *) ioaddr->ctl_addr);
 		ap->last_ctl = tf->ctl;
 		ata_wait_idle(ap);
 	}
 	if (is_addr && (tf->flags & ATA_TFLAG_LBA48)) {
-		writew(tf->feature | (((u16)tf->hob_feature) << 8), ioaddr->feature_addr);
-		writew(tf->nsect | (((u16)tf->hob_nsect) << 8), ioaddr->nsect_addr);
-		writew(tf->lbal | (((u16)tf->hob_lbal) << 8), ioaddr->lbal_addr);
-		writew(tf->lbam | (((u16)tf->hob_lbam) << 8), ioaddr->lbam_addr);
-		writew(tf->lbah | (((u16)tf->hob_lbah) << 8), ioaddr->lbah_addr);
+		writew(tf->feature | (((u16)tf->hob_feature) << 8),
+		       (void __iomem *) ioaddr->feature_addr);
+		writew(tf->nsect | (((u16)tf->hob_nsect) << 8),
+		       (void __iomem *) ioaddr->nsect_addr);
+		writew(tf->lbal | (((u16)tf->hob_lbal) << 8),
+		       (void __iomem *) ioaddr->lbal_addr);
+		writew(tf->lbam | (((u16)tf->hob_lbam) << 8),
+		       (void __iomem *) ioaddr->lbam_addr);
+		writew(tf->lbah | (((u16)tf->hob_lbah) << 8),
+		       (void __iomem *) ioaddr->lbah_addr);
 	} else if (is_addr) {
-		writew(tf->feature, ioaddr->feature_addr);
-		writew(tf->nsect, ioaddr->nsect_addr);
-		writew(tf->lbal, ioaddr->lbal_addr);
-		writew(tf->lbam, ioaddr->lbam_addr);
-		writew(tf->lbah, ioaddr->lbah_addr);
+		writew(tf->feature, (void __iomem *) ioaddr->feature_addr);
+		writew(tf->nsect, (void __iomem *) ioaddr->nsect_addr);
+		writew(tf->lbal, (void __iomem *) ioaddr->lbal_addr);
+		writew(tf->lbam, (void __iomem *) ioaddr->lbam_addr);
+		writew(tf->lbah, (void __iomem *) ioaddr->lbah_addr);
 	}
 
 	if (tf->flags & ATA_TFLAG_DEVICE)
-		writeb(tf->device, ioaddr->device_addr);
+		writeb(tf->device, (void __iomem *) ioaddr->device_addr);
 
 	ata_wait_idle(ap);
 }
@@ -141,12 +146,12 @@ static void k2_sata_tf_read(struct ata_p
 	u16 nsect, lbal, lbam, lbah, feature;
 
 	tf->command = k2_stat_check_status(ap);
-	tf->device = readw(ioaddr->device_addr);
-	feature = readw(ioaddr->error_addr);
-	nsect = readw(ioaddr->nsect_addr);
-	lbal = readw(ioaddr->lbal_addr);
-	lbam = readw(ioaddr->lbam_addr);
-	lbah = readw(ioaddr->lbah_addr);
+	tf->device = readw((void __iomem *) ioaddr->device_addr);
+	feature = readw((void __iomem *) ioaddr->error_addr);
+	nsect = readw((void __iomem *) ioaddr->nsect_addr);
+	lbal = readw((void __iomem *) ioaddr->lbal_addr);
+	lbam = readw((void __iomem *) ioaddr->lbam_addr);
+	lbah = readw((void __iomem *) ioaddr->lbah_addr);
 
 	tf->feature = feature;
 	tf->nsect = nsect;
diff --git a/drivers/scsi/sata_vsc.c b/drivers/scsi/sata_vsc.c
index 2e2c3b7..cacacc5 100644
--- a/drivers/scsi/sata_vsc.c
+++ b/drivers/scsi/sata_vsc.c
@@ -130,21 +130,26 @@ static void vsc_sata_tf_load(struct ata_
 		vsc_intr_mask_update(ap, tf->ctl & ATA_NIEN);
 	}
 	if (is_addr && (tf->flags & ATA_TFLAG_LBA48)) {
-		writew(tf->feature | (((u16)tf->hob_feature) << 8), ioaddr->feature_addr);
-		writew(tf->nsect | (((u16)tf->hob_nsect) << 8), ioaddr->nsect_addr);
-		writew(tf->lbal | (((u16)tf->hob_lbal) << 8), ioaddr->lbal_addr);
-		writew(tf->lbam | (((u16)tf->hob_lbam) << 8), ioaddr->lbam_addr);
-		writew(tf->lbah | (((u16)tf->hob_lbah) << 8), ioaddr->lbah_addr);
+		writew(tf->feature | (((u16)tf->hob_feature) << 8),
+		       (void __iomem *) ioaddr->feature_addr);
+		writew(tf->nsect | (((u16)tf->hob_nsect) << 8),
+		       (void __iomem *) ioaddr->nsect_addr);
+		writew(tf->lbal | (((u16)tf->hob_lbal) << 8),
+		       (void __iomem *) ioaddr->lbal_addr);
+		writew(tf->lbam | (((u16)tf->hob_lbam) << 8),
+		       (void __iomem *) ioaddr->lbam_addr);
+		writew(tf->lbah | (((u16)tf->hob_lbah) << 8),
+		       (void __iomem *) ioaddr->lbah_addr);
 	} else if (is_addr) {
-		writew(tf->feature, ioaddr->feature_addr);
-		writew(tf->nsect, ioaddr->nsect_addr);
-		writew(tf->lbal, ioaddr->lbal_addr);
-		writew(tf->lbam, ioaddr->lbam_addr);
-		writew(tf->lbah, ioaddr->lbah_addr);
+		writew(tf->feature, (void __iomem *) ioaddr->feature_addr);
+		writew(tf->nsect, (void __iomem *) ioaddr->nsect_addr);
+		writew(tf->lbal, (void __iomem *) ioaddr->lbal_addr);
+		writew(tf->lbam, (void __iomem *) ioaddr->lbam_addr);
+		writew(tf->lbah, (void __iomem *) ioaddr->lbah_addr);
 	}
 
 	if (tf->flags & ATA_TFLAG_DEVICE)
-		writeb(tf->device, ioaddr->device_addr);
+		writeb(tf->device, (void __iomem *) ioaddr->device_addr);
 
 	ata_wait_idle(ap);
 }
@@ -156,12 +161,12 @@ static void vsc_sata_tf_read(struct ata_
 	u16 nsect, lbal, lbam, lbah, feature;
 
 	tf->command = ata_check_status(ap);
-	tf->device = readw(ioaddr->device_addr);
-	feature = readw(ioaddr->error_addr);
-	nsect = readw(ioaddr->nsect_addr);
-	lbal = readw(ioaddr->lbal_addr);
-	lbam = readw(ioaddr->lbam_addr);
-	lbah = readw(ioaddr->lbah_addr);
+	tf->device = readw((void __iomem *) ioaddr->device_addr);
+	feature = readw((void __iomem *) ioaddr->error_addr);
+	nsect = readw((void __iomem *) ioaddr->nsect_addr);
+	lbal = readw((void __iomem *) ioaddr->lbal_addr);
+	lbam = readw((void __iomem *) ioaddr->lbam_addr);
+	lbah = readw((void __iomem *) ioaddr->lbah_addr);
 
 	tf->feature = feature;
 	tf->nsect = nsect;
@@ -279,8 +284,8 @@ static void __devinit vsc_sata_setup_por
 	port->ctl_addr		= base + VSC_SATA_TF_CTL_OFFSET;
 	port->bmdma_addr	= base + VSC_SATA_DMA_CMD_OFFSET;
 	port->scr_addr		= base + VSC_SATA_SCR_STATUS_OFFSET;
-	writel(0, base + VSC_SATA_UP_DESCRIPTOR_OFFSET);
-	writel(0, base + VSC_SATA_UP_DATA_BUFFER_OFFSET);
+	writel(0, (void __iomem *) (base + VSC_SATA_UP_DESCRIPTOR_OFFSET));
+	writel(0, (void __iomem *) (base + VSC_SATA_UP_DATA_BUFFER_OFFSET));
 }
 
 

Re: [PATCH] Add cast to __iomem pointer in scsi drivers

[linux-os (Dick Johnson)]

On Tue, 14 Feb 2006, Takashi Iwai wrote:

> Add the missing cast to __iomem pointer in some scsi drivers.
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
>
> ---
> drivers/scsi/megaraid.c |    8 ++++----
> drivers/scsi/sata_svw.c |   41 +++++++++++++++++++++++------------------
> drivers/scsi/sata_vsc.c |   43 ++++++++++++++++++++++++-------------------
> 3 files changed, 51 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
> index d101a8a..13c9395 100644
> --- a/drivers/scsi/megaraid.c
> +++ b/drivers/scsi/megaraid.c
> @@ -72,10 +72,10 @@ static unsigned short int max_mbox_busy_
> module_param(max_mbox_busy_wait, ushort, 0);
> MODULE_PARM_DESC(max_mbox_busy_wait, "Maximum wait for mailbox in microseconds if busy (default=MBOX_BUSY_WAIT=10)");
>
> -#define RDINDOOR(adapter)		readl((adapter)->base + 0x20)
> -#define RDOUTDOOR(adapter)		readl((adapter)->base + 0x2C)
> -#define WRINDOOR(adapter,value)		writel(value, (adapter)->base + 0x20)
> -#define WROUTDOOR(adapter,value)	writel(value, (adapter)->base + 0x2C)
> +#define RDINDOOR(adapter)		readl((void __iomem *)((adapter)->base + 0x20))
> +#define RDOUTDOOR(adapter)		readl((void __iomem *)((adapter)->base + 0x2C))
> +#define WRINDOOR(adapter,value)		writel(value, (void __iomem *)((adapter)->base + 0x20))
> +#define WROUTDOOR(adapter,value)	writel(value, (void __iomem *)((adapter)->base + 0x2C))
>
> /*
>  * Global variables
> diff --git a/drivers/scsi/sata_svw.c b/drivers/scsi/sata_svw.c
> index d847256..862a3ba 100644
> --- a/drivers/scsi/sata_svw.c
> +++ b/drivers/scsi/sata_svw.c
> @@ -110,26 +110,31 @@ static void k2_sata_tf_load(struct ata_p
> 	unsigned int is_addr = tf->flags & ATA_TFLAG_ISADDR;
>
> 	if (tf->ctl != ap->last_ctl) {
> -		writeb(tf->ctl, ioaddr->ctl_addr);
> +		writeb(tf->ctl, (void __iomem *) ioaddr->ctl_addr);
> 		ap->last_ctl = tf->ctl;
> 		ata_wait_idle(ap);
> 	}
> 	if (is_addr && (tf->flags & ATA_TFLAG_LBA48)) {
> -		writew(tf->feature | (((u16)tf->hob_feature) << 8), ioaddr->feature_addr);
> -		writew(tf->nsect | (((u16)tf->hob_nsect) << 8), ioaddr->nsect_addr);
> -		writew(tf->lbal | (((u16)tf->hob_lbal) << 8), ioaddr->lbal_addr);
> -		writew(tf->lbam | (((u16)tf->hob_lbam) << 8), ioaddr->lbam_addr);
> -		writew(tf->lbah | (((u16)tf->hob_lbah) << 8), ioaddr->lbah_addr);
> +		writew(tf->feature | (((u16)tf->hob_feature) << 8),
> +		       (void __iomem *) ioaddr->feature_addr);
> +		writew(tf->nsect | (((u16)tf->hob_nsect) << 8),
> +		       (void __iomem *) ioaddr->nsect_addr);
> +		writew(tf->lbal | (((u16)tf->hob_lbal) << 8),
> +		       (void __iomem *) ioaddr->lbal_addr);
> +		writew(tf->lbam | (((u16)tf->hob_lbam) << 8),
> +		       (void __iomem *) ioaddr->lbam_addr);
> +		writew(tf->lbah | (((u16)tf->hob_lbah) << 8),
> +		       (void __iomem *) ioaddr->lbah_addr);
> 	} else if (is_addr) {
> -		writew(tf->feature, ioaddr->feature_addr);
> -		writew(tf->nsect, ioaddr->nsect_addr);
> -		writew(tf->lbal, ioaddr->lbal_addr);
> -		writew(tf->lbam, ioaddr->lbam_addr);
> -		writew(tf->lbah, ioaddr->lbah_addr);
> +		writew(tf->feature, (void __iomem *) ioaddr->feature_addr);
> +		writew(tf->nsect, (void __iomem *) ioaddr->nsect_addr);
> +		writew(tf->lbal, (void __iomem *) ioaddr->lbal_addr);
> +		writew(tf->lbam, (void __iomem *) ioaddr->lbam_addr);
> +		writew(tf->lbah, (void __iomem *) ioaddr->lbah_addr);
> 	}
>
> 	if (tf->flags & ATA_TFLAG_DEVICE)
> -		writeb(tf->device, ioaddr->device_addr);
> +		writeb(tf->device, (void __iomem *) ioaddr->device_addr);
>
> 	ata_wait_idle(ap);
> }
> @@ -141,12 +146,12 @@ static void k2_sata_tf_read(struct ata_p
> 	u16 nsect, lbal, lbam, lbah, feature;
>
> 	tf->command = k2_stat_check_status(ap);
> -	tf->device = readw(ioaddr->device_addr);
> -	feature = readw(ioaddr->error_addr);
> -	nsect = readw(ioaddr->nsect_addr);
> -	lbal = readw(ioaddr->lbal_addr);
> -	lbam = readw(ioaddr->lbam_addr);
> -	lbah = readw(ioaddr->lbah_addr);
> +	tf->device = readw((void __iomem *) ioaddr->device_addr);
> +	feature = readw((void __iomem *) ioaddr->error_addr);
> +	nsect = readw((void __iomem *) ioaddr->nsect_addr);
> +	lbal = readw((void __iomem *) ioaddr->lbal_addr);
> +	lbam = readw((void __iomem *) ioaddr->lbam_addr);
> +	lbah = readw((void __iomem *) ioaddr->lbah_addr);
>
> 	tf->feature = feature;
> 	tf->nsect = nsect;
> diff --git a/drivers/scsi/sata_vsc.c b/drivers/scsi/sata_vsc.c
> index 2e2c3b7..cacacc5 100644
> --- a/drivers/scsi/sata_vsc.c
> +++ b/drivers/scsi/sata_vsc.c
> @@ -130,21 +130,26 @@ static void vsc_sata_tf_load(struct ata_
> 		vsc_intr_mask_update(ap, tf->ctl & ATA_NIEN);
> 	}
> 	if (is_addr && (tf->flags & ATA_TFLAG_LBA48)) {
> -		writew(tf->feature | (((u16)tf->hob_feature) << 8), ioaddr->feature_addr);
> -		writew(tf->nsect | (((u16)tf->hob_nsect) << 8), ioaddr->nsect_addr);
> -		writew(tf->lbal | (((u16)tf->hob_lbal) << 8), ioaddr->lbal_addr);
> -		writew(tf->lbam | (((u16)tf->hob_lbam) << 8), ioaddr->lbam_addr);
> -		writew(tf->lbah | (((u16)tf->hob_lbah) << 8), ioaddr->lbah_addr);
> +		writew(tf->feature | (((u16)tf->hob_feature) << 8),
> +		       (void __iomem *) ioaddr->feature_addr);
> +		writew(tf->nsect | (((u16)tf->hob_nsect) << 8),
> +		       (void __iomem *) ioaddr->nsect_addr);
> +		writew(tf->lbal | (((u16)tf->hob_lbal) << 8),
> +		       (void __iomem *) ioaddr->lbal_addr);
> +		writew(tf->lbam | (((u16)tf->hob_lbam) << 8),
> +		       (void __iomem *) ioaddr->lbam_addr);
> +		writew(tf->lbah | (((u16)tf->hob_lbah) << 8),
> +		       (void __iomem *) ioaddr->lbah_addr);
> 	} else if (is_addr) {
> -		writew(tf->feature, ioaddr->feature_addr);
> -		writew(tf->nsect, ioaddr->nsect_addr);
> -		writew(tf->lbal, ioaddr->lbal_addr);
> -		writew(tf->lbam, ioaddr->lbam_addr);
> -		writew(tf->lbah, ioaddr->lbah_addr);
> +		writew(tf->feature, (void __iomem *) ioaddr->feature_addr);
> +		writew(tf->nsect, (void __iomem *) ioaddr->nsect_addr);
> +		writew(tf->lbal, (void __iomem *) ioaddr->lbal_addr);
> +		writew(tf->lbam, (void __iomem *) ioaddr->lbam_addr);
> +		writew(tf->lbah, (void __iomem *) ioaddr->lbah_addr);
> 	}
>
> 	if (tf->flags & ATA_TFLAG_DEVICE)
> -		writeb(tf->device, ioaddr->device_addr);
> +		writeb(tf->device, (void __iomem *) ioaddr->device_addr);
>
> 	ata_wait_idle(ap);
> }
> @@ -156,12 +161,12 @@ static void vsc_sata_tf_read(struct ata_
> 	u16 nsect, lbal, lbam, lbah, feature;
>
> 	tf->command = ata_check_status(ap);
> -	tf->device = readw(ioaddr->device_addr);
> -	feature = readw(ioaddr->error_addr);
> -	nsect = readw(ioaddr->nsect_addr);
> -	lbal = readw(ioaddr->lbal_addr);
> -	lbam = readw(ioaddr->lbam_addr);
> -	lbah = readw(ioaddr->lbah_addr);
> +	tf->device = readw((void __iomem *) ioaddr->device_addr);
> +	feature = readw((void __iomem *) ioaddr->error_addr);
> +	nsect = readw((void __iomem *) ioaddr->nsect_addr);
> +	lbal = readw((void __iomem *) ioaddr->lbal_addr);
> +	lbam = readw((void __iomem *) ioaddr->lbam_addr);
> +	lbah = readw((void __iomem *) ioaddr->lbah_addr);
>
> 	tf->feature = feature;
> 	tf->nsect = nsect;
> @@ -279,8 +284,8 @@ static void __devinit vsc_sata_setup_por
> 	port->ctl_addr		= base + VSC_SATA_TF_CTL_OFFSET;
> 	port->bmdma_addr	= base + VSC_SATA_DMA_CMD_OFFSET;
> 	port->scr_addr		= base + VSC_SATA_SCR_STATUS_OFFSET;
> -	writel(0, base + VSC_SATA_UP_DESCRIPTOR_OFFSET);
> -	writel(0, base + VSC_SATA_UP_DATA_BUFFER_OFFSET);
> +	writel(0, (void __iomem *) (base + VSC_SATA_UP_DESCRIPTOR_OFFSET));
> +	writel(0, (void __iomem *) (base + VSC_SATA_UP_DATA_BUFFER_OFFSET));
> }


With all these casts, doesn't it point out that something is wrong
with writel(), writew(), readl(), and readw() ??? The cast's to
volatile types should be within the macros, not scattered
throughout everyone's driver code!

Cheers,
Dick Johnson
Penguin : Linux version 2.6.13.4 on an i686 machine (5589.66 BogoMips).
Warning : 98.36% of all statistics are fiction.
_
\x1a\x04

****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

Re: [PATCH] Add cast to __iomem pointer in scsi drivers

[Al Viro]

On Tue, Feb 14, 2006 at 09:13:16PM +0100, Takashi Iwai wrote:
> Add the missing cast to __iomem pointer in some scsi drivers.

NAK.  Don't add casts just to make gcc and/or sparse to STFU.  In particular,
sata side of that had been beaten to death many times over - see archives.

Re: [PATCH] Add cast to __iomem pointer in scsi drivers

[Takashi Iwai]

At Tue, 14 Feb 2006 15:35:29 -0500,
linux-os (Dick Johnson) wrote:
> 
> 
> On Tue, 14 Feb 2006, Takashi Iwai wrote:
> 
> > Add the missing cast to __iomem pointer in some scsi drivers.
> >
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> >
> > ---
> > drivers/scsi/megaraid.c |    8 ++++----
> > drivers/scsi/sata_svw.c |   41 +++++++++++++++++++++++------------------
> > drivers/scsi/sata_vsc.c |   43 ++++++++++++++++++++++++-------------------
> > 3 files changed, 51 insertions(+), 41 deletions(-)
> >
> > diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
> > index d101a8a..13c9395 100644
> > --- a/drivers/scsi/megaraid.c
> > +++ b/drivers/scsi/megaraid.c
> > @@ -72,10 +72,10 @@ static unsigned short int max_mbox_busy_
> > module_param(max_mbox_busy_wait, ushort, 0);
> > MODULE_PARM_DESC(max_mbox_busy_wait, "Maximum wait for mailbox in microseconds if busy (default=MBOX_BUSY_WAIT=10)");
> >
> > -#define RDINDOOR(adapter)		readl((adapter)->base + 0x20)
> > -#define RDOUTDOOR(adapter)		readl((adapter)->base + 0x2C)
> > -#define WRINDOOR(adapter,value)		writel(value, (adapter)->base + 0x20)
> > -#define WROUTDOOR(adapter,value)	writel(value, (adapter)->base + 0x2C)
> > +#define RDINDOOR(adapter)		readl((void __iomem *)((adapter)->base + 0x20))
> > +#define RDOUTDOOR(adapter)		readl((void __iomem *)((adapter)->base + 0x2C))
> > +#define WRINDOOR(adapter,value)		writel(value, (void __iomem *)((adapter)->base + 0x20))
> > +#define WROUTDOOR(adapter,value)	writel(value, (void __iomem *)((adapter)->base + 0x2C))
> >
> > /*
> >  * Global variables
> > diff --git a/drivers/scsi/sata_svw.c b/drivers/scsi/sata_svw.c
> > index d847256..862a3ba 100644
> > --- a/drivers/scsi/sata_svw.c
> > +++ b/drivers/scsi/sata_svw.c
> > @@ -110,26 +110,31 @@ static void k2_sata_tf_load(struct ata_p
> > 	unsigned int is_addr = tf->flags & ATA_TFLAG_ISADDR;
> >
> > 	if (tf->ctl != ap->last_ctl) {
> > -		writeb(tf->ctl, ioaddr->ctl_addr);
> > +		writeb(tf->ctl, (void __iomem *) ioaddr->ctl_addr);
> > 		ap->last_ctl = tf->ctl;
> > 		ata_wait_idle(ap);
> > 	}
> > 	if (is_addr && (tf->flags & ATA_TFLAG_LBA48)) {
> > -		writew(tf->feature | (((u16)tf->hob_feature) << 8), ioaddr->feature_addr);
> > -		writew(tf->nsect | (((u16)tf->hob_nsect) << 8), ioaddr->nsect_addr);
> > -		writew(tf->lbal | (((u16)tf->hob_lbal) << 8), ioaddr->lbal_addr);
> > -		writew(tf->lbam | (((u16)tf->hob_lbam) << 8), ioaddr->lbam_addr);
> > -		writew(tf->lbah | (((u16)tf->hob_lbah) << 8), ioaddr->lbah_addr);
> > +		writew(tf->feature | (((u16)tf->hob_feature) << 8),
> > +		       (void __iomem *) ioaddr->feature_addr);
> > +		writew(tf->nsect | (((u16)tf->hob_nsect) << 8),
> > +		       (void __iomem *) ioaddr->nsect_addr);
> > +		writew(tf->lbal | (((u16)tf->hob_lbal) << 8),
> > +		       (void __iomem *) ioaddr->lbal_addr);
> > +		writew(tf->lbam | (((u16)tf->hob_lbam) << 8),
> > +		       (void __iomem *) ioaddr->lbam_addr);
> > +		writew(tf->lbah | (((u16)tf->hob_lbah) << 8),
> > +		       (void __iomem *) ioaddr->lbah_addr);
> > 	} else if (is_addr) {
> > -		writew(tf->feature, ioaddr->feature_addr);
> > -		writew(tf->nsect, ioaddr->nsect_addr);
> > -		writew(tf->lbal, ioaddr->lbal_addr);
> > -		writew(tf->lbam, ioaddr->lbam_addr);
> > -		writew(tf->lbah, ioaddr->lbah_addr);
> > +		writew(tf->feature, (void __iomem *) ioaddr->feature_addr);
> > +		writew(tf->nsect, (void __iomem *) ioaddr->nsect_addr);
> > +		writew(tf->lbal, (void __iomem *) ioaddr->lbal_addr);
> > +		writew(tf->lbam, (void __iomem *) ioaddr->lbam_addr);
> > +		writew(tf->lbah, (void __iomem *) ioaddr->lbah_addr);
> > 	}
> >
> > 	if (tf->flags & ATA_TFLAG_DEVICE)
> > -		writeb(tf->device, ioaddr->device_addr);
> > +		writeb(tf->device, (void __iomem *) ioaddr->device_addr);
> >
> > 	ata_wait_idle(ap);
> > }
> > @@ -141,12 +146,12 @@ static void k2_sata_tf_read(struct ata_p
> > 	u16 nsect, lbal, lbam, lbah, feature;
> >
> > 	tf->command = k2_stat_check_status(ap);
> > -	tf->device = readw(ioaddr->device_addr);
> > -	feature = readw(ioaddr->error_addr);
> > -	nsect = readw(ioaddr->nsect_addr);
> > -	lbal = readw(ioaddr->lbal_addr);
> > -	lbam = readw(ioaddr->lbam_addr);
> > -	lbah = readw(ioaddr->lbah_addr);
> > +	tf->device = readw((void __iomem *) ioaddr->device_addr);
> > +	feature = readw((void __iomem *) ioaddr->error_addr);
> > +	nsect = readw((void __iomem *) ioaddr->nsect_addr);
> > +	lbal = readw((void __iomem *) ioaddr->lbal_addr);
> > +	lbam = readw((void __iomem *) ioaddr->lbam_addr);
> > +	lbah = readw((void __iomem *) ioaddr->lbah_addr);
> >
> > 	tf->feature = feature;
> > 	tf->nsect = nsect;
> > diff --git a/drivers/scsi/sata_vsc.c b/drivers/scsi/sata_vsc.c
> > index 2e2c3b7..cacacc5 100644
> > --- a/drivers/scsi/sata_vsc.c
> > +++ b/drivers/scsi/sata_vsc.c
> > @@ -130,21 +130,26 @@ static void vsc_sata_tf_load(struct ata_
> > 		vsc_intr_mask_update(ap, tf->ctl & ATA_NIEN);
> > 	}
> > 	if (is_addr && (tf->flags & ATA_TFLAG_LBA48)) {
> > -		writew(tf->feature | (((u16)tf->hob_feature) << 8), ioaddr->feature_addr);
> > -		writew(tf->nsect | (((u16)tf->hob_nsect) << 8), ioaddr->nsect_addr);
> > -		writew(tf->lbal | (((u16)tf->hob_lbal) << 8), ioaddr->lbal_addr);
> > -		writew(tf->lbam | (((u16)tf->hob_lbam) << 8), ioaddr->lbam_addr);
> > -		writew(tf->lbah | (((u16)tf->hob_lbah) << 8), ioaddr->lbah_addr);
> > +		writew(tf->feature | (((u16)tf->hob_feature) << 8),
> > +		       (void __iomem *) ioaddr->feature_addr);
> > +		writew(tf->nsect | (((u16)tf->hob_nsect) << 8),
> > +		       (void __iomem *) ioaddr->nsect_addr);
> > +		writew(tf->lbal | (((u16)tf->hob_lbal) << 8),
> > +		       (void __iomem *) ioaddr->lbal_addr);
> > +		writew(tf->lbam | (((u16)tf->hob_lbam) << 8),
> > +		       (void __iomem *) ioaddr->lbam_addr);
> > +		writew(tf->lbah | (((u16)tf->hob_lbah) << 8),
> > +		       (void __iomem *) ioaddr->lbah_addr);
> > 	} else if (is_addr) {
> > -		writew(tf->feature, ioaddr->feature_addr);
> > -		writew(tf->nsect, ioaddr->nsect_addr);
> > -		writew(tf->lbal, ioaddr->lbal_addr);
> > -		writew(tf->lbam, ioaddr->lbam_addr);
> > -		writew(tf->lbah, ioaddr->lbah_addr);
> > +		writew(tf->feature, (void __iomem *) ioaddr->feature_addr);
> > +		writew(tf->nsect, (void __iomem *) ioaddr->nsect_addr);
> > +		writew(tf->lbal, (void __iomem *) ioaddr->lbal_addr);
> > +		writew(tf->lbam, (void __iomem *) ioaddr->lbam_addr);
> > +		writew(tf->lbah, (void __iomem *) ioaddr->lbah_addr);
> > 	}
> >
> > 	if (tf->flags & ATA_TFLAG_DEVICE)
> > -		writeb(tf->device, ioaddr->device_addr);
> > +		writeb(tf->device, (void __iomem *) ioaddr->device_addr);
> >
> > 	ata_wait_idle(ap);
> > }
> > @@ -156,12 +161,12 @@ static void vsc_sata_tf_read(struct ata_
> > 	u16 nsect, lbal, lbam, lbah, feature;
> >
> > 	tf->command = ata_check_status(ap);
> > -	tf->device = readw(ioaddr->device_addr);
> > -	feature = readw(ioaddr->error_addr);
> > -	nsect = readw(ioaddr->nsect_addr);
> > -	lbal = readw(ioaddr->lbal_addr);
> > -	lbam = readw(ioaddr->lbam_addr);
> > -	lbah = readw(ioaddr->lbah_addr);
> > +	tf->device = readw((void __iomem *) ioaddr->device_addr);
> > +	feature = readw((void __iomem *) ioaddr->error_addr);
> > +	nsect = readw((void __iomem *) ioaddr->nsect_addr);
> > +	lbal = readw((void __iomem *) ioaddr->lbal_addr);
> > +	lbam = readw((void __iomem *) ioaddr->lbam_addr);
> > +	lbah = readw((void __iomem *) ioaddr->lbah_addr);
> >
> > 	tf->feature = feature;
> > 	tf->nsect = nsect;
> > @@ -279,8 +284,8 @@ static void __devinit vsc_sata_setup_por
> > 	port->ctl_addr		= base + VSC_SATA_TF_CTL_OFFSET;
> > 	port->bmdma_addr	= base + VSC_SATA_DMA_CMD_OFFSET;
> > 	port->scr_addr		= base + VSC_SATA_SCR_STATUS_OFFSET;
> > -	writel(0, base + VSC_SATA_UP_DESCRIPTOR_OFFSET);
> > -	writel(0, base + VSC_SATA_UP_DATA_BUFFER_OFFSET);
> > +	writel(0, (void __iomem *) (base + VSC_SATA_UP_DESCRIPTOR_OFFSET));
> > +	writel(0, (void __iomem *) (base + VSC_SATA_UP_DATA_BUFFER_OFFSET));
> > }
> 
> 
> With all these casts, doesn't it point out that something is wrong
> with writel(), writew(), readl(), and readw() ??? The cast's to
> volatile types should be within the macros, not scattered
> throughout everyone's driver code!

The patch is just for fixing compile warnings.

readl(), writel() and co are inline functions, and they should be cast
explicitly on the caller side.


Takashi

Re: [PATCH] Add cast to __iomem pointer in scsi drivers

[Mark Lord]

Takashi Iwai wrote:
> At Tue, 14 Feb 2006 15:35:29 -0500,
> linux-os (Dick Johnson) wrote:
>>
>> On Tue, 14 Feb 2006, Takashi Iwai wrote:
>>
>>> Add the missing cast to __iomem pointer in some scsi drivers.
..
>>> -#define WROUTDOOR(adapter,value)	writel(value, (adapter)->base + 0x2C)
...
>>> +#define WROUTDOOR(adapter,value)	writel(value, (void __iomem *)((adapter)->base + 0x2C))
..
>> With all these casts, doesn't it point out that something is wrong
>> with writel(), writew(), readl(), and readw() ??? The cast's to
>> volatile types should be within the macros, not scattered
>> throughout everyone's driver code!
> 
> The patch is just for fixing compile warnings.
> 
> readl(), writel() and co are inline functions, and they should be cast
> explicitly on the caller side.

I think Linus's intent when he added the __iomem "feature", was that code
should change the underlying data declarations to match.  So rather than
casting things left and right, and defeating compiler diagnostics by doing so,
perhaps the actual data type for "base" et al. should get changed to match.

Cheers

Re: [PATCH] Add cast to __iomem pointer in scsi drivers

[Takashi Iwai]

At Tue, 14 Feb 2006 15:46:22 -0500,
Mark Lord wrote:
> 
> Takashi Iwai wrote:
> > At Tue, 14 Feb 2006 15:35:29 -0500,
> > linux-os (Dick Johnson) wrote:
> >>
> >> On Tue, 14 Feb 2006, Takashi Iwai wrote:
> >>
> >>> Add the missing cast to __iomem pointer in some scsi drivers.
> ..
> >>> -#define WROUTDOOR(adapter,value)	writel(value, (adapter)->base + 0x2C)
> ...
> >>> +#define WROUTDOOR(adapter,value)	writel(value, (void __iomem *)((adapter)->base + 0x2C))
> ..
> >> With all these casts, doesn't it point out that something is wrong
> >> with writel(), writew(), readl(), and readw() ??? The cast's to
> >> volatile types should be within the macros, not scattered
> >> throughout everyone's driver code!
> > 
> > The patch is just for fixing compile warnings.
> > 
> > readl(), writel() and co are inline functions, and they should be cast
> > explicitly on the caller side.
> 
> I think Linus's intent when he added the __iomem "feature", was that code
> should change the underlying data declarations to match.  So rather than
> casting things left and right, and defeating compiler diagnostics by doing so,
> perhaps the actual data type for "base" et al. should get changed to match.

Yes, that'll be the best solution.  But, in these drivers, the same
struct fields are used for both inl() and writel() depending on the
flag, so you cannot change the type.

Hm, looks like I hit a dreadful case without a good solution.


Takashi

Re: [PATCH] Add cast to __iomem pointer in scsi drivers

[Russell King]

On Tue, Feb 14, 2006 at 09:59:05PM +0100, Takashi Iwai wrote:
> Yes, that'll be the best solution.  But, in these drivers, the same
> struct fields are used for both inl() and writel() depending on the
> flag, so you cannot change the type.

Which is why we have io{read|write}[bwl], iomap_port, etc.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: [PATCH] Add cast to __iomem pointer in scsi drivers

[Al Viro]

On Tue, Feb 14, 2006 at 09:59:05PM +0100, Takashi Iwai wrote:
> Yes, that'll be the best solution.  But, in these drivers, the same
> struct fields are used for both inl() and writel() depending on the
> flag, so you cannot change the type.
> 
> Hm, looks like I hit a dreadful case without a good solution.

ioread*/iowrite*

Re: [PATCH] Add cast to __iomem pointer in scsi drivers

[Takashi Iwai]

At Tue, 14 Feb 2006 21:05:38 +0000,
Al Viro wrote:
> 
> On Tue, Feb 14, 2006 at 09:59:05PM +0100, Takashi Iwai wrote:
> > Yes, that'll be the best solution.  But, in these drivers, the same
> > struct fields are used for both inl() and writel() depending on the
> > flag, so you cannot change the type.
> > 
> > Hm, looks like I hit a dreadful case without a good solution.
> 
> ioread*/iowrite*

Ah, that's it.  Thanks.

So, the right fix is to rewrite struct ata_ioport to use iomem
pointers and all accessses to ioread*/iowrite*() ?
OK, it's a longer way to go :)


Takashi 

Re: [PATCH] Add cast to __iomem pointer in scsi drivers

[Al Viro]

On Tue, Feb 14, 2006 at 10:14:27PM +0100, Takashi Iwai wrote:
> At Tue, 14 Feb 2006 21:05:38 +0000,
> Al Viro wrote:
> > 
> > On Tue, Feb 14, 2006 at 09:59:05PM +0100, Takashi Iwai wrote:
> > > Yes, that'll be the best solution.  But, in these drivers, the same
> > > struct fields are used for both inl() and writel() depending on the
> > > flag, so you cannot change the type.
> > > 
> > > Hm, looks like I hit a dreadful case without a good solution.
> > 
> > ioread*/iowrite*
> 
> Ah, that's it.  Thanks.
> 
> So, the right fix is to rewrite struct ata_ioport to use iomem
> pointers and all accessses to ioread*/iowrite*() ?
> OK, it's a longer way to go :)

Jeff has that partially done...