On Wed, 24 Apr 2024 at 14:17, Wren Turkal <wt@xxxxxxxxxxxxxxxx> wrote: > > On 4/24/24 4:56 AM, Bartosz Golaszewski wrote: > > On Wed, Apr 24, 2024 at 1:53 PM Wren Turkal <wt@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx> > >>>> said: > >>>>> On 4/22/24 6:00 AM, Bartosz Golaszewski wrote: > >>>>>> From: Bartosz Golaszewski<bartosz.golaszewski@xxxxxxxxxx> > >>>>>> > >>>>>> 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!