This patch adds a new set_quality_report mgmt handler to set
the quality report feature. The feature is removed from the
experimental features at the same time.
Signed-off-by: Joseph Hwang <[email protected]>
---
Changes in v4:
- return current settings for set_quality_report.
Changes in v3:
- This is a new patch to enable the quality report feature.
The reading and setting of the quality report feature are
removed from the experimental features.
include/net/bluetooth/mgmt.h | 7 ++
net/bluetooth/mgmt.c | 168 +++++++++++++++--------------------
2 files changed, 81 insertions(+), 94 deletions(-)
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 83b602636262..74d253ff617a 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -109,6 +109,7 @@ struct mgmt_rp_read_index_list {
#define MGMT_SETTING_STATIC_ADDRESS 0x00008000
#define MGMT_SETTING_PHY_CONFIGURATION 0x00010000
#define MGMT_SETTING_WIDEBAND_SPEECH 0x00020000
+#define MGMT_SETTING_QUALITY_REPORT 0x00040000
#define MGMT_OP_READ_INFO 0x0004
#define MGMT_READ_INFO_SIZE 0
@@ -838,6 +839,12 @@ struct mgmt_cp_add_adv_patterns_monitor_rssi {
} __packed;
#define MGMT_ADD_ADV_PATTERNS_MONITOR_RSSI_SIZE 8
+#define MGMT_OP_SET_QUALITY_REPORT 0x0057
+struct mgmt_cp_set_quality_report {
+ __u8 action;
+} __packed;
+#define MGMT_SET_QUALITY_REPORT_SIZE 1
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 5e48576041fb..ccd77b94905b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -857,6 +857,10 @@ static u32 get_supported_settings(struct hci_dev *hdev)
settings |= MGMT_SETTING_PHY_CONFIGURATION;
+ if (hdev && (aosp_has_quality_report(hdev) ||
+ hdev->set_quality_report))
+ settings |= MGMT_SETTING_QUALITY_REPORT;
+
return settings;
}
@@ -928,6 +932,9 @@ static u32 get_current_settings(struct hci_dev *hdev)
if (hci_dev_test_flag(hdev, HCI_WIDEBAND_SPEECH_ENABLED))
settings |= MGMT_SETTING_WIDEBAND_SPEECH;
+ if (hci_dev_test_flag(hdev, HCI_QUALITY_REPORT))
+ settings |= MGMT_SETTING_QUALITY_REPORT;
+
return settings;
}
@@ -3871,12 +3878,6 @@ static const u8 debug_uuid[16] = {
};
#endif
-/* 330859bc-7506-492d-9370-9a6f0614037f */
-static const u8 quality_report_uuid[16] = {
- 0x7f, 0x03, 0x14, 0x06, 0x6f, 0x9a, 0x70, 0x93,
- 0x2d, 0x49, 0x06, 0x75, 0xbc, 0x59, 0x08, 0x33,
-};
-
/* a6695ace-ee7f-4fb9-881a-5fac66c629af */
static const u8 offload_codecs_uuid[16] = {
0xaf, 0x29, 0xc6, 0x66, 0xac, 0x5f, 0x1a, 0x88,
@@ -3898,7 +3899,7 @@ static const u8 rpa_resolution_uuid[16] = {
static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
void *data, u16 data_len)
{
- char buf[102]; /* Enough space for 5 features: 2 + 20 * 5 */
+ char buf[82]; /* Enough space for 4 features: 2 + 20 * 4 */
struct mgmt_rp_read_exp_features_info *rp = (void *)buf;
u16 idx = 0;
u32 flags;
@@ -3939,18 +3940,6 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
idx++;
}
- if (hdev && (aosp_has_quality_report(hdev) ||
- hdev->set_quality_report)) {
- if (hci_dev_test_flag(hdev, HCI_QUALITY_REPORT))
- flags = BIT(0);
- else
- flags = 0;
-
- memcpy(rp->features[idx].uuid, quality_report_uuid, 16);
- rp->features[idx].flags = cpu_to_le32(flags);
- idx++;
- }
-
if (hdev && hdev->get_data_path_id) {
if (hci_dev_test_flag(hdev, HCI_OFFLOAD_CODECS_ENABLED))
flags = BIT(0);
@@ -4163,80 +4152,6 @@ static int set_rpa_resolution_func(struct sock *sk, struct hci_dev *hdev,
return err;
}
-static int set_quality_report_func(struct sock *sk, struct hci_dev *hdev,
- struct mgmt_cp_set_exp_feature *cp,
- u16 data_len)
-{
- struct mgmt_rp_set_exp_feature rp;
- bool val, changed;
- int err;
-
- /* Command requires to use a valid controller index */
- if (!hdev)
- return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
- MGMT_OP_SET_EXP_FEATURE,
- MGMT_STATUS_INVALID_INDEX);
-
- /* Parameters are limited to a single octet */
- if (data_len != MGMT_SET_EXP_FEATURE_SIZE + 1)
- return mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_SET_EXP_FEATURE,
- MGMT_STATUS_INVALID_PARAMS);
-
- /* Only boolean on/off is supported */
- if (cp->param[0] != 0x00 && cp->param[0] != 0x01)
- return mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_SET_EXP_FEATURE,
- MGMT_STATUS_INVALID_PARAMS);
-
- hci_req_sync_lock(hdev);
-
- val = !!cp->param[0];
- changed = (val != hci_dev_test_flag(hdev, HCI_QUALITY_REPORT));
-
- if (!aosp_has_quality_report(hdev) && !hdev->set_quality_report) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_SET_EXP_FEATURE,
- MGMT_STATUS_NOT_SUPPORTED);
- goto unlock_quality_report;
- }
-
- if (changed) {
- if (hdev->set_quality_report)
- err = hdev->set_quality_report(hdev, val);
- else
- err = aosp_set_quality_report(hdev, val);
-
- if (err) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_SET_EXP_FEATURE,
- MGMT_STATUS_FAILED);
- goto unlock_quality_report;
- }
-
- if (val)
- hci_dev_set_flag(hdev, HCI_QUALITY_REPORT);
- else
- hci_dev_clear_flag(hdev, HCI_QUALITY_REPORT);
- }
-
- bt_dev_dbg(hdev, "quality report enable %d changed %d", val, changed);
-
- memcpy(rp.uuid, quality_report_uuid, 16);
- rp.flags = cpu_to_le32(val ? BIT(0) : 0);
- hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
-
- err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_SET_EXP_FEATURE, 0,
- &rp, sizeof(rp));
-
- if (changed)
- exp_feature_changed(hdev, quality_report_uuid, val, sk);
-
-unlock_quality_report:
- hci_req_sync_unlock(hdev);
- return err;
-}
-
static int set_offload_codec_func(struct sock *sk, struct hci_dev *hdev,
struct mgmt_cp_set_exp_feature *cp,
u16 data_len)
@@ -4363,7 +4278,6 @@ static const struct mgmt_exp_feature {
EXP_FEAT(debug_uuid, set_debug_func),
#endif
EXP_FEAT(rpa_resolution_uuid, set_rpa_resolution_func),
- EXP_FEAT(quality_report_uuid, set_quality_report_func),
EXP_FEAT(offload_codecs_uuid, set_offload_codec_func),
EXP_FEAT(le_simultaneous_roles_uuid, set_le_simultaneous_roles_func),
@@ -8653,6 +8567,71 @@ static int get_adv_size_info(struct sock *sk, struct hci_dev *hdev,
MGMT_STATUS_SUCCESS, &rp, sizeof(rp));
}
+static int set_quality_report(struct sock *sk, struct hci_dev *hdev,
+ void *data, u16 data_len)
+{
+ struct mgmt_cp_set_quality_report *cp = data;
+ bool enable, changed;
+ int err;
+
+ /* Command requires to use a valid controller index */
+ if (!hdev)
+ return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
+ MGMT_OP_SET_QUALITY_REPORT,
+ MGMT_STATUS_INVALID_INDEX);
+
+ /* Only 0 (off) and 1 (on) is supported */
+ if (cp->action != 0x00 && cp->action != 0x01)
+ return mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_QUALITY_REPORT,
+ MGMT_STATUS_INVALID_PARAMS);
+
+ hci_req_sync_lock(hdev);
+
+ enable = !!cp->action;
+ changed = (enable != hci_dev_test_flag(hdev, HCI_QUALITY_REPORT));
+
+ if (!aosp_has_quality_report(hdev) && !hdev->set_quality_report) {
+ err = mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_QUALITY_REPORT,
+ MGMT_STATUS_NOT_SUPPORTED);
+ goto unlock_quality_report;
+ }
+
+ if (changed) {
+ if (hdev->set_quality_report)
+ err = hdev->set_quality_report(hdev, enable);
+ else
+ err = aosp_set_quality_report(hdev, enable);
+
+ if (err) {
+ err = mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_QUALITY_REPORT,
+ MGMT_STATUS_FAILED);
+ goto unlock_quality_report;
+ }
+
+ if (enable)
+ hci_dev_set_flag(hdev, HCI_QUALITY_REPORT);
+ else
+ hci_dev_clear_flag(hdev, HCI_QUALITY_REPORT);
+ }
+
+ bt_dev_dbg(hdev, "quality report enable %d changed %d",
+ enable, changed);
+
+ err = send_settings_rsp(sk, MGMT_OP_SET_QUALITY_REPORT, hdev);
+ if (err < 0)
+ goto unlock_quality_report;
+
+ if (changed)
+ err = new_settings(hdev, sk);
+
+unlock_quality_report:
+ hci_req_sync_unlock(hdev);
+ return err;
+}
+
static const struct hci_mgmt_handler mgmt_handlers[] = {
{ NULL }, /* 0x0000 (no command) */
{ read_version, MGMT_READ_VERSION_SIZE,
@@ -8779,6 +8758,7 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
{ add_adv_patterns_monitor_rssi,
MGMT_ADD_ADV_PATTERNS_MONITOR_RSSI_SIZE,
HCI_MGMT_VAR_LEN },
+ { set_quality_report, MGMT_SET_QUALITY_REPORT_SIZE },
};
void mgmt_index_added(struct hci_dev *hdev)
--
2.35.1.265.g69c8d7142f-goog
Hi Joseph,
url: https://github.com/0day-ci/linux/commits/Joseph-Hwang/Bluetooth-aosp-surface-AOSP-quality-report-through-mgmt/20220215-213800
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: i386-randconfig-m021-20220214 (https://download.01.org/0day-ci/archive/20220216/[email protected]/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
New smatch warnings:
net/bluetooth/mgmt.c:860 get_supported_settings() warn: variable dereferenced before check 'hdev' (see line 826)
vim +/hdev +860 net/bluetooth/mgmt.c
69ab39ea5da03e Johan Hedberg 2011-12-15 816 static u32 get_supported_settings(struct hci_dev *hdev)
69ab39ea5da03e Johan Hedberg 2011-12-15 817 {
69ab39ea5da03e Johan Hedberg 2011-12-15 818 u32 settings = 0;
69ab39ea5da03e Johan Hedberg 2011-12-15 819
69ab39ea5da03e Johan Hedberg 2011-12-15 820 settings |= MGMT_SETTING_POWERED;
b2939475eb6a35 Johan Hedberg 2014-07-30 821 settings |= MGMT_SETTING_BONDABLE;
b1de97d8c06d9d Marcel Holtmann 2014-01-31 822 settings |= MGMT_SETTING_DEBUG_KEYS;
3742abfc4e853f Johan Hedberg 2014-07-08 823 settings |= MGMT_SETTING_CONNECTABLE;
3742abfc4e853f Johan Hedberg 2014-07-08 824 settings |= MGMT_SETTING_DISCOVERABLE;
69ab39ea5da03e Johan Hedberg 2011-12-15 825
ed3fa31f35896b Andre Guedes 2012-07-24 @826 if (lmp_bredr_capable(hdev)) {
1a47aee85f8a08 Johan Hedberg 2013-03-15 827 if (hdev->hci_ver >= BLUETOOTH_VER_1_2)
33c525c0a37abd Johan Hedberg 2012-10-24 828 settings |= MGMT_SETTING_FAST_CONNECTABLE;
69ab39ea5da03e Johan Hedberg 2011-12-15 829 settings |= MGMT_SETTING_BREDR;
69ab39ea5da03e Johan Hedberg 2011-12-15 830 settings |= MGMT_SETTING_LINK_SECURITY;
a82974c9f4ed07 Marcel Holtmann 2013-10-11 831
a82974c9f4ed07 Marcel Holtmann 2013-10-11 832 if (lmp_ssp_capable(hdev)) {
a82974c9f4ed07 Marcel Holtmann 2013-10-11 833 settings |= MGMT_SETTING_SSP;
b560a208cda029 Luiz Augusto von Dentz 2020-08-06 834 if (IS_ENABLED(CONFIG_BT_HS))
d7b7e79688c07b Marcel Holtmann 2012-02-20 835 settings |= MGMT_SETTING_HS;
848566b381e72b Marcel Holtmann 2013-10-01 836 }
e98d2ce293a941 Marcel Holtmann 2014-01-10 837
05b3c3e7905d00 Marcel Holtmann 2014-12-31 838 if (lmp_sc_capable(hdev))
e98d2ce293a941 Marcel Holtmann 2014-01-10 839 settings |= MGMT_SETTING_SECURE_CONN;
4b127bd5f2cc1b Alain Michaud 2020-02-27 840
00bce3fb0642b3 Alain Michaud 2020-03-05 841 if (test_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
4b127bd5f2cc1b Alain Michaud 2020-02-27 842 &hdev->quirks))
00bce3fb0642b3 Alain Michaud 2020-03-05 843 settings |= MGMT_SETTING_WIDEBAND_SPEECH;
a82974c9f4ed07 Marcel Holtmann 2013-10-11 844 }
d7b7e79688c07b Marcel Holtmann 2012-02-20 845
eeca6f891305a8 Johan Hedberg 2013-09-25 846 if (lmp_le_capable(hdev)) {
69ab39ea5da03e Johan Hedberg 2011-12-15 847 settings |= MGMT_SETTING_LE;
a3209694f82a22 Johan Hedberg 2014-05-26 848 settings |= MGMT_SETTING_SECURE_CONN;
0f4bd942f13dd1 Johan Hedberg 2014-02-22 849 settings |= MGMT_SETTING_PRIVACY;
93690c227acf08 Marcel Holtmann 2015-03-06 850 settings |= MGMT_SETTING_STATIC_ADDRESS;
cbbdfa6f331980 Sathish Narasimman 2020-07-23 851 settings |= MGMT_SETTING_ADVERTISING;
eeca6f891305a8 Johan Hedberg 2013-09-25 852 }
69ab39ea5da03e Johan Hedberg 2011-12-15 853
eb1904f49d3e11 Marcel Holtmann 2014-07-04 854 if (test_bit(HCI_QUIRK_EXTERNAL_CONFIG, &hdev->quirks) ||
Unchecked dereferences throughout.
eb1904f49d3e11 Marcel Holtmann 2014-07-04 855 hdev->set_bdaddr)
9fc3bfb681bdf5 Marcel Holtmann 2014-07-04 856 settings |= MGMT_SETTING_CONFIGURATION;
9fc3bfb681bdf5 Marcel Holtmann 2014-07-04 857
6244691fec4dd0 Jaganath Kanakkassery 2018-07-19 858 settings |= MGMT_SETTING_PHY_CONFIGURATION;
6244691fec4dd0 Jaganath Kanakkassery 2018-07-19 859
edbb68b1006482 Joseph Hwang 2022-02-15 @860 if (hdev && (aosp_has_quality_report(hdev) ||
^^^^
Checked too late
edbb68b1006482 Joseph Hwang 2022-02-15 861 hdev->set_quality_report))
edbb68b1006482 Joseph Hwang 2022-02-15 862 settings |= MGMT_SETTING_QUALITY_REPORT;
edbb68b1006482 Joseph Hwang 2022-02-15 863
69ab39ea5da03e Johan Hedberg 2011-12-15 864 return settings;
69ab39ea5da03e Johan Hedberg 2011-12-15 865 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Hi Marcel, thank you for reviewing the patches! I have some questions.
Please read my replies in the lines below. Thanks!
On Thu, Feb 17, 2022 at 9:04 PM Marcel Holtmann <[email protected]> wrote:
>
> Hi Joseph,
>
> > This patch adds a new set_quality_report mgmt handler to set
> > the quality report feature. The feature is removed from the
> > experimental features at the same time.
> >
> > Signed-off-by: Joseph Hwang <[email protected]>
> > ---
> >
> > Changes in v4:
> > - return current settings for set_quality_report.
> >
> > Changes in v3:
> > - This is a new patch to enable the quality report feature.
> > The reading and setting of the quality report feature are
> > removed from the experimental features.
> >
> > include/net/bluetooth/mgmt.h | 7 ++
> > net/bluetooth/mgmt.c | 168 +++++++++++++++--------------------
> > 2 files changed, 81 insertions(+), 94 deletions(-)
> >
> > diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> > index 83b602636262..74d253ff617a 100644
> > --- a/include/net/bluetooth/mgmt.h
> > +++ b/include/net/bluetooth/mgmt.h
> > @@ -109,6 +109,7 @@ struct mgmt_rp_read_index_list {
> > #define MGMT_SETTING_STATIC_ADDRESS 0x00008000
> > #define MGMT_SETTING_PHY_CONFIGURATION 0x00010000
> > #define MGMT_SETTING_WIDEBAND_SPEECH 0x00020000
> > +#define MGMT_SETTING_QUALITY_REPORT 0x00040000
> >
> > #define MGMT_OP_READ_INFO 0x0004
> > #define MGMT_READ_INFO_SIZE 0
> > @@ -838,6 +839,12 @@ struct mgmt_cp_add_adv_patterns_monitor_rssi {
> > } __packed;
> > #define MGMT_ADD_ADV_PATTERNS_MONITOR_RSSI_SIZE 8
> >
> > +#define MGMT_OP_SET_QUALITY_REPORT 0x0057
> > +struct mgmt_cp_set_quality_report {
> > + __u8 action;
> > +} __packed;
> > +#define MGMT_SET_QUALITY_REPORT_SIZE 1
> > +
> > #define MGMT_EV_CMD_COMPLETE 0x0001
> > struct mgmt_ev_cmd_complete {
> > __le16 opcode;
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index 5e48576041fb..ccd77b94905b 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
> > @@ -857,6 +857,10 @@ static u32 get_supported_settings(struct hci_dev *hdev)
> >
> > settings |= MGMT_SETTING_PHY_CONFIGURATION;
> >
> > + if (hdev && (aosp_has_quality_report(hdev) ||
> > + hdev->set_quality_report))
> > + settings |= MGMT_SETTING_QUALITY_REPORT;
> > +
>
> the hdev check here is useless. The hdev structure is always valid.
>
> > return settings;
> > }
> >
> > @@ -928,6 +932,9 @@ static u32 get_current_settings(struct hci_dev *hdev)
> > if (hci_dev_test_flag(hdev, HCI_WIDEBAND_SPEECH_ENABLED))
> > settings |= MGMT_SETTING_WIDEBAND_SPEECH;
> >
> > + if (hci_dev_test_flag(hdev, HCI_QUALITY_REPORT))
> > + settings |= MGMT_SETTING_QUALITY_REPORT;
> > +
> > return settings;
> > }
> >
> > @@ -3871,12 +3878,6 @@ static const u8 debug_uuid[16] = {
> > };
> > #endif
> >
> > -/* 330859bc-7506-492d-9370-9a6f0614037f */
> > -static const u8 quality_report_uuid[16] = {
> > - 0x7f, 0x03, 0x14, 0x06, 0x6f, 0x9a, 0x70, 0x93,
> > - 0x2d, 0x49, 0x06, 0x75, 0xbc, 0x59, 0x08, 0x33,
> > -};
> > -
> > /* a6695ace-ee7f-4fb9-881a-5fac66c629af */
> > static const u8 offload_codecs_uuid[16] = {
> > 0xaf, 0x29, 0xc6, 0x66, 0xac, 0x5f, 0x1a, 0x88,
> > @@ -3898,7 +3899,7 @@ static const u8 rpa_resolution_uuid[16] = {
> > static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
> > void *data, u16 data_len)
> > {
> > - char buf[102]; /* Enough space for 5 features: 2 + 20 * 5 */
> > + char buf[82]; /* Enough space for 4 features: 2 + 20 * 4 */
> > struct mgmt_rp_read_exp_features_info *rp = (void *)buf;
> > u16 idx = 0;
> > u32 flags;
> > @@ -3939,18 +3940,6 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
> > idx++;
> > }
> >
> > - if (hdev && (aosp_has_quality_report(hdev) ||
> > - hdev->set_quality_report)) {
> > - if (hci_dev_test_flag(hdev, HCI_QUALITY_REPORT))
> > - flags = BIT(0);
> > - else
> > - flags = 0;
> > -
> > - memcpy(rp->features[idx].uuid, quality_report_uuid, 16);
> > - rp->features[idx].flags = cpu_to_le32(flags);
> > - idx++;
> > - }
> > -
>
> (I see, you copied it from here. Yes, here you need to check for hdev!)
>
> > if (hdev && hdev->get_data_path_id) {
> > if (hci_dev_test_flag(hdev, HCI_OFFLOAD_CODECS_ENABLED))
> > flags = BIT(0);
> > @@ -4163,80 +4152,6 @@ static int set_rpa_resolution_func(struct sock *sk, struct hci_dev *hdev,
> > return err;
> > }
> >
> > -static int set_quality_report_func(struct sock *sk, struct hci_dev *hdev,
> > - struct mgmt_cp_set_exp_feature *cp,
> > - u16 data_len)
> > -{
> > - struct mgmt_rp_set_exp_feature rp;
> > - bool val, changed;
> > - int err;
> > -
> > - /* Command requires to use a valid controller index */
> > - if (!hdev)
> > - return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
> > - MGMT_OP_SET_EXP_FEATURE,
> > - MGMT_STATUS_INVALID_INDEX);
> > -
> > - /* Parameters are limited to a single octet */
> > - if (data_len != MGMT_SET_EXP_FEATURE_SIZE + 1)
> > - return mgmt_cmd_status(sk, hdev->id,
> > - MGMT_OP_SET_EXP_FEATURE,
> > - MGMT_STATUS_INVALID_PARAMS);
> > -
> > - /* Only boolean on/off is supported */
> > - if (cp->param[0] != 0x00 && cp->param[0] != 0x01)
> > - return mgmt_cmd_status(sk, hdev->id,
> > - MGMT_OP_SET_EXP_FEATURE,
> > - MGMT_STATUS_INVALID_PARAMS);
> > -
> > - hci_req_sync_lock(hdev);
> > -
> > - val = !!cp->param[0];
> > - changed = (val != hci_dev_test_flag(hdev, HCI_QUALITY_REPORT));
> > -
> > - if (!aosp_has_quality_report(hdev) && !hdev->set_quality_report) {
> > - err = mgmt_cmd_status(sk, hdev->id,
> > - MGMT_OP_SET_EXP_FEATURE,
> > - MGMT_STATUS_NOT_SUPPORTED);
> > - goto unlock_quality_report;
> > - }
> > -
> > - if (changed) {
> > - if (hdev->set_quality_report)
> > - err = hdev->set_quality_report(hdev, val);
> > - else
> > - err = aosp_set_quality_report(hdev, val);
> > -
> > - if (err) {
> > - err = mgmt_cmd_status(sk, hdev->id,
> > - MGMT_OP_SET_EXP_FEATURE,
> > - MGMT_STATUS_FAILED);
> > - goto unlock_quality_report;
> > - }
> > -
> > - if (val)
> > - hci_dev_set_flag(hdev, HCI_QUALITY_REPORT);
> > - else
> > - hci_dev_clear_flag(hdev, HCI_QUALITY_REPORT);
> > - }
> > -
> > - bt_dev_dbg(hdev, "quality report enable %d changed %d", val, changed);
> > -
> > - memcpy(rp.uuid, quality_report_uuid, 16);
> > - rp.flags = cpu_to_le32(val ? BIT(0) : 0);
> > - hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
> > -
> > - err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_SET_EXP_FEATURE, 0,
> > - &rp, sizeof(rp));
> > -
> > - if (changed)
> > - exp_feature_changed(hdev, quality_report_uuid, val, sk);
> > -
> > -unlock_quality_report:
> > - hci_req_sync_unlock(hdev);
> > - return err;
> > -}
> > -
> > static int set_offload_codec_func(struct sock *sk, struct hci_dev *hdev,
> > struct mgmt_cp_set_exp_feature *cp,
> > u16 data_len)
> > @@ -4363,7 +4278,6 @@ static const struct mgmt_exp_feature {
> > EXP_FEAT(debug_uuid, set_debug_func),
> > #endif
> > EXP_FEAT(rpa_resolution_uuid, set_rpa_resolution_func),
> > - EXP_FEAT(quality_report_uuid, set_quality_report_func),
> > EXP_FEAT(offload_codecs_uuid, set_offload_codec_func),
> > EXP_FEAT(le_simultaneous_roles_uuid, set_le_simultaneous_roles_func),
> >
> > @@ -8653,6 +8567,71 @@ static int get_adv_size_info(struct sock *sk, struct hci_dev *hdev,
> > MGMT_STATUS_SUCCESS, &rp, sizeof(rp));
> > }
> >
> > +static int set_quality_report(struct sock *sk, struct hci_dev *hdev,
> > + void *data, u16 data_len)
> > +{
> > + struct mgmt_cp_set_quality_report *cp = data;
> > + bool enable, changed;
> > + int err;
> > +
> > + /* Command requires to use a valid controller index */
> > + if (!hdev)
> > + return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
> > + MGMT_OP_SET_QUALITY_REPORT,
> > + MGMT_STATUS_INVALID_INDEX);
> > +
> > + /* Only 0 (off) and 1 (on) is supported */
> > + if (cp->action != 0x00 && cp->action != 0x01)
> > + return mgmt_cmd_status(sk, hdev->id,
> > + MGMT_OP_SET_QUALITY_REPORT,
> > + MGMT_STATUS_INVALID_PARAMS);
> > +
> > + hci_req_sync_lock(hdev);
> > +
> > + enable = !!cp->action;
> > + changed = (enable != hci_dev_test_flag(hdev, HCI_QUALITY_REPORT));
> > +
> > + if (!aosp_has_quality_report(hdev) && !hdev->set_quality_report) {
> > + err = mgmt_cmd_status(sk, hdev->id,
> > + MGMT_OP_SET_QUALITY_REPORT,
> > + MGMT_STATUS_NOT_SUPPORTED);
> > + goto unlock_quality_report;
> > + }
> > +
> > + if (changed) {
> > + if (hdev->set_quality_report)
> > + err = hdev->set_quality_report(hdev, enable);
> > + else
> > + err = aosp_set_quality_report(hdev, enable);
> > +
> > + if (err) {
> > + err = mgmt_cmd_status(sk, hdev->id,
> > + MGMT_OP_SET_QUALITY_REPORT,
> > + MGMT_STATUS_FAILED);
> > + goto unlock_quality_report;
> > + }
> > +
> > + if (enable)
> > + hci_dev_set_flag(hdev, HCI_QUALITY_REPORT);
> > + else
> > + hci_dev_clear_flag(hdev, HCI_QUALITY_REPORT);
> > + }
> > +
> > + bt_dev_dbg(hdev, "quality report enable %d changed %d",
> > + enable, changed);
> > +
> > + err = send_settings_rsp(sk, MGMT_OP_SET_QUALITY_REPORT, hdev);
> > + if (err < 0)
> > + goto unlock_quality_report;
> > +
> > + if (changed)
> > + err = new_settings(hdev, sk);
> > +
> > +unlock_quality_report:
> > + hci_req_sync_unlock(hdev);
> > + return err;
> > +}
> > +
> > static const struct hci_mgmt_handler mgmt_handlers[] = {
> > { NULL }, /* 0x0000 (no command) */
> > { read_version, MGMT_READ_VERSION_SIZE,
> > @@ -8779,6 +8758,7 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
> > { add_adv_patterns_monitor_rssi,
> > MGMT_ADD_ADV_PATTERNS_MONITOR_RSSI_SIZE,
> > HCI_MGMT_VAR_LEN },
> > + { set_quality_report, MGMT_SET_QUALITY_REPORT_SIZE },
> > };
> >
> > void mgmt_index_added(struct hci_dev *hdev)
>
> So this patch I actually get merged first.
I do not see this patch getting merged yet. I guess I still need to
remove the “hdev” that you mentioned above and re-submit this patch?
>
> However we still need to figure out if this setting is suppose to survive power off/on cycles. Right now as far as I can tell it is added to hci_dev_clear_volatile_flags and thus needs to be set again after power on.
Thank you for pointing this out. Whether the setting should survive
power off/on cycles is not mentioned in the AOSP BQR and Intel
telemetry specifications. I did a quick test on an Intel platform, the
setting does NOT survive over power cycles probably due to the HCI
Reset command during power off. Hence, I will need to address this
issue by restoring it explicitly. Please let me send separate patches
later to address this issue for both Intel and AOSP specs.
>
> Is this the expected behavior? I don’t think we want that. Since normally all other settings are restored after power on. And it is explicitly mentioned in doc/mgmt-api.txt as well.
I will mention this in the Set Powered Command in doc/mgmt-api.txt in
the separate patches later.
>
> Regards
>
> Marcel
>
>
--
Joseph Shyh-In Hwang
Email: [email protected]