Re: [PATCH v2] usb: implement proper subclass protocol translation

[ShiHao <i.shihao.999@gmail.com>]

 
> I'm sorry that you went through all this work, but it turns out that 
> this is almost totally unnecessary.  I should have realized this 
> earlier, but I didn't check the context of the patch until now.
> 
> At the place where you're changing the code, we know that srb->cmnd[0] 
> is always going to be REQUEST_SENSE = 0x03 (read the scsi_eh_prep_cmnd() 
> routine in drivers/scsi/scsi_error.c).  And we also know that 
> srb->cmnd_len will be set to 6, so there's no need to set it to 6 again.
> 
> All that this code needs to do is set srb->cmnd_len to 12 for the 
> special subclasses.  Nothing else.  I'm not even sure what the FIXME 
> is referring to.
> 
> Alan Stern
> 
> > -		/* FIXME: we must do the protocol translation here */
> > +		/* Handle usb subclass protocol translation */
> >  		if (us->subclass == USB_SC_RBC || us->subclass == USB_SC_SCSI ||
> > -				us->subclass == USB_SC_CYP_ATACB)
> > -			srb->cmd_len = 6;
> > -		else
> > +		    us->subclass == USB_SC_CYP_ATACB) {
> > +			/* Determine cmd_len based on scsi opcode group */
> > +			if (srb->cmnd[0] <= 0x1F)
> > +				srb->cmd_len = 6;
> > +			else if (srb->cmnd[0] <= 0x7F)
> > +				srb->cmd_len = 10;
> > +			else if (srb->cmnd[0] <= 0x9F)
> > +				srb->cmd_len = 16;
> > +			else if (srb->cmnd[0] <= 0xBF)
> > +				srb->cmd_len = 12;
> > +			else if (srb->cmnd[0] <= 0xDF)
> > +				srb->cmd_len = 16;
> > +			else
> > +				srb->cmd_len = 6;
> > +		} else {
> > +			/* Use fixed value for non-legacy subclasses */
> >  			srb->cmd_len = 12;
> > +		}

Hi alan ,

Well alan thanks for the review . Thank you so much for 
giving your percious time to this matter . I am just a newbie
in the kernel and i was just trying to learn things just to be a
kernel hacker your review has taught me how to submit a good 
commit and write code that is acceptable to the kernel .Thanks 
both greg  and you for reviewing my work . Thank you so much 
looking forward to submit  bug fixes :) . 


thanks 
shihao