2010-05-01 16:29:09

by Gábor Stefanik

[permalink] [raw]
Subject: [PATCH] ssb: Implement fast powerup delay calculation

Implement fast powerup delay calculation, as described
in the recently updated specs. The 4325 part is coming soon.

Signed-off-by: G?bor Stefanik<[email protected]>
---

To people experiencing DMA errors: please test this patch, it touches
the PMU code, one of the suspected problem areas.

I'm submitting this patch using Thunderbird 3; so I cannot be held liable
if it gets damaged in transit. Blame Mozilla. :-)

Index: wireless-testing/drivers/ssb/driver_chipcommon.c
===================================================================
--- wireless-testing.orig/drivers/ssb/driver_chipcommon.c
+++ wireless-testing/drivers/ssb/driver_chipcommon.c
@@ -209,7 +209,24 @@ static void chipco_powercontrol_init(str
}
}

-static void calc_fast_powerup_delay(struct ssb_chipcommon *cc)
+static u16 pmu_fast_powerup_delay(struct ssb_chipcommon *cc)
+{
+ struct ssb_bus *bus = cc->dev->bus;
+
+ switch (bus->chip_id) {
+ case 0x4312:
+ case 0x4322:
+ case 0x4328:
+ return 7000;
+ case 0x4325:
+ /* TODO: */
+ default:
+ break;
+ }
+ return 15000;
+}
+
+static u16 calc_fast_powerup_delay(struct ssb_chipcommon *cc)
{
struct ssb_bus *bus = cc->dev->bus;
int minfreq;
@@ -217,26 +234,33 @@ static void calc_fast_powerup_delay(stru
u32 pll_on_delay;

if (bus->bustype != SSB_BUSTYPE_PCI)
- return;
+ return 0;
+ if (cc->capabilities& SSB_CHIPCO_CAP_PMU)
+ return pmu_fast_powerup_delay(cc);
if (!(cc->capabilities& SSB_CHIPCO_CAP_PCTL))
- return;
+ return 0;

minfreq = chipco_pctl_clockfreqlimit(cc, 0);
pll_on_delay = chipco_read32(cc, SSB_CHIPCO_PLLONDELAY);
tmp = (((pll_on_delay + 2) * 1000000) + (minfreq - 1)) / minfreq;
SSB_WARN_ON(tmp& ~0xFFFF);

- cc->fast_pwrup_delay = tmp;
+ return tmp;
}

void ssb_chipcommon_init(struct ssb_chipcommon *cc)
{
+ u16 delay;
+
if (!cc->dev)
return; /* We don't have a ChipCommon */
ssb_pmu_init(cc);
chipco_powercontrol_init(cc);
ssb_chipco_set_clockmode(cc, SSB_CLKMODE_FAST);
- calc_fast_powerup_delay(cc);
+ delay = calc_fast_powerup_delay(cc);
+ ssb_printk(KERN_INFO PFX "fast_pwrup_delay is %d\n", delay);
+ cc->fast_pwrup_delay = delay;
+ ssb_write16(cc->dev, SSB_MMIO_POWERUP_DELAY, delay);
}

void ssb_chipco_suspend(struct ssb_chipcommon *cc)
Index: wireless-testing/include/linux/ssb/ssb_regs.h
===================================================================
--- wireless-testing.orig/include/linux/ssb/ssb_regs.h
+++ wireless-testing/include/linux/ssb/ssb_regs.h
@@ -26,6 +26,7 @@
#define SSB_EUART (SSB_EXTIF_BASE + 0x00800000)
#define SSB_LED (SSB_EXTIF_BASE + 0x00900000)

+#define SSB_MMIO_POWERUP_DELAY 0x06A8

/* Enumeration space constants */
#define SSB_CORE_SIZE 0x1000 /* Size of a core MMIO area */




2010-05-01 18:43:56

by Gábor Stefanik

[permalink] [raw]
Subject: Re: [PATCH] ssb: Implement fast powerup delay calculation

On Sat, May 1, 2010 at 8:40 PM, Michael Buesch <[email protected]> wrote:
> On Saturday 01 May 2010 20:27:42 Larry Finger wrote:
>> OK, I changed the spec to remove the write.
>
> Uhm, I think the spec was perfecly fine.
>
> --
> Greetings, Michael.
>

Yes. The specs stated "do this during 802.11 core init", the problem
was the patch doing the write during ChipCommon init, much earlier.

--
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

2010-05-01 18:15:58

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH] ssb: Implement fast powerup delay calculation

On Saturday 01 May 2010 19:42:27 Larry Finger wrote:
> I may have gotten the MMIO offset part wrong in the specs, but the
> Broadcom driver definitely writes to offset 0x648 at that point.

I do not think you are right.

First: The write has to happen to the 802.11 core and not to the chipcommon core.

Second: The code that was touched by G?bor is _very_ early in the SSB init.
Way ahead of the wireless core init. The specs state that the delay value has to be written
after the wireless core init is nearly done.
We do _exactly_ that in b43.

The only thing that needs to be done is to add the additional switch statement.
No MMIO writes have to be touched.

--
Greetings, Michael.

2010-05-01 17:19:45

by Gábor Stefanik

[permalink] [raw]
Subject: Re: [PATCH] ssb: Implement fast powerup delay calculation

2010/5/1 G?bor Stefanik <[email protected]>:
> 2010/5/1 Michael Buesch <[email protected]>:
>> On Saturday 01 May 2010 18:29:06 G?bor Stefanik wrote:
>>> + ? ? ssb_write16(cc->dev, SSB_MMIO_POWERUP_DELAY, delay);
>>> ? }
>>>
>>> ? void ssb_chipco_suspend(struct ssb_chipcommon *cc)
>>> Index: wireless-testing/include/linux/ssb/ssb_regs.h
>>> ===================================================================
>>> --- wireless-testing.orig/include/linux/ssb/ssb_regs.h
>>> +++ wireless-testing/include/linux/ssb/ssb_regs.h
>>> @@ -26,6 +26,7 @@
>>> ? #define ? ? SSB_EUART ? ? ? ? ? ? ? (SSB_EXTIF_BASE + 0x00800000)
>>> ? #define ? ? SSB_LED ? ? ? ? ? ? ? ? (SSB_EXTIF_BASE + 0x00900000)
>>>
>>> +#define SSB_MMIO_POWERUP_DELAY ? ? ? 0x06A8
>>
>> I think you are really confusing something here.
>> That register is a wireless core register and we already write it in b43.
>>
>> --
>> Greetings, Michael.
>>
>
> This is what I am implementing: http://bcm-v4.sipsolutions.net/802.11/Init
> Here, it clearly says MMIO offset.
>
> --
> Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
>

Sorry, now I understand. The MMIO write is unnecessary.

--
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

2010-05-01 17:42:32

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] ssb: Implement fast powerup delay calculation

On 05/01/2010 12:19 PM, Michael Buesch wrote:
> On Saturday 01 May 2010 19:13:31 G?bor Stefanik wrote:
>> 2010/5/1 Michael Buesch <[email protected]>:
>>> On Saturday 01 May 2010 18:29:06 G?bor Stefanik wrote:
>>>> + ssb_write16(cc->dev, SSB_MMIO_POWERUP_DELAY, delay);
>>>> }
>>>>
>>>> void ssb_chipco_suspend(struct ssb_chipcommon *cc)
>>>> Index: wireless-testing/include/linux/ssb/ssb_regs.h
>>>> ===================================================================
>>>> --- wireless-testing.orig/include/linux/ssb/ssb_regs.h
>>>> +++ wireless-testing/include/linux/ssb/ssb_regs.h
>>>> @@ -26,6 +26,7 @@
>>>> #define SSB_EUART (SSB_EXTIF_BASE + 0x00800000)
>>>> #define SSB_LED (SSB_EXTIF_BASE + 0x00900000)
>>>>
>>>> +#define SSB_MMIO_POWERUP_DELAY 0x06A8
>>>
>>> I think you are really confusing something here.
>>> That register is a wireless core register and we already write it in b43.
>>>
>>> --
>>> Greetings, Michael.
>>>
>>
>> This is what I am implementing: http://bcm-v4.sipsolutions.net/802.11/Init
>> Here, it clearly says MMIO offset.
>
> Yeah. Just what I said. It is an 802.11 core register.
> We already write it in b43/main.c/b43_chip_init().
> It is _wrong_ to write to the chipcommon at that offset.

I may have gotten the MMIO offset part wrong in the specs, but the
Broadcom driver definitely writes to offset 0x648 at that point.

I know that b43 writes that location; however, it is much later in the
startup sequence. Whether that is important is unknown at this point.

The previous code resulted in a value of zero being written to the
location in question, but the value for the 4315 is 7000.

Larry

2010-05-01 18:27:46

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] ssb: Implement fast powerup delay calculation

On 05/01/2010 01:15 PM, Michael Buesch wrote:
> On Saturday 01 May 2010 19:42:27 Larry Finger wrote:
>> I may have gotten the MMIO offset part wrong in the specs, but the
>> Broadcom driver definitely writes to offset 0x648 at that point.
>
> I do not think you are right.
>
> First: The write has to happen to the 802.11 core and not to the chipcommon core.
>
> Second: The code that was touched by G?bor is _very_ early in the SSB init.
> Way ahead of the wireless core init. The specs state that the delay value has to be written
> after the wireless core init is nearly done.
> We do _exactly_ that in b43.
>
> The only thing that needs to be done is to add the additional switch statement.
> No MMIO writes have to be touched.

OK, I changed the spec to remove the write.

Larry

2010-05-01 17:48:28

by Gábor Stefanik

[permalink] [raw]
Subject: Re: [PATCH] ssb: Implement fast powerup delay calculation

On Sat, May 1, 2010 at 7:42 PM, Larry Finger <[email protected]> wrote:
> On 05/01/2010 12:19 PM, Michael Buesch wrote:
>> On Saturday 01 May 2010 19:13:31 G?bor Stefanik wrote:
>>> 2010/5/1 Michael Buesch <[email protected]>:
>>>> On Saturday 01 May 2010 18:29:06 G?bor Stefanik wrote:
>>>>> + ? ? ssb_write16(cc->dev, SSB_MMIO_POWERUP_DELAY, delay);
>>>>> ? }
>>>>>
>>>>> ? void ssb_chipco_suspend(struct ssb_chipcommon *cc)
>>>>> Index: wireless-testing/include/linux/ssb/ssb_regs.h
>>>>> ===================================================================
>>>>> --- wireless-testing.orig/include/linux/ssb/ssb_regs.h
>>>>> +++ wireless-testing/include/linux/ssb/ssb_regs.h
>>>>> @@ -26,6 +26,7 @@
>>>>> ? #define ? ? SSB_EUART ? ? ? ? ? ? ? (SSB_EXTIF_BASE + 0x00800000)
>>>>> ? #define ? ? SSB_LED ? ? ? ? ? ? ? ? (SSB_EXTIF_BASE + 0x00900000)
>>>>>
>>>>> +#define SSB_MMIO_POWERUP_DELAY ? ? ? 0x06A8
>>>>
>>>> I think you are really confusing something here.
>>>> That register is a wireless core register and we already write it in b43.
>>>>
>>>> --
>>>> Greetings, Michael.
>>>>
>>>
>>> This is what I am implementing: http://bcm-v4.sipsolutions.net/802.11/Init
>>> Here, it clearly says MMIO offset.
>>
>> Yeah. Just what I said. It is an 802.11 core register.
>> We already write it in b43/main.c/b43_chip_init().
>> It is _wrong_ to write to the chipcommon at that offset.
>
> I may have gotten the MMIO offset part wrong in the specs, but the
> Broadcom driver definitely writes to offset 0x648 at that point.
>
> I know that b43 writes that location; however, it is much later in the
> startup sequence. Whether that is important is unknown at this point.
>
> The previous code resulted in a value of zero being written to the
> location in question, but the value for the 4315 is 7000.
>
> Larry
>

According to http://bcm-v4.sipsolutions.net/802.11/Init, b43 is right
- the write should go to the 802.11 core, not ChipCommon.

--
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

2010-05-01 17:07:11

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH] ssb: Implement fast powerup delay calculation

On Saturday 01 May 2010 18:29:06 G?bor Stefanik wrote:
> + ssb_write16(cc->dev, SSB_MMIO_POWERUP_DELAY, delay);
> }
>
> void ssb_chipco_suspend(struct ssb_chipcommon *cc)
> Index: wireless-testing/include/linux/ssb/ssb_regs.h
> ===================================================================
> --- wireless-testing.orig/include/linux/ssb/ssb_regs.h
> +++ wireless-testing/include/linux/ssb/ssb_regs.h
> @@ -26,6 +26,7 @@
> #define SSB_EUART (SSB_EXTIF_BASE + 0x00800000)
> #define SSB_LED (SSB_EXTIF_BASE + 0x00900000)
>
> +#define SSB_MMIO_POWERUP_DELAY 0x06A8

I think you are really confusing something here.
That register is a wireless core register and we already write it in b43.

--
Greetings, Michael.

2010-05-01 17:19:44

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH] ssb: Implement fast powerup delay calculation

On Saturday 01 May 2010 19:13:31 G?bor Stefanik wrote:
> 2010/5/1 Michael Buesch <[email protected]>:
> > On Saturday 01 May 2010 18:29:06 G?bor Stefanik wrote:
> >> + ssb_write16(cc->dev, SSB_MMIO_POWERUP_DELAY, delay);
> >> }
> >>
> >> void ssb_chipco_suspend(struct ssb_chipcommon *cc)
> >> Index: wireless-testing/include/linux/ssb/ssb_regs.h
> >> ===================================================================
> >> --- wireless-testing.orig/include/linux/ssb/ssb_regs.h
> >> +++ wireless-testing/include/linux/ssb/ssb_regs.h
> >> @@ -26,6 +26,7 @@
> >> #define SSB_EUART (SSB_EXTIF_BASE + 0x00800000)
> >> #define SSB_LED (SSB_EXTIF_BASE + 0x00900000)
> >>
> >> +#define SSB_MMIO_POWERUP_DELAY 0x06A8
> >
> > I think you are really confusing something here.
> > That register is a wireless core register and we already write it in b43.
> >
> > --
> > Greetings, Michael.
> >
>
> This is what I am implementing: http://bcm-v4.sipsolutions.net/802.11/Init
> Here, it clearly says MMIO offset.

Yeah. Just what I said. It is an 802.11 core register.
We already write it in b43/main.c/b43_chip_init().
It is _wrong_ to write to the chipcommon at that offset.

--
Greetings, Michael.

2010-05-01 18:40:48

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH] ssb: Implement fast powerup delay calculation

On Saturday 01 May 2010 20:27:42 Larry Finger wrote:
> OK, I changed the spec to remove the write.

Uhm, I think the spec was perfecly fine.

--
Greetings, Michael.

2010-05-01 17:13:54

by Gábor Stefanik

[permalink] [raw]
Subject: Re: [PATCH] ssb: Implement fast powerup delay calculation

2010/5/1 Michael Buesch <[email protected]>:
> On Saturday 01 May 2010 18:29:06 G?bor Stefanik wrote:
>> + ? ? ssb_write16(cc->dev, SSB_MMIO_POWERUP_DELAY, delay);
>> ? }
>>
>> ? void ssb_chipco_suspend(struct ssb_chipcommon *cc)
>> Index: wireless-testing/include/linux/ssb/ssb_regs.h
>> ===================================================================
>> --- wireless-testing.orig/include/linux/ssb/ssb_regs.h
>> +++ wireless-testing/include/linux/ssb/ssb_regs.h
>> @@ -26,6 +26,7 @@
>> ? #define ? ? SSB_EUART ? ? ? ? ? ? ? (SSB_EXTIF_BASE + 0x00800000)
>> ? #define ? ? SSB_LED ? ? ? ? ? ? ? ? (SSB_EXTIF_BASE + 0x00900000)
>>
>> +#define SSB_MMIO_POWERUP_DELAY ? ? ? 0x06A8
>
> I think you are really confusing something here.
> That register is a wireless core register and we already write it in b43.
>
> --
> Greetings, Michael.
>

This is what I am implementing: http://bcm-v4.sipsolutions.net/802.11/Init
Here, it clearly says MMIO offset.

--
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

2010-05-01 16:33:09

by Gábor Stefanik

[permalink] [raw]
Subject: Re: [PATCH] ssb: Implement fast powerup delay calculation

2010/5/1 G?bor Stefanik <[email protected]>:
> Implement fast powerup delay calculation, as described
> in the recently updated specs. The 4325 part is coming soon.
>
> Signed-off-by: G?bor Stefanik<[email protected]>
> ---
>
> To people experiencing DMA errors: please test this patch, it touches
> the PMU code, one of the suspected problem areas.
>
> I'm submitting this patch using Thunderbird 3; so I cannot be held liable
> if it gets damaged in transit. Blame Mozilla. :-)

Apparently the nasty space-doubling issue seen in the Tb3 beta is
still present in the final version... here is the patch as an
attachment.

--G?bor


Attachments:
ssb_fast_pwrup_delay.patch (2.43 kB)