[PATCH 3/8] [SCSI] dc395x: use NULL instead of 0

[PATCH 3/8] [SCSI] dc395x: use NULL instead of 0

[Jingoo Han]

%p is used, thus NULL should be used instead of 0.
Also, bit maskings are added in order to fix the following
sparse warnings:

drivers/scsi/dc395x.c:1525:17: warning: Using plain integer as NULL pointer
drivers/scsi/dc395x.c:4186:48: warning: cast truncates bits from constant value (1de1 becomes e1)
drivers/scsi/dc395x.c:4188:45: warning: cast truncates bits from constant value (391 becomes 91)
drivers/scsi/dc395x.c:4192:44: warning: cast truncates bits from constant value (1de1 becomes e1)
drivers/scsi/dc395x.c:4194:44: warning: cast truncates bits from constant value (391 becomes 91)

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/scsi/dc395x.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
index e73445b..3b09c16 100644
--- a/drivers/scsi/dc395x.c
+++ b/drivers/scsi/dc395x.c
@@ -1526,7 +1526,7 @@ static u8 start_scsi(struct AdapterCtlBlk* acb, struct DeviceCtlBlk* dcb,
 			"command while another command (0x%p) is active.",
 			srb->cmd,
 			acb->active_dcb->active_srb ?
-			    acb->active_dcb->active_srb->cmd : 0);
+			    acb->active_dcb->active_srb->cmd : NULL);
 		return 1;
 	}
 	if (DC395x_read16(acb, TRM_S1040_SCSI_STATUS) & SCSIINTERRUPT) {
@@ -4183,15 +4183,17 @@ static void check_eeprom(struct NvRamType *eeprom, unsigned long io_port)
 		 */
 		dprintkl(KERN_WARNING,
 			"EEProm checksum error: using default values and options.\n");
-		eeprom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
+		eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);
 		eeprom->sub_vendor_id[1] = (u8)(PCI_VENDOR_ID_TEKRAM >> 8);
-		eeprom->sub_sys_id[0] = (u8)PCI_DEVICE_ID_TEKRAM_TRMS1040;
+		eeprom->sub_sys_id[0] =
+		    (u8)(PCI_DEVICE_ID_TEKRAM_TRMS1040 & 0xff);
 		eeprom->sub_sys_id[1] =
 		    (u8)(PCI_DEVICE_ID_TEKRAM_TRMS1040 >> 8);
 		eeprom->sub_class = 0x00;
-		eeprom->vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
+		eeprom->vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);
 		eeprom->vendor_id[1] = (u8)(PCI_VENDOR_ID_TEKRAM >> 8);
-		eeprom->device_id[0] = (u8)PCI_DEVICE_ID_TEKRAM_TRMS1040;
+		eeprom->device_id[0] =
+		    (u8)(PCI_DEVICE_ID_TEKRAM_TRMS1040 & 0xff);
 		eeprom->device_id[1] =
 		    (u8)(PCI_DEVICE_ID_TEKRAM_TRMS1040 >> 8);
 		eeprom->reserved = 0x00;
-- 
1.7.10.4

Re: [PATCH 3/8] [SCSI] dc395x: use NULL instead of 0

[Oliver Neukum]

On Wed, 2013-08-07 at 12:55 +0900, Jingoo Han wrote:

> @@ -4183,15 +4183,17 @@ static void check_eeprom(struct NvRamType *eeprom, unsigned long io_port)
>  		 */
>  		dprintkl(KERN_WARNING,
>  			"EEProm checksum error: using default values and options.\n");
> -		eeprom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
> +		eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);

Hi,

if you are fixing these issues please use the proper macros for
conversion of endianness.

	Regards
		Oliver

Re: [PATCH 3/8] [SCSI] dc395x: use NULL instead of 0

[Jingoo Han]

On Wednesday, August 07, 2013 3:50 PM, Oliver Neukum wrote:
> On Wed, 2013-08-07 at 12:55 +0900, Jingoo Han wrote:
> 
> > @@ -4183,15 +4183,17 @@ static void check_eeprom(struct NvRamType *eeprom, unsigned long io_port)
> >  		 */
> >  		dprintkl(KERN_WARNING,
> >  			"EEProm checksum error: using default values and options.\n");
> > -		eeprom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
> > +		eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);
> 
> Hi,
> 
> if you are fixing these issues please use the proper macros for
> conversion of endianness.

Sorry, I cannot understand exactly what you mean. :(
Would you please let me know which macros can be used?

Best regards,
Jingoo Han

Re: [PATCH 3/8] [SCSI] dc395x: use NULL instead of 0

[Jingoo Han]

On Wednesday, August 07, 2013 3:50 PM, Oliver Neukum wrote:
> On Wed, 2013-08-07 at 12:55 +0900, Jingoo Han wrote:
> 
> > @@ -4183,15 +4183,17 @@ static void check_eeprom(struct NvRamType *eeprom, unsigned long io_port)
> >  		 */
> >  		dprintkl(KERN_WARNING,
> >  			"EEProm checksum error: using default values and options.\n");
> > -		eeprom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
> > +		eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);
> 
> Hi,
> 
> if you are fixing these issues please use the proper macros for
> conversion of endianness.

Then, do you mean the following? :)

-	prom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
+	eprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & le16_to_cpu(0xff));


Best regards,
Jingoo Han

Re: [PATCH 3/8] [SCSI] dc395x: use NULL instead of 0

[Julian Calaby]

Hi Jingoo,

On Wed, Aug 7, 2013 at 5:45 PM, Jingoo Han <jg1.han@samsung.com> wrote:
> On Wednesday, August 07, 2013 3:50 PM, Oliver Neukum wrote:
>> On Wed, 2013-08-07 at 12:55 +0900, Jingoo Han wrote:
>>
>> > @@ -4183,15 +4183,17 @@ static void check_eeprom(struct NvRamType *eeprom, unsigned long io_port)
>> >              */
>> >             dprintkl(KERN_WARNING,
>> >                     "EEProm checksum error: using default values and options.\n");
>> > -           eeprom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
>> > +           eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);
>>
>> Hi,
>>
>> if you are fixing these issues please use the proper macros for
>> conversion of endianness.
>
> Then, do you mean the following? :)
>
> -       prom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
> +       eprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & le16_to_cpu(0xff));

No.

The issue is that the driver is doing things like this:

eeprom->member[0] = (u8)(CONSTANT & 0xff);
eeprom->member[1] = (u8)(CONSTANT >> 8);

Which is exactly the same as code along the lines of:

eeprom->member = cpu_to_le16(CONSTANT);

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

Re: [PATCH 3/8] [SCSI] dc395x: use NULL instead of 0

[Jingoo Han]

> -----Original Message-----
> From: Julian Calaby [mailto:julian.calaby@gmail.com]
> Sent: Wednesday, August 07, 2013 5:21 PM
> To: Jingoo Han
> Cc: Oliver Neukum; James Bottomley; Ali Akcaagac; Jamie Lenehan; dc395x@twibble.org; James Bottomley;
> linux-scsi
> Subject: Re: [PATCH 3/8] [SCSI] dc395x: use NULL instead of 0
> 
> Hi Jingoo,
> 
> On Wed, Aug 7, 2013 at 5:45 PM, Jingoo Han <jg1.han@samsung.com> wrote:
> > On Wednesday, August 07, 2013 3:50 PM, Oliver Neukum wrote:
> >> On Wed, 2013-08-07 at 12:55 +0900, Jingoo Han wrote:
> >>
> >> > @@ -4183,15 +4183,17 @@ static void check_eeprom(struct NvRamType *eeprom, unsigned long io_port)
> >> >              */
> >> >             dprintkl(KERN_WARNING,
> >> >                     "EEProm checksum error: using default values and options.\n");
> >> > -           eeprom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
> >> > +           eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);
> >>
> >> Hi,
> >>
> >> if you are fixing these issues please use the proper macros for
> >> conversion of endianness.
> >
> > Then, do you mean the following? :)
> >
> > -       prom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
> > +       eprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & le16_to_cpu(0xff));
> 
> No.
> 
> The issue is that the driver is doing things like this:
> 
> eeprom->member[0] = (u8)(CONSTANT & 0xff);
> eeprom->member[1] = (u8)(CONSTANT >> 8);
> 
> Which is exactly the same as code along the lines of:
> 
> eeprom->member = cpu_to_le16(CONSTANT);

However, when I compile the following, it makes build error.

-               eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);
-               eeprom->sub_vendor_id[1] = (u8)(PCI_VENDOR_ID_TEKRAM >> 8);
+               eeprom->sub_vendor_id = cpu_to_le16(PCI_VENDOR_ID_TEKRAM);

Best regards,
Jingoo Han

Re: [PATCH 3/8] [SCSI] dc395x: use NULL instead of 0

[Julian Calaby]

Hi Jingoo,

On Wed, Aug 7, 2013 at 6:36 PM, Jingoo Han <jg1.han@samsung.com> wrote:
>
>
>> -----Original Message-----
>> From: Julian Calaby [mailto:julian.calaby@gmail.com]
>> Sent: Wednesday, August 07, 2013 5:21 PM
>> To: Jingoo Han
>> Cc: Oliver Neukum; James Bottomley; Ali Akcaagac; Jamie Lenehan; dc395x@twibble.org; James Bottomley;
>> linux-scsi
>> Subject: Re: [PATCH 3/8] [SCSI] dc395x: use NULL instead of 0
>>
>> Hi Jingoo,
>>
>> On Wed, Aug 7, 2013 at 5:45 PM, Jingoo Han <jg1.han@samsung.com> wrote:
>> > On Wednesday, August 07, 2013 3:50 PM, Oliver Neukum wrote:
>> >> On Wed, 2013-08-07 at 12:55 +0900, Jingoo Han wrote:
>> >>
>> >> > @@ -4183,15 +4183,17 @@ static void check_eeprom(struct NvRamType *eeprom, unsigned long io_port)
>> >> >              */
>> >> >             dprintkl(KERN_WARNING,
>> >> >                     "EEProm checksum error: using default values and options.\n");
>> >> > -           eeprom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
>> >> > +           eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);
>> >>
>> >> Hi,
>> >>
>> >> if you are fixing these issues please use the proper macros for
>> >> conversion of endianness.
>> >
>> > Then, do you mean the following? :)
>> >
>> > -       prom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
>> > +       eprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & le16_to_cpu(0xff));
>>
>> No.
>>
>> The issue is that the driver is doing things like this:
>>
>> eeprom->member[0] = (u8)(CONSTANT & 0xff);
>> eeprom->member[1] = (u8)(CONSTANT >> 8);
>>
>> Which is exactly the same as code along the lines of:
>>
>> eeprom->member = cpu_to_le16(CONSTANT);
>
> However, when I compile the following, it makes build error.
>
> -               eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);
> -               eeprom->sub_vendor_id[1] = (u8)(PCI_VENDOR_ID_TEKRAM >> 8);
> +               eeprom->sub_vendor_id = cpu_to_le16(PCI_VENDOR_ID_TEKRAM);

Of course it does.

I said code along the lines of. You'll need to do more than just what
I suggested, including changing the definition of the eeprom struct
and fixing any other places where it's used / set.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

Re: [PATCH 3/8] [SCSI] dc395x: use NULL instead of 0

[Jingoo Han]

On Wed, Wednesday, August 07, 2013 5:40 PM, Julian Calaby > wrote:
> On Wed, Aug 7, 2013 at 6:36 PM, Jingoo Han <jg1.han@samsung.com> wrote:
> > On Wednesday, August 07, 2013 5:21 PM, Julian Calaby wrote:
> >> On Wed, Aug 7, 2013 at 5:45 PM, Jingoo Han <jg1.han@samsung.com> wrote:
> >> > On Wednesday, August 07, 2013 3:50 PM, Oliver Neukum wrote:
> >> >> On Wed, 2013-08-07 at 12:55 +0900, Jingoo Han wrote:
> >> >>
> >> >> > @@ -4183,15 +4183,17 @@ static void check_eeprom(struct NvRamType *eeprom, unsigned long
> io_port)
> >> >> >              */
> >> >> >             dprintkl(KERN_WARNING,
> >> >> >                     "EEProm checksum error: using default values and options.\n");
> >> >> > -           eeprom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
> >> >> > +           eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);
> >> >>
> >> >> Hi,
> >> >>
> >> >> if you are fixing these issues please use the proper macros for
> >> >> conversion of endianness.
> >> >
> >> > Then, do you mean the following? :)
> >> >
> >> > -       prom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
> >> > +       eprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & le16_to_cpu(0xff));
> >>
> >> No.
> >>
> >> The issue is that the driver is doing things like this:
> >>
> >> eeprom->member[0] = (u8)(CONSTANT & 0xff);
> >> eeprom->member[1] = (u8)(CONSTANT >> 8);
> >>
> >> Which is exactly the same as code along the lines of:
> >>
> >> eeprom->member = cpu_to_le16(CONSTANT);
> >
> > However, when I compile the following, it makes build error.
> >
> > -               eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);
> > -               eeprom->sub_vendor_id[1] = (u8)(PCI_VENDOR_ID_TEKRAM >> 8);
> > +               eeprom->sub_vendor_id = cpu_to_le16(PCI_VENDOR_ID_TEKRAM);
> 
> Of course it does.
> 
> I said code along the lines of. You'll need to do more than just what
> I suggested, including changing the definition of the eeprom struct
> and fixing any other places where it's used / set.

I fixed it as below. In this case, it does not make any warnings.
Oliver Neukum, do you mean the following?

struct NvRamType {
-	u8 sub_vendor_id[2];    /* 0,1  Sub Vendor ID   */
+	u16 sub_vendor_id;      /* 0,1  Sub Vendor ID   */

-		eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);
-		eeprom->sub_vendor_id[1] = (u8)(PCI_VENDOR_ID_TEKRAM >> 8);
+		eeprom->sub_vendor_id = cpu_to_le16(PCI_VENDOR_ID_TEKRAM);

Best regards,
Jingoo Han

Re: [PATCH 3/8] [SCSI] dc395x: use NULL instead of 0

[Oliver Neukum]

On Wed, 2013-08-07 at 15:58 +0900, Jingoo Han wrote:
> On Wednesday, August 07, 2013 3:50 PM, Oliver Neukum wrote:
> > On Wed, 2013-08-07 at 12:55 +0900, Jingoo Han wrote:
> > 
> > > @@ -4183,15 +4183,17 @@ static void check_eeprom(struct NvRamType *eeprom, unsigned long io_port)
> > >  		 */
> > >  		dprintkl(KERN_WARNING,
> > >  			"EEProm checksum error: using default values and options.\n");
> > > -		eeprom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM;
> > > +		eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff);
> > 
> > Hi,
> > 
> > if you are fixing these issues please use the proper macros for
> > conversion of endianness.
> 
> Sorry, I cannot understand exactly what you mean. :(
> Would you please let me know which macros can be used?

In this case constant_cpu_to_le16() would be the macro you want.
Have a look at include/uapi/linux/byteorder/

	Regards
		Oliver