Hi,

I'm adding bluetooth list to the discussion. Full patch is available
here:

https://patchwork.kernel.org/patch/5712591/

Larry Finger <[email protected]> writes:

> From: Troy Tan <[email protected]>
>
> This patch adds the routines used to communicate between the RTL8812AE (wifi)
> device and the RTL8761AU (bluetooth) device that are part of the same chip.
> Unlike other similar dual-function devices, this chip does not contain special
> hardware that lets the firmware pass coexistence info from one part to the
> other. As a result, this driver implements such communication as a kernel
> socket.
>
> Signed-off-by: Troy Tan <[email protected]>
> Signed-off-by: Larry Finger <[email protected]>
> ---
> V2 - Add comments explaining the routine that sends a message via the
> socket.

The commit log is not still explaining that much about the actual
functionality, so I investigated on my own:

> +static u8 rtl_btcoex_create_kernel_socket(struct rtl_priv *rtlpriv,
> +                                       u8 is_invite)
> +{
> +     struct bt_coex_info *pcoex_info = &rtlpriv->coex_info;
> +     s8 kernel_socket_err;
> +
> +     BTC_PRINT(BTC_MSG_SOCKET, SOCKET_CRITICAL,
> +               "%s CONNECT_PORT %d\n", __func__, CONNECT_PORT);
> +
> +     if (!pcoex_info) {
> +             BTC_PRINT(BTC_MSG_SOCKET, SOCKET_CRITICAL, "coex_info: NULL\n");
> +             return _FAIL;
> +     }
> +
> +     kernel_socket_err = sock_create(PF_INET, SOCK_DGRAM, 0,
> +                                     &pcoex_info->udpsock);
> +     BTC_PRINT(BTC_MSG_SOCKET, SOCKET_CRITICAL,
> +               "binding socket, err = %d\n", kernel_socket_err);
> +
> +     if (kernel_socket_err < 0) {
> +             BTC_PRINT(BTC_MSG_SOCKET, SOCKET_CRITICAL,
> +                       "Error during creation of socket error:%d\n",
> +                       kernel_socket_err);
> +             return _FAIL;
> +     }
> +     memset(&pcoex_info->sin, 0, sizeof(pcoex_info->sin));
> +     pcoex_info->sin.sin_family = AF_INET;
> +     pcoex_info->sin.sin_port = htons(CONNECT_PORT);
> +     pcoex_info->sin.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
> +
> +     memset(&pcoex_info->bt_addr, 0, sizeof(pcoex_info->bt_addr));
> +     pcoex_info->bt_addr.sin_family = AF_INET;
> +     pcoex_info->bt_addr.sin_port = htons(CONNECT_PORT_BT);
> +     pcoex_info->bt_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
> +
> +     pcoex_info->sk_store = NULL;
> +
> +     kernel_socket_err =
> +        pcoex_info->udpsock->ops->bind(pcoex_info->udpsock,
> +                                       (struct sockaddr *)&pcoex_info->sin,
> +                                       sizeof(pcoex_info->sin));
> +     if (kernel_socket_err == 0) {
> +             BTC_PRINT(BTC_MSG_SOCKET, SOCKET_CRITICAL,
> +                       "binding socket success\n");
> +             pcoex_info->udpsock->sk->sk_data_ready =
> +                     rtl_btcoex_recvmsg_int;
> +             pcoex_info->sock_open |=  KERNEL_SOCKET_OK;
> +             pcoex_info->bt_attend = false;
> +             BTC_PRINT(BTC_MSG_SOCKET, SOCKET_CRITICAL,
> +                       "WIFI sending attend_req\n");
> +             rtl_btcoex_sendmsgbysocket(rtlpriv, attend_req,
> +                                        sizeof(attend_req), true);
> +             return _SUCCESS;

So the wireless driver communicates with the bluetooth driver (which is
not in upstream) via a localhost UDP connection?

> +#define CONNECT_PORT 30000
> +#define CONNECT_PORT_BT 30001

And these are the UDP ports used.

> +struct hci_link_info {
> +     u16             connect_handle;
> +     u8              incoming_traffic_mode;
> +     u8              outgoing_traffic_mode;
> +     u8              bt_profile;
> +     u8              bt_corespec;
> +     s8              bt_RSSI;
> +     u8              traffic_profile;
> +     u8              link_role;
> +};
> +
> +#define      MAX_BT_ACL_LINK_NUM             8
> +
> +struct hci_ext_config {
> +     struct hci_link_info    acl_link[MAX_BT_ACL_LINK_NUM];
> +     u8      bt_operation_code;
> +     u16     current_connect_handle;
> +     u8      current_incoming_traffic_mode;
> +     u8      current_outgoing_traffic_mode;
> +
> +     u8      number_of_acl;
> +     u8      number_of_sco;
> +     u8      current_bt_status;
> +     u16     hci_ext_ver;
> +     bool    enable_wifi_scan_notify;
> +};

[...]

> +enum HCI_STATUS {
> +     /* Success */
> +     HCI_STATUS_SUCCESS                              = 0x00,
> +     /* Unknown HCI Command */
> +     HCI_STATUS_UNKNOW_HCI_CMD                       = 0x01,
> +     /* Unknown Connection Identifier */
> +     HCI_STATUS_UNKNOW_CONNECT_ID                    = 0X02,
> +     /* Hardware Failure */
> +     HCI_STATUS_HW_FAIL                              = 0X03,
> +     /* Page Timeout */
> +     HCI_STATUS_PAGE_TIMEOUT                         = 0X04,
> +     /* Authentication Failure */
> +     HCI_STATUS_AUTH_FAIL                            = 0X05,
> +     /* PIN or Key Missing */
> +     HCI_STATUS_PIN_OR_KEY_MISSING                   = 0X06,
> +     /* Memory Capacity Exceeded */
> +     HCI_STATUS_MEM_CAP_EXCEED                       = 0X07,
> +     /* Connection Timeout */
> +     HCI_STATUS_CONNECT_TIMEOUT                      = 0X08,
> +     /* Connection Limit Exceeded */

And here's part of the information exchanged between the drivers.

This is something which needs to be properly reviewed both in wireless
and bluetooth lists, a wireless driver cannot just invent these on their
own. And using UDP sockets for this is in my opinion just horrible.

I know there's a general need for something similar like this, but it
needs to properly discussed and designed.

Comments?

-- 
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to