Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 16 Nov 2007 17:56:58 +0100
From:      =?ISO-8859-1?Q?S=F8ren_Schmidt?= <sos@deepcore.dk>
To:        Nate Lawson <nate@root.org>
Cc:        cvs-src@FreeBSD.ORG, Scott Long <scottl@samsco.org>, src-committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG
Subject:   Re: cvs commit: src/sys/dev/ata atapi-cd.c atapi-cd.h
Message-ID:  <473DCBDA.1080104@deepcore.dk>
In-Reply-To: <473DCAA7.3090408@root.org>
References:  <200710260859.l9Q8xPdP099307@repoman.freebsd.org> <473DC220.6030809@samsco.org> <473DC93D.5060205@deepcore.dk> <473DCAA7.3090408@root.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Nate Lawson wrote:
> S=F8ren Schmidt wrote:
>  =20
>> Scott Long wrote:
>>    =20
>>> SXren Schmidt wrote:
>>>      =20
>>>> 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
>>>>        =20
>>> 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 prio=
r
>>> to 10/26 looks like it does this, so again I'm curious as to what ,
>>> motivated the change.
>>>      =20
>> The code is only intended to work around the verbose error chatter fro=
m
>> 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 ca=
n
>> 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 re=
al
>> fix via GEOM done, but mind you the old code will not work on SATA ATA=
PI
>> drives so a simple rollback of the commit is not really a fix...
>>    =20
>
> 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.
>  =20
What I've sortof talked phk into is that if I return the "right" error=20
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=20
cannot send ioctl calls to an empty drive.

-S=F8ren





Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?473DCBDA.1080104>