Hi Marcel: What I plan to do about adapter power cycle for quality report: Step 1: At power off, do not clear the flag of HCI_QUALITY_REPORT in hci_dev_clear_volatile_flags(). Will use HCI_QUALITY_REPORT to track if the quality report is enabled before power off. Will also disable the quality report explicitly in hci_sync.c:hci_dev_open_sync() just before aosp_do_close() so that different vendor chips will have quality off at power off. Step 2: In hci_sync.c:hci_dev_open_sync(), re-enable quality report explicitly just after aosp_do_open() if HCI_QUALITY_REPORT is true so that all vendor chips have quality back on at power on. If the quality report is not enabled before power cycle, HCI_QUALITY_REPORT will always stay false. Nothing mentioned above will be executed in this case. One thing is worth noting here. The HCI_QUALITY_REPORT represents the host setting instead of the controller state. During power off, the HCI_QUALITY_REPORT host setting remains true while the controller state about quality report is turned off. This behavior is similar to "wide-band-speech" whose setting remains true during power off. Does this sound good to you? If yes, I will append a new patch to the next Series-version. Thanks! On Sun, Mar 6, 2022 at 5:53 AM Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote: > > Hi Joseph, > > >>> Add the MGMT quality report command and event in doc/mgmt-api.txt. > >>> > >>> Signed-off-by: Joseph Hwang <josephsih@xxxxxxxxxxxx> > >>> --- > >>> > >>> Changes in v4: > >>> - Use "Quality Report Event" without the prefix "Bluetooth" word. > >>> - Combine both MGMT quality report command and event changes in a > >>> single patch. > >>> > >>> Changes in v3: > >>> - Swap AOSP Bluetooth Quality Report Event and Intel Telemetry Event. > >>> - Add 5 new patches (5/9 - 9/9) to enable the quality report > >>> feature via MGMT_OP_SET_QUALITY_REPORT instead of through the > >>> experimental features. > >>> > >>> Changes in v2: > >>> - This is a new patch for adding the event in doc/mgmt-api.txt > >>> > >>> doc/mgmt-api.txt | 61 ++++++++++++++++++++++++++++++++++++++++++++++++ > >>> 1 file changed, 61 insertions(+) > >>> > >>> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt > >>> index ebe56afa4..a494f5d7e 100644 > >>> --- a/doc/mgmt-api.txt > >>> +++ b/doc/mgmt-api.txt > >>> @@ -332,6 +332,7 @@ Read Controller Information Command > >>> 15 Static Address > >>> 16 PHY Configuration > >>> 17 Wideband Speech > >>> + 18 Quality Report > >>> > >>> This command generates a Command Complete event on success or > >>> a Command Status event on failure. > >>> @@ -2924,6 +2925,7 @@ Read Extended Controller Information Command > >>> 15 Static Address > >>> 16 PHY Configuration > >>> 17 Wideband Speech > >>> + 18 Quality Report > >>> > >>> The EIR_Data field contains information about class of device, > >>> local name and other values. Not all of them might be present. For > >>> @@ -3858,6 +3860,46 @@ Add Advertisement Patterns Monitor With RSSI Threshold Command > >>> Invalid Parameters > >>> > >>> > >>> +Set Quality Report Command > >>> +========================== > >>> + > >>> + Command Code: 0x0057 > >>> + Controller Index: <controller id> > >>> + Command Parameters: Action (1 Octet) > >> > >> I remember mentioning that we should use Quality_Report instead of Action. > >> > >>> + Return Parameters: Current_Settings (4 Octets) > >>> + > >>> + This command is used to enable and disable the controller's quality > >>> + report feature. The allowed values for the Action command parameter > >>> + are 0x00 and 0x01. All other values will return Invalid Parameters. > >>> + > >>> + The value 0x00 disables the Quality Report, and the value 0x01 > >>> + enables the Quality Report feature. > >>> + > >>> + This command is only available for the controllers that support > >>> + either AOSP Bluetooth quality report or Intel telemetry event. > >> > >> The details below are interesting, but don’t have to be in this document. It is supported if the Supported_Settings indicate support for it. > >> > >>> + For a controller supporting the AOSP specification, it should call > >>> + hci_set_aosp_capable() in its driver. The controller should also > >>> + return version_supported v0.98 or higher in its Vendor-specific > >>> + capabilities responding to the LE_Get_Vendor_Capabilities_Command. > >>> + On the other hand, for a controller supporting Intel specification, > >>> + it should set up the set_quality_report callback properly. The driver > >>> + is responsible of setting up the quality report capability as > >>> + described above; otherwise, a Not Supported status will be returned. > >>> + > >>> + This command requires to use a valid controller index. Otherwise, > >>> + an Invalid Index status will be returned. > >>> + > >>> + The command is sent to the controller to enable/disable the quality > >>> + report feature, and generates a Command Complete event on success. > >>> + If the controller failed to execute the action, a Failed status will > >>> + be returned. > >> > >> Can this be used when powered off, is it remembered over power off/on cycles etc. > > > > It is not remembered by the Intel controller over power cycles. I will > > test the other AOSP vendors, and plan to address this issue in > > separate patches in which I will describe the behavior explicitly. > > Thanks. > > I think this means that on every power on we have to reprogram it if it was enabled before. That is how we handle other settings. They survive power cycles. And since we have a Current_Settings flags for this, I would expect it to behave exactly the same. > > Regards > > Marcel > -- Joseph Shyh-In Hwang Email: josephsih@xxxxxxxxxx
