| Message ID | 20210509224226.348127-1-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | brcmfmac: Obtain reset GPIO | expand |
+ 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
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
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
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
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
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
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 --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;
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(+)