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 <d...@freebsd.org> wrote: > Author: dim > Date: Wed Dec 21 17:16:43 2011 > New Revision: 228785 > URL: http://svn.freebsd.org/changeset/base/228785 > > Log: > Fix shift overflow problem in sys/dev/ath/ath_hal/ar5210/ar5210_power.c > and sys/dev/ath/ath_hal/ar5211/ar5211_power.c: > > sys/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 bits > [-Wshift-overflow] > OS_REG_RMW_FIELD(ah, AR_SCR, AR_SCR_SLE, AR_SCR_SLE_ALLOW); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > sys/dev/ath/ath_hal/ah_internal.h:472:42: note: expanded from: > (OS_REG_READ(_a, _r) &~ (_f)) | (((_v) << _f##_S) & (_f))) > ^ > sys/dev/ath/ah_osdep.h:127:49: note: expanded from: > (bus_space_handle_t)(_ah)->ah_sh, (_reg), (_val)) > ^~~~ > > The AR_SCR_SLE_{WAKE,SLP,NORM} values are pre-shifted in ar5210reg.h and > ar5211reg.h, while they should be unshifted, like in ar5212reg.h. Then, > when the OS_REG_RMW_FIELD() macro shifts them again, the values will > overflow, becoming effectively zero. > > MFC after: 1 week > > Modified: > head/sys/dev/ath/ath_hal/ar5210/ar5210reg.h > head/sys/dev/ath/ath_hal/ar5211/ar5211reg.h > > Modified: head/sys/dev/ath/ath_hal/ar5210/ar5210reg.h > ============================================================================== > --- head/sys/dev/ath/ath_hal/ar5210/ar5210reg.h Wed Dec 21 17:03:30 2011 > (r228784) > +++ head/sys/dev/ath/ath_hal/ar5210/ar5210reg.h Wed Dec 21 17:16:43 2011 > (r228785) > @@ -245,9 +245,9 @@ > #define AR_SCR_SLDUR 0x0000ffff /* sleep duration */ > #define AR_SCR_SLE 0x00030000 /* sleep enable */ > #define AR_SCR_SLE_S 16 > -#define AR_SCR_SLE_WAKE 0x00000000 /* force wake */ > -#define AR_SCR_SLE_SLP 0x00010000 /* force sleep */ > -#define AR_SCR_SLE_ALLOW 0x00020000 /* allow to control > sleep */ > +#define AR_SCR_SLE_WAKE 0 /* force wake */ > +#define AR_SCR_SLE_SLP 1 /* force sleep */ > +#define AR_SCR_SLE_ALLOW 2 /* allow to control > sleep */ > #define AR_SCR_BITS "\20\20SLE_SLP\21SLE_ALLOW" > > #define AR_INTPEND_IP 0x00000001 /* interrupt pending > */ > > Modified: head/sys/dev/ath/ath_hal/ar5211/ar5211reg.h > ============================================================================== > --- head/sys/dev/ath/ath_hal/ar5211/ar5211reg.h Wed Dec 21 17:03:30 2011 > (r228784) > +++ head/sys/dev/ath/ath_hal/ar5211/ar5211reg.h Wed Dec 21 17:16:43 2011 > (r228785) > @@ -618,9 +618,9 @@ > #define AR_SCR_SLDUR_S 0 > #define AR_SCR_SLE 0x00030000 /* sleep enable mask */ > #define AR_SCR_SLE_S 16 /* sleep enable bits shift */ > -#define AR_SCR_SLE_WAKE 0x00000000 /* force wake */ > -#define AR_SCR_SLE_SLP 0x00010000 /* force sleep */ > -#define AR_SCR_SLE_NORM 0x00020000 /* sleep logic normal > operation */ > +#define AR_SCR_SLE_WAKE 0 /* force wake */ > +#define AR_SCR_SLE_SLP 1 /* force sleep */ > +#define AR_SCR_SLE_NORM 2 /* sleep logic normal > operation */ > #define AR_SCR_SLE_UNITS 0x00000008 /* SCR units/TU */ > #define AR_SCR_BITS "\20\20SLE_SLP\21SLE" > _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"