Signed-off-by: Rafał Miłecki <[email protected]>
---
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
--
1.7.3.4
2011/4/20 Larry Finger <[email protected]>:
> On 04/19/2011 08:17 PM, Rafał Miłecki wrote:
>>
>> Signed-off-by: Rafał Miłecki<[email protected]>
>> ---
>> 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ł
On Wed, Apr 20, 2011 at 18:15, Rafał Miłecki <[email protected]> wrote:
[snip]
>
> It was bad idea to send patch so late. Did I really update comments
> hoping it will work? :|
I read the patch and wondered what the heck you were doing, but I
thought to myself: Rafał knows way more about ssb and the related code
than I do, so surely he knows what he's doing. =)
Thanks,
--
Julian Calaby
Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
On 04/20/2011 03:15 AM, Rafał Miłecki wrote:
> 2011/4/20 Larry Finger<[email protected]>:
>> On 04/19/2011 08:17 PM, Rafał Miłecki wrote:
>>>
>>> Signed-off-by: Rafał Miłecki<[email protected]>
>>> ---
>>> 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? :|
I thought you were using the experimental, virtual compiler that produces code
based on intent as written in the comments. :)
Larry
On 04/19/2011 08:17 PM, Rafał Miłecki wrote:
> Signed-off-by: Rafał Miłecki<[email protected]>
> ---
> 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.
Larry