| Message ID | 64b59ca66cc22e6433a044e7bba2b3e97c810dc2.1651647576.git.hakan.jansson@infineon.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Bluetooth: hci_bcm: Autobaud mode support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Wed, May 4, 2022 at 11:04 AM Hakan Jansson <hakan.jansson@infineon.com> wrote: > + brcm,uses-autobaud-mode: > + type: boolean > + description: > > + The controller should be started in autobaud mode by asserting > + BT_UART_CTS_N (i.e. host RTS) during startup. Only HCI commands supported > + in autobaud mode should be used until patch FW has been loaded. Things like gnss uses the current-speed attribute to set a serial link speed, maybe also Bluetooth? Would it make sense to use current-speed-auto; As a flag for these things in general? Yours, Linus Walleij
Hi Linus, Thanks for responding. On 5/5/2022 12:15 AM, Linus Walleij wrote: > On Wed, May 4, 2022 at 11:04 AM Hakan Jansson > <hakan.jansson@infineon.com> wrote: > >> + brcm,uses-autobaud-mode: >> + type: boolean >> + description: > >> + The controller should be started in autobaud mode by asserting >> + BT_UART_CTS_N (i.e. host RTS) during startup. Only HCI commands supported >> + in autobaud mode should be used until patch FW has been loaded. > Things like gnss uses the current-speed attribute to set a serial link speed, > maybe also Bluetooth? As far as I can tell, not many Bluetooth drivers use the current-speed attribute yet but perhaps it would be a good idea to start using it more broadly in the future to set the initial serial link speed. > > Would it make sense to use > > current-speed-auto; > > As a flag for these things in general? I suppose a general flag could be useful but to be honest I don't know if any other devices besides the ones using the Broadcom driver has any use for it. You would probably also still want to be able to use current-speed to set the link speed and end up using both current-speed=x and current-speed-auto at the same time, which might look a little confusing? Please let me know if you'd still prefer "current-speed-auto" over "brcm,uses-autobaud-mode" and I'll revise the patch and rename it! > > Yours, > Linus Walleij Thanks, Håkan
On Thu, May 5, 2022 at 3:11 PM Hakan Jansson <hakan.jansson@infineon.com> wrote: > I suppose a general flag could be useful but to be honest I don't know > if any other devices besides the ones using the Broadcom driver has any > use for it. You would probably also still want to be able to use > current-speed to set the link speed and end up using both > current-speed=x and current-speed-auto at the same time, which might > look a little confusing? I do not think it is more confusing than being able to use current-speed and brcm,uses-autobaud-mode at the same time. > Please let me know if you'd still prefer "current-speed-auto" over > "brcm,uses-autobaud-mode" and I'll revise the patch and rename it! It actually depends a bit. This: > >> + The controller should be started in autobaud mode by asserting > >> + BT_UART_CTS_N (i.e. host RTS) during startup. Only HCI commands supported > >> + in autobaud mode should be used until patch FW has been loaded. sounds a bit vague? Does it mean that CTS is asserted, then you send a bit (CTS then goes low) and then CTS is asserted again when the device is ready to receieve more data? i.e is this some kind of one-bit mode, because it doesn't sound like it is using CTS as it was used in legacy modems. Some more explanation of this mode is needed so we can understand if this is something generic or a BRCM-only thing. Yours, Linus Walleij
On 5/5/2022 4:13 PM, Linus Walleij wrote: > I suppose a general flag could be useful but to be honest I don't know >> if any other devices besides the ones using the Broadcom driver has any >> use for it. You would probably also still want to be able to use >> current-speed to set the link speed and end up using both >> current-speed=x and current-speed-auto at the same time, which might >> look a little confusing? > I do not think it is more confusing than being able to use > current-speed and brcm,uses-autobaud-mode at the same time. > >> Please let me know if you'd still prefer "current-speed-auto" over >> "brcm,uses-autobaud-mode" and I'll revise the patch and rename it! > It actually depends a bit. > > This: > >>>> + The controller should be started in autobaud mode by asserting >>>> + BT_UART_CTS_N (i.e. host RTS) during startup. Only HCI commands supported >>>> + in autobaud mode should be used until patch FW has been loaded. > sounds a bit vague? Yes, perhaps. I was thinking the details could be helpful but I can see how they might be perceived as vague and confusing. Maybe it would be better to just leave it at "The controller should be started in autobaud mode"? > > Does it mean that CTS is asserted, then you send a bit (CTS then goes low) > and then CTS is asserted again when the device is ready to receieve more > data? i.e is this some kind of one-bit mode, because it doesn't sound like > it is using CTS as it was used in legacy modems. CTS and RTS are actually used in the normal way during communication. The host will assert its RTS to indicate being ready to receive data from the controller. This flag just controls whether this happens before or after the controller is powered on. The controller will check the initial state of its BT_UART_CTS_N pin (connected to host's RTS) when starting up. It will enter autobaud mode if the signal is already asserted. > Some more explanation of this mode is needed so we can understand if > this is something generic or a BRCM-only thing. > > Yours, > Linus Walleij Thanks, Håkan
Hi Linus, I checked the state of this patch on Devicetree Bindings Patchwork and it's marked "Changes Requested". I'd be happy to revise the patch but it's not clear to me what changes are requested. Could you please help guide me how to proceed? Sorry if I'm missing something obvious here. Thanks, Håkan On 5/5/2022 6:16 PM, Hakan Jansson wrote: > > > On 5/5/2022 4:13 PM, Linus Walleij wrote: >> I suppose a general flag could be useful but to be honest I don't know >>> if any other devices besides the ones using the Broadcom driver has any >>> use for it. You would probably also still want to be able to use >>> current-speed to set the link speed and end up using both >>> current-speed=x and current-speed-auto at the same time, which might >>> look a little confusing? >> I do not think it is more confusing than being able to use >> current-speed and brcm,uses-autobaud-mode at the same time. >> >>> Please let me know if you'd still prefer "current-speed-auto" over >>> "brcm,uses-autobaud-mode" and I'll revise the patch and rename it! >> It actually depends a bit. >> >> This: >> >>>>> + The controller should be started in autobaud mode by asserting >>>>> + BT_UART_CTS_N (i.e. host RTS) during startup. Only HCI >>>>> commands supported >>>>> + in autobaud mode should be used until patch FW has been >>>>> loaded. >> sounds a bit vague? > > Yes, perhaps. I was thinking the details could be helpful but I can > see how they might be perceived as vague and confusing. Maybe it would > be better to just leave it at "The controller should be started in > autobaud mode"? > >> >> Does it mean that CTS is asserted, then you send a bit (CTS then goes >> low) >> and then CTS is asserted again when the device is ready to receieve more >> data? i.e is this some kind of one-bit mode, because it doesn't sound >> like >> it is using CTS as it was used in legacy modems. > > CTS and RTS are actually used in the normal way during communication. > The host will assert its RTS to indicate being ready to receive data > from the controller. This flag just controls whether this happens > before or after the controller is powered on. The controller will > check the initial state of its BT_UART_CTS_N pin (connected to host's > RTS) when starting up. It will enter autobaud mode if the signal is > already asserted. > >> Some more explanation of this mode is needed so we can understand if >> this is something generic or a BRCM-only thing. >> >> Yours, >> Linus Walleij > > Thanks, > Håkan
On Thu, May 19, 2022 at 4:04 PM Hakan Jansson <hakan.jansson@infineon.com> wrote: > I checked the state of this patch on Devicetree Bindings Patchwork and > it's marked "Changes Requested". I'd be happy to revise the patch but > it's not clear to me what changes are requested. Could you please help > guide me how to proceed? Sorry if it's not clear but I'm essentially requesting that the document describe how autobaud mode actually works. The binding documentation must help DT authors to know what they should do with this property. Yours, Linus Walleij
diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml b/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml index 5aac094fd217..26bba3f1c935 100644 --- a/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml +++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml @@ -92,6 +92,13 @@ properties: pcm-sync-mode: slave, master pcm-clock-mode: slave, master + brcm,uses-autobaud-mode: + type: boolean + description: > + The controller should be started in autobaud mode by asserting + BT_UART_CTS_N (i.e. host RTS) during startup. Only HCI commands supported + in autobaud mode should be used until patch FW has been loaded. + interrupts: items: - description: Handle to the line HOST_WAKE used to wake
Some devices (e.g. CYW5557x) require autobaud mode to enable FW loading. Autobaud mode can also be required on some boards where the controller device is using a non-standard baud rate when first powered on. This patch adds a property, "brcm,uses-autobaud-mode", to enable autobaud mode selection. Signed-off-by: Hakan Jansson <hakan.jansson@infineon.com> --- .../devicetree/bindings/net/broadcom-bluetooth.yaml | 7 +++++++ 1 file changed, 7 insertions(+)