Bluetooth: hci_core: return cmd status in __hci_cmd_sync()
Subsystems (2)
PATCH v3 1/2 Bluetooth: hci_core: return cmd status in __hci_cmd_sync()
Handle command complete event, extract command status from reply and return it if it is an error.
This will allow to simplify error test in calling functions.
--- net/bluetooth/hci_core.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
net/bluetooth/hci_core.c +12 -8
@@ -94,7 +94,6 @@ static ssize_t dut_mode_write(struct file *file, const char __user *user_buf,char buf[32 ];size_t buf_size =min (count, (sizeof (buf)-1 ));bool enable; -int err;if (!test_bit (HCI_UP , &hdev->flags))return -ENETDOWN ;
@@ -121,12 +120,6 @@ static ssize_t dut_mode_write(struct file *file, const char __user *user_buf,if (IS_ERR (skb))return PTR_ERR (skb); - err = -bt_to_errno (skb->data[0 ]); -kfree_skb (skb); - -if (err <0 ) -return err; -hci_dev_change_flag (hdev,HCI_DUT_MODE );return count;
@@ -234,7 +227,18 @@ EXPORT_SYMBOL(__hci_cmd_sync_ev);struct sk_buff *__hci_cmd_sync (struct hci_dev *hdev,u16 opcode,u32 plen,const void *param,u32 timeout) { -return __hci_cmd_sync_ev (hdev, opcode, plen, param,0 , timeout); +struct sk_buff *skb; +int err; + + skb =__hci_cmd_sync_ev (hdev, opcode, plen, param,0 , timeout); +if (!IS_ERR (skb) && skb->data[0 ]) { + err = -bt_to_errno (skb->data[0 ]); +kfree_skb (skb); + +return ERR_PTR (err); + } + +return skb; }EXPORT_SYMBOL (__hci_cmd_sync);
Signature · 1 line
1.9.1
PATCH v3 2/2 Bluetooth: Update calls to __hci_cmd_sync()
Remove test of command reply status as it is already performed by __hci_cmd_sync().
--- drivers/bluetooth/btbcm.c | 6 ----- drivers/bluetooth/btintel.c | 6 ----- drivers/bluetooth/btusb.c | 53 +++------------------------------------------ 3 files changed, 3 insertions(+), 62 deletions(-)
drivers/bluetooth/btbcm.c +0 -6
@@ -55,12 +55,6 @@ int btbcm_check_bdaddr(struct hci_dev *hdev) } bda = (struct hci_rp_read_bd_addr *)skb->data; -if (bda->status) { -BT_ERR ("%s: BCM: Device address result failed (%02x)" , - hdev->name, bda->status); -kfree_skb (skb); -return -bt_to_errno (bda->status); - }/* The address 00:20:70:02:A0:00 indicates a BCM20702A0 controller * with no configured address.
drivers/bluetooth/btintel.c +0 -6
@@ -53,12 +53,6 @@ int btintel_check_bdaddr(struct hci_dev *hdev) } bda = (struct hci_rp_read_bd_addr *)skb->data; -if (bda->status) { -BT_ERR ("%s: Intel device address result failed (%02x)" , - hdev->name, bda->status); -kfree_skb (skb); -return -bt_to_errno (bda->status); - }/* For some Intel based controllers, the default Bluetooth device * address 00:03:19:9E:8B:00 can be found. These controllers are
drivers/bluetooth/btusb.c +3 -50
115 lines not on this page · raw message
Signature · 1 line
1.9.1
Hi Fred,
Quoted 7 lines · Frederic Danis
net/bluetooth/hci_core.c +2
-1@@ -234,7 +227,18 @@ EXPORT_SYMBOL(__hci_cmd_sync_ev);struct sk_buff *__hci_cmd_sync (struct hci_dev *hdev,u16 opcode,u32 plen,const void *param,u32 timeout) { -return __hci_cmd_sync_ev (hdev, opcode, plen, param,0 , timeout); +struct sk_buff *skb; +int err;
net/bluetooth/hci_core.c EXPORT_SYMBOL(__hci_cmd_sync_ev); · line 241
Quoted 10 lines
net/bluetooth/hci_core.c +10
-0+ + skb =__hci_cmd_sync_ev (hdev, opcode, plen, param,0 , timeout); +if (!IS_ERR (skb) && skb->data[0 ]) { + err = -bt_to_errno (skb->data[0 ]); +kfree_skb (skb); + +return ERR_PTR (err); + } + +return skb;
Are you absolutely sure that this is needed? To my understanding the __hci_cmd_sync_ev() function should already be returning an error if we got a non-zero status either through a Command Complete or a Command Status event.
For both of these events the status is collected up in the event handlers called by hci_event_packet() and then passed as the second parameter to req_complete_skb(). The req_complete_skb() callback in turn is hci_req_sync_complete() for __hci_cmd_sync_ev() which stores the status in hdev->req_result. The hdev->req_result is then further converted through bt_to_errno() back in __hci_cmd_sync_ev().
So to me it seems like your addition above should not be needed. What we do want however is all the code removals in your patches that remove code that was (incorrectly) assuming that non-zero statuses would be returned as actual skbs rather than errors.
Johan