mbox series

[RFC,0/8] usb: phy: msm: various cleanups

Message ID 1463606653-325131-1-git-send-email-arnd@arndb.de
Headers show
Series usb: phy: msm: various cleanups | expand

Message

Arnd Bergmann May 18, 2016, 9:24 p.m. UTC
I stumbled over this warning last week, which showed up after I had
removed an incorrect patch from my randconfig build setup:

drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_probe':
drivers/usb/phy/phy-msm-usb.c:1735:14: error: 'regs[0].consumer' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  motg->vddcx = regs[0].consumer;
  ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
drivers/usb/phy/phy-msm-usb.c:1736:14: error: 'regs[1].consumer' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  motg->v3p3  = regs[1].consumer;
  ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
drivers/usb/phy/phy-msm-usb.c:1737:14: error: 'regs[2].consumer' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  motg->v1p8  = regs[2].consumer;
  ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~

Having already fixed the same problem in the phy-qcom-8x16-usb.c
driver before, I tried to do the same thing here, but it turned
out to be somewhat different, and I ended up running into several
unrelated issues in the driver that I now try to fix up.

The series is not tested beyond verifying that it no longer causes
randconfig warnings, and some patches are not entirely obvious.
In particular the ehci-msm and chipidea changes are probably a
good idea, but they actually change the behavior of the drivers
in a way that I cannot verify through inspection alone. The
last patch in the series probably requires some changes to the
devicetree files to go along with it.

Please have a look and test this, provided the patches make sense
conceptually.

	Arnd

Arnd Bergmann (8):
  usb: phy: move msm_hsusb.h into driver
  usb: ehci-msm: call usb_phy_init instead of open-coding it
  usb: chipidea: msm: remove open-coded phy init
  usb: phy: move TCSR driver into new file
  usb: phy: msm: move register definitions into driver
  usb: phy: qcom: use bulk regulator interfaces
  usb: phy: msm: remove v1p8/v3p3 voltage setting
  usb: phy: msm: disable regulator for remove()

 drivers/soc/qcom/Kconfig           |   6 +
 drivers/soc/qcom/Makefile          |   1 +
 drivers/soc/qcom/qcom-tcsr.c       |  57 +++++
 drivers/usb/chipidea/ci_hdrc_msm.c |   6 -
 drivers/usb/host/ehci-msm.c        |  17 +-
 drivers/usb/phy/Kconfig            |   1 +
 drivers/usb/phy/phy-msm-usb.c      | 432 ++++++++++++++++++++++++-------------
 include/linux/soc/qcom/tcsr.h      |  13 ++
 include/linux/usb/msm_hsusb.h      | 200 -----------------
 include/linux/usb/msm_hsusb_hw.h   |  77 -------
 10 files changed, 367 insertions(+), 443 deletions(-)
 create mode 100644 drivers/soc/qcom/qcom-tcsr.c
 create mode 100644 include/linux/soc/qcom/tcsr.h
 delete mode 100644 include/linux/usb/msm_hsusb.h
 delete mode 100644 include/linux/usb/msm_hsusb_hw.h

-- 
2.7.0
Cc: Andy Gross <andy.gross@linaro.org>
Cc: David Brown <david.brown@linaro.org>
Cc: Peter Chen <Peter.Chen@nxp.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Mark Brown <broonie@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-soc@vger.kernel.org
Cc: linux-usb@vger.kernel.org

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Andy Gross May 19, 2016, 7:08 p.m. UTC | #1
On 18 May 2016 at 16:24, Arnd Bergmann <arnd@arndb.de> wrote:

<snip>

> +/*

> + * This abstracts the TCSR register area in Qualcomm SoCs, originally

> + * introduced by Tim Bird as part of the phy-msm-usb.ko device driver,

> + * and split out by Arnd Bergmann into a separate file.

> + *

> + * This file shouldn't really exist, since we have no way to detect

> + * if the TCSR actually exists in the hardcoded location, or if it

> + * is compatible with the version that was originally used.

> + *

> + * If the assumptions ever change, we have to come up with a better

> + * solution.

> + */

> +#include <linux/module.h>

> +#include <linux/io.h>

> +

> +/* USB phy selector - in TCSR address range */

> +#define USB2_PHY_SEL         0xfd4ab000

> +

> +/*

> + * qcom_tcsr_phy_sel -- Select secondary PHY via TCSR

> + *

> + * Select the secondary PHY using the TCSR register, if phy-num=1

> + * in the DTS (or phy_number is set in the platform data).  The

> + * SOC has 2 PHYs which can be used with the OTG port, and this

> + * code allows configuring the correct one.

> + *

> + * Note: This resolves the problem I was seeing where I couldn't

> + * get the USB driver working at all on a dragonboard, from cold

> + * boot.  This patch depends on patch 5/14 from Ivan's msm USB

> + * patch set.  It does not use DT for the register address, as

> + * there's no evidence that this address changes between SoC

> + * versions.

> + *             - Tim

> + */

> +int qcom_tcsr_phy_sel(u32 val)

> +{

> +       void __iomem *phy_select;

> +       int ret;

> +

> +       phy_select = ioremap(USB2_PHY_SEL, 4);

> +

> +       if (!phy_select) {

> +               ret = -ENOMEM;

> +               goto out;

> +       }

> +       /* Enable second PHY with the OTG port */

> +       writel(0x1, phy_select);

> +       ret = 0;

> +out:

> +       iounmap(phy_select);

> +       return ret;

> +}

> +EXPORT_SYMBOL_GPL(qcom_tcsr_phy_sel);


I'd rather do something like what we did for the GSBI.  It needed to
change some phy related bits in the TCSR as well.  We defined the TCSR
as a syscon, with binding documentation under mfd.  If we add a syscon
entry and use it if it is present, we can deal with that pretty
easily.  The offsets change for each soc, and this would also fix that
issue because we can change offset based on tcsr compat.

Regards,

Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown May 20, 2016, 12:44 p.m. UTC | #2
On Fri, May 20, 2016 at 02:24:14PM +0200, Arnd Bergmann wrote:
> On Thursday 19 May 2016 14:08:43 Andy Gross wrote:


> > I'd rather do something like what we did for the GSBI.  It needed to

> > change some phy related bits in the TCSR as well.  We defined the TCSR

> > as a syscon, with binding documentation under mfd.  If we add a syscon

> > entry and use it if it is present, we can deal with that pretty

> > easily.  The offsets change for each soc, and this would also fix that

> > issue because we can change offset based on tcsr compat.


> Works for me, but be aware that this will break the server chips,

> as ACPI has no support for regmap devices.


Just to be clear there's nothing precluding the use of regmap on ACPI
devices, it's syscon it doesn't have anything for.