From owner-freebsd-current@FreeBSD.ORG Fri Aug 7 17:07:30 2009 Return-Path: Delivered-To: freebsd-current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 05FAD106566B; Fri, 7 Aug 2009 17:07:30 +0000 (UTC) (envelope-from nox@jelal.kn-bremen.de) Received: from smtp.kn-bremen.de (gelbbaer.kn-bremen.de [78.46.108.116]) by mx1.freebsd.org (Postfix) with ESMTP id 8A15D8FC15; Fri, 7 Aug 2009 17:07:29 +0000 (UTC) Received: by smtp.kn-bremen.de (Postfix, from userid 10) id 5C3811E002F9; Fri, 7 Aug 2009 19:07:28 +0200 (CEST) Received: from triton.kn-bremen.de (noident@localhost [127.0.0.1]) by triton.kn-bremen.de (8.14.3/8.14.3) with ESMTP id n77H4RUO004485; Fri, 7 Aug 2009 19:04:27 +0200 (CEST) (envelope-from nox@triton.kn-bremen.de) Received: (from nox@localhost) by triton.kn-bremen.de (8.14.3/8.14.3/Submit) id n77H4RdF004484; Fri, 7 Aug 2009 19:04:27 +0200 (CEST) (envelope-from nox) From: Juergen Lock Date: Fri, 7 Aug 2009 19:04:26 +0200 To: Alexander Motin Message-ID: <20090807170426.GA4292@triton.kn-bremen.de> References: <20090806184510.GA12039@triton.kn-bremen.de> <4A7B3328.5020307@FreeBSD.org> <20090806200715.GA16313@triton.kn-bremen.de> <20090806222127.GB1940@triton.kn-bremen.de> <4A7BBA52.306@samsco.org> <4A7BD57E.3040201@FreeBSD.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4A7BD57E.3040201@FreeBSD.org> User-Agent: Mutt/1.5.20 (2009-06-14) Cc: freebsd-current@FreeBSD.org, markus@FreeBSD.org, Juergen Lock Subject: Re: cdparanoia patch for ahci(4)/siis(4) (next version) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 07 Aug 2009 17:07:30 -0000 On Fri, Aug 07, 2009 at 10:19:26AM +0300, Alexander Motin wrote: > Scott Long wrote: > > Juergen Lock wrote: > >> On Thu, Aug 06, 2009 at 10:07:15PM +0200, Juergen Lock wrote: > >>> On Thu, Aug 06, 2009 at 10:46:48PM +0300, Alexander Motin wrote: > >>>> Juergen Lock wrote: > >>>>> 2. cdda/dae seems to be broken entirely with ahci(4) as well as > >>>>> siis(4) (I remember a report about it being broken for usb optical > >>>>> drives too so maybe this is related?) - I tested with the > >>>>> audio/cdparanoia port as well as with > >>>>> mplayer -cdrom-device /dev/cd{0,1} cdda://... > >>>>> (mplayer needs to be built with the libparanoia knob on for this) - > >>>>> this > >>>>> does work with atapicam(4) without ahci/siis so it can't be cd(4)'s > >>>>> fault alone. On siis(4) it seems to just fail while on ahci(4) (I > >>>>> still > >>>>> have another optical drive on there, it's on the board's amd sb700) > >>>>> it causes the sata channel to be reset endlessly until I ^C mplayer: > >>>>> > >>>>> Soo, anyone have ideas/patches/things they want me to check for this? > >>>> But this appeared to to be really trivial. cdparanoia uses extremely > >>>> simple method for detecting ATAPI devices - it checks that SIM is > >>>> named "ata". Trivial single line hack made it successfully play some > >>>> old AudioCD in SATA drive on SiI3132 controller for me, while I am > >>>> typing this. Probably we should invent better way to do this. > >>> Oooh! :) I need to test this... > >> > >> Yup, works here too on siis and ahci with the following patch: > >> (maintainer Cc'd) > >> > >> Index: interface/scsi_interface.c > >> @@ -1480,9 +1480,12 @@ > >> /* > >> * if the bus device name is `ata', we're (obviously) > >> * running ATAPICAM. > >> + * XXX same for the new ahci(4) and siis(4) drivers... > >> */ > >> > >> - if (strncmp(d->ccb->cpi.dev_name, "ata", 3) == 0) { > >> + if (strncmp(d->ccb->cpi.dev_name, "ata", 3) == 0 || > >> + strncmp(d->ccb->cpi.dev_name, "ahcich", 6) == 0 || > >> + strncmp(d->ccb->cpi.dev_name, "siisch", 6) == 0) { > >> cdmessage(d, "\tDrive is ATAPI (using ATAPICAM)\n"); > >> d->is_atapi = 1; > >> } else { > >> > >> Thanx, :) > >> Juergen > > > > This is fine for the moment, but unmaintainable in the long run as more > > and more drives are written. cdparanoia needs to look at protocol and > > transport attributes, not device names. > > CAM reports SCSI protocol for ATAPI devices at this moment. It is not > good probably. but changing it now may be painful. Checks like > d->ccb->cpi.transport == XPORT_ATA || > d->ccb->cpi.transport == XPORT_SATA > should be for now. "ata" hack should also stay there for now, as > ATAPICAM emulates SCSI transport now, but not a new ATA one. Ok checking for XPORT_(S)ATA works for me as well: Index: interface/scsi_interface.c @@ -1480,10 +1480,16 @@ /* * if the bus device name is `ata', we're (obviously) * running ATAPICAM. + * same for the new ahci(4) and siis(4) drivers and future others + * which use SATA transport too... */ - if (strncmp(d->ccb->cpi.dev_name, "ata", 3) == 0) { - cdmessage(d, "\tDrive is ATAPI (using ATAPICAM)\n"); + if (strncmp(d->ccb->cpi.dev_name, "ata", 3) == 0 || +#if __FreeBSD_version >= 800102 + d->ccb->cpi.transport == XPORT_SATA || +#endif + d->ccb->cpi.transport == XPORT_ATA) { + cdmessage(d, "\tDrive is ATAPI (using ATAPICAM or direct CAM (S)ATA transport)\n"); d->is_atapi = 1; } else { cdmessage(d, "\tDrive is SCSI\n"); One question remains tho: What if someone connects a sata drive to a sas controller, will that still be XPORT_SAS then? In that case I'd say we'd need to check for that as well, or maybe just assume there are no `real' sas optical drives and treat everything sas as sata here... Thanx, Juergen