Re: [PATCH] brcmfmac: add support for external 32khz clock
On Wed, Nov 08, 2017 at 10:43:05PM +1100, Simon Shields wrote: > Hi Arend, > > On Wed, Nov 08, 2017 at 11:38:11AM +0100, Arend van Spriel wrote: > > + Stefan > > > > On 11/7/2017 2:31 PM, Simon Shields wrote: > > > Hi Arend, > > > > > > On Tue, Nov 07, 2017 at 12:09:23PM +0100, Arend van Spriel wrote: > > > > On 11/6/2017 12:27 PM, Simon Shields wrote: > > > > > On Mon, Nov 06, 2017 at 11:59:37AM +0100, Arend van Spriel wrote: > > > > > > On 11/4/2017 2:24 PM, Simon Shields wrote: > > > > > > > Some boards use an external 32khz clock for low-power > > > > > > > mode timing. Make sure the clock is powered on while the chipset > > > > > > > is active. > > > > > > > > > > > > Do you have such a board? With the little documentation I can get > > > > > > my hands > > > > > > on here I wonder whether the clock needs to be enabled before the > > > > > > device is > > > > > > powered. If you have the hardware I would like to check some > > > > > > registers in > > > > > > the device. > > > > > > > > > > > > > > > > Yes. Trats2 (exynos4412-based) has such a setup. The BCM4334 works > > > > > fine > > > > > with this patch and one more that enables the WL_REG_EN pin when > > > > > brcmfmac is probed. > > > > > > > > Ok. So this is exactly the thing I was wondering about. So it makes me > > > > curious how the WL_REG_EN patch looks like. Can you provide that? > > > > > > > > > > Here[0] is a link to the patch in its current state. Obviously, it's not > > > ready at all for mainlining :-) > > > > > > [0]: > > > https://github.com/fourkbomb/linux/commit/436e59e58b44d856c186fc4767560cecbcbc0c59.patch > > > > Thanks. Indeed doing it in module_init of brcmfmac is not going to fly. > > Actually the MMC stack has a mechanism to power the SDIO device. This can be > > configured through the device tree [1]. I just checked and it actually > > includes specifying the external clock as well. > > > > It looks like mmc-pwrseq-simple would handle the clock case, but it > doesn't handle the regulator properly: it sets the GPIO to high before > powering on, then lowers it immediately after powering it on. > > I guess a simpler approach here would be to extend pwrseq-simple to behave > as we need it to. I missed something really obvious... All that's needed is to invert the GPIO, like this[0]. Thanks for the pointer :) Cheers, Simon [0]: https://patchwork.kernel.org/patch/10048501/
Re: [PATCH] brcmfmac: add support for external 32khz clock
Hi Arend, On Wed, Nov 08, 2017 at 11:38:11AM +0100, Arend van Spriel wrote: > + Stefan > > On 11/7/2017 2:31 PM, Simon Shields wrote: > > Hi Arend, > > > > On Tue, Nov 07, 2017 at 12:09:23PM +0100, Arend van Spriel wrote: > > > On 11/6/2017 12:27 PM, Simon Shields wrote: > > > > On Mon, Nov 06, 2017 at 11:59:37AM +0100, Arend van Spriel wrote: > > > > > On 11/4/2017 2:24 PM, Simon Shields wrote: > > > > > > Some boards use an external 32khz clock for low-power > > > > > > mode timing. Make sure the clock is powered on while the chipset > > > > > > is active. > > > > > > > > > > Do you have such a board? With the little documentation I can get my > > > > > hands > > > > > on here I wonder whether the clock needs to be enabled before the > > > > > device is > > > > > powered. If you have the hardware I would like to check some > > > > > registers in > > > > > the device. > > > > > > > > > > > > > Yes. Trats2 (exynos4412-based) has such a setup. The BCM4334 works fine > > > > with this patch and one more that enables the WL_REG_EN pin when > > > > brcmfmac is probed. > > > > > > Ok. So this is exactly the thing I was wondering about. So it makes me > > > curious how the WL_REG_EN patch looks like. Can you provide that? > > > > > > > Here[0] is a link to the patch in its current state. Obviously, it's not > > ready at all for mainlining :-) > > > > [0]: > > https://github.com/fourkbomb/linux/commit/436e59e58b44d856c186fc4767560cecbcbc0c59.patch > > Thanks. Indeed doing it in module_init of brcmfmac is not going to fly. > Actually the MMC stack has a mechanism to power the SDIO device. This can be > configured through the device tree [1]. I just checked and it actually > includes specifying the external clock as well. > It looks like mmc-pwrseq-simple would handle the clock case, but it doesn't handle the regulator properly: it sets the GPIO to high before powering on, then lowers it immediately after powering it on. I guess a simpler approach here would be to extend pwrseq-simple to behave as we need it to. > Regards, > Arend > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt Cheers, Simon
Re: [PATCH] brcmfmac: add support for external 32khz clock
On Mon, Nov 06, 2017 at 11:59:37AM +0100, Arend van Spriel wrote: > On 11/4/2017 2:24 PM, Simon Shields wrote: > > Some boards use an external 32khz clock for low-power > > mode timing. Make sure the clock is powered on while the chipset > > is active. > > Do you have such a board? With the little documentation I can get my hands > on here I wonder whether the clock needs to be enabled before the device is > powered. If you have the hardware I would like to check some registers in > the device. > Yes. Trats2 (exynos4412-based) has such a setup. The BCM4334 works fine with this patch and one more that enables the WL_REG_EN pin when brcmfmac is probed. Without this patch (and only enabling WL_REG_EN), the chip is detected but attempting to initialise it fails with a bunch of timeouts. > Regards, > Arend Cheers, Simon
Re: [PATCH] brcmfmac: add support for external 32khz clock
Some boards use an external 32khz clock for low-power
mode timing. Make sure the clock is powered on while the chipset
is active.
Signed-off-by: Simon Shields <si...@lineageos.org>
---
.../devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 2 ++
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h | 2 ++
drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 5 +
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c| 10 ++
4 files changed, 19 insertions(+)
diff --git
a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
index b2bd4704f859..37add5e29272 100644
--- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
+++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
@@ -17,6 +17,8 @@ Optional properties:
When not specified the device will use in-band SDIO interrupts.
- interrupt-names : name of the out-of-band interrupt, which must be set
to "host-wake".
+ - clocks : external 32khz clock
+ - clock-names : name of the external 32khz clock, must be "32khz"
Example:
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
index a62f8e70b320..2e7fabae81d3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
@@ -51,6 +51,7 @@ 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
+ * @clk: External 32khz clock, if present.
* @bus: Bus specific platform data. Only SDIO at the mmoment.
*/
struct brcmf_mp_device {
@@ -60,6 +61,7 @@ struct brcmf_mp_device {
boolroamoff;
boolignore_probe_fail;
struct brcmfmac_pd_cc *country_codes;
+ struct clk *clk;
union {
struct brcmfmac_sdio_pd sdio;
} bus;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
index aee6e5937c41..46f42a2c3d2b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
@@ -13,6 +13,7 @@
* OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
* CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
+#include
#include
#include
#include
@@ -39,6 +40,10 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type
bus_type,
if (of_property_read_u32(np, "brcm,drive-strength", ) == 0)
sdio->drive_strength = val;
+ settings->clk = devm_clk_get(dev, "32khz");
+ if (IS_ERR(settings->clk))
+ settings->clk = NULL;
+
/* make sure there are interrupts defined in the node */
if (!of_find_property(np, "interrupts", NULL))
return;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 613caca7dc02..f4ceca6dbe74 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -14,6 +14,7 @@
* CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
+#include
#include
#include
#include
@@ -3853,6 +3854,11 @@ brcmf_sdio_probe_attach(struct brcmf_sdio *bus)
brcmf_err("Failed to get device parameters\n");
goto fail;
}
+
+ /* enable external 32khz clock, if present */
+ if (sdiodev->settings->clk)
+ clk_prepare_enable(sdiodev->settings->clk);
+
/* platform specific configuration:
* alignments must be at least 4 bytes for ADMA
*/
@@ -4270,6 +4276,10 @@ void brcmf_sdio_remove(struct brcmf_sdio *bus)
}
brcmf_chip_detach(bus->ci);
}
+
+ if (bus->sdiodev->settings->clk)
+ clk_disable_unprepare(bus->sdiodev->settings->clk);
+
if (bus->sdiodev->settings)
brcmf_release_module_param(bus->sdiodev->settings);
--
2.15.0
[PATCH] brcmfmac: add support for external 32khz clock
Some boards use an external 32khz clock for low-power
mode timing. Make sure the clock is powered on while the chipset
is active.
Signed-off-by: Simon Shields <si...@lineageos.org>
---
.../devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 2 ++
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h | 2 ++
drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 5 +
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c| 10 ++
4 files changed, 19 insertions(+)
diff --git
a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
index b2bd4704f859..37add5e29272 100644
--- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
+++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
@@ -17,6 +17,8 @@ Optional properties:
When not specified the device will use in-band SDIO interrupts.
- interrupt-names : name of the out-of-band interrupt, which must be set
to "host-wake".
+ - clocks : external 32khz clock
+ - clock-names : name of the external 32khz clock, must be "32khz"
Example:
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
index a62f8e70b320..2e7fabae81d3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
@@ -51,6 +51,7 @@ 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
+ * @clk: External 32khz clock, if present.
* @bus: Bus specific platform data. Only SDIO at the mmoment.
*/
struct brcmf_mp_device {
@@ -60,6 +61,7 @@ struct brcmf_mp_device {
boolroamoff;
boolignore_probe_fail;
struct brcmfmac_pd_cc *country_codes;
+ struct clk *clk;
union {
struct brcmfmac_sdio_pd sdio;
} bus;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
index aee6e5937c41..46f42a2c3d2b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
@@ -13,6 +13,7 @@
* OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
* CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
+#include
#include
#include
#include
@@ -39,6 +40,10 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type
bus_type,
if (of_property_read_u32(np, "brcm,drive-strength", ) == 0)
sdio->drive_strength = val;
+ settings->clk = devm_clk_get(dev, "32khz");
+ if (IS_ERR(settings->clk))
+ settings->clk = NULL;
+
/* make sure there are interrupts defined in the node */
if (!of_find_property(np, "interrupts", NULL))
return;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 613caca7dc02..f4ceca6dbe74 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -14,6 +14,7 @@
* CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
+#include
#include
#include
#include
@@ -3853,6 +3854,11 @@ brcmf_sdio_probe_attach(struct brcmf_sdio *bus)
brcmf_err("Failed to get device parameters\n");
goto fail;
}
+
+ /* enable external 32khz clock, if present */
+ if (sdiodev->settings->clk)
+ clk_prepare_enable(sdiodev->settings->clk);
+
/* platform specific configuration:
* alignments must be at least 4 bytes for ADMA
*/
@@ -4270,6 +4276,10 @@ void brcmf_sdio_remove(struct brcmf_sdio *bus)
}
brcmf_chip_detach(bus->ci);
}
+
+ if (bus->sdiodev->settings->clk)
+ clk_disable_unprepare(bus->sdiodev->settings->clk);
+
if (bus->sdiodev->settings)
brcmf_release_module_param(bus->sdiodev->settings);
--
2.15.0
