diff mbox series

[v1] Bluetooth: hci_qca: Replace devm_gpiod_get() with devm_gpiod_get_optional()

Message ID 20200304131645.22057-1-rjliao@codeaurora.org (mailing list archive)
State Accepted
Commit 77131dfec6af114efd32610b4a6bbecd934e37d5
Headers show
Series [v1] Bluetooth: hci_qca: Replace devm_gpiod_get() with devm_gpiod_get_optional() | expand

Commit Message

Rocky Liao March 4, 2020, 1:16 p.m. UTC
This patch replaces devm_gpiod_get() with devm_gpiod_get_optional() to get
bt_en and replaces devm_clk_get() with devm_clk_get_optional() to get
susclk. It also uses NULL check to determine whether the resource is
available or not.

Fixes: 8a208b24d770 ("Bluetooth: hci_qca: Make bt_en and susclk not mandatory for QCA Rome")
Signed-off-by: Rocky Liao <rjliao@codeaurora.org>
---
 drivers/bluetooth/hci_qca.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Marcel Holtmann March 4, 2020, 4:02 p.m. UTC | #1
Hi Rocky,

> This patch replaces devm_gpiod_get() with devm_gpiod_get_optional() to get
> bt_en and replaces devm_clk_get() with devm_clk_get_optional() to get
> susclk. It also uses NULL check to determine whether the resource is
> available or not.
> 
> Fixes: 8a208b24d770 ("Bluetooth: hci_qca: Make bt_en and susclk not mandatory for QCA Rome")
> Signed-off-by: Rocky Liao <rjliao@codeaurora.org>
> ---
> drivers/bluetooth/hci_qca.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel
Johan Hovold March 11, 2020, 9:24 a.m. UTC | #2
On Wed, Mar 04, 2020 at 09:16:45PM +0800, Rocky Liao wrote:
> This patch replaces devm_gpiod_get() with devm_gpiod_get_optional() to get
> bt_en and replaces devm_clk_get() with devm_clk_get_optional() to get
> susclk. It also uses NULL check to determine whether the resource is
> available or not.
> 
> Fixes: 8a208b24d770 ("Bluetooth: hci_qca: Make bt_en and susclk not mandatory for QCA Rome")
> Signed-off-by: Rocky Liao <rjliao@codeaurora.org>
> ---
>  drivers/bluetooth/hci_qca.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

> @@ -1901,15 +1901,15 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>  		}
>  	} else {
>  		qcadev->btsoc_type = QCA_ROME;
> -		qcadev->bt_en = devm_gpiod_get(&serdev->dev, "enable",
> +		qcadev->bt_en = devm_gpiod_get_optional(&serdev->dev, "enable",
>  					       GPIOD_OUT_LOW);
> -		if (IS_ERR(qcadev->bt_en)) {
> +		if (!qcadev->bt_en) {
>  			dev_warn(&serdev->dev, "failed to acquire enable gpio\n");

Shouldn't this be dev_dbg() if the gpio is indeed optional?

>  			power_ctrl_enabled = false;
>  		}
>  
> -		qcadev->susclk = devm_clk_get(&serdev->dev, NULL);
> -		if (IS_ERR(qcadev->susclk)) {
> +		qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL);
> +		if (!qcadev->susclk) {
>  			dev_warn(&serdev->dev, "failed to acquire clk\n");

Same here.

>  		} else {
>  			err = clk_set_rate(qcadev->susclk, SUSCLK_RATE_32KHZ);
> @@ -1924,7 +1924,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>  		err = hci_uart_register_device(&qcadev->serdev_hu, &qca_proto);
>  		if (err) {
>  			BT_ERR("Rome serdev registration failed");
> -			if (!IS_ERR(qcadev->susclk))
> +			if (qcadev->susclk)
>  				clk_disable_unprepare(qcadev->susclk);

The clock framework allows you to pass in NULL precisely to be able to
handle optional resources without sprinkling conditionals all over (the
gpio subsystem does not however).

Applies to clk_set_rate() etc. above as well.

>  			return err;
>  		}
> @@ -1945,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 if (!IS_ERR(qcadev->susclk))
> +	else if (qcadev->susclk)
>  		clk_disable_unprepare(qcadev->susclk);

Same here.

>  	hci_uart_unregister_device(&qcadev->serdev_hu);

Johan
diff mbox series

Patch

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 325baa046c3a..439392b1c043 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1562,7 +1562,7 @@  static int qca_power_on(struct hci_dev *hdev)
 		ret = qca_wcn3990_init(hu);
 	} else {
 		qcadev = serdev_device_get_drvdata(hu->serdev);
-		if (!IS_ERR(qcadev->bt_en)) {
+		if (qcadev->bt_en) {
 			gpiod_set_value_cansleep(qcadev->bt_en, 1);
 			/* Controller needs time to bootup. */
 			msleep(150);
@@ -1752,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 if (!IS_ERR(qcadev->bt_en)) {
+	} else if (qcadev->bt_en) {
 		gpiod_set_value_cansleep(qcadev->bt_en, 0);
 	}
 }
@@ -1901,15 +1901,15 @@  static int qca_serdev_probe(struct serdev_device *serdev)
 		}
 	} else {
 		qcadev->btsoc_type = QCA_ROME;
-		qcadev->bt_en = devm_gpiod_get(&serdev->dev, "enable",
+		qcadev->bt_en = devm_gpiod_get_optional(&serdev->dev, "enable",
 					       GPIOD_OUT_LOW);
-		if (IS_ERR(qcadev->bt_en)) {
+		if (!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)) {
+		qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL);
+		if (!qcadev->susclk) {
 			dev_warn(&serdev->dev, "failed to acquire clk\n");
 		} else {
 			err = clk_set_rate(qcadev->susclk, SUSCLK_RATE_32KHZ);
@@ -1924,7 +1924,7 @@  static int qca_serdev_probe(struct serdev_device *serdev)
 		err = hci_uart_register_device(&qcadev->serdev_hu, &qca_proto);
 		if (err) {
 			BT_ERR("Rome serdev registration failed");
-			if (!IS_ERR(qcadev->susclk))
+			if (qcadev->susclk)
 				clk_disable_unprepare(qcadev->susclk);
 			return err;
 		}
@@ -1945,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 if (!IS_ERR(qcadev->susclk))
+	else if (qcadev->susclk)
 		clk_disable_unprepare(qcadev->susclk);
 
 	hci_uart_unregister_device(&qcadev->serdev_hu);