[RFT][PATCH] b43: fix logic in GPIO configuration

[RFT][PATCH] b43: fix logic in GPIO configuration

[Michael Büsch]

On Tue,  6 Mar 2012 23:11:38 +0100
Rafa? Mi?ecki <zajec5@gmail.com> wrote:

> By using reverted mask we were taking over pins we were not supporsed to
> touch. After fixing this workaround for BCM5354 should not be needed
> anymore.
> 
> Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
> ---
>  drivers/net/wireless/b43/main.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
> index 1d633f3..8a89885 100644
> --- a/drivers/net/wireless/b43/main.c
> +++ b/drivers/net/wireless/b43/main.c
> @@ -2706,8 +2706,6 @@ static int b43_gpio_init(struct b43_wldev *dev)
>  		mask |= 0x0060;
>  		set |= 0x0060;
>  	}
> -	if (dev->dev->chip_id == 0x5354)
> -		set &= 0xff02;
>  	if (0 /* FIXME: conditional unknown */ ) {
>  		b43_write16(dev, B43_MMIO_GPIO_MASK,
>  			    b43_read16(dev, B43_MMIO_GPIO_MASK)
> @@ -2730,7 +2728,7 @@ static int b43_gpio_init(struct b43_wldev *dev)
>  	case B43_BUS_BCMA:
>  		bcma_cc_write32(&dev->dev->bdev->bus->drv_cc, BCMA_CC_GPIOCTL,
>  				(bcma_cc_read32(&dev->dev->bdev->bus->drv_cc,
> -					BCMA_CC_GPIOCTL) & mask) | set);
> +					BCMA_CC_GPIOCTL) & ~mask) | set);
>  		break;
>  #endif
>  #ifdef CONFIG_B43_SSB
> @@ -2739,7 +2737,7 @@ static int b43_gpio_init(struct b43_wldev *dev)
>  		if (gpiodev)
>  			ssb_write32(gpiodev, B43_GPIO_CONTROL,
>  				    (ssb_read32(gpiodev, B43_GPIO_CONTROL)
> -				    & mask) | set);
> +				    & ~mask) | set);
>  		break;
>  #endif
>  	}

This patch makes sense. Can you also check b43legacy? I guess it has the same problem.

And while we're at it, you could also remove these two lines:

2737         if (dev->dev->core_rev >= 2)
2738                 mask |= 0x0010; /* FIXME: This is redundant. */

because core_rev is always >= 2 in b43 and as the FIXME says the bit is already set.


Also, take a look at b43_gpio_cleanup() (in b43 and b43legacy).
It always forces all bits to zero. This clearly is wrong, too. It is supposed to
revert the changes done in the b43 GPIO setup. It seems too complicated to
fixup the code to actually do this, though. I think we should simply get rid of
b43_gpio_cleanup() completely and simply leave the GPIOs as-is. No kittens
will be hurt by doing so; so it's ok.

-- 
Greetings, Michael.

PGP encryption is encouraged / 908D8B0E
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/b43-dev/attachments/20120306/68cb7f1b/attachment.sig>

[RFT][PATCH] b43: fix logic in GPIO configuration

[Rafał Miłecki]

By using reverted mask we were taking over pins we were not supporsed to
touch. After fixing this workaround for BCM5354 should not be needed
anymore.

Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
---
 drivers/net/wireless/b43/main.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
index 1d633f3..8a89885 100644
--- a/drivers/net/wireless/b43/main.c
+++ b/drivers/net/wireless/b43/main.c
@@ -2706,8 +2706,6 @@ static int b43_gpio_init(struct b43_wldev *dev)
 		mask |= 0x0060;
 		set |= 0x0060;
 	}
-	if (dev->dev->chip_id == 0x5354)
-		set &= 0xff02;
 	if (0 /* FIXME: conditional unknown */ ) {
 		b43_write16(dev, B43_MMIO_GPIO_MASK,
 			    b43_read16(dev, B43_MMIO_GPIO_MASK)
@@ -2730,7 +2728,7 @@ static int b43_gpio_init(struct b43_wldev *dev)
 	case B43_BUS_BCMA:
 		bcma_cc_write32(&dev->dev->bdev->bus->drv_cc, BCMA_CC_GPIOCTL,
 				(bcma_cc_read32(&dev->dev->bdev->bus->drv_cc,
-					BCMA_CC_GPIOCTL) & mask) | set);
+					BCMA_CC_GPIOCTL) & ~mask) | set);
 		break;
 #endif
 #ifdef CONFIG_B43_SSB
@@ -2739,7 +2737,7 @@ static int b43_gpio_init(struct b43_wldev *dev)
 		if (gpiodev)
 			ssb_write32(gpiodev, B43_GPIO_CONTROL,
 				    (ssb_read32(gpiodev, B43_GPIO_CONTROL)
-				    & mask) | set);
+				    & ~mask) | set);
 		break;
 #endif
 	}
-- 
1.7.3.4

[RFT][PATCH] b43: fix logic in GPIO configuration

[Hauke Mehrtens]

On 03/06/2012 11:11 PM, Rafa? Mi?ecki wrote:
> By using reverted mask we were taking over pins we were not supporsed to
> touch. After fixing this workaround for BCM5354 should not be needed
> anymore.
> 
> Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
> ---
>  drivers/net/wireless/b43/main.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
> index 1d633f3..8a89885 100644
> --- a/drivers/net/wireless/b43/main.c
> +++ b/drivers/net/wireless/b43/main.c
> @@ -2706,8 +2706,6 @@ static int b43_gpio_init(struct b43_wldev *dev)
>  		mask |= 0x0060;
>  		set |= 0x0060;
>  	}
> -	if (dev->dev->chip_id == 0x5354)
> -		set &= 0xff02;
>  	if (0 /* FIXME: conditional unknown */ ) {
>  		b43_write16(dev, B43_MMIO_GPIO_MASK,
>  			    b43_read16(dev, B43_MMIO_GPIO_MASK)
> @@ -2730,7 +2728,7 @@ static int b43_gpio_init(struct b43_wldev *dev)
>  	case B43_BUS_BCMA:
>  		bcma_cc_write32(&dev->dev->bdev->bus->drv_cc, BCMA_CC_GPIOCTL,
>  				(bcma_cc_read32(&dev->dev->bdev->bus->drv_cc,
> -					BCMA_CC_GPIOCTL) & mask) | set);
> +					BCMA_CC_GPIOCTL) & ~mask) | set);
>  		break;
>  #endif
>  #ifdef CONFIG_B43_SSB
> @@ -2739,7 +2737,7 @@ static int b43_gpio_init(struct b43_wldev *dev)
>  		if (gpiodev)
>  			ssb_write32(gpiodev, B43_GPIO_CONTROL,
>  				    (ssb_read32(gpiodev, B43_GPIO_CONTROL)
> -				    & mask) | set);
> +				    & ~mask) | set);
>  		break;
>  #endif
>  	}

Hi,

This patch did not helped fixing my problem. The ucode still takes over
the GPIOs of the buttons with firmware version 666.2. I am getting the
following messages:

[   83.620000] b43-phy0: Loading firmware version 666.2 (2011-02-23
01:15:07)
[   83.624000] Init value of B43_GPIO_CONTROL: 0x0
[   83.632000] b43-phy0 debug: b2062: Using crystal tab entry 19200 kHz.
[   83.640000] hotplug_button: action: pressed, name: reset, seen: 17179652
[   83.644000] hotplug_button: action: pressed, name: ses, seen: 17179652
[   89.404000] b43-phy0 debug: Chip initialized

[RFT][PATCH] b43: fix logic in GPIO configuration

[Michael Büsch]

On Tue, 06 Mar 2012 23:55:08 +0100
Hauke Mehrtens <hauke@hauke-m.de> wrote:

> This patch did not helped fixing my problem. The ucode still takes over
> the GPIOs of the buttons with firmware version 666.2. I am getting the
> following messages:
> 
> [   83.620000] b43-phy0: Loading firmware version 666.2 (2011-02-23
> 01:15:07)
> [   83.624000] Init value of B43_GPIO_CONTROL: 0x0


I think this is caused by b43_gpio_cleanup. Can you try removing that function (or just returning early, for the time being.)

-- 
Greetings, Michael.

PGP encryption is encouraged / 908D8B0E
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/b43-dev/attachments/20120307/8834d02e/attachment.sig>

[RFT][PATCH] b43: fix logic in GPIO configuration

[Rafał Miłecki]

W dniu 6 marca 2012 23:55 u?ytkownik Hauke Mehrtens <hauke@hauke-m.de> napisa?:
> On 03/06/2012 11:11 PM, Rafa? Mi?ecki wrote:
>> By using reverted mask we were taking over pins we were not supporsed to
>> touch. After fixing this workaround for BCM5354 should not be needed
>> anymore.
>>
>> Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
>> ---
>> ?drivers/net/wireless/b43/main.c | ? ?6 ++----
>> ?1 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
>> index 1d633f3..8a89885 100644
>> --- a/drivers/net/wireless/b43/main.c
>> +++ b/drivers/net/wireless/b43/main.c
>> @@ -2706,8 +2706,6 @@ static int b43_gpio_init(struct b43_wldev *dev)
>> ? ? ? ? ? ? ? mask |= 0x0060;
>> ? ? ? ? ? ? ? set |= 0x0060;
>> ? ? ? }
>> - ? ? if (dev->dev->chip_id == 0x5354)
>> - ? ? ? ? ? ? set &= 0xff02;
>> ? ? ? if (0 /* FIXME: conditional unknown */ ) {
>> ? ? ? ? ? ? ? b43_write16(dev, B43_MMIO_GPIO_MASK,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? b43_read16(dev, B43_MMIO_GPIO_MASK)
>> @@ -2730,7 +2728,7 @@ static int b43_gpio_init(struct b43_wldev *dev)
>> ? ? ? case B43_BUS_BCMA:
>> ? ? ? ? ? ? ? bcma_cc_write32(&dev->dev->bdev->bus->drv_cc, BCMA_CC_GPIOCTL,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (bcma_cc_read32(&dev->dev->bdev->bus->drv_cc,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? BCMA_CC_GPIOCTL) & mask) | set);
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? BCMA_CC_GPIOCTL) & ~mask) | set);
>> ? ? ? ? ? ? ? break;
>> ?#endif
>> ?#ifdef CONFIG_B43_SSB
>> @@ -2739,7 +2737,7 @@ static int b43_gpio_init(struct b43_wldev *dev)
>> ? ? ? ? ? ? ? if (gpiodev)
>> ? ? ? ? ? ? ? ? ? ? ? ssb_write32(gpiodev, B43_GPIO_CONTROL,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (ssb_read32(gpiodev, B43_GPIO_CONTROL)
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? & mask) | set);
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? & ~mask) | set);
>> ? ? ? ? ? ? ? break;
>> ?#endif
>> ? ? ? }
>
> Hi,
>
> This patch did not helped fixing my problem. The ucode still takes over
> the GPIOs of the buttons with firmware version 666.2. I am getting the
> following messages:
>
> [ ? 83.620000] b43-phy0: Loading firmware version 666.2 (2011-02-23
> 01:15:07)
> [ ? 83.624000] Init value of B43_GPIO_CONTROL: 0x0
> [ ? 83.632000] b43-phy0 debug: b2062: Using crystal tab entry 19200 kHz.
> [ ? 83.640000] hotplug_button: action: pressed, name: reset, seen: 17179652
> [ ? 83.644000] hotplug_button: action: pressed, name: ses, seen: 17179652
> [ ? 89.404000] b43-phy0 debug: Chip initialized

Interesting/weird. I suspected there are some bits set in
B43_GPIO_CONTROL we should not clean. And you just adjusted "set" to
keep them set. But now it seems B43_GPIO_CONTROL is *zero* at the
beginning and we actually have to *set* 0xFF00 to stop firmware
touching them? Really tricky, I didn't suspect we may need to change
default state of non-b43-related GPIOs.

-- 
Rafa?

[RFT][PATCH] b43: fix logic in GPIO configuration

[Rafał Miłecki]

2012/3/7 Michael B?sch <m@bues.ch>:
> On Tue, 06 Mar 2012 23:55:08 +0100
> Hauke Mehrtens <hauke@hauke-m.de> wrote:
>
>> This patch did not helped fixing my problem. The ucode still takes over
>> the GPIOs of the buttons with firmware version 666.2. I am getting the
>> following messages:
>>
>> [ ? 83.620000] b43-phy0: Loading firmware version 666.2 (2011-02-23
>> 01:15:07)
>> [ ? 83.624000] Init value of B43_GPIO_CONTROL: 0x0
>
>
> I think this is caused by b43_gpio_cleanup. Can you try removing that function (or just returning early, for the time being.)

B43_GPIO_CONTROL is zero by default (after boot), so AFAIU setting it
back to zero in b43_gpio_cleanup shouldn't break anything.

-- 
Rafa?

[RFT][PATCH] b43: fix logic in GPIO configuration

[Rafał Miłecki]

W dniu 7 marca 2012 07:52 u?ytkownik Rafa? Mi?ecki <zajec5@gmail.com> napisa?:
> W dniu 6 marca 2012 23:55 u?ytkownik Hauke Mehrtens <hauke@hauke-m.de> napisa?:
>> On 03/06/2012 11:11 PM, Rafa? Mi?ecki wrote:
>>> By using reverted mask we were taking over pins we were not supporsed to
>>> touch. After fixing this workaround for BCM5354 should not be needed
>>> anymore.
>>>
>>> Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
>>> ---
>>> ?drivers/net/wireless/b43/main.c | ? ?6 ++----
>>> ?1 files changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
>>> index 1d633f3..8a89885 100644
>>> --- a/drivers/net/wireless/b43/main.c
>>> +++ b/drivers/net/wireless/b43/main.c
>>> @@ -2706,8 +2706,6 @@ static int b43_gpio_init(struct b43_wldev *dev)
>>> ? ? ? ? ? ? ? mask |= 0x0060;
>>> ? ? ? ? ? ? ? set |= 0x0060;
>>> ? ? ? }
>>> - ? ? if (dev->dev->chip_id == 0x5354)
>>> - ? ? ? ? ? ? set &= 0xff02;
>>> ? ? ? if (0 /* FIXME: conditional unknown */ ) {
>>> ? ? ? ? ? ? ? b43_write16(dev, B43_MMIO_GPIO_MASK,
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? b43_read16(dev, B43_MMIO_GPIO_MASK)
>>> @@ -2730,7 +2728,7 @@ static int b43_gpio_init(struct b43_wldev *dev)
>>> ? ? ? case B43_BUS_BCMA:
>>> ? ? ? ? ? ? ? bcma_cc_write32(&dev->dev->bdev->bus->drv_cc, BCMA_CC_GPIOCTL,
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (bcma_cc_read32(&dev->dev->bdev->bus->drv_cc,
>>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? BCMA_CC_GPIOCTL) & mask) | set);
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? BCMA_CC_GPIOCTL) & ~mask) | set);
>>> ? ? ? ? ? ? ? break;
>>> ?#endif
>>> ?#ifdef CONFIG_B43_SSB
>>> @@ -2739,7 +2737,7 @@ static int b43_gpio_init(struct b43_wldev *dev)
>>> ? ? ? ? ? ? ? if (gpiodev)
>>> ? ? ? ? ? ? ? ? ? ? ? ssb_write32(gpiodev, B43_GPIO_CONTROL,
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (ssb_read32(gpiodev, B43_GPIO_CONTROL)
>>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? & mask) | set);
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? & ~mask) | set);
>>> ? ? ? ? ? ? ? break;
>>> ?#endif
>>> ? ? ? }
>>
>> Hi,
>>
>> This patch did not helped fixing my problem. The ucode still takes over
>> the GPIOs of the buttons with firmware version 666.2. I am getting the
>> following messages:
>>
>> [ ? 83.620000] b43-phy0: Loading firmware version 666.2 (2011-02-23
>> 01:15:07)
>> [ ? 83.624000] Init value of B43_GPIO_CONTROL: 0x0
>> [ ? 83.632000] b43-phy0 debug: b2062: Using crystal tab entry 19200 kHz.
>> [ ? 83.640000] hotplug_button: action: pressed, name: reset, seen: 17179652
>> [ ? 83.644000] hotplug_button: action: pressed, name: ses, seen: 17179652
>> [ ? 89.404000] b43-phy0 debug: Chip initialized
>
> Interesting/weird. I suspected there are some bits set in
> B43_GPIO_CONTROL we should not clean. And you just adjusted "set" to
> keep them set. But now it seems B43_GPIO_CONTROL is *zero* at the
> beginning and we actually have to *set* 0xFF00 to stop firmware
> touching them? Really tricky, I didn't suspect we may need to change
> default state of non-b43-related GPIOs.

Ahh, sorry. For all the time I believed the code you added was:
set |= 0xff02;

However the code you *really* added is:
set &= 0xff02;

Sorry for messing. OK, let's short analyze that. By default we set "set" to:
set = 0x0000000F;
so you code basically makes it
set = 0x00000002;

0x2 (BIT 1) is wlan led on your device. So you let firmware use wlan
led, but you stop it from touching power led (0x1), reset button (0x4)
and ses button (0x8). OK, it makes sense.

In the brcm80211 code there was unused si_gpioreserve function. I
guess it used to be called in some way like:
if (dev->dev->chip_id == 0x5354)
si_gpioreserve(sih, 0x2, 1+)

-- 
Rafa?

[RFT][PATCH] b43: fix logic in GPIO configuration

[Hauke Mehrtens]

On 03/07/2012 08:23 AM, Rafa? Mi?ecki wrote:
> W dniu 7 marca 2012 07:52 u?ytkownik Rafa? Mi?ecki <zajec5@gmail.com> napisa?:
>> W dniu 6 marca 2012 23:55 u?ytkownik Hauke Mehrtens <hauke@hauke-m.de> napisa?:
>>> On 03/06/2012 11:11 PM, Rafa? Mi?ecki wrote:
>>>> By using reverted mask we were taking over pins we were not supporsed to
>>>> touch. After fixing this workaround for BCM5354 should not be needed
>>>> anymore.
>>>>
>>>> Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
>>>> ---
>>>>  drivers/net/wireless/b43/main.c |    6 ++----
>>>>  1 files changed, 2 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
>>>> index 1d633f3..8a89885 100644
>>>> --- a/drivers/net/wireless/b43/main.c
>>>> +++ b/drivers/net/wireless/b43/main.c
>>>> @@ -2706,8 +2706,6 @@ static int b43_gpio_init(struct b43_wldev *dev)
>>>>               mask |= 0x0060;
>>>>               set |= 0x0060;
>>>>       }
>>>> -     if (dev->dev->chip_id == 0x5354)
>>>> -             set &= 0xff02;
>>>>       if (0 /* FIXME: conditional unknown */ ) {
>>>>               b43_write16(dev, B43_MMIO_GPIO_MASK,
>>>>                           b43_read16(dev, B43_MMIO_GPIO_MASK)
>>>> @@ -2730,7 +2728,7 @@ static int b43_gpio_init(struct b43_wldev *dev)
>>>>       case B43_BUS_BCMA:
>>>>               bcma_cc_write32(&dev->dev->bdev->bus->drv_cc, BCMA_CC_GPIOCTL,
>>>>                               (bcma_cc_read32(&dev->dev->bdev->bus->drv_cc,
>>>> -                                     BCMA_CC_GPIOCTL) & mask) | set);
>>>> +                                     BCMA_CC_GPIOCTL) & ~mask) | set);
>>>>               break;
>>>>  #endif
>>>>  #ifdef CONFIG_B43_SSB
>>>> @@ -2739,7 +2737,7 @@ static int b43_gpio_init(struct b43_wldev *dev)
>>>>               if (gpiodev)
>>>>                       ssb_write32(gpiodev, B43_GPIO_CONTROL,
>>>>                                   (ssb_read32(gpiodev, B43_GPIO_CONTROL)
>>>> -                                 & mask) | set);
>>>> +                                 & ~mask) | set);
>>>>               break;
>>>>  #endif
>>>>       }
>>>
>>> Hi,
>>>
>>> This patch did not helped fixing my problem. The ucode still takes over
>>> the GPIOs of the buttons with firmware version 666.2. I am getting the
>>> following messages:
>>>
>>> [   83.620000] b43-phy0: Loading firmware version 666.2 (2011-02-23
>>> 01:15:07)
>>> [   83.624000] Init value of B43_GPIO_CONTROL: 0x0
>>> [   83.632000] b43-phy0 debug: b2062: Using crystal tab entry 19200 kHz.
>>> [   83.640000] hotplug_button: action: pressed, name: reset, seen: 17179652
>>> [   83.644000] hotplug_button: action: pressed, name: ses, seen: 17179652
>>> [   89.404000] b43-phy0 debug: Chip initialized
>>
>> Interesting/weird. I suspected there are some bits set in
>> B43_GPIO_CONTROL we should not clean. And you just adjusted "set" to
>> keep them set. But now it seems B43_GPIO_CONTROL is *zero* at the
>> beginning and we actually have to *set* 0xFF00 to stop firmware
>> touching them? Really tricky, I didn't suspect we may need to change
>> default state of non-b43-related GPIOs.
> 
> Ahh, sorry. For all the time I believed the code you added was:
> set |= 0xff02;
> 
> However the code you *really* added is:
> set &= 0xff02;
> 
> Sorry for messing. OK, let's short analyze that. By default we set "set" to:
> set = 0x0000000F;
> so you code basically makes it
> set = 0x00000002;
> 
> 0x2 (BIT 1) is wlan led on your device. So you let firmware use wlan
> led, but you stop it from touching power led (0x1), reset button (0x4)
> and ses button (0x8). OK, it makes sense.
Yes
It used the GPIO pins with the bit numbers set to 1, at least for the
first 4 ones. I just made it just use the GPIO for the wlan LED. Should
I still test some patches on the device?
> 
> In the brcm80211 code there was unused si_gpioreserve function. I
> guess it used to be called in some way like:
> if (dev->dev->chip_id == 0x5354)
> si_gpioreserve(sih, 0x2, 1+)

Where did you find this code? I was unable to find any code adding some
special gpio handling for the bcm5354 in my sources.

Hauke

[RFT][PATCH] b43: fix logic in GPIO configuration

[Rafał Miłecki]

W dniu 10 marca 2012 00:17 u?ytkownik Hauke Mehrtens <hauke@hauke-m.de> napisa?:
> On 03/07/2012 08:23 AM, Rafa? Mi?ecki wrote:
>> W dniu 7 marca 2012 07:52 u?ytkownik Rafa? Mi?ecki <zajec5@gmail.com> napisa?:
>>> W dniu 6 marca 2012 23:55 u?ytkownik Hauke Mehrtens <hauke@hauke-m.de> napisa?:
>>>> On 03/06/2012 11:11 PM, Rafa? Mi?ecki wrote:
>>>>> By using reverted mask we were taking over pins we were not supporsed to
>>>>> touch. After fixing this workaround for BCM5354 should not be needed
>>>>> anymore.
>>>>>
>>>>> Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
>>>>> ---
>>>>> ?drivers/net/wireless/b43/main.c | ? ?6 ++----
>>>>> ?1 files changed, 2 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
>>>>> index 1d633f3..8a89885 100644
>>>>> --- a/drivers/net/wireless/b43/main.c
>>>>> +++ b/drivers/net/wireless/b43/main.c
>>>>> @@ -2706,8 +2706,6 @@ static int b43_gpio_init(struct b43_wldev *dev)
>>>>> ? ? ? ? ? ? ? mask |= 0x0060;
>>>>> ? ? ? ? ? ? ? set |= 0x0060;
>>>>> ? ? ? }
>>>>> - ? ? if (dev->dev->chip_id == 0x5354)
>>>>> - ? ? ? ? ? ? set &= 0xff02;
>>>>> ? ? ? if (0 /* FIXME: conditional unknown */ ) {
>>>>> ? ? ? ? ? ? ? b43_write16(dev, B43_MMIO_GPIO_MASK,
>>>>> ? ? ? ? ? ? ? ? ? ? ? ? ? b43_read16(dev, B43_MMIO_GPIO_MASK)
>>>>> @@ -2730,7 +2728,7 @@ static int b43_gpio_init(struct b43_wldev *dev)
>>>>> ? ? ? case B43_BUS_BCMA:
>>>>> ? ? ? ? ? ? ? bcma_cc_write32(&dev->dev->bdev->bus->drv_cc, BCMA_CC_GPIOCTL,
>>>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (bcma_cc_read32(&dev->dev->bdev->bus->drv_cc,
>>>>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? BCMA_CC_GPIOCTL) & mask) | set);
>>>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? BCMA_CC_GPIOCTL) & ~mask) | set);
>>>>> ? ? ? ? ? ? ? break;
>>>>> ?#endif
>>>>> ?#ifdef CONFIG_B43_SSB
>>>>> @@ -2739,7 +2737,7 @@ static int b43_gpio_init(struct b43_wldev *dev)
>>>>> ? ? ? ? ? ? ? if (gpiodev)
>>>>> ? ? ? ? ? ? ? ? ? ? ? ssb_write32(gpiodev, B43_GPIO_CONTROL,
>>>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (ssb_read32(gpiodev, B43_GPIO_CONTROL)
>>>>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? & mask) | set);
>>>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? & ~mask) | set);
>>>>> ? ? ? ? ? ? ? break;
>>>>> ?#endif
>>>>> ? ? ? }
>>>>
>>>> Hi,
>>>>
>>>> This patch did not helped fixing my problem. The ucode still takes over
>>>> the GPIOs of the buttons with firmware version 666.2. I am getting the
>>>> following messages:
>>>>
>>>> [ ? 83.620000] b43-phy0: Loading firmware version 666.2 (2011-02-23
>>>> 01:15:07)
>>>> [ ? 83.624000] Init value of B43_GPIO_CONTROL: 0x0
>>>> [ ? 83.632000] b43-phy0 debug: b2062: Using crystal tab entry 19200 kHz.
>>>> [ ? 83.640000] hotplug_button: action: pressed, name: reset, seen: 17179652
>>>> [ ? 83.644000] hotplug_button: action: pressed, name: ses, seen: 17179652
>>>> [ ? 89.404000] b43-phy0 debug: Chip initialized
>>>
>>> Interesting/weird. I suspected there are some bits set in
>>> B43_GPIO_CONTROL we should not clean. And you just adjusted "set" to
>>> keep them set. But now it seems B43_GPIO_CONTROL is *zero* at the
>>> beginning and we actually have to *set* 0xFF00 to stop firmware
>>> touching them? Really tricky, I didn't suspect we may need to change
>>> default state of non-b43-related GPIOs.
>>
>> Ahh, sorry. For all the time I believed the code you added was:
>> set |= 0xff02;
>>
>> However the code you *really* added is:
>> set &= 0xff02;
>>
>> Sorry for messing. OK, let's short analyze that. By default we set "set" to:
>> set = 0x0000000F;
>> so you code basically makes it
>> set = 0x00000002;
>>
>> 0x2 (BIT 1) is wlan led on your device. So you let firmware use wlan
>> led, but you stop it from touching power led (0x1), reset button (0x4)
>> and ses button (0x8). OK, it makes sense.
> Yes
> It used the GPIO pins with the bit numbers set to 1, at least for the
> first 4 ones. I just made it just use the GPIO for the wlan LED. Should
> I still test some patches on the device?
>>
>> In the brcm80211 code there was unused si_gpioreserve function. I
>> guess it used to be called in some way like:
>> if (dev->dev->chip_id == 0x5354)
>> si_gpioreserve(sih, 0x2, 1+)
>
> Where did you find this code? I was unable to find any code adding some
> special gpio handling for the bcm5354 in my sources.

As I said, I've found only si_gpioreserve function. I didn't find
special handling of BCM5354, I only guess something like that was used
internally at Broadcom in their wl. Nothing like that was released
with brcm80211.

If you wish to see si_gpioreserve function, take a look at

commit a9533e7ea3c410fed2f4cd8b3e1e213e48529b75
Author: Henry Ptasinski <henryp@broadcom.com>
Date:   Wed Sep 8 21:04:42 2010 -0700

    Staging: Add initial release of brcm80211 - Broadcom 802.11n
wireless LAN driver.

-- 
Rafa?