We will need to call the qca_power_off() API to power off Rome, add the
support into it. QCA Rome is using bt_en GPIO for power off, so we just
need to pull down this bt_en GPIO to power off it.
Signed-off-by: Rocky Liao <[email protected]>
---
drivers/bluetooth/hci_qca.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index b602ed01505b..43fd84028786 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1413,13 +1413,26 @@ static void qca_power_shutdown(struct hci_uart *hu)
static int qca_power_off(struct hci_dev *hdev)
{
struct hci_uart *hu = hci_get_drvdata(hdev);
+ struct qca_serdev *qcadev;
+ enum qca_btsoc_type soc_type = qca_soc_type(hu);
+
+ if (qca_is_wcn399x(soc_type)) {
+ /* Perform pre shutdown command */
+ qca_send_pre_shutdown_cmd(hdev);
+
+ usleep_range(8000, 10000);
- /* Perform pre shutdown command */
- qca_send_pre_shutdown_cmd(hdev);
+ qca_power_shutdown(hu);
+ } else {
+ if (hu->serdev) {
+ qcadev = serdev_device_get_drvdata(hu->serdev);
+
+ gpiod_set_value_cansleep(qcadev->bt_en, 0);
- usleep_range(8000, 10000);
+ usleep_range(8000, 10000);
+ }
+ }
- qca_power_shutdown(hu);
return 0;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
This patch adds the HCI command timeout handling, it will trigger btsoc
to report its memory dump via vendor specific events when hit the defined
max HCI command timeout count. After all the memory dump VSE are sent, the
btsoc will also send a HCI_HW_ERROR event to host and this will cause a new
hci down/up process and the btsoc will be re-initialized.
Signed-off-by: Rocky Liao <[email protected]>
---
drivers/bluetooth/hci_qca.c | 40 +++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 7e202041ed77..bc74d69b3d80 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -47,6 +47,8 @@
#define IBS_HOST_TX_IDLE_TIMEOUT_MS 2000
#define CMD_TRANS_TIMEOUT_MS 100
+#define QCA_BTSOC_DUMP_CMD 0xFB
+
/* susclk rate */
#define SUSCLK_RATE_32KHZ 32768
@@ -56,6 +58,9 @@
/* max retry count when init fails */
#define QCA_MAX_INIT_RETRY_COUNT 3
+/* when hit the max cmd time out count, trigger btsoc dump */
+#define QCA_MAX_CMD_TIMEOUT_COUNT 3
+
enum qca_flags {
QCA_IBS_ENABLED,
QCA_DROP_VENDOR_EVENT,
@@ -170,6 +175,7 @@ static int qca_regulator_enable(struct qca_serdev *qcadev);
static void qca_regulator_disable(struct qca_serdev *qcadev);
static void qca_power_shutdown(struct hci_uart *hu);
static int qca_power_off(struct hci_dev *hdev);
+static void qca_cmd_timeout(struct hci_uart *hu);
static enum qca_btsoc_type qca_soc_type(struct hci_uart *hu)
{
@@ -1337,6 +1343,8 @@ static int qca_setup(struct hci_uart *hu)
if (!ret) {
set_bit(QCA_IBS_ENABLED, &qca->flags);
qca_debugfs_init(hdev);
+ hdev->cmd_timeout = qca_cmd_timeout;
+ qca->cmd_timeout_cnt = 0;
} else if (ret == -ENOENT) {
/* No patch/nvm-config found, run with original fw/config */
ret = 0;
@@ -1467,6 +1475,38 @@ static int qca_power_off(struct hci_dev *hdev)
return 0;
}
+static int qca_send_btsoc_dump_cmd(struct hci_uart *hu)
+{
+ int err = 0;
+ struct sk_buff *skb = NULL;
+ struct qca_data *qca = hu->priv;
+
+ BT_DBG("hu %p sending btsoc dump command", hu);
+
+ skb = bt_skb_alloc(1, GFP_ATOMIC);
+ if (!skb) {
+ BT_ERR("Failed to allocate memory for qca dump command");
+ return -ENOMEM;
+ }
+
+ skb_put_u8(skb, QCA_BTSOC_DUMP_CMD);
+
+ skb_queue_tail(&qca->txq, skb);
+
+ return err;
+}
+
+
+static void qca_cmd_timeout(struct hci_uart *hu)
+{
+ struct qca_data *qca = hu->priv;
+
+ BT_ERR("hu %p hci cmd timeout count=0x%x", hu, ++qca->cmd_timeout_cnt);
+
+ if (qca->cmd_timeout_cnt >= QCA_MAX_CMD_TIMEOUT_COUNT)
+ qca_send_btsoc_dump_cmd(hu);
+}
+
static int qca_regulator_enable(struct qca_serdev *qcadev)
{
struct qca_power *power = qcadev->bt_power;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
This patch adds the retry of btsoc initialization when it fails. There are
reports that the btsoc initialization may fail on some platforms but the
repro ratio is very low. The failure may be caused by UART, platform HW or
the btsoc itself but it's very difficlut to root cause, given the repro
ratio is very low. Add a retry for the btsoc initialization will resolve
most of the failures and make Bluetooth finally works.
Signed-off-by: Rocky Liao <[email protected]>
---
drivers/bluetooth/hci_qca.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 43fd84028786..45042aa27fa4 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -53,6 +53,9 @@
/* Controller debug log header */
#define QCA_DEBUG_HANDLE 0x2EDC
+/* max retry count when init fails */
+#define QCA_MAX_INIT_RETRY_COUNT 3
+
enum qca_flags {
QCA_IBS_ENABLED,
QCA_DROP_VENDOR_EVENT,
@@ -1257,7 +1260,9 @@ static int qca_setup(struct hci_uart *hu)
{
struct hci_dev *hdev = hu->hdev;
struct qca_data *qca = hu->priv;
+ struct qca_serdev *qcadev;
unsigned int speed, qca_baudrate = QCA_BAUDRATE_115200;
+ unsigned int init_retry_count = 0;
enum qca_btsoc_type soc_type = qca_soc_type(hu);
const char *firmware_name = qca_get_firmware_name(hu);
int ret;
@@ -1275,6 +1280,7 @@ static int qca_setup(struct hci_uart *hu)
*/
set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
+retry:
if (qca_is_wcn399x(soc_type)) {
bt_dev_info(hdev, "setting up wcn3990");
@@ -1293,6 +1299,12 @@ static int qca_setup(struct hci_uart *hu)
return ret;
} else {
bt_dev_info(hdev, "ROME setup");
+ if (hu->serdev) {
+ qcadev = serdev_device_get_drvdata(hu->serdev);
+ gpiod_set_value_cansleep(qcadev->bt_en, 1);
+ /* Controller needs time to bootup. */
+ msleep(150);
+ }
qca_set_speed(hu, QCA_INIT_SPEED);
}
@@ -1329,6 +1341,20 @@ static int qca_setup(struct hci_uart *hu)
* patch/nvm-config is found, so run with original fw/config.
*/
ret = 0;
+ } else {
+ if (init_retry_count < QCA_MAX_INIT_RETRY_COUNT) {
+ qca_power_off(hdev);
+ if (hu->serdev) {
+ serdev_device_close(hu->serdev);
+ ret = serdev_device_open(hu->serdev);
+ if (ret) {
+ bt_dev_err(hu->hdev, "open port fail");
+ return ret;
+ }
+ }
+ init_retry_count++;
+ goto retry;
+ }
}
/* Setup bdaddr */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
This patch registers hdev->shutdown() callback and also sets
HCI_QUIRK_NON_PERSISTENT_SETUP for QCA Rome. It will power-off the BT chip
during hci down and power-on/initialize the chip again during hci up.
Signed-off-by: Rocky Liao <[email protected]>
---
drivers/bluetooth/hci_qca.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 45042aa27fa4..7e202041ed77 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1300,6 +1300,11 @@ static int qca_setup(struct hci_uart *hu)
} else {
bt_dev_info(hdev, "ROME setup");
if (hu->serdev) {
+ /* Enable NON_PERSISTENT_SETUP QUIRK to ensure to
+ * execute setup for every hci up.
+ */
+ set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
+ hu->hdev->shutdown = qca_power_off;
qcadev = serdev_device_get_drvdata(hu->serdev);
gpiod_set_value_cansleep(qcadev->bt_en, 1);
/* Controller needs time to bootup. */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Rocky,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on bluetooth-next/master]
[also build test ERROR on linux/master linus/master v5.5-rc3 next-20191220]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Rocky-Liao/Bluetooth-hci_qca-Add-QCA-Rome-power-off-support-to-the-qca_power_off/20191226-050217
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: nds32-allyesconfig (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=9.2.0 make.cross ARCH=nds32
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
All errors (new ones prefixed by >>):
drivers/bluetooth/hci_qca.c: In function 'qca_setup':
>> drivers/bluetooth/hci_qca.c:1346:21: error: assignment to 'void (*)(struct hci_dev *)' from incompatible pointer type 'void (*)(struct hci_uart *)' [-Werror=incompatible-pointer-types]
1346 | hdev->cmd_timeout = qca_cmd_timeout;
| ^
drivers/bluetooth/hci_qca.c:1347:6: error: 'struct qca_data' has no member named 'cmd_timeout_cnt'
1347 | qca->cmd_timeout_cnt = 0;
| ^~
In file included from drivers/bluetooth/hci_qca.c:33:
drivers/bluetooth/hci_qca.c: In function 'qca_cmd_timeout':
drivers/bluetooth/hci_qca.c:1504:54: error: 'struct qca_data' has no member named 'cmd_timeout_cnt'
1504 | BT_ERR("hu %p hci cmd timeout count=0x%x", hu, ++qca->cmd_timeout_cnt);
| ^~
include/net/bluetooth/bluetooth.h:138:45: note: in definition of macro 'BT_ERR'
138 | #define BT_ERR(fmt, ...) bt_err(fmt "\n", ##__VA_ARGS__)
| ^~~~~~~~~~~
drivers/bluetooth/hci_qca.c:1506:9: error: 'struct qca_data' has no member named 'cmd_timeout_cnt'
1506 | if (qca->cmd_timeout_cnt >= QCA_MAX_CMD_TIMEOUT_COUNT)
| ^~
cc1: some warnings being treated as errors
vim +1346 drivers/bluetooth/hci_qca.c
1264
1265 static int qca_setup(struct hci_uart *hu)
1266 {
1267 struct hci_dev *hdev = hu->hdev;
1268 struct qca_data *qca = hu->priv;
1269 struct qca_serdev *qcadev;
1270 unsigned int speed, qca_baudrate = QCA_BAUDRATE_115200;
1271 unsigned int init_retry_count = 0;
1272 enum qca_btsoc_type soc_type = qca_soc_type(hu);
1273 const char *firmware_name = qca_get_firmware_name(hu);
1274 int ret;
1275 int soc_ver = 0;
1276
1277 ret = qca_check_speeds(hu);
1278 if (ret)
1279 return ret;
1280
1281 /* Patch downloading has to be done without IBS mode */
1282 clear_bit(QCA_IBS_ENABLED, &qca->flags);
1283
1284 /* Enable controller to do both LE scan and BR/EDR inquiry
1285 * simultaneously.
1286 */
1287 set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
1288
1289 retry:
1290 if (qca_is_wcn399x(soc_type)) {
1291 bt_dev_info(hdev, "setting up wcn3990");
1292
1293 /* Enable NON_PERSISTENT_SETUP QUIRK to ensure to execute
1294 * setup for every hci up.
1295 */
1296 set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
1297 set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
1298 hu->hdev->shutdown = qca_power_off;
1299 ret = qca_wcn3990_init(hu);
1300 if (ret)
1301 return ret;
1302
1303 ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
1304 if (ret)
1305 return ret;
1306 } else {
1307 bt_dev_info(hdev, "ROME setup");
1308 if (hu->serdev) {
1309 /* Enable NON_PERSISTENT_SETUP QUIRK to ensure to
1310 * execute setup for every hci up.
1311 */
1312 set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
1313 hu->hdev->shutdown = qca_power_off;
1314 qcadev = serdev_device_get_drvdata(hu->serdev);
1315 gpiod_set_value_cansleep(qcadev->bt_en, 1);
1316 /* Controller needs time to bootup. */
1317 msleep(150);
1318 }
1319 qca_set_speed(hu, QCA_INIT_SPEED);
1320 }
1321
1322 /* Setup user speed if needed */
1323 speed = qca_get_speed(hu, QCA_OPER_SPEED);
1324 if (speed) {
1325 ret = qca_set_speed(hu, QCA_OPER_SPEED);
1326 if (ret)
1327 return ret;
1328
1329 qca_baudrate = qca_get_baudrate_value(speed);
1330 }
1331
1332 if (!qca_is_wcn399x(soc_type)) {
1333 /* Get QCA version information */
1334 ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
1335 if (ret)
1336 return ret;
1337 }
1338
1339 bt_dev_info(hdev, "QCA controller version 0x%08x", soc_ver);
1340 /* Setup patch / NVM configurations */
1341 ret = qca_uart_setup(hdev, qca_baudrate, soc_type, soc_ver,
1342 firmware_name);
1343 if (!ret) {
1344 set_bit(QCA_IBS_ENABLED, &qca->flags);
1345 qca_debugfs_init(hdev);
> 1346 hdev->cmd_timeout = qca_cmd_timeout;
1347 qca->cmd_timeout_cnt = 0;
1348 } else if (ret == -ENOENT) {
1349 /* No patch/nvm-config found, run with original fw/config */
1350 ret = 0;
1351 } else if (ret == -EAGAIN) {
1352 /*
1353 * Userspace firmware loader will return -EAGAIN in case no
1354 * patch/nvm-config is found, so run with original fw/config.
1355 */
1356 ret = 0;
1357 } else {
1358 if (init_retry_count < QCA_MAX_INIT_RETRY_COUNT) {
1359 qca_power_off(hdev);
1360 if (hu->serdev) {
1361 serdev_device_close(hu->serdev);
1362 ret = serdev_device_open(hu->serdev);
1363 if (ret) {
1364 bt_dev_err(hu->hdev, "open port fail");
1365 return ret;
1366 }
1367 }
1368 init_retry_count++;
1369 goto retry;
1370 }
1371 }
1372
1373 /* Setup bdaddr */
1374 if (qca_is_wcn399x(soc_type))
1375 hu->hdev->set_bdaddr = qca_set_bdaddr;
1376 else
1377 hu->hdev->set_bdaddr = qca_set_bdaddr_rome;
1378
1379 return ret;
1380 }
1381
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation
We will need to call the qca_power_off() API to power off Rome, add the
support into it. QCA Rome is using bt_en GPIO for power off, so we just
need to pull down this bt_en GPIO to power off it.
Signed-off-by: Rocky Liao <[email protected]>
---
Changes in v2: None
drivers/bluetooth/hci_qca.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index b602ed01505b..43fd84028786 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1413,13 +1413,26 @@ static void qca_power_shutdown(struct hci_uart *hu)
static int qca_power_off(struct hci_dev *hdev)
{
struct hci_uart *hu = hci_get_drvdata(hdev);
+ struct qca_serdev *qcadev;
+ enum qca_btsoc_type soc_type = qca_soc_type(hu);
+
+ if (qca_is_wcn399x(soc_type)) {
+ /* Perform pre shutdown command */
+ qca_send_pre_shutdown_cmd(hdev);
+
+ usleep_range(8000, 10000);
- /* Perform pre shutdown command */
- qca_send_pre_shutdown_cmd(hdev);
+ qca_power_shutdown(hu);
+ } else {
+ if (hu->serdev) {
+ qcadev = serdev_device_get_drvdata(hu->serdev);
+
+ gpiod_set_value_cansleep(qcadev->bt_en, 0);
- usleep_range(8000, 10000);
+ usleep_range(8000, 10000);
+ }
+ }
- qca_power_shutdown(hu);
return 0;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
This patch adds the HCI command timeout handling, it will trigger btsoc
to report its memory dump via vendor specific events when hit the defined
max HCI command timeout count. After all the memory dump VSE are sent, the
btsoc will also send a HCI_HW_ERROR event to host and this will cause a new
hci down/up process and the btsoc will be re-initialized.
Signed-off-by: Rocky Liao <[email protected]>
---
Changes in v2:
- Fix build error
drivers/bluetooth/hci_qca.c | 43 +++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 7e202041ed77..34ef73daebb2 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -47,6 +47,8 @@
#define IBS_HOST_TX_IDLE_TIMEOUT_MS 2000
#define CMD_TRANS_TIMEOUT_MS 100
+#define QCA_BTSOC_DUMP_CMD 0xFB
+
/* susclk rate */
#define SUSCLK_RATE_32KHZ 32768
@@ -56,6 +58,9 @@
/* max retry count when init fails */
#define QCA_MAX_INIT_RETRY_COUNT 3
+/* when hit the max cmd time out count, trigger btsoc dump */
+#define QCA_MAX_CMD_TIMEOUT_COUNT 3
+
enum qca_flags {
QCA_IBS_ENABLED,
QCA_DROP_VENDOR_EVENT,
@@ -123,6 +128,8 @@ struct qca_data {
u64 rx_votes_off;
u64 votes_on;
u64 votes_off;
+
+ u32 cmd_timeout_cnt;
};
enum qca_speed_type {
@@ -170,6 +177,7 @@ static int qca_regulator_enable(struct qca_serdev *qcadev);
static void qca_regulator_disable(struct qca_serdev *qcadev);
static void qca_power_shutdown(struct hci_uart *hu);
static int qca_power_off(struct hci_dev *hdev);
+static void qca_cmd_timeout(struct hci_dev *hdev);
static enum qca_btsoc_type qca_soc_type(struct hci_uart *hu)
{
@@ -1337,6 +1345,8 @@ static int qca_setup(struct hci_uart *hu)
if (!ret) {
set_bit(QCA_IBS_ENABLED, &qca->flags);
qca_debugfs_init(hdev);
+ hdev->cmd_timeout = qca_cmd_timeout;
+ qca->cmd_timeout_cnt = 0;
} else if (ret == -ENOENT) {
/* No patch/nvm-config found, run with original fw/config */
ret = 0;
@@ -1467,6 +1477,39 @@ static int qca_power_off(struct hci_dev *hdev)
return 0;
}
+static int qca_send_btsoc_dump_cmd(struct hci_uart *hu)
+{
+ int err = 0;
+ struct sk_buff *skb = NULL;
+ struct qca_data *qca = hu->priv;
+
+ BT_DBG("hu %p sending btsoc dump command", hu);
+
+ skb = bt_skb_alloc(1, GFP_ATOMIC);
+ if (!skb) {
+ BT_ERR("Failed to allocate memory for qca dump command");
+ return -ENOMEM;
+ }
+
+ skb_put_u8(skb, QCA_BTSOC_DUMP_CMD);
+
+ skb_queue_tail(&qca->txq, skb);
+
+ return err;
+}
+
+
+static void qca_cmd_timeout(struct hci_dev *hdev)
+{
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+ struct qca_data *qca = hu->priv;
+
+ BT_ERR("hu %p hci cmd timeout count=0x%x", hu, ++qca->cmd_timeout_cnt);
+
+ if (qca->cmd_timeout_cnt >= QCA_MAX_CMD_TIMEOUT_COUNT)
+ qca_send_btsoc_dump_cmd(hu);
+}
+
static int qca_regulator_enable(struct qca_serdev *qcadev)
{
struct qca_power *power = qcadev->bt_power;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Rocky,
> This patch adds the HCI command timeout handling, it will trigger btsoc
> to report its memory dump via vendor specific events when hit the defined
> max HCI command timeout count. After all the memory dump VSE are sent, the
> btsoc will also send a HCI_HW_ERROR event to host and this will cause a new
> hci down/up process and the btsoc will be re-initialized.
>
> Signed-off-by: Rocky Liao <[email protected]>
> ---
>
> Changes in v2:
> - Fix build error
>
> drivers/bluetooth/hci_qca.c | 43 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 7e202041ed77..34ef73daebb2 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -47,6 +47,8 @@
> #define IBS_HOST_TX_IDLE_TIMEOUT_MS 2000
> #define CMD_TRANS_TIMEOUT_MS 100
>
> +#define QCA_BTSOC_DUMP_CMD 0xFB
> +
> /* susclk rate */
> #define SUSCLK_RATE_32KHZ 32768
>
> @@ -56,6 +58,9 @@
> /* max retry count when init fails */
> #define QCA_MAX_INIT_RETRY_COUNT 3
>
> +/* when hit the max cmd time out count, trigger btsoc dump */
> +#define QCA_MAX_CMD_TIMEOUT_COUNT 3
> +
> enum qca_flags {
> QCA_IBS_ENABLED,
> QCA_DROP_VENDOR_EVENT,
> @@ -123,6 +128,8 @@ struct qca_data {
> u64 rx_votes_off;
> u64 votes_on;
> u64 votes_off;
> +
> + u32 cmd_timeout_cnt;
> };
>
> enum qca_speed_type {
> @@ -170,6 +177,7 @@ static int qca_regulator_enable(struct qca_serdev *qcadev);
> static void qca_regulator_disable(struct qca_serdev *qcadev);
> static void qca_power_shutdown(struct hci_uart *hu);
> static int qca_power_off(struct hci_dev *hdev);
> +static void qca_cmd_timeout(struct hci_dev *hdev);
I thought that I already mentioned that these forward declaration have to be removed. I have no plan to take patches that even add more forward declarations.
>
> static enum qca_btsoc_type qca_soc_type(struct hci_uart *hu)
> {
> @@ -1337,6 +1345,8 @@ static int qca_setup(struct hci_uart *hu)
> if (!ret) {
> set_bit(QCA_IBS_ENABLED, &qca->flags);
> qca_debugfs_init(hdev);
> + hdev->cmd_timeout = qca_cmd_timeout;
Why set it here? Set in the probe() callback.
> + qca->cmd_timeout_cnt = 0;
This is a race conditions if not all chip types will use NON_PERSISTENT_SETUP.
> } else if (ret == -ENOENT) {
> /* No patch/nvm-config found, run with original fw/config */
> ret = 0;
> @@ -1467,6 +1477,39 @@ static int qca_power_off(struct hci_dev *hdev)
> return 0;
> }
>
> +static int qca_send_btsoc_dump_cmd(struct hci_uart *hu)
> +{
> + int err = 0;
> + struct sk_buff *skb = NULL;
> + struct qca_data *qca = hu->priv;
> +
> + BT_DBG("hu %p sending btsoc dump command", hu);
> +
> + skb = bt_skb_alloc(1, GFP_ATOMIC);
> + if (!skb) {
> + BT_ERR("Failed to allocate memory for qca dump command");
> + return -ENOMEM;
> + }
> +
> + skb_put_u8(skb, QCA_BTSOC_DUMP_CMD);
> +
> + skb_queue_tail(&qca->txq, skb);
> +
> + return err;
> +}
> +
> +
No double empty lines.
> +static void qca_cmd_timeout(struct hci_dev *hdev)
> +{
> + struct hci_uart *hu = hci_get_drvdata(hdev);
> + struct qca_data *qca = hu->priv;
> +
> + BT_ERR("hu %p hci cmd timeout count=0x%x", hu, ++qca->cmd_timeout_cnt);
> +
> + if (qca->cmd_timeout_cnt >= QCA_MAX_CMD_TIMEOUT_COUNT)
> + qca_send_btsoc_dump_cmd(hu);
> +}
> +
> static int qca_regulator_enable(struct qca_serdev *qcadev)
> {
> struct qca_power *power = qcadev->bt_power;
Regards
Marcel
Hi Rocky,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on linux/master linus/master v5.5-rc3 next-20191220]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Rocky-Liao/Bluetooth-hci_qca-Add-QCA-Rome-power-off-support-to-the-qca_power_off/20191226-050217
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-129-g341daf20-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
sparse warnings: (new ones prefixed by >>)
drivers/bluetooth/hci_qca.c:1504:9: sparse: sparse: no member 'cmd_timeout_cnt' in struct qca_data
drivers/bluetooth/hci_qca.c:1506:16: sparse: sparse: no member 'cmd_timeout_cnt' in struct qca_data
>> drivers/bluetooth/hci_qca.c:1346:35: sparse: sparse: incorrect type in assignment (incompatible argument 1 (different base types))
>> drivers/bluetooth/hci_qca.c:1346:35: sparse: expected void ( *cmd_timeout )( ... )
>> drivers/bluetooth/hci_qca.c:1346:35: sparse: got void ( * )( ... )
drivers/bluetooth/hci_qca.c:1347:20: sparse: sparse: no member 'cmd_timeout_cnt' in struct qca_data
vim +1346 drivers/bluetooth/hci_qca.c
1264
1265 static int qca_setup(struct hci_uart *hu)
1266 {
1267 struct hci_dev *hdev = hu->hdev;
1268 struct qca_data *qca = hu->priv;
1269 struct qca_serdev *qcadev;
1270 unsigned int speed, qca_baudrate = QCA_BAUDRATE_115200;
1271 unsigned int init_retry_count = 0;
1272 enum qca_btsoc_type soc_type = qca_soc_type(hu);
1273 const char *firmware_name = qca_get_firmware_name(hu);
1274 int ret;
1275 int soc_ver = 0;
1276
1277 ret = qca_check_speeds(hu);
1278 if (ret)
1279 return ret;
1280
1281 /* Patch downloading has to be done without IBS mode */
1282 clear_bit(QCA_IBS_ENABLED, &qca->flags);
1283
1284 /* Enable controller to do both LE scan and BR/EDR inquiry
1285 * simultaneously.
1286 */
1287 set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
1288
1289 retry:
1290 if (qca_is_wcn399x(soc_type)) {
1291 bt_dev_info(hdev, "setting up wcn3990");
1292
1293 /* Enable NON_PERSISTENT_SETUP QUIRK to ensure to execute
1294 * setup for every hci up.
1295 */
1296 set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
1297 set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
1298 hu->hdev->shutdown = qca_power_off;
1299 ret = qca_wcn3990_init(hu);
1300 if (ret)
1301 return ret;
1302
1303 ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
1304 if (ret)
1305 return ret;
1306 } else {
1307 bt_dev_info(hdev, "ROME setup");
1308 if (hu->serdev) {
1309 /* Enable NON_PERSISTENT_SETUP QUIRK to ensure to
1310 * execute setup for every hci up.
1311 */
1312 set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
1313 hu->hdev->shutdown = qca_power_off;
1314 qcadev = serdev_device_get_drvdata(hu->serdev);
1315 gpiod_set_value_cansleep(qcadev->bt_en, 1);
1316 /* Controller needs time to bootup. */
1317 msleep(150);
1318 }
1319 qca_set_speed(hu, QCA_INIT_SPEED);
1320 }
1321
1322 /* Setup user speed if needed */
1323 speed = qca_get_speed(hu, QCA_OPER_SPEED);
1324 if (speed) {
1325 ret = qca_set_speed(hu, QCA_OPER_SPEED);
1326 if (ret)
1327 return ret;
1328
1329 qca_baudrate = qca_get_baudrate_value(speed);
1330 }
1331
1332 if (!qca_is_wcn399x(soc_type)) {
1333 /* Get QCA version information */
1334 ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
1335 if (ret)
1336 return ret;
1337 }
1338
1339 bt_dev_info(hdev, "QCA controller version 0x%08x", soc_ver);
1340 /* Setup patch / NVM configurations */
1341 ret = qca_uart_setup(hdev, qca_baudrate, soc_type, soc_ver,
1342 firmware_name);
1343 if (!ret) {
1344 set_bit(QCA_IBS_ENABLED, &qca->flags);
1345 qca_debugfs_init(hdev);
> 1346 hdev->cmd_timeout = qca_cmd_timeout;
1347 qca->cmd_timeout_cnt = 0;
1348 } else if (ret == -ENOENT) {
1349 /* No patch/nvm-config found, run with original fw/config */
1350 ret = 0;
1351 } else if (ret == -EAGAIN) {
1352 /*
1353 * Userspace firmware loader will return -EAGAIN in case no
1354 * patch/nvm-config is found, so run with original fw/config.
1355 */
1356 ret = 0;
1357 } else {
1358 if (init_retry_count < QCA_MAX_INIT_RETRY_COUNT) {
1359 qca_power_off(hdev);
1360 if (hu->serdev) {
1361 serdev_device_close(hu->serdev);
1362 ret = serdev_device_open(hu->serdev);
1363 if (ret) {
1364 bt_dev_err(hu->hdev, "open port fail");
1365 return ret;
1366 }
1367 }
1368 init_retry_count++;
1369 goto retry;
1370 }
1371 }
1372
1373 /* Setup bdaddr */
1374 if (qca_is_wcn399x(soc_type))
1375 hu->hdev->set_bdaddr = qca_set_bdaddr;
1376 else
1377 hu->hdev->set_bdaddr = qca_set_bdaddr_rome;
1378
1379 return ret;
1380 }
1381
1382 static const struct hci_uart_proto qca_proto = {
1383 .id = HCI_UART_QCA,
1384 .name = "QCA",
1385 .manufacturer = 29,
1386 .init_speed = 115200,
1387 .oper_speed = 3000000,
1388 .open = qca_open,
1389 .close = qca_close,
1390 .flush = qca_flush,
1391 .setup = qca_setup,
1392 .recv = qca_recv,
1393 .enqueue = qca_enqueue,
1394 .dequeue = qca_dequeue,
1395 };
1396
1397 static const struct qca_vreg_data qca_soc_data_wcn3990 = {
1398 .soc_type = QCA_WCN3990,
1399 .vregs = (struct qca_vreg []) {
1400 { "vddio", 15000 },
1401 { "vddxo", 80000 },
1402 { "vddrf", 300000 },
1403 { "vddch0", 450000 },
1404 },
1405 .num_vregs = 4,
1406 };
1407
1408 static const struct qca_vreg_data qca_soc_data_wcn3991 = {
1409 .soc_type = QCA_WCN3991,
1410 .vregs = (struct qca_vreg []) {
1411 { "vddio", 15000 },
1412 { "vddxo", 80000 },
1413 { "vddrf", 300000 },
1414 { "vddch0", 450000 },
1415 },
1416 .num_vregs = 4,
1417 };
1418
1419 static const struct qca_vreg_data qca_soc_data_wcn3998 = {
1420 .soc_type = QCA_WCN3998,
1421 .vregs = (struct qca_vreg []) {
1422 { "vddio", 10000 },
1423 { "vddxo", 80000 },
1424 { "vddrf", 300000 },
1425 { "vddch0", 450000 },
1426 },
1427 .num_vregs = 4,
1428 };
1429
1430 static void qca_power_shutdown(struct hci_uart *hu)
1431 {
1432 struct qca_serdev *qcadev;
1433 struct qca_data *qca = hu->priv;
1434 unsigned long flags;
1435
1436 qcadev = serdev_device_get_drvdata(hu->serdev);
1437
1438 /* From this point we go into power off state. But serial port is
1439 * still open, stop queueing the IBS data and flush all the buffered
1440 * data in skb's.
1441 */
1442 spin_lock_irqsave(&qca->hci_ibs_lock, flags);
1443 clear_bit(QCA_IBS_ENABLED, &qca->flags);
1444 qca_flush(hu);
1445 spin_unlock_irqrestore(&qca->hci_ibs_lock, flags);
1446
1447 host_set_baudrate(hu, 2400);
1448 qca_send_power_pulse(hu, false);
1449 qca_regulator_disable(qcadev);
1450 }
1451
1452 static int qca_power_off(struct hci_dev *hdev)
1453 {
1454 struct hci_uart *hu = hci_get_drvdata(hdev);
1455 struct qca_serdev *qcadev;
1456 enum qca_btsoc_type soc_type = qca_soc_type(hu);
1457
1458 if (qca_is_wcn399x(soc_type)) {
1459 /* Perform pre shutdown command */
1460 qca_send_pre_shutdown_cmd(hdev);
1461
1462 usleep_range(8000, 10000);
1463
1464 qca_power_shutdown(hu);
1465 } else {
1466 if (hu->serdev) {
1467 qcadev = serdev_device_get_drvdata(hu->serdev);
1468
1469 gpiod_set_value_cansleep(qcadev->bt_en, 0);
1470
1471 usleep_range(8000, 10000);
1472 }
1473 }
1474
1475 return 0;
1476 }
1477
1478 static int qca_send_btsoc_dump_cmd(struct hci_uart *hu)
1479 {
1480 int err = 0;
1481 struct sk_buff *skb = NULL;
1482 struct qca_data *qca = hu->priv;
1483
1484 BT_DBG("hu %p sending btsoc dump command", hu);
1485
1486 skb = bt_skb_alloc(1, GFP_ATOMIC);
1487 if (!skb) {
1488 BT_ERR("Failed to allocate memory for qca dump command");
1489 return -ENOMEM;
1490 }
1491
1492 skb_put_u8(skb, QCA_BTSOC_DUMP_CMD);
1493
1494 skb_queue_tail(&qca->txq, skb);
1495
1496 return err;
1497 }
1498
1499
1500 static void qca_cmd_timeout(struct hci_uart *hu)
1501 {
1502 struct qca_data *qca = hu->priv;
1503
> 1504 BT_ERR("hu %p hci cmd timeout count=0x%x", hu, ++qca->cmd_timeout_cnt);
1505
1506 if (qca->cmd_timeout_cnt >= QCA_MAX_CMD_TIMEOUT_COUNT)
1507 qca_send_btsoc_dump_cmd(hu);
1508 }
1509
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation
We will need to call the qca_power_off() API to power off Rome, add the
support into it. QCA Rome is using bt_en GPIO for power off, so we just
need to pull down this bt_en GPIO to power off it.
Signed-off-by: Rocky Liao <[email protected]>
---
Changes in v2: None
Changes in v3: None
drivers/bluetooth/hci_qca.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index b602ed01505b..43fd84028786 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1413,13 +1413,26 @@ static void qca_power_shutdown(struct hci_uart *hu)
static int qca_power_off(struct hci_dev *hdev)
{
struct hci_uart *hu = hci_get_drvdata(hdev);
+ struct qca_serdev *qcadev;
+ enum qca_btsoc_type soc_type = qca_soc_type(hu);
+
+ if (qca_is_wcn399x(soc_type)) {
+ /* Perform pre shutdown command */
+ qca_send_pre_shutdown_cmd(hdev);
+
+ usleep_range(8000, 10000);
- /* Perform pre shutdown command */
- qca_send_pre_shutdown_cmd(hdev);
+ qca_power_shutdown(hu);
+ } else {
+ if (hu->serdev) {
+ qcadev = serdev_device_get_drvdata(hu->serdev);
+
+ gpiod_set_value_cansleep(qcadev->bt_en, 0);
- usleep_range(8000, 10000);
+ usleep_range(8000, 10000);
+ }
+ }
- qca_power_shutdown(hu);
return 0;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
This patch registers hdev->shutdown() callback and also sets
HCI_QUIRK_NON_PERSISTENT_SETUP for QCA Rome. It will power-off the BT chip
during hci down and power-on/initialize the chip again during hci up.
Signed-off-by: Rocky Liao <[email protected]>
---
Changes in v2: None
Changes in v3:
- Move the quirk and callback register to probe()
drivers/bluetooth/hci_qca.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 45042aa27fa4..ca0b38f065e5 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1532,6 +1532,7 @@ static int qca_init_regulators(struct qca_power *qca,
static int qca_serdev_probe(struct serdev_device *serdev)
{
struct qca_serdev *qcadev;
+ struct hci_dev *hdev;
const struct qca_vreg_data *data;
int err;
@@ -1596,8 +1597,14 @@ static int qca_serdev_probe(struct serdev_device *serdev)
return err;
err = hci_uart_register_device(&qcadev->serdev_hu, &qca_proto);
- if (err)
+ if (err) {
clk_disable_unprepare(qcadev->susclk);
+ goto out;
+ }
+
+ hdev = qcadev->serdev_hu.hdev;
+ set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
+ hdev->shutdown = qca_power_off;
}
out: return err;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Current qca_power_shutdown() only supports wcn399x, this patch adds Rome
power off support to it. For Rome it just needs to pull down the bt_en
GPIO to power off it. This patch also replaces all the power off operation
in qca_close() with the unified qca_power_shutdown() call.
Signed-off-by: Rocky Liao <[email protected]>
---
Changes in v2: None
Changes in v3: None
Changes in v4:
-rebased the patch with latest code base
-moved the change from qca_power_off() to qca_power_shutdown()
-replaced all the power off operation in qca_close() with
qca_power_shutdown()
-updated commit message
drivers/bluetooth/hci_qca.c | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 992622dc1263..ecb74965be10 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -663,7 +663,6 @@ static int qca_flush(struct hci_uart *hu)
/* Close protocol */
static int qca_close(struct hci_uart *hu)
{
- struct qca_serdev *qcadev;
struct qca_data *qca = hu->priv;
BT_DBG("hu %p qca close", hu);
@@ -679,14 +678,7 @@ static int qca_close(struct hci_uart *hu)
destroy_workqueue(qca->workqueue);
qca->hu = NULL;
- if (hu->serdev) {
- qcadev = serdev_device_get_drvdata(hu->serdev);
- if (qca_is_wcn399x(qcadev->btsoc_type))
- qca_power_shutdown(hu);
- else
- gpiod_set_value_cansleep(qcadev->bt_en, 0);
-
- }
+ qca_power_shutdown(hu);
kfree_skb(qca->rx_skb);
@@ -1685,6 +1677,7 @@ static void qca_power_shutdown(struct hci_uart *hu)
struct qca_serdev *qcadev;
struct qca_data *qca = hu->priv;
unsigned long flags;
+ enum qca_btsoc_type soc_type = qca_soc_type(hu);
qcadev = serdev_device_get_drvdata(hu->serdev);
@@ -1697,11 +1690,22 @@ static void qca_power_shutdown(struct hci_uart *hu)
qca_flush(hu);
spin_unlock_irqrestore(&qca->hci_ibs_lock, flags);
- host_set_baudrate(hu, 2400);
- qca_send_power_pulse(hu, false);
- qca_regulator_disable(qcadev);
hu->hdev->hw_error = NULL;
hu->hdev->cmd_timeout = NULL;
+
+ /* Non-serdev device usually is powered by external power
+ * and don't need additional action in driver for power down
+ */
+ if (!hu->serdev)
+ return;
+
+ if (qca_is_wcn399x(soc_type)) {
+ host_set_baudrate(hu, 2400);
+ qca_send_power_pulse(hu, false);
+ qca_regulator_disable(qcadev);
+ } else {
+ gpiod_set_value_cansleep(qcadev->bt_en, 0);
+ }
}
static int qca_power_off(struct hci_dev *hdev)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
This patch registers hdev->shutdown() callback and also sets
HCI_QUIRK_NON_PERSISTENT_SETUP for QCA Rome. It will power-off the BT chip
during hci down and power-on/initialize the chip again during hci up. As
wcn399x already enabled this, this patch also removed the callback register
and QUIRK setting in qca_setup() for wcn399x and uniformly do this in the
probe() routine.
Signed-off-by: Rocky Liao <[email protected]>
---
Changes in v2: None
Changes in v3:
-moved the quirk and callback register to probe()
Changes in v4:
-rebased the patch with latest code
-moved the quirk and callback register to probe() for wcn399x
-updated commit message
drivers/bluetooth/hci_qca.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 1139142e8eed..3c6c6bd20177 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1569,12 +1569,7 @@ static int qca_setup(struct hci_uart *hu)
return ret;
if (qca_is_wcn399x(soc_type)) {
- /* Enable NON_PERSISTENT_SETUP QUIRK to ensure to execute
- * setup for every hci up.
- */
- set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
- hu->hdev->shutdown = qca_power_off;
ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
if (ret)
@@ -1813,6 +1808,7 @@ static int qca_init_regulators(struct qca_power *qca,
static int qca_serdev_probe(struct serdev_device *serdev)
{
struct qca_serdev *qcadev;
+ struct hci_dev *hdev;
const struct qca_vreg_data *data;
int err;
@@ -1881,7 +1877,13 @@ static int qca_serdev_probe(struct serdev_device *serdev)
clk_disable_unprepare(qcadev->susclk);
}
-out: return err;
+out:
+ if (!err) {
+ hdev = qcadev->serdev_hu.hdev;
+ set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
+ hdev->shutdown = qca_power_off;
+ }
+ return err;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
This patch adds the retry of btsoc initialization when it fails. There are
reports that the btsoc initialization may fail on some platforms but the
repro ratio is very low. The symptoms is the firmware downloading failed
due to the UART write timed out. The failure may be caused by UART,
platform HW or the btsoc itself but it's very difficlut to root cause,
given the repro ratio is very low. Add a retry for the btsoc initialization
can work around most of the failures and make Bluetooth finally works.
Signed-off-by: Rocky Liao <[email protected]>
---
Changes in v2: None
Changes in v3: None
Changes in v4:
-rebased the patch with latet code
-refined macro and variable name
-updated commit message
drivers/bluetooth/hci_qca.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index ecb74965be10..1139142e8eed 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -55,6 +55,9 @@
/* Controller debug log header */
#define QCA_DEBUG_HANDLE 0x2EDC
+/* max retry count when init fails */
+#define MAX_INIT_RETRIES 3
+
/* Controller dump header */
#define QCA_SSR_DUMP_HANDLE 0x0108
#define QCA_DUMP_PACKET_SIZE 255
@@ -1539,6 +1542,7 @@ static int qca_setup(struct hci_uart *hu)
struct hci_dev *hdev = hu->hdev;
struct qca_data *qca = hu->priv;
unsigned int speed, qca_baudrate = QCA_BAUDRATE_115200;
+ unsigned int retries = 0;
enum qca_btsoc_type soc_type = qca_soc_type(hu);
const char *firmware_name = qca_get_firmware_name(hu);
int ret;
@@ -1559,6 +1563,7 @@ static int qca_setup(struct hci_uart *hu)
bt_dev_info(hdev, "setting up %s",
qca_is_wcn399x(soc_type) ? "wcn399x" : "ROME");
+retry:
ret = qca_power_on(hdev);
if (ret)
return ret;
@@ -1613,6 +1618,20 @@ static int qca_setup(struct hci_uart *hu)
* patch/nvm-config is found, so run with original fw/config.
*/
ret = 0;
+ } else {
+ if (retries < MAX_INIT_RETRIES) {
+ qca_power_shutdown(hu);
+ if (hu->serdev) {
+ serdev_device_close(hu->serdev);
+ ret = serdev_device_open(hu->serdev);
+ if (ret) {
+ bt_dev_err(hdev, "failed to open port");
+ return ret;
+ }
+ }
+ retries++;
+ goto retry;
+ }
}
/* Setup bdaddr */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
On Wed, Jan 15, 2020 at 04:55:50PM +0800, Rocky Liao wrote:
> Current qca_power_shutdown() only supports wcn399x, this patch adds Rome
> power off support to it. For Rome it just needs to pull down the bt_en
> GPIO to power off it. This patch also replaces all the power off operation
> in qca_close() with the unified qca_power_shutdown() call.
>
> Signed-off-by: Rocky Liao <[email protected]>
> ---
>
> Changes in v2: None
> Changes in v3: None
> Changes in v4:
> -rebased the patch with latest code base
> -moved the change from qca_power_off() to qca_power_shutdown()
> -replaced all the power off operation in qca_close() with
> qca_power_shutdown()
> -updated commit message
>
> drivers/bluetooth/hci_qca.c | 28 ++++++++++++++++------------
> 1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 992622dc1263..ecb74965be10 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -663,7 +663,6 @@ static int qca_flush(struct hci_uart *hu)
> /* Close protocol */
> static int qca_close(struct hci_uart *hu)
> {
> - struct qca_serdev *qcadev;
> struct qca_data *qca = hu->priv;
>
> BT_DBG("hu %p qca close", hu);
> @@ -679,14 +678,7 @@ static int qca_close(struct hci_uart *hu)
> destroy_workqueue(qca->workqueue);
> qca->hu = NULL;
>
> - if (hu->serdev) {
> - qcadev = serdev_device_get_drvdata(hu->serdev);
> - if (qca_is_wcn399x(qcadev->btsoc_type))
> - qca_power_shutdown(hu);
> - else
> - gpiod_set_value_cansleep(qcadev->bt_en, 0);
> -
> - }
> + qca_power_shutdown(hu);
>
> kfree_skb(qca->rx_skb);
>
> @@ -1685,6 +1677,7 @@ static void qca_power_shutdown(struct hci_uart *hu)
> struct qca_serdev *qcadev;
> struct qca_data *qca = hu->priv;
> unsigned long flags;
> + enum qca_btsoc_type soc_type = qca_soc_type(hu);
>
> qcadev = serdev_device_get_drvdata(hu->serdev);
>
> @@ -1697,11 +1690,22 @@ static void qca_power_shutdown(struct hci_uart *hu)
> qca_flush(hu);
> spin_unlock_irqrestore(&qca->hci_ibs_lock, flags);
>
> - host_set_baudrate(hu, 2400);
> - qca_send_power_pulse(hu, false);
> - qca_regulator_disable(qcadev);
> hu->hdev->hw_error = NULL;
> hu->hdev->cmd_timeout = NULL;
This is now done before the power off and not after, I suppose it doesn't
make a difference.
> +
> + /* Non-serdev device usually is powered by external power
> + * and don't need additional action in driver for power down
> + */
> + if (!hu->serdev)
> + return;
> +
> + if (qca_is_wcn399x(soc_type)) {
> + host_set_baudrate(hu, 2400);
> + qca_send_power_pulse(hu, false);
> + qca_regulator_disable(qcadev);
> + } else {
> + gpiod_set_value_cansleep(qcadev->bt_en, 0);
> + }
> }
Reviewed-by: Matthias Kaehlcke <[email protected]>
On Wed, Jan 15, 2020 at 04:55:51PM +0800, Rocky Liao wrote:
> This patch adds the retry of btsoc initialization when it fails. There are
> reports that the btsoc initialization may fail on some platforms but the
> repro ratio is very low. The symptoms is the firmware downloading failed
> due to the UART write timed out. The failure may be caused by UART,
> platform HW or the btsoc itself but it's very difficlut to root cause,
> given the repro ratio is very low. Add a retry for the btsoc initialization
> can work around most of the failures and make Bluetooth finally works.
>
> Signed-off-by: Rocky Liao <[email protected]>
> ---
>
> Changes in v2: None
> Changes in v3: None
> Changes in v4:
> -rebased the patch with latet code
> -refined macro and variable name
> -updated commit message
>
> drivers/bluetooth/hci_qca.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index ecb74965be10..1139142e8eed 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -55,6 +55,9 @@
> /* Controller debug log header */
> #define QCA_DEBUG_HANDLE 0x2EDC
>
> +/* max retry count when init fails */
> +#define MAX_INIT_RETRIES 3
> +
> /* Controller dump header */
> #define QCA_SSR_DUMP_HANDLE 0x0108
> #define QCA_DUMP_PACKET_SIZE 255
> @@ -1539,6 +1542,7 @@ static int qca_setup(struct hci_uart *hu)
> struct hci_dev *hdev = hu->hdev;
> struct qca_data *qca = hu->priv;
> unsigned int speed, qca_baudrate = QCA_BAUDRATE_115200;
> + unsigned int retries = 0;
> enum qca_btsoc_type soc_type = qca_soc_type(hu);
> const char *firmware_name = qca_get_firmware_name(hu);
> int ret;
> @@ -1559,6 +1563,7 @@ static int qca_setup(struct hci_uart *hu)
> bt_dev_info(hdev, "setting up %s",
> qca_is_wcn399x(soc_type) ? "wcn399x" : "ROME");
>
> +retry:
> ret = qca_power_on(hdev);
> if (ret)
> return ret;
> @@ -1613,6 +1618,20 @@ static int qca_setup(struct hci_uart *hu)
> * patch/nvm-config is found, so run with original fw/config.
> */
> ret = 0;
> + } else {
> + if (retries < MAX_INIT_RETRIES) {
> + qca_power_shutdown(hu);
> + if (hu->serdev) {
> + serdev_device_close(hu->serdev);
> + ret = serdev_device_open(hu->serdev);
> + if (ret) {
> + bt_dev_err(hdev, "failed to open port");
> + return ret;
> + }
> + }
> + retries++;
> + goto retry;
> + }
> }
>
> /* Setup bdaddr */
> --
Assuming that this is really a rare condition:
Reviewed-by: Matthias Kaehlcke <[email protected]>
On Wed, Jan 15, 2020 at 04:55:52PM +0800, Rocky Liao wrote:
> This patch registers hdev->shutdown() callback and also sets
> HCI_QUIRK_NON_PERSISTENT_SETUP for QCA Rome. It will power-off the BT chip
> during hci down and power-on/initialize the chip again during hci up. As
> wcn399x already enabled this, this patch also removed the callback register
> and QUIRK setting in qca_setup() for wcn399x and uniformly do this in the
> probe() routine.
>
> Signed-off-by: Rocky Liao <[email protected]>
> ---
>
> Changes in v2: None
> Changes in v3:
> -moved the quirk and callback register to probe()
> Changes in v4:
> -rebased the patch with latest code
> -moved the quirk and callback register to probe() for wcn399x
> -updated commit message
>
> drivers/bluetooth/hci_qca.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 1139142e8eed..3c6c6bd20177 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -1569,12 +1569,7 @@ static int qca_setup(struct hci_uart *hu)
> return ret;
>
> if (qca_is_wcn399x(soc_type)) {
> - /* Enable NON_PERSISTENT_SETUP QUIRK to ensure to execute
> - * setup for every hci up.
> - */
> - set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
> set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
I guess this should also move to _probe() eventually, but it's not really
the scope of this patch.
> - hu->hdev->shutdown = qca_power_off;
>
> ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
> if (ret)
> @@ -1813,6 +1808,7 @@ static int qca_init_regulators(struct qca_power *qca,
> static int qca_serdev_probe(struct serdev_device *serdev)
> {
> struct qca_serdev *qcadev;
> + struct hci_dev *hdev;
> const struct qca_vreg_data *data;
> int err;
>
> @@ -1881,7 +1877,13 @@ static int qca_serdev_probe(struct serdev_device *serdev)
> clk_disable_unprepare(qcadev->susclk);
> }
>
> -out: return err;
> +out:
> + if (!err) {
> + hdev = qcadev->serdev_hu.hdev;
> + set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
> + hdev->shutdown = qca_power_off;
> + }
> + return err;
Since there is no unwinding in case of an error I would suggest to
change the jumps to the 'out' label further above to 'return err;'
and change the above lines to:
hdev = qcadev->serdev_hu.hdev;
set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
hdev->shutdown = qca_power_off;
return 0;
This will also require to return directly when hci_uart_register_device()
fails.
Hi Rocky,
> Current qca_power_shutdown() only supports wcn399x, this patch adds Rome
> power off support to it. For Rome it just needs to pull down the bt_en
> GPIO to power off it. This patch also replaces all the power off operation
> in qca_close() with the unified qca_power_shutdown() call.
>
> Signed-off-by: Rocky Liao <[email protected]>
> ---
>
> Changes in v2: None
> Changes in v3: None
> Changes in v4:
> -rebased the patch with latest code base
> -moved the change from qca_power_off() to qca_power_shutdown()
> -replaced all the power off operation in qca_close() with
> qca_power_shutdown()
> -updated commit message
>
> drivers/bluetooth/hci_qca.c | 28 ++++++++++++++++------------
> 1 file changed, 16 insertions(+), 12 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
This patch registers hdev->shutdown() callback and also sets
HCI_QUIRK_NON_PERSISTENT_SETUP for QCA Rome. It will power-off the BT chip
during hci down and power-on/initialize the chip again during hci up. As
wcn399x already enabled this, this patch also removed the callback register
and QUIRK setting in qca_setup() for wcn399x and uniformly do this in the
probe() routine.
Signed-off-by: Rocky Liao <[email protected]>
---
Changes in v2: None
Changes in v3:
-moved the quirk and callback register to probe()
Changes in v4:
-rebased the patch with latest code
-moved the quirk and callback register to probe() for wcn399x
-updated commit message
Changed in v5:
-removed the "out" label and return err when fails
drivers/bluetooth/hci_qca.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 1139142e8eed..d6e0c99ee5eb 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1569,12 +1569,7 @@ static int qca_setup(struct hci_uart *hu)
return ret;
if (qca_is_wcn399x(soc_type)) {
- /* Enable NON_PERSISTENT_SETUP QUIRK to ensure to execute
- * setup for every hci up.
- */
- set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
- hu->hdev->shutdown = qca_power_off;
ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
if (ret)
@@ -1813,6 +1808,7 @@ static int qca_init_regulators(struct qca_power *qca,
static int qca_serdev_probe(struct serdev_device *serdev)
{
struct qca_serdev *qcadev;
+ struct hci_dev *hdev;
const struct qca_vreg_data *data;
int err;
@@ -1838,7 +1834,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
data->num_vregs);
if (err) {
BT_ERR("Failed to init regulators:%d", err);
- goto out;
+ return err;
}
qcadev->bt_power->vregs_on = false;
@@ -1851,7 +1847,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
err = hci_uart_register_device(&qcadev->serdev_hu, &qca_proto);
if (err) {
BT_ERR("wcn3990 serdev registration failed");
- goto out;
+ return err;
}
} else {
qcadev->btsoc_type = QCA_ROME;
@@ -1877,12 +1873,18 @@ static int qca_serdev_probe(struct serdev_device *serdev)
return err;
err = hci_uart_register_device(&qcadev->serdev_hu, &qca_proto);
- if (err)
+ if (err) {
+ BT_ERR("Rome serdev registration failed");
clk_disable_unprepare(qcadev->susclk);
+ return err;
+ }
}
-out: return err;
+ hdev = qcadev->serdev_hu.hdev;
+ set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
+ hdev->shutdown = qca_power_off;
+ return 0;
}
static void qca_serdev_remove(struct serdev_device *serdev)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Rocky,
> This patch registers hdev->shutdown() callback and also sets
> HCI_QUIRK_NON_PERSISTENT_SETUP for QCA Rome. It will power-off the BT chip
> during hci down and power-on/initialize the chip again during hci up. As
> wcn399x already enabled this, this patch also removed the callback register
> and QUIRK setting in qca_setup() for wcn399x and uniformly do this in the
> probe() routine.
>
> Signed-off-by: Rocky Liao <[email protected]>
> ---
>
> Changes in v2: None
> Changes in v3:
> -moved the quirk and callback register to probe()
> Changes in v4:
> -rebased the patch with latest code
> -moved the quirk and callback register to probe() for wcn399x
> -updated commit message
> Changed in v5:
> -removed the "out" label and return err when fails
>
> drivers/bluetooth/hci_qca.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel