* [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() @ 2024-04-22 13:00 Bartosz Golaszewski 2024-04-24 4:05 ` Krzysztof Kozlowski ` (2 more replies) 0 siblings, 3 replies; 32+ messages in thread From: Bartosz Golaszewski @ 2024-04-22 13:00 UTC (permalink / raw) To: Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski Cc: linux-bluetooth, linux-kernel, Bartosz Golaszewski, Wren Turkal, Zijun Hu From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Any return value from gpiod_get_optional() other than a pointer to a GPIO descriptor or a NULL-pointer is an error and the driver should abort probing. That being said: commit 56d074d26c58 ("Bluetooth: hci_qca: don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets power_ctrl_enabled on NULL-pointer returned by devm_gpiod_get_optional(). Restore this behavior but bail-out on errors. Reported-by: Wren Turkal <wt@penguintechs.org> Reported-by: Zijun Hu <quic_zijuhu@quicinc.com> Closes: https://lore.kernel.org/linux-bluetooth/1713449192-25926-2-git-send-email-quic_zijuhu@quicinc.com/ Fixes: 56d074d26c58 ("Bluetooth: hci_qca: don't use IS_ERR_OR_NULL() with gpiod_get_optional()") Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> --- drivers/bluetooth/hci_qca.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c index 92fa20f5ac7d..739248c6d6b9 100644 --- a/drivers/bluetooth/hci_qca.c +++ b/drivers/bluetooth/hci_qca.c @@ -2327,9 +2327,12 @@ static int qca_serdev_probe(struct serdev_device *serdev) (data->soc_type == QCA_WCN6750 || data->soc_type == QCA_WCN6855)) { dev_err(&serdev->dev, "failed to acquire BT_EN gpio\n"); - power_ctrl_enabled = false; + return PTR_ERR(qcadev->bt_en); } + if (!qcadev->bt_en) + power_ctrl_enabled = false; + qcadev->sw_ctrl = devm_gpiod_get_optional(&serdev->dev, "swctrl", GPIOD_IN); if (IS_ERR(qcadev->sw_ctrl) && -- 2.40.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-22 13:00 [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() Bartosz Golaszewski @ 2024-04-24 4:05 ` Krzysztof Kozlowski 2024-04-24 4:55 ` quic_zijuhu 2024-04-24 5:07 ` Wren Turkal 2 siblings, 0 replies; 32+ messages in thread From: Krzysztof Kozlowski @ 2024-04-24 4:05 UTC (permalink / raw) To: Bartosz Golaszewski, Marcel Holtmann, Luiz Augusto von Dentz Cc: linux-bluetooth, linux-kernel, Bartosz Golaszewski, Wren Turkal, Zijun Hu On 22/04/2024 15:00, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Any return value from gpiod_get_optional() other than a pointer to a > GPIO descriptor or a NULL-pointer is an error and the driver should > abort probing. That being said: commit 56d074d26c58 ("Bluetooth: hci_qca: > don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets > power_ctrl_enabled on NULL-pointer returned by > devm_gpiod_get_optional(). Restore this behavior but bail-out on errors. > > Reported-by: Wren Turkal <wt@penguintechs.org> > Reported-by: Zijun Hu <quic_zijuhu@quicinc.com> > Closes: https://lore.kernel.org/linux-bluetooth/1713449192-25926-2-git-send-email-quic_zijuhu@quicinc.com/ > Fixes: 56d074d26c58 ("Bluetooth: hci_qca: don't use IS_ERR_OR_NULL() with gpiod_get_optional()") > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-22 13:00 [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() Bartosz Golaszewski 2024-04-24 4:05 ` Krzysztof Kozlowski @ 2024-04-24 4:55 ` quic_zijuhu 2024-04-24 5:07 ` Wren Turkal 2 siblings, 0 replies; 32+ messages in thread From: quic_zijuhu @ 2024-04-24 4:55 UTC (permalink / raw) To: Bartosz Golaszewski, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski Cc: linux-bluetooth, linux-kernel, Bartosz Golaszewski, Wren Turkal, Greg KH On 4/22/2024 9:00 PM, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Any return value from gpiod_get_optional() other than a pointer to a > GPIO descriptor or a NULL-pointer is an error and the driver should > abort probing. That being said: commit 56d074d26c58 ("Bluetooth: hci_qca: > don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets > power_ctrl_enabled on NULL-pointer returned by > devm_gpiod_get_optional(). Restore this behavior but bail-out on errors. > > Reported-by: Wren Turkal <wt@penguintechs.org> > Reported-by: Zijun Hu <quic_zijuhu@quicinc.com> > Closes: https://lore.kernel.org/linux-bluetooth/1713449192-25926-2-git-send-email-quic_zijuhu@quicinc.com/ > Fixes: 56d074d26c58 ("Bluetooth: hci_qca: don't use IS_ERR_OR_NULL() with gpiod_get_optional()") > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- > drivers/bluetooth/hci_qca.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c > index 92fa20f5ac7d..739248c6d6b9 100644 > --- a/drivers/bluetooth/hci_qca.c > +++ b/drivers/bluetooth/hci_qca.c > @@ -2327,9 +2327,12 @@ static int qca_serdev_probe(struct serdev_device *serdev) > (data->soc_type == QCA_WCN6750 || > data->soc_type == QCA_WCN6855)) { > dev_err(&serdev->dev, "failed to acquire BT_EN gpio\n"); > - power_ctrl_enabled = false; > + return PTR_ERR(qcadev->bt_en); > } > For QCA6390, control logic do not reach here, so it has no any help for QCA6390. > + if (!qcadev->bt_en) > + power_ctrl_enabled = false; > + property enable-gpios are obviously marked as required for WCN6750 and WCN6855 by dts binding spec as shown by below link: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml if user don't config the property then "qcadev->bt_en is NULL", can we still treat this case as good case? that is why i changed my fix from reverting the wrong commit to only focusing on QCA6390. i will give different fix for other impacted QCA controllers. > qcadev->sw_ctrl = devm_gpiod_get_optional(&serdev->dev, "swctrl", > GPIOD_IN); > if (IS_ERR(qcadev->sw_ctrl) && NAK as explained above. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-22 13:00 [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() Bartosz Golaszewski 2024-04-24 4:05 ` Krzysztof Kozlowski 2024-04-24 4:55 ` quic_zijuhu @ 2024-04-24 5:07 ` Wren Turkal 2024-04-24 9:04 ` Bartosz Golaszewski 2 siblings, 1 reply; 32+ messages in thread From: Wren Turkal @ 2024-04-24 5:07 UTC (permalink / raw) To: Bartosz Golaszewski, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski Cc: linux-bluetooth, linux-kernel, Bartosz Golaszewski, Zijun Hu On 4/22/24 6:00 AM, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Any return value from gpiod_get_optional() other than a pointer to a > GPIO descriptor or a NULL-pointer is an error and the driver should > abort probing. That being said: commit 56d074d26c58 ("Bluetooth: hci_qca: > don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets > power_ctrl_enabled on NULL-pointer returned by > devm_gpiod_get_optional(). Restore this behavior but bail-out on errors. Nack. This patch does fixes neither the disable/re-enable problem nor the warm boot problem. Zijun replied to this patch also with what I think is the proper reasoning for why it doesn't fix my setup. > > Reported-by: Wren Turkal <wt@penguintechs.org> > Reported-by: Zijun Hu <quic_zijuhu@quicinc.com> > Closes: https://lore.kernel.org/linux-bluetooth/1713449192-25926-2-git-send-email-quic_zijuhu@quicinc.com/ > Fixes: 56d074d26c58 ("Bluetooth: hci_qca: don't use IS_ERR_OR_NULL() with gpiod_get_optional()") > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- > drivers/bluetooth/hci_qca.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c > index 92fa20f5ac7d..739248c6d6b9 100644 > --- a/drivers/bluetooth/hci_qca.c > +++ b/drivers/bluetooth/hci_qca.c > @@ -2327,9 +2327,12 @@ static int qca_serdev_probe(struct serdev_device *serdev) > (data->soc_type == QCA_WCN6750 || > data->soc_type == QCA_WCN6855)) { > dev_err(&serdev->dev, "failed to acquire BT_EN gpio\n"); > - power_ctrl_enabled = false; > + return PTR_ERR(qcadev->bt_en); > } > > + if (!qcadev->bt_en) > + power_ctrl_enabled = false; > + > qcadev->sw_ctrl = devm_gpiod_get_optional(&serdev->dev, "swctrl", > GPIOD_IN); > if (IS_ERR(qcadev->sw_ctrl) && -- You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 5:07 ` Wren Turkal @ 2024-04-24 9:04 ` Bartosz Golaszewski 2024-04-24 9:32 ` quic_zijuhu ` (2 more replies) 0 siblings, 3 replies; 32+ messages in thread From: Bartosz Golaszewski @ 2024-04-24 9:04 UTC (permalink / raw) To: Wren Turkal Cc: linux-bluetooth, linux-kernel, Bartosz Golaszewski, Zijun Hu, Bartosz Golaszewski, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On Wed, 24 Apr 2024 07:07:05 +0200, Wren Turkal <wt@penguintechs.org> said: > On 4/22/24 6:00 AM, Bartosz Golaszewski wrote: >> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >> >> Any return value from gpiod_get_optional() other than a pointer to a >> GPIO descriptor or a NULL-pointer is an error and the driver should >> abort probing. That being said: commit 56d074d26c58 ("Bluetooth: hci_qca: >> don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets >> power_ctrl_enabled on NULL-pointer returned by >> devm_gpiod_get_optional(). Restore this behavior but bail-out on errors. > > Nack. This patch does fixes neither the disable/re-enable problem nor > the warm boot problem. > > Zijun replied to this patch also with what I think is the proper > reasoning for why it doesn't fix my setup. > Indeed, I only addressed a single issue here and not the code under the default: label of the switch case. Sorry. Could you give the following diff a try? Bart diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c index 92fa20f5ac7d..0e98ad2c0c9d 100644 --- a/drivers/bluetooth/hci_qca.c +++ b/drivers/bluetooth/hci_qca.c @@ -2327,16 +2327,21 @@ static int qca_serdev_probe(struct serdev_device *serdev) (data->soc_type == QCA_WCN6750 || data->soc_type == QCA_WCN6855)) { dev_err(&serdev->dev, "failed to acquire BT_EN gpio\n"); - power_ctrl_enabled = false; + return PTR_ERR(qcadev->bt_en); } + if (!qcadev->bt_en) + power_ctrl_enabled = false; + qcadev->sw_ctrl = devm_gpiod_get_optional(&serdev->dev, "swctrl", GPIOD_IN); if (IS_ERR(qcadev->sw_ctrl) && (data->soc_type == QCA_WCN6750 || data->soc_type == QCA_WCN6855 || - data->soc_type == QCA_WCN7850)) - dev_warn(&serdev->dev, "failed to acquire SW_CTRL gpio\n"); + data->soc_type == QCA_WCN7850)) { + dev_err(&serdev->dev, "failed to acquire SW_CTRL gpio\n"); + return PTR_ERR(qcadev->sw_ctrl); + } qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL); if (IS_ERR(qcadev->susclk)) { @@ -2355,10 +2360,13 @@ static int qca_serdev_probe(struct serdev_device *serdev) qcadev->bt_en = devm_gpiod_get_optional(&serdev->dev, "enable", GPIOD_OUT_LOW); if (IS_ERR(qcadev->bt_en)) { - dev_warn(&serdev->dev, "failed to acquire enable gpio\n"); - power_ctrl_enabled = false; + dev_err(&serdev->dev, "failed to acquire enable gpio\n"); + return PTR_ERR(qcadev->bt_en); } + if (!qcadev->bt_en) + power_ctrl_enabled = false; + qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL); if (IS_ERR(qcadev->susclk)) { dev_warn(&serdev->dev, "failed to acquire clk\n"); ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 9:04 ` Bartosz Golaszewski @ 2024-04-24 9:32 ` quic_zijuhu 2024-04-24 9:50 ` Krzysztof Kozlowski 2024-04-24 11:16 ` Wren Turkal 2024-04-24 11:25 ` Wren Turkal 2 siblings, 1 reply; 32+ messages in thread From: quic_zijuhu @ 2024-04-24 9:32 UTC (permalink / raw) To: Bartosz Golaszewski, Wren Turkal Cc: linux-bluetooth, linux-kernel, Bartosz Golaszewski, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/2024 5:04 PM, Bartosz Golaszewski wrote: > On Wed, 24 Apr 2024 07:07:05 +0200, Wren Turkal <wt@penguintechs.org> said: >> On 4/22/24 6:00 AM, Bartosz Golaszewski wrote: >>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>> >>> Any return value from gpiod_get_optional() other than a pointer to a >>> GPIO descriptor or a NULL-pointer is an error and the driver should >>> abort probing. That being said: commit 56d074d26c58 ("Bluetooth: hci_qca: >>> don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets >>> power_ctrl_enabled on NULL-pointer returned by >>> devm_gpiod_get_optional(). Restore this behavior but bail-out on errors. >> >> Nack. This patch does fixes neither the disable/re-enable problem nor >> the warm boot problem. >> >> Zijun replied to this patch also with what I think is the proper >> reasoning for why it doesn't fix my setup. >> > > Indeed, I only addressed a single issue here and not the code under the > default: label of the switch case. Sorry. > > Could you give the following diff a try? > > Bart > > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c > index 92fa20f5ac7d..0e98ad2c0c9d 100644 > --- a/drivers/bluetooth/hci_qca.c > +++ b/drivers/bluetooth/hci_qca.c > @@ -2327,16 +2327,21 @@ static int qca_serdev_probe(struct > serdev_device *serdev) > (data->soc_type == QCA_WCN6750 || > data->soc_type == QCA_WCN6855)) { > dev_err(&serdev->dev, "failed to acquire BT_EN gpio\n"); > - power_ctrl_enabled = false; > + return PTR_ERR(qcadev->bt_en); > } > > + if (!qcadev->bt_en) > + power_ctrl_enabled = false; > + > qcadev->sw_ctrl = devm_gpiod_get_optional(&serdev->dev, "swctrl", > GPIOD_IN); > if (IS_ERR(qcadev->sw_ctrl) && > (data->soc_type == QCA_WCN6750 || > data->soc_type == QCA_WCN6855 || > - data->soc_type == QCA_WCN7850)) > - dev_warn(&serdev->dev, "failed to acquire SW_CTRL gpio\n"); > + data->soc_type == QCA_WCN7850)) { > + dev_err(&serdev->dev, "failed to acquire SW_CTRL gpio\n"); > + return PTR_ERR(qcadev->sw_ctrl); > + } > > qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL); > if (IS_ERR(qcadev->susclk)) { > @@ -2355,10 +2360,13 @@ static int qca_serdev_probe(struct > serdev_device *serdev) > qcadev->bt_en = devm_gpiod_get_optional(&serdev->dev, "enable", > GPIOD_OUT_LOW); > if (IS_ERR(qcadev->bt_en)) { > - dev_warn(&serdev->dev, "failed to acquire enable gpio\n"); > - power_ctrl_enabled = false; > + dev_err(&serdev->dev, "failed to acquire enable gpio\n"); > + return PTR_ERR(qcadev->bt_en); > } > > + if (!qcadev->bt_en) > + power_ctrl_enabled = false; > + > qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL); > if (IS_ERR(qcadev->susclk)) { > dev_warn(&serdev->dev, "failed to acquire clk\n"); i suggest stop here and request you code review for my changes, i found the issue and given fix for my concern. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 9:32 ` quic_zijuhu @ 2024-04-24 9:50 ` Krzysztof Kozlowski 0 siblings, 0 replies; 32+ messages in thread From: Krzysztof Kozlowski @ 2024-04-24 9:50 UTC (permalink / raw) To: quic_zijuhu, Bartosz Golaszewski, Wren Turkal Cc: linux-bluetooth, linux-kernel, Bartosz Golaszewski, Marcel Holtmann, Luiz Augusto von Dentz On 24/04/2024 11:32, quic_zijuhu wrote: > On 4/24/2024 5:04 PM, Bartosz Golaszewski wrote: >> On Wed, 24 Apr 2024 07:07:05 +0200, Wren Turkal <wt@penguintechs.org> said: >>> On 4/22/24 6:00 AM, Bartosz Golaszewski wrote: >>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>> >>>> Any return value from gpiod_get_optional() other than a pointer to a >>>> GPIO descriptor or a NULL-pointer is an error and the driver should >>>> abort probing. That being said: commit 56d074d26c58 ("Bluetooth: hci_qca: >>>> don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets >>>> power_ctrl_enabled on NULL-pointer returned by >>>> devm_gpiod_get_optional(). Restore this behavior but bail-out on errors. >>> >>> Nack. This patch does fixes neither the disable/re-enable problem nor >>> the warm boot problem. >>> >>> Zijun replied to this patch also with what I think is the proper >>> reasoning for why it doesn't fix my setup. >>> >> >> Indeed, I only addressed a single issue here and not the code under the >> default: label of the switch case. Sorry. >> >> Could you give the following diff a try? >> >> Bart >> >> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >> index 92fa20f5ac7d..0e98ad2c0c9d 100644 >> --- a/drivers/bluetooth/hci_qca.c >> +++ b/drivers/bluetooth/hci_qca.c >> @@ -2327,16 +2327,21 @@ static int qca_serdev_probe(struct >> serdev_device *serdev) >> (data->soc_type == QCA_WCN6750 || >> data->soc_type == QCA_WCN6855)) { >> dev_err(&serdev->dev, "failed to acquire BT_EN gpio\n"); >> - power_ctrl_enabled = false; >> + return PTR_ERR(qcadev->bt_en); >> } >> >> + if (!qcadev->bt_en) >> + power_ctrl_enabled = false; >> + >> qcadev->sw_ctrl = devm_gpiod_get_optional(&serdev->dev, "swctrl", >> GPIOD_IN); >> if (IS_ERR(qcadev->sw_ctrl) && >> (data->soc_type == QCA_WCN6750 || >> data->soc_type == QCA_WCN6855 || >> - data->soc_type == QCA_WCN7850)) >> - dev_warn(&serdev->dev, "failed to acquire SW_CTRL gpio\n"); >> + data->soc_type == QCA_WCN7850)) { >> + dev_err(&serdev->dev, "failed to acquire SW_CTRL gpio\n"); >> + return PTR_ERR(qcadev->sw_ctrl); >> + } >> >> qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL); >> if (IS_ERR(qcadev->susclk)) { >> @@ -2355,10 +2360,13 @@ static int qca_serdev_probe(struct >> serdev_device *serdev) >> qcadev->bt_en = devm_gpiod_get_optional(&serdev->dev, "enable", >> GPIOD_OUT_LOW); >> if (IS_ERR(qcadev->bt_en)) { >> - dev_warn(&serdev->dev, "failed to acquire enable gpio\n"); >> - power_ctrl_enabled = false; >> + dev_err(&serdev->dev, "failed to acquire enable gpio\n"); >> + return PTR_ERR(qcadev->bt_en); >> } >> >> + if (!qcadev->bt_en) >> + power_ctrl_enabled = false; >> + >> qcadev->susclk = devm_clk_get_optional(&serdev->dev, NULL); >> if (IS_ERR(qcadev->susclk)) { >> dev_warn(&serdev->dev, "failed to acquire clk\n"); > i suggest stop here and request you code review for my changes, i found > the issue and given fix for my concern. What are you answering to? What the heck are you implying here? I think this crosses some threshold of ridiculous mailings. Please get your managers or colleagues to review your patches and process you follow. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 9:04 ` Bartosz Golaszewski 2024-04-24 9:32 ` quic_zijuhu @ 2024-04-24 11:16 ` Wren Turkal 2024-04-24 11:53 ` Wren Turkal 2024-04-24 11:25 ` Wren Turkal 2 siblings, 1 reply; 32+ messages in thread From: Wren Turkal @ 2024-04-24 11:16 UTC (permalink / raw) To: Bartosz Golaszewski Cc: linux-bluetooth, linux-kernel, Bartosz Golaszewski, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/24 2:04 AM, Bartosz Golaszewski wrote: > On Wed, 24 Apr 2024 07:07:05 +0200, Wren Turkal<wt@penguintechs.org> said: >> On 4/22/24 6:00 AM, Bartosz Golaszewski wrote: >>> From: Bartosz Golaszewski<bartosz.golaszewski@linaro.org> >>> >>> Any return value from gpiod_get_optional() other than a pointer to a >>> GPIO descriptor or a NULL-pointer is an error and the driver should >>> abort probing. That being said: commit 56d074d26c58 ("Bluetooth: hci_qca: >>> don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets >>> power_ctrl_enabled on NULL-pointer returned by >>> devm_gpiod_get_optional(). Restore this behavior but bail-out on errors. >> Nack. This patch does fixes neither the disable/re-enable problem nor >> the warm boot problem. >> >> Zijun replied to this patch also with what I think is the proper >> reasoning for why it doesn't fix my setup. >> > Indeed, I only addressed a single issue here and not the code under the > default: label of the switch case. Sorry. > > Could you give the following diff a try? I had a feeling that was what was going on. I'll give the patch a shot. wt -- You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 11:16 ` Wren Turkal @ 2024-04-24 11:53 ` Wren Turkal 2024-04-24 11:56 ` Bartosz Golaszewski 0 siblings, 1 reply; 32+ messages in thread From: Wren Turkal @ 2024-04-24 11:53 UTC (permalink / raw) To: Bartosz Golaszewski Cc: linux-bluetooth, linux-kernel, Bartosz Golaszewski, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/24 4:16 AM, Wren Turkal wrote: > On 4/24/24 2:04 AM, Bartosz Golaszewski wrote: >> On Wed, 24 Apr 2024 07:07:05 +0200, Wren Turkal<wt@penguintechs.org> >> said: >>> On 4/22/24 6:00 AM, Bartosz Golaszewski wrote: >>>> From: Bartosz Golaszewski<bartosz.golaszewski@linaro.org> >>>> >>>> Any return value from gpiod_get_optional() other than a pointer to a >>>> GPIO descriptor or a NULL-pointer is an error and the driver should >>>> abort probing. That being said: commit 56d074d26c58 ("Bluetooth: >>>> hci_qca: >>>> don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets >>>> power_ctrl_enabled on NULL-pointer returned by >>>> devm_gpiod_get_optional(). Restore this behavior but bail-out on >>>> errors. >>> Nack. This patch does fixes neither the disable/re-enable problem nor >>> the warm boot problem. >>> >>> Zijun replied to this patch also with what I think is the proper >>> reasoning for why it doesn't fix my setup. >>> >> Indeed, I only addressed a single issue here and not the code under the >> default: label of the switch case. Sorry. >> >> Could you give the following diff a try? > > I had a feeling that was what was going on. I'll give the patch a shot. > > wt Considering this patch is basically equivalent to patch 1/2 from Zijun, I am not surprised that is works similarly. I.e. on a cold boot, I can disable/re-enable bluetooth as many time as I want. However, since this patch doesn't include the quirk fix from Zijun's patchset (patch 2/2), bluetooth fails to work after a warm boot. @Zijun, this patch looks more idiomatic when I look at the surrounding code than your patch 1/2. Notice how it doesn't use the "else if" construct. It does the NULL test separately after checking for errors. -- You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 11:53 ` Wren Turkal @ 2024-04-24 11:56 ` Bartosz Golaszewski 2024-04-24 12:09 ` Wren Turkal 2024-04-24 12:17 ` Wren Turkal 0 siblings, 2 replies; 32+ messages in thread From: Bartosz Golaszewski @ 2024-04-24 11:56 UTC (permalink / raw) To: Wren Turkal Cc: linux-bluetooth, linux-kernel, Bartosz Golaszewski, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On Wed, Apr 24, 2024 at 1:53 PM Wren Turkal <wt@penguintechs.org> wrote: > > On 4/24/24 4:16 AM, Wren Turkal wrote: > > On 4/24/24 2:04 AM, Bartosz Golaszewski wrote: > >> On Wed, 24 Apr 2024 07:07:05 +0200, Wren Turkal<wt@penguintechs.org> > >> said: > >>> On 4/22/24 6:00 AM, Bartosz Golaszewski wrote: > >>>> From: Bartosz Golaszewski<bartosz.golaszewski@linaro.org> > >>>> > >>>> Any return value from gpiod_get_optional() other than a pointer to a > >>>> GPIO descriptor or a NULL-pointer is an error and the driver should > >>>> abort probing. That being said: commit 56d074d26c58 ("Bluetooth: > >>>> hci_qca: > >>>> don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets > >>>> power_ctrl_enabled on NULL-pointer returned by > >>>> devm_gpiod_get_optional(). Restore this behavior but bail-out on > >>>> errors. > >>> Nack. This patch does fixes neither the disable/re-enable problem nor > >>> the warm boot problem. > >>> > >>> Zijun replied to this patch also with what I think is the proper > >>> reasoning for why it doesn't fix my setup. > >>> > >> Indeed, I only addressed a single issue here and not the code under the > >> default: label of the switch case. Sorry. > >> > >> Could you give the following diff a try? > > > > I had a feeling that was what was going on. I'll give the patch a shot. > > > > wt > > Considering this patch is basically equivalent to patch 1/2 from Zijun, > I am not surprised that is works similarly. I.e. on a cold boot, I can > disable/re-enable bluetooth as many time as I want. > Zijun didn't bail out on errors which is the issue the original patch tried to address and this one preserves. > However, since this patch doesn't include the quirk fix from Zijun's > patchset (patch 2/2), bluetooth fails to work after a warm boot. > That's OK, we have the first part right. Let's now see if we can reuse patch 2/2 from Zijun. > @Zijun, this patch looks more idiomatic when I look at the surrounding > code than your patch 1/2. Notice how it doesn't use the "else if" > construct. It does the NULL test separately after checking for errors. > > -- > You're more amazing than you think! Bart ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 11:56 ` Bartosz Golaszewski @ 2024-04-24 12:09 ` Wren Turkal 2024-04-24 12:17 ` Wren Turkal 1 sibling, 0 replies; 32+ messages in thread From: Wren Turkal @ 2024-04-24 12:09 UTC (permalink / raw) To: Bartosz Golaszewski Cc: linux-bluetooth, linux-kernel, Bartosz Golaszewski, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/24 4:56 AM, Bartosz Golaszewski wrote: > On Wed, Apr 24, 2024 at 1:53 PM Wren Turkal <wt@penguintechs.org> wrote: >> >> On 4/24/24 4:16 AM, Wren Turkal wrote: >>> On 4/24/24 2:04 AM, Bartosz Golaszewski wrote: >>>> On Wed, 24 Apr 2024 07:07:05 +0200, Wren Turkal<wt@penguintechs.org> >>>> said: >>>>> On 4/22/24 6:00 AM, Bartosz Golaszewski wrote: >>>>>> From: Bartosz Golaszewski<bartosz.golaszewski@linaro.org> >>>>>> >>>>>> Any return value from gpiod_get_optional() other than a pointer to a >>>>>> GPIO descriptor or a NULL-pointer is an error and the driver should >>>>>> abort probing. That being said: commit 56d074d26c58 ("Bluetooth: >>>>>> hci_qca: >>>>>> don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets >>>>>> power_ctrl_enabled on NULL-pointer returned by >>>>>> devm_gpiod_get_optional(). Restore this behavior but bail-out on >>>>>> errors. >>>>> Nack. This patch does fixes neither the disable/re-enable problem nor >>>>> the warm boot problem. >>>>> >>>>> Zijun replied to this patch also with what I think is the proper >>>>> reasoning for why it doesn't fix my setup. >>>>> >>>> Indeed, I only addressed a single issue here and not the code under the >>>> default: label of the switch case. Sorry. >>>> >>>> Could you give the following diff a try? >>> >>> I had a feeling that was what was going on. I'll give the patch a shot. >>> >>> wt >> >> Considering this patch is basically equivalent to patch 1/2 from Zijun, >> I am not surprised that is works similarly. I.e. on a cold boot, I can >> disable/re-enable bluetooth as many time as I want. >> > > Zijun didn't bail out on errors which is the issue the original patch > tried to address and this one preserves. Ah, I see that. Yeah, Zijun's 1/2 doesn't have the returns. >> However, since this patch doesn't include the quirk fix from Zijun's >> patchset (patch 2/2), bluetooth fails to work after a warm boot. >> > > That's OK, we have the first part right. Let's now see if we can reuse > patch 2/2 from Zijun. > >> @Zijun, this patch looks more idiomatic when I look at the surrounding >> code than your patch 1/2. Notice how it doesn't use the "else if" >> construct. It does the NULL test separately after checking for errors. >> >> -- >> You're more amazing than you think! > > Bart -- You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 11:56 ` Bartosz Golaszewski 2024-04-24 12:09 ` Wren Turkal @ 2024-04-24 12:17 ` Wren Turkal 2024-04-24 12:20 ` Bartosz Golaszewski 1 sibling, 1 reply; 32+ messages in thread From: Wren Turkal @ 2024-04-24 12:17 UTC (permalink / raw) To: Bartosz Golaszewski Cc: linux-bluetooth, linux-kernel, Bartosz Golaszewski, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/24 4:56 AM, Bartosz Golaszewski wrote: > On Wed, Apr 24, 2024 at 1:53 PM Wren Turkal <wt@penguintechs.org> wrote: >> >> On 4/24/24 4:16 AM, Wren Turkal wrote: >>> On 4/24/24 2:04 AM, Bartosz Golaszewski wrote: >>>> On Wed, 24 Apr 2024 07:07:05 +0200, Wren Turkal<wt@penguintechs.org> >>>> said: >>>>> On 4/22/24 6:00 AM, Bartosz Golaszewski wrote: >>>>>> From: Bartosz Golaszewski<bartosz.golaszewski@linaro.org> >>>>>> >>>>>> Any return value from gpiod_get_optional() other than a pointer to a >>>>>> GPIO descriptor or a NULL-pointer is an error and the driver should >>>>>> abort probing. That being said: commit 56d074d26c58 ("Bluetooth: >>>>>> hci_qca: >>>>>> don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets >>>>>> power_ctrl_enabled on NULL-pointer returned by >>>>>> devm_gpiod_get_optional(). Restore this behavior but bail-out on >>>>>> errors. >>>>> Nack. This patch does fixes neither the disable/re-enable problem nor >>>>> the warm boot problem. >>>>> >>>>> Zijun replied to this patch also with what I think is the proper >>>>> reasoning for why it doesn't fix my setup. >>>>> >>>> Indeed, I only addressed a single issue here and not the code under the >>>> default: label of the switch case. Sorry. >>>> >>>> Could you give the following diff a try? >>> >>> I had a feeling that was what was going on. I'll give the patch a shot. >>> >>> wt >> >> Considering this patch is basically equivalent to patch 1/2 from Zijun, >> I am not surprised that is works similarly. I.e. on a cold boot, I can >> disable/re-enable bluetooth as many time as I want. >> > > Zijun didn't bail out on errors which is the issue the original patch > tried to address and this one preserves. > >> However, since this patch doesn't include the quirk fix from Zijun's >> patchset (patch 2/2), bluetooth fails to work after a warm boot. >> > > That's OK, we have the first part right. Let's now see if we can reuse > patch 2/2 from Zijun. I'm compiling it right now. Be back soon. >> @Zijun, this patch looks more idiomatic when I look at the surrounding >> code than your patch 1/2. Notice how it doesn't use the "else if" >> construct. It does the NULL test separately after checking for errors. >> >> -- >> You're more amazing than you think! > > Bart -- You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 12:17 ` Wren Turkal @ 2024-04-24 12:20 ` Bartosz Golaszewski 2024-04-24 12:23 ` Bartosz Golaszewski 2024-04-24 12:24 ` Wren Turkal 0 siblings, 2 replies; 32+ messages in thread From: Bartosz Golaszewski @ 2024-04-24 12:20 UTC (permalink / raw) To: Wren Turkal Cc: Bartosz Golaszewski, linux-bluetooth, linux-kernel, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On Wed, 24 Apr 2024 at 14:17, Wren Turkal <wt@penguintechs.org> wrote: > > On 4/24/24 4:56 AM, Bartosz Golaszewski wrote: > > On Wed, Apr 24, 2024 at 1:53 PM Wren Turkal <wt@penguintechs.org> wrote: > >> > >> On 4/24/24 4:16 AM, Wren Turkal wrote: > >>> On 4/24/24 2:04 AM, Bartosz Golaszewski wrote: > >>>> On Wed, 24 Apr 2024 07:07:05 +0200, Wren Turkal<wt@penguintechs.org> > >>>> said: > >>>>> On 4/22/24 6:00 AM, Bartosz Golaszewski wrote: > >>>>>> From: Bartosz Golaszewski<bartosz.golaszewski@linaro.org> > >>>>>> > >>>>>> Any return value from gpiod_get_optional() other than a pointer to a > >>>>>> GPIO descriptor or a NULL-pointer is an error and the driver should > >>>>>> abort probing. That being said: commit 56d074d26c58 ("Bluetooth: > >>>>>> hci_qca: > >>>>>> don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets > >>>>>> power_ctrl_enabled on NULL-pointer returned by > >>>>>> devm_gpiod_get_optional(). Restore this behavior but bail-out on > >>>>>> errors. > >>>>> Nack. This patch does fixes neither the disable/re-enable problem nor > >>>>> the warm boot problem. > >>>>> > >>>>> Zijun replied to this patch also with what I think is the proper > >>>>> reasoning for why it doesn't fix my setup. > >>>>> > >>>> Indeed, I only addressed a single issue here and not the code under the > >>>> default: label of the switch case. Sorry. > >>>> > >>>> Could you give the following diff a try? > >>> > >>> I had a feeling that was what was going on. I'll give the patch a shot. > >>> > >>> wt > >> > >> Considering this patch is basically equivalent to patch 1/2 from Zijun, > >> I am not surprised that is works similarly. I.e. on a cold boot, I can > >> disable/re-enable bluetooth as many time as I want. > >> > > > > Zijun didn't bail out on errors which is the issue the original patch > > tried to address and this one preserves. > > > >> However, since this patch doesn't include the quirk fix from Zijun's > >> patchset (patch 2/2), bluetooth fails to work after a warm boot. > >> > > > > That's OK, we have the first part right. Let's now see if we can reuse > > patch 2/2 from Zijun. > > I'm compiling it right now. Be back soon. > Well I doubt it's correct as it removed Krzysztof's fix which looks right. If I were to guess I'd say we need some mix of both. Bart > >> @Zijun, this patch looks more idiomatic when I look at the surrounding > >> code than your patch 1/2. Notice how it doesn't use the "else if" > >> construct. It does the NULL test separately after checking for errors. > >> > >> -- > >> You're more amazing than you think! > > > > Bart > > -- > You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 12:20 ` Bartosz Golaszewski @ 2024-04-24 12:23 ` Bartosz Golaszewski 2024-04-24 12:24 ` Wren Turkal 1 sibling, 0 replies; 32+ messages in thread From: Bartosz Golaszewski @ 2024-04-24 12:23 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Wren Turkal, linux-bluetooth, linux-kernel, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On Wed, Apr 24, 2024 at 2:20 PM Bartosz Golaszewski <bartosz.golaszewski@linaro.org> wrote: > > On Wed, 24 Apr 2024 at 14:17, Wren Turkal <wt@penguintechs.org> wrote: > > > > On 4/24/24 4:56 AM, Bartosz Golaszewski wrote: > > > On Wed, Apr 24, 2024 at 1:53 PM Wren Turkal <wt@penguintechs.org> wrote: > > >> > > >> On 4/24/24 4:16 AM, Wren Turkal wrote: > > >>> On 4/24/24 2:04 AM, Bartosz Golaszewski wrote: > > >>>> On Wed, 24 Apr 2024 07:07:05 +0200, Wren Turkal<wt@penguintechs.org> > > >>>> said: > > >>>>> On 4/22/24 6:00 AM, Bartosz Golaszewski wrote: > > >>>>>> From: Bartosz Golaszewski<bartosz.golaszewski@linaro.org> > > >>>>>> > > >>>>>> Any return value from gpiod_get_optional() other than a pointer to a > > >>>>>> GPIO descriptor or a NULL-pointer is an error and the driver should > > >>>>>> abort probing. That being said: commit 56d074d26c58 ("Bluetooth: > > >>>>>> hci_qca: > > >>>>>> don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets > > >>>>>> power_ctrl_enabled on NULL-pointer returned by > > >>>>>> devm_gpiod_get_optional(). Restore this behavior but bail-out on > > >>>>>> errors. > > >>>>> Nack. This patch does fixes neither the disable/re-enable problem nor > > >>>>> the warm boot problem. > > >>>>> > > >>>>> Zijun replied to this patch also with what I think is the proper > > >>>>> reasoning for why it doesn't fix my setup. > > >>>>> > > >>>> Indeed, I only addressed a single issue here and not the code under the > > >>>> default: label of the switch case. Sorry. > > >>>> > > >>>> Could you give the following diff a try? > > >>> > > >>> I had a feeling that was what was going on. I'll give the patch a shot. > > >>> > > >>> wt > > >> > > >> Considering this patch is basically equivalent to patch 1/2 from Zijun, > > >> I am not surprised that is works similarly. I.e. on a cold boot, I can > > >> disable/re-enable bluetooth as many time as I want. > > >> > > > > > > Zijun didn't bail out on errors which is the issue the original patch > > > tried to address and this one preserves. > > > > > >> However, since this patch doesn't include the quirk fix from Zijun's > > >> patchset (patch 2/2), bluetooth fails to work after a warm boot. > > >> > > > > > > That's OK, we have the first part right. Let's now see if we can reuse > > > patch 2/2 from Zijun. > > > > I'm compiling it right now. Be back soon. > > > > Well I doubt it's correct as it removed Krzysztof's fix which looks > right. If I were to guess I'd say we need some mix of both. > > Bart > Anyway, these are two separate issues. Let me send a v2 of this one to address at least one of them. I think Krzysztof will be better at fixing the other one as it's his patch that caused the second regression. Bart ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 12:20 ` Bartosz Golaszewski 2024-04-24 12:23 ` Bartosz Golaszewski @ 2024-04-24 12:24 ` Wren Turkal 2024-04-24 12:27 ` Bartosz Golaszewski 1 sibling, 1 reply; 32+ messages in thread From: Wren Turkal @ 2024-04-24 12:24 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Bartosz Golaszewski, linux-bluetooth, linux-kernel, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/24 5:20 AM, Bartosz Golaszewski wrote: > On Wed, 24 Apr 2024 at 14:17, Wren Turkal <wt@penguintechs.org> wrote: >> >> On 4/24/24 4:56 AM, Bartosz Golaszewski wrote: >>> On Wed, Apr 24, 2024 at 1:53 PM Wren Turkal <wt@penguintechs.org> wrote: >>>> >>>> On 4/24/24 4:16 AM, Wren Turkal wrote: >>>>> On 4/24/24 2:04 AM, Bartosz Golaszewski wrote: >>>>>> On Wed, 24 Apr 2024 07:07:05 +0200, Wren Turkal<wt@penguintechs.org> >>>>>> said: >>>>>>> On 4/22/24 6:00 AM, Bartosz Golaszewski wrote: >>>>>>>> From: Bartosz Golaszewski<bartosz.golaszewski@linaro.org> >>>>>>>> >>>>>>>> Any return value from gpiod_get_optional() other than a pointer to a >>>>>>>> GPIO descriptor or a NULL-pointer is an error and the driver should >>>>>>>> abort probing. That being said: commit 56d074d26c58 ("Bluetooth: >>>>>>>> hci_qca: >>>>>>>> don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets >>>>>>>> power_ctrl_enabled on NULL-pointer returned by >>>>>>>> devm_gpiod_get_optional(). Restore this behavior but bail-out on >>>>>>>> errors. >>>>>>> Nack. This patch does fixes neither the disable/re-enable problem nor >>>>>>> the warm boot problem. >>>>>>> >>>>>>> Zijun replied to this patch also with what I think is the proper >>>>>>> reasoning for why it doesn't fix my setup. >>>>>>> >>>>>> Indeed, I only addressed a single issue here and not the code under the >>>>>> default: label of the switch case. Sorry. >>>>>> >>>>>> Could you give the following diff a try? >>>>> >>>>> I had a feeling that was what was going on. I'll give the patch a shot. >>>>> >>>>> wt >>>> >>>> Considering this patch is basically equivalent to patch 1/2 from Zijun, >>>> I am not surprised that is works similarly. I.e. on a cold boot, I can >>>> disable/re-enable bluetooth as many time as I want. >>>> >>> >>> Zijun didn't bail out on errors which is the issue the original patch >>> tried to address and this one preserves. >>> >>>> However, since this patch doesn't include the quirk fix from Zijun's >>>> patchset (patch 2/2), bluetooth fails to work after a warm boot. >>>> >>> >>> That's OK, we have the first part right. Let's now see if we can reuse >>> patch 2/2 from Zijun. >> >> I'm compiling it right now. Be back soon. >> > > Well I doubt it's correct as it removed Krzysztof's fix which looks > right. If I were to guess I'd say we need some mix of both. Patch 2/2 remove K's fix? I thought only 1/2 did that. To be specific, I have applied your patch and Zijun's 2/2 only. > Bart > >>>> @Zijun, this patch looks more idiomatic when I look at the surrounding >>>> code than your patch 1/2. Notice how it doesn't use the "else if" >>>> construct. It does the NULL test separately after checking for errors. >>>> >>>> -- >>>> You're more amazing than you think! >>> >>> Bart >> >> -- >> You're more amazing than you think! -- You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 12:24 ` Wren Turkal @ 2024-04-24 12:27 ` Bartosz Golaszewski 2024-04-24 12:30 ` Wren Turkal ` (2 more replies) 0 siblings, 3 replies; 32+ messages in thread From: Bartosz Golaszewski @ 2024-04-24 12:27 UTC (permalink / raw) To: Wren Turkal Cc: Bartosz Golaszewski, linux-bluetooth, linux-kernel, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On Wed, Apr 24, 2024 at 2:24 PM Wren Turkal <wt@penguintechs.org> wrote: > >>> > >>> That's OK, we have the first part right. Let's now see if we can reuse > >>> patch 2/2 from Zijun. > >> > >> I'm compiling it right now. Be back soon. > >> > > > > Well I doubt it's correct as it removed Krzysztof's fix which looks > > right. If I were to guess I'd say we need some mix of both. > > Patch 2/2 remove K's fix? I thought only 1/2 did that. > > To be specific, I have applied your patch and Zijun's 2/2 only. > No, patch 1/2 from Zijun reverted my changes. Patch 2/2 removes Krzysztof's changes and replaces them with a different if else. This patch is a better alternative to Zijun's patch 1/2. For 2/2, I'll let Krzysztof handle it. Bart ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 12:27 ` Bartosz Golaszewski @ 2024-04-24 12:30 ` Wren Turkal 2024-04-24 12:57 ` Bartosz Golaszewski 2024-04-24 12:57 ` Wren Turkal 2024-04-24 13:12 ` quic_zijuhu 2 siblings, 1 reply; 32+ messages in thread From: Wren Turkal @ 2024-04-24 12:30 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Bartosz Golaszewski, linux-bluetooth, linux-kernel, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/24 5:27 AM, Bartosz Golaszewski wrote: > On Wed, Apr 24, 2024 at 2:24 PM Wren Turkal <wt@penguintechs.org> wrote: >>>>> >>>>> That's OK, we have the first part right. Let's now see if we can reuse >>>>> patch 2/2 from Zijun. >>>> >>>> I'm compiling it right now. Be back soon. >>>> >>> >>> Well I doubt it's correct as it removed Krzysztof's fix which looks >>> right. If I were to guess I'd say we need some mix of both. >> >> Patch 2/2 remove K's fix? I thought only 1/2 did that. >> >> To be specific, I have applied your patch and Zijun's 2/2 only. >> > > No, patch 1/2 from Zijun reverted my changes. Patch 2/2 removes > Krzysztof's changes and replaces them with a different if else. This > patch is a better alternative to Zijun's patch 1/2. For 2/2, I'll let > Krzysztof handle it. Got it. Thx. BTW, should this patch's commit message include the following? Tested-by: "Wren Turkal" <wt@penguintechs.org> If so, please feel free to add it. wt -- You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 12:30 ` Wren Turkal @ 2024-04-24 12:57 ` Bartosz Golaszewski 0 siblings, 0 replies; 32+ messages in thread From: Bartosz Golaszewski @ 2024-04-24 12:57 UTC (permalink / raw) To: Wren Turkal Cc: Bartosz Golaszewski, linux-bluetooth, linux-kernel, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On Wed, 24 Apr 2024 at 14:30, Wren Turkal <wt@penguintechs.org> wrote: > > On 4/24/24 5:27 AM, Bartosz Golaszewski wrote: > > On Wed, Apr 24, 2024 at 2:24 PM Wren Turkal <wt@penguintechs.org> wrote: > >>>>> > >>>>> That's OK, we have the first part right. Let's now see if we can reuse > >>>>> patch 2/2 from Zijun. > >>>> > >>>> I'm compiling it right now. Be back soon. > >>>> > >>> > >>> Well I doubt it's correct as it removed Krzysztof's fix which looks > >>> right. If I were to guess I'd say we need some mix of both. > >> > >> Patch 2/2 remove K's fix? I thought only 1/2 did that. > >> > >> To be specific, I have applied your patch and Zijun's 2/2 only. > >> > > > > No, patch 1/2 from Zijun reverted my changes. Patch 2/2 removes > > Krzysztof's changes and replaces them with a different if else. This > > patch is a better alternative to Zijun's patch 1/2. For 2/2, I'll let > > Krzysztof handle it. > > Got it. Thx. > > BTW, should this patch's commit message include the following? > > Tested-by: "Wren Turkal" <wt@penguintechs.org> > Please respond with this tag under the v2 patch and b4, patchwork or whatever other tools the maintainer will use will pick it up. Thanks, Bartosz > If so, please feel free to add it. > > wt > -- > You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 12:27 ` Bartosz Golaszewski 2024-04-24 12:30 ` Wren Turkal @ 2024-04-24 12:57 ` Wren Turkal 2024-04-24 13:12 ` quic_zijuhu 2 siblings, 0 replies; 32+ messages in thread From: Wren Turkal @ 2024-04-24 12:57 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Bartosz Golaszewski, linux-bluetooth, linux-kernel, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/24 5:27 AM, Bartosz Golaszewski wrote: > On Wed, Apr 24, 2024 at 2:24 PM Wren Turkal <wt@penguintechs.org> wrote: >>>>> >>>>> That's OK, we have the first part right. Let's now see if we can reuse >>>>> patch 2/2 from Zijun. >>>> >>>> I'm compiling it right now. Be back soon. >>>> >>> >>> Well I doubt it's correct as it removed Krzysztof's fix which looks >>> right. If I were to guess I'd say we need some mix of both. >> >> Patch 2/2 remove K's fix? I thought only 1/2 did that. >> >> To be specific, I have applied your patch and Zijun's 2/2 only. >> > > No, patch 1/2 from Zijun reverted my changes. Patch 2/2 removes > Krzysztof's changes and replaces them with a different if else. This > patch is a better alternative to Zijun's patch 1/2. For 2/2, I'll let > Krzysztof handle it. Hey there. So, applying Zijun 2/2 on top of your patch does fix the warm boot problem for me. I can't speak about whether it's the right fix. @Krzysztof Is the correct just add the conditional return for the quirk without reverting your earlier change? wt -- You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 12:27 ` Bartosz Golaszewski 2024-04-24 12:30 ` Wren Turkal 2024-04-24 12:57 ` Wren Turkal @ 2024-04-24 13:12 ` quic_zijuhu 2024-04-24 13:26 ` Wren Turkal 2 siblings, 1 reply; 32+ messages in thread From: quic_zijuhu @ 2024-04-24 13:12 UTC (permalink / raw) To: Bartosz Golaszewski, Wren Turkal Cc: Bartosz Golaszewski, linux-bluetooth, linux-kernel, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/2024 8:27 PM, Bartosz Golaszewski wrote: > On Wed, Apr 24, 2024 at 2:24 PM Wren Turkal <wt@penguintechs.org> wrote: >>>>> >>>>> That's OK, we have the first part right. Let's now see if we can reuse >>>>> patch 2/2 from Zijun. >>>> >>>> I'm compiling it right now. Be back soon. >>>> >>> >>> Well I doubt it's correct as it removed Krzysztof's fix which looks >>> right. If I were to guess I'd say we need some mix of both. >> >> Patch 2/2 remove K's fix? I thought only 1/2 did that. >> >> To be specific, I have applied your patch and Zijun's 2/2 only. >> > > No, patch 1/2 from Zijun reverted my changes. Patch 2/2 removes > Krzysztof's changes and replaces them with a different if else. This > patch is a better alternative to Zijun's patch 1/2. For 2/2, I'll let > Krzysztof handle it. > do you really realize what do you talk about? do you really listen what do @Wren says? he says that my patch 2/2 is right based on several verification results. BTW, my 2/2 fix don't have anything about DTS usage. he maybe be a DTS expert but not BT from his present fix history for bluetooth system. > Bart ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 13:12 ` quic_zijuhu @ 2024-04-24 13:26 ` Wren Turkal 2024-04-24 13:30 ` quic_zijuhu 2024-04-24 13:53 ` Bartosz Golaszewski 0 siblings, 2 replies; 32+ messages in thread From: Wren Turkal @ 2024-04-24 13:26 UTC (permalink / raw) To: quic_zijuhu, Bartosz Golaszewski Cc: Bartosz Golaszewski, linux-bluetooth, linux-kernel, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/24 6:12 AM, quic_zijuhu wrote: > On 4/24/2024 8:27 PM, Bartosz Golaszewski wrote: >> On Wed, Apr 24, 2024 at 2:24 PM Wren Turkal <wt@penguintechs.org> wrote: >>>>>> >>>>>> That's OK, we have the first part right. Let's now see if we can reuse >>>>>> patch 2/2 from Zijun. >>>>> >>>>> I'm compiling it right now. Be back soon. >>>>> >>>> >>>> Well I doubt it's correct as it removed Krzysztof's fix which looks >>>> right. If I were to guess I'd say we need some mix of both. >>> >>> Patch 2/2 remove K's fix? I thought only 1/2 did that. >>> >>> To be specific, I have applied your patch and Zijun's 2/2 only. >>> >> >> No, patch 1/2 from Zijun reverted my changes. Patch 2/2 removes >> Krzysztof's changes and replaces them with a different if else. This >> patch is a better alternative to Zijun's patch 1/2. For 2/2, I'll let >> Krzysztof handle it. >> > do you really realize what do you talk about? > do you really listen what do @Wren says? > > he says that my patch 2/2 is right based on several verification results. she, not he > BTW, my 2/2 fix don't have anything about DTS usage. I think the problem with your 2/2 patch is that it removes the conditional bailing if the device is shutdown or not open. Maybe this patch instead? diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c index 2f7ae38d85eb..fcac44ae7898 100644 --- a/drivers/bluetooth/hci_qca.c +++ b/drivers/bluetooth/hci_qca.c @@ -2456,6 +2456,10 @@ static void qca_serdev_shutdown(struct device *dev) !test_bit(HCI_RUNNING, &hdev->flags)) return; + if (test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks) || + hci_dev_test_flag(hdev, HCI_SETUP)) + return; + serdev_device_write_flush(serdev); ret = serdev_device_write_buf(serdev, ibs_wake_cmd, sizeof(ibs_wake_cmd)); > he maybe be a DTS expert but not BT from his present fix history for > bluetooth system. -- You're more amazing than you think! ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 13:26 ` Wren Turkal @ 2024-04-24 13:30 ` quic_zijuhu 2024-04-24 22:09 ` Wren Turkal 2024-04-24 13:53 ` Bartosz Golaszewski 1 sibling, 1 reply; 32+ messages in thread From: quic_zijuhu @ 2024-04-24 13:30 UTC (permalink / raw) To: Wren Turkal, Bartosz Golaszewski Cc: Bartosz Golaszewski, linux-bluetooth, linux-kernel, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/2024 9:26 PM, Wren Turkal wrote: > On 4/24/24 6:12 AM, quic_zijuhu wrote: >> On 4/24/2024 8:27 PM, Bartosz Golaszewski wrote: >>> On Wed, Apr 24, 2024 at 2:24 PM Wren Turkal <wt@penguintechs.org> wrote: >>>>>>> >>>>>>> That's OK, we have the first part right. Let's now see if we can >>>>>>> reuse >>>>>>> patch 2/2 from Zijun. >>>>>> >>>>>> I'm compiling it right now. Be back soon. >>>>>> >>>>> >>>>> Well I doubt it's correct as it removed Krzysztof's fix which looks >>>>> right. If I were to guess I'd say we need some mix of both. >>>> >>>> Patch 2/2 remove K's fix? I thought only 1/2 did that. >>>> >>>> To be specific, I have applied your patch and Zijun's 2/2 only. >>>> >>> >>> No, patch 1/2 from Zijun reverted my changes. Patch 2/2 removes >>> Krzysztof's changes and replaces them with a different if else. This >>> patch is a better alternative to Zijun's patch 1/2. For 2/2, I'll let >>> Krzysztof handle it. >>> >> do you really realize what do you talk about? >> do you really listen what do @Wren says? >> >> he says that my patch 2/2 is right based on several verification results. > > she, not he > >> BTW, my 2/2 fix don't have anything about DTS usage. > > I think the problem with your 2/2 patch is that it removes the > conditional bailing if the device is shutdown or not open. > > Maybe this patch instead? > > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c > index 2f7ae38d85eb..fcac44ae7898 100644 > --- a/drivers/bluetooth/hci_qca.c > +++ b/drivers/bluetooth/hci_qca.c > @@ -2456,6 +2456,10 @@ static void qca_serdev_shutdown(struct device *dev) > !test_bit(HCI_RUNNING, &hdev->flags)) > return; > > + if (test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, > &hdev->quirks) || > + hci_dev_test_flag(hdev, HCI_SETUP)) > + return; > + > serdev_device_write_flush(serdev); > ret = serdev_device_write_buf(serdev, ibs_wake_cmd, > sizeof(ibs_wake_cmd)); > >> he maybe be a DTS expert but not BT from his present fix history for >> bluetooth system. > > Hi Wren, i think i don't need to care about why wrong condition cause wrong results. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 13:30 ` quic_zijuhu @ 2024-04-24 22:09 ` Wren Turkal 0 siblings, 0 replies; 32+ messages in thread From: Wren Turkal @ 2024-04-24 22:09 UTC (permalink / raw) To: quic_zijuhu, Bartosz Golaszewski Cc: Bartosz Golaszewski, linux-bluetooth, linux-kernel, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/24 6:30 AM, quic_zijuhu wrote: > On 4/24/2024 9:26 PM, Wren Turkal wrote: >> On 4/24/24 6:12 AM, quic_zijuhu wrote: >>> On 4/24/2024 8:27 PM, Bartosz Golaszewski wrote: >>>> On Wed, Apr 24, 2024 at 2:24 PM Wren Turkal <wt@penguintechs.org> wrote: >>>>>>>> >>>>>>>> That's OK, we have the first part right. Let's now see if we can >>>>>>>> reuse >>>>>>>> patch 2/2 from Zijun. >>>>>>> >>>>>>> I'm compiling it right now. Be back soon. >>>>>>> >>>>>> >>>>>> Well I doubt it's correct as it removed Krzysztof's fix which looks >>>>>> right. If I were to guess I'd say we need some mix of both. >>>>> >>>>> Patch 2/2 remove K's fix? I thought only 1/2 did that. >>>>> >>>>> To be specific, I have applied your patch and Zijun's 2/2 only. >>>>> >>>> >>>> No, patch 1/2 from Zijun reverted my changes. Patch 2/2 removes >>>> Krzysztof's changes and replaces them with a different if else. This >>>> patch is a better alternative to Zijun's patch 1/2. For 2/2, I'll let >>>> Krzysztof handle it. >>>> >>> do you really realize what do you talk about? >>> do you really listen what do @Wren says? >>> >>> he says that my patch 2/2 is right based on several verification results. >> >> she, not he >> >>> BTW, my 2/2 fix don't have anything about DTS usage. >> >> I think the problem with your 2/2 patch is that it removes the >> conditional bailing if the device is shutdown or not open. >> >> Maybe this patch instead? >> >> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >> index 2f7ae38d85eb..fcac44ae7898 100644 >> --- a/drivers/bluetooth/hci_qca.c >> +++ b/drivers/bluetooth/hci_qca.c >> @@ -2456,6 +2456,10 @@ static void qca_serdev_shutdown(struct device *dev) >> !test_bit(HCI_RUNNING, &hdev->flags)) >> return; >> >> + if (test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, >> &hdev->quirks) || >> + hci_dev_test_flag(hdev, HCI_SETUP)) >> + return; >> + >> serdev_device_write_flush(serdev); >> ret = serdev_device_write_buf(serdev, ibs_wake_cmd, >> sizeof(ibs_wake_cmd)); >> >>> he maybe be a DTS expert but not BT from his present fix history for >>> bluetooth system. >> >> > Hi Wren, > i think i don't need to care about why wrong condition cause wrong results. I can't speak the intellectual purity of this patch. However, I can say that the patch works for my hardware. I am able to disable/enable on cold boot and on warm boot. @Zijun, Functionally, it seems incorporate your new logic without reverting Krzysztof's. Are you saying this is incorrect logic? wt -- You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 13:26 ` Wren Turkal 2024-04-24 13:30 ` quic_zijuhu @ 2024-04-24 13:53 ` Bartosz Golaszewski 2024-04-24 22:17 ` Wren Turkal 1 sibling, 1 reply; 32+ messages in thread From: Bartosz Golaszewski @ 2024-04-24 13:53 UTC (permalink / raw) To: Wren Turkal Cc: quic_zijuhu, Bartosz Golaszewski, linux-bluetooth, linux-kernel, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On Wed, 24 Apr 2024 at 15:26, Wren Turkal <wt@penguintechs.org> wrote: > > On 4/24/24 6:12 AM, quic_zijuhu wrote: > > On 4/24/2024 8:27 PM, Bartosz Golaszewski wrote: > >> On Wed, Apr 24, 2024 at 2:24 PM Wren Turkal <wt@penguintechs.org> wrote: > >>>>>> > >>>>>> That's OK, we have the first part right. Let's now see if we can reuse > >>>>>> patch 2/2 from Zijun. > >>>>> > >>>>> I'm compiling it right now. Be back soon. > >>>>> > >>>> > >>>> Well I doubt it's correct as it removed Krzysztof's fix which looks > >>>> right. If I were to guess I'd say we need some mix of both. > >>> > >>> Patch 2/2 remove K's fix? I thought only 1/2 did that. > >>> > >>> To be specific, I have applied your patch and Zijun's 2/2 only. > >>> > >> > >> No, patch 1/2 from Zijun reverted my changes. Patch 2/2 removes > >> Krzysztof's changes and replaces them with a different if else. This > >> patch is a better alternative to Zijun's patch 1/2. For 2/2, I'll let > >> Krzysztof handle it. > >> > > do you really realize what do you talk about? > > do you really listen what do @Wren says? > > > > he says that my patch 2/2 is right based on several verification results. > > she, not he > > > BTW, my 2/2 fix don't have anything about DTS usage. > > I think the problem with your 2/2 patch is that it removes the > conditional bailing if the device is shutdown or not open. > > Maybe this patch instead? > > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c > index 2f7ae38d85eb..fcac44ae7898 100644 > --- a/drivers/bluetooth/hci_qca.c > +++ b/drivers/bluetooth/hci_qca.c > @@ -2456,6 +2456,10 @@ static void qca_serdev_shutdown(struct device *dev) > !test_bit(HCI_RUNNING, &hdev->flags)) > return; > > + if (test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, > &hdev->quirks) || > + hci_dev_test_flag(hdev, HCI_SETUP)) > + return; > + > serdev_device_write_flush(serdev); > ret = serdev_device_write_buf(serdev, ibs_wake_cmd, > sizeof(ibs_wake_cmd)); > > > he maybe be a DTS expert but not BT from his present fix history for > > bluetooth system. > > Did you test it? Does it work? If so, please consider sending it upstream for review. You can keep Zijun's authorship but add your own SoB tag at the end and mention what you did. Something like this: [Wren: kept Krzysztof's fix] Signed-off-by: Wren... Bartosz ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 13:53 ` Bartosz Golaszewski @ 2024-04-24 22:17 ` Wren Turkal 2024-04-24 23:35 ` quic_zijuhu 0 siblings, 1 reply; 32+ messages in thread From: Wren Turkal @ 2024-04-24 22:17 UTC (permalink / raw) To: Bartosz Golaszewski Cc: quic_zijuhu, Bartosz Golaszewski, linux-bluetooth, linux-kernel, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/24 6:53 AM, Bartosz Golaszewski wrote: > On Wed, 24 Apr 2024 at 15:26, Wren Turkal <wt@penguintechs.org> wrote: >> >> On 4/24/24 6:12 AM, quic_zijuhu wrote: >>> On 4/24/2024 8:27 PM, Bartosz Golaszewski wrote: >>>> On Wed, Apr 24, 2024 at 2:24 PM Wren Turkal <wt@penguintechs.org> wrote: >>>>>>>> >>>>>>>> That's OK, we have the first part right. Let's now see if we can reuse >>>>>>>> patch 2/2 from Zijun. >>>>>>> >>>>>>> I'm compiling it right now. Be back soon. >>>>>>> >>>>>> >>>>>> Well I doubt it's correct as it removed Krzysztof's fix which looks >>>>>> right. If I were to guess I'd say we need some mix of both. >>>>> >>>>> Patch 2/2 remove K's fix? I thought only 1/2 did that. >>>>> >>>>> To be specific, I have applied your patch and Zijun's 2/2 only. >>>>> >>>> >>>> No, patch 1/2 from Zijun reverted my changes. Patch 2/2 removes >>>> Krzysztof's changes and replaces them with a different if else. This >>>> patch is a better alternative to Zijun's patch 1/2. For 2/2, I'll let >>>> Krzysztof handle it. >>>> >>> do you really realize what do you talk about? >>> do you really listen what do @Wren says? >>> >>> he says that my patch 2/2 is right based on several verification results. >> >> she, not he >> >>> BTW, my 2/2 fix don't have anything about DTS usage. >> >> I think the problem with your 2/2 patch is that it removes the >> conditional bailing if the device is shutdown or not open. >> >> Maybe this patch instead? >> >> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >> index 2f7ae38d85eb..fcac44ae7898 100644 >> --- a/drivers/bluetooth/hci_qca.c >> +++ b/drivers/bluetooth/hci_qca.c >> @@ -2456,6 +2456,10 @@ static void qca_serdev_shutdown(struct device *dev) >> !test_bit(HCI_RUNNING, &hdev->flags)) >> return; >> >> + if (test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, >> &hdev->quirks) || >> + hci_dev_test_flag(hdev, HCI_SETUP)) >> + return; >> + >> serdev_device_write_flush(serdev); >> ret = serdev_device_write_buf(serdev, ibs_wake_cmd, >> sizeof(ibs_wake_cmd)); >> >>> he maybe be a DTS expert but not BT from his present fix history for >>> bluetooth system. >> >> > > Did you test it? Does it work? If so, please consider sending it > upstream for review. > > You can keep Zijun's authorship but add your own SoB tag at the end > and mention what you did. Something like this: > > [Wren: kept Krzysztof's fix] > Signed-off-by: Wren... > > Bartosz @Bartosz, I have tested this, and it works functionally for my setup. I cannot detect a difference between this and Zijun's logic when I compile a kernel with this patch. @Zijun, I think you have objections to this patch. I would like to make sure I hear your concern. Can you please take through it like I'm a 5 year old who barely knows C? In retrospect, I guess that I would be a pretty precocious 5 year old. LOL. In all seriousness, @Zijun, I really appreciate the work you did on this. I would like to understand why you assert that removing the logic of Krzysztof is appropriate. Again, I am not a kernel developer, so this stuff is really outside my wheelhouse. Having said that, by my reading, which may very well be worng, it seems like you are just adding another case that is orthogonal to K's conditions. I'd like to truly understand you position to know if the patch I am suggesting is somehow harmful. This is an earnest question. I really want to respect your expertise here, and I really want you to know how much I appreciate your work. wt -- You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 22:17 ` Wren Turkal @ 2024-04-24 23:35 ` quic_zijuhu 2024-04-25 2:34 ` Wren Turkal 0 siblings, 1 reply; 32+ messages in thread From: quic_zijuhu @ 2024-04-24 23:35 UTC (permalink / raw) To: Wren Turkal, Bartosz Golaszewski Cc: Bartosz Golaszewski, linux-bluetooth, linux-kernel, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/25/2024 6:17 AM, Wren Turkal wrote: > On 4/24/24 6:53 AM, Bartosz Golaszewski wrote: >> On Wed, 24 Apr 2024 at 15:26, Wren Turkal <wt@penguintechs.org> wrote: >>> >>> On 4/24/24 6:12 AM, quic_zijuhu wrote: >>>> On 4/24/2024 8:27 PM, Bartosz Golaszewski wrote: >>>>> On Wed, Apr 24, 2024 at 2:24 PM Wren Turkal <wt@penguintechs.org> >>>>> wrote: >>>>>>>>> >>>>>>>>> That's OK, we have the first part right. Let's now see if we >>>>>>>>> can reuse >>>>>>>>> patch 2/2 from Zijun. >>>>>>>> >>>>>>>> I'm compiling it right now. Be back soon. >>>>>>>> >>>>>>> >>>>>>> Well I doubt it's correct as it removed Krzysztof's fix which looks >>>>>>> right. If I were to guess I'd say we need some mix of both. >>>>>> >>>>>> Patch 2/2 remove K's fix? I thought only 1/2 did that. >>>>>> >>>>>> To be specific, I have applied your patch and Zijun's 2/2 only. >>>>>> >>>>> >>>>> No, patch 1/2 from Zijun reverted my changes. Patch 2/2 removes >>>>> Krzysztof's changes and replaces them with a different if else. This >>>>> patch is a better alternative to Zijun's patch 1/2. For 2/2, I'll let >>>>> Krzysztof handle it. >>>>> >>>> do you really realize what do you talk about? >>>> do you really listen what do @Wren says? >>>> >>>> he says that my patch 2/2 is right based on several verification >>>> results. >>> >>> she, not he >>> >>>> BTW, my 2/2 fix don't have anything about DTS usage. >>> >>> I think the problem with your 2/2 patch is that it removes the >>> conditional bailing if the device is shutdown or not open. >>> >>> Maybe this patch instead? >>> >>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >>> index 2f7ae38d85eb..fcac44ae7898 100644 >>> --- a/drivers/bluetooth/hci_qca.c >>> +++ b/drivers/bluetooth/hci_qca.c >>> @@ -2456,6 +2456,10 @@ static void qca_serdev_shutdown(struct device >>> *dev) >>> !test_bit(HCI_RUNNING, &hdev->flags)) >>> return; >>> >>> + if (test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, >>> &hdev->quirks) || >>> + hci_dev_test_flag(hdev, HCI_SETUP)) >>> + return; >>> + >>> serdev_device_write_flush(serdev); >>> ret = serdev_device_write_buf(serdev, ibs_wake_cmd, >>> sizeof(ibs_wake_cmd)); >>> >>>> he maybe be a DTS expert but not BT from his present fix history for >>>> bluetooth system. >>> >>> >> >> Did you test it? Does it work? If so, please consider sending it >> upstream for review. >> >> You can keep Zijun's authorship but add your own SoB tag at the end >> and mention what you did. Something like this: >> [V7 2/2] as shown by below link is the formal fix. https://patchwork.kernel.org/project/bluetooth/patch/1713932807-19619-3-git-send-email-quic_zijuhu@quicinc.com/ this fix logic was introduced from the very beginning when i saw your issue description as shown by below link https://lore.kernel.org/all/1713095825-4954-3-git-send-email-quic_zijuhu@quicinc.com/#t >> [Wren: kept Krzysztof's fix] >> Signed-off-by: Wren... >> >> Bartosz > > @Bartosz, I have tested this, and it works functionally for my setup. I > cannot detect a difference between this and Zijun's logic when I compile > a kernel with this patch. > > @Zijun, I think you have objections to this patch. I would like to make > sure I hear your concern. Can you please take through it like I'm a 5 > year old who barely knows C? In retrospect, I guess that I would be a > pretty precocious 5 year old. LOL. > > In all seriousness, @Zijun, I really appreciate the work you did on > this. I would like to understand why you assert that removing the logic > of Krzysztof is appropriate. Again, I am not a kernel developer, so this > stuff is really outside my wheelhouse. Having said that, by my reading, > which may very well be worng, it seems like you are just adding another > case that is orthogonal to K's conditions. I'd like to truly understand > you position to know if the patch I am suggesting is somehow harmful. > This is an earnest question. I really want to respect your expertise > here, and I really want you to know how much I appreciate your work. > you maybe see all replies of [2/2] patch for this issue within below link. i believe you will understand it. the bottom of the link includes all reply history. https://lore.kernel.org/all/fe1a0e3b-3408-4a33-90e9-d4ffcfc7a99b@quicinc.com/ > wt ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 23:35 ` quic_zijuhu @ 2024-04-25 2:34 ` Wren Turkal 0 siblings, 0 replies; 32+ messages in thread From: Wren Turkal @ 2024-04-25 2:34 UTC (permalink / raw) To: quic_zijuhu, Bartosz Golaszewski Cc: Bartosz Golaszewski, linux-bluetooth, linux-kernel, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/24 4:35 PM, quic_zijuhu wrote: > On 4/25/2024 6:17 AM, Wren Turkal wrote: >> On 4/24/24 6:53 AM, Bartosz Golaszewski wrote: >>> On Wed, 24 Apr 2024 at 15:26, Wren Turkal <wt@penguintechs.org> wrote: >>>> >>>> On 4/24/24 6:12 AM, quic_zijuhu wrote: >>>>> On 4/24/2024 8:27 PM, Bartosz Golaszewski wrote: >>>>>> On Wed, Apr 24, 2024 at 2:24 PM Wren Turkal <wt@penguintechs.org> >>>>>> wrote: >>>>>>>>>> >>>>>>>>>> That's OK, we have the first part right. Let's now see if we >>>>>>>>>> can reuse >>>>>>>>>> patch 2/2 from Zijun. >>>>>>>>> >>>>>>>>> I'm compiling it right now. Be back soon. >>>>>>>>> >>>>>>>> >>>>>>>> Well I doubt it's correct as it removed Krzysztof's fix which looks >>>>>>>> right. If I were to guess I'd say we need some mix of both. >>>>>>> >>>>>>> Patch 2/2 remove K's fix? I thought only 1/2 did that. >>>>>>> >>>>>>> To be specific, I have applied your patch and Zijun's 2/2 only. >>>>>>> >>>>>> >>>>>> No, patch 1/2 from Zijun reverted my changes. Patch 2/2 removes >>>>>> Krzysztof's changes and replaces them with a different if else. This >>>>>> patch is a better alternative to Zijun's patch 1/2. For 2/2, I'll let >>>>>> Krzysztof handle it. >>>>>> >>>>> do you really realize what do you talk about? >>>>> do you really listen what do @Wren says? >>>>> >>>>> he says that my patch 2/2 is right based on several verification >>>>> results. >>>> >>>> she, not he >>>> >>>>> BTW, my 2/2 fix don't have anything about DTS usage. >>>> >>>> I think the problem with your 2/2 patch is that it removes the >>>> conditional bailing if the device is shutdown or not open. >>>> >>>> Maybe this patch instead? >>>> >>>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >>>> index 2f7ae38d85eb..fcac44ae7898 100644 >>>> --- a/drivers/bluetooth/hci_qca.c >>>> +++ b/drivers/bluetooth/hci_qca.c >>>> @@ -2456,6 +2456,10 @@ static void qca_serdev_shutdown(struct device >>>> *dev) >>>> !test_bit(HCI_RUNNING, &hdev->flags)) >>>> return; >>>> >>>> + if (test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, >>>> &hdev->quirks) || >>>> + hci_dev_test_flag(hdev, HCI_SETUP)) >>>> + return; >>>> + >>>> serdev_device_write_flush(serdev); >>>> ret = serdev_device_write_buf(serdev, ibs_wake_cmd, >>>> sizeof(ibs_wake_cmd)); >>>> >>>>> he maybe be a DTS expert but not BT from his present fix history for >>>>> bluetooth system. >>>> >>>> >>> >>> Did you test it? Does it work? If so, please consider sending it >>> upstream for review. >>> >>> You can keep Zijun's authorship but add your own SoB tag at the end >>> and mention what you did. Something like this: >>> > > [V7 2/2] as shown by below link is the formal fix. > https://patchwork.kernel.org/project/bluetooth/patch/1713932807-19619-3-git-send-email-quic_zijuhu@quicinc.com/ > > this fix logic was introduced from the very beginning when i saw your > issue description as shown by below link > https://lore.kernel.org/all/1713095825-4954-3-git-send-email-quic_zijuhu@quicinc.com/#t This is not quite fully correct. You did introduce that logic. However, you also removed the other conditional. That is what K.K. was objecting to. I literally copied your logic without deleting K.K.'s logic. My specific question is does that make my change incorrect in some way. >>> [Wren: kept Krzysztof's fix] >>> Signed-off-by: Wren... >>> >>> Bartosz >> >> @Bartosz, I have tested this, and it works functionally for my setup. I >> cannot detect a difference between this and Zijun's logic when I compile >> a kernel with this patch. >> >> @Zijun, I think you have objections to this patch. I would like to make >> sure I hear your concern. Can you please take through it like I'm a 5 >> year old who barely knows C? In retrospect, I guess that I would be a >> pretty precocious 5 year old. LOL. >> >> In all seriousness, @Zijun, I really appreciate the work you did on >> this. I would like to understand why you assert that removing the logic >> of Krzysztof is appropriate. Again, I am not a kernel developer, so this >> stuff is really outside my wheelhouse. Having said that, by my reading, >> which may very well be worng, it seems like you are just adding another >> case that is orthogonal to K's conditions. I'd like to truly understand >> you position to know if the patch I am suggesting is somehow harmful. >> This is an earnest question. I really want to respect your expertise >> here, and I really want you to know how much I appreciate your work. >> you maybe see all replies of [2/2] patch for this issue within below > link. i believe you will understand it. the bottom of the link includes > all reply history. > https://lore.kernel.org/all/fe1a0e3b-3408-4a33-90e9-d4ffcfc7a99b@quicinc.com/ I read through the discussion again. Can you please define VSC? I'm not sure what that means. With regard to your change's logic. First a kernel dev newbie question, does "hci_dev_test_flag(hdev, HCI_SETUP)" test whether the hardware has ever been setup? The rest of my discussions assumes that is the case. So, basically, the logic in your change is something like this: if dev_must_be_setup_every_time_opened || dev_is_in_setup_state bail out on shutdown logic If I am getting this correctly, you're saying that any device that must be setup is already by definition not needing setup and should not have shutdown logic run on it. Okay, so question: Is it possible for a device to need setup (both not have the quirk and "hci_dev_test_flag(hdev, HCI_SETUP)" to have BT off or not be running? Assuming the answer to that question is, "It is not possible." How do we know that? I think that may be what K.K. is saying is not obvious. If the code ever gets into that state, shouldn't we at least log that the state is not an expected state? I am now worried that allowing an unexpected state through could result in a unintended logic executing. How do I know I don't need to worry about that? wt -- You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 9:04 ` Bartosz Golaszewski 2024-04-24 9:32 ` quic_zijuhu 2024-04-24 11:16 ` Wren Turkal @ 2024-04-24 11:25 ` Wren Turkal 2024-04-24 11:53 ` Bartosz Golaszewski 2 siblings, 1 reply; 32+ messages in thread From: Wren Turkal @ 2024-04-24 11:25 UTC (permalink / raw) To: Bartosz Golaszewski Cc: linux-bluetooth, linux-kernel, Bartosz Golaszewski, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/24 2:04 AM, Bartosz Golaszewski wrote: > On Wed, 24 Apr 2024 07:07:05 +0200, Wren Turkal<wt@penguintechs.org> said: >> On 4/22/24 6:00 AM, Bartosz Golaszewski wrote: >>> From: Bartosz Golaszewski<bartosz.golaszewski@linaro.org> >>> >>> Any return value from gpiod_get_optional() other than a pointer to a >>> GPIO descriptor or a NULL-pointer is an error and the driver should >>> abort probing. That being said: commit 56d074d26c58 ("Bluetooth: hci_qca: >>> don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets >>> power_ctrl_enabled on NULL-pointer returned by >>> devm_gpiod_get_optional(). Restore this behavior but bail-out on errors. >> Nack. This patch does fixes neither the disable/re-enable problem nor >> the warm boot problem. >> >> Zijun replied to this patch also with what I think is the proper >> reasoning for why it doesn't fix my setup. >> > Indeed, I only addressed a single issue here and not the code under the > default: label of the switch case. Sorry. > > Could you give the following diff a try? I am compiling a kernel the patch right now, but I did want to let you know that the patch got corrupted by extra line wrapping. I'm not sure how you're sending it, but I thought you might want to know. wt -- You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 11:25 ` Wren Turkal @ 2024-04-24 11:53 ` Bartosz Golaszewski 2024-04-24 11:59 ` Wren Turkal 0 siblings, 1 reply; 32+ messages in thread From: Bartosz Golaszewski @ 2024-04-24 11:53 UTC (permalink / raw) To: Wren Turkal Cc: linux-bluetooth, linux-kernel, Bartosz Golaszewski, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On Wed, Apr 24, 2024 at 1:25 PM Wren Turkal <wt@penguintechs.org> wrote: > > On 4/24/24 2:04 AM, Bartosz Golaszewski wrote: > > On Wed, 24 Apr 2024 07:07:05 +0200, Wren Turkal<wt@penguintechs.org> said: > >> On 4/22/24 6:00 AM, Bartosz Golaszewski wrote: > >>> From: Bartosz Golaszewski<bartosz.golaszewski@linaro.org> > >>> > >>> Any return value from gpiod_get_optional() other than a pointer to a > >>> GPIO descriptor or a NULL-pointer is an error and the driver should > >>> abort probing. That being said: commit 56d074d26c58 ("Bluetooth: hci_qca: > >>> don't use IS_ERR_OR_NULL() with gpiod_get_optional()") no longer sets > >>> power_ctrl_enabled on NULL-pointer returned by > >>> devm_gpiod_get_optional(). Restore this behavior but bail-out on errors. > >> Nack. This patch does fixes neither the disable/re-enable problem nor > >> the warm boot problem. > >> > >> Zijun replied to this patch also with what I think is the proper > >> reasoning for why it doesn't fix my setup. > >> > > Indeed, I only addressed a single issue here and not the code under the > > default: label of the switch case. Sorry. > > > > Could you give the following diff a try? > > I am compiling a kernel the patch right now, but I did want to let you > know that the patch got corrupted by extra line wrapping. I'm not sure > how you're sending it, but I thought you might want to know. > This must be your email client wrapping lines over a certain limit. Try and get the diff from lore[1], it should be fine. Bart [1] https://lore.kernel.org/lkml/CAMRc=MepDwUbAKrWgm0CXKObqy8=igtug0QDgo-CgwxjZCAC2Q@mail.gmail.com/ ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 11:53 ` Bartosz Golaszewski @ 2024-04-24 11:59 ` Wren Turkal 2024-04-24 12:01 ` Bartosz Golaszewski 0 siblings, 1 reply; 32+ messages in thread From: Wren Turkal @ 2024-04-24 11:59 UTC (permalink / raw) To: Bartosz Golaszewski Cc: linux-bluetooth, linux-kernel, Bartosz Golaszewski, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/24 4:53 AM, Bartosz Golaszewski wrote: > This must be your email client wrapping lines over a certain limit. > Try and get the diff from lore[1], it should be fine. > > Bart > > [1]https://lore.kernel.org/lkml/CAMRc=MepDwUbAKrWgm0CXKObqy8=igtug0QDgo-CgwxjZCAC2Q@mail.gmail.com/ I don't think it's my client. The extra newlines are right there in the lore link. Look at the line that starts with "@@". That line is wrapped. The following line ("serdev_device *serdev)") should be at the end of the previous line. The same thing happened on the second "@@" line as well. wt -- You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 11:59 ` Wren Turkal @ 2024-04-24 12:01 ` Bartosz Golaszewski 2024-04-24 12:05 ` Wren Turkal 0 siblings, 1 reply; 32+ messages in thread From: Bartosz Golaszewski @ 2024-04-24 12:01 UTC (permalink / raw) To: Wren Turkal Cc: Bartosz Golaszewski, linux-bluetooth, linux-kernel, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On Wed, 24 Apr 2024 at 13:59, Wren Turkal <wt@penguintechs.org> wrote: > > On 4/24/24 4:53 AM, Bartosz Golaszewski wrote: > > This must be your email client wrapping lines over a certain limit. > > Try and get the diff from lore[1], it should be fine. > > > > Bart > > > > [1]https://lore.kernel.org/lkml/CAMRc=MepDwUbAKrWgm0CXKObqy8=igtug0QDgo-CgwxjZCAC2Q@mail.gmail.com/ > > I don't think it's my client. The extra newlines are right there in the > lore link. > > Look at the line that starts with "@@". That line is wrapped. The > following line ("serdev_device *serdev)") should be at the end of the > previous line. The same thing happened on the second "@@" line as well. > Indeed. I just noticed that it applies fine with git apply and figured the output must be right. Anyway, this is not a proper patch, I will send one once I adapt Zijun's code. Bart ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() 2024-04-24 12:01 ` Bartosz Golaszewski @ 2024-04-24 12:05 ` Wren Turkal 0 siblings, 0 replies; 32+ messages in thread From: Wren Turkal @ 2024-04-24 12:05 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Bartosz Golaszewski, linux-bluetooth, linux-kernel, Zijun Hu, Marcel Holtmann, Luiz Augusto von Dentz, Krzysztof Kozlowski On 4/24/24 5:01 AM, Bartosz Golaszewski wrote: > On Wed, 24 Apr 2024 at 13:59, Wren Turkal <wt@penguintechs.org> wrote: >> >> On 4/24/24 4:53 AM, Bartosz Golaszewski wrote: >>> This must be your email client wrapping lines over a certain limit. >>> Try and get the diff from lore[1], it should be fine. >>> >>> Bart >>> >>> [1]https://lore.kernel.org/lkml/CAMRc=MepDwUbAKrWgm0CXKObqy8=igtug0QDgo-CgwxjZCAC2Q@mail.gmail.com/ >> >> I don't think it's my client. The extra newlines are right there in the >> lore link. >> >> Look at the line that starts with "@@". That line is wrapped. The >> following line ("serdev_device *serdev)") should be at the end of the >> previous line. The same thing happened on the second "@@" line as well. >> > > Indeed. I just noticed that it applies fine with git apply and figured > the output must be right. Anyway, this is not a proper patch, I will > send one once I adapt Zijun's code. > > Bart Weird. Git apply failed for me. That's how I noticed it. From my terminal: ➜ linux git:(my_master) ✗ git apply ../blah.diff error: corrupt patch at line 6 FWIW, I also tried pasting it into stdin by running `git apply` as well. wt -- You're more amazing than you think! ^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2024-04-25 2:34 UTC | newest] Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-04-22 13:00 [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional() Bartosz Golaszewski 2024-04-24 4:05 ` Krzysztof Kozlowski 2024-04-24 4:55 ` quic_zijuhu 2024-04-24 5:07 ` Wren Turkal 2024-04-24 9:04 ` Bartosz Golaszewski 2024-04-24 9:32 ` quic_zijuhu 2024-04-24 9:50 ` Krzysztof Kozlowski 2024-04-24 11:16 ` Wren Turkal 2024-04-24 11:53 ` Wren Turkal 2024-04-24 11:56 ` Bartosz Golaszewski 2024-04-24 12:09 ` Wren Turkal 2024-04-24 12:17 ` Wren Turkal 2024-04-24 12:20 ` Bartosz Golaszewski 2024-04-24 12:23 ` Bartosz Golaszewski 2024-04-24 12:24 ` Wren Turkal 2024-04-24 12:27 ` Bartosz Golaszewski 2024-04-24 12:30 ` Wren Turkal 2024-04-24 12:57 ` Bartosz Golaszewski 2024-04-24 12:57 ` Wren Turkal 2024-04-24 13:12 ` quic_zijuhu 2024-04-24 13:26 ` Wren Turkal 2024-04-24 13:30 ` quic_zijuhu 2024-04-24 22:09 ` Wren Turkal 2024-04-24 13:53 ` Bartosz Golaszewski 2024-04-24 22:17 ` Wren Turkal 2024-04-24 23:35 ` quic_zijuhu 2024-04-25 2:34 ` Wren Turkal 2024-04-24 11:25 ` Wren Turkal 2024-04-24 11:53 ` Bartosz Golaszewski 2024-04-24 11:59 ` Wren Turkal 2024-04-24 12:01 ` Bartosz Golaszewski 2024-04-24 12:05 ` Wren Turkal
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).