svn commit: r228785 - in head/sys/dev/ath/ath_hal: ar5210 ar5211

[Adrian Chadd]

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 <dim at 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"
>