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

next in thread | previous in thread | raw e-mail | index | archive | help
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=20
GEOM as it will read from a nonexisting media (if the drive is empty)=20
during boot, and clutter the console needlessly. There is currently no=20
way to prevent this, but I've talked to phk about it lately so this can=20
be left out alltogether.
Anyhow the only failure I've seen so far is Parallels when using a CD=20
image as a virtual CDROM drive, it does a poor HW emu in that case if=20
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...


-S=F8ren




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