[openib-general] Re: [PATCH] SRP : Use correct port identifier format according to target io_class

[Roland Dreier]

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