Date: Fri, 16 Nov 2007 10:12:01 -0700 From: Scott Long <scottl@samsco.org> To: =?ISO-8859-1?Q?S=F8ren_Schmidt?= <sos@deepcore.dk> Cc: cvs-src@FreeBSD.ORG, src-committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG, Nate Lawson <nate@root.org> Subject: Re: cvs commit: src/sys/dev/ata atapi-cd.c atapi-cd.h Message-ID: <473DCF61.6010107@samsco.org> In-Reply-To: <473DCBDA.1080104@deepcore.dk> References: <200710260859.l9Q8xPdP099307@repoman.freebsd.org> <473DC220.6030809@samsco.org> <473DC93D.5060205@deepcore.dk> <473DCAA7.3090408@root.org> <473DCBDA.1080104@deepcore.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
Søren Schmidt wrote: > Nate Lawson wrote: >> Søren Schmidt wrote: >> >>> Scott Long wrote: >>> >>>> SXren Schmidt wrote: >>>> >>>>> sos 2007-10-26 08:59:24 UTC >>>>> >>>>> FreeBSD src repository >>>>> >>>>> Modified files: >>>>> sys/dev/ata atapi-cd.c atapi-cd.h Log: >>>>> Update the way we get the mode pages on probe. >>>>> Revision Changes Path >>>>> 1.194 +22 -25 src/sys/dev/ata/atapi-cd.c >>>>> 1.47 +1 -0 src/sys/dev/ata/atapi-cd.h >>>>> >>>> Just curious, what was the motivation for changing from doing a >>>> TUR to doing a MODE_SENSE in g_access? Your new code now relies >>>> on what I'd consider is a fairly obscure feature of the SFF-8020i >>>> spec, and one that contradicts the MMC and SPC specs that are now >>>> used as the normative references for packet commands. There's at >>>> least once case, the virtual CDROM emulator in Parallels, that >>>> appears not to support this feature, and I'd bet pretty heavily >>>> that there are a number of real devices that won't support it >>>> either. >>>> >>>> Without reverting to the old TUR code, an easy way forward is to >>>> not fail the g_access command for the MST_FMT_NONE case. This is >>>> generally incorrect anyways as it just means that the device can't >>>> specifically identify the media though it knows that the media is >>>> inserted and valid. Since this constant evaluates to 0x00, ignoring >>>> it will also allow devices that don't support it to still work. The >>>> only side effect is that devices that don't support it will also not >>>> be able to report MST_NO_DISC. These devices will get handled later >>>> as a side effect of reporting a bogus media size. >>>> >>>> Ultimately, I do think that the code needs to go back to using a TUR >>>> and interpreting sense, asc, and ascq codes correctly. The code prior >>>> to 10/26 looks like it does this, so again I'm curious as to what , >>>> motivated the change. >>>> >>> The code is only intended to work around the verbose error chatter from >>> GEOM as it will read from a nonexisting media (if the drive is empty) >>> during boot, and clutter the console needlessly. There is currently no >>> way to prevent this, but I've talked to phk about it lately so this can >>> be left out alltogether. >>> Anyhow the only failure I've seen so far is Parallels when using a CD >>> image as a virtual CDROM drive, it does a poor HW emu in that case if >>> you ask me :) >>> >>> Anyhow you are free to do what you want with the code until I get a real >>> fix via GEOM done, but mind you the old code will not work on SATA ATAPI >>> drives so a simple rollback of the commit is not really a fix... >>> >> >> While there may be a way to solve this automatically for ATA, this is >> also an issue for floppy drives. GEOM tries to read the floppy on boot >> to see if there's a label, even if no media is present. I think GEOM >> should have a way of asking drives if they can tell if they have media >> present and not probe them if they can't. A separate env flag could be >> set to say "always probe removable media" if users want the previous >> behavior. For most of us, this just causes boot delay. >> > What I've sortof talked phk into is that if I return the "right" error > code in the "start" function, it will back off and not retry nor chatter. > We still need the "access" function to return OK if HW present or we > cannot send ioctl calls to an empty drive. > IMHO, overloading g_start just adds more cruft to the problem. I'm torn between saying that g_access should be extended to give more than just a simple yes/no error return, and saying that g_access is fundamentally flawed and that this kind of info should be relayed via GEOM attributes. As for rolling back the change or not, I don't see anywhere in the SFF-8020i spec where it says that the CAP_PAGE command is guaranteed to complete successfully and not return a CHECKCOND. It seems perfectly valid for it to fail if there is no media. But even disregarding that, the current code is still intended return a failure to g_access if there's no media; doesn't that still preclude you from being able to send ioctls to the device? Scott
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?473DCF61.6010107>