2011/4/20 Larry Finger <Larry.Finger@xxxxxxxxxxxx>: > On 04/19/2011 08:17 PM, RafaÅ MiÅecki wrote: >> >> Signed-off-by: RafaÅ MiÅecki<zajec5@xxxxxxxxx> >> --- >> With updated defines following MMIO with defines makes sense: >> Âread32 0xf04001e0 -> Â0x00010000 >> write32 0xf04001e0<- 0x00010002 |= SSB_CHIPCO_CLKCTLST_FORCEHT >> Âread32 0xf04001e0 -> Â0x00010002 >> (...) >> Âread32 0xf04001e0 -> Â0x00010002 >> Âread32 0xf04001e0 -> Â0x00010002 >> Âread32 0xf04001e0 -> Â0x00030002 Â Â Â(& ÂSSB_CHIPCO_CLKCTLST_HAVEHT) >> >> Of course MMIO does not come from 4328. It is from 4312 and wl (just SSB >> defs). >> >> The tricky part is that we were using SSB_CHIPCO_CLKCTLST_HAVEHT in PMU >> driver. >> >> My guess is that we were always checking for the wrong register and we got >> false positives on test for turning PLL down. >> --- >> Âinclude/linux/ssb/ssb_driver_chipcommon.h | Â Â9 +++++++-- >> Â1 files changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/include/linux/ssb/ssb_driver_chipcommon.h >> b/include/linux/ssb/ssb_driver_chipcommon.h >> index ba83bc5..45e7b6c 100644 >> --- a/include/linux/ssb/ssb_driver_chipcommon.h >> +++ b/include/linux/ssb/ssb_driver_chipcommon.h >> @@ -133,6 +133,9 @@ >> Â#define SSB_CHIPCO_GPIOIRQ Â Â Â Â Â Â0x0074 >> Â#define SSB_CHIPCO_WATCHDOG Â Â Â Â Â 0x0080 >> Â#define SSB_CHIPCO_GPIOTIMER Â Â Â Â Â0x0088 Â Â Â Â Â/* LED powersave >> (corerev>= 16) */ >> +#define ÂSSB_CHIPCO_GPIOTIMER_OFFTIME Â0x0000FFFF >> +#define ÂSSB_CHIPCO_GPIOTIMER_OFFTIME_SHIFT Â Â0 >> +#define ÂSSB_CHIPCO_GPIOTIMER_ONTIME Â 0xFFFF0000 >> Â#define ÂSSB_CHIPCO_GPIOTIMER_ONTIME_SHIFT Â Â16 >> Â#define SSB_CHIPCO_GPIOTOUTM Â Â Â Â Â0x008C Â Â Â Â Â/* LED powersave >> (corerev>= 16) */ >> Â#define SSB_CHIPCO_CLOCK_N Â Â Â Â Â Â0x0090 >> @@ -191,8 +194,10 @@ >> Â#define ÂSSB_CHIPCO_CLKCTLST_HAVEALPREQ Â Â Â 0x00000008 /* ALP available >> request */ >> Â#define ÂSSB_CHIPCO_CLKCTLST_HAVEHTREQ Â Â Â Â0x00000010 /* HT available >> request */ >> Â#define ÂSSB_CHIPCO_CLKCTLST_HWCROFF Â0x00000020 /* Force HW clock >> request off */ >> -#define ÂSSB_CHIPCO_CLKCTLST_HAVEHT Â Â0x00010000 /* HT available */ >> -#define ÂSSB_CHIPCO_CLKCTLST_HAVEALP Â 0x00020000 /* APL available */ >> +#define ÂSSB_CHIPCO_CLKCTLST_HAVEHT Â Â0x00010000 /* ALP available */ >> +#define ÂSSB_CHIPCO_CLKCTLST_HAVEALP Â 0x00020000 /* HT available */ >> +#define ÂSSB_CHIPCO_CLKCTLST_4328A0_HAVEHT Â Â 0x00010000 /* 4328a0 has >> reversed bits */ >> +#define ÂSSB_CHIPCO_CLKCTLST_4328A0_HAVEALP Â Â0x00020000 /* 4328a0 has >> reversed bits */ >> Â#define SSB_CHIPCO_HW_WORKAROUND Â Â Â0x01E4 /* Hardware workaround >> (rev>= 20) */ >> Â#define SSB_CHIPCO_UART0_DATA Â Â Â Â 0x0300 >> Â#define SSB_CHIPCO_UART0_IMR Â Â Â Â Â0x0304 > > I agree that the HAVEHT bit is different for the 4328 than for the rest of > the versions, but I think you did not get the rest quite right. Having the > HT bit attached to a symbol named ALP and vice versa will only create > confusion. Make SSB_CHIPCO_CLKCTLST_HAVEHT be 0x00020000 and > SSB_CHIPCO_CLKCTLST_HAVEALP be 0x000100000. It was bad idea to send patch so late. Did I really update comments hoping it will work? :| -- RafaÅ -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html