Re: [PATCH v2] arm64: dts: qcom: sc7180: Configure USB as wakeup source

[Matthias Kaehlcke <mka@chromium.org>]

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>