Bluetooth: qca: don't disable power management for QCA6390

From: Bartosz Golaszewski <[email protected]>

We unnecessarily fallthrough the case for QCA6390 when initializing the device and hit the condition where - due to the lack of the enable-gpio
- we disable power management despite using the power sequencer. We don't need to look for clocks on this model so it makes more sense to just register the hci device and break the switch.

Reported-by Dmitry Baryshkov <[email protected]>

Fixes 4029dba6b6f1 ("Bluetooth: qca: use the power sequencer for QCA6390")

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/bluetooth/hci_qca.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

drivers/bluetooth/hci_qca.c +7 -2

index 2b31f3dc33a9..bc6a49ba26f9 100644
@@ -2402,7 +2402,13 @@ static int qca_serdev_probe(struct serdev_device *serdev)
                                                           "bluetooth");
                if (IS_ERR(qcadev->bt_power->pwrseq))
                        return PTR_ERR(qcadev->bt_power->pwrseq);
-               fallthrough;
+
+               err = hci_uart_register_device(&qcadev->serdev_hu, &qca_proto);
+               if (err) {
+                       BT_ERR("qca6390 serdev registration failed");
+                       return err;
+               }
+               break;
 
        default:
                qcadev->bt_en = devm_gpiod_get_optional(&serdev->dev, "enable",
--

2.43.0

Quoted 9 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

We unnecessarily fallthrough the case for QCA6390 when initializing the device and hit the condition where - due to the lack of the enable-gpio
- we disable power management despite using the power sequencer. We don't need to look for clocks on this model so it makes more sense to just register the hci device and break the switch.

Quoted 3 lines

Reported-by Dmitry Baryshkov <[email protected]>

Fixes 4029dba6b6f1 ("Bluetooth: qca: use the power sequencer for QCA6390")

Signed-off-by Bartosz Golaszewski <[email protected]>

Is this going to break the QCA6390 as present on M.2 / PCIe cards? Or the older DT which didn't have pwrseq entries?

Quoted 15 lines · Dmitry Baryshkov

Quoted 12 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

We unnecessarily fallthrough the case for QCA6390 when initializing the device and hit the condition where - due to the lack of the enable-gpio
- we disable power management despite using the power sequencer. We don't need to look for clocks on this model so it makes more sense to just register the hci device and break the switch.

Reported-by Dmitry Baryshkov <[email protected]>

Fixes 4029dba6b6f1 ("Bluetooth: qca: use the power sequencer for QCA6390")

Signed-off-by Bartosz Golaszewski <[email protected]>

Quoted 3 lines

Is this going to break the QCA6390 as present on M.2 / PCIe cards? Or the older DT which didn't have pwrseq entries?

Neither of these has clocks that need to be driven by linux. The only user of QCA6390 Bluetooth in mainline is RB5. Bindings didn't exist before so no commitment was ever made.

Quoted 18 lines · Bartosz Golaszewski

Quoted 15 lines · Dmitry Baryshkov

Quoted 12 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

We unnecessarily fallthrough the case for QCA6390 when initializing the device and hit the condition where - due to the lack of the enable-gpio
- we disable power management despite using the power sequencer. We don't need to look for clocks on this model so it makes more sense to just register the hci device and break the switch.

Reported-by Dmitry Baryshkov <[email protected]>

Fixes 4029dba6b6f1 ("Bluetooth: qca: use the power sequencer for QCA6390")

Signed-off-by Bartosz Golaszewski <[email protected]>

Quoted 7 lines

Quoted 3 lines

Is this going to break the QCA6390 as present on M.2 / PCIe cards? Or the older DT which didn't have pwrseq entries?

Neither of these has clocks that need to be driven by linux. The only user of QCA6390 Bluetooth in mainline is RB5. Bindings didn't exist before so no commitment was ever made.

This might make some laptop users unhappy. But anyway, restarting the hci0 now gives:

[   24.387344] Bluetooth: hci0: setting up ROME/QCA6390
[   24.387439] qcom_geni_serial 998000.serial: serial engine reports 0
RX bytes in!
[   24.554349] qcom_geni_serial 998000.serial: serial engine reports 0
RX bytes in!
[   24.562056] arm-smmu 15000000.iommu: Unhandled context fault:
fsr=0x402, iova=0xfffd1080, fsynr=0x750013, cbfrsynra=0x5a3, cb=3
[   26.914225] Bluetooth: hci0: command 0xfc00 tx timeout
[   35.042619] Bluetooth: hci0: Reading QCA version information failed (-110)
[   35.049721] Bluetooth: hci0: Retry BT power ON:0

[   37.539492] Bluetooth: hci0: command 0xfc00 tx timeout
[   45.539519] Bluetooth: hci0: Reading QCA version information failed (-110)
[   45.546667] Bluetooth: hci0: Retry BT power ON:1
[   48.035863] Bluetooth: hci0: command 0xfc00 tx timeout
[   56.034783] Bluetooth: hci0: Reading QCA version information failed (-110)
[   56.041901] Bluetooth: hci0: Retry BT power ON:2
[   58.532174] Bluetooth: hci0: command 0xfc00 tx timeout
[   66.531928] Bluetooth: hci0: Reading QCA version information failed (-110)

Quoted 40 lines · Dmitry Baryshkov

Quoted 18 lines · Bartosz Golaszewski

Quoted 15 lines · Dmitry Baryshkov

Quoted 12 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

We unnecessarily fallthrough the case for QCA6390 when initializing the device and hit the condition where - due to the lack of the enable-gpio
- we disable power management despite using the power sequencer. We don't need to look for clocks on this model so it makes more sense to just register the hci device and break the switch.

Reported-by Dmitry Baryshkov <[email protected]>

Fixes 4029dba6b6f1 ("Bluetooth: qca: use the power sequencer for QCA6390")

Signed-off-by Bartosz Golaszewski <[email protected]>

Quoted 7 lines

Quoted 3 lines

Is this going to break the QCA6390 as present on M.2 / PCIe cards? Or the older DT which didn't have pwrseq entries?

Neither of these has clocks that need to be driven by linux. The only user of QCA6390 Bluetooth in mainline is RB5. Bindings didn't exist before so no commitment was ever made.

This might make some laptop users unhappy. But anyway, restarting the hci0 now gives:

[   24.387344] Bluetooth: hci0: setting up ROME/QCA6390
[   24.387439] qcom_geni_serial 998000.serial: serial engine reports 0
RX bytes in!
[   24.554349] qcom_geni_serial 998000.serial: serial engine reports 0
RX bytes in!
[   24.562056] arm-smmu 15000000.iommu: Unhandled context fault:
fsr=0x402, iova=0xfffd1080, fsynr=0x750013, cbfrsynra=0x5a3, cb=3
[   26.914225] Bluetooth: hci0: command 0xfc00 tx timeout
[   35.042619] Bluetooth: hci0: Reading QCA version information failed (-110)
[   35.049721] Bluetooth: hci0: Retry BT power ON:0

Quoted 10 lines

[   37.539492] Bluetooth: hci0: command 0xfc00 tx timeout
[   45.539519] Bluetooth: hci0: Reading QCA version information failed (-110)
[   45.546667] Bluetooth: hci0: Retry BT power ON:1
[   48.035863] Bluetooth: hci0: command 0xfc00 tx timeout
[   56.034783] Bluetooth: hci0: Reading QCA version information failed (-110)
[   56.041901] Bluetooth: hci0: Retry BT power ON:2
[   58.532174] Bluetooth: hci0: command 0xfc00 tx timeout
[   66.531928] Bluetooth: hci0: Reading QCA version information failed (-110)

How do you reproduce it because it works fine for me:

Quoted 5 lines

Neither of these has clocks that need to be driven by linux. The only user of QCA6390 Bluetooth in mainline is RB5. Bindings didn't exist before so no commitment was ever made.

This might make some laptop users unhappy.

Quoted 6 lines · Dmitry Baryshkov

Quoted 5 lines

Neither of these has clocks that need to be driven by linux. The only user of QCA6390 Bluetooth in mainline is RB5. Bindings didn't exist before so no commitment was ever made.

Quoted 1 line

This might make some laptop users unhappy.

Like I said: without upstreamed DT bindings, we have never made any commitment about the device properties. I doubt anyone will complain though, I haven't seen any DT with QCA6390 with clock properties yet. I wouldn't stress it for now.

Quoted 9 lines · Bartosz Golaszewski

Quoted 6 lines · Dmitry Baryshkov

Quoted 5 lines

Neither of these has clocks that need to be driven by linux. The only user of QCA6390 Bluetooth in mainline is RB5. Bindings didn't exist before so no commitment was ever made.

Quoted 6 lines

Quoted 1 line

This might make some laptop users unhappy.

Like I said: without upstreamed DT bindings, we have never made any commitment about the device properties. I doubt anyone will complain though, I haven't seen any DT with QCA6390 with clock properties yet. I wouldn't stress it for now.

I was thinking about x86 laptops / M.2 cards. I'll see if I can locate one.

Quoted 18 lines · Dmitry Baryshkov

Quoted 15 lines · Bartosz Golaszewski

Quoted 7 lines · Dmitry Baryshkov

Quoted 5 lines

Neither of these has clocks that need to be driven by linux. The only user of QCA6390 Bluetooth in mainline is RB5. Bindings didn't exist before so no commitment was ever made.

This might make some laptop users unhappy.

Like I said: without upstreamed DT bindings, we have never made any commitment about the device properties. I doubt anyone will complain though, I haven't seen any DT with QCA6390 with clock properties yet. I wouldn't stress it for now.

Quoted 2 lines

I was thinking about x86 laptops / M.2 cards. I'll see if I can locate one.

I don't get it, how could they ever get the clocks property without it being defined in firmware?

Quoted 21 lines · Bartosz Golaszewski

Quoted 18 lines · Dmitry Baryshkov

Quoted 15 lines · Bartosz Golaszewski

Quoted 7 lines · Dmitry Baryshkov

Quoted 5 lines

Neither of these has clocks that need to be driven by linux. The only user of QCA6390 Bluetooth in mainline is RB5. Bindings didn't exist before so no commitment was ever made.

This might make some laptop users unhappy.

Like I said: without upstreamed DT bindings, we have never made any commitment about the device properties. I doubt anyone will complain though, I haven't seen any DT with QCA6390 with clock properties yet. I wouldn't stress it for now.

Quoted 5 lines

Quoted 2 lines

I was thinking about x86 laptops / M.2 cards. I'll see if I can locate one.

I don't get it, how could they ever get the clocks property without it being defined in firmware?

The clock and bt_en are optional.

Quoted 29 lines · Dmitry Baryshkov

Quoted 26 lines · Bartosz Golaszewski

Quoted 11 lines · Dmitry Baryshkov

Quoted 9 lines · Bartosz Golaszewski

Quoted 6 lines · Dmitry Baryshkov

Quoted 5 lines

Neither of these has clocks that need to be driven by linux. The only user of QCA6390 Bluetooth in mainline is RB5. Bindings didn't exist before so no commitment was ever made.

Quoted 9 lines

Quoted 6 lines

Quoted 1 line

This might make some laptop users unhappy.

Like I said: without upstreamed DT bindings, we have never made any commitment about the device properties. I doubt anyone will complain though, I haven't seen any DT with QCA6390 with clock properties yet. I wouldn't stress it for now.

I was thinking about x86 laptops / M.2 cards. I'll see if I can locate one.

I don't get it, how could they ever get the clocks property without it being defined in firmware?

Quoted 2 lines

The clock and bt_en are optional.

But you're worrying that the lack of this optional clock for QCA6390 will break it on some M.2 card? That cannot happen, can it? If it's on an M.2 card then it would never be described in ACPI. clk_get_optional() will always return NULL.