Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <72cd312f-f843-6a85-b9e7-db8fcb952af8@infineon.com>
Date:   Tue, 21 Jun 2022 19:15:33 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH 4/4] Bluetooth: hci_bcm: Increase host baudrate for
 CYW55572 in autobaud mode
Content-Language: en-US
To:     Paul Menzel <pmenzel@molgen.mpg.de>
CC:     "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        <netdev@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>,
        Marcel Holtmann <marcel@holtmann.org>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        "Luiz Augusto von Dentz" <luiz.dentz@gmail.com>,
        <linux-bluetooth@vger.kernel.org>
References: <cover.1655723462.git.hakan.jansson@infineon.com>
 <386b205422099c795272ad8b792091b692def3cd.1655723462.git.hakan.jansson@infineon.com>
 <1a554d8e-c479-f646-ce9d-25871affbcee@molgen.mpg.de>
From:   Hakan Jansson <hakan.jansson@infineon.com>
In-Reply-To: <1a554d8e-c479-f646-ce9d-25871affbcee@molgen.mpg.de>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Paul,

On 6/20/2022 2:21 PM, Paul Menzel wrote:
>> Add device specific data for max baudrate in autobaud mode. This 
>> allows the
>> host to use a baudrate higher than "init speed" when loading FW in 
>> autobaud
>> mode.
>
> Please mention 921600 in the commit message, and maybe also document
> what the current default is.

Sure, I can do that if I submit a new rev. The default is 115200.

> Please also add the measurement data to the commit message, that means,
> how much is the time to load the firmware decreased.

The actual load time will depend on the specific FW used but I could add 
an example. It would be in the order of seconds.

>> Signed-off-by: Hakan Jansson <hakan.jansson@infineon.com>
>> ---
>>   drivers/bluetooth/hci_bcm.c | 20 ++++++++++++++++----
>>   1 file changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
>> index 0ae627c293c5..d7e0b75db8a6 100644
>> --- a/drivers/bluetooth/hci_bcm.c
>> +++ b/drivers/bluetooth/hci_bcm.c
>> @@ -53,10 +53,12 @@
>>    * struct bcm_device_data - device specific data
>>    * @no_early_set_baudrate: Disallow set baudrate before driver setup()
>>    * @drive_rts_on_open: drive RTS signal on ->open() when platform 
>> requires it
>> + * @max_autobaud_speed: max baudrate supported by device in autobaud 
>> mode
>>    */
>>   struct bcm_device_data {
>>       bool    no_early_set_baudrate;
>>       bool    drive_rts_on_open;
>> +     u32     max_autobaud_speed;
>
> Why specify the length, and not just `unsigned int`? Maybe also add the
> unit to the variable name?

See below.

>>   };
>>
>>   /**
>> @@ -100,6 +102,7 @@ struct bcm_device_data {
>>    * @drive_rts_on_open: drive RTS signal on ->open() when platform 
>> requires it
>>    * @pcm_int_params: keep the initial PCM configuration
>>    * @use_autobaud_mode: start Bluetooth device in autobaud mode
>> + * @max_autobaud_speed: max baudrate supported by device in autobaud 
>> mode
>>    */
>>   struct bcm_device {
>>       /* Must be the first member, hci_serdev.c expects this. */
>> @@ -139,6 +142,7 @@ struct bcm_device {
>>       bool                    drive_rts_on_open;
>>       bool                    use_autobaud_mode;
>>       u8                      pcm_int_params[5];
>> +     u32                     max_autobaud_speed;
>
> Ditto.

I'm trying to following the style of the existing code which already had 
struct members "oper_speed" and "init_speed" declared as u32.


Regards,
Håkan