Date: Sun, 13 Dec 2009 21:16:43 +0100 From: Marius Strobl <marius@alchemy.franken.de> To: Sam Leffler <sam@errno.com> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r200481 - head/sys/dev/ata Message-ID: <20091213201643.GH74529@alchemy.franken.de> In-Reply-To: <4B25440D.3050107@errno.com> References: <200912131826.nBDIQJWa093845@svn.freebsd.org> <4B25440D.3050107@errno.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20091213201643.GH74529>