On 20/05/2022 17:07, Hakan Jansson wrote:
> Some devices (e.g. CYW5557x) require autobaud mode to enable FW loading.
Which devices support this? You probably need allOf:if:then.
> 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.
Don't use "This patch":
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
>
> Signed-off-by: Hakan Jansson <[email protected]>
> ---
> V1 -> V2: Modify property description
>
> .../devicetree/bindings/net/broadcom-bluetooth.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml b/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml
> index 5aac094fd217..a29f059c21cc 100644
> --- a/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml
> +++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml
> @@ -92,6 +92,15 @@ properties:
> pcm-sync-mode: slave, master
> pcm-clock-mode: slave, master
>
> + brcm,uses-autobaud-mode:
Based on description, I understand the host triggers using autobaud.
However here you word it as "uses", so it is independent of host, it
looks like property of a device. The commit msg describes it even
different - "require autobaud".
This leads to second issue - it looks like there is some hardware
property (requiring to use autobaud) which should be described by
bindings. But instead you describe desired operational feature.
> + type: boolean
> + description: >
No need for '>'.
> + Setting this property will make the host (driver) assert the controller
> + chip's BT_UART_CTS_N prior to asserting BT_REG_ON. This will make the
> + controller start up in autobaud mode. The controller will then detect the
> + baud rate of the first incoming (HCI Reset) command from the host and
> + subsequently use that baud rate.
> +
> interrupts:
> items:
> - description: Handle to the line HOST_WAKE used to wake
Best regards,
Krzysztof
Hi Krzysztof,
Thanks for responding.
On 5/22/2022 10:14 AM, Krzysztof Kozlowski wrote:
>> Some devices (e.g. CYW5557x) require autobaud mode to enable FW loading.
> Which devices support this? You probably need allOf:if:then.
Most devices _support_ autobaud mode but I don't have a definitive list.
The CYW5557x is the first device family to _require_ autobaud mode for
FW loading as far as I know.
>> 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.
> Don't use "This patch":
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
Sorry, will change in next rev.
>> Signed-off-by: Hakan Jansson <[email protected]>
>> ---
>> V1 -> V2: Modify property description
>>
>> .../devicetree/bindings/net/broadcom-bluetooth.yaml | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml b/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml
>> index 5aac094fd217..a29f059c21cc 100644
>> --- a/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml
>> +++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml
>> @@ -92,6 +92,15 @@ properties:
>> pcm-sync-mode: slave, master
>> pcm-clock-mode: slave, master
>>
>> + brcm,uses-autobaud-mode:
> Based on description, I understand the host triggers using autobaud.
Correct, the host triggers using autobaud.
> However here you word it as "uses", so it is independent of host, it
> looks like property of a device.
I've been thinking of it as a a property of a specific board HW,
inherited either from a property of the device being used or from a
property of the HW design (e.g. a PCB using an alternate crystal
frequency yielding a non-standard baud rate).
> The commit msg describes it even
> different - "require autobaud".
Yes, the commit message describes two separate reasons why autobaud mode
would be required:
1. Requirement coming from Device: "Some devices (e.g. CYW5557x) require
autobaud mode to enable FW loading."
2. Requirement coming from HW design: "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 leads to second issue - it looks like there is some hardware
> property (requiring to use autobaud) which should be described by
> bindings. But instead you describe desired operational feature.
Sorry about that. I've really been struggling with what should go into
the description. Any suggestions or hints would be much appreciated.
How about, changing the property name to "brcm,requires-autobaud-mode"
and the description to:
"Set this property if autobaud mode is required. Autobaud mode is
required if the device's baud rate in normal mode is not supported by
the host or if the device requires autobaud mode startup before loading FW."
Would that be an appropriate name and description?
>> + type: boolean
>> + description: >
> No need for '>'.
Ok, will remove in next rev.
>> + Setting this property will make the host (driver) assert the controller
>> + chip's BT_UART_CTS_N prior to asserting BT_REG_ON. This will make the
>> + controller start up in autobaud mode. The controller will then detect the
>> + baud rate of the first incoming (HCI Reset) command from the host and
>> + subsequently use that baud rate.
>> +
>> interrupts:
>> items:
>> - description: Handle to the line HOST_WAKE used to wake
>
Thanks,
Håkan
On 23/05/2022 11:21, Hakan Jansson wrote:
> Hi Krzysztof,
>
> Thanks for responding.
>
> On 5/22/2022 10:14 AM, Krzysztof Kozlowski wrote:
>>> Some devices (e.g. CYW5557x) require autobaud mode to enable FW loading.
>> Which devices support this? You probably need allOf:if:then.
>
> Most devices _support_ autobaud mode but I don't have a definitive list.
> The CYW5557x is the first device family to _require_ autobaud mode for
> FW loading as far as I know.
>
>>> 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.
>> Don't use "This patch":
>> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
>
> Sorry, will change in next rev.
>
>>> Signed-off-by: Hakan Jansson <[email protected]>
>>> ---
>>> V1 -> V2: Modify property description
>>>
>>> .../devicetree/bindings/net/broadcom-bluetooth.yaml | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml b/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml
>>> index 5aac094fd217..a29f059c21cc 100644
>>> --- a/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml
>>> +++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml
>>> @@ -92,6 +92,15 @@ properties:
>>> pcm-sync-mode: slave, master
>>> pcm-clock-mode: slave, master
>>>
>>> + brcm,uses-autobaud-mode:
>> Based on description, I understand the host triggers using autobaud.
>
> Correct, the host triggers using autobaud.
>
>> However here you word it as "uses", so it is independent of host, it
>> looks like property of a device.
>
> I've been thinking of it as a a property of a specific board HW,
> inherited either from a property of the device being used or from a
> property of the HW design (e.g. a PCB using an alternate crystal
> frequency yielding a non-standard baud rate).
>
>> The commit msg describes it even
>> different - "require autobaud".
>
> Yes, the commit message describes two separate reasons why autobaud mode
> would be required:
>
> 1. Requirement coming from Device: "Some devices (e.g. CYW5557x) require
> autobaud mode to enable FW loading."
>
> 2. Requirement coming from HW design: "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 leads to second issue - it looks like there is some hardware
>> property (requiring to use autobaud) which should be described by
>> bindings. But instead you describe desired operational feature.
>
> Sorry about that. I've really been struggling with what should go into
> the description. Any suggestions or hints would be much appreciated.
>
> How about, changing the property name to "brcm,requires-autobaud-mode"
> and the description to:
> "Set this property if autobaud mode is required. Autobaud mode is
> required if the device's baud rate in normal mode is not supported by
> the host or if the device requires autobaud mode startup before loading FW."
>
> Would that be an appropriate name and description?
Yes, sounds good. I also have trouble to name it nicely.
Sorry for late reply.
Best regards,
Krzysztof