sdio wifi oob irq support for sunxi

From: Chen-Yu Tsai <[email protected]>

The irq/pin mapping is used to lookup the pin to mux to the irq function when the irq is enabled. It is created when gpio_to_irq is called. Creating the mapping during init allows us to map the interrupts directly from the device tree.

Hi,

Quoted 2 lines · Hans de Goede

From: Chen-Yu Tsai <[email protected]>

Quoted 4 lines

The irq/pin mapping is used to lookup the pin to mux to the irq function when the irq is enabled. It is created when gpio_to_irq is called. Creating the mapping during init allows us to map the interrupts directly from the device tree.

Can you be a bit more precise on this?

What is the issue that this patch fix?

From: Chen-Yu Tsai <[email protected]>

The irq/pin mapping is used to lookup the pin to mux to the irq function when the irq is enabled. It is created when gpio_to_irq is called. Creating the mapping during init allows us to map the interrupts directly from the device tree.

Signed-off-by Chen-Yu Tsai <[email protected]>

Signed-off-by Hans de Goede <[email protected]>

Quoted 7 lines · Hans de Goede

From: Chen-Yu Tsai <[email protected]>

The irq/pin mapping is used to lookup the pin to mux to the irq function when the irq is enabled. It is created when gpio_to_irq is called. Creating the mapping during init allows us to map the interrupts directly from the device tree.

Quoted 2 lines

Signed-off-by Chen-Yu Tsai <[email protected]>

Signed-off-by Hans de Goede <[email protected]>

This is exactly correct. Patch applied.

I tried to hack the sunxi driver to even use the gpiolib irqchip helpers but exactly this complex map thing in ->irq_array[] git me stuck. If any of you guys could be so nice to take a stab at this (compare to other commits converting drivers to use gpiolibs irqchip helpers) then I'd be very happy.

On Tue, May 27, 2014 at 10:11 PM, Linus Walleij

Quoted 6 lines

I tried to hack the sunxi driver to even use the gpiolib irqchip helpers but exactly this complex map thing in ->irq_array[] git me stuck. If any of you guys could be so nice to take a stab at this (compare to other commits converting drivers to use gpiolibs irqchip helpers) then I'd be very happy.

We still need to rework the whole irqchip stuff for sun6i/sun8i, which have 1 parent interrupt per EINT capable pin bank.

Earlier discussions with Maxime (on IRC IIRC) suggested moving this over to gpiolib irqchip helpers might be more work than using our own irqchip implementation, requiring 1 gpiochip per pin group? (not sure)

From: Chen-Yu Tsai <[email protected]>

The sunxi pinctrl irq chip driver does not support wakeup at the moment. Adding IRQCHIP_SKIP_SET_WAKE lets the irqs work with drivers using wakeup.

Also add a name to the irq chip.

Signed-off-by Chen-Yu Tsai <[email protected]>

Signed-off-by Hans de Goede <[email protected]>

---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/pinctrl/sunxi/pinctrl-sunxi.c +0 -0

index db9ccd6..ec60c2e 100644

Quoted 14 lines · Hans de Goede

From: Chen-Yu Tsai <[email protected]>

The sunxi pinctrl irq chip driver does not support wakeup at the moment. Adding IRQCHIP_SKIP_SET_WAKE lets the irqs work with drivers using wakeup.

Also add a name to the irq chip.

Signed-off-by Chen-Yu Tsai <[email protected]>

Signed-off-by Hans de Goede <[email protected]>

---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/pinctrl/sunxi/pinctrl-sunxi.c static struct irq_chip sunxi_pinctrl_irq_chip = { · line 650

Quoted 10 lines

drivers/pinctrl/sunxi/pinctrl-sunxi.c +2 -0

index db9ccd6..ec60c2e 100644
@@ -646,6 +646,8 @@ static struct irq_chip sunxi_pinctrl_irq_chip = {
 	.irq_mask_ack	= sunxi_pinctrl_irq_mask_ack,
 	.irq_unmask	= sunxi_pinctrl_irq_unmask,
 	.irq_set_type	= sunxi_pinctrl_irq_set_type,
+	.name		= "sunxi-pio",
+	.flags		= IRQCHIP_SKIP_SET_WAKE,

I'd rather see the name set to dev_name() or something like that. This will not work that great with multiple pin controller supporting interrupts.

Hi,

Quoted 15 lines · Maxime Ripard

Quoted 14 lines · Hans de Goede

From: Chen-Yu Tsai <[email protected]>

The sunxi pinctrl irq chip driver does not support wakeup at the moment. Adding IRQCHIP_SKIP_SET_WAKE lets the irqs work with drivers using wakeup.

Also add a name to the irq chip.

Signed-off-by Chen-Yu Tsai <[email protected]>

Signed-off-by Hans de Goede <[email protected]>

---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 2 ++
 1 file changed, 2 insertions(+)

Quoted 14 lines

drivers/pinctrl/sunxi/pinctrl-sunxi.c static struct irq_chip sunxi_pinctrl_irq_chip = { · line 650

Quoted 10 lines

drivers/pinctrl/sunxi/pinctrl-sunxi.c +2 -0

index db9ccd6..ec60c2e 100644
@@ -646,6 +646,8 @@ static struct irq_chip sunxi_pinctrl_irq_chip = {
     .irq_mask_ack   = sunxi_pinctrl_irq_mask_ack,
     .irq_unmask     = sunxi_pinctrl_irq_unmask,
     .irq_set_type   = sunxi_pinctrl_irq_set_type,
+    .name           = "sunxi-pio",
+    .flags          = IRQCHIP_SKIP_SET_WAKE,

I'd rather see the name set to dev_name() or something like that. This will not work that great with multiple pin controller supporting interrupts.

That would require moving the irq_chip struct to become a member of the sunxi_pinctrl struct. Not undoable, but that seems like something which should be done in a separate patch. So shall I just drop the .name part of this patch for now ?

Quoted 13 lines · Hans de Goede

From: Chen-Yu Tsai <[email protected]>

The sunxi pinctrl irq chip driver does not support wakeup at the moment. Adding IRQCHIP_SKIP_SET_WAKE lets the irqs work with drivers using wakeup.

Also add a name to the irq chip.

Signed-off-by Chen-Yu Tsai <[email protected]> Signed-off-by: Hans

de Goede <[email protected]> --- drivers/pinctrl/sunxi/pinctrl-sunxi.c | 2 ++ 1 file changed, 2 insertions(+)

Quoted 9 lines

(quoted diff) +0 -0

b/drivers/pinctrl/sunxi/pinctrl-sunxi.c index db9ccd6..ec60c2e 100644 --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c @@ -646,6 +646,8 @@
static struct irq_chip sunxi_pinctrl_irq_chip = { .irq_mask_ack =
sunxi_pinctrl_irq_mask_ack, .irq_unmask =
sunxi_pinctrl_irq_unmask, .irq_set_type =
sunxi_pinctrl_irq_set_type, + .name = "sunxi-pio", + .flags =
IRQCHIP_SKIP_SET_WAKE,

I'd rather see the name set to dev_name() or something like that. This will not work that great with multiple pin controller supporting interrupts.

Quoted 14 lines · Maxime Ripard

Quoted 13 lines · Hans de Goede

From: Chen-Yu Tsai <[email protected]>

The sunxi pinctrl irq chip driver does not support wakeup at the moment. Adding IRQCHIP_SKIP_SET_WAKE lets the irqs work with drivers using wakeup.

Also add a name to the irq chip.

Signed-off-by Chen-Yu Tsai <[email protected]> Signed-off-by: Hans

de Goede <[email protected]> --- drivers/pinctrl/sunxi/pinctrl-sunxi.c | 2 ++ 1 file changed, 2 insertions(+)

Quoted 13 lines

Quoted 9 lines

(quoted diff) +0 -0

b/drivers/pinctrl/sunxi/pinctrl-sunxi.c index db9ccd6..ec60c2e 100644 --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c @@ -646,6 +646,8 @@
static struct irq_chip sunxi_pinctrl_irq_chip = { .irq_mask_ack =
sunxi_pinctrl_irq_mask_ack, .irq_unmask =
sunxi_pinctrl_irq_unmask, .irq_set_type =
sunxi_pinctrl_irq_set_type, + .name = "sunxi-pio", + .flags =
IRQCHIP_SKIP_SET_WAKE,

I'd rather see the name set to dev_name() or something like that. This will not work that great with multiple pin controller supporting interrupts.

Correct me if I'm wrong, but I've always thought that struct irq_chip describes a class of IRQ controllers, not particular instances.

Quoted 30 lines · Tomasz Figa

Quoted 28 lines · Maxime Ripard

Quoted 14 lines · Hans de Goede

From: Chen-Yu Tsai <[email protected]>

The sunxi pinctrl irq chip driver does not support wakeup at the moment. Adding IRQCHIP_SKIP_SET_WAKE lets the irqs work with drivers using wakeup.

Also add a name to the irq chip.

Signed-off-by Chen-Yu Tsai <[email protected]> Signed-off-by:

Hans de Goede <[email protected]> --- drivers/pinctrl/sunxi/pinctrl-sunxi.c | 2 ++ 1 file changed, 2 insertions(+)

(quoted diff) +0 -0

Quoted 9 lines

b/drivers/pinctrl/sunxi/pinctrl-sunxi.c index db9ccd6..ec60c2e 100644 --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c @@ -646,6 +646,8 @@ static struct irq_chip sunxi_pinctrl_irq_chip = {
.irq_mask_ack = sunxi_pinctrl_irq_mask_ack, .irq_unmask =
sunxi_pinctrl_irq_unmask, .irq_set_type =
sunxi_pinctrl_irq_set_type, + .name = "sunxi-pio", + .flags
= IRQCHIP_SKIP_SET_WAKE,

I'd rather see the name set to dev_name() or something like that. This will not work that great with multiple pin controller supporting interrupts.

Quoted 3 lines

Correct me if I'm wrong, but I've always thought that struct irq_chip describes a class of IRQ controllers, not particular instances.

It's not said as such in <linux/irq.h>, but maybe..

With level triggered interrupt mask / unmask will get called for each interrupt, doing the somewhat expensive mux setting on each unmask thus is not a good idea. Instead move it to the set_type callback, which is typically done only once for each irq.

Quoted 4 lines · Hans de Goede

With level triggered interrupt mask / unmask will get called for each interrupt, doing the somewhat expensive mux setting on each unmask thus is not a good idea. Instead move it to the set_type callback, which is typically done only once for each irq.

*This* is the bad idea. Nothing prevents you from calling gpio_get_value whenever you just got your interrupt, that will change the muxing, and never change it back, effectively breaking the interrupts.

Hi,

Quoted 10 lines · Maxime Ripard

Quoted 4 lines · Hans de Goede

With level triggered interrupt mask / unmask will get called for each interrupt, doing the somewhat expensive mux setting on each unmask thus is not a good idea. Instead move it to the set_type callback, which is typically done only once for each irq.

*This* is the bad idea. Nothing prevents you from calling gpio_get_value whenever you just got your interrupt, that will change the muxing, and never change it back, effectively breaking the interrupts.

Hmm, interesting point, but your assuming 2 things which are both not true:

1) That calling gpiod_get_value changes the muxing, which is not true, all gpiod_get_value variants end up in drivers/gpio/gpiolib.c _gpiod_get_raw_value() which does no such thing

2) That unmask will always get called after the gpio_get_value to restore the mux setting for us, which is not guaranteed at all. Before this patch set pinctrl-sunxi.c was using handle_simple_irq which does not mask / unmask at all.

And even with an irq-handler which masks / unmasks, what if the irq wakes up a thread and that thread then does the gpio_get_value ?

Note this is *exactly* what e.g. the mmc gpio card-detect code does, it uses a thread to read the gpio for debouncing.

Luckily 2. is not a problem, since 1. means that the mux won't get changed at all so we don't need to change it back.

With level triggered interrupt mask / unmask will get called for each interrupt, doing the somewhat expensive mux setting on each unmask thus is not a good idea. Instead move it to the set_type callback, which is typically done only once for each irq.

Signed-off-by Hans de Goede <[email protected]>

Quoted 5 lines · Hans de Goede

With level triggered interrupt mask / unmask will get called for each interrupt, doing the somewhat expensive mux setting on each unmask thus is not a good idea. Instead move it to the set_type callback, which is typically done only once for each irq.

Quoted 1 line

Signed-off-by Hans de Goede <[email protected]>

Yes move it out of mask/unmask but no, not into set_type().

Can you not use the irqchip startup()/shutdown() callbacks instead?

And how come we have no clean separation between gpiochip and the pinmux parts here, why cant we just call pinctrl_request_gpio() and pinctrl_free_gpio() here instead? Or maybe pinctrl_gpio_direction_input() and have that set up the muxing in the pinctrl driver side?

Quoted 9 lines · Linus Walleij

Quoted 6 lines · Hans de Goede

With level triggered interrupt mask / unmask will get called for each interrupt, doing the somewhat expensive mux setting on each unmask thus is not a good idea. Instead move it to the set_type callback, which is typically done only once for each irq.

Signed-off-by Hans de Goede <[email protected]>

Quoted 4 lines

Yes move it out of mask/unmask but no, not into set_type().

Can you not use the irqchip startup()/shutdown() callbacks instead?

I think we can use irq_request_resources then
https://lkml.org/lkml/2014/3/12/307

We could even merge the gpio_to_irq code into it.

It's called in __setup_irq, so it's guaranteed to be called, and it will bail out early without doing anything, since it's one of the first thing __setup_irq does.

Hi,

Quoted 10 lines · Maxime Ripard

Quoted 9 lines · Linus Walleij

Quoted 6 lines · Hans de Goede

With level triggered interrupt mask / unmask will get called for each interrupt, doing the somewhat expensive mux setting on each unmask thus is not a good idea. Instead move it to the set_type callback, which is typically done only once for each irq.

Signed-off-by Hans de Goede <[email protected]>

Quoted 7 lines

Quoted 4 lines

Yes move it out of mask/unmask but no, not into set_type().

Can you not use the irqchip startup()/shutdown() callbacks instead?

I think we can use irq_request_resources then
https://lkml.org/lkml/2014/3/12/307

Sounds good, I'll modify the patch to move it here before posting a v2 of this series. Note v2 likely won't happen till this weekend, -ENOTIME.

Quoted 1 line

We could even merge the gpio_to_irq code into it.

Erm, no we need that as a separate function for the gpio_chip's to_irq callback.

Quoted 26 lines · Hans de Goede

Hi,

Quoted 10 lines · Maxime Ripard

Quoted 9 lines · Linus Walleij

Quoted 6 lines · Hans de Goede

With level triggered interrupt mask / unmask will get called for each interrupt, doing the somewhat expensive mux setting on each unmask thus is not a good idea. Instead move it to the set_type callback, which is typically done only once for each irq.

Signed-off-by Hans de Goede <[email protected]>

Quoted 7 lines

Quoted 4 lines

Yes move it out of mask/unmask but no, not into set_type().

Can you not use the irqchip startup()/shutdown() callbacks instead?

I think we can use irq_request_resources then
https://lkml.org/lkml/2014/3/12/307

Sounds good, I'll modify the patch to move it here before posting a v2 of this series. Note v2 likely won't happen till this weekend, -ENOTIME.

Quoted 1 line

We could even merge the gpio_to_irq code into it.

Quoted 2 lines

Erm, no we need that as a separate function for the gpio_chip's to_irq callback.

Linus sent a patch stating otherwise a few weeks ago, and was suggesting moving it to irq_startup.

https://lkml.org/lkml/2014/5/9/50

Hi,

Quoted 27 lines · Maxime Ripard

Quoted 26 lines · Hans de Goede

Hi,

Quoted 10 lines · Maxime Ripard

Quoted 9 lines · Linus Walleij

Quoted 6 lines · Hans de Goede

With level triggered interrupt mask / unmask will get called for each interrupt, doing the somewhat expensive mux setting on each unmask thus is not a good idea. Instead move it to the set_type callback, which is typically done only once for each irq.

Signed-off-by Hans de Goede <[email protected]>

Quoted 7 lines

Quoted 4 lines

Yes move it out of mask/unmask but no, not into set_type().

Can you not use the irqchip startup()/shutdown() callbacks instead?

I think we can use irq_request_resources then
https://lkml.org/lkml/2014/3/12/307

Sounds good, I'll modify the patch to move it here before posting a v2 of this series. Note v2 likely won't happen till this weekend, -ENOTIME.

Quoted 1 line

We could even merge the gpio_to_irq code into it.

Quoted 7 lines

Quoted 2 lines

Erm, no we need that as a separate function for the gpio_chip's to_irq callback.

Linus sent a patch stating otherwise a few weeks ago, and was suggesting moving it to irq_startup.

https://lkml.org/lkml/2014/5/9/50

That is not going to work, that patch uses gpiochip_irqchip_add, which in turn uses gpiochip_to_irq as to_irq handler, which assumes that gpio offset == irq offset, which is not true for sunxi-pinctrl.

Specifically gpio_chio_to_irq does:

static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset)

{
        return irq_find_mapping(chip->irqdomain, offset);
}

Where as the sunxi code does (simplified):

static int sunxi_pinctrl_gpio_to_irq(struct gpio_chip *chip, unsigned offset)

{
	struct sunxi_desc_function *desc = sunxi_pinctrl_desc_find_function_by_pin(pctl, offset, "irq");
	return irq_find_mapping(pctl->domain, desc->irqnum);\
}

For level triggered gpio interrupts we need to use handle_fasteoi_irq, like we do with the irq-sunxi-nmi driver. This is necessary to give threaded interrupt handlers a chance to actuall clear the source of the interrupt (which may involve sleeping waitnig for i2c / spi / mmc transfers), before acknowledging the interrupt.

Signed-off-by Hans de Goede <[email protected]>

Quoted 7 lines · Hans de Goede

For level triggered gpio interrupts we need to use handle_fasteoi_irq, like we do with the irq-sunxi-nmi driver. This is necessary to give threaded interrupt handlers a chance to actuall clear the source of the interrupt (which may involve sleeping waitnig for i2c / spi / mmc transfers), before acknowledging the interrupt.

Signed-off-by Hans de Goede <[email protected]>

I haven't seen any feedback on the pin controller patches in this series from Maxime, I might have missed it due to being tied to some other patch in the series.

Anyway, this needs to be rebased and reposted anyway, I just merged some base patches from Maxime for external interrupts that will go in after the merge window, I need some help to figure out what and how to merge here.

It has taken me a long long time to get the OOB interrupt working on the AP6210 sdio wifi/bt module found on various Allwinner A20 boards. In the end I found these magic register pokes in the cubietruck kernel tree:
https://github.com/cubieboard2/linux-sunxi/commit/7f08ba395617d17e7a711507503d89a50406fe7a

I'm not entirely sure if this specific to the AP6210 module, or if this should be done for all BCM43362 sdio devices.

Signed-off-by Hans de Goede<[email protected]>

---
  drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 18 ++++++++++++++++++
  1 file changed, 18 insertions(+)

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +18 -0

index 0fc707c..2369a0f 100644
@@ -39,7 +39,9 @@
  #include<brcm_hw_ids.h>
  #include<brcmu_utils.h>
  #include<brcmu_wifi.h>
+#include<chipcommon.h>
  #include<soc.h>
+#include "chip.h"
  #include "dhd_bus.h"
  #include "dhd_dbg.h"
  #include "sdio_host.h"

@@ -119,6 +121,7 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
  {
  	int ret = 0;
  	u8 data;
+	u32 addr, gpiocontrol;
  	unsigned long flags;
 
  	if ((sdiodev->pdata)&&  (sdiodev->pdata->oob_irq_supported)) {

@@ -148,6 +151,21 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
 
  		sdio_claim_host(sdiodev->func[1]);
 
+		if (sdiodev->bus_if->chip == BCM43362_CHIP_ID) {
+			addr = CORE_CC_REG(SI_ENUM_BASE, gpiocontrol);
+			gpiocontrol = brcmf_sdiod_regrl(sdiodev, addr,&ret);
+			gpiocontrol |= 0x2;
+			brcmf_sdiod_regwl(sdiodev, addr, gpiocontrol,&ret);
+
+			/* SPROM_ADDR_HIGH ? perhaps the defines name is off */
+			brcmf_sdiod_regwb(sdiodev, SBSDIO_SPROM_ADDR_HIGH, 0xf,
+					&ret);
+			brcmf_sdiod_regwb(sdiodev, SBSDIO_CHIP_CTRL_DATA, 0,
+					&ret);
+			brcmf_sdiod_regwb(sdiodev, SBSDIO_CHIP_CTRL_EN, 0x2,
+					&ret);
+		}
+
  		/* must configure SDIO_CCCR_IENx to enable irq */
  		data = brcmf_sdiod_regrb(sdiodev, SDIO_CCCR_IENx,&ret);
  		data |= 1<<  SDIO_FUNC_1 | 1<<  SDIO_FUNC_2 | 1;

It has taken me a long long time to get the OOB interrupt working on the AP6210 sdio wifi/bt module found on various Allwinner A20 boards. In the end I found these magic register pokes in the cubietruck kernel tree:
https://github.com/cubieboard2/linux-sunxi/commit/7f08ba395617d17e7a711507503d89a50406fe7a

I'm not entirely sure if this specific to the AP6210 module, or if this should be done for all BCM43362 sdio devices.

Signed-off-by Hans de Goede<[email protected]>

---
  drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 18 ++++++++++++++++++
  1 file changed, 18 insertions(+)

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +3 -0

index 0fc707c..2369a0f 100644
@@ -39,7 +39,9 @@
  #include<brcm_hw_ids.h>
  #include<brcmu_utils.h>
  #include<brcmu_wifi.h>
+#include<chipcommon.h>
  #include<soc.h>
+#include "chip.h"
  #include "dhd_bus.h"
  #include "dhd_dbg.h"
  #include "sdio_host.h"

@@ -119,6 +121,7 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
  {
  	int ret = 0;
  	u8 data;
+	u32 addr, gpiocontrol;
  	unsigned long flags;
 
  	if ((sdiodev->pdata)&&  (sdiodev->pdata->oob_irq_supported)) {

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +8 -0

+			brcmf_sdiod_regwb(sdiodev, SBSDIO_SPROM_ADDR_HIGH, 0xf,
+					&ret);
+			brcmf_sdiod_regwb(sdiodev, SBSDIO_CHIP_CTRL_DATA, 0,
+					&ret);
+			brcmf_sdiod_regwb(sdiodev, SBSDIO_CHIP_CTRL_EN, 0x2,
+					&ret);
+		}
+
  		/* must configure SDIO_CCCR_IENx to enable irq */
  		data = brcmf_sdiod_regrb(sdiodev, SDIO_CCCR_IENx,&ret);
  		data |= 1<<  SDIO_FUNC_1 | 1<<  SDIO_FUNC_2 | 1;

Quoted 7 lines · Hans de Goede

It has taken me a long long time to get the OOB interrupt working on the AP6210 sdio wifi/bt module found on various Allwinner A20 boards. In the end I found these magic register pokes in the cubietruck kernel tree:
https://github.com/cubieboard2/linux-sunxi/commit/7f08ba395617d17e7a711507503d89a50406fe7a

I'm not entirely sure if this specific to the AP6210 module, or if this should be done for all BCM43362 sdio devices.

Let keep it as this for now. I added some remarks below.

Regards,
Arend

Quoted 28 lines

Signed-off-by Hans de Goede<[email protected]>

---
  drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 18 ++++++++++++++++++
  1 file changed, 18 insertions(+)

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +3 -0

index 0fc707c..2369a0f 100644
@@ -39,7 +39,9 @@
  #include<brcm_hw_ids.h>
  #include<brcmu_utils.h>
  #include<brcmu_wifi.h>
+#include<chipcommon.h>
  #include<soc.h>
+#include "chip.h"
  #include "dhd_bus.h"
  #include "dhd_dbg.h"
  #include "sdio_host.h"

@@ -119,6 +121,7 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
  {
      int ret = 0;
      u8 data;
+    u32 addr, gpiocontrol;
      unsigned long flags;
 
      if ((sdiodev->pdata)&&  (sdiodev->pdata->oob_irq_supported)) {

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev) · line 154

Quoted 4 lines

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +1 -0

@@ -148,6 +151,21 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
 
          sdio_claim_host(sdiodev->func[1]);
 
+        if (sdiodev->bus_if->chip == BCM43362_CHIP_ID) {

+ /* assign GPIO to SDIO core */

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev) · line 160

Quoted 6 lines

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +6 -0

+            addr = CORE_CC_REG(SI_ENUM_BASE, gpiocontrol);
+            gpiocontrol = brcmf_sdiod_regrl(sdiodev, addr,&ret);
+            gpiocontrol |= 0x2;
+            brcmf_sdiod_regwl(sdiodev, addr, gpiocontrol,&ret);
+
+            /* SPROM_ADDR_HIGH ? perhaps the defines name is off */

These names are all off. These should be:
SBSDIO_SPROM_ADDR_HIGH = SBSDIO_GPIO_SELECT
SBSDIO_CHIP_CTRL_DATA = SBSDIO_GPIO_OUT
SBSDIO_CHIP_CTRL_EN = SBSDIO_GPIO_EN

Hi,

Quoted 58 lines · Arend van Spriel

Quoted 8 lines · Hans de Goede

It has taken me a long long time to get the OOB interrupt working on the AP6210 sdio wifi/bt module found on various Allwinner A20 boards. In the end I found these magic register pokes in the cubietruck kernel tree:
https://github.com/cubieboard2/linux-sunxi/commit/7f08ba395617d17e7a711507503d89a50406fe7a

I'm not entirely sure if this specific to the AP6210 module, or if this should be done for all BCM43362 sdio devices.

Let keep it as this for now. I added some remarks below.

Regards,
Arend

Quoted 30 lines

Signed-off-by Hans de Goede<[email protected]>

---
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 18 ++++++++++++++++++

1 file changed, 18 insertions(+)

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +3 -0

index 0fc707c..2369a0f 100644
@@ -39,7 +39,9 @@
 #include<brcm_hw_ids.h>
 #include<brcmu_utils.h>
 #include<brcmu_wifi.h>
+#include<chipcommon.h>
 #include<soc.h>
+#include "chip.h"
 #include "dhd_bus.h"
 #include "dhd_dbg.h"
 #include "sdio_host.h"

@@ -119,6 +121,7 @@ int brcmf_sdiod_intr_register(struct
 brcmf_sdio_dev *sdiodev)
 {
 int ret = 0;
 u8 data;
+ u32 addr, gpiocontrol;
 unsigned long flags;
 

if ((sdiodev->pdata)&& (sdiodev->pdata->oob_irq_supported)) {

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c int brcmf_sdiod_intr_register(struct · line 155

Quoted 5 lines

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +1 -0

@@ -148,6 +151,21 @@ int brcmf_sdiod_intr_register(struct
 brcmf_sdio_dev *sdiodev)
 
 sdio_claim_host(sdiodev->func[1]);
 
+ if (sdiodev->bus_if->chip == BCM43362_CHIP_ID) {

+ /* assign GPIO to SDIO core */

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev) · line 160

Quoted 6 lines

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +6 -0

+ addr = CORE_CC_REG(SI_ENUM_BASE, gpiocontrol);
+ gpiocontrol = brcmf_sdiod_regrl(sdiodev, addr,&ret);
+ gpiocontrol |= 0x2;
+ brcmf_sdiod_regwl(sdiodev, addr, gpiocontrol,&ret);
+
+ /* SPROM_ADDR_HIGH ? perhaps the defines name is off */

Quoted 4 lines

These names are all off. These should be:
SBSDIO_SPROM_ADDR_HIGH = SBSDIO_GPIO_SELECT
SBSDIO_CHIP_CTRL_DATA = SBSDIO_GPIO_OUT
SBSDIO_CHIP_CTRL_EN = SBSDIO_GPIO_EN

Ok, so I guess I should do a pre-patch adding defines for these, should these replace the existing defines for these addresses in:
drivers/net/wireless/brcm80211/brcmfmac/sdio_host.h

Or should the pre-patch add defines with the new names and keep the old ones ?

Hi All,

Here is a patch series adding support for oob irqs for the ap6210 sdio wifi modules found on various Allwinner A20 boards.

A lot of it is ground work which should be useful for adding oob irq support to other sdio wifi models too.

The first 5 patches are sunxi pinctrl patches and should go upstream through the pinctrl tree.

Patch 6 adds generic support for having per sdio function child nodes in devicetree and should go upstream through the mmc tree.

Patch 7 and 8 add oob irq support to the brcmfmac driver. Patch 9 pokes some magic bits needed to enable the oob irq, this was taken from an android code dump and good do with a good review from someone from Broadcom, Arend ?

Patch 10 and 11 add the necessary dts bits to actually enable the sdio irq and should go upstream through Maxime's dt tree.

Please review / merge.

Thanks & Regards,

Hans

Quoted 11 lines · Hans de Goede

Hi All,

Here is a patch series adding support for oob irqs for the ap6210 sdio wifi modules found on various Allwinner A20 boards.

A lot of it is ground work which should be useful for adding oob irq support to other sdio wifi models too.

The first 5 patches are sunxi pinctrl patches and should go upstream through the pinctrl tree.

Quoted 6 lines

Patch 6 adds generic support for having per sdio function child nodes in devicetree and should go upstream through the mmc tree.

Patch 7 and 8 add oob irq support to the brcmfmac driver. Patch 9 pokes some magic bits needed to enable the oob irq, this was taken from an android code dump and good do with a good review from someone from Broadcom, Arend ?

If Arend approves, I'm fine with these going through another appropriate tree.

Quoted 9 lines

Patch 10 and 11 add the necessary dts bits to actually enable the sdio irq and should go upstream through Maxime's dt tree.

Please review / merge.

Thanks& Regards,

Hans