| Message ID | 20210118112827.6192-1-max.chou@realtek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Bluetooth: btrtl: Enable WBS for the specific Realtek devices | expand |
Hi Max, On Mon, Jan 18, 2021 at 3:28 AM <max.chou@realtek.com> wrote: > > From: Max Chou <max.chou@realtek.com> > > By this change, it will enable WBS supported on the specific Realtek BT > devices, such as RTL8822C and RTL8852A. > In the future, it's able to maintain what the Realtek devices support WBS > here. > > Tested-by: Hilda Wu <hildawu@realtek.com> > Signed-off-by: Max Chou <max.chou@realtek.com> > --- > drivers/bluetooth/btrtl.c | 26 +++++++++++++++++++++++--- > 1 file changed, 23 insertions(+), 3 deletions(-) > > diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c > index 24f03a1f8d57..835819c47ae6 100644 > --- a/drivers/bluetooth/btrtl.c > +++ b/drivers/bluetooth/btrtl.c > @@ -38,6 +38,19 @@ > .hci_ver = (hciv), \ > .hci_bus = (bus) > > +enum btrtl_chip_id { > + CHIP_ID_8723A, /* index 0 for RTL8723A*/ > + CHIP_ID_8723B, /* index 1 for RTL8723B*/ > + CHIP_ID_8821A, /* index 2 for RTL8821A*/ > + CHIP_ID_8761A, /* index 3 for RTL8761A*/ > + CHIP_ID_8822B = 8, /* index 8 for RTL8822B */ > + CHIP_ID_8723D, /* index 9 for RTL8723D */ > + CHIP_ID_8821C, /* index 10 for RTL8821C */ > + CHIP_ID_8822C = 13, /* index 13 for RTL8822C */ > + CHIP_ID_8761B, /* index 14 for RTL8761B */ > + CHIP_ID_8852A = 18, /* index 18 for RTL8852A */ > +}; > + > struct id_table { > __u16 match_flags; > __u16 lmp_subver; > @@ -58,6 +71,7 @@ struct btrtl_device_info { > u8 *cfg_data; > int cfg_len; > bool drop_fw; > + int project_id; > }; > > static const struct id_table ic_id_table[] = { > @@ -307,8 +321,10 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev, > > /* Find project_id in table */ > for (i = 0; i < ARRAY_SIZE(project_id_to_lmp_subver); i++) { > - if (project_id == project_id_to_lmp_subver[i].id) > + if (project_id == project_id_to_lmp_subver[i].id) { > + btrtl_dev->project_id = project_id; > break; > + } > } > > if (i >= ARRAY_SIZE(project_id_to_lmp_subver)) { > @@ -725,12 +741,16 @@ int btrtl_setup_realtek(struct hci_dev *hdev) > /* Enable central-peripheral role (able to create new connections with > * an existing connection in slave role). > */ > - switch (btrtl_dev->ic_info->lmp_subver) { > - case RTL_ROM_LMP_8822B: > + /* Enable WBS supported for the specific Realtek devices. */ > + switch (btrtl_dev->project_id) { > + case CHIP_ID_8822C: > + case CHIP_ID_8852A: > set_bit(HCI_QUIRK_VALID_LE_STATES, &hdev->quirks); > + set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, &hdev->quirks); > break; > default: > rtl_dev_dbg(hdev, "Central-peripheral role not enabled."); > + rtl_dev_dbg(hdev, "WBS supported not enabled."); > break; > } There is a null-check on btrtl_dev->ic_info right above this switch statement (https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=b649813eadbc062d8682f7a20aa025275707dd1f). Is this still necessary with this change? (Sorry, I missed this during our previous review). > > -- > 2.17.1 > Abhishek
Hi! Abhishek, My bad! I should check the code base is the latest and follow it. I'll submit the v2 patch. Thanks for your review. BRs, Max -----Original Message----- From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> Sent: Tuesday, January 19, 2021 12:41 PM To: Max Chou <max.chou@realtek.com> Cc: Marcel Holtmann <marcel@holtmann.org>; Johan Hedberg <johan.hedberg@gmail.com>; Luiz Augusto von Dentz <luiz.dentz@gmail.com>; Bluez mailing list <linux-bluetooth@vger.kernel.org>; LKML <linux-kernel@vger.kernel.org>; alex_lu@realsil.com.cn; Hilda Wu <hildawu@realtek.com>; KidmanLee <kidman@realtek.com>; Shyh-In Hwang <josephsih@chromium.org> Subject: Re: [PATCH] Bluetooth: btrtl: Enable WBS for the specific Realtek devices Hi Max, On Mon, Jan 18, 2021 at 3:28 AM <max.chou@realtek.com> wrote: > > From: Max Chou <max.chou@realtek.com> > > By this change, it will enable WBS supported on the specific Realtek > BT devices, such as RTL8822C and RTL8852A. > In the future, it's able to maintain what the Realtek devices support > WBS here. > > Tested-by: Hilda Wu <hildawu@realtek.com> > Signed-off-by: Max Chou <max.chou@realtek.com> > --- > drivers/bluetooth/btrtl.c | 26 +++++++++++++++++++++++--- > 1 file changed, 23 insertions(+), 3 deletions(-) > > diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c > index 24f03a1f8d57..835819c47ae6 100644 > --- a/drivers/bluetooth/btrtl.c > +++ b/drivers/bluetooth/btrtl.c > @@ -38,6 +38,19 @@ > .hci_ver = (hciv), \ > .hci_bus = (bus) > > +enum btrtl_chip_id { > + CHIP_ID_8723A, /* index 0 for RTL8723A*/ > + CHIP_ID_8723B, /* index 1 for RTL8723B*/ > + CHIP_ID_8821A, /* index 2 for RTL8821A*/ > + CHIP_ID_8761A, /* index 3 for RTL8761A*/ > + CHIP_ID_8822B = 8, /* index 8 for RTL8822B */ > + CHIP_ID_8723D, /* index 9 for RTL8723D */ > + CHIP_ID_8821C, /* index 10 for RTL8821C */ > + CHIP_ID_8822C = 13, /* index 13 for RTL8822C */ > + CHIP_ID_8761B, /* index 14 for RTL8761B */ > + CHIP_ID_8852A = 18, /* index 18 for RTL8852A */ > +}; > + > struct id_table { > __u16 match_flags; > __u16 lmp_subver; > @@ -58,6 +71,7 @@ struct btrtl_device_info { > u8 *cfg_data; > int cfg_len; > bool drop_fw; > + int project_id; > }; > > static const struct id_table ic_id_table[] = { @@ -307,8 +321,10 @@ > static int rtlbt_parse_firmware(struct hci_dev *hdev, > > /* Find project_id in table */ > for (i = 0; i < ARRAY_SIZE(project_id_to_lmp_subver); i++) { > - if (project_id == project_id_to_lmp_subver[i].id) > + if (project_id == project_id_to_lmp_subver[i].id) { > + btrtl_dev->project_id = project_id; > break; > + } > } > > if (i >= ARRAY_SIZE(project_id_to_lmp_subver)) { @@ -725,12 > +741,16 @@ int btrtl_setup_realtek(struct hci_dev *hdev) > /* Enable central-peripheral role (able to create new connections with > * an existing connection in slave role). > */ > - switch (btrtl_dev->ic_info->lmp_subver) { > - case RTL_ROM_LMP_8822B: > + /* Enable WBS supported for the specific Realtek devices. */ > + switch (btrtl_dev->project_id) { > + case CHIP_ID_8822C: > + case CHIP_ID_8852A: > set_bit(HCI_QUIRK_VALID_LE_STATES, &hdev->quirks); > + set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, > + &hdev->quirks); > break; > default: > rtl_dev_dbg(hdev, "Central-peripheral role not > enabled."); > + rtl_dev_dbg(hdev, "WBS supported not enabled."); > break; > } There is a null-check on btrtl_dev->ic_info right above this switch statement (https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=b649813eadbc062d8682f7a20aa025275707dd1f). Is this still necessary with this change? (Sorry, I missed this during our previous review). > > -- > 2.17.1 > Abhishek ------Please consider the environment before printing this e-mail.
diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c index 24f03a1f8d57..835819c47ae6 100644 --- a/drivers/bluetooth/btrtl.c +++ b/drivers/bluetooth/btrtl.c @@ -38,6 +38,19 @@ .hci_ver = (hciv), \ .hci_bus = (bus) +enum btrtl_chip_id { + CHIP_ID_8723A, /* index 0 for RTL8723A*/ + CHIP_ID_8723B, /* index 1 for RTL8723B*/ + CHIP_ID_8821A, /* index 2 for RTL8821A*/ + CHIP_ID_8761A, /* index 3 for RTL8761A*/ + CHIP_ID_8822B = 8, /* index 8 for RTL8822B */ + CHIP_ID_8723D, /* index 9 for RTL8723D */ + CHIP_ID_8821C, /* index 10 for RTL8821C */ + CHIP_ID_8822C = 13, /* index 13 for RTL8822C */ + CHIP_ID_8761B, /* index 14 for RTL8761B */ + CHIP_ID_8852A = 18, /* index 18 for RTL8852A */ +}; + struct id_table { __u16 match_flags; __u16 lmp_subver; @@ -58,6 +71,7 @@ struct btrtl_device_info { u8 *cfg_data; int cfg_len; bool drop_fw; + int project_id; }; static const struct id_table ic_id_table[] = { @@ -307,8 +321,10 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev, /* Find project_id in table */ for (i = 0; i < ARRAY_SIZE(project_id_to_lmp_subver); i++) { - if (project_id == project_id_to_lmp_subver[i].id) + if (project_id == project_id_to_lmp_subver[i].id) { + btrtl_dev->project_id = project_id; break; + } } if (i >= ARRAY_SIZE(project_id_to_lmp_subver)) { @@ -725,12 +741,16 @@ int btrtl_setup_realtek(struct hci_dev *hdev) /* Enable central-peripheral role (able to create new connections with * an existing connection in slave role). */ - switch (btrtl_dev->ic_info->lmp_subver) { - case RTL_ROM_LMP_8822B: + /* Enable WBS supported for the specific Realtek devices. */ + switch (btrtl_dev->project_id) { + case CHIP_ID_8822C: + case CHIP_ID_8852A: set_bit(HCI_QUIRK_VALID_LE_STATES, &hdev->quirks); + set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, &hdev->quirks); break; default: rtl_dev_dbg(hdev, "Central-peripheral role not enabled."); + rtl_dev_dbg(hdev, "WBS supported not enabled."); break; }