Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\))
Subject: Re: [PATCH v2] Bluetooth: btrtl: Ask 8821C to drop old firmware
From:   Kai-Heng Feng <kai.heng.feng@canonical.com>
In-Reply-To: <20201026082838.26532-1-kai.heng.feng@canonical.com>
Date:   Mon, 2 Nov 2020 21:32:33 +0800
Cc:     Marcel Holtmann <marcel@holtmann.org>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        "open list:BLUETOOTH DRIVERS" <linux-bluetooth@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Content-Transfer-Encoding: 8BIT
Message-Id: <1A6AB99C-425D-42D5-B1F4-818AD7525F53@canonical.com>
References: <20201026082838.26532-1-kai.heng.feng@canonical.com>
To:     Max Chou <max.chou@realtek.com>, alex_lu <alex_lu@realsil.com.cn>
Precedence: bulk


> On Oct 26, 2020, at 16:28, Kai-Heng Feng <kai.heng.feng@canonical.com> wrote:
> 
> Some platforms keep USB power even when they are powered off and in S5,
> this makes Realtek 8821C keep its firmware even after a cold boot, and
> make 8821C never load new firmware.
> 
> So use vendor specific HCI command to ask 8821C drop its firmware after
> system shutdown.
> 
> Newer firmware doesn't have this issue so we only use this trick for old
> 8821C firmware version.
> 
> Suggested-by: Max Chou <max.chou@realtek.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

Max and Alex,

Can you please ack or review the patch?

Kai-Heng

> ---
> v2:
> - Fix incorrect parAnthesis on le16_to_cpu.
> - Ensure firmware gets re-uploaded in initialization.
> 
> drivers/bluetooth/btrtl.c | 46 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
> 
> diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> index 3a9afc905f24..37e24bbb2eb4 100644
> --- a/drivers/bluetooth/btrtl.c
> +++ b/drivers/bluetooth/btrtl.c
> @@ -55,6 +55,7 @@ struct btrtl_device_info {
> 	int fw_len;
> 	u8 *cfg_data;
> 	int cfg_len;
> +	bool drop_fw;
> };
> 
> static const struct id_table ic_id_table[] = {
> @@ -563,6 +564,8 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
> 	u16 hci_rev, lmp_subver;
> 	u8 hci_ver;
> 	int ret;
> +	u16 opcode;
> +	u8 cmd[2];
> 
> 	btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL);
> 	if (!btrtl_dev) {
> @@ -584,6 +587,49 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
> 	hci_ver = resp->hci_ver;
> 	hci_rev = le16_to_cpu(resp->hci_rev);
> 	lmp_subver = le16_to_cpu(resp->lmp_subver);
> +
> +	if (resp->hci_ver == 0x8 && le16_to_cpu(resp->hci_rev) == 0x826c &&
> +	    resp->lmp_ver == 0x8 && le16_to_cpu(resp->lmp_subver) == 0xa99e)
> +		btrtl_dev->drop_fw = true;
> +
> +	if (btrtl_dev->drop_fw) {
> +		opcode = hci_opcode_pack(0x3f, 0x66);
> +		cmd[0] = opcode & 0xff;
> +		cmd[1] = opcode >> 8;
> +
> +		skb = bt_skb_alloc(sizeof(cmd), GFP_KERNEL);
> +		if (IS_ERR(skb))
> +			goto out_free;
> +
> +		skb_put_data(skb, cmd, sizeof(cmd));
> +		hci_skb_pkt_type(skb) = HCI_COMMAND_PKT;
> +
> +		hdev->send(hdev, skb);
> +
> +		/* Ensure the above vendor command is sent to controller and
> +		 * process has done.
> +		 */
> +		msleep(200);
> +
> +		/* Read the local version again. Expect to have the vanilla
> +		 * version as cold boot.
> +		 */
> +		skb = btrtl_read_local_version(hdev);
> +		if (IS_ERR(skb)) {
> +			ret = PTR_ERR(skb);
> +			goto err_free;
> +		}
> +
> +		resp = (struct hci_rp_read_local_version *)skb->data;
> +		rtl_dev_info(hdev, "examining hci_ver=%02x hci_rev=%04x lmp_ver=%02x lmp_subver=%04x",
> +			     resp->hci_ver, resp->hci_rev,
> +			     resp->lmp_ver, resp->lmp_subver);
> +
> +		hci_ver = resp->hci_ver;
> +		hci_rev = le16_to_cpu(resp->hci_rev);
> +		lmp_subver = le16_to_cpu(resp->lmp_subver);
> +	}
> +out_free:
> 	kfree_skb(skb);
> 
> 	btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev, hci_ver,
> -- 
> 2.17.1
>