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;
Date:   Wed, 28 Sep 2022 19:24:43 +0000
From:   Matthias Kaehlcke <mka@chromium.org>
To:     Doug Anderson <dianders@chromium.org>
Cc:     Andy Gross <agross@kernel.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>
Subject: Re: [PATCH v2] arm64: dts: qcom: sc7180: Configure USB as wakeup
 source
Message-ID: <YzSfe2dY2fyXDJLl@google.com>
References: <20220901102946.v2.1.I347ea409ee3134bd32a29e33fecd1a6ef32085a0@changeid>
 <CAD=FV=WBzVTaz1dtMswNMWhBzBBUQZTxqXff_DgiuP6WJgG8Qg@mail.gmail.com>
 <YxeZrvKvRB/ct3Ss@google.com>
 <CAD=FV=VsRi2kt9K9E+VOEGqdJFT43-aj415Gk2Q=OP64L-JAUA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CAD=FV=VsRi2kt9K9E+VOEGqdJFT43-aj415Gk2Q=OP64L-JAUA@mail.gmail.com>
Precedence: bulk

Hi Bjorn,

This patch should be ready for landing, could you pick it?

Thanks

Matthias

On Tue, Sep 06, 2022 at 12:22:42PM -0700, Doug Anderson wrote:
> 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>