[PATCH v2] ARM: dts: ux500: Fix BT+WLAN on Samsung phones

[PATCH v2] ARM: dts: ux500: Fix BT+WLAN on Samsung phones

[Linus Walleij]

Some of the Samsung phones had set GPIO 215 as voltage
regulator GPIO, while in fact this is always on GPIO 222.

Using GPIO 215 meant that the Bluetooth would not come
on properly since both WLAN and Bluetooth use GPIO 222
to power up. The Bluetooth part refers to this GPIO
line as "shutdown-gpios" which is kind of true, but
fails to mention that it will also shut down the WLAN
when it is disabled, which was causing bugs.

So we change the regulator for WLAN and Bluetooth to
GPIO 222 and edit some names and comments to make it
clear that this is the common regulator for both
functions. Both WLAN and Bluetooth now use this
regulator for powering up, and the regulator
framework will reference count it and make sure
that one part does not power down the other.

However GPIO 215 still needs to be driven high
(active low) since otherwise the WLAN part of the
chip is being held in RESET, and that will make the
SDIO WLAN not enumerate.

To solve this, make the pin config entry for the power
regulator also drive GPIO 215 high, but leave it to
the driver to later on acquire and control this GPIO
line. This is a bit of a hack and marked as such, but
it works just fine: both SDIO WLAN and Bluetooth come
up just fine on all the phones, with the right firmware
provided.

Cc: Stephan Gerhold <stephan@gerhold.net>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Change compatible strings on the WLAN chips to be
  specific-to-generic indicating the exact model.
---
 .../arm/boot/dts/ste-ux500-samsung-golden.dts | 101 ++++++++++++------
 .../arm/boot/dts/ste-ux500-samsung-janice.dts |  34 +++---
 .../arm/boot/dts/ste-ux500-samsung-skomer.dts |  74 ++++++++++---
 3 files changed, 142 insertions(+), 67 deletions(-)

diff --git a/arch/arm/boot/dts/ste-ux500-samsung-golden.dts b/arch/arm/boot/dts/ste-ux500-samsung-golden.dts
index 0d43ee6583cf..af0fefca162b 100644
--- a/arch/arm/boot/dts/ste-ux500-samsung-golden.dts
+++ b/arch/arm/boot/dts/ste-ux500-samsung-golden.dts
@@ -71,6 +71,28 @@ magnetometer@c {
 		};
 	};
 
+	/*
+	 * This regulator is a GPIO line that drives the Broadcom WLAN
+	 * and Bluetooth line BT_VREG_EN high and enables the internal
+	 * regulators inside the chip.
+	 *
+	 * The voltage specified here is only used to determine the OCR mask,
+	 * the for the SDIO connector, the chip is actually connected
+	 * directly to VBAT.
+	 */
+	wl_bt_reg: regulator-gpio-wlan-bt {
+		compatible = "regulator-fixed";
+		regulator-name = "BT_VREG_EN";
+		regulator-min-microvolt = <3000000>;
+		regulator-max-microvolt = <3000000>;
+		startup-delay-us = <100000>;
+		/* GPIO222 (BT_VREG_EN) */
+		gpio = <&gpio6 30 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		pinctrl-names = "default";
+		pinctrl-0 = <&wlan_bt_ldo_en_default>;
+	};
+
 	soc {
 		/* External Micro SD card slot */
 		mmc@80126000 {
@@ -111,8 +133,6 @@ mmc@80118000 {
 			non-removable;
 			cap-sd-highspeed;
 
-			vmmc-supply = <&wl_reg_on>;
-
 			pinctrl-names = "default", "sleep";
 			pinctrl-0 = <&mc1_a_2_default>;
 			pinctrl-1 = <&mc1_a_2_sleep>;
@@ -120,10 +140,22 @@ mmc@80118000 {
 			#address-cells = <1>;
 			#size-cells = <0>;
 
+			/*
+			 * GPIO-controlled voltage enablement: this drives
+			 * the BT_VREG_EN line high when we use this device.
+			 * Represented as regulator to fill OCR mask and to
+			 * be usable in parallel with the Bluetooth chip.
+			 */
+			vmmc-supply = <&wl_bt_reg>;
+
 			wifi@1 {
-				compatible = "brcm,bcm4329-fmac";
+				/* BRCM4334 actually */
+				compatible = "brcm,bcm4334-fmac", "brcm,bcm4329-fmac";
 				reg = <1>;
 
+				/* GPIO215  WLAN_RST_N */
+				reset-gpios = <&gpio6 23 GPIO_ACTIVE_LOW>;
+
 				/* GPIO216 (WLAN_HOST_WAKE) */
 				interrupt-parent = <&gpio6>;
 				interrupts = <24 IRQ_TYPE_EDGE_FALLING>;
@@ -162,9 +194,17 @@ uart@80120000 {
 			pinctrl-1 = <&u0_a_1_sleep>;
 
 			bluetooth {
+				/* BRCM4334B0 actually */
 				compatible = "brcm,bcm4330-bt";
-				/* GPIO222 (BT_VREG_ON) */
-				shutdown-gpios = <&gpio6 30 GPIO_ACTIVE_HIGH>;
+				/*
+				 * We actually have shutdown-gpios, BT_VREG_ON on GPIO222,
+				 * but since this GPIO is shared with the WLAN chip, we need
+				 * to reference the regulator instead. The regulator
+				 * framework will reference count the GPIO usage and
+				 * make sure we can use the same GPIO for several supplies.
+				 */
+				// shutdown-gpios = <&gpio6 30 GPIO_ACTIVE_HIGH>;
+				vbat-supply = <&wl_bt_reg>;
 				/* GPIO199 (BT_WAKE) */
 				device-wakeup-gpios = <&gpio6 7 GPIO_ACTIVE_HIGH>;
 				/* GPIO97 (BT_HOST_WAKE) */
@@ -447,28 +487,6 @@ sd_level_translator: regulator-sd-level-translator {
 		pinctrl-0 = <&sd_level_translator_default>;
 	};
 
-	/*
-	 * WL_REG_ON takes WLAN out of reset and enables the internal regulators.
-	 * The voltage specified here is only used to determine the OCR mask,
-	 * the BCM chip is actually connected directly to VBAT.
-	 */
-	wl_reg_on: regulator-wl-reg-on {
-		compatible = "regulator-fixed";
-
-		regulator-name = "wl-reg-on";
-		regulator-min-microvolt = <3000000>;
-		regulator-max-microvolt = <3000000>;
-
-		startup-delay-us = <100000>;
-
-		/* GPIO215 (WLAN_EN) */
-		gpio = <&gpio6 23 GPIO_ACTIVE_HIGH>;
-		enable-active-high;
-
-		pinctrl-names = "default";
-		pinctrl-0 = <&wlan_en_default>;
-	};
-
 	/* MIC5366 GPIO-controlled regulator */
 	panel_reg_1v8: regulator-panel-1v8 {
 		compatible = "regulator-fixed";
@@ -641,25 +659,38 @@ golden_cfg1 {
 				ste,config = <&gpio_in_pd>;
 			};
 		};
+	};
 
-		wlan_en_default: wlan_en_default {
+	bluetooth {
+		bluetooth_default: bluetooth_default {
 			golden_cfg1 {
-				pins = "GPIO215_AH13";	/* WLAN_EN */
+				pins = "GPIO199_AH23";	/* BT_WAKE */
 				ste,config = <&gpio_out_lo>;
 			};
+			golden_cfg2 {
+				pins = "GPIO97_D9";	/* BT_HOST_WAKE */
+				ste,config = <&gpio_in_nopull>;
+			};
 		};
 	};
 
-	bluetooth {
-		bluetooth_default: bluetooth_default {
+	/* GPIO that enables the WLAN and Bluetooth internal LDO regulators */
+	wlan-bt-ldo {
+		wlan_bt_ldo_en_default: wlan_bt_ldo_default {
+			/* GPIO222 BT_VREG_ON */
 			golden_cfg1 {
-				pins = "GPIO199_AH23",	/* BT_WAKE */
-				       "GPIO222_AJ9";	/* BT_VREG_ON */
+				pins = "GPIO222_AJ9";
 				ste,config = <&gpio_out_lo>;
 			};
+			/*
+			 * HACK: this is the WL_RESET line that technically has nothing
+			 * to do with the regulator, but in order for the SDIO card
+			 * to enumerate, this must be driven high, so we need to do
+			 * that here.
+			 */
 			golden_cfg2 {
-				pins = "GPIO97_D9";	/* BT_HOST_WAKE */
-				ste,config = <&gpio_in_nopull>;
+				pins = "GPIO215_AH13";	/* WLAN_EN */
+				ste,config = <&gpio_out_hi>;
 			};
 		};
 	};
diff --git a/arch/arm/boot/dts/ste-ux500-samsung-janice.dts b/arch/arm/boot/dts/ste-ux500-samsung-janice.dts
index 7411bfeda285..065a7b33132a 100644
--- a/arch/arm/boot/dts/ste-ux500-samsung-janice.dts
+++ b/arch/arm/boot/dts/ste-ux500-samsung-janice.dts
@@ -135,14 +135,14 @@ lcd_1v8_reg: regulator-gpio-lcd-1v8 {
 
 	/*
 	 * This regulator is a GPIO line that drives the Broadcom WLAN
-	 * line BT_VREG_EN high and enables the internal regulators
-	 * inside the chip.
+	 * and Bluetooth line BT_VREG_EN high and enables the internal
+	 * regulators inside the chip.
 	 *
 	 * The voltage specified here is only used to determine the OCR mask,
 	 * the for the SDIO connector, the chip is actually connected
 	 * directly to VBAT.
 	 */
-	wl_bt_reg: regulator-gpio-wlan {
+	wl_bt_reg: regulator-gpio-wlan-bt {
 		compatible = "regulator-fixed";
 		regulator-name = "BT_VREG_EN";
 		regulator-min-microvolt = <3000000>;
@@ -152,7 +152,7 @@ wl_bt_reg: regulator-gpio-wlan {
 		gpio = <&gpio6 30 GPIO_ACTIVE_HIGH>;
 		enable-active-high;
 		pinctrl-names = "default";
-		pinctrl-0 = <&wlan_ldo_en_default>;
+		pinctrl-0 = <&wlan_bt_ldo_en_default>;
 	};
 
 
@@ -401,15 +401,13 @@ mmc@80118000 {
 			status = "okay";
 
 			wifi@1 {
-				/* Actually BRCM4330 */
-				compatible = "brcm,bcm4329-fmac";
+				compatible = "brcm,bcm4330-fmac", "brcm,bcm4329-fmac";
 				reg = <1>;
 				/* GPIO216 WL_HOST_WAKE */
 				interrupt-parent = <&gpio6>;
 				interrupts = <24 IRQ_TYPE_EDGE_FALLING>;
 				interrupt-names = "host-wake";
 				/* GPIO215  WLAN_RST_N */
-				/* FIXME: kernel does not use this assert/deassert */
 				reset-gpios = <&gpio6 23 GPIO_ACTIVE_LOW>;
 				pinctrl-names = "default";
 				pinctrl-0 = <&wlan_default_mode>;
@@ -439,6 +437,7 @@ uart@80120000 {
 			status = "okay";
 
 			bluetooth {
+				/* BRCM4330B1 actually */
 				compatible = "brcm,bcm4330-bt";
 				/*
 				 * We actually have shutdown-gpios, BT_VREG_EN on GPIO222,
@@ -756,14 +755,24 @@ janice_cfg1 {
 			};
 		};
 	};
-	/* GPIO that enables the WLAN internal LDO regulators */
-	wlan-ldo {
-		wlan_ldo_en_default: wlan_ldo_default {
+	/* GPIO that enables the WLAN and Bluetooth internal LDO regulators */
+	wlan-bt-ldo {
+		wlan_bt_ldo_en_default: wlan_bt_ldo_default {
 			/* GPIO222 BT_VREG_ON */
 			janice_cfg1 {
 				pins = "GPIO222_AJ9";
 				ste,config = <&gpio_out_lo>;
 			};
+			/*
+			 * HACK: this is the WL_RESET line that technically has nothing
+			 * to do with the regulator, but in order for the SDIO card
+			 * to enumerate, this must be driven high, so we need to do
+			 * that here.
+			 */
+			janice_cfg2 {
+				pins = "GPIO215_AH13";	/* RESET_N */
+				ste,config = <&gpio_out_hi>;
+			};
 		};
 	};
 	/* Flash and torch */
@@ -875,11 +884,6 @@ janice_cfg2 {
 	};
 	wlan {
 		wlan_default_mode: wlan_default {
-			/* GPIO215 used for RESET_N */
-			janice_cfg1 {
-				pins = "GPIO215_AH13";
-				ste,config = <&gpio_out_lo>;
-			};
 			/* GPIO216 for WL_HOST_WAKE */
 			janice_cfg2 {
 				pins = "GPIO216_AG12";
diff --git a/arch/arm/boot/dts/ste-ux500-samsung-skomer.dts b/arch/arm/boot/dts/ste-ux500-samsung-skomer.dts
index d28a00757d0b..f73a39f5a696 100644
--- a/arch/arm/boot/dts/ste-ux500-samsung-skomer.dts
+++ b/arch/arm/boot/dts/ste-ux500-samsung-skomer.dts
@@ -52,17 +52,26 @@ ldo_3v3_reg: regulator-gpio-ldo-3v3 {
 		pinctrl-0 = <&emmc_ldo_en_default_mode>;
 	};
 
-	wlan_en: regulator-gpio-wlan-en {
+	/*
+	 * This regulator is a GPIO line that drives the Broadcom WLAN
+	 * and Bluetooth line BT_VREG_EN high and enables the internal
+	 * regulators inside the chip.
+	 *
+	 * The voltage specified here is only used to determine the OCR mask,
+	 * the for the SDIO connector, the chip is actually connected
+	 * directly to VBAT.
+	 */
+	wl_bt_reg: regulator-gpio-wlan-bt {
 		compatible = "regulator-fixed";
-		regulator-name = "wl-reg-on";
+		regulator-name = "BT_VREG_EN";
 		regulator-min-microvolt = <3000000>;
 		regulator-max-microvolt = <3000000>;
-		startup-delay-us = <200000>;
-		/* GPIO215 WLAN_EN */
-		gpio = <&gpio6 23 GPIO_ACTIVE_HIGH>;
+		startup-delay-us = <100000>;
+		/* GPIO222 (BT_VREG_EN) */
+		gpio = <&gpio6 30 GPIO_ACTIVE_HIGH>;
 		enable-active-high;
 		pinctrl-names = "default";
-		pinctrl-0 = <&wlan_en_default_mode>;
+		pinctrl-0 = <&wlan_bt_ldo_en_default>;
 	};
 
 	vibrator {
@@ -202,7 +211,6 @@ mmc@80118000 {
 			bus-width = <4>;
 			non-removable;
 			cap-sd-highspeed;
-			vmmc-supply = <&wlan_en>;
 			pinctrl-names = "default", "sleep";
 			pinctrl-0 = <&mc1_a_2_default>;
 			pinctrl-1 = <&mc1_a_2_sleep>;
@@ -210,9 +218,19 @@ mmc@80118000 {
 			#address-cells = <1>;
 			#size-cells = <0>;
 
+			/*
+			 * GPIO-controlled voltage enablement: this drives
+			 * the BT_VREG_EN line high when we use this device.
+			 * Represented as regulator to fill OCR mask and to
+			 * be usable in parallel with the Bluetooth chip.
+			 */
+			vmmc-supply = <&wl_bt_reg>;
+
 			wifi@1 {
-				compatible = "brcm,bcm4329-fmac";
+				compatible = "brcm,bcm4334-fmac", "brcm,bcm4329-fmac";
 				reg = <1>;
+				/* GPIO215  WLAN_RST_N */
+				reset-gpios = <&gpio6 23 GPIO_ACTIVE_LOW>;
 				/* GPIO216 WL_HOST_WAKE */
 				interrupt-parent = <&gpio6>;
 				interrupts = <24 IRQ_TYPE_EDGE_FALLING>;
@@ -245,10 +263,18 @@ uart@80120000 {
 			pinctrl-1 = <&u0_a_1_sleep>;
 			status = "okay";
 
-			/* FIXME: not quite working yet, probably needs regulators */
 			bluetooth {
+				/* BRCM4334B0 actually */
 				compatible = "brcm,bcm4330-bt";
-				shutdown-gpios = <&gpio6 30 GPIO_ACTIVE_HIGH>;
+				/*
+				 * We actually have shutdown-gpios, BT_VREG_ON on GPIO222,
+				 * but since this GPIO is shared with the WLAN chip, we need
+				 * to reference the regulator instead. The regulator
+				 * framework will reference count the GPIO usage and
+				 * make sure we can use the same GPIO for several supplies.
+				 */
+				// shutdown-gpios = <&gpio6 30 GPIO_ACTIVE_HIGH>;
+				vbat-supply = <&wl_bt_reg>;
 				device-wakeup-gpios = <&gpio6 7 GPIO_ACTIVE_HIGH>;
 				host-wakeup-gpios = <&gpio3 1 GPIO_ACTIVE_HIGH>;
 				pinctrl-names = "default";
@@ -598,17 +624,11 @@ skomer_cfg1 {
 				ste,config = <&gpio_in_pd>;
 			};
 		};
-		wlan_en_default_mode: wlan_en_default {
-			skomer_cfg2 {
-				pins = "GPIO215_AH13";
-				ste,config = <&gpio_out_lo>;
-			};
-		};
 	};
 	bluetooth {
 		bluetooth_default_mode: bluetooth_default {
 			skomer_cfg1 {
-				pins = "GPIO199_AH23", "GPIO222_AJ9";
+				pins = "GPIO199_AH23";
 				ste,config = <&gpio_out_lo>;
 			};
 			skomer_cfg2 {
@@ -617,6 +637,26 @@ skomer_cfg2 {
 			};
 		};
 	};
+	/* GPIO that enables the WLAN and Bluetooth internal LDO regulators */
+	wlan-bt-ldo {
+		wlan_bt_ldo_en_default: wlan_bt_ldo_default {
+			/* GPIO222 BT_VREG_ON */
+			skomer_cfg1 {
+				pins = "GPIO222_AJ9";
+				ste,config = <&gpio_out_lo>;
+			};
+			/*
+			 * HACK: this is the WL_RESET line that technically has nothing
+			 * to do with the regulator, but in order for the SDIO card
+			 * to enumerate, this must be driven high, so we need to do
+			 * that here.
+			 */
+			skomer_cfg2 {
+				pins = "GPIO215_AH13";	/* RESET_N */
+				ste,config = <&gpio_out_hi>;
+			};
+		};
+	};
 	vibrator {
 		vibrator_default: vibrator_default {
 			skomer_cfg1 {
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] ARM: dts: ux500: Fix BT+WLAN on Samsung phones

[Stephan Gerhold]

On Mon, Mar 15, 2021 at 12:07:50PM +0100, Linus Walleij wrote:
> Some of the Samsung phones had set GPIO 215 as voltage
> regulator GPIO, while in fact this is always on GPIO 222.
> 
> Using GPIO 215 meant that the Bluetooth would not come
> on properly since both WLAN and Bluetooth use GPIO 222
> to power up. The Bluetooth part refers to this GPIO
> line as "shutdown-gpios" which is kind of true, but
> fails to mention that it will also shut down the WLAN
> when it is disabled, which was causing bugs.
> 

Hmm, I'm a bit confused here. The datasheet for BCM4334 [1] that I was
looking at back when I added WiFi/BT to samsung-golden says:

"Section 22: Power-Up Sequence and Timing
 Note: The WL_REG_ON and BT_REG_ON signals are ORed in the BCM4334. [...]
 If two independent host GPIOs are used (one for WL_REG_ON and one for
 BT_REG_ON), then only one of the two signals needs to be high to enable
 the BCM4334 regulators."

and the "Description of Control Signals" part after that repeats the
same, and even shows diagrams that only one of them should be high to
enable only WiFi or only BT.

According to the schematics for samsung-golden:
  - GPIO215 goes to WL_REG_ON of BCM4334
  - GPIO222 goes to BT_REG_ON of BCM4334

So I would say the device tree is correct the way it is, unless the
datasheet is wrong... I wonder if there is some other problem here,
I think WiFi/BT used to probe just fine on samsung-golden...

Stephan

[1]: https://www.cypress.com/file/298706/download

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] ARM: dts: ux500: Fix BT+WLAN on Samsung phones

[Linus Walleij]

On Wed, Mar 17, 2021 at 4:28 PM Stephan Gerhold <stephan@gerhold.net> wrote:

> According to the schematics for samsung-golden:
>   - GPIO215 goes to WL_REG_ON of BCM4334
>   - GPIO222 goes to BT_REG_ON of BCM4334

Yes it appears you are right...

This is in conflict with the source code in the vendor tree:
board-golden-pins.c:

        GPIO215_GPIO            | PIN_OUTPUT_LOW,       /* WLAN_RST_N */
        GPIO222_GPIO            | PIN_OUTPUT_LOW,       /* BT_VREG_EN */

However the BCM4334 does not have appear to have
any signal named WLAN_RST_N.

So I suspect a copy/paste error from Janice, which might
need this because of differences between BCM4330 and
BCM4334.

However another part of the patch is still needed because
GPIO215 is default initilized as low:

                wlan_en_default: wlan_en_default {
                        golden_cfg1 {
                                pins = "GPIO215_AH13";  /* WLAN_EN */
                                ste,config = <&gpio_out_lo>;
                        };
                };

It needs to be high, or the SDIO will not enumerate unless
someone powers on the Bluetooth by chance. (The driver
probes after the SDIO card is detected ... catch 22.)

Also IMO GPIO 222 should be modeled as a regulator and
not assigned to "shutdown-gpios", which could be a separate
patch. The whole notion of "shutdown-gpios" seems backward
as the driver handles regulators just fine.

I'll rehash and compare the BCM4330 and BCM4334
datasheets.

Thanks!
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] ARM: dts: ux500: Fix BT+WLAN on Samsung phones

[Stephan Gerhold]

On Wed, Mar 17, 2021 at 04:53:58PM +0100, Linus Walleij wrote:
> On Wed, Mar 17, 2021 at 4:28 PM Stephan Gerhold <stephan@gerhold.net> wrote:
> 
> > According to the schematics for samsung-golden:
> >   - GPIO215 goes to WL_REG_ON of BCM4334
> >   - GPIO222 goes to BT_REG_ON of BCM4334
> 
> Yes it appears you are right...
> 
> This is in conflict with the source code in the vendor tree:
> board-golden-pins.c:
> 
>         GPIO215_GPIO            | PIN_OUTPUT_LOW,       /* WLAN_RST_N */
>         GPIO222_GPIO            | PIN_OUTPUT_LOW,       /* BT_VREG_EN */
> 
> However the BCM4334 does not have appear to have
> any signal named WLAN_RST_N.
> 
> So I suspect a copy/paste error from Janice, which might
> need this because of differences between BCM4330 and
> BCM4334.
> 

Huh, so it gets more confusing: unlike BCM4334, the BCM4330 datasheet [2]
mentions a BT_RST_N but not a WLAN_RST_N... :O

[2]: https://www.cypress.com/file/298676/download

> However another part of the patch is still needed because
> GPIO215 is default initilized as low:
> 
>                 wlan_en_default: wlan_en_default {
>                         golden_cfg1 {
>                                 pins = "GPIO215_AH13";  /* WLAN_EN */
>                                 ste,config = <&gpio_out_lo>;
>                         };
>                 };
> 
> It needs to be high, or the SDIO will not enumerate unless
> someone powers on the Bluetooth by chance. (The driver
> probes after the SDIO card is detected ... catch 22.)
> 

At least on samsung-golden, GPIO215 belongs to the regulator-wl-reg-on,
which is the vmmc-supply for the SDIO card. I would expect the MMC core
to power that up before scanning for SDIO cards.

> Also IMO GPIO 222 should be modeled as a regulator and
> not assigned to "shutdown-gpios", which could be a separate
> patch. The whole notion of "shutdown-gpios" seems backward
> as the driver handles regulators just fine.
> 

Yeah I suppose it might be better modelled as a regulator. But I guess
in that case we would need support for another regulator.
The "vbat" and "vddio" are likely supposed to point to the "BT_VDDBAT"
and "VDDIO" input pins of the BCM4334.

Stephan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] ARM: dts: ux500: Fix BT+WLAN on Samsung phones

[Linus Walleij]

On Wed, Mar 17, 2021 at 5:17 PM Stephan Gerhold <stephan@gerhold.net> wrote:
> On Wed, Mar 17, 2021 at 04:53:58PM +0100, Linus Walleij wrote:
> > On Wed, Mar 17, 2021 at 4:28 PM Stephan Gerhold <stephan@gerhold.net> wrote:
> >
> > > According to the schematics for samsung-golden:
> > >   - GPIO215 goes to WL_REG_ON of BCM4334
> > >   - GPIO222 goes to BT_REG_ON of BCM4334
> >
> > Yes it appears you are right...
> >
> > This is in conflict with the source code in the vendor tree:
> > board-golden-pins.c:
> >
> >         GPIO215_GPIO            | PIN_OUTPUT_LOW,       /* WLAN_RST_N */
> >         GPIO222_GPIO            | PIN_OUTPUT_LOW,       /* BT_VREG_EN */
> >
> > However the BCM4334 does not have appear to have
> > any signal named WLAN_RST_N.
> >
> > So I suspect a copy/paste error from Janice, which might
> > need this because of differences between BCM4330 and
> > BCM4334.
> >
>
> Huh, so it gets more confusing: unlike BCM4334, the BCM4330 datasheet [2]
> mentions a BT_RST_N but not a WLAN_RST_N... :O

Yeah. BT_RST_N exists on 4330 and is gone on BCM4334.

I'll think of something.

> > However another part of the patch is still needed because
> > GPIO215 is default initilized as low:
> >
> >                 wlan_en_default: wlan_en_default {
> >                         golden_cfg1 {
> >                                 pins = "GPIO215_AH13";  /* WLAN_EN */
> >                                 ste,config = <&gpio_out_lo>;
> >                         };
> >                 };
> >
> > It needs to be high, or the SDIO will not enumerate unless
> > someone powers on the Bluetooth by chance. (The driver
> > probes after the SDIO card is detected ... catch 22.)
>
> At least on samsung-golden, GPIO215 belongs to the regulator-wl-reg-on,
> which is the vmmc-supply for the SDIO card. I would expect the MMC core
> to power that up before scanning for SDIO cards.

Hm, makes sense. But this got it working for me :/
I'll investigate.

> > Also IMO GPIO 222 should be modeled as a regulator and
> > not assigned to "shutdown-gpios", which could be a separate
> > patch. The whole notion of "shutdown-gpios" seems backward
> > as the driver handles regulators just fine.
> >
>
> Yeah I suppose it might be better modelled as a regulator. But I guess
> in that case we would need support for another regulator.
> The "vbat" and "vddio" are likely supposed to point to the "BT_VDDBAT"
> and "VDDIO" input pins of the BCM4334.

We seldom model VBAT supplies since it is always on and can't
be turned off/on or done much with. The only time we do that is
when we need to know the voltage.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel