2024-04-24 13:10:32

by Wren Turkal

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

On 4/24/24 5:29 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski<[email protected]>
>
> 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.
> While at it: also bail-out on error returned when trying to get the
> "swctrl" GPIO.
>
> Reported-by: Wren Turkal<[email protected]>
> Reported-by: Zijun Hu<[email protected]>
> Closes:https://lore.kernel.org/linux-bluetooth/[email protected]/
> Fixes: 56d074d26c58 ("Bluetooth: hci_qca: don't use IS_ERR_OR_NULL() with gpiod_get_optional()")
> Reviewed-by: Krzysztof Kozlowski<[email protected]>
> Signed-off-by: Bartosz Golaszewski<[email protected]>

Tested-by: "Wren Turkal" <[email protected]>


Like this?
wt
--
You're more amazing than you think!


2024-04-24 13:19:21

by Bartosz Golaszewski

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

On Wed, 24 Apr 2024 at 15:10, Wren Turkal <[email protected]> wrote:
>
> On 4/24/24 5:29 AM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski<[email protected]>
> >
> > 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.
> > While at it: also bail-out on error returned when trying to get the
> > "swctrl" GPIO.
> >
> > Reported-by: Wren Turkal<[email protected]>
> > Reported-by: Zijun Hu<[email protected]>
> > Closes:https://lore.kernel.org/linux-bluetooth/[email protected]/
> > Fixes: 56d074d26c58 ("Bluetooth: hci_qca: don't use IS_ERR_OR_NULL() with gpiod_get_optional()")
> > Reviewed-by: Krzysztof Kozlowski<[email protected]>
> > Signed-off-by: Bartosz Golaszewski<[email protected]>
>
> Tested-by: "Wren Turkal" <[email protected]>
>
>
> Like this?

Yes, awesome, thanks.

This is how reviewing works too in the kernel, look at what Krzysztof
did under v1, he just wrote:

Reviewed-by: Krzysztof Kozlowski<[email protected]>

And mailing list tools will pick it up.

Bartosz

2024-04-24 13:23:26

by quic_zijuhu

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

On 4/24/2024 9:18 PM, Bartosz Golaszewski wrote:
> On Wed, 24 Apr 2024 at 15:10, Wren Turkal <[email protected]> wrote:
>>
>> On 4/24/24 5:29 AM, Bartosz Golaszewski wrote:
>>> From: Bartosz Golaszewski<[email protected]>
>>>
>>> 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.
>>> While at it: also bail-out on error returned when trying to get the
>>> "swctrl" GPIO.
>>>
>>> Reported-by: Wren Turkal<[email protected]>
>>> Reported-by: Zijun Hu<[email protected]>
>>> Closes:https://lore.kernel.org/linux-bluetooth/[email protected]/
>>> Fixes: 56d074d26c58 ("Bluetooth: hci_qca: don't use IS_ERR_OR_NULL() with gpiod_get_optional()")
>>> Reviewed-by: Krzysztof Kozlowski<[email protected]>
>>> Signed-off-by: Bartosz Golaszewski<[email protected]>
>>
>> Tested-by: "Wren Turkal" <[email protected]>
>>
>>
>> Like this?
>
> Yes, awesome, thanks.
>
> This is how reviewing works too in the kernel, look at what Krzysztof
> did under v1, he just wrote:
>
> Reviewed-by: Krzysztof Kozlowski<[email protected]>
>
v1 have obvious something wrong as i pointed and verified.
so i think it is not suitable to attach v1's review-by tag to v2 anyway.

> And mailing list tools will pick it up.
>
> Bartosz