diff mbox series

[v2,2/5] arm64: dts: rockchip: Add bluetooth rfkill to Khadas Edge2

Message ID 20240617071112.3133101-3-jacobe.zang@wesion.com (mailing list archive)
State New, archived
Headers show
Series arm64: dts: Add board support for Khadas Edge2 | expand

Commit Message

Jacobe Zang June 17, 2024, 7:11 a.m. UTC
Add bluetooth node which managed by rfkill, bluetooth and
wlan controller on Khadas Edge2 was BCM43438. In uart9 add
RTS node in pinctrl.

Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com>
---
 .../boot/dts/rockchip/rk3588s-khadas-edge2.dts  | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Alexey Charkov June 17, 2024, 9:19 a.m. UTC | #1
On 17/06/2024 10:11, Jacobe Zang wrote:
> Add bluetooth node which managed by rfkill, bluetooth and
> wlan controller on Khadas Edge2 was BCM43438. In uart9 add
> RTS node in pinctrl.

You refer to wlan in the commit message, but there is nothing wlan 
related in the patch itself. Update the commit message perhaps?

> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com>
> ---
>   .../boot/dts/rockchip/rk3588s-khadas-edge2.dts  | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts b/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
> index 8c0bc675690dd..a82f10312eacd 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
> @@ -77,6 +77,14 @@ blue_led: led-2 {
>   		};
>   	};
>   
> +	bluetooth-rfkill {
> +		compatible = "rfkill-gpio";
> +		label = "rfkill-bluetooth";
> +		radio-type = "bluetooth";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&bt_reset_pin>;

Does it actually work this way? I thought you'd need to also reference 
the respective GPIO to be able to trigger its state, not just switch the 
pinctrl configuration to GPIO.


I'm also wondering if bt_reset_pin is the right one to use here. On my 
Rock 5B I had to trigger bt_wake_pin to get Bluetooth up and running.


Best regards,

Alexey
Jacobe Zang June 17, 2024, 9:55 a.m. UTC | #2
> You refer to wlan in the commit message, but there is nothing wlan
> related in the patch itself. Update the commit message perhaps?

At that time, I want to describe that wlan and bluetooth were from the module named BCM43438. But, it's true that wlan do not be metioned in the code. I will update it in next version.

> I thought you'd need to also reference the respective GPIO to be able to trigger its 
> state, not just switch the pinctrl configuration to GPIO.

You means the shutdown-gpios? I configure it in the bluetooth node, it can't be configure in rfkill node at the same time.

> I'm also wondering if bt_reset_pin is the right one to use here. On my
> Rock 5B I had to trigger bt_wake_pin to get Bluetooth up and running.

Yes, I try to configure bt_reset_pin, bt_wake_host_irq and  bt_wake_pin in pinctrl separately. Each of them works well. After I connected a BT device to Edge2, I input rfkill block 1, bluetooth function failed. Then I input rfkill unblock 1, it can rework. So at last I only configure the bt_reset_pin.

---
Best Regards
Jacobe
Alexey Charkov June 17, 2024, 10:20 a.m. UTC | #3
(sorry, the previous one got sent in HTML by accident)

On 17/06/2024 12:55, Jacobe Zang wrote:
>> You refer to wlan in the commit message, but there is nothing wlan
>> related in the patch itself. Update the commit message perhaps?
> At that time, I want to describe that wlan and bluetooth were from the module named BCM43438. But, it's true that wlan do not be metioned in the code. I will update it in next version.
>
>> I thought you'd need to also reference the respective GPIO to be able to trigger its
>> state, not just switch the pinctrl configuration to GPIO.
> You means the shutdown-gpios? I configure it in the bluetooth node, it can't be configure in rfkill node at the same time.

In Documentation/devicetree/bindings/net/rfkill-gpio.yaml shutdown-gpios 
is listed as a required property, did you try building the sources with 
CHECK_DTBS=1 in make arguments?

If you already control this GPIO from elsewhere (such as from the 
bluetooth driver), then perhaps you don't need to define a separate 
rfkill device at all.

>> I'm also wondering if bt_reset_pin is the right one to use here. On my
>> Rock 5B I had to trigger bt_wake_pin to get Bluetooth up and running.
> Yes, I try to configure bt_reset_pin, bt_wake_host_irq and  bt_wake_pin in pinctrl separately. Each of them works well. After I connected a BT device to Edge2, I input rfkill block 1, bluetooth function failed. Then I input rfkill unblock 1, it can rework. So at last I only configure the bt_reset_pin.

That doesn't necessarily mean that the hardware gets an rfkill signal, 
what you describe above could also come from the software alone. It 
would be great to somehow check if the physical pin state gets triggered 
once you switch rfkill state from userspace, and then that the bluetooth 
device handles it appropriately (e.g. gets into a hardware-disable state 
somehow).

Best regards,
Alexey
Jacobe Zang June 17, 2024, 11:22 a.m. UTC | #4
> If you already control this GPIO from elsewhere (such as from the
> bluetooth driver), then perhaps you don't need to define a separate
> rfkill device at all.

Yes, I missed the error log before. The rfkill driver didn't probe successfully. I will remove this rfkill node and reserve bluetooth node next time.

---
Best Regards
Jacobe
Sebastian Reichel June 18, 2024, 12:13 a.m. UTC | #5
Hi,

On Mon, Jun 17, 2024 at 11:22:57AM GMT, Jacobe Zang wrote:
> > If you already control this GPIO from elsewhere (such as from the
> > bluetooth driver), then perhaps you don't need to define a separate
> > rfkill device at all.
> 
> Yes, I missed the error log before. The rfkill driver didn't probe
> successfully. I will remove this rfkill node and reserve bluetooth
> node next time.

Does it work with the RTC clock disabled? Otherwise this also needs
the pwrseq work I mentioned in the WLAN patch.

Greetings,

-- Sebastian
Jacobe Zang June 18, 2024, 2:34 a.m. UTC | #6
Hi Sebastian,

> Does it work with the RTC clock disabled? Otherwise this also needs
> the pwrseq work I mentioned in the WLAN patch.

After I disabled the RTC clock, bluetooth stopped working. I also check the Amlogic chip, some SOCs have the pwrseq defined in DT, maybe rk3588 need to add it as well.

---
Best Regards
Jacobe
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts b/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
index 8c0bc675690dd..a82f10312eacd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts
@@ -77,6 +77,14 @@  blue_led: led-2 {
 		};
 	};
 
+	bluetooth-rfkill {
+		compatible = "rfkill-gpio";
+		label = "rfkill-bluetooth";
+		radio-type = "bluetooth";
+		pinctrl-names = "default";
+		pinctrl-0 = <&bt_reset_pin>;
+	};
+
 	vcc3v3_pcie_wl: vcc3v3-pcie-wl-regulator {
 		compatible = "regulator-fixed";
 		enable-active-high;
@@ -750,8 +758,15 @@  &uart2 {
 
 &uart9 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&uart9m2_xfer &uart9m2_ctsn>;
+	pinctrl-0 = <&uart9m2_xfer &uart9m2_ctsn &uart9m2_rtsn>;
 	status = "okay";
+
+	bluetooth {
+		compatible = "brcm,bcm43438-bt";
+		pinctrl-names = "default";
+		pinctrl-0 = <&bt_reset_pin &bt_wake_host_irq &bt_wake_pin>;
+		shutdown-gpios = <&gpio0 RK_PD4 GPIO_ACTIVE_HIGH>;
+	};
 };
 
 &u2phy0 {