From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chunfeng Yun Subject: Re: [PATCH usb-next v10 3/8] usb: core: add a wrapper for the USB PHYs on the HCD Date: Tue, 20 Mar 2018 20:04:27 +0800 Message-ID: <1521547467.3717.92.camel@mhfsdcap03> References: <20180218184504.3331-1-martin.blumenstingl@googlemail.com> <20180218184504.3331-4-martin.blumenstingl@googlemail.com> <7f511a6f-f3a2-2002-f601-5ce1742f4539@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+glpam-linux-mediatek=m.gmane.org-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org To: Martin Blumenstingl , Roger Quadros Cc: mark.rutland-5wv7dgnIgG8@public.gmane.org, Peter.Chen-3arQi8VN3Tc@public.gmane.org, Neil Armstrong , linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, KISHON VIJAY ABRAHAM , linux-ci5G2KO2hbZ+pU9mqzGVBQ@public.gmane.org, felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org, stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, arnd-r2nGTMty4D4@public.gmane.org, yixun.lan-LpR1jeaWuhtBDgjK7y7TUQ@public.gmane.org, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, "Gerlach, Dave" , Keerthy , linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, mathias.nyman-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org List-Id: linux-tegra@vger.kernel.org Hi Martin & Roger: On Mon, 2018-03-19 at 17:12 +0100, Martin Blumenstingl wrote: > Hi Roger, > > On Mon, Mar 19, 2018 at 9:49 AM, Roger Quadros wrote: > > Hi, > > > > On 19/03/18 00:29, Martin Blumenstingl wrote: > >> Hi Roger, > >> > >> On Fri, Mar 16, 2018 at 3:32 PM, Roger Quadros wrote: > >>> +some TI folks > >>> > >>> Hi Martin, > >>> > >>> On 18/02/18 20:44, Martin Blumenstingl wrote: > >>>> Many SoC platforms have separate devices for the USB PHY which are > >>>> registered through the generic PHY framework. These PHYs have to be > >>>> enabled to make the USB controller actually work. They also have to be > >>>> disabled again on shutdown/suspend. > >>>> > >>>> Currently (at least) the following HCI platform drivers are using custom > >>>> code to obtain all PHYs via devicetree for the roothub/controller and > >>>> disable/enable them when required: > >>>> - ehci-platform.c has ehci_platform_power_{on,off} > >>>> - xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off} > >>>> - ohci-platform.c has ohci_platform_power_{on,off} > >>>> > >>>> With this new wrapper the USB PHYs can be specified directly in the > >>>> USB controller's devicetree node (just like on the drivers listed > >>>> above). This allows SoCs like the Amlogic Meson GXL family to operate > >>>> correctly once this is wired up correctly. These SoCs use a dwc3 > >>>> controller and require all USB PHYs to be initialized (if one of the USB > >>>> PHYs it not initialized then none of USB port works at all). > >>>> > >>>> Signed-off-by: Martin Blumenstingl > >>>> Tested-by: Yixun Lan > >>>> Cc: Neil Armstrong > >>>> Cc: Chunfeng Yun > >>> > >>> This patch is breaking low power cases on TI SoCs when USB is in host mode. > >>> I'll explain why below. > >> based on your explanation and reading the TI PHY drivers I am assuming > >> that the affected SoCs are using the "phy-omap-usb2" driver > >> > > yes and phy-ti-pipe3 as well i.e. "ti,phy-usb3" and "ti,omap-usb3" > I missed that, thanks > > >>>> --- > >>>> drivers/usb/core/Makefile | 2 +- > >>>> drivers/usb/core/phy.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++ > >>>> drivers/usb/core/phy.h | 7 ++ > >>>> 3 files changed, 166 insertions(+), 1 deletion(-) > >>>> create mode 100644 drivers/usb/core/phy.c > >>>> create mode 100644 drivers/usb/core/phy.h > >>>> > >>>> diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile > >>>> index 92c9cefb4317..18e874b0441e 100644 > >>>> --- a/drivers/usb/core/Makefile > >>>> +++ b/drivers/usb/core/Makefile > >>>> @@ -6,7 +6,7 @@ > >>>> usbcore-y := usb.o hub.o hcd.o urb.o message.o driver.o > >>>> usbcore-y += config.o file.o buffer.o sysfs.o endpoint.o > >>>> usbcore-y += devio.o notify.o generic.o quirks.o devices.o > >>>> -usbcore-y += port.o > >>>> +usbcore-y += phy.o port.o > >>>> > >>>> usbcore-$(CONFIG_OF) += of.o > >>>> usbcore-$(CONFIG_USB_PCI) += hcd-pci.o > >>>> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c > >>>> new file mode 100644 > >>>> index 000000000000..09b7c43c0ea4 > >>>> --- /dev/null > >>>> +++ b/drivers/usb/core/phy.c > >>>> @@ -0,0 +1,158 @@ > >>>> +// SPDX-License-Identifier: GPL-2.0+ > >>>> +/* > >>>> + * A wrapper for multiple PHYs which passes all phy_* function calls to > >>>> + * multiple (actual) PHY devices. This is comes handy when initializing > >>>> + * all PHYs on a HCD and to keep them all in the same state. > >>>> + * > >>>> + * Copyright (C) 2018 Martin Blumenstingl > >>>> + */ > >>>> + > >>>> +#include > >>>> +#include > >>>> +#include > >>>> +#include > >>>> + > >>>> +#include "phy.h" > >>>> + > >>>> +struct usb_phy_roothub { > >>>> + struct phy *phy; > >>>> + struct list_head list; > >>>> +}; > >>>> + > >>>> +static struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev) > >>>> +{ > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + > >>>> + roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL); > >>>> + if (!roothub_entry) > >>>> + return ERR_PTR(-ENOMEM); > >>>> + > >>>> + INIT_LIST_HEAD(&roothub_entry->list); > >>>> + > >>>> + return roothub_entry; > >>>> +} > >>>> + > >>>> +static int usb_phy_roothub_add_phy(struct device *dev, int index, > >>>> + struct list_head *list) > >>>> +{ > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + struct phy *phy = devm_of_phy_get_by_index(dev, dev->of_node, index); > >>>> + > >>>> + if (IS_ERR_OR_NULL(phy)) { > >>>> + if (!phy || PTR_ERR(phy) == -ENODEV) > >>>> + return 0; > >>>> + else > >>>> + return PTR_ERR(phy); > >>>> + } > >>>> + > >>>> + roothub_entry = usb_phy_roothub_alloc(dev); > >>>> + if (IS_ERR(roothub_entry)) > >>>> + return PTR_ERR(roothub_entry); > >>>> + > >>>> + roothub_entry->phy = phy; > >>>> + > >>>> + list_add_tail(&roothub_entry->list, list); > >>>> + > >>>> + return 0; > >>>> +} > >>>> + > >>>> +struct usb_phy_roothub *usb_phy_roothub_init(struct device *dev) > >>>> +{ > >>>> + struct usb_phy_roothub *phy_roothub; > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + struct list_head *head; > >>>> + int i, num_phys, err; > >>>> + > >>>> + num_phys = of_count_phandle_with_args(dev->of_node, "phys", > >>>> + "#phy-cells"); > >>>> + if (num_phys <= 0) > >>>> + return NULL; > >>>> + > >>>> + phy_roothub = usb_phy_roothub_alloc(dev); > >>>> + if (IS_ERR(phy_roothub)) > >>>> + return phy_roothub; > >>>> + > >>>> + for (i = 0; i < num_phys; i++) { > >>>> + err = usb_phy_roothub_add_phy(dev, i, &phy_roothub->list); > >>>> + if (err) > >>>> + goto err_out; > >>>> + } > >>>> + > >>>> + head = &phy_roothub->list; > >>>> + > >>>> + list_for_each_entry(roothub_entry, head, list) { > >>>> + err = phy_init(roothub_entry->phy); > >>> > >>> The phy_init() function actually enables the PHY clocks. > >>> It should be moved to the usb_phy_roothub_exit() routine just before calling phy_power_on(). > >> do you mean that phy_init should be moved to usb_phy_roothub_power_on > >> (just before phy_power_on is called within usb_phy_roothub_power_on)? > >> > > > > Yes. > > > >> an earlier version of my patch did exactly this, but it caused > >> problems during a suspend/resume cycle on Mediatek devices > >> Chunfeng Yun reported that issue here [0], quote from that mail for > >> easier reading: > >> "In order to keep link state on mt8173, we just power off all phys(not > >> exit) when system enter suspend, then power on them again (needn't > >> init, otherwise device will be disconnected) when system resume, this > >> can avoid re-enumerating device." > >> > >>>> + if (err) > >>>> + goto err_exit_phys; > >>>> + } > >>>> + > >>>> + return phy_roothub; > >>>> + > >>>> +err_exit_phys: > >>>> + list_for_each_entry_continue_reverse(roothub_entry, head, list) > >>>> + phy_exit(roothub_entry->phy); > >>>> + > >>>> +err_out: > >>>> + return ERR_PTR(err); > >>>> +} > >>>> +EXPORT_SYMBOL_GPL(usb_phy_roothub_init); > >>>> + > >>>> +int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub) > >>>> +{ > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + struct list_head *head; > >>>> + int err, ret = 0; > >>>> + > >>>> + if (!phy_roothub) > >>>> + return 0; > >>>> + > >>>> + head = &phy_roothub->list; > >>>> + > >>>> + list_for_each_entry(roothub_entry, head, list) { > >>>> + err = phy_exit(roothub_entry->phy); > >>>> + if (err) > >>>> + ret = ret; > >>>> + } > >>> > >>> phy_exit() should be moved to usb_phy_roothub_poweroff() just after calling phy_power_off(). > >> if I understood Chunfeng Yun correctly this will require > >> re-enumeration of the USB devices after a suspend/resume cycle on > >> Mediatek SoCs > >> > > > > OK. I suppose that there are 2 cases > > 1) Mediatek's case: USB controller context retained across suspend/resume. > > Remote wakeup probably required. > > No re-enumeration preferred after resume. phy_exit()/phy_init() must not be called > > during suspend/resume to keep PHY link active. > Documentation/devicetree/bindings/usb/mediatek,mtu3.txt indeed shows > that the parent of the USB controller can be marked as "wakeup-source" > > > 2) TI's case: low power important: USB context is lost, OK to re-enumerate. > > phy_exit()/phy_init() must be called during suspend/resume. > ACK > > >>> With that there is nothing else being done here. Shouldn't we be doing the equivalent of > >>> usb_phy_roothub_del_phy() and usb_phy_roothub_free() here? > >>> > >>>> + > >>>> + return ret; > >>>> +} > >>>> +EXPORT_SYMBOL_GPL(usb_phy_roothub_exit); > >>>> + > >>>> +int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub) > >>>> +{ > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + struct list_head *head; > >>>> + int err; > >>>> + > >>>> + if (!phy_roothub) > >>>> + return 0; > >>>> + > >>>> + head = &phy_roothub->list; > >>>> + > >>>> + list_for_each_entry(roothub_entry, head, list) { > >>>> + err = phy_power_on(roothub_entry->phy); > >>>> + if (err) > >>>> + goto err_out; > >>>> + } > >>>> + > >>>> + return 0; > >>>> + > >>>> +err_out: > >>>> + list_for_each_entry_continue_reverse(roothub_entry, head, list) > >>>> + phy_power_off(roothub_entry->phy); > >>>> + > >>>> + return err; > >>>> +} > >>>> +EXPORT_SYMBOL_GPL(usb_phy_roothub_power_on); > >>>> + > >>>> +void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub) > >>>> +{ > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + > >>>> + if (!phy_roothub) > >>>> + return; > >>>> + > >>>> + list_for_each_entry_reverse(roothub_entry, &phy_roothub->list, list) > >>>> + phy_power_off(roothub_entry->phy); > >>> > >>> Not doing the phy_exit() here leaves the clocks enabled on our SoC and > >>> we're no longer able to reach low power states on system suspend. > >> I'm not sure where this problem should be solved: > >> - set skip_phy_initialization in struct usb_hcd to 1 for the affected > >> TI platforms > > > > Many TI platforms are affected, omap5*, dra7*, am43* > > > >> - fix this in the usb_phy_roothub code > > > > I'd vote for fixing it in the usb_phy_roothub code. How? > > How about using the device_can_wakeup() to decide if we should call phy_exit()/init() or not? > > If the USB device can't wakeup the system there is no point in keeping it powered/clocked right? > @Chunfeng: can you confirm Roger's idea that we could call phy_exit if > the controller is *NOT* marked as "wakeup-source"? > I am also not sure if it would work, since the "wakeup-source" > property is defined on the USB controller's parent node in case of the > Mediatek MTU3 (Mediatek USB3.0 DRD) controller > Very sorry, I forgot that MTU3 & xHCI drivers always set them as wakeup devices by device_init_wakeup(dev, true),but not dependent on "wakeup-source" property, so maybe we can use device_can_wakeup() to decide whether call phy_exit()/init() or not. > >> - fix this in the PHY driver > > > > There is nothing to fix in the PHY driver. It is doing what it is supposed to do. > I actually wonder if phy_ops should have explicit suspend/resume support: > - assuming we define two new callbacks: .suspend and .resume > - the PHY framework could call .power_off by default if .suspend is not defined > - the PHY framework could call .power_on by default if .resume is not defined > - drivers could set .suspend and .resume on their own, allowing more > fine-grained control by for example *only* stopping the clock (but not > re-initializing the registers, etc.) > > @Roger: what do you think about this? > Kishon (the PHY framework maintainer) is also CC'ed - I would like to > hear his opinion too > > >> - somewhere else > >> > >>>> +} > >>>> +EXPORT_SYMBOL_GPL(usb_phy_roothub_power_off); > >>>> diff --git a/drivers/usb/core/phy.h b/drivers/usb/core/phy.h > >>>> new file mode 100644 > >>>> index 000000000000..6fde59bfbff8 > >>>> --- /dev/null > >>>> +++ b/drivers/usb/core/phy.h > >>>> @@ -0,0 +1,7 @@ > >>>> +struct usb_phy_roothub; > >>>> + > >>>> +struct usb_phy_roothub *usb_phy_roothub_init(struct device *dev); > >>>> +int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub); > >>>> + > >>>> +int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub); > >>>> +void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub); > >>>> > >>> > > > > > > > > -- > > cheers, > > -roger > > > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > > Regards > Martin From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [usb-next,v10,3/8] usb: core: add a wrapper for the USB PHYs on the HCD From: Chunfeng Yun Message-Id: <1521547467.3717.92.camel@mhfsdcap03> Date: Tue, 20 Mar 2018 20:04:27 +0800 To: Martin Blumenstingl , Roger Quadros Cc: linux-usb@vger.kernel.org, mathias.nyman@intel.com, arnd@arndb.de, gregkh@linuxfoundation.org, felipe.balbi@linux.intel.com, linux-omap@vger.kernel.org, linux-tegra@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, jonathanh@nvidia.com, thierry.reding@gmail.com, stern@rowland.harvard.edu, linux@prisktech.co.nz, Peter.Chen@nxp.com, matthias.bgg@gmail.com, mark.rutland@arm.com, robh+dt@kernel.org, Neil Armstrong , linux-amlogic@lists.infradead.org, yixun.lan@amlogic.com, Keerthy , "Gerlach, Dave" , KISHON VIJAY ABRAHAM List-ID: SGkgTWFydGluICYgUm9nZXI6CgpPbiBNb24sIDIwMTgtMDMtMTkgYXQgMTc6MTIgKzAxMDAsIE1h cnRpbiBCbHVtZW5zdGluZ2wgd3JvdGU6Cj4gSGkgUm9nZXIsCj4gCj4gT24gTW9uLCBNYXIgMTks IDIwMTggYXQgOTo0OSBBTSwgUm9nZXIgUXVhZHJvcyA8cm9nZXJxQHRpLmNvbT4gd3JvdGU6Cj4g PiBIaSwKPiA+Cj4gPiBPbiAxOS8wMy8xOCAwMDoyOSwgTWFydGluIEJsdW1lbnN0aW5nbCB3cm90 ZToKPiA+PiBIaSBSb2dlciwKPiA+Pgo+ID4+IE9uIEZyaSwgTWFyIDE2LCAyMDE4IGF0IDM6MzIg UE0sIFJvZ2VyIFF1YWRyb3MgPHJvZ2VycUB0aS5jb20+IHdyb3RlOgo+ID4+PiArc29tZSBUSSBm b2xrcwo+ID4+Pgo+ID4+PiBIaSBNYXJ0aW4sCj4gPj4+Cj4gPj4+IE9uIDE4LzAyLzE4IDIwOjQ0 LCBNYXJ0aW4gQmx1bWVuc3RpbmdsIHdyb3RlOgo+ID4+Pj4gTWFueSBTb0MgcGxhdGZvcm1zIGhh dmUgc2VwYXJhdGUgZGV2aWNlcyBmb3IgdGhlIFVTQiBQSFkgd2hpY2ggYXJlCj4gPj4+PiByZWdp c3RlcmVkIHRocm91Z2ggdGhlIGdlbmVyaWMgUEhZIGZyYW1ld29yay4gVGhlc2UgUEhZcyBoYXZl IHRvIGJlCj4gPj4+PiBlbmFibGVkIHRvIG1ha2UgdGhlIFVTQiBjb250cm9sbGVyIGFjdHVhbGx5 IHdvcmsuIFRoZXkgYWxzbyBoYXZlIHRvIGJlCj4gPj4+PiBkaXNhYmxlZCBhZ2FpbiBvbiBzaHV0 ZG93bi9zdXNwZW5kLgo+ID4+Pj4KPiA+Pj4+IEN1cnJlbnRseSAoYXQgbGVhc3QpIHRoZSBmb2xs b3dpbmcgSENJIHBsYXRmb3JtIGRyaXZlcnMgYXJlIHVzaW5nIGN1c3RvbQo+ID4+Pj4gY29kZSB0 byBvYnRhaW4gYWxsIFBIWXMgdmlhIGRldmljZXRyZWUgZm9yIHRoZSByb290aHViL2NvbnRyb2xs ZXIgYW5kCj4gPj4+PiBkaXNhYmxlL2VuYWJsZSB0aGVtIHdoZW4gcmVxdWlyZWQ6Cj4gPj4+PiAt IGVoY2ktcGxhdGZvcm0uYyBoYXMgZWhjaV9wbGF0Zm9ybV9wb3dlcl97b24sb2ZmfQo+ID4+Pj4g LSB4aGNpLW10ay5jIGhhcyB4aGNpX210a19waHlfe2luaXQsZXhpdCxwb3dlcl9vbixwb3dlcl9v ZmZ9Cj4gPj4+PiAtIG9oY2ktcGxhdGZvcm0uYyBoYXMgb2hjaV9wbGF0Zm9ybV9wb3dlcl97b24s b2ZmfQo+ID4+Pj4KPiA+Pj4+IFdpdGggdGhpcyBuZXcgd3JhcHBlciB0aGUgVVNCIFBIWXMgY2Fu IGJlIHNwZWNpZmllZCBkaXJlY3RseSBpbiB0aGUKPiA+Pj4+IFVTQiBjb250cm9sbGVyJ3MgZGV2 aWNldHJlZSBub2RlIChqdXN0IGxpa2Ugb24gdGhlIGRyaXZlcnMgbGlzdGVkCj4gPj4+PiBhYm92 ZSkuIFRoaXMgYWxsb3dzIFNvQ3MgbGlrZSB0aGUgQW1sb2dpYyBNZXNvbiBHWEwgZmFtaWx5IHRv IG9wZXJhdGUKPiA+Pj4+IGNvcnJlY3RseSBvbmNlIHRoaXMgaXMgd2lyZWQgdXAgY29ycmVjdGx5 LiBUaGVzZSBTb0NzIHVzZSBhIGR3YzMKPiA+Pj4+IGNvbnRyb2xsZXIgYW5kIHJlcXVpcmUgYWxs IFVTQiBQSFlzIHRvIGJlIGluaXRpYWxpemVkIChpZiBvbmUgb2YgdGhlIFVTQgo+ID4+Pj4gUEhZ cyBpdCBub3QgaW5pdGlhbGl6ZWQgdGhlbiBub25lIG9mIFVTQiBwb3J0IHdvcmtzIGF0IGFsbCku Cj4gPj4+Pgo+ID4+Pj4gU2lnbmVkLW9mZi1ieTogTWFydGluIEJsdW1lbnN0aW5nbCA8bWFydGlu LmJsdW1lbnN0aW5nbEBnb29nbGVtYWlsLmNvbT4KPiA+Pj4+IFRlc3RlZC1ieTogWWl4dW4gTGFu IDx5aXh1bi5sYW5AYW1sb2dpYy5jb20+Cj4gPj4+PiBDYzogTmVpbCBBcm1zdHJvbmcgPG5hcm1z dHJvbmdAYmF5bGlicmUuY29tPgo+ID4+Pj4gQ2M6IENodW5mZW5nIFl1biA8Y2h1bmZlbmcueXVu QG1lZGlhdGVrLmNvbT4KPiA+Pj4KPiA+Pj4gVGhpcyBwYXRjaCBpcyBicmVha2luZyBsb3cgcG93 ZXIgY2FzZXMgb24gVEkgU29DcyB3aGVuIFVTQiBpcyBpbiBob3N0IG1vZGUuCj4gPj4+IEknbGwg ZXhwbGFpbiB3aHkgYmVsb3cuCj4gPj4gYmFzZWQgb24geW91ciBleHBsYW5hdGlvbiBhbmQgcmVh ZGluZyB0aGUgVEkgUEhZIGRyaXZlcnMgSSBhbSBhc3N1bWluZwo+ID4+IHRoYXQgdGhlIGFmZmVj dGVkIFNvQ3MgYXJlIHVzaW5nIHRoZSAicGh5LW9tYXAtdXNiMiIgZHJpdmVyCj4gPj4KPiA+IHll cyBhbmQgcGh5LXRpLXBpcGUzIGFzIHdlbGwgaS5lLiAidGkscGh5LXVzYjMiIGFuZCAidGksb21h cC11c2IzIgo+IEkgbWlzc2VkIHRoYXQsIHRoYW5rcwo+IAo+ID4+Pj4gLS0tCj4gPj4+PiAgZHJp dmVycy91c2IvY29yZS9NYWtlZmlsZSB8ICAgMiArLQo+ID4+Pj4gIGRyaXZlcnMvdXNiL2NvcmUv cGh5LmMgICAgfCAxNTggKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr KysrKwo+ID4+Pj4gIGRyaXZlcnMvdXNiL2NvcmUvcGh5LmggICAgfCAgIDcgKysKPiA+Pj4+ICAz IGZpbGVzIGNoYW5nZWQsIDE2NiBpbnNlcnRpb25zKCspLCAxIGRlbGV0aW9uKC0pCj4gPj4+PiAg Y3JlYXRlIG1vZGUgMTAwNjQ0IGRyaXZlcnMvdXNiL2NvcmUvcGh5LmMKPiA+Pj4+ICBjcmVhdGUg bW9kZSAxMDA2NDQgZHJpdmVycy91c2IvY29yZS9waHkuaAo+ID4+Pj4KPiA+Pj4+IGRpZmYgLS1n aXQgYS9kcml2ZXJzL3VzYi9jb3JlL01ha2VmaWxlIGIvZHJpdmVycy91c2IvY29yZS9NYWtlZmls ZQo+ID4+Pj4gaW5kZXggOTJjOWNlZmI0MzE3Li4xOGU4NzRiMDQ0MWUgMTAwNjQ0Cj4gPj4+PiAt LS0gYS9kcml2ZXJzL3VzYi9jb3JlL01ha2VmaWxlCj4gPj4+PiArKysgYi9kcml2ZXJzL3VzYi9j b3JlL01ha2VmaWxlCj4gPj4+PiBAQCAtNiw3ICs2LDcgQEAKPiA+Pj4+ICB1c2Jjb3JlLXkgOj0g dXNiLm8gaHViLm8gaGNkLm8gdXJiLm8gbWVzc2FnZS5vIGRyaXZlci5vCj4gPj4+PiAgdXNiY29y ZS15ICs9IGNvbmZpZy5vIGZpbGUubyBidWZmZXIubyBzeXNmcy5vIGVuZHBvaW50Lm8KPiA+Pj4+ ICB1c2Jjb3JlLXkgKz0gZGV2aW8ubyBub3RpZnkubyBnZW5lcmljLm8gcXVpcmtzLm8gZGV2aWNl cy5vCj4gPj4+PiAtdXNiY29yZS15ICs9IHBvcnQubwo+ID4+Pj4gK3VzYmNvcmUteSArPSBwaHku byBwb3J0Lm8KPiA+Pj4+Cj4gPj4+PiAgdXNiY29yZS0kKENPTkZJR19PRikgICAgICAgICArPSBv Zi5vCj4gPj4+PiAgdXNiY29yZS0kKENPTkZJR19VU0JfUENJKSAgICAgICAgICAgICs9IGhjZC1w Y2kubwo+ID4+Pj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvdXNiL2NvcmUvcGh5LmMgYi9kcml2ZXJz L3VzYi9jb3JlL3BoeS5jCj4gPj4+PiBuZXcgZmlsZSBtb2RlIDEwMDY0NAo+ID4+Pj4gaW5kZXgg MDAwMDAwMDAwMDAwLi4wOWI3YzQzYzBlYTQKPiA+Pj4+IC0tLSAvZGV2L251bGwKPiA+Pj4+ICsr KyBiL2RyaXZlcnMvdXNiL2NvcmUvcGh5LmMKPiA+Pj4+IEBAIC0wLDAgKzEsMTU4IEBACj4gPj4+ PiArLy8gU1BEWC1MaWNlbnNlLUlkZW50aWZpZXI6IEdQTC0yLjArCj4gPj4+PiArLyoKPiA+Pj4+ ICsgKiBBIHdyYXBwZXIgZm9yIG11bHRpcGxlIFBIWXMgd2hpY2ggcGFzc2VzIGFsbCBwaHlfKiBm dW5jdGlvbiBjYWxscyB0bwo+ID4+Pj4gKyAqIG11bHRpcGxlIChhY3R1YWwpIFBIWSBkZXZpY2Vz LiBUaGlzIGlzIGNvbWVzIGhhbmR5IHdoZW4gaW5pdGlhbGl6aW5nCj4gPj4+PiArICogYWxsIFBI WXMgb24gYSBIQ0QgYW5kIHRvIGtlZXAgdGhlbSBhbGwgaW4gdGhlIHNhbWUgc3RhdGUuCj4gPj4+ PiArICoKPiA+Pj4+ICsgKiBDb3B5cmlnaHQgKEMpIDIwMTggTWFydGluIEJsdW1lbnN0aW5nbCA8 bWFydGluLmJsdW1lbnN0aW5nbEBnb29nbGVtYWlsLmNvbT4KPiA+Pj4+ICsgKi8KPiA+Pj4+ICsK PiA+Pj4+ICsjaW5jbHVkZSA8bGludXgvZGV2aWNlLmg+Cj4gPj4+PiArI2luY2x1ZGUgPGxpbnV4 L2xpc3QuaD4KPiA+Pj4+ICsjaW5jbHVkZSA8bGludXgvcGh5L3BoeS5oPgo+ID4+Pj4gKyNpbmNs dWRlIDxsaW51eC9vZi5oPgo+ID4+Pj4gKwo+ID4+Pj4gKyNpbmNsdWRlICJwaHkuaCIKPiA+Pj4+ ICsKPiA+Pj4+ICtzdHJ1Y3QgdXNiX3BoeV9yb290aHViIHsKPiA+Pj4+ICsgICAgIHN0cnVjdCBw aHkgICAgICAgICAgICAgICpwaHk7Cj4gPj4+PiArICAgICBzdHJ1Y3QgbGlzdF9oZWFkICAgICAg ICBsaXN0Owo+ID4+Pj4gK307Cj4gPj4+PiArCj4gPj4+PiArc3RhdGljIHN0cnVjdCB1c2JfcGh5 X3Jvb3RodWIgKnVzYl9waHlfcm9vdGh1Yl9hbGxvYyhzdHJ1Y3QgZGV2aWNlICpkZXYpCj4gPj4+ PiArewo+ID4+Pj4gKyAgICAgc3RydWN0IHVzYl9waHlfcm9vdGh1YiAqcm9vdGh1Yl9lbnRyeTsK PiA+Pj4+ICsKPiA+Pj4+ICsgICAgIHJvb3RodWJfZW50cnkgPSBkZXZtX2t6YWxsb2MoZGV2LCBz aXplb2YoKnJvb3RodWJfZW50cnkpLCBHRlBfS0VSTkVMKTsKPiA+Pj4+ICsgICAgIGlmICghcm9v dGh1Yl9lbnRyeSkKPiA+Pj4+ICsgICAgICAgICAgICAgcmV0dXJuIEVSUl9QVFIoLUVOT01FTSk7 Cj4gPj4+PiArCj4gPj4+PiArICAgICBJTklUX0xJU1RfSEVBRCgmcm9vdGh1Yl9lbnRyeS0+bGlz dCk7Cj4gPj4+PiArCj4gPj4+PiArICAgICByZXR1cm4gcm9vdGh1Yl9lbnRyeTsKPiA+Pj4+ICt9 Cj4gPj4+PiArCj4gPj4+PiArc3RhdGljIGludCB1c2JfcGh5X3Jvb3RodWJfYWRkX3BoeShzdHJ1 Y3QgZGV2aWNlICpkZXYsIGludCBpbmRleCwKPiA+Pj4+ICsgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgIHN0cnVjdCBsaXN0X2hlYWQgKmxpc3QpCj4gPj4+PiArewo+ID4+Pj4gKyAgICAg c3RydWN0IHVzYl9waHlfcm9vdGh1YiAqcm9vdGh1Yl9lbnRyeTsKPiA+Pj4+ICsgICAgIHN0cnVj dCBwaHkgKnBoeSA9IGRldm1fb2ZfcGh5X2dldF9ieV9pbmRleChkZXYsIGRldi0+b2Zfbm9kZSwg aW5kZXgpOwo+ID4+Pj4gKwo+ID4+Pj4gKyAgICAgaWYgKElTX0VSUl9PUl9OVUxMKHBoeSkpIHsK PiA+Pj4+ICsgICAgICAgICAgICAgaWYgKCFwaHkgfHwgUFRSX0VSUihwaHkpID09IC1FTk9ERVYp Cj4gPj4+PiArICAgICAgICAgICAgICAgICAgICAgcmV0dXJuIDA7Cj4gPj4+PiArICAgICAgICAg ICAgIGVsc2UKPiA+Pj4+ICsgICAgICAgICAgICAgICAgICAgICByZXR1cm4gUFRSX0VSUihwaHkp Owo+ID4+Pj4gKyAgICAgfQo+ID4+Pj4gKwo+ID4+Pj4gKyAgICAgcm9vdGh1Yl9lbnRyeSA9IHVz Yl9waHlfcm9vdGh1Yl9hbGxvYyhkZXYpOwo+ID4+Pj4gKyAgICAgaWYgKElTX0VSUihyb290aHVi X2VudHJ5KSkKPiA+Pj4+ICsgICAgICAgICAgICAgcmV0dXJuIFBUUl9FUlIocm9vdGh1Yl9lbnRy eSk7Cj4gPj4+PiArCj4gPj4+PiArICAgICByb290aHViX2VudHJ5LT5waHkgPSBwaHk7Cj4gPj4+ PiArCj4gPj4+PiArICAgICBsaXN0X2FkZF90YWlsKCZyb290aHViX2VudHJ5LT5saXN0LCBsaXN0 KTsKPiA+Pj4+ICsKPiA+Pj4+ICsgICAgIHJldHVybiAwOwo+ID4+Pj4gK30KPiA+Pj4+ICsKPiA+ Pj4+ICtzdHJ1Y3QgdXNiX3BoeV9yb290aHViICp1c2JfcGh5X3Jvb3RodWJfaW5pdChzdHJ1Y3Qg ZGV2aWNlICpkZXYpCj4gPj4+PiArewo+ID4+Pj4gKyAgICAgc3RydWN0IHVzYl9waHlfcm9vdGh1 YiAqcGh5X3Jvb3RodWI7Cj4gPj4+PiArICAgICBzdHJ1Y3QgdXNiX3BoeV9yb290aHViICpyb290 aHViX2VudHJ5Owo+ID4+Pj4gKyAgICAgc3RydWN0IGxpc3RfaGVhZCAqaGVhZDsKPiA+Pj4+ICsg ICAgIGludCBpLCBudW1fcGh5cywgZXJyOwo+ID4+Pj4gKwo+ID4+Pj4gKyAgICAgbnVtX3BoeXMg PSBvZl9jb3VudF9waGFuZGxlX3dpdGhfYXJncyhkZXYtPm9mX25vZGUsICJwaHlzIiwKPiA+Pj4+ ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIiNwaHktY2VsbHMi KTsKPiA+Pj4+ICsgICAgIGlmIChudW1fcGh5cyA8PSAwKQo+ID4+Pj4gKyAgICAgICAgICAgICBy ZXR1cm4gTlVMTDsKPiA+Pj4+ICsKPiA+Pj4+ICsgICAgIHBoeV9yb290aHViID0gdXNiX3BoeV9y b290aHViX2FsbG9jKGRldik7Cj4gPj4+PiArICAgICBpZiAoSVNfRVJSKHBoeV9yb290aHViKSkK PiA+Pj4+ICsgICAgICAgICAgICAgcmV0dXJuIHBoeV9yb290aHViOwo+ID4+Pj4gKwo+ID4+Pj4g KyAgICAgZm9yIChpID0gMDsgaSA8IG51bV9waHlzOyBpKyspIHsKPiA+Pj4+ICsgICAgICAgICAg ICAgZXJyID0gdXNiX3BoeV9yb290aHViX2FkZF9waHkoZGV2LCBpLCAmcGh5X3Jvb3RodWItPmxp c3QpOwo+ID4+Pj4gKyAgICAgICAgICAgICBpZiAoZXJyKQo+ID4+Pj4gKyAgICAgICAgICAgICAg ICAgICAgIGdvdG8gZXJyX291dDsKPiA+Pj4+ICsgICAgIH0KPiA+Pj4+ICsKPiA+Pj4+ICsgICAg IGhlYWQgPSAmcGh5X3Jvb3RodWItPmxpc3Q7Cj4gPj4+PiArCj4gPj4+PiArICAgICBsaXN0X2Zv cl9lYWNoX2VudHJ5KHJvb3RodWJfZW50cnksIGhlYWQsIGxpc3QpIHsKPiA+Pj4+ICsgICAgICAg ICAgICAgZXJyID0gcGh5X2luaXQocm9vdGh1Yl9lbnRyeS0+cGh5KTsKPiA+Pj4KPiA+Pj4gVGhl IHBoeV9pbml0KCkgZnVuY3Rpb24gYWN0dWFsbHkgZW5hYmxlcyB0aGUgUEhZIGNsb2Nrcy4KPiA+ Pj4gSXQgc2hvdWxkIGJlIG1vdmVkIHRvIHRoZSB1c2JfcGh5X3Jvb3RodWJfZXhpdCgpIHJvdXRp bmUganVzdCBiZWZvcmUgY2FsbGluZyBwaHlfcG93ZXJfb24oKS4KPiA+PiBkbyB5b3UgbWVhbiB0 aGF0IHBoeV9pbml0IHNob3VsZCBiZSBtb3ZlZCB0byB1c2JfcGh5X3Jvb3RodWJfcG93ZXJfb24K PiA+PiAoanVzdCBiZWZvcmUgcGh5X3Bvd2VyX29uIGlzIGNhbGxlZCB3aXRoaW4gdXNiX3BoeV9y b290aHViX3Bvd2VyX29uKT8KPiA+Pgo+ID4KPiA+IFllcy4KPiA+Cj4gPj4gYW4gZWFybGllciB2 ZXJzaW9uIG9mIG15IHBhdGNoIGRpZCBleGFjdGx5IHRoaXMsIGJ1dCBpdCBjYXVzZWQKPiA+PiBw cm9ibGVtcyBkdXJpbmcgYSBzdXNwZW5kL3Jlc3VtZSBjeWNsZSBvbiBNZWRpYXRlayBkZXZpY2Vz Cj4gPj4gQ2h1bmZlbmcgWXVuIHJlcG9ydGVkIHRoYXQgaXNzdWUgaGVyZSBbMF0sIHF1b3RlIGZy b20gdGhhdCBtYWlsIGZvcgo+ID4+IGVhc2llciByZWFkaW5nOgo+ID4+ICJJbiBvcmRlciB0byBr ZWVwIGxpbmsgc3RhdGUgb24gbXQ4MTczLCB3ZSBqdXN0IHBvd2VyIG9mZiBhbGwgcGh5cyhub3QK PiA+PiBleGl0KSB3aGVuIHN5c3RlbSBlbnRlciBzdXNwZW5kLCB0aGVuIHBvd2VyIG9uIHRoZW0g YWdhaW4gKG5lZWRuJ3QKPiA+PiBpbml0LCBvdGhlcndpc2UgZGV2aWNlIHdpbGwgYmUgZGlzY29u bmVjdGVkKSB3aGVuIHN5c3RlbSByZXN1bWUsIHRoaXMKPiA+PiBjYW4gYXZvaWQgcmUtZW51bWVy YXRpbmcgZGV2aWNlLiIKPiA+Pgo+ID4+Pj4gKyAgICAgICAgICAgICBpZiAoZXJyKQo+ID4+Pj4g KyAgICAgICAgICAgICAgICAgICAgIGdvdG8gZXJyX2V4aXRfcGh5czsKPiA+Pj4+ICsgICAgIH0K PiA+Pj4+ICsKPiA+Pj4+ICsgICAgIHJldHVybiBwaHlfcm9vdGh1YjsKPiA+Pj4+ICsKPiA+Pj4+ ICtlcnJfZXhpdF9waHlzOgo+ID4+Pj4gKyAgICAgbGlzdF9mb3JfZWFjaF9lbnRyeV9jb250aW51 ZV9yZXZlcnNlKHJvb3RodWJfZW50cnksIGhlYWQsIGxpc3QpCj4gPj4+PiArICAgICAgICAgICAg IHBoeV9leGl0KHJvb3RodWJfZW50cnktPnBoeSk7Cj4gPj4+PiArCj4gPj4+PiArZXJyX291dDoK PiA+Pj4+ICsgICAgIHJldHVybiBFUlJfUFRSKGVycik7Cj4gPj4+PiArfQo+ID4+Pj4gK0VYUE9S VF9TWU1CT0xfR1BMKHVzYl9waHlfcm9vdGh1Yl9pbml0KTsKPiA+Pj4+ICsKPiA+Pj4+ICtpbnQg dXNiX3BoeV9yb290aHViX2V4aXQoc3RydWN0IHVzYl9waHlfcm9vdGh1YiAqcGh5X3Jvb3RodWIp Cj4gPj4+PiArewo+ID4+Pj4gKyAgICAgc3RydWN0IHVzYl9waHlfcm9vdGh1YiAqcm9vdGh1Yl9l bnRyeTsKPiA+Pj4+ICsgICAgIHN0cnVjdCBsaXN0X2hlYWQgKmhlYWQ7Cj4gPj4+PiArICAgICBp bnQgZXJyLCByZXQgPSAwOwo+ID4+Pj4gKwo+ID4+Pj4gKyAgICAgaWYgKCFwaHlfcm9vdGh1YikK PiA+Pj4+ICsgICAgICAgICAgICAgcmV0dXJuIDA7Cj4gPj4+PiArCj4gPj4+PiArICAgICBoZWFk ID0gJnBoeV9yb290aHViLT5saXN0Owo+ID4+Pj4gKwo+ID4+Pj4gKyAgICAgbGlzdF9mb3JfZWFj aF9lbnRyeShyb290aHViX2VudHJ5LCBoZWFkLCBsaXN0KSB7Cj4gPj4+PiArICAgICAgICAgICAg IGVyciA9IHBoeV9leGl0KHJvb3RodWJfZW50cnktPnBoeSk7Cj4gPj4+PiArICAgICAgICAgICAg IGlmIChlcnIpCj4gPj4+PiArICAgICAgICAgICAgICAgICAgICAgcmV0ID0gcmV0Owo+ID4+Pj4g KyAgICAgfQo+ID4+Pgo+ID4+PiBwaHlfZXhpdCgpIHNob3VsZCBiZSBtb3ZlZCB0byB1c2JfcGh5 X3Jvb3RodWJfcG93ZXJvZmYoKSBqdXN0IGFmdGVyIGNhbGxpbmcgcGh5X3Bvd2VyX29mZigpLgo+ ID4+IGlmIEkgdW5kZXJzdG9vZCBDaHVuZmVuZyBZdW4gY29ycmVjdGx5IHRoaXMgd2lsbCByZXF1 aXJlCj4gPj4gcmUtZW51bWVyYXRpb24gb2YgdGhlIFVTQiBkZXZpY2VzIGFmdGVyIGEgc3VzcGVu ZC9yZXN1bWUgY3ljbGUgb24KPiA+PiBNZWRpYXRlayBTb0NzCj4gPj4KPiA+Cj4gPiBPSy4gSSBz dXBwb3NlIHRoYXQgdGhlcmUgYXJlIDIgY2FzZXMKPiA+IDEpIE1lZGlhdGVrJ3MgY2FzZTogVVNC IGNvbnRyb2xsZXIgY29udGV4dCByZXRhaW5lZCBhY3Jvc3Mgc3VzcGVuZC9yZXN1bWUuCj4gPiBS ZW1vdGUgd2FrZXVwIHByb2JhYmx5IHJlcXVpcmVkLgo+ID4gTm8gcmUtZW51bWVyYXRpb24gcHJl ZmVycmVkIGFmdGVyIHJlc3VtZS4gcGh5X2V4aXQoKS9waHlfaW5pdCgpIG11c3Qgbm90IGJlIGNh bGxlZAo+ID4gZHVyaW5nIHN1c3BlbmQvcmVzdW1lIHRvIGtlZXAgUEhZIGxpbmsgYWN0aXZlLgo+ IERvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy91c2IvbWVkaWF0ZWssbXR1My50eHQg aW5kZWVkIHNob3dzCj4gdGhhdCB0aGUgcGFyZW50IG9mIHRoZSBVU0IgY29udHJvbGxlciBjYW4g YmUgbWFya2VkIGFzICJ3YWtldXAtc291cmNlIgo+IAo+ID4gMikgVEkncyBjYXNlOiBsb3cgcG93 ZXIgaW1wb3J0YW50OiBVU0IgY29udGV4dCBpcyBsb3N0LCBPSyB0byByZS1lbnVtZXJhdGUuCj4g PiBwaHlfZXhpdCgpL3BoeV9pbml0KCkgbXVzdCBiZSBjYWxsZWQgZHVyaW5nIHN1c3BlbmQvcmVz dW1lLgo+IEFDSwo+IAo+ID4+PiBXaXRoIHRoYXQgdGhlcmUgaXMgbm90aGluZyBlbHNlIGJlaW5n IGRvbmUgaGVyZS4gU2hvdWxkbid0IHdlIGJlIGRvaW5nIHRoZSBlcXVpdmFsZW50IG9mCj4gPj4+ IHVzYl9waHlfcm9vdGh1Yl9kZWxfcGh5KCkgYW5kIHVzYl9waHlfcm9vdGh1Yl9mcmVlKCkgaGVy ZT8KPiA+Pj4KPiA+Pj4+ICsKPiA+Pj4+ICsgICAgIHJldHVybiByZXQ7Cj4gPj4+PiArfQo+ID4+ Pj4gK0VYUE9SVF9TWU1CT0xfR1BMKHVzYl9waHlfcm9vdGh1Yl9leGl0KTsKPiA+Pj4+ICsKPiA+ Pj4+ICtpbnQgdXNiX3BoeV9yb290aHViX3Bvd2VyX29uKHN0cnVjdCB1c2JfcGh5X3Jvb3RodWIg KnBoeV9yb290aHViKQo+ID4+Pj4gK3sKPiA+Pj4+ICsgICAgIHN0cnVjdCB1c2JfcGh5X3Jvb3Ro dWIgKnJvb3RodWJfZW50cnk7Cj4gPj4+PiArICAgICBzdHJ1Y3QgbGlzdF9oZWFkICpoZWFkOwo+ ID4+Pj4gKyAgICAgaW50IGVycjsKPiA+Pj4+ICsKPiA+Pj4+ICsgICAgIGlmICghcGh5X3Jvb3Ro dWIpCj4gPj4+PiArICAgICAgICAgICAgIHJldHVybiAwOwo+ID4+Pj4gKwo+ID4+Pj4gKyAgICAg aGVhZCA9ICZwaHlfcm9vdGh1Yi0+bGlzdDsKPiA+Pj4+ICsKPiA+Pj4+ICsgICAgIGxpc3RfZm9y X2VhY2hfZW50cnkocm9vdGh1Yl9lbnRyeSwgaGVhZCwgbGlzdCkgewo+ID4+Pj4gKyAgICAgICAg ICAgICBlcnIgPSBwaHlfcG93ZXJfb24ocm9vdGh1Yl9lbnRyeS0+cGh5KTsKPiA+Pj4+ICsgICAg ICAgICAgICAgaWYgKGVycikKPiA+Pj4+ICsgICAgICAgICAgICAgICAgICAgICBnb3RvIGVycl9v dXQ7Cj4gPj4+PiArICAgICB9Cj4gPj4+PiArCj4gPj4+PiArICAgICByZXR1cm4gMDsKPiA+Pj4+ ICsKPiA+Pj4+ICtlcnJfb3V0Ogo+ID4+Pj4gKyAgICAgbGlzdF9mb3JfZWFjaF9lbnRyeV9jb250 aW51ZV9yZXZlcnNlKHJvb3RodWJfZW50cnksIGhlYWQsIGxpc3QpCj4gPj4+PiArICAgICAgICAg ICAgIHBoeV9wb3dlcl9vZmYocm9vdGh1Yl9lbnRyeS0+cGh5KTsKPiA+Pj4+ICsKPiA+Pj4+ICsg ICAgIHJldHVybiBlcnI7Cj4gPj4+PiArfQo+ID4+Pj4gK0VYUE9SVF9TWU1CT0xfR1BMKHVzYl9w aHlfcm9vdGh1Yl9wb3dlcl9vbik7Cj4gPj4+PiArCj4gPj4+PiArdm9pZCB1c2JfcGh5X3Jvb3Ro dWJfcG93ZXJfb2ZmKHN0cnVjdCB1c2JfcGh5X3Jvb3RodWIgKnBoeV9yb290aHViKQo+ID4+Pj4g K3sKPiA+Pj4+ICsgICAgIHN0cnVjdCB1c2JfcGh5X3Jvb3RodWIgKnJvb3RodWJfZW50cnk7Cj4g Pj4+PiArCj4gPj4+PiArICAgICBpZiAoIXBoeV9yb290aHViKQo+ID4+Pj4gKyAgICAgICAgICAg ICByZXR1cm47Cj4gPj4+PiArCj4gPj4+PiArICAgICBsaXN0X2Zvcl9lYWNoX2VudHJ5X3JldmVy c2Uocm9vdGh1Yl9lbnRyeSwgJnBoeV9yb290aHViLT5saXN0LCBsaXN0KQo+ID4+Pj4gKyAgICAg ICAgICAgICBwaHlfcG93ZXJfb2ZmKHJvb3RodWJfZW50cnktPnBoeSk7Cj4gPj4+Cj4gPj4+IE5v dCBkb2luZyB0aGUgcGh5X2V4aXQoKSBoZXJlIGxlYXZlcyB0aGUgY2xvY2tzIGVuYWJsZWQgb24g b3VyIFNvQyBhbmQKPiA+Pj4gd2UncmUgbm8gbG9uZ2VyIGFibGUgdG8gcmVhY2ggbG93IHBvd2Vy IHN0YXRlcyBvbiBzeXN0ZW0gc3VzcGVuZC4KPiA+PiBJJ20gbm90IHN1cmUgd2hlcmUgdGhpcyBw cm9ibGVtIHNob3VsZCBiZSBzb2x2ZWQ6Cj4gPj4gLSBzZXQgc2tpcF9waHlfaW5pdGlhbGl6YXRp b24gaW4gc3RydWN0IHVzYl9oY2QgdG8gMSBmb3IgdGhlIGFmZmVjdGVkCj4gPj4gVEkgcGxhdGZv cm1zCj4gPgo+ID4gTWFueSBUSSBwbGF0Zm9ybXMgYXJlIGFmZmVjdGVkLCBvbWFwNSosIGRyYTcq LCBhbTQzKgo+ID4KPiA+PiAtIGZpeCB0aGlzIGluIHRoZSB1c2JfcGh5X3Jvb3RodWIgY29kZQo+ ID4KPiA+IEknZCB2b3RlIGZvciBmaXhpbmcgaXQgaW4gdGhlIHVzYl9waHlfcm9vdGh1YiBjb2Rl LiBIb3c/Cj4gPiBIb3cgYWJvdXQgdXNpbmcgdGhlIGRldmljZV9jYW5fd2FrZXVwKCkgdG8gZGVj aWRlIGlmIHdlIHNob3VsZCBjYWxsIHBoeV9leGl0KCkvaW5pdCgpIG9yIG5vdD8KPiA+IElmIHRo ZSBVU0IgZGV2aWNlIGNhbid0IHdha2V1cCB0aGUgc3lzdGVtIHRoZXJlIGlzIG5vIHBvaW50IGlu IGtlZXBpbmcgaXQgcG93ZXJlZC9jbG9ja2VkIHJpZ2h0Pwo+IEBDaHVuZmVuZzogY2FuIHlvdSBj b25maXJtIFJvZ2VyJ3MgaWRlYSB0aGF0IHdlIGNvdWxkIGNhbGwgcGh5X2V4aXQgaWYKPiB0aGUg Y29udHJvbGxlciBpcyAqTk9UKiBtYXJrZWQgYXMgIndha2V1cC1zb3VyY2UiPwo+IEkgYW0gYWxz byBub3Qgc3VyZSBpZiBpdCB3b3VsZCB3b3JrLCBzaW5jZSB0aGUgIndha2V1cC1zb3VyY2UiCj4g cHJvcGVydHkgaXMgZGVmaW5lZCBvbiB0aGUgVVNCIGNvbnRyb2xsZXIncyBwYXJlbnQgbm9kZSBp biBjYXNlIG9mIHRoZQo+IE1lZGlhdGVrIE1UVTMgKE1lZGlhdGVrIFVTQjMuMCBEUkQpIGNvbnRy b2xsZXIKPiAKVmVyeSBzb3JyeSwgSSBmb3Jnb3QgdGhhdCBNVFUzICYgeEhDSSBkcml2ZXJzIGFs d2F5cyBzZXQgdGhlbSBhcyB3YWtldXAKZGV2aWNlcyBieSBkZXZpY2VfaW5pdF93YWtldXAoZGV2 LCB0cnVlKSxidXQgbm90IGRlcGVuZGVudCBvbgoid2FrZXVwLXNvdXJjZSIgcHJvcGVydHksIHNv IG1heWJlIHdlIGNhbiB1c2UgZGV2aWNlX2Nhbl93YWtldXAoKSB0bwpkZWNpZGUgd2hldGhlciBj YWxsIHBoeV9leGl0KCkvaW5pdCgpIG9yIG5vdC4KCj4gPj4gLSBmaXggdGhpcyBpbiB0aGUgUEhZ IGRyaXZlcgo+ID4KPiA+IFRoZXJlIGlzIG5vdGhpbmcgdG8gZml4IGluIHRoZSBQSFkgZHJpdmVy LiBJdCBpcyBkb2luZyB3aGF0IGl0IGlzIHN1cHBvc2VkIHRvIGRvLgo+IEkgYWN0dWFsbHkgd29u ZGVyIGlmIHBoeV9vcHMgc2hvdWxkIGhhdmUgZXhwbGljaXQgc3VzcGVuZC9yZXN1bWUgc3VwcG9y dDoKPiAtIGFzc3VtaW5nIHdlIGRlZmluZSB0d28gbmV3IGNhbGxiYWNrczogLnN1c3BlbmQgYW5k IC5yZXN1bWUKPiAtIHRoZSBQSFkgZnJhbWV3b3JrIGNvdWxkIGNhbGwgLnBvd2VyX29mZiBieSBk ZWZhdWx0IGlmIC5zdXNwZW5kIGlzIG5vdCBkZWZpbmVkCj4gLSB0aGUgUEhZIGZyYW1ld29yayBj b3VsZCBjYWxsIC5wb3dlcl9vbiBieSBkZWZhdWx0IGlmIC5yZXN1bWUgaXMgbm90IGRlZmluZWQK PiAtIGRyaXZlcnMgY291bGQgc2V0IC5zdXNwZW5kIGFuZCAucmVzdW1lIG9uIHRoZWlyIG93biwg YWxsb3dpbmcgbW9yZQo+IGZpbmUtZ3JhaW5lZCBjb250cm9sIGJ5IGZvciBleGFtcGxlICpvbmx5 KiBzdG9wcGluZyB0aGUgY2xvY2sgKGJ1dCBub3QKPiByZS1pbml0aWFsaXppbmcgdGhlIHJlZ2lz dGVycywgZXRjLikKPiAKPiBAUm9nZXI6IHdoYXQgZG8geW91IHRoaW5rIGFib3V0IHRoaXM/Cj4g S2lzaG9uICh0aGUgUEhZIGZyYW1ld29yayBtYWludGFpbmVyKSBpcyBhbHNvIENDJ2VkIC0gSSB3 b3VsZCBsaWtlIHRvCj4gaGVhciBoaXMgb3BpbmlvbiB0b28KPiAKPiA+PiAtIHNvbWV3aGVyZSBl bHNlCj4gPj4KPiA+Pj4+ICt9Cj4gPj4+PiArRVhQT1JUX1NZTUJPTF9HUEwodXNiX3BoeV9yb290 aHViX3Bvd2VyX29mZik7Cj4gPj4+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy91c2IvY29yZS9waHku aCBiL2RyaXZlcnMvdXNiL2NvcmUvcGh5LmgKPiA+Pj4+IG5ldyBmaWxlIG1vZGUgMTAwNjQ0Cj4g Pj4+PiBpbmRleCAwMDAwMDAwMDAwMDAuLjZmZGU1OWJmYmZmOAo+ID4+Pj4gLS0tIC9kZXYvbnVs bAo+ID4+Pj4gKysrIGIvZHJpdmVycy91c2IvY29yZS9waHkuaAo+ID4+Pj4gQEAgLTAsMCArMSw3 IEBACj4gPj4+PiArc3RydWN0IHVzYl9waHlfcm9vdGh1YjsKPiA+Pj4+ICsKPiA+Pj4+ICtzdHJ1 Y3QgdXNiX3BoeV9yb290aHViICp1c2JfcGh5X3Jvb3RodWJfaW5pdChzdHJ1Y3QgZGV2aWNlICpk ZXYpOwo+ID4+Pj4gK2ludCB1c2JfcGh5X3Jvb3RodWJfZXhpdChzdHJ1Y3QgdXNiX3BoeV9yb290 aHViICpwaHlfcm9vdGh1Yik7Cj4gPj4+PiArCj4gPj4+PiAraW50IHVzYl9waHlfcm9vdGh1Yl9w b3dlcl9vbihzdHJ1Y3QgdXNiX3BoeV9yb290aHViICpwaHlfcm9vdGh1Yik7Cj4gPj4+PiArdm9p ZCB1c2JfcGh5X3Jvb3RodWJfcG93ZXJfb2ZmKHN0cnVjdCB1c2JfcGh5X3Jvb3RodWIgKnBoeV9y b290aHViKTsKPiA+Pj4+Cj4gPj4+Cj4gPgo+ID4gPHNuaXA+Cj4gPgo+ID4gLS0KPiA+IGNoZWVy cywKPiA+IC1yb2dlcgo+ID4KPiA+IFRleGFzIEluc3RydW1lbnRzIEZpbmxhbmQgT3ksIFBvcmtr YWxhbmthdHUgMjIsIDAwMTgwIEhlbHNpbmtpLiBZLXR1bm51cy9CdXNpbmVzcyBJRDogMDYxNTUy MS00LiBLb3RpcGFpa2thL0RvbWljaWxlOiBIZWxzaW5raQo+IAo+IAo+IFJlZ2FyZHMKPiBNYXJ0 aW4KLS0tClRvIHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1 YnNjcmliZSBsaW51eC11c2IiIGluCnRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9A dmdlci5rZXJuZWwub3JnCk1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5l bC5vcmcvbWFqb3Jkb21vLWluZm8uaHRtbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: chunfeng.yun@mediatek.com (Chunfeng Yun) Date: Tue, 20 Mar 2018 20:04:27 +0800 Subject: [PATCH usb-next v10 3/8] usb: core: add a wrapper for the USB PHYs on the HCD In-Reply-To: References: <20180218184504.3331-1-martin.blumenstingl@googlemail.com> <20180218184504.3331-4-martin.blumenstingl@googlemail.com> <7f511a6f-f3a2-2002-f601-5ce1742f4539@ti.com> Message-ID: <1521547467.3717.92.camel@mhfsdcap03> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Martin & Roger: On Mon, 2018-03-19 at 17:12 +0100, Martin Blumenstingl wrote: > Hi Roger, > > On Mon, Mar 19, 2018 at 9:49 AM, Roger Quadros wrote: > > Hi, > > > > On 19/03/18 00:29, Martin Blumenstingl wrote: > >> Hi Roger, > >> > >> On Fri, Mar 16, 2018 at 3:32 PM, Roger Quadros wrote: > >>> +some TI folks > >>> > >>> Hi Martin, > >>> > >>> On 18/02/18 20:44, Martin Blumenstingl wrote: > >>>> Many SoC platforms have separate devices for the USB PHY which are > >>>> registered through the generic PHY framework. These PHYs have to be > >>>> enabled to make the USB controller actually work. They also have to be > >>>> disabled again on shutdown/suspend. > >>>> > >>>> Currently (at least) the following HCI platform drivers are using custom > >>>> code to obtain all PHYs via devicetree for the roothub/controller and > >>>> disable/enable them when required: > >>>> - ehci-platform.c has ehci_platform_power_{on,off} > >>>> - xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off} > >>>> - ohci-platform.c has ohci_platform_power_{on,off} > >>>> > >>>> With this new wrapper the USB PHYs can be specified directly in the > >>>> USB controller's devicetree node (just like on the drivers listed > >>>> above). This allows SoCs like the Amlogic Meson GXL family to operate > >>>> correctly once this is wired up correctly. These SoCs use a dwc3 > >>>> controller and require all USB PHYs to be initialized (if one of the USB > >>>> PHYs it not initialized then none of USB port works at all). > >>>> > >>>> Signed-off-by: Martin Blumenstingl > >>>> Tested-by: Yixun Lan > >>>> Cc: Neil Armstrong > >>>> Cc: Chunfeng Yun > >>> > >>> This patch is breaking low power cases on TI SoCs when USB is in host mode. > >>> I'll explain why below. > >> based on your explanation and reading the TI PHY drivers I am assuming > >> that the affected SoCs are using the "phy-omap-usb2" driver > >> > > yes and phy-ti-pipe3 as well i.e. "ti,phy-usb3" and "ti,omap-usb3" > I missed that, thanks > > >>>> --- > >>>> drivers/usb/core/Makefile | 2 +- > >>>> drivers/usb/core/phy.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++ > >>>> drivers/usb/core/phy.h | 7 ++ > >>>> 3 files changed, 166 insertions(+), 1 deletion(-) > >>>> create mode 100644 drivers/usb/core/phy.c > >>>> create mode 100644 drivers/usb/core/phy.h > >>>> > >>>> diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile > >>>> index 92c9cefb4317..18e874b0441e 100644 > >>>> --- a/drivers/usb/core/Makefile > >>>> +++ b/drivers/usb/core/Makefile > >>>> @@ -6,7 +6,7 @@ > >>>> usbcore-y := usb.o hub.o hcd.o urb.o message.o driver.o > >>>> usbcore-y += config.o file.o buffer.o sysfs.o endpoint.o > >>>> usbcore-y += devio.o notify.o generic.o quirks.o devices.o > >>>> -usbcore-y += port.o > >>>> +usbcore-y += phy.o port.o > >>>> > >>>> usbcore-$(CONFIG_OF) += of.o > >>>> usbcore-$(CONFIG_USB_PCI) += hcd-pci.o > >>>> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c > >>>> new file mode 100644 > >>>> index 000000000000..09b7c43c0ea4 > >>>> --- /dev/null > >>>> +++ b/drivers/usb/core/phy.c > >>>> @@ -0,0 +1,158 @@ > >>>> +// SPDX-License-Identifier: GPL-2.0+ > >>>> +/* > >>>> + * A wrapper for multiple PHYs which passes all phy_* function calls to > >>>> + * multiple (actual) PHY devices. This is comes handy when initializing > >>>> + * all PHYs on a HCD and to keep them all in the same state. > >>>> + * > >>>> + * Copyright (C) 2018 Martin Blumenstingl > >>>> + */ > >>>> + > >>>> +#include > >>>> +#include > >>>> +#include > >>>> +#include > >>>> + > >>>> +#include "phy.h" > >>>> + > >>>> +struct usb_phy_roothub { > >>>> + struct phy *phy; > >>>> + struct list_head list; > >>>> +}; > >>>> + > >>>> +static struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev) > >>>> +{ > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + > >>>> + roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL); > >>>> + if (!roothub_entry) > >>>> + return ERR_PTR(-ENOMEM); > >>>> + > >>>> + INIT_LIST_HEAD(&roothub_entry->list); > >>>> + > >>>> + return roothub_entry; > >>>> +} > >>>> + > >>>> +static int usb_phy_roothub_add_phy(struct device *dev, int index, > >>>> + struct list_head *list) > >>>> +{ > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + struct phy *phy = devm_of_phy_get_by_index(dev, dev->of_node, index); > >>>> + > >>>> + if (IS_ERR_OR_NULL(phy)) { > >>>> + if (!phy || PTR_ERR(phy) == -ENODEV) > >>>> + return 0; > >>>> + else > >>>> + return PTR_ERR(phy); > >>>> + } > >>>> + > >>>> + roothub_entry = usb_phy_roothub_alloc(dev); > >>>> + if (IS_ERR(roothub_entry)) > >>>> + return PTR_ERR(roothub_entry); > >>>> + > >>>> + roothub_entry->phy = phy; > >>>> + > >>>> + list_add_tail(&roothub_entry->list, list); > >>>> + > >>>> + return 0; > >>>> +} > >>>> + > >>>> +struct usb_phy_roothub *usb_phy_roothub_init(struct device *dev) > >>>> +{ > >>>> + struct usb_phy_roothub *phy_roothub; > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + struct list_head *head; > >>>> + int i, num_phys, err; > >>>> + > >>>> + num_phys = of_count_phandle_with_args(dev->of_node, "phys", > >>>> + "#phy-cells"); > >>>> + if (num_phys <= 0) > >>>> + return NULL; > >>>> + > >>>> + phy_roothub = usb_phy_roothub_alloc(dev); > >>>> + if (IS_ERR(phy_roothub)) > >>>> + return phy_roothub; > >>>> + > >>>> + for (i = 0; i < num_phys; i++) { > >>>> + err = usb_phy_roothub_add_phy(dev, i, &phy_roothub->list); > >>>> + if (err) > >>>> + goto err_out; > >>>> + } > >>>> + > >>>> + head = &phy_roothub->list; > >>>> + > >>>> + list_for_each_entry(roothub_entry, head, list) { > >>>> + err = phy_init(roothub_entry->phy); > >>> > >>> The phy_init() function actually enables the PHY clocks. > >>> It should be moved to the usb_phy_roothub_exit() routine just before calling phy_power_on(). > >> do you mean that phy_init should be moved to usb_phy_roothub_power_on > >> (just before phy_power_on is called within usb_phy_roothub_power_on)? > >> > > > > Yes. > > > >> an earlier version of my patch did exactly this, but it caused > >> problems during a suspend/resume cycle on Mediatek devices > >> Chunfeng Yun reported that issue here [0], quote from that mail for > >> easier reading: > >> "In order to keep link state on mt8173, we just power off all phys(not > >> exit) when system enter suspend, then power on them again (needn't > >> init, otherwise device will be disconnected) when system resume, this > >> can avoid re-enumerating device." > >> > >>>> + if (err) > >>>> + goto err_exit_phys; > >>>> + } > >>>> + > >>>> + return phy_roothub; > >>>> + > >>>> +err_exit_phys: > >>>> + list_for_each_entry_continue_reverse(roothub_entry, head, list) > >>>> + phy_exit(roothub_entry->phy); > >>>> + > >>>> +err_out: > >>>> + return ERR_PTR(err); > >>>> +} > >>>> +EXPORT_SYMBOL_GPL(usb_phy_roothub_init); > >>>> + > >>>> +int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub) > >>>> +{ > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + struct list_head *head; > >>>> + int err, ret = 0; > >>>> + > >>>> + if (!phy_roothub) > >>>> + return 0; > >>>> + > >>>> + head = &phy_roothub->list; > >>>> + > >>>> + list_for_each_entry(roothub_entry, head, list) { > >>>> + err = phy_exit(roothub_entry->phy); > >>>> + if (err) > >>>> + ret = ret; > >>>> + } > >>> > >>> phy_exit() should be moved to usb_phy_roothub_poweroff() just after calling phy_power_off(). > >> if I understood Chunfeng Yun correctly this will require > >> re-enumeration of the USB devices after a suspend/resume cycle on > >> Mediatek SoCs > >> > > > > OK. I suppose that there are 2 cases > > 1) Mediatek's case: USB controller context retained across suspend/resume. > > Remote wakeup probably required. > > No re-enumeration preferred after resume. phy_exit()/phy_init() must not be called > > during suspend/resume to keep PHY link active. > Documentation/devicetree/bindings/usb/mediatek,mtu3.txt indeed shows > that the parent of the USB controller can be marked as "wakeup-source" > > > 2) TI's case: low power important: USB context is lost, OK to re-enumerate. > > phy_exit()/phy_init() must be called during suspend/resume. > ACK > > >>> With that there is nothing else being done here. Shouldn't we be doing the equivalent of > >>> usb_phy_roothub_del_phy() and usb_phy_roothub_free() here? > >>> > >>>> + > >>>> + return ret; > >>>> +} > >>>> +EXPORT_SYMBOL_GPL(usb_phy_roothub_exit); > >>>> + > >>>> +int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub) > >>>> +{ > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + struct list_head *head; > >>>> + int err; > >>>> + > >>>> + if (!phy_roothub) > >>>> + return 0; > >>>> + > >>>> + head = &phy_roothub->list; > >>>> + > >>>> + list_for_each_entry(roothub_entry, head, list) { > >>>> + err = phy_power_on(roothub_entry->phy); > >>>> + if (err) > >>>> + goto err_out; > >>>> + } > >>>> + > >>>> + return 0; > >>>> + > >>>> +err_out: > >>>> + list_for_each_entry_continue_reverse(roothub_entry, head, list) > >>>> + phy_power_off(roothub_entry->phy); > >>>> + > >>>> + return err; > >>>> +} > >>>> +EXPORT_SYMBOL_GPL(usb_phy_roothub_power_on); > >>>> + > >>>> +void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub) > >>>> +{ > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + > >>>> + if (!phy_roothub) > >>>> + return; > >>>> + > >>>> + list_for_each_entry_reverse(roothub_entry, &phy_roothub->list, list) > >>>> + phy_power_off(roothub_entry->phy); > >>> > >>> Not doing the phy_exit() here leaves the clocks enabled on our SoC and > >>> we're no longer able to reach low power states on system suspend. > >> I'm not sure where this problem should be solved: > >> - set skip_phy_initialization in struct usb_hcd to 1 for the affected > >> TI platforms > > > > Many TI platforms are affected, omap5*, dra7*, am43* > > > >> - fix this in the usb_phy_roothub code > > > > I'd vote for fixing it in the usb_phy_roothub code. How? > > How about using the device_can_wakeup() to decide if we should call phy_exit()/init() or not? > > If the USB device can't wakeup the system there is no point in keeping it powered/clocked right? > @Chunfeng: can you confirm Roger's idea that we could call phy_exit if > the controller is *NOT* marked as "wakeup-source"? > I am also not sure if it would work, since the "wakeup-source" > property is defined on the USB controller's parent node in case of the > Mediatek MTU3 (Mediatek USB3.0 DRD) controller > Very sorry, I forgot that MTU3 & xHCI drivers always set them as wakeup devices by device_init_wakeup(dev, true),but not dependent on "wakeup-source" property, so maybe we can use device_can_wakeup() to decide whether call phy_exit()/init() or not. > >> - fix this in the PHY driver > > > > There is nothing to fix in the PHY driver. It is doing what it is supposed to do. > I actually wonder if phy_ops should have explicit suspend/resume support: > - assuming we define two new callbacks: .suspend and .resume > - the PHY framework could call .power_off by default if .suspend is not defined > - the PHY framework could call .power_on by default if .resume is not defined > - drivers could set .suspend and .resume on their own, allowing more > fine-grained control by for example *only* stopping the clock (but not > re-initializing the registers, etc.) > > @Roger: what do you think about this? > Kishon (the PHY framework maintainer) is also CC'ed - I would like to > hear his opinion too > > >> - somewhere else > >> > >>>> +} > >>>> +EXPORT_SYMBOL_GPL(usb_phy_roothub_power_off); > >>>> diff --git a/drivers/usb/core/phy.h b/drivers/usb/core/phy.h > >>>> new file mode 100644 > >>>> index 000000000000..6fde59bfbff8 > >>>> --- /dev/null > >>>> +++ b/drivers/usb/core/phy.h > >>>> @@ -0,0 +1,7 @@ > >>>> +struct usb_phy_roothub; > >>>> + > >>>> +struct usb_phy_roothub *usb_phy_roothub_init(struct device *dev); > >>>> +int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub); > >>>> + > >>>> +int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub); > >>>> +void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub); > >>>> > >>> > > > > > > > > -- > > cheers, > > -roger > > > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > > Regards > Martin From mboxrd@z Thu Jan 1 00:00:00 1970 From: chunfeng.yun@mediatek.com (Chunfeng Yun) Date: Tue, 20 Mar 2018 20:04:27 +0800 Subject: [PATCH usb-next v10 3/8] usb: core: add a wrapper for the USB PHYs on the HCD In-Reply-To: References: <20180218184504.3331-1-martin.blumenstingl@googlemail.com> <20180218184504.3331-4-martin.blumenstingl@googlemail.com> <7f511a6f-f3a2-2002-f601-5ce1742f4539@ti.com> Message-ID: <1521547467.3717.92.camel@mhfsdcap03> To: linus-amlogic@lists.infradead.org List-Id: linus-amlogic.lists.infradead.org Hi Martin & Roger: On Mon, 2018-03-19 at 17:12 +0100, Martin Blumenstingl wrote: > Hi Roger, > > On Mon, Mar 19, 2018 at 9:49 AM, Roger Quadros wrote: > > Hi, > > > > On 19/03/18 00:29, Martin Blumenstingl wrote: > >> Hi Roger, > >> > >> On Fri, Mar 16, 2018 at 3:32 PM, Roger Quadros wrote: > >>> +some TI folks > >>> > >>> Hi Martin, > >>> > >>> On 18/02/18 20:44, Martin Blumenstingl wrote: > >>>> Many SoC platforms have separate devices for the USB PHY which are > >>>> registered through the generic PHY framework. These PHYs have to be > >>>> enabled to make the USB controller actually work. They also have to be > >>>> disabled again on shutdown/suspend. > >>>> > >>>> Currently (at least) the following HCI platform drivers are using custom > >>>> code to obtain all PHYs via devicetree for the roothub/controller and > >>>> disable/enable them when required: > >>>> - ehci-platform.c has ehci_platform_power_{on,off} > >>>> - xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off} > >>>> - ohci-platform.c has ohci_platform_power_{on,off} > >>>> > >>>> With this new wrapper the USB PHYs can be specified directly in the > >>>> USB controller's devicetree node (just like on the drivers listed > >>>> above). This allows SoCs like the Amlogic Meson GXL family to operate > >>>> correctly once this is wired up correctly. These SoCs use a dwc3 > >>>> controller and require all USB PHYs to be initialized (if one of the USB > >>>> PHYs it not initialized then none of USB port works at all). > >>>> > >>>> Signed-off-by: Martin Blumenstingl > >>>> Tested-by: Yixun Lan > >>>> Cc: Neil Armstrong > >>>> Cc: Chunfeng Yun > >>> > >>> This patch is breaking low power cases on TI SoCs when USB is in host mode. > >>> I'll explain why below. > >> based on your explanation and reading the TI PHY drivers I am assuming > >> that the affected SoCs are using the "phy-omap-usb2" driver > >> > > yes and phy-ti-pipe3 as well i.e. "ti,phy-usb3" and "ti,omap-usb3" > I missed that, thanks > > >>>> --- > >>>> drivers/usb/core/Makefile | 2 +- > >>>> drivers/usb/core/phy.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++ > >>>> drivers/usb/core/phy.h | 7 ++ > >>>> 3 files changed, 166 insertions(+), 1 deletion(-) > >>>> create mode 100644 drivers/usb/core/phy.c > >>>> create mode 100644 drivers/usb/core/phy.h > >>>> > >>>> diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile > >>>> index 92c9cefb4317..18e874b0441e 100644 > >>>> --- a/drivers/usb/core/Makefile > >>>> +++ b/drivers/usb/core/Makefile > >>>> @@ -6,7 +6,7 @@ > >>>> usbcore-y := usb.o hub.o hcd.o urb.o message.o driver.o > >>>> usbcore-y += config.o file.o buffer.o sysfs.o endpoint.o > >>>> usbcore-y += devio.o notify.o generic.o quirks.o devices.o > >>>> -usbcore-y += port.o > >>>> +usbcore-y += phy.o port.o > >>>> > >>>> usbcore-$(CONFIG_OF) += of.o > >>>> usbcore-$(CONFIG_USB_PCI) += hcd-pci.o > >>>> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c > >>>> new file mode 100644 > >>>> index 000000000000..09b7c43c0ea4 > >>>> --- /dev/null > >>>> +++ b/drivers/usb/core/phy.c > >>>> @@ -0,0 +1,158 @@ > >>>> +// SPDX-License-Identifier: GPL-2.0+ > >>>> +/* > >>>> + * A wrapper for multiple PHYs which passes all phy_* function calls to > >>>> + * multiple (actual) PHY devices. This is comes handy when initializing > >>>> + * all PHYs on a HCD and to keep them all in the same state. > >>>> + * > >>>> + * Copyright (C) 2018 Martin Blumenstingl > >>>> + */ > >>>> + > >>>> +#include > >>>> +#include > >>>> +#include > >>>> +#include > >>>> + > >>>> +#include "phy.h" > >>>> + > >>>> +struct usb_phy_roothub { > >>>> + struct phy *phy; > >>>> + struct list_head list; > >>>> +}; > >>>> + > >>>> +static struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev) > >>>> +{ > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + > >>>> + roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL); > >>>> + if (!roothub_entry) > >>>> + return ERR_PTR(-ENOMEM); > >>>> + > >>>> + INIT_LIST_HEAD(&roothub_entry->list); > >>>> + > >>>> + return roothub_entry; > >>>> +} > >>>> + > >>>> +static int usb_phy_roothub_add_phy(struct device *dev, int index, > >>>> + struct list_head *list) > >>>> +{ > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + struct phy *phy = devm_of_phy_get_by_index(dev, dev->of_node, index); > >>>> + > >>>> + if (IS_ERR_OR_NULL(phy)) { > >>>> + if (!phy || PTR_ERR(phy) == -ENODEV) > >>>> + return 0; > >>>> + else > >>>> + return PTR_ERR(phy); > >>>> + } > >>>> + > >>>> + roothub_entry = usb_phy_roothub_alloc(dev); > >>>> + if (IS_ERR(roothub_entry)) > >>>> + return PTR_ERR(roothub_entry); > >>>> + > >>>> + roothub_entry->phy = phy; > >>>> + > >>>> + list_add_tail(&roothub_entry->list, list); > >>>> + > >>>> + return 0; > >>>> +} > >>>> + > >>>> +struct usb_phy_roothub *usb_phy_roothub_init(struct device *dev) > >>>> +{ > >>>> + struct usb_phy_roothub *phy_roothub; > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + struct list_head *head; > >>>> + int i, num_phys, err; > >>>> + > >>>> + num_phys = of_count_phandle_with_args(dev->of_node, "phys", > >>>> + "#phy-cells"); > >>>> + if (num_phys <= 0) > >>>> + return NULL; > >>>> + > >>>> + phy_roothub = usb_phy_roothub_alloc(dev); > >>>> + if (IS_ERR(phy_roothub)) > >>>> + return phy_roothub; > >>>> + > >>>> + for (i = 0; i < num_phys; i++) { > >>>> + err = usb_phy_roothub_add_phy(dev, i, &phy_roothub->list); > >>>> + if (err) > >>>> + goto err_out; > >>>> + } > >>>> + > >>>> + head = &phy_roothub->list; > >>>> + > >>>> + list_for_each_entry(roothub_entry, head, list) { > >>>> + err = phy_init(roothub_entry->phy); > >>> > >>> The phy_init() function actually enables the PHY clocks. > >>> It should be moved to the usb_phy_roothub_exit() routine just before calling phy_power_on(). > >> do you mean that phy_init should be moved to usb_phy_roothub_power_on > >> (just before phy_power_on is called within usb_phy_roothub_power_on)? > >> > > > > Yes. > > > >> an earlier version of my patch did exactly this, but it caused > >> problems during a suspend/resume cycle on Mediatek devices > >> Chunfeng Yun reported that issue here [0], quote from that mail for > >> easier reading: > >> "In order to keep link state on mt8173, we just power off all phys(not > >> exit) when system enter suspend, then power on them again (needn't > >> init, otherwise device will be disconnected) when system resume, this > >> can avoid re-enumerating device." > >> > >>>> + if (err) > >>>> + goto err_exit_phys; > >>>> + } > >>>> + > >>>> + return phy_roothub; > >>>> + > >>>> +err_exit_phys: > >>>> + list_for_each_entry_continue_reverse(roothub_entry, head, list) > >>>> + phy_exit(roothub_entry->phy); > >>>> + > >>>> +err_out: > >>>> + return ERR_PTR(err); > >>>> +} > >>>> +EXPORT_SYMBOL_GPL(usb_phy_roothub_init); > >>>> + > >>>> +int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub) > >>>> +{ > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + struct list_head *head; > >>>> + int err, ret = 0; > >>>> + > >>>> + if (!phy_roothub) > >>>> + return 0; > >>>> + > >>>> + head = &phy_roothub->list; > >>>> + > >>>> + list_for_each_entry(roothub_entry, head, list) { > >>>> + err = phy_exit(roothub_entry->phy); > >>>> + if (err) > >>>> + ret = ret; > >>>> + } > >>> > >>> phy_exit() should be moved to usb_phy_roothub_poweroff() just after calling phy_power_off(). > >> if I understood Chunfeng Yun correctly this will require > >> re-enumeration of the USB devices after a suspend/resume cycle on > >> Mediatek SoCs > >> > > > > OK. I suppose that there are 2 cases > > 1) Mediatek's case: USB controller context retained across suspend/resume. > > Remote wakeup probably required. > > No re-enumeration preferred after resume. phy_exit()/phy_init() must not be called > > during suspend/resume to keep PHY link active. > Documentation/devicetree/bindings/usb/mediatek,mtu3.txt indeed shows > that the parent of the USB controller can be marked as "wakeup-source" > > > 2) TI's case: low power important: USB context is lost, OK to re-enumerate. > > phy_exit()/phy_init() must be called during suspend/resume. > ACK > > >>> With that there is nothing else being done here. Shouldn't we be doing the equivalent of > >>> usb_phy_roothub_del_phy() and usb_phy_roothub_free() here? > >>> > >>>> + > >>>> + return ret; > >>>> +} > >>>> +EXPORT_SYMBOL_GPL(usb_phy_roothub_exit); > >>>> + > >>>> +int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub) > >>>> +{ > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + struct list_head *head; > >>>> + int err; > >>>> + > >>>> + if (!phy_roothub) > >>>> + return 0; > >>>> + > >>>> + head = &phy_roothub->list; > >>>> + > >>>> + list_for_each_entry(roothub_entry, head, list) { > >>>> + err = phy_power_on(roothub_entry->phy); > >>>> + if (err) > >>>> + goto err_out; > >>>> + } > >>>> + > >>>> + return 0; > >>>> + > >>>> +err_out: > >>>> + list_for_each_entry_continue_reverse(roothub_entry, head, list) > >>>> + phy_power_off(roothub_entry->phy); > >>>> + > >>>> + return err; > >>>> +} > >>>> +EXPORT_SYMBOL_GPL(usb_phy_roothub_power_on); > >>>> + > >>>> +void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub) > >>>> +{ > >>>> + struct usb_phy_roothub *roothub_entry; > >>>> + > >>>> + if (!phy_roothub) > >>>> + return; > >>>> + > >>>> + list_for_each_entry_reverse(roothub_entry, &phy_roothub->list, list) > >>>> + phy_power_off(roothub_entry->phy); > >>> > >>> Not doing the phy_exit() here leaves the clocks enabled on our SoC and > >>> we're no longer able to reach low power states on system suspend. > >> I'm not sure where this problem should be solved: > >> - set skip_phy_initialization in struct usb_hcd to 1 for the affected > >> TI platforms > > > > Many TI platforms are affected, omap5*, dra7*, am43* > > > >> - fix this in the usb_phy_roothub code > > > > I'd vote for fixing it in the usb_phy_roothub code. How? > > How about using the device_can_wakeup() to decide if we should call phy_exit()/init() or not? > > If the USB device can't wakeup the system there is no point in keeping it powered/clocked right? > @Chunfeng: can you confirm Roger's idea that we could call phy_exit if > the controller is *NOT* marked as "wakeup-source"? > I am also not sure if it would work, since the "wakeup-source" > property is defined on the USB controller's parent node in case of the > Mediatek MTU3 (Mediatek USB3.0 DRD) controller > Very sorry, I forgot that MTU3 & xHCI drivers always set them as wakeup devices by device_init_wakeup(dev, true),but not dependent on "wakeup-source" property, so maybe we can use device_can_wakeup() to decide whether call phy_exit()/init() or not. > >> - fix this in the PHY driver > > > > There is nothing to fix in the PHY driver. It is doing what it is supposed to do. > I actually wonder if phy_ops should have explicit suspend/resume support: > - assuming we define two new callbacks: .suspend and .resume > - the PHY framework could call .power_off by default if .suspend is not defined > - the PHY framework could call .power_on by default if .resume is not defined > - drivers could set .suspend and .resume on their own, allowing more > fine-grained control by for example *only* stopping the clock (but not > re-initializing the registers, etc.) > > @Roger: what do you think about this? > Kishon (the PHY framework maintainer) is also CC'ed - I would like to > hear his opinion too > > >> - somewhere else > >> > >>>> +} > >>>> +EXPORT_SYMBOL_GPL(usb_phy_roothub_power_off); > >>>> diff --git a/drivers/usb/core/phy.h b/drivers/usb/core/phy.h > >>>> new file mode 100644 > >>>> index 000000000000..6fde59bfbff8 > >>>> --- /dev/null > >>>> +++ b/drivers/usb/core/phy.h > >>>> @@ -0,0 +1,7 @@ > >>>> +struct usb_phy_roothub; > >>>> + > >>>> +struct usb_phy_roothub *usb_phy_roothub_init(struct device *dev); > >>>> +int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub); > >>>> + > >>>> +int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub); > >>>> +void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub); > >>>> > >>> > > > > > > > > -- > > cheers, > > -roger > > > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > > Regards > Martin