fix drivers/scsi/sd.c warnings
PATCH 1/1 fix drivers/scsi/sd.c warnings
Somebody forgot to test with CONFIG_LBD=n:
drivers/scsi/sd.c: In function `sd_init_command':drivers/scsi/sd.c:304: warning: right shift count >= width of type drivers/scsi/sd.c:305: warning: right shift count >= width of type drivers/scsi/sd.c:306: warning: right shift count >= width of type drivers/scsi/sd.c:307: warning: right shift count >= width of type drivers/scsi/sd.c: In function `sd_read_capacity':drivers/scsi/sd.c:1016: warning: left shift count >= width of type drivers/scsi/sd.c:1017: warning: left shift count >= width of type drivers/scsi/sd.c:1018: warning: left shift count >= width of type drivers/scsi/sd.c:1019: warning: left shift count >= width of type
drivers/scsi/sd.c renamed from drivers/scsi/sd.c~scsi-warnings +5 -5
@@ -174,7 +174,7 @@ static int sd_init_command(struct scsi_c {unsigned int this_count, timeout;struct gendisk *disk; -sector_t block; +u64 block;struct scsi_device *sdp = SCpnt->device; timeout =SD_TIMEOUT ;
@@ -1013,10 +1013,10 @@ repeat: (buffer[2 ] <<8 ) | buffer[3]); }else { - sdkp->capacity = 1 + (((sector_t)buffer[0] << 56) | - ((sector_t)buffer[1] << 48) | - ((sector_t)buffer[2] << 40) | - ((sector_t)buffer[3] << 32) | + sdkp->capacity = 1 + (((u64)buffer[0] << 56) | + ((u64)buffer[1] << 48) | + ((u64)buffer[2] << 40) | + ((u64)buffer[3] << 32) | ((sector_t)buffer[4] << 24) | ((sector_t)buffer[5] << 16) | ((sector_t)buffer[6] << 8) |
_
Quoted 1 line · [email protected] said:
Somebody forgot to test with
CONFIG_LBD=n:
Well, OK, that would be me (but then you guessed that).
I enhanced your patch slightly, since we would get a wraparound on x86 without CONFIG_LBD for large devices, so now it will assume the maximum (2Tb) and print a warning.
Hopefully, it should also be obvious to the compiler at compile time that longrc can only be set if sizeof(block) > 4 and therefore allow it to optimise correctly (or am I just being optimistic).
That's also why I was reluctant to make block a u64, since then we'd always run the > 0xffffffff test, which we know will never be true if LBD is not set.
How does the attached work?
James
===== drivers/scsi/sd.c 1.98 vs edited =====
drivers/scsi/sd.c +18 -14
@@ -301,10 +301,10 @@if (block >0xffffffff ) { SCpnt->cmnd[0 ] +=READ_16 -READ_6 ; - SCpnt->cmnd[2 ] = (unsigned char ) (block >>56 ) &0xff ; - SCpnt->cmnd[3 ] = (unsigned char ) (block >>48 ) &0xff ; - SCpnt->cmnd[4 ] = (unsigned char ) (block >>40 ) &0xff ; - SCpnt->cmnd[5 ] = (unsigned char ) (block >>32 ) &0xff ; + SCpnt->cmnd[2 ] =sizeof (block) >4 ? (unsigned char ) (block >>56 ) &0xff :0 ; + SCpnt->cmnd[3 ] =sizeof (block) >4 ? (unsigned char ) (block >>48 ) &0xff :0 ; + SCpnt->cmnd[4 ] =sizeof (block) >4 ? (unsigned char ) (block >>40 ) &0xff :0 ; + SCpnt->cmnd[5 ] =sizeof (block) >4 ? (unsigned char ) (block >>32 ) &0xff :0 ; SCpnt->cmnd[6 ] = (unsigned char ) (block >>24 ) &0xff ; SCpnt->cmnd[7 ] = (unsigned char ) (block >>16 ) &0xff ; SCpnt->cmnd[8 ] = (unsigned char ) (block >>8 ) &0xff ;
@@ -931,7 +931,7 @@if (longrc) {memset ((void *) cmd,0 ,16 ); cmd[0 ] =SERVICE_ACTION_IN ; - cmd[1 ] =0x10 ;/* READ CAPACITY (16) */ + cmd[1 ] =SAI_READ_CAPACITY_16 ; cmd[13 ] =12 ;memset ((void *) buffer,0 ,12 ); }else {
@@ -1002,21 +1002,25 @@ sector_size = (buffer[4 ] <<24 ) | (buffer[5 ] <<16 ) | (buffer[6 ] <<8 ) | buffer[7 ];if (buffer[0 ] ==0xff && buffer[1 ] ==0xff && - buffer[2 ] ==0xff && buffer[3 ] ==0xff ) { -printk (KERN_NOTICE "%s : very big device. try to use" -" READ CAPACITY(16).\n" , diskname); - longrc =1 ; - goto repeat; + buffer[2 ] ==0xff && buffer[3 ] ==0xff ) { +if (sizeof (sdkp->capacity) >4 ) { +printk (KERN_NOTICE "%s : very big device. try to use" +" READ CAPACITY(16).\n" , diskname); + longrc =1 ; + goto repeat; + }else { +printk (KERN_ERR "%s: too big for kernel. Assuming maximum 2Tb\n" , diskname); + } } sdkp->capacity =1 + (((sector_t )buffer[0 ] <<24 ) | (buffer[1 ] <<16 ) | (buffer[2 ] <<8 ) | buffer[3 ]); }else { - sdkp->capacity = 1 + (((sector_t)buffer[0] << 56) | - ((sector_t)buffer[1] << 48) | - ((sector_t)buffer[2] << 40) | - ((sector_t)buffer[3] << 32) | + sdkp->capacity = 1 + (((u64)buffer[0] << 56) | + ((u64)buffer[1] << 48) | + ((u64)buffer[2] << 40) | + ((u64)buffer[3] << 32) | ((sector_t)buffer[4] << 24) | ((sector_t)buffer[5] << 16) | ((sector_t)buffer[6] << 8) |
===== include/scsi/scsi.h 1.7 vs edited =====
include/scsi/scsi.h +2 -0
@@ -99,6 +99,8 @@#define READ_16 0x88#define WRITE_16 0x8a#define SERVICE_ACTION_IN 0x9e +/* values for service action in */ +#define SAI_READ_CAPACITY_16 0x10/*