Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 13 Dec 2009 11:44:13 -0800
From:      Sam Leffler <sam@errno.com>
To:        Marius Strobl <marius@FreeBSD.org>
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:  <4B25440D.3050107@errno.com>
In-Reply-To: <200912131826.nBDIQJWa093845@svn.freebsd.org>
References:  <200912131826.nBDIQJWa093845@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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.

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?

	Sam



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4B25440D.3050107>