* [PATCH 0/1] usb: gadget: f_mass_storage: Make CD-ROM emulation work with Mac OS-X @ 2022-01-10 6:23 Jack Pham 2022-01-10 6:23 ` [PATCH] " Jack Pham 0 siblings, 1 reply; 7+ messages in thread From: Jack Pham @ 2022-01-10 6:23 UTC (permalink / raw) To: Greg KH, Roger Quadros, Alan Stern, Michal Nazarewicz Cc: linux-usb, Jack Pham Hi, This is a resurrection of a *very* old patch [1] from 2011 that apparently never landed upstream. One of our customers is reporting exactly the failure described in $SUBJECT, and mentioned that applying this change in their downstream kernel allows their mass storage function to successfully connect to OS X hosts. So perhaps we can finally try to merge it, better late than never. The original series was applicable to both the mass_storage and now-defunct file_storage gadget functions. So I've simply squashed the changes into a single patch applicable to f_mass_storage.c @Roger I've kept the authorship email from your original submission since it was retained from "git am". I'm not sure if it's ok/appropriate to update it to your current @kernel.org address? [1] https://lore.kernel.org/lkml/1302015569-9668-1-git-send-email-roger.quadros@nokia.com/T/#u Roger Quadros (1): usb: gadget: f_mass_storage: Make CD-ROM emulation work with Mac OS-X drivers/usb/gadget/function/f_mass_storage.c | 73 +++++++++++++++++++++++----- 1 file changed, 61 insertions(+), 12 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] usb: gadget: f_mass_storage: Make CD-ROM emulation work with Mac OS-X 2022-01-10 6:23 [PATCH 0/1] usb: gadget: f_mass_storage: Make CD-ROM emulation work with Mac OS-X Jack Pham @ 2022-01-10 6:23 ` Jack Pham 2022-01-10 6:27 ` Jack Pham 2022-01-10 6:30 ` [PATCH v2] " Jack Pham 0 siblings, 2 replies; 7+ messages in thread From: Jack Pham @ 2022-01-10 6:23 UTC (permalink / raw) To: Greg KH, Roger Quadros, Alan Stern, Michal Nazarewicz Cc: linux-usb, Roger Quadros, Jack Pham From: Roger Quadros <roger.quadros@nokia.com> 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. Change-Id: Ifdbe10e779bc8609a11eeab6ab6de549c6223fe2 Signed-off-by: Roger Quadros <roger.quadros@nokia.com> Signed-off-by: Jack Pham <quic_jackp@quicinc.com> --- drivers/usb/gadget/function/f_mass_storage.c | 73 +++++++++++++++++++++++----- 1 file changed, 61 insertions(+), 12 deletions(-) diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 73a28f8..1f7f4dd6 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -1188,6 +1188,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 i, len; if ((common->cmnd[1] & ~0x02) != 0 || /* Mask away MSF */ start_track > 1) { @@ -1195,18 +1197,65 @@ 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; + + switch (format) { + case 0: + /* Formatted TOC */ + len = 4 + 2*8; /* 4 byte header + 2 descriptors */ + memset(buf, 0, len); + len -= 2; /* TOC Length excludes length field */ + buf[1] = len; /* 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); + + 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; + + case 2: + /* Raw TOC */ + len = 4 + 3*11; /* 4 byte header + 3 descriptors */ + memset(buf, 0, len); /* Header + A0, A1 & A2 descriptors */ + len -= 2; /* TOC Length excludes length field */ + buf[1] = len; /* TOC data length */ + 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); - 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; + return len; + + default: + /* Multi-session, PMA, ATIP, CD-TEXT not supported/required */ + curlun->sense_data = SS_INVALID_FIELD_IN_CDB; + return -EINVAL; + } } static int do_mode_sense(struct fsg_common *common, struct fsg_buffhd *bh) @@ -1933,7 +1982,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); -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: gadget: f_mass_storage: Make CD-ROM emulation work with Mac OS-X 2022-01-10 6:23 ` [PATCH] " Jack Pham @ 2022-01-10 6:27 ` Jack Pham 2022-01-10 6:30 ` [PATCH v2] " Jack Pham 1 sibling, 0 replies; 7+ messages in thread From: Jack Pham @ 2022-01-10 6:27 UTC (permalink / raw) To: Greg KH, Roger Quadros, Alan Stern, Michal Nazarewicz Cc: linux-usb, Roger Quadros On Sun, Jan 09, 2022 at 10:23:59PM -0800, Jack Pham wrote: > From: Roger Quadros <roger.quadros@nokia.com> > > 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. > > Change-Id: Ifdbe10e779bc8609a11eeab6ab6de549c6223fe2 Argh, sorry about this! Will remove in V2. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] usb: gadget: f_mass_storage: Make CD-ROM emulation work with Mac OS-X 2022-01-10 6:23 ` [PATCH] " Jack Pham 2022-01-10 6:27 ` Jack Pham @ 2022-01-10 6:30 ` Jack Pham 2022-01-12 20:05 ` Alan Stern 1 sibling, 1 reply; 7+ messages in thread From: Jack Pham @ 2022-01-10 6:30 UTC (permalink / raw) To: Greg KH, Roger Quadros, Alan Stern, Michal Nazarewicz Cc: linux-usb, Roger Quadros, Jack Pham From: Roger Quadros <roger.quadros@nokia.com> 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 <roger.quadros@nokia.com> Signed-off-by: Jack Pham <quic_jackp@quicinc.com> --- v2: Removed Change-Id tag. drivers/usb/gadget/function/f_mass_storage.c | 73 +++++++++++++++++++++++----- 1 file changed, 61 insertions(+), 12 deletions(-) diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 73a28f8..1f7f4dd6 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -1188,6 +1188,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 i, len; if ((common->cmnd[1] & ~0x02) != 0 || /* Mask away MSF */ start_track > 1) { @@ -1195,18 +1197,65 @@ 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; + + switch (format) { + case 0: + /* Formatted TOC */ + len = 4 + 2*8; /* 4 byte header + 2 descriptors */ + memset(buf, 0, len); + len -= 2; /* TOC Length excludes length field */ + buf[1] = len; /* 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); + + 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; + + case 2: + /* Raw TOC */ + len = 4 + 3*11; /* 4 byte header + 3 descriptors */ + memset(buf, 0, len); /* Header + A0, A1 & A2 descriptors */ + len -= 2; /* TOC Length excludes length field */ + buf[1] = len; /* TOC data length */ + 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); - 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; + return len; + + default: + /* Multi-session, PMA, ATIP, CD-TEXT not supported/required */ + curlun->sense_data = SS_INVALID_FIELD_IN_CDB; + return -EINVAL; + } } static int do_mode_sense(struct fsg_common *common, struct fsg_buffhd *bh) @@ -1933,7 +1982,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); -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] usb: gadget: f_mass_storage: Make CD-ROM emulation work with Mac OS-X 2022-01-10 6:30 ` [PATCH v2] " Jack Pham @ 2022-01-12 20:05 ` Alan Stern 2022-01-13 5:22 ` Jack Pham 0 siblings, 1 reply; 7+ messages in thread From: Alan Stern @ 2022-01-12 20:05 UTC (permalink / raw) To: Jack Pham Cc: Greg KH, Roger Quadros, Michal Nazarewicz, linux-usb, Roger Quadros On Sun, Jan 09, 2022 at 10:30:30PM -0800, Jack Pham wrote: > From: Roger Quadros <roger.quadros@nokia.com> > > 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 <roger.quadros@nokia.com> > Signed-off-by: Jack Pham <quic_jackp@quicinc.com> > --- > v2: Removed Change-Id tag. > > drivers/usb/gadget/function/f_mass_storage.c | 73 +++++++++++++++++++++++----- > 1 file changed, 61 insertions(+), 12 deletions(-) > > diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c > index 73a28f8..1f7f4dd6 100644 > --- a/drivers/usb/gadget/function/f_mass_storage.c > +++ b/drivers/usb/gadget/function/f_mass_storage.c > @@ -1188,6 +1188,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 i, len; > > if ((common->cmnd[1] & ~0x02) != 0 || /* Mask away MSF */ > start_track > 1) { > @@ -1195,18 +1197,65 @@ 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; Hmmm. According to this part later on: > @@ -1933,7 +1982,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"); common->cmnd[2] can never be anything other than 0. So this computation and the test immediately below are pointless -- unless you change the argument to check_command(). Alan Stern > + /* > + * 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; > + > + switch (format) { > + case 0: > + /* Formatted TOC */ > + len = 4 + 2*8; /* 4 byte header + 2 descriptors */ > + memset(buf, 0, len); > + len -= 2; /* TOC Length excludes length field */ > + buf[1] = len; /* 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); > + > + 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; > + > + case 2: > + /* Raw TOC */ > + len = 4 + 3*11; /* 4 byte header + 3 descriptors */ > + memset(buf, 0, len); /* Header + A0, A1 & A2 descriptors */ > + len -= 2; /* TOC Length excludes length field */ > + buf[1] = len; /* TOC data length */ > + 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); > > - 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; > + return len; > + > + default: > + /* Multi-session, PMA, ATIP, CD-TEXT not supported/required */ > + curlun->sense_data = SS_INVALID_FIELD_IN_CDB; > + return -EINVAL; > + } > } > > static int do_mode_sense(struct fsg_common *common, struct fsg_buffhd *bh) > @@ -1933,7 +1982,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); > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] usb: gadget: f_mass_storage: Make CD-ROM emulation work with Mac OS-X 2022-01-12 20:05 ` Alan Stern @ 2022-01-13 5:22 ` Jack Pham 2022-01-13 15:20 ` Alan Stern 0 siblings, 1 reply; 7+ messages in thread From: Jack Pham @ 2022-01-13 5:22 UTC (permalink / raw) To: Alan Stern Cc: Greg KH, Roger Quadros, Michal Nazarewicz, linux-usb, Roger Quadros Hi Alan, On Wed, Jan 12, 2022 at 03:05:35PM -0500, Alan Stern wrote: > On Sun, Jan 09, 2022 at 10:30:30PM -0800, Jack Pham wrote: > > From: Roger Quadros <roger.quadros@nokia.com> > > > > 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 <roger.quadros@nokia.com> > > Signed-off-by: Jack Pham <quic_jackp@quicinc.com> > > --- > > v2: Removed Change-Id tag. > > > > drivers/usb/gadget/function/f_mass_storage.c | 73 +++++++++++++++++++++++----- > > 1 file changed, 61 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c > > index 73a28f8..1f7f4dd6 100644 > > --- a/drivers/usb/gadget/function/f_mass_storage.c > > +++ b/drivers/usb/gadget/function/f_mass_storage.c > > @@ -1188,6 +1188,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 i, len; > > > > if ((common->cmnd[1] & ~0x02) != 0 || /* Mask away MSF */ > > start_track > 1) { > > @@ -1195,18 +1197,65 @@ 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; > > Hmmm. According to this part later on: > > > @@ -1933,7 +1982,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"); > > common->cmnd[2] can never be anything other than 0. Ah, that is true. So to allow for cmnd[2] (as well as cmnd[9] as intended by the patch) to be non-zero, the "mask" argument should rather be: (0xf<<6) | (3<<1) In other words a bitmask of 0x3c6, corresponding to command data bytes 1, 2, 6, 7, 8 and 9. Is my understanding correct? > So this computation and the test immediately below are pointless -- > unless you change the argument to check_command(). If you are referring to the "if (format == 0)" check, then I believe you are right. > > + /* > > + * 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; It seems this is the gist of the patch. Without changing the mask parameter to check_command() above, we know we can only reach here if format = common->cmnd[2] is 0. However this snippet is then reassigning format from the upper bits of the 9th byte which could be non-zero, at least in the case of MacOS. So this patch does seem a bit incomplete as you point out and maybe updating the mask as above should help to allow both fields to determine the format for any non-zero TOC type. But I was trying to confirm these details from the SFF-8020i spec as mentioned in the original comment above. I was only able to find a draft copy [1] from a web search which stated: "Format field definition: When Format in Byte 2 is zero, then Byte 9 is used. Other values for this field are reserved for definition in MMC." Note: The Format field in Byte 9 is a vendor-specific area and will be removed in subsequent versions of this specification. Functionality is moving to Byte 2." However when trying to look up the latest official release of SFF-8020 the SNIA website [2] lists it as having been expired and incorporated into the SCSI MMC specification. Consequently, I haven't yet been able to look further into the SCSI MMC spec itself as it seems it is only available for a fee from the ANSI/INCITS website [3]. I'm hoping you or maybe somebody on this list might have more knowledge on these details. So I'm left wondering whether the Format field in Byte 9 is even standardized, or if it is a remnant of an older or possibly draft/non-final specification. Yet we clearly have a host that is relying on this behavior, so there is utility in this patch. FWIW, here are the raw bytes of the READ TOC request transaction as issued by the MacOS host, obtained from a bus analyzer trace, which this patch is purportedly fixing: 55 53 42 43 19 00 00 00 FE FF 00 00 80 00 0A 43 ^^ cmnd[0] i.e. OpCode 02 00 00 00 00 00 FF FE 80 00 00 00 00 00 00 ^^ ^^ cmnd[2]==0 || cmnd[9], upper 2 bits == 0x2 Thanks, Jack [1] https://www.bswd.com/sff8020i.pdf [2] https://www.snia.org/technology-communities/sff/specifications [3] https://webstore.ansi.org/standards/incits/ansiincits4302007 > > + > > + switch (format) { > > + case 0: > > + /* Formatted TOC */ > > + len = 4 + 2*8; /* 4 byte header + 2 descriptors */ > > + memset(buf, 0, len); > > + len -= 2; /* TOC Length excludes length field */ > > + buf[1] = len; /* 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); > > + > > + 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; > > + > > + case 2: > > + /* Raw TOC */ > > + len = 4 + 3*11; /* 4 byte header + 3 descriptors */ > > + memset(buf, 0, len); /* Header + A0, A1 & A2 descriptors */ > > + len -= 2; /* TOC Length excludes length field */ > > + buf[1] = len; /* TOC data length */ > > + 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); > > > > - 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; > > + return len; > > + > > + default: > > + /* Multi-session, PMA, ATIP, CD-TEXT not supported/required */ > > + curlun->sense_data = SS_INVALID_FIELD_IN_CDB; > > + return -EINVAL; > > + } > > } > > > > static int do_mode_sense(struct fsg_common *common, struct fsg_buffhd *bh) > > @@ -1933,7 +1982,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); > > -- > > 2.7.4 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] usb: gadget: f_mass_storage: Make CD-ROM emulation work with Mac OS-X 2022-01-13 5:22 ` Jack Pham @ 2022-01-13 15:20 ` Alan Stern 0 siblings, 0 replies; 7+ messages in thread From: Alan Stern @ 2022-01-13 15:20 UTC (permalink / raw) To: Jack Pham Cc: Greg KH, Roger Quadros, Michal Nazarewicz, linux-usb, Roger Quadros On Wed, Jan 12, 2022 at 09:22:53PM -0800, Jack Pham wrote: > Hi Alan, > > On Wed, Jan 12, 2022 at 03:05:35PM -0500, Alan Stern wrote: > > On Sun, Jan 09, 2022 at 10:30:30PM -0800, Jack Pham wrote: > > > From: Roger Quadros <roger.quadros@nokia.com> > > > > > > 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 <roger.quadros@nokia.com> > > > Signed-off-by: Jack Pham <quic_jackp@quicinc.com> > > > --- > > > v2: Removed Change-Id tag. > > > > > > drivers/usb/gadget/function/f_mass_storage.c | 73 +++++++++++++++++++++++----- > > > 1 file changed, 61 insertions(+), 12 deletions(-) > > > > > > diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c > > > index 73a28f8..1f7f4dd6 100644 > > > --- a/drivers/usb/gadget/function/f_mass_storage.c > > > +++ b/drivers/usb/gadget/function/f_mass_storage.c > > > @@ -1188,6 +1188,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 i, len; > > > > > > if ((common->cmnd[1] & ~0x02) != 0 || /* Mask away MSF */ > > > start_track > 1) { > > > @@ -1195,18 +1197,65 @@ 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; > > > > Hmmm. According to this part later on: > > > > > @@ -1933,7 +1982,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"); > > > > common->cmnd[2] can never be anything other than 0. > > Ah, that is true. So to allow for cmnd[2] (as well as cmnd[9] as > intended by the patch) to be non-zero, the "mask" argument should rather > be: > > (0xf<<6) | (3<<1) Indeed. > In other words a bitmask of 0x3c6, corresponding to command data bytes > 1, 2, 6, 7, 8 and 9. Is my understanding correct? Yes, that's right. > > So this computation and the test immediately below are pointless -- > > unless you change the argument to check_command(). > > If you are referring to the "if (format == 0)" check, then I believe you > are right. > > > > + /* > > > + * 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; > > It seems this is the gist of the patch. Without changing the mask > parameter to check_command() above, we know we can only reach here if > format = common->cmnd[2] is 0. However this snippet is then reassigning > format from the upper bits of the 9th byte which could be non-zero, at > least in the case of MacOS. > > So this patch does seem a bit incomplete as you point out and maybe > updating the mask as above should help to allow both fields to determine > the format for any non-zero TOC type. > > But I was trying to confirm these details from the SFF-8020i spec as > mentioned in the original comment above. I was only able to find a > draft copy [1] from a web search which stated: > > "Format field definition: When Format in Byte 2 is zero, then > Byte 9 is used. Other values for this field are reserved for > definition in MMC." > > Note: The Format field in Byte 9 is a vendor-specific area and > will be removed in subsequent versions of this specification. > Functionality is moving to Byte 2." > > However when trying to look up the latest official release of SFF-8020 > the SNIA website [2] lists it as having been expired and incorporated > into the SCSI MMC specification. Consequently, I haven't yet been able > to look further into the SCSI MMC spec itself as it seems it is only > available for a fee from the ANSI/INCITS website [3]. I'm hoping you or > maybe somebody on this list might have more knowledge on these details. I thought I had a copy of the old SFF8020i spec somewhere, but now I can't find it. :-( > So I'm left wondering whether the Format field in Byte 9 is even > standardized, or if it is a remnant of an older or possibly > draft/non-final specification. Yet we clearly have a host that is > relying on this behavior, so there is utility in this patch. > > FWIW, here are the raw bytes of the READ TOC request transaction as > issued by the MacOS host, obtained from a bus analyzer trace, which this > patch is purportedly fixing: > > 55 53 42 43 19 00 00 00 FE FF 00 00 80 00 0A 43 > ^^ > cmnd[0] i.e. OpCode > 02 00 00 00 00 00 FF FE 80 00 00 00 00 00 00 > ^^ ^^ > cmnd[2]==0 || > cmnd[9], upper 2 bits == 0x2 Well, there are really just two things we need to worry about: The driver must continue to work properly on all the systems that are using it now. The driver should be able to work with Mac OS-X. I think if you simply change the mask value then we'll be okay. Alan Stern > Thanks, > Jack > > [1] https://www.bswd.com/sff8020i.pdf > [2] https://www.snia.org/technology-communities/sff/specifications > [3] https://webstore.ansi.org/standards/incits/ansiincits4302007 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-01-13 15:20 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-10 6:23 [PATCH 0/1] usb: gadget: f_mass_storage: Make CD-ROM emulation work with Mac OS-X Jack Pham 2022-01-10 6:23 ` [PATCH] " Jack Pham 2022-01-10 6:27 ` Jack Pham 2022-01-10 6:30 ` [PATCH v2] " Jack Pham 2022-01-12 20:05 ` Alan Stern 2022-01-13 5:22 ` Jack Pham 2022-01-13 15:20 ` Alan Stern
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).