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

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

[Arnd Bergmann]

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

[RFC 1/8] usb: phy: move msm_hsusb.h into driver

[Arnd Bergmann]

As a preparation for another cleanup, this moves the header file
for the phy-msm-usb driver into the driver itself. No other file
includes it any more, and we don't really want it in the global
namespace anyway.

Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
 drivers/usb/phy/phy-msm-usb.c | 178 ++++++++++++++++++++++++++++++++++++-
 include/linux/usb/msm_hsusb.h | 200 ------------------------------------------
 2 files changed, 177 insertions(+), 201 deletions(-)
 delete mode 100644 include/linux/usb/msm_hsusb.h

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 72b387d592c2..8a34759727bb 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -18,6 +18,7 @@
 
 #include <linux/module.h>
 #include <linux/device.h>
+#include <linux/extcon.h>
 #include <linux/gpio/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
@@ -35,6 +36,8 @@
 #include <linux/of_device.h>
 #include <linux/reboot.h>
 #include <linux/reset.h>
+#include <linux/types.h>
+#include <linux/usb/otg.h>
 
 #include <linux/usb.h>
 #include <linux/usb/otg.h>
@@ -42,10 +45,183 @@
 #include <linux/usb/ulpi.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/hcd.h>
-#include <linux/usb/msm_hsusb.h>
 #include <linux/usb/msm_hsusb_hw.h>
 #include <linux/regulator/consumer.h>
 
+/**
+ * OTG control
+ *
+ * OTG_NO_CONTROL	Id/VBUS notifications not required. Useful in host
+ *                      only configuration.
+ * OTG_PHY_CONTROL	Id/VBUS notifications comes form USB PHY.
+ * OTG_PMIC_CONTROL	Id/VBUS notifications comes from PMIC hardware.
+ * OTG_USER_CONTROL	Id/VBUS notifcations comes from User via sysfs.
+ *
+ */
+enum otg_control_type {
+	OTG_NO_CONTROL = 0,
+	OTG_PHY_CONTROL,
+	OTG_PMIC_CONTROL,
+	OTG_USER_CONTROL,
+};
+
+/**
+ * PHY used in
+ *
+ * INVALID_PHY			Unsupported PHY
+ * CI_45NM_INTEGRATED_PHY	Chipidea 45nm integrated PHY
+ * SNPS_28NM_INTEGRATED_PHY	Synopsis 28nm integrated PHY
+ *
+ */
+enum msm_usb_phy_type {
+	INVALID_PHY = 0,
+	CI_45NM_INTEGRATED_PHY,
+	SNPS_28NM_INTEGRATED_PHY,
+};
+
+#define IDEV_CHG_MAX	1500
+#define IUNIT		100
+
+/**
+ * Different states involved in USB charger detection.
+ *
+ * USB_CHG_STATE_UNDEFINED	USB charger is not connected or detection
+ *                              process is not yet started.
+ * USB_CHG_STATE_WAIT_FOR_DCD	Waiting for Data pins contact.
+ * USB_CHG_STATE_DCD_DONE	Data pin contact is detected.
+ * USB_CHG_STATE_PRIMARY_DONE	Primary detection is completed (Detects
+ *                              between SDP and DCP/CDP).
+ * USB_CHG_STATE_SECONDARY_DONE	Secondary detection is completed (Detects
+ *                              between DCP and CDP).
+ * USB_CHG_STATE_DETECTED	USB charger type is determined.
+ *
+ */
+enum usb_chg_state {
+	USB_CHG_STATE_UNDEFINED = 0,
+	USB_CHG_STATE_WAIT_FOR_DCD,
+	USB_CHG_STATE_DCD_DONE,
+	USB_CHG_STATE_PRIMARY_DONE,
+	USB_CHG_STATE_SECONDARY_DONE,
+	USB_CHG_STATE_DETECTED,
+};
+
+/**
+ * USB charger types
+ *
+ * USB_INVALID_CHARGER	Invalid USB charger.
+ * USB_SDP_CHARGER	Standard downstream port. Refers to a downstream port
+ *                      on USB2.0 compliant host/hub.
+ * USB_DCP_CHARGER	Dedicated charger port (AC charger/ Wall charger).
+ * USB_CDP_CHARGER	Charging downstream port. Enumeration can happen and
+ *                      IDEV_CHG_MAX can be drawn irrespective of USB state.
+ *
+ */
+enum usb_chg_type {
+	USB_INVALID_CHARGER = 0,
+	USB_SDP_CHARGER,
+	USB_DCP_CHARGER,
+	USB_CDP_CHARGER,
+};
+
+/**
+ * struct msm_otg_platform_data - platform device data
+ *              for msm_otg driver.
+ * @phy_init_seq: PHY configuration sequence values. Value of -1 is reserved as
+ *              "do not overwrite default vaule at this address".
+ * @phy_init_sz: PHY configuration sequence size.
+ * @vbus_power: VBUS power on/off routine.
+ * @power_budget: VBUS power budget in mA (0 will be treated as 500mA).
+ * @mode: Supported mode (OTG/peripheral/host).
+ * @otg_control: OTG switch controlled by user/Id pin
+ */
+struct msm_otg_platform_data {
+	int *phy_init_seq;
+	int phy_init_sz;
+	void (*vbus_power)(bool on);
+	unsigned power_budget;
+	enum usb_dr_mode mode;
+	enum otg_control_type otg_control;
+	enum msm_usb_phy_type phy_type;
+	void (*setup_gpio)(enum usb_otg_state state);
+};
+
+/**
+ * struct msm_usb_cable - structure for exteternal connector cable
+ *			  state tracking
+ * @nb: hold event notification callback
+ * @conn: used for notification registration
+ */
+struct msm_usb_cable {
+	struct notifier_block		nb;
+	struct extcon_dev		*extcon;
+};
+
+/**
+ * struct msm_otg: OTG driver data. Shared by HCD and DCD.
+ * @otg: USB OTG Transceiver structure.
+ * @pdata: otg device platform data.
+ * @irq: IRQ number assigned for HSUSB controller.
+ * @clk: clock struct of usb_hs_clk.
+ * @pclk: clock struct of usb_hs_pclk.
+ * @core_clk: clock struct of usb_hs_core_clk.
+ * @regs: ioremapped register base address.
+ * @inputs: OTG state machine inputs(Id, SessValid etc).
+ * @sm_work: OTG state machine work.
+ * @in_lpm: indicates low power mode (LPM) state.
+ * @async_int: Async interrupt arrived.
+ * @cur_power: The amount of mA available from downstream port.
+ * @chg_work: Charger detection work.
+ * @chg_state: The state of charger detection process.
+ * @chg_type: The type of charger attached.
+ * @dcd_retires: The retry count used to track Data contact
+ *               detection process.
+ * @manual_pullup: true if VBUS is not routed to USB controller/phy
+ *	and controller driver therefore enables pull-up explicitly before
+ *	starting controller using usbcmd run/stop bit.
+ * @vbus: VBUS signal state trakining, using extcon framework
+ * @id: ID signal state trakining, using extcon framework
+ * @switch_gpio: Descriptor for GPIO used to control external Dual
+ *               SPDT USB Switch.
+ * @reboot: Used to inform the driver to route USB D+/D- line to Device
+ *	    connector
+ */
+struct msm_otg {
+	struct usb_phy phy;
+	struct msm_otg_platform_data *pdata;
+	int irq;
+	struct clk *clk;
+	struct clk *pclk;
+	struct clk *core_clk;
+	void __iomem *regs;
+#define ID		0
+#define B_SESS_VLD	1
+	unsigned long inputs;
+	struct work_struct sm_work;
+	atomic_t in_lpm;
+	int async_int;
+	unsigned cur_power;
+	int phy_number;
+	struct delayed_work chg_work;
+	enum usb_chg_state chg_state;
+	enum usb_chg_type chg_type;
+	u8 dcd_retries;
+	struct regulator *v3p3;
+	struct regulator *v1p8;
+	struct regulator *vddcx;
+
+	struct reset_control *phy_rst;
+	struct reset_control *link_rst;
+	int vdd_levels[3];
+
+	bool manual_pullup;
+
+	struct msm_usb_cable vbus;
+	struct msm_usb_cable id;
+
+	struct gpio_desc *switch_gpio;
+	struct notifier_block reboot;
+};
+
 #define MSM_USB_BASE	(motg->regs)
 #define DRIVER_NAME	"msm_otg"
 
diff --git a/include/linux/usb/msm_hsusb.h b/include/linux/usb/msm_hsusb.h
deleted file mode 100644
index 8c8f6854c993..000000000000
--- a/include/linux/usb/msm_hsusb.h
+++ /dev/null
@@ -1,200 +0,0 @@
-/* linux/include/asm-arm/arch-msm/hsusb.h
- *
- * Copyright (C) 2008 Google, Inc.
- * Author: Brian Swetland <swetland-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
- * Copyright (c) 2009-2011, Code Aurora Forum. All rights reserved.
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- */
-
-#ifndef __ASM_ARCH_MSM_HSUSB_H
-#define __ASM_ARCH_MSM_HSUSB_H
-
-#include <linux/extcon.h>
-#include <linux/types.h>
-#include <linux/usb/otg.h>
-#include <linux/clk.h>
-
-/**
- * OTG control
- *
- * OTG_NO_CONTROL	Id/VBUS notifications not required. Useful in host
- *                      only configuration.
- * OTG_PHY_CONTROL	Id/VBUS notifications comes form USB PHY.
- * OTG_PMIC_CONTROL	Id/VBUS notifications comes from PMIC hardware.
- * OTG_USER_CONTROL	Id/VBUS notifcations comes from User via sysfs.
- *
- */
-enum otg_control_type {
-	OTG_NO_CONTROL = 0,
-	OTG_PHY_CONTROL,
-	OTG_PMIC_CONTROL,
-	OTG_USER_CONTROL,
-};
-
-/**
- * PHY used in
- *
- * INVALID_PHY			Unsupported PHY
- * CI_45NM_INTEGRATED_PHY	Chipidea 45nm integrated PHY
- * SNPS_28NM_INTEGRATED_PHY	Synopsis 28nm integrated PHY
- *
- */
-enum msm_usb_phy_type {
-	INVALID_PHY = 0,
-	CI_45NM_INTEGRATED_PHY,
-	SNPS_28NM_INTEGRATED_PHY,
-};
-
-#define IDEV_CHG_MAX	1500
-#define IUNIT		100
-
-/**
- * Different states involved in USB charger detection.
- *
- * USB_CHG_STATE_UNDEFINED	USB charger is not connected or detection
- *                              process is not yet started.
- * USB_CHG_STATE_WAIT_FOR_DCD	Waiting for Data pins contact.
- * USB_CHG_STATE_DCD_DONE	Data pin contact is detected.
- * USB_CHG_STATE_PRIMARY_DONE	Primary detection is completed (Detects
- *                              between SDP and DCP/CDP).
- * USB_CHG_STATE_SECONDARY_DONE	Secondary detection is completed (Detects
- *                              between DCP and CDP).
- * USB_CHG_STATE_DETECTED	USB charger type is determined.
- *
- */
-enum usb_chg_state {
-	USB_CHG_STATE_UNDEFINED = 0,
-	USB_CHG_STATE_WAIT_FOR_DCD,
-	USB_CHG_STATE_DCD_DONE,
-	USB_CHG_STATE_PRIMARY_DONE,
-	USB_CHG_STATE_SECONDARY_DONE,
-	USB_CHG_STATE_DETECTED,
-};
-
-/**
- * USB charger types
- *
- * USB_INVALID_CHARGER	Invalid USB charger.
- * USB_SDP_CHARGER	Standard downstream port. Refers to a downstream port
- *                      on USB2.0 compliant host/hub.
- * USB_DCP_CHARGER	Dedicated charger port (AC charger/ Wall charger).
- * USB_CDP_CHARGER	Charging downstream port. Enumeration can happen and
- *                      IDEV_CHG_MAX can be drawn irrespective of USB state.
- *
- */
-enum usb_chg_type {
-	USB_INVALID_CHARGER = 0,
-	USB_SDP_CHARGER,
-	USB_DCP_CHARGER,
-	USB_CDP_CHARGER,
-};
-
-/**
- * struct msm_otg_platform_data - platform device data
- *              for msm_otg driver.
- * @phy_init_seq: PHY configuration sequence values. Value of -1 is reserved as
- *              "do not overwrite default vaule at this address".
- * @phy_init_sz: PHY configuration sequence size.
- * @vbus_power: VBUS power on/off routine.
- * @power_budget: VBUS power budget in mA (0 will be treated as 500mA).
- * @mode: Supported mode (OTG/peripheral/host).
- * @otg_control: OTG switch controlled by user/Id pin
- */
-struct msm_otg_platform_data {
-	int *phy_init_seq;
-	int phy_init_sz;
-	void (*vbus_power)(bool on);
-	unsigned power_budget;
-	enum usb_dr_mode mode;
-	enum otg_control_type otg_control;
-	enum msm_usb_phy_type phy_type;
-	void (*setup_gpio)(enum usb_otg_state state);
-};
-
-/**
- * struct msm_usb_cable - structure for exteternal connector cable
- *			  state tracking
- * @nb: hold event notification callback
- * @conn: used for notification registration
- */
-struct msm_usb_cable {
-	struct notifier_block		nb;
-	struct extcon_dev		*extcon;
-};
-
-/**
- * struct msm_otg: OTG driver data. Shared by HCD and DCD.
- * @otg: USB OTG Transceiver structure.
- * @pdata: otg device platform data.
- * @irq: IRQ number assigned for HSUSB controller.
- * @clk: clock struct of usb_hs_clk.
- * @pclk: clock struct of usb_hs_pclk.
- * @core_clk: clock struct of usb_hs_core_clk.
- * @regs: ioremapped register base address.
- * @inputs: OTG state machine inputs(Id, SessValid etc).
- * @sm_work: OTG state machine work.
- * @in_lpm: indicates low power mode (LPM) state.
- * @async_int: Async interrupt arrived.
- * @cur_power: The amount of mA available from downstream port.
- * @chg_work: Charger detection work.
- * @chg_state: The state of charger detection process.
- * @chg_type: The type of charger attached.
- * @dcd_retires: The retry count used to track Data contact
- *               detection process.
- * @manual_pullup: true if VBUS is not routed to USB controller/phy
- *	and controller driver therefore enables pull-up explicitly before
- *	starting controller using usbcmd run/stop bit.
- * @vbus: VBUS signal state trakining, using extcon framework
- * @id: ID signal state trakining, using extcon framework
- * @switch_gpio: Descriptor for GPIO used to control external Dual
- *               SPDT USB Switch.
- * @reboot: Used to inform the driver to route USB D+/D- line to Device
- *	    connector
- */
-struct msm_otg {
-	struct usb_phy phy;
-	struct msm_otg_platform_data *pdata;
-	int irq;
-	struct clk *clk;
-	struct clk *pclk;
-	struct clk *core_clk;
-	void __iomem *regs;
-#define ID		0
-#define B_SESS_VLD	1
-	unsigned long inputs;
-	struct work_struct sm_work;
-	atomic_t in_lpm;
-	int async_int;
-	unsigned cur_power;
-	int phy_number;
-	struct delayed_work chg_work;
-	enum usb_chg_state chg_state;
-	enum usb_chg_type chg_type;
-	u8 dcd_retries;
-	struct regulator *v3p3;
-	struct regulator *v1p8;
-	struct regulator *vddcx;
-
-	struct reset_control *phy_rst;
-	struct reset_control *link_rst;
-	int vdd_levels[3];
-
-	bool manual_pullup;
-
-	struct msm_usb_cable vbus;
-	struct msm_usb_cable id;
-
-	struct gpio_desc *switch_gpio;
-	struct notifier_block reboot;
-};

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2318 bytes --]


Hi,

Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> writes:
> 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()

I managed to apply a few of these. Some didn't apply. If you can rebase
on my testing/next and fix comments, then I can apply the rest.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

[RFC 2/8] usb: ehci-msm: call usb_phy_init instead of open-coding it

[Arnd Bergmann]

While looking at the phy-msm-usb driver, I noticed that its registers
are also accessed by teh ehci-msm driver, basically duplicating part
of the logic that is already present in the phy driver.

This removes the duplicate code from the ehci driver and instead
calls the usb_phy_init() function.

I did not test this change, so please review the patch carefully
and wait for a positive test result before applying.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/usb/host/ehci-msm.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c
index d3afc89d00f5..4a9f31f77c46 100644
--- a/drivers/usb/host/ehci-msm.c
+++ b/drivers/usb/host/ehci-msm.c
@@ -30,15 +30,12 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/usb/otg.h>
-#include <linux/usb/msm_hsusb_hw.h>
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
 #include <linux/acpi.h>
 
 #include "ehci.h"
 
-#define MSM_USB_BASE (hcd->regs)
-
 #define DRIVER_DESC "Qualcomm On-Chip EHCI Host Controller"
 
 static const char hcd_name[] = "ehci-msm";
@@ -49,24 +46,13 @@ static int ehci_msm_reset(struct usb_hcd *hcd)
 	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
 	int retval;
 
-	ehci->caps = USB_CAPLENGTH;
+	ehci->caps = hcd->regs + 0x100;
 	hcd->has_tt = 1;
 
 	retval = ehci_setup(hcd);
 	if (retval)
 		return retval;
 
-	/* select ULPI phy and clear other status/control bits in PORTSC */
-	writel(PORTSC_PTS_ULPI, USB_PORTSC);
-	/* bursts of unspecified length. */
-	writel(0, USB_AHBBURST);
-	/* Use the AHB transactor, allow posted data writes */
-	writel(0x8, USB_AHBMODE);
-	/* Disable streaming mode and select host mode */
-	writel(0x13, USB_USBMODE);
-	/* Disable ULPI_TX_PKT_EN_CLR_FIX which is valid only for HSIC */
-	writel(readl(USB_GENCONFIG_2) & ~ULPI_TX_PKT_EN_CLR_FIX, USB_GENCONFIG_2);
-
 	return 0;
 }
 
@@ -144,6 +130,7 @@ static int ehci_msm_probe(struct platform_device *pdev)
 		pm_runtime_no_callbacks(&pdev->dev);
 		pm_runtime_enable(&pdev->dev);
 	} else {
+		usb_phy_init(phy);
 		ret = usb_add_hcd(hcd, hcd->irq, IRQF_SHARED);
 		if (ret)
 			goto put_hcd;
-- 
2.7.0

[RFC 3/8] usb: chipidea: msm: remove open-coded phy init

[Arnd Bergmann]

After commit 233c7daf4eec ("usb: chipidea: msm: Initialize PHY on reset
event"), we always call usb_phy_init() when receiving a
CI_HDRC_CONTROLLER_RESET_EVENT event, but we also set the
USB_AHBBURST and USB_AHBMODE registers to the same values that
the reset function does.

This removes the duplicate initialization, to avoid the dependency
on the header file and the phy registers.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/usb/chipidea/ci_hdrc_msm.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c
index 3889809fd0c4..7cae571d15f8 100644
--- a/drivers/usb/chipidea/ci_hdrc_msm.c
+++ b/drivers/usb/chipidea/ci_hdrc_msm.c
@@ -8,15 +8,12 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
-#include <linux/usb/msm_hsusb_hw.h>
 #include <linux/usb/ulpi.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/chipidea.h>
 
 #include "ci.h"
 
-#define MSM_USB_BASE	(ci->hw_bank.abs)
-
 static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
 {
 	struct device *dev = ci->gadget.dev.parent;
@@ -24,9 +21,6 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
 	switch (event) {
 	case CI_HDRC_CONTROLLER_RESET_EVENT:
 		dev_dbg(dev, "CI_HDRC_CONTROLLER_RESET_EVENT received\n");
-		writel(0, USB_AHBBURST);
-		/* use AHB transactor, allow posted data writes */
-		writel(0x8, USB_AHBMODE);
 		usb_phy_init(ci->usb_phy);
 		break;
 	case CI_HDRC_CONTROLLER_STOPPED_EVENT:
-- 
2.7.0

[RFC 4/8] usb: phy: move TCSR driver into new file

[Arnd Bergmann]

The phy-msm-usb driver open-codes access to the Top Control and Status
Register area for setting the phy mode, without any serialization or
checks if these registers are actually present.

This moves the hack to a more prominent location in the hope that it
can eventually get cleaned up.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/soc/qcom/Kconfig         |  6 +++++
 drivers/soc/qcom/Makefile        |  1 +
 drivers/soc/qcom/qcom-tcsr.c     | 57 ++++++++++++++++++++++++++++++++++++++++
 drivers/usb/phy/Kconfig          |  1 +
 drivers/usb/phy/phy-msm-usb.c    | 10 +++----
 include/linux/soc/qcom/tcsr.h    | 13 +++++++++
 include/linux/usb/msm_hsusb_hw.h |  3 ---
 7 files changed, 81 insertions(+), 10 deletions(-)
 create mode 100644 drivers/soc/qcom/qcom-tcsr.c
 create mode 100644 include/linux/soc/qcom/tcsr.h

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 461b387d03cc..37fe433d49db 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -76,3 +76,9 @@ config QCOM_WCNSS_CTRL
 	help
 	  Client driver for the WCNSS_CTRL SMD channel, used to download nv
 	  firmware to a newly booted WCNSS chip.
+
+config QCOM_TCSR
+	bool
+	help
+	  This is a driver for the "Top Control and Status Register" area,
+	  and is used by the usb PHY driver.
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index fdd664edf0bd..672d392aa6c4 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -6,4 +6,5 @@ obj-$(CONFIG_QCOM_SMEM) +=	smem.o
 obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
 obj-$(CONFIG_QCOM_SMP2P)	+= smp2p.o
 obj-$(CONFIG_QCOM_SMSM)	+= smsm.o
+obj-$(CONFIG_QCOM_TCSR) += qcom-tcsr.o
 obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
diff --git a/drivers/soc/qcom/qcom-tcsr.c b/drivers/soc/qcom/qcom-tcsr.c
new file mode 100644
index 000000000000..fdb4ed762292
--- /dev/null
+++ b/drivers/soc/qcom/qcom-tcsr.c
@@ -0,0 +1,57 @@
+/*
+ * 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);
+
+MODULE_AUTHOR("Tim Bird <tbird20d@gmail.com>");
+MODULE_DESCRIPTION("Qualcomm TCSR abstraction");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index c6904742e2aa..5427f48973cf 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -143,6 +143,7 @@ config USB_MSM_OTG
 	depends on RESET_CONTROLLER
 	depends on EXTCON
 	select USB_PHY
+	select QCOM_TCSR if ARCH_QCOM
 	help
 	  Enable this to support the USB OTG transceiver on Qualcomm chips. It
 	  handles PHY initialization, clock management, and workarounds
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 8a34759727bb..f559d73fd7bc 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -31,6 +31,7 @@
 #include <linux/uaccess.h>
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
+#include <linux/soc/qcom/tcsr.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -1820,7 +1821,6 @@ static int msm_otg_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct msm_otg *motg;
 	struct usb_phy *phy;
-	void __iomem *phy_select;
 
 	motg = devm_kzalloc(&pdev->dev, sizeof(struct msm_otg), GFP_KERNEL);
 	if (!motg)
@@ -1882,13 +1882,9 @@ static int msm_otg_probe(struct platform_device *pdev)
 	 * the dwc3 driver does not set this bit in an incompatible way.
 	 */
 	if (motg->phy_number) {
-		phy_select = devm_ioremap_nocache(&pdev->dev, USB2_PHY_SEL, 4);
-		if (!phy_select) {
-			ret = -ENOMEM;
+		ret = qcom_tcsr_phy_sel(0x1);
+		if (ret)
 			goto unregister_extcon;
-		}
-		/* Enable second PHY with the OTG port */
-		writel(0x1, phy_select);
 	}
 
 	dev_info(&pdev->dev, "OTG regs = %p\n", motg->regs);
diff --git a/include/linux/soc/qcom/tcsr.h b/include/linux/soc/qcom/tcsr.h
new file mode 100644
index 000000000000..7332a07806ca
--- /dev/null
+++ b/include/linux/soc/qcom/tcsr.h
@@ -0,0 +1,13 @@
+#ifndef __QCOM_TCSR_H
+#define __QCOM_TCSR_H
+
+#ifdef CONFIG_QCOM_TCSR
+int qcom_tcsr_phy_sel(u32 val);
+#else
+static inline int qcom_tcsr_phy_sel(u32 val)
+{
+	return 0;
+}
+#endif
+
+#endif
diff --git a/include/linux/usb/msm_hsusb_hw.h b/include/linux/usb/msm_hsusb_hw.h
index 974c3796a23f..368e94389bf1 100644
--- a/include/linux/usb/msm_hsusb_hw.h
+++ b/include/linux/usb/msm_hsusb_hw.h
@@ -16,9 +16,6 @@
 #ifndef __LINUX_USB_GADGET_MSM72K_UDC_H__
 #define __LINUX_USB_GADGET_MSM72K_UDC_H__
 
-/* USB phy selector - in TCSR address range */
-#define USB2_PHY_SEL         0xfd4ab000
-
 #define USB_AHBBURST         (MSM_USB_BASE + 0x0090)
 #define USB_AHBMODE          (MSM_USB_BASE + 0x0098)
 #define USB_GENCONFIG_2      (MSM_USB_BASE + 0x00a0)
-- 
2.7.0

Re: [RFC 4/8] usb: phy: move TCSR driver into new file

[Andy Gross]

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

Re: [RFC 4/8] usb: phy: move TCSR driver into new file

[Arnd Bergmann]

On Thursday 19 May 2016 14:08:43 Andy Gross wrote:
> > + *             - 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.

Works for me, but be aware that this will break the server chips,
as ACPI has no support for regmap devices.

I think that's fine, they should really handle this register access
in the firmware anyway rather than relying on a hardcoded MMIO
location.

	Arnd

Re: [RFC 4/8] usb: phy: move TCSR driver into new file

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 780 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [RFC 4/8] usb: phy: move TCSR driver into new file

[Arnd Bergmann]

On Friday 20 May 2016 13:44:14 Mark Brown wrote:
> 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.
> 

Yes, that's what I meant.

	Arnd

[RFC 5/8] usb: phy: msm: move register definitions into driver

[Arnd Bergmann]

The linux/usb/msm_hsusb_hw.h header is now only included
by one file and can be merged into that for simplicity.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/usb/phy/phy-msm-usb.c    | 56 +++++++++++++++++++++++++++++-
 include/linux/usb/msm_hsusb_hw.h | 74 ----------------------------------------
 2 files changed, 55 insertions(+), 75 deletions(-)
 delete mode 100644 include/linux/usb/msm_hsusb_hw.h

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index f559d73fd7bc..ea4ed6c17b00 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -46,9 +46,63 @@
 #include <linux/usb/ulpi.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/hcd.h>
-#include <linux/usb/msm_hsusb_hw.h>
 #include <linux/regulator/consumer.h>
 
+#define USB_AHBBURST         (MSM_USB_BASE + 0x0090)
+#define USB_AHBMODE          (MSM_USB_BASE + 0x0098)
+#define USB_GENCONFIG_2      (MSM_USB_BASE + 0x00a0)
+#define ULPI_TX_PKT_EN_CLR_FIX	BIT(19)
+
+#define USB_CAPLENGTH        (MSM_USB_BASE + 0x0100) /* 8 bit */
+
+#define USB_USBCMD           (MSM_USB_BASE + 0x0140)
+#define USB_PORTSC           (MSM_USB_BASE + 0x0184)
+#define USB_OTGSC            (MSM_USB_BASE + 0x01A4)
+#define USB_USBMODE          (MSM_USB_BASE + 0x01A8)
+#define USB_PHY_CTRL         (MSM_USB_BASE + 0x0240)
+#define USB_PHY_CTRL2        (MSM_USB_BASE + 0x0278)
+
+#define GENCONFIG_2_SESS_VLD_CTRL_EN	BIT(7)
+#define USBCMD_SESS_VLD_CTRL		BIT(25)
+
+#define USBCMD_RESET   2
+#define USB_USBINTR          (MSM_USB_BASE + 0x0148)
+
+#define PORTSC_PHCD            (1 << 23) /* phy suspend mode */
+#define PORTSC_PTS_MASK        (3 << 30)
+#define PORTSC_PTS_ULPI        (2 << 30)
+#define PORTSC_PTS_SERIAL      (3 << 30)
+
+#define USB_ULPI_VIEWPORT    (MSM_USB_BASE + 0x0170)
+#define ULPI_RUN              (1 << 30)
+#define ULPI_WRITE            (1 << 29)
+#define ULPI_READ             (0 << 29)
+#define ULPI_ADDR(n)          (((n) & 255) << 16)
+#define ULPI_DATA(n)          ((n) & 255)
+#define ULPI_DATA_READ(n)     (((n) >> 8) & 255)
+
+/* synopsys 28nm phy registers */
+#define ULPI_PWR_CLK_MNG_REG	0x88
+#define OTG_COMP_DISABLE	BIT(0)
+
+#define ULPI_MISC_A			0x96
+#define ULPI_MISC_A_VBUSVLDEXTSEL	BIT(1)
+#define ULPI_MISC_A_VBUSVLDEXT		BIT(0)
+
+#define ASYNC_INTR_CTRL         (1 << 29) /* Enable async interrupt */
+#define ULPI_STP_CTRL           (1 << 30) /* Block communication with PHY */
+#define PHY_RETEN               (1 << 1) /* PHY retention enable/disable */
+#define PHY_POR_ASSERT		(1 << 0) /* USB2 28nm PHY POR ASSERT */
+
+/* OTG definitions */
+#define OTGSC_INTSTS_MASK	(0x7f << 16)
+#define OTGSC_ID		(1 << 8)
+#define OTGSC_BSV		(1 << 11)
+#define OTGSC_IDIS		(1 << 16)
+#define OTGSC_BSVIS		(1 << 19)
+#define OTGSC_IDIE		(1 << 24)
+#define OTGSC_BSVIE		(1 << 27)
+
 /**
  * OTG control
  *
diff --git a/include/linux/usb/msm_hsusb_hw.h b/include/linux/usb/msm_hsusb_hw.h
deleted file mode 100644
index 368e94389bf1..000000000000
--- a/include/linux/usb/msm_hsusb_hw.h
+++ /dev/null
@@ -1,74 +0,0 @@
-/*
- * Copyright (C) 2007 Google, Inc.
- * Author: Brian Swetland <swetland@google.com>
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- */
-
-#ifndef __LINUX_USB_GADGET_MSM72K_UDC_H__
-#define __LINUX_USB_GADGET_MSM72K_UDC_H__
-
-#define USB_AHBBURST         (MSM_USB_BASE + 0x0090)
-#define USB_AHBMODE          (MSM_USB_BASE + 0x0098)
-#define USB_GENCONFIG_2      (MSM_USB_BASE + 0x00a0)
-#define ULPI_TX_PKT_EN_CLR_FIX	BIT(19)
-
-#define USB_CAPLENGTH        (MSM_USB_BASE + 0x0100) /* 8 bit */
-
-#define USB_USBCMD           (MSM_USB_BASE + 0x0140)
-#define USB_PORTSC           (MSM_USB_BASE + 0x0184)
-#define USB_OTGSC            (MSM_USB_BASE + 0x01A4)
-#define USB_USBMODE          (MSM_USB_BASE + 0x01A8)
-#define USB_PHY_CTRL         (MSM_USB_BASE + 0x0240)
-#define USB_PHY_CTRL2        (MSM_USB_BASE + 0x0278)
-
-#define GENCONFIG_2_SESS_VLD_CTRL_EN	BIT(7)
-#define USBCMD_SESS_VLD_CTRL		BIT(25)
-
-#define USBCMD_RESET   2
-#define USB_USBINTR          (MSM_USB_BASE + 0x0148)
-
-#define PORTSC_PHCD            (1 << 23) /* phy suspend mode */
-#define PORTSC_PTS_MASK        (3 << 30)
-#define PORTSC_PTS_ULPI        (2 << 30)
-#define PORTSC_PTS_SERIAL      (3 << 30)
-
-#define USB_ULPI_VIEWPORT    (MSM_USB_BASE + 0x0170)
-#define ULPI_RUN              (1 << 30)
-#define ULPI_WRITE            (1 << 29)
-#define ULPI_READ             (0 << 29)
-#define ULPI_ADDR(n)          (((n) & 255) << 16)
-#define ULPI_DATA(n)          ((n) & 255)
-#define ULPI_DATA_READ(n)     (((n) >> 8) & 255)
-
-/* synopsys 28nm phy registers */
-#define ULPI_PWR_CLK_MNG_REG	0x88
-#define OTG_COMP_DISABLE	BIT(0)
-
-#define ULPI_MISC_A			0x96
-#define ULPI_MISC_A_VBUSVLDEXTSEL	BIT(1)
-#define ULPI_MISC_A_VBUSVLDEXT		BIT(0)
-
-#define ASYNC_INTR_CTRL         (1 << 29) /* Enable async interrupt */
-#define ULPI_STP_CTRL           (1 << 30) /* Block communication with PHY */
-#define PHY_RETEN               (1 << 1) /* PHY retention enable/disable */
-#define PHY_POR_ASSERT		(1 << 0) /* USB2 28nm PHY POR ASSERT */
-
-/* OTG definitions */
-#define OTGSC_INTSTS_MASK	(0x7f << 16)
-#define OTGSC_ID		(1 << 8)
-#define OTGSC_BSV		(1 << 11)
-#define OTGSC_IDIS		(1 << 16)
-#define OTGSC_BSVIS		(1 << 19)
-#define OTGSC_IDIE		(1 << 24)
-#define OTGSC_BSVIE		(1 << 27)
-
-#endif /* __LINUX_USB_GADGET_MSM72K_UDC_H__ */
-- 
2.7.0

[RFC 6/8] usb: phy: qcom: use bulk regulator interfaces

[Arnd Bergmann]

When build-testing the phy-msm-usb driver, we can run into a gcc warning:

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]
drivers/usb/phy/phy-msm-usb.c:1736:14: error: 'regs[1].consumer' may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/usb/phy/phy-msm-usb.c:1737:14: error: 'regs[2].consumer' may be used uninitialized in this function [-Werror=maybe-uninitialized]

Like in the phy-qcom-8x16-usb.c driver that was reworked in 7e8ac87a4474
("usb: phy: qcom-8x16: fix regulator API abuse"), this warning goes
away once we consistently use the bulk regulator API, which also
simplifies the driver a bit.

This patch should not change the behavior of the driver, other than
how we now first set the voltage on all regulators and then enable
or disable them, rather than doing it one regulator at a time.
In particular, it looks like a mistake that msm_otg_remove() disables
only two of the three regulators that are enabled in msm_otg_probe(),
but changing that is left for another patch.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/usb/phy/phy-msm-usb.c | 155 ++++++++++++++++++------------------------
 1 file changed, 65 insertions(+), 90 deletions(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index ea4ed6c17b00..bdcf049e465f 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -260,9 +260,7 @@ struct msm_otg {
 	enum usb_chg_state chg_state;
 	enum usb_chg_type chg_type;
 	u8 dcd_retries;
-	struct regulator *v3p3;
-	struct regulator *v1p8;
-	struct regulator *vddcx;
+	struct regulator_bulk_data regulators[3];
 
 	struct reset_control *phy_rst;
 	struct reset_control *link_rst;
@@ -303,96 +301,69 @@ enum vdd_levels {
 	VDD_LEVEL_MAX,
 };
 
+enum regulators {
+	VDDCX = 0,
+	V3P3  = 1,
+	V1P8  = 2,
+};
+
 static int msm_hsusb_init_vddcx(struct msm_otg *motg, int init)
 {
 	int ret = 0;
+	struct regulator *vddcx = motg->regulators[VDDCX].consumer;
 
-	if (init) {
-		ret = regulator_set_voltage(motg->vddcx,
+	if (init)
+		ret = regulator_set_voltage(vddcx,
 				motg->vdd_levels[VDD_LEVEL_MIN],
 				motg->vdd_levels[VDD_LEVEL_MAX]);
-		if (ret) {
-			dev_err(motg->phy.dev, "Cannot set vddcx voltage\n");
-			return ret;
-		}
-
-		ret = regulator_enable(motg->vddcx);
-		if (ret)
-			dev_err(motg->phy.dev, "unable to enable hsusb vddcx\n");
-	} else {
-		ret = regulator_set_voltage(motg->vddcx, 0,
+	else
+		ret = regulator_set_voltage(vddcx, 0,
 				motg->vdd_levels[VDD_LEVEL_MAX]);
-		if (ret)
-			dev_err(motg->phy.dev, "Cannot set vddcx voltage\n");
-		ret = regulator_disable(motg->vddcx);
-		if (ret)
-			dev_err(motg->phy.dev, "unable to disable hsusb vddcx\n");
-	}
+
+
+	if (ret)
+		dev_err(motg->phy.dev, "Cannot set vddcx voltage\n");
 
 	return ret;
 }
 
-static int msm_hsusb_ldo_init(struct msm_otg *motg, int init)
+static int msm_hsusb_ldo_init(struct msm_otg *motg)
 {
 	int rc = 0;
 
-	if (init) {
-		rc = regulator_set_voltage(motg->v3p3, USB_PHY_3P3_VOL_MIN,
-				USB_PHY_3P3_VOL_MAX);
-		if (rc) {
-			dev_err(motg->phy.dev, "Cannot set v3p3 voltage\n");
-			goto exit;
-		}
-		rc = regulator_enable(motg->v3p3);
-		if (rc) {
-			dev_err(motg->phy.dev, "unable to enable the hsusb 3p3\n");
-			goto exit;
-		}
-		rc = regulator_set_voltage(motg->v1p8, USB_PHY_1P8_VOL_MIN,
-				USB_PHY_1P8_VOL_MAX);
-		if (rc) {
-			dev_err(motg->phy.dev, "Cannot set v1p8 voltage\n");
-			goto disable_3p3;
-		}
-		rc = regulator_enable(motg->v1p8);
-		if (rc) {
-			dev_err(motg->phy.dev, "unable to enable the hsusb 1p8\n");
-			goto disable_3p3;
-		}
-
-		return 0;
+	rc = regulator_set_voltage(motg->regulators[V3P3].consumer,
+			USB_PHY_3P3_VOL_MIN, USB_PHY_3P3_VOL_MAX);
+	if (rc) {
+		dev_err(motg->phy.dev, "Cannot set v3p3 voltage\n");
+		goto exit;
+	}
+	rc = regulator_set_voltage(motg->regulators[V1P8].consumer,
+			USB_PHY_1P8_VOL_MIN, USB_PHY_1P8_VOL_MAX);
+	if (rc) {
+		dev_err(motg->phy.dev, "Cannot set v1p8 voltage\n");
 	}
-
-	regulator_disable(motg->v1p8);
-disable_3p3:
-	regulator_disable(motg->v3p3);
 exit:
 	return rc;
 }
 
 static int msm_hsusb_ldo_set_mode(struct msm_otg *motg, int on)
 {
-	int ret = 0;
+	int ret;
+	int load_v1p8 = on ? USB_PHY_1P8_HPM_LOAD : USB_PHY_1P8_LPM_LOAD;
+	int load_v3p3 = on ? USB_PHY_3P3_HPM_LOAD : USB_PHY_3P3_LPM_LOAD;
+	const char *state = on ? "HPM" : "LPM";
 
-	if (on) {
-		ret = regulator_set_load(motg->v1p8, USB_PHY_1P8_HPM_LOAD);
-		if (ret < 0) {
-			pr_err("Could not set HPM for v1p8\n");
-			return ret;
-		}
-		ret = regulator_set_load(motg->v3p3, USB_PHY_3P3_HPM_LOAD);
-		if (ret < 0) {
-			pr_err("Could not set HPM for v3p3\n");
-			regulator_set_load(motg->v1p8, USB_PHY_1P8_LPM_LOAD);
-			return ret;
-		}
-	} else {
-		ret = regulator_set_load(motg->v1p8, USB_PHY_1P8_LPM_LOAD);
-		if (ret < 0)
-			pr_err("Could not set LPM for v1p8\n");
-		ret = regulator_set_load(motg->v3p3, USB_PHY_3P3_LPM_LOAD);
-		if (ret < 0)
-			pr_err("Could not set LPM for v3p3\n");
+	ret = regulator_set_load(motg->regulators[V1P8].consumer, load_v1p8);
+	if (ret < 0) {
+		pr_err("Could not set %s for v1p8\n", state);
+		return ret;
+	}
+	ret = regulator_set_load(motg->regulators[V3P3].consumer, load_v3p3);
+	if (ret < 0) {
+		pr_err("Could not set %s for v3p3\n", state);
+		regulator_set_load(motg->regulators[V1P8].consumer,
+				   USB_PHY_1P8_LPM_LOAD);
+		return ret;
 	}
 
 	pr_debug("reg (%s)\n", on ? "HPM" : "LPM");
@@ -701,7 +672,8 @@ static int msm_hsusb_config_vddcx(struct msm_otg *motg, int high)
 	else
 		min_vol = motg->vdd_levels[VDD_LEVEL_NONE];
 
-	ret = regulator_set_voltage(motg->vddcx, min_vol, max_vol);
+	ret = regulator_set_voltage(motg->regulators[VDDCX].consumer,
+				    min_vol, max_vol);
 	if (ret) {
 		pr_err("Cannot set vddcx voltage\n");
 		return ret;
@@ -1868,7 +1840,6 @@ static int msm_otg_reboot_notify(struct notifier_block *this,
 
 static int msm_otg_probe(struct platform_device *pdev)
 {
-	struct regulator_bulk_data regs[3];
 	int ret = 0;
 	struct device_node *np = pdev->dev.of_node;
 	struct msm_otg_platform_data *pdata;
@@ -1950,18 +1921,16 @@ static int msm_otg_probe(struct platform_device *pdev)
 		goto unregister_extcon;
 	}
 
-	regs[0].supply = "vddcx";
-	regs[1].supply = "v3p3";
-	regs[2].supply = "v1p8";
+	motg->regulators[VDDCX].supply = "vddcx";
+	motg->regulators[V3P3].supply  = "v3p3";
+	motg->regulators[V1P8].supply  = "v1p8";
 
-	ret = devm_regulator_bulk_get(motg->phy.dev, ARRAY_SIZE(regs), regs);
+	ret = devm_regulator_bulk_get(motg->phy.dev,
+				      ARRAY_SIZE(motg->regulators),
+				      motg->regulators);
 	if (ret)
 		goto unregister_extcon;
 
-	motg->vddcx = regs[0].consumer;
-	motg->v3p3  = regs[1].consumer;
-	motg->v1p8  = regs[2].consumer;
-
 	clk_set_rate(motg->clk, 60000000);
 
 	clk_prepare_enable(motg->clk);
@@ -1976,15 +1945,21 @@ static int msm_otg_probe(struct platform_device *pdev)
 		goto disable_clks;
 	}
 
-	ret = msm_hsusb_ldo_init(motg, 1);
+	ret = msm_hsusb_ldo_init(motg);
 	if (ret) {
 		dev_err(&pdev->dev, "hsusb vreg configuration failed\n");
-		goto disable_vddcx;
+		goto disable_clks;
+	}
+	ret = regulator_bulk_enable(ARRAY_SIZE(motg->regulators), motg->regulators);
+	if (ret)
+		{
+		dev_err(&pdev->dev, "hsusb could not enable regulators\n");
+		goto disable_clks;
 	}
 	ret = msm_hsusb_ldo_set_mode(motg, 1);
 	if (ret) {
 		dev_err(&pdev->dev, "hsusb vreg enable failed\n");
-		goto disable_ldo;
+		goto disable_regulators;
 	}
 
 	writel(0, USB_USBINTR);
@@ -1996,7 +1971,7 @@ static int msm_otg_probe(struct platform_device *pdev)
 					"msm_otg", motg);
 	if (ret) {
 		dev_err(&pdev->dev, "request irq failed\n");
-		goto disable_ldo;
+		goto disable_regulators;
 	}
 
 	phy->init = msm_phy_init;
@@ -2015,7 +1990,7 @@ static int msm_otg_probe(struct platform_device *pdev)
 	ret = usb_add_phy_dev(&motg->phy);
 	if (ret) {
 		dev_err(&pdev->dev, "usb_add_phy failed\n");
-		goto disable_ldo;
+		goto disable_regulators;
 	}
 
 	platform_set_drvdata(pdev, motg);
@@ -2044,10 +2019,9 @@ static int msm_otg_probe(struct platform_device *pdev)
 
 	return 0;
 
-disable_ldo:
-	msm_hsusb_ldo_init(motg, 0);
-disable_vddcx:
+disable_regulators:
 	msm_hsusb_init_vddcx(motg, 0);
+	regulator_bulk_disable(ARRAY_SIZE(motg->regulators), motg->regulators);
 disable_clks:
 	clk_disable_unprepare(motg->pclk);
 	clk_disable_unprepare(motg->clk);
@@ -2114,7 +2088,8 @@ static int msm_otg_remove(struct platform_device *pdev)
 	clk_disable_unprepare(motg->clk);
 	if (!IS_ERR(motg->core_clk))
 		clk_disable_unprepare(motg->core_clk);
-	msm_hsusb_ldo_init(motg, 0);
+	/* vddcx is left active */
+	regulator_bulk_disable(2, &motg->regulators[V3P3]);
 
 	pm_runtime_set_suspended(&pdev->dev);
 
-- 
2.7.0

[RFC 7/8] usb: phy: msm: remove v1p8/v3p3 voltage setting

[Arnd Bergmann]

As pointed out by Mark Brown, regulators that are always set
to a fixed voltage range should be initialized from the values
in the device tree rather than having client drivers set the
voltage manually.

This removes the msm_hsusb_ldo_init() function that adapts
the voltage on the fixed 1.8v and 3.3v regulators in the
MSM usb phy driver. For consistency, the msm_hsusb_init_vddcx()
is also removed, and instead we just set the voltages
directly where needed, reducing the code size further and
making it more obvious what is actually going on here.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/usb/phy/phy-msm-usb.c | 100 +++++++++---------------------------------
 1 file changed, 21 insertions(+), 79 deletions(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index bdcf049e465f..570fd3433937 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -281,13 +281,9 @@ struct msm_otg {
 #define ULPI_IO_TIMEOUT_USEC	(10 * 1000)
 #define LINK_RESET_TIMEOUT_USEC	(250 * 1000)
 
-#define USB_PHY_3P3_VOL_MIN	3050000 /* uV */
-#define USB_PHY_3P3_VOL_MAX	3300000 /* uV */
 #define USB_PHY_3P3_HPM_LOAD	50000	/* uA */
 #define USB_PHY_3P3_LPM_LOAD	4000	/* uA */
 
-#define USB_PHY_1P8_VOL_MIN	1800000 /* uV */
-#define USB_PHY_1P8_VOL_MAX	1800000 /* uV */
 #define USB_PHY_1P8_HPM_LOAD	50000	/* uA */
 #define USB_PHY_1P8_LPM_LOAD	4000	/* uA */
 
@@ -307,51 +303,18 @@ enum regulators {
 	V1P8  = 2,
 };
 
-static int msm_hsusb_init_vddcx(struct msm_otg *motg, int init)
-{
-	int ret = 0;
-	struct regulator *vddcx = motg->regulators[VDDCX].consumer;
-
-	if (init)
-		ret = regulator_set_voltage(vddcx,
-				motg->vdd_levels[VDD_LEVEL_MIN],
-				motg->vdd_levels[VDD_LEVEL_MAX]);
-	else
-		ret = regulator_set_voltage(vddcx, 0,
-				motg->vdd_levels[VDD_LEVEL_MAX]);
-
-
-	if (ret)
-		dev_err(motg->phy.dev, "Cannot set vddcx voltage\n");
-
-	return ret;
-}
-
-static int msm_hsusb_ldo_init(struct msm_otg *motg)
-{
-	int rc = 0;
-
-	rc = regulator_set_voltage(motg->regulators[V3P3].consumer,
-			USB_PHY_3P3_VOL_MIN, USB_PHY_3P3_VOL_MAX);
-	if (rc) {
-		dev_err(motg->phy.dev, "Cannot set v3p3 voltage\n");
-		goto exit;
-	}
-	rc = regulator_set_voltage(motg->regulators[V1P8].consumer,
-			USB_PHY_1P8_VOL_MIN, USB_PHY_1P8_VOL_MAX);
-	if (rc) {
-		dev_err(motg->phy.dev, "Cannot set v1p8 voltage\n");
-	}
-exit:
-	return rc;
-}
-
 static int msm_hsusb_ldo_set_mode(struct msm_otg *motg, int on)
 {
+	int load_v1p8 = USB_PHY_1P8_LPM_LOAD;
+	int load_v3p3 = USB_PHY_3P3_LPM_LOAD;
+	const char *state = "LPM";
 	int ret;
-	int load_v1p8 = on ? USB_PHY_1P8_HPM_LOAD : USB_PHY_1P8_LPM_LOAD;
-	int load_v3p3 = on ? USB_PHY_3P3_HPM_LOAD : USB_PHY_3P3_LPM_LOAD;
-	const char *state = on ? "HPM" : "LPM";
+
+	if (on) {
+		load_v1p8 = USB_PHY_1P8_HPM_LOAD;
+		load_v3p3 = USB_PHY_3P3_HPM_LOAD;
+		state = "HPM";
+	}
 
 	ret = regulator_set_load(motg->regulators[V1P8].consumer, load_v1p8);
 	if (ret < 0) {
@@ -366,7 +329,7 @@ static int msm_hsusb_ldo_set_mode(struct msm_otg *motg, int on)
 		return ret;
 	}
 
-	pr_debug("reg (%s)\n", on ? "HPM" : "LPM");
+	pr_debug("reg (%s)\n", state);
 	return ret < 0 ? ret : 0;
 }
 
@@ -661,29 +624,6 @@ static int msm_phy_init(struct usb_phy *phy)
 
 #ifdef CONFIG_PM
 
-static int msm_hsusb_config_vddcx(struct msm_otg *motg, int high)
-{
-	int max_vol = motg->vdd_levels[VDD_LEVEL_MAX];
-	int min_vol;
-	int ret;
-
-	if (high)
-		min_vol = motg->vdd_levels[VDD_LEVEL_MIN];
-	else
-		min_vol = motg->vdd_levels[VDD_LEVEL_NONE];
-
-	ret = regulator_set_voltage(motg->regulators[VDDCX].consumer,
-				    min_vol, max_vol);
-	if (ret) {
-		pr_err("Cannot set vddcx voltage\n");
-		return ret;
-	}
-
-	pr_debug("%s: min_vol:%d max_vol:%d\n", __func__, min_vol, max_vol);
-
-	return ret;
-}
-
 static int msm_otg_suspend(struct msm_otg *motg)
 {
 	struct usb_phy *phy = &motg->phy;
@@ -765,7 +705,9 @@ static int msm_otg_suspend(struct msm_otg *motg)
 	if (motg->pdata->phy_type == SNPS_28NM_INTEGRATED_PHY &&
 			motg->pdata->otg_control == OTG_PMIC_CONTROL) {
 		msm_hsusb_ldo_set_mode(motg, 0);
-		msm_hsusb_config_vddcx(motg, 0);
+		regulator_set_voltage(motg->regulators[VDDCX].consumer,
+				      motg->vdd_levels[VDD_LEVEL_NONE],
+				      motg->vdd_levels[VDD_LEVEL_MAX]);
 	}
 
 	if (device_may_wakeup(phy->dev))
@@ -805,7 +747,9 @@ static int msm_otg_resume(struct msm_otg *motg)
 			addr = USB_PHY_CTRL2;
 
 		msm_hsusb_ldo_set_mode(motg, 1);
-		msm_hsusb_config_vddcx(motg, 1);
+		regulator_set_voltage(motg->regulators[VDDCX].consumer,
+				      motg->vdd_levels[VDD_LEVEL_MIN],
+				      motg->vdd_levels[VDD_LEVEL_MAX]);
 		writel(readl(addr) & ~PHY_RETEN, addr);
 	}
 
@@ -1939,17 +1883,14 @@ static int msm_otg_probe(struct platform_device *pdev)
 	if (!IS_ERR(motg->core_clk))
 		clk_prepare_enable(motg->core_clk);
 
-	ret = msm_hsusb_init_vddcx(motg, 1);
+	ret = regulator_set_voltage(motg->regulators[VDDCX].consumer,
+				    motg->vdd_levels[VDD_LEVEL_MIN],
+				    motg->vdd_levels[VDD_LEVEL_MAX]);
 	if (ret) {
 		dev_err(&pdev->dev, "hsusb vddcx configuration failed\n");
 		goto disable_clks;
 	}
 
-	ret = msm_hsusb_ldo_init(motg);
-	if (ret) {
-		dev_err(&pdev->dev, "hsusb vreg configuration failed\n");
-		goto disable_clks;
-	}
 	ret = regulator_bulk_enable(ARRAY_SIZE(motg->regulators), motg->regulators);
 	if (ret)
 		{
@@ -2020,7 +1961,8 @@ static int msm_otg_probe(struct platform_device *pdev)
 	return 0;
 
 disable_regulators:
-	msm_hsusb_init_vddcx(motg, 0);
+	regulator_set_voltage(motg->regulators[VDDCX].consumer, 0,
+			      motg->vdd_levels[VDD_LEVEL_MAX]);
 	regulator_bulk_disable(ARRAY_SIZE(motg->regulators), motg->regulators);
 disable_clks:
 	clk_disable_unprepare(motg->pclk);
-- 
2.7.0

[RFC 8/8] usb: phy: msm: disable regulator for remove()

[Arnd Bergmann]

It seems odd that this driver explicitly enables the vddcx regulator
on probe along with the other regulators, but doesn't disable it
again when the device is removed.

This changes the remove callback to handle all three regulators the
same way and disable them in the end.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/usb/phy/phy-msm-usb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 570fd3433937..48fe48e5bf6d 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -2030,8 +2030,7 @@ static int msm_otg_remove(struct platform_device *pdev)
 	clk_disable_unprepare(motg->clk);
 	if (!IS_ERR(motg->core_clk))
 		clk_disable_unprepare(motg->core_clk);
-	/* vddcx is left active */
-	regulator_bulk_disable(2, &motg->regulators[V3P3]);
+	regulator_bulk_disable(ARRAY_SIZE(motg->regulators), motg->regulators);
 
 	pm_runtime_set_suspended(&pdev->dev);
 
-- 
2.7.0

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

[Andy Gross]

On 19 May 2016 at 01:52, Peter Chen <hzpeterchen@gmail.com> wrote:
>
>
> On Thu, May 19, 2016 at 5:24 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>>
>> 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.
>>
>>
>
>
> MSM USB driver uses phy driver for dual-role (otg) switch, chipidea
> driver for peripheral function, a dedicated ehci file for host driver,
> so these three driver will access the same controller register space,
> it is not a good practice.

Agreed.  It's a huge mess.  And it is currently being reworked.

> In fact, MSM USB2 can use chipidea driver as controller driver, and its
> phy driver dedicate for USB PHY function with some changes.

Yeah I noticed with the addition of the host controller we can do this, which
was the impetus for reworking.  In the end, we'll have just the qcom-ci and
a phy.


Regards,
Andy