| Message ID | 20171117223543.32429-7-martin.blumenstingl@googlemail.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
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@googlemail.com> > --- > 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
Hi Marcel, On Sun, Nov 19, 2017 at 9:29 AM, Marcel Holtmann <marcel@holtmann.org> wrote: > 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@googlemail.com> >> --- >> 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); sounds reasonable, this way new devices can be added which don't need any vendor setup I'll fix this in the next version, thanks for spotting this >> + >> + 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. OK, I'll add a comment *why* it's needed (without a delay between toggling the GPIOs the device does not respond in the following btrtl_initialize call) the numbers however are based on "trial and error" as I could not find any documentation for these >> + >> + 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. you are right, this may be obsolete since the __hci_cmd_sync call above is already waiting for a reply. I'll verify this and either remove this msleep or add a comment why it's needed >> + >> + 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 > Regards Martin
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@googlemail.com> > --- > 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 so I just posted a bt3wire.c driver that is serdev only and written from scratch. On a RPi3 Broadcom chip it kinda works. I think there is a lot of extra work to be done, but this might be a better starting point for Realtek UART devices. Regards Marcel
Hi Marcel, Martin, On 03/16/2018 03:22 PM, Marcel Holtmann wrote: > 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@googlemail.com> >> --- >> 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 > > so I just posted a bt3wire.c driver that is serdev only and written from scratch. On a RPi3 Broadcom chip it kinda works. I think there is a lot of extra work to be done, but this might be a better starting point for Realtek UART devices. I picked up the v2 version of this RFC and rebased it to the latest bluetooth-next and added ACPI support. It also kinda works, but there's a few problems left to track down and I still haven't looked into getting rid of the config blobs. It sounded like Martin wasn't going to be able to get to this until at least the 4.18 cycle, so unless he objects I'm going to look at getting what I've got working with that bt3wire.c driver. Regards, Jeremy
Hi Jeremy, >>> 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@googlemail.com> >>> --- >>> 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 >> >> so I just posted a bt3wire.c driver that is serdev only and written from scratch. On a RPi3 Broadcom chip it kinda works. I think there is a lot of extra work to be done, but this might be a better starting point for Realtek UART devices. > > I picked up the v2 version of this RFC and rebased it to the latest > bluetooth-next and added ACPI support. It also kinda works, but there's > a few problems left to track down and I still haven't looked into > getting rid of the config blobs. > > It sounded like Martin wasn't going to be able to get to this until > at least the 4.18 cycle, so unless he objects I'm going to look at > getting what I've got working with that bt3wire.c driver. yes please, have a look at the bt3wire.c code that I send and see if you can make it work with Realtek controllers. On the RPi3 with the Broadcom controller it kinda works, but firmware loading and btmgmt power on/off seem to have issues. The power on/off might be caused because I moved the serdev_device_open and link establishment into the hdev->open callback and don’t clean up properly on hdev->close. However I really think it belongs there and thus we should make this work. You will notice that the link establishment is also kinda slow (at least on Broadcom devices) where the messages only happen once per second. We can try to send every 250 msec and see if that changes things. If the Bluetooth stack is powered down, we should also not have the serial device opened. On USB it is the same since it is bound, but no URBs are active. For me the real advantage of a standalone bt3wire.c is that we no longer have spagetti code through hci_ldisc.c and hci_serdev.c and hci_*.c drivers that got rather complicated to follow. On a side note, in btuart.c, I put all the serdev settings into the hdev->setup callback, but I think with bt3wire.c that is not possible due to 3-Wire protocol. So I assume we need vendor_open, vendor_close and vendor_setup additions in bt3wire.c Regards Marcel
Hi Jeremy, On Sat, Mar 17, 2018 at 11:50 PM, Jeremy Cline <jeremy@jcline.org> wrote: > Hi Marcel, Martin, > > On 03/16/2018 03:22 PM, Marcel Holtmann wrote: >> 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@googlemail.com> >>> --- >>> 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 >> >> so I just posted a bt3wire.c driver that is serdev only and written from scratch. On a RPi3 Broadcom chip it kinda works. I think there is a lot of extra work to be done, but this might be a better starting point for Realtek UART devices. > > I picked up the v2 version of this RFC and rebased it to the latest > bluetooth-next and added ACPI support. It also kinda works, but there's > a few problems left to track down and I still haven't looked into > getting rid of the config blobs. > > It sounded like Martin wasn't going to be able to get to this until > at least the 4.18 cycle, so unless he objects I'm going to look at > getting what I've got working with that bt3wire.c driver. please go ahead - I am still busy with other things thank you for taking this over! Regards Martin
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; + + 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); + + 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); + + 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); }
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@googlemail.com> --- drivers/bluetooth/Kconfig | 1 + drivers/bluetooth/hci_h5.c | 195 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 195 insertions(+), 1 deletion(-)