Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1403311 > unrolled thread

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

Started byArnd Bergmann <arnd@arndb.de>
First post2016-05-18 23:30 +0200
Last post2016-05-19 17:30 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [RFC 0/8] usb: phy: msm: various cleanups Arnd Bergmann <arnd@arndb.de> - 2016-05-18 23:30 +0200
    [RFC 5/8] usb: phy: msm: move register definitions into driver Arnd Bergmann <arnd@arndb.de> - 2016-05-18 23:30 +0200
    Re: [RFC 0/8] usb: phy: msm: various cleanups Andy Gross <andy.gross@linaro.org> - 2016-05-19 17:30 +0200

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

FromArnd Bergmann <arnd@arndb.de>
Date2016-05-18 23:30 +0200
Subject[RFC 0/8] usb: phy: msm: various cleanups
Message-ID<rAhct-7rh-3@gated-at.bofh.it>
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

[toc] | [next] | [standalone]


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

FromArnd Bergmann <arnd@arndb.de>
Date2016-05-18 23:30 +0200
Subject[RFC 5/8] usb: phy: msm: move register definitions into driver
Message-ID<rAhcu-7rh-35@gated-at.bofh.it>
In reply to#1403311
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

[toc] | [prev] | [next] | [standalone]


#1403813

FromAndy Gross <andy.gross@linaro.org>
Date2016-05-19 17:30 +0200
Message-ID<rAy3D-1w7-17@gated-at.bofh.it>
In reply to#1403311
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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web