| Message ID | 1524506380-7417-1-git-send-email-loic.poulain@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] Bluetooth: Add __hci_cmd_sync_noev function | expand |
Hi Loic, > This function allows to send a HCI command without expecting any > controller event/response in return. > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > --- > include/net/bluetooth/hci_core.h | 2 ++ > net/bluetooth/hci_core.c | 17 +++++++++++++++++ > 2 files changed, 19 insertions(+) > > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > index b619a19..d48a7df 100644 > --- a/include/net/bluetooth/hci_core.h > +++ b/include/net/bluetooth/hci_core.h > @@ -1393,6 +1393,8 @@ struct sk_buff *__hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen, > const void *param, u32 timeout); > struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen, > const void *param, u8 event, u32 timeout); > +int __hci_cmd_sync_noev(struct hci_dev *hdev, u16 opcode, u32 plen, > + const void *param); > > int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, > const void *param); > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > index 40d260f..2831c4e 100644 > --- a/net/bluetooth/hci_core.c > +++ b/net/bluetooth/hci_core.c > @@ -3459,6 +3459,23 @@ struct sk_buff *hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen, > } > EXPORT_SYMBOL(hci_cmd_sync); > > +int __hci_cmd_sync_noev(struct hci_dev *hdev, u16 opcode, u32 plen, > + const void *param) > +{ > + struct sk_buff *skb = hci_prepare_cmd(hdev, opcode, plen, param); > + > + if (!skb) { > + bt_dev_err(hdev, "no memory for command (opcode 0x%4.4x)", > + opcode); > + return -ENOMEM; > + } > + > + hci_send_frame(hdev, skb); > + > + return 0; > +} > +EXPORT_SYMBOL(__hci_cmd_sync_noev); so this is not a _sync command. You are not waiting for the completion. So this is more like __hci_cmd_send or __hci_cmd_submit. I am also debating if this should be limited to vendor OGF only, meaning it should throw an error for all others. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Marcel, > > so this is not a _sync command. You are not waiting for the completion. So this is more like __hci_cmd_send or __hci_cmd_submit. I am also debating if this should be limited to vendor OGF only, meaning it should throw an error for all others. This makes sense, I agree we should not allow standard HCI command to be unresponded and strictly follow the specification. I think __hci_cmd_send/submit is a bit too generic and we already have a hci_send_cmd which has a different behavior, don't want to cause confusion. So what about __hci_cmd_noresp or __hci_cmd_vendor...? Regards, Loic -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Marcel, On 25 April 2018 at 14:10, Marcel Holtmann <marcel@holtmann.org> wrote: > Hi Loic, > >>> so this is not a _sync command. You are not waiting for the completion. So this is more like __hci_cmd_send or __hci_cmd_submit. I am also debating if this should be limited to vendor OGF only, meaning it should throw an error for all others. >> >> This makes sense, I agree we should not allow standard HCI command to >> be unresponded and strictly follow the specification. >> I think __hci_cmd_send/submit is a bit too generic and we already have >> a hci_send_cmd which has a different behavior, don't want to cause >> confusion. So what about __hci_cmd_noresp or __hci_cmd_vendor...? > > so perfect would be that we have at least a “sync” command in that sense that we know the controller has send it to the hardware. However from a driver framework point of view we do not have this concept. And we don’t have it, because that kind of “flow control” is not needed since we have HCI command and also ACL packet flow control that should be used. > > My concern is that you can easily overload the controller since there are no checks and balances from a HCI point of view. The transport might have some, but otherwise you push things in a queue and hope that all goes well. Yes, we are out of spec here and the only overload protection is UART hardware flow control in this case. > Anyway, I am against __hci_cmd_vendor since that is a bit misleading and the _noresp seems a bit off as well. We could do __hci_cmd_dispatch or __hci_cmd_post. I am not convinced that __hci_cmd_send would be actually that bad. It is essentially the just that (except it bypasses the hdev->cmd_q. Fair enough. Regards, Loic -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index b619a19..d48a7df 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1393,6 +1393,8 @@ struct sk_buff *__hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen, const void *param, u32 timeout); struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen, const void *param, u8 event, u32 timeout); +int __hci_cmd_sync_noev(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param); int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, const void *param); diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 40d260f..2831c4e 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -3459,6 +3459,23 @@ struct sk_buff *hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen, } EXPORT_SYMBOL(hci_cmd_sync); +int __hci_cmd_sync_noev(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param) +{ + struct sk_buff *skb = hci_prepare_cmd(hdev, opcode, plen, param); + + if (!skb) { + bt_dev_err(hdev, "no memory for command (opcode 0x%4.4x)", + opcode); + return -ENOMEM; + } + + hci_send_frame(hdev, skb); + + return 0; +} +EXPORT_SYMBOL(__hci_cmd_sync_noev); + /* Send ACL data */ static void hci_add_acl_hdr(struct sk_buff *skb, __u16 handle, __u16 flags) {
This function allows to send a HCI command without expecting any controller event/response in return. Signed-off-by: Loic Poulain <loic.poulain@linaro.org> --- include/net/bluetooth/hci_core.h | 2 ++ net/bluetooth/hci_core.c | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html