On Thu, Oct 20, 2011 at 4:29 PM, Yuping Luo <lypingsh@xxxxxxxxx> wrote:
> 2011/10/18 Michal Nazarewicz <mina86@xxxxxxxxxx>:
>> On Mon, 17 Oct 2011 19:56:20 -0700, Yuping Luo <lypingsh@xxxxxxxxx> wrote:
>>
>>> On Tue, Oct 18, 2011 at 4:30 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
>>> wrote:
>>>>
>>>> On Mon, 17 Oct 2011, Michal Nazarewicz wrote:
>>>>
>>>>> On Mon, 17 Oct 2011 02:01:48 -0700, Yuping Luo <lypingsh@xxxxxxxxx>
>>>>> wrote:
>>>>> > here you are , :)
>>>>> >
>>>>> > diff --git a/drivers/usb/gadget/f_mass_storage.c
>>>>> > b/drivers/usb/gadget/f_mass_storage.c
>>>>> > index 524381a..6a5c42d 100644
>>>>> > --- a/drivers/usb/gadget/f_mass_storage.c
>>>>> > +++ b/drivers/usb/gadget/f_mass_storage.c
>>>>> > @@ -379,6 +379,7 @@ struct fsg_common {
>>>>> > enum data_direction data_dir;
>>>>> > u32 data_size;
>>>>> > u32 data_size_from_cmnd;
>>>>> > + u32 data_size_is_in_blocks;
>>>>>
>>>>> Personally, I'd make it an argument to check_command() or create a
>>>>> simple
>>>>> wrapper for check_command() that calculates the size.
>>>>
>>>> I considered making this a new argument to check_command(). In the end
>>>> I decided against it, because check_command() already has a lot of
>>>> arguments, and also it would be necessary to add the new argument to
>>>> every call (of which there are quite a few).
>>>>
>>>> Using a wrapper routine is a good idea -- it didn't occur to me before.
>>>>
>>>> Alan Stern
>>>>
>>> the codes only occur one time, which makes the wrapper unnecessary.
>>
>> In do_scsi_command() you add a bunch of ->data_size_is_in_blocks = 1
>> statements. Instead of doing that, you might create a
>> check_command_size_in_blocks() function like so:
>>
>> static int check_command_size_in_blocks(...)
>> {
>> if (*->curlun)
>> *->->data_size_from_cmnd <<= *->curlun->blkbits;
>> return check_command(...);
>> }
>>
>> and call it instead. This would make data_size_is_in_blocks unnecessary
>> which IMO is a good thing since it is a hidden state.
>>
> got it , thanks.
>> --
>> Best regards, _ _
>> .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
>> ..o | Computer Science, Michał “mina86” Nazarewicz (o o)
>> ooo +----<email/xmpp: mpn@xxxxxxxxxx>--------------ooO--(_)--Ooo--
>>
I send the whole fixed patch for your review, if desirable , Barry
will send one formal patch for push
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index a354648..6ecaef6 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -1889,17 +1890,14 @@ static int check_command(struct fsg_common
*common, int cmnd_size,
common->lun, lun);
/* Check the LUN */
- if (common->lun < common->nluns) {
- curlun = &common->luns[common->lun];
- common->curlun = curlun;
+ curlun = common->curlun;
+ if (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;
/*
@@ -1945,6 +1943,18 @@ static int check_command(struct fsg_common
*common, int cmnd_size,
return 0;
}
+/* wrapper of check_command for data size in blocks handling */
+static int check_command_size_in_blocks(struct fsg_common *common,
+ int cmnd_size, enum data_direction data_dir,
+ unsigned int mask, int needs_medium, const char *name)
+{
+ if (common->curlun)
+ common->data_size_from_cmnd <<= common->curlun->blkbits;
+ return check_command(common, cmnd_size, data_dir,
+ mask, needs_medium, name);
+}
+
+
static int do_scsi_command(struct fsg_common *common)
{
struct fsg_buffhd *bh;
@@ -2027,9 +2037,9 @@ static int do_scsi_command(struct fsg_common *common)
case READ_6:
i = common->cmnd[4];
- common->data_size_from_cmnd = (i == 0 ? 256 : i) <<
- common->curlun->blkbits;
- reply = check_command(common, 6, DATA_DIR_TO_HOST,
+ common->data_size_from_cmnd = (i == 0 ? 256 : i);
+ reply = check_command_size_in_blocks(common, 6,
+ DATA_DIR_TO_HOST,
(7<<1) | (1<<4), 1,
"READ(6)");
if (reply == 0)
@@ -2038,9 +2048,9 @@ static int do_scsi_command(struct fsg_common *common)
case READ_10:
common->data_size_from_cmnd =
- get_unaligned_be16(&common->cmnd[7]) <<
- common->curlun->blkbits;
- reply = check_command(common, 10, DATA_DIR_TO_HOST,
+ get_unaligned_be16(&common->cmnd[7]);
+ reply = check_command_size_in_blocks(common, 10,
+ DATA_DIR_TO_HOST,
(1<<1) | (0xf<<2) | (3<<7), 1,
"READ(10)");
if (reply == 0)
@@ -2049,9 +2059,9 @@ static int do_scsi_command(struct fsg_common *common)
case READ_12:
common->data_size_from_cmnd =
- get_unaligned_be32(&common->cmnd[6]) <<
- common->curlun->blkbits;
- reply = check_command(common, 12, DATA_DIR_TO_HOST,
+ get_unaligned_be32(&common->cmnd[6]);
+ reply = check_command_size_in_blocks(common, 12,
+ DATA_DIR_TO_HOST,
(1<<1) | (0xf<<2) | (0xf<<6), 1,
"READ(12)");
if (reply == 0)
@@ -2150,9 +2160,9 @@ static int do_scsi_command(struct fsg_common *common)
case WRITE_6:
i = common->cmnd[4];
- common->data_size_from_cmnd = (i == 0 ? 256 : i) <<
- common->curlun->blkbits;
- reply = check_command(common, 6, DATA_DIR_FROM_HOST,
+ common->data_size_from_cmnd = (i == 0 ? 256 : i);
+ reply = check_command_size_in_blocks(common, 6,
+ DATA_DIR_FROM_HOST,
(7<<1) | (1<<4), 1,
"WRITE(6)");
if (reply == 0)
@@ -2161,9 +2171,9 @@ static int do_scsi_command(struct fsg_common *common)
case WRITE_10:
common->data_size_from_cmnd =
- get_unaligned_be16(&common->cmnd[7]) <<
- common->curlun->blkbits;
- reply = check_command(common, 10, DATA_DIR_FROM_HOST,
+ get_unaligned_be16(&common->cmnd[7]);
+ reply = check_command_size_in_blocks(common, 10,
+ DATA_DIR_FROM_HOST,
(1<<1) | (0xf<<2) | (3<<7), 1,
"WRITE(10)");
if (reply == 0)
@@ -2172,9 +2182,9 @@ static int do_scsi_command(struct fsg_common *common)
case WRITE_12:
common->data_size_from_cmnd =
- get_unaligned_be32(&common->cmnd[6]) <<
- common->curlun->blkbits;
- reply = check_command(common, 12, DATA_DIR_FROM_HOST,
+ get_unaligned_be32(&common->cmnd[6]);
+ reply = check_command_size_in_blocks(common, 12,
+ DATA_DIR_FROM_HOST,
(1<<1) | (0xf<<2) | (0xf<<6), 1,
"WRITE(12)");
if (reply == 0)
@@ -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..06490ca 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -2289,15 +2295,14 @@ static int check_command(struct fsg_dev *fsg,
int cmnd_size,
fsg->lun = lun; // Use LUN from the command
/* Check the LUN */
- if (fsg->lun < fsg->nluns) {
- fsg->curlun = curlun = &fsg->luns[fsg->lun];
+ curlun = fsg->curlun;
+ if (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;
fsg->bad_lun_okay = 0;
/* INQUIRY and REQUEST SENSE commands are explicitly allowed
@@ -2339,6 +2344,16 @@ static int check_command(struct fsg_dev *fsg,
int cmnd_size,
return 0;
}
+/* wrapper of check_command for data size in blocks handling */
+static int check_command_size_in_blocks(struct fsg_dev *fsg, int cmnd_size,
+ enum data_direction data_dir, unsigned int mask,
+ int needs_medium, const char *name)
+{
+ if (fsg->curlun)
+ fsg->data_size_from_cmnd <<= fsg->curlun->blkbits;
+ return check_command(fsg, cmnd_size, data_dir,
+ mask, needs_medium, name);
+}
static int do_scsi_command(struct fsg_dev *fsg)
{
@@ -2413,8 +2428,9 @@ static int do_scsi_command(struct fsg_dev *fsg)
case READ_6:
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,
+ fsg->data_size_from_cmnd = (i == 0 ? 256 : i);
+ if ((reply = check_command_size_in_blocks(fsg, 6,
+ DATA_DIR_TO_HOST,
(7<<1) | (1<<4), 1,
"READ(6)")) == 0)
reply = do_read(fsg);
@@ -2422,8 +2438,9 @@ static int do_scsi_command(struct fsg_dev *fsg)
case READ_10:
fsg->data_size_from_cmnd =
- get_unaligned_be16(&fsg->cmnd[7]) << fsg->curlun->blkbits;
- if ((reply = check_command(fsg, 10, DATA_DIR_TO_HOST,
+ get_unaligned_be16(&fsg->cmnd[7]);
+ if ((reply = check_command_size_in_blocks(fsg, 10,
+ DATA_DIR_TO_HOST,
(1<<1) | (0xf<<2) | (3<<7), 1,
"READ(10)")) == 0)
reply = do_read(fsg);
@@ -2431,8 +2448,9 @@ static int do_scsi_command(struct fsg_dev *fsg)
case READ_12:
fsg->data_size_from_cmnd =
- get_unaligned_be32(&fsg->cmnd[6]) << fsg->curlun->blkbits;
- if ((reply = check_command(fsg, 12, DATA_DIR_TO_HOST,
+ get_unaligned_be32(&fsg->cmnd[6]);
+ if ((reply = check_command_size_in_blocks(fsg, 12,
+ DATA_DIR_TO_HOST,
(1<<1) | (0xf<<2) | (0xf<<6), 1,
"READ(12)")) == 0)
reply = do_read(fsg);
@@ -2517,8 +2535,9 @@ static int do_scsi_command(struct fsg_dev *fsg)
case WRITE_6:
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,
+ fsg->data_size_from_cmnd = (i == 0 ? 256 : i);
+ if ((reply = check_command_size_in_blocks(fsg, 6,
+ DATA_DIR_FROM_HOST,
(7<<1) | (1<<4), 1,
"WRITE(6)")) == 0)
reply = do_write(fsg);
@@ -2526,8 +2545,9 @@ static int do_scsi_command(struct fsg_dev *fsg)
case WRITE_10:
fsg->data_size_from_cmnd =
- get_unaligned_be16(&fsg->cmnd[7]) << fsg->curlun->blkbits;
- if ((reply = check_command(fsg, 10, DATA_DIR_FROM_HOST,
+ get_unaligned_be16(&fsg->cmnd[7]);
+ if ((reply = check_command_size_in_blocks(fsg, 10,
+ DATA_DIR_FROM_HOST,
(1<<1) | (0xf<<2) | (3<<7), 1,
"WRITE(10)")) == 0)
reply = do_write(fsg);
@@ -2535,8 +2555,9 @@ static int do_scsi_command(struct fsg_dev *fsg)
case WRITE_12:
fsg->data_size_from_cmnd =
- get_unaligned_be32(&fsg->cmnd[6]) << fsg->curlun->blkbits;
- if ((reply = check_command(fsg, 12, DATA_DIR_FROM_HOST,
+ get_unaligned_be32(&fsg->cmnd[6]);
+ if ((reply = check_command_size_in_blocks(fsg, 12,
+ DATA_DIR_FROM_HOST,
(1<<1) | (0xf<<2) | (0xf<<6), 1,
"WRITE(12)")) == 0)
reply = do_write(fsg);
@@ -2683,6 +2704,14 @@ static int get_next_command(struct fsg_dev *fsg)
bh->state = BUF_STATE_EMPTY;
} else { // USB_PR_CB or USB_PR_CBI
+ struct usb_request *req;
+ struct fsg_bulk_cb_wrap *cbw;
+
+ /* update lun */
+ bh = fsg->next_buffhd_to_fill;
+ req = bh->outreq;
+ cbw = req->buf;
+ fsg->lun = cbw->Lun;
/* Wait for the next command to arrive */
while (fsg->cbbuf_cmnd_size == 0) {
@@ -2705,6 +2734,13 @@ static int get_next_command(struct fsg_dev *fsg)
fsg->cbbuf_cmnd_size = 0;
spin_unlock_irq(&fsg->lock);
}
+
+ /* Update current lun */
+ if (fsg->lun >= 0 && fsg->lun < fsg->nluns)
+ fsg->curlun = &fsg->luns[fsg->lun];
+ else
+ fsg->curlun = NULL;
+
return rc;
}
thanks
Yuping Luo
--
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
