From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: AG47ELtPGdUjI9v3+fhMo2VtMBMEBkVEcJKUJJqxkRwkfrZkWS8EfEilsPqkKA9S19/23vc8LWlg ARC-Seal: i=1; a=rsa-sha256; t=1519824832; cv=none; d=google.com; s=arc-20160816; b=DfXlbcSZ68TJpMwVnoOirmhTgHT/KdadlO7QTKx/iJV6T5JS272NKnmV6xcD+qnnEr FLdVFKEMUr1X/yMOMh0PG3SHCXm9R3PYOkODDwn66yzF9LeiwmaMvVM+xVkz6QYZ60Fu ozaHX3dewMY5Wk1XlU7lV18kPmda93XeziqJRoHdjgZH5P4dPTD+ifLq63bXxtQ2HdEn ZhbeMo5U15vpNit8ZLsAyIHrd1DZjYoJJwyvPxEXObF0NE/K9cGqaCnJjYcZrVEJl/Qb 7mOK2UodtuLCy20E/Ra0ncfyX7K+EtbkIa9z3m83CzSWF8nocot7iwdpxWDu1yPTwa+U Qbfg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :arc-authentication-results; bh=LsPRR15ehlwUJ6p0Ikd8VfsCkgBsIwypW1BdjDHOVlw=; b=dBogP9q3p6t7Xjzi1CNY4Kr5XvS5gcIuupCiZmbc7mryOdDz/L2lJZar1qZJPuRCIm 4qFGqGy1HRDUP0cSgUxmFseLLTa5TciiKuetm0bUvPhsNB890PAu9+zK6tjo5dO4jUdE 06zbcgtzWTte94W3USTJ/nMY3Ycva9XA+IwKtoLa5I/m9v8dDMMqAIPEE3GdKgpvfzMi YITgtJjnTmJFc5u47eOE7U5W4zsjeI38XyF+X6hxN1RuHTrAvubVphAFj037MlT/5Hit C2f+Gq6z8Ull1mVvsTgwcgNQ8NwNsrmaCbj1mkIAasieXdm9g30se7emk7yzBtq2LUps ZrfQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of robin.murphy@arm.com designates 217.140.101.70 as permitted sender) smtp.mailfrom=robin.murphy@arm.com Authentication-Results: mx.google.com; spf=pass (google.com: domain of robin.murphy@arm.com designates 217.140.101.70 as permitted sender) smtp.mailfrom=robin.murphy@arm.com Subject: Re: [PATCH v3] usb: host: ehci-platform: add support for optional external vbus supply To: Amelie Delaunay , Greg Kroah-Hartman , Rob Herring , Mark Rutland , Tony Prisk , Alan Stern , Roger Quadros , Felipe Balbi Cc: devicetree@vger.kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org References: <1519393611-31367-1-git-send-email-amelie.delaunay@st.com> From: Robin Murphy Message-ID: Date: Wed, 28 Feb 2018 13:33:48 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <1519393611-31367-1-git-send-email-amelie.delaunay@st.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1592910586052663664?= X-GMAIL-MSGID: =?utf-8?q?1593651843977584442?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: Hi Amelie, Just a couple of drive-by coding style comments... On 23/02/18 13:46, Amelie Delaunay wrote: > On some boards, especially when vbus supply requires large current, > and the charge pump on the PHY isn't enough, an external vbus power switch > may be used. > Add support for optional external vbus supply per port in ehci-platform. > > Signed-off-by: Amelie Delaunay > > --- > Changes in v3: > * Address Felipe Balbi comments: reduce indentation in > ehci_platform_port_power. > * Address Roger Quadros and Alan Stern comments: platforms can have one > external vbus supply per port, so add support to get as many optional > regulator as implemented ports on the host controller. > > Changes in v2: > * Address Roger Quadros comments: move regulator_enable/disable from > ehci_platform_power_on/off to ehci_platform_port_power. > --- > Documentation/devicetree/bindings/usb/usb-ehci.txt | 1 + > drivers/usb/host/ehci-platform.c | 52 +++++++++++++++++++++- > 2 files changed, 52 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt > index 3efde12..cd576db 100644 > --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt > +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt > @@ -19,6 +19,7 @@ Optional properties: > - phys : phandle + phy specifier pair > - phy-names : "usb" > - resets : phandle + reset specifier pair > + - portN_vbus-supply : phandle of regulator supplying vbus for port N > > Example (Sequoia 440EPx): > ehci@e0000300 { > diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c > index b065a96..8e9f201 100644 > --- a/drivers/usb/host/ehci-platform.c > +++ b/drivers/usb/host/ehci-platform.c > @@ -29,6 +29,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -46,6 +47,7 @@ struct ehci_platform_priv { > struct reset_control *rsts; > struct phy **phys; > int num_phys; > + struct regulator **vbus_supplies; > bool reset_on_resume; > }; > > @@ -56,7 +58,8 @@ static int ehci_platform_reset(struct usb_hcd *hcd) > struct platform_device *pdev = to_platform_device(hcd->self.controller); > struct usb_ehci_pdata *pdata = dev_get_platdata(&pdev->dev); > struct ehci_hcd *ehci = hcd_to_ehci(hcd); > - int retval; > + struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd); > + int portnum, n_ports, retval; > > ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug; > > @@ -71,11 +74,57 @@ static int ehci_platform_reset(struct usb_hcd *hcd) > if (retval) > return retval; > > + n_ports = HCS_N_PORTS(ehci->hcs_params); > + priv->vbus_supplies = devm_kcalloc(&pdev->dev, n_ports, > + sizeof(struct regulator *), Using "sizeof(*priv->vbus_supplies)" here will prevent people sending annoying cleanup patches later. > + GFP_KERNEL); > + if (!priv->vbus_supplies) > + return -ENOMEM; > + > + for (portnum = 0; portnum < n_ports; portnum++) { > + struct regulator *vbus_supply; > + char id[20]; > + > + sprintf(id, "port%d_vbus", portnum); > + > + vbus_supply = devm_regulator_get_optional(&pdev->dev, id); > + if (IS_ERR(vbus_supply)) { > + retval = PTR_ERR(vbus_supply); > + if (retval == -ENODEV) > + priv->vbus_supplies[portnum] = NULL; The array element here hasn't yet been assigned to since kcalloc() initially zeroed it, so this is entirely redundant - you can simply make the comparison a "!=" and remove the "else" case. Robin. > + else > + return retval; > + } else { > + priv->vbus_supplies[portnum] = vbus_supply; > + } > + } > + > if (pdata->no_io_watchdog) > ehci->need_io_watchdog = 0; > return 0; > } > > +static int ehci_platform_port_power(struct usb_hcd *hcd, int portnum, > + bool enable) > +{ > + struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd); > + int ret; > + > + if (!priv->vbus_supplies[portnum]) > + return 0; > + > + if (enable) > + ret = regulator_enable(priv->vbus_supplies[portnum]); > + else > + ret = regulator_disable(priv->vbus_supplies[portnum]); > + if (ret) > + dev_err(hcd->self.controller, > + "failed to %s vbus supply for port %d: %d\n", > + enable ? "enable" : "disable", portnum, ret); > + > + return ret; > +} > + > static int ehci_platform_power_on(struct platform_device *dev) > { > struct usb_hcd *hcd = platform_get_drvdata(dev); > @@ -134,6 +183,7 @@ static struct hc_driver __read_mostly ehci_platform_hc_driver; > static const struct ehci_driver_overrides platform_overrides __initconst = { > .reset = ehci_platform_reset, > .extra_priv_size = sizeof(struct ehci_platform_priv), > + .port_power = ehci_platform_port_power, > }; > > static struct usb_ehci_pdata ehci_platform_defaults = { > 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: [v3] usb: host: ehci-platform: add support for optional external vbus supply From: Robin Murphy Message-Id: Date: Wed, 28 Feb 2018 13:33:48 +0000 To: Amelie Delaunay , Greg Kroah-Hartman , Rob Herring , Mark Rutland , Tony Prisk , Alan Stern , Roger Quadros , Felipe Balbi Cc: devicetree@vger.kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org List-ID: SGkgQW1lbGllLAoKSnVzdCBhIGNvdXBsZSBvZiBkcml2ZS1ieSBjb2Rpbmcgc3R5bGUgY29tbWVu dHMuLi4KCk9uIDIzLzAyLzE4IDEzOjQ2LCBBbWVsaWUgRGVsYXVuYXkgd3JvdGU6Cj4gT24gc29t ZSBib2FyZHMsIGVzcGVjaWFsbHkgd2hlbiB2YnVzIHN1cHBseSByZXF1aXJlcyBsYXJnZSBjdXJy ZW50LAo+IGFuZCB0aGUgY2hhcmdlIHB1bXAgb24gdGhlIFBIWSBpc24ndCBlbm91Z2gsIGFuIGV4 dGVybmFsIHZidXMgcG93ZXIgc3dpdGNoCj4gbWF5IGJlIHVzZWQuCj4gQWRkIHN1cHBvcnQgZm9y IG9wdGlvbmFsIGV4dGVybmFsIHZidXMgc3VwcGx5IHBlciBwb3J0IGluIGVoY2ktcGxhdGZvcm0u Cj4gCj4gU2lnbmVkLW9mZi1ieTogQW1lbGllIERlbGF1bmF5IDxhbWVsaWUuZGVsYXVuYXlAc3Qu Y29tPgo+IAo+IC0tLQo+IENoYW5nZXMgaW4gdjM6Cj4gICAqIEFkZHJlc3MgRmVsaXBlIEJhbGJp IGNvbW1lbnRzOiByZWR1Y2UgaW5kZW50YXRpb24gaW4KPiAgICAgZWhjaV9wbGF0Zm9ybV9wb3J0 X3Bvd2VyLgo+ICAgKiBBZGRyZXNzIFJvZ2VyIFF1YWRyb3MgYW5kIEFsYW4gU3Rlcm4gY29tbWVu dHM6IHBsYXRmb3JtcyBjYW4gaGF2ZSBvbmUKPiAgICAgZXh0ZXJuYWwgdmJ1cyBzdXBwbHkgcGVy IHBvcnQsIHNvIGFkZCBzdXBwb3J0IHRvIGdldCBhcyBtYW55IG9wdGlvbmFsCj4gICAgIHJlZ3Vs YXRvciBhcyBpbXBsZW1lbnRlZCBwb3J0cyBvbiB0aGUgaG9zdCBjb250cm9sbGVyLgo+IAo+IENo YW5nZXMgaW4gdjI6Cj4gICAqIEFkZHJlc3MgUm9nZXIgUXVhZHJvcyBjb21tZW50czogbW92ZSBy ZWd1bGF0b3JfZW5hYmxlL2Rpc2FibGUgZnJvbQo+ICAgICBlaGNpX3BsYXRmb3JtX3Bvd2VyX29u L29mZiB0byBlaGNpX3BsYXRmb3JtX3BvcnRfcG93ZXIuCj4gLS0tCj4gICBEb2N1bWVudGF0aW9u L2RldmljZXRyZWUvYmluZGluZ3MvdXNiL3VzYi1laGNpLnR4dCB8ICAxICsKPiAgIGRyaXZlcnMv dXNiL2hvc3QvZWhjaS1wbGF0Zm9ybS5jICAgICAgICAgICAgICAgICAgIHwgNTIgKysrKysrKysr KysrKysrKysrKysrLQo+ICAgMiBmaWxlcyBjaGFuZ2VkLCA1MiBpbnNlcnRpb25zKCspLCAxIGRl bGV0aW9uKC0pCj4gCj4gZGlmZiAtLWdpdCBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5k aW5ncy91c2IvdXNiLWVoY2kudHh0IGIvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdz L3VzYi91c2ItZWhjaS50eHQKPiBpbmRleCAzZWZkZTEyLi5jZDU3NmRiIDEwMDY0NAo+IC0tLSBh L0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy91c2IvdXNiLWVoY2kudHh0Cj4gKysr IGIvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL3VzYi91c2ItZWhjaS50eHQKPiBA QCAtMTksNiArMTksNyBAQCBPcHRpb25hbCBwcm9wZXJ0aWVzOgo+ICAgIC0gcGh5cyA6IHBoYW5k bGUgKyBwaHkgc3BlY2lmaWVyIHBhaXIKPiAgICAtIHBoeS1uYW1lcyA6ICJ1c2IiCj4gICAgLSBy ZXNldHMgOiBwaGFuZGxlICsgcmVzZXQgc3BlY2lmaWVyIHBhaXIKPiArIC0gcG9ydE5fdmJ1cy1z dXBwbHkgOiBwaGFuZGxlIG9mIHJlZ3VsYXRvciBzdXBwbHlpbmcgdmJ1cyBmb3IgcG9ydCBOCj4g ICAKPiAgIEV4YW1wbGUgKFNlcXVvaWEgNDQwRVB4KToKPiAgICAgICBlaGNpQGUwMDAwMzAwIHsK PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy91c2IvaG9zdC9laGNpLXBsYXRmb3JtLmMgYi9kcml2ZXJz L3VzYi9ob3N0L2VoY2ktcGxhdGZvcm0uYwo+IGluZGV4IGIwNjVhOTYuLjhlOWYyMDEgMTAwNjQ0 Cj4gLS0tIGEvZHJpdmVycy91c2IvaG9zdC9laGNpLXBsYXRmb3JtLmMKPiArKysgYi9kcml2ZXJz L3VzYi9ob3N0L2VoY2ktcGxhdGZvcm0uYwo+IEBAIC0yOSw2ICsyOSw3IEBACj4gICAjaW5jbHVk ZSA8bGludXgvb2YuaD4KPiAgICNpbmNsdWRlIDxsaW51eC9waHkvcGh5Lmg+Cj4gICAjaW5jbHVk ZSA8bGludXgvcGxhdGZvcm1fZGV2aWNlLmg+Cj4gKyNpbmNsdWRlIDxsaW51eC9yZWd1bGF0b3Iv Y29uc3VtZXIuaD4KPiAgICNpbmNsdWRlIDxsaW51eC9yZXNldC5oPgo+ICAgI2luY2x1ZGUgPGxp bnV4L3VzYi5oPgo+ICAgI2luY2x1ZGUgPGxpbnV4L3VzYi9oY2QuaD4KPiBAQCAtNDYsNiArNDcs NyBAQCBzdHJ1Y3QgZWhjaV9wbGF0Zm9ybV9wcml2IHsKPiAgIAlzdHJ1Y3QgcmVzZXRfY29udHJv bCAqcnN0czsKPiAgIAlzdHJ1Y3QgcGh5ICoqcGh5czsKPiAgIAlpbnQgbnVtX3BoeXM7Cj4gKwlz dHJ1Y3QgcmVndWxhdG9yICoqdmJ1c19zdXBwbGllczsKPiAgIAlib29sIHJlc2V0X29uX3Jlc3Vt ZTsKPiAgIH07Cj4gICAKPiBAQCAtNTYsNyArNTgsOCBAQCBzdGF0aWMgaW50IGVoY2lfcGxhdGZv cm1fcmVzZXQoc3RydWN0IHVzYl9oY2QgKmhjZCkKPiAgIAlzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNl ICpwZGV2ID0gdG9fcGxhdGZvcm1fZGV2aWNlKGhjZC0+c2VsZi5jb250cm9sbGVyKTsKPiAgIAlz dHJ1Y3QgdXNiX2VoY2lfcGRhdGEgKnBkYXRhID0gZGV2X2dldF9wbGF0ZGF0YSgmcGRldi0+ZGV2 KTsKPiAgIAlzdHJ1Y3QgZWhjaV9oY2QgKmVoY2kgPSBoY2RfdG9fZWhjaShoY2QpOwo+IC0JaW50 IHJldHZhbDsKPiArCXN0cnVjdCBlaGNpX3BsYXRmb3JtX3ByaXYgKnByaXYgPSBoY2RfdG9fZWhj aV9wcml2KGhjZCk7Cj4gKwlpbnQgcG9ydG51bSwgbl9wb3J0cywgcmV0dmFsOwo+ICAgCj4gICAJ ZWhjaS0+aGFzX3N5bm9wc3lzX2hjX2J1ZyA9IHBkYXRhLT5oYXNfc3lub3BzeXNfaGNfYnVnOwo+ ICAgCj4gQEAgLTcxLDExICs3NCw1NyBAQCBzdGF0aWMgaW50IGVoY2lfcGxhdGZvcm1fcmVzZXQo c3RydWN0IHVzYl9oY2QgKmhjZCkKPiAgIAlpZiAocmV0dmFsKQo+ICAgCQlyZXR1cm4gcmV0dmFs Owo+ICAgCj4gKwluX3BvcnRzID0gSENTX05fUE9SVFMoZWhjaS0+aGNzX3BhcmFtcyk7Cj4gKwlw cml2LT52YnVzX3N1cHBsaWVzID0gZGV2bV9rY2FsbG9jKCZwZGV2LT5kZXYsIG5fcG9ydHMsCj4g KwkJCQkJICAgc2l6ZW9mKHN0cnVjdCByZWd1bGF0b3IgKiksCgpVc2luZyAic2l6ZW9mKCpwcml2 LT52YnVzX3N1cHBsaWVzKSIgaGVyZSB3aWxsIHByZXZlbnQgcGVvcGxlIHNlbmRpbmcgCmFubm95 aW5nIGNsZWFudXAgcGF0Y2hlcyBsYXRlci4KCj4gKwkJCQkJICAgR0ZQX0tFUk5FTCk7Cj4gKwlp ZiAoIXByaXYtPnZidXNfc3VwcGxpZXMpCj4gKwkJcmV0dXJuIC1FTk9NRU07Cj4gKwo+ICsJZm9y IChwb3J0bnVtID0gMDsgcG9ydG51bSA8IG5fcG9ydHM7IHBvcnRudW0rKykgewo+ICsJCXN0cnVj dCByZWd1bGF0b3IgKnZidXNfc3VwcGx5Owo+ICsJCWNoYXIgaWRbMjBdOwo+ICsKPiArCQlzcHJp bnRmKGlkLCAicG9ydCVkX3ZidXMiLCBwb3J0bnVtKTsKPiArCj4gKwkJdmJ1c19zdXBwbHkgPSBk ZXZtX3JlZ3VsYXRvcl9nZXRfb3B0aW9uYWwoJnBkZXYtPmRldiwgaWQpOwo+ICsJCWlmIChJU19F UlIodmJ1c19zdXBwbHkpKSB7Cj4gKwkJCXJldHZhbCA9IFBUUl9FUlIodmJ1c19zdXBwbHkpOwo+ ICsJCQlpZiAocmV0dmFsID09IC1FTk9ERVYpCj4gKwkJCQlwcml2LT52YnVzX3N1cHBsaWVzW3Bv cnRudW1dID0gTlVMTDsKClRoZSBhcnJheSBlbGVtZW50IGhlcmUgaGFzbid0IHlldCBiZWVuIGFz c2lnbmVkIHRvIHNpbmNlIGtjYWxsb2MoKSAKaW5pdGlhbGx5IHplcm9lZCBpdCwgc28gdGhpcyBp cyBlbnRpcmVseSByZWR1bmRhbnQgLSB5b3UgY2FuIHNpbXBseSBtYWtlIAp0aGUgY29tcGFyaXNv biBhICIhPSIgYW5kIHJlbW92ZSB0aGUgImVsc2UiIGNhc2UuCgpSb2Jpbi4KCj4gKwkJCWVsc2UK PiArCQkJCXJldHVybiByZXR2YWw7Cj4gKwkJfSBlbHNlIHsKPiArCQkJcHJpdi0+dmJ1c19zdXBw bGllc1twb3J0bnVtXSA9IHZidXNfc3VwcGx5Owo+ICsJCX0KPiArCX0KPiArCj4gICAJaWYgKHBk YXRhLT5ub19pb193YXRjaGRvZykKPiAgIAkJZWhjaS0+bmVlZF9pb193YXRjaGRvZyA9IDA7Cj4g ICAJcmV0dXJuIDA7Cj4gICB9Cj4gICAKPiArc3RhdGljIGludCBlaGNpX3BsYXRmb3JtX3BvcnRf cG93ZXIoc3RydWN0IHVzYl9oY2QgKmhjZCwgaW50IHBvcnRudW0sCj4gKwkJCQkgICAgYm9vbCBl bmFibGUpCj4gK3sKPiArCXN0cnVjdCBlaGNpX3BsYXRmb3JtX3ByaXYgKnByaXYgPSBoY2RfdG9f ZWhjaV9wcml2KGhjZCk7Cj4gKwlpbnQgcmV0Owo+ICsKPiArCWlmICghcHJpdi0+dmJ1c19zdXBw bGllc1twb3J0bnVtXSkKPiArCQlyZXR1cm4gMDsKPiArCj4gKwlpZiAoZW5hYmxlKQo+ICsJCXJl dCA9IHJlZ3VsYXRvcl9lbmFibGUocHJpdi0+dmJ1c19zdXBwbGllc1twb3J0bnVtXSk7Cj4gKwll bHNlCj4gKwkJcmV0ID0gcmVndWxhdG9yX2Rpc2FibGUocHJpdi0+dmJ1c19zdXBwbGllc1twb3J0 bnVtXSk7Cj4gKwlpZiAocmV0KQo+ICsJCWRldl9lcnIoaGNkLT5zZWxmLmNvbnRyb2xsZXIsCj4g KwkJCSJmYWlsZWQgdG8gJXMgdmJ1cyBzdXBwbHkgZm9yIHBvcnQgJWQ6ICVkXG4iLAo+ICsJCQll bmFibGUgPyAiZW5hYmxlIiA6ICJkaXNhYmxlIiwgcG9ydG51bSwgcmV0KTsKPiArCj4gKwlyZXR1 cm4gcmV0Owo+ICt9Cj4gKwo+ICAgc3RhdGljIGludCBlaGNpX3BsYXRmb3JtX3Bvd2VyX29uKHN0 cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKmRldikKPiAgIHsKPiAgIAlzdHJ1Y3QgdXNiX2hjZCAqaGNk ID0gcGxhdGZvcm1fZ2V0X2RydmRhdGEoZGV2KTsKPiBAQCAtMTM0LDYgKzE4Myw3IEBAIHN0YXRp YyBzdHJ1Y3QgaGNfZHJpdmVyIF9fcmVhZF9tb3N0bHkgZWhjaV9wbGF0Zm9ybV9oY19kcml2ZXI7 Cj4gICBzdGF0aWMgY29uc3Qgc3RydWN0IGVoY2lfZHJpdmVyX292ZXJyaWRlcyBwbGF0Zm9ybV9v dmVycmlkZXMgX19pbml0Y29uc3QgPSB7Cj4gICAJLnJlc2V0ID0JCWVoY2lfcGxhdGZvcm1fcmVz ZXQsCj4gICAJLmV4dHJhX3ByaXZfc2l6ZSA9CXNpemVvZihzdHJ1Y3QgZWhjaV9wbGF0Zm9ybV9w cml2KSwKPiArCS5wb3J0X3Bvd2VyID0JCWVoY2lfcGxhdGZvcm1fcG9ydF9wb3dlciwKPiAgIH07 Cj4gICAKPiAgIHN0YXRpYyBzdHJ1Y3QgdXNiX2VoY2lfcGRhdGEgZWhjaV9wbGF0Zm9ybV9kZWZh dWx0cyA9IHsKPgotLS0KVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxp bmUgInVuc3Vic2NyaWJlIGxpbnV4LXVzYiIgaW4KdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1h am9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcKTW9yZSBtYWpvcmRvbW8gaW5mbyBhdCAgaHR0cDovL3Zn ZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: robin.murphy@arm.com (Robin Murphy) Date: Wed, 28 Feb 2018 13:33:48 +0000 Subject: [PATCH v3] usb: host: ehci-platform: add support for optional external vbus supply In-Reply-To: <1519393611-31367-1-git-send-email-amelie.delaunay@st.com> References: <1519393611-31367-1-git-send-email-amelie.delaunay@st.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Amelie, Just a couple of drive-by coding style comments... On 23/02/18 13:46, Amelie Delaunay wrote: > On some boards, especially when vbus supply requires large current, > and the charge pump on the PHY isn't enough, an external vbus power switch > may be used. > Add support for optional external vbus supply per port in ehci-platform. > > Signed-off-by: Amelie Delaunay > > --- > Changes in v3: > * Address Felipe Balbi comments: reduce indentation in > ehci_platform_port_power. > * Address Roger Quadros and Alan Stern comments: platforms can have one > external vbus supply per port, so add support to get as many optional > regulator as implemented ports on the host controller. > > Changes in v2: > * Address Roger Quadros comments: move regulator_enable/disable from > ehci_platform_power_on/off to ehci_platform_port_power. > --- > Documentation/devicetree/bindings/usb/usb-ehci.txt | 1 + > drivers/usb/host/ehci-platform.c | 52 +++++++++++++++++++++- > 2 files changed, 52 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt > index 3efde12..cd576db 100644 > --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt > +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt > @@ -19,6 +19,7 @@ Optional properties: > - phys : phandle + phy specifier pair > - phy-names : "usb" > - resets : phandle + reset specifier pair > + - portN_vbus-supply : phandle of regulator supplying vbus for port N > > Example (Sequoia 440EPx): > ehci at e0000300 { > diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c > index b065a96..8e9f201 100644 > --- a/drivers/usb/host/ehci-platform.c > +++ b/drivers/usb/host/ehci-platform.c > @@ -29,6 +29,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -46,6 +47,7 @@ struct ehci_platform_priv { > struct reset_control *rsts; > struct phy **phys; > int num_phys; > + struct regulator **vbus_supplies; > bool reset_on_resume; > }; > > @@ -56,7 +58,8 @@ static int ehci_platform_reset(struct usb_hcd *hcd) > struct platform_device *pdev = to_platform_device(hcd->self.controller); > struct usb_ehci_pdata *pdata = dev_get_platdata(&pdev->dev); > struct ehci_hcd *ehci = hcd_to_ehci(hcd); > - int retval; > + struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd); > + int portnum, n_ports, retval; > > ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug; > > @@ -71,11 +74,57 @@ static int ehci_platform_reset(struct usb_hcd *hcd) > if (retval) > return retval; > > + n_ports = HCS_N_PORTS(ehci->hcs_params); > + priv->vbus_supplies = devm_kcalloc(&pdev->dev, n_ports, > + sizeof(struct regulator *), Using "sizeof(*priv->vbus_supplies)" here will prevent people sending annoying cleanup patches later. > + GFP_KERNEL); > + if (!priv->vbus_supplies) > + return -ENOMEM; > + > + for (portnum = 0; portnum < n_ports; portnum++) { > + struct regulator *vbus_supply; > + char id[20]; > + > + sprintf(id, "port%d_vbus", portnum); > + > + vbus_supply = devm_regulator_get_optional(&pdev->dev, id); > + if (IS_ERR(vbus_supply)) { > + retval = PTR_ERR(vbus_supply); > + if (retval == -ENODEV) > + priv->vbus_supplies[portnum] = NULL; The array element here hasn't yet been assigned to since kcalloc() initially zeroed it, so this is entirely redundant - you can simply make the comparison a "!=" and remove the "else" case. Robin. > + else > + return retval; > + } else { > + priv->vbus_supplies[portnum] = vbus_supply; > + } > + } > + > if (pdata->no_io_watchdog) > ehci->need_io_watchdog = 0; > return 0; > } > > +static int ehci_platform_port_power(struct usb_hcd *hcd, int portnum, > + bool enable) > +{ > + struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd); > + int ret; > + > + if (!priv->vbus_supplies[portnum]) > + return 0; > + > + if (enable) > + ret = regulator_enable(priv->vbus_supplies[portnum]); > + else > + ret = regulator_disable(priv->vbus_supplies[portnum]); > + if (ret) > + dev_err(hcd->self.controller, > + "failed to %s vbus supply for port %d: %d\n", > + enable ? "enable" : "disable", portnum, ret); > + > + return ret; > +} > + > static int ehci_platform_power_on(struct platform_device *dev) > { > struct usb_hcd *hcd = platform_get_drvdata(dev); > @@ -134,6 +183,7 @@ static struct hc_driver __read_mostly ehci_platform_hc_driver; > static const struct ehci_driver_overrides platform_overrides __initconst = { > .reset = ehci_platform_reset, > .extra_priv_size = sizeof(struct ehci_platform_priv), > + .port_power = ehci_platform_port_power, > }; > > static struct usb_ehci_pdata ehci_platform_defaults = { >