* Re: [PATCH 1/2] Bluetooth: btusb: Record debug log for Mediatek Chip.
[not found] <20210609190734.30088-1-mark-yw.chen@mediatek.com>
@ 2021-06-09 19:17 ` Marcel Holtmann
[not found] ` <20210609190734.30088-2-mark-yw.chen@mediatek.com>
1 sibling, 0 replies; 2+ messages in thread
From: Marcel Holtmann @ 2021-06-09 19:17 UTC (permalink / raw)
To: mark-yw.chen
Cc: Johan Hedberg, chris.lu, will-cy.lee, Sean Wang,
Bluez mailing list, linux-mediatek, open list, michaelfsun,
shawnku, jemele
Hi Mark,
> 1. Add btusb_recv_bulk_mtk to receive debug log.
> 2. Send the debug log via hci_recv_diag channel.
> 3. The upper layerd use hci_channel_minitor to receive
> these packets.
>
> Signed-off-by: mark-yw.chen <mark-yw.chen@mediatek.com>
> ---
> drivers/bluetooth/btusb.c | 67 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index b015dcecfb13..2a55ae0a5f8c 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3851,6 +3851,72 @@ static int btusb_mtk_shutdown(struct hci_dev *hdev)
> return 0;
> }
>
> +static int btusb_recv_bulk_mtk(struct btusb_data *data, void *buffer, int count)
> +{
> + struct sk_buff *skb;
> + unsigned long flags;
> + int err = 0;
> +
> + spin_lock_irqsave(&data->rxlock, flags);
> + skb = data->acl_skb;
> +
> + while (count) {
> + int len;
> +
> + if (!skb) {
> + skb = bt_skb_alloc(HCI_MAX_FRAME_SIZE, GFP_ATOMIC);
> + if (!skb) {
> + err = -ENOMEM;
> + break;
> + }
> +
> + hci_skb_pkt_type(skb) = HCI_ACLDATA_PKT;
> + hci_skb_expect(skb) = HCI_ACL_HDR_SIZE;
> + }
> +
> + len = min_t(uint, hci_skb_expect(skb), count);
> + skb_put_data(skb, buffer, len);
> +
> + count -= len;
> + buffer += len;
> + hci_skb_expect(skb) -= len;
> +
> + if (skb->len == HCI_ACL_HDR_SIZE) {
> + __le16 dlen = hci_acl_hdr(skb)->dlen;
> +
> + /* Complete ACL header */
> + hci_skb_expect(skb) = __le16_to_cpu(dlen);
> +
> + if (skb_tailroom(skb) < hci_skb_expect(skb)) {
> + kfree_skb(skb);
> + skb = NULL;
> +
> + err = -EILSEQ;
> + break;
> + }
> + }
> +
> + if (!hci_skb_expect(skb)) {
> + /* Complete frame */
> + if (skb->data[0] == 0x6f && skb->data[1] == 0xfc) {
> + usb_disable_autosuspend(data->udev);
> + hci_recv_diag(data->hdev, skb);
> + } else if ((skb->data[0] == 0xfe || skb->data[0] == 0xff) &&
> + skb->data[1] == 0x05) {
> + hci_recv_diag(data->hdev, skb);
> + } else {
> + hci_recv_frame(data->hdev, skb);
> + }
> + skb = NULL;
> + }
> + }
> +
> + data->acl_skb = skb;
> + spin_unlock_irqrestore(&data->rxlock, flags);
> +
> + return err;
> +}
> +
> MODULE_FIRMWARE(FIRMWARE_MT7663);
> MODULE_FIRMWARE(FIRMWARE_MT7668);
>
> @@ -4669,6 +4735,7 @@ static int btusb_probe(struct usb_interface *intf,
> hdev->setup = btusb_mtk_setup;
> hdev->shutdown = btusb_mtk_shutdown;
> hdev->manufacturer = 70;
> + data->recv_bulk = btusb_recv_bulk_mtk;
> set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
NO. The recv_bulk was meant for cases when bulk was faster and needed to redirected as interrupt/event data. Doing a complete copy of btusb_recv_bulk is not acceptable. You need to introduce data->recv_acl.
Regards
Marcel
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH 2/2] Bluetooth: btusb: Support Bluetooth Reset for Mediatek Chip.
[not found] ` <20210609190734.30088-2-mark-yw.chen@mediatek.com>
@ 2021-06-09 19:21 ` Marcel Holtmann
0 siblings, 0 replies; 2+ messages in thread
From: Marcel Holtmann @ 2021-06-09 19:21 UTC (permalink / raw)
To: mark-yw.chen
Cc: Johan Hedberg, chris.lu, will-cy.lee, sean.wang, linux-bluetooth,
linux-mediatek, linux-kernel, michaelfsun, shawnku, jemele
Hi Mark,
> This change enable Mediatek reset mechanism via USB, it's a specific
> reset mechanism with Mediatek chips. To handle error recovery, it use
> the cmd_timoeut to reset Mediatek Bluetooth.
>
> Signed-off-by: mark-yw.chen <mark-yw.chen@mediatek.com>
> ---
> drivers/bluetooth/btusb.c | 145 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 145 insertions(+)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 2a55ae0a5f8c..acc23afbfe0f 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3075,6 +3075,18 @@ static int btusb_shutdown_intel_new(struct hci_dev *hdev)
> return 0;
> }
>
> +/* UHW CR mapping */
> +#define BT_MISC 0x70002510
> +#define BT_SUBSYS_RST 0x70002610
> +#define UDMA_INT_STA_BT 0x74000024
> +#define UDMA_INT_STA_BT1 0x74000308
> +#define BT_WDT_STATUS 0x740003A0
> +#define EP_RST_OPT 0x74011890
> +#define EP_RST_IN_OUT_OPT 0x00010001
> +#define BT_RST_DONE 0x00000100
> +#define BT_RESET_WAIT_MS 100
> +#define BT_RESET_NUM_TRIES 10
> +
> #define FIRMWARE_MT7663 "mediatek/mt7663pr2h.bin"
> #define FIRMWARE_MT7668 "mediatek/mt7668pr2h.bin"
>
> @@ -3649,6 +3661,59 @@ static int btusb_mtk_func_query(struct hci_dev *hdev)
> return status;
> }
>
> +static int btusb_mtk_uhw_reg_write(struct btusb_data *data, u32 reg, u32 val)
> +{
> + int pipe, err, size = sizeof(u32);
> + u8 buf[4];
> +
> + buf[0] = (val & 0x00ff);
> + buf[1] = ((val >> 8) & 0x00ff);
> + buf[2] = ((val >> 16) & 0x00ff);
> + buf[3] = ((val >> 24) & 0x00ff);
can we not just just put_unalined_le32 in these cases.
> +
> + pipe = usb_sndctrlpipe(data->udev, 0);
> + err = usb_control_msg(data->udev, pipe, 0x02,
> + 0x5E,
> + reg >> 16, reg & 0xffff,
> + buf, size, USB_CTRL_SET_TIMEOUT);
The extra size variable is pointless.
> + if (err < 0) {
> + BT_ERR("Failed to write uhw reg(%d)", err);
Please use bt_dev_err.
> + goto err_free_buf;
> + }
> +
> +err_free_buf:
> +
> + return err;
> +}
> +
> +static int btusb_mtk_uhw_reg_read(struct btusb_data *data, u32 reg, u32 *val)
> +{
> + int pipe, err, size = sizeof(u32);
> + void *buf;
> +
> + buf = kzalloc(size, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
Pointless size variable. Just use the value 4.
> +
> + pipe = usb_rcvctrlpipe(data->udev, 0);
> + err = usb_control_msg(data->udev, pipe, 0x01,
> + 0xDE,
> + reg >> 16, reg & 0xffff,
> + buf, size, USB_CTRL_SET_TIMEOUT);
> + if (err < 0) {
> + BT_ERR("Failed to read uhw reg(%d)", err);
> + goto err_free_buf;
> + }
> +
> + *val = get_unaligned_le32(buf);
> + BT_INFO("%s: reg=%x, value=0x%08x", __func__, reg, *val);
> +
> +err_free_buf:
> + kfree(buf);
> +
> + return err;
> +}
> +
> static int btusb_mtk_reg_read(struct btusb_data *data, u32 reg, u32 *val)
> {
> int pipe, err, size = sizeof(u32);
> @@ -3728,6 +3793,7 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
> dev_id & 0xffff, (fw_version & 0xff) + 1);
> err = btusb_mtk_setup_firmware_79xx(hdev, fw_bin_name);
>
> + btusb_mtk_uhw_reg_write(data, EP_RST_OPT, EP_RST_IN_OUT_OPT);
Extra line below and comment above on what this is doing.
> /* Enable Bluetooth protocol */
> param = 1;
> wmt_params.op = BTMTK_WMT_FUNC_CTRL;
> @@ -3851,6 +3917,84 @@ static int btusb_mtk_shutdown(struct hci_dev *hdev)
> return 0;
> }
>
> +static void btusb_mtk_cmd_timeout(struct hci_dev *hdev)
> +{
> + struct btusb_data *data = hci_get_drvdata(hdev);
> + u32 val;
> + int err, retry = 0;
> +
> + /*
> + * Toggle the bluetooth reset via usb. It's mediatek spicific reset
> + * mechanism, and the reset flow won't replug the USB. Thus, it must
> + * handle the urb transaction, such as stop_traffic and submit_urb.
> + * After bluetooth reset completed, we will re-download the firmware
> + * and send hw_err to host.
> + */
Wrong comment style.
> + if (test_and_set_bit(BTUSB_HW_RESET_ACTIVE, &data->flags)) {
> + bt_dev_err(hdev, "last reset failed? Not resetting again");
> + return;
> + }
> +
> + if (enable_autosuspend)
> + usb_disable_autosuspend(data->udev);
> +
> + data->sco_num = 0;
> +
> + btusb_stop_traffic(data);
> + usb_kill_anchored_urbs(&data->tx_anchor);
> +
> + /* For reset */
> + bt_dev_info(hdev, "Initiating reset mechanism via uhw");
Extra empty line.
> + /*
> + * Todo - add error handling for usb no response case and uhw
> + * something worng.
> + */
Wrong comment style.
> + btusb_mtk_uhw_reg_write(data, EP_RST_OPT, EP_RST_IN_OUT_OPT);
> + btusb_mtk_uhw_reg_read(data, BT_WDT_STATUS, &val);
> +
> + /* Write Reset CR to 1 */
> + btusb_mtk_uhw_reg_write(data, BT_SUBSYS_RST, 1);
> +
> + btusb_mtk_uhw_reg_write(data, UDMA_INT_STA_BT, 0x000000FF);
> + btusb_mtk_uhw_reg_read(data, UDMA_INT_STA_BT, &val);
> + btusb_mtk_uhw_reg_write(data, UDMA_INT_STA_BT1, 0x000000FF);
> + btusb_mtk_uhw_reg_read(data, UDMA_INT_STA_BT1, &val);
> + msleep(1);
> +
> + /* Write Reset CR to 0 */
> + btusb_mtk_uhw_reg_write(data, BT_SUBSYS_RST, 0);
> +
> + /* Read reset CR */
> + btusb_mtk_uhw_reg_read(data, BT_SUBSYS_RST, &val);
> +
> + do {
> + btusb_mtk_uhw_reg_read(data, BT_MISC, &val);
> + if ((val & 0x00000100) == BT_RST_DONE) {
> + bt_dev_info(hdev, "Bluetooth Reset Sucessfully");
> + break;
> + } else
> + bt_dev_dbg(hdev, "Polling Bluetooth Reset CR");
Close here as well with { }.
> +
> + retry++;
> + msleep(BT_RESET_WAIT_MS);
> + } while (retry < BT_RESET_NUM_TRIES);
> +
> + btusb_mtk_id_get(data, 0x70010200, &val);
> + bt_dev_info(hdev, "device id (%x)", val);
> +
> + err = btusb_mtk_setup(hdev);
> + if (err < 0) {
> + bt_dev_err(hdev, "mtk_setup failed, err = %d", err);
> + return;
> + }
> + hci_reset_dev(hdev);
> +
> + if (enable_autosuspend)
> + usb_enable_autosuspend(data->udev);
> +
> + clear_bit(BTUSB_HW_RESET_ACTIVE, &data->flags);
> +}
> +
> static int btusb_recv_bulk_mtk(struct btusb_data *data, void *buffer, int count)
> {
> struct sk_buff *skb;
> @@ -4736,6 +4880,7 @@ static int btusb_probe(struct usb_interface *intf,
> hdev->shutdown = btusb_mtk_shutdown;
> hdev->manufacturer = 70;
> data->recv_bulk = btusb_recv_bulk_mtk;
> + hdev->cmd_timeout = btusb_mtk_cmd_timeout;
> set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
> }
Regards
Marcel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-06-09 19:21 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20210609190734.30088-1-mark-yw.chen@mediatek.com>
2021-06-09 19:17 ` [PATCH 1/2] Bluetooth: btusb: Record debug log for Mediatek Chip Marcel Holtmann
[not found] ` <20210609190734.30088-2-mark-yw.chen@mediatek.com>
2021-06-09 19:21 ` [PATCH 2/2] Bluetooth: btusb: Support Bluetooth Reset " Marcel Holtmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).