From mboxrd@z Thu Jan 1 00:00:00 1970 From: Martin Blumenstingl Subject: Re: [usb-next PATCH v11 3/8] usb: core: add a wrapper for the USB PHYs on the HCD Date: Thu, 5 Apr 2018 22:04:01 +0200 Message-ID: References: <20180303214309.25643-1-martin.blumenstingl@googlemail.com> <20180303214309.25643-4-martin.blumenstingl@googlemail.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: Masahiro Yamada Cc: Mark Rutland , Peter.Chen-3arQi8VN3Tc@public.gmane.org, Neil Armstrong , linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Thierry Reding , Tony Prisk , Felipe Balbi , Jon Hunter , Alan Stern , Chunfeng Yun , DTML , Arnd Bergmann , yixun.lan-LpR1jeaWuhtBDgjK7y7TUQ@public.gmane.org, Rob Herring , "moderated list:ARM/Mediatek SoC support" , Matthias Brugger , linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Linux-OMAP , linux-arm-kernel , Greg Kroah-Hartman , linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, mathias.nyman-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org List-Id: linux-tegra@vger.kernel.org Hello, (great to hear that this might be useful on Socionext SoCs as well :)) On Wed, Apr 4, 2018 at 2:10 PM, Masahiro Yamada wrote: > 2018-03-04 6:43 GMT+09:00 Martin Blumenstingl > : >> 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 >> --- >> 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; > > > I think phy_roothub->phy is always empty, > and only phy_roothub->list is used. > > > I just wondered if we can directly put 'struct list_head' into > 'struct usb_hcd'. maybe you can try it and send a patch (note: please wait until v4.17-rc2-ish because there are some fixes that will be applied by Greg after v4.17-rc1 is out, as well as the issues you noted below)? I believe it's not critical so there's plenty of time to get it into v4.18 >> 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 device; > > is also needed for usb_phy_roothub_init(). do you think that I should still fix this in 4.17? >> +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); > > > Better to add an include guard? > SPDX-License-Identifier for this header too? good catch - I'll send a patch for this too (along with the missing forward declaration for struct device) 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,v11,3/8] usb: core: add a wrapper for the USB PHYs on the HCD From: Martin Blumenstingl Message-Id: Date: Thu, 5 Apr 2018 22:04:01 +0200 To: Masahiro Yamada Cc: linux-usb@vger.kernel.org, mathias.nyman@intel.com, Arnd Bergmann , Greg Kroah-Hartman , Felipe Balbi , Linux-OMAP , linux-tegra@vger.kernel.org, "moderated list:ARM/Mediatek SoC support" , linux-arm-kernel , DTML , Jon Hunter , Thierry Reding , Alan Stern , Tony Prisk , Peter.Chen@nxp.com, Matthias Brugger , Mark Rutland , Rob Herring , Neil Armstrong , linux-amlogic@lists.infradead.org, yixun.lan@amlogic.com, Chunfeng Yun List-ID: SGVsbG8sCgooZ3JlYXQgdG8gaGVhciB0aGF0IHRoaXMgbWlnaHQgYmUgdXNlZnVsIG9uIFNvY2lv bmV4dCBTb0NzIGFzIHdlbGwgOikpCgpPbiBXZWQsIEFwciA0LCAyMDE4IGF0IDI6MTAgUE0sIE1h c2FoaXJvIFlhbWFkYQo8eWFtYWRhLm1hc2FoaXJvQHNvY2lvbmV4dC5jb20+IHdyb3RlOgo+IDIw MTgtMDMtMDQgNjo0MyBHTVQrMDk6MDAgTWFydGluIEJsdW1lbnN0aW5nbAo+IDxtYXJ0aW4uYmx1 bWVuc3RpbmdsQGdvb2dsZW1haWwuY29tPjoKPj4gTWFueSBTb0MgcGxhdGZvcm1zIGhhdmUgc2Vw YXJhdGUgZGV2aWNlcyBmb3IgdGhlIFVTQiBQSFkgd2hpY2ggYXJlCj4+IHJlZ2lzdGVyZWQgdGhy b3VnaCB0aGUgZ2VuZXJpYyBQSFkgZnJhbWV3b3JrLiBUaGVzZSBQSFlzIGhhdmUgdG8gYmUKPj4g ZW5hYmxlZCB0byBtYWtlIHRoZSBVU0IgY29udHJvbGxlciBhY3R1YWxseSB3b3JrLiBUaGV5IGFs c28gaGF2ZSB0byBiZQo+PiBkaXNhYmxlZCBhZ2FpbiBvbiBzaHV0ZG93bi9zdXNwZW5kLgo+Pgo+ PiBDdXJyZW50bHkgKGF0IGxlYXN0KSB0aGUgZm9sbG93aW5nIEhDSSBwbGF0Zm9ybSBkcml2ZXJz IGFyZSB1c2luZyBjdXN0b20KPj4gY29kZSB0byBvYnRhaW4gYWxsIFBIWXMgdmlhIGRldmljZXRy ZWUgZm9yIHRoZSByb290aHViL2NvbnRyb2xsZXIgYW5kCj4+IGRpc2FibGUvZW5hYmxlIHRoZW0g d2hlbiByZXF1aXJlZDoKPj4gLSBlaGNpLXBsYXRmb3JtLmMgaGFzIGVoY2lfcGxhdGZvcm1fcG93 ZXJfe29uLG9mZn0KPj4gLSB4aGNpLW10ay5jIGhhcyB4aGNpX210a19waHlfe2luaXQsZXhpdCxw b3dlcl9vbixwb3dlcl9vZmZ9Cj4+IC0gb2hjaS1wbGF0Zm9ybS5jIGhhcyBvaGNpX3BsYXRmb3Jt X3Bvd2VyX3tvbixvZmZ9Cj4+Cj4+IFdpdGggdGhpcyBuZXcgd3JhcHBlciB0aGUgVVNCIFBIWXMg Y2FuIGJlIHNwZWNpZmllZCBkaXJlY3RseSBpbiB0aGUKPj4gVVNCIGNvbnRyb2xsZXIncyBkZXZp Y2V0cmVlIG5vZGUgKGp1c3QgbGlrZSBvbiB0aGUgZHJpdmVycyBsaXN0ZWQKPj4gYWJvdmUpLiBU aGlzIGFsbG93cyBTb0NzIGxpa2UgdGhlIEFtbG9naWMgTWVzb24gR1hMIGZhbWlseSB0byBvcGVy YXRlCj4+IGNvcnJlY3RseSBvbmNlIHRoaXMgaXMgd2lyZWQgdXAgY29ycmVjdGx5LiBUaGVzZSBT b0NzIHVzZSBhIGR3YzMKPj4gY29udHJvbGxlciBhbmQgcmVxdWlyZSBhbGwgVVNCIFBIWXMgdG8g YmUgaW5pdGlhbGl6ZWQgKGlmIG9uZSBvZiB0aGUgVVNCCj4+IFBIWXMgaXQgbm90IGluaXRpYWxp emVkIHRoZW4gbm9uZSBvZiBVU0IgcG9ydCB3b3JrcyBhdCBhbGwpLgo+Pgo+PiBTaWduZWQtb2Zm LWJ5OiBNYXJ0aW4gQmx1bWVuc3RpbmdsIDxtYXJ0aW4uYmx1bWVuc3RpbmdsQGdvb2dsZW1haWwu Y29tPgo+PiBUZXN0ZWQtYnk6IFlpeHVuIExhbiA8eWl4dW4ubGFuQGFtbG9naWMuY29tPgo+PiBD YzogTmVpbCBBcm1zdHJvbmcgPG5hcm1zdHJvbmdAYmF5bGlicmUuY29tPgo+PiBDYzogQ2h1bmZl bmcgWXVuIDxjaHVuZmVuZy55dW5AbWVkaWF0ZWsuY29tPgo+PiAtLS0KPj4gIGRyaXZlcnMvdXNi L2NvcmUvTWFrZWZpbGUgfCAgIDIgKy0KPj4gIGRyaXZlcnMvdXNiL2NvcmUvcGh5LmMgICAgfCAx NTggKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKwo+PiAgZHJp dmVycy91c2IvY29yZS9waHkuaCAgICB8ICAgNyArKwo+PiAgMyBmaWxlcyBjaGFuZ2VkLCAxNjYg aW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQo+PiAgY3JlYXRlIG1vZGUgMTAwNjQ0IGRyaXZl cnMvdXNiL2NvcmUvcGh5LmMKPj4gIGNyZWF0ZSBtb2RlIDEwMDY0NCBkcml2ZXJzL3VzYi9jb3Jl L3BoeS5oCj4+Cj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3VzYi9jb3JlL01ha2VmaWxlIGIvZHJp dmVycy91c2IvY29yZS9NYWtlZmlsZQo+PiBpbmRleCA5MmM5Y2VmYjQzMTcuLjE4ZTg3NGIwNDQx ZSAxMDA2NDQKPj4gLS0tIGEvZHJpdmVycy91c2IvY29yZS9NYWtlZmlsZQo+PiArKysgYi9kcml2 ZXJzL3VzYi9jb3JlL01ha2VmaWxlCj4+IEBAIC02LDcgKzYsNyBAQAo+PiAgdXNiY29yZS15IDo9 IHVzYi5vIGh1Yi5vIGhjZC5vIHVyYi5vIG1lc3NhZ2UubyBkcml2ZXIubwo+PiAgdXNiY29yZS15 ICs9IGNvbmZpZy5vIGZpbGUubyBidWZmZXIubyBzeXNmcy5vIGVuZHBvaW50Lm8KPj4gIHVzYmNv cmUteSArPSBkZXZpby5vIG5vdGlmeS5vIGdlbmVyaWMubyBxdWlya3MubyBkZXZpY2VzLm8KPj4g LXVzYmNvcmUteSArPSBwb3J0Lm8KPj4gK3VzYmNvcmUteSArPSBwaHkubyBwb3J0Lm8KPj4KPj4g IHVzYmNvcmUtJChDT05GSUdfT0YpICAgICAgICAgICArPSBvZi5vCj4+ICB1c2Jjb3JlLSQoQ09O RklHX1VTQl9QQ0kpICAgICAgICAgICAgICArPSBoY2QtcGNpLm8KPj4gZGlmZiAtLWdpdCBhL2Ry aXZlcnMvdXNiL2NvcmUvcGh5LmMgYi9kcml2ZXJzL3VzYi9jb3JlL3BoeS5jCj4+IG5ldyBmaWxl IG1vZGUgMTAwNjQ0Cj4+IGluZGV4IDAwMDAwMDAwMDAwMC4uMDliN2M0M2MwZWE0Cj4+IC0tLSAv ZGV2L251bGwKPj4gKysrIGIvZHJpdmVycy91c2IvY29yZS9waHkuYwo+PiBAQCAtMCwwICsxLDE1 OCBAQAo+PiArLy8gU1BEWC1MaWNlbnNlLUlkZW50aWZpZXI6IEdQTC0yLjArCj4+ICsvKgo+PiAr ICogQSB3cmFwcGVyIGZvciBtdWx0aXBsZSBQSFlzIHdoaWNoIHBhc3NlcyBhbGwgcGh5XyogZnVu Y3Rpb24gY2FsbHMgdG8KPj4gKyAqIG11bHRpcGxlIChhY3R1YWwpIFBIWSBkZXZpY2VzLiBUaGlz IGlzIGNvbWVzIGhhbmR5IHdoZW4gaW5pdGlhbGl6aW5nCj4+ICsgKiBhbGwgUEhZcyBvbiBhIEhD RCBhbmQgdG8ga2VlcCB0aGVtIGFsbCBpbiB0aGUgc2FtZSBzdGF0ZS4KPj4gKyAqCj4+ICsgKiBD b3B5cmlnaHQgKEMpIDIwMTggTWFydGluIEJsdW1lbnN0aW5nbCA8bWFydGluLmJsdW1lbnN0aW5n bEBnb29nbGVtYWlsLmNvbT4KPj4gKyAqLwo+PiArCj4+ICsjaW5jbHVkZSA8bGludXgvZGV2aWNl Lmg+Cj4+ICsjaW5jbHVkZSA8bGludXgvbGlzdC5oPgo+PiArI2luY2x1ZGUgPGxpbnV4L3BoeS9w aHkuaD4KPj4gKyNpbmNsdWRlIDxsaW51eC9vZi5oPgo+PiArCj4+ICsjaW5jbHVkZSAicGh5Lmgi Cj4+ICsKPj4gK3N0cnVjdCB1c2JfcGh5X3Jvb3RodWIgewo+PiArICAgICAgIHN0cnVjdCBwaHkg ICAgICAgICAgICAgICpwaHk7Cj4+ICsgICAgICAgc3RydWN0IGxpc3RfaGVhZCAgICAgICAgbGlz dDsKPj4gK307Cj4+ICsKPj4gK3N0YXRpYyBzdHJ1Y3QgdXNiX3BoeV9yb290aHViICp1c2JfcGh5 X3Jvb3RodWJfYWxsb2Moc3RydWN0IGRldmljZSAqZGV2KQo+PiArewo+PiArICAgICAgIHN0cnVj dCB1c2JfcGh5X3Jvb3RodWIgKnJvb3RodWJfZW50cnk7Cj4+ICsKPj4gKyAgICAgICByb290aHVi X2VudHJ5ID0gZGV2bV9remFsbG9jKGRldiwgc2l6ZW9mKCpyb290aHViX2VudHJ5KSwgR0ZQX0tF Uk5FTCk7Cj4+ICsgICAgICAgaWYgKCFyb290aHViX2VudHJ5KQo+PiArICAgICAgICAgICAgICAg cmV0dXJuIEVSUl9QVFIoLUVOT01FTSk7Cj4+ICsKPj4gKyAgICAgICBJTklUX0xJU1RfSEVBRCgm cm9vdGh1Yl9lbnRyeS0+bGlzdCk7Cj4+ICsKPj4gKyAgICAgICByZXR1cm4gcm9vdGh1Yl9lbnRy eTsKPj4gK30KPj4gKwo+PiArc3RhdGljIGludCB1c2JfcGh5X3Jvb3RodWJfYWRkX3BoeShzdHJ1 Y3QgZGV2aWNlICpkZXYsIGludCBpbmRleCwKPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICBzdHJ1Y3QgbGlzdF9oZWFkICpsaXN0KQo+PiArewo+PiArICAgICAgIHN0cnVjdCB1 c2JfcGh5X3Jvb3RodWIgKnJvb3RodWJfZW50cnk7Cj4+ICsgICAgICAgc3RydWN0IHBoeSAqcGh5 ID0gZGV2bV9vZl9waHlfZ2V0X2J5X2luZGV4KGRldiwgZGV2LT5vZl9ub2RlLCBpbmRleCk7Cj4+ ICsKPj4gKyAgICAgICBpZiAoSVNfRVJSX09SX05VTEwocGh5KSkgewo+PiArICAgICAgICAgICAg ICAgaWYgKCFwaHkgfHwgUFRSX0VSUihwaHkpID09IC1FTk9ERVYpCj4+ICsgICAgICAgICAgICAg ICAgICAgICAgIHJldHVybiAwOwo+PiArICAgICAgICAgICAgICAgZWxzZQo+PiArICAgICAgICAg ICAgICAgICAgICAgICByZXR1cm4gUFRSX0VSUihwaHkpOwo+PiArICAgICAgIH0KPj4gKwo+PiAr ICAgICAgIHJvb3RodWJfZW50cnkgPSB1c2JfcGh5X3Jvb3RodWJfYWxsb2MoZGV2KTsKPj4gKyAg ICAgICBpZiAoSVNfRVJSKHJvb3RodWJfZW50cnkpKQo+PiArICAgICAgICAgICAgICAgcmV0dXJu IFBUUl9FUlIocm9vdGh1Yl9lbnRyeSk7Cj4+ICsKPj4gKyAgICAgICByb290aHViX2VudHJ5LT5w aHkgPSBwaHk7Cj4+ICsKPj4gKyAgICAgICBsaXN0X2FkZF90YWlsKCZyb290aHViX2VudHJ5LT5s aXN0LCBsaXN0KTsKPj4gKwo+PiArICAgICAgIHJldHVybiAwOwo+PiArfQo+PiArCj4+ICtzdHJ1 Y3QgdXNiX3BoeV9yb290aHViICp1c2JfcGh5X3Jvb3RodWJfaW5pdChzdHJ1Y3QgZGV2aWNlICpk ZXYpCj4+ICt7Cj4+ICsgICAgICAgc3RydWN0IHVzYl9waHlfcm9vdGh1YiAqcGh5X3Jvb3RodWI7 Cj4+ICsgICAgICAgc3RydWN0IHVzYl9waHlfcm9vdGh1YiAqcm9vdGh1Yl9lbnRyeTsKPj4gKyAg ICAgICBzdHJ1Y3QgbGlzdF9oZWFkICpoZWFkOwo+PiArICAgICAgIGludCBpLCBudW1fcGh5cywg ZXJyOwo+PiArCj4+ICsgICAgICAgbnVtX3BoeXMgPSBvZl9jb3VudF9waGFuZGxlX3dpdGhfYXJn cyhkZXYtPm9mX25vZGUsICJwaHlzIiwKPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICIjcGh5LWNlbGxzIik7Cj4+ICsgICAgICAgaWYgKG51bV9waHlzIDw9 IDApCj4+ICsgICAgICAgICAgICAgICByZXR1cm4gTlVMTDsKPj4gKwo+PiArICAgICAgIHBoeV9y b290aHViID0gdXNiX3BoeV9yb290aHViX2FsbG9jKGRldik7Cj4+ICsgICAgICAgaWYgKElTX0VS UihwaHlfcm9vdGh1YikpCj4+ICsgICAgICAgICAgICAgICByZXR1cm4gcGh5X3Jvb3RodWI7Cj4+ ICsKPj4gKyAgICAgICBmb3IgKGkgPSAwOyBpIDwgbnVtX3BoeXM7IGkrKykgewo+PiArICAgICAg ICAgICAgICAgZXJyID0gdXNiX3BoeV9yb290aHViX2FkZF9waHkoZGV2LCBpLCAmcGh5X3Jvb3Ro dWItPmxpc3QpOwo+PiArICAgICAgICAgICAgICAgaWYgKGVycikKPj4gKyAgICAgICAgICAgICAg ICAgICAgICAgZ290byBlcnJfb3V0Owo+PiArICAgICAgIH0KPj4gKwo+PiArICAgICAgIGhlYWQg PSAmcGh5X3Jvb3RodWItPmxpc3Q7Cj4KPgo+IEkgdGhpbmsgcGh5X3Jvb3RodWItPnBoeSBpcyBh bHdheXMgZW1wdHksCj4gYW5kIG9ubHkgcGh5X3Jvb3RodWItPmxpc3QgaXMgdXNlZC4KPgo+Cj4g SSBqdXN0IHdvbmRlcmVkIGlmIHdlIGNhbiBkaXJlY3RseSBwdXQgJ3N0cnVjdCBsaXN0X2hlYWQn IGludG8KPiAnc3RydWN0IHVzYl9oY2QnLgptYXliZSB5b3UgY2FuIHRyeSBpdCBhbmQgc2VuZCBh IHBhdGNoIChub3RlOiBwbGVhc2Ugd2FpdCB1bnRpbAp2NC4xNy1yYzItaXNoIGJlY2F1c2UgdGhl cmUgYXJlIHNvbWUgZml4ZXMgdGhhdCB3aWxsIGJlIGFwcGxpZWQgYnkKR3JlZyBhZnRlciB2NC4x Ny1yYzEgaXMgb3V0LCBhcyB3ZWxsIGFzIHRoZSBpc3N1ZXMgeW91IG5vdGVkIGJlbG93KT8KSSBi ZWxpZXZlIGl0J3Mgbm90IGNyaXRpY2FsIHNvIHRoZXJlJ3MgcGxlbnR5IG9mIHRpbWUgdG8gZ2V0 IGl0IGludG8gdjQuMTgKCj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3VzYi9jb3JlL3BoeS5oIGIv ZHJpdmVycy91c2IvY29yZS9waHkuaAo+PiBuZXcgZmlsZSBtb2RlIDEwMDY0NAo+PiBpbmRleCAw MDAwMDAwMDAwMDAuLjZmZGU1OWJmYmZmOAo+PiAtLS0gL2Rldi9udWxsCj4+ICsrKyBiL2RyaXZl cnMvdXNiL2NvcmUvcGh5LmgKPj4gQEAgLTAsMCArMSw3IEBACj4+ICtzdHJ1Y3QgdXNiX3BoeV9y b290aHViOwo+Cj4gc3RydWN0IGRldmljZTsKPgo+IGlzIGFsc28gbmVlZGVkIGZvciB1c2JfcGh5 X3Jvb3RodWJfaW5pdCgpLgpkbyB5b3UgdGhpbmsgdGhhdCBJIHNob3VsZCBzdGlsbCBmaXggdGhp cyBpbiA0LjE3PwoKPj4gK3N0cnVjdCB1c2JfcGh5X3Jvb3RodWIgKnVzYl9waHlfcm9vdGh1Yl9p bml0KHN0cnVjdCBkZXZpY2UgKmRldik7Cj4+ICtpbnQgdXNiX3BoeV9yb290aHViX2V4aXQoc3Ry dWN0IHVzYl9waHlfcm9vdGh1YiAqcGh5X3Jvb3RodWIpOwo+PiArCj4+ICtpbnQgdXNiX3BoeV9y b290aHViX3Bvd2VyX29uKHN0cnVjdCB1c2JfcGh5X3Jvb3RodWIgKnBoeV9yb290aHViKTsKPj4g K3ZvaWQgdXNiX3BoeV9yb290aHViX3Bvd2VyX29mZihzdHJ1Y3QgdXNiX3BoeV9yb290aHViICpw aHlfcm9vdGh1Yik7Cj4KPgo+IEJldHRlciB0byBhZGQgYW4gaW5jbHVkZSBndWFyZD8KPiBTUERY LUxpY2Vuc2UtSWRlbnRpZmllciBmb3IgdGhpcyBoZWFkZXIgdG9vPwpnb29kIGNhdGNoIC0gSSds bCBzZW5kIGEgcGF0Y2ggZm9yIHRoaXMgdG9vIChhbG9uZyB3aXRoIHRoZSBtaXNzaW5nCmZvcndh cmQgZGVjbGFyYXRpb24gZm9yIHN0cnVjdCBkZXZpY2UpCgoKUmVnYXJkcwpNYXJ0aW4KLS0tClRv IHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBs aW51eC11c2IiIGluCnRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJu ZWwub3JnCk1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFq b3Jkb21vLWluZm8uaHRtbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: martin.blumenstingl@googlemail.com (Martin Blumenstingl) Date: Thu, 5 Apr 2018 22:04:01 +0200 Subject: [usb-next PATCH v11 3/8] usb: core: add a wrapper for the USB PHYs on the HCD In-Reply-To: References: <20180303214309.25643-1-martin.blumenstingl@googlemail.com> <20180303214309.25643-4-martin.blumenstingl@googlemail.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hello, (great to hear that this might be useful on Socionext SoCs as well :)) On Wed, Apr 4, 2018 at 2:10 PM, Masahiro Yamada wrote: > 2018-03-04 6:43 GMT+09:00 Martin Blumenstingl > : >> 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 >> --- >> 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; > > > I think phy_roothub->phy is always empty, > and only phy_roothub->list is used. > > > I just wondered if we can directly put 'struct list_head' into > 'struct usb_hcd'. maybe you can try it and send a patch (note: please wait until v4.17-rc2-ish because there are some fixes that will be applied by Greg after v4.17-rc1 is out, as well as the issues you noted below)? I believe it's not critical so there's plenty of time to get it into v4.18 >> 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 device; > > is also needed for usb_phy_roothub_init(). do you think that I should still fix this in 4.17? >> +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); > > > Better to add an include guard? > SPDX-License-Identifier for this header too? good catch - I'll send a patch for this too (along with the missing forward declaration for struct device) Regards Martin From mboxrd@z Thu Jan 1 00:00:00 1970 From: martin.blumenstingl@googlemail.com (Martin Blumenstingl) Date: Thu, 5 Apr 2018 22:04:01 +0200 Subject: [usb-next PATCH v11 3/8] usb: core: add a wrapper for the USB PHYs on the HCD In-Reply-To: References: <20180303214309.25643-1-martin.blumenstingl@googlemail.com> <20180303214309.25643-4-martin.blumenstingl@googlemail.com> Message-ID: To: linus-amlogic@lists.infradead.org List-Id: linus-amlogic.lists.infradead.org Hello, (great to hear that this might be useful on Socionext SoCs as well :)) On Wed, Apr 4, 2018 at 2:10 PM, Masahiro Yamada wrote: > 2018-03-04 6:43 GMT+09:00 Martin Blumenstingl > : >> 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 >> --- >> 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; > > > I think phy_roothub->phy is always empty, > and only phy_roothub->list is used. > > > I just wondered if we can directly put 'struct list_head' into > 'struct usb_hcd'. maybe you can try it and send a patch (note: please wait until v4.17-rc2-ish because there are some fixes that will be applied by Greg after v4.17-rc1 is out, as well as the issues you noted below)? I believe it's not critical so there's plenty of time to get it into v4.18 >> 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 device; > > is also needed for usb_phy_roothub_init(). do you think that I should still fix this in 4.17? >> +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); > > > Better to add an include guard? > SPDX-License-Identifier for this header too? good catch - I'll send a patch for this too (along with the missing forward declaration for struct device) Regards Martin