[PATCH 1/2] ide: ide-tape.c sparse annotations and unaligned access removal

[PATCH 1/2] ide: ide-tape.c sparse annotations and unaligned access removal

[Harvey Harrison]

If this is actually unaligned the access of speed/max_speed above
is already broken and needs a get_unaligned.  Otherwise it is
aligned and they can be removed.

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
Bart, if this truly should be unaligned, let me know and I'll send you
a patch fixing the speed/maxspeed issue.

 drivers/ide/ide-tape.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 34fc925..40b09b2 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -655,10 +655,10 @@ static void ide_tape_callback(ide_drive_t *drive)
 			uptodate = 0;
 		} else {
 			debug_log(DBG_SENSE, "Block Location - %u\n",
-					be32_to_cpu(*(u32 *)&readpos[4]));
+					be32_to_cpup((__be32 *)&readpos[4]));
 
 			tape->partition = readpos[1];
-			tape->first_frame = be32_to_cpu(*(u32 *)&readpos[4]);
+			tape->first_frame = be32_to_cpup((__be32 *)&readpos[4]);
 			set_bit(IDE_DFLAG_ADDRESS_VALID, &drive->dev_flags);
 		}
 	}
@@ -2381,13 +2381,13 @@ static void idetape_get_mode_sense_results(ide_drive_t *drive)
 	caps = pc.buf + 4 + pc.buf[3];
 
 	/* convert to host order and save for later use */
-	speed = be16_to_cpu(*(u16 *)&caps[14]);
-	max_speed = be16_to_cpu(*(u16 *)&caps[8]);
+	speed = be16_to_cpup((__be16 *)&caps[14]);
+	max_speed = be16_to_cpup((__be16 *)&caps[8]);
 
-	put_unaligned(max_speed, (u16 *)&caps[8]);
-	put_unaligned(be16_to_cpu(*(u16 *)&caps[12]), (u16 *)&caps[12]);
-	put_unaligned(speed, (u16 *)&caps[14]);
-	put_unaligned(be16_to_cpu(*(u16 *)&caps[16]), (u16 *)&caps[16]);
+	*(u16 *)&caps[8] = max_speed;
+	*(u16 *)&caps[12] = be16_to_cpup((__be16 *)&caps[12]);
+	*(u16 *)&caps[14] = speed;
+	*(u16 *)&caps[16] = be16_to_cpup((__be16 *)&caps[16]);
 
 	if (!speed) {
 		printk(KERN_INFO "ide-tape: %s: invalid tape speed "
-- 
1.5.6.3.499.geae9

Re: [PATCH 1/2] ide: ide-tape.c sparse annotations and unaligned access removal

[Bartlomiej Zolnierkiewicz]

On Tuesday 15 July 2008, Harvey Harrison wrote:
> If this is actually unaligned the access of speed/max_speed above
> is already broken and needs a get_unaligned.  Otherwise it is
> aligned and they can be removed.
> 
> Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
> ---
> Bart, if this truly should be unaligned, let me know and I'll send you
> a patch fixing the speed/maxspeed issue.

I applied the patch as it is for now as I don't see a reason to
use unaligned access there but I could have forgotten some details
(added Borislav to cc:).

I've also noticed that there are some put_unaligned()-s left in
idetape_get_mode_sense_results(), is this intended?

>  drivers/ide/ide-tape.c |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> index 34fc925..40b09b2 100644
> --- a/drivers/ide/ide-tape.c
> +++ b/drivers/ide/ide-tape.c
> @@ -655,10 +655,10 @@ static void ide_tape_callback(ide_drive_t *drive)
>  			uptodate = 0;
>  		} else {
>  			debug_log(DBG_SENSE, "Block Location - %u\n",
> -					be32_to_cpu(*(u32 *)&readpos[4]));
> +					be32_to_cpup((__be32 *)&readpos[4]));
>  
>  			tape->partition = readpos[1];
> -			tape->first_frame = be32_to_cpu(*(u32 *)&readpos[4]);
> +			tape->first_frame = be32_to_cpup((__be32 *)&readpos[4]);
>  			set_bit(IDE_DFLAG_ADDRESS_VALID, &drive->dev_flags);
>  		}
>  	}
> @@ -2381,13 +2381,13 @@ static void idetape_get_mode_sense_results(ide_drive_t *drive)
>  	caps = pc.buf + 4 + pc.buf[3];
>  
>  	/* convert to host order and save for later use */
> -	speed = be16_to_cpu(*(u16 *)&caps[14]);
> -	max_speed = be16_to_cpu(*(u16 *)&caps[8]);
> +	speed = be16_to_cpup((__be16 *)&caps[14]);
> +	max_speed = be16_to_cpup((__be16 *)&caps[8]);
>  
> -	put_unaligned(max_speed, (u16 *)&caps[8]);
> -	put_unaligned(be16_to_cpu(*(u16 *)&caps[12]), (u16 *)&caps[12]);
> -	put_unaligned(speed, (u16 *)&caps[14]);
> -	put_unaligned(be16_to_cpu(*(u16 *)&caps[16]), (u16 *)&caps[16]);
> +	*(u16 *)&caps[8] = max_speed;
> +	*(u16 *)&caps[12] = be16_to_cpup((__be16 *)&caps[12]);
> +	*(u16 *)&caps[14] = speed;
> +	*(u16 *)&caps[16] = be16_to_cpup((__be16 *)&caps[16]);
>  
>  	if (!speed) {
>  		printk(KERN_INFO "ide-tape: %s: invalid tape speed "

Re: [PATCH 1/2] ide: ide-tape.c sparse annotations and unaligned access removal

[Harvey Harrison]

On Wed, 2008-07-16 at 19:43 +0200, Bartlomiej Zolnierkiewicz wrote:
> On Tuesday 15 July 2008, Harvey Harrison wrote:
> > If this is actually unaligned the access of speed/max_speed above
> > is already broken and needs a get_unaligned.  Otherwise it is
> > aligned and they can be removed.
> > 
> > Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
> > ---
> > Bart, if this truly should be unaligned, let me know and I'll send you
> > a patch fixing the speed/maxspeed issue.
> 
> I applied the patch as it is for now as I don't see a reason to
> use unaligned access there but I could have forgotten some details
> (added Borislav to cc:).
> 
> I've also noticed that there are some put_unaligned()-s left in
> idetape_get_mode_sense_results(), is this intended?

I just fixed the immediate ones I saw right below the aligned access to 
speed/max_speed.  I can't be sure about tape->caps being aligned or not,
but at least the following incremental patch is ok if caps is 2-byte
aligned.  Feel free to fold it into my original or put my signed-off by on
a new commit.

diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 40b09b2..9c3ce71 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -2392,12 +2392,12 @@ static void idetape_get_mode_sense_results(ide_drive_t *drive)
 	if (!speed) {
 		printk(KERN_INFO "ide-tape: %s: invalid tape speed "
 				"(assuming 650KB/sec)\n", drive->name);
-		put_unaligned(650, (u16 *)&caps[14]);
+		*(u16 *)&caps[14] = 650;
 	}
 	if (!max_speed) {
 		printk(KERN_INFO "ide-tape: %s: invalid max_speed "
 				"(assuming 650KB/sec)\n", drive->name);
-		put_unaligned(650, (u16 *)&caps[8]);
+		*(u16 *)&caps[8] = 650;
 	}
 
 	memcpy(&tape->caps, caps, 20);

Re: [PATCH 1/2] ide: ide-tape.c sparse annotations and unaligned access removal

[Bartlomiej Zolnierkiewicz]

On Wednesday 16 July 2008, Harvey Harrison wrote:
> On Wed, 2008-07-16 at 19:43 +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Tuesday 15 July 2008, Harvey Harrison wrote:
> > > If this is actually unaligned the access of speed/max_speed above
> > > is already broken and needs a get_unaligned.  Otherwise it is
> > > aligned and they can be removed.
> > > 
> > > Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
> > > ---
> > > Bart, if this truly should be unaligned, let me know and I'll send you
> > > a patch fixing the speed/maxspeed issue.
> > 
> > I applied the patch as it is for now as I don't see a reason to
> > use unaligned access there but I could have forgotten some details
> > (added Borislav to cc:).
> > 
> > I've also noticed that there are some put_unaligned()-s left in
> > idetape_get_mode_sense_results(), is this intended?
> 
> I just fixed the immediate ones I saw right below the aligned access to 
> speed/max_speed.  I can't be sure about tape->caps being aligned or not,
> but at least the following incremental patch is ok if caps is 2-byte
> aligned.  Feel free to fold it into my original or put my signed-off by on
> a new commit.

Thanks, I folded it into original patch.