Hi Martin,
> Realtek RTL8723BS and RTL8723DS are SDIO wifi chips with an embedded
> Bluetooth controller which connects to the host via UART.
> The H5 protocol is used for communication between host and device.
>
> The Realtek "rtl8723bs_bt" and "rtl8723ds_bt" userspace Bluetooth UART
> initialization tools (rtk_hciattach) use the following sequence:
> 1) send H5 sync pattern (already supported by hci_h5)
> 2) get LMP version (already supported by btrtl)
> 3) get ROM version (already supported by btrtl)
> 4) load the firmware and config for the current chipset (already
> supported by btrtl)
> 5) read UART settings from the config blob (already supported by btrtl)
> 6) send UART settings via a vendor command to the device (which changes
> the baudrate of the device and enables or disables flow control
> depending on the config)
> 7) change the baudrate and flow control settings on the host
> 8) send the firmware and config blob to the device (already supported by
> btrtl)
>
> This uses the serdev library as well as the existing btrtl driver to
> initialize the Bluetooth functionality, which consists of:
> - identifying the device and loading the corresponding firmware and
> config blobs (steps #2, #3 and #4)
> - configuring the baudrate and flow control (steps #6 and #7)
> - uploading the firmware to the device (step #8)
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
> ---
> drivers/bluetooth/Kconfig | 1 +
> drivers/bluetooth/hci_h5.c | 195 ++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 195 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 60e1c7d6986d..3001f1200c72 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -146,6 +146,7 @@ config BT_HCIUART_LL
> config BT_HCIUART_3WIRE
> bool "Three-wire UART (H5) protocol support"
> depends on BT_HCIUART
> + select BT_RTL if SERIAL_DEV_BUS
> help
> The HCI Three-wire UART Transport Layer makes it possible to
> user the Bluetooth HCI over a serial port interface. The HCI
> diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
> index 6a8d0d06aba7..7d584e5928bf 100644
> --- a/drivers/bluetooth/hci_h5.c
> +++ b/drivers/bluetooth/hci_h5.c
> @@ -28,7 +28,14 @@
> #include <net/bluetooth/bluetooth.h>
> #include <net/bluetooth/hci_core.h>
>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/serdev.h>
> +
> #include "hci_uart.h"
> +#include "btrtl.h"
>
> #define HCI_3WIRE_ACK_PKT 0
> #define HCI_3WIRE_LINK_PKT 15
> @@ -97,6 +104,13 @@ struct h5 {
> } sleep;
> };
>
> +struct h5_device {
> + struct hci_uart hu;
> + struct gpio_desc *disable_gpio;
> + struct gpio_desc *reset_gpio;
> + int (*vendor_setup)(struct h5_device *h5_dev);
> +};
> +
> static void h5_reset_rx(struct h5 *h5);
>
> static void h5_link_control(struct hci_uart *hu, const void *data, size_t len)
> @@ -190,6 +204,7 @@ static int h5_open(struct hci_uart *hu)
> {
> struct h5 *h5;
> const unsigned char sync[] = { 0x01, 0x7e };
> + int err;
>
> BT_DBG("hu %p", hu);
>
> @@ -210,6 +225,14 @@ static int h5_open(struct hci_uart *hu)
>
> h5->tx_win = H5_TX_WIN_MAX;
>
> + if (hu->serdev) {
> + err = serdev_device_open(hu->serdev);
> + if (err) {
> + bt_dev_err(hu->hdev, "failed to open serdev: %d", err);
> + return err;
> + }
> + }
> +
> set_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags);
>
> /* Send initial sync request */
> @@ -219,6 +242,23 @@ static int h5_open(struct hci_uart *hu)
> return 0;
> }
>
> +static int h5_setup(struct hci_uart *hu)
> +{
> + int err;
> + struct h5_device *h5_dev;
> +
> + if (!hu->serdev)
> + return 0;
> +
> + h5_dev = serdev_device_get_drvdata(hu->serdev);
> +
> + err = h5_dev->vendor_setup(h5_dev);
> + if (err)
> + return err;
if (h5_dev->vendor_setup)
return h5_dev->vendor_setup(h5_dev);
> +
> + return 0;
> +}
> +
> static int h5_close(struct hci_uart *hu)
> {
> struct h5 *h5 = hu->priv;
> @@ -229,6 +269,15 @@ static int h5_close(struct hci_uart *hu)
> skb_queue_purge(&h5->rel);
> skb_queue_purge(&h5->unrel);
>
> + if (hu->serdev) {
> + struct h5_device *h5_dev;
> +
> + h5_dev = serdev_device_get_drvdata(hu->serdev);
> + gpiod_set_value_cansleep(h5_dev->disable_gpio, 1);
> +
> + serdev_device_close(hu->serdev);
> + }
> +
> kfree(h5);
>
> return 0;
> @@ -316,7 +365,10 @@ static void h5_handle_internal_rx(struct hci_uart *hu)
> h5->tx_win = (data[2] & 0x07);
> BT_DBG("Three-wire init complete. tx_win %u", h5->tx_win);
> h5->state = H5_ACTIVE;
> - hci_uart_init_ready(hu);
> +
> + /* serdev does not support the "init_ready" signal */
> + if (!hu->serdev)
> + hci_uart_init_ready(hu);
> return;
> } else if (memcmp(data, sleep_req, 2) == 0) {
> BT_DBG("Peer went to sleep");
> @@ -739,10 +791,147 @@ static int h5_flush(struct hci_uart *hu)
> return 0;
> }
>
> +#if IS_ENABLED(CONFIG_SERIAL_DEV_BUS)
> +static int h5_setup_realtek(struct h5_device *h5_dev)
> +{
> + struct hci_uart *hu = &h5_dev->hu;
> + int err = 0, retry = 3;
> + struct sk_buff *skb;
> + struct btrtl_device_info *btrtl_dev;
> + __le32 baudrate_data;
> + u32 device_baudrate;
> + unsigned int controller_baudrate;
> + bool flow_control;
> +
> + /* devices always start with flow control disabled and even parity */
> + serdev_device_set_flow_control(hu->serdev, false);
> + serdev_device_set_parity(hu->serdev, true, false);
> +
> + do {
> + /* Configure BT_DISn and BT_RST_N to LOW state */
> + gpiod_set_value_cansleep(h5_dev->reset_gpio, 1);
> + gpiod_set_value_cansleep(h5_dev->disable_gpio, 1);
> + msleep(500);
> + gpiod_set_value_cansleep(h5_dev->reset_gpio, 0);
> + gpiod_set_value_cansleep(h5_dev->disable_gpio, 0);
> + msleep(500);
I really hate random msleep() without comments. Explain in the comment block why this specific wait is good.
> +
> + btrtl_dev = btrtl_initialize(hu->hdev);
> + if (!IS_ERR(btrtl_dev))
> + break;
> +
> + /* Toggle BT_DISn and retry */
> + } while (retry--);
> +
> + if (IS_ERR(btrtl_dev))
> + return PTR_ERR(btrtl_dev);
> +
> + err = btrtl_get_uart_settings(hu->hdev, btrtl_dev,
> + &controller_baudrate, &device_baudrate,
> + &flow_control);
> + if (err)
> + goto out_free;
> +
> + baudrate_data = cpu_to_le32(device_baudrate);
> + skb = __hci_cmd_sync(hu->hdev, 0xfc17, sizeof(baudrate_data),
> + &baudrate_data, HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb)) {
> + bt_dev_err(hu->hdev, "set baud rate command failed");
> + err = -PTR_ERR(skb);
> + goto out_free;
> + } else {
> + kfree_skb(skb);
> + }
> +
> + msleep(500);
Same here, explain why this time is the right time to wait.
> +
> + serdev_device_set_baudrate(hu->serdev, controller_baudrate);
> + serdev_device_set_flow_control(hu->serdev, flow_control);
> +
> + err = btrtl_download_firmware(hu->hdev, btrtl_dev);
> +
> +out_free:
> + btrtl_free(btrtl_dev);
> +
> + return err;
> +}
> +
> +static const struct hci_uart_proto h5p;
> +
> +static int hci_h5_probe(struct serdev_device *serdev)
> +{
> + struct hci_uart *hu;
> + struct h5_device *h5_dev;
> +
> + h5_dev = devm_kzalloc(&serdev->dev, sizeof(*h5_dev), GFP_KERNEL);
> + if (!h5_dev)
> + return -ENOMEM;
> +
> + hu = &h5_dev->hu;
> + hu->serdev = serdev;
> +
> + serdev_device_set_drvdata(serdev, h5_dev);
> +
> + h5_dev->vendor_setup = of_device_get_match_data(&serdev->dev);
> +
> + h5_dev->disable_gpio = devm_gpiod_get_optional(&serdev->dev, "disable",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(h5_dev->disable_gpio))
> + return PTR_ERR(h5_dev->disable_gpio);
> +
> + h5_dev->reset_gpio = devm_gpiod_get_optional(&serdev->dev, "reset",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(h5_dev->reset_gpio))
> + return PTR_ERR(h5_dev->reset_gpio);
> +
> + hci_uart_set_speeds(hu, 115200, 0);
> +
> + return hci_uart_register_device(hu, &h5p);
> +}
> +
> +static void hci_h5_remove(struct serdev_device *serdev)
> +{
> + struct h5_device *h5_dev = serdev_device_get_drvdata(serdev);
> + struct hci_uart *hu = &h5_dev->hu;
> + struct hci_dev *hdev = hu->hdev;
> +
> + cancel_work_sync(&hu->write_work);
> +
> + hci_unregister_dev(hdev);
> + hci_free_dev(hdev);
> + hu->proto->close(hu);
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id hci_h5_of_match[] = {
> + {
> + .compatible = "realtek,rtl8723bs-bluetooth",
> + .data = h5_setup_realtek
> + },
> + {
> + .compatible = "realtek,rtl8723ds-bluetooth",
> + .data = h5_setup_realtek
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, hci_h5_of_match);
> +#endif
> +
> +static struct serdev_device_driver hci_h5_drv = {
> + .driver = {
> + .name = "hci-h5",
> + .of_match_table = of_match_ptr(hci_h5_of_match),
> + },
> + .probe = hci_h5_probe,
> + .remove = hci_h5_remove,
> +};
> +#endif
> +
> static const struct hci_uart_proto h5p = {
> .id = HCI_UART_3WIRE,
> .name = "Three-wire (H5)",
> .open = h5_open,
> + .setup = h5_setup,
> .close = h5_close,
> .recv = h5_recv,
> .enqueue = h5_enqueue,
> @@ -752,10 +941,14 @@ static const struct hci_uart_proto h5p = {
>
> int __init h5_init(void)
> {
> + serdev_device_driver_register(&hci_h5_drv);
> +
> return hci_uart_register_proto(&h5p);
> }
>
> int __exit h5_deinit(void)
> {
> + serdev_device_driver_unregister(&hci_h5_drv);
> +
> return hci_uart_unregister_proto(&h5p);
> }
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
