From patchwork Wed Mar 23 16:29:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 102538 Delivered-To: patch@linaro.org Received: by 10.112.199.169 with SMTP id jl9csp129639lbc; Wed, 23 Mar 2016 09:29:42 -0700 (PDT) X-Received: by 10.98.68.91 with SMTP id r88mr5616813pfa.12.1458750582776; Wed, 23 Mar 2016 09:29:42 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id q84si5208908pfa.197.2016.03.23.09.29.42; Wed, 23 Mar 2016 09:29:42 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753022AbcCWQ3l (ORCPT + 29 others); Wed, 23 Mar 2016 12:29:41 -0400 Received: from mout.kundenserver.de ([212.227.17.13]:65446 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750779AbcCWQ3j (ORCPT ); Wed, 23 Mar 2016 12:29:39 -0400 Received: from wuerfel.lan. ([78.42.132.4]) by mrelayeu.kundenserver.de (mreue103) with ESMTPA (Nemesis) id 0LvhpU-1ZhvCH1xwC-017YNt; Wed, 23 Mar 2016 17:29:29 +0100 From: Arnd Bergmann To: Felipe Balbi Cc: Mark Brown , Arnd Bergmann , Greg Kroah-Hartman , "Ivan T. Ivanov" , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] usb-phy: qcom-8x16: fix regulator API abuse Date: Wed, 23 Mar 2016 17:29:14 +0100 Message-Id: <1458750566-1915909-1-git-send-email-arnd@arndb.de> X-Mailer: git-send-email 2.7.0 X-Provags-ID: V03:K0:JaAg//bEJkON0/kvTK9HVKT5LB6ol8sJO57tJKLh1Ts86cTaH79 IAm0j3AR9dGwcofJa0B0RwXADygNLUZvOTH9zW8MzW2P2xt2JI+0KGsrdZPKDQTFtUkS/v3 NGRhkx071E/mAPVVMM6PEitBe6zDy9uojup4oG+e6Fw2kWp2qoH7XxzETvBm2N51+TzHDjn cCyqm26GH8fyrnaAB0UGA== X-UI-Out-Filterresults: notjunk:1; V01:K0:Wzwd4O1P0Mg=:HZe6Qyx2aO4NrsWPw6DbLn UQ8m8RYyfc/QDxsXXd4s3TSpjQhM06qFxFZzNez/e/+OIJKFdNMSqQ3fNbaUG2foDf8grIiPL YP6Jst2uFLZ0GRvmzkIU84RQ35n2+CE5Dil6fEMRsG7RvXZbgJWq9UazpHR2PPbeOWf9+itoC Dsk/kUowb9EMCpNvUWhVma6CnqOEDUG9qc7iurXGtFykhQz933sVE7mLN62uBxTSQxqIKoAgs ToOhWvi7ToozonT2iFYBm7IEA7FSSsm6jWqm7vPEGnC7Sd1kBQ8uw8Yftnc1x6AagbOm4Vs+F 9DIcxBZJchmAJU7rgIhqC83t9W4Ap1B1OCWzoMsFI2equLijSN4HdLYymi6aUvMWHmpIybMb/ tw61dQkCfsPIu9+AC0M9hXaDxSW5xogXBSo5HTN28o0G2VRtu1FHLUXhCmhEm1Ea1T4PgeO6M dQoFh0JlTRHUGWzwDL3t9C/PpkXfDPfCC2Z547W3SjvdRvBSDW6AQh2gyLjAe/vxt4ZtZvnjY q++jkdkgyLFIhoZ8Uv/cH3CQS9JZd2KH8vekWlnCt19N3scM2y0Jue3yYQV4Ttptk//PRE1+U R0CSjyoV2CBjBLVcSBHP6duwAmBEnTB+iFFZm1lfuhwNSPhZuzRRPZ6t/N+D5+PXyDjRZm4TT iVxxoy6c1X3Vkguq6R57xuhcJw8wgO1n3eUvK5ecs+C4F4Wqn/4yA602OIX8WIkofHxI= Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org gcc warns about the use of regulators in phy_8x16_probe: drivers/usb/phy/phy-qcom-8x16-usb.c: In function 'phy_8x16_probe': drivers/usb/phy/phy-qcom-8x16-usb.c:284:13: error: 'regs[0].consumer' may be used uninitialized in this function [-Werror=maybe-uninitialized] drivers/usb/phy/phy-qcom-8x16-usb.c:285:13: error: 'regs[1].consumer' may be used uninitialized in this function [-Werror=maybe-uninitialized] drivers/usb/phy/phy-qcom-8x16-usb.c:286:12: error: 'regs[2].consumer' may be used uninitialized in this function [-Werror=maybe-uninitialized] According to Mark Brown, this is the result of various abuses of the PHY interfaces [1], so let's fix the driver instead. This puts the regulator bulk data into the device structure so it gets properly initialized and lets us call regulator_bulk_enable() and regulator_bulk_disable() rather than open-coding them. Setting the voltages the way the driver does is rather pointless because for each regulator there is only one valid voltage range, so that can just get set up in the DT. As there doesn't seem to be any user of the newly added driver yet, we can simply make sure the DTs are setting this up right when they get added. I'm also fixing the handling of regulator_bulk_enable() failure. Right now, the driver just ignores any failure, which doesn't make sense, so I'm changing it to loudly complain (in case we actually had a bug here) and error out. Doing a fly-by review of the driver, I notice a couple of other problems that I'm not addressing here: - It really should not have been written as a USB PHY driver, but instead should use the PHY subsystem. - The DT compatible string does not follow the usual conventions, and it should have a proper identifier in it rather than a wildcard. - The example in the devicetree binding lists a register address that is the same as the actual EHCI host controller in the SoC as well as the otg-snps and the ci-hdrc device, which indicates that these are probably not even distinct devices (or all but one of them are wrong), and if more than one of them tries to request the resources correctly, they fail. [1] https://lkml.org/lkml/2016/1/26/267 Signed-off-by: Arnd Bergmann --- drivers/usb/phy/phy-qcom-8x16-usb.c | 72 ++++++------------------------------- 1 file changed, 11 insertions(+), 61 deletions(-) -- 2.7.0 diff --git a/drivers/usb/phy/phy-qcom-8x16-usb.c b/drivers/usb/phy/phy-qcom-8x16-usb.c index 579587d97217..3d7af85aecb9 100644 --- a/drivers/usb/phy/phy-qcom-8x16-usb.c +++ b/drivers/usb/phy/phy-qcom-8x16-usb.c @@ -65,9 +65,7 @@ struct phy_8x16 { void __iomem *regs; struct clk *core_clk; struct clk *iface_clk; - struct regulator *v3p3; - struct regulator *v1p8; - struct regulator *vdd; + struct regulator_bulk_data regulator[3]; struct reset_control *phy_reset; @@ -78,51 +76,6 @@ struct phy_8x16 { struct notifier_block reboot_notify; }; -static int phy_8x16_regulators_enable(struct phy_8x16 *qphy) -{ - int ret; - - ret = regulator_set_voltage(qphy->vdd, HSPHY_VDD_MIN, HSPHY_VDD_MAX); - if (ret) - return ret; - - ret = regulator_enable(qphy->vdd); - if (ret) - return ret; - - ret = regulator_set_voltage(qphy->v3p3, HSPHY_3P3_MIN, HSPHY_3P3_MAX); - if (ret) - goto off_vdd; - - ret = regulator_enable(qphy->v3p3); - if (ret) - goto off_vdd; - - ret = regulator_set_voltage(qphy->v1p8, HSPHY_1P8_MIN, HSPHY_1P8_MAX); - if (ret) - goto off_3p3; - - ret = regulator_enable(qphy->v1p8); - if (ret) - goto off_3p3; - - return 0; - -off_3p3: - regulator_disable(qphy->v3p3); -off_vdd: - regulator_disable(qphy->vdd); - - return ret; -} - -static void phy_8x16_regulators_disable(struct phy_8x16 *qphy) -{ - regulator_disable(qphy->v1p8); - regulator_disable(qphy->v3p3); - regulator_disable(qphy->vdd); -} - static int phy_8x16_notify_connect(struct usb_phy *phy, enum usb_device_speed speed) { @@ -261,7 +214,6 @@ static void phy_8x16_shutdown(struct usb_phy *phy) static int phy_8x16_read_devicetree(struct phy_8x16 *qphy) { - struct regulator_bulk_data regs[3]; struct device *dev = qphy->phy.dev; int ret; @@ -273,18 +225,15 @@ static int phy_8x16_read_devicetree(struct phy_8x16 *qphy) if (IS_ERR(qphy->iface_clk)) return PTR_ERR(qphy->iface_clk); - regs[0].supply = "v3p3"; - regs[1].supply = "v1p8"; - regs[2].supply = "vddcx"; + qphy->regulator[0].supply = "v3p3"; + qphy->regulator[1].supply = "v1p8"; + qphy->regulator[2].supply = "vddcx"; - ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(regs), regs); + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(qphy->regulator), + qphy->regulator); if (ret) return ret; - qphy->v3p3 = regs[0].consumer; - qphy->v1p8 = regs[1].consumer; - qphy->vdd = regs[2].consumer; - qphy->phy_reset = devm_reset_control_get(dev, "phy"); if (IS_ERR(qphy->phy_reset)) return PTR_ERR(qphy->phy_reset); @@ -364,8 +313,9 @@ static int phy_8x16_probe(struct platform_device *pdev) if (ret < 0) goto off_core; - ret = phy_8x16_regulators_enable(qphy); - if (0 && ret) + ret = regulator_bulk_enable(ARRAY_SIZE(qphy->regulator), + qphy->regulator); + if (WARN_ON(ret)) goto off_clks; qphy->vbus_notify.notifier_call = phy_8x16_vbus_notify; @@ -387,7 +337,7 @@ off_extcon: extcon_unregister_notifier(qphy->vbus_edev, EXTCON_USB, &qphy->vbus_notify); off_power: - phy_8x16_regulators_disable(qphy); + regulator_bulk_disable(ARRAY_SIZE(qphy->regulator), qphy->regulator); off_clks: clk_disable_unprepare(qphy->iface_clk); off_core: @@ -413,7 +363,7 @@ static int phy_8x16_remove(struct platform_device *pdev) clk_disable_unprepare(qphy->iface_clk); clk_disable_unprepare(qphy->core_clk); - phy_8x16_regulators_disable(qphy); + regulator_bulk_disable(ARRAY_SIZE(qphy->regulator), qphy->regulator); return 0; }