summaryrefslogtreecommitdiffstats
path: root/drivers/usb/phy
diff options
context:
space:
mode:
authorArnd Bergmann <arnd@arndb.de>2016-03-23 17:29:14 +0100
committerFelipe Balbi <felipe.balbi@linux.intel.com>2016-03-30 13:34:04 +0300
commit7e8ac87a44746b03a37d296fdb3e6b5d96350952 (patch)
tree7fb58720b18cc387c53510f32b67692462df50da /drivers/usb/phy
parent743bc4b069517d3bae69349a1a23511eaaae5ede (diff)
downloadlinux-7e8ac87a44746b03a37d296fdb3e6b5d96350952.tar.bz2
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@arndb.de> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Diffstat (limited to 'drivers/usb/phy')
-rw-r--r--drivers/usb/phy/phy-qcom-8x16-usb.c72
1 files 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;
}