On Jun 3, 2008, at 12:12 AM, Alex Williamson wrote:
Hi Alex,
I think I've incorporated all your comments. I tested with sg_raw; the
embedded size value is constant regardless of the allocation length in
the command. I added extra parameters to ide_dvd_read_structure() but I
still had to pass the IDEState pointer for things like the sector count,
and maybe others in the future as more command options are added. I
This is perfectly fine. To clean up things, we would simply need an abstract CDROMState that includes the same values. The only thing we can't provide in that one are IDE status codes.
On Jun 3, 2008, at 12:45 AM, Alex Williamson wrote:
This patch just updates the previous to apply cleanly since the get
configuration change went in. Thanks,
Alex
On Mon, 2008-06-02 at 16:12 -0600, Alex Williamson wrote:
Hi Alex,
I think I've incorporated all your comments. I tested with sg_raw; the
embedded size value is constant regardless of the allocation length in
the command. I added extra parameters to ide_dvd_read_structure() but I
still had to pass the IDEState pointer for things like the sector count,
and maybe others in the future as more command options are added. I
implemented the 0xff command based on what I saw using sg_raw on real
hardware. I'll note that it seems to be device dependent whether this
command works when the drive is empty or contains CD media.
Hopefully we're getting close, let me know if you have further comments.
Thanks,
Alex
Fix ATAPI read drive structure command
Previous version ignored the allocation length parameter and read the
format byte from the wrong location. Re-implement to support the full
requirements for DVD-ROM and allow for easy extension later.
Signed-off-by: Alex Williamson <address@hidden>
--
@@ -1741,16 +1845,11 @@
/*
* the number of sectors from the media tells us which profile
* to use as current. 0 means there is no media
- *
- * XXX: fails to detect correctly DVDs with less data burned
- * than what a CD can hold
*/
- if (s -> nb_sectors) {
- if (s -> nb_sectors > CD_MAX_SECTORS)
- cpu_to_ube16(buf + 6, MMC_PROFILE_DVD_ROM);
- else
- cpu_to_ube16(buf + 6, MMC_PROFILE_CD_ROM);
- }
+ if (media_is_dvd(s))
+ cpu_to_ube16(buf + 6, MMC_PROFILE_DVD_ROM);
+ else if (media_is_cd(s))
+ cpu_to_ube16(buf + 6, MMC_PROFILE_CD_ROM);
After having looked at the spec and a real dvd rom output I got uncertain if this is correct. Shouldn't capabilities provide the capabilities of the drive and not the medium?
Apart from that the patch looks fine to me.
Alex
|