[PATCH 0/2] Enable bluetooth on BeagleV-Ahead

[PATCH 0/2] Enable bluetooth on BeagleV-Ahead

[Thomas Gerner]

The single board system BeagleV-Ahead uses a Ampak AP6203BM chip for wifi
and bluetooth. Wifi is already enabled for this board. The bluetooth
module of the chip connects by UART and PCM to the TH1520 SOC. The PCM
interface is currently not used because of missing I2S drivers for the
SOC.

This patch set enables bluetooth.

Thomas Gerner (2):
  riscv: dts: thead: Enable bluetooth on BeagleV-Ahead
  Bluetooth: btbcm: disable read tx power for BeagleV-Ahead

 drivers/bluetooth/btbcm.c                     | 19 +++-
 .../boot/dts/thead/th1520-beaglev-ahead.dts   | 89 +++++++++++++++++++
 2 files changed, 105 insertions(+), 3 deletions(-)

-- 
2.53.0

[PATCH 2/2] Bluetooth: btbcm: disable read tx power for BeagleV-Ahead

[Thomas Gerner]

The bluetooth module of the Ampak AP6203BM is identified by the btbcm
driver as BCM4362A2.

The firmware of the bluetooth module on the BeagleV-Ahead reports a
capability of reading LE tx power. However reading fails. To get
bluetooth working it is necessary to disable reading of LE tx power on
the BeagleV-Ahead board. The disable is implemented in a similar way as
for several Apple boards but uses the board compatible identification
from the DTB as there is no DMI.

Signed-off-by: Thomas Gerner <thomas.gerner@muenchen-mail.de>
---
 drivers/bluetooth/btbcm.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index 463d59890bef..2d02cc1863d0 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -436,6 +436,11 @@ static const struct dmi_system_id disable_broken_read_transmit_power[] = {
 	{ }
 };
 
+static const char *board_broken_read_transmit_power[] __maybe_unused = {
+	"beagle,beaglev-ahead",
+	NULL
+};
+
 static int btbcm_read_info(struct hci_dev *hdev)
 {
 	struct sk_buff *skb;
@@ -537,12 +542,13 @@ static const struct bcm_subver_table bcm_usb_subver_table[] = {
  * This currently only looks up the device tree board appendix,
  * but can be expanded to other mechanisms.
  */
-static const char *btbcm_get_board_name(struct device *dev)
+static const char *btbcm_get_board_name(struct hci_dev *hdev)
 {
 #ifdef CONFIG_OF
 	struct device_node *root __free(device_node) = of_find_node_by_path("/");
 	char *board_type;
 	const char *tmp;
+	const char **s;
 
 	if (!root)
 		return NULL;
@@ -550,8 +556,15 @@ static const char *btbcm_get_board_name(struct device *dev)
 	if (of_property_read_string_index(root, "compatible", 0, &tmp))
 		return NULL;
 
+	/* disable broken Read LE Min/Max Tx Power on first board found in list */
+	for (s = board_broken_read_transmit_power; *s; s++)
+		if (!strcmp(tmp, *s)) {
+			hci_set_quirk(hdev, HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER);
+			break;
+		}
+
 	/* get rid of any '/' in the compatible string */
-	board_type = devm_kstrdup(dev, tmp, GFP_KERNEL);
+	board_type = devm_kstrdup(&hdev->dev, tmp, GFP_KERNEL);
 	if (!board_type)
 		return NULL;
 
@@ -577,7 +590,7 @@ int btbcm_initialize(struct hci_dev *hdev, bool *fw_load_done, bool use_autobaud
 	const struct firmware *fw;
 	int i, err;
 
-	board_name = btbcm_get_board_name(&hdev->dev);
+	board_name = btbcm_get_board_name(hdev);
 
 	/* Reset */
 	err = btbcm_reset(hdev);
-- 
2.53.0

Re: [PATCH 2/2] Bluetooth: btbcm: disable read tx power for BeagleV-Ahead

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bluetooth: btbcm: devm_kstrdup memory leak of board string on repeated initialization
--

commit d78be9f56325d78f88a3903ffe62866de6e22e52
Author: Thomas Gerner <thomas.gerner@muenchen-mail.de>

Bluetooth: btbcm: disable read tx power for BeagleV-Ahead

The bluetooth module on the BeagleV-Ahead reports LE tx power read
capabilities but fails when actually doing it. This patch adds a quirk to
disable this reading based on the board's compatible string from the DTB.

> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
> index 463d59890befe..2d02cc1863d0b 100644
> --- a/drivers/bluetooth/btbcm.c
> +++ b/drivers/bluetooth/btbcm.c
[ ... ]
> @@ -537,12 +542,13 @@ static const struct bcm_subver_table bcm_usb_subver_table[] = {
>   * This currently only looks up the device tree board appendix,
>   * but can be expanded to other mechanisms.
>   */
> -static const char *btbcm_get_board_name(struct device *dev)
> +static const char *btbcm_get_board_name(struct hci_dev *hdev)
>  {
>  #ifdef CONFIG_OF
>  	struct device_node *root __free(device_node) = of_find_node_by_path("/");
>  	char *board_type;
>  	const char *tmp;
> +	const char **s;
>  
>  	if (!root)
>  		return NULL;
> @@ -550,8 +556,15 @@ static const char *btbcm_get_board_name(struct device *dev)
>  	if (of_property_read_string_index(root, "compatible", 0, &tmp))
>  		return NULL;
>  
> +	/* disable broken Read LE Min/Max Tx Power on first board found in list */
> +	for (s = board_broken_read_transmit_power; *s; s++)
> +		if (!strcmp(tmp, *s)) {
> +			hci_set_quirk(hdev, HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER);
> +			break;
> +		}
> +
>  	/* get rid of any '/' in the compatible string */
> -	board_type = devm_kstrdup(dev, tmp, GFP_KERNEL);
> +	board_type = devm_kstrdup(&hdev->dev, tmp, GFP_KERNEL);

[Severity: Low]
Can this leak the board string on repeated initialization?

During device setup, btbcm_setup_patchram() calls btbcm_initialize(), which
calls btbcm_get_board_name(). The board_type string is allocated using
devm_kstrdup() and returned, but it is only stored locally as board_name in
btbcm_initialize() and discarded when the function returns.

Since btbcm_setup_patchram() subsequently calls btbcm_finalize(), which will
invoke btbcm_initialize() a second time if the firmware was successfully
loaded, the devm_kstrdup() allocation happens again, adding another permanent
entry to the devres list.

Could kstrdup() be used instead so the memory can be explicitly freed when it
is no longer needed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791720467.git.thomas.gerner@muenchen-mail.de?part=2

[PATCH 1/2] riscv: dts: thead: Enable bluetooth on BeagleV-Ahead

[Thomas Gerner]

On the BeagleV-Ahead the bluetooth module of the Ampak AP6203BM is
identified by the btbcm driver as BCM4362A2. The module is connected by
UART4, several gpio pins and an I2S interface. The I2S is currently not
used as there is no driver. 

Signed-off-by: Thomas Gerner <thomas.gerner@muenchen-mail.de>
---
 .../boot/dts/thead/th1520-beaglev-ahead.dts   | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
index e16484a47653..bf64eb667745 100644
--- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
+++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
@@ -76,6 +76,23 @@ led-5 {
 		};
 	};
 
+	reg_vbat_1v8: regulator-vbat-1v8 {
+		compatible = "regulator-fixed";
+		regulator-boot-on;
+		regulator-max-microvolt = <1800000>;
+		regulator-min-microvolt = <1800000>;
+		regulator-name = "vbat-1v8";
+	};
+
+	reg_vbat_3v3: regulator-vbat-3v3 {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		regulator-boot-on;
+		regulator-max-microvolt = <3300000>;
+		regulator-min-microvolt = <3300000>;
+		regulator-name = "vbat-3v3";
+	};
+
 	hdmi-connector {
 		compatible = "hdmi-connector";
 		type = "d";
@@ -152,6 +169,31 @@ led-pins {
 };
 
 &padctrl0_apsys {
+	bluetooth_in_pins: bluetooth-0 {
+		in-pins {
+			bias-disable;
+			drive-strength = <1>;
+			function = "gpio";
+			input-enable;
+			input-schmitt-enable;
+			pins = "GPIO2_30";
+			slew-rate = <0>;
+		};
+	};
+
+	bluetooth_out_pins: bluetooth-1 {
+		out-pins {
+			bias-disable;
+			drive-strength = <3>;
+			function = "gpio";
+			input-disable;
+			input-schmitt-disable;
+			pins = "SDIO1_DETN",	/* GPIO2_29 */
+			       "SDIO1_WPRTN";	/* GPIO2_28 */
+			slew-rate = <0>;
+		};
+	};
+
 	gmac0_pins: gmac0-0 {
 		tx-pins {
 			pins = "GMAC0_TX_CLK",
@@ -268,6 +310,30 @@ reg-on-pins {
 	};
 };
 
+&padctrl1_apsys {
+	uart4_pins: uart4-0 {
+		rx-pins {
+			bias-disable;
+			drive-strength = <1>;
+			function = "uart";
+			input-enable;
+			pins = "UART4_RXD", "UART4_CTSN";
+			input-schmitt-enable;
+			slew-rate = <0>;
+		};
+
+		tx-pins {
+			bias-disable;
+			drive-strength = <3>;
+			function = "uart";
+			input-disable;
+			pins = "UART4_TXD", "UART4_RTSN";
+			input-schmitt-disable;
+			slew-rate = <0>;
+		};
+	};
+};
+
 &sdio0 {
 	bus-width = <4>;
 	max-frequency = <198000000>;
@@ -314,3 +380,26 @@ &uart0 {
 	pinctrl-0 = <&uart0_pins>;
 	status = "okay";
 };
+
+&uart4 {
+	pinctrl-0 = <&uart4_pins>;
+	pinctrl-names = "default";
+	uart-has-rtscts;
+
+	status = "okay";
+
+	bluetooth {
+		compatible = "brcm,bcm4329-bt";
+		device-wakeup-gpios = <&gpio2 29 GPIO_ACTIVE_HIGH>;
+		interrupt-parent = <&gpio2>;
+		interrupts = <30 IRQ_TYPE_EDGE_RISING>;
+		interrupt-names = "host-wakeup";
+		max-speed = <921600>;
+		pinctrl-0 = <&bluetooth_in_pins>;
+		pinctrl-1 = <&bluetooth_out_pins>;
+		pinctrl-names = "default";
+		shutdown-gpios = <&gpio2 28 GPIO_ACTIVE_HIGH>; 	/* BT-REG-ON */
+		vbat-supply = <&reg_vbat_3v3>;
+		vddio-supply = <&reg_vbat_1v8>;
+	};
+};
-- 
2.53.0

Re: [PATCH 1/2] riscv: dts: thead: Enable bluetooth on BeagleV-Ahead

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dts: missing pinctrl state name for pinctrl-1 in bluetooth node
--

commit d08d8ee5ff25b62d26075da6c5a8d9e0d4d9c5bc
Author: Thomas Gerner <thomas.gerner@muenchen-mail.de>

riscv: dts: thead: Enable bluetooth on BeagleV-Ahead

On the BeagleV-Ahead the bluetooth module of the Ampak AP6203BM is
identified by the btbcm driver as BCM4362A2. The module is connected by
UART4, several gpio pins and an I2S interface.

> diff --git a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> index e16484a476534..bf64eb667745f 100644
> --- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> +++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts

[ ... ]

> @@ -314,3 +380,26 @@ &uart0 {
>  	pinctrl-0 = <&uart0_pins>;
>  	status = "okay";
>  };
> +
> +&uart4 {
> +	pinctrl-0 = <&uart4_pins>;
> +	pinctrl-names = "default";
> +	uart-has-rtscts;
> +
> +	status = "okay";
> +
> +	bluetooth {
> +		compatible = "brcm,bcm4329-bt";
> +		device-wakeup-gpios = <&gpio2 29 GPIO_ACTIVE_HIGH>;
> +		interrupt-parent = <&gpio2>;
> +		interrupts = <30 IRQ_TYPE_EDGE_RISING>;
> +		interrupt-names = "host-wakeup";
> +		max-speed = <921600>;
> +		pinctrl-0 = <&bluetooth_in_pins>;
> +		pinctrl-1 = <&bluetooth_out_pins>;
> +		pinctrl-names = "default";

[Severity: Medium]
Does this leave the pins in pinctrl-1 unconfigured?

Since there is no second name provided in pinctrl-names, the pinctrl subsystem
assigns the auto-generated name "1" to pinctrl-1. The driver core only requests
standard states like "default", which could mean the pin configuration defined
in bluetooth_out_pins (such as drive strength, bias, and slew rate for the
wakeup and shutdown GPIOs) is silently ignored and never applied.

> +		shutdown-gpios = <&gpio2 28 GPIO_ACTIVE_HIGH>; 	/* BT-REG-ON */
> +		vbat-supply = <&reg_vbat_3v3>;
> +		vddio-supply = <&reg_vbat_1v8>;
> +	};
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791720467.git.thomas.gerner@muenchen-mail.de?part=1