Hi Arseniy,
On Thu, Jan 30, 2025 at 1:34 PM Arseniy Krasnov
<avkrasnov@xxxxxxxxxxxxxxxxx> wrote:
>
> Hi, sorry ping
>
> Thanks
>
> On 08.01.2025 15:39, Arseniy Krasnov wrote:
> >
> >
> > On 19.12.2024 23:07, Arseniy Krasnov wrote:
> >>
> >>
> >> On 19.12.2024 22:18, Luiz Augusto von Dentz wrote:
> >>> Hi Arseniy,
> >>>
> >>> On Tue, Dec 17, 2024 at 3:12 AM Arseniy Krasnov
> >>> <avkrasnov@xxxxxxxxxxxxxxxxx> wrote:
> >>>>
> >>>> 'hci_register_dev()' calls power up function, which is executed by
> >>>> kworker - 'hci_power_on()'. This function does access to bluetooth chip
> >>>> using callbacks from 'hci_ldisc.c', for example 'hci_uart_send_frame()'.
> >>>> Now 'hci_uart_send_frame()' checks 'HCI_UART_PROTO_READY' bit set, and
> >>>> if not - it fails. Problem is that 'HCI_UART_PROTO_READY' is set after
> >>>> 'hci_register_dev()', and there is tiny chance that 'hci_power_on()' will
> >>>> be executed before setting this bit. In that case HCI init logic fails.
> >>>>
> >>>> Patch moves setting of 'HCI_UART_PROTO_READY' before calling function
> >>>> 'hci_uart_register_dev()'.
> >>>>
> >>>> Signed-off-by: Arseniy Krasnov <avkrasnov@xxxxxxxxxxxxxxxxx>
> >>>> ---
> >>>> Changelog v1->v2:
> >>>> * Move 'set_bit()' before 'hci_uart_register_dev()' instead of
> >>>> adding new bit 'HCI_UART_PROTO_INIT'.
> >>>
> >>> What drivers/controllers this was tested with? I want to make sure
> >>> this doesn't cause regressions to other drivers if there are perhaps
> >>> some drivers assuming HCI_UART_PROTO_READY was set in a certain order.
> >>
> >> Hi,
> >>
> >> I tested this on:
> >>
> >> CONFIG_BT=y
> >> CONFIG_BT_HCIUART=y
> >> CONFIG_BT_HCIUART_H4=y
> >
> > Hi, sorry, but is this enough info about my test env ?
> >
> > Thanks
> >
> >>
> >> Yes, my v1 patchset with extra INIT bit was targeted to keep
> >> original behaviour - e.g. PROTO_READY bit usage still the same,
> >> just adding extra bit to handle this specific case.
> >>
> >> Thanks
> >>
> >>>
> >>>> drivers/bluetooth/hci_ldisc.c | 3 ++-
> >>>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> >>>> index 30192bb083549..07b9aa09bbe2e 100644
> >>>> --- a/drivers/bluetooth/hci_ldisc.c
> >>>> +++ b/drivers/bluetooth/hci_ldisc.c
> >>>> @@ -704,12 +704,13 @@ static int hci_uart_set_proto(struct hci_uart *hu, int id)
> >>>>
> >>>> hu->proto = p;
> >>>>
> >>>> + set_bit(HCI_UART_PROTO_READY, &hu->flags);
> >>>> +
> >>>> err = hci_uart_register_dev(hu);
> >>>> if (err) {
> >>>> return err;
> >>>> }
> >>>>
> >>>> - set_bit(HCI_UART_PROTO_READY, &hu->flags);
> >>>> return 0;
> >>>> }
> >>>>
> >>>> --
> >>>> 2.30.1
> >>>
> >>>
> >>>
Please resend it.
--
Luiz Augusto von Dentz
