| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1458750566-1915909-1-git-send-email-arnd@arndb.de>
Date: Wed, 23 Mar 2016 17:29:14 +0100
From: Arnd Bergmann <arnd@...db.de>
To: Felipe Balbi <balbi@...nel.org>
Cc: Mark Brown <broonie@...nel.org>, Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Ivan T. Ivanov" <ivan.ivanov@...aro.org>,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] usb-phy: qcom-8x16: fix regulator API abuse
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 <arnd@...db.de>
---
drivers/usb/phy/phy-qcom-8x16-usb.c | 72 ++++++-------------------------------
1 file changed, 11 insertions(+), 61 deletions(-)
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;
}
--
2.7.0
Powered by blists - more mailing lists