: Fix compiler warning in drivers/scsi/dpt_i2o.c
PATCH 1/1 : Fix compiler warning in drivers/scsi/dpt_i2o.c
Hi there!
The attachted patch fixes a compiler warning in drivers/scsi/dpt_i2o.c.
If you reply to this post, please CC me as I'm not suscribed to the list.
Best Regards
Signature · 2 lines
Stefan Roas [email protected]
drivers/scsi/dpt_i2o.c +12 -12
@@ -2027,8 +2027,8 @@ } reply = (ulong )bus_to_virt (m); -if (readl (reply) &MSG_FAIL ) { -u32 old_m =readl (reply+28 ); +if (readl ((void *)reply) &MSG_FAIL ) { +u32 old_m =readl ((void *)reply+28 );ulong msg;u32 old_context;PDEBUG ("%s: Failed message\n" ,pHba->name);
@@ -2039,34 +2039,34 @@ }// Transaction context is 0 in failed reply frame msg = (ulong)(pHba->msg_addr_virt + old_m); - old_context =readl (msg+12 ); + old_context =readl ((void *)msg+12 );writel (old_context, reply+12 );adpt_send_nop (pHba, old_m); } - context =readl (reply+8 ); + context =readl ((void *)reply+8 );if (context &0x40000000 ){// IOCTL -ulong p = (ulong)(readl (reply+12 )); +ulong p = (ulong)(readl ((void *)reply+12 ));if ( p !=0 ) {memcpy ((void *)p, (void *)reply,REPLY_FRAME_SIZE *4 ); }// All IOCTLs will also be post wait }if (context &0x80000000 ){// Post wait message - status =readl (reply+16 ); + status =readl ((void *)reply+16 );if (status >>24 ){ status &=0xffff ;/* Get detail status */ }else { status =I2O_POST_WAIT_OK ; }if (!(context &0x40000000 )) { - cmd = (struct scsi_cmnd *)readl (reply+12 ); + cmd = (struct scsi_cmnd *)readl ((void *)reply+12 );if (cmd !=NULL ) {printk (KERN_WARNING "%s: Apparent SCSI cmd in Post Wait Context - cmd=%p context=%x\n" , pHba->name, cmd, context); } }adpt_i2o_post_wait_complete (context, status); }else {// SCSI message - cmd = (struct scsi_cmnd *)readl (reply+12 ); + cmd = (struct scsi_cmnd *)readl ((void *)reply+12 );if (cmd !=NULL ){if (cmd->serial_number !=0 ) {// If not timedout adpt_i2o_to_scsi (reply, cmd);
@@ -2236,16 +2236,16 @@adpt_hba * pHba;u32 hba_status;u32 dev_status; -u32 reply_flags =readl (reply) &0xff00 ;// Leave it shifted up 8 bits +u32 reply_flags =readl ((void *)reply) &0xff00 ;// Leave it shifted up 8 bits // I know this would look cleaner if I just read bytes // but the model I have been using for all the rest of the // io is in 4 byte words - so I keep that model -u16 detailed_status =readl (reply+16 ) &0xffff ; +u16 detailed_status =readl ((void *)reply+16 ) &0xffff ; dev_status = (detailed_status &0xff ); hba_status = detailed_status >>8 ;// calculate resid for sg - cmd->resid = cmd->request_bufflen -readl (reply+5 ); + cmd->resid = cmd->request_bufflen -readl ((void *)reply+5 ); pHba = (adpt_hba *) cmd->device->host->hostdata[0 ];
@@ -2256,7 +2256,7 @@case I2O_SCSI_DSC_SUCCESS : cmd->result = (DID_OK <<16 );// handle underflow -if (readl (reply+5 ) < cmd->underflow ) { +if (readl ((void *)reply+5 ) < cmd->underflow ) { cmd->result = (DID_ERROR <<16 );printk (KERN_WARNING "%s: SCSI CMD underflow\n" ,pHba->name); }
Quoted 6 lines · Stefan Roas
Hi there!
The attachted patch fixes a compiler warning in
drivers/scsi/dpt_i2o.c.If you reply to this post, please CC me as I'm not suscribed to the list.
This patch looks suspiciously like it is sweeping the problem under the carpet. Does bus_to_virt() return an void __iomem *?
reply should really be an void __iomem *
Quoted 90 lines
Best Regards
Signature · 4 lines
Stefan Roas [email protected] diff -rNu a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.cdrivers/scsi/dpt_i2o.c +12
-12@@ -2027,8 +2027,8 @@ } reply = (ulong )bus_to_virt (m); -if (readl (reply) &MSG_FAIL ) { -u32 old_m =readl (reply+28 ); +if (readl ((void *)reply) &MSG_FAIL ) { +u32 old_m =readl ((void *)reply+28 );ulong msg;u32 old_context;PDEBUG ("%s: Failed message\n" ,pHba->name);@@ -2039,34 +2039,34 @@ }// Transaction context is 0 in failed reply frame msg = (ulong)(pHba->msg_addr_virt + old_m); - old_context =readl (msg+12 ); + old_context =readl ((void *)msg+12 );writel (old_context, reply+12 );adpt_send_nop (pHba, old_m); } - context =readl (reply+8 ); + context =readl ((void *)reply+8 );if (context &0x40000000 ){// IOCTL -ulong p = (ulong)(readl (reply+12 )); +ulong p = (ulong)(readl ((void *)reply+12 ));if ( p !=0 ) {memcpy ((void *)p, (void *)reply,REPLY_FRAME_SIZE *4 ); }// All IOCTLs will also be post wait }if (context &0x80000000 ){// Post wait message - status =readl (reply+16 ); + status =readl ((void *)reply+16 );if (status >>24 ){ status &=0xffff ;/* Get detail status */ }else { status =I2O_POST_WAIT_OK ; }if (!(context &0x40000000 )) { - cmd = (struct scsi_cmnd *)readl (reply+12 ); + cmd = (struct scsi_cmnd *)readl ((void *)reply+12 );if (cmd !=NULL ) {printk (KERN_WARNING "%s: Apparent SCSI cmd in Post Wait Context - cmd=%p context=%x\n" , pHba->name, cmd, context); } }adpt_i2o_post_wait_complete (context, status); }else {// SCSI message - cmd = (struct scsi_cmnd *)readl (reply+12 ); + cmd = (struct scsi_cmnd *)readl ((void *)reply+12 );if (cmd !=NULL ){if (cmd->serial_number !=0 ) {// If not timedout adpt_i2o_to_scsi (reply, cmd);@@ -2236,16 +2236,16 @@adpt_hba * pHba;u32 hba_status;u32 dev_status; -u32 reply_flags =readl (reply) &0xff00 ;// Leave it shifted up 8 bits +u32 reply_flags =readl ((void *)reply) &0xff00 ;// Leave it shifted up 8 bits // I know this would look cleaner if I just read bytes // but the model I have been using for all the rest of the // io is in 4 byte words - so I keep that model -u16 detailed_status =readl (reply+16 ) &0xffff ; +u16 detailed_status =readl ((void *)reply+16 ) &0xffff ; dev_status = (detailed_status &0xff ); hba_status = detailed_status >>8 ;// calculate resid for sg - cmd->resid = cmd->request_bufflen -readl (reply+5 ); + cmd->resid = cmd->request_bufflen -readl ((void *)reply+5 ); pHba = (adpt_hba *) cmd->device->host->hostdata[0 ];@@ -2256,7 +2256,7 @@case I2O_SCSI_DSC_SUCCESS : cmd->result = (DID_OK <<16 );// handle underflow -if (readl (reply+5 ) < cmd->underflow ) { +if (readl ((void *)reply+5 ) < cmd->underflow ) { cmd->result = (DID_ERROR <<16 );printk (KERN_WARNING "%s: SCSI CMD underflow\n" ,pHba->name); }
Signature · 3 lines
Ben ([email protected], http://www.fluff.org/) 'a smiley only costs 4 bytes'
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 4 lines · Ben Dooks
This patch looks suspiciously like it is sweeping the problem
under the carpet. Doesbus_to_virt()return anvoid __iomem *?reply should really be an
void __iomem *
bus_to_virt returns void *.
adpt_isr casts the return value to ulong though and adpt_i2o_to_scsi takes an ulong as its first argument and passes it to readl without a cast then.
But I agree, the patch just silences the compiler warning. Maybe it would be a better solution to change the types of reply and msg in adpt_isr as well as the first argument to adpt_i2o_to_scsi to void __iomem *.
Best Regards,
- --
Stefan Roas
[email protected]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)
iD8DBQFCNe7CwvfNuQ9pAq8RAmDBAJ0XyRMogsfgX3L6+SzIzp6VPYuynQCgjpZ0 94sI0Fr7V8anGTsQQw+COcM=
=+qfk
-----END PGP SIGNATURE-----
Quoted 9 lines · Stefan Roas
Quoted 4 lines · Ben Dooks
This patch looks suspiciously like it is sweeping the problem
under the carpet. Doesbus_to_virt()return anvoid __iomem *?reply should really be an
void __iomem *
bus_to_virtreturns void *.
Quoted 8 lines
adpt_isrcasts the return value to ulong though andadpt_i2o_to_scsitakes an ulong as its first argument and passes it to readl without a cast then.But I agree, the patch just silences the compiler warning. Maybe it would be a better solution to change the types of reply and msg in
adpt_isras well as the first argument toadpt_i2o_to_scsito void__iomem*.
However, bus_to_virt() can only be used on addresses which correspond with system RAM. readl and friends should not be used on such a memory space. So, this driver is doing lots of bogus stuff.
The warnings are perfectly valid and should therefore stay.
If "reply" is actually referring to some system memory, the readl/writel shouldn't be there, and we should be accessing system memory directly via a bona-fide structure. If it isn't, then the bus_to_virt() is completely bogus and unportable, and that's where we _must_ raise a warning.
However, I do agree with changing "msg" to be void __iomem *, and removing the (ulong) cast, since pHba->msg_addr_virt is already void __iomem *.
(note: I'm just taking a passing interest in this thread, from the point of view of an architecture maintainer who wants these types of things to be Correct(tm)...)
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 Serial core