linux-kernel - Re: [V5 1/2] arm64: dts: qcom: sc7280-idp: Configure CTS pin to bias-bus-hold for bluetooth

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAD=FV=XBNizxCh7Jr11fmbBpthwPRj8MWj2VQqOWt94RrcGWxQ@mail.gmail.com>
Date:   Wed, 20 Apr 2022 09:07:34 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Vijaya Krishna Nivarthi <quic_vnivarth@...cinc.com>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        krzysztof.kozlowski+dt@...aro.org,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
        quic_msavaliy@...cinc.com
Subject: Re: [V5 1/2] arm64: dts: qcom: sc7280-idp: Configure CTS pin to
 bias-bus-hold for bluetooth

Hi,

On Wed, Apr 20, 2022 at 12:27 AM Vijaya Krishna Nivarthi
<quic_vnivarth@...cinc.com> wrote:
>
> WLAN rail was leaking power during RBSC/sleep even after turning BT off.
> Change active and sleep pinctrl configurations to handle same.
>
> Signed-off-by: Vijaya Krishna Nivarthi <quic_vnivarth@...cinc.com>
> Reviewed-by: Douglas Anderson <dianders@...omium.org>
> ---
> v5: modify subject to include bluetooth
> v4: modify subject of patch to indicate file it is applying to
> v3: apply same change to active state and other sc7280*.dts* as well
> v2: used bias-bus-hold as per review comments
> v1: intial patch used bias-disable for sleep pinctrl in sc7280-idp only
> ---
>  arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index 015a347..85e7467 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -400,10 +400,10 @@
>
>  &qup_uart7_cts {
>         /*
> -        * Configure a pull-down on CTS to match the pull of
> -        * the Bluetooth module.
> +        * Configure a bias-bus-hold on CTS to lower power usage
> +        * when BT is turned off.

So you skipped half of Bjorn's feedback here. He said:

--

This comment would just leave a future reader with the question about
_why_ does this lower the power usage...

This problem you're seeing is likely to come back in the next platform
and your successor (or even yourself) will have no use of this comment
to figure out what bias to configure on these pins.

--

I personally am not convinced the comment is super valuable now that
we're using bias-bus-hold. The reason we want bias-bus-hold is
basically the exact reason that bias-bus-hold exists in the first
place and I personally wouldn't expect a big comment every place we
use bias-bus-hold. That being said, Bjorn is the maintainer and not
me, so he's the one you need to make happy.

What about:

Configure a bias-bus-hold on CTS to lower power usage when BT is
turned off. Bus hold will maintain a low power state regardless of
whether the Bluetooth module drives the pin in either direction or
leaves the pin fully unpowered.

-Doug