Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220901102946.v2.1.I347ea409ee3134bd32a29e33fecd1a6ef32085a0@changeid>
 <CAD=FV=WBzVTaz1dtMswNMWhBzBBUQZTxqXff_DgiuP6WJgG8Qg@mail.gmail.com> <YxeZrvKvRB/ct3Ss@google.com>
In-Reply-To: <YxeZrvKvRB/ct3Ss@google.com>
From:   Doug Anderson <dianders@chromium.org>
Date:   Tue, 6 Sep 2022 12:22:42 -0700
Message-ID: <CAD=FV=VsRi2kt9K9E+VOEGqdJFT43-aj415Gk2Q=OP64L-JAUA@mail.gmail.com>
Subject: Re: [PATCH v2] arm64: dts: qcom: sc7180: Configure USB as wakeup source
To:     Matthias Kaehlcke <mka@chromium.org>
Cc:     Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Konrad Dybcio <konrad.dybcio@somainline.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        Krishna Kurapati <quic_kriskura@quicinc.com>,
        LKML <linux-kernel@vger.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        Bjorn Andersson <andersson@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi,

On Tue, Sep 6, 2022 at 12:04 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Hi Doug,
>
> On Tue, Sep 06, 2022 at 11:33:56AM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Sep 1, 2022 at 10:29 AM Matthias Kaehlcke <mka@chromium.org> wrote:
> > >
> > > The dwc3 USB controller of the sc7180 supports USB remote
> > > wakeup, configure it as a wakeup source.
> > >
> > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > > ---
> > >
> > > Changes in v2:
> > > - use qcom/arm64-for-6.1 as base, v1 was unintendedly based on a
> > >   downstream branch that was used for testing
> > >
> > >  arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > index e8debb0da411..af5bab27eaf3 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > @@ -2782,6 +2782,8 @@ usb_1: usb@a6f8800 {
> > >                                         <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3 0>;
> > >                         interconnect-names = "usb-ddr", "apps-usb";
> > >
> > > +                       wakeup-source;
> > > +
> >
> > The patch documenting this property has landed in commit 416b61893860
> > ("dt-bindings: usb: qcom,dwc3: add wakeup-source property"). I guess
> > the only question is whether this should be in the general sc7180
> > device tree file or just for trogdor.
>
> I had a similar comment on the patch for sc7280 [1], there the rationale
> for putting the property into the .dtsi of the SoC was that the wakeup
> capability is provided by the SoC.
>
> For sc8280xp.dtsi the property is also in the .dtsi of the SoC:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/tree/arch/arm64/boot/dts/qcom/sc8280xp.dtsi?h=arm64-for-6.1#n1315
>
> > Any chance it could cause problems for devices that aren't designed like
> > trogdor?
>
> Probably not in a functional sense, however power consumption during system
> suspend is slightly higher (2-3 mW) when USB wakeup is enabled. Boards can
> disable wakeup by deleting the property in their .dtsi file, though it
> is not necessarily evident that this is an option to reduce power
> consumption.
>
> [1] https://patchwork.kernel.org/project/linux-arm-msm/patch/1649321104-31322-7-git-send-email-quic_c_sanm@quicinc.com/

OK then.

Reviewed-by: Douglas Anderson <dianders@chromium.org>