Re: [PATCH 1/2] Bluetooth: Add __hci_cmd_sync_noev function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Marcel,

On 25 April 2018 at 14:10, Marcel Holtmann <marcel@xxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux