Cold on & off support for Qualcomm BT chip wcn3990
v2 cover Cold on & off support for Qualcomm BT chip wcn3990
This patch will power off regulators while hci0 down and turn on them back on hci0 up. Every time we power off we call proto specific close function, so that we will free the memory of Qualcomm BT specific buffers. Will call proto open to assign the memory and turn on the regulators, before calling qualcomm setup.
v2:
* Removed vendor specific hdev->open and close functions.
* Added hdev->shutdown function to turn off the regs.
v1:
* initial patch.
Balakrishna Godavarthi (1):
Bluetooth: hci_qca: Add poweroff support during hci down for wcn3990
drivers/bluetooth/hci_qca.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v2 1/1 Bluetooth: hci_qca: Add poweroff support during hci down for wcn3990
This patch enables power off support for hci down and power on support for hci up. As wcn3990 power sources are ignited by regulators, we will turn off them during hci down, i.e. an complete power off of wcn3990. So while hci up, we will call vendor specific open/close and setup which will turn on the regulators, requests BT chip version and download the firmware.
--- drivers/bluetooth/hci_qca.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
drivers/bluetooth/hci_qca.c +33 -0
@@ -168,6 +168,7 @@ struct qca_serdev {static int qca_power_setup (struct hci_uart *hu,bool on);static void qca_power_shutdown (struct hci_uart *hu); +static int qca_power_off (struct hci_dev *hdev);static void __serial_clock_on (struct tty_struct *tty) {
@@ -1099,8 +1100,26 @@ static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type)static int qca_wcn3990_init (struct hci_uart *hu) {struct hci_dev *hdev = hu->hdev; +struct qca_serdev *qcadev;int ret; +/* Check for vregs status, may be hci0 down has turned +* off the vregs. +*/ + qcadev =serdev_device_get_drvdata (hu->serdev); +if (qcadev->bt_power->vregs_on == false) { +serdev_device_close (hu->serdev); + ret =qca_power_setup (hu, true); +if (ret) +return ret; + + ret =serdev_device_open (hu->serdev); +if (ret) { +bt_dev_err (hu->hdev,"failed to open port" ); +return ret; + } + } +/* Forcefully enable wcn3990 to enter in to boot mode. */ host_set_baudrate (hu,2400 ); ret =qca_send_power_pulse (hdev,QCA_WCN3990_POWEROFF_PULSE );
@@ -1152,6 +1171,12 @@ static int qca_setup(struct hci_uart *hu)if (qcadev->btsoc_type ==QCA_WCN3990 ) {bt_dev_info (hdev,"setting up wcn3990" ); + +/* Enable NON_PERSISTENT_SETUP QUIRK to ensure to execute +* setup for every hci0 up. +*/ +set_bit (HCI_QUIRK_NON_PERSISTENT_SETUP , &hdev->quirks); + hu->hdev->shutdown = qca_power_off; ret =qca_wcn3990_init (hu);if (ret)return ret;
@@ -1242,6 +1267,14 @@ static void qca_power_shutdown(struct hci_uart *hu)qca_power_setup (hu, false); } +static int qca_power_off (struct hci_dev *hdev) +{ +struct hci_uart *hu =hci_get_drvdata (hdev); + +qca_power_shutdown (hu); +return 0 ; +} +static int qca_enable_regulator (struct qca_vreg vregs,struct regulator *regulator) {
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2 1/1] Bluetooth: hci_qca: Add poweroff support during hci down for wcn3990
Quoted 25 lines · Balakrishna Godavarthi
This patch enables power off support for hci down and power on support for hci up. As wcn3990 power sources are ignited by regulators, we will turn off them during hci down, i.e. an complete power off of wcn3990. So while hci up, we will call vendor specific open/close and setup which will turn on the regulators, requests BT chip version and download the firmware.
Signed-off-by Balakrishna Godavarthi <[email protected]>--- drivers/bluetooth/hci_qca.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)drivers/bluetooth/hci_qca.c +1
-0@@ -168,6 +168,7 @@ struct qca_serdev {static int qca_power_setup (struct hci_uart *hu,bool on);static void qca_power_shutdown (struct hci_uart *hu); +static int qca_power_off (struct hci_dev *hdev);static void __serial_clock_on (struct tty_struct *tty) {
drivers/bluetooth/hci_qca.c static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type) · line 1108
Quoted 9 lines
drivers/bluetooth/hci_qca.c +4
-0@@ -1099,8 +1100,26 @@ static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type)static int qca_wcn3990_init (struct hci_uart *hu) {struct hci_dev *hdev = hu->hdev; +struct qca_serdev *qcadev;int ret; +/* Check for vregs status, may be hci0 down has turned +* off the vregs. +*/
nit: it's not necessarily 'hci0', there may be systems with more than one Bluetooth controller. You could say hciN instead, you might also want to put quotes around 'hciN down' to make clearer this is referring to a command.
drivers/bluetooth/hci_qca.c static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type) · line 1110
Quoted 2 lines
drivers/bluetooth/hci_qca.c +2
-0+ qcadev =serdev_device_get_drvdata (hu->serdev); +if (qcadev->bt_power->vregs_on == false) {
nit:
if (!qcadev->bt_power->vregs_on) {is more common and easier to read IMO
drivers/bluetooth/hci_qca.c static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type) · line 1118
Quoted 8 lines
drivers/bluetooth/hci_qca.c +8
-0+serdev_device_close (hu->serdev); + ret =qca_power_setup (hu, true); +if (ret) +return ret; + + ret =serdev_device_open (hu->serdev); +if (ret) { +bt_dev_err (hu->hdev,"failed to open port" );
switch the regulators off again?
Quoted 8 lines
drivers/bluetooth/hci_qca.c +4
-0+return ret; + } + } +/* Forcefully enable wcn3990 to enter in to boot mode. */ host_set_baudrate (hu,2400 ); ret =qca_send_power_pulse (hdev,QCA_WCN3990_POWEROFF_PULSE );
drivers/bluetooth/hci_qca.c static int qca_setup(struct hci_uart *hu) · line 1176
Quoted 6 lines
drivers/bluetooth/hci_qca.c +3
-0@@ -1152,6 +1171,12 @@ static int qca_setup(struct hci_uart *hu)if (qcadev->btsoc_type ==QCA_WCN3990 ) {bt_dev_info (hdev,"setting up wcn3990" ); + +/* Enable NON_PERSISTENT_SETUP QUIRK to ensure to execute +* setup for every hci0 up.
nit: 'hciN up'?
Quoted 7 lines
drivers/bluetooth/hci_qca.c +3
-0+*/ +set_bit (HCI_QUIRK_NON_PERSISTENT_SETUP , &hdev->quirks); + hu->hdev->shutdown = qca_power_off; ret =qca_wcn3990_init (hu);if (ret)return ret;
drivers/bluetooth/hci_qca.c static void qca_power_shutdown(struct hci_uart *hu) · line 1274
Quoted 8 lines
drivers/bluetooth/hci_qca.c +5
-0@@ -1242,6 +1267,14 @@ static void qca_power_shutdown(struct hci_uart *hu)qca_power_setup (hu, false); } +static int qca_power_off (struct hci_dev *hdev) +{ +struct hci_uart *hu =hci_get_drvdata (hdev); + +qca_power_shutdown (hu);
Would it make sense to add a return value qca_power_shutdown() now that it is called by a non-void function? Might not be worth the hassle though, hci_dev_do_close() - the only caller of hdev->shutdown() - doesn't check the return value either.
Cheers
Matthias
Re: [PATCH v2 1/1] Bluetooth: hci_qca: Add poweroff support during hci down for wcn3990
Hi Matthias,
Quoted 33 lines · Matthias Kaehlcke
Quoted 32 lines · Balakrishna Godavarthi
This patch enables power off support for hci down and power on support for hci up. As wcn3990 power sources are ignited by regulators, we will turn off them during hci down, i.e. an complete power off of wcn3990. So while hci up, we will call vendor specific open/close and setup which will turn on the regulators, requests BT chip version and download the firmware.
Signed-off-by Balakrishna Godavarthi <[email protected]>--- drivers/bluetooth/hci_qca.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)drivers/bluetooth/hci_qca.c +2
-0@@ -168,6 +168,7 @@ struct qca_serdev {static int qca_power_setup (struct hci_uart *hu,bool on);static void qca_power_shutdown (struct hci_uart *hu); +static int qca_power_off (struct hci_dev *hdev);static void __serial_clock_on (struct tty_struct *tty) {@@ -1099,8 +1100,26 @@ static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type)static int qca_wcn3990_init (struct hci_uart *hu) {struct hci_dev *hdev = hu->hdev; +struct qca_serdev *qcadev;int ret;
Quoted 9 lines
drivers/bluetooth/hci_qca.c static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type) · line 1108
Quoted 3 lines
drivers/bluetooth/hci_qca.c +3
-0+/* Check for vregs status, may be hci0 down has turned +* off the vregs. +*/ nit: it's not necessarily 'hci0', there may be systems with more than one Bluetooth controller. You could say hciN instead, you might also want to put quotes around 'hciN down' to make clearer this is referring to a command.
[Bala]: will update.
Quoted 8 lines
drivers/bluetooth/hci_qca.c static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type) · line 1110
Quoted 2 lines
drivers/bluetooth/hci_qca.c +2
-0+ qcadev =serdev_device_get_drvdata (hu->serdev); +if (qcadev->bt_power->vregs_on == false) {nit:
if (!qcadev->bt_power->vregs_on) {is more common and easier to read IMO
[Bala]: will update.
Quoted 10 lines
drivers/bluetooth/hci_qca.c static int qca_set_speed(struct hci_uart *hu, enum qca_speed_type speed_type) · line 1118
Quoted 8 lines
drivers/bluetooth/hci_qca.c +8
-0+serdev_device_close (hu->serdev); + ret =qca_power_setup (hu, true); +if (ret) +return ret; + + ret =serdev_device_open (hu->serdev); +if (ret) { +bt_dev_err (hu->hdev,"failed to open port" );switch the regulators off again?
[Bala]: it is not required to call the regulators off, when the serdev open fails.
as the non zero return status will call the hci_dev_do_close()
i.e. hdev->shutdown()
Quoted 10 lines
Quoted 10 lines
drivers/bluetooth/hci_qca.c +4
-0+return ret; + } + } +/* Forcefully enable wcn3990 to enter in to boot mode. */ host_set_baudrate (hu,2400 );drivers/bluetooth/hci_qca.c +0
-0ret =qca_send_power_pulse (hdev,QCA_WCN3990_POWEROFF_PULSE );@@ -1152,6 +1171,12 @@ static int qca_setup(struct hci_uart *hu)
Quoted 7 lines
drivers/bluetooth/hci_qca.c static int qca_setup(struct hci_uart *hu) · line 1176
Quoted 5 lines
drivers/bluetooth/hci_qca.c +3
-0if (qcadev->btsoc_type ==QCA_WCN3990 ) {bt_dev_info (hdev,"setting up wcn3990" ); + +/* Enable NON_PERSISTENT_SETUP QUIRK to ensure to execute +* setup for every hci0 up. nit: 'hciN up'?
[Bala]: will update.
Quoted 12 lines
Quoted 12 lines
drivers/bluetooth/hci_qca.c +3
-0+*/ +set_bit (HCI_QUIRK_NON_PERSISTENT_SETUP , &hdev->quirks); + hu->hdev->shutdown = qca_power_off; ret =qca_wcn3990_init (hu);if (ret)return ret;@@ -1242,6 +1267,14 @@ static void qca_power_shutdown(struct hci_uart *hu)qca_power_setup (hu, false); }
Quoted 11 lines
drivers/bluetooth/hci_qca.c static void qca_power_shutdown(struct hci_uart *hu) · line 1274
Quoted 5 lines
drivers/bluetooth/hci_qca.c +5
-0+static int qca_power_off (struct hci_dev *hdev) +{ +struct hci_uart *hu =hci_get_drvdata (hdev); + +qca_power_shutdown (hu);Would it make sense to add a return value
qca_power_shutdown()now that it is called by a non-void function? Might not be worth the hassle though,hci_dev_do_close()- the only caller ofhdev->shutdown()- doesn't check the return value either.
[Bala]: even i felt the same, but hci_dev_do_close() is not checking for the status.
Quoted 3 lines
Cheers
Matthias
Signature · 2 lines
Regards Balakrishna.
nit: Not sure there is much value in a cover letter for a 'series' with a single patch. The changelog and additional comments (if needed) can be placed in the post with the patch below '---'.
Quoted 19 lines · Balakrishna Godavarthi
This patch will power off regulators while hci0 down and turn on them back on hci0 up. Every time we power off we call proto specific close function, so that we will free the memory of Qualcomm BT specific buffers. Will call proto open to assign the memory and turn on the regulators, before calling qualcomm setup.
v2:
* Removed vendor specifichdev->openand close functions.
* Addedhdev->shutdownfunction to turn off the regs.v1:
* initial patch.Balakrishna Godavarthi (1):
Bluetooth:hci_qca: Add poweroff support during hci down for wcn3990drivers/bluetooth/hci_qca.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)