The GPIO polarity of the 'shutdown-gpios' property needs to be
ACTIVE_HIGH or the Bluetooth device won't work.
This also matches what other devices with the same BT device have.

When changing from 'reset-gpios' to 'shutdown-gpios', I forgot to also
correct the GPIO polarity, so do that now.

Fixes: a3a625086192 ("arm64: dts: rockchip: Fix reset-gpios property on brcm BT nodes")
Signed-off-by: Diederik de Haas <didi.debian@cknow.org>
---
 arch/arm64/boot/dts/rockchip/rk3566-pinenote.dtsi  | 2 +-
 arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3.dtsi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3566-pinenote.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-pinenote.dtsi
index a477bd992b40..0131f2cdd312 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-pinenote.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3566-pinenote.dtsi
@@ -688,7 +688,7 @@ bluetooth {
 		host-wakeup-gpios = <&gpio0 RK_PC3 GPIO_ACTIVE_HIGH>;
 		pinctrl-0 = <&bt_enable_h>, <&bt_host_wake_l>, <&bt_wake_h>;
 		pinctrl-names = "default";
-		shutdown-gpios = <&gpio0 RK_PC4 GPIO_ACTIVE_LOW>;
+		shutdown-gpios = <&gpio0 RK_PC4 GPIO_ACTIVE_HIGH>;
 		vbat-supply = <&vcc_wl>;
 		vddio-supply = <&vcca_1v8_pmu>;
 	};
diff --git a/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3.dtsi
index e9fa9bee995a..1e36f73840da 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3566-radxa-cm3.dtsi
@@ -404,7 +404,7 @@ bluetooth {
 		host-wakeup-gpios = <&gpio2 RK_PB1 GPIO_ACTIVE_HIGH>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&bt_host_wake_h &bt_reg_on_h &bt_wake_host_h>;
-		shutdown-gpios = <&gpio2 RK_PC0 GPIO_ACTIVE_LOW>;
+		shutdown-gpios = <&gpio2 RK_PC0 GPIO_ACTIVE_HIGH>;
 		vbat-supply = <&vcc_3v3>;
 		vddio-supply = <&vcc_1v8>;
 	};
-- 
2.45.2

On 18/10/2024 11:22, Diederik de Haas wrote:
> The GPIO polarity of the 'shutdown-gpios' property needs to be
> ACTIVE_HIGH or the Bluetooth device won't work.
> This also matches what other devices with the same BT device have.

This should match the hardware. What if the Linux driver is just buggy?

Best regards,
Krzysztof

On Fri Oct 18, 2024 at 12:11 PM CEST, Krzysztof Kozlowski wrote:
> On 18/10/2024 11:22, Diederik de Haas wrote:
> > The GPIO polarity of the 'shutdown-gpios' property needs to be
> > ACTIVE_HIGH or the Bluetooth device won't work.
> > This also matches what other devices with the same BT device have.
>
> This should match the hardware.

Fair point.

The only documentation I have for the AzureWave-CM256SM is a datasheet.

Para "3.4 Power up Timing Sequence" of that datasheet has the following:

"The AW-CM256SM has two signals that enable or disable the Bluetooth
and WLAN circuits and the internal regulator blocks, allowing the host
to control power consumption."

WL_REG_ON:
"When this pin is high, the regulators are enabled and the WLAN section
is out of reset. When this pin is low, the WLAN section is in reset. If
BT_REG_ON and WL_REG_ON are both low, the regulators are disabled."

BT_REG_ON:
"If both BT_REG_ON and WL_REG_ON are low, the regulators will be
disabled. When this pin is low and WL_REG_ON is high, the BT section
is in reset."

From that I conclude that BT_REG_ON needs to be high to get BT in a
working state.

But that datasheet is in its 19th revision, so can I assume that it now
is correct? (genuine question)

That's why I also looked at devices which uses the same hardware:
https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/WHENCE?h=20241017#n2973
and those all had ACTIVE_HIGH for 'shutdown-gpios' in their hardware
description (ie DeviceTree) and if they were all broken, I think we
would've heard about it.

And my own testing showed it not working with ACTIVE_LOW and working
with ACTIVE_HIGH.
I just messed up my previous patch submission process, which this patch
is meant to correct.

> What if the Linux driver is just buggy?

Given the amount of 'spam' I get in `bluetoothctl` when it is working, I
think there's a real possibility that that is (also) the case.

Cheers,
  Diederik