Hi Greg,
Please accept these patches if no more comments. Thanks.
Mac OS-X expects CD-ROM TOC in raw format (i.e. format:2). It also
sends the READ_TOC CDB in old style SFF8020i format. i.e. 2 format bits
are encoded in MSBs of CDB byte 9.
The first patch introduces a fsg_get_toc() helper which builds the
Table of Contents (TOC) that can be used for the READ_TOC command.
The remaining two patches simply make file_storage gadget and mass_storage
gadget use the new helper function.
Changes in v3:
- return length fixed for fsg_get_toc()
Changes in v2:
- Review comments incorporated.
---
Roger Quadros (3):
usb: gadget: storage: Add fsg_get_toc helper
usb: gadget: file_storage: Make CD-ROM emulation work with Mac OS-X
usb: gadget: f_mass_storage: Make CD-ROM emulation work with Mac OS-X
drivers/usb/gadget/f_mass_storage.c | 31 ++++++++++-------
drivers/usb/gadget/file_storage.c | 31 ++++++++++-------
drivers/usb/gadget/storage_common.c | 65 +++++++++++++++++++++++++++++++++++
3 files changed, 101 insertions(+), 26 deletions(-)
fsg_get_toc can be used by both storage gadgets to build a CD-ROM
Table of Contents which can be used as data for READ_TOC command.
Currently fsg_get_toc supports two TOC formats, i.e.
format 0: Formatted TOC and format 2: Raw TOC.
Raw TOC is required for CD-ROM emulation to work with Mac OS-X.
Signed-off-by: Roger Quadros <[email protected]>
---
drivers/usb/gadget/storage_common.c | 65 +++++++++++++++++++++++++++++++++++
1 files changed, 65 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index b015561..38fd675 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -655,6 +655,71 @@ static void store_cdrom_address(u8 *dest, int msf, u32 addr)
}
}
+/**
+ * fsg_get_toc() - Builds a TOC with required format @format.
+ * @curlun: The LUN for which the TOC has to be built
+ * @msf: Min Sec Frame format or LBA format for address
+ * @format: TOC format code
+ * @buf: The buffer into which the TOC is built
+ *
+ * Builds a Table of Content which can be used as data for READ_TOC command.
+ * The TOC simulates a single session, single track CD-ROM mode 1 disc.
+ *
+ * Returns number of bytes written to @buf, -EINVAL if format not supported.
+ */
+static int fsg_get_toc(struct fsg_lun *curlun, int msf, int format, u8 *buf)
+{
+ int i, len;
+ switch (format) {
+ case 0:
+ /* Formatted TOC */
+ len = 4 + 2*8; /* 4 byte header + 2 descriptors */
+ memset(buf, 0, len);
+ buf[1] = len - 2; /* TOC Length excludes length field */
+ buf[2] = 1; /* First track number */
+ buf[3] = 1; /* Last track number */
+ buf[5] = 0x16; /* Data track, copying allowed */
+ buf[6] = 0x01; /* Only track is number 1 */
+ store_cdrom_address(&buf[8], msf, 0);
+
+ buf[13] = 0x16; /* Lead-out track is data */
+ buf[14] = 0xAA; /* Lead-out track number */
+ store_cdrom_address(&buf[16], msf, curlun->num_sectors);
+ return len;
+ break;
+
+ case 2:
+ /* Raw TOC */
+ len = 4 + 3*11; /* 4 byte header + 3 descriptors */
+ memset(buf, 0, len); /* Header + A0, A1 & A2 descriptors */
+ buf[1] = len - 2; /* TOC Length excludes length field */
+ buf[2] = 1; /* First complete session */
+ buf[3] = 1; /* Last complete session */
+
+ buf += 4;
+ /* fill in A0, A1 and A2 points */
+ for (i = 0; i < 3; i++) {
+ buf[0] = 1; /* Session number */
+ buf[1] = 0x16; /* Data track, copying allowed */
+ /* 2 - Track number 0 -> TOC */
+ buf[3] = 0xA0 + i; /* A0, A1, A2 point */
+ /* 4, 5, 6 - Min, sec, frame is zero */
+ buf[8] = 1; /* Pmin: last track number */
+ buf += 11; /* go to next track descriptor */
+ }
+ buf -= 11; /* go back to A2 descriptor */
+
+ /* For A2, 7, 8, 9, 10 - zero, Pmin, Psec, Pframe of Lead out */
+ store_cdrom_address(&buf[7], msf, curlun->num_sectors);
+ return len;
+ break;
+ default:
+ /* Multi-session, PMA, ATIP, CD-TEXT not supported/required */
+ return -EINVAL;
+ break;
+ }
+}
+
/*-------------------------------------------------------------------------*/
--
1.6.0.4
Mac OS-X expects CD-ROM TOC in raw format (i.e. format:2). It also
sends the READ_TOC CDB in old style SFF8020i format. i.e. 2 format bits
are encoded in MSBs of CDB byte 9.
This patch will enable CD-ROM emulation to work with Mac OS-X. Tested on
Mac OS X v10.6.3.
Signed-off-by: Roger Quadros <[email protected]>
---
drivers/usb/gadget/f_mass_storage.c | 31 ++++++++++++++++++-------------
1 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index 6d8e533..ef6c65a 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -1315,6 +1315,8 @@ static int do_read_toc(struct fsg_common *common, struct fsg_buffhd *bh)
int msf = common->cmnd[1] & 0x02;
int start_track = common->cmnd[6];
u8 *buf = (u8 *)bh->buf;
+ u8 format;
+ int ret;
if ((common->cmnd[1] & ~0x02) != 0 || /* Mask away MSF */
start_track > 1) {
@@ -1322,18 +1324,21 @@ static int do_read_toc(struct fsg_common *common, struct fsg_buffhd *bh)
return -EINVAL;
}
- memset(buf, 0, 20);
- buf[1] = (20-2); /* TOC data length */
- buf[2] = 1; /* First track number */
- buf[3] = 1; /* Last track number */
- buf[5] = 0x16; /* Data track, copying allowed */
- buf[6] = 0x01; /* Only track is number 1 */
- store_cdrom_address(&buf[8], msf, 0);
+ format = common->cmnd[2] & 0xf;
+ /*
+ * Check if CDB is old style SFF-8020i
+ * i.e. format is in 2 MSBs of byte 9
+ * Mac OS-X host sends us this.
+ */
+ if (format == 0)
+ format = (common->cmnd[9] >> 6) & 0x3;
- buf[13] = 0x16; /* Lead-out track is data */
- buf[14] = 0xAA; /* Lead-out track number */
- store_cdrom_address(&buf[16], msf, curlun->num_sectors);
- return 20;
+ ret = fsg_get_toc(curlun, msf, format, buf);
+ if (ret < 0) {
+ curlun->sense_data = SS_INVALID_FIELD_IN_CDB;
+ return -EINVAL;
+ }
+ return ret;
}
static int do_mode_sense(struct fsg_common *common, struct fsg_buffhd *bh)
@@ -2103,7 +2108,7 @@ static int do_scsi_command(struct fsg_common *common)
common->data_size_from_cmnd =
get_unaligned_be16(&common->cmnd[7]);
reply = check_command(common, 10, DATA_DIR_TO_HOST,
- (7<<6) | (1<<1), 1,
+ (0xf<<6) | (1<<1), 1,
"READ TOC");
if (reply == 0)
reply = do_read_toc(common, bh);
--
1.6.0.4
Mac OS-X expects CD-ROM TOC in raw format (i.e. format:2). It also
sends the READ_TOC CDB in old style SFF8020i format. i.e. 2 format bits
are encoded in MSBs of CDB byte 9.
This patch will enable CD-ROM emulation to work with Mac OS-X. Tested on
Mac OS X v10.6.3.
Signed-off-by: Roger Quadros <[email protected]>
---
drivers/usb/gadget/file_storage.c | 31 ++++++++++++++++++-------------
1 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
index a6eacb5..03beb2e 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -1696,6 +1696,8 @@ static int do_read_toc(struct fsg_dev *fsg, struct fsg_buffhd *bh)
int msf = fsg->cmnd[1] & 0x02;
int start_track = fsg->cmnd[6];
u8 *buf = (u8 *) bh->buf;
+ u8 format;
+ int ret;
if ((fsg->cmnd[1] & ~0x02) != 0 || /* Mask away MSF */
start_track > 1) {
@@ -1703,18 +1705,21 @@ static int do_read_toc(struct fsg_dev *fsg, struct fsg_buffhd *bh)
return -EINVAL;
}
- memset(buf, 0, 20);
- buf[1] = (20-2); /* TOC data length */
- buf[2] = 1; /* First track number */
- buf[3] = 1; /* Last track number */
- buf[5] = 0x16; /* Data track, copying allowed */
- buf[6] = 0x01; /* Only track is number 1 */
- store_cdrom_address(&buf[8], msf, 0);
+ format = fsg->cmnd[2] & 0xf;
+ /*
+ * Check if CDB is old style SFF-8020i
+ * i.e. format is in 2 MSBs of byte 9
+ * Mac OS-X host sends us this.
+ */
+ if (format == 0)
+ format = (fsg->cmnd[9] >> 6) & 0x3;
- buf[13] = 0x16; /* Lead-out track is data */
- buf[14] = 0xAA; /* Lead-out track number */
- store_cdrom_address(&buf[16], msf, curlun->num_sectors);
- return 20;
+ ret = fsg_get_toc(curlun, msf, format, buf);
+ if (ret < 0) {
+ curlun->sense_data = SS_INVALID_FIELD_IN_CDB;
+ return -EINVAL;
+ }
+ return ret;
}
@@ -2486,7 +2491,7 @@ static int do_scsi_command(struct fsg_dev *fsg)
goto unknown_cmnd;
fsg->data_size_from_cmnd = get_unaligned_be16(&fsg->cmnd[7]);
if ((reply = check_command(fsg, 10, DATA_DIR_TO_HOST,
- (7<<6) | (1<<1), 1,
+ (0xf<<6) | (1<<1), 1,
"READ TOC")) == 0)
reply = do_read_toc(fsg, bh);
break;
--
1.6.0.4
On Tue, 05 Apr 2011 16:59:27 +0200, Roger Quadros
<[email protected]> wrote:
> --- a/drivers/usb/gadget/storage_common.c
> +++ b/drivers/usb/gadget/storage_common.c
> +static int fsg_get_toc(struct fsg_lun *curlun, int msf, int format, u8
> *buf)
> +{
> + int i, len;
> + switch (format) {
> + case 0:
> + /* Formatted TOC */
> + len = 4 + 2*8; /* 4 byte header + 2 descriptors */
> + memset(buf, 0, len);
> + buf[1] = len - 2; /* TOC Length excludes length field */
> + buf[2] = 1; /* First track number */
> + buf[3] = 1; /* Last track number */
> + buf[5] = 0x16; /* Data track, copying allowed */
> + buf[6] = 0x01; /* Only track is number 1 */
> + store_cdrom_address(&buf[8], msf, 0);
> +
> + buf[13] = 0x16; /* Lead-out track is data */
> + buf[14] = 0xAA; /* Lead-out track number */
> + store_cdrom_address(&buf[16], msf, curlun->num_sectors);
> + return len;
> + break;
Useless break. Or, you could put "return len;" at the end of the function.
> +
> + case 2:
> + /* Raw TOC */
> + len = 4 + 3*11; /* 4 byte header + 3 descriptors */
> + memset(buf, 0, len); /* Header + A0, A1 & A2 descriptors */
> + buf[1] = len - 2; /* TOC Length excludes length field */
> + buf[2] = 1; /* First complete session */
> + buf[3] = 1; /* Last complete session */
> +
> + buf += 4;
> + /* fill in A0, A1 and A2 points */
> + for (i = 0; i < 3; i++) {
> + buf[0] = 1; /* Session number */
> + buf[1] = 0x16; /* Data track, copying allowed */
> + /* 2 - Track number 0 -> TOC */
> + buf[3] = 0xA0 + i; /* A0, A1, A2 point */
> + /* 4, 5, 6 - Min, sec, frame is zero */
> + buf[8] = 1; /* Pmin: last track number */
> + buf += 11; /* go to next track descriptor */
> + }
> + buf -= 11; /* go back to A2 descriptor */
> +
> + /* For A2, 7, 8, 9, 10 - zero, Pmin, Psec, Pframe of Lead out */
> + store_cdrom_address(&buf[7], msf, curlun->num_sectors);
> + return len;
> + break;
Same here.
> + default:
> + /* Multi-session, PMA, ATIP, CD-TEXT not supported/required */
> + return -EINVAL;
Can we fall back to the old behaviour here?
> + break;
And here.
> + }
> +}
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michal "mina86" Nazarewicz (o o)
ooo +-----<email/xmpp: [email protected]>-----ooO--(_)--Ooo--
On Tue, 05 Apr 2011 16:59:28 +0200, Roger Quadros
<[email protected]> wrote:
> --- a/drivers/usb/gadget/file_storage.c
> +++ b/drivers/usb/gadget/file_storage.c
> @@ -1703,18 +1705,21 @@ static int do_read_toc(struct fsg_dev *fsg,
> struct fsg_buffhd *bh)
> return -EINVAL;
> }
> - memset(buf, 0, 20);
> - buf[1] = (20-2); /* TOC data length */
> - buf[2] = 1; /* First track number */
> - buf[3] = 1; /* Last track number */
> - buf[5] = 0x16; /* Data track, copying allowed */
> - buf[6] = 0x01; /* Only track is number 1 */
> - store_cdrom_address(&buf[8], msf, 0);
> + format = fsg->cmnd[2] & 0xf;
> + /*
> + * Check if CDB is old style SFF-8020i
> + * i.e. format is in 2 MSBs of byte 9
> + * Mac OS-X host sends us this.
> + */
> + if (format == 0)
> + format = (fsg->cmnd[9] >> 6) & 0x3;
> - buf[13] = 0x16; /* Lead-out track is data */
> - buf[14] = 0xAA; /* Lead-out track number */
> - store_cdrom_address(&buf[16], msf, curlun->num_sectors);
> - return 20;
> + ret = fsg_get_toc(curlun, msf, format, buf);
> + if (ret < 0) {
> + curlun->sense_data = SS_INVALID_FIELD_IN_CDB;
> + return -EINVAL;
> + }
I'd drop the “return -EINVAL;” and let “return ret;” below handle it.
This obviously also apply to patch to f_mass_storage.c (BTW. I would
merge the two patches).
> + return ret;
> }
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michal "mina86" Nazarewicz (o o)
ooo +-----<email/xmpp: [email protected]>-----ooO--(_)--Ooo--
Hi,
Looks like I somehow missed this mail.
On 04/05/2011 06:43 PM, ext Michal Nazarewicz wrote:
> On Tue, 05 Apr 2011 16:59:27 +0200, Roger Quadros <[email protected]> wrote:
>> --- a/drivers/usb/gadget/storage_common.c
>> +++ b/drivers/usb/gadget/storage_common.c
>
>> +static int fsg_get_toc(struct fsg_lun *curlun, int msf, int format, u8 *buf)
>> +{
>> + int i, len;
>> + switch (format) {
>> + case 0:
>> + /* Formatted TOC */
>> + len = 4 + 2*8; /* 4 byte header + 2 descriptors */
>> + memset(buf, 0, len);
>> + buf[1] = len - 2; /* TOC Length excludes length field */
>> + buf[2] = 1; /* First track number */
>> + buf[3] = 1; /* Last track number */
>> + buf[5] = 0x16; /* Data track, copying allowed */
>> + buf[6] = 0x01; /* Only track is number 1 */
>> + store_cdrom_address(&buf[8], msf, 0);
>> +
>> + buf[13] = 0x16; /* Lead-out track is data */
>> + buf[14] = 0xAA; /* Lead-out track number */
>> + store_cdrom_address(&buf[16], msf, curlun->num_sectors);
>> + return len;
>> + break;
>
> Useless break. Or, you could put "return len;" at the end of the function.
>
Yes I will fix this.
>> +
>> + case 2:
>> + /* Raw TOC */
>> + len = 4 + 3*11; /* 4 byte header + 3 descriptors */
>> + memset(buf, 0, len); /* Header + A0, A1 & A2 descriptors */
>> + buf[1] = len - 2; /* TOC Length excludes length field */
>> + buf[2] = 1; /* First complete session */
>> + buf[3] = 1; /* Last complete session */
>> +
>> + buf += 4;
>> + /* fill in A0, A1 and A2 points */
>> + for (i = 0; i < 3; i++) {
>> + buf[0] = 1; /* Session number */
>> + buf[1] = 0x16; /* Data track, copying allowed */
>> + /* 2 - Track number 0 -> TOC */
>> + buf[3] = 0xA0 + i; /* A0, A1, A2 point */
>> + /* 4, 5, 6 - Min, sec, frame is zero */
>> + buf[8] = 1; /* Pmin: last track number */
>> + buf += 11; /* go to next track descriptor */
>> + }
>> + buf -= 11; /* go back to A2 descriptor */
>> +
>> + /* For A2, 7, 8, 9, 10 - zero, Pmin, Psec, Pframe of Lead out */
>> + store_cdrom_address(&buf[7], msf, curlun->num_sectors);
>> + return len;
>> + break;
>
> Same here.
ok
>
>> + default:
>> + /* Multi-session, PMA, ATIP, CD-TEXT not supported/required */
>> + return -EINVAL;
>
> Can we fall back to the old behaviour here?
What old behaviour? I didn't understand.
>
>> + break;
>
> And here.
>
>> + }
>> +}
>
--
regards,
-roger
On 04/05/2011 06:45 PM, ext Michal Nazarewicz wrote:
> On Tue, 05 Apr 2011 16:59:28 +0200, Roger Quadros <[email protected]> wrote:
>> --- a/drivers/usb/gadget/file_storage.c
>> +++ b/drivers/usb/gadget/file_storage.c
>> @@ -1703,18 +1705,21 @@ static int do_read_toc(struct fsg_dev *fsg, struct
>> fsg_buffhd *bh)
>> return -EINVAL;
>> }
>> - memset(buf, 0, 20);
>> - buf[1] = (20-2); /* TOC data length */
>> - buf[2] = 1; /* First track number */
>> - buf[3] = 1; /* Last track number */
>> - buf[5] = 0x16; /* Data track, copying allowed */
>> - buf[6] = 0x01; /* Only track is number 1 */
>> - store_cdrom_address(&buf[8], msf, 0);
>> + format = fsg->cmnd[2] & 0xf;
>> + /*
>> + * Check if CDB is old style SFF-8020i
>> + * i.e. format is in 2 MSBs of byte 9
>> + * Mac OS-X host sends us this.
>> + */
>> + if (format == 0)
>> + format = (fsg->cmnd[9] >> 6) & 0x3;
>> - buf[13] = 0x16; /* Lead-out track is data */
>> - buf[14] = 0xAA; /* Lead-out track number */
>> - store_cdrom_address(&buf[16], msf, curlun->num_sectors);
>> - return 20;
>> + ret = fsg_get_toc(curlun, msf, format, buf);
>> + if (ret < 0) {
>> + curlun->sense_data = SS_INVALID_FIELD_IN_CDB;
>> + return -EINVAL;
>> + }
>
> I'd drop the “return -EINVAL;” and let “return ret;” below handle it.
> This obviously also apply to patch to f_mass_storage.c (BTW. I would
> merge the two patches).
OK, I will fix this.
>
>> + return ret;
>> }
>
Thanks for review.
--
regards,
-roger
> On 04/05/2011 06:43 PM, ext Michal Nazarewicz wrote:
>> On Tue, 05 Apr 2011 16:59:27 +0200, Roger Quadros wrote:
>>> + default:
>>> + /* Multi-session, PMA, ATIP, CD-TEXT not supported/required */
>>> + return -EINVAL;
> On 04/05/2011 06:43 PM, ext Michal Nazarewicz wrote:
>> Can we fall back to the old behaviour here?
On Wed, 11 May 2011 09:49:36 +0200, Roger Quadros <roger.
> What old behaviour? I didn't understand.
Before we were just ignoring the type of ToC requested, right?
So can we fail back to the ToC we served then when we hit
an unsupported TOC? Or does it make no sense at all? I'm not
saying it does. It probably doesn't. ;)
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michal "mina86" Nazarewicz (o o)
ooo +-----<email/xmpp: [email protected]>-----ooO--(_)--Ooo--
On 05/11/2011 03:19 PM, ext Michal Nazarewicz wrote:
>> On 04/05/2011 06:43 PM, ext Michal Nazarewicz wrote:
>>> On Tue, 05 Apr 2011 16:59:27 +0200, Roger Quadros wrote:
>>>> + default:
>>>> + /* Multi-session, PMA, ATIP, CD-TEXT not supported/required */
>>>> + return -EINVAL;
>
>> On 04/05/2011 06:43 PM, ext Michal Nazarewicz wrote:
>>> Can we fall back to the old behaviour here?
>
> On Wed, 11 May 2011 09:49:36 +0200, Roger Quadros <roger.
>> What old behaviour? I didn't understand.
>
> Before we were just ignoring the type of ToC requested, right?
> So can we fail back to the ToC we served then when we hit
> an unsupported TOC? Or does it make no sense at all? I'm not
> saying it does. It probably doesn't. ;)
>
Before we were returning -EINVAL for unsupported request. We do the same now.
--
regards,
-roger
> On 05/11/2011 03:19 PM, ext Michal Nazarewicz wrote:
>> Before we were just ignoring the type of ToC requested, right?
>> So can we fail back to the ToC we served then when we hit
>> an unsupported TOC? Or does it make no sense at all? I'm not
>> saying it does. It probably doesn't. ;)
On Wed, 11 May 2011 14:31:49 +0200, Roger Quadros wrote:
> Before we were returning -EINVAL for unsupported request. We do the same
> now.
Cool than, disregard my comment. ;)
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michal "mina86" Nazarewicz (o o)
ooo +-----<email/xmpp: [email protected]>-----ooO--(_)--Ooo--