Skip to contentSkip to conversation

usb/phy add sprd ums512 usbphy

RFC PATCH In review Cixi Geng 3 participants 6 messages Mar 12, 2023 – Mar 21, 2023

Subsystems (2)
lore
Share & tools
lore

Cixi Geng

author
#0 lore raw

RFC PATCH 1/1 usb/phy add sprd ums512 usbphy

From: Cixi Geng <[email protected]>

This driver is support USB2 phy for Spreadtrum UMS512 SOC's,

---
 drivers/usb/phy/Kconfig           |  10 +
 drivers/usb/phy/Makefile          |   1 +
 drivers/usb/phy/phy-sprd-ums512.c | 511 ++++++++++++++++++++++++++++++
 drivers/usb/phy/phy-sprd-ums512.h |  39 +++
 4 files changed, 561 insertions(+)
 create mode 100644 drivers/usb/phy/phy-sprd-ums512.c
 create mode 100644 drivers/usb/phy/phy-sprd-ums512.h
drivers/usb/phy/Kconfig +10 -0
index 5f629d7cad64..fa5564e6f3a3 100644
@@ -158,6 +158,16 @@ config USB_TEGRA_PHY
 	  This driver provides PHY support for the USB controllers found
 	  on NVIDIA Tegra SoC's.
 
+config USB_SPRD_UMS512_PHY
+	tristate "Spreadtrum ums512 USB2 PHY Driver"
+	depends on ARCH_SPRD || COMPILE_TEST
+	select USB_PHY
+	select EXTCON_USB_GPIO
+	help
+	  Enable this to support the SPRD ums512 USB2 PHY that is part of SOC.
+	  This driver takes care of all the PHY functionality, normally paired with
+	  DesignWare USB20 (DRD) Controller.
+
 config USB_ULPI
 	bool "Generic ULPI Transceiver Driver"
 	depends on ARM || ARM64 || COMPILE_TEST
drivers/usb/phy/Makefile +1 -0
index e5d619b4d8f6..ce45ee0f12a8 100644
@@ -22,4 +22,5 @@ obj-$(CONFIG_USB_MV_OTG)		+= phy-mv-usb.o
 obj-$(CONFIG_USB_MXS_PHY)		+= phy-mxs-usb.o
 obj-$(CONFIG_USB_ULPI)			+= phy-ulpi.o
 obj-$(CONFIG_USB_ULPI_VIEWPORT)		+= phy-ulpi-viewport.o
+obj-$(CONFIG_USB_SPRD_UMS512_PHY)	+= phy-sprd-ums512.o
 obj-$(CONFIG_KEYSTONE_USB_PHY)		+= phy-keystone.o
drivers/usb/phy/phy-sprd-ums512.c new file +511 -0

511 lines not on this page · raw message

drivers/usb/phy/phy-sprd-ums512.h new file +39 -0
new file mode 100644
index 000000000000..903da0573eae
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0+ OR MIT */
+/*
+ * Spreadtrum UMS512 SOC USB registers file
+ *
+ * Copyright C 2022, Spreadtrum Communications Inc.
+ */
+
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DATABUS16_8			0x10000000
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DMPULLDOWN			0x8
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DPPULLDOWN			0x10
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW_EN			0x1
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_L			0x8
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_S			0x10
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_RESERVED			0xffff
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TFREGRES			0x1f80000
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TUNEHSAMP			0x6000000
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT			0x10000
+#define MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DMPULLDOWN		0x2
+#define MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DPPULLDOWN		0x4
+#define MASK_AON_APB_ANA_EB						0x1000
+#define MASK_AON_APB_CGM_DPHY_REF_EN					0x400
+#define MASK_AON_APB_CGM_OTG_REF_EN					0x1000
+#define MASK_AON_APB_OTG_PHY_SOFT_RST					0x200
+#define MASK_AON_APB_OTG_UTMI_EB					0x100
+#define MASK_AON_APB_OTG_UTMI_SOFT_RST					0x100
+#define MASK_AON_APB_OTG_VBUS_VALID_PHYREG				0x1000000
+#define MASK_AON_APB_USB2_PHY_IDDIG					0x8
+#define MASK_AON_APB_UTMI_WIDTH_SEL					0x40000000
+#define REG_ANLG_PHY_G2_ANALOG_USB20_USB20_BATTER_PLL			0x005c
+#define REG_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW			0x0070
+#define REG_ANLG_PHY_G2_ANALOG_USB20_USB20_TRIMMING			0x0064
+#define REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1			0x0058
+#define REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL2			0x0060
+#define REG_ANLG_PHY_G2_ANALOG_USB20_REG_SEL_CFG_0			0x0074
+#define REG_AON_APB_APB_EB1						0x0004
+#define REG_AON_APB_APB_RST1						0x0010
+#define REG_AON_APB_CGM_REG1						0x0138
+#define REG_AON_APB_OTG_PHY_CTRL					0x0208
+#define REG_AON_APB_OTG_PHY_TEST					0x0204
Signature · 1 line
2.34.1
maintainer
#1 lore raw
Quoted 5 lines · Cixi Geng

From: Cixi Geng <[email protected]>

This driver is support USB2 phy for Spreadtrum UMS512 SOC's,

Why is this "RFC"? What is left to do on it to be accepted?

Quoted 5 lines
---
 drivers/usb/phy/Kconfig           |  10 +
 drivers/usb/phy/Makefile          |   1 +
 drivers/usb/phy/phy-sprd-ums512.c | 511 ++++++++++++++++++++++++++++++
 drivers/usb/phy/phy-sprd-ums512.h |  39 +++

Why do you need a .h file for a single .c file? Please just put them all together into one file.

thanks,

greg k-h

author
#2 lore raw
Quoted 8 lines · Greg KH
Quoted 5 lines · Cixi Geng

From: Cixi Geng <[email protected]>

This driver is support USB2 phy for Spreadtrum UMS512 SOC's,

Why is this "RFC"?  What is left to do on it to be accepted?

this is just the driver code, lack of dt-bindings

Quoted 14 lines
Quoted 7 lines

---
 drivers/usb/phy/Kconfig           |  10 +
 drivers/usb/phy/Makefile          |   1 +
 drivers/usb/phy/phy-sprd-ums512.c | 511
++++++++++++++++++++++++++++++
 drivers/usb/phy/phy-sprd-ums512.h |  39 +++

Why do you need a .h file for a single .c file?  Please just put them all together into one file.

thanks,

greg k-h

#3 lore raw
In reply to Cixi Geng
Quoted 120 lines · Cixi Geng

From: Cixi Geng <[email protected]>

This driver is support USB2 phy for Spreadtrum UMS512 SOC's,

---
  drivers/usb/phy/Kconfig           |  10 +
  drivers/usb/phy/Makefile          |   1 +
  drivers/usb/phy/phy-sprd-ums512.c | 511 ++++++++++++++++++++++++++++++
  drivers/usb/phy/phy-sprd-ums512.h |  39 +++
  4 files changed, 561 insertions(+)

create mode 100644 drivers/usb/phy/phy-sprd-ums512.c create mode 100644 drivers/usb/phy/phy-sprd-ums512.h

drivers/usb/phy/Kconfig +10 -0
index 5f629d7cad64..fa5564e6f3a3 100644
@@ -158,6 +158,16 @@ config USB_TEGRA_PHY
  	  This driver provides PHY support for the USB controllers found
  	  on NVIDIA Tegra SoC's.
 
+config USB_SPRD_UMS512_PHY
+	tristate "Spreadtrum ums512 USB2 PHY Driver"
+	depends on ARCH_SPRD || COMPILE_TEST
+	select USB_PHY
+	select EXTCON_USB_GPIO
+	help
+	  Enable this to support the SPRD ums512 USB2 PHY that is part of SOC.
+	  This driver takes care of all the PHY functionality, normally paired with
+	  DesignWare USB20 (DRD) Controller.
+
  config USB_ULPI
  	bool "Generic ULPI Transceiver Driver"
  	depends on ARM || ARM64 || COMPILE_TEST
drivers/usb/phy/Makefile +1 -0
index e5d619b4d8f6..ce45ee0f12a8 100644
@@ -22,4 +22,5 @@ obj-$(CONFIG_USB_MV_OTG)		+= phy-mv-usb.o
  obj-$(CONFIG_USB_MXS_PHY)		+= phy-mxs-usb.o
  obj-$(CONFIG_USB_ULPI)			+= phy-ulpi.o
  obj-$(CONFIG_USB_ULPI_VIEWPORT)		+= phy-ulpi-viewport.o
+obj-$(CONFIG_USB_SPRD_UMS512_PHY)	+= phy-sprd-ums512.o
  obj-$(CONFIG_KEYSTONE_USB_PHY)		+= phy-keystone.o
drivers/usb/phy/phy-sprd-ums512.c new file +69 -0
new file mode 100644
index 000000000000..025f45e8d509
@@ -0,0 +1,511 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Unisoc USB PHY driver
+ *
+ * Copyright (C) 2023 Unisoc Inc.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+#include <linux/usb/otg.h>
+#include <linux/usb/phy.h>
+#include <uapi/linux/usb/charger.h>
+
+#include "phy-sprd-ums512.h"
+
+struct sprd_hsphy {
+	struct device		*dev;
+	struct usb_phy		phy;
+	struct regulator	*vdd;
+	struct regmap		*hsphy_glb;
+	struct regmap		*ana_g2;
+	struct regmap		*pmic;
+	u32			vdd_vol;
+	atomic_t		reset;
+	atomic_t		inited;
+	bool			is_host;
+};
+
+#define TUNEHSAMP_2_6MA		(3 << 25)
+#define TFREGRES_TUNE_VALUE	(0x14 << 19)
+#define SC2730_CHARGE_STATUS	0x1b9c
+#define BIT_CHG_DET_DONE	BIT(11)
+#define BIT_SDP_INT		BIT(7)
+#define BIT_DCP_INT		BIT(6)
+#define BIT_CDP_INT		BIT(5)
+
+static enum usb_charger_type sc27xx_charger_detect(struct regmap *regmap)
+{
+	enum usb_charger_type type;
+	u32 status = 0, val;
+	int ret, cnt = 10;
+
+	do {
+		ret = regmap_read(regmap, SC2730_CHARGE_STATUS, &val);
+		if (ret)
+			return UNKNOWN_TYPE;
+
+		if (val & BIT_CHG_DET_DONE) {
+			status = val & (BIT_CDP_INT | BIT_DCP_INT | BIT_SDP_INT);
+			break;
+		}
+
+		msleep(200);
+	} while (--cnt > 0);
+
+	switch (status) {
+	case BIT_CDP_INT:
+		type = CDP_TYPE;
+		break;
+	case BIT_DCP_INT:
+		type = DCP_TYPE;

drivers/usb/phy/phy-sprd-ums512.c line 79

Quoted 10 lines
drivers/usb/phy/phy-sprd-ums512.c new file +10 -0
+		break;
+	case BIT_SDP_INT:
+		type = SDP_TYPE;
+		break;
+	default:
+		type = UNKNOWN_TYPE;
+	}
+
+	return type;
+}

Please do not add duplicate code, and use the sprd_pmic_detect_charger_type() instead, which had been exported in the mainline.

Quoted 215 lines
drivers/usb/phy/phy-sprd-ums512.c new file +215 -0
+
+static inline void sprd_hsphy_enable(struct sprd_hsphy *sprd_phy)
+{
+	u32 reg, msk;
+
+	/* enable usb module */
+	reg = msk = (MASK_AON_APB_OTG_UTMI_EB | MASK_AON_APB_ANA_EB);
+	regmap_update_bits(sprd_phy->hsphy_glb, REG_AON_APB_APB_EB1, msk, reg);
+	reg = msk = MASK_AON_APB_CGM_OTG_REF_EN |
+		MASK_AON_APB_CGM_DPHY_REF_EN;
+	regmap_update_bits(sprd_phy->hsphy_glb, REG_AON_APB_CGM_REG1, msk, reg);
+}
+
+static inline void sprd_hsphy_power_down(struct sprd_hsphy *sprd_phy)
+{
+	u32 reg, msk;
+
+	/* usb power down */
+	reg = msk = (MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_L |
+		MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_S);
+	regmap_update_bits(sprd_phy->ana_g2,
+		REG_ANLG_PHY_G2_ANALOG_USB20_USB20_BATTER_PLL, msk, reg);
+}
+
+static inline void sprd_hsphy_reset_core(struct sprd_hsphy *sprd_phy)
+{
+	u32 reg, msk;
+
+	/* Reset PHY */
+	reg = msk = MASK_AON_APB_OTG_PHY_SOFT_RST | MASK_AON_APB_OTG_UTMI_SOFT_RST;
+
+	regmap_update_bits(sprd_phy->hsphy_glb, REG_AON_APB_APB_RST1, msk, reg);
+	/* USB PHY reset need to delay 20ms~30ms */
+	usleep_range(20000, 30000);
+	regmap_update_bits(sprd_phy->hsphy_glb, REG_AON_APB_APB_RST1, msk, 0);
+}
+
+static int sprd_hostphy_set(struct usb_phy *u_phy, int on)
+{
+	struct sprd_hsphy *sprd_phy = container_of(u_phy, struct sprd_hsphy, phy);
+	u32 reg, msk;
+	int ret = 0;
+
+	if (on) {
+		msk = MASK_AON_APB_USB2_PHY_IDDIG;
+		ret |= regmap_update_bits(sprd_phy->hsphy_glb,
+			REG_AON_APB_OTG_PHY_CTRL, msk, 0);
+
+		msk = MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DMPULLDOWN |
+			MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DPPULLDOWN;
+		ret |= regmap_update_bits(sprd_phy->ana_g2,
+			REG_ANLG_PHY_G2_ANALOG_USB20_REG_SEL_CFG_0,
+			msk, msk);
+
+		msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DMPULLDOWN |
+			MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DPPULLDOWN;
+		ret |= regmap_update_bits(sprd_phy->ana_g2,
+			REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL2,
+			msk, msk);
+
+		reg = 0x200;
+		msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_RESERVED;
+		ret |= regmap_update_bits(sprd_phy->ana_g2,
+			REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1,
+			msk, reg);
+		sprd_phy->is_host = true;
+	} else {
+		reg = msk = MASK_AON_APB_USB2_PHY_IDDIG;
+		ret |= regmap_update_bits(sprd_phy->hsphy_glb,
+			REG_AON_APB_OTG_PHY_CTRL, msk, reg);
+
+		msk = MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DMPULLDOWN |
+			MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DPPULLDOWN;
+		ret |= regmap_update_bits(sprd_phy->ana_g2,
+			REG_ANLG_PHY_G2_ANALOG_USB20_REG_SEL_CFG_0,
+			msk, msk);
+
+		msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DMPULLDOWN |
+			MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DPPULLDOWN;
+		ret |= regmap_update_bits(sprd_phy->ana_g2,
+			REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL2,
+			msk, 0);
+
+		msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_RESERVED;
+		ret |= regmap_update_bits(sprd_phy->ana_g2,
+			REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1,
+			msk, 0);
+		sprd_phy->is_host = false;
+	}
+	return ret;
+}
+
+static int sprd_hsphy_init(struct usb_phy *u_phy)
+{
+	struct sprd_hsphy *sprd_phy = container_of(u_phy, struct sprd_hsphy, phy);
+	u32 reg, msk;
+	int ret;
+
+	if (atomic_read(&sprd_phy->inited)) {
+		dev_dbg(u_phy->dev, "%s is already inited!\n", __func__);
+		return 0;
+	}
+
+	/* Turn On VDD */
+	regulator_set_voltage(sprd_phy->vdd, sprd_phy->vdd_vol, sprd_phy->vdd_vol);
+	if (!regulator_is_enabled(sprd_phy->vdd)) {
+		ret = regulator_enable(sprd_phy->vdd);
+		if (ret)
+			return ret;
+	}
+
+	sprd_hsphy_enable(sprd_phy);
+	regmap_update_bits(sprd_phy->ana_g2, REG_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW,
+				MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW_EN, 0);
+
+	/* usb phy power */
+	msk = (MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_L |
+		MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_S);
+	regmap_update_bits(sprd_phy->ana_g2, REG_ANLG_PHY_G2_ANALOG_USB20_USB20_BATTER_PLL,
+			msk, 0);
+
+	/* usb vbus valid */
+	reg = msk = MASK_AON_APB_OTG_VBUS_VALID_PHYREG;
+	regmap_update_bits(sprd_phy->hsphy_glb, REG_AON_APB_OTG_PHY_TEST, msk, reg);
+
+	reg = msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT;
+	regmap_update_bits(sprd_phy->ana_g2, REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1,
+			msk, reg);
+
+	/* for SPRD phy utmi_width sel */
+	reg = msk = MASK_AON_APB_UTMI_WIDTH_SEL;
+	regmap_update_bits(sprd_phy->hsphy_glb, REG_AON_APB_OTG_PHY_CTRL, msk, reg);
+
+	reg = msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DATABUS16_8;
+	regmap_update_bits(sprd_phy->ana_g2, REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1,
+			msk, reg);
+
+	reg = TUNEHSAMP_2_6MA;
+	msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TUNEHSAMP;
+	regmap_update_bits(sprd_phy->ana_g2, REG_ANLG_PHY_G2_ANALOG_USB20_USB20_TRIMMING,
+			msk, reg);
+
+	reg = TFREGRES_TUNE_VALUE;
+	msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TFREGRES;
+	regmap_update_bits(sprd_phy->ana_g2, REG_ANLG_PHY_G2_ANALOG_USB20_USB20_TRIMMING,
+			msk, reg);
+
+	if (!atomic_read(&sprd_phy->reset)) {
+		sprd_hsphy_reset_core(sprd_phy);
+		atomic_set(&sprd_phy->reset, 1);
+	}
+
+	atomic_set(&sprd_phy->inited, 1);
+
+	return 0;
+}
+
+static void sprd_hsphy_shutdown(struct usb_phy *u_phy)
+{
+	struct sprd_hsphy *sprd_phy = container_of(u_phy, struct sprd_hsphy, phy);
+	u32 reg, msk;
+
+	if (!atomic_read(&sprd_phy->inited)) {
+		dev_dbg(sprd_phy->dev, "%s is already shut down\n", __func__);
+		return;
+	}
+
+	/* usb vbus */
+	msk = MASK_AON_APB_OTG_VBUS_VALID_PHYREG;
+	regmap_update_bits(sprd_phy->hsphy_glb, REG_AON_APB_OTG_PHY_TEST, msk, 0);
+	msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT;
+	regmap_update_bits(sprd_phy->ana_g2,
+		REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1, msk, 0);
+
+	sprd_hsphy_power_down(sprd_phy);
+	regmap_update_bits(sprd_phy->ana_g2,
+		REG_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW,
+		msk, reg);
+
+	/* usb cgm ref */
+	msk = MASK_AON_APB_CGM_OTG_REF_EN |
+		MASK_AON_APB_CGM_DPHY_REF_EN;
+	regmap_update_bits(sprd_phy->hsphy_glb, REG_AON_APB_CGM_REG1, msk, 0);
+
+	if (regulator_is_enabled(sprd_phy->vdd))
+		regulator_disable(sprd_phy->vdd);
+
+	atomic_set(&sprd_phy->inited, 0);
+	atomic_set(&sprd_phy->reset, 0);
+}
+
+static ssize_t vdd_voltage_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct sprd_hsphy *sprd_phy = dev_get_drvdata(dev);
+
+	if (!sprd_phy)
+		return -EINVAL;
+
+	return sprintf(buf, "%d\n", sprd_phy->vdd_vol);
+}
+
+static ssize_t vdd_voltage_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t size)
+{
+	struct sprd_hsphy *sprd_phy = dev_get_drvdata(dev);
+	u32 vol;
+
+	if (!sprd_phy)
+		return -EINVAL;
+
+	if (kstrtouint(buf, 16, &vol) < 0)
+		return -EINVAL;

drivers/usb/phy/phy-sprd-ums512.c line 304

Quoted 10 lines
drivers/usb/phy/phy-sprd-ums512.c new file +10 -0
+
+	if (vol < 1200000 || vol > 3750000) {
+		dev_err(dev, "Invalid voltage value %d\n", vol);
+		return -EINVAL;
+	}
+	sprd_phy->vdd_vol = vol;
+
+	return size;
+}
+static DEVICE_ATTR_RW(vdd_voltage);

Why add voltage setting interface in a usb phy driver? should move to regulator driver?

Quoted 75 lines
drivers/usb/phy/phy-sprd-ums512.c new file +75 -0
+
+static struct attribute *usb_hsphy_attrs[] = {
+	&dev_attr_vdd_voltage.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(usb_hsphy);
+
+static int sprd_hsphy_vbus_notify(struct notifier_block *nb,
+				  unsigned long event, void *data)
+{
+	struct usb_phy *u_phy = container_of(nb, struct usb_phy, vbus_nb);
+	struct sprd_hsphy *sprd_phy = container_of(u_phy, struct sprd_hsphy, phy);
+	u32 reg, msk;
+
+	if (sprd_phy->is_host || u_phy->last_event == USB_EVENT_ID) {
+		dev_info(sprd_phy->dev, "USB PHY is host mode\n");
+		return 0;
+	}
+
+	if (event) {
+		/* usb vbus valid */
+		reg = msk = MASK_AON_APB_OTG_VBUS_VALID_PHYREG;
+		regmap_update_bits(sprd_phy->hsphy_glb,
+			REG_AON_APB_OTG_PHY_TEST, msk, reg);
+
+		reg = msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT;
+		regmap_update_bits(sprd_phy->ana_g2,
+			REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1, msk, reg);
+		usb_phy_set_charger_state(u_phy, USB_CHARGER_PRESENT);
+	} else {
+		/* usb vbus invalid */
+		msk = MASK_AON_APB_OTG_VBUS_VALID_PHYREG;
+		regmap_update_bits(sprd_phy->hsphy_glb, REG_AON_APB_OTG_PHY_TEST,
+			msk, 0);
+		msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT;
+		regmap_update_bits(sprd_phy->ana_g2,
+			REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1, msk, 0);
+		usb_phy_set_charger_state(u_phy, USB_CHARGER_ABSENT);
+	}
+
+	return 0;
+}
+
+static enum usb_charger_type sprd_hsphy_charger_detect(struct usb_phy *u_phy)
+{
+	struct sprd_hsphy *sprd_phy = container_of(u_phy, struct sprd_hsphy, phy);
+
+	return sc27xx_charger_detect(sprd_phy->pmic);
+}
+
+static int sprd_hsphy_probe(struct platform_device *pdev)
+{
+	struct device_node *regmap_np;
+	struct platform_device *regmap_pdev;
+	struct sprd_hsphy *sprd_phy;
+	struct device *dev = &pdev->dev;
+	int ret;
+	struct usb_otg *otg;
+
+	sprd_phy = devm_kzalloc(dev, sizeof(*sprd_phy), GFP_KERNEL);
+	if (!sprd_phy)
+		return -ENOMEM;
+
+	regmap_np = of_find_compatible_node(NULL, NULL, "sprd,sc27xx-syscon");
+	if (!regmap_np) {
+		dev_err(dev, "unable to get syscon node\n");
+		return -ENODEV;
+	}
+
+	regmap_pdev = of_find_device_by_node(regmap_np);
+	if (!regmap_pdev) {
+		dev_err(dev, "unable to get syscon platform device\n");
+		ret = -ENODEV;
+		goto device_node_err;
+	}

drivers/usb/phy/phy-sprd-ums512.c line 389

Quoted 10 lines
drivers/usb/phy/phy-sprd-ums512.c new file +10 -0
+
+	sprd_phy->pmic = dev_get_regmap(regmap_pdev->dev.parent, NULL);
+	if (!sprd_phy->pmic) {
+		dev_err(dev, "unable to get pmic regmap device\n");
+		ret = -ENODEV;
+		goto platform_device_err;
+	}
+
+	ret = of_property_read_u32(dev->of_node, "sprd,vdd-voltage",
+				   &sprd_phy->vdd_vol);

Where is the DT binding secription?

drivers/usb/phy/phy-sprd-ums512.c line 395

Quoted 6 lines
drivers/usb/phy/phy-sprd-ums512.c new file +6 -0
+	if (ret < 0) {
+		dev_err(dev, "unable to read ssphy vdd voltage\n");
+		goto platform_device_err;
+	}
+
+	sprd_phy->vdd = devm_regulator_get(dev, "vdd");

ditto.
Please add the DT binding firstly.

Quoted 27 lines
drivers/usb/phy/phy-sprd-ums512.c new file +27 -0
+	if (IS_ERR(sprd_phy->vdd)) {
+		dev_err(dev, "unable to get ssphy vdd supply\n");
+		ret = PTR_ERR(sprd_phy->vdd);
+		goto platform_device_err;
+	}
+
+	ret = regulator_set_voltage(sprd_phy->vdd, sprd_phy->vdd_vol, sprd_phy->vdd_vol);
+	if (ret < 0) {
+		dev_err(dev, "fail to set ssphy vdd voltage at %dmV\n", sprd_phy->vdd_vol);
+		goto platform_device_err;
+	}
+
+	otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
+	if (!otg) {
+		ret = -ENOMEM;
+		goto platform_device_err;
+	}
+
+	sprd_phy->ana_g2 = syscon_regmap_lookup_by_phandle(dev->of_node,
+				 "sprd,syscon-anag2");
+	if (IS_ERR(sprd_phy->ana_g2)) {
+		dev_err(&pdev->dev, "ap USB anag2 syscon failed!\n");
+		ret = PTR_ERR(sprd_phy->ana_g2);
+		goto platform_device_err;
+	}
+
+	sprd_phy->hsphy_glb = syscon_regmap_lookup_by_phandle(dev->of_node,

drivers/usb/phy/phy-sprd-ums512.c line 432

Quoted 10 lines
drivers/usb/phy/phy-sprd-ums512.c new file +10 -0
+				 "sprd,syscon-enable");
+	if (IS_ERR(sprd_phy->hsphy_glb)) {
+		dev_err(&pdev->dev, "ap USB aon apb syscon failed!\n");
+		ret = PTR_ERR(sprd_phy->hsphy_glb);
+		goto platform_device_err;
+	}
+
+	sprd_hsphy_enable(sprd_phy);
+
+	sprd_hsphy_power_down(sprd_phy);

Why doing this? please add comments to explain these strange hardware operation.

Quoted 18 lines
drivers/usb/phy/phy-sprd-ums512.c new file +18 -0
+
+	sprd_phy->phy.dev = dev;
+	sprd_phy->phy.label = "sprd-hsphy";
+	sprd_phy->phy.otg = otg;
+	sprd_phy->phy.init = sprd_hsphy_init;
+	sprd_phy->phy.shutdown = sprd_hsphy_shutdown;
+	sprd_phy->phy.set_vbus = sprd_hostphy_set;
+	sprd_phy->phy.type = USB_PHY_TYPE_USB2;
+	sprd_phy->phy.vbus_nb.notifier_call = sprd_hsphy_vbus_notify;
+	sprd_phy->phy.charger_detect = sprd_hsphy_charger_detect;
+	otg->usb_phy = &sprd_phy->phy;
+
+	platform_set_drvdata(pdev, sprd_phy);
+
+	ret = usb_add_phy_dev(&sprd_phy->phy);
+	if (ret) {
+		dev_err(dev, "fail to add phy\n");
+		goto platform_device_err;

drivers/usb/phy/phy-sprd-ums512.c line 460

Quoted 10 lines
drivers/usb/phy/phy-sprd-ums512.c new file +10 -0
+	}
+
+	ret = sysfs_create_groups(&dev->kobj, usb_hsphy_groups);
+	if (ret)
+		dev_warn(dev, "failed to create usb hsphy attributes\n");
+
+	if (extcon_get_state(sprd_phy->phy.edev, EXTCON_USB) > 0)
+		usb_phy_set_charger_state(&sprd_phy->phy, USB_CHARGER_PRESENT);
+
+	dev_info(dev, "sprd usb phy probe ok !\n");

Please remove these useless printing.

drivers/usb/phy/phy-sprd-ums512.c line 463

Quoted 3 lines
drivers/usb/phy/phy-sprd-ums512.c new file +3 -0
+
+platform_device_err:
+device_node_err:

2 same level label?

Quoted 83 lines
drivers/usb/phy/phy-sprd-ums512.c new file +48 -0
+	of_node_put(regmap_np);
+
+	return ret;
+}
+
+static int sprd_hsphy_remove(struct platform_device *pdev)
+{
+	struct sprd_hsphy *sprd_phy = platform_get_drvdata(pdev);
+
+	sysfs_remove_groups(&pdev->dev.kobj, usb_hsphy_groups);
+	usb_remove_phy(&sprd_phy->phy);
+	if (regulator_is_enabled(sprd_phy->vdd))
+		regulator_disable(sprd_phy->vdd);
+
+	return 0;
+}
+
+static const struct of_device_id sprd_hsphy_match[] = {
+	{ .compatible = "sprd,ums512-phy"},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, sprd_hsphy_match);
+
+static struct platform_driver sprd_hsphy_driver = {
+	.probe = sprd_hsphy_probe,
+	.remove = sprd_hsphy_remove,
+	.driver = {
+		.name = "sprd-hsphy",
+		.of_match_table = sprd_hsphy_match,
+	},
+};
+
+static int __init sprd_hsphy_driver_init(void)
+{
+	return platform_driver_register(&sprd_hsphy_driver);
+}
+
+static void __exit sprd_hsphy_driver_exit(void)
+{
+	platform_driver_unregister(&sprd_hsphy_driver);
+}
+
+late_initcall(sprd_hsphy_driver_init);
+module_exit(sprd_hsphy_driver_exit);
+
+MODULE_DESCRIPTION("UNISOC USB PHY driver");
+MODULE_LICENSE("GPL");
drivers/usb/phy/phy-sprd-ums512.h new file +29 -0
new file mode 100644
index 000000000000..903da0573eae
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0+ OR MIT */
+/*
+ * Spreadtrum UMS512 SOC USB registers file
+ *
+ * Copyright C 2022, Spreadtrum Communications Inc.
+ */
+
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DATABUS16_8			0x10000000
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DMPULLDOWN			0x8
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DPPULLDOWN			0x10
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW_EN			0x1
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_L			0x8
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_S			0x10
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_RESERVED			0xffff
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TFREGRES			0x1f80000
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TUNEHSAMP			0x6000000
+#define MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT			0x10000
+#define MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DMPULLDOWN		0x2
+#define MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DPPULLDOWN		0x4
+#define MASK_AON_APB_ANA_EB						0x1000
+#define MASK_AON_APB_CGM_DPHY_REF_EN					0x400
+#define MASK_AON_APB_CGM_OTG_REF_EN					0x1000
+#define MASK_AON_APB_OTG_PHY_SOFT_RST					0x200
+#define MASK_AON_APB_OTG_UTMI_EB					0x100
+#define MASK_AON_APB_OTG_UTMI_SOFT_RST					0x100
+#define MASK_AON_APB_OTG_VBUS_VALID_PHYREG				0x1000000
+#define MASK_AON_APB_USB2_PHY_IDDIG					0x8
+#define MASK_AON_APB_UTMI_WIDTH_SEL					0x40000000
+#define REG_ANLG_PHY_G2_ANALOG_USB20_USB20_BATTER_PLL			0x005c

drivers/usb/phy/phy-sprd-ums512.h line 39

Quoted 10 lines
drivers/usb/phy/phy-sprd-ums512.h new file +10 -0
+#define REG_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW			0x0070
+#define REG_ANLG_PHY_G2_ANALOG_USB20_USB20_TRIMMING			0x0064
+#define REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1			0x0058
+#define REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL2			0x0060
+#define REG_ANLG_PHY_G2_ANALOG_USB20_REG_SEL_CFG_0			0x0074
+#define REG_AON_APB_APB_EB1						0x0004
+#define REG_AON_APB_APB_RST1						0x0010
+#define REG_AON_APB_CGM_REG1						0x0138
+#define REG_AON_APB_OTG_PHY_CTRL					0x0208
+#define REG_AON_APB_OTG_PHY_TEST					0x0204

Move them to the driver file and please rename the ugly macro names, too long :(

And why not move the usb phy driver to be a generic phy driver? I mean move it to the drivers/phy.

author
#4 lore raw
Quoted 140 lines · Baolin Wang
Quoted 137 lines · Cixi Geng

From: Cixi Geng <[email protected]>

This driver is support USB2 phy for Spreadtrum UMS512 SOC's,

---
  drivers/usb/phy/Kconfig           |  10 +
  drivers/usb/phy/Makefile          |   1 +
  drivers/usb/phy/phy-sprd-ums512.c | 511
++++++++++++++++++++++++++++++
  drivers/usb/phy/phy-sprd-ums512.h |  39 +++

  4 files changed, 561 insertions(+)

  create mode 100644 drivers/usb/phy/phy-sprd-ums512.c create mode 100644 drivers/usb/phy/phy-sprd-ums512.h

drivers/usb/phy/Kconfig +0 -0
index 5f629d7cad64..fa5564e6f3a3 100644
@@ -158,6 +158,16 @@ config USB_TEGRA_PHY

          This driver provides PHY support for the USB controllers found
          on NVIDIA Tegra SoC's.

+config USB_SPRD_UMS512_PHY
+       tristate "Spreadtrum ums512 USB2 PHY Driver"
+       depends on ARCH_SPRD || COMPILE_TEST
+       select USB_PHY
+       select EXTCON_USB_GPIO
+       help
+         Enable this to support the SPRD ums512 USB2 PHY that is part of SOC.
+         This driver takes care of all the PHY functionality, normally paired with
+         DesignWare USB20 (DRD) Controller.
+
  config USB_ULPI
        bool "Generic ULPI Transceiver Driver"
        depends on ARM || ARM64 || COMPILE_TEST

drivers/usb/phy/Makefile +0 -0
index e5d619b4d8f6..ce45ee0f12a8 100644
@@ -22,4 +22,5 @@ obj-$(CONFIG_USB_MV_OTG)              += phy-mv-

usb.o
  obj-$(CONFIG_USB_MXS_PHY)             += phy-mxs-usb.o obj-$(CONFIG_USB_ULPI)                        += phy-ulpi.o obj-$(CONFIG_USB_ULPI_VIEWPORT)               += phy-ulpi- viewport.o
+obj-$(CONFIG_USB_SPRD_UMS512_PHY)      += phy-sprd-ums512.o obj-$(CONFIG_KEYSTONE_USB_PHY)                += phy-keystone.o

drivers/usb/phy/phy-sprd-ums512.c new file +45 -0
new file mode 100644
index 000000000000..025f45e8d509
@@ -0,0 +1,511 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Unisoc USB PHY driver
+ *
+ * Copyright (C) 2023 Unisoc Inc.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+#include <linux/usb/otg.h>
+#include <linux/usb/phy.h>
+#include <uapi/linux/usb/charger.h>
+
+#include "phy-sprd-ums512.h"
+
+struct sprd_hsphy {
+       struct device           *dev;
+       struct usb_phy          phy;
+       struct regulator        *vdd;
+       struct regmap           *hsphy_glb;
+       struct regmap           *ana_g2;
+       struct regmap           *pmic;
+       u32                     vdd_vol;
+       atomic_t                reset;
+       atomic_t                inited;
+       bool                    is_host;
+};
+
+#define TUNEHSAMP_2_6MA                (3 << 25)
+#define TFREGRES_TUNE_VALUE    (0x14 << 19)
+#define SC2730_CHARGE_STATUS   0x1b9c
+#define BIT_CHG_DET_DONE       BIT(11)
+#define BIT_SDP_INT            BIT(7)
+#define BIT_DCP_INT            BIT(6)
+#define BIT_CDP_INT            BIT(5)
+
+static enum usb_charger_type sc27xx_charger_detect(struct regmap

*regmap)

drivers/usb/phy/phy-sprd-ums512.c new file +7 -0
+{
+       enum usb_charger_type type;
+       u32 status = 0, val;
+       int ret, cnt = 10;
+
+       do {
+               ret = regmap_read(regmap, SC2730_CHARGE_STATUS,

&val);
+               if (ret)
+                       return UNKNOWN_TYPE;
+
+               if (val & BIT_CHG_DET_DONE) {
+                       status = val & (BIT_CDP_INT | BIT_DCP_INT | BIT_SDP_INT);
+                       break;
+               }

drivers/usb/phy/phy-sprd-ums512.c new file +17 -0
+
+               msleep(200);
+       } while (--cnt > 0);
+
+       switch (status) {
+       case BIT_CDP_INT:
+               type = CDP_TYPE;
+               break;
+       case BIT_DCP_INT:
+               type = DCP_TYPE;
+               break;
+       case BIT_SDP_INT:
+               type = SDP_TYPE;
+               break;
+       default:
+               type = UNKNOWN_TYPE;
+       }
Quoted 8 lines

drivers/usb/phy/phy-sprd-ums512.c line 79

Quoted 3 lines
drivers/usb/phy/phy-sprd-ums512.c new file +3 -0
+
+       return type;
+}

Please do not add duplicate code, and use the sprd_pmic_detect_charger_type() instead, which had been exported in the
mainline.

ok, I will remove this next patchset.

Quoted 382 lines
Quoted 256 lines
drivers/usb/phy/phy-sprd-ums512.c new file +7 -0
+
+static inline void sprd_hsphy_enable(struct sprd_hsphy *sprd_phy)
+{
+       u32 reg, msk;
+
+       /* enable usb module */
+       reg = msk = (MASK_AON_APB_OTG_UTMI_EB |

MASK_AON_APB_ANA_EB);
+       regmap_update_bits(sprd_phy->hsphy_glb,
REG_AON_APB_APB_EB1, msk, reg);
+       reg = msk = MASK_AON_APB_CGM_OTG_REF_EN |
+               MASK_AON_APB_CGM_DPHY_REF_EN;
+       regmap_update_bits(sprd_phy->hsphy_glb,
REG_AON_APB_CGM_REG1, msk, reg);

drivers/usb/phy/phy-sprd-ums512.c new file +3 -0
+}
+
+static inline void sprd_hsphy_power_down(struct sprd_hsphy

*sprd_phy)

drivers/usb/phy/phy-sprd-ums512.c new file +8 -0
+{
+       u32 reg, msk;
+
+       /* usb power down */
+       reg = msk = (MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_L |
+               MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_S);
+       regmap_update_bits(sprd_phy->ana_g2,
+               REG_ANLG_PHY_G2_ANALOG_USB20_USB20_BATTER_PLL, msk,

reg);

drivers/usb/phy/phy-sprd-ums512.c new file +3 -0
+}
+
+static inline void sprd_hsphy_reset_core(struct sprd_hsphy

*sprd_phy)

drivers/usb/phy/phy-sprd-ums512.c new file +5 -0
+{
+       u32 reg, msk;
+
+       /* Reset PHY */
+       reg = msk = MASK_AON_APB_OTG_PHY_SOFT_RST |

MASK_AON_APB_OTG_UTMI_SOFT_RST;
+
+       regmap_update_bits(sprd_phy->hsphy_glb, REG_AON_APB_APB_RST1, msk, reg);
+       /* USB PHY reset need to delay 20ms~30ms */ +       usleep_range(20000, 30000);
+       regmap_update_bits(sprd_phy->hsphy_glb, REG_AON_APB_APB_RST1, msk, 0);

drivers/usb/phy/phy-sprd-ums512.c new file +5 -0
+}
+
+static int sprd_hostphy_set(struct usb_phy *u_phy, int on)
+{
+       struct sprd_hsphy *sprd_phy = container_of(u_phy, struct

sprd_hsphy, phy);
+       u32 reg, msk;
+       int ret = 0;

drivers/usb/phy/phy-sprd-ums512.c new file +7 -0
+
+       if (on) {
+               msk = MASK_AON_APB_USB2_PHY_IDDIG;
+               ret |= regmap_update_bits(sprd_phy->hsphy_glb,
+                       REG_AON_APB_OTG_PHY_CTRL, msk, 0);
+
+               msk =

MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DMPULLDOWN | +                       MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20 _DPPULLDOWN;
+               ret |= regmap_update_bits(sprd_phy->ana_g2, +                       REG_ANLG_PHY_G2_ANALOG_USB20_REG_SEL_CFG_0, +                       msk, msk);
+
+               msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DMPULLDOWN |
+                       MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DPPULLD OWN;
+               ret |= regmap_update_bits(sprd_phy->ana_g2, +                       REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL 2,
+                       msk, msk);
+
+               reg = 0x200;
+               msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_RESERVED; +               ret |= regmap_update_bits(sprd_phy->ana_g2, +                       REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL 1,
+                       msk, reg);
+               sprd_phy->is_host = true;
+       } else {
+               reg = msk = MASK_AON_APB_USB2_PHY_IDDIG; +               ret |= regmap_update_bits(sprd_phy->hsphy_glb, +                       REG_AON_APB_OTG_PHY_CTRL, msk, reg); +
+               msk = MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DMPULLDOWN | +                       MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20 _DPPULLDOWN;
+               ret |= regmap_update_bits(sprd_phy->ana_g2, +                       REG_ANLG_PHY_G2_ANALOG_USB20_REG_SEL_CFG_0, +                       msk, msk);
+
+               msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DMPULLDOWN |
+                       MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DPPULLD OWN;
+               ret |= regmap_update_bits(sprd_phy->ana_g2, +                       REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL 2,
+                       msk, 0);
+
+               msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_RESERVED; +               ret |= regmap_update_bits(sprd_phy->ana_g2, +                       REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL 1,
+                       msk, 0);
+               sprd_phy->is_host = false;
+       }
+       return ret;

drivers/usb/phy/phy-sprd-ums512.c new file +5 -0
+}
+
+static int sprd_hsphy_init(struct usb_phy *u_phy)
+{
+       struct sprd_hsphy *sprd_phy = container_of(u_phy, struct

sprd_hsphy, phy);
+       u32 reg, msk;
+       int ret;
+
+       if (atomic_read(&sprd_phy->inited)) {
+               dev_dbg(u_phy->dev, "%s is already inited!\n", __func__);
+               return 0;
+       }
+
+       /* Turn On VDD */
+       regulator_set_voltage(sprd_phy->vdd, sprd_phy->vdd_vol, sprd_phy->vdd_vol);
+       if (!regulator_is_enabled(sprd_phy->vdd)) {
+               ret = regulator_enable(sprd_phy->vdd);
+               if (ret)
+                       return ret;
+       }
+
+       sprd_hsphy_enable(sprd_phy);
+       regmap_update_bits(sprd_phy->ana_g2, REG_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW,
+                               MASK_ANLG_PHY_G2_ANALOG_USB20_USB20 _ISO_SW_EN, 0);
+
+       /* usb phy power */
+       msk = (MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_L | +               MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_S); +       regmap_update_bits(sprd_phy->ana_g2, REG_ANLG_PHY_G2_ANALOG_USB20_USB20_BATTER_PLL,
+                       msk, 0);
+
+       /* usb vbus valid */
+       reg = msk = MASK_AON_APB_OTG_VBUS_VALID_PHYREG; +       regmap_update_bits(sprd_phy->hsphy_glb, REG_AON_APB_OTG_PHY_TEST, msk, reg);
+
+       reg = msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT; +       regmap_update_bits(sprd_phy->ana_g2, REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1,
+                       msk, reg);
+
+       /* for SPRD phy utmi_width sel */
+       reg = msk = MASK_AON_APB_UTMI_WIDTH_SEL;
+       regmap_update_bits(sprd_phy->hsphy_glb, REG_AON_APB_OTG_PHY_CTRL, msk, reg);
+
+       reg = msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DATABUS16_8;
+       regmap_update_bits(sprd_phy->ana_g2, REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1,
+                       msk, reg);
+
+       reg = TUNEHSAMP_2_6MA;
+       msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TUNEHSAMP; +       regmap_update_bits(sprd_phy->ana_g2, REG_ANLG_PHY_G2_ANALOG_USB20_USB20_TRIMMING,
+                       msk, reg);
+
+       reg = TFREGRES_TUNE_VALUE;
+       msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TFREGRES; +       regmap_update_bits(sprd_phy->ana_g2, REG_ANLG_PHY_G2_ANALOG_USB20_USB20_TRIMMING,
+                       msk, reg);

drivers/usb/phy/phy-sprd-ums512.c new file +14 -0
+
+       if (!atomic_read(&sprd_phy->reset)) {
+               sprd_hsphy_reset_core(sprd_phy);
+               atomic_set(&sprd_phy->reset, 1);
+       }
+
+       atomic_set(&sprd_phy->inited, 1);
+
+       return 0;
+}
+
+static void sprd_hsphy_shutdown(struct usb_phy *u_phy)
+{
+       struct sprd_hsphy *sprd_phy = container_of(u_phy, struct

sprd_hsphy, phy);
+       u32 reg, msk;
+
+       if (!atomic_read(&sprd_phy->inited)) {
+               dev_dbg(sprd_phy->dev, "%s is already shut down\n", __func__);
+               return;
+       }
+
+       /* usb vbus */
+       msk = MASK_AON_APB_OTG_VBUS_VALID_PHYREG;
+       regmap_update_bits(sprd_phy->hsphy_glb, REG_AON_APB_OTG_PHY_TEST, msk, 0);
+       msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT; +       regmap_update_bits(sprd_phy->ana_g2,
+               REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1, msk, 0);

drivers/usb/phy/phy-sprd-ums512.c new file +10 -0
+
+       sprd_hsphy_power_down(sprd_phy);
+       regmap_update_bits(sprd_phy->ana_g2,
+               REG_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW,
+               msk, reg);
+
+       /* usb cgm ref */
+       msk = MASK_AON_APB_CGM_OTG_REF_EN |
+               MASK_AON_APB_CGM_DPHY_REF_EN;
+       regmap_update_bits(sprd_phy->hsphy_glb,

REG_AON_APB_CGM_REG1, msk, 0);

drivers/usb/phy/phy-sprd-ums512.c new file +32 -0
+
+       if (regulator_is_enabled(sprd_phy->vdd))
+               regulator_disable(sprd_phy->vdd);
+
+       atomic_set(&sprd_phy->inited, 0);
+       atomic_set(&sprd_phy->reset, 0);
+}
+
+static ssize_t vdd_voltage_show(struct device *dev,
+                               struct device_attribute *attr,
+                               char *buf)
+{
+       struct sprd_hsphy *sprd_phy = dev_get_drvdata(dev);
+
+       if (!sprd_phy)
+               return -EINVAL;
+
+       return sprintf(buf, "%d\n", sprd_phy->vdd_vol);
+}
+
+static ssize_t vdd_voltage_store(struct device *dev,
+                                struct device_attribute *attr,
+                                const char *buf, size_t size)
+{
+       struct sprd_hsphy *sprd_phy = dev_get_drvdata(dev);
+       u32 vol;
+
+       if (!sprd_phy)
+               return -EINVAL;
+
+       if (kstrtouint(buf, 16, &vol) < 0)
+               return -EINVAL;

drivers/usb/phy/phy-sprd-ums512.c line 304

Quoted 10 lines
drivers/usb/phy/phy-sprd-ums512.c new file +10 -0
+
+       if (vol < 1200000 || vol > 3750000) {
+               dev_err(dev, "Invalid voltage value %d\n", vol);
+               return -EINVAL;
+       }
+       sprd_phy->vdd_vol = vol;
+
+       return size;
+}
+static DEVICE_ATTR_RW(vdd_voltage);

Why add voltage setting interface in a usb phy driver? should move to regulator driver?

Quoted 94 lines
drivers/usb/phy/phy-sprd-ums512.c new file +11 -0
+
+static struct attribute *usb_hsphy_attrs[] = {
+       &dev_attr_vdd_voltage.attr,
+       NULL
+};
+ATTRIBUTE_GROUPS(usb_hsphy);
+
+static int sprd_hsphy_vbus_notify(struct notifier_block *nb,
+                                 unsigned long event, void *data)
+{
+       struct usb_phy *u_phy = container_of(nb, struct usb_phy,

vbus_nb);
+       struct sprd_hsphy *sprd_phy = container_of(u_phy, struct sprd_hsphy, phy);
+       u32 reg, msk;
+
+       if (sprd_phy->is_host || u_phy->last_event == USB_EVENT_ID) {
+               dev_info(sprd_phy->dev, "USB PHY is host mode\n"); +               return 0;
+       }

drivers/usb/phy/phy-sprd-ums512.c new file +8 -0
+
+       if (event) {
+               /* usb vbus valid */
+               reg = msk = MASK_AON_APB_OTG_VBUS_VALID_PHYREG;
+               regmap_update_bits(sprd_phy->hsphy_glb,
+                       REG_AON_APB_OTG_PHY_TEST, msk, reg);
+
+               reg = msk =

MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT;
+               regmap_update_bits(sprd_phy->ana_g2,
+                       REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL 1, msk, reg);
+               usb_phy_set_charger_state(u_phy, USB_CHARGER_PRESENT);
+       } else {
+               /* usb vbus invalid */
+               msk = MASK_AON_APB_OTG_VBUS_VALID_PHYREG; +               regmap_update_bits(sprd_phy->hsphy_glb, REG_AON_APB_OTG_PHY_TEST,
+                       msk, 0);
+               msk = MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT;
+               regmap_update_bits(sprd_phy->ana_g2,
+                       REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL 1, msk, 0);
+               usb_phy_set_charger_state(u_phy, USB_CHARGER_ABSENT);
+       }

drivers/usb/phy/phy-sprd-ums512.c new file +5 -0
+
+       return 0;
+}
+
+static enum usb_charger_type sprd_hsphy_charger_detect(struct

usb_phy *u_phy)
+{
+       struct sprd_hsphy *sprd_phy = container_of(u_phy, struct sprd_hsphy, phy);

drivers/usb/phy/phy-sprd-ums512.c new file +14 -0
+
+       return sc27xx_charger_detect(sprd_phy->pmic);
+}
+
+static int sprd_hsphy_probe(struct platform_device *pdev)
+{
+       struct device_node *regmap_np;
+       struct platform_device *regmap_pdev;
+       struct sprd_hsphy *sprd_phy;
+       struct device *dev = &pdev->dev;
+       int ret;
+       struct usb_otg *otg;
+
+       sprd_phy = devm_kzalloc(dev, sizeof(*sprd_phy),

GFP_KERNEL);
+       if (!sprd_phy)
+               return -ENOMEM;
+
+       regmap_np = of_find_compatible_node(NULL, NULL, "sprd,sc27xx-syscon");
+       if (!regmap_np) {
+               dev_err(dev, "unable to get syscon node\n"); +               return -ENODEV;
+       }
+
+       regmap_pdev = of_find_device_by_node(regmap_np); +       if (!regmap_pdev) {
+               dev_err(dev, "unable to get syscon platform device\n");
+               ret = -ENODEV;
+               goto device_node_err;
+       }
+
+       sprd_phy->pmic = dev_get_regmap(regmap_pdev->dev.parent, NULL);
+       if (!sprd_phy->pmic) {

Quoted 8 lines

+               dev_err(dev, "unable to get pmic regmap device\n"); +               ret = -ENODEV;
+               goto platform_device_err;
+       }
+
+       ret = of_property_read_u32(dev->of_node, "sprd,vdd- voltage",
+                                  &sprd_phy->vdd_vol);

Where is the DT binding secription?

Quoted 6 lines

+       if (ret < 0) {
+               dev_err(dev, "unable to read ssphy vdd voltage\n"); +               goto platform_device_err;
+       }
+
+       sprd_phy->vdd = devm_regulator_get(dev, "vdd");

Quoted 2 lines

ditto.
Please add the DT binding firstly.

next patchset I wiil add the bindings file.

Quoted 33 lines
Quoted 33 lines

+       if (IS_ERR(sprd_phy->vdd)) {
+               dev_err(dev, "unable to get ssphy vdd supply\n"); +               ret = PTR_ERR(sprd_phy->vdd);
+               goto platform_device_err;
+       }
+
+       ret = regulator_set_voltage(sprd_phy->vdd, sprd_phy-

Quoted 1 line

vdd_vol, sprd_phy->vdd_vol);

+       if (ret < 0) {
+               dev_err(dev, "fail to set ssphy vdd voltage at %dmV\n", sprd_phy->vdd_vol);
+               goto platform_device_err;
+       }

drivers/usb/phy/phy-sprd-ums512.c new file +8 -0
+
+       otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
+       if (!otg) {
+               ret = -ENOMEM;
+               goto platform_device_err;
+       }
+
+       sprd_phy->ana_g2 = syscon_regmap_lookup_by_phandle(dev-
Quoted 1 line

of_node,

+                                "sprd,syscon-anag2"); +       if (IS_ERR(sprd_phy->ana_g2)) {
+               dev_err(&pdev->dev, "ap USB anag2 syscon failed!\n");
+               ret = PTR_ERR(sprd_phy->ana_g2);
+               goto platform_device_err;
+       }
+
+       sprd_phy->hsphy_glb = syscon_regmap_lookup_by_phandle(dev-

Quoted 1 line

of_node,

Quoted 14 lines

drivers/usb/phy/phy-sprd-ums512.c line 254

Quoted 11 lines

+                                "sprd,syscon-enable"); +       if (IS_ERR(sprd_phy->hsphy_glb)) {
+               dev_err(&pdev->dev, "ap USB aon apb syscon failed!\n");
+               ret = PTR_ERR(sprd_phy->hsphy_glb); +               goto platform_device_err;
+       }

drivers/usb/phy/phy-sprd-ums512.c new file +4 -0
+
+       sprd_hsphy_enable(sprd_phy);
+
+       sprd_hsphy_power_down(sprd_phy);

Why doing this? please add comments to explain these strange hardware operation.

the two actions seems redundant, it should in sprd_hsphy_init. I need to test on the Soc. if not effect, I wiil remove it.

Quoted 196 lines
Quoted 25 lines

+
+       sprd_phy->phy.dev = dev;
+       sprd_phy->phy.label = "sprd-hsphy";
+       sprd_phy->phy.otg = otg;
+       sprd_phy->phy.init = sprd_hsphy_init;
+       sprd_phy->phy.shutdown = sprd_hsphy_shutdown;
+       sprd_phy->phy.set_vbus = sprd_hostphy_set;
+       sprd_phy->phy.type = USB_PHY_TYPE_USB2;
+       sprd_phy->phy.vbus_nb.notifier_call = sprd_hsphy_vbus_notify;
+       sprd_phy->phy.charger_detect = sprd_hsphy_charger_detect; +       otg->usb_phy = &sprd_phy->phy;

drivers/usb/phy/phy-sprd-ums512.c new file +12 -0
+
+       platform_set_drvdata(pdev, sprd_phy);
+
+       ret = usb_add_phy_dev(&sprd_phy->phy);
+       if (ret) {
+               dev_err(dev, "fail to add phy\n");
+               goto platform_device_err;
+       }
+
+       ret = sysfs_create_groups(&dev->kobj, usb_hsphy_groups);
+       if (ret)
+               dev_warn(dev, "failed to create usb hsphy
Quoted 7 lines

attributes\n");
+
+       if (extcon_get_state(sprd_phy->phy.edev, EXTCON_USB) > 0) +               usb_phy_set_charger_state(&sprd_phy->phy, USB_CHARGER_PRESENT);
+
+       dev_info(dev, "sprd usb phy probe ok !\n");

Please remove these useless printing.

drivers/usb/phy/phy-sprd-ums512.c line 463

Quoted 3 lines
drivers/usb/phy/phy-sprd-ums512.c new file +3 -0
+
+platform_device_err:
+device_node_err:

2 same level label?

Quoted 142 lines

+       of_node_put(regmap_np);

drivers/usb/phy/phy-sprd-ums512.c new file +47 -0
+
+       return ret;
+}
+
+static int sprd_hsphy_remove(struct platform_device *pdev)
+{
+       struct sprd_hsphy *sprd_phy = platform_get_drvdata(pdev);
+
+       sysfs_remove_groups(&pdev->dev.kobj, usb_hsphy_groups);
+       usb_remove_phy(&sprd_phy->phy);
+       if (regulator_is_enabled(sprd_phy->vdd))
+               regulator_disable(sprd_phy->vdd);
+
+       return 0;
+}
+
+static const struct of_device_id sprd_hsphy_match[] = {
+       { .compatible = "sprd,ums512-phy"},
+       {},
+};
+
+MODULE_DEVICE_TABLE(of, sprd_hsphy_match);
+
+static struct platform_driver sprd_hsphy_driver = {
+       .probe = sprd_hsphy_probe,
+       .remove = sprd_hsphy_remove,
+       .driver = {
+               .name = "sprd-hsphy",
+               .of_match_table = sprd_hsphy_match,
+       },
+};
+
+static int __init sprd_hsphy_driver_init(void)
+{
+       return platform_driver_register(&sprd_hsphy_driver);
+}
+
+static void __exit sprd_hsphy_driver_exit(void)
+{
+       platform_driver_unregister(&sprd_hsphy_driver);
+}
+
+late_initcall(sprd_hsphy_driver_init);
+module_exit(sprd_hsphy_driver_exit);
+
+MODULE_DESCRIPTION("UNISOC USB PHY driver");
+MODULE_LICENSE("GPL");
drivers/usb/phy/phy-sprd-ums512.h new file +8 -0
new file mode 100644
index 000000000000..903da0573eae
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0+ OR MIT */
+/*
+ * Spreadtrum UMS512 SOC USB registers file
+ *
+ * Copyright C 2022, Spreadtrum Communications Inc.
+ */
+
+#define

MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DATABUS16_8 0x10000000
+#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DMPULLDOWN                 0x8 +#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DPPULLDOWN                 0x10 +#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW_EN                  0x1 +#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_L                    0x8 +#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_S                    0x10 +#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_RESERVED                   0xff ff
+#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TFREGRES                   0x1f 80000
+#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TUNEHSAMP                  0x60 00000
+#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT                 0x10 000
+#define MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DMPULLDOWN         0x2 +#define MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DPPULLDOWN         0x4 +#define
MASK_AON_APB_ANA_EB                                            0x10 00
+#define
MASK_AON_APB_CGM_DPHY_REF_EN                                   0x40 0
+#define
MASK_AON_APB_CGM_OTG_REF_EN                                    0x10 00
+#define
MASK_AON_APB_OTG_PHY_SOFT_RST                                  0x20 0
+#define
MASK_AON_APB_OTG_UTMI_EB                                       0x10 0
+#define
MASK_AON_APB_OTG_UTMI_SOFT_RST                                 0x10 0
+#define
MASK_AON_APB_OTG_VBUS_VALID_PHYREG                             0x10 00000
+#define
MASK_AON_APB_USB2_PHY_IDDIG                                    0x8 +#define
MASK_AON_APB_UTMI_WIDTH_SEL                                    0x40 000000
+#define
REG_ANLG_PHY_G2_ANALOG_USB20_USB20_BATTER_PLL                  0x00 5c
+#define
REG_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW                      0x00 70
+#define
REG_ANLG_PHY_G2_ANALOG_USB20_USB20_TRIMMING                    0x00 64
+#define
REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1                   0x00 58
+#define
REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL2                   0x00 60
+#define
REG_ANLG_PHY_G2_ANALOG_USB20_REG_SEL_CFG_0                     0x00 74
+#define
REG_AON_APB_APB_EB1                                            0x00 04
+#define
REG_AON_APB_APB_RST1                                           0x00 10
+#define

Quoted 8 lines

REG_AON_APB_CGM_REG1                                           0x01 38
+#define
REG_AON_APB_OTG_PHY_CTRL                                       0x02 08
+#define
REG_AON_APB_OTG_PHY_TEST                                       0x02 04

Move them to the driver file and please rename the ugly macro names, too
long :(

Quoted 3 lines

And why not move the usb phy driver to be a generic phy driver? I mean
move it to the drivers/phy.

Do you mean all the usb-phy need move to driver/phy? or just this driver?

#5 lore raw
Quoted 3 lines · Cixi Geng
Quoted 2 lines · Baolin Wang

...

Quoted 110 lines
Quoted 110 lines
Quoted 97 lines
drivers/usb/phy/phy-sprd-ums512.c new file +3 -0
+
+MODULE_DESCRIPTION("UNISOC USB PHY driver");
+MODULE_LICENSE("GPL");
drivers/usb/phy/phy-sprd-ums512.h new file +8 -0
new file mode 100644
index 000000000000..903da0573eae
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0+ OR MIT */
+/*
+ * Spreadtrum UMS512 SOC USB registers file
+ *
+ * Copyright C 2022, Spreadtrum Communications Inc.
+ */
+
+#define

MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DATABUS16_8
    0x10000000
+#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DMPULLDOWN                 0x8 +#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_DPPULLDOWN                 0x10 +#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW_EN                  0x1 +#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_L                    0x8 +#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_PS_PD_S                    0x10 +#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_RESERVED                   0xff ff
+#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TFREGRES                   0x1f 80000
+#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_TUNEHSAMP                  0x60 00000
+#define
MASK_ANLG_PHY_G2_ANALOG_USB20_USB20_VBUSVLDEXT                 0x10 000
+#define MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DMPULLDOWN         0x2 +#define MASK_ANLG_PHY_G2_DBG_SEL_ANALOG_USB20_USB20_DPPULLDOWN         0x4 +#define
MASK_AON_APB_ANA_EB                                            0x10 00
+#define
MASK_AON_APB_CGM_DPHY_REF_EN                                   0x40 0
+#define
MASK_AON_APB_CGM_OTG_REF_EN                                    0x10 00
+#define
MASK_AON_APB_OTG_PHY_SOFT_RST                                  0x20 0
+#define
MASK_AON_APB_OTG_UTMI_EB                                       0x10 0
+#define
MASK_AON_APB_OTG_UTMI_SOFT_RST                                 0x10 0
+#define
MASK_AON_APB_OTG_VBUS_VALID_PHYREG                             0x10 00000
+#define
MASK_AON_APB_USB2_PHY_IDDIG                                    0x8 +#define
MASK_AON_APB_UTMI_WIDTH_SEL                                    0x40 000000
+#define
REG_ANLG_PHY_G2_ANALOG_USB20_USB20_BATTER_PLL                  0x00 5c
+#define
REG_ANLG_PHY_G2_ANALOG_USB20_USB20_ISO_SW                      0x00 70
+#define
REG_ANLG_PHY_G2_ANALOG_USB20_USB20_TRIMMING                    0x00 64
+#define
REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL1                   0x00 58
+#define
REG_ANLG_PHY_G2_ANALOG_USB20_USB20_UTMI_CTL2                   0x00 60
+#define
REG_ANLG_PHY_G2_ANALOG_USB20_REG_SEL_CFG_0                     0x00 74
+#define
REG_AON_APB_APB_EB1                                            0x00 04
+#define
REG_AON_APB_APB_RST1                                           0x00 10
+#define

Quoted 8 lines

REG_AON_APB_CGM_REG1                                           0x01 38
+#define
REG_AON_APB_OTG_PHY_CTRL                                       0x02 08
+#define
REG_AON_APB_OTG_PHY_TEST                                       0x02 04

Move them to the driver file and please rename the ugly macro names, too
long :(

Quoted 5 lines
Quoted 3 lines

And why not move the usb phy driver to be a generic phy driver? I mean
move it to the drivers/phy.

Do you mean all the usb-phy need move to driver/phy? or just this driver?

If I remember correctly, new phy drivers should go to the generic phy framework unless there are some reasons to prevent.

Greg, how do you think?