* [PATCH v1] Bluetooth: hci_qca: Make bt_en and susclk not mandatory for QCA Rome
@ 2020-03-04 1:54 Rocky Liao
2020-03-04 7:25 ` Marcel Holtmann
2020-03-04 8:53 ` Johan Hovold
0 siblings, 2 replies; 4+ messages in thread
From: Rocky Liao @ 2020-03-04 1:54 UTC (permalink / raw)
To: marcel, johan.hedberg
Cc: linux-kernel, linux-bluetooth, linux-arm-msm, bgodavar, c-hbandi,
hemantg, mka, Rocky Liao
On some platforms the bt_en pin and susclk are default on and there
is no exposed resource to control them. This patch makes the bt_en
and susclk not mandatory to have BT work. It also will not set the
HCI_QUIRK_NON_PERSISTENT_SETUP and shutdown() callback if bt_en is
not available.
Signed-off-by: Rocky Liao <rjliao@codeaurora.org>
---
drivers/bluetooth/hci_qca.c | 47 ++++++++++++++++++++-----------------
1 file changed, 26 insertions(+), 21 deletions(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index bf436d6e638e..325baa046c3a 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1562,9 +1562,11 @@ static int qca_power_on(struct hci_dev *hdev)
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);
+ if (!IS_ERR(qcadev->bt_en)) {
+ gpiod_set_value_cansleep(qcadev->bt_en, 1);
+ /* Controller needs time to bootup. */
+ msleep(150);
+ }
}
return ret;
@@ -1750,7 +1752,7 @@ static void qca_power_shutdown(struct hci_uart *hu)
host_set_baudrate(hu, 2400);
qca_send_power_pulse(hu, false);
qca_regulator_disable(qcadev);
- } else {
+ } else if (!IS_ERR(qcadev->bt_en)) {
gpiod_set_value_cansleep(qcadev->bt_en, 0);
}
}
@@ -1852,6 +1854,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
struct hci_dev *hdev;
const struct qca_vreg_data *data;
int err;
+ bool power_ctrl_enabled = true;
qcadev = devm_kzalloc(&serdev->dev, sizeof(*qcadev), GFP_KERNEL);
if (!qcadev)
@@ -1901,35 +1904,37 @@ static int qca_serdev_probe(struct serdev_device *serdev)
qcadev->bt_en = devm_gpiod_get(&serdev->dev, "enable",
GPIOD_OUT_LOW);
if (IS_ERR(qcadev->bt_en)) {
- dev_err(&serdev->dev, "failed to acquire enable gpio\n");
- return PTR_ERR(qcadev->bt_en);
+ dev_warn(&serdev->dev, "failed to acquire enable gpio\n");
+ power_ctrl_enabled = false;
}
qcadev->susclk = devm_clk_get(&serdev->dev, NULL);
if (IS_ERR(qcadev->susclk)) {
- dev_err(&serdev->dev, "failed to acquire clk\n");
- return PTR_ERR(qcadev->susclk);
- }
-
- err = clk_set_rate(qcadev->susclk, SUSCLK_RATE_32KHZ);
- if (err)
- return err;
+ dev_warn(&serdev->dev, "failed to acquire clk\n");
+ } else {
+ err = clk_set_rate(qcadev->susclk, SUSCLK_RATE_32KHZ);
+ if (err)
+ return err;
- err = clk_prepare_enable(qcadev->susclk);
- if (err)
- return err;
+ err = clk_prepare_enable(qcadev->susclk);
+ if (err)
+ return err;
+ }
err = hci_uart_register_device(&qcadev->serdev_hu, &qca_proto);
if (err) {
BT_ERR("Rome serdev registration failed");
- clk_disable_unprepare(qcadev->susclk);
+ if (!IS_ERR(qcadev->susclk))
+ clk_disable_unprepare(qcadev->susclk);
return err;
}
}
- hdev = qcadev->serdev_hu.hdev;
- set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
- hdev->shutdown = qca_power_off;
+ if (power_ctrl_enabled) {
+ hdev = qcadev->serdev_hu.hdev;
+ set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
+ hdev->shutdown = qca_power_off;
+ }
return 0;
}
@@ -1940,7 +1945,7 @@ static void qca_serdev_remove(struct serdev_device *serdev)
if (qca_is_wcn399x(qcadev->btsoc_type))
qca_power_shutdown(&qcadev->serdev_hu);
- else
+ else if (!IS_ERR(qcadev->susclk))
clk_disable_unprepare(qcadev->susclk);
hci_uart_unregister_device(&qcadev->serdev_hu);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v1] Bluetooth: hci_qca: Make bt_en and susclk not mandatory for QCA Rome
2020-03-04 1:54 [PATCH v1] Bluetooth: hci_qca: Make bt_en and susclk not mandatory for QCA Rome Rocky Liao
@ 2020-03-04 7:25 ` Marcel Holtmann
2020-03-04 8:53 ` Johan Hovold
1 sibling, 0 replies; 4+ messages in thread
From: Marcel Holtmann @ 2020-03-04 7:25 UTC (permalink / raw)
To: Rocky Liao
Cc: Johan Hedberg, linux-kernel, linux-bluetooth, linux-arm-msm,
bgodavar, c-hbandi, hemantg, mka
Hi Rocky,
> On some platforms the bt_en pin and susclk are default on and there
> is no exposed resource to control them. This patch makes the bt_en
> and susclk not mandatory to have BT work. It also will not set the
> HCI_QUIRK_NON_PERSISTENT_SETUP and shutdown() callback if bt_en is
> not available.
>
> Signed-off-by: Rocky Liao <rjliao@codeaurora.org>
> ---
> drivers/bluetooth/hci_qca.c | 47 ++++++++++++++++++++-----------------
> 1 file changed, 26 insertions(+), 21 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] Bluetooth: hci_qca: Make bt_en and susclk not mandatory for QCA Rome
2020-03-04 1:54 [PATCH v1] Bluetooth: hci_qca: Make bt_en and susclk not mandatory for QCA Rome Rocky Liao
2020-03-04 7:25 ` Marcel Holtmann
@ 2020-03-04 8:53 ` Johan Hovold
2020-03-04 12:03 ` Rocky Liao
1 sibling, 1 reply; 4+ messages in thread
From: Johan Hovold @ 2020-03-04 8:53 UTC (permalink / raw)
To: Rocky Liao
Cc: marcel, johan.hedberg, linux-kernel, linux-bluetooth,
linux-arm-msm, bgodavar, c-hbandi, hemantg, mka
On Wed, Mar 04, 2020 at 09:54:29AM +0800, Rocky Liao wrote:
> On some platforms the bt_en pin and susclk are default on and there
> is no exposed resource to control them. This patch makes the bt_en
> and susclk not mandatory to have BT work. It also will not set the
> HCI_QUIRK_NON_PERSISTENT_SETUP and shutdown() callback if bt_en is
> not available.
>
> Signed-off-by: Rocky Liao <rjliao@codeaurora.org>
> ---
> drivers/bluetooth/hci_qca.c | 47 ++++++++++++++++++++-----------------
> 1 file changed, 26 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index bf436d6e638e..325baa046c3a 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -1562,9 +1562,11 @@ static int qca_power_on(struct hci_dev *hdev)
> 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);
> + if (!IS_ERR(qcadev->bt_en)) {
> + gpiod_set_value_cansleep(qcadev->bt_en, 1);
> + /* Controller needs time to bootup. */
> + msleep(150);
> + }
> }
>
> return ret;
> @@ -1750,7 +1752,7 @@ static void qca_power_shutdown(struct hci_uart *hu)
> host_set_baudrate(hu, 2400);
> qca_send_power_pulse(hu, false);
> qca_regulator_disable(qcadev);
> - } else {
> + } else if (!IS_ERR(qcadev->bt_en)) {
> gpiod_set_value_cansleep(qcadev->bt_en, 0);
> }
> }
> @@ -1852,6 +1854,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
> struct hci_dev *hdev;
> const struct qca_vreg_data *data;
> int err;
> + bool power_ctrl_enabled = true;
>
> qcadev = devm_kzalloc(&serdev->dev, sizeof(*qcadev), GFP_KERNEL);
> if (!qcadev)
> @@ -1901,35 +1904,37 @@ static int qca_serdev_probe(struct serdev_device *serdev)
> qcadev->bt_en = devm_gpiod_get(&serdev->dev, "enable",
> GPIOD_OUT_LOW);
Shouldn't you use devm_gpiod_get_optional()?
> if (IS_ERR(qcadev->bt_en)) {
> - dev_err(&serdev->dev, "failed to acquire enable gpio\n");
> - return PTR_ERR(qcadev->bt_en);
> + dev_warn(&serdev->dev, "failed to acquire enable gpio\n");
> + power_ctrl_enabled = false;
And bail out on errors, but treat NULL as !port_ctrl_enabled?
> }
>
> qcadev->susclk = devm_clk_get(&serdev->dev, NULL);
And devm_clk_get_optional() here.
Etc.
Also does the devicetree binding need to be updated to reflect this
change?
Johan
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] Bluetooth: hci_qca: Make bt_en and susclk not mandatory for QCA Rome
2020-03-04 8:53 ` Johan Hovold
@ 2020-03-04 12:03 ` Rocky Liao
0 siblings, 0 replies; 4+ messages in thread
From: Rocky Liao @ 2020-03-04 12:03 UTC (permalink / raw)
To: Johan Hovold
Cc: marcel, johan.hedberg, linux-kernel, linux-bluetooth,
linux-arm-msm, bgodavar, c-hbandi, hemantg, mka,
linux-bluetooth-owner
在 2020-03-04 16:53,Johan Hovold 写道:
> On Wed, Mar 04, 2020 at 09:54:29AM +0800, Rocky Liao wrote:
>> On some platforms the bt_en pin and susclk are default on and there
>> is no exposed resource to control them. This patch makes the bt_en
>> and susclk not mandatory to have BT work. It also will not set the
>> HCI_QUIRK_NON_PERSISTENT_SETUP and shutdown() callback if bt_en is
>> not available.
>>
>> Signed-off-by: Rocky Liao <rjliao@codeaurora.org>
>> ---
>> drivers/bluetooth/hci_qca.c | 47
>> ++++++++++++++++++++-----------------
>> 1 file changed, 26 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index bf436d6e638e..325baa046c3a 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -1562,9 +1562,11 @@ static int qca_power_on(struct hci_dev *hdev)
>> 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);
>> + if (!IS_ERR(qcadev->bt_en)) {
>> + gpiod_set_value_cansleep(qcadev->bt_en, 1);
>> + /* Controller needs time to bootup. */
>> + msleep(150);
>> + }
>> }
>>
>> return ret;
>> @@ -1750,7 +1752,7 @@ static void qca_power_shutdown(struct hci_uart
>> *hu)
>> host_set_baudrate(hu, 2400);
>> qca_send_power_pulse(hu, false);
>> qca_regulator_disable(qcadev);
>> - } else {
>> + } else if (!IS_ERR(qcadev->bt_en)) {
>> gpiod_set_value_cansleep(qcadev->bt_en, 0);
>> }
>> }
>> @@ -1852,6 +1854,7 @@ static int qca_serdev_probe(struct serdev_device
>> *serdev)
>> struct hci_dev *hdev;
>> const struct qca_vreg_data *data;
>> int err;
>> + bool power_ctrl_enabled = true;
>>
>> qcadev = devm_kzalloc(&serdev->dev, sizeof(*qcadev), GFP_KERNEL);
>> if (!qcadev)
>> @@ -1901,35 +1904,37 @@ static int qca_serdev_probe(struct
>> serdev_device *serdev)
>> qcadev->bt_en = devm_gpiod_get(&serdev->dev, "enable",
>> GPIOD_OUT_LOW);
>
> Shouldn't you use devm_gpiod_get_optional()?
>
>> if (IS_ERR(qcadev->bt_en)) {
>> - dev_err(&serdev->dev, "failed to acquire enable gpio\n");
>> - return PTR_ERR(qcadev->bt_en);
>> + dev_warn(&serdev->dev, "failed to acquire enable gpio\n");
>> + power_ctrl_enabled = false;
>
> And bail out on errors, but treat NULL as !port_ctrl_enabled?
>
>> }
>>
>> qcadev->susclk = devm_clk_get(&serdev->dev, NULL);
>
> And devm_clk_get_optional() here.
>
> Etc.
>
> Also does the devicetree binding need to be updated to reflect this
> change?
>
OK, I will update to use the suggested API.
The devicetress binding doesn't need to be updated as they are already
optional there.
> Johan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-04 12:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-04 1:54 [PATCH v1] Bluetooth: hci_qca: Make bt_en and susclk not mandatory for QCA Rome Rocky Liao
2020-03-04 7:25 ` Marcel Holtmann
2020-03-04 8:53 ` Johan Hovold
2020-03-04 12:03 ` Rocky Liao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).