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

Groups > linux.kernel > #1403310 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC 4/8] usb: phy: move TCSR driver into new file Arnd Bergmann <arnd@arndb.de> - 2016-05-18 23:30 +0200
    Re: [RFC 4/8] usb: phy: move TCSR driver into new file Andy Gross <andy.gross@linaro.org> - 2016-05-19 21:10 +0200
      Re: [RFC 4/8] usb: phy: move TCSR driver into new file Arnd Bergmann <arnd@arndb.de> - 2016-05-20 14:30 +0200
        Re: [RFC 4/8] usb: phy: move TCSR driver into new file Mark Brown <broonie@kernel.org> - 2016-05-20 14:50 +0200
          Re: [RFC 4/8] usb: phy: move TCSR driver into new file Arnd Bergmann <arnd@arndb.de> - 2016-05-20 16:20 +0200

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

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

[toc] | [next] | [standalone]

#1403931

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

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

#1404387

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

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

#1404401

[Multipart message — attachments visible in raw view] — view raw

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.

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

#1404484

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web