Re: [PATCH v2 1/2] Bluetooth: btqca: Introduce generic QCA ROME support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ben,

> This is for supporting BT for QCA ROME with specific vendor
> specific HCI commands and initialization. This will have USB/UART
> implementation both, but for now, adding UART vendor specific
> commands to patch downloading and set BDaddress using VS command.
> 
> Signed-off-by: Ben Young Tae Kim <ytkim@xxxxxxxxxxxxxxxx>
> ---
> drivers/bluetooth/Kconfig  |   4 +
> drivers/bluetooth/Makefile |   1 +
> drivers/bluetooth/btqca.c  | 547 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/bluetooth/btqca.h  | 166 ++++++++++++++
> 4 files changed, 718 insertions(+)
> create mode 100644 drivers/bluetooth/btqca.c
> create mode 100644 drivers/bluetooth/btqca.h
> 
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 79e8234..f580334 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -13,6 +13,10 @@ config BT_RTL
> 	tristate
> 	select FW_LOADER
> 
> +config BT_QCA
> +	tristate
> +	select FW_LOADER
> +
> config BT_HCIBTUSB
> 	tristate "HCI USB driver"
> 	depends on USB
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index f40e194..15a0d1d 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -22,6 +22,7 @@ obj-$(CONFIG_BT_MRVL_SDIO)	+= btmrvl_sdio.o
> obj-$(CONFIG_BT_WILINK)		+= btwilink.o
> obj-$(CONFIG_BT_BCM)		+= btbcm.o
> obj-$(CONFIG_BT_RTL)		+= btrtl.o
> +obj-$(CONFIG_BT_QCA)		+= btqca.o
> 
> btmrvl-y			:= btmrvl_main.o
> btmrvl-$(CONFIG_DEBUG_FS)	+= btmrvl_debugfs.o
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> new file mode 100644
> index 0000000..b087e07
> --- /dev/null
> +++ b/drivers/bluetooth/btqca.c
> @@ -0,0 +1,547 @@
> +/*
> + *  Bluetooth supports for Qualcomm Atheros chips
> + *
> + *  Copyright (c) 2015 The Linux Foundation. All rights reserved.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2
> + *  as published by the Free Software Foundation
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + */
> +#include <linux/module.h>
> +#include <linux/firmware.h>
> +#include <linux/tty.h>
> +
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +#include "btqca.h"
> +
> +#ifdef CONFIG_SERIAL_MSM_HS
> +#include <linux/platform_data/msm_serial_hs.h>
> +#endif

nope. Submit a sperate patch when this actually hits upstream. And I still prefer DT over platform data.

> +
> +#define VERSION "0.1"
> +
> +static uint8_t rome_user_baud_rate = QCA_BAUDRATE_115200;

Why was this global again?

> +
> +static int rome_patch_ver_req(struct hci_dev *hdev, u32 *rome_version)
> +{
> +	struct sk_buff *skb;
> +	struct edl_event_hdr *edl;
> +	struct rome_version *ver;
> +	char cmd;
> +	int ret = 0;

Do not initialize variable if not needed.

> +
> +	BT_DBG("%s: ROME Patch Version Request", hdev->name);
> +
> +	cmd = EDL_PATCH_VER_REQ_CMD;
> +	skb = __hci_cmd_sync(hdev, EDL_PATCH_CMD_OPCODE, EDL_PATCH_CMD_LEN,
> +			     &cmd, HCI_INIT_TIMEOUT);
> +	if (IS_ERR(skb)) {
> +		ret = PTR_ERR(skb);

Lets keep using err instead of ret to be consistent.

> +		BT_ERR("%s: Failed to read version of ROME (%d)", hdev->name,
> +		       ret);
> +		return ret;
> +	}
> +
> +	if (skb->len != sizeof(*edl) + sizeof(*ver)) {
> +		BT_ERR("%s: version size mismatch len %d", hdev->name,
> +		       skb->len);
> +		ret = -EILSEQ;
> +		goto out;
> +	}
> +
> +	edl = (struct edl_event_hdr *)(skb->data);
> +	if (!edl || edl->data == NULL) {
> +		BT_ERR("%s: tlv no hdr or no data", hdev->name);
> +		ret = -EILSEQ;
> +		goto out;
> +	}
> +
> +	if (edl->cresp != EDL_CMD_REQ_RES_EVT ||
> +	    edl->rtype != EDL_APP_VER_RES_EVT) {
> +		BT_ERR("%s: wrong packet received %d %d", hdev->name,
> +		       edl->cresp, edl->rtype);
> +		ret = -EIO;
> +		goto out;
> +	}
> +
> +	ver = (struct rome_version *)(edl->data);
> +
> +	BT_DBG("%s:Product:0x%08x", hdev->name, le32_to_cpu(ver->product_id));
> +	BT_DBG("%s:Patch  :0x%08x", hdev->name, le16_to_cpu(ver->patch_ver));
> +	BT_DBG("%s:ROM    :0x%08x", hdev->name, le16_to_cpu(ver->rome_ver));
> +	BT_DBG("%s:SOC    :0x%08x", hdev->name, le32_to_cpu(ver->soc_id));
> +
> +	/* ROME chipset Version can be decided by patch and SOC
> +	 * Version, combination with upper 2 bytes from soc
> +	 * and lower 2 bytes from patch will be used
> +	 */
> +	*rome_version = (le32_to_cpu(ver->soc_id) << 16) |
> +		        (le16_to_cpu(ver->rome_ver) & 0x0000ffff);
> +
> +out:
> +	kfree_skb(skb);
> +
> +	return ret;
> +}
> +
> +static int rome_reset(struct hci_dev *hdev)
> +{
> +	struct sk_buff *skb;
> +	int err;
> +
> +	BT_DBG("%s: ROME HCI_RESET", hdev->name);
> +
> +	skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
> +	if (IS_ERR(skb)) {
> +		err = PTR_ERR(skb);
> +		BT_ERR("%s: BCM: Reset failed (%d)", hdev->name, err);
> +		return err;
> +	}
> +	kfree_skb(skb);
> +
> +	return 0;
> +}
> +
> +static void rome_tlv_check_data(u8 type, const struct firmware *fw)
> +{
> +	const u8 *data;
> +	u32 type_len;
> +	u16 tag_id, tag_len;
> +	int idx, length;
> +	struct tlv_type_hdr *tlv;
> +	struct tlv_type_patch *tlv_patch;
> +	struct tlv_type_nvm *tlv_nvm;
> +
> +	tlv = (struct tlv_type_hdr*)fw->data;
> +
> +	type_len = le32_to_cpu(tlv->type_len);
> +
> +	BT_INFO("TLV Type\t\t : 0x%x", type_len & 0x000000ff);
> +	length = (type_len >> 8) & 0x00ffffff;
> +	BT_INFO("Length\t\t : %d bytes", length);
> +
> +	switch (type) {
> +	case TLV_TYPE_PATCH:
> +		tlv_patch = (struct tlv_type_patch *)tlv->data;
> +		BT_INFO("Total Length\t\t : %d bytes",
> +			le32_to_cpu(tlv_patch->total_size));
> +		BT_INFO("Patch Data Length\t : %d bytes",
> +			le32_to_cpu(tlv_patch->data_length));
> +		BT_INFO("Signing Format Version : 0x%x",
> +			tlv_patch->format_version);
> +		BT_INFO("Signature Algorithm\t : 0x%x",
> +			tlv_patch->signature);
> +		BT_INFO("Reserved\t\t : 0x%x",
> +			le16_to_cpu(tlv_patch->reserved1));
> +		BT_INFO("Product ID\t\t : 0x%04x",
> +			le16_to_cpu(tlv_patch->product_id));
> +		BT_INFO("Rom Build Version\t : 0x%04x",
> +			le16_to_cpu(tlv_patch->rom_build));
> +		BT_INFO("Patch Version\t\t : 0x%04x",
> +			le16_to_cpu(tlv_patch->patch_version));
> +		BT_INFO("Reserved\t\t : 0x%x",
> +			le16_to_cpu(tlv_patch->reserved2));
> +		BT_INFO("Patch Entry Address\t : 0x%x",
> +			le32_to_cpu(tlv_patch->entry));
> +		break;
> +
> +	case TLV_TYPE_NVM:
> +		idx = 0;
> +		data = (void*) tlv->data;

Why are you casting here? And btw the cast is not according to coding style.

> +		do {
> +			tlv_nvm = (struct tlv_type_nvm *)(data+idx);

data + idx with proper spaces between.

> +
> +			tag_id = le16_to_cpu(tlv_nvm->tag_id);
> +			tag_len = le16_to_cpu(tlv_nvm->tag_len);
> +
> +			BT_DBG("TAG ID\t\t : %d", tag_id);
> +			BT_DBG("TAG Length\t\t : %d", tag_len);
> +			BT_DBG("TAG Data\t\t : ");
> +
> +			/* Update NVM tags as needed */
> +			switch (tag_id) {
> +			case 17:
> +			#ifndef QCA_FEATURE_UART_IN_BAND_SLEEP
> +				/* HCI transport layer parameters
> +				 * disabling software inband sleep
> +				 * until hci driver supports IBS
> +				 */
> +				tlv_nvm->data[0] &= ~0x80;
> +			#endif
> +				/* UART Baud Rate */
> +				tlv_nvm->data[2] = rome_user_baud_rate;
> +				break;
> +
> +			#ifndef QCA_FEATURE_DEEP_SLEEP
> +			case 27:
> +				/* Sleep enable mask
> +				 * disabling deep_sleep until IBS driver is up
> +				 */
> +				tlv_nvm->data[0] &= ~0x01;
> +			#endif
> +
> +				break;
> +			}
> +
> +			rome_debug_dump(tlv_nvm->data, tag_len, true);

I am actually really in favor of this rome_debug_dump and especially not as exported function. I prefer that we rather use the hex dump feature of the kernel or not bother to dump the TLVs at all here. Just report the errors.

I mean, we can also a userspace utility that verifies firmwares and lets it dissect the TLVs. We are doing that for the Intel chips with the bluemoon utility. So I am would welcome that.

> +
> +			idx += (sizeof(u16) + sizeof(u16) + 8 + tag_len);
> +		} while (idx < length);
> +		break;
> +
> +	default:
> +		BT_INFO("unknown TLV type %d", type);

This is an error?

> +		break;
> +	}
> +
> +	return;

return statement for void functions are useless. Please remove.

> +}
> +
> +static int rome_tlv_send_segment(struct hci_dev *hdev, int idx, int seg_size,
> +				 u8 *data)
> +{
> +	struct sk_buff *skb;
> +	struct edl_event_hdr *edl;
> +	struct tlv_seg_resp *tlv_resp;
> +	u8 cmd[MAX_SIZE_PER_TLV_SEGMENT+2];

Spaces between + operator.

> +	int ret = 0;

Only initialize variables if really needed.

> +
> +	BT_DBG("%s: Download segment #%d size %d", hdev->name, idx, seg_size);
> +
> +	cmd[0] = EDL_PATCH_TLV_REQ_CMD;
> +	cmd[1] = seg_size;
> +	memcpy(cmd+2, data, seg_size);
> +
> +	skb = __hci_cmd_sync(hdev, EDL_PATCH_CMD_OPCODE, seg_size+2, cmd,

seq_size + 2, please use proper spaces.


> +			     HCI_INIT_TIMEOUT);
> +	if (IS_ERR(skb)) {
> +		ret = PTR_ERR(skb);
> +		BT_ERR("%s: Failed to send TLV segment(%d)", hdev->name, ret);
> +		return ret;
> +	}
> +
> +	if (skb->len != sizeof(*edl)+sizeof(*tlv_resp)) {
> +		BT_ERR("%s: tlv response size mismatch", hdev->name);
> +		ret = -EILSEQ;
> +		goto out;
> +	}
> +
> +	edl = (struct edl_event_hdr *)(skb->data);
> +	if (!edl || edl->data == NULL) {
> +		BT_ERR("%s: tlv no hdr or no data", hdev->name);
> +		ret = -EILSEQ;
> +		goto out;
> +	}
> +
> +	tlv_resp = (struct tlv_seg_resp *)(edl->data);
> +
> +	if (edl->cresp != EDL_CMD_REQ_RES_EVT ||
> +	    edl->rtype != EDL_TVL_DNLD_RES_EVT || tlv_resp->result != 0x00) {
> +		BT_ERR("%s: tlv error stat 0x%x rtype 0x%x (0x%x)", hdev->name,
> +		       edl->cresp, edl->rtype, tlv_resp->result);
> +		ret = -EIO;
> +	}
> +
> +out:
> +	kfree_skb(skb);
> +
> +	return ret;
> +}
> +
> +static int rome_tlv_download_request(struct hci_dev *hdev,
> +				     const struct firmware *fw)
> +{
> +	int total_segment, remain_size;
> +	int ret, i;
> +	u8 *buffer, *data;
> +
> +	if (!fw || !fw->data)
> +		return -EINVAL;
> +
> +	total_segment = fw->size / MAX_SIZE_PER_TLV_SEGMENT;
> +	remain_size = (fw->size < MAX_SIZE_PER_TLV_SEGMENT)? fw->size:

Please double check your missing spaces. There should be one before?

> +		      fw->size % MAX_SIZE_PER_TLV_SEGMENT;

However, we are we not using min_t here.

> +
> +	BT_DBG("%s: Total size %ld, total segment num %d, remain size %d",
> +	       hdev->name, fw->size, total_segment, remain_size);

So fw->size is a size_t. Can you please carry this forward and not just turn things into an int.

I also know that %ld is not correct for size_t. It must be %zu.

> +
> +	data = (u8 *)fw->data;

Why are we casting here. fw->data is already u8. Please get your const usage right. I really dislike casting const away. That should be the last resort.

> +	for (i = 0; i < total_segment; i++) {
> +		buffer = data + i*MAX_SIZE_PER_TLV_SEGMENT;

Spaces around * operator.

> +		ret = rome_tlv_send_segment(hdev, i, MAX_SIZE_PER_TLV_SEGMENT,
> +					    buffer);
> +		if (ret < 0)
> +			return -EIO;
> +	}
> +
> +	if (remain_size) {
> +		buffer = data + total_segment * MAX_SIZE_PER_TLV_SEGMENT;
> +		ret = rome_tlv_send_segment(hdev, total_segment, remain_size,
> +					    buffer);
> +		if (ret < 0)
> +			return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rome_download_firmware_file(struct hci_dev *hdev, u8 type,
> +				       char *fwname)

const char *fwname. And rome_download_firmware seems long enough for a name.

> +{
> +	const struct firmware *fw;
> +	int ret;
> +
> +	BT_INFO("%s: ROME Downloading %s", hdev->name, fwname);
> +
> +	ret = request_firmware(&fw, fwname, &hdev->dev);
> +	if (ret) {
> +		BT_ERR("%s: failed to request file: %s (%d)", hdev->name,
> +		       fwname, ret);
> +		return ret;
> +	}
> +
> +	rome_tlv_check_data(type, fw);
> +
> +	ret = rome_tlv_download_request(hdev, fw);
> +	if (ret) {
> +		BT_ERR("%s: fail to download file: %s (%d)", hdev->name,
> +		       fwname, ret);
> +	}
> +
> +	release_firmware(fw);
> +
> +	return ret;
> +}
> +
> +static int rome_inject_cmd_complete(struct hci_dev *hdev, __u16 opcode,
> +				    u8 result, void *params, int len)
> +{
> +	struct sk_buff *skb;
> +	struct hci_event_hdr *hdr;
> +	struct hci_ev_cmd_complete *evt;
> +
> +	skb = bt_skb_alloc(sizeof(*hdr) + sizeof(*evt) + 1 + len, GFP_ATOMIC);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	hdr = (struct hci_event_hdr *)skb_put(skb, sizeof(*hdr));
> +	hdr->evt = HCI_EV_CMD_COMPLETE;
> +	hdr->plen = sizeof(*evt) + 1 + len;
> +
> +	evt = (struct hci_ev_cmd_complete *)skb_put(skb, sizeof(*evt));
> +	evt->ncmd = 0x01;
> +	evt->opcode = cpu_to_le16(opcode);
> +
> +	memcpy(skb_put(skb, 1), &result, 1);
> +	memcpy(skb_put(skb, len), params, len);
> +
> +	bt_cb(skb)->pkt_type = HCI_EVENT_PKT;
> +
> +	return hci_recv_frame(hdev, skb);
> +}

Sadly, we have to do that, hence my comment with the hci_cmd_send that we might export to drivers.

> +
> +void rome_debug_dump(const u8 *cmd, int size, bool outbound)
> +{
> +#ifdef QCA_FEATURE_DEBUG
> +	int i;
> +	char printout[150], hex[10];
> +
> +	if (outbound)
> +		snprintf(printout, 150, "SEND -> ");
> +	else
> +		snprintf(printout, 150, "RECV <- ");
> +
> +	for (i = 0; i < size && i < 30; i++) {
> +		snprintf(hex, sizeof(hex), " %02x", cmd[i]);
> +		strncat(printout, hex, 150);
> +	}
> +
> +	BT_INFO("%s", printout);
> +#else
> +	return;
> +#endif
> +}
> +EXPORT_SYMBOL_GPL(rome_debug_dump);

I am really disliking this one. Start using the kernel's hex dump support.

> +
> +int rome_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> +{
> +	struct sk_buff *skb;
> +	u8 cmd[9];
> +	int err;
> +
> +	cmd[0] = EDL_NVM_ACCESS_SET_REQ_CMD;
> +	cmd[1] = 0x02; 			/* TAG ID */
> +	cmd[2] = sizeof(bdaddr_t);	/* size */
> +	memcpy(cmd + 3, bdaddr, sizeof(bdaddr_t));
> +
> +	skb = __hci_cmd_sync(hdev, EDL_NVM_ACCESS_OPCODE, sizeof(cmd), cmd,
> +			     HCI_INIT_TIMEOUT);
> +	if (IS_ERR(skb)) {
> +		err = PTR_ERR(skb);
> +		BT_ERR("%s: Change address command failed (%d)",
> +		       hdev->name, err);
> +		return err;
> +	}
> +	kfree_skb(skb);
> +
> +	/* apply NVM changes to the controller */
> +	rome_reset(hdev);

This needs a bit better comment explaining why this is needed. This is different from all other controllers where the HCI_Reset that follows this command handles it.

Keep in mind that hdev->set_bdaddr is property integrated with the mgmt command Set Public Address and the Unconfigured State. So the core takes care of resetting the controller via HCI_Reset.

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rome_set_bdaddr);
> +
> +uint8_t rome_get_baudrate(int speed)
> +{
> +	switch(speed) {
> +	case 9600:
> +		return QCA_BAUDRATE_9600;
> +	case 19200:
> +		return QCA_BAUDRATE_19200;
> +	case 38400:
> +		return QCA_BAUDRATE_38400;
> +	case 57600:
> +		return QCA_BAUDRATE_57600;
> +	case 115200:
> +		return QCA_BAUDRATE_115200;
> +	case 230400:
> +		return QCA_BAUDRATE_230400;
> +	case 460800:
> +		return QCA_BAUDRATE_460800;
> +	case 500000:
> +		return QCA_BAUDRATE_500000;
> +	case 921600:
> +		return QCA_BAUDRATE_921600;
> +	case 1000000:
> +		return QCA_BAUDRATE_1000000;
> +	case 2000000:
> +		return QCA_BAUDRATE_2000000;
> +	case 3000000:
> +		return QCA_BAUDRATE_3000000;
> +	case 3500000:
> +		return QCA_BAUDRATE_3500000;
> +	default:
> +		return QCA_BAUDRATE_115200;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(rome_get_baudrate);

Not sure it is a good idea having this exported by this module. That seems to be something internal to hci_qca.c

> +
> +void rome_serial_clock_on(struct tty_struct *tty)
> +{
> +#ifdef CONFIG_SERIAL_MSM_HS
> +	struct uart_state *state = tty->driver_data;
> +	struct uart_port *port = state->uart_port;
> +
> +	msm_hs_request_clock_on(port);
> +#endif
> +}
> +EXPORT_SYMBOL_GPL(rome_serial_clock_on);
> +
> +void rome_serial_clock_off(struct tty_struct *tty)
> +{
> +#ifdef CONFIG_SERIAL_MSM_HS
> +	struct uart_state *state = tty->driver_data;
> +	struct uart_port *port = state->uart_port;
> +
> +	msm_hs_request_clock_off(port);
> +#endif
> +}
> +EXPORT_SYMBOL_GPL(rome_serial_clock_off);

Remove these until you got things upstream and we are actually using DT.

> +
> +int rome_uart_setup(struct hci_dev *hdev, int uart_speed)
> +{
> +	u32 rome_ver = 0;
> +	char fwname[64];
> +	int err;
> +
> +	BT_DBG("%s: rome_uart_setup", hdev->name);
> +
> +	rome_user_baud_rate = rome_get_baudrate(uart_speed);
> +
> +	/* get ROME version information */
> +	err = rome_patch_ver_req(hdev, &rome_ver);
> +	if (err < 0 || rome_ver == 0) {
> +		BT_ERR("%s: Fail to get ROME version 0x%x",hdev->name, err);
> +		return err;
> +	}
> +
> +	BT_INFO("%s: ROME controller version 0x%08x", hdev->name, rome_ver);
> +
> +	/* download rampatch file */
> +	snprintf(fwname, sizeof(fwname), "qca/rampatch_%08x.bin", rome_ver);
> +	err = rome_download_firmware_file(hdev, TLV_TYPE_PATCH, fwname);
> +	if (err < 0) {
> +		BT_ERR("%s: can't download rampatch (%d)", hdev->name, err);
> +		return err;
> +	}
> +
> +	/* download NVM configuration */
> +	snprintf(fwname, sizeof(fwname), "qca/nvm_%08x.bin", rome_ver);
> +	err = rome_download_firmware_file(hdev, TLV_TYPE_NVM, fwname);
> +	if (err < 0) {
> +		BT_ERR("%s: can't download NVM file (%d)", hdev->name, err);
> +		return err;
> +	}
> +
> +	/* perform HCI reset */
> +	err = rome_reset(hdev);
> +	if (err < 0) {
> +		BT_ERR("%s: can't run HCI_RESET (%d)", hdev->name, err);
> +		return err;
> +	}
> +
> +	BT_INFO("%s: ROME uart setup is completed", hdev->name);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rome_uart_setup);
> +
> +int rome_vendor_frame(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +	struct hci_event_hdr *hdr = (void*)skb->data;
> +
> +	if (hdr->evt == HCI_VENDOR_PKT) {
> +		u16 opcode = 0x00;
> +		u8 *packet;
> +		int len;
> +
> +		/* Change vendor event to command complete event with
> +		 * coresponding data which will help us unblock
> +		 * __hci_cmd_sync() calls
> +		 */
> +		packet = skb->data+sizeof(*hdr);
> +		len = skb->len-sizeof(*hdr);
> +
> +		if (hdev->sent_cmd != NULL) {
> +			struct hci_command_hdr *sent;
> +			sent = (void*)hdev->sent_cmd->data;
> +			opcode = __le16_to_cpu(sent->opcode);
> +		}
> +
> +		rome_inject_cmd_complete(hdev, opcode, 0x00, packet, len);
> +
> +		kfree_skb(skb);
> +
> +		return true;
> +	}
> +
> +	return false;
> +}
> +EXPORT_SYMBOL_GPL(rome_vendor_frame);

Explain to me what is actually going on here? With these things I prefer that proper comments are added to explain what is going on so that people can follow this. Can you share the HCI vendor command documentation offline with me.

Also I get the feeling that doing this via h4_recv_buf if needed is a way better approach.

> +
> +MODULE_AUTHOR("Ben Young Tae Kim <ytkim@xxxxxxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Bluetooth support for Qualcomm Atheros family ver " VERSION);
> +MODULE_VERSION(VERSION);
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
> new file mode 100644
> index 0000000..47dbb74
> --- /dev/null
> +++ b/drivers/bluetooth/btqca.h
> @@ -0,0 +1,166 @@
> +/*
> + *  Bluetooth supports for Qualcomm Atheros ROME chips
> + *
> + *  Copyright (c) 2015 The Linux Foundation. All rights reserved.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2
> + *  as published by the Free Software Foundation
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + */
> +
> +#define QCA_FEATURE_UART_IN_BAND_SLEEP
> +#define QCA_FEATURE_DEEP_SLEEP
> +//#define QCA_FEATURE_DEBUG
> +
> +#define QCA_ROME_BAUDRATE_3M		(0x0E)
> +#define EDL_PATCH_BAUDRATE		(0xFC48)
> +#define EDL_PATCH_CMD_OPCODE		(0xFC00)
> +#define EDL_NVM_ACCESS_OPCODE		(0xFC0B)
> +#define EDL_PATCH_CMD_LEN		(1)
> +#define EDL_PATCH_VER_REQ_CMD		(0x19)
> +#define EDL_PATCH_TLV_REQ_CMD		(0x1E)
> +#define EDL_NVM_ACCESS_SET_REQ_CMD	(0x01)
> +#define MAX_SIZE_PER_TLV_SEGMENT	(243)
> +
> +#define EDL_CMD_REQ_RES_EVT		(0x00)
> +#define EDL_PATCH_VER_RES_EVT		(0x19)
> +#define EDL_APP_VER_RES_EVT		(0x02)
> +#define EDL_TVL_DNLD_RES_EVT		(0x04)
> +#define EDL_CMD_EXE_STATUS_EVT		(0x00)
> +#define EDL_SET_BAUDRATE_RSP_EVT	(0x92)
> +#define EDL_NVM_ACCESS_CODE_EVT		(0x0B)
> +
> +enum qca_bardrate_type {
> +	QCA_BAUDRATE_115200 	= 0,
> +	QCA_BAUDRATE_57600,
> +	QCA_BAUDRATE_38400,
> +	QCA_BAUDRATE_19200,
> +	QCA_BAUDRATE_9600,
> +	QCA_BAUDRATE_230400,
> +	QCA_BAUDRATE_250000,
> +	QCA_BAUDRATE_460800,
> +	QCA_BAUDRATE_500000,
> +	QCA_BAUDRATE_720000,
> +	QCA_BAUDRATE_921600,
> +	QCA_BAUDRATE_1000000,
> +	QCA_BAUDRATE_1250000,
> +	QCA_BAUDRATE_2000000,
> +	QCA_BAUDRATE_3000000,
> +	QCA_BAUDRATE_4000000,
> +	QCA_BAUDRATE_1600000,
> +	QCA_BAUDRATE_3200000,
> +	QCA_BAUDRATE_3500000,
> +	QCA_BAUDRATE_AUTO 	= 0xFE,
> +	QCA_BAUDRATE_RESERVED
> +};
> +
> +enum rome_tlv_type {
> +	TLV_TYPE_PATCH = 1,
> +	TLV_TYPE_NVM
> +};
> +
> +struct edl_event_hdr {
> +	__u8	state;
> +	__u8	cresp;
> +	__u8	rtype;
> +	__u8	data[0];
> +} __packed;
> +
> +struct rome_version {
> +	__le32 product_id;
> +	__le16 patch_ver;
> +	__le16 rome_ver;
> +	__le32 soc_id;
> +} __packed;
> +
> +struct tlv_seg_resp {
> +	__u8 result;
> +} __packed;
> +
> +struct tlv_type_patch
> +{

The { belong on the previous line.

> +	__le32 total_size;
> +	__le32 data_length;
> +	__u8 format_version;
> +	__u8 signature;
> +	__le16 reserved1;
> +	__le16 product_id;
> +	__le16 rom_build;
> +	__le16 patch_version;
> +	__le16 reserved2;
> +	__le32 entry;
> +} __packed;
> +
> +struct tlv_type_nvm
> +{

The { belongs on the previous line.

> +	__le16 tag_id;
> +	__le16 tag_len;
> +	__le32 reserve1;
> +	__le32 reserve2;
> +	__u8 data[0];
> +} __packed;
> +
> +struct tlv_type_hdr
> +{

The { belongs on the previous line.

> +	__le32 type_len;
> +	__u8 data[0];
> +} __packed;
> +
> +#if IS_ENABLED(CONFIG_BT_QCA)
> +
> +uint8_t rome_get_baudrate(int speed);
> +int rome_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
> +void rome_serial_clock_on(struct tty_struct *tty);
> +void rome_serial_clock_off(struct tty_struct *tty);
> +int rome_uart_setup(struct hci_dev *hdev, int speed);
> +int rome_vendor_frame(struct hci_dev *hdev, struct sk_buff *skb);
> +void rome_debug_dump(const u8 *cmd, int size, bool outbound);
> +
> +#else
> +
> +static inline uint8_t rome_get_baudrate(int speed)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline int rome_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline void rome_serial_clock_on(struct tty_struct *tty)
> +{
> +	return -EOPNOTSUPP;

Have you actually even compile tested this config option. void function do not return a value.

> +}
> +
> +static inline void rome_serial_clock_off(struct tty_struct *tty)
> +{
> +	return -EOPNOTSUPP;

See above.

> +}
> +
> +static inline int rome_uart_setup(struct hci_dev *hdev, int speed)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline int rome_vendor_frame(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline static void rome_debug_dump(const u8 *cmd, int size, bool outbound)
> +{
> +	return -EOPNOTSUPP;

See above.

> +}
> +
> +#endif

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



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux