On Wed, Oct 12, 2011 at 9:36 AM, Barry Song <Baohua.Song@xxxxxxx> wrote: > From: YuPing Luo <yuping.luo@xxxxxxx> > > With Peiyu's patch "gadget: mass_storage: adapt logic block size to bound block > devices" (http://www.spinics.net/lists/linux-usb/msg50791.html), now mass storage > can adjust logic block size dynamically based on real devices. > Then there is one issue caused by it, if two luns have different logic block size, > mass storage can't work. > Let's check the current software flow: > 1. get_next_command(): call received_cbw(); > 2. received_cbw(): update common->lun = cbw->Lun, but common->curlen is not updated; > 3. do_scsi_command(): in READ_X and WRITE_X commands, common->data_size_from_cmnd is > updated by common->curlun->blkbits; > 4. check_command(): update common->curlun according to common->lun > As you can see, the step 3 uses wrong common->curlun, then wrong common->curlun->blkbits. > If the two luns have same blkbits, there isn't issue. Otherwise, both will fail. > This patch moves the common->curlun update to step 2(received_cbw()), then make sure > step 3 gets right blkbits and right data_size_from_cmnd. > > Cc: Peiyu Li <peiyu.li@xxxxxxx> > Signed-off-by: YuPing Luo <yuping.luo@xxxxxxx> > Signed-off-by: Barry Song <Baohua.Song@xxxxxxx> > --- > drivers/usb/gadget/f_mass_storage.c | 20 +++++++++++++++++--- > drivers/usb/gadget/file_storage.c | 20 ++++++++++++++++++-- > 2 files changed, 35 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c > index a354648..d52aa64 100644 > --- a/drivers/usb/gadget/f_mass_storage.c > +++ b/drivers/usb/gadget/f_mass_storage.c > @@ -1890,15 +1890,13 @@ static int check_command(struct fsg_common *common, int cmnd_size, > > /* Check the LUN */ > if (common->lun < common->nluns) { > - curlun = &common->luns[common->lun]; > - common->curlun = curlun; > + curlun = common->curlun; > if (common->cmnd[0] != REQUEST_SENSE) { > curlun->sense_data = SS_NO_SENSE; > curlun->sense_data_info = 0; > curlun->info_valid = 0; > } > } else { > - common->curlun = NULL; > curlun = NULL; > common->bad_lun_okay = 0; > > @@ -2026,6 +2024,8 @@ static int do_scsi_command(struct fsg_common *common) > break; > > case READ_6: > + if (unlikely(!common->curlun)) > + goto unknown_cmnd; > i = common->cmnd[4]; > common->data_size_from_cmnd = (i == 0 ? 256 : i) << > common->curlun->blkbits; > @@ -2037,6 +2037,8 @@ static int do_scsi_command(struct fsg_common *common) > break; > > case READ_10: > + if (unlikely(!common->curlun)) > + goto unknown_cmnd; > common->data_size_from_cmnd = > get_unaligned_be16(&common->cmnd[7]) << > common->curlun->blkbits; > @@ -2048,6 +2050,8 @@ static int do_scsi_command(struct fsg_common *common) > break; > > case READ_12: > + if (unlikely(!common->curlun)) > + goto unknown_cmnd; > common->data_size_from_cmnd = > get_unaligned_be32(&common->cmnd[6]) << > common->curlun->blkbits; > @@ -2149,6 +2153,8 @@ static int do_scsi_command(struct fsg_common *common) > break; > > case WRITE_6: > + if (unlikely(!common->curlun)) > + goto unknown_cmnd; > i = common->cmnd[4]; > common->data_size_from_cmnd = (i == 0 ? 256 : i) << > common->curlun->blkbits; > @@ -2160,6 +2166,8 @@ static int do_scsi_command(struct fsg_common *common) > break; > > case WRITE_10: > + if (unlikely(!common->curlun)) > + goto unknown_cmnd; > common->data_size_from_cmnd = > get_unaligned_be16(&common->cmnd[7]) << > common->curlun->blkbits; > @@ -2171,6 +2179,8 @@ static int do_scsi_command(struct fsg_common *common) > break; > > case WRITE_12: > + if (unlikely(!common->curlun)) > + goto unknown_cmnd; > common->data_size_from_cmnd = > get_unaligned_be32(&common->cmnd[6]) << > common->curlun->blkbits; > @@ -2289,6 +2299,10 @@ static int received_cbw(struct fsg_dev *fsg, struct fsg_buffhd *bh) > if (common->data_size == 0) > common->data_dir = DATA_DIR_NONE; > common->lun = cbw->Lun; > + if (common->lun >= 0 && common->lun < common->nluns) > + common->curlun = &common->luns[common->lun]; > + else > + common->curlun = NULL; > common->tag = cbw->Tag; > return 0; > } > diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c > index e8600f2..c884624 100644 > --- a/drivers/usb/gadget/file_storage.c > +++ b/drivers/usb/gadget/file_storage.c > @@ -2290,14 +2290,14 @@ static int check_command(struct fsg_dev *fsg, int cmnd_size, > > /* Check the LUN */ > if (fsg->lun < fsg->nluns) { > - fsg->curlun = curlun = &fsg->luns[fsg->lun]; > + curlun = fsg->curlun; > if (fsg->cmnd[0] != REQUEST_SENSE) { > curlun->sense_data = SS_NO_SENSE; > curlun->sense_data_info = 0; > curlun->info_valid = 0; > } > } else { > - fsg->curlun = curlun = NULL; > + curlun = NULL; > fsg->bad_lun_okay = 0; > > /* INQUIRY and REQUEST SENSE commands are explicitly allowed > @@ -2412,6 +2412,8 @@ static int do_scsi_command(struct fsg_dev *fsg) > break; > > case READ_6: > + if (unlikely(!fsg->curlun)) > + goto unknown_cmnd; > i = fsg->cmnd[4]; > fsg->data_size_from_cmnd = (i == 0 ? 256 : i) << fsg->curlun->blkbits; > if ((reply = check_command(fsg, 6, DATA_DIR_TO_HOST, > @@ -2421,6 +2423,8 @@ static int do_scsi_command(struct fsg_dev *fsg) > break; > > case READ_10: > + if (unlikely(!fsg->curlun)) > + goto unknown_cmnd; > fsg->data_size_from_cmnd = > get_unaligned_be16(&fsg->cmnd[7]) << fsg->curlun->blkbits; > if ((reply = check_command(fsg, 10, DATA_DIR_TO_HOST, > @@ -2430,6 +2434,8 @@ static int do_scsi_command(struct fsg_dev *fsg) > break; > > case READ_12: > + if (unlikely(!fsg->curlun)) > + goto unknown_cmnd; > fsg->data_size_from_cmnd = > get_unaligned_be32(&fsg->cmnd[6]) << fsg->curlun->blkbits; > if ((reply = check_command(fsg, 12, DATA_DIR_TO_HOST, > @@ -2516,6 +2522,8 @@ static int do_scsi_command(struct fsg_dev *fsg) > break; > > case WRITE_6: > + if (unlikely(!fsg->curlun)) > + goto unknown_cmnd; > i = fsg->cmnd[4]; > fsg->data_size_from_cmnd = (i == 0 ? 256 : i) << fsg->curlun->blkbits; > if ((reply = check_command(fsg, 6, DATA_DIR_FROM_HOST, > @@ -2525,6 +2533,8 @@ static int do_scsi_command(struct fsg_dev *fsg) > break; > > case WRITE_10: > + if (unlikely(!fsg->curlun)) > + goto unknown_cmnd; > fsg->data_size_from_cmnd = > get_unaligned_be16(&fsg->cmnd[7]) << fsg->curlun->blkbits; > if ((reply = check_command(fsg, 10, DATA_DIR_FROM_HOST, > @@ -2534,6 +2544,8 @@ static int do_scsi_command(struct fsg_dev *fsg) > break; > > case WRITE_12: > + if (unlikely(!fsg->curlun)) > + goto unknown_cmnd; > fsg->data_size_from_cmnd = > get_unaligned_be32(&fsg->cmnd[6]) << fsg->curlun->blkbits; > if ((reply = check_command(fsg, 12, DATA_DIR_FROM_HOST, > @@ -2642,6 +2654,10 @@ static int received_cbw(struct fsg_dev *fsg, struct fsg_buffhd *bh) > if (fsg->data_size == 0) > fsg->data_dir = DATA_DIR_NONE; > fsg->lun = cbw->Lun; > + if (fsg->lun >= 0 && fsg->lun < fsg->nluns) > + fsg->curlun = &fsg->luns[fsg->lun]; > + else > + fsg->curlun = NULL; > fsg->tag = cbw->Tag; > return 0; > } > -- > 1.7.1 > > > > Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom > More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > Does this mean usb mass storage gadget can be uses as CDROM and disk simutaneously? I wan't to export two media to PC, one is normal usb-disk, one is CDROM. Br, yingchun -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
