All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH] ssb: cc: add & fix defines
@ 2011-04-20  1:17 ` Rafał Miłecki
  0 siblings, 0 replies; 9+ messages in thread
From: Rafał Miłecki @ 2011-04-20  1:17 UTC (permalink / raw)
  To: linux-wireless, John W. Linville, Michael Büsch
  Cc: b43-dev, Rafał Miłecki

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
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


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [RFC][PATCH] ssb: cc: add & fix defines
@ 2011-04-20  1:17 ` Rafał Miłecki
  0 siblings, 0 replies; 9+ messages in thread
From: Rafał Miłecki @ 2011-04-20  1:17 UTC (permalink / raw)
  To: linux-wireless, John W. Linville, Michael Büsch
  Cc: b43-dev, Rafał Miłecki

Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
---
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

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [RFC][PATCH] ssb: cc: add & fix defines
  2011-04-20  1:17 ` Rafał Miłecki
@ 2011-04-20  1:47   ` Larry Finger
  -1 siblings, 0 replies; 9+ messages in thread
From: Larry Finger @ 2011-04-20  1:47 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: linux-wireless, John W. Linville, Michael Büsch, b43-dev

On 04/19/2011 08:17 PM, Rafał Miłecki wrote:
> Signed-off-by: Rafał Miłecki<zajec5@gmail.com>
> ---
> 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


^ permalink raw reply	[flat|nested] 9+ messages in thread
* [RFC][PATCH] ssb: cc: add & fix defines
@ 2011-04-20  1:47   ` Larry Finger
  0 siblings, 0 replies; 9+ messages in thread
From: Larry Finger @ 2011-04-20  1:47 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: linux-wireless, John W. Linville, Michael Büsch, b43-dev

On 04/19/2011 08:17 PM, Rafa? Mi?ecki wrote:
> Signed-off-by: Rafa? Mi?ecki<zajec5@gmail.com>
> ---
> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [RFC][PATCH] ssb: cc: add & fix defines
  2011-04-20  1:47   ` Larry Finger
@ 2011-04-20  8:15     ` Rafał Miłecki
  -1 siblings, 0 replies; 9+ messages in thread
From: Rafał Miłecki @ 2011-04-20  8:15 UTC (permalink / raw)
  To: Larry Finger
  Cc: linux-wireless, John W. Linville, Michael Büsch, b43-dev

2011/4/20 Larry Finger <Larry.Finger@lwfinger.net>:
> On 04/19/2011 08:17 PM, Rafał Miłecki wrote:
>>
>> Signed-off-by: Rafał Miłecki<zajec5@gmail.com>
>> ---
>> 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ł

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [RFC][PATCH] ssb: cc: add & fix defines
@ 2011-04-20  8:15     ` Rafał Miłecki
  0 siblings, 0 replies; 9+ messages in thread
From: Rafał Miłecki @ 2011-04-20  8:15 UTC (permalink / raw)
  To: Larry Finger
  Cc: linux-wireless, John W. Linville, Michael Büsch, b43-dev

2011/4/20 Larry Finger <Larry.Finger@lwfinger.net>:
> On 04/19/2011 08:17 PM, Rafa? Mi?ecki wrote:
>>
>> Signed-off-by: Rafa? Mi?ecki<zajec5@gmail.com>
>> ---
>> 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?

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [RFC][PATCH] ssb: cc: add & fix defines
  2011-04-20  8:15     ` Rafał Miłecki
@ 2011-04-20 14:44       ` Larry Finger
  -1 siblings, 0 replies; 9+ messages in thread
From: Larry Finger @ 2011-04-20 14:44 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: linux-wireless, John W. Linville, Michael Büsch, b43-dev

On 04/20/2011 03:15 AM, Rafał Miłecki wrote:
> 2011/4/20 Larry Finger<Larry.Finger@lwfinger.net>:
>> On 04/19/2011 08:17 PM, Rafał Miłecki wrote:
>>>
>>> Signed-off-by: Rafał Miłecki<zajec5@gmail.com>
>>> ---
>>> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [RFC][PATCH] ssb: cc: add & fix defines
@ 2011-04-20 14:44       ` Larry Finger
  0 siblings, 0 replies; 9+ messages in thread
From: Larry Finger @ 2011-04-20 14:44 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: linux-wireless, John W. Linville, Michael Büsch, b43-dev

On 04/20/2011 03:15 AM, Rafa? Mi?ecki wrote:
> 2011/4/20 Larry Finger<Larry.Finger@lwfinger.net>:
>> On 04/19/2011 08:17 PM, Rafa? Mi?ecki wrote:
>>>
>>> Signed-off-by: Rafa? Mi?ecki<zajec5@gmail.com>
>>> ---
>>> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [RFC][PATCH] ssb: cc: add & fix defines
  2011-04-20  8:15     ` Rafał Miłecki
  (?)
  (?)
@ 2011-04-21  2:26     ` Julian Calaby
  -1 siblings, 0 replies; 9+ messages in thread
From: Julian Calaby @ 2011-04-21  2:26 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Larry Finger, linux-wireless, John W. Linville,
	Michael Büsch, b43-dev

On Wed, Apr 20, 2011 at 18:15, Rafał Miłecki <zajec5@gmail.com> 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: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-04-21  2:26 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-20  1:17 [RFC][PATCH] ssb: cc: add & fix defines Rafał Miłecki
2011-04-20  1:17 ` Rafał Miłecki
2011-04-20  1:47 ` Larry Finger
2011-04-20  1:47   ` Larry Finger
2011-04-20  8:15   ` Rafał Miłecki
2011-04-20  8:15     ` Rafał Miłecki
2011-04-20 14:44     ` Larry Finger
2011-04-20 14:44       ` Larry Finger
2011-04-21  2:26     ` Julian Calaby

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.