From: Doug Anderson <dianders@chromium.org> To: Rob Herring <robh@kernel.org> Cc: "Minas Harutyunyan" <hminas@synopsys.com>, "Felipe Balbi" <felipe.balbi@linux.intel.com>, "Heiko Stübner" <heiko@sntech.de>, "Alan Stern" <stern@rowland.harvard.edu>, "Artur Petrosyan" <Arthur.Petrosyan@synopsys.com>, "Alexandru M Stan" <amstan@chromium.org>, "open list:ARM/Rockchip SoC..." <linux-rockchip@lists.infradead.org>, "William Wu" <william.wu@rock-chips.com>, linux-usb@vger.kernel.org, "Stefan Wahren" <stefan.wahren@i2se.com>, "Randy Li" <ayaka@soulik.info>, Chris <zyw@rock-chips.com>, "Matthias Kaehlcke" <mka@chromium.org>, "Ryan Case" <ryandcase@chromium.org>, "Amelie Delaunay" <amelie.delaunay@st.com>, "Julius Werner" <jwerner@chromium.org>, "Dinh Nguyen" <dinguyen@opensource.altera.com>, "Elaine Zhang" <zhangqing@rock-chips.com>, devicetree@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Mark Rutland" <mark.rutland@arm.com> Subject: [v2,3/5] Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB Date: Mon, 29 Apr 2019 22:25:52 -0700 [thread overview] Message-ID: <CAD=FV=UdtgAZBxuwjrrXtKT1sMfaELF8V193BF=UHg9fvM+yRw@mail.gmail.com> (raw) Hi, On Mon, Apr 29, 2019 at 6:23 PM Rob Herring <robh@kernel.org> wrote: > > On Wed, Apr 17, 2019 at 05:13:54PM -0700, Douglas Anderson wrote: > > Some SoCs with a dwc2 USB controller may need to keep the PHY on to > > support remote wakeup. Allow specifying this as a device tree > > property. > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > --- > > For relevant prior discussion on this patch, see: > > > > https://lkml.kernel.org/r/1435017144-2971-3-git-send-email-dianders@chromium.org > > > > I didn't make any changes from the prior version since I never found > > out what Rob thought of my previous arguments. If folks want a > > change, perhaps they could choose from these options: > > > > 1. Assume that all dwc2 hosts would like to keep their PHY on for > > suspend if there's a USB wakeup enabled, thus we totally drop this > > binding. This doesn't seem super great to me since I'd bet that > > many devices that use dwc2 weren't designed for USB wakeup (they > > may not keep enough clocks or rails on) so we might be wasting > > power for nothing. > > 1b. Use SoC specific compatible strings to enable/disable remote > wake-up. We can debate what the default is I guess. Unfortunately it's more than just SoC. While you need the SoC to be able to support this type of wakeup, you also need the board design, firmware design, regulator design, etc. ...so I don't think we can just use the SoC specific compatible string. In fact, while testing this I found that USB wakeup was totally broken unless I enabled "deep suspend" mode on my system. Something about the clocks / wakeup sources in the shallow suspend totally blocked it and I couldn't figure out what. ...so I believe it really needs to be something where someone has said: I tested it out on this board and everything is setup properly to support USB wakeup. > > 2. Rename this property to "snps,wakeup-from-suspend-with-phy" to make > > it more obvious that this property is intended both to document > > that wakeup from suspend is possible and that we need the PHY for > > said wakeup. > > 3. Rename this property to "snps,can-wakeup-from-suspend" and assume > > it's implicit that if we can wakeup from suspend that we need to > > keep the PHY on. If/when someone shows that a device exists using > > dwc2 where we can wakeup from suspend without the PHY they can add > > a new property. > > > > Changes in v2: None > > > > Documentation/devicetree/bindings/usb/dwc2.txt | 3 +++ > > 1 file changed, 3 insertions(+)
WARNING: multiple messages have this Message-ID (diff)
From: Doug Anderson <dianders@chromium.org> To: Rob Herring <robh@kernel.org> Cc: "Minas Harutyunyan" <hminas@synopsys.com>, "Felipe Balbi" <felipe.balbi@linux.intel.com>, "Heiko Stübner" <heiko@sntech.de>, "Alan Stern" <stern@rowland.harvard.edu>, "Artur Petrosyan" <Arthur.Petrosyan@synopsys.com>, "Alexandru M Stan" <amstan@chromium.org>, "open list:ARM/Rockchip SoC..." <linux-rockchip@lists.infradead.org>, "William Wu" <william.wu@rock-chips.com>, linux-usb@vger.kernel.org, "Stefan Wahren" <stefan.wahren@i2se.com>, "Randy Li" <ayaka@soulik.info>, Chris <zyw@rock-chips.com>, "Matthias Kaehlcke" <mka@chromium.org>, "Ryan Case" <ryandcase@chromium.org>, "Amelie Delaunay" <amelie.delaunay@st.com>, "Julius Werner" <jwerner@chromium.org>, "Dinh Nguyen" <dinguyen@opensource.altera.com>, "Elaine Zhang" <zhangqing@rock-chips.com>, devicetree@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Mark Rutland" <mark.rutland@arm.com> Subject: Re: [PATCH v2 3/5] Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB Date: Mon, 29 Apr 2019 22:25:52 -0700 [thread overview] Message-ID: <CAD=FV=UdtgAZBxuwjrrXtKT1sMfaELF8V193BF=UHg9fvM+yRw@mail.gmail.com> (raw) Message-ID: <20190430052552.tlq5lkFaYfvXsiNcDNC56iZaH0ksk-g_7eQ9GmN2d1w@z> (raw) In-Reply-To: <20190430012328.GA25660@bogus> Hi, On Mon, Apr 29, 2019 at 6:23 PM Rob Herring <robh@kernel.org> wrote: > > On Wed, Apr 17, 2019 at 05:13:54PM -0700, Douglas Anderson wrote: > > Some SoCs with a dwc2 USB controller may need to keep the PHY on to > > support remote wakeup. Allow specifying this as a device tree > > property. > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > --- > > For relevant prior discussion on this patch, see: > > > > https://lkml.kernel.org/r/1435017144-2971-3-git-send-email-dianders@chromium.org > > > > I didn't make any changes from the prior version since I never found > > out what Rob thought of my previous arguments. If folks want a > > change, perhaps they could choose from these options: > > > > 1. Assume that all dwc2 hosts would like to keep their PHY on for > > suspend if there's a USB wakeup enabled, thus we totally drop this > > binding. This doesn't seem super great to me since I'd bet that > > many devices that use dwc2 weren't designed for USB wakeup (they > > may not keep enough clocks or rails on) so we might be wasting > > power for nothing. > > 1b. Use SoC specific compatible strings to enable/disable remote > wake-up. We can debate what the default is I guess. Unfortunately it's more than just SoC. While you need the SoC to be able to support this type of wakeup, you also need the board design, firmware design, regulator design, etc. ...so I don't think we can just use the SoC specific compatible string. In fact, while testing this I found that USB wakeup was totally broken unless I enabled "deep suspend" mode on my system. Something about the clocks / wakeup sources in the shallow suspend totally blocked it and I couldn't figure out what. ...so I believe it really needs to be something where someone has said: I tested it out on this board and everything is setup properly to support USB wakeup. > > 2. Rename this property to "snps,wakeup-from-suspend-with-phy" to make > > it more obvious that this property is intended both to document > > that wakeup from suspend is possible and that we need the PHY for > > said wakeup. > > 3. Rename this property to "snps,can-wakeup-from-suspend" and assume > > it's implicit that if we can wakeup from suspend that we need to > > keep the PHY on. If/when someone shows that a device exists using > > dwc2 where we can wakeup from suspend without the PHY they can add > > a new property. > > > > Changes in v2: None > > > > Documentation/devicetree/bindings/usb/dwc2.txt | 3 +++ > > 1 file changed, 3 insertions(+)
next prev reply other threads:[~2019-04-30 5:25 UTC|newest] Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-18 0:13 [PATCH v2 0/5] USB: dwc2: Allow wakeup from suspend; enable for rk3288-veyron Douglas Anderson 2019-04-18 0:13 ` [v2,1/5] usb: dwc2: bus suspend/resume for hosts with DWC2_POWER_DOWN_PARAM_NONE Doug Anderson 2019-04-18 0:13 ` [PATCH v2 1/5] " Douglas Anderson 2019-04-29 8:43 ` [v2,1/5] " Artur Petrosyan 2019-04-29 8:43 ` [PATCH v2 1/5] " Artur Petrosyan 2019-04-29 17:33 ` [v2,1/5] " Doug Anderson 2019-04-29 17:33 ` [PATCH v2 1/5] " Doug Anderson 2019-04-30 6:05 ` [v2,1/5] " Artur Petrosyan 2019-04-30 6:05 ` [PATCH v2 1/5] " Artur Petrosyan 2019-05-01 1:51 ` [v2,1/5] " Doug Anderson 2019-05-01 1:51 ` [PATCH v2 1/5] " Doug Anderson 2019-05-03 8:20 ` [v2,1/5] " Artur Petrosyan 2019-05-03 8:20 ` [PATCH v2 1/5] " Artur Petrosyan 2019-05-03 15:03 ` [v2,1/5] " Doug Anderson 2019-05-03 15:03 ` [PATCH v2 1/5] " Doug Anderson 2019-05-07 7:26 ` Artur Petrosyan 2019-05-01 23:58 ` [v2,1/5] " Doug Anderson 2019-05-01 23:58 ` [PATCH v2 1/5] " Doug Anderson 2019-05-03 8:25 ` [v2,1/5] " Artur Petrosyan 2019-05-03 8:25 ` [PATCH v2 1/5] " Artur Petrosyan 2019-05-03 15:07 ` [v2,1/5] " Doug Anderson 2019-05-03 15:07 ` [PATCH v2 1/5] " Doug Anderson 2019-05-07 7:05 ` Artur Petrosyan 2019-04-18 0:13 ` [v2,2/5] USB: Export usb_wakeup_enabled_descendants() Doug Anderson 2019-04-18 0:13 ` [PATCH v2 2/5] " Douglas Anderson 2019-04-18 0:13 ` [v2,3/5] Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB Doug Anderson 2019-04-18 0:13 ` [PATCH v2 3/5] " Douglas Anderson 2019-04-25 12:40 ` [v2,3/5] " Felipe Balbi 2019-04-25 12:40 ` [PATCH v2 3/5] " Felipe Balbi 2019-04-25 18:09 ` [v2,3/5] " Doug Anderson 2019-04-25 18:09 ` [PATCH v2 3/5] " Doug Anderson 2019-04-25 19:58 ` [v2,3/5] " Doug Anderson 2019-04-25 19:58 ` [PATCH v2 3/5] " Doug Anderson 2019-05-02 18:36 ` [v2,3/5] " Doug Anderson 2019-05-02 18:36 ` [PATCH v2 3/5] " Doug Anderson 2019-04-30 1:23 ` [v2,3/5] " Rob Herring 2019-04-30 1:23 ` [PATCH v2 3/5] " Rob Herring 2019-04-30 5:25 ` Doug Anderson [this message] 2019-04-30 5:25 ` Doug Anderson 2019-04-18 0:13 ` [v2,4/5] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled Doug Anderson 2019-04-18 0:13 ` [PATCH v2 4/5] " Douglas Anderson 2019-04-25 12:41 ` [v2,4/5] " Felipe Balbi 2019-04-25 12:41 ` [PATCH v2 4/5] " Felipe Balbi 2019-04-18 0:13 ` [v2,5/5] ARM: dts: rockchip: Allow wakeup from rk3288-veyron's dwc2 USB ports Doug Anderson 2019-04-18 0:13 ` [PATCH v2 5/5] " Douglas Anderson 2019-04-18 12:40 ` [PATCH v2 0/5] USB: dwc2: Allow wakeup from suspend; enable for rk3288-veyron Minas Harutyunyan 2019-04-18 15:54 ` Doug Anderson 2019-04-19 11:43 ` Artur Petrosyan 2019-04-19 16:44 ` Artur Petrosyan 2019-04-22 15:50 ` Artur Petrosyan
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAD=FV=UdtgAZBxuwjrrXtKT1sMfaELF8V193BF=UHg9fvM+yRw@mail.gmail.com' \ --to=dianders@chromium.org \ --cc=Arthur.Petrosyan@synopsys.com \ --cc=amelie.delaunay@st.com \ --cc=amstan@chromium.org \ --cc=ayaka@soulik.info \ --cc=devicetree@vger.kernel.org \ --cc=dinguyen@opensource.altera.com \ --cc=felipe.balbi@linux.intel.com \ --cc=gregkh@linuxfoundation.org \ --cc=heiko@sntech.de \ --cc=hminas@synopsys.com \ --cc=jwerner@chromium.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-rockchip@lists.infradead.org \ --cc=linux-usb@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mka@chromium.org \ --cc=robh@kernel.org \ --cc=ryandcase@chromium.org \ --cc=stefan.wahren@i2se.com \ --cc=stern@rowland.harvard.edu \ --cc=william.wu@rock-chips.com \ --cc=zhangqing@rock-chips.com \ --cc=zyw@rock-chips.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).