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@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx> >>>>> 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@xxxxxxxxxxx/ 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@xxxxxxxxxxx/#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@xxxxxxxxxxx/ > wt
