Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned by gpiod_get_optional()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On 4/24/24 5:20 AM, Bartosz Golaszewski wrote:
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>
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:
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
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.


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.


@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!


You're more amazing than you think!

You're more amazing than you think!

[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux