Overall seems OK.  Some comments:

 > +    }
 > +    else {  

should be written just as

        } else {

(this appears quite a few times)

 >      { SRP_OPT_MAX_CMD_PER_LUN,      "max_cmd_per_lun=%d"    },
 > +    { SRP_OPT_IO_CLASS,             "io_class=%x"   },
 >      { SRP_OPT_ERR,                  NULL                    }

please keep the formatting consistent here.

 > +                            target->io_class = (unsigned short)(token);

why is the cast needed here?

 > +    /*Set default IO class of target to Rev 16A*/
 > +    target->io_class   = SRP_REV16A_IO_CLASS;

just delete this comment -- anyone who can't figure out what the next
line is doing probably won't be able to figure out the comment either.

 > +#define SRP_REV10_IO_CLASS   0xFF00
 > +#define SRP_REV16A_IO_CLASS  0x0100

I think these should be in an enum in <scsi/srp.h>, since they're
generic constants from the SRP spec.

Can you regenerate the patch and resend?

Thanks,
  Roland
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to