Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-3998-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240422130036.31856-1-brgl@bgdev.pl> <99242a7c-53bf-4227-9623-7bc092f564b3@penguintechs.org>
 <CAMRc=MepDwUbAKrWgm0CXKObqy8=igtug0QDgo-CgwxjZCAC2Q@mail.gmail.com>
 <b982b73d-0540-409c-a6e2-0e03ecb11715@penguintechs.org> <0381f39c-38ba-4a2b-915c-f14c5f911eb9@penguintechs.org>
 <CAMRc=MfnEct7ThQhCA3AoY7hxq8j1mmFLNNkK17+RSvJxs67XQ@mail.gmail.com>
 <2371f538-ec53-4037-b171-c62bf4e06eb1@penguintechs.org> <CACMJSeunUaj0cxLaN4MpFmX5vTOx_vnWjBN4Y2FavdQoQxFRkg@mail.gmail.com>
 <4c33304a-5dbc-450a-b874-e04ba9e93150@penguintechs.org> <CAMRc=MeDQFHX9r-sHNxqkfpi=LkHoRSL7i6dWokB-J+J03rBEg@mail.gmail.com>
 <93e15ecf-252f-4789-a23b-a28280f6aa84@quicinc.com> <e64b89b3-a8f5-44e7-ae7f-fc4298841f1c@penguintechs.org>
In-Reply-To: <e64b89b3-a8f5-44e7-ae7f-fc4298841f1c@penguintechs.org>
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Date: Wed, 24 Apr 2024 15:53:40 +0200
Message-ID: <CACMJSet1wjEive0-Z=0_eiY7umBoSF_VX3QvVgiwqxhTSDZg6g@mail.gmail.com>
Subject: Re: [PATCH] Bluetooth: qca: set power_ctrl_enabled on NULL returned
 by gpiod_get_optional()
To: Wren Turkal <wt@penguintechs.org>
Cc: quic_zijuhu <quic_zijuhu@quicinc.com>, Bartosz Golaszewski <brgl@bgdev.pl>, 
	linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, 
	Marcel Holtmann <marcel@holtmann.org>, Luiz Augusto von Dentz <luiz.dentz@gmail.com>, 
	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

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=E2=80=AFPM Wren Turkal <wt@penguintechs.o=
rg> wrote:
> >>>>>>
> >>>>>> That's OK, we have the first part right. Let's now see if we can r=
euse
> >>>>>> 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 result=
s.
>
> 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 =3D 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