Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <1644843828-20464-1-git-send-email-quic_vnivarth@quicinc.com>
In-Reply-To: <1644843828-20464-1-git-send-email-quic_vnivarth@quicinc.com>
From:   Doug Anderson <dianders@chromium.org>
Date:   Wed, 2 Mar 2022 08:42:05 -0800
Message-ID: <CAD=FV=XtkXganjG=ddgaq4-p+cAK1c4jqG=8Ux_xxdUrPpXhVA@mail.gmail.com>
Subject: Re: [PATCH] arm64: dts: qcom: sc7280: Configure cts sleep pinctrl to
 bias-disable for sc7280-idp
To:     Vijaya Krishna Nivarthi <quic_vnivarth@quicinc.com>
Cc:     Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>,
        msavaliy@qti.qualcomm.com, Matthias Kaehlcke <mka@chromium.org>,
        Stephen Boyd <swboyd@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi,

On Mon, Feb 14, 2022 at 5:04 AM Vijaya Krishna Nivarthi
<quic_vnivarth@quicinc.com> wrote:
>
> WLAN rail was leaking power during RBSC/sleep even after turning BT off.
> Change sleep pinctrl configuration to handle same.
>
> Signed-off-by: Vijaya Krishna Nivarthi <quic_vnivarth@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index d623d71..de18319 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -516,10 +516,10 @@
>                 pins = "gpio28";
>                 function = "gpio";
>                 /*
> -                * Configure a pull-down on CTS to match the pull of
> -                * the Bluetooth module.
> +                * Configure a disable on CTS to lower power usage
> +                * when BT is turned off.
>                  */
> -               bias-pull-down;
> +               bias-disable;

Why is sc7280 different from all of the previous devices? Did the
Bluetooth firmware change or are we measuring a different case? I know
we spent a lot of time carefully choosing each of these pulls before
so before changing them we should understand what changed.

CTS is an input from the AP's perspective, right? From the AP's
perspective then the case we need to be careful of is to prevent this
line from every floating while the AP is turned on. Specifically,
consider this case:

1. AP is powered on but has no pull on this line

2. The Bluetooth chip is powered off or otherwise configured to not
drive this line.

In that case the line will be floating. Its voltage will wander
around, influenced by other parts of the system. The downside here is
that, so I'm told, this will cause power draw on the AP. Each time the
voltage on the line floats between trigger points that the AP is
watching for it will trigger some logic in the AP and cause power
consumption. That's really not ideal.

So by disabling this pull you need to be _really_ sure that there's no
case where the AP is on and the Bluetooth chip is powered off / not
driving the line. In the past I don't think we were convinced of this,
which is why we configured a pull but tried to match it with what the
Bluetooth chip would do anyway.


So... How about using bias-bus-hold instead? That has the advantage of
keeping the line from floating but also shouldn't cause a constant
power draw. I believe it was created _exactly_ to deal with this type
of case. I don't think I was even aware that we supported
bias-bus-hold the last time we had this discussion and it seems like
it would solve the problem nicely. Does it work for you?

-Doug