From owner-svn-src-head@FreeBSD.ORG Wed Dec 21 21:52:06 2011 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 0256B1065670; Wed, 21 Dec 2011 21:52:06 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-vw0-f54.google.com (mail-vw0-f54.google.com [209.85.212.54]) by mx1.freebsd.org (Postfix) with ESMTP id 83FE38FC0A; Wed, 21 Dec 2011 21:52:05 +0000 (UTC) Received: by vbbfr13 with SMTP id fr13so10746126vbb.13 for ; Wed, 21 Dec 2011 13:52:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=IbyYDXBl8TTuM0qUQYlQO3hOfNF2lCbKWQGeVZS+WUM=; b=vtpuPXdOs6xIhj9O+OkHV5O7z1sQmNe57UcQrumH33T1S23vO2WWxIMPw3c3GdD15j UFxmENILpUw0o0tP55MhdrOW9yN/4zOOnI1xo835Pim6Egf/oE2pEyIf7bdH1HwLqMqa EHJBcsSNEqWe3OJHBVhJcRRVZ4Jw9wfWsqp8M= MIME-Version: 1.0 Received: by 10.52.22.193 with SMTP id g1mr1416109vdf.77.1324504324830; Wed, 21 Dec 2011 13:52:04 -0800 (PST) Sender: adrian.chadd@gmail.com Received: by 10.52.158.104 with HTTP; Wed, 21 Dec 2011 13:52:04 -0800 (PST) In-Reply-To: <201112211716.pBLHGhDH078507@svn.freebsd.org> References: <201112211716.pBLHGhDH078507@svn.freebsd.org> Date: Wed, 21 Dec 2011 13:52:04 -0800 X-Google-Sender-Auth: Pxia-fiooV7nuxnPDVlE7f7Uv_E Message-ID: From: Adrian Chadd To: Dimitry Andric Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r228785 - in head/sys/dev/ath/ath_hal: ar5210 ar5211 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: Wed, 21 Dec 2011 21:52:06 -0000 Erm, why did you do this without first getting clearance from someone who has the hardware to test it? Just because it looks obviously wrong to you, doesn't at all mean that it's "wrong". It's quite possible that the driver _requires_ those bits to be written to the hardware as 0. I'd appreciate it if would please revert this and other ath/hal changes until I've had time to research them and test them out. Thanks, Adrian On 21 December 2011 09:16, Dimitry Andric wrote: > Author: dim > Date: Wed Dec 21 17:16:43 2011 > New Revision: 228785 > URL: http://svn.freebsd.org/changeset/base/228785 > > Log: > =A0Fix shift overflow problem in sys/dev/ath/ath_hal/ar5210/ar5210_power.= c > =A0and sys/dev/ath/ath_hal/ar5211/ar5211_power.c: > > =A0sys/dev/ath/ath_hal/ar5210/ar5210_power.c:36:3: warning: signed shift = result (0x200000000) requires 35 bits to represent, but 'int' only has 32 b= its [-Wshift-overflow] > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0OS_REG_RMW_FIELD(ah, AR_SCR, AR_SCR_SL= E, AR_SCR_SLE_ALLOW); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~= ~~~~~~~~~~~~~~~~~~~~ > =A0sys/dev/ath/ath_hal/ah_internal.h:472:42: note: expanded from: > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(OS_REG_READ(_a, _r) &~ (_f)) | (((_v)= << _f##_S) & (_f))) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ^ > =A0sys/dev/ath/ah_osdep.h:127:49: note: expanded from: > =A0 =A0 =A0 =A0 =A0 =A0 =A0(bus_space_handle_t)(_ah)->ah_sh, (_reg), (_va= l)) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ^~~~ > > =A0The AR_SCR_SLE_{WAKE,SLP,NORM} values are pre-shifted in ar5210reg.h a= nd > =A0ar5211reg.h, while they should be unshifted, like in ar5212reg.h. =A0T= hen, > =A0when the OS_REG_RMW_FIELD() macro shifts them again, the values will > =A0overflow, becoming effectively zero. > > =A0MFC after: 1 week > > Modified: > =A0head/sys/dev/ath/ath_hal/ar5210/ar5210reg.h > =A0head/sys/dev/ath/ath_hal/ar5211/ar5211reg.h > > Modified: head/sys/dev/ath/ath_hal/ar5210/ar5210reg.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- head/sys/dev/ath/ath_hal/ar5210/ar5210reg.h Wed Dec 21 17:03:30 2011 = =A0 =A0 =A0 =A0(r228784) > +++ head/sys/dev/ath/ath_hal/ar5210/ar5210reg.h Wed Dec 21 17:16:43 2011 = =A0 =A0 =A0 =A0(r228785) > @@ -245,9 +245,9 @@ > =A0#define =A0 =A0 =A0 =A0AR_SCR_SLDUR =A0 =A0 =A0 =A0 =A0 =A00x0000ffff = =A0 =A0 =A0/* sleep duration */ > =A0#define =A0 =A0 =A0 =A0AR_SCR_SLE =A0 =A0 =A0 =A0 =A0 =A0 =A00x0003000= 0 =A0 =A0 =A0/* sleep enable */ > =A0#define =A0 =A0 =A0 =A0AR_SCR_SLE_S =A0 =A0 =A0 =A0 =A0 =A016 > -#define =A0 =A0 =A0 =A0AR_SCR_SLE_WAKE =A0 =A0 =A0 =A0 0x00000000 =A0 = =A0 =A0/* force wake */ > -#define =A0 =A0 =A0 =A0AR_SCR_SLE_SLP =A0 =A0 =A0 =A0 =A00x00010000 =A0 = =A0 =A0/* force sleep */ > -#define =A0 =A0 =A0 =A0AR_SCR_SLE_ALLOW =A0 =A0 =A0 =A00x00020000 =A0 = =A0 =A0/* allow to control sleep */ > +#define =A0 =A0 =A0 =A0AR_SCR_SLE_WAKE =A0 =A0 =A0 =A0 0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 /* force wake */ > +#define =A0 =A0 =A0 =A0AR_SCR_SLE_SLP =A0 =A0 =A0 =A0 =A01 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 /* force sleep */ > +#define =A0 =A0 =A0 =A0AR_SCR_SLE_ALLOW =A0 =A0 =A0 =A02 =A0 =A0 =A0 =A0= =A0 =A0 =A0 /* allow to control sleep */ > =A0#define =A0 =A0 =A0 =A0AR_SCR_BITS =A0 =A0 "\20\20SLE_SLP\21SLE_ALLOW" > > =A0#define =A0 =A0 =A0 =A0AR_INTPEND_IP =A0 =A0 =A0 =A0 =A0 0x00000001 = =A0 =A0 =A0/* interrupt pending */ > > Modified: head/sys/dev/ath/ath_hal/ar5211/ar5211reg.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- head/sys/dev/ath/ath_hal/ar5211/ar5211reg.h Wed Dec 21 17:03:30 2011 = =A0 =A0 =A0 =A0(r228784) > +++ head/sys/dev/ath/ath_hal/ar5211/ar5211reg.h Wed Dec 21 17:16:43 2011 = =A0 =A0 =A0 =A0(r228785) > @@ -618,9 +618,9 @@ > =A0#define =A0 =A0 =A0 =A0AR_SCR_SLDUR_S =A00 > =A0#define =A0 =A0 =A0 =A0AR_SCR_SLE =A0 =A0 =A00x00030000 =A0 =A0 =A0/* = sleep enable mask */ > =A0#define =A0 =A0 =A0 =A0AR_SCR_SLE_S =A0 =A016 =A0 =A0 =A0 =A0 =A0 =A0 = =A0/* sleep enable bits shift */ > -#define =A0 =A0 =A0 =A0AR_SCR_SLE_WAKE 0x00000000 =A0 =A0 =A0/* force wa= ke */ > -#define =A0 =A0 =A0 =A0AR_SCR_SLE_SLP =A00x00010000 =A0 =A0 =A0/* force = sleep */ > -#define =A0 =A0 =A0 =A0AR_SCR_SLE_NORM 0x00020000 =A0 =A0 =A0/* sleep lo= gic normal operation */ > +#define =A0 =A0 =A0 =A0AR_SCR_SLE_WAKE 0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* = force wake */ > +#define =A0 =A0 =A0 =A0AR_SCR_SLE_SLP =A01 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /= * force sleep */ > +#define =A0 =A0 =A0 =A0AR_SCR_SLE_NORM 2 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* = sleep logic normal operation */ > =A0#define =A0 =A0 =A0 =A0AR_SCR_SLE_UNITS =A0 =A0 =A0 =A00x00000008 =A0 = =A0 =A0/* SCR units/TU */ > =A0#define =A0 =A0 =A0 =A0AR_SCR_BITS =A0 =A0 "\20\20SLE_SLP\21SLE" >