rtl8723{bc}s is WiFi/Bluetooth module, WiFi part is connected over SDIO
while bluetooth is connected over UART.
This series adds support for rtl8723bs/rtl8723cs bluetooth.
Bluetooth module in rtl8723cs is buggy - it advertises some features that
it doesn't support and responds either with garbage or with errors on
corresponding commands. Unfortunately that confuses bluetooth stack,
so we need to introduce another quirk.
Patches for Pine64 and Sopine are provided as examples since WiFi/BT module
is optional on these boards.
Vasily Khoruzhick (8):
Bluetooth: Add new quirk for broken local ext features max_page
Bluetooth: hci_h5: Add support for reset GPIO
dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth
Bluetooth: hci_h5: Add support for binding RTL8723BS with device tree
Bluetooth: btrtl: add support for the RTL8723CS
arm64: allwinner: a64: enable Bluetooth On Pinebook
DO-NOT-MERGE: arm64: allwinner: a64: enable Bluetooth On Pine64
DO-NOT-MERGE: arm64: allwinner: a64: enable Bluetooth On SoPine
baseboard
.../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++
.../boot/dts/allwinner/sun50i-a64-pine64.dts | 10 +-
.../dts/allwinner/sun50i-a64-pinebook.dts | 14 ++
.../allwinner/sun50i-a64-sopine-baseboard.dts | 14 ++
drivers/bluetooth/btrtl.c | 128 +++++++++++++++++-
drivers/bluetooth/btrtl.h | 12 ++
drivers/bluetooth/hci_h5.c | 33 +++++
include/net/bluetooth/hci.h | 7 +
net/bluetooth/hci_event.c | 4 +-
9 files changed, 252 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
--
2.20.1
Some boards (e.g. Pine64 and Pinebook) wire a GPIO to reset pin of
RTL8723BS
Signed-off-by: Vasily Khoruzhick <[email protected]>
---
drivers/bluetooth/hci_h5.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index 069d1c8fde73..7ead8c3d8657 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -107,6 +107,7 @@ struct h5 {
const struct h5_vnd *vnd;
const char *id;
+ struct gpio_desc *reset_gpio;
struct gpio_desc *enable_gpio;
struct gpio_desc *device_wake_gpio;
};
@@ -833,6 +834,10 @@ static int h5_serdev_probe(struct serdev_device *serdev)
if (IS_ERR(h5->device_wake_gpio))
return PTR_ERR(h5->device_wake_gpio);
+ h5->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(h5->reset_gpio))
+ return PTR_ERR(h5->reset_gpio);
+
return hci_uart_register_device(&h5->serdev_hu, &h5p);
}
@@ -921,6 +926,9 @@ static void h5_btrtl_open(struct h5 *h5)
/* The controller needs up to 500ms to wakeup */
gpiod_set_value_cansleep(h5->enable_gpio, 1);
+ /* Take it out of reset */
+ gpiod_set_value_cansleep(h5->reset_gpio, 0);
+ msleep(100);
gpiod_set_value_cansleep(h5->device_wake_gpio, 1);
msleep(500);
}
@@ -928,6 +936,7 @@ static void h5_btrtl_open(struct h5 *h5)
static void h5_btrtl_close(struct h5 *h5)
{
gpiod_set_value_cansleep(h5->device_wake_gpio, 0);
+ gpiod_set_value_cansleep(h5->reset_gpio, 1);
gpiod_set_value_cansleep(h5->enable_gpio, 0);
}
--
2.20.1
Add binding document for bluetooth part of RTL8723BS/RTL8723CS
Signed-off-by: Vasily Khoruzhick <[email protected]>
---
.../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
1 file changed, 35 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
new file mode 100644
index 000000000000..8357f242ae4c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
@@ -0,0 +1,35 @@
+RTL8723BS/RTL8723CS Bluetooth
+---------------------
+
+RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
+BT is connected over serial. It speaks H5 protocol with few extra commands
+to upload firmware and change module speed.
+
+Required properties:
+
+ - compatible: should be one of the following:
+ * "realtek,rtl8723bs-bt"
+ * "realtek,rtl8723cs-bt"
+Optional properties:
+
+ - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
+ - enable-gpios: GPIO specifier, used to enable the BT module (active high)
+ - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
+ - firmware-postfix: firmware postfix to be used for firmware config
+ - reset-gpios: GPIO specifier, used to reset the BT module (active high)
+
+Example:
+
+&uart1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
+ status = "okay";
+
+ bluetooth {
+ compatible = "realtek,rtl8723bs-bt";
+ reset-gpios = <&r_pio 0 4 GPIO_ACTIVE_LOW>; /* PL4 */
+ device-wake-gpios = <&r_pio 0 5 GPIO_ACTIVE_HIGH>; /* PL5 */
+ host-wakeup-gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
+ firmware-postfix="pine64";
+ };
+};
--
2.20.1
RTL8723BS is often used in ARM boards, so add ability to bind it
using device tree.
Signed-off-by: Vasily Khoruzhick <[email protected]>
---
drivers/bluetooth/hci_h5.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index 7ead8c3d8657..44f848c02c63 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -26,6 +26,7 @@
#include <linux/gpio/consumer.h>
#include <linux/kernel.h>
#include <linux/mod_devicetable.h>
+#include <linux/of_device.h>
#include <linux/serdev.h>
#include <linux/skbuff.h>
@@ -823,6 +824,11 @@ static int h5_serdev_probe(struct serdev_device *serdev)
if (h5->vnd->acpi_gpio_map)
devm_acpi_dev_add_driver_gpios(dev,
h5->vnd->acpi_gpio_map);
+ } else {
+ h5->vnd = (const struct h5_vnd *)
+ of_device_get_match_data(&serdev->dev);
+ of_property_read_string(serdev->dev.of_node,
+ "firmware-postfix", &h5->id);
}
h5->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
@@ -1024,12 +1030,26 @@ static const struct dev_pm_ops h5_serdev_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(h5_serdev_suspend, h5_serdev_resume)
};
+static struct h5_vnd rtl8723_of_vnd = {
+ .setup = h5_btrtl_setup,
+ .open = h5_btrtl_open,
+ .close = h5_btrtl_close,
+};
+
+static const struct of_device_id h5_of_match[] = {
+ { .compatible = "realtek,rtl8723bs-bt", .data = &rtl8723_of_vnd },
+ { .compatible = "realtek,rtl8723cs-bt", .data = &rtl8723_of_vnd },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, h5_of_match);
+
static struct serdev_device_driver h5_serdev_driver = {
.probe = h5_serdev_probe,
.remove = h5_serdev_remove,
.driver = {
.name = "hci_uart_h5",
.acpi_match_table = ACPI_PTR(h5_acpi_match),
+ .of_match_table = of_match_ptr(h5_of_match),
.pm = &h5_serdev_pm_ops,
},
};
--
2.20.1
SoPine has optional RTL8723BS WiFi + BT module, BT is connected to UART1
and uses PL4 as BT reset, PL5 as device wake GPIO, PL6 as host wake GPIO
the I2C controlling signals are connected to R_I2C bus.
Enable it in the device tree.
Signed-off-by: Vasily Khoruzhick <[email protected]>
---
.../dts/allwinner/sun50i-a64-sopine-baseboard.dts | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
index e6fb9683f213..691ffad390c6 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
@@ -204,6 +204,20 @@
status = "okay";
};
+&uart1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
+ status = "okay";
+
+ bluetooth {
+ compatible = "realtek,rtl8723bs-bt";
+ reset-gpios = <&r_pio 0 4 GPIO_ACTIVE_LOW>; /* PL4 */
+ device-wake-gpios = <&r_pio 0 5 GPIO_ACTIVE_HIGH>; /* PL5 */
+ host-wake-gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
+ firmware-postfix = "pine64";
+ };
+};
+
&usb_otg {
dr_mode = "host";
status = "okay";
--
2.20.1
Pine64 has optional RTL8723BS WiFi + BT module, BT is connected to UART1
and uses PL4 as BT reset, PL5 as device wake GPIO, PL6 as host wake GPIO
the I2C controlling signals are connected to R_I2C bus.
Enable it in the device tree.
Signed-off-by: Vasily Khoruzhick <[email protected]>
---
arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index 216f2f5db5ef..4d48aa31d420 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -302,7 +302,15 @@
&uart1 {
pinctrl-names = "default";
pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
- status = "disabled";
+ status = "okay";
+
+ bluetooth {
+ compatible = "realtek,rtl8723bs-bt";
+ reset-gpios = <&r_pio 0 4 GPIO_ACTIVE_LOW>; /* PL4 */
+ device-wake-gpios = <&r_pio 0 5 GPIO_ACTIVE_HIGH>; /* PL5 */
+ host-wake-gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
+ firmware-postfix = "pine64";
+ };
};
/* On Pi-2 connector */
--
2.20.1
The Realtek RTL8723CS is SDIO WiFi chip. It also contains a Bluetooth
module which is connected via UART to the host.
It shares lmp subversion with 8703B, so Realtek's userspace
initialization tool (rtk_hciattach) differentiates varieties of RTL8723CS
(CG, VF, XX) with RTL8703B using vendor's command to read chip type.
Also this chip declares support for some features it doesn't support
so add a quirk to indicate that these features are broken.
Signed-off-by: Vasily Khoruzhick <[email protected]>
---
drivers/bluetooth/btrtl.c | 128 ++++++++++++++++++++++++++++++++++++-
drivers/bluetooth/btrtl.h | 12 ++++
drivers/bluetooth/hci_h5.c | 4 ++
3 files changed, 141 insertions(+), 3 deletions(-)
diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
index 41405de27d66..ec51667c4549 100644
--- a/drivers/bluetooth/btrtl.c
+++ b/drivers/bluetooth/btrtl.c
@@ -27,8 +27,12 @@
#define VERSION "0.1"
+#define RTL_CHIP_8723CS_CG 3
+#define RTL_CHIP_8723CS_VF 4
+#define RTL_CHIP_8723CS_XX 5
#define RTL_EPATCH_SIGNATURE "Realtech"
#define RTL_ROM_LMP_3499 0x3499
+#define RTL_ROM_LMP_8703B 0x8703
#define RTL_ROM_LMP_8723A 0x1200
#define RTL_ROM_LMP_8723B 0x8723
#define RTL_ROM_LMP_8821A 0x8821
@@ -40,6 +44,7 @@
#define IC_MATCH_FL_HCIREV (1 << 1)
#define IC_MATCH_FL_HCIVER (1 << 2)
#define IC_MATCH_FL_HCIBUS (1 << 3)
+#define IC_MATCH_FL_CHIP_TYPE (1 << 4)
#define IC_INFO(lmps, hcir) \
.match_flags = IC_MATCH_FL_LMPSUBV | IC_MATCH_FL_HCIREV, \
.lmp_subver = (lmps), \
@@ -51,6 +56,7 @@ struct id_table {
__u16 hci_rev;
__u8 hci_ver;
__u8 hci_bus;
+ __u8 chip_type;
bool config_needed;
bool has_rom_version;
char *fw_name;
@@ -98,6 +104,39 @@ static const struct id_table ic_id_table[] = {
.fw_name = "rtl_bt/rtl8723b_fw.bin",
.cfg_name = "rtl_bt/rtl8723b_config" },
+ /* 8723CS-CG */
+ { .match_flags = IC_MATCH_FL_LMPSUBV | IC_MATCH_FL_CHIP_TYPE |
+ IC_MATCH_FL_HCIBUS,
+ .lmp_subver = RTL_ROM_LMP_8703B,
+ .chip_type = RTL_CHIP_8723CS_CG,
+ .hci_bus = HCI_UART,
+ .config_needed = true,
+ .has_rom_version = true,
+ .fw_name = "rtl_bt/rtl8723cs_cg_fw.bin",
+ .cfg_name = "rtl_bt/rtl8723cs_cg_config" },
+
+ /* 8723CS-VF */
+ { .match_flags = IC_MATCH_FL_LMPSUBV | IC_MATCH_FL_CHIP_TYPE |
+ IC_MATCH_FL_HCIBUS,
+ .lmp_subver = RTL_ROM_LMP_8703B,
+ .chip_type = RTL_CHIP_8723CS_VF,
+ .hci_bus = HCI_UART,
+ .config_needed = true,
+ .has_rom_version = true,
+ .fw_name = "rtl_bt/rtl8723cs_vf_fw.bin",
+ .cfg_name = "rtl_bt/rtl8723cs_vf_config" },
+
+ /* 8723CS-XX */
+ { .match_flags = IC_MATCH_FL_LMPSUBV | IC_MATCH_FL_CHIP_TYPE |
+ IC_MATCH_FL_HCIBUS,
+ .lmp_subver = RTL_ROM_LMP_8703B,
+ .chip_type = RTL_CHIP_8723CS_XX,
+ .hci_bus = HCI_UART,
+ .config_needed = true,
+ .has_rom_version = true,
+ .fw_name = "rtl_bt/rtl8723cs_xx_fw.bin",
+ .cfg_name = "rtl_bt/rtl8723cs_xx_config" },
+
/* 8723D */
{ IC_INFO(RTL_ROM_LMP_8723B, 0xd),
.config_needed = true,
@@ -154,7 +193,8 @@ static const struct id_table ic_id_table[] = {
};
static const struct id_table *btrtl_match_ic(u16 lmp_subver, u16 hci_rev,
- u8 hci_ver, u8 hci_bus)
+ u8 hci_ver, u8 hci_bus,
+ u8 chip_type)
{
int i;
@@ -171,6 +211,9 @@ static const struct id_table *btrtl_match_ic(u16 lmp_subver, u16 hci_rev,
if ((ic_id_table[i].match_flags & IC_MATCH_FL_HCIBUS) &&
(ic_id_table[i].hci_bus != hci_bus))
continue;
+ if ((ic_id_table[i].match_flags & IC_MATCH_FL_CHIP_TYPE) &&
+ (ic_id_table[i].chip_type != chip_type))
+ continue;
break;
}
@@ -232,6 +275,7 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev,
{ RTL_ROM_LMP_8723B, 1 },
{ RTL_ROM_LMP_8821A, 2 },
{ RTL_ROM_LMP_8761A, 3 },
+ { RTL_ROM_LMP_8703B, 7 },
{ RTL_ROM_LMP_8822B, 8 },
{ RTL_ROM_LMP_8723B, 9 }, /* 8723D */
{ RTL_ROM_LMP_8821A, 10 }, /* 8821C */
@@ -507,6 +551,48 @@ static struct sk_buff *btrtl_read_local_version(struct hci_dev *hdev)
return skb;
}
+static bool rtl_has_chip_type(u16 lmp_subver)
+{
+ switch (lmp_subver) {
+ case RTL_ROM_LMP_8703B:
+ return true;
+ default:
+ break;
+ }
+
+ return false;
+}
+
+static int rtl_read_chip_type(struct hci_dev *hdev, u8 *type)
+{
+ struct rtl_chip_type_evt *chip_type;
+ struct sk_buff *skb;
+ const unsigned char cmd_buf[] = {0x00, 0x94, 0xa0, 0x00, 0xb0};
+
+ /* Read RTL chip type command */
+ skb = __hci_cmd_sync(hdev, 0xfc61, 5, cmd_buf, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ rtl_dev_err(hdev, "Read chip type failed (%ld)",
+ PTR_ERR(skb));
+ return PTR_ERR(skb);
+ }
+
+ if (skb->len != sizeof(*chip_type)) {
+ rtl_dev_err(hdev, "RTL chip type event length mismatch");
+ kfree_skb(skb);
+ return -EIO;
+ }
+
+ chip_type = (struct rtl_chip_type_evt *)skb->data;
+ rtl_dev_info(hdev, "chip_type status=%x type=%x",
+ chip_type->status, chip_type->type);
+
+ *type = chip_type->type & 0x0f;
+
+ kfree_skb(skb);
+ return 0;
+}
+
void btrtl_free(struct btrtl_device_info *btrtl_dev)
{
kfree(btrtl_dev->fw_data);
@@ -523,7 +609,7 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
struct hci_rp_read_local_version *resp;
char cfg_name[40];
u16 hci_rev, lmp_subver;
- u8 hci_ver;
+ u8 hci_ver, chip_type = 0;
int ret;
btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL);
@@ -548,8 +634,14 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
lmp_subver = le16_to_cpu(resp->lmp_subver);
kfree_skb(skb);
+ if (rtl_has_chip_type(lmp_subver)) {
+ ret = rtl_read_chip_type(hdev, &chip_type);
+ if (ret)
+ goto err_free;
+ }
+
btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev, hci_ver,
- hdev->bus);
+ hdev->bus, chip_type);
if (!btrtl_dev->ic_info) {
rtl_dev_err(hdev, "rtl: unknown IC info, lmp subver %04x, hci rev %04x, hci ver %04x",
@@ -618,6 +710,7 @@ int btrtl_download_firmware(struct hci_dev *hdev,
case RTL_ROM_LMP_8821A:
case RTL_ROM_LMP_8761A:
case RTL_ROM_LMP_8822B:
+ case RTL_ROM_LMP_8703B:
return btrtl_setup_rtl8723b(hdev, btrtl_dev);
default:
rtl_dev_info(hdev, "rtl: assuming no firmware upload needed\n");
@@ -636,7 +729,12 @@ int btrtl_setup_realtek(struct hci_dev *hdev)
return PTR_ERR(btrtl_dev);
ret = btrtl_download_firmware(hdev, btrtl_dev);
+ if (ret)
+ goto out_free;
+ btrtl_apply_quirks(hdev, btrtl_dev);
+
+out_free:
btrtl_free(btrtl_dev);
return ret;
@@ -751,6 +849,24 @@ int btrtl_get_uart_settings(struct hci_dev *hdev,
}
EXPORT_SYMBOL_GPL(btrtl_get_uart_settings);
+void btrtl_apply_quirks(struct hci_dev *hdev,
+ struct btrtl_device_info *btrtl_dev)
+{
+ switch (btrtl_dev->ic_info->lmp_subver) {
+ case RTL_ROM_LMP_8703B:
+ /* 8723CS reports two pages for local ext features,
+ * but it doesn't support any features from page 2 -
+ * it either responds with garbage or with error status
+ */
+ set_bit(HCI_QUIRK_BROKEN_LOCAL_EXT_FTR_MAX_PAGE,
+ &hdev->quirks);
+ break;
+ default:
+ break;
+ }
+}
+EXPORT_SYMBOL_GPL(btrtl_apply_quirks);
+
MODULE_AUTHOR("Daniel Drake <[email protected]>");
MODULE_DESCRIPTION("Bluetooth support for Realtek devices ver " VERSION);
MODULE_VERSION(VERSION);
@@ -760,6 +876,12 @@ MODULE_FIRMWARE("rtl_bt/rtl8723b_fw.bin");
MODULE_FIRMWARE("rtl_bt/rtl8723b_config.bin");
MODULE_FIRMWARE("rtl_bt/rtl8723bs_fw.bin");
MODULE_FIRMWARE("rtl_bt/rtl8723bs_config.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_cg_fw.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_cg_config.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_vf_fw.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_vf_config.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_xx_fw.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8723cs_xx_config.bin");
MODULE_FIRMWARE("rtl_bt/rtl8723ds_fw.bin");
MODULE_FIRMWARE("rtl_bt/rtl8723ds_config.bin");
MODULE_FIRMWARE("rtl_bt/rtl8761a_fw.bin");
diff --git a/drivers/bluetooth/btrtl.h b/drivers/bluetooth/btrtl.h
index f5e36f3993a8..db4e4a1542b7 100644
--- a/drivers/bluetooth/btrtl.h
+++ b/drivers/bluetooth/btrtl.h
@@ -24,6 +24,11 @@
struct btrtl_device_info;
+struct rtl_chip_type_evt {
+ __u8 status;
+ __u8 type;
+} __packed;
+
struct rtl_download_cmd {
__u8 index;
__u8 data[RTL_FRAG_LEN];
@@ -69,6 +74,8 @@ int btrtl_get_uart_settings(struct hci_dev *hdev,
struct btrtl_device_info *btrtl_dev,
unsigned int *controller_baudrate,
u32 *device_baudrate, bool *flow_control);
+void btrtl_apply_quirks(struct hci_dev *hdev,
+ struct btrtl_device_info *btrtl_dev);
#else
@@ -100,6 +107,11 @@ static inline int btrtl_get_uart_settings(struct hci_dev *hdev,
bool *flow_control)
{
return -ENOENT;
+
+static inline void btrtl_apply_quirks(struct hci_dev *hdev,
+ struct btrtl_device_info *btrtl_dev)
+{
+}
}
#endif
diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index 44f848c02c63..ebd84e9d7aed 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -916,6 +916,10 @@ static int h5_btrtl_setup(struct h5 *h5)
err = btrtl_download_firmware(h5->hu->hdev, btrtl_dev);
/* Give the device some time before the hci-core sends it a reset */
usleep_range(10000, 20000);
+ if (err)
+ goto out_free;
+
+ btrtl_apply_quirks(h5->hu->hdev, btrtl_dev);
out_free:
btrtl_free(btrtl_dev);
--
2.20.1
Pinebook has an RTL8723CS WiFi + BT chip, BT is connected to UART1
and uses PL4 as BT reset, PL5 as device wake GPIO, PL6 as host wake GPIO
the I2C controlling signals are connected to R_I2C bus.
Enable it in the device tree.
Signed-off-by: Vasily Khoruzhick <[email protected]>
---
.../boot/dts/allwinner/sun50i-a64-pinebook.dts | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
index d22736a62481..8fc55dfa7034 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
@@ -340,6 +340,20 @@
status = "okay";
};
+&uart1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
+ status = "okay";
+
+ bluetooth {
+ compatible = "realtek,rtl8723cs-bt";
+ reset-gpios = <&r_pio 0 4 GPIO_ACTIVE_LOW>; /* PL4 */
+ device-wake-gpios = <&r_pio 0 5 GPIO_ACTIVE_LOW>; /* PL5 */
+ host-wake-gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
+ firmware-postfix = "pinebook";
+ };
+};
+
&usb_otg {
dr_mode = "host";
};
--
2.20.1
Some adapters (e.g. RTL8723CS) advertise that they have more than
2 pages for local ext features, but they don't support any features
declared in these pages. RTL8723CS reports max_page = 2 and declares
support for sync train and secure connection, but it responds with
either garbage or with error in status on corresponding commands.
Signed-off-by: Vasily Khoruzhick <[email protected]>
---
include/net/bluetooth/hci.h | 7 +++++++
net/bluetooth/hci_event.c | 4 +++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index c36dc1e20556..d7849f128d58 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -192,6 +192,13 @@ enum {
*
*/
HCI_QUIRK_NON_PERSISTENT_SETUP,
+
+ /* When this quirk is set, max_page for local extended features
+ * is set to 1, even if controller reports higher number. Some
+ * controllers (e.g. RTL8723CS) report more pages, but they
+ * don't actually support features declared there.
+ */
+ HCI_QUIRK_BROKEN_LOCAL_EXT_FTR_MAX_PAGE,
};
/* HCI device flags */
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ac2826ce162b..9c59af7de197 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -639,7 +639,9 @@ static void hci_cc_read_local_ext_features(struct hci_dev *hdev,
if (rp->status)
return;
- if (hdev->max_page < rp->max_page)
+ if (!test_bit(HCI_QUIRK_BROKEN_LOCAL_EXT_FTR_MAX_PAGE,
+ &hdev->quirks) &&
+ hdev->max_page < rp->max_page)
hdev->max_page = rp->max_page;
if (rp->page < HCI_MAX_PAGES)
--
2.20.1
On Sat, Jan 19, 2019 at 1:02 AM Vasily Khoruzhick <[email protected]> wrote:
>
> Some boards (e.g. Pine64 and Pinebook) wire a GPIO to reset pin of
> RTL8723BS
Pine64 / Pinebook don't have the enable pin. One could say that the
enable pin and the reset pin you add here are actually the same thing.
ChenYu
> Signed-off-by: Vasily Khoruzhick <[email protected]>
> ---
> drivers/bluetooth/hci_h5.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
> index 069d1c8fde73..7ead8c3d8657 100644
> --- a/drivers/bluetooth/hci_h5.c
> +++ b/drivers/bluetooth/hci_h5.c
> @@ -107,6 +107,7 @@ struct h5 {
> const struct h5_vnd *vnd;
> const char *id;
>
> + struct gpio_desc *reset_gpio;
> struct gpio_desc *enable_gpio;
> struct gpio_desc *device_wake_gpio;
> };
> @@ -833,6 +834,10 @@ static int h5_serdev_probe(struct serdev_device *serdev)
> if (IS_ERR(h5->device_wake_gpio))
> return PTR_ERR(h5->device_wake_gpio);
>
> + h5->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(h5->reset_gpio))
> + return PTR_ERR(h5->reset_gpio);
> +
> return hci_uart_register_device(&h5->serdev_hu, &h5p);
> }
>
> @@ -921,6 +926,9 @@ static void h5_btrtl_open(struct h5 *h5)
>
> /* The controller needs up to 500ms to wakeup */
> gpiod_set_value_cansleep(h5->enable_gpio, 1);
> + /* Take it out of reset */
> + gpiod_set_value_cansleep(h5->reset_gpio, 0);
> + msleep(100);
> gpiod_set_value_cansleep(h5->device_wake_gpio, 1);
> msleep(500);
> }
> @@ -928,6 +936,7 @@ static void h5_btrtl_open(struct h5 *h5)
> static void h5_btrtl_close(struct h5 *h5)
> {
> gpiod_set_value_cansleep(h5->device_wake_gpio, 0);
> + gpiod_set_value_cansleep(h5->reset_gpio, 1);
> gpiod_set_value_cansleep(h5->enable_gpio, 0);
> }
>
> --
> 2.20.1
>
Hi Vasily,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on bluetooth-next/master]
[also build test ERROR on v5.0-rc2 next-20190116]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Vasily-Khoruzhick/Add-support-for-rtl8723bs-rtl8723cs-bluetooth/20190121-190321
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: x86_64-randconfig-x016-201903 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-14) 8.2.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
In file included from drivers/bluetooth/btusb.c:39:
drivers/bluetooth/btrtl.h: In function 'btrtl_get_uart_settings':
>> drivers/bluetooth/btrtl.h:111:20: error: invalid storage class for function 'btrtl_apply_quirks'
static inline void btrtl_apply_quirks(struct hci_dev *hdev,
^~~~~~~~~~~~~~~~~~
>> drivers/bluetooth/btrtl.h:111:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
static inline void btrtl_apply_quirks(struct hci_dev *hdev,
^~~~~~
vim +/btrtl_apply_quirks +111 drivers/bluetooth/btrtl.h
102
103 static inline int btrtl_get_uart_settings(struct hci_dev *hdev,
104 struct btrtl_device_info *btrtl_dev,
105 unsigned int *controller_baudrate,
106 u32 *device_baudrate,
107 bool *flow_control)
108 {
109 return -ENOENT;
110
> 111 static inline void btrtl_apply_quirks(struct hci_dev *hdev,
112 struct btrtl_device_info *btrtl_dev)
113 {
114 }
115 }
116
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Vasily,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on bluetooth-next/master]
[also build test ERROR on v5.0-rc2 next-20190116]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Vasily-Khoruzhick/Add-support-for-rtl8723bs-rtl8723cs-bluetooth/20190121-190321
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: nds32-allyesconfig (attached as .config)
compiler: nds32le-linux-gcc (GCC) 6.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=6.4.0 make.cross ARCH=nds32
All errors (new ones prefixed by >>):
>> drivers/bluetooth/hci_h5.c:1034:12: error: 'h5_btrtl_setup' undeclared here (not in a function)
.setup = h5_btrtl_setup,
^~~~~~~~~~~~~~
>> drivers/bluetooth/hci_h5.c:1035:11: error: 'h5_btrtl_open' undeclared here (not in a function)
.open = h5_btrtl_open,
^~~~~~~~~~~~~
>> drivers/bluetooth/hci_h5.c:1036:12: error: 'h5_btrtl_close' undeclared here (not in a function)
.close = h5_btrtl_close,
^~~~~~~~~~~~~~
vim +/h5_btrtl_setup +1034 drivers/bluetooth/hci_h5.c
1032
1033 static struct h5_vnd rtl8723_of_vnd = {
> 1034 .setup = h5_btrtl_setup,
> 1035 .open = h5_btrtl_open,
> 1036 .close = h5_btrtl_close,
1037 };
1038
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Vasily,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on bluetooth-next/master]
[also build test ERROR on v5.0-rc2 next-20190116]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Vasily-Khoruzhick/Add-support-for-rtl8723bs-rtl8723cs-bluetooth/20190121-190321
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
>> drivers/bluetooth/hci_h5.c:1034:12: error: 'h5_btrtl_setup' undeclared here (not in a function); did you mean 'h5_setup'?
.setup = h5_btrtl_setup,
^~~~~~~~~~~~~~
h5_setup
>> drivers/bluetooth/hci_h5.c:1035:11: error: 'h5_btrtl_open' undeclared here (not in a function); did you mean 'h5_open'?
.open = h5_btrtl_open,
^~~~~~~~~~~~~
h5_open
>> drivers/bluetooth/hci_h5.c:1036:12: error: 'h5_btrtl_close' undeclared here (not in a function); did you mean 'h5_close'?
.close = h5_btrtl_close,
^~~~~~~~~~~~~~
h5_close
vim +1034 drivers/bluetooth/hci_h5.c
1032
1033 static struct h5_vnd rtl8723_of_vnd = {
> 1034 .setup = h5_btrtl_setup,
> 1035 .open = h5_btrtl_open,
> 1036 .close = h5_btrtl_close,
1037 };
1038
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
> Add binding document for bluetooth part of RTL8723BS/RTL8723CS
>
> Signed-off-by: Vasily Khoruzhick <[email protected]>
> ---
> .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
> 1 file changed, 35 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>
> diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> new file mode 100644
> index 000000000000..8357f242ae4c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> @@ -0,0 +1,35 @@
> +RTL8723BS/RTL8723CS Bluetooth
> +---------------------
> +
> +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
> +BT is connected over serial. It speaks H5 protocol with few extra commands
> +to upload firmware and change module speed.
> +
> +Required properties:
> +
> + - compatible: should be one of the following:
> + * "realtek,rtl8723bs-bt"
> + * "realtek,rtl8723cs-bt"
> +Optional properties:
> +
> + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
> + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
> + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
> + - firmware-postfix: firmware postfix to be used for firmware config
How is this used?
Rob
On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <[email protected]> wrote:
>
> On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
> > Add binding document for bluetooth part of RTL8723BS/RTL8723CS
> >
> > Signed-off-by: Vasily Khoruzhick <[email protected]>
> > ---
> > .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
> > 1 file changed, 35 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> >
> > diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > new file mode 100644
> > index 000000000000..8357f242ae4c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > @@ -0,0 +1,35 @@
> > +RTL8723BS/RTL8723CS Bluetooth
> > +---------------------
> > +
> > +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
> > +BT is connected over serial. It speaks H5 protocol with few extra commands
> > +to upload firmware and change module speed.
> > +
> > +Required properties:
> > +
> > + - compatible: should be one of the following:
> > + * "realtek,rtl8723bs-bt"
> > + * "realtek,rtl8723cs-bt"
> > +Optional properties:
> > +
> > + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
> > + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
> > + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
> > + - firmware-postfix: firmware postfix to be used for firmware config
>
> How is this used?
rtl8723bs-bt needs 2 firmware binaries -- one is actual firmware,
another is firmware config which is specific to the board. If
firmware-postfix is specified, driver appends it to the name of config
and requests board-specific config while loading firmware. I.e. if
'pine64' is specified as firmware-postfix driver will load
rtl8723bs_config-pine64.bin.
> Rob
>
Hi Vasily,
> Vasily Khoruzhick <[email protected]> hat am 18. Februar 2019 um 22:24 geschrieben:
>
>
> On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <[email protected]> wrote:
> >
> > On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
> > > Add binding document for bluetooth part of RTL8723BS/RTL8723CS
> > >
> > > Signed-off-by: Vasily Khoruzhick <[email protected]>
> > > ---
> > > .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
> > > 1 file changed, 35 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > new file mode 100644
> > > index 000000000000..8357f242ae4c
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > @@ -0,0 +1,35 @@
> > > +RTL8723BS/RTL8723CS Bluetooth
> > > +---------------------
> > > +
> > > +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
> > > +BT is connected over serial. It speaks H5 protocol with few extra commands
> > > +to upload firmware and change module speed.
> > > +
> > > +Required properties:
> > > +
> > > + - compatible: should be one of the following:
> > > + * "realtek,rtl8723bs-bt"
> > > + * "realtek,rtl8723cs-bt"
> > > +Optional properties:
> > > +
> > > + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
> > > + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
> > > + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
> > > + - firmware-postfix: firmware postfix to be used for firmware config
> >
sorry, i didn't noticed your great series before. David and i working at the same stuff but for the Asus Tinker Board.
I created a similiar yet untested patch version for hci_h5 [1]. Maybe it's useful.
Just a comment about the binding. It's really necessary to add the reset-gpio? Can't we use the enable-gpio with inverse polarity for this?
Stefan
> Stefan Wahren <[email protected]> hat am 18. Februar 2019 um 23:08 geschrieben:
>
>
> Hi Vasily,
>
> > Vasily Khoruzhick <[email protected]> hat am 18. Februar 2019 um 22:24 geschrieben:
> >
> >
> > On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <[email protected]> wrote:
> > >
> > > On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
> > > > Add binding document for bluetooth part of RTL8723BS/RTL8723CS
> > > >
> > > > Signed-off-by: Vasily Khoruzhick <[email protected]>
> > > > ---
> > > > .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
> > > > 1 file changed, 35 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > > new file mode 100644
> > > > index 000000000000..8357f242ae4c
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > > @@ -0,0 +1,35 @@
> > > > +RTL8723BS/RTL8723CS Bluetooth
> > > > +---------------------
> > > > +
> > > > +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
> > > > +BT is connected over serial. It speaks H5 protocol with few extra commands
> > > > +to upload firmware and change module speed.
> > > > +
> > > > +Required properties:
> > > > +
> > > > + - compatible: should be one of the following:
> > > > + * "realtek,rtl8723bs-bt"
> > > > + * "realtek,rtl8723cs-bt"
> > > > +Optional properties:
> > > > +
> > > > + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
> > > > + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
> > > > + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
> > > > + - firmware-postfix: firmware postfix to be used for firmware config
> > >
>
> sorry, i didn't noticed your great series before. David and i working at the same stuff but for the Asus Tinker Board.
>
> I created a similiar yet untested patch version for hci_h5 [1]. Maybe it's useful.
>
> Just a comment about the binding. It's really necessary to add the reset-gpio? Can't we use the enable-gpio with inverse polarity for this?
>
> Stefan
[1] - https://gist.github.com/lategoodbye/79bac99d4f1158a719a48ea3c45eb7f1
On Mon, Feb 18, 2019 at 2:08 PM Stefan Wahren <[email protected]> wrote:
>
> Hi Vasily,
Hi Stefan,
> > Vasily Khoruzhick <[email protected]> hat am 18. Februar 2019 um 22:24 geschrieben:
> >
> >
> > On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <[email protected]> wrote:
> > >
> > > On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
> > > > Add binding document for bluetooth part of RTL8723BS/RTL8723CS
> > > >
> > > > Signed-off-by: Vasily Khoruzhick <[email protected]>
> > > > ---
> > > > .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
> > > > 1 file changed, 35 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > > new file mode 100644
> > > > index 000000000000..8357f242ae4c
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > > @@ -0,0 +1,35 @@
> > > > +RTL8723BS/RTL8723CS Bluetooth
> > > > +---------------------
> > > > +
> > > > +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
> > > > +BT is connected over serial. It speaks H5 protocol with few extra commands
> > > > +to upload firmware and change module speed.
> > > > +
> > > > +Required properties:
> > > > +
> > > > + - compatible: should be one of the following:
> > > > + * "realtek,rtl8723bs-bt"
> > > > + * "realtek,rtl8723cs-bt"
> > > > +Optional properties:
> > > > +
> > > > + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
> > > > + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
> > > > + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
> > > > + - firmware-postfix: firmware postfix to be used for firmware config
> > >
>
> sorry, i didn't noticed your great series before. David and i working at the same stuff but for the Asus Tinker Board.
>
> I created a similiar yet untested patch version for hci_h5 [1]. Maybe it's useful.
Looks good to me, but you may need to add firmware-postfix.
> Just a comment about the binding. It's really necessary to add the reset-gpio? Can't we use the enable-gpio with inverse polarity for this?
Yes, we can use enable-gpio instead of reset-gpio on pine64 and pinebook.
> Stefan
Hi Vasily,
Am 18.02.19 um 22:24 schrieb Vasily Khoruzhick:
> On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <[email protected]> wrote:
>> On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
>>> Add binding document for bluetooth part of RTL8723BS/RTL8723CS
>>>
>>> Signed-off-by: Vasily Khoruzhick <[email protected]>
>>> ---
>>> .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
>>> 1 file changed, 35 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>> new file mode 100644
>>> index 000000000000..8357f242ae4c
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>> @@ -0,0 +1,35 @@
>>> +RTL8723BS/RTL8723CS Bluetooth
>>> +---------------------
>>> +
>>> +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
>>> +BT is connected over serial. It speaks H5 protocol with few extra commands
>>> +to upload firmware and change module speed.
>>> +
>>> +Required properties:
>>> +
>>> + - compatible: should be one of the following:
>>> + * "realtek,rtl8723bs-bt"
>>> + * "realtek,rtl8723cs-bt"
>>> +Optional properties:
>>> +
>>> + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
>>> + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
>>> + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
>>> + - firmware-postfix: firmware postfix to be used for firmware config
>> How is this used?
> rtl8723bs-bt needs 2 firmware binaries -- one is actual firmware,
> another is firmware config which is specific to the board. If
> firmware-postfix is specified, driver appends it to the name of config
> and requests board-specific config while loading firmware. I.e. if
> 'pine64' is specified as firmware-postfix driver will load
> rtl8723bs_config-pine64.bin.
this is a common problem (e.g. brcmfmac has the same problem [1]).
Why not using the "model" property instead of inventing a new software
specific property?
[1] - https://patchwork.kernel.org/patch/10764463/
>
>> Rob
>>
> _
On Mon, Feb 18, 2019 at 3:24 PM Vasily Khoruzhick <[email protected]> wrote:
>
> On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <[email protected]> wrote:
> >
> > On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
> > > Add binding document for bluetooth part of RTL8723BS/RTL8723CS
> > >
> > > Signed-off-by: Vasily Khoruzhick <[email protected]>
> > > ---
> > > .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
> > > 1 file changed, 35 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > new file mode 100644
> > > index 000000000000..8357f242ae4c
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > @@ -0,0 +1,35 @@
> > > +RTL8723BS/RTL8723CS Bluetooth
> > > +---------------------
> > > +
> > > +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
> > > +BT is connected over serial. It speaks H5 protocol with few extra commands
> > > +to upload firmware and change module speed.
> > > +
> > > +Required properties:
> > > +
> > > + - compatible: should be one of the following:
> > > + * "realtek,rtl8723bs-bt"
> > > + * "realtek,rtl8723cs-bt"
> > > +Optional properties:
> > > +
> > > + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
> > > + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
> > > + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
> > > + - firmware-postfix: firmware postfix to be used for firmware config
> >
> > How is this used?
>
> rtl8723bs-bt needs 2 firmware binaries -- one is actual firmware,
> another is firmware config which is specific to the board. If
> firmware-postfix is specified, driver appends it to the name of config
> and requests board-specific config while loading firmware. I.e. if
> 'pine64' is specified as firmware-postfix driver will load
> rtl8723bs_config-pine64.bin.
We already have 'firmware-name' defined and I'd prefer not to have
another way to do things. The difference is just you have to give the
full filename.
Also, on other chips with board specific config blobs, there's been
some discussion of converting those to DT properties. That depends on
you knowing what's in the blob and having a reasonable number of
parameter to make properties.
Rob
On Mon, Feb 18, 2019 at 4:28 PM Vasily Khoruzhick <[email protected]> wrote:
>
> On Mon, Feb 18, 2019 at 2:08 PM Stefan Wahren <[email protected]> wrote:
> >
> > Hi Vasily,
>
> Hi Stefan,
>
> > > Vasily Khoruzhick <[email protected]> hat am 18. Februar 2019 um 22:24 geschrieben:
> > >
> > >
> > > On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <[email protected]> wrote:
> > > >
> > > > On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
> > > > > Add binding document for bluetooth part of RTL8723BS/RTL8723CS
> > > > >
> > > > > Signed-off-by: Vasily Khoruzhick <[email protected]>
> > > > > ---
> > > > > .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
> > > > > 1 file changed, 35 insertions(+)
> > > > > create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > > > new file mode 100644
> > > > > index 000000000000..8357f242ae4c
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > > > @@ -0,0 +1,35 @@
> > > > > +RTL8723BS/RTL8723CS Bluetooth
> > > > > +---------------------
> > > > > +
> > > > > +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
> > > > > +BT is connected over serial. It speaks H5 protocol with few extra commands
> > > > > +to upload firmware and change module speed.
> > > > > +
> > > > > +Required properties:
> > > > > +
> > > > > + - compatible: should be one of the following:
> > > > > + * "realtek,rtl8723bs-bt"
> > > > > + * "realtek,rtl8723cs-bt"
> > > > > +Optional properties:
> > > > > +
> > > > > + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
> > > > > + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
> > > > > + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
> > > > > + - firmware-postfix: firmware postfix to be used for firmware config
> > > >
> >
> > sorry, i didn't noticed your great series before. David and i working at the same stuff but for the Asus Tinker Board.
> >
> > I created a similiar yet untested patch version for hci_h5 [1]. Maybe it's useful.
>
> Looks good to me, but you may need to add firmware-postfix.
>
> > Just a comment about the binding. It's really necessary to add the reset-gpio? Can't we use the enable-gpio with inverse polarity for this?
>
> Yes, we can use enable-gpio instead of reset-gpio on pine64 and pinebook.
Then why do we have both? Reset and enable are distinct. The inverse
of enable-gpios is typically powerdown-gpios, not reset-gpios.
Rob
On 19/02/2019 14:17, Rob Herring wrote:
> On Mon, Feb 18, 2019 at 4:28 PM Vasily Khoruzhick <[email protected]> wrote:
>> On Mon, Feb 18, 2019 at 2:08 PM Stefan Wahren <[email protected]> wrote:
>>> Hi Vasily,
>> Hi Stefan,
>>
>>>> Vasily Khoruzhick <[email protected]> hat am 18. Februar 2019 um 22:24 geschrieben:
>>>>
>>>>
>>>> On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <[email protected]> wrote:
>>>>> On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
>>>>>> Add binding document for bluetooth part of RTL8723BS/RTL8723CS
>>>>>>
>>>>>> Signed-off-by: Vasily Khoruzhick <[email protected]>
>>>>>> ---
>>>>>> .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
>>>>>> 1 file changed, 35 insertions(+)
>>>>>> create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>>> new file mode 100644
>>>>>> index 000000000000..8357f242ae4c
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>>> @@ -0,0 +1,35 @@
>>>>>> +RTL8723BS/RTL8723CS Bluetooth
>>>>>> +---------------------
>>>>>> +
>>>>>> +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
>>>>>> +BT is connected over serial. It speaks H5 protocol with few extra commands
>>>>>> +to upload firmware and change module speed.
>>>>>> +
>>>>>> +Required properties:
>>>>>> +
>>>>>> + - compatible: should be one of the following:
>>>>>> + * "realtek,rtl8723bs-bt"
>>>>>> + * "realtek,rtl8723cs-bt"
>>>>>> +Optional properties:
>>>>>> +
>>>>>> + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
>>>>>> + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
>>>>>> + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
>>>>>> + - firmware-postfix: firmware postfix to be used for firmware config
>>> sorry, i didn't noticed your great series before. David and i working at the same stuff but for the Asus Tinker Board.
>>>
>>> I created a similiar yet untested patch version for hci_h5 [1]. Maybe it's useful.
>> Looks good to me, but you may need to add firmware-postfix.
>>
>>> Just a comment about the binding. It's really necessary to add the reset-gpio? Can't we use the enable-gpio with inverse polarity for this?
>> Yes, we can use enable-gpio instead of reset-gpio on pine64 and pinebook.
> Then why do we have both? Reset and enable are distinct. The inverse
> of enable-gpios is typically powerdown-gpios, not reset-gpios.
>
> Rob
Both data sheets that I know:
http://cit.odessa.ua/media/pdf/Intel-Compute-Stick/FN-Link_F23BDSM25-W1.pdf
http://files.pine64.org/doc/datasheet/pine64/RTL8723BS.pdf
BT_RST_N BT Reset IN / BT_DIS# General Purpose I/O Pin
So from the datasheet there is only one pin. And from its name it sounds
like reset.
This said though the datasheets of these Realtek devices are a bit thin ....
On Tue, Feb 19, 2019 at 6:14 AM Rob Herring <[email protected]> wrote:
>
> On Mon, Feb 18, 2019 at 3:24 PM Vasily Khoruzhick <[email protected]> wrote:
> >
> > On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <[email protected]> wrote:
> > >
> > > On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
> > > > Add binding document for bluetooth part of RTL8723BS/RTL8723CS
> > > >
> > > > Signed-off-by: Vasily Khoruzhick <[email protected]>
> > > > ---
> > > > .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
> > > > 1 file changed, 35 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > > new file mode 100644
> > > > index 000000000000..8357f242ae4c
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> > > > @@ -0,0 +1,35 @@
> > > > +RTL8723BS/RTL8723CS Bluetooth
> > > > +---------------------
> > > > +
> > > > +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
> > > > +BT is connected over serial. It speaks H5 protocol with few extra commands
> > > > +to upload firmware and change module speed.
> > > > +
> > > > +Required properties:
> > > > +
> > > > + - compatible: should be one of the following:
> > > > + * "realtek,rtl8723bs-bt"
> > > > + * "realtek,rtl8723cs-bt"
> > > > +Optional properties:
> > > > +
> > > > + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
> > > > + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
> > > > + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
> > > > + - firmware-postfix: firmware postfix to be used for firmware config
> > >
> > > How is this used?
> >
> > rtl8723bs-bt needs 2 firmware binaries -- one is actual firmware,
> > another is firmware config which is specific to the board. If
> > firmware-postfix is specified, driver appends it to the name of config
> > and requests board-specific config while loading firmware. I.e. if
> > 'pine64' is specified as firmware-postfix driver will load
> > rtl8723bs_config-pine64.bin.
>
> We already have 'firmware-name' defined and I'd prefer not to have
> another way to do things. The difference is just you have to give the
> full filename.
OK
> Also, on other chips with board specific config blobs, there's been
> some discussion of converting those to DT properties. That depends on
> you knowing what's in the blob and having a reasonable number of
> parameter to make properties.
We'd have to RE binary format of this config first.
> Rob
On Wed, Feb 20, 2019 at 5:09 AM David Summers
<[email protected]> wrote:
>
> On 19/02/2019 14:17, Rob Herring wrote:
> > On Mon, Feb 18, 2019 at 4:28 PM Vasily Khoruzhick <[email protected]> wrote:
> >> On Mon, Feb 18, 2019 at 2:08 PM Stefan Wahren <[email protected]> wrote:
> >>> Hi Vasily,
> >> Hi Stefan,
> >>
> >>>> Vasily Khoruzhick <[email protected]> hat am 18. Februar 2019 um 22:24 geschrieben:
> >>>>
> >>>>
> >>>> On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <[email protected]> wrote:
> >>>>> On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
> >>>>>> Add binding document for bluetooth part of RTL8723BS/RTL8723CS
> >>>>>>
> >>>>>> Signed-off-by: Vasily Khoruzhick <[email protected]>
> >>>>>> ---
> >>>>>> .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
> >>>>>> 1 file changed, 35 insertions(+)
> >>>>>> create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> >>>>>>
> >>>>>> diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> >>>>>> new file mode 100644
> >>>>>> index 000000000000..8357f242ae4c
> >>>>>> --- /dev/null
> >>>>>> +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> >>>>>> @@ -0,0 +1,35 @@
> >>>>>> +RTL8723BS/RTL8723CS Bluetooth
> >>>>>> +---------------------
> >>>>>> +
> >>>>>> +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
> >>>>>> +BT is connected over serial. It speaks H5 protocol with few extra commands
> >>>>>> +to upload firmware and change module speed.
> >>>>>> +
> >>>>>> +Required properties:
> >>>>>> +
> >>>>>> + - compatible: should be one of the following:
> >>>>>> + * "realtek,rtl8723bs-bt"
> >>>>>> + * "realtek,rtl8723cs-bt"
> >>>>>> +Optional properties:
> >>>>>> +
> >>>>>> + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
> >>>>>> + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
> >>>>>> + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
> >>>>>> + - firmware-postfix: firmware postfix to be used for firmware config
> >>> sorry, i didn't noticed your great series before. David and i working at the same stuff but for the Asus Tinker Board.
> >>>
> >>> I created a similiar yet untested patch version for hci_h5 [1]. Maybe it's useful.
> >> Looks good to me, but you may need to add firmware-postfix.
> >>
> >>> Just a comment about the binding. It's really necessary to add the reset-gpio? Can't we use the enable-gpio with inverse polarity for this?
> >> Yes, we can use enable-gpio instead of reset-gpio on pine64 and pinebook.
> > Then why do we have both? Reset and enable are distinct. The inverse
> > of enable-gpios is typically powerdown-gpios, not reset-gpios.
> >
> > Rob
>
> Both data sheets that I know:
>
> http://cit.odessa.ua/media/pdf/Intel-Compute-Stick/FN-Link_F23BDSM25-W1.pdf
> http://files.pine64.org/doc/datasheet/pine64/RTL8723BS.pdf
>
> BT_RST_N BT Reset IN / BT_DIS# General Purpose I/O Pin
>
> So from the datasheet there is only one pin. And from its name it sounds
> like reset.
Yes there's only one pin. Would be nice if we could use "enable" though.
That is what ACPI uses.
> This said though the datasheets of these Realtek devices are a bit thin ....
These datasheets are for the integrated modules (the chip plus passive
components).
I don't think the datasheet for the chip itself is public.
ChenYu
Hi Vasily,
Just catching up with this series - good that you are doing it. We need
it for users on ArchLinux ARM!
On question though, what is "firmware-postfix" used for? I see in the
documentation, that it points to the board name. But can't see where
else it is used.
Is there a need to have the board name?
Anyway good work.
David.
On 18/02/2019 22:28, Vasily Khoruzhick wrote:
> On Mon, Feb 18, 2019 at 2:08 PM Stefan Wahren <[email protected]> wrote:
>> Hi Vasily,
> Hi Stefan,
>
>>> Vasily Khoruzhick <[email protected]> hat am 18. Februar 2019 um 22:24 geschrieben:
>>>
>>>
>>> On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <[email protected]> wrote:
>>>> On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
>>>>> Add binding document for bluetooth part of RTL8723BS/RTL8723CS
>>>>>
>>>>> Signed-off-by: Vasily Khoruzhick <[email protected]>
>>>>> ---
>>>>> .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
>>>>> 1 file changed, 35 insertions(+)
>>>>> create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>> new file mode 100644
>>>>> index 000000000000..8357f242ae4c
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>> @@ -0,0 +1,35 @@
>>>>> +RTL8723BS/RTL8723CS Bluetooth
>>>>> +---------------------
>>>>> +
>>>>> +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
>>>>> +BT is connected over serial. It speaks H5 protocol with few extra commands
>>>>> +to upload firmware and change module speed.
>>>>> +
>>>>> +Required properties:
>>>>> +
>>>>> + - compatible: should be one of the following:
>>>>> + * "realtek,rtl8723bs-bt"
>>>>> + * "realtek,rtl8723cs-bt"
>>>>> +Optional properties:
>>>>> +
>>>>> + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
>>>>> + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
>>>>> + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
>>>>> + - firmware-postfix: firmware postfix to be used for firmware config
>> sorry, i didn't noticed your great series before. David and i working at the same stuff but for the Asus Tinker Board.
>>
>> I created a similiar yet untested patch version for hci_h5 [1]. Maybe it's useful.
> Looks good to me, but you may need to add firmware-postfix.
>
>> Just a comment about the binding. It's really necessary to add the reset-gpio? Can't we use the enable-gpio with inverse polarity for this?
> Yes, we can use enable-gpio instead of reset-gpio on pine64 and pinebook.
>
>> Stefan
On Fri, Feb 22, 2019 at 11:14 AM David Summers
<[email protected]> wrote:
>
> Hi Vasily,
>
> Just catching up with this series - good that you are doing it. We need
> it for users on ArchLinux ARM!
>
> On question though, what is "firmware-postfix" used for? I see in the
> documentation, that it points to the board name. But can't see where
> else it is used.
See h5_serdev_probe() in drivers/bluetooth/hci_h5.c. Basically it
specifies what firmware config to use.
I agree with Rob that we should probably use firmware-name here instead.
> Is there a need to have the board name?
As far as I understand firmware config depends on board, so I think
it's a good idea to use board name here.
> Anyway good work.
Thanks!
>
> David.
>
> On 18/02/2019 22:28, Vasily Khoruzhick wrote:
> > On Mon, Feb 18, 2019 at 2:08 PM Stefan Wahren <[email protected]> wrote:
> >> Hi Vasily,
> > Hi Stefan,
> >
> >>> Vasily Khoruzhick <[email protected]> hat am 18. Februar 2019 um 22:24 geschrieben:
> >>>
> >>>
> >>> On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <[email protected]> wrote:
> >>>> On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
> >>>>> Add binding document for bluetooth part of RTL8723BS/RTL8723CS
> >>>>>
> >>>>> Signed-off-by: Vasily Khoruzhick <[email protected]>
> >>>>> ---
> >>>>> .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
> >>>>> 1 file changed, 35 insertions(+)
> >>>>> create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> >>>>> new file mode 100644
> >>>>> index 000000000000..8357f242ae4c
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
> >>>>> @@ -0,0 +1,35 @@
> >>>>> +RTL8723BS/RTL8723CS Bluetooth
> >>>>> +---------------------
> >>>>> +
> >>>>> +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
> >>>>> +BT is connected over serial. It speaks H5 protocol with few extra commands
> >>>>> +to upload firmware and change module speed.
> >>>>> +
> >>>>> +Required properties:
> >>>>> +
> >>>>> + - compatible: should be one of the following:
> >>>>> + * "realtek,rtl8723bs-bt"
> >>>>> + * "realtek,rtl8723cs-bt"
> >>>>> +Optional properties:
> >>>>> +
> >>>>> + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
> >>>>> + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
> >>>>> + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
> >>>>> + - firmware-postfix: firmware postfix to be used for firmware config
> >> sorry, i didn't noticed your great series before. David and i working at the same stuff but for the Asus Tinker Board.
> >>
> >> I created a similiar yet untested patch version for hci_h5 [1]. Maybe it's useful.
> > Looks good to me, but you may need to add firmware-postfix.
> >
> >> Just a comment about the binding. It's really necessary to add the reset-gpio? Can't we use the enable-gpio with inverse polarity for this?
> > Yes, we can use enable-gpio instead of reset-gpio on pine64 and pinebook.
> >
> >> Stefan
>
>
On 22/02/2019 19:21, Vasily Khoruzhick wrote:
> On Fri, Feb 22, 2019 at 11:14 AM David Summers
> <[email protected]> wrote:
>> Hi Vasily,
>>
>> Just catching up with this series - good that you are doing it. We need
>> it for users on ArchLinux ARM!
>>
>> On question though, what is "firmware-postfix" used for? I see in the
>> documentation, that it points to the board name. But can't see where
>> else it is used.
> See h5_serdev_probe() in drivers/bluetooth/hci_h5.c. Basically it
> specifies what firmware config to use.
> I agree with Rob that we should probably use firmware-name here instead.
Ah OK followed it. its sets h5->id than in h5_btrtl_setup (in hci_h5)
calls up btrtl_initialise (in btrtl.c) and that gives the file to load.
Guess now need to trace what firmware files are available.
Oh yes - see why you called it postfix, as that is the name given in
btrl_initialise. I can't comment on what name is best, but I can see the
sense of keeping the device tree hook the same as used as in
btrtl_initialise.
>> Is there a need to have the board name?
> As far as I understand firmware config depends on board, so I think
> it's a good idea to use board name here.
>
>> Anyway good work.
> Thanks!
>
>> David.
>>
>> On 18/02/2019 22:28, Vasily Khoruzhick wrote:
>>> On Mon, Feb 18, 2019 at 2:08 PM Stefan Wahren <[email protected]> wrote:
>>>> Hi Vasily,
>>> Hi Stefan,
>>>
>>>>> Vasily Khoruzhick <[email protected]> hat am 18. Februar 2019 um 22:24 geschrieben:
>>>>>
>>>>>
>>>>> On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <[email protected]> wrote:
>>>>>> On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
>>>>>>> Add binding document for bluetooth part of RTL8723BS/RTL8723CS
>>>>>>>
>>>>>>> Signed-off-by: Vasily Khoruzhick <[email protected]>
>>>>>>> ---
>>>>>>> .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
>>>>>>> 1 file changed, 35 insertions(+)
>>>>>>> create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..8357f242ae4c
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>>>> @@ -0,0 +1,35 @@
>>>>>>> +RTL8723BS/RTL8723CS Bluetooth
>>>>>>> +---------------------
>>>>>>> +
>>>>>>> +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
>>>>>>> +BT is connected over serial. It speaks H5 protocol with few extra commands
>>>>>>> +to upload firmware and change module speed.
>>>>>>> +
>>>>>>> +Required properties:
>>>>>>> +
>>>>>>> + - compatible: should be one of the following:
>>>>>>> + * "realtek,rtl8723bs-bt"
>>>>>>> + * "realtek,rtl8723cs-bt"
>>>>>>> +Optional properties:
>>>>>>> +
>>>>>>> + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
>>>>>>> + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
>>>>>>> + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
>>>>>>> + - firmware-postfix: firmware postfix to be used for firmware config
>>>> sorry, i didn't noticed your great series before. David and i working at the same stuff but for the Asus Tinker Board.
>>>>
>>>> I created a similiar yet untested patch version for hci_h5 [1]. Maybe it's useful.
>>> Looks good to me, but you may need to add firmware-postfix.
>>>
>>>> Just a comment about the binding. It's really necessary to add the reset-gpio? Can't we use the enable-gpio with inverse polarity for this?
>>> Yes, we can use enable-gpio instead of reset-gpio on pine64 and pinebook.
>>>
>>>> Stefan
>>
Hi Vasily, Hi Alex,
On 22/02/2019 11:21, Vasily Khoruzhick wrote:
> I agree with Rob that we should probably use firmware-name here instead.
Have you considered skipping this property for v1 of this series?
We can still add that property (as optional one) later on if we really
see the need for it. (The btrtl code should already support the case
where NULL is passed as "postfix")
I checked the public rtl8723bs_bt [0] and rtl8723ds_bt [1] git repos and
they each contain only one config blob. The blob from the rtl8723bs_bt
repo worked on my two Amlogic boards (data only, sound input/output not
tested), even though Amlogic seems to ship different blobs: [2]
>> Is there a need to have the board name?
>
> As far as I understand firmware config depends on board, so I think
> it's a good idea to use board name here.
I also added Alex Lu from Realtek / Realsil to this email.
Alex, I hope that you can help us with the "Bluetooth config" format for
the Realtek WiFi and Bluetooth combo chips - mainly the ones which
connect to the host using SDIO.
This is important for us because the question came up whether we can
describe everything that's part of the "config blob" as device-tree
properties. If we knew the format we could generate the "config blob"
on-the-fly (either by fully generating it, taking a blob - maybe with
only the smallest set of config data - as "template" and update values
on-the-fly, etc.)
Marcel wrote a tool [3] which handles the basic config format. However,
we're still missing a lot of details (only 3 offsets are known,
"UART_CONFIG" contains 16 bytes but we only know the purpose of 4 of
these, ...).
I would highly appreciate if you give us enough details so we can extend
Marcel's tool to display the human-readable representation of the config
blobs from rtl8723bs_bt [0] and rtl8723ds_bt [1].
Vasily, thank you for your effort on this topic so far!
If you keep me CC'ed on v2 of your series then I can test it on two of
my Amlogic boards (which come with a RTL8723BS).
[0] https://github.com/lwfinger/rtl8723bs_bt/tree/09eb91f52a639ec5e4c5c4c98dc2afede046cf20
[1] https://github.com/ayufan-pine64/rtl8723ds_bt/tree/fab21b52250d67857b694f961e1ff8618e678d89/8723D
[2] https://github.com/khadas/android_hardware_realtek/tree/bd3b113266c353aafcbf528a0334d28090ff249b/rtkbt/system/etc/firmware
[3] https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/rtlfw.c?id=261948090e9073514ac4b5f64c8715cf0a71eafa
Hi Martin,
Thanks for your information.
The config is related to eFuse in chips. I'm sorry that the details can't be open.
Only some special configurations are related to the host platforms, such as UART working baudrate, hardware flow control, PCM settings, etc. These are the settings for HCI UART and PCM Interface.
Most of configurations are only relevant to chips.
Thanks,
BRs,
Alex Lu.
________________________________________
??????: Martin Blumenstingl [[email protected]]
????ʱ??: 2019??3??2?? 4:15
?ռ???: [email protected]; ½??ΰ
????: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Martin Blumenstingl
????: Re: [PATCH 3/8] dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth
Hi Vasily, Hi Alex,
On 22/02/2019 11:21, Vasily Khoruzhick wrote:
> I agree with Rob that we should probably use firmware-name here instead.
Have you considered skipping this property for v1 of this series?
We can still add that property (as optional one) later on if we really
see the need for it. (The btrtl code should already support the case
where NULL is passed as "postfix")
I checked the public rtl8723bs_bt [0] and rtl8723ds_bt [1] git repos and
they each contain only one config blob. The blob from the rtl8723bs_bt
repo worked on my two Amlogic boards (data only, sound input/output not
tested), even though Amlogic seems to ship different blobs: [2]
>> Is there a need to have the board name?
>
> As far as I understand firmware config depends on board, so I think
> it's a good idea to use board name here.
I also added Alex Lu from Realtek / Realsil to this email.
Alex, I hope that you can help us with the "Bluetooth config" format for
the Realtek WiFi and Bluetooth combo chips - mainly the ones which
connect to the host using SDIO.
This is important for us because the question came up whether we can
describe everything that's part of the "config blob" as device-tree
properties. If we knew the format we could generate the "config blob"
on-the-fly (either by fully generating it, taking a blob - maybe with
only the smallest set of config data - as "template" and update values
on-the-fly, etc.)
Marcel wrote a tool [3] which handles the basic config format. However,
we're still missing a lot of details (only 3 offsets are known,
"UART_CONFIG" contains 16 bytes but we only know the purpose of 4 of
these, ...).
I would highly appreciate if you give us enough details so we can extend
Marcel's tool to display the human-readable representation of the config
blobs from rtl8723bs_bt [0] and rtl8723ds_bt [1].
Vasily, thank you for your effort on this topic so far!
If you keep me CC'ed on v2 of your series then I can test it on two of
my Amlogic boards (which come with a RTL8723BS).
[0] https://github.com/lwfinger/rtl8723bs_bt/tree/09eb91f52a639ec5e4c5c4c98dc2afede046cf20
[1] https://github.com/ayufan-pine64/rtl8723ds_bt/tree/fab21b52250d67857b694f961e1ff8618e678d89/8723D
[2] https://github.com/khadas/android_hardware_realtek/tree/bd3b113266c353aafcbf528a0334d28090ff249b/rtkbt/system/etc/firmware
[3] https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/rtlfw.c?id=261948090e9073514ac4b5f64c8715cf0a71eafa
Hi Alex,
On Sat, Mar 2, 2019 at 10:30 AM 陆朱伟 <[email protected]> wrote:
>
> Hi Martin,
> Thanks for your information.
thank you for the quick reply!
> The config is related to eFuse in chips. I'm sorry that the details can't be open.
> Only some special configurations are related to the host platforms, such as UART working baudrate, hardware flow control, PCM settings, etc. These are the settings for HCI UART and PCM Interface.
> Most of configurations are only relevant to chips.
please let me repeat this with my own words to see if I understand the
"config blob" correctly. Feel free to correct me if anything is wrong:
- the data in the config blob "patches" eFuse values at runtime (non-persistent)
- we know the offsets of the UART_CONFIG, PCM_SETTING and BD_ADDR -
these can be "board specific" (like baud rate, flow control, ...)
- most other values from the "config blob" are "chip specific" -
meaning they are identical for each board and they only depend on the
chip
do you have any suggestions how we can support multiple boards (the
main goal of this whole discussion is: how to do this "correct")?
so far different approaches were discussed (not only in this thread,
but also in the past):
- use a separate config blob for each board. this means that whenever
a new board is supported (for example by adding a .dts for it to the
mainline kernel) then Bluetooth won't work out-of-the-box unless a
config is provided.
- specify the properties of the connection to the chip (for example by
adding a device-tree property for the speed, flow control, ...) and
generate the config based on the device-tree properties.
- (I am open for other suggestions, please let us know if you have any)
I would like to hear your opinion on this topic and especially the
reasons behind your suggestions.
Best regards
Martin
Hi Martin,
You're right about the config blob.
Thanks for your invitation to the discussion.
In my option, specifying the properties of the connection to the chip would be better.
Thanks,
BRs,
Alex Lu.
-----邮件原件-----
发件人: Martin Blumenstingl [mailto:[email protected]]
发送时间: 2019年3月3日 0:44
收件人: 陆朱伟
抄送: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
主题: Re: [PATCH 3/8] dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth
Hi Alex,
On Sat, Mar 2, 2019 at 10:30 AM 陆朱伟 <[email protected]> wrote:
>
> Hi Martin,
> Thanks for your information.
thank you for the quick reply!
> The config is related to eFuse in chips. I'm sorry that the details can't be open.
> Only some special configurations are related to the host platforms, such as UART working baudrate, hardware flow control, PCM settings, etc. These are the settings for HCI UART and PCM Interface.
> Most of configurations are only relevant to chips.
please let me repeat this with my own words to see if I understand the
"config blob" correctly. Feel free to correct me if anything is wrong:
- the data in the config blob "patches" eFuse values at runtime (non-persistent)
- we know the offsets of the UART_CONFIG, PCM_SETTING and BD_ADDR -
these can be "board specific" (like baud rate, flow control, ...)
- most other values from the "config blob" are "chip specific" -
meaning they are identical for each board and they only depend on the
chip
do you have any suggestions how we can support multiple boards (the
main goal of this whole discussion is: how to do this "correct")?
so far different approaches were discussed (not only in this thread,
but also in the past):
- use a separate config blob for each board. this means that whenever
a new board is supported (for example by adding a .dts for it to the
mainline kernel) then Bluetooth won't work out-of-the-box unless a
config is provided.
- specify the properties of the connection to the chip (for example by
adding a device-tree property for the speed, flow control, ...) and
generate the config based on the device-tree properties.
- (I am open for other suggestions, please let us know if you have any)
I would like to hear your opinion on this topic and especially the
reasons behind your suggestions.
Best regards
Martin
------Please consider the environment before printing this e-mail.
On Dienstag, 19. Februar 2019 15:14:01 CEST Rob Herring wrote:
> > > How is this used?
> >
> > rtl8723bs-bt needs 2 firmware binaries -- one is actual firmware,
> > another is firmware config which is specific to the board. If
> > firmware-postfix is specified, driver appends it to the name of config
> > and requests board-specific config while loading firmware. I.e. if
> > 'pine64' is specified as firmware-postfix driver will load
> > rtl8723bs_config-pine64.bin.
>
> We already have 'firmware-name' defined and I'd prefer not to have
> another way to do things. The difference is just you have to give the
> full filename.
>
Hi Rob,
I'm working on a v2 for this patchset and I've looked on how using "firmware-
name" with the full filename would be possible but as David Summers has already
written [1], the existing code [2] takes this "postfix" as parameter and
basically fills it into a filename template ("${CFG_NAME}-${POSTFIX}.bin"). So
either we stay with the "firmware-postfix" property or the existing code would
have to be modified to accomodate the full filename; but if using firmware-postfix
is unacceptable, I can rework the existing code.
Luca
[1] https://lore.kernel.org/netdev/[email protected]/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/
drivers/bluetooth/btrtl.c#n566