Hi! Paul, Thanks for your feedback. I just send the V3 to fix auto build error because I did not rebase for the previous patch. Please check my comment for the questions. Thanks. > -----Original Message----- > From: Paul Menzel [mailto:pmenzel@xxxxxxxxxxxxx] > Sent: Tuesday, August 20, 2024 1:37 PM > To: Max Chou <max.chou@xxxxxxxxxxx> > Cc: Marcel Holtmann <marcel@xxxxxxxxxxxx>; Luiz Augusto von Dentz > <luiz.dentz@xxxxxxxxx>; linux-bluetooth@xxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx; Hilda Wu <hildawu@xxxxxxxxxxx>; > alex_lu@xxxxxxxxxxxxxx; KidmanLee <kidman@xxxxxxxxxxx>; Karen Hsu > <karenhsu@xxxxxxxxxxx> > Subject: Re: [PATCH V2 RESEND] Bluetooth: btrtl: Add the support for > RTL8922A > > > External mail. > > > > Dear Max, > > > Thank you for your patch. > > Am 20.08.24 um 07:16 schrieb max.chou@xxxxxxxxxxx: > > From: Max Chou <max.chou@xxxxxxxxxxx> > There're some different chips in RTL8922A series. RTL8922AE and RTL8922AU are BT USB I/F, and this patch was committed for that. RTL8922AE-VR is BT UART I/F. I think " RTL8922A BT controller on USB interface." is more common. There is the same description for the past patch as well. > The article *the* in the commit message summary is uncommon. Maybe: > > Add support for RTL8922A > > > Add the support for RTL8922A BT controller on USB interface. > > Ditto. > > > The necessary firmware will be submitted to linux-firmware project. > For USB I/F, Realtek assigns the FW filename as rtlXXXXXu_fw.bin. When this patch is merged, I will submit the FW file to linux-firmware. > Maybe add the firmware file name `rtl8922au_fw`? > > If you could also document the device you tested with, that means which > has that Bluetooth controller, that might be good to know too. > > > The device info from /sys/kernel/debug/usb/devices as below. > > > > T: Bus=01 Lev=02 Prnt=02 Port=02 Cnt=01 Dev#= 3 Spd=12 MxCh= 0 > > D: Ver= 1.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs= 1 > > P: Vendor=0bda ProdID=8922 Rev= 0.00 > > S: Manufacturer=Realtek > > S: Product=Bluetooth Radio > > S: SerialNumber=00E04C885A01 > > C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=500mA > > I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb > > E: Ad=81(I) Atr=03(Int.) MxPS= 16 Ivl=1ms > > E: Ad=02(O) Atr=02(Bulk) MxPS= 64 Ivl=0ms > > E: Ad=82(I) Atr=02(Bulk) MxPS= 64 Ivl=0ms > > I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb > > E: Ad=03(O) Atr=01(Isoc) MxPS= 0 Ivl=1ms > > E: Ad=83(I) Atr=01(Isoc) MxPS= 0 Ivl=1ms > > I: If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb > > E: Ad=03(O) Atr=01(Isoc) MxPS= 9 Ivl=1ms > > E: Ad=83(I) Atr=01(Isoc) MxPS= 9 Ivl=1ms > > I: If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb > > E: Ad=03(O) Atr=01(Isoc) MxPS= 17 Ivl=1ms > > E: Ad=83(I) Atr=01(Isoc) MxPS= 17 Ivl=1ms > > I: If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb > > E: Ad=03(O) Atr=01(Isoc) MxPS= 25 Ivl=1ms > > E: Ad=83(I) Atr=01(Isoc) MxPS= 25 Ivl=1ms > > I: If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb > > E: Ad=03(O) Atr=01(Isoc) MxPS= 33 Ivl=1ms > > E: Ad=83(I) Atr=01(Isoc) MxPS= 33 Ivl=1ms > > I: If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb > > E: Ad=03(O) Atr=01(Isoc) MxPS= 49 Ivl=1ms > > E: Ad=83(I) Atr=01(Isoc) MxPS= 49 Ivl=1ms > > I: If#= 1 Alt= 6 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb > > E: Ad=03(O) Atr=01(Isoc) MxPS= 63 Ivl=1ms > > E: Ad=83(I) Atr=01(Isoc) MxPS= 63 Ivl=1ms > > > > Signed-off-by: Max Chou <max.chou@xxxxxxxxxxx> > > --- > > change: > > v2: edit commit log > > --- > > drivers/bluetooth/btrtl.c | 16 ++++++++++++++++ > > drivers/bluetooth/btusb.c | 13 +++++++++++++ > > 2 files changed, 29 insertions(+) > > > > diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c > > index f2f37143c454..555c2964ec1b 100644 > > --- a/drivers/bluetooth/btrtl.c > > +++ b/drivers/bluetooth/btrtl.c > > @@ -30,6 +30,7 @@ > > #define RTL_ROM_LMP_8822B 0x8822 > > #define RTL_ROM_LMP_8852A 0x8852 > > #define RTL_ROM_LMP_8851B 0x8851 > > +#define RTL_ROM_LMP_8922A 0x8922 > > #define RTL_CONFIG_MAGIC 0x8723ab55 > > > > #define RTL_VSC_OP_COREDUMP 0xfcff > > @@ -69,6 +70,7 @@ enum btrtl_chip_id { > > CHIP_ID_8852B = 20, > > CHIP_ID_8852C = 25, > > CHIP_ID_8851B = 36, > > + CHIP_ID_8922A = 44, > > CHIP_ID_8852BT = 47, > > }; > > > > @@ -309,6 +311,15 @@ static const struct id_table ic_id_table[] = { > > .cfg_name = "rtl_bt/rtl8851bu_config", > > .hw_info = "rtl8851bu" }, > > > > + /* 8922A */ > > + { IC_INFO(RTL_ROM_LMP_8922A, 0xa, 0xc, HCI_USB), > > + .config_needed = false, > > + .has_rom_version = true, > > + .has_msft_ext = true, > > + .fw_name = "rtl_bt/rtl8922au_fw", > > + .cfg_name = "rtl_bt/rtl8922au_config", > > + .hw_info = "rtl8922au" }, > > + > The chip id is 44 for RTL8922A. I exactly sort it. (between 36 and 47) > Why not sort it? > > > /* 8852BT/8852BE-VT */ > > { IC_INFO(RTL_ROM_LMP_8852A, 0x87, 0xc, HCI_USB), > > .config_needed = false, > > @@ -655,6 +666,7 @@ static int rtlbt_parse_firmware(struct hci_dev > *hdev, > > { RTL_ROM_LMP_8852A, 20 }, /* 8852B */ > > { RTL_ROM_LMP_8852A, 25 }, /* 8852C */ > > { RTL_ROM_LMP_8851B, 36 }, /* 8851B */ > > + { RTL_ROM_LMP_8922A, 44 }, /* 8922A */ > > { RTL_ROM_LMP_8852A, 47 }, /* 8852BT */ > > }; > > > > @@ -1255,6 +1267,7 @@ int btrtl_download_firmware(struct hci_dev > *hdev, > > case RTL_ROM_LMP_8852A: > > case RTL_ROM_LMP_8703B: > > case RTL_ROM_LMP_8851B: > > + case RTL_ROM_LMP_8922A: > > err = btrtl_setup_rtl8723b(hdev, btrtl_dev); > > break; > > default: > > @@ -1286,6 +1299,7 @@ void btrtl_set_quirks(struct hci_dev *hdev, struct > btrtl_device_info *btrtl_dev) > > case CHIP_ID_8852B: > > case CHIP_ID_8852C: > > case CHIP_ID_8851B: > > + case CHIP_ID_8922A: > > case CHIP_ID_8852BT: > In rtlbt_parse_firmware(), chip id 44 is between 36(8851B) and 47(8852BT). Hence, I just followed the sequence here as well. > Why not sort it? > > > set_bit(HCI_QUIRK_VALID_LE_STATES, &hdev->quirks); > > set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, > > &hdev->quirks); @@ -1529,3 +1543,5 @@ > MODULE_FIRMWARE("rtl_bt/rtl8852btu_config.bin"); > > MODULE_FIRMWARE("rtl_bt/rtl8852cu_fw.bin"); > > MODULE_FIRMWARE("rtl_bt/rtl8852cu_fw_v2.bin"); > > MODULE_FIRMWARE("rtl_bt/rtl8852cu_config.bin"); > > +MODULE_FIRMWARE("rtl_bt/rtl8922au_fw.bin"); > > +MODULE_FIRMWARE("rtl_bt/rtl8922au_config.bin"); > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > > index acdba5d77694..8f931cfbac11 100644 > > --- a/drivers/bluetooth/btusb.c > > +++ b/drivers/bluetooth/btusb.c > > @@ -540,6 +540,8 @@ static const struct usb_device_id quirks_table[] = { > > > BTUSB_WIDEBAND_SPEECH }, > > { USB_DEVICE(0x13d3, 0x3592), .driver_info = BTUSB_REALTEK | > > > > BTUSB_WIDEBAND_SPEECH }, > > + { USB_DEVICE(0x0489, 0xe122), .driver_info = BTUSB_REALTEK | > > + > > + BTUSB_WIDEBAND_SPEECH }, > > Why not sort it? > > > > > /* Realtek 8852BE Bluetooth devices */ > > { USB_DEVICE(0x0cb8, 0xc559), .driver_info = BTUSB_REALTEK | > @@ > > -564,6 +566,17 @@ static const struct usb_device_id quirks_table[] = { > > /* Realtek 8852BT/8852BE-VT Bluetooth devices */ > > { USB_DEVICE(0x0bda, 0x8520), .driver_info = BTUSB_REALTEK | > > > > BTUSB_WIDEBAND_SPEECH }, > > + > > + /* Realtek 8922AE Bluetooth devices */ > > + { USB_DEVICE(0x0bda, 0x8922), .driver_info = BTUSB_REALTEK | > > + > BTUSB_WIDEBAND_SPEECH }, > > + { USB_DEVICE(0x13d3, 0x3617), .driver_info = BTUSB_REALTEK | > > + > BTUSB_WIDEBAND_SPEECH }, > > + { USB_DEVICE(0x13d3, 0x3616), .driver_info = BTUSB_REALTEK | > > + > > + BTUSB_WIDEBAND_SPEECH }, > > Why not sort it? > > > + { USB_DEVICE(0x0489, 0xe130), .driver_info = BTUSB_REALTEK | > > + > > + BTUSB_WIDEBAND_SPEECH }, > > + > > /* Realtek Bluetooth devices */ > > { USB_VENDOR_AND_INTERFACE_INFO(0x0bda, 0xe0, 0x01, 0x01), > > .driver_info = BTUSB_REALTEK },
