Received-SPF: pass (google.com: domain of linux-kernel+bounces-23387-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <9f776426-b01f-4204-b508-485d3f13e933@molgen.mpg.de>
Date: Thu, 11 Jan 2024 11:33:44 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1] Bluetooth: btnxpuart: Disable Power Save feature on
 startup
Content-Language: en-US
To: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
Cc: marcel@holtmann.org, johan.hedberg@gmail.com, luiz.dentz@gmail.com,
 amitkumar.karwar@nxp.com, linux-bluetooth@vger.kernel.org,
 linux-kernel@vger.kernel.org, sherry.sun@nxp.com, rohit.fule@nxp.com
References: <20240111080642.270123-1-neeraj.sanjaykale@nxp.com>
From: Paul Menzel <pmenzel@molgen.mpg.de>
In-Reply-To: <20240111080642.270123-1-neeraj.sanjaykale@nxp.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Dear Neeraj,


Thank you for your patch. Some nits below.

Am 11.01.24 um 09:06 schrieb Neeraj Sanjay Kale:
> This sets the default power save mode setting to enabled.

This contradicts the commit message summary, which says “Disable”.

> The power save feature is now stable and stress test issues,
> such as the TX timeout error, has been resolved.

have been

Please reference one commit, that fixed this.

> With this setting, the driver will send the vendor command
> to FW at startup, to enable power save feature.
> User can disable this feature using the following vendor
> command:
> hcitool cmd 3f 23 03 00 00 (HCI_NXP_AUTO_SLEEP_MODE)

It would help me, if you used the full 75 characters per line for commit 
messages, and would not break the line after every sentence. If a 
paragraph makes sense, please separate it by a  blank line.

> Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> ---
>   drivers/bluetooth/btnxpuart.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
> index 7f88b6f52f26..42e929f0d141 100644
> --- a/drivers/bluetooth/btnxpuart.c
> +++ b/drivers/bluetooth/btnxpuart.c
> @@ -281,7 +281,7 @@ static u8 crc8_table[CRC8_TABLE_SIZE];
>   
>   /* Default configurations */
>   #define DEFAULT_H2C_WAKEUP_MODE	WAKEUP_METHOD_BREAK
> -#define DEFAULT_PS_MODE		PS_MODE_DISABLE
> +#define DEFAULT_PS_MODE		PS_MODE_ENABLE
>   #define FW_INIT_BAUDRATE	HCI_NXP_PRI_BAUDRATE
>   
>   static struct sk_buff *nxp_drv_send_cmd(struct hci_dev *hdev, u16 opcode,

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul