This patch adds a unified API qca_power_on() to support both wcn399x and
Rome power on. For wcn399x it calls the qca_wcn3990_init() to init the
regulators, and for Rome it pulls up the bt_en GPIO to power up the btsoc.
Signed-off-by: Rocky Liao <[email protected]>
---
drivers/bluetooth/hci_qca.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 9392cc7f9908..f6555bd1adbc 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1532,6 +1532,27 @@ static int qca_wcn3990_init(struct hci_uart *hu)
return 0;
}
+static int qca_power_on(struct hci_dev *hdev)
+{
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+ enum qca_btsoc_type soc_type = qca_soc_type(hu);
+ struct qca_serdev *qcadev;
+ int ret = 0;
+
+ if (qca_is_wcn399x(soc_type)) {
+ ret = qca_wcn3990_init(hu);
+ } else {
+ 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);
+ }
+ }
+
+ return ret;
+}
+
static int qca_setup(struct hci_uart *hu)
{
struct hci_dev *hdev = hu->hdev;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Rocky,
On Tue, Jan 07, 2020 at 01:26:01PM +0800, Rocky Liao wrote:
> This patch adds a unified API qca_power_on() to support both wcn399x and
> Rome power on. For wcn399x it calls the qca_wcn3990_init() to init the
> regulators, and for Rome it pulls up the bt_en GPIO to power up the btsoc.
>
> Signed-off-by: Rocky Liao <[email protected]>
> ---
> drivers/bluetooth/hci_qca.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 9392cc7f9908..f6555bd1adbc 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -1532,6 +1532,27 @@ static int qca_wcn3990_init(struct hci_uart *hu)
> return 0;
> }
>
> +static int qca_power_on(struct hci_dev *hdev)
> +{
> + struct hci_uart *hu = hci_get_drvdata(hdev);
> + enum qca_btsoc_type soc_type = qca_soc_type(hu);
> + struct qca_serdev *qcadev;
> + int ret = 0;
another option would be to return directly from the if/else branches,
but either way is fine.
> +
> + if (qca_is_wcn399x(soc_type)) {
> + ret = qca_wcn3990_init(hu);
> + } else {
> + 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);
> + }
> + }
> +
> + return ret;
> +}
> +
I expected qca_power_on() would be called from qca_open(), but as is
this would only work for ROME, and not WCN399x, which only enables
the regulators in qca_open(), qca_wcn3990_init() is called from
qca_setup(). Is there a particular reason for this assymmetry between
the ROME and WCN399x initialization (i.e. one is fully powered up after
open(), the other not)?
Hi Matt,
在 2020-01-08 01:31，Matthias Kaehlcke 写道：
> Hi Rocky,
>
> On Tue, Jan 07, 2020 at 01:26:01PM +0800, Rocky Liao wrote:
>> This patch adds a unified API qca_power_on() to support both wcn399x
>> and
>> Rome power on. For wcn399x it calls the qca_wcn3990_init() to init the
>> regulators, and for Rome it pulls up the bt_en GPIO to power up the
>> btsoc.
>>
>> Signed-off-by: Rocky Liao <[email protected]>
>> ---
>> drivers/bluetooth/hci_qca.c | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>>
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index 9392cc7f9908..f6555bd1adbc 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -1532,6 +1532,27 @@ static int qca_wcn3990_init(struct hci_uart
>> *hu)
>> return 0;
>> }
>>
>> +static int qca_power_on(struct hci_dev *hdev)
>> +{
>> + struct hci_uart *hu = hci_get_drvdata(hdev);
>> + enum qca_btsoc_type soc_type = qca_soc_type(hu);
>> + struct qca_serdev *qcadev;
>> + int ret = 0;
>
> another option would be to return directly from the if/else branches,
> but either way is fine.
>
>> +
>> + if (qca_is_wcn399x(soc_type)) {
>> + ret = qca_wcn3990_init(hu);
>> + } else {
>> + 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);
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>
> I expected qca_power_on() would be called from qca_open(), but as is
> this would only work for ROME, and not WCN399x, which only enables
> the regulators in qca_open(), qca_wcn3990_init() is called from
> qca_setup(). Is there a particular reason for this assymmetry between
> the ROME and WCN399x initialization (i.e. one is fully powered up after
> open(), the other not)?
I prefer to move the power on call from qca_open() to qca_setup() for
Rome,
I don't see any reason for this difference with wcn399x.I will send
patch for
this if you have no concern.
This patch adds a unified API qca_power_on() to support both wcn399x and
Rome power on. For wcn399x it calls the qca_wcn3990_init() to init the
regulators, and for Rome it pulls up the bt_en GPIO to power up the btsoc.
Signed-off-by: Rocky Liao <[email protected]>
---
Changes in v2: None
drivers/bluetooth/hci_qca.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 9392cc7f9908..f6555bd1adbc 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1532,6 +1532,27 @@ static int qca_wcn3990_init(struct hci_uart *hu)
return 0;
}
+static int qca_power_on(struct hci_dev *hdev)
+{
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+ enum qca_btsoc_type soc_type = qca_soc_type(hu);
+ struct qca_serdev *qcadev;
+ int ret = 0;
+
+ if (qca_is_wcn399x(soc_type)) {
+ ret = qca_wcn3990_init(hu);
+ } else {
+ 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);
+ }
+ }
+
+ return ret;
+}
+
static int qca_setup(struct hci_uart *hu)
{
struct hci_dev *hdev = hu->hdev;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Rocky,
On Wed, Jan 08, 2020 at 05:08:02PM +0800, Rocky Liao wrote:
> This patch adds a unified API qca_power_on() to support both wcn399x and
> Rome power on. For wcn399x it calls the qca_wcn3990_init() to init the
> regulators, and for Rome it pulls up the bt_en GPIO to power up the btsoc.
>
> Signed-off-by: Rocky Liao <[email protected]>
> ---
>
> Changes in v2: None
>
> drivers/bluetooth/hci_qca.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 9392cc7f9908..f6555bd1adbc 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -1532,6 +1532,27 @@ static int qca_wcn3990_init(struct hci_uart *hu)
> return 0;
> }
>
> +static int qca_power_on(struct hci_dev *hdev)
> +{
> + struct hci_uart *hu = hci_get_drvdata(hdev);
> + enum qca_btsoc_type soc_type = qca_soc_type(hu);
> + struct qca_serdev *qcadev;
> + int ret = 0;
> +
> + if (qca_is_wcn399x(soc_type)) {
Why not include the qca_regulator_enable() call from qca_open() here?
It is clearly part of power on.
> + ret = qca_wcn3990_init(hu);
> + } else {
> + if (hu->serdev) {
nit: you could save a level of indentation (and IMO improve
readability) by doing:
if (!hu->serdev)
return 0;
> + qcadev = serdev_device_get_drvdata(hu->serdev);
> + gpiod_set_value_cansleep(qcadev->bt_en, 1);
> + /* Controller needs time to bootup. */
> + msleep(150);
> + }
> + }
> +
> + return ret;
> +}
> +
I think common practice would be to combine the 3 patches of this series
into one. The new function doesn't really add any new functionality, but
is a refactoring. This is more evident if you see in a single diff that
the pieces in qca_power_on() are removed elsewhere.
Hi Matt,
在 2020-01-09 02:34，Matthias Kaehlcke 写道：
> Hi Rocky,
>
> On Wed, Jan 08, 2020 at 05:08:02PM +0800, Rocky Liao wrote:
>> This patch adds a unified API qca_power_on() to support both wcn399x
>> and
>> Rome power on. For wcn399x it calls the qca_wcn3990_init() to init the
>> regulators, and for Rome it pulls up the bt_en GPIO to power up the
>> btsoc.
>>
>> Signed-off-by: Rocky Liao <[email protected]>
>> ---
>>
>> Changes in v2: None
>>
>> drivers/bluetooth/hci_qca.c | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>>
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index 9392cc7f9908..f6555bd1adbc 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -1532,6 +1532,27 @@ static int qca_wcn3990_init(struct hci_uart
>> *hu)
>> return 0;
>> }
>>
>> +static int qca_power_on(struct hci_dev *hdev)
>> +{
>> + struct hci_uart *hu = hci_get_drvdata(hdev);
>> + enum qca_btsoc_type soc_type = qca_soc_type(hu);
>> + struct qca_serdev *qcadev;
>> + int ret = 0;
>> +
>> + if (qca_is_wcn399x(soc_type)) {
>
> Why not include the qca_regulator_enable() call from qca_open() here?
> It is clearly part of power on.
>
OK
>> + ret = qca_wcn3990_init(hu);
>> + } else {
>> + if (hu->serdev) {
>
> nit: you could save a level of indentation (and IMO improve
> readability) by doing:
>
> if (!hu->serdev)
> return 0;
>
OK
>> + qcadev = serdev_device_get_drvdata(hu->serdev);
>> + gpiod_set_value_cansleep(qcadev->bt_en, 1);
>> + /* Controller needs time to bootup. */
>> + msleep(150);
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>
> I think common practice would be to combine the 3 patches of this
> series
> into one. The new function doesn't really add any new functionality,
> but
> is a refactoring. This is more evident if you see in a single diff that
> the pieces in qca_power_on() are removed elsewhere.
OK
在 2020-01-09 11:22，Rocky Liao 写道：
> Hi Matt,
>
> 在 2020-01-09 02:34，Matthias Kaehlcke 写道：
>> Hi Rocky,
>>
>> On Wed, Jan 08, 2020 at 05:08:02PM +0800, Rocky Liao wrote:
>>> This patch adds a unified API qca_power_on() to support both wcn399x
>>> and
>>> Rome power on. For wcn399x it calls the qca_wcn3990_init() to init
>>> the
>>> regulators, and for Rome it pulls up the bt_en GPIO to power up the
>>> btsoc.
>>>
>>> Signed-off-by: Rocky Liao <[email protected]>
>>> ---
>>>
>>> Changes in v2: None
>>>
>>> drivers/bluetooth/hci_qca.c | 21 +++++++++++++++++++++
>>> 1 file changed, 21 insertions(+)
>>>
>>> diff --git a/drivers/bluetooth/hci_qca.c
>>> b/drivers/bluetooth/hci_qca.c
>>> index 9392cc7f9908..f6555bd1adbc 100644
>>> --- a/drivers/bluetooth/hci_qca.c
>>> +++ b/drivers/bluetooth/hci_qca.c
>>> @@ -1532,6 +1532,27 @@ static int qca_wcn3990_init(struct hci_uart
>>> *hu)
>>> return 0;
>>> }
>>>
>>> +static int qca_power_on(struct hci_dev *hdev)
>>> +{
>>> + struct hci_uart *hu = hci_get_drvdata(hdev);
>>> + enum qca_btsoc_type soc_type = qca_soc_type(hu);
>>> + struct qca_serdev *qcadev;
>>> + int ret = 0;
>>> +
>>> + if (qca_is_wcn399x(soc_type)) {
>>
>> Why not include the qca_regulator_enable() call from qca_open() here?
>> It is clearly part of power on.
>>
> OK
>
qca_wcn3990_init() already have the qca_regulator_enable() call, so we
just
need to remove it from qca_open().
>>> + ret = qca_wcn3990_init(hu);
>>> + } else {
>>> + if (hu->serdev) {
>>
>> nit: you could save a level of indentation (and IMO improve
>> readability) by doing:
>>
>> if (!hu->serdev)
>> return 0;
>>
> OK
>
>>> + qcadev = serdev_device_get_drvdata(hu->serdev);
>>> + gpiod_set_value_cansleep(qcadev->bt_en, 1);
>>> + /* Controller needs time to bootup. */
>>> + msleep(150);
>>> + }
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>
>> I think common practice would be to combine the 3 patches of this
>> series
>> into one. The new function doesn't really add any new functionality,
>> but
>> is a refactoring. This is more evident if you see in a single diff
>> that
>> the pieces in qca_power_on() are removed elsewhere.
> OK
This patch adds a unified API qca_power_on() to support both wcn399x and
Rome power on. For wcn399x it calls the qca_wcn3990_init() to init the
regulators, and for Rome it pulls up the bt_en GPIO to power up the btsoc.
It also moves all the power up operation from hdev->open() to
hdev->setup().
Signed-off-by: Rocky Liao <[email protected]>
---
Changes in v2: None
Changes in v3:
-combined the changes of patch 2 and 3 into this patch
-updated the commit message
drivers/bluetooth/hci_qca.c | 46 ++++++++++++++++++++++++-------------
1 file changed, 30 insertions(+), 16 deletions(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 82e4cd4b6663..427e381a08b4 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -541,7 +541,6 @@ static int qca_open(struct hci_uart *hu)
{
struct qca_serdev *qcadev;
struct qca_data *qca;
- int ret;
BT_DBG("hu %p qca_open", hu);
@@ -582,23 +581,10 @@ static int qca_open(struct hci_uart *hu)
hu->priv = qca;
if (hu->serdev) {
-
qcadev = serdev_device_get_drvdata(hu->serdev);
- if (!qca_is_wcn399x(qcadev->btsoc_type)) {
- gpiod_set_value_cansleep(qcadev->bt_en, 1);
- /* Controller needs time to bootup. */
- msleep(150);
- } else {
+ if (qca_is_wcn399x(qcadev->btsoc_type)) {
hu->init_speed = qcadev->init_speed;
hu->oper_speed = qcadev->oper_speed;
- ret = qca_regulator_enable(qcadev);
- if (ret) {
- destroy_workqueue(qca->workqueue);
- kfree_skb(qca->rx_skb);
- hu->priv = NULL;
- kfree(qca);
- return ret;
- }
}
}
@@ -1531,6 +1517,31 @@ static int qca_wcn3990_init(struct hci_uart *hu)
return 0;
}
+static int qca_power_on(struct hci_dev *hdev)
+{
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+ enum qca_btsoc_type soc_type = qca_soc_type(hu);
+ struct qca_serdev *qcadev;
+ int ret = 0;
+
+ /* Non-serdev device usually is powered by external power
+ * and don't need additional action in driver for power on
+ */
+ if (!hu->serdev)
+ return 0;
+
+ if (qca_is_wcn399x(soc_type)) {
+ ret = qca_wcn3990_init(hu);
+ } else {
+ qcadev = serdev_device_get_drvdata(hu->serdev);
+ gpiod_set_value_cansleep(qcadev->bt_en, 1);
+ /* Controller needs time to bootup. */
+ msleep(150);
+ }
+
+ return ret;
+}
+
static int qca_setup(struct hci_uart *hu)
{
struct hci_dev *hdev = hu->hdev;
@@ -1562,7 +1573,7 @@ static int qca_setup(struct hci_uart *hu)
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_wcn3990_init(hu);
+ ret = qca_power_on(hdev);
if (ret)
return ret;
@@ -1571,6 +1582,9 @@ static int qca_setup(struct hci_uart *hu)
return ret;
} else {
bt_dev_info(hdev, "ROME setup");
+ ret = qca_power_on(hdev);
+ if (ret)
+ return ret;
qca_set_speed(hu, QCA_INIT_SPEED);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Rocky,
On Thu, Jan 09, 2020 at 01:14:27PM +0800, Rocky Liao wrote:
> This patch adds a unified API qca_power_on() to support both wcn399x and
> Rome power on. For wcn399x it calls the qca_wcn3990_init() to init the
> regulators, and for Rome it pulls up the bt_en GPIO to power up the btsoc.
> It also moves all the power up operation from hdev->open() to
> hdev->setup().
>
> Signed-off-by: Rocky Liao <[email protected]>
> ---
>
> Changes in v2: None
> Changes in v3:
> -combined the changes of patch 2 and 3 into this patch
it would be better to actually describe what was done, "patch 2 and 3"
doesn't provide much useful information.
> -updated the commit message
>
> drivers/bluetooth/hci_qca.c | 46 ++++++++++++++++++++++++-------------
> 1 file changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 82e4cd4b6663..427e381a08b4 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -541,7 +541,6 @@ static int qca_open(struct hci_uart *hu)
> {
> struct qca_serdev *qcadev;
> struct qca_data *qca;
> - int ret;
>
> BT_DBG("hu %p qca_open", hu);
>
> @@ -582,23 +581,10 @@ static int qca_open(struct hci_uart *hu)
> hu->priv = qca;
>
> if (hu->serdev) {
> -
> qcadev = serdev_device_get_drvdata(hu->serdev);
> - if (!qca_is_wcn399x(qcadev->btsoc_type)) {
> - gpiod_set_value_cansleep(qcadev->bt_en, 1);
> - /* Controller needs time to bootup. */
> - msleep(150);
> - } else {
> + if (qca_is_wcn399x(qcadev->btsoc_type)) {
> hu->init_speed = qcadev->init_speed;
> hu->oper_speed = qcadev->oper_speed;
> - ret = qca_regulator_enable(qcadev);
> - if (ret) {
> - destroy_workqueue(qca->workqueue);
> - kfree_skb(qca->rx_skb);
> - hu->priv = NULL;
> - kfree(qca);
> - return ret;
> - }
> }
> }
>
> @@ -1531,6 +1517,31 @@ static int qca_wcn3990_init(struct hci_uart *hu)
> return 0;
> }
>
> +static int qca_power_on(struct hci_dev *hdev)
> +{
> + struct hci_uart *hu = hci_get_drvdata(hdev);
> + enum qca_btsoc_type soc_type = qca_soc_type(hu);
> + struct qca_serdev *qcadev;
> + int ret = 0;
> +
> + /* Non-serdev device usually is powered by external power
> + * and don't need additional action in driver for power on
> + */
> + if (!hu->serdev)
> + return 0;
> +
> + if (qca_is_wcn399x(soc_type)) {
> + ret = qca_wcn3990_init(hu);
Since there is no real need to add the qca_regulator_enable() call from
qca_open() here qca_power_on() is now essentially a fancy wrapper for
qca_wcn3990_init(), but I guess that's ok.
> + } else {
> + qcadev = serdev_device_get_drvdata(hu->serdev);
> + gpiod_set_value_cansleep(qcadev->bt_en, 1);
> + /* Controller needs time to bootup. */
> + msleep(150);
> + }
> +
> + return ret;
> +}
> +
> static int qca_setup(struct hci_uart *hu)
> {
> struct hci_dev *hdev = hu->hdev;
> @@ -1562,7 +1573,7 @@ static int qca_setup(struct hci_uart *hu)
> 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_wcn3990_init(hu);
> + ret = qca_power_on(hdev);
> if (ret)
> return ret;
>
> @@ -1571,6 +1582,9 @@ static int qca_setup(struct hci_uart *hu)
> return ret;
> } else {
> bt_dev_info(hdev, "ROME setup");
> + ret = qca_power_on(hdev);
> + if (ret)
> + return ret;
> qca_set_speed(hu, QCA_INIT_SPEED);
> }
It would be nice if we could get away with a single qca_power_on() call
for WCN399x and ROME. How about this before 'if (qca_is_wcn399x(soc_type))':
bt_dev_info(hdev, "setting up %s",
qca_is_wcn399x(soc_type)? "wcn399x" : "ROME");
ret = qca_power_on(hdev);
if (ret)
return ret;
In any case:
Reviewed-by: Matthias Kaehlcke <[email protected]>
This patch adds a unified API qca_power_on() to support both wcn399x and
Rome power on. For wcn399x it calls the qca_wcn3990_init() to init the
regulators, and for Rome it pulls up the bt_en GPIO to power up the btsoc.
It also moves all the power up operation from hdev->open() to
hdev->setup().
Signed-off-by: Rocky Liao <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
Changes in v2: None
Changes in v3:
-moved all the power up operation from open() to setup()
-updated the commit message
Changes in v4:
-made a single call to qca_power_on() in setup()
drivers/bluetooth/hci_qca.c | 48 +++++++++++++++++++++++--------------
1 file changed, 30 insertions(+), 18 deletions(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 82e4cd4b6663..6a67e5489b16 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -541,7 +541,6 @@ static int qca_open(struct hci_uart *hu)
{
struct qca_serdev *qcadev;
struct qca_data *qca;
- int ret;
BT_DBG("hu %p qca_open", hu);
@@ -582,23 +581,10 @@ static int qca_open(struct hci_uart *hu)
hu->priv = qca;
if (hu->serdev) {
-
qcadev = serdev_device_get_drvdata(hu->serdev);
- if (!qca_is_wcn399x(qcadev->btsoc_type)) {
- gpiod_set_value_cansleep(qcadev->bt_en, 1);
- /* Controller needs time to bootup. */
- msleep(150);
- } else {
+ if (qca_is_wcn399x(qcadev->btsoc_type)) {
hu->init_speed = qcadev->init_speed;
hu->oper_speed = qcadev->oper_speed;
- ret = qca_regulator_enable(qcadev);
- if (ret) {
- destroy_workqueue(qca->workqueue);
- kfree_skb(qca->rx_skb);
- hu->priv = NULL;
- kfree(qca);
- return ret;
- }
}
}
@@ -1531,6 +1517,31 @@ static int qca_wcn3990_init(struct hci_uart *hu)
return 0;
}
+static int qca_power_on(struct hci_dev *hdev)
+{
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+ enum qca_btsoc_type soc_type = qca_soc_type(hu);
+ struct qca_serdev *qcadev;
+ int ret = 0;
+
+ /* Non-serdev device usually is powered by external power
+ * and don't need additional action in driver for power on
+ */
+ if (!hu->serdev)
+ return 0;
+
+ if (qca_is_wcn399x(soc_type)) {
+ ret = qca_wcn3990_init(hu);
+ } else {
+ qcadev = serdev_device_get_drvdata(hu->serdev);
+ gpiod_set_value_cansleep(qcadev->bt_en, 1);
+ /* Controller needs time to bootup. */
+ msleep(150);
+ }
+
+ return ret;
+}
+
static int qca_setup(struct hci_uart *hu)
{
struct hci_dev *hdev = hu->hdev;
@@ -1553,6 +1564,10 @@ static int qca_setup(struct hci_uart *hu)
*/
set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
+ ret = qca_power_on(hdev);
+ if (ret)
+ return ret;
+
if (qca_is_wcn399x(soc_type)) {
bt_dev_info(hdev, "setting up wcn3990");
@@ -1562,9 +1577,6 @@ static int qca_setup(struct hci_uart *hu)
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_wcn3990_init(hu);
- if (ret)
- return ret;
ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
if (ret)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Rocky,
On Fri, Jan 10, 2020 at 11:32:14AM +0800, Rocky Liao wrote:
> This patch adds a unified API qca_power_on() to support both wcn399x and
> Rome power on. For wcn399x it calls the qca_wcn3990_init() to init the
> regulators, and for Rome it pulls up the bt_en GPIO to power up the btsoc.
> It also moves all the power up operation from hdev->open() to
> hdev->setup().
>
> Signed-off-by: Rocky Liao <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
> ---
>
> Changes in v2: None
> Changes in v3:
> -moved all the power up operation from open() to setup()
> -updated the commit message
> Changes in v4:
> -made a single call to qca_power_on() in setup()
>
>
> drivers/bluetooth/hci_qca.c | 48 +++++++++++++++++++++++--------------
> 1 file changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 82e4cd4b6663..6a67e5489b16 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -541,7 +541,6 @@ static int qca_open(struct hci_uart *hu)
> {
> struct qca_serdev *qcadev;
> struct qca_data *qca;
> - int ret;
>
> BT_DBG("hu %p qca_open", hu);
>
> @@ -582,23 +581,10 @@ static int qca_open(struct hci_uart *hu)
> hu->priv = qca;
>
> if (hu->serdev) {
> -
> qcadev = serdev_device_get_drvdata(hu->serdev);
> - if (!qca_is_wcn399x(qcadev->btsoc_type)) {
> - gpiod_set_value_cansleep(qcadev->bt_en, 1);
> - /* Controller needs time to bootup. */
> - msleep(150);
> - } else {
> + if (qca_is_wcn399x(qcadev->btsoc_type)) {
> hu->init_speed = qcadev->init_speed;
> hu->oper_speed = qcadev->oper_speed;
> - ret = qca_regulator_enable(qcadev);
> - if (ret) {
> - destroy_workqueue(qca->workqueue);
> - kfree_skb(qca->rx_skb);
> - hu->priv = NULL;
> - kfree(qca);
> - return ret;
> - }
> }
> }
>
> @@ -1531,6 +1517,31 @@ static int qca_wcn3990_init(struct hci_uart *hu)
> return 0;
> }
>
> +static int qca_power_on(struct hci_dev *hdev)
> +{
> + struct hci_uart *hu = hci_get_drvdata(hdev);
> + enum qca_btsoc_type soc_type = qca_soc_type(hu);
> + struct qca_serdev *qcadev;
> + int ret = 0;
> +
> + /* Non-serdev device usually is powered by external power
> + * and don't need additional action in driver for power on
> + */
> + if (!hu->serdev)
> + return 0;
> +
> + if (qca_is_wcn399x(soc_type)) {
> + ret = qca_wcn3990_init(hu);
> + } else {
> + qcadev = serdev_device_get_drvdata(hu->serdev);
> + gpiod_set_value_cansleep(qcadev->bt_en, 1);
> + /* Controller needs time to bootup. */
> + msleep(150);
> + }
> +
> + return ret;
> +}
> +
> static int qca_setup(struct hci_uart *hu)
> {
> struct hci_dev *hdev = hu->hdev;
> @@ -1553,6 +1564,10 @@ static int qca_setup(struct hci_uart *hu)
> */
> set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
>
> + ret = qca_power_on(hdev);
> + if (ret)
> + return ret;
> +
Now if qca_power_on() fails there is no log entry indicating that Bluetooth
setup was running. That's why I suggested to put this log entry before
the call, and remove the corresponding entries from the if/else branches:
bt_dev_info(hdev, "setting up %s",
qca_is_wcn399x(soc_type)? "wcn399x" : "ROME");
> if (qca_is_wcn399x(soc_type)) {
> bt_dev_info(hdev, "setting up wcn3990");
>
> @@ -1562,9 +1577,6 @@ static int qca_setup(struct hci_uart *hu)
> 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_wcn3990_init(hu);
> - if (ret)
> - return ret;
>
> ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
> if (ret)
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
在 2020-01-11 08:31，Matthias Kaehlcke 写道：
> Hi Rocky,
>
> On Fri, Jan 10, 2020 at 11:32:14AM +0800, Rocky Liao wrote:
>> This patch adds a unified API qca_power_on() to support both wcn399x
>> and
>> Rome power on. For wcn399x it calls the qca_wcn3990_init() to init the
>> regulators, and for Rome it pulls up the bt_en GPIO to power up the
>> btsoc.
>> It also moves all the power up operation from hdev->open() to
>> hdev->setup().
>>
>> Signed-off-by: Rocky Liao <[email protected]>
>> Reviewed-by: Matthias Kaehlcke <[email protected]>
>> ---
>>
>> Changes in v2: None
>> Changes in v3:
>> -moved all the power up operation from open() to setup()
>> -updated the commit message
>> Changes in v4:
>> -made a single call to qca_power_on() in setup()
>>
>>
>> drivers/bluetooth/hci_qca.c | 48
>> +++++++++++++++++++++++--------------
>> 1 file changed, 30 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index 82e4cd4b6663..6a67e5489b16 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -541,7 +541,6 @@ static int qca_open(struct hci_uart *hu)
>> {
>> struct qca_serdev *qcadev;
>> struct qca_data *qca;
>> - int ret;
>>
>> BT_DBG("hu %p qca_open", hu);
>>
>> @@ -582,23 +581,10 @@ static int qca_open(struct hci_uart *hu)
>> hu->priv = qca;
>>
>> if (hu->serdev) {
>> -
>> qcadev = serdev_device_get_drvdata(hu->serdev);
>> - if (!qca_is_wcn399x(qcadev->btsoc_type)) {
>> - gpiod_set_value_cansleep(qcadev->bt_en, 1);
>> - /* Controller needs time to bootup. */
>> - msleep(150);
>> - } else {
>> + if (qca_is_wcn399x(qcadev->btsoc_type)) {
>> hu->init_speed = qcadev->init_speed;
>> hu->oper_speed = qcadev->oper_speed;
>> - ret = qca_regulator_enable(qcadev);
>> - if (ret) {
>> - destroy_workqueue(qca->workqueue);
>> - kfree_skb(qca->rx_skb);
>> - hu->priv = NULL;
>> - kfree(qca);
>> - return ret;
>> - }
>> }
>> }
>>
>> @@ -1531,6 +1517,31 @@ static int qca_wcn3990_init(struct hci_uart
>> *hu)
>> return 0;
>> }
>>
>> +static int qca_power_on(struct hci_dev *hdev)
>> +{
>> + struct hci_uart *hu = hci_get_drvdata(hdev);
>> + enum qca_btsoc_type soc_type = qca_soc_type(hu);
>> + struct qca_serdev *qcadev;
>> + int ret = 0;
>> +
>> + /* Non-serdev device usually is powered by external power
>> + * and don't need additional action in driver for power on
>> + */
>> + if (!hu->serdev)
>> + return 0;
>> +
>> + if (qca_is_wcn399x(soc_type)) {
>> + ret = qca_wcn3990_init(hu);
>> + } else {
>> + qcadev = serdev_device_get_drvdata(hu->serdev);
>> + gpiod_set_value_cansleep(qcadev->bt_en, 1);
>> + /* Controller needs time to bootup. */
>> + msleep(150);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> static int qca_setup(struct hci_uart *hu)
>> {
>> struct hci_dev *hdev = hu->hdev;
>> @@ -1553,6 +1564,10 @@ static int qca_setup(struct hci_uart *hu)
>> */
>> set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
>>
>> + ret = qca_power_on(hdev);
>> + if (ret)
>> + return ret;
>> +
>
> Now if qca_power_on() fails there is no log entry indicating that
> Bluetooth
> setup was running. That's why I suggested to put this log entry before
> the call, and remove the corresponding entries from the if/else
> branches:
>
> bt_dev_info(hdev, "setting up %s",
> qca_is_wcn399x(soc_type)? "wcn399x" : "ROME");
>
Sorry I missed that and will update the patch soon.
>> if (qca_is_wcn399x(soc_type)) {
>> bt_dev_info(hdev, "setting up wcn3990");
>>
>> @@ -1562,9 +1577,6 @@ static int qca_setup(struct hci_uart *hu)
>> 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_wcn3990_init(hu);
>> - if (ret)
>> - return ret;
>>
>> ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
>> if (ret)
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum, a Linux Foundation Collaborative Project
Sorry, please ignore this one. I was wanting to send v5...
在 2020-01-13 12:29，Rocky Liao 写道：
> This patch adds a unified API qca_power_on() to support both wcn399x
> and
> Rome power on. For wcn399x it calls the qca_wcn3990_init() to init the
> regulators, and for Rome it pulls up the bt_en GPIO to power up the
> btsoc.
> It also moves all the power up operation from hdev->open() to
> hdev->setup().
>
> Signed-off-by: Rocky Liao <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
> ---
>
> Changes in v2: None
> Changes in v3:
> -moved all the power up operation from open() to setup()
> -updated the commit message
> Changes in v4:
> -made a single call to qca_power_on() in setup()
>
>
> drivers/bluetooth/hci_qca.c | 48 +++++++++++++++++++++++--------------
> 1 file changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 82e4cd4b6663..6a67e5489b16 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -541,7 +541,6 @@ static int qca_open(struct hci_uart *hu)
> {
> struct qca_serdev *qcadev;
> struct qca_data *qca;
> - int ret;
>
> BT_DBG("hu %p qca_open", hu);
>
> @@ -582,23 +581,10 @@ static int qca_open(struct hci_uart *hu)
> hu->priv = qca;
>
> if (hu->serdev) {
> -
> qcadev = serdev_device_get_drvdata(hu->serdev);
> - if (!qca_is_wcn399x(qcadev->btsoc_type)) {
> - gpiod_set_value_cansleep(qcadev->bt_en, 1);
> - /* Controller needs time to bootup. */
> - msleep(150);
> - } else {
> + if (qca_is_wcn399x(qcadev->btsoc_type)) {
> hu->init_speed = qcadev->init_speed;
> hu->oper_speed = qcadev->oper_speed;
> - ret = qca_regulator_enable(qcadev);
> - if (ret) {
> - destroy_workqueue(qca->workqueue);
> - kfree_skb(qca->rx_skb);
> - hu->priv = NULL;
> - kfree(qca);
> - return ret;
> - }
> }
> }
>
> @@ -1531,6 +1517,31 @@ static int qca_wcn3990_init(struct hci_uart *hu)
> return 0;
> }
>
> +static int qca_power_on(struct hci_dev *hdev)
> +{
> + struct hci_uart *hu = hci_get_drvdata(hdev);
> + enum qca_btsoc_type soc_type = qca_soc_type(hu);
> + struct qca_serdev *qcadev;
> + int ret = 0;
> +
> + /* Non-serdev device usually is powered by external power
> + * and don't need additional action in driver for power on
> + */
> + if (!hu->serdev)
> + return 0;
> +
> + if (qca_is_wcn399x(soc_type)) {
> + ret = qca_wcn3990_init(hu);
> + } else {
> + qcadev = serdev_device_get_drvdata(hu->serdev);
> + gpiod_set_value_cansleep(qcadev->bt_en, 1);
> + /* Controller needs time to bootup. */
> + msleep(150);
> + }
> +
> + return ret;
> +}
> +
> static int qca_setup(struct hci_uart *hu)
> {
> struct hci_dev *hdev = hu->hdev;
> @@ -1553,6 +1564,10 @@ static int qca_setup(struct hci_uart *hu)
> */
> set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
>
> + ret = qca_power_on(hdev);
> + if (ret)
> + return ret;
> +
> if (qca_is_wcn399x(soc_type)) {
> bt_dev_info(hdev, "setting up wcn3990");
>
> @@ -1562,9 +1577,6 @@ static int qca_setup(struct hci_uart *hu)
> 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_wcn3990_init(hu);
> - if (ret)
> - return ret;
>
> ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
> if (ret)
Sorry, please ignore this one. I was wanting to send v5...
在 2020-01-13 12:29，Rocky Liao 写道：
> This patch adds a unified API qca_power_on() to support both wcn399x
> and
> Rome power on. For wcn399x it calls the qca_wcn3990_init() to init the
> regulators, and for Rome it pulls up the bt_en GPIO to power up the
> btsoc.
> It also moves all the power up operation from hdev->open() to
> hdev->setup().
>
> Signed-off-by: Rocky Liao <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
> ---
>
> Changes in v2: None
> Changes in v3:
> -moved all the power up operation from open() to setup()
> -updated the commit message
> Changes in v4:
> -made a single call to qca_power_on() in setup()
>
>
> drivers/bluetooth/hci_qca.c | 48 +++++++++++++++++++++++--------------
> 1 file changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 82e4cd4b6663..6a67e5489b16 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -541,7 +541,6 @@ static int qca_open(struct hci_uart *hu)
> {
> struct qca_serdev *qcadev;
> struct qca_data *qca;
> - int ret;
>
> BT_DBG("hu %p qca_open", hu);
>
> @@ -582,23 +581,10 @@ static int qca_open(struct hci_uart *hu)
> hu->priv = qca;
>
> if (hu->serdev) {
> -
> qcadev = serdev_device_get_drvdata(hu->serdev);
> - if (!qca_is_wcn399x(qcadev->btsoc_type)) {
> - gpiod_set_value_cansleep(qcadev->bt_en, 1);
> - /* Controller needs time to bootup. */
> - msleep(150);
> - } else {
> + if (qca_is_wcn399x(qcadev->btsoc_type)) {
> hu->init_speed = qcadev->init_speed;
> hu->oper_speed = qcadev->oper_speed;
> - ret = qca_regulator_enable(qcadev);
> - if (ret) {
> - destroy_workqueue(qca->workqueue);
> - kfree_skb(qca->rx_skb);
> - hu->priv = NULL;
> - kfree(qca);
> - return ret;
> - }
> }
> }
>
> @@ -1531,6 +1517,31 @@ static int qca_wcn3990_init(struct hci_uart *hu)
> return 0;
> }
>
> +static int qca_power_on(struct hci_dev *hdev)
> +{
> + struct hci_uart *hu = hci_get_drvdata(hdev);
> + enum qca_btsoc_type soc_type = qca_soc_type(hu);
> + struct qca_serdev *qcadev;
> + int ret = 0;
> +
> + /* Non-serdev device usually is powered by external power
> + * and don't need additional action in driver for power on
> + */
> + if (!hu->serdev)
> + return 0;
> +
> + if (qca_is_wcn399x(soc_type)) {
> + ret = qca_wcn3990_init(hu);
> + } else {
> + qcadev = serdev_device_get_drvdata(hu->serdev);
> + gpiod_set_value_cansleep(qcadev->bt_en, 1);
> + /* Controller needs time to bootup. */
> + msleep(150);
> + }
> +
> + return ret;
> +}
> +
> static int qca_setup(struct hci_uart *hu)
> {
> struct hci_dev *hdev = hu->hdev;
> @@ -1553,6 +1564,10 @@ static int qca_setup(struct hci_uart *hu)
> */
> set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
>
> + ret = qca_power_on(hdev);
> + if (ret)
> + return ret;
> +
> if (qca_is_wcn399x(soc_type)) {
> bt_dev_info(hdev, "setting up wcn3990");
>
> @@ -1562,9 +1577,6 @@ static int qca_setup(struct hci_uart *hu)
> 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_wcn3990_init(hu);
> - if (ret)
> - return ret;
>
> ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
> if (ret)
This patch adds a unified API qca_power_on() to support both wcn399x and
Rome power on. For wcn399x it calls the qca_wcn3990_init() to init the
regulators, and for Rome it pulls up the bt_en GPIO to power up the btsoc.
It also moves all the power up operation from hdev->open() to
hdev->setup().
Signed-off-by: Rocky Liao <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
Changes in v2: None
Changes in v3:
-moved all the power up operation from open() to setup()
-updated the commit message
Changes in v4:
-made a single call to qca_power_on() in setup()
Changes in v5:
-modified the debug log location
drivers/bluetooth/hci_qca.c | 54 ++++++++++++++++++++++---------------
1 file changed, 33 insertions(+), 21 deletions(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 82e4cd4b6663..992622dc1263 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -541,7 +541,6 @@ static int qca_open(struct hci_uart *hu)
{
struct qca_serdev *qcadev;
struct qca_data *qca;
- int ret;
BT_DBG("hu %p qca_open", hu);
@@ -582,23 +581,10 @@ static int qca_open(struct hci_uart *hu)
hu->priv = qca;
if (hu->serdev) {
-
qcadev = serdev_device_get_drvdata(hu->serdev);
- if (!qca_is_wcn399x(qcadev->btsoc_type)) {
- gpiod_set_value_cansleep(qcadev->bt_en, 1);
- /* Controller needs time to bootup. */
- msleep(150);
- } else {
+ if (qca_is_wcn399x(qcadev->btsoc_type)) {
hu->init_speed = qcadev->init_speed;
hu->oper_speed = qcadev->oper_speed;
- ret = qca_regulator_enable(qcadev);
- if (ret) {
- destroy_workqueue(qca->workqueue);
- kfree_skb(qca->rx_skb);
- hu->priv = NULL;
- kfree(qca);
- return ret;
- }
}
}
@@ -1531,6 +1517,31 @@ static int qca_wcn3990_init(struct hci_uart *hu)
return 0;
}
+static int qca_power_on(struct hci_dev *hdev)
+{
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+ enum qca_btsoc_type soc_type = qca_soc_type(hu);
+ struct qca_serdev *qcadev;
+ int ret = 0;
+
+ /* Non-serdev device usually is powered by external power
+ * and don't need additional action in driver for power on
+ */
+ if (!hu->serdev)
+ return 0;
+
+ if (qca_is_wcn399x(soc_type)) {
+ ret = qca_wcn3990_init(hu);
+ } else {
+ qcadev = serdev_device_get_drvdata(hu->serdev);
+ gpiod_set_value_cansleep(qcadev->bt_en, 1);
+ /* Controller needs time to bootup. */
+ msleep(150);
+ }
+
+ return ret;
+}
+
static int qca_setup(struct hci_uart *hu)
{
struct hci_dev *hdev = hu->hdev;
@@ -1553,24 +1564,25 @@ static int qca_setup(struct hci_uart *hu)
*/
set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
- if (qca_is_wcn399x(soc_type)) {
- bt_dev_info(hdev, "setting up wcn3990");
+ bt_dev_info(hdev, "setting up %s",
+ qca_is_wcn399x(soc_type) ? "wcn399x" : "ROME");
+ ret = qca_power_on(hdev);
+ if (ret)
+ 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_wcn3990_init(hu);
- if (ret)
- return ret;
ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
if (ret)
return ret;
} else {
- bt_dev_info(hdev, "ROME setup");
qca_set_speed(hu, QCA_INIT_SPEED);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
This patch adds a unified API qca_power_on() to support both wcn399x and
Rome power on. For wcn399x it calls the qca_wcn3990_init() to init the
regulators, and for Rome it pulls up the bt_en GPIO to power up the btsoc.
It also moves all the power up operation from hdev->open() to
hdev->setup().
Signed-off-by: Rocky Liao <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
Changes in v2: None
Changes in v3:
-moved all the power up operation from open() to setup()
-updated the commit message
Changes in v4:
-made a single call to qca_power_on() in setup()
drivers/bluetooth/hci_qca.c | 48 +++++++++++++++++++++++--------------
1 file changed, 30 insertions(+), 18 deletions(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 82e4cd4b6663..6a67e5489b16 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -541,7 +541,6 @@ static int qca_open(struct hci_uart *hu)
{
struct qca_serdev *qcadev;
struct qca_data *qca;
- int ret;
BT_DBG("hu %p qca_open", hu);
@@ -582,23 +581,10 @@ static int qca_open(struct hci_uart *hu)
hu->priv = qca;
if (hu->serdev) {
-
qcadev = serdev_device_get_drvdata(hu->serdev);
- if (!qca_is_wcn399x(qcadev->btsoc_type)) {
- gpiod_set_value_cansleep(qcadev->bt_en, 1);
- /* Controller needs time to bootup. */
- msleep(150);
- } else {
+ if (qca_is_wcn399x(qcadev->btsoc_type)) {
hu->init_speed = qcadev->init_speed;
hu->oper_speed = qcadev->oper_speed;
- ret = qca_regulator_enable(qcadev);
- if (ret) {
- destroy_workqueue(qca->workqueue);
- kfree_skb(qca->rx_skb);
- hu->priv = NULL;
- kfree(qca);
- return ret;
- }
}
}
@@ -1531,6 +1517,31 @@ static int qca_wcn3990_init(struct hci_uart *hu)
return 0;
}
+static int qca_power_on(struct hci_dev *hdev)
+{
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+ enum qca_btsoc_type soc_type = qca_soc_type(hu);
+ struct qca_serdev *qcadev;
+ int ret = 0;
+
+ /* Non-serdev device usually is powered by external power
+ * and don't need additional action in driver for power on
+ */
+ if (!hu->serdev)
+ return 0;
+
+ if (qca_is_wcn399x(soc_type)) {
+ ret = qca_wcn3990_init(hu);
+ } else {
+ qcadev = serdev_device_get_drvdata(hu->serdev);
+ gpiod_set_value_cansleep(qcadev->bt_en, 1);
+ /* Controller needs time to bootup. */
+ msleep(150);
+ }
+
+ return ret;
+}
+
static int qca_setup(struct hci_uart *hu)
{
struct hci_dev *hdev = hu->hdev;
@@ -1553,6 +1564,10 @@ static int qca_setup(struct hci_uart *hu)
*/
set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
+ ret = qca_power_on(hdev);
+ if (ret)
+ return ret;
+
if (qca_is_wcn399x(soc_type)) {
bt_dev_info(hdev, "setting up wcn3990");
@@ -1562,9 +1577,6 @@ static int qca_setup(struct hci_uart *hu)
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_wcn3990_init(hu);
- if (ret)
- return ret;
ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
if (ret)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project