cvs commit: src/sys/dev/ata atapi-cd.c atapi-cd.h

[Søren Schmidt]

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.

-Søren