From owner-svn-src-head@FreeBSD.ORG Sun Dec 13 20:16:51 2009 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0A0B01065692; Sun, 13 Dec 2009 20:16:51 +0000 (UTC) (envelope-from marius@alchemy.franken.de) Received: from alchemy.franken.de (alchemy.franken.de [194.94.249.214]) by mx1.freebsd.org (Postfix) with ESMTP id 9F2308FC0A; Sun, 13 Dec 2009 20:16:50 +0000 (UTC) Received: from alchemy.franken.de (localhost [127.0.0.1]) by alchemy.franken.de (8.14.3/8.14.3/ALCHEMY.FRANKEN.DE) with ESMTP id nBDKGiTK063772; Sun, 13 Dec 2009 21:16:44 +0100 (CET) (envelope-from marius@alchemy.franken.de) Received: (from marius@localhost) by alchemy.franken.de (8.14.3/8.14.3/Submit) id nBDKGiZD063771; Sun, 13 Dec 2009 21:16:44 +0100 (CET) (envelope-from marius) Date: Sun, 13 Dec 2009 21:16:43 +0100 From: Marius Strobl To: Sam Leffler Message-ID: <20091213201643.GH74529@alchemy.franken.de> References: <200912131826.nBDIQJWa093845@svn.freebsd.org> <4B25440D.3050107@errno.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4B25440D.3050107@errno.com> User-Agent: Mutt/1.4.2.3i Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r200481 - head/sys/dev/ata X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 13 Dec 2009 20:16:51 -0000 On Sun, Dec 13, 2009 at 11:44:13AM -0800, Sam Leffler wrote: > Marius Strobl wrote: > >Author: marius > >Date: Sun Dec 13 18:26:19 2009 > >New Revision: 200481 > >URL: http://svn.freebsd.org/changeset/base/200481 > > > >Log: > > Specify the capability and media bits of the capabilities page in > > native, i.e. big-endian, format and convert as appropriate like we > > also do with the multibyte fields of the other pages. This fixes > > the output of acd_describe() to match reality on big-endian machines > > without breaking it on little-endian ones. While at it, also convert > > the remaining multibyte fields of the pages read although they are > > currently unused for consistency and in order to prevent possible > > similar bugs in the future. > > > > MFC after: 1 week > > > >Modified: > > head/sys/dev/ata/atapi-cd.c > > head/sys/dev/ata/atapi-cd.h > > > >Modified: head/sys/dev/ata/atapi-cd.c > >============================================================================== > >--- head/sys/dev/ata/atapi-cd.c Sun Dec 13 17:49:22 2009 (r200480) > >+++ head/sys/dev/ata/atapi-cd.c Sun Dec 13 18:26:19 2009 (r200481) > >@@ -1206,6 +1206,7 @@ acd_read_track_info(device_t dev, int32_ > > if ((error = ata_atapicmd(dev, ccb, (caddr_t)info, sizeof(*info), > > ATA_R_READ, 30))) > > return error; > >+ info->data_length = ntohs(info->data_length); > > info->track_start_addr = ntohl(info->track_start_addr); > > info->next_writeable_addr = ntohl(info->next_writeable_addr); > > info->free_blocks = ntohl(info->free_blocks); > >@@ -1644,12 +1645,17 @@ acd_get_cap(device_t dev) > > for (count = 0 ; count < 5 ; count++) { > > if (!ata_atapicmd(dev, ccb, (caddr_t)&cdp->cap, sizeof(cdp->cap), > > ATA_R_READ | ATA_R_QUIET, 5)) { > >+ cdp->cap.data_length = ntohs(cdp->cap.data_length); > >+ cdp->cap.blk_desc_len = ntohs(cdp->cap.blk_desc_len); > >+ cdp->cap.media = ntohs(cdp->cap.media); > >+ cdp->cap.capabilities = ntohs(cdp->cap.capabilities); > > cdp->cap.max_read_speed = ntohs(cdp->cap.max_read_speed); > >+ cdp->cap.max_vol_levels = ntohs(cdp->cap.max_vol_levels); > >+ cdp->cap.buf_size = ntohs(cdp->cap.buf_size); > > cdp->cap.cur_read_speed = ntohs(cdp->cap.cur_read_speed); > > cdp->cap.max_write_speed = ntohs(cdp->cap.max_write_speed); > > cdp->cap.cur_write_speed = > > max(ntohs(cdp->cap.cur_write_speed),177); > >- cdp->cap.max_vol_levels = ntohs(cdp->cap.max_vol_levels); > >- cdp->cap.buf_size = ntohs(cdp->cap.buf_size); > >+ cdp->cap.copy_protect_rev = ntohs(cdp->cap.copy_protect_rev); > > } > > } > > } > > I don't think this should use ntoh* but instead use le*/be* macros. > I agree but ata(4) uses hton*(9) and ntoh*(9) throughout its source so changing it to use bytorder(9) functions instead should be a separate style change. > Separately the ata code was very broken in it's handling of big-endian > byte order; e.g. for arm I had to supply wrong bus space methods as a > workaround. Do these changes take this byte-order confusion into account? > At least on sparc64 I haven't seen such problems. There all ATA controllers are PCI which uses little-endian bus space accessors. AFAICT at least atapi-cd(4) now does the right things as the native byteorder of the pages indeed is big-endian as explicitly mentioned in MMC-5. Marius