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

Jingoo Han
2013-08-07T08:51:03Z

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