diff mbox series

brcmfmac: Obtain reset GPIO

Message ID 20210509224226.348127-1-linus.walleij@linaro.org
State New
Headers show
Series brcmfmac: Obtain reset GPIO | expand

Commit Message

Linus Walleij May 9, 2021, 10:42 p.m. UTC 
This grabs the reset GPIO and holds it de-asserted, if available.
Asserting this signal will make the SDIO card re-enumerate.

Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 .../net/wireless/broadcom/brcm80211/brcmfmac/common.c  | 10 ++++++++++
 .../net/wireless/broadcom/brcm80211/brcmfmac/common.h  |  4 ++++
 2 files changed, 14 insertions(+)

-- 
2.30.2

Comments

Arend van Spriel May 10, 2021, 7:37 a.m. UTC | #1 
+ Uffe

On 5/10/2021 12:42 AM, Linus Walleij wrote:
> This grabs the reset GPIO and holds it de-asserted, if available.

> Asserting this signal will make the SDIO card re-enumerate.


looks ok to me, but could this also be done through SDIO power sequence 
stuff?

Regards,
Arend

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.
Ulf Hansson May 10, 2021, 9:14 a.m. UTC | #2 
On Mon, 10 May 2021 at 09:37, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
>

> + Uffe

>

> On 5/10/2021 12:42 AM, Linus Walleij wrote:

> > This grabs the reset GPIO and holds it de-asserted, if available.

> > Asserting this signal will make the SDIO card re-enumerate.

>

> looks ok to me, but could this also be done through SDIO power sequence

> stuff?


Yes, it certainly looks like that to me. It should be the mmc
host/core that manages the power on/off thingy for the SDIO card.

[...]

Kind regards
Uffe
Arend van Spriel May 10, 2021, 9:27 a.m. UTC | #3 
On 5/10/2021 11:14 AM, Ulf Hansson wrote:
> On Mon, 10 May 2021 at 09:37, Arend Van Spriel

> <arend.vanspriel@broadcom.com> wrote:

>>

>> + Uffe

>>

>> On 5/10/2021 12:42 AM, Linus Walleij wrote:

>>> This grabs the reset GPIO and holds it de-asserted, if available.

>>> Asserting this signal will make the SDIO card re-enumerate.

>>

>> looks ok to me, but could this also be done through SDIO power sequence

>> stuff?

> 

> Yes, it certainly looks like that to me. It should be the mmc

> host/core that manages the power on/off thingy for the SDIO card.


Thanks, Uffe

This is not directly power on/off, but a separate "reset" GPIO. However, 
checking in pwrseq_simple.c I see:

struct mmc_pwrseq_simple {
	struct mmc_pwrseq pwrseq;
	bool clk_enabled;
	u32 post_power_on_delay_ms;
	u32 power_off_delay_us;
	struct clk *ext_clk;
	struct gpio_descs *reset_gpios;
};

So the term 'reset_gpios' is also used in pwrseq context.

Regards,
Arend
Ulf Hansson May 11, 2021, 8:48 a.m. UTC | #4 
On Mon, 10 May 2021 at 11:27, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
>

>

>

> On 5/10/2021 11:14 AM, Ulf Hansson wrote:

> > On Mon, 10 May 2021 at 09:37, Arend Van Spriel

> > <arend.vanspriel@broadcom.com> wrote:

> >>

> >> + Uffe

> >>

> >> On 5/10/2021 12:42 AM, Linus Walleij wrote:

> >>> This grabs the reset GPIO and holds it de-asserted, if available.

> >>> Asserting this signal will make the SDIO card re-enumerate.

> >>

> >> looks ok to me, but could this also be done through SDIO power sequence

> >> stuff?

> >

> > Yes, it certainly looks like that to me. It should be the mmc

> > host/core that manages the power on/off thingy for the SDIO card.

>

> Thanks, Uffe

>

> This is not directly power on/off, but a separate "reset" GPIO. However,

> checking in pwrseq_simple.c I see:

>

> struct mmc_pwrseq_simple {

>         struct mmc_pwrseq pwrseq;

>         bool clk_enabled;

>         u32 post_power_on_delay_ms;

>         u32 power_off_delay_us;

>         struct clk *ext_clk;

>         struct gpio_descs *reset_gpios;

> };

>

> So the term 'reset_gpios' is also used in pwrseq context.


I think this boils down to that to allow the mmc core to detect and
initialize the SDIO card, it needs to manage potential reset pins as
well.

In cases when the SDIO func driver may need to execute a reset, the
mmc core provides two APIs, mmc_hw|sw_reset().

Does this make sense to you?

Kind regards
Uffe
Arend van Spriel May 12, 2021, 7:42 a.m. UTC | #5 
On 5/11/2021 10:48 AM, Ulf Hansson wrote:
> On Mon, 10 May 2021 at 11:27, Arend van Spriel

> <arend.vanspriel@broadcom.com> wrote:

>>

>>

>>

>> On 5/10/2021 11:14 AM, Ulf Hansson wrote:

>>> On Mon, 10 May 2021 at 09:37, Arend Van Spriel

>>> <arend.vanspriel@broadcom.com> wrote:

>>>>

>>>> + Uffe

>>>>

>>>> On 5/10/2021 12:42 AM, Linus Walleij wrote:

>>>>> This grabs the reset GPIO and holds it de-asserted, if available.

>>>>> Asserting this signal will make the SDIO card re-enumerate.

>>>>

>>>> looks ok to me, but could this also be done through SDIO power sequence

>>>> stuff?

>>>

>>> Yes, it certainly looks like that to me. It should be the mmc

>>> host/core that manages the power on/off thingy for the SDIO card.

>>

>> Thanks, Uffe

>>

>> This is not directly power on/off, but a separate "reset" GPIO. However,

>> checking in pwrseq_simple.c I see:

>>

>> struct mmc_pwrseq_simple {

>>          struct mmc_pwrseq pwrseq;

>>          bool clk_enabled;

>>          u32 post_power_on_delay_ms;

>>          u32 power_off_delay_us;

>>          struct clk *ext_clk;

>>          struct gpio_descs *reset_gpios;

>> };

>>

>> So the term 'reset_gpios' is also used in pwrseq context.

> 

> I think this boils down to that to allow the mmc core to detect and

> initialize the SDIO card, it needs to manage potential reset pins as

> well.

> 

> In cases when the SDIO func driver may need to execute a reset, the

> mmc core provides two APIs, mmc_hw|sw_reset().

> 

> Does this make sense to you?


It does to me.

Regards,
Arend
Linus Walleij May 19, 2021, 11:41 p.m. UTC | #6 
On Tue, May 11, 2021 at 10:48 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:

> I think this boils down to that to allow the mmc core to detect and

> initialize the SDIO card, it needs to manage potential reset pins as

> well.

>

> In cases when the SDIO func driver may need to execute a reset, the

> mmc core provides two APIs, mmc_hw|sw_reset().

>

> Does this make sense to you?


I think so, in this case (Samsung Janice on Ux500) the boot loader
desserts reset so we are fine, but I think it should be utilized properly
somehow, the vendor went to lots of trouble to put this reset line
there physically.

What is my next step? :D

Yours,
Linus Walleij
Ulf Hansson May 21, 2021, 11:44 a.m. UTC | #7 
On Thu, 20 May 2021 at 01:41, Linus Walleij <linus.walleij@linaro.org> wrote:
>

> On Tue, May 11, 2021 at 10:48 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:

>

> > I think this boils down to that to allow the mmc core to detect and

> > initialize the SDIO card, it needs to manage potential reset pins as

> > well.

> >

> > In cases when the SDIO func driver may need to execute a reset, the

> > mmc core provides two APIs, mmc_hw|sw_reset().

> >

> > Does this make sense to you?

>

> I think so, in this case (Samsung Janice on Ux500) the boot loader

> desserts reset so we are fine, but I think it should be utilized properly

> somehow, the vendor went to lots of trouble to put this reset line

> there physically.

>

> What is my next step? :D


Have a look at the mmc-pwrseq simple DT bindings.
Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.yaml

If things are as simple as I hope/think, all you should need to do is
to add a mmc-pwrseq node where you specify the GPIO reset line. Then
from the mmc controller node, just add a phandle to the mmc-pwrseq
node and things should just work. :-)

git grep mmc-pwrseq should give you a bunch of references of existing users.

Kind regards
Uffe
diff mbox series

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index e3758bd86acf..40e18ebfe1ea 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -446,6 +446,16 @@  struct brcmf_mp_device *brcmf_get_module_param(struct device *dev,
 		brcmf_dmi_probe(settings, chip, chiprev);
 		brcmf_of_probe(dev, bus_type, settings);
 	}
+	/* Fetch WL_RESET GPIO and de-assert it, if available */
+	settings->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (settings->reset) {
+		/*
+		 * If we found a reset GPIO, we may have just de-asserted it
+		 * so wait some 8 ms for PLLs to lock, se figure 32, WLAN
+		 * boot-up sequence in the manual.
+		 */
+		usleep_range(8000, 10000);
+	}
 	return settings;
 }
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
index 8b5f49997c8b..4209e71ebcdd 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
@@ -7,6 +7,7 @@ 
 
 #include <linux/platform_device.h>
 #include <linux/platform_data/brcmfmac.h>
+#include <linux/gpio/consumer.h>
 #include "fwil_types.h"
 
 #define BRCMF_FW_ALTPATH_LEN			256
@@ -39,6 +40,8 @@  extern struct brcmf_mp_global_t brcmf_mp_global;
  * @roamoff: Firmware roaming off?
  * @ignore_probe_fail: Ignore probe failure.
  * @country_codes: If available, pointer to struct for translating country codes
+ * @board_type: String with the board type name
+ * @reset: GPIO descriptor for the RESET line
  * @bus: Bus specific platform data. Only SDIO at the mmoment.
  */
 struct brcmf_mp_device {
@@ -50,6 +53,7 @@  struct brcmf_mp_device {
 	bool		ignore_probe_fail;
 	struct brcmfmac_pd_cc *country_codes;
 	const char	*board_type;
+	struct gpio_desc *reset;
 	union {
 		struct brcmfmac_sdio_pd sdio;
 	} bus;