Messages in this thread | | | Date | Mon, 18 Nov 2019 06:01:53 +0000 | From | bgodavar@codeauro ... | Subject | Re: [PATCH v1] Bluetooth: hci_qca: Enable clocks required for BT SOC |
| |
On 2019-11-14 14:25, Marcel Holtmann wrote: > Hi Balakrishna, > >> Instead of relying on other subsytem to turn ON clocks >> required for BT SoC to operate, voting them from the driver. >> >> Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org> >> --- >> drivers/bluetooth/hci_qca.c | 31 +++++++++++++++++++++++++++++-- >> 1 file changed, 29 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >> index f10bdf8e1fc5..dc95e378574b 100644 >> --- a/drivers/bluetooth/hci_qca.c >> +++ b/drivers/bluetooth/hci_qca.c >> @@ -164,6 +164,7 @@ struct qca_serdev { >> }; >> >> static int qca_regulator_enable(struct qca_serdev *qcadev); >> +static int qca_power_on(struct qca_serdev *qcadev); > > I really dislike forward declaration. Only use them if they are really > really needed. That said, this driver might actually need cleanups > since I just realized it has tons of forward declarations. >
[Bala]: agree, will work on this change.
>> 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); >> @@ -528,7 +529,7 @@ static int qca_open(struct hci_uart *hu) >> } else { >> hu->init_speed = qcadev->init_speed; >> hu->oper_speed = qcadev->oper_speed; >> - ret = qca_regulator_enable(qcadev); >> + ret = qca_power_on(qcadev); >> if (ret) { >> destroy_workqueue(qca->workqueue); >> kfree_skb(qca->rx_skb); >> @@ -1214,7 +1215,7 @@ static int qca_wcn3990_init(struct hci_uart *hu) >> qcadev = serdev_device_get_drvdata(hu->serdev); >> if (!qcadev->bt_power->vregs_on) { >> serdev_device_close(hu->serdev); >> - ret = qca_regulator_enable(qcadev); >> + ret = qca_power_on(qcadev); >> if (ret) >> return ret; >> >> @@ -1408,6 +1409,9 @@ static void qca_power_shutdown(struct hci_uart >> *hu) >> host_set_baudrate(hu, 2400); >> qca_send_power_pulse(hu, false); >> qca_regulator_disable(qcadev); >> + >> + if (qcadev->susclk) >> + clk_disable_unprepare(qcadev->susclk); >> } >> >> static int qca_power_off(struct hci_dev *hdev) >> @@ -1423,6 +1427,20 @@ static int qca_power_off(struct hci_dev *hdev) >> return 0; >> } >> >> +static int qca_power_on(struct qca_serdev *qcadev) >> +{ >> + int err; >> + >> + if (qcadev->susclk) { >> + err = clk_prepare_enable(qcadev->susclk); >> + if (err) >> + return err; >> + } >> + >> + qca_regulator_enable(qcadev); >> + return 0; >> +} >> + >> static int qca_regulator_enable(struct qca_serdev *qcadev) >> { >> struct qca_power *power = qcadev->bt_power; >> @@ -1523,6 +1541,15 @@ static int qca_serdev_probe(struct >> serdev_device *serdev) >> >> qcadev->bt_power->vregs_on = false; >> >> + if (qcadev->btsoc_type == QCA_WCN3990 || >> + qcadev->btsoc_type == QCA_WCN3991) { > > There comes a point when using a switch statement is a lot easier to > read. See if that has been reached and if there are other places that > would benefit from a cleanup patch. >
[Bala] : will work on this.
> Regards > > Marcel
| |