linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 1/5] usb: phy: add usb phy notify port status API
@ 2023-06-14  9:28 Stanley Chang
  2023-06-14  9:28 ` [PATCH v4 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY Stanley Chang
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Stanley Chang @ 2023-06-14  9:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Stanley Chang, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
	Douglas Anderson, Flavio Suligoi, Matthias Kaehlcke, linux-phy,
	devicetree, linux-kernel, linux-usb

In Realtek SoC, the parameter of usb phy is designed to can dynamic
tuning base on port status. Therefore, add a notify callback of phy
driver when usb port status change.

The Realtek phy driver is designed to dynamically adjust disconnection
level and calibrate phy parameters. When the device connected bit changes
and when the disconnected bit changes, do port status change notification:

Check if portstatus is USB_PORT_STAT_CONNECTION and portchange is
USB_PORT_STAT_C_CONNECTION.
1. The device is connected, the driver lowers the disconnection level and
   calibrates the phy parameters.
2. The device disconnects, the driver increases the disconnect level and
   calibrates the phy parameters.

When controller to notify connect that device is already ready. If we
adjust the disconnection level in notify_connect, the disconnect may have
been triggered at this stage. So we need to change that as early as
possible. Therefore, we add an api to notify phy the port status changes.

Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
---
v3 to v4 change:
    Fix the warning for checkpatch with strict.
v2 to v3 change:
    Add more comments about the reason for adding this api
v1 to v2 change:
    No change
---
 drivers/usb/core/hub.c  | 13 +++++++++++++
 include/linux/usb/phy.h | 13 +++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 97a0f8faea6e..10f3364c3fc2 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -614,6 +614,19 @@ static int hub_ext_port_status(struct usb_hub *hub, int port1, int type,
 		ret = 0;
 	}
 	mutex_unlock(&hub->status_mutex);
+
+	if (!ret) {
+		struct usb_device *hdev = hub->hdev;
+
+		if (hdev && !hdev->parent) {
+			struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
+
+			if (hcd->usb_phy)
+				usb_phy_notify_port_status(hcd->usb_phy,
+							   port1 - 1, *status, *change);
+		}
+	}
+
 	return ret;
 }
 
diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
index e4de6bc1f69b..b513749582d7 100644
--- a/include/linux/usb/phy.h
+++ b/include/linux/usb/phy.h
@@ -144,6 +144,10 @@ struct usb_phy {
 	 */
 	int	(*set_wakeup)(struct usb_phy *x, bool enabled);
 
+	/* notify phy port status change */
+	int	(*notify_port_status)(struct usb_phy *x, int port,
+				      u16 portstatus, u16 portchange);
+
 	/* notify phy connect status change */
 	int	(*notify_connect)(struct usb_phy *x,
 			enum usb_device_speed speed);
@@ -316,6 +320,15 @@ usb_phy_set_wakeup(struct usb_phy *x, bool enabled)
 		return 0;
 }
 
+static inline int
+usb_phy_notify_port_status(struct usb_phy *x, int port, u16 portstatus, u16 portchange)
+{
+	if (x && x->notify_port_status)
+		return x->notify_port_status(x, port, portstatus, portchange);
+	else
+		return 0;
+}
+
 static inline int
 usb_phy_notify_connect(struct usb_phy *x, enum usb_device_speed speed)
 {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
  2023-06-14  9:28 [PATCH v4 1/5] usb: phy: add usb phy notify port status API Stanley Chang
@ 2023-06-14  9:28 ` Stanley Chang
  2023-06-14 15:10   ` kernel test robot
  2023-06-17  8:34   ` Krzysztof Kozlowski
  2023-06-14  9:28 ` [PATCH v4 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY Stanley Chang
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 15+ messages in thread
From: Stanley Chang @ 2023-06-14  9:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Stanley Chang, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Alan Stern, Flavio Suligoi,
	Douglas Anderson, Bagas Sanjaya, Matthias Kaehlcke, Ray Chi,
	linux-phy, devicetree, linux-kernel, linux-usb

Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
controller. Added the driver to drive the USB 2.0 PHY transceivers.

Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
---
v3 to v4 change:
    1. Remove the parameter and non hardware properties from dts.
       Add the compatible data included the config and parameter.
    2. Fix the warning for checkpatch with strict.
    3. Remove useless debug messages.
    4. Fix the incorrect or useless "if() return" which pointer check.
    5. Remove the phy power domain control due to we control on other
       driver.
v2 to v3 change:
    1. Broken down into two patches, one for each of USB 2 & 3 PHY.
    2. Removed parameter v1 support for simplification.
    3. Use remove_new for driver remove callback.
v1 to v2 change:
    1. Move the drivers to drivers/phy/ for generic phy driver
    2. Use the generic phy driver api to initialize phy
    3. Use readl/writel to instead phy_read/phy_write directly.
    4. Use read_poll_timeout() in function utmi_wait_register.
    5. Revised some coding styles.
    6. fix the compiler warning for kernel test robot.
---
 drivers/phy/Kconfig                |    1 +
 drivers/phy/Makefile               |    1 +
 drivers/phy/realtek/Kconfig        |   14 +
 drivers/phy/realtek/Makefile       |    2 +
 drivers/phy/realtek/phy-rtk-usb2.c | 1593 ++++++++++++++++++++++++++++
 5 files changed, 1611 insertions(+)
 create mode 100644 drivers/phy/realtek/Kconfig
 create mode 100644 drivers/phy/realtek/Makefile
 create mode 100644 drivers/phy/realtek/phy-rtk-usb2.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index f46e3148d286..6d04a0029c6c 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -86,6 +86,7 @@ source "drivers/phy/motorola/Kconfig"
 source "drivers/phy/mscc/Kconfig"
 source "drivers/phy/qualcomm/Kconfig"
 source "drivers/phy/ralink/Kconfig"
+source "drivers/phy/realtek/Kconfig"
 source "drivers/phy/renesas/Kconfig"
 source "drivers/phy/rockchip/Kconfig"
 source "drivers/phy/samsung/Kconfig"
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 54f312c10a40..ba7c100b14fc 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -26,6 +26,7 @@ obj-y					+= allwinner/	\
 					   mscc/	\
 					   qualcomm/	\
 					   ralink/	\
+					   realtek/	\
 					   renesas/	\
 					   rockchip/	\
 					   samsung/	\
diff --git a/drivers/phy/realtek/Kconfig b/drivers/phy/realtek/Kconfig
new file mode 100644
index 000000000000..2f2f453729d5
--- /dev/null
+++ b/drivers/phy/realtek/Kconfig
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Phy drivers for Realtek platforms
+#
+config PHY_RTK_RTD_USB2PHY
+	tristate "Realtek RTD USB2 PHY Transceiver Driver"
+	depends on USB_SUPPORT
+	select GENERIC_PHY
+	select USB_PHY
+	help
+	  Enable this to support Realtek SoC USB2 phy transceiver.
+	  The DHC (digital home center) RTD series SoCs used the Synopsys
+	  DWC3 USB IP. This driver will do the PHY initialization
+	  of the parameters.
diff --git a/drivers/phy/realtek/Makefile b/drivers/phy/realtek/Makefile
new file mode 100644
index 000000000000..cf5d440841a2
--- /dev/null
+++ b/drivers/phy/realtek/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_PHY_RTK_RTD_USB2PHY)	+= phy-rtk-usb2.o
diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
new file mode 100644
index 000000000000..d16991034492
--- /dev/null
+++ b/drivers/phy/realtek/phy-rtk-usb2.c
@@ -0,0 +1,1593 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  phy-rtk-usb2.c RTK usb2.0 PHY driver
+ *
+ * Copyright (C) 2023 Realtek Semiconductor Corporation
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/uaccess.h>
+#include <linux/debugfs.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/regmap.h>
+#include <linux/sys_soc.h>
+#include <linux/mfd/syscon.h>
+#include <linux/phy/phy.h>
+#include <linux/usb.h>
+#include <linux/usb/phy.h>
+#include <linux/usb/hcd.h>
+
+/* GUSB2PHYACCn register */
+#define PHY_NEW_REG_REQ BIT(25)
+#define PHY_VSTS_BUSY   BIT(23)
+#define PHY_VCTRL_SHIFT 8
+#define PHY_REG_DATA_MASK 0xff
+
+#define GET_LOW_NIBBLE(addr) ((addr) & 0x0f)
+#define GET_HIGH_NIBBLE(addr) (((addr) & 0xf0) >> 4)
+
+#define EFUS_USB_DC_CAL_RATE 2
+#define EFUS_USB_DC_CAL_MAX 7
+
+#define EFUS_USB_DC_DIS_RATE 1
+#define EFUS_USB_DC_DIS_MAX 7
+
+#define MAX_PHY_DATA_SIZE 20
+#define OFFEST_PHY_READ 0x20
+
+#define MAX_USB_PHY_NUM 4
+#define MAX_USB_PHY_PAGE0_DATA_SIZE 16
+#define MAX_USB_PHY_PAGE1_DATA_SIZE 16
+#define MAX_USB_PHY_PAGE2_DATA_SIZE 8
+
+#define SET_PAGE_OFFSET 0xf4
+#define SET_PAGE_0 0x9b
+#define SET_PAGE_1 0xbb
+#define SET_PAGE_2 0xdb
+
+#define PAGE_START 0xe0
+#define PAGE0_0XE4 0xe4
+#define PAGE0_0XE6 0xe6
+#define PAGE0_0XE7 0xe7
+#define PAGE1_0XE0 0xe0
+#define PAGE1_0XE2 0xe2
+
+#define SENSITIVITY_CTRL (BIT(4) | BIT(5) | BIT(6))
+#define ENABLE_AUTO_SENSITIVITY_CALIBRATION BIT(2)
+#define DEFAULT_DC_DRIVING_VALUE (0x8)
+#define DEFAULT_DC_DISCONNECTION_VALUE (0x6)
+#define HS_CLK_SELECT BIT(6)
+
+struct phy_reg {
+	void __iomem *reg_wrap_vstatus;
+	void __iomem *reg_gusb2phyacc0;
+	int vstatus_index;
+};
+
+struct phy_data {
+	u8 addr;
+	u8 data;
+};
+
+struct phy_cfg {
+	int page0_size;
+	struct phy_data page0[MAX_USB_PHY_PAGE0_DATA_SIZE];
+	int page1_size;
+	struct phy_data page1[MAX_USB_PHY_PAGE1_DATA_SIZE];
+	int page2_size;
+	struct phy_data page2[MAX_USB_PHY_PAGE2_DATA_SIZE];
+
+	bool check_efuse;
+	int check_efuse_version;
+#define CHECK_EFUSE_V1 1
+#define CHECK_EFUSE_V2 2
+	int efuse_dc_driving_rate;
+	int efuse_dc_disconnect_rate;
+	int dc_driving_mask;
+	int dc_disconnect_mask;
+	bool usb_dc_disconnect_at_page0;
+	int driving_updated_for_dev_dis;
+
+	bool do_toggle;
+	bool do_toggle_driving;
+	bool use_default_parameter;
+	bool is_double_sensitivity_mode;
+};
+
+struct phy_parameter {
+	struct phy_reg phy_reg;
+
+	/* Get from efuse */
+	s8 efuse_usb_dc_cal;
+	s8 efuse_usb_dc_dis;
+
+	/* Get from dts */
+	bool inverse_hstx_sync_clock;
+	u32 driving_level;
+	s32 driving_compensate;
+	s32 disconnection_compensate;
+};
+
+struct rtk_phy {
+	struct usb_phy phy;
+	struct device *dev;
+
+	struct phy_cfg *phy_cfg;
+	int num_phy;
+	struct phy_parameter *phy_parameter;
+
+	struct dentry *debug_dir;
+};
+
+/* mapping 0xE0 to 0 ... 0xE7 to 7, 0xF0 to 8 ,,, 0xF7 to 15 */
+static inline int page_addr_to_array_index(u8 addr)
+{
+	return (int)((((addr) - PAGE_START) & 0x7) +
+		((((addr) - PAGE_START) & 0x10) >> 1));
+}
+
+static inline u8 array_index_to_page_addr(int index)
+{
+	return ((((index) + PAGE_START) & 0x7) +
+		((((index) & 0x8) << 1) + PAGE_START));
+}
+
+#define PHY_IO_TIMEOUT_USEC		(50000)
+#define PHY_IO_DELAY_US			(100)
+
+static inline int utmi_wait_register(void __iomem *reg, u32 mask, u32 result)
+{
+	int ret;
+	unsigned int val;
+
+	ret = read_poll_timeout(readl, val, ((val & mask) == result),
+				PHY_IO_DELAY_US, PHY_IO_TIMEOUT_USEC, false, reg);
+	if (ret) {
+		pr_err("%s can't program USB phy\n", __func__);
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static char rtk_phy_read(struct phy_reg *phy_reg, char addr)
+{
+	void __iomem *reg_gusb2phyacc0 = phy_reg->reg_gusb2phyacc0;
+	unsigned int val;
+	int ret = 0;
+
+	addr -= OFFEST_PHY_READ;
+
+	/* polling until VBusy == 0 */
+	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	if (ret)
+		return (char)ret;
+
+	/* VCtrl = low nibble of addr, and set PHY_NEW_REG_REQ */
+	val = PHY_NEW_REG_REQ | (GET_LOW_NIBBLE(addr) << PHY_VCTRL_SHIFT);
+	writel(val, reg_gusb2phyacc0);
+	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	if (ret)
+		return (char)ret;
+
+	/* VCtrl = high nibble of addr, and set PHY_NEW_REG_REQ */
+	val = PHY_NEW_REG_REQ | (GET_HIGH_NIBBLE(addr) << PHY_VCTRL_SHIFT);
+	writel(val, reg_gusb2phyacc0);
+	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	if (ret)
+		return (char)ret;
+
+	val = readl(reg_gusb2phyacc0);
+
+	return (char)(val & PHY_REG_DATA_MASK);
+}
+
+static int rtk_phy_write(struct phy_reg *phy_reg, char addr, char data)
+{
+	unsigned int val;
+	void __iomem *reg_wrap_vstatus = phy_reg->reg_wrap_vstatus;
+	void __iomem *reg_gusb2phyacc0 = phy_reg->reg_gusb2phyacc0;
+	int shift_bits = phy_reg->vstatus_index * 8;
+	int ret = 0;
+
+	/* write data to VStatusOut2 (data output to phy) */
+	writel((u32)data << shift_bits, reg_wrap_vstatus);
+
+	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	if (ret)
+		return ret;
+
+	/* VCtrl = low nibble of addr, set PHY_NEW_REG_REQ */
+	val = PHY_NEW_REG_REQ | (GET_LOW_NIBBLE(addr) << PHY_VCTRL_SHIFT);
+
+	writel(val, reg_gusb2phyacc0);
+	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	if (ret)
+		return ret;
+
+	/* VCtrl = high nibble of addr, set PHY_NEW_REG_REQ */
+	val = PHY_NEW_REG_REQ | (GET_HIGH_NIBBLE(addr) << PHY_VCTRL_SHIFT);
+
+	writel(val, reg_gusb2phyacc0);
+	ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int rtk_phy_set_page(struct phy_reg *phy_reg, int page)
+{
+	switch (page) {
+	case 0:
+		return rtk_phy_write(phy_reg, SET_PAGE_OFFSET, SET_PAGE_0);
+	case 1:
+		return rtk_phy_write(phy_reg, SET_PAGE_OFFSET, SET_PAGE_1);
+	case 2:
+		return rtk_phy_write(phy_reg, SET_PAGE_OFFSET, SET_PAGE_2);
+	default:
+		pr_err("%s error page=%d\n", __func__, page);
+	}
+
+	return -1;
+}
+
+static u8 __updated_dc_disconnect_level_page0_0xe4(struct phy_cfg *phy_cfg,
+						   struct phy_parameter *phy_parameter, u8 data)
+{
+	u8 val;
+	s32 __val;
+	s32 dc_disconnect_mask = phy_cfg->dc_disconnect_mask;
+	int offset = 4;
+
+	__val = (s32)((data >> offset) & dc_disconnect_mask)
+		     + phy_parameter->efuse_usb_dc_dis
+		     + phy_parameter->disconnection_compensate;
+
+	if (__val > dc_disconnect_mask)
+		__val = dc_disconnect_mask;
+	else if (__val < 0)
+		__val = 0;
+
+	val = (data & (~(dc_disconnect_mask << offset))) |
+		    (__val & dc_disconnect_mask) << offset;
+
+	return val;
+}
+
+/* updated disconnect level at page0 */
+static void update_dc_disconnect_level_at_page0(struct rtk_phy *rtk_phy,
+						struct phy_parameter *phy_parameter, bool update)
+{
+	struct phy_cfg *phy_cfg;
+	struct phy_reg *phy_reg;
+	struct phy_data *phy_data_page;
+	struct phy_data *phy_data;
+	u8 addr, data;
+	int offset = 4;
+	s32 dc_disconnect_mask;
+	int i;
+
+	phy_cfg = rtk_phy->phy_cfg;
+	phy_reg = &phy_parameter->phy_reg;
+
+	/* Set page 0 */
+	phy_data_page = phy_cfg->page0;
+	rtk_phy_set_page(phy_reg, 0);
+
+	i = page_addr_to_array_index(PAGE0_0XE4);
+	phy_data = phy_data_page + i;
+	if (!phy_data->addr) {
+		phy_data->addr = PAGE0_0XE4;
+		phy_data->data = rtk_phy_read(phy_reg, PAGE0_0XE4);
+	}
+
+	addr = phy_data->addr;
+	data = phy_data->data;
+	dc_disconnect_mask = phy_cfg->dc_disconnect_mask;
+
+	if (update)
+		data = __updated_dc_disconnect_level_page0_0xe4(phy_cfg, phy_parameter, data);
+	else
+		data = (data & ~(dc_disconnect_mask << offset)) |
+			(DEFAULT_DC_DISCONNECTION_VALUE << offset);
+
+	if (rtk_phy_write(phy_reg, addr, data))
+		dev_err(rtk_phy->dev,
+			"[%s:%d] Error page1 addr=0x%x value=0x%x\n",
+			__func__, __LINE__,
+			addr, data);
+}
+
+static u8 __updated_dc_disconnect_level_page1_0xe2(struct phy_cfg *phy_cfg,
+						   struct phy_parameter *phy_parameter, u8 data)
+{
+	u8 val;
+	s32 __val;
+	s32 dc_disconnect_mask = phy_cfg->dc_disconnect_mask;
+
+	if (phy_cfg->check_efuse_version == CHECK_EFUSE_V1) {
+		__val = (s32)(data & dc_disconnect_mask)
+			    + phy_parameter->efuse_usb_dc_dis
+			    + phy_parameter->disconnection_compensate;
+	} else { /* for CHECK_EFUSE_V2 or no efuse */
+		if (phy_parameter->efuse_usb_dc_dis)
+			__val = (s32)(phy_parameter->efuse_usb_dc_dis +
+				    phy_parameter->disconnection_compensate);
+		else
+			__val = (s32)((data & dc_disconnect_mask) +
+				    phy_parameter->disconnection_compensate);
+	}
+
+	if (__val > dc_disconnect_mask)
+		__val = dc_disconnect_mask;
+	else if (__val < 0)
+		__val = 0;
+
+	val = (data & (~dc_disconnect_mask)) | (__val & dc_disconnect_mask);
+
+	return val;
+}
+
+/* updated disconnect level at page1 */
+static void update_dc_disconnect_level_at_page1(struct rtk_phy *rtk_phy,
+						struct phy_parameter *phy_parameter, bool update)
+{
+	struct phy_cfg *phy_cfg;
+	struct phy_data *phy_data_page;
+	struct phy_data *phy_data;
+	struct phy_reg *phy_reg;
+	u8 addr, data;
+	s32 dc_disconnect_mask;
+	int i;
+
+	phy_cfg = rtk_phy->phy_cfg;
+	phy_reg = &phy_parameter->phy_reg;
+
+	/* Set page 1 */
+	phy_data_page = phy_cfg->page1;
+	rtk_phy_set_page(phy_reg, 1);
+
+	i = page_addr_to_array_index(PAGE1_0XE2);
+	phy_data = phy_data_page + i;
+	if (!phy_data->addr) {
+		phy_data->addr = PAGE1_0XE2;
+		phy_data->data = rtk_phy_read(phy_reg, PAGE1_0XE2);
+	}
+
+	addr = phy_data->addr;
+	data = phy_data->data;
+	dc_disconnect_mask = phy_cfg->dc_disconnect_mask;
+
+	if (update)
+		data = __updated_dc_disconnect_level_page1_0xe2(phy_cfg, phy_parameter, data);
+	else
+		data = (data & ~dc_disconnect_mask) | DEFAULT_DC_DISCONNECTION_VALUE;
+
+	if (rtk_phy_write(phy_reg, addr, data))
+		dev_err(rtk_phy->dev,
+			"[%s:%d] Error page1 addr=0x%x value=0x%x\n",
+			__func__, __LINE__,
+			addr, data);
+}
+
+static void update_dc_disconnect_level(struct rtk_phy *rtk_phy,
+				       struct phy_parameter *phy_parameter, bool update)
+{
+	struct phy_cfg *phy_cfg = rtk_phy->phy_cfg;
+
+	if (phy_cfg->usb_dc_disconnect_at_page0)
+		update_dc_disconnect_level_at_page0(rtk_phy, phy_parameter, update);
+	else
+		update_dc_disconnect_level_at_page1(rtk_phy, phy_parameter, update);
+}
+
+static u8 __update_dc_driving_page0_0xe4(struct phy_cfg *phy_cfg,
+					 struct phy_parameter *phy_parameter, u8 data)
+{
+	s32 driving_compensate = phy_parameter->driving_compensate;
+	s32 dc_driving_mask = phy_cfg->dc_driving_mask;
+	s32 __val;
+	u8 val;
+
+	if (phy_cfg->check_efuse_version == CHECK_EFUSE_V1) {
+		__val = (s32)(data & dc_driving_mask) + driving_compensate
+			    + phy_parameter->efuse_usb_dc_cal;
+	} else { /* for CHECK_EFUSE_V2 or no efuse */
+		if (phy_parameter->efuse_usb_dc_cal)
+			__val = (s32)((phy_parameter->efuse_usb_dc_cal & dc_driving_mask)
+				    + driving_compensate);
+		else
+			__val = (s32)(data & dc_driving_mask);
+	}
+
+	if (__val > dc_driving_mask)
+		__val = dc_driving_mask;
+	else if (__val < 0)
+		__val = 0;
+
+	val = (data & (~dc_driving_mask)) | (__val & dc_driving_mask);
+
+	return val;
+}
+
+static void update_dc_driving_level(struct rtk_phy *rtk_phy,
+				    struct phy_parameter *phy_parameter)
+{
+	struct phy_cfg *phy_cfg;
+	struct phy_reg *phy_reg;
+
+	phy_reg = &phy_parameter->phy_reg;
+	phy_cfg = rtk_phy->phy_cfg;
+	if (!phy_cfg->page0[4].addr) {
+		rtk_phy_set_page(phy_reg, 0);
+		phy_cfg->page0[4].addr = PAGE0_0XE4;
+		phy_cfg->page0[4].data = rtk_phy_read(phy_reg, PAGE0_0XE4);
+	}
+
+	if (phy_parameter->driving_level != DEFAULT_DC_DRIVING_VALUE) {
+		u32 dc_driving_mask;
+		u8 driving_level;
+		u8 data;
+
+		data = phy_cfg->page0[4].data;
+		dc_driving_mask = phy_cfg->dc_driving_mask;
+		driving_level = data & dc_driving_mask;
+
+		dev_dbg(rtk_phy->dev, "%s driving_level=%d => dts driving_level=%d\n",
+			__func__, driving_level, phy_parameter->driving_level);
+
+		phy_cfg->page0[4].data = (data & (~dc_driving_mask)) |
+			    (phy_parameter->driving_level & dc_driving_mask);
+	}
+
+	phy_cfg->page0[4].data = __update_dc_driving_page0_0xe4(phy_cfg,
+								phy_parameter,
+								phy_cfg->page0[4].data);
+}
+
+static void update_hs_clk_select(struct rtk_phy *rtk_phy,
+				 struct phy_parameter *phy_parameter)
+{
+	struct phy_cfg *phy_cfg;
+	struct phy_reg *phy_reg;
+
+	phy_cfg = rtk_phy->phy_cfg;
+	phy_reg = &phy_parameter->phy_reg;
+
+	if (phy_parameter->inverse_hstx_sync_clock) {
+		if (!phy_cfg->page0[6].addr) {
+			rtk_phy_set_page(phy_reg, 0);
+			phy_cfg->page0[6].addr = PAGE0_0XE6;
+			phy_cfg->page0[6].data = rtk_phy_read(phy_reg, PAGE0_0XE6);
+		}
+
+		phy_cfg->page0[6].data = phy_cfg->page0[6].data | HS_CLK_SELECT;
+	}
+}
+
+static void do_rtk_phy_toggle(struct rtk_phy *rtk_phy,
+			      int index, bool connect)
+{
+	struct phy_parameter *phy_parameter;
+	struct phy_cfg *phy_cfg;
+	struct phy_reg *phy_reg;
+	struct phy_data *phy_data_page;
+	u8 addr, data;
+	int i;
+
+	phy_cfg = rtk_phy->phy_cfg;
+	phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
+	phy_reg = &phy_parameter->phy_reg;
+
+	if (!phy_cfg->do_toggle)
+		goto out;
+
+	if (phy_cfg->is_double_sensitivity_mode)
+		goto do_toggle_driving;
+
+	/* Set page 0 */
+	rtk_phy_set_page(phy_reg, 0);
+
+	addr = PAGE0_0XE7;
+	data = rtk_phy_read(phy_reg, addr);
+
+	if (connect)
+		rtk_phy_write(phy_reg, addr, data & (~SENSITIVITY_CTRL));
+	else
+		rtk_phy_write(phy_reg, addr, data | (SENSITIVITY_CTRL));
+
+do_toggle_driving:
+
+	if (!phy_cfg->do_toggle_driving)
+		goto do_toggle;
+
+	/* Page 0 addr 0xE4 driving capability */
+
+	/* Set page 0 */
+	phy_data_page = phy_cfg->page0;
+	rtk_phy_set_page(phy_reg, 0);
+
+	i = page_addr_to_array_index(PAGE0_0XE4);
+	addr = phy_data_page[i].addr;
+	data = phy_data_page[i].data;
+
+	if (connect) {
+		rtk_phy_write(phy_reg, addr, data);
+	} else {
+		u8 val;
+		s32 __val;
+		s32 driving_updated =
+			    phy_cfg->driving_updated_for_dev_dis;
+		s32 dc_driving_mask = phy_cfg->dc_driving_mask;
+
+		__val = (s32)(data & dc_driving_mask) + driving_updated;
+
+		if (__val > dc_driving_mask)
+			__val = dc_driving_mask;
+		else if (__val < 0)
+			__val = 0;
+
+		val = (data & (~dc_driving_mask)) | (__val & dc_driving_mask);
+
+		rtk_phy_write(phy_reg, addr, val);
+	}
+
+do_toggle:
+	/* restore dc disconnect level before toggle */
+	update_dc_disconnect_level(rtk_phy, phy_parameter, false);
+
+	/* Set page 1 */
+	rtk_phy_set_page(phy_reg, 1);
+
+	addr = PAGE1_0XE0;
+	data = rtk_phy_read(phy_reg, addr);
+
+	rtk_phy_write(phy_reg, addr, data &
+		      (~ENABLE_AUTO_SENSITIVITY_CALIBRATION));
+	mdelay(1);
+	rtk_phy_write(phy_reg, addr, data |
+		      (ENABLE_AUTO_SENSITIVITY_CALIBRATION));
+
+	/* update dc disconnect level after toggle */
+	update_dc_disconnect_level(rtk_phy, phy_parameter, true);
+
+out:
+	return;
+}
+
+static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
+{
+	struct phy_parameter *phy_parameter;
+	struct phy_cfg *phy_cfg;
+	struct phy_data *phy_data_page;
+	struct phy_reg *phy_reg;
+	int i;
+
+	phy_cfg = rtk_phy->phy_cfg;
+	phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
+	phy_reg = &phy_parameter->phy_reg;
+
+	if (phy_cfg->use_default_parameter) {
+		dev_dbg(rtk_phy->dev, "%s phy#%d use default parameter\n",
+			__func__, index);
+		goto do_toggle;
+	}
+
+	/* Set page 0 */
+	phy_data_page = phy_cfg->page0;
+	rtk_phy_set_page(phy_reg, 0);
+
+	for (i = 0; i < phy_cfg->page0_size; i++) {
+		struct phy_data *phy_data = phy_data_page + i;
+		u8 addr = phy_data->addr;
+		u8 data = phy_data->data;
+
+		if (!addr)
+			continue;
+
+		if (rtk_phy_write(phy_reg, addr, data)) {
+			dev_err(rtk_phy->dev,
+				"[%s:%d] Error page0 addr=0x%x value=0x%x\n",
+				__func__, __LINE__, addr, data);
+			return -EINVAL;
+		}
+	}
+
+	/* Set page 1 */
+	phy_data_page = phy_cfg->page1;
+	rtk_phy_set_page(phy_reg, 1);
+
+	for (i = 0; i < phy_cfg->page1_size; i++) {
+		struct phy_data *phy_data = phy_data_page + i;
+		u8 addr = phy_data->addr;
+		u8 data = phy_data->data;
+
+		if (!addr)
+			continue;
+
+		if (rtk_phy_write(phy_reg, addr, data)) {
+			dev_err(rtk_phy->dev,
+				"[%s:%d] Error page1 addr=0x%x value=0x%x\n",
+				__func__, __LINE__,
+				addr, data);
+			return -EINVAL;
+		}
+	}
+
+	if (phy_cfg->page2_size == 0)
+		goto do_toggle;
+
+	/* Set page 2 */
+	phy_data_page = phy_cfg->page2;
+	rtk_phy_set_page(phy_reg, 2);
+
+	for (i = 0; i < phy_cfg->page2_size; i++) {
+		struct phy_data *phy_data = phy_data_page + i;
+		u8 addr = phy_data->addr;
+		u8 data = phy_data->data;
+
+		if (!addr)
+			continue;
+
+		if (rtk_phy_write(phy_reg, addr, data)) {
+			dev_err(rtk_phy->dev,
+				"[%s:%d] Error page2 addr=0x%x value=0x%x\n",
+				__func__, __LINE__, addr, data);
+			return -1;
+		}
+		dev_dbg(rtk_phy->dev, "[%s:%d] Good page2 addr=0x%x value=0x%x\n",
+			__func__, __LINE__, addr,
+			rtk_phy_read(phy_reg, addr));
+	}
+
+do_toggle:
+	do_rtk_phy_toggle(rtk_phy, index, false);
+
+	return 0;
+}
+
+static int rtk_phy_init(struct phy *phy)
+{
+	struct rtk_phy *rtk_phy = phy_get_drvdata(phy);
+	unsigned long phy_init_time = jiffies;
+	int i, ret = 0;
+
+	if (!rtk_phy)
+		return -EINVAL;
+
+	for (i = 0; i < rtk_phy->num_phy; i++)
+		ret = do_rtk_phy_init(rtk_phy, i);
+
+	dev_dbg(rtk_phy->dev, "Initialized RTK USB 2.0 PHY (take %dms)\n",
+		jiffies_to_msecs(jiffies - phy_init_time));
+	return ret;
+}
+
+static int rtk_phy_exit(struct phy *phy)
+{
+	return 0;
+}
+
+static const struct phy_ops ops = {
+	.init		= rtk_phy_init,
+	.exit		= rtk_phy_exit,
+	.owner		= THIS_MODULE,
+};
+
+static void rtk_phy_toggle(struct usb_phy *usb2_phy, bool connect, int port)
+{
+	int index = port;
+	struct rtk_phy *rtk_phy = NULL;
+
+	rtk_phy = dev_get_drvdata(usb2_phy->dev);
+
+	if (index > rtk_phy->num_phy) {
+		pr_err("%s %d ERROR! port=%d > num_phy=%d\n",
+		       __func__, __LINE__, index, rtk_phy->num_phy);
+		return;
+	}
+
+	do_rtk_phy_toggle(rtk_phy, index, connect);
+}
+
+static int rtk_phy_notify_port_status(struct usb_phy *x, int port,
+				      u16 portstatus, u16 portchange)
+{
+	bool connect = false;
+
+	pr_debug("%s port=%d portstatus=0x%x portchange=0x%x\n",
+		 __func__, port, (int)portstatus, (int)portchange);
+	if (portstatus & USB_PORT_STAT_CONNECTION)
+		connect = true;
+
+	if (portchange & USB_PORT_STAT_C_CONNECTION)
+		rtk_phy_toggle(x, connect, port);
+
+	return 0;
+}
+
+#ifdef CONFIG_DEBUG_FS
+static struct dentry *create_phy_debug_root(void)
+{
+	struct dentry *phy_debug_root;
+
+	phy_debug_root = debugfs_lookup("phy", usb_debug_root);
+	if (!phy_debug_root)
+		phy_debug_root = debugfs_create_dir("phy", usb_debug_root);
+
+	return phy_debug_root;
+}
+
+static int rtk_usb2_parameter_show(struct seq_file *s, void *unused)
+{
+	struct rtk_phy *rtk_phy = s->private;
+	struct phy_cfg *phy_cfg;
+	int i, index;
+
+	phy_cfg = rtk_phy->phy_cfg;
+
+	seq_puts(s, "Property:\n");
+	seq_printf(s, "  check_efuse: %s\n",
+		   phy_cfg->check_efuse ? "Enable" : "Disable");
+	seq_printf(s, "  check_efuse_version: %d\n",
+		   phy_cfg->check_efuse_version);
+	seq_printf(s, "  efuse_dc_driving_rate: %d\n",
+		   phy_cfg->efuse_dc_driving_rate);
+	seq_printf(s, "  dc_driving_mask: 0x%x\n",
+		   phy_cfg->dc_driving_mask);
+	seq_printf(s, "  efuse_dc_disconnect_rate: %d\n",
+		   phy_cfg->efuse_dc_disconnect_rate);
+	seq_printf(s, "  dc_disconnect_mask: 0x%x\n",
+		   phy_cfg->dc_disconnect_mask);
+	seq_printf(s, "  usb_dc_disconnect_at_page0: %s\n",
+		   phy_cfg->usb_dc_disconnect_at_page0 ? "true" : "false");
+	seq_printf(s, "  do_toggle: %s\n",
+		   phy_cfg->do_toggle ? "Enable" : "Disable");
+	seq_printf(s, "  do_toggle_driving: %s\n",
+		   phy_cfg->do_toggle_driving ? "Enable" : "Disable");
+	seq_printf(s, "  driving_updated_for_dev_dis: 0x%x\n",
+		   phy_cfg->driving_updated_for_dev_dis);
+	seq_printf(s, "  use_default_parameter: %s\n",
+		   phy_cfg->use_default_parameter ? "Enable" : "Disable");
+	seq_printf(s, "  is_double_sensitivity_mode: %s\n",
+		   phy_cfg->is_double_sensitivity_mode ? "Enable" : "Disable");
+
+	for (index = 0; index < rtk_phy->num_phy; index++) {
+		struct phy_parameter *phy_parameter;
+		struct phy_reg *phy_reg;
+		struct phy_data *phy_data_page;
+
+		phy_parameter =  &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
+		phy_reg = &phy_parameter->phy_reg;
+
+		seq_printf(s, "PHY %d:\n", index);
+
+		seq_puts(s, "Page 0:\n");
+		/* Set page 0 */
+		phy_data_page = phy_cfg->page0;
+		rtk_phy_set_page(phy_reg, 0);
+
+		for (i = 0; i < phy_cfg->page0_size; i++) {
+			struct phy_data *phy_data = phy_data_page + i;
+			u8 addr = array_index_to_page_addr(i);
+			u8 data = phy_data->data;
+			u8 value = rtk_phy_read(phy_reg, addr);
+
+			if (phy_data->addr)
+				seq_printf(s, "  Page 0: addr=0x%x data=0x%02x ==> read value=0x%02x\n",
+					   addr, data, value);
+			else
+				seq_printf(s, "  Page 0: addr=0x%x data=none ==> read value=0x%02x\n",
+					   addr, value);
+		}
+
+		seq_puts(s, "Page 1:\n");
+		/* Set page 1 */
+		phy_data_page = phy_cfg->page1;
+		rtk_phy_set_page(phy_reg, 1);
+
+		for (i = 0; i < phy_cfg->page1_size; i++) {
+			struct phy_data *phy_data = phy_data_page + i;
+			u8 addr = array_index_to_page_addr(i);
+			u8 data = phy_data->data;
+			u8 value = rtk_phy_read(phy_reg, addr);
+
+			if (phy_data->addr)
+				seq_printf(s, "  Page 1: addr=0x%x data=0x%02x ==> read value=0x%02x\n",
+					   addr, data, value);
+			else
+				seq_printf(s, "  Page 1: addr=0x%x data=none ==> read value=0x%02x\n",
+					   addr, value);
+		}
+
+		if (phy_cfg->page2_size == 0)
+			goto out;
+
+		seq_puts(s, "Page 2:\n");
+		/* Set page 2 */
+		phy_data_page = phy_cfg->page2;
+		rtk_phy_set_page(phy_reg, 2);
+
+		for (i = 0; i < phy_cfg->page2_size; i++) {
+			struct phy_data *phy_data = phy_data_page + i;
+			u8 addr = array_index_to_page_addr(i);
+			u8 data = phy_data->data;
+			u8 value = rtk_phy_read(phy_reg, addr);
+
+			if (phy_data->addr)
+				seq_printf(s, "  Page 2: addr=0x%x data=0x%02x ==> read value=0x%02x\n",
+					   addr, data, value);
+			else
+				seq_printf(s, "  Page 2: addr=0x%x data=none ==> read value=0x%02x\n",
+					   addr, value);
+		}
+
+out:
+		seq_puts(s, "PHY Property:\n");
+		seq_printf(s, "  efuse_usb_dc_cal: %d\n",
+			   (int)phy_parameter->efuse_usb_dc_cal);
+		seq_printf(s, "  efuse_usb_dc_dis: %d\n",
+			   (int)phy_parameter->efuse_usb_dc_dis);
+		seq_printf(s, "  inverse_hstx_sync_clock: %s\n",
+			   phy_parameter->inverse_hstx_sync_clock ? "Enable" : "Disable");
+		seq_printf(s, "  driving_level: %d\n",
+			   phy_parameter->driving_level);
+		seq_printf(s, "  driving_compensate: %d\n",
+			   phy_parameter->driving_compensate);
+		seq_printf(s, "  disconnection_compensate: %d\n",
+			   phy_parameter->disconnection_compensate);
+	}
+
+	return 0;
+}
+
+static int rtk_usb2_parameter_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, rtk_usb2_parameter_show, inode->i_private);
+}
+
+static const struct file_operations rtk_usb2_parameter_fops = {
+	.open			= rtk_usb2_parameter_open,
+	.read			= seq_read,
+	.llseek			= seq_lseek,
+	.release		= single_release,
+};
+
+static int __get_parameter_at_page(struct seq_file *s,
+				   struct rtk_phy *rtk_phy,
+				   struct phy_data *phy_data_page,
+				   const char *phy_page, const char *phy_addr)
+{
+	struct phy_data *phy_data;
+	u32 addr;
+	int i, ret;
+
+	ret = kstrtouint(phy_addr, 16, &addr);
+	if (ret < 0) {
+		pr_err("%s::kstrtouint() failed\n", __func__);
+		return -EINVAL;
+	}
+	i = page_addr_to_array_index(addr);
+	phy_data = (phy_data_page + i);
+
+	if (phy_data->addr)
+		seq_printf(s, "Now Parameter %s addr 0x%02x = 0x%02x\n",
+			   phy_page, phy_data->addr, phy_data->data);
+	else
+		seq_printf(s, "Now Parameter %s addr 0x%02x is default\n",
+			   phy_page, addr);
+
+	return 0;
+}
+
+static int __set_parameter_at_page(struct rtk_phy *rtk_phy,
+				   struct phy_reg *phy_reg, struct phy_parameter *phy_parameter,
+				   struct phy_data *phy_data_page,
+				   const char *phy_page, const char *phy_addr,
+				   const char *phy_value)
+{
+	struct phy_data *phy_data;
+	u32 addr, value;
+	int i, ret;
+
+	ret = kstrtouint(phy_addr, 16, &addr);
+	if (ret < 0)
+		return -EINVAL;
+
+	ret = kstrtouint(phy_value, 16, &value);
+	if (ret < 0)
+		return -EINVAL;
+
+	i = page_addr_to_array_index(addr);
+	phy_data = (phy_data_page + i);
+
+	if (phy_data->addr) {
+		phy_data->data = value;
+	} else {
+		phy_data->addr = addr;
+		phy_data->data = value;
+	}
+
+	if (rtk_phy_write(phy_reg, addr, value))
+		dev_err(rtk_phy->dev,
+			"[%s:%d] Error: addr=0x%02x value=0x%02x\n",
+			__func__, __LINE__, addr, value);
+
+	return 0;
+}
+
+static int rtk_usb2_set_parameter_show(struct seq_file *s, void *unused)
+{
+	const struct file *file = s->file;
+	const char *file_name = file_dentry(file)->d_iname;
+	struct dentry *p_dentry = file_dentry(file)->d_parent;
+	const char *dir_name = p_dentry->d_iname;
+	struct dentry *pp_dentry = p_dentry->d_parent;
+	const char *phy_dir_name = pp_dentry->d_iname;
+	struct rtk_phy *rtk_phy = s->private;
+	struct phy_cfg *phy_cfg;
+	struct phy_parameter *phy_parameter = NULL;
+	int ret = 0;
+	int index;
+
+	for (index = 0; index < rtk_phy->num_phy; index++) {
+		size_t sz = 30;
+		char name[30] = {0};
+
+		snprintf(name, sz, "phy%d", index);
+		if (strncmp(name, phy_dir_name, strlen(name)) == 0) {
+			phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
+			break;
+		}
+	}
+
+	if (!phy_parameter)
+		return -EINVAL;
+
+	phy_cfg = rtk_phy->phy_cfg;
+
+	if (strcmp("page0", dir_name) == 0)
+		ret = __get_parameter_at_page(s, rtk_phy, phy_cfg->page0,
+					      dir_name, file_name);
+	else if (strcmp("page1", dir_name) == 0)
+		ret = __get_parameter_at_page(s, rtk_phy, phy_cfg->page1,
+					      dir_name, file_name);
+	else if (strcmp("page2", dir_name) == 0)
+		ret = __get_parameter_at_page(s, rtk_phy, phy_cfg->page2,
+					      dir_name, file_name);
+
+	if (ret < 0)
+		return ret;
+
+	seq_puts(s, "Set phy parameter by following command\n");
+	seq_printf(s, "echo \"value\" > %s/%s/%s\n",
+		   phy_dir_name, dir_name, file_name);
+
+	return 0;
+}
+
+static int rtk_usb2_set_parameter_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, rtk_usb2_set_parameter_show, inode->i_private);
+}
+
+static ssize_t rtk_usb2_set_parameter_write(struct file *file,
+					    const char __user *ubuf, size_t count,
+					    loff_t *ppos)
+{
+	const char *file_name = file_dentry(file)->d_iname;
+	struct dentry *p_dentry = file_dentry(file)->d_parent;
+	const char *dir_name = p_dentry->d_iname;
+	struct dentry *pp_dentry = p_dentry->d_parent;
+	const char *phy_dir_name = pp_dentry->d_iname;
+	struct seq_file *s = file->private_data;
+	struct rtk_phy *rtk_phy = s->private;
+	struct phy_reg *phy_reg;
+	struct phy_cfg *phy_cfg;
+	struct phy_parameter *phy_parameter = NULL;
+	int ret = 0;
+	char buffer[40] = {0};
+	int index;
+
+	if (copy_from_user(&buffer, ubuf,
+			   min_t(size_t, sizeof(buffer) - 1, count)))
+		return -EFAULT;
+
+	for (index = 0; index < rtk_phy->num_phy; index++) {
+		size_t sz = 30;
+		char name[30] = {0};
+
+		snprintf(name, sz, "phy%d", index);
+		if (strncmp(name, phy_dir_name, strlen(name)) == 0) {
+			phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
+			break;
+		}
+	}
+
+	if (!phy_parameter)
+		return -EINVAL;
+
+	phy_cfg = rtk_phy->phy_cfg;
+	phy_reg = &phy_parameter->phy_reg;
+
+	if (strcmp("page0", dir_name) == 0) {
+		rtk_phy_set_page(phy_reg, 0);
+		ret = __set_parameter_at_page(rtk_phy, phy_reg, phy_parameter,
+					      phy_cfg->page0, dir_name, file_name, buffer);
+	} else if (strcmp("page1", dir_name) == 0) {
+		rtk_phy_set_page(phy_reg, 1);
+		ret = __set_parameter_at_page(rtk_phy, phy_reg, phy_parameter,
+					      phy_cfg->page1, dir_name, file_name, buffer);
+	} else if (strcmp("page2", dir_name) == 0) {
+		rtk_phy_set_page(phy_reg, 2);
+		ret = __set_parameter_at_page(rtk_phy, phy_reg, phy_parameter,
+					      phy_cfg->page2, dir_name, file_name, buffer);
+	}
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static const struct file_operations rtk_usb2_set_parameter_fops = {
+	.open			= rtk_usb2_set_parameter_open,
+	.write			= rtk_usb2_set_parameter_write,
+	.read			= seq_read,
+	.llseek			= seq_lseek,
+	.release		= single_release,
+};
+
+static int create_debug_set_parameter_files(struct rtk_phy *rtk_phy,
+					    struct dentry *phy_dir,
+					    const char *page, size_t addr_size)
+{
+	struct dentry *page_dir;
+	int i;
+
+	page_dir = debugfs_create_dir(page, phy_dir);
+	if (!page_dir)
+		return -EINVAL;
+
+	for (i = 0; i < addr_size; i++) {
+		size_t sz = 30;
+		char name[30] = {0};
+
+		snprintf(name, sz, "%x", array_index_to_page_addr(i));
+
+		debugfs_create_file(name, 0644,
+				    page_dir, rtk_phy,
+				    &rtk_usb2_set_parameter_fops);
+	}
+
+	return 0;
+}
+
+static inline void create_debug_files(struct rtk_phy *rtk_phy)
+{
+	struct dentry *phy_debug_root = NULL;
+	struct dentry *set_parameter_dir = NULL;
+
+	phy_debug_root = create_phy_debug_root();
+	if (!phy_debug_root)
+		return;
+
+	rtk_phy->debug_dir = debugfs_create_dir(dev_name(rtk_phy->dev),
+						phy_debug_root);
+	if (!rtk_phy->debug_dir)
+		return;
+
+	if (!debugfs_create_file("parameter", 0444, rtk_phy->debug_dir, rtk_phy,
+				 &rtk_usb2_parameter_fops))
+		goto file_error;
+
+	set_parameter_dir = debugfs_create_dir("set_parameter",
+					       rtk_phy->debug_dir);
+	if (set_parameter_dir) {
+		struct phy_cfg *phy_cfg;
+		int index, ret;
+
+		phy_cfg = rtk_phy->phy_cfg;
+
+		for (index = 0; index < rtk_phy->num_phy; index++) {
+			struct dentry *phy_dir;
+			size_t sz = 30;
+			char name[30] = {0};
+
+			snprintf(name, sz, "phy%d", index);
+
+			phy_dir = debugfs_create_dir(name, set_parameter_dir);
+			if (!phy_dir)
+				goto file_error;
+
+			ret = create_debug_set_parameter_files(rtk_phy, phy_dir,
+							       "page0", phy_cfg->page0_size);
+			if (ret < 0)
+				goto file_error;
+
+			ret = create_debug_set_parameter_files(rtk_phy, phy_dir,
+							       "page1", phy_cfg->page1_size);
+			if (ret < 0)
+				goto file_error;
+
+			ret = create_debug_set_parameter_files(rtk_phy, phy_dir,
+							       "page2", phy_cfg->page2_size);
+			if (ret < 0)
+				goto file_error;
+		}
+	}
+
+	return;
+
+file_error:
+	debugfs_remove_recursive(rtk_phy->debug_dir);
+}
+
+static inline void remove_debug_files(struct rtk_phy *rtk_phy)
+{
+	debugfs_remove_recursive(rtk_phy->debug_dir);
+}
+#else
+static inline void create_debug_files(struct rtk_phy *rtk_phy) { }
+static inline void remove_debug_files(struct rtk_phy *rtk_phy) { }
+#endif /* CONFIG_DEBUG_FS */
+
+static int get_phy_data_by_efuse(struct rtk_phy *rtk_phy,
+				 struct phy_parameter *phy_parameter, int index)
+{
+	struct phy_cfg *phy_cfg = rtk_phy->phy_cfg;
+	u8 value = 0;
+	struct nvmem_cell *cell;
+	struct soc_device_attribute rtk_soc_groot[] = {
+		    { .family = "Realtek Groot",},
+		    { /* empty */ } };
+
+	if (!phy_cfg->check_efuse)
+		goto out;
+
+	/* Read efuse for usb dc cal */
+	cell = nvmem_cell_get(rtk_phy->dev, "usb-dc-cal");
+	if (IS_ERR(cell)) {
+		dev_dbg(rtk_phy->dev, "%s no usb-dc-cal: %ld\n",
+			__func__, PTR_ERR(cell));
+	} else {
+		unsigned char *buf;
+		size_t buf_size;
+
+		buf = nvmem_cell_read(cell, &buf_size);
+		value = buf[0] & phy_cfg->dc_driving_mask;
+
+		kfree(buf);
+		nvmem_cell_put(cell);
+	}
+
+	if (phy_cfg->check_efuse_version == CHECK_EFUSE_V1) {
+		int rate = phy_cfg->efuse_dc_driving_rate;
+
+		if (value <= EFUS_USB_DC_CAL_MAX)
+			phy_parameter->efuse_usb_dc_cal = (int8_t)(value * rate);
+		else
+			phy_parameter->efuse_usb_dc_cal = -(int8_t)
+				    ((EFUS_USB_DC_CAL_MAX & value) * rate);
+
+		if (soc_device_match(rtk_soc_groot)) {
+			dev_dbg(rtk_phy->dev, "For groot IC we need a workaround to adjust efuse_usb_dc_cal\n");
+
+			/* We don't multiple dc_cal_rate=2 for positive dc cal compensate */
+			if (value <= EFUS_USB_DC_CAL_MAX)
+				phy_parameter->efuse_usb_dc_cal = (int8_t)(value);
+
+			/* We set max dc cal compensate is 0x8 if otp is 0x7 */
+			if (value == 0x7)
+				phy_parameter->efuse_usb_dc_cal = (int8_t)(value + 1);
+		}
+	} else { /* for CHECK_EFUSE_V2 */
+		phy_parameter->efuse_usb_dc_cal = value & phy_cfg->dc_driving_mask;
+	}
+
+	/* Read efuse for usb dc disconnect level */
+	value = 0;
+	cell = nvmem_cell_get(rtk_phy->dev, "usb-dc-dis");
+	if (IS_ERR(cell)) {
+		dev_dbg(rtk_phy->dev, "%s no usb-dc-dis: %ld\n",
+			__func__, PTR_ERR(cell));
+	} else {
+		unsigned char *buf;
+		size_t buf_size;
+
+		buf = nvmem_cell_read(cell, &buf_size);
+		value = buf[0] & phy_cfg->dc_disconnect_mask;
+
+		kfree(buf);
+		nvmem_cell_put(cell);
+	}
+
+	if (phy_cfg->check_efuse_version == CHECK_EFUSE_V1) {
+		int rate = phy_cfg->efuse_dc_disconnect_rate;
+
+		if (value <= EFUS_USB_DC_DIS_MAX)
+			phy_parameter->efuse_usb_dc_dis = (int8_t)(value * rate);
+		else
+			phy_parameter->efuse_usb_dc_dis = -(int8_t)
+				    ((EFUS_USB_DC_DIS_MAX & value) * rate);
+	} else { /* for CHECK_EFUSE_V2 */
+		phy_parameter->efuse_usb_dc_dis = value & phy_cfg->dc_disconnect_mask;
+	}
+
+out:
+	return 0;
+}
+
+static int parse_phy_data(struct rtk_phy *rtk_phy)
+{
+	struct device *dev = rtk_phy->dev;
+	struct device_node *node;
+	struct phy_cfg *phy_cfg;
+	struct phy_parameter *phy_parameter;
+	int ret = 0;
+	int index;
+
+	node = dev->of_node;
+	phy_cfg = rtk_phy->phy_cfg;
+
+	rtk_phy->phy_parameter = devm_kzalloc(dev, sizeof(struct phy_parameter) *
+						rtk_phy->num_phy, GFP_KERNEL);
+	if (!rtk_phy->phy_parameter)
+		return -ENOMEM;
+
+	for (index = 0; index < rtk_phy->num_phy; index++) {
+		phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
+
+		phy_parameter->phy_reg.reg_wrap_vstatus = of_iomap(dev->of_node, 0);
+		phy_parameter->phy_reg.reg_gusb2phyacc0 = of_iomap(dev->of_node, 1) + index;
+		phy_parameter->phy_reg.vstatus_index = index;
+
+		if (of_property_read_bool(node, "realtek,inverse-hstx-sync-clock"))
+			phy_parameter->inverse_hstx_sync_clock = true;
+		else
+			phy_parameter->inverse_hstx_sync_clock = false;
+
+		if (of_property_read_u32_index(node, "realtek,driving-level",
+					       index, &phy_parameter->driving_level))
+			phy_parameter->driving_level = DEFAULT_DC_DRIVING_VALUE;
+
+		if (of_property_read_u32_index(node, "realtek,driving-compensate",
+					       index, &phy_parameter->driving_compensate))
+			phy_parameter->driving_compensate = 0;
+
+		if (of_property_read_u32_index(node, "realtek,disconnection-compensate",
+					       index, &phy_parameter->disconnection_compensate))
+			phy_parameter->disconnection_compensate = 0;
+
+		get_phy_data_by_efuse(rtk_phy, phy_parameter, index);
+
+		update_dc_driving_level(rtk_phy, phy_parameter);
+
+		update_hs_clk_select(rtk_phy, phy_parameter);
+	}
+
+	return ret;
+}
+
+static int rtk_usb2phy_probe(struct platform_device *pdev)
+{
+	struct rtk_phy *rtk_phy;
+	struct device *dev = &pdev->dev;
+	struct device_node *node;
+	struct phy *generic_phy;
+	struct phy_provider *phy_provider;
+	const struct phy_cfg *phy_cfg;
+	int ret = 0;
+
+	phy_cfg = of_device_get_match_data(dev);
+	if (!phy_cfg) {
+		dev_err(dev, "phy config are not assigned!\n");
+		return -EINVAL;
+	}
+
+	rtk_phy = devm_kzalloc(dev, sizeof(*rtk_phy), GFP_KERNEL);
+	if (!rtk_phy)
+		return -ENOMEM;
+
+	rtk_phy->dev			= &pdev->dev;
+	rtk_phy->phy.dev		= rtk_phy->dev;
+	rtk_phy->phy.label		= "rtk-usb2phy";
+	rtk_phy->phy.notify_port_status = rtk_phy_notify_port_status;
+
+	rtk_phy->phy_cfg = devm_kzalloc(dev, sizeof(*phy_cfg), GFP_KERNEL);
+
+	memcpy(rtk_phy->phy_cfg, phy_cfg, sizeof(*phy_cfg));
+
+	node = dev->of_node;
+
+	if (of_device_is_compatible(node, "realtek,rtd1395-usb2phy-2port"))
+		rtk_phy->num_phy = 2;
+	else
+		rtk_phy->num_phy = 1;
+
+	ret = parse_phy_data(rtk_phy);
+	if (ret)
+		goto err;
+
+	platform_set_drvdata(pdev, rtk_phy);
+
+	generic_phy = devm_phy_create(rtk_phy->dev, NULL, &ops);
+	if (IS_ERR(generic_phy))
+		return PTR_ERR(generic_phy);
+
+	phy_set_drvdata(generic_phy, rtk_phy);
+
+	phy_provider = devm_of_phy_provider_register(rtk_phy->dev,
+						     of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	ret = usb_add_phy_dev(&rtk_phy->phy);
+	if (ret)
+		goto err;
+
+	create_debug_files(rtk_phy);
+
+err:
+	dev_dbg(dev, "Probe RTK USB 2.0 PHY (ret=%d)\n", ret);
+
+	return ret;
+}
+
+static void rtk_usb2phy_remove(struct platform_device *pdev)
+{
+	struct rtk_phy *rtk_phy = platform_get_drvdata(pdev);
+
+	remove_debug_files(rtk_phy);
+
+	usb_remove_phy(&rtk_phy->phy);
+}
+
+static const struct phy_cfg rtd1295_phy_cfg = {
+	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
+	.page0 = { [0] = {0xe0, 0x90},
+		   [3] = {0xe3, 0x3a},
+		   [4] = {0xe4, 0x68},
+		   [6] = {0xe6, 0x91},
+		  [13] = {0xf5, 0x81},
+		  [15] = {0xf7, 0x02}, },
+	.page1_size = 8,
+	.page1 = { /* default parameter */ },
+	.page2_size = 0,
+	.page2 = { /* no parameter */ },
+	.check_efuse = false,
+	.check_efuse_version = CHECK_EFUSE_V1,
+	.efuse_dc_driving_rate = 1,
+	.dc_driving_mask = 0xf,
+	.efuse_dc_disconnect_rate = EFUS_USB_DC_DIS_RATE,
+	.dc_disconnect_mask = 0xf,
+	.usb_dc_disconnect_at_page0 = true,
+	.do_toggle = true,
+	.do_toggle_driving = false,
+	.driving_updated_for_dev_dis = 0xf,
+	.use_default_parameter = false,
+	.is_double_sensitivity_mode = false,
+};
+
+static const struct phy_cfg rtd1395_phy_cfg = {
+	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
+	.page0 = { [4] = {0xe4, 0xac},
+		  [13] = {0xf5, 0x00},
+		  [15] = {0xf7, 0x02}, },
+	.page1_size = 8,
+	.page1 = { /* default parameter */ },
+	.page2_size = 0,
+	.page2 = { /* no parameter */ },
+	.check_efuse = false,
+	.check_efuse_version = CHECK_EFUSE_V1,
+	.efuse_dc_driving_rate = 1,
+	.dc_driving_mask = 0xf,
+	.efuse_dc_disconnect_rate = EFUS_USB_DC_DIS_RATE,
+	.dc_disconnect_mask = 0xf,
+	.usb_dc_disconnect_at_page0 = true,
+	.do_toggle = true,
+	.do_toggle_driving = false,
+	.driving_updated_for_dev_dis = 0xf,
+	.use_default_parameter = false,
+	.is_double_sensitivity_mode = false,
+};
+
+static const struct phy_cfg rtd1619_phy_cfg = {
+	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
+	.page0 = { [4] = {0xe4, 0x68}, },
+	.page1_size = 8,
+	.page1 = { /* default parameter */ },
+	.page2_size = 0,
+	.page2 = { /* no parameter */ },
+	.check_efuse = true,
+	.check_efuse_version = CHECK_EFUSE_V1,
+	.efuse_dc_driving_rate = 1,
+	.dc_driving_mask = 0xf,
+	.efuse_dc_disconnect_rate = EFUS_USB_DC_DIS_RATE,
+	.dc_disconnect_mask = 0xf,
+	.usb_dc_disconnect_at_page0 = true,
+	.do_toggle = true,
+	.do_toggle_driving = false,
+	.driving_updated_for_dev_dis = 0xf,
+	.use_default_parameter = false,
+	.is_double_sensitivity_mode = false,
+};
+
+static const struct phy_cfg rtd1319_phy_cfg = {
+	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
+	.page0 = { [0] = {0xe0, 0x18},
+		   [4] = {0xe4, 0x6a},
+		   [7] = {0xe7, 0x71},
+		  [13] = {0xf5, 0x15},
+		  [15] = {0xf7, 0x32}, },
+	.page1_size = 8,
+	.page1 = { [3] = {0xe3, 0x44}, },
+	.page2_size = MAX_USB_PHY_PAGE2_DATA_SIZE,
+	.page2 = { [0] = {0xe0, 0x01}, },
+	.check_efuse = true,
+	.check_efuse_version = CHECK_EFUSE_V1,
+	.efuse_dc_driving_rate = 1,
+	.dc_driving_mask = 0xf,
+	.efuse_dc_disconnect_rate = EFUS_USB_DC_DIS_RATE,
+	.dc_disconnect_mask = 0xf,
+	.usb_dc_disconnect_at_page0 = true,
+	.do_toggle = true,
+	.do_toggle_driving = true,
+	.driving_updated_for_dev_dis = 0xf,
+	.use_default_parameter = false,
+	.is_double_sensitivity_mode = true,
+};
+
+static const struct phy_cfg rtd1312c_phy_cfg = {
+	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
+	.page0 = { [0] = {0xe0, 0x14},
+		   [4] = {0xe4, 0x67},
+		   [5] = {0xe5, 0x55}, },
+	.page1_size = 8,
+	.page1 = { [3] = {0xe3, 0x23},
+		   [6] = {0xe6, 0x58}, },
+	.page2_size = MAX_USB_PHY_PAGE2_DATA_SIZE,
+	.page2 = { /* default parameter */ },
+	.check_efuse = true,
+	.check_efuse_version = CHECK_EFUSE_V1,
+	.efuse_dc_driving_rate = 1,
+	.dc_driving_mask = 0xf,
+	.efuse_dc_disconnect_rate = EFUS_USB_DC_DIS_RATE,
+	.dc_disconnect_mask = 0xf,
+	.usb_dc_disconnect_at_page0 = true,
+	.do_toggle = true,
+	.do_toggle_driving = true,
+	.driving_updated_for_dev_dis = 0xf,
+	.use_default_parameter = false,
+	.is_double_sensitivity_mode = true,
+};
+
+static const struct phy_cfg rtd1619b_phy_cfg = {
+	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
+	.page0 = { [0] = {0xe0, 0xa3},
+		   [4] = {0xe4, 0x88},
+		   [5] = {0xe5, 0x4f},
+		   [6] = {0xe6, 0x02}, },
+	.page1_size = 8,
+	.page1 = { [3] = {0xe3, 0x64}, },
+	.page2_size = MAX_USB_PHY_PAGE2_DATA_SIZE,
+	.page2 = { [7] = {0xe7, 0x45}, },
+	.check_efuse = true,
+	.check_efuse_version = CHECK_EFUSE_V1,
+	.efuse_dc_driving_rate = EFUS_USB_DC_CAL_RATE,
+	.dc_driving_mask = 0x1f,
+	.efuse_dc_disconnect_rate = EFUS_USB_DC_DIS_RATE,
+	.dc_disconnect_mask = 0xf,
+	.usb_dc_disconnect_at_page0 = false,
+	.do_toggle = true,
+	.do_toggle_driving = true,
+	.driving_updated_for_dev_dis = 0x8,
+	.use_default_parameter = false,
+	.is_double_sensitivity_mode = true,
+};
+
+static const struct phy_cfg rtd1319d_phy_cfg = {
+	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
+	.page0 = { [0] = {0xe0, 0xa3},
+		   [4] = {0xe4, 0x8e},
+		   [5] = {0xe5, 0x4f},
+		   [6] = {0xe6, 0x02}, },
+	.page1_size = MAX_USB_PHY_PAGE1_DATA_SIZE,
+	.page1 = { [14] = {0xf5, 0x1}, },
+	.page2_size = MAX_USB_PHY_PAGE2_DATA_SIZE,
+	.page2 = { [7] = {0xe7, 0x44}, },
+	.check_efuse = true,
+	.check_efuse_version = CHECK_EFUSE_V1,
+	.efuse_dc_driving_rate = EFUS_USB_DC_CAL_RATE,
+	.dc_driving_mask = 0x1f,
+	.efuse_dc_disconnect_rate = EFUS_USB_DC_DIS_RATE,
+	.dc_disconnect_mask = 0xf,
+	.usb_dc_disconnect_at_page0 = false,
+	.do_toggle = true,
+	.do_toggle_driving = false,
+	.driving_updated_for_dev_dis = 0x8,
+	.use_default_parameter = false,
+	.is_double_sensitivity_mode = true,
+};
+
+static const struct phy_cfg rtd1315e_phy_cfg = {
+	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
+	.page0 = { [0] = {0xe0, 0xa3},
+		   [4] = {0xe4, 0x8c},
+		   [5] = {0xe5, 0x4f},
+		   [6] = {0xe6, 0x02}, },
+	.page1_size = MAX_USB_PHY_PAGE1_DATA_SIZE,
+	.page1 = { [3] = {0xe3, 0x7f},
+		  [14] = {0xf5, 0x01}, },
+	.page2_size = MAX_USB_PHY_PAGE2_DATA_SIZE,
+	.page2 = { [7] = {0xe7, 0x44}, },
+	.check_efuse = true,
+	.check_efuse_version = CHECK_EFUSE_V2,
+	.efuse_dc_driving_rate = EFUS_USB_DC_CAL_RATE,
+	.dc_driving_mask = 0x1f,
+	.efuse_dc_disconnect_rate = EFUS_USB_DC_DIS_RATE,
+	.dc_disconnect_mask = 0xf,
+	.usb_dc_disconnect_at_page0 = false,
+	.do_toggle = true,
+	.do_toggle_driving = false,
+	.driving_updated_for_dev_dis = 0x8,
+	.use_default_parameter = false,
+	.is_double_sensitivity_mode = true,
+};
+
+static const struct phy_cfg rtk_phy_cfg = {
+	.page0_size = MAX_USB_PHY_PAGE0_DATA_SIZE,
+	.page0 = { /* use default */ },
+	.page1_size = MAX_USB_PHY_PAGE1_DATA_SIZE,
+	.page1 = { /* use default */ },
+	.page2_size = MAX_USB_PHY_PAGE2_DATA_SIZE,
+	.page2 = { /* use default */ },
+	.check_efuse = false,
+	.check_efuse_version = CHECK_EFUSE_V2,
+	.efuse_dc_driving_rate = EFUS_USB_DC_CAL_RATE,
+	.dc_driving_mask = 0x1f,
+	.efuse_dc_disconnect_rate = EFUS_USB_DC_DIS_RATE,
+	.dc_disconnect_mask = 0xf,
+	.usb_dc_disconnect_at_page0 = false,
+	.do_toggle = false,
+	.do_toggle_driving = false,
+	.driving_updated_for_dev_dis = 0x8,
+	.use_default_parameter = false,
+	.is_double_sensitivity_mode = true,
+};
+
+static const struct of_device_id usbphy_rtk_dt_match[] = {
+	{ .compatible = "realtek,rtd1295-usb2phy", .data = &rtd1295_phy_cfg },
+	{ .compatible = "realtek,rtd1312c-usb2phy", .data = &rtd1312c_phy_cfg },
+	{ .compatible = "realtek,rtd1315e-usb2phy", .data = &rtd1315e_phy_cfg },
+	{ .compatible = "realtek,rtd1319-usb2phy", .data = &rtd1319_phy_cfg },
+	{ .compatible = "realtek,rtd1319d-usb2phy", .data = &rtd1319d_phy_cfg },
+	{ .compatible = "realtek,rtd1395-usb2phy", .data = &rtd1395_phy_cfg },
+	{ .compatible = "realtek,rtd1395-usb2phy-2port", .data = &rtd1395_phy_cfg },
+	{ .compatible = "realtek,rtd1619-usb2phy", .data = &rtd1619_phy_cfg },
+	{ .compatible = "realtek,rtd1619b-usb2phy", .data = &rtd1619b_phy_cfg },
+	{ .compatible = "realtek,usb2phy", .data = &rtk_phy_cfg },
+	{},
+};
+MODULE_DEVICE_TABLE(of, usbphy_rtk_dt_match);
+
+static struct platform_driver rtk_usb2phy_driver = {
+	.probe		= rtk_usb2phy_probe,
+	.remove_new	= rtk_usb2phy_remove,
+	.driver		= {
+		.name	= "rtk-usb2phy",
+		.owner	= THIS_MODULE,
+		.of_match_table = usbphy_rtk_dt_match,
+	},
+};
+
+module_platform_driver(rtk_usb2phy_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform: rtk-usb2phy");
+MODULE_AUTHOR("Stanley Chang <stanley_chang@realtek.com>");
+MODULE_DESCRIPTION("Realtek usb 2.0 phy driver");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY
  2023-06-14  9:28 [PATCH v4 1/5] usb: phy: add usb phy notify port status API Stanley Chang
  2023-06-14  9:28 ` [PATCH v4 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY Stanley Chang
@ 2023-06-14  9:28 ` Stanley Chang
  2023-06-14 16:13   ` kernel test robot
  2023-06-17  8:35   ` Krzysztof Kozlowski
  2023-06-14  9:28 ` [PATCH v4 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY Stanley Chang
  2023-06-14  9:28 ` [PATCH v4 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY Stanley Chang
  3 siblings, 2 replies; 15+ messages in thread
From: Stanley Chang @ 2023-06-14  9:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Stanley Chang, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Alan Stern, Bagas Sanjaya,
	Matthias Kaehlcke, Douglas Anderson, Flavio Suligoi, Ray Chi,
	linux-phy, devicetree, linux-kernel, linux-usb

Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
controller. Added the driver to drive the USB 3.0 PHY transceivers.

Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
---
v3 to v4 change:
    1. Remove the parameter and non hardware properties from dts.
       Add the compatible data included the config and parameter.
    2. Fix the warning for checkpatch with strict.
    3. Remove useless debug messages.
    4. Fix the incorrect or useless "if() return" which pointer check.
v2 to v3 change:
    1. Broken down into two patches, one for each of USB 2 & 3 PHY.
    2. Removed parameter v1 support for simplification.
    3. Used remove_new for driver remove callback.
v1 to v2 change:
    1. Move the drivers to drivers/phy/ for generic phy driver
    2. Use the generic phy driver api to initialize phy
    3. Use readl/writel to instead phy_read/phy_write directly.
    4. Use read_poll_timeout() in function utmi_wait_register.
    5. Revised some coding styles.
    6. fix the compiler warning for kernel test robot.
---
 drivers/phy/realtek/Kconfig        |  11 +
 drivers/phy/realtek/Makefile       |   1 +
 drivers/phy/realtek/phy-rtk-usb3.c | 990 +++++++++++++++++++++++++++++
 3 files changed, 1002 insertions(+)
 create mode 100644 drivers/phy/realtek/phy-rtk-usb3.c

diff --git a/drivers/phy/realtek/Kconfig b/drivers/phy/realtek/Kconfig
index 2f2f453729d5..a5a5a71edc9c 100644
--- a/drivers/phy/realtek/Kconfig
+++ b/drivers/phy/realtek/Kconfig
@@ -12,3 +12,14 @@ config PHY_RTK_RTD_USB2PHY
 	  The DHC (digital home center) RTD series SoCs used the Synopsys
 	  DWC3 USB IP. This driver will do the PHY initialization
 	  of the parameters.
+
+config PHY_RTK_RTD_USB3PHY
+	tristate "Realtek RTD USB3 PHY Transceiver Driver"
+	depends on USB_SUPPORT
+	select GENERIC_PHY
+	select USB_PHY
+	help
+	  Enable this to support Realtek SoC USB3 phy transceiver.
+	  The DHC (digital home center) RTD series SoCs used the Synopsys
+	  DWC3 USB IP. This driver will do the PHY initialization
+	  of the parameters.
diff --git a/drivers/phy/realtek/Makefile b/drivers/phy/realtek/Makefile
index cf5d440841a2..ed7b47ff8a26 100644
--- a/drivers/phy/realtek/Makefile
+++ b/drivers/phy/realtek/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_PHY_RTK_RTD_USB2PHY)	+= phy-rtk-usb2.o
+obj-$(CONFIG_PHY_RTK_RTD_USB3PHY)	+= phy-rtk-usb3.o
diff --git a/drivers/phy/realtek/phy-rtk-usb3.c b/drivers/phy/realtek/phy-rtk-usb3.c
new file mode 100644
index 000000000000..e75e52abf84f
--- /dev/null
+++ b/drivers/phy/realtek/phy-rtk-usb3.c
@@ -0,0 +1,990 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  phy-rtk-usb3.c RTK usb3.0 phy driver
+ *
+ * copyright (c) 2023 realtek semiconductor corporation
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/uaccess.h>
+#include <linux/debugfs.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/regmap.h>
+#include <linux/sys_soc.h>
+#include <linux/mfd/syscon.h>
+#include <linux/phy/phy.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+#include <linux/usb/phy.h>
+
+#define USB_MDIO_CTRL_PHY_BUSY BIT(7)
+#define USB_MDIO_CTRL_PHY_WRITE BIT(0)
+#define USB_MDIO_CTRL_PHY_ADDR_SHIFT 8
+#define USB_MDIO_CTRL_PHY_DATA_SHIFT 16
+
+#define MAX_USB_PHY_DATA_SIZE 0x30
+#define PHY_ADDR_0X09 0x09
+#define PHY_ADDR_0X0B 0x0b
+#define PHY_ADDR_0X0D 0x0d
+#define PHY_ADDR_0X10 0x10
+#define PHY_ADDR_0X1F 0x1f
+#define PHY_ADDR_0X20 0x20
+#define PHY_ADDR_0X21 0x21
+#define PHY_ADDR_0X30 0x30
+
+#define REG_0X09_FORCE_CALIBRATION BIT(9)
+#define REG_0X0B_RX_OFFSET_RANGE_MASK 0xc
+#define REG_0X0D_RX_DEBUG_TEST_EN BIT(6)
+#define REG_0X10_DEBUG_MODE_SETTING 0x3c0
+#define REG_0X10_DEBUG_MODE_SETTING_MASK 0x3f8
+#define REG_0X1F_RX_OFFSET_CODE_MASK 0x1e
+
+#define USB_U3_TX_LFPS_SWING_TRIM_SHIFT 4
+#define USB_U3_TX_LFPS_SWING_TRIM_MASK 0xf
+#define AMPLITUDE_CONTROL_COARSE_MASK 0xff
+#define AMPLITUDE_CONTROL_FINE_MASK 0xffff
+#define AMPLITUDE_CONTROL_COARSE_DEFAULT 0xff
+#define AMPLITUDE_CONTROL_FINE_DEFAULT 0xffff
+
+#define PHY_ADDR_MAP_ARRAY_INDEX(addr) (addr)
+#define ARRAY_INDEX_MAP_PHY_ADDR(index) (index)
+
+struct phy_reg {
+	void __iomem *reg_mdio_ctl;
+};
+
+struct phy_data {
+	u8 addr;
+	u16 data;
+};
+
+struct phy_cfg {
+	int param_size;
+	struct phy_data param[MAX_USB_PHY_DATA_SIZE];
+
+	bool check_efuse;
+	bool do_toggle;
+	bool do_toggle_once;
+	bool use_default_parameter;
+	bool check_rx_front_end_offset;
+};
+
+struct phy_parameter {
+	struct phy_reg phy_reg;
+
+	/* Get from efuse */
+	u8 efuse_usb_u3_tx_lfps_swing_trim;
+
+	/* Get from dts */
+	u32 amplitude_control_coarse;
+	u32 amplitude_control_fine;
+};
+
+struct rtk_phy {
+	struct usb_phy phy;
+	struct device *dev;
+
+	struct phy_cfg *phy_cfg;
+	int num_phy;
+	struct phy_parameter *phy_parameter;
+
+	struct dentry *debug_dir;
+};
+
+#define PHY_IO_TIMEOUT_USEC		(50000)
+#define PHY_IO_DELAY_US			(100)
+
+static inline int utmi_wait_register(void __iomem *reg, u32 mask, u32 result)
+{
+	int ret;
+	unsigned int val;
+
+	ret = read_poll_timeout(readl, val, ((val & mask) == result),
+				PHY_IO_DELAY_US, PHY_IO_TIMEOUT_USEC, false, reg);
+	if (ret) {
+		pr_err("%s can't program USB phy\n", __func__);
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int rtk_phy3_wait_vbusy(struct phy_reg *phy_reg)
+{
+	return utmi_wait_register(phy_reg->reg_mdio_ctl, USB_MDIO_CTRL_PHY_BUSY, 0);
+}
+
+static u16 rtk_phy_read(struct phy_reg *phy_reg, char addr)
+{
+	unsigned int _addr;
+	u32 value;
+
+	_addr = (addr << USB_MDIO_CTRL_PHY_ADDR_SHIFT);
+
+	writel(_addr, phy_reg->reg_mdio_ctl);
+
+	rtk_phy3_wait_vbusy(phy_reg);
+
+	value = readl(phy_reg->reg_mdio_ctl);
+	value = value >> USB_MDIO_CTRL_PHY_DATA_SHIFT;
+
+	return (u16)value;
+}
+
+static int rtk_phy_write(struct phy_reg *phy_reg, char addr, u16 data)
+{
+	unsigned int val;
+
+	val = USB_MDIO_CTRL_PHY_WRITE |
+		    (addr << USB_MDIO_CTRL_PHY_ADDR_SHIFT) |
+		    (data << USB_MDIO_CTRL_PHY_DATA_SHIFT);
+
+	writel(val, phy_reg->reg_mdio_ctl);
+
+	rtk_phy3_wait_vbusy(phy_reg);
+
+	return 0;
+}
+
+static void do_rtk_usb3_phy_toggle(struct rtk_phy *rtk_phy, int index, bool connect)
+{
+	struct phy_cfg *phy_cfg = rtk_phy->phy_cfg;
+	struct phy_reg *phy_reg;
+	struct phy_parameter *phy_parameter;
+	struct phy_data *phy_data;
+	u8 addr;
+	u16 data;
+	int i;
+
+	phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
+	phy_reg = &phy_parameter->phy_reg;
+
+	if (!phy_cfg->do_toggle)
+		return;
+
+	i = PHY_ADDR_MAP_ARRAY_INDEX(PHY_ADDR_0X09);
+	phy_data = phy_cfg->param + i;
+	addr = phy_data->addr;
+	data = phy_data->data;
+
+	if (!addr && !data) {
+		addr = PHY_ADDR_0X09;
+		data = rtk_phy_read(phy_reg, addr);
+		phy_data->addr = addr;
+		phy_data->data = data;
+	}
+
+	rtk_phy_write(phy_reg, addr, data & (~REG_0X09_FORCE_CALIBRATION));
+	mdelay(1);
+	rtk_phy_write(phy_reg, addr, data | REG_0X09_FORCE_CALIBRATION);
+}
+
+static int do_rtk_phy_init(struct rtk_phy *rtk_phy, int index)
+{
+	struct phy_cfg *phy_cfg;
+	struct phy_reg *phy_reg;
+	struct phy_parameter *phy_parameter;
+	int i = 0;
+
+	phy_cfg = rtk_phy->phy_cfg;
+	phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
+	phy_reg = &phy_parameter->phy_reg;
+
+	if (phy_cfg->use_default_parameter)
+		goto do_toggle;
+
+	for (i = 0; i < phy_cfg->param_size; i++) {
+		struct phy_data *phy_data = phy_cfg->param + i;
+		u8 addr = phy_data->addr;
+		u16 data = phy_data->data;
+
+		if (!addr && !data)
+			continue;
+
+		rtk_phy_write(phy_reg, addr, data);
+	}
+
+do_toggle:
+	if (phy_cfg->do_toggle_once)
+		phy_cfg->do_toggle = true;
+
+	do_rtk_usb3_phy_toggle(rtk_phy, index, false);
+
+	if (phy_cfg->do_toggle_once) {
+		u16 check_value = 0;
+		int count = 10;
+		u16 value_0x0D, value_0x10;
+
+		/* Enable Debug mode by set 0x0D and 0x10 */
+		value_0x0D = rtk_phy_read(phy_reg, PHY_ADDR_0X0D);
+		value_0x10 = rtk_phy_read(phy_reg, PHY_ADDR_0X10);
+
+		rtk_phy_write(phy_reg, PHY_ADDR_0X0D,
+			      value_0x0D | REG_0X0D_RX_DEBUG_TEST_EN);
+		rtk_phy_write(phy_reg, PHY_ADDR_0X10,
+			      (value_0x10 & ~REG_0X10_DEBUG_MODE_SETTING_MASK) |
+			      REG_0X10_DEBUG_MODE_SETTING);
+
+		check_value = rtk_phy_read(phy_reg, PHY_ADDR_0X30);
+
+		while (!(check_value & BIT(15))) {
+			check_value = rtk_phy_read(phy_reg, PHY_ADDR_0X30);
+			mdelay(1);
+			if (count-- < 0)
+				break;
+		}
+
+		if (!(check_value & BIT(15)))
+			dev_info(rtk_phy->dev, "toggle fail addr=0x%02x, data=0x%04x\n",
+				 PHY_ADDR_0X30, check_value);
+
+		/* Disable Debug mode by set 0x0D and 0x10 to default*/
+		rtk_phy_write(phy_reg, PHY_ADDR_0X0D, value_0x0D);
+		rtk_phy_write(phy_reg, PHY_ADDR_0X10, value_0x10);
+
+		phy_cfg->do_toggle = false;
+	}
+
+	if (phy_cfg->check_rx_front_end_offset) {
+		u16 rx_offset_code, rx_offset_range;
+		u16 code_mask = REG_0X1F_RX_OFFSET_CODE_MASK;
+		u16 range_mask = REG_0X0B_RX_OFFSET_RANGE_MASK;
+		bool do_update = false;
+
+		rx_offset_code = rtk_phy_read(phy_reg, PHY_ADDR_0X1F);
+		if (((rx_offset_code & code_mask) == 0x0) ||
+		    ((rx_offset_code & code_mask) == code_mask))
+			do_update = true;
+
+		rx_offset_range = rtk_phy_read(phy_reg, PHY_ADDR_0X0B);
+		if (((rx_offset_range & range_mask) == range_mask) && do_update) {
+			dev_warn(rtk_phy->dev, "Don't update rx_offset_range (rx_offset_code=0x%x, rx_offset_range=0x%x)\n",
+				 rx_offset_code, rx_offset_range);
+			do_update = false;
+		}
+
+		if (do_update) {
+			u16 tmp1, tmp2;
+
+			tmp1 = rx_offset_range & (~range_mask);
+			tmp2 = rx_offset_range & range_mask;
+			tmp2 += (1 << 2);
+			rx_offset_range = tmp1 | (tmp2 & range_mask);
+			rtk_phy_write(phy_reg, PHY_ADDR_0X0B, rx_offset_range);
+			goto do_toggle;
+		}
+	}
+
+	return 0;
+}
+
+static int rtk_phy_init(struct phy *phy)
+{
+	struct rtk_phy *rtk_phy = phy_get_drvdata(phy);
+	int ret = 0;
+	int i;
+	unsigned long phy_init_time = jiffies;
+
+	for (i = 0; i < rtk_phy->num_phy; i++)
+		ret = do_rtk_phy_init(rtk_phy, i);
+
+	dev_dbg(rtk_phy->dev, "Initialized RTK USB 3.0 PHY (take %dms)\n",
+		jiffies_to_msecs(jiffies - phy_init_time));
+
+	return ret;
+}
+
+static int rtk_phy_exit(struct phy *phy)
+{
+	return 0;
+}
+
+static const struct phy_ops ops = {
+	.init		= rtk_phy_init,
+	.exit		= rtk_phy_exit,
+	.owner		= THIS_MODULE,
+};
+
+static void rtk_phy_toggle(struct usb_phy *usb3_phy, bool connect, int port)
+{
+	int index = port;
+	struct rtk_phy *rtk_phy = NULL;
+
+	rtk_phy = dev_get_drvdata(usb3_phy->dev);
+
+	if (index > rtk_phy->num_phy) {
+		pr_err("%s %d ERROR! port=%d > num_phy=%d\n",
+		       __func__, __LINE__, index, rtk_phy->num_phy);
+		return;
+	}
+
+	do_rtk_usb3_phy_toggle(rtk_phy, index, connect);
+}
+
+static int rtk_phy_notify_port_status(struct usb_phy *x, int port,
+				      u16 portstatus, u16 portchange)
+{
+	bool connect = false;
+
+	pr_debug("%s port=%d portstatus=0x%x portchange=0x%x\n",
+		 __func__, port, (int)portstatus, (int)portchange);
+	if (portstatus & USB_PORT_STAT_CONNECTION)
+		connect = true;
+
+	if (portchange & USB_PORT_STAT_C_CONNECTION)
+		rtk_phy_toggle(x, connect, port);
+
+	return 0;
+}
+
+#ifdef CONFIG_DEBUG_FS
+static struct dentry *create_phy_debug_root(void)
+{
+	struct dentry *phy_debug_root;
+
+	phy_debug_root = debugfs_lookup("phy", usb_debug_root);
+	if (!phy_debug_root)
+		phy_debug_root = debugfs_create_dir("phy", usb_debug_root);
+
+	return phy_debug_root;
+}
+
+static int rtk_usb3_parameter_show(struct seq_file *s, void *unused)
+{
+	struct rtk_phy *rtk_phy = s->private;
+	struct phy_cfg *phy_cfg;
+	int i, index;
+
+	phy_cfg = rtk_phy->phy_cfg;
+
+	seq_puts(s, "Property:\n");
+	seq_printf(s, "  check_efuse: %s\n",
+		   phy_cfg->check_efuse ? "Enable" : "Disable");
+	seq_printf(s, "  do_toggle: %s\n",
+		   phy_cfg->do_toggle ? "Enable" : "Disable");
+	seq_printf(s, "  do_toggle_once: %s\n",
+		   phy_cfg->do_toggle_once ? "Enable" : "Disable");
+	seq_printf(s, "  use_default_parameter: %s\n",
+		   phy_cfg->use_default_parameter ? "Enable" : "Disable");
+
+	for (index = 0; index < rtk_phy->num_phy; index++) {
+		struct phy_reg *phy_reg;
+		struct phy_parameter *phy_parameter;
+
+		phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
+		phy_reg = &phy_parameter->phy_reg;
+
+		seq_printf(s, "PHY %d:\n", index);
+
+		for (i = 0; i < phy_cfg->param_size; i++) {
+			struct phy_data *phy_data = phy_cfg->param + i;
+			u8 addr = ARRAY_INDEX_MAP_PHY_ADDR(i);
+			u16 data = phy_data->data;
+
+			if (!phy_data->addr && !data)
+				seq_printf(s, "  addr = 0x%02x, data = none   ==> read value = 0x%04x\n",
+					   addr, rtk_phy_read(phy_reg, addr));
+			else
+				seq_printf(s, "  addr = 0x%02x, data = 0x%04x ==> read value = 0x%04x\n",
+					   addr, data, rtk_phy_read(phy_reg, addr));
+		}
+
+		seq_puts(s, "PHY Property:\n");
+		seq_printf(s, "  efuse_usb_u3_tx_lfps_swing_trim: 0x%x\n",
+			   (int)phy_parameter->efuse_usb_u3_tx_lfps_swing_trim);
+		seq_printf(s, "  amplitude_control_coarse: 0x%x\n",
+			   (int)phy_parameter->amplitude_control_coarse);
+		seq_printf(s, "  amplitude_control_fine: 0x%x\n",
+			   (int)phy_parameter->amplitude_control_fine);
+	}
+
+	return 0;
+}
+
+static int rtk_usb3_parameter_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, rtk_usb3_parameter_show, inode->i_private);
+}
+
+static const struct file_operations rtk_usb3_parameter_fops = {
+	.open			= rtk_usb3_parameter_open,
+	.read			= seq_read,
+	.llseek			= seq_lseek,
+	.release		= single_release,
+};
+
+static int __get_parameter_at_page(struct seq_file *s, struct rtk_phy *rtk_phy,
+				   struct phy_data *phy_data_array, const char *phy_addr)
+{
+	struct phy_data *phy_data;
+	u32 addr;
+	int i, ret;
+
+	ret = kstrtouint(phy_addr, 16, &addr);
+	if (ret < 0) {
+		pr_err("%s::kstrtouint() failed\n", __func__);
+		return -EINVAL;
+	}
+
+	i = PHY_ADDR_MAP_ARRAY_INDEX(addr);
+	phy_data = (phy_data_array + i);
+
+	if (phy_data->addr && phy_data->data)
+		seq_printf(s, "Now Parameter addr 0x%02x = 0x%04x\n",
+			   phy_data->addr, phy_data->data);
+	else
+		seq_printf(s, "Now Parameter addr 0x%02x is default\n",
+			   addr);
+
+	return 0;
+}
+
+static int __set_parameter_at_page(struct rtk_phy *rtk_phy, struct phy_reg *phy_reg,
+				   struct phy_parameter *phy_parameter,
+				   struct phy_data *phy_data_array,
+				   const char *phy_addr, const char *phy_value)
+{
+	struct phy_data *phy_data;
+	u32 addr, value;
+	int i, ret;
+
+	ret = kstrtouint(phy_addr, 16, &addr);
+	if (ret < 0) {
+		pr_err("%s::kstrtouint() failed\n", __func__);
+		return -EINVAL;
+	}
+	ret = kstrtouint(phy_value, 16, &value);
+	if (ret < 0) {
+		pr_err("%s::kstrtouint() failed\n", __func__);
+		return -EINVAL;
+	}
+
+	i = PHY_ADDR_MAP_ARRAY_INDEX(addr);
+	phy_data = (phy_data_array + i);
+
+	phy_data->addr = addr;
+	phy_data->data = value;
+
+	if (rtk_phy_write(phy_reg, addr, value))
+		dev_err(rtk_phy->dev,
+			"[%s:%d] Error: addr=0x%02x value=0x%04x\n",
+			__func__, __LINE__, addr, value);
+
+	return 0;
+}
+
+static int rtk_usb3_set_parameter_show(struct seq_file *s, void *unused)
+{
+	const struct file *file = s->file;
+	const char *file_name = file_dentry(file)->d_iname;
+	struct dentry *p_dentry = file_dentry(file)->d_parent;
+	const char *phy_dir_name = p_dentry->d_iname;
+	struct rtk_phy *rtk_phy = s->private;
+	struct phy_cfg *phy_cfg;
+	struct phy_parameter *phy_parameter = NULL;
+	int ret, index;
+
+	for (index = 0; index < rtk_phy->num_phy; index++) {
+		size_t sz = 30;
+		char name[30] = {0};
+
+		snprintf(name, sz, "phy%d", index);
+		if (strncmp(name, phy_dir_name, strlen(name)) == 0) {
+			phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
+			break;
+		}
+	}
+	if (!phy_parameter)
+		return -EINVAL;
+
+	phy_cfg = rtk_phy->phy_cfg;
+
+	ret = __get_parameter_at_page(s, rtk_phy, phy_cfg->param, file_name);
+	if (ret < 0)
+		return ret;
+
+	seq_puts(s, "Set phy parameter by following command\n");
+	seq_printf(s, "echo \"value\" > %s/%s\n",
+		   phy_dir_name, file_name);
+
+	return 0;
+}
+
+static int rtk_usb3_set_parameter_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, rtk_usb3_set_parameter_show, inode->i_private);
+}
+
+static ssize_t rtk_usb3_set_parameter_write(struct file *file,
+					    const char __user *ubuf, size_t count, loff_t *ppos)
+{
+	const char *file_name = file_dentry(file)->d_iname;
+	struct dentry *p_dentry = file_dentry(file)->d_parent;
+	const char *phy_dir_name = p_dentry->d_iname;
+	struct seq_file *s = file->private_data;
+	struct rtk_phy *rtk_phy = s->private;
+	struct phy_cfg *phy_cfg;
+	struct phy_reg *phy_reg;
+	struct phy_parameter *phy_parameter = NULL;
+	int ret = 0;
+	char buffer[40] = {0};
+	int index;
+
+	if (copy_from_user(&buffer, ubuf, min_t(size_t, sizeof(buffer) - 1, count)))
+		return -EFAULT;
+
+	for (index = 0; index < rtk_phy->num_phy; index++) {
+		size_t sz = 30;
+		char name[30] = {0};
+
+		snprintf(name, sz, "phy%d", index);
+		if (strncmp(name, phy_dir_name, strlen(name)) == 0) {
+			phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
+			break;
+		}
+	}
+
+	if (!phy_parameter)
+		return -EINVAL;
+
+	phy_cfg = rtk_phy->phy_cfg;
+	phy_reg = &phy_parameter->phy_reg;
+
+	ret = __set_parameter_at_page(rtk_phy, phy_reg, phy_parameter,
+				      phy_cfg->param, file_name, buffer);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static const struct file_operations rtk_usb3_set_parameter_fops = {
+	.open			= rtk_usb3_set_parameter_open,
+	.write			= rtk_usb3_set_parameter_write,
+	.read			= seq_read,
+	.llseek			= seq_lseek,
+	.release		= single_release,
+};
+
+static int create_debug_set_parameter_files(struct rtk_phy *rtk_phy,
+					    struct dentry *phy_dir, size_t addr_size)
+{
+	int i;
+
+	for (i = 0; i < addr_size; i++) {
+		size_t sz = 30;
+		char name[30] = {0};
+
+		snprintf(name, sz, "%02x", ARRAY_INDEX_MAP_PHY_ADDR(i));
+
+		debugfs_create_file(name, 0644, phy_dir, rtk_phy,
+				    &rtk_usb3_set_parameter_fops);
+	}
+
+	return 0;
+}
+
+static inline void create_debug_files(struct rtk_phy *rtk_phy)
+{
+	struct dentry *phy_debug_root = NULL;
+	struct dentry *set_parameter_dir = NULL;
+
+	phy_debug_root = create_phy_debug_root();
+
+	if (!phy_debug_root)
+		return;
+
+	rtk_phy->debug_dir = debugfs_create_dir(dev_name(rtk_phy->dev), phy_debug_root);
+	if (!rtk_phy->debug_dir)
+		return;
+
+	if (!debugfs_create_file("parameter", 0444, rtk_phy->debug_dir, rtk_phy,
+				 &rtk_usb3_parameter_fops))
+		goto file_error;
+
+	set_parameter_dir = debugfs_create_dir("set_parameter", rtk_phy->debug_dir);
+	if (set_parameter_dir) {
+		struct phy_cfg *phy_cfg;
+		int index, ret;
+
+		phy_cfg = rtk_phy->phy_cfg;
+
+		for (index = 0; index < rtk_phy->num_phy; index++) {
+			struct dentry *phy_dir;
+			size_t sz = 30;
+			char name[30] = {0};
+
+			snprintf(name, sz, "phy%d", index);
+
+			phy_dir = debugfs_create_dir(name, set_parameter_dir);
+			if (!phy_dir)
+				goto file_error;
+
+			ret = create_debug_set_parameter_files(rtk_phy, phy_dir,
+							       phy_cfg->param_size);
+			if (ret < 0)
+				goto file_error;
+		}
+	}
+
+	return;
+
+file_error:
+	debugfs_remove_recursive(rtk_phy->debug_dir);
+}
+
+static inline void remove_debug_files(struct rtk_phy *rtk_phy)
+{
+	debugfs_remove_recursive(rtk_phy->debug_dir);
+}
+#else
+static inline void create_debug_files(struct rtk_phy *rtk_phy) { }
+static inline void remove_debug_files(struct rtk_phy *rtk_phy) { }
+#endif /* CONFIG_DEBUG_FS */
+
+static int get_phy_data_by_efuse(struct rtk_phy *rtk_phy,
+				 struct phy_parameter *phy_parameter, int index)
+{
+	struct phy_cfg *phy_cfg = rtk_phy->phy_cfg;
+	u8 value = 0;
+	struct nvmem_cell *cell;
+
+	if (!phy_cfg->check_efuse)
+		goto out;
+
+	cell = nvmem_cell_get(rtk_phy->dev, "usb_u3_tx_lfps_swing_trim");
+	if (IS_ERR(cell)) {
+		dev_warn(rtk_phy->dev,
+			 "%s get usb_u3_tx_lfps_swing_trim failed: %ld\n",
+			 __func__, PTR_ERR(cell));
+	} else {
+		unsigned char *buf;
+		size_t buf_size;
+
+		buf = nvmem_cell_read(cell, &buf_size);
+		value = buf[0] & USB_U3_TX_LFPS_SWING_TRIM_MASK;
+
+		kfree(buf);
+		nvmem_cell_put(cell);
+	}
+
+	if (value > 0 && value < 0x8)
+		phy_parameter->efuse_usb_u3_tx_lfps_swing_trim = 0x8;
+	else
+		phy_parameter->efuse_usb_u3_tx_lfps_swing_trim = (u8)value;
+
+out:
+	return 0;
+}
+
+static void update_amplitude_control_value(struct rtk_phy *rtk_phy,
+					   struct phy_parameter *phy_parameter)
+{
+	struct phy_cfg *phy_cfg;
+	struct phy_reg *phy_reg;
+
+	phy_reg = &phy_parameter->phy_reg;
+	phy_cfg = rtk_phy->phy_cfg;
+
+	if (phy_parameter->amplitude_control_coarse != AMPLITUDE_CONTROL_COARSE_DEFAULT) {
+		u16 val_mask = AMPLITUDE_CONTROL_COARSE_MASK;
+		u16 data;
+
+		if (!phy_cfg->param[PHY_ADDR_0X20].addr && !phy_cfg->param[PHY_ADDR_0X20].data) {
+			phy_cfg->param[PHY_ADDR_0X20].addr = PHY_ADDR_0X20;
+			data = rtk_phy_read(phy_reg, PHY_ADDR_0X20);
+		} else {
+			data = phy_cfg->param[PHY_ADDR_0X20].data;
+		}
+
+		data &= (~val_mask);
+		data |= (phy_parameter->amplitude_control_coarse & val_mask);
+
+		phy_cfg->param[PHY_ADDR_0X20].data = data;
+	}
+
+	if (phy_parameter->efuse_usb_u3_tx_lfps_swing_trim) {
+		u8 efuse_val = phy_parameter->efuse_usb_u3_tx_lfps_swing_trim;
+		u16 val_mask = USB_U3_TX_LFPS_SWING_TRIM_MASK;
+		int val_shift = USB_U3_TX_LFPS_SWING_TRIM_SHIFT;
+		u16 data;
+
+		if (!phy_cfg->param[PHY_ADDR_0X20].addr && !phy_cfg->param[PHY_ADDR_0X20].data) {
+			phy_cfg->param[PHY_ADDR_0X20].addr = PHY_ADDR_0X20;
+			data = rtk_phy_read(phy_reg, PHY_ADDR_0X20);
+		} else {
+			data = phy_cfg->param[PHY_ADDR_0X20].data;
+		}
+
+		data &= ~(val_mask << val_shift);
+		data |= ((efuse_val & val_mask) << val_shift);
+
+		phy_cfg->param[PHY_ADDR_0X20].data = data;
+	}
+
+	if (phy_parameter->amplitude_control_fine != AMPLITUDE_CONTROL_FINE_DEFAULT) {
+		u16 val_mask = AMPLITUDE_CONTROL_FINE_MASK;
+
+		if (!phy_cfg->param[PHY_ADDR_0X21].addr && !phy_cfg->param[PHY_ADDR_0X21].data)
+			phy_cfg->param[PHY_ADDR_0X21].addr = PHY_ADDR_0X21;
+
+		phy_cfg->param[PHY_ADDR_0X21].data =
+			    phy_parameter->amplitude_control_fine & val_mask;
+	}
+}
+
+static int parse_phy_data(struct rtk_phy *rtk_phy)
+{
+	struct device *dev = rtk_phy->dev;
+	struct phy_parameter *phy_parameter;
+	int ret = 0;
+	int index;
+
+	rtk_phy->phy_parameter = devm_kzalloc(dev, sizeof(struct phy_parameter) *
+					      rtk_phy->num_phy, GFP_KERNEL);
+	if (!rtk_phy->phy_parameter)
+		return -ENOMEM;
+
+	for (index = 0; index < rtk_phy->num_phy; index++) {
+		phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
+
+		phy_parameter->phy_reg.reg_mdio_ctl = of_iomap(dev->of_node, 0) + index;
+
+		/* Amplitude control address 0x20 bit 0 to bit 7 */
+		if (of_property_read_u32(dev->of_node, "realtek,amplitude-control-coarse-tuning",
+					 &phy_parameter->amplitude_control_coarse))
+			phy_parameter->amplitude_control_coarse = AMPLITUDE_CONTROL_COARSE_DEFAULT;
+
+		/* Amplitude control address 0x21 bit 0 to bit 16 */
+		if (of_property_read_u32(dev->of_node, "realtek,amplitude-control-fine-tuning",
+					 &phy_parameter->amplitude_control_fine))
+			phy_parameter->amplitude_control_fine = AMPLITUDE_CONTROL_FINE_DEFAULT;
+
+		get_phy_data_by_efuse(rtk_phy, phy_parameter, index);
+
+		update_amplitude_control_value(rtk_phy, phy_parameter);
+	}
+
+	return ret;
+}
+
+static int rtk_usb3phy_probe(struct platform_device *pdev)
+{
+	struct rtk_phy *rtk_phy;
+	struct device *dev = &pdev->dev;
+	struct device_node *node;
+	struct phy *generic_phy;
+	struct phy_provider *phy_provider;
+	const struct phy_cfg *phy_cfg;
+	int ret;
+
+	phy_cfg = of_device_get_match_data(dev);
+	if (!phy_cfg) {
+		dev_err(dev, "phy config are not assigned!\n");
+		return -EINVAL;
+	}
+
+	rtk_phy = devm_kzalloc(dev, sizeof(*rtk_phy), GFP_KERNEL);
+	if (!rtk_phy)
+		return -ENOMEM;
+
+	rtk_phy->dev			= &pdev->dev;
+	rtk_phy->phy.dev		= rtk_phy->dev;
+	rtk_phy->phy.label		= "rtk-usb3phy";
+	rtk_phy->phy.notify_port_status = rtk_phy_notify_port_status;
+
+	rtk_phy->phy_cfg = devm_kzalloc(dev, sizeof(*phy_cfg), GFP_KERNEL);
+
+	memcpy(rtk_phy->phy_cfg, phy_cfg, sizeof(*phy_cfg));
+
+	node = dev->of_node;
+
+	rtk_phy->num_phy = 1;
+
+	ret = parse_phy_data(rtk_phy);
+	if (ret)
+		goto err;
+
+	platform_set_drvdata(pdev, rtk_phy);
+
+	generic_phy = devm_phy_create(rtk_phy->dev, NULL, &ops);
+	if (IS_ERR(generic_phy))
+		return PTR_ERR(generic_phy);
+
+	phy_set_drvdata(generic_phy, rtk_phy);
+
+	phy_provider = devm_of_phy_provider_register(rtk_phy->dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	ret = usb_add_phy_dev(&rtk_phy->phy);
+	if (ret)
+		goto err;
+
+	create_debug_files(rtk_phy);
+
+err:
+	return ret;
+}
+
+static void rtk_usb3phy_remove(struct platform_device *pdev)
+{
+	struct rtk_phy *rtk_phy = platform_get_drvdata(pdev);
+
+	remove_debug_files(rtk_phy);
+
+	usb_remove_phy(&rtk_phy->phy);
+}
+
+static const struct phy_cfg rtd1295_phy_cfg = {
+	.param_size = MAX_USB_PHY_DATA_SIZE,
+	.param = {  [0] = {0x01, 0x4008},  [1] = {0x01, 0xe046},
+		    [2] = {0x02, 0x6046},  [3] = {0x03, 0x2779},
+		    [4] = {0x04, 0x72f5},  [5] = {0x05, 0x2ad3},
+		    [6] = {0x06, 0x000e},  [7] = {0x07, 0x2e00},
+		    [8] = {0x08, 0x3591},  [9] = {0x09, 0x525c},
+		   [10] = {0x0a, 0xa600}, [11] = {0x0b, 0xa904},
+		   [12] = {0x0c, 0xc000}, [13] = {0x0d, 0xef1c},
+		   [14] = {0x0e, 0x2000}, [15] = {0x0f, 0x0000},
+		   [16] = {0x10, 0x000c}, [17] = {0x11, 0x4c00},
+		   [18] = {0x12, 0xfc00}, [19] = {0x13, 0x0c81},
+		   [20] = {0x14, 0xde01}, [21] = {0x15, 0x0000},
+		   [22] = {0x16, 0x0000}, [23] = {0x17, 0x0000},
+		   [24] = {0x18, 0x0000}, [25] = {0x19, 0x4004},
+		   [26] = {0x1a, 0x1260}, [27] = {0x1b, 0xff00},
+		   [28] = {0x1c, 0xcb00}, [29] = {0x1d, 0xa03f},
+		   [30] = {0x1e, 0xc2e0}, [31] = {0x1f, 0x2807},
+		   [32] = {0x20, 0x947a}, [33] = {0x21, 0x88aa},
+		   [34] = {0x22, 0x0057}, [35] = {0x23, 0xab66},
+		   [36] = {0x24, 0x0800}, [37] = {0x25, 0x0000},
+		   [38] = {0x26, 0x040a}, [39] = {0x27, 0x01d6},
+		   [40] = {0x28, 0xf8c2}, [41] = {0x29, 0x3080},
+		   [42] = {0x2a, 0x3082}, [43] = {0x2b, 0x2078},
+		   [44] = {0x2c, 0xffff}, [45] = {0x2d, 0xffff},
+		   [46] = {0x2e, 0x0000}, [47] = {0x2f, 0x0040}, },
+	.check_efuse = false,
+	.do_toggle = true,
+	.do_toggle_once = false,
+	.use_default_parameter = false,
+	.check_rx_front_end_offset = false,
+};
+
+static const struct phy_cfg rtd1619_phy_cfg = {
+	.param_size = MAX_USB_PHY_DATA_SIZE,
+	.param = {  [8] = {0x08, 0x3591},
+		   [38] = {0x26, 0x840b},
+		   [40] = {0x28, 0xf842}, },
+	.check_efuse = false,
+	.do_toggle = true,
+	.do_toggle_once = false,
+	.use_default_parameter = false,
+	.check_rx_front_end_offset = false,
+};
+
+static const struct phy_cfg rtd1319_phy_cfg = {
+	.param_size = MAX_USB_PHY_DATA_SIZE,
+	.param = {  [1] = {0x01, 0xac86},
+		    [6] = {0x06, 0x0003},
+		    [9] = {0x09, 0x924c},
+		   [10] = {0x0a, 0xa608},
+		   [11] = {0x0b, 0xb905},
+		   [14] = {0x0e, 0x2010},
+		   [32] = {0x20, 0x705a},
+		   [33] = {0x21, 0xf645},
+		   [34] = {0x22, 0x0013},
+		   [35] = {0x23, 0xcb66},
+		   [41] = {0x29, 0xff00}, },
+	.check_efuse = true,
+	.do_toggle = true,
+	.do_toggle_once = false,
+	.use_default_parameter = false,
+	.check_rx_front_end_offset = false,
+};
+
+static const struct phy_cfg rtd1619b_phy_cfg = {
+	.param_size = MAX_USB_PHY_DATA_SIZE,
+	.param = {  [1] = {0x01, 0xac8c},
+		    [6] = {0x06, 0x0017},
+		    [9] = {0x09, 0x724c},
+		   [10] = {0x0a, 0xb610},
+		   [11] = {0x0b, 0xb90d},
+		   [13] = {0x0d, 0xef2a},
+		   [15] = {0x0f, 0x9050},
+		   [16] = {0x10, 0x000c},
+		   [32] = {0x20, 0x70ff},
+		   [34] = {0x22, 0x0013},
+		   [35] = {0x23, 0xdb66},
+		   [38] = {0x26, 0x8609},
+		   [41] = {0x29, 0xff13},
+		   [42] = {0x2a, 0x3070}, },
+	.check_efuse = true,
+	.do_toggle = false,
+	.do_toggle_once = true,
+	.use_default_parameter = false,
+	.check_rx_front_end_offset = false,
+};
+
+static const  struct phy_cfg rtd1319d_phy_cfg = {
+	.param_size = MAX_USB_PHY_DATA_SIZE,
+	.param = {  [1] = {0x01, 0xac89},
+		    [4] = {0x04, 0xf2f5},
+		    [6] = {0x06, 0x0017},
+		    [9] = {0x09, 0x424c},
+		   [10] = {0x0a, 0x9610},
+		   [11] = {0x0b, 0x9901},
+		   [12] = {0x0c, 0xf000},
+		   [13] = {0x0d, 0xef2a},
+		   [14] = {0x0e, 0x1000},
+		   [15] = {0x0f, 0x9050},
+		   [32] = {0x20, 0x7077},
+		   [35] = {0x23, 0x0b62},
+		   [37] = {0x25, 0x10ec},
+		   [42] = {0x2a, 0x3070}, },
+	.check_efuse = true,
+	.do_toggle = false,
+	.do_toggle_once = true,
+	.use_default_parameter = false,
+	.check_rx_front_end_offset = true,
+};
+
+static const struct phy_cfg rtk_phy_cfg = {
+	.param_size = MAX_USB_PHY_DATA_SIZE,
+	.param = { /* default parameter */ },
+	.check_efuse = false,
+	.do_toggle = false,
+	.do_toggle_once = false,
+	.use_default_parameter = true,
+	.check_rx_front_end_offset = false,
+};
+
+static const struct of_device_id usbphy_rtk_dt_match[] = {
+	{ .compatible = "realtek,rtd1295-usb3phy", .data = &rtd1295_phy_cfg },
+	{ .compatible = "realtek,rtd1319-usb3phy", .data = &rtd1319_phy_cfg },
+	{ .compatible = "realtek,rtd1319d-usb3phy", .data = &rtd1319d_phy_cfg },
+	{ .compatible = "realtek,rtd1619-usb3phy", .data = &rtd1619_phy_cfg },
+	{ .compatible = "realtek,rtd1619b-usb3phy", .data = &rtd1619b_phy_cfg },
+	{ .compatible = "realtek,usb3phy", .data = &rtk_phy_cfg },
+	{},
+};
+MODULE_DEVICE_TABLE(of, usbphy_rtk_dt_match);
+
+static struct platform_driver rtk_usb3phy_driver = {
+	.probe		= rtk_usb3phy_probe,
+	.remove_new	= rtk_usb3phy_remove,
+	.driver		= {
+		.name	= "rtk-usb3phy",
+		.owner	= THIS_MODULE,
+		.of_match_table = usbphy_rtk_dt_match,
+	},
+};
+
+module_platform_driver(rtk_usb3phy_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform: rtk-usb3phy");
+MODULE_AUTHOR("Stanley Chang <stanley_chang@realtek.com>");
+MODULE_DESCRIPTION("Realtek usb 3.0 phy driver");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY
  2023-06-14  9:28 [PATCH v4 1/5] usb: phy: add usb phy notify port status API Stanley Chang
  2023-06-14  9:28 ` [PATCH v4 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY Stanley Chang
  2023-06-14  9:28 ` [PATCH v4 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY Stanley Chang
@ 2023-06-14  9:28 ` Stanley Chang
  2023-06-17  8:22   ` Krzysztof Kozlowski
  2023-06-14  9:28 ` [PATCH v4 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY Stanley Chang
  3 siblings, 1 reply; 15+ messages in thread
From: Stanley Chang @ 2023-06-14  9:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Stanley Chang, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Alan Stern, Matthias Kaehlcke,
	Douglas Anderson, Ray Chi, Flavio Suligoi, linux-phy, devicetree,
	linux-kernel, linux-usb

Add the documentation explain the property about Realtek USB PHY driver.

Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
controller. Added the driver to drive the USB 2.0 PHY transceivers.

Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
---
v3 to v4 change:
    1. Remove the parameter and non hardware properties from dts.
    2. Using the compatible data included the config and parameter
       in driver.
v2 to v3 change:
    1. Broken down into two patches, one for each of USB 2 & 3.
    2. Add more description about Realtek RTD SoCs architecture.
    3. Removed parameter v1 support for simplification.
    4. Revised the compatible name for fallback compatible.
    5. Remove some properties that can be set in the driver.
v1 to v2 change:
    Add phy-cells for generic phy driver
---
 .../bindings/phy/realtek,usb2phy.yaml         | 145 ++++++++++++++++++
 1 file changed, 145 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
new file mode 100644
index 000000000000..cfd77143475c
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
@@ -0,0 +1,145 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2023 Realtek Semiconductor Corporation
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/realtek,usb2phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek DHC SoCs USB 2.0 PHY
+
+maintainers:
+  - Stanley Chang <stanley_chang@realtek.com>
+
+description:
+  Realtek USB 2.0 PHY support the digital home center (DHC) RTD series SoCs.
+  The USB 2.0 PHY driver is designed to support the XHCI controller. The SoCs
+  support multiple XHCI controllers. One PHY device node maps to one XHCI
+  controller.
+
+  RTD1295/RTD1619 SoCs USB
+  The USB architecture includes three XHCI controllers.
+  Each XHCI maps to one USB 2.0 PHY and map one USB 3.0 PHY on some
+  controllers.
+  XHCI controller#0 -- usb2phy -- phy#0
+                    |- usb3phy -- phy#0
+  XHCI controller#1 -- usb2phy -- phy#0
+  XHCI controller#2 -- usb2phy -- phy#0
+                    |- usb3phy -- phy#0
+
+  RTD1395 SoCs USB
+  The USB architecture includes two XHCI controllers.
+  The controller#0 has one USB 2.0 PHY. The controller#1 includes two USB 2.0
+  PHY.
+  XHCI controller#0 -- usb2phy -- phy#0
+  XHCI controller#1 -- usb2phy -- phy#0
+                               |- phy#1
+
+  RTD1319/RTD1619b SoCs USB
+  The USB architecture includes three XHCI controllers.
+  Each XHCI maps to one USB 2.0 PHY and map one USB 3.0 PHY on controllers#2.
+  XHCI controller#0 -- usb2phy -- phy#0
+  XHCI controller#1 -- usb2phy -- phy#0
+  XHCI controller#2 -- usb2phy -- phy#0
+                    |- usb3phy -- phy#0
+
+  RTD1319d SoCs USB
+  The USB architecture includes three XHCI controllers.
+  Each xhci maps to one USB 2.0 PHY and map one USB 3.0 PHY on controllers#0.
+  XHCI controller#0 -- usb2phy -- phy#0
+                    |- usb3phy -- phy#0
+  XHCI controller#1 -- usb2phy -- phy#0
+  XHCI controller#2 -- usb2phy -- phy#0
+
+  RTD1312c/RTD1315e SoCs USB
+  The USB architecture includes three XHCI controllers.
+  Each XHCI maps to one USB 2.0 PHY.
+  XHCI controller#0 -- usb2phy -- phy#0
+  XHCI controller#1 -- usb2phy -- phy#0
+  XHCI controller#2 -- usb2phy -- phy#0
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - realtek,rtd1295-usb2phy
+          - realtek,rtd1312c-usb2phy
+          - realtek,rtd1315e-usb2phy
+          - realtek,rtd1319-usb2phy
+          - realtek,rtd1319d-usb2phy
+          - realtek,rtd1395-usb2phy
+          - realtek,rtd1395-usb2phy-2port
+          - realtek,rtd1619-usb2phy
+          - realtek,rtd1619b-usb2phy
+      - const: realtek,usb2phy
+
+  reg:
+    items:
+      - description: PHY data registers
+      - description: PHY control registers
+
+  "#phy-cells":
+    const: 0
+
+  nvmem-cells:
+    maxItems: 2
+    description:
+      Phandles to nvmem cell that contains the trimming data.
+      If unspecified, default value is used.
+
+  nvmem-cell-names:
+    items:
+      - const: usb-dc-cal
+      - const: usb-dc-dis
+    description:
+      The following names, which correspond to each nvmem-cells.
+      usb-dc-cal is the driving level for each phy specified via efuse.
+      usb-dc-dis is the disconnection level for each phy specified via efuse.
+
+  realtek,inverse-hstx-sync-clock:
+    description:
+      For one of the phys of RTD1619b SoC, the synchronous clock of the
+      high-speed tx must be inverted. So this property is used to set the
+      inverted clock.
+    type: boolean
+
+  realtek,driving-level:
+    description:
+      Each board or port may have a different driving capability. This
+      will adjust the driving level value if the value is not the default.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 31
+
+  realtek,driving-compensate:
+    description:
+      For RTD1315e SoC, the driving level can be adjusted by reading the
+      efuse table. Therefore, this property provides drive compensation for
+      different boards with different drive capabilities.
+    $ref: /schemas/types.yaml#/definitions/int32
+    minimum: -8
+    maximum: 8
+
+  realtek,disconnection-compensate:
+    description:
+      This adjusts the disconnection level compensation for the different
+      boards with different disconnection level.
+    $ref: /schemas/types.yaml#/definitions/int32
+    minimum: -8
+    maximum: 8
+
+required:
+  - compatible
+  - reg
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    usb_port0_usb2phy: usb-phy@13214 {
+        compatible = "realtek,rtd1319d-usb2phy", "realtek,usb2phy";
+        reg = <0x13214 0x4>, <0x28280 0x4>;
+        #phy-cells = <0>;
+
+        realtek,driving-level = <0xe>;
+    };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY
  2023-06-14  9:28 [PATCH v4 1/5] usb: phy: add usb phy notify port status API Stanley Chang
                   ` (2 preceding siblings ...)
  2023-06-14  9:28 ` [PATCH v4 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY Stanley Chang
@ 2023-06-14  9:28 ` Stanley Chang
  2023-06-17  8:36   ` Krzysztof Kozlowski
  3 siblings, 1 reply; 15+ messages in thread
From: Stanley Chang @ 2023-06-14  9:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Stanley Chang, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Alan Stern, Flavio Suligoi,
	Bagas Sanjaya, Matthias Kaehlcke, Ray Chi, linux-phy, devicetree,
	linux-kernel, linux-usb

Add the documentation explain the property about Realtek USB PHY driver.

Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
controller. Added the driver to drive the  USB 3.0 PHY transceivers.

Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
---
v3 to v4 change:
    1. Remove the parameter and non hardware properties from dts.
    2. Using the compatible data included the config and parameter
       in driver.
v2 to v3 change:
    1. Broken down into two patches, one for each of USB 2 & 3.
    2. Add more description about Realtek RTD SoCs architecture.
    3. Removed parameter v1 support for simplification.
    4. Revised the compatible name for fallback compatible.
    5. Remove some properties that can be set in the driver.
v1 to v2 change:
    Add phy-cells for generic phy driver
---
 .../bindings/phy/realtek,usb3phy.yaml         | 105 ++++++++++++++++++
 1 file changed, 105 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml b/Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml
new file mode 100644
index 000000000000..0f849cf942e8
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml
@@ -0,0 +1,105 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2023 Realtek Semiconductor Corporation
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/realtek,usb3phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek DHC SoCs USB 3.0 PHY
+
+maintainers:
+  - Stanley Chang <stanley_chang@realtek.com>
+
+description:
+  Realtek USB 3.0 PHY support the digital home center (DHC) RTD series SoCs.
+  The USB 3.0 PHY driver is designed to support the XHCI controller. The SoCs
+  support multiple XHCI controllers. One PHY device node maps to one XHCI
+  controller.
+
+  RTD1295/RTD1619 SoCs USB
+  The USB architecture includes three XHCI controllers.
+  Each XHCI maps to one USB 2.0 PHY and map one USB 3.0 PHY on some
+  controllers.
+  XHCI controller#0 -- usb2phy -- phy#0
+                    |- usb3phy -- phy#0
+  XHCI controller#1 -- usb2phy -- phy#0
+  XHCI controller#2 -- usb2phy -- phy#0
+                    |- usb3phy -- phy#0
+
+  RTD1319/RTD1619b SoCs USB
+  The USB architecture includes three XHCI controllers.
+  Each XHCI maps to one USB 2.0 PHY and map one USB 3.0 PHY on controllers#2.
+  XHCI controller#0 -- usb2phy -- phy#0
+  XHCI controller#1 -- usb2phy -- phy#0
+  XHCI controller#2 -- usb2phy -- phy#0
+                    |- usb3phy -- phy#0
+
+  RTD1319d SoCs USB
+  The USB architecture includes three XHCI controllers.
+  Each xhci maps to one USB 2.0 PHY and map one USB 3.0 PHY on controllers#0.
+  XHCI controller#0 -- usb2phy -- phy#0
+                    |- usb3phy -- phy#0
+  XHCI controller#1 -- usb2phy -- phy#0
+  XHCI controller#2 -- usb2phy -- phy#0
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - realtek,rtd1295-usb3phy
+          - realtek,rtd1319-usb3phy
+          - realtek,rtd1319d-usb3phy
+          - realtek,rtd1619-usb3phy
+          - realtek,rtd1619b-usb3phy
+      - const: realtek,usb3phy
+
+  reg:
+    description: PHY data registers
+    maxItems: 1
+
+  "#phy-cells":
+    const: 0
+
+  nvmem-cells:
+    maxItems: 1
+    description: A phandle to the tx lfps swing trim data provided by
+      a nvmem device, if unspecified, default values shall be used.
+
+  nvmem-cell-names:
+    items:
+      - const: usb_u3_tx_lfps_swing_trim
+
+  realtek,amplitude-control-coarse-tuning:
+    description:
+      This adjusts the signal amplitude for normal operation and beacon LFPS.
+      This value is a parameter for coarse tuning.
+      For different boards, if the default value is inappropriate, this
+      property can be assigned to adjust.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 255
+
+  realtek,amplitude-control-fine-tuning:
+    description:
+      This adjusts the signal amplitude for normal operation and beacon LFPS.
+      This value is used for fine-tuning parameters.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 65535
+
+required:
+  - compatible
+  - reg
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    usb_port2_usb3phy: usb-phy@13e10 {
+        compatible = "realtek,rtd1319d-usb3phy", "realtek,usb3phy";
+        reg = <0x13e10 0x4>;
+        #phy-cells = <0>;
+
+        realtek,amplitude-control-coarse-tuning = <0x77>;
+    };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
  2023-06-14  9:28 ` [PATCH v4 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY Stanley Chang
@ 2023-06-14 15:10   ` kernel test robot
  2023-06-17  8:34   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 15+ messages in thread
From: kernel test robot @ 2023-06-14 15:10 UTC (permalink / raw)
  To: Stanley Chang, Greg Kroah-Hartman
  Cc: oe-kbuild-all, Stanley Chang, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alan Stern,
	Flavio Suligoi, Douglas Anderson, Bagas Sanjaya,
	Matthias Kaehlcke, Ray Chi, linux-phy, devicetree, linux-kernel,
	linux-usb

Hi Stanley,

kernel test robot noticed the following build warnings:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus robh/for-next linus/master v6.4-rc6 next-20230614]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Stanley-Chang/phy-realtek-usb-Add-driver-for-the-Realtek-SoC-USB-2-0-PHY/20230614-173349
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20230614092850.21460-2-stanley_chang%40realtek.com
patch subject: [PATCH v4 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20230614/202306142352.e4eBd3HX-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 12.3.0
reproduce (this is a W=1 build):
        mkdir -p ~/bin
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git remote add usb https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
        git fetch usb usb-testing
        git checkout usb/usb-testing
        b4 shazam https://lore.kernel.org/r/20230614092850.21460-2-stanley_chang@realtek.com
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=sparc olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash drivers/phy/realtek/

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306142352.e4eBd3HX-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/phy/realtek/phy-rtk-usb2.c: In function 'parse_phy_data':
>> drivers/phy/realtek/phy-rtk-usb2.c:1229:25: warning: variable 'phy_cfg' set but not used [-Wunused-but-set-variable]
    1229 |         struct phy_cfg *phy_cfg;
         |                         ^~~~~~~


vim +/phy_cfg +1229 drivers/phy/realtek/phy-rtk-usb2.c

  1224	
  1225	static int parse_phy_data(struct rtk_phy *rtk_phy)
  1226	{
  1227		struct device *dev = rtk_phy->dev;
  1228		struct device_node *node;
> 1229		struct phy_cfg *phy_cfg;
  1230		struct phy_parameter *phy_parameter;
  1231		int ret = 0;
  1232		int index;
  1233	
  1234		node = dev->of_node;
  1235		phy_cfg = rtk_phy->phy_cfg;
  1236	
  1237		rtk_phy->phy_parameter = devm_kzalloc(dev, sizeof(struct phy_parameter) *
  1238							rtk_phy->num_phy, GFP_KERNEL);
  1239		if (!rtk_phy->phy_parameter)
  1240			return -ENOMEM;
  1241	
  1242		for (index = 0; index < rtk_phy->num_phy; index++) {
  1243			phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
  1244	
  1245			phy_parameter->phy_reg.reg_wrap_vstatus = of_iomap(dev->of_node, 0);
  1246			phy_parameter->phy_reg.reg_gusb2phyacc0 = of_iomap(dev->of_node, 1) + index;
  1247			phy_parameter->phy_reg.vstatus_index = index;
  1248	
  1249			if (of_property_read_bool(node, "realtek,inverse-hstx-sync-clock"))
  1250				phy_parameter->inverse_hstx_sync_clock = true;
  1251			else
  1252				phy_parameter->inverse_hstx_sync_clock = false;
  1253	
  1254			if (of_property_read_u32_index(node, "realtek,driving-level",
  1255						       index, &phy_parameter->driving_level))
  1256				phy_parameter->driving_level = DEFAULT_DC_DRIVING_VALUE;
  1257	
  1258			if (of_property_read_u32_index(node, "realtek,driving-compensate",
  1259						       index, &phy_parameter->driving_compensate))
  1260				phy_parameter->driving_compensate = 0;
  1261	
  1262			if (of_property_read_u32_index(node, "realtek,disconnection-compensate",
  1263						       index, &phy_parameter->disconnection_compensate))
  1264				phy_parameter->disconnection_compensate = 0;
  1265	
  1266			get_phy_data_by_efuse(rtk_phy, phy_parameter, index);
  1267	
  1268			update_dc_driving_level(rtk_phy, phy_parameter);
  1269	
  1270			update_hs_clk_select(rtk_phy, phy_parameter);
  1271		}
  1272	
  1273		return ret;
  1274	}
  1275	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY
  2023-06-14  9:28 ` [PATCH v4 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY Stanley Chang
@ 2023-06-14 16:13   ` kernel test robot
  2023-06-17  8:35   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 15+ messages in thread
From: kernel test robot @ 2023-06-14 16:13 UTC (permalink / raw)
  To: Stanley Chang, Greg Kroah-Hartman
  Cc: oe-kbuild-all, Stanley Chang, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alan Stern,
	Bagas Sanjaya, Matthias Kaehlcke, Douglas Anderson,
	Flavio Suligoi, Ray Chi, linux-phy, devicetree, linux-kernel,
	linux-usb

Hi Stanley,

kernel test robot noticed the following build warnings:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus robh/for-next linus/master v6.4-rc6 next-20230614]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Stanley-Chang/phy-realtek-usb-Add-driver-for-the-Realtek-SoC-USB-2-0-PHY/20230614-173349
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20230614092850.21460-3-stanley_chang%40realtek.com
patch subject: [PATCH v4 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20230614/202306142340.nvui81i4-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 12.3.0
reproduce (this is a W=1 build):
        mkdir -p ~/bin
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git remote add usb https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
        git fetch usb usb-testing
        git checkout usb/usb-testing
        b4 shazam https://lore.kernel.org/r/20230614092850.21460-3-stanley_chang@realtek.com
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=sparc olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash drivers/phy/realtek/

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306142340.nvui81i4-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/phy/realtek/phy-rtk-usb3.c: In function 'rtk_usb3phy_probe':
>> drivers/phy/realtek/phy-rtk-usb3.c:779:29: warning: variable 'node' set but not used [-Wunused-but-set-variable]
     779 |         struct device_node *node;
         |                             ^~~~


vim +/node +779 drivers/phy/realtek/phy-rtk-usb3.c

   774	
   775	static int rtk_usb3phy_probe(struct platform_device *pdev)
   776	{
   777		struct rtk_phy *rtk_phy;
   778		struct device *dev = &pdev->dev;
 > 779		struct device_node *node;
   780		struct phy *generic_phy;
   781		struct phy_provider *phy_provider;
   782		const struct phy_cfg *phy_cfg;
   783		int ret;
   784	
   785		phy_cfg = of_device_get_match_data(dev);
   786		if (!phy_cfg) {
   787			dev_err(dev, "phy config are not assigned!\n");
   788			return -EINVAL;
   789		}
   790	
   791		rtk_phy = devm_kzalloc(dev, sizeof(*rtk_phy), GFP_KERNEL);
   792		if (!rtk_phy)
   793			return -ENOMEM;
   794	
   795		rtk_phy->dev			= &pdev->dev;
   796		rtk_phy->phy.dev		= rtk_phy->dev;
   797		rtk_phy->phy.label		= "rtk-usb3phy";
   798		rtk_phy->phy.notify_port_status = rtk_phy_notify_port_status;
   799	
   800		rtk_phy->phy_cfg = devm_kzalloc(dev, sizeof(*phy_cfg), GFP_KERNEL);
   801	
   802		memcpy(rtk_phy->phy_cfg, phy_cfg, sizeof(*phy_cfg));
   803	
   804		node = dev->of_node;
   805	
   806		rtk_phy->num_phy = 1;
   807	
   808		ret = parse_phy_data(rtk_phy);
   809		if (ret)
   810			goto err;
   811	
   812		platform_set_drvdata(pdev, rtk_phy);
   813	
   814		generic_phy = devm_phy_create(rtk_phy->dev, NULL, &ops);
   815		if (IS_ERR(generic_phy))
   816			return PTR_ERR(generic_phy);
   817	
   818		phy_set_drvdata(generic_phy, rtk_phy);
   819	
   820		phy_provider = devm_of_phy_provider_register(rtk_phy->dev, of_phy_simple_xlate);
   821		if (IS_ERR(phy_provider))
   822			return PTR_ERR(phy_provider);
   823	
   824		ret = usb_add_phy_dev(&rtk_phy->phy);
   825		if (ret)
   826			goto err;
   827	
   828		create_debug_files(rtk_phy);
   829	
   830	err:
   831		return ret;
   832	}
   833	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY
  2023-06-14  9:28 ` [PATCH v4 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY Stanley Chang
@ 2023-06-17  8:22   ` Krzysztof Kozlowski
  2023-06-20  8:58     ` Stanley Chang[昌育德]
  0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-17  8:22 UTC (permalink / raw)
  To: Stanley Chang, Greg Kroah-Hartman
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Alan Stern, Matthias Kaehlcke,
	Douglas Anderson, Ray Chi, Flavio Suligoi, linux-phy, devicetree,
	linux-kernel, linux-usb

On 14/06/2023 11:28, Stanley Chang wrote:
> Add the documentation explain the property about Realtek USB PHY driver.
> 
> Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
> controller. Added the driver to drive the USB 2.0 PHY transceivers.
> 
> Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
> ---
> v3 to v4 change:
>     1. Remove the parameter and non hardware properties from dts.
>     2. Using the compatible data included the config and parameter
>        in driver.
> v2 to v3 change:
>     1. Broken down into two patches, one for each of USB 2 & 3.
>     2. Add more description about Realtek RTD SoCs architecture.
>     3. Removed parameter v1 support for simplification.
>     4. Revised the compatible name for fallback compatible.
>     5. Remove some properties that can be set in the driver.
> v1 to v2 change:
>     Add phy-cells for generic phy driver
> ---
>  .../bindings/phy/realtek,usb2phy.yaml         | 145 ++++++++++++++++++
>  1 file changed, 145 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
> new file mode 100644
> index 000000000000..cfd77143475c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
> @@ -0,0 +1,145 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Realtek Semiconductor Corporation
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/realtek,usb2phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Realtek DHC SoCs USB 2.0 PHY
> +
> +maintainers:
> +  - Stanley Chang <stanley_chang@realtek.com>
> +
> +description:
> +  Realtek USB 2.0 PHY support the digital home center (DHC) RTD series SoCs.
> +  The USB 2.0 PHY driver is designed to support the XHCI controller. The SoCs
> +  support multiple XHCI controllers. One PHY device node maps to one XHCI
> +  controller.
> +
> +  RTD1295/RTD1619 SoCs USB
> +  The USB architecture includes three XHCI controllers.
> +  Each XHCI maps to one USB 2.0 PHY and map one USB 3.0 PHY on some
> +  controllers.
> +  XHCI controller#0 -- usb2phy -- phy#0
> +                    |- usb3phy -- phy#0
> +  XHCI controller#1 -- usb2phy -- phy#0
> +  XHCI controller#2 -- usb2phy -- phy#0
> +                    |- usb3phy -- phy#0
> +
> +  RTD1395 SoCs USB
> +  The USB architecture includes two XHCI controllers.
> +  The controller#0 has one USB 2.0 PHY. The controller#1 includes two USB 2.0
> +  PHY.
> +  XHCI controller#0 -- usb2phy -- phy#0
> +  XHCI controller#1 -- usb2phy -- phy#0
> +                               |- phy#1
> +
> +  RTD1319/RTD1619b SoCs USB
> +  The USB architecture includes three XHCI controllers.
> +  Each XHCI maps to one USB 2.0 PHY and map one USB 3.0 PHY on controllers#2.
> +  XHCI controller#0 -- usb2phy -- phy#0
> +  XHCI controller#1 -- usb2phy -- phy#0
> +  XHCI controller#2 -- usb2phy -- phy#0
> +                    |- usb3phy -- phy#0
> +
> +  RTD1319d SoCs USB
> +  The USB architecture includes three XHCI controllers.
> +  Each xhci maps to one USB 2.0 PHY and map one USB 3.0 PHY on controllers#0.
> +  XHCI controller#0 -- usb2phy -- phy#0
> +                    |- usb3phy -- phy#0
> +  XHCI controller#1 -- usb2phy -- phy#0
> +  XHCI controller#2 -- usb2phy -- phy#0
> +
> +  RTD1312c/RTD1315e SoCs USB
> +  The USB architecture includes three XHCI controllers.
> +  Each XHCI maps to one USB 2.0 PHY.
> +  XHCI controller#0 -- usb2phy -- phy#0
> +  XHCI controller#1 -- usb2phy -- phy#0
> +  XHCI controller#2 -- usb2phy -- phy#0
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - realtek,rtd1295-usb2phy
> +          - realtek,rtd1312c-usb2phy
> +          - realtek,rtd1315e-usb2phy
> +          - realtek,rtd1319-usb2phy
> +          - realtek,rtd1319d-usb2phy
> +          - realtek,rtd1395-usb2phy
> +          - realtek,rtd1395-usb2phy-2port
> +          - realtek,rtd1619-usb2phy
> +          - realtek,rtd1619b-usb2phy
> +      - const: realtek,usb2phy

That's not what your driver is saying... This patchset has random set of
changes.

I suggest to drop "realtek,usb2phy".

> +
> +  reg:
> +    items:
> +      - description: PHY data registers
> +      - description: PHY control registers
> +
> +  "#phy-cells":
> +    const: 0
> +
> +  nvmem-cells:
> +    maxItems: 2
> +    description:
> +      Phandles to nvmem cell that contains the trimming data.
> +      If unspecified, default value is used.
> +
> +  nvmem-cell-names:
> +    items:
> +      - const: usb-dc-cal
> +      - const: usb-dc-dis
> +    description:
> +      The following names, which correspond to each nvmem-cells.
> +      usb-dc-cal is the driving level for each phy specified via efuse.
> +      usb-dc-dis is the disconnection level for each phy specified via efuse.
> +
> +  realtek,inverse-hstx-sync-clock:
> +    description:
> +      For one of the phys of RTD1619b SoC, the synchronous clock of the
> +      high-speed tx must be inverted. So this property is used to set the
> +      inverted clock.

Drop last sentence, it is redundant.

> +    type: boolean
> +
> +  realtek,driving-level:
> +    description:
> +      Each board or port may have a different driving capability. This
> +      will adjust the driving level value if the value is not the default.

I don't understand it. What is "driving capability" and if each port can
have it different, why do you need property for this?

You mention some default - why it is not expressed as "default: xx"?

What do the values mean?

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 31
> +
> +  realtek,driving-compensate:
> +    description:
> +      For RTD1315e SoC, the driving level can be adjusted by reading the
> +      efuse table. Therefore, this property provides drive compensation for
> +      different boards with different drive capabilities.

if driving level can be read from nvmem, why do you have
realtek,driving-level in the first place? Don't you miss here some allOf
making this constrained per variant?

"Therefore" means "for that reason" or "as a consequence". How is this
property a consequence of reading driving level from efuse? Is it then
mutually exclusive with "realtek,driving-level"? But your schema does
not express it.

> +    $ref: /schemas/types.yaml#/definitions/int32
> +    minimum: -8
> +    maximum: 8
> +
> +  realtek,disconnection-compensate:
> +    description:
> +      This adjusts the disconnection level compensation for the different
> +      boards with different disconnection level.
> +    $ref: /schemas/types.yaml#/definitions/int32
> +    minimum: -8
> +    maximum: 8
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    usb_port0_usb2phy: usb-phy@13214 {
> +        compatible = "realtek,rtd1319d-usb2phy", "realtek,usb2phy";
> +        reg = <0x13214 0x4>, <0x28280 0x4>;
> +        #phy-cells = <0>;
> +
> +        realtek,driving-level = <0xe>;

Why this example is so empty? You have at least 6 more properties which
should be shown here.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
  2023-06-14  9:28 ` [PATCH v4 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY Stanley Chang
  2023-06-14 15:10   ` kernel test robot
@ 2023-06-17  8:34   ` Krzysztof Kozlowski
  2023-06-20  8:58     ` Stanley Chang[昌育德]
  1 sibling, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-17  8:34 UTC (permalink / raw)
  To: Stanley Chang, Greg Kroah-Hartman
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Alan Stern, Flavio Suligoi,
	Douglas Anderson, Bagas Sanjaya, Matthias Kaehlcke, Ray Chi,
	linux-phy, devicetree, linux-kernel, linux-usb

On 14/06/2023 11:28, Stanley Chang wrote:
> Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
> controller. Added the driver to drive the USB 2.0 PHY transceivers.
> 
> Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
> ---
> v3 to v4 change:
>     1. Remove the parameter and non hardware properties from dts.
>        Add the compatible data included the config and parameter.
>     2. Fix the warning for checkpatch with strict.
>     3. Remove useless debug messages.
>     4. Fix the incorrect or useless "if() return" which pointer check.
>     5. Remove the phy power domain control due to we control on other
>        driver.
> v2 to v3 change:
>     1. Broken down into two patches, one for each of USB 2 & 3 PHY.
>     2. Removed parameter v1 support for simplification.
>     3. Use remove_new for driver remove callback.
> v1 to v2 change:
>     1. Move the drivers to drivers/phy/ for generic phy driver
>     2. Use the generic phy driver api to initialize phy
>     3. Use readl/writel to instead phy_read/phy_write directly.
>     4. Use read_poll_timeout() in function utmi_wait_register.
>     5. Revised some coding styles.
>     6. fix the compiler warning for kernel test robot.
> ---

...

> +/* updated disconnect level at page0 */
> +static void update_dc_disconnect_level_at_page0(struct rtk_phy *rtk_phy,
> +						struct phy_parameter *phy_parameter, bool update)
> +{
> +	struct phy_cfg *phy_cfg;
> +	struct phy_reg *phy_reg;
> +	struct phy_data *phy_data_page;
> +	struct phy_data *phy_data;
> +	u8 addr, data;
> +	int offset = 4;
> +	s32 dc_disconnect_mask;
> +	int i;
> +
> +	phy_cfg = rtk_phy->phy_cfg;
> +	phy_reg = &phy_parameter->phy_reg;
> +
> +	/* Set page 0 */
> +	phy_data_page = phy_cfg->page0;
> +	rtk_phy_set_page(phy_reg, 0);
> +
> +	i = page_addr_to_array_index(PAGE0_0XE4);
> +	phy_data = phy_data_page + i;
> +	if (!phy_data->addr) {
> +		phy_data->addr = PAGE0_0XE4;
> +		phy_data->data = rtk_phy_read(phy_reg, PAGE0_0XE4);
> +	}
> +
> +	addr = phy_data->addr;
> +	data = phy_data->data;
> +	dc_disconnect_mask = phy_cfg->dc_disconnect_mask;
> +
> +	if (update)
> +		data = __updated_dc_disconnect_level_page0_0xe4(phy_cfg, phy_parameter, data);
> +	else
> +		data = (data & ~(dc_disconnect_mask << offset)) |
> +			(DEFAULT_DC_DISCONNECTION_VALUE << offset);
> +
> +	if (rtk_phy_write(phy_reg, addr, data))
> +		dev_err(rtk_phy->dev,
> +			"[%s:%d] Error page1 addr=0x%x value=0x%x\n",
> +			__func__, __LINE__,
> +			addr, data);

Is addr a kernel address or any memory (not SFR) address? If so, you
cannot print it.

> +}
> +
> +static u8 __updated_dc_disconnect_level_page1_0xe2(struct phy_cfg *phy_cfg,
> +						   struct phy_parameter *phy_parameter, u8 data)
> +{
> +	u8 val;
> +	s32 __val;
> +	s32 dc_disconnect_mask = phy_cfg->dc_disconnect_mask;
> +
> +	if (phy_cfg->check_efuse_version == CHECK_EFUSE_V1) {
> +		__val = (s32)(data & dc_disconnect_mask)
> +			    + phy_parameter->efuse_usb_dc_dis
> +			    + phy_parameter->disconnection_compensate;
> +	} else { /* for CHECK_EFUSE_V2 or no efuse */
> +		if (phy_parameter->efuse_usb_dc_dis)
> +			__val = (s32)(phy_parameter->efuse_usb_dc_dis +
> +				    phy_parameter->disconnection_compensate);
> +		else
> +			__val = (s32)((data & dc_disconnect_mask) +
> +				    phy_parameter->disconnection_compensate);
> +	}
> +
> +	if (__val > dc_disconnect_mask)
> +		__val = dc_disconnect_mask;
> +	else if (__val < 0)
> +		__val = 0;
> +
> +	val = (data & (~dc_disconnect_mask)) | (__val & dc_disconnect_mask);
> +
> +	return val;
> +}
> +
> +/* updated disconnect level at page1 */
> +static void update_dc_disconnect_level_at_page1(struct rtk_phy *rtk_phy,
> +						struct phy_parameter *phy_parameter, bool update)
> +{
> +	struct phy_cfg *phy_cfg;
> +	struct phy_data *phy_data_page;
> +	struct phy_data *phy_data;
> +	struct phy_reg *phy_reg;
> +	u8 addr, data;
> +	s32 dc_disconnect_mask;
> +	int i;
> +
> +	phy_cfg = rtk_phy->phy_cfg;
> +	phy_reg = &phy_parameter->phy_reg;
> +
> +	/* Set page 1 */
> +	phy_data_page = phy_cfg->page1;
> +	rtk_phy_set_page(phy_reg, 1);
> +
> +	i = page_addr_to_array_index(PAGE1_0XE2);
> +	phy_data = phy_data_page + i;
> +	if (!phy_data->addr) {
> +		phy_data->addr = PAGE1_0XE2;
> +		phy_data->data = rtk_phy_read(phy_reg, PAGE1_0XE2);
> +	}
> +
> +	addr = phy_data->addr;
> +	data = phy_data->data;
> +	dc_disconnect_mask = phy_cfg->dc_disconnect_mask;
> +
> +	if (update)
> +		data = __updated_dc_disconnect_level_page1_0xe2(phy_cfg, phy_parameter, data);
> +	else
> +		data = (data & ~dc_disconnect_mask) | DEFAULT_DC_DISCONNECTION_VALUE;
> +
> +	if (rtk_phy_write(phy_reg, addr, data))
> +		dev_err(rtk_phy->dev,
> +			"[%s:%d] Error page1 addr=0x%x value=0x%x\n",
> +			__func__, __LINE__,
> +			addr, data);

Ditto and in all other places.

> +}
> +
> +static void update_dc_disconnect_level(struct rtk_phy *rtk_phy,
> +				       struct phy_parameter *phy_parameter, bool update)
> +{
> +	struct phy_cfg *phy_cfg = rtk_phy->phy_cfg;
> +
> +	if (phy_cfg->usb_dc_disconnect_at_page0)
> +		update_dc_disconnect_level_at_page0(rtk_phy, phy_parameter, update);
> +	else
> +		update_dc_disconnect_level_at_page1(rtk_phy, phy_parameter, update);
> +}
> +
> +static u8 __update_dc_driving_page0_0xe4(struct phy_cfg *phy_cfg,
> +					 struct phy_parameter *phy_parameter, u8 data)
> +{
> +	s32 driving_compensate = phy_parameter->driving_compensate;
> +	s32 dc_driving_mask = phy_cfg->dc_driving_mask;
> +	s32 __val;
> +	u8 val;

Two variables with the same name. No, it is not readable code.

...

> +static void update_dc_driving_level(struct rtk_phy *rtk_phy,
> +				    struct phy_parameter *phy_parameter)
> +{
> +	struct phy_cfg *phy_cfg;
> +	struct phy_reg *phy_reg;
> +

...
> +
> +static const struct phy_ops ops = {
> +	.init		= rtk_phy_init,
> +	.exit		= rtk_phy_exit,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static void rtk_phy_toggle(struct usb_phy *usb2_phy, bool connect, int port)
> +{
> +	int index = port;
> +	struct rtk_phy *rtk_phy = NULL;
> +
> +	rtk_phy = dev_get_drvdata(usb2_phy->dev);
> +
> +	if (index > rtk_phy->num_phy) {
> +		pr_err("%s %d ERROR! port=%d > num_phy=%d\n",

dev_err

> +		       __func__, __LINE__, index, rtk_phy->num_phy);

all these func and LINE point to poor code quality and poor debugging
practices. These are added dugin development, not for production code,
because error message should be obvious. Your usage of pr_err, func,
LINE and some unprecise messages suggests this is not ready.

Fix all your error messages to be meaningful.

> +		return
> +	}
> +
> +	do_rtk_phy_toggle(rtk_phy, index, connect);
> +}
> +
> +static int rtk_phy_notify_port_status(struct usb_phy *x, int port,
> +				      u16 portstatus, u16 portchange)
> +{
> +	bool connect = false;
> +
> +	pr_debug("%s port=%d portstatus=0x%x portchange=0x%x\n",
> +		 __func__, port, (int)portstatus, (int)portchange);
> +	if (portstatus & USB_PORT_STAT_CONNECTION)
> +		connect = true;
> +
> +	if (portchange & USB_PORT_STAT_C_CONNECTION)
> +		rtk_phy_toggle(x, connect, port);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_DEBUG_FS
> +static struct dentry *create_phy_debug_root(void)
> +{
> +	struct dentry *phy_debug_root;
> +
> +	phy_debug_root = debugfs_lookup("phy", usb_debug_root);
> +	if (!phy_debug_root)
> +		phy_debug_root = debugfs_create_dir("phy", usb_debug_root);
> +
> +	return phy_debug_root;
> +}
> +
> +static int rtk_usb2_parameter_show(struct seq_file *s, void *unused)
> +{
> +	struct rtk_phy *rtk_phy = s->private;
> +	struct phy_cfg *phy_cfg;
> +	int i, index;
> +
> +	phy_cfg = rtk_phy->phy_cfg;
> +
> +	seq_puts(s, "Property:\n");
> +	seq_printf(s, "  check_efuse: %s\n",
> +		   phy_cfg->check_efuse ? "Enable" : "Disable");
> +	seq_printf(s, "  check_efuse_version: %d\n",
> +		   phy_cfg->check_efuse_version);
> +	seq_printf(s, "  efuse_dc_driving_rate: %d\n",
> +		   phy_cfg->efuse_dc_driving_rate);
> +	seq_printf(s, "  dc_driving_mask: 0x%x\n",
> +		   phy_cfg->dc_driving_mask);
> +	seq_printf(s, "  efuse_dc_disconnect_rate: %d\n",
> +		   phy_cfg->efuse_dc_disconnect_rate);
> +	seq_printf(s, "  dc_disconnect_mask: 0x%x\n",
> +		   phy_cfg->dc_disconnect_mask);
> +	seq_printf(s, "  usb_dc_disconnect_at_page0: %s\n",
> +		   phy_cfg->usb_dc_disconnect_at_page0 ? "true" : "false");
> +	seq_printf(s, "  do_toggle: %s\n",
> +		   phy_cfg->do_toggle ? "Enable" : "Disable");
> +	seq_printf(s, "  do_toggle_driving: %s\n",
> +		   phy_cfg->do_toggle_driving ? "Enable" : "Disable");
> +	seq_printf(s, "  driving_updated_for_dev_dis: 0x%x\n",
> +		   phy_cfg->driving_updated_for_dev_dis);
> +	seq_printf(s, "  use_default_parameter: %s\n",
> +		   phy_cfg->use_default_parameter ? "Enable" : "Disable");
> +	seq_printf(s, "  is_double_sensitivity_mode: %s\n",
> +		   phy_cfg->is_double_sensitivity_mode ? "Enable" : "Disable");
> +
> +	for (index = 0; index < rtk_phy->num_phy; index++) {
> +		struct phy_parameter *phy_parameter;
> +		struct phy_reg *phy_reg;
> +		struct phy_data *phy_data_page;
> +
> +		phy_parameter =  &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
> +		phy_reg = &phy_parameter->phy_reg;
> +
> +		seq_printf(s, "PHY %d:\n", index);
> +
> +		seq_puts(s, "Page 0:\n");
> +		/* Set page 0 */
> +		phy_data_page = phy_cfg->page0;
> +		rtk_phy_set_page(phy_reg, 0);
> +
> +		for (i = 0; i < phy_cfg->page0_size; i++) {
> +			struct phy_data *phy_data = phy_data_page + i;
> +			u8 addr = array_index_to_page_addr(i);
> +			u8 data = phy_data->data;
> +			u8 value = rtk_phy_read(phy_reg, addr);
> +
> +			if (phy_data->addr)
> +				seq_printf(s, "  Page 0: addr=0x%x data=0x%02x ==> read value=0x%02x\n",
> +					   addr, data, value);
> +			else
> +				seq_printf(s, "  Page 0: addr=0x%x data=none ==> read value=0x%02x\n",
> +					   addr, value);
> +		}
> +
> +		seq_puts(s, "Page 1:\n");
> +		/* Set page 1 */
> +		phy_data_page = phy_cfg->page1;
> +		rtk_phy_set_page(phy_reg, 1);
> +
> +		for (i = 0; i < phy_cfg->page1_size; i++) {
> +			struct phy_data *phy_data = phy_data_page + i;
> +			u8 addr = array_index_to_page_addr(i);
> +			u8 data = phy_data->data;
> +			u8 value = rtk_phy_read(phy_reg, addr);
> +
> +			if (phy_data->addr)
> +				seq_printf(s, "  Page 1: addr=0x%x data=0x%02x ==> read value=0x%02x\n",
> +					   addr, data, value);
> +			else
> +				seq_printf(s, "  Page 1: addr=0x%x data=none ==> read value=0x%02x\n",
> +					   addr, value);
> +		}
> +
> +		if (phy_cfg->page2_size == 0)
> +			goto out;
> +
> +		seq_puts(s, "Page 2:\n");
> +		/* Set page 2 */
> +		phy_data_page = phy_cfg->page2;
> +		rtk_phy_set_page(phy_reg, 2);
> +
> +		for (i = 0; i < phy_cfg->page2_size; i++) {
> +			struct phy_data *phy_data = phy_data_page + i;
> +			u8 addr = array_index_to_page_addr(i);
> +			u8 data = phy_data->data;
> +			u8 value = rtk_phy_read(phy_reg, addr);
> +
> +			if (phy_data->addr)
> +				seq_printf(s, "  Page 2: addr=0x%x data=0x%02x ==> read value=0x%02x\n",
> +					   addr, data, value);
> +			else
> +				seq_printf(s, "  Page 2: addr=0x%x data=none ==> read value=0x%02x\n",
> +					   addr, value);
> +		}
> +
> +out:
> +		seq_puts(s, "PHY Property:\n");
> +		seq_printf(s, "  efuse_usb_dc_cal: %d\n",
> +			   (int)phy_parameter->efuse_usb_dc_cal);
> +		seq_printf(s, "  efuse_usb_dc_dis: %d\n",
> +			   (int)phy_parameter->efuse_usb_dc_dis);
> +		seq_printf(s, "  inverse_hstx_sync_clock: %s\n",
> +			   phy_parameter->inverse_hstx_sync_clock ? "Enable" : "Disable");
> +		seq_printf(s, "  driving_level: %d\n",
> +			   phy_parameter->driving_level);
> +		seq_printf(s, "  driving_compensate: %d\n",
> +			   phy_parameter->driving_compensate);
> +		seq_printf(s, "  disconnection_compensate: %d\n",
> +			   phy_parameter->disconnection_compensate);
> +	}
> +
> +	return 0;
> +}
> +
> +static int rtk_usb2_parameter_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, rtk_usb2_parameter_show, inode->i_private);
> +}
> +
> +static const struct file_operations rtk_usb2_parameter_fops = {
> +	.open			= rtk_usb2_parameter_open,
> +	.read			= seq_read,
> +	.llseek			= seq_lseek,
> +	.release		= single_release,
> +};
> +
> +static int __get_parameter_at_page(struct seq_file *s,
> +				   struct rtk_phy *rtk_phy,
> +				   struct phy_data *phy_data_page,
> +				   const char *phy_page, const char *phy_addr)
> +{
> +	struct phy_data *phy_data;
> +	u32 addr;
> +	int i, ret;
> +
> +	ret = kstrtouint(phy_addr, 16, &addr);
> +	if (ret < 0) {
> +		pr_err("%s::kstrtouint() failed\n", __func__);
> +		return -EINVAL;
> +	}
> +	i = page_addr_to_array_index(addr);
> +	phy_data = (phy_data_page + i);
> +
> +	if (phy_data->addr)
> +		seq_printf(s, "Now Parameter %s addr 0x%02x = 0x%02x\n",
> +			   phy_page, phy_data->addr, phy_data->data);
> +	else
> +		seq_printf(s, "Now Parameter %s addr 0x%02x is default\n",
> +			   phy_page, addr);
> +
> +	return 0;
> +}
> +
> +static int __set_parameter_at_page(struct rtk_phy *rtk_phy,
> +				   struct phy_reg *phy_reg, struct phy_parameter *phy_parameter,
> +				   struct phy_data *phy_data_page,
> +				   const char *phy_page, const char *phy_addr,
> +				   const char *phy_value)
> +{
> +	struct phy_data *phy_data;
> +	u32 addr, value;
> +	int i, ret;
> +
> +	ret = kstrtouint(phy_addr, 16, &addr);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	ret = kstrtouint(phy_value, 16, &value);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	i = page_addr_to_array_index(addr);
> +	phy_data = (phy_data_page + i);
> +
> +	if (phy_data->addr) {
> +		phy_data->data = value;
> +	} else {
> +		phy_data->addr = addr;
> +		phy_data->data = value;
> +	}
> +
> +	if (rtk_phy_write(phy_reg, addr, value))
> +		dev_err(rtk_phy->dev,
> +			"[%s:%d] Error: addr=0x%02x value=0x%02x\n",
> +			__func__, __LINE__, addr, value);
> +
> +	return 0;
> +}
> +
> +static int rtk_usb2_set_parameter_show(struct seq_file *s, void *unused)
> +{
> +	const struct file *file = s->file;
> +	const char *file_name = file_dentry(file)->d_iname;
> +	struct dentry *p_dentry = file_dentry(file)->d_parent;
> +	const char *dir_name = p_dentry->d_iname;
> +	struct dentry *pp_dentry = p_dentry->d_parent;
> +	const char *phy_dir_name = pp_dentry->d_iname;
> +	struct rtk_phy *rtk_phy = s->private;
> +	struct phy_cfg *phy_cfg;
> +	struct phy_parameter *phy_parameter = NULL;
> +	int ret = 0;
> +	int index;
> +
> +	for (index = 0; index < rtk_phy->num_phy; index++) {
> +		size_t sz = 30;
> +		char name[30] = {0};
> +
> +		snprintf(name, sz, "phy%d", index);
> +		if (strncmp(name, phy_dir_name, strlen(name)) == 0) {
> +			phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
> +			break;
> +		}
> +	}
> +
> +	if (!phy_parameter)
> +		return -EINVAL;
> +
> +	phy_cfg = rtk_phy->phy_cfg;
> +
> +	if (strcmp("page0", dir_name) == 0)
> +		ret = __get_parameter_at_page(s, rtk_phy, phy_cfg->page0,
> +					      dir_name, file_name);
> +	else if (strcmp("page1", dir_name) == 0)
> +		ret = __get_parameter_at_page(s, rtk_phy, phy_cfg->page1,
> +					      dir_name, file_name);
> +	else if (strcmp("page2", dir_name) == 0)
> +		ret = __get_parameter_at_page(s, rtk_phy, phy_cfg->page2,
> +					      dir_name, file_name);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	seq_puts(s, "Set phy parameter by following command\n");
> +	seq_printf(s, "echo \"value\" > %s/%s/%s\n",
> +		   phy_dir_name, dir_name, file_name);
> +
> +	return 0;
> +}
> +
> +static int rtk_usb2_set_parameter_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, rtk_usb2_set_parameter_show, inode->i_private);
> +}
> +
> +static ssize_t rtk_usb2_set_parameter_write(struct file *file,
> +					    const char __user *ubuf, size_t count,
> +					    loff_t *ppos)
> +{
> +	const char *file_name = file_dentry(file)->d_iname;
> +	struct dentry *p_dentry = file_dentry(file)->d_parent;
> +	const char *dir_name = p_dentry->d_iname;
> +	struct dentry *pp_dentry = p_dentry->d_parent;
> +	const char *phy_dir_name = pp_dentry->d_iname;
> +	struct seq_file *s = file->private_data;
> +	struct rtk_phy *rtk_phy = s->private;
> +	struct phy_reg *phy_reg;
> +	struct phy_cfg *phy_cfg;
> +	struct phy_parameter *phy_parameter = NULL;
> +	int ret = 0;
> +	char buffer[40] = {0};
> +	int index;
> +
> +	if (copy_from_user(&buffer, ubuf,
> +			   min_t(size_t, sizeof(buffer) - 1, count)))
> +		return -EFAULT;
> +
> +	for (index = 0; index < rtk_phy->num_phy; index++) {
> +		size_t sz = 30;
> +		char name[30] = {0};
> +
> +		snprintf(name, sz, "phy%d", index);
> +		if (strncmp(name, phy_dir_name, strlen(name)) == 0) {
> +			phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
> +			break;
> +		}
> +	}
> +
> +	if (!phy_parameter)
> +		return -EINVAL;
> +
> +	phy_cfg = rtk_phy->phy_cfg;
> +	phy_reg = &phy_parameter->phy_reg;
> +
> +	if (strcmp("page0", dir_name) == 0) {
> +		rtk_phy_set_page(phy_reg, 0);
> +		ret = __set_parameter_at_page(rtk_phy, phy_reg, phy_parameter,
> +					      phy_cfg->page0, dir_name, file_name, buffer);
> +	} else if (strcmp("page1", dir_name) == 0) {
> +		rtk_phy_set_page(phy_reg, 1);
> +		ret = __set_parameter_at_page(rtk_phy, phy_reg, phy_parameter,
> +					      phy_cfg->page1, dir_name, file_name, buffer);
> +	} else if (strcmp("page2", dir_name) == 0) {
> +		rtk_phy_set_page(phy_reg, 2);
> +		ret = __set_parameter_at_page(rtk_phy, phy_reg, phy_parameter,
> +					      phy_cfg->page2, dir_name, file_name, buffer);
> +	}
> +	if (ret < 0)
> +		return ret;
> +
> +	return count;
> +}
> +
> +static const struct file_operations rtk_usb2_set_parameter_fops = {
> +	.open			= rtk_usb2_set_parameter_open,
> +	.write			= rtk_usb2_set_parameter_write,

NAK. You just created user interface via debugfs. You cannot. Reading
for some debug is okay, but configuring device via undocumented debugfs
is a source of troubles.

Drop all writes to debugfs.

...

> +
> +static int parse_phy_data(struct rtk_phy *rtk_phy)
> +{
> +	struct device *dev = rtk_phy->dev;
> +	struct device_node *node;

By convention:
s/node/np/

> +	struct phy_cfg *phy_cfg;
> +	struct phy_parameter *phy_parameter;
> +	int ret = 0;
> +	int index;
> +
> +	node = dev->of_node;

Keep it in variable definition.

> +	phy_cfg = rtk_phy->phy_cfg;
> +
> +	rtk_phy->phy_parameter = devm_kzalloc(dev, sizeof(struct phy_parameter) *
> +						rtk_phy->num_phy, GFP_KERNEL);
> +	if (!rtk_phy->phy_parameter)
> +		return -ENOMEM;
> +
> +	for (index = 0; index < rtk_phy->num_phy; index++) {
> +		phy_parameter = &((struct phy_parameter *)rtk_phy->phy_parameter)[index];
> +
> +		phy_parameter->phy_reg.reg_wrap_vstatus = of_iomap(dev->of_node, 0);
> +		phy_parameter->phy_reg.reg_gusb2phyacc0 = of_iomap(dev->of_node, 1) + index;
> +		phy_parameter->phy_reg.vstatus_index = index;
> +
> +		if (of_property_read_bool(node, "realtek,inverse-hstx-sync-clock"))
> +			phy_parameter->inverse_hstx_sync_clock = true;
> +		else
> +			phy_parameter->inverse_hstx_sync_clock = false;
> +
> +		if (of_property_read_u32_index(node, "realtek,driving-level",
> +					       index, &phy_parameter->driving_level))
> +			phy_parameter->driving_level = DEFAULT_DC_DRIVING_VALUE;
> +
> +		if (of_property_read_u32_index(node, "realtek,driving-compensate",
> +					       index, &phy_parameter->driving_compensate))
> +			phy_parameter->driving_compensate = 0;
> +
> +		if (of_property_read_u32_index(node, "realtek,disconnection-compensate",
> +					       index, &phy_parameter->disconnection_compensate))
> +			phy_parameter->disconnection_compensate = 0;
> +
> +		get_phy_data_by_efuse(rtk_phy, phy_parameter, index);
> +
> +		update_dc_driving_level(rtk_phy, phy_parameter);
> +
> +		update_hs_clk_select(rtk_phy, phy_parameter);
> +	}
> +
> +	return ret;
> +}
> +
> +static int rtk_usb2phy_probe(struct platform_device *pdev)
> +{
> +	struct rtk_phy *rtk_phy;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node;
> +	struct phy *generic_phy;
> +	struct phy_provider *phy_provider;
> +	const struct phy_cfg *phy_cfg;
> +	int ret = 0;
> +
> +	phy_cfg = of_device_get_match_data(dev);
> +	if (!phy_cfg) {
> +		dev_err(dev, "phy config are not assigned!\n");
> +		return -EINVAL;
> +	}
> +
> +	rtk_phy = devm_kzalloc(dev, sizeof(*rtk_phy), GFP_KERNEL);
> +	if (!rtk_phy)
> +		return -ENOMEM;
> +
> +	rtk_phy->dev			= &pdev->dev;
> +	rtk_phy->phy.dev		= rtk_phy->dev;
> +	rtk_phy->phy.label		= "rtk-usb2phy";
> +	rtk_phy->phy.notify_port_status = rtk_phy_notify_port_status;
> +
> +	rtk_phy->phy_cfg = devm_kzalloc(dev, sizeof(*phy_cfg), GFP_KERNEL);
> +
> +	memcpy(rtk_phy->phy_cfg, phy_cfg, sizeof(*phy_cfg));
> +
> +	node = dev->of_node;

Drop it. Useless assignment.

> +
> +	if (of_device_is_compatible(node, "realtek,rtd1395-usb2phy-2port"))

No, customize variant with driver_data. Don't embed compatibles in the code.


> +		rtk_phy->num_phy = 2;
> +	else
> +		rtk_phy->num_phy = 1;
> +
> +	ret = parse_phy_data(rtk_phy);
> +	if (ret)
> +		goto err;
> +
> +	platform_set_drvdata(pdev, rtk_phy);
> +
> +	generic_phy = devm_phy_create(rtk_phy->dev, NULL, &ops);
> +	if (IS_ERR(generic_phy))
> +		return PTR_ERR(generic_phy);
> +
> +	phy_set_drvdata(generic_phy, rtk_phy);
> +
> +	phy_provider = devm_of_phy_provider_register(rtk_phy->dev,
> +						     of_phy_simple_xlate);
> +	if (IS_ERR(phy_provider))
> +		return PTR_ERR(phy_provider);
> +
> +	ret = usb_add_phy_dev(&rtk_phy->phy);
> +	if (ret)
> +		goto err;
> +
> +	create_debug_files(rtk_phy);
> +
> +err:
> +	dev_dbg(dev, "Probe RTK USB 2.0 PHY (ret=%d)\n", ret);

NAK. I made it pretty clear last time.


This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.

> +
> +	return ret;
> +}
> +
> +static void rtk_usb2phy_remove(struct platform_device *pdev)
> +{
> +	struct rtk_phy *rtk_phy = platform_get_drvdata(pdev);
> +
> +	remove_debug_files(rtk_phy);
> +
> +	usb_remove_phy(&rtk_phy->phy);
> +}

...

> +
> +static const struct of_device_id usbphy_rtk_dt_match[] = {
> +	{ .compatible = "realtek,rtd1295-usb2phy", .data = &rtd1295_phy_cfg },
> +	{ .compatible = "realtek,rtd1312c-usb2phy", .data = &rtd1312c_phy_cfg },
> +	{ .compatible = "realtek,rtd1315e-usb2phy", .data = &rtd1315e_phy_cfg },
> +	{ .compatible = "realtek,rtd1319-usb2phy", .data = &rtd1319_phy_cfg },
> +	{ .compatible = "realtek,rtd1319d-usb2phy", .data = &rtd1319d_phy_cfg },
> +	{ .compatible = "realtek,rtd1395-usb2phy", .data = &rtd1395_phy_cfg },
> +	{ .compatible = "realtek,rtd1395-usb2phy-2port", .data = &rtd1395_phy_cfg },
> +	{ .compatible = "realtek,rtd1619-usb2phy", .data = &rtd1619_phy_cfg },
> +	{ .compatible = "realtek,rtd1619b-usb2phy", .data = &rtd1619b_phy_cfg },
> +	{ .compatible = "realtek,usb2phy", .data = &rtk_phy_cfg },

This is now even more suprising. Drop "realtek,usb2phy"

> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, usbphy_rtk_dt_match);
> +
> +static struct platform_driver rtk_usb2phy_driver = {
> +	.probe		= rtk_usb2phy_probe,
> +	.remove_new	= rtk_usb2phy_remove,
> +	.driver		= {
> +		.name	= "rtk-usb2phy",
> +		.owner	= THIS_MODULE,

??? Didn't you base your driver on some really, really ancient code
(like 5 years old)? If so, please don't.

Run coccicenelle/coccicheck, smatch and sparse, to avoid common mistakes.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY
  2023-06-14  9:28 ` [PATCH v4 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY Stanley Chang
  2023-06-14 16:13   ` kernel test robot
@ 2023-06-17  8:35   ` Krzysztof Kozlowski
  2023-06-20  9:00     ` Stanley Chang[昌育德]
  1 sibling, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-17  8:35 UTC (permalink / raw)
  To: Stanley Chang, Greg Kroah-Hartman
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Alan Stern, Bagas Sanjaya,
	Matthias Kaehlcke, Douglas Anderson, Flavio Suligoi, Ray Chi,
	linux-phy, devicetree, linux-kernel, linux-usb

On 14/06/2023 11:28, Stanley Chang wrote:
> Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
> controller. Added the driver to drive the USB 3.0 PHY transceivers.
> 
> Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
> ---

All my comments apply here as well.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY
  2023-06-14  9:28 ` [PATCH v4 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY Stanley Chang
@ 2023-06-17  8:36   ` Krzysztof Kozlowski
  2023-06-20  9:00     ` Stanley Chang[昌育德]
  0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-17  8:36 UTC (permalink / raw)
  To: Stanley Chang, Greg Kroah-Hartman
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Alan Stern, Flavio Suligoi,
	Bagas Sanjaya, Matthias Kaehlcke, Ray Chi, linux-phy, devicetree,
	linux-kernel, linux-usb

On 14/06/2023 11:28, Stanley Chang wrote:
> Add the documentation explain the property about Realtek USB PHY driver.
> 
> Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
> controller. Added the driver to drive the  USB 3.0 PHY transceivers.
> 
> Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
> ---
> v3 to v4 change:
>     1. Remove the parameter and non hardware properties from dts.
>     2. Using the compatible data included the config and parameter
>        in driver.
> v2 to v3 change:
>     1. Broken down into two patches, one for each of USB 2 & 3.
>     2. Add more description about Realtek RTD SoCs architecture.
>     3. Removed parameter v1 support for simplification.
>     4. Revised the compatible name for fallback compatible.
>     5. Remove some properties that can be set in the driver.
> v1 to v2 change:
>     Add phy-cells for generic phy driver
> ---
>  .../bindings/phy/realtek,usb3phy.yaml         | 105 ++++++++++++++++++
>  1 file changed, 105 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml b/Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml
> new file mode 100644
> index 000000000000..0f849cf942e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Realtek Semiconductor Corporation
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/realtek,usb3phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Realtek DHC SoCs USB 3.0 PHY
> +
> +maintainers:
> +  - Stanley Chang <stanley_chang@realtek.com>
> +
> +description:
> +  Realtek USB 3.0 PHY support the digital home center (DHC) RTD series SoCs.
> +  The USB 3.0 PHY driver is designed to support the XHCI controller. The SoCs
> +  support multiple XHCI controllers. One PHY device node maps to one XHCI
> +  controller.
> +
> +  RTD1295/RTD1619 SoCs USB
> +  The USB architecture includes three XHCI controllers.
> +  Each XHCI maps to one USB 2.0 PHY and map one USB 3.0 PHY on some
> +  controllers.
> +  XHCI controller#0 -- usb2phy -- phy#0
> +                    |- usb3phy -- phy#0
> +  XHCI controller#1 -- usb2phy -- phy#0
> +  XHCI controller#2 -- usb2phy -- phy#0
> +                    |- usb3phy -- phy#0
> +
> +  RTD1319/RTD1619b SoCs USB
> +  The USB architecture includes three XHCI controllers.
> +  Each XHCI maps to one USB 2.0 PHY and map one USB 3.0 PHY on controllers#2.
> +  XHCI controller#0 -- usb2phy -- phy#0
> +  XHCI controller#1 -- usb2phy -- phy#0
> +  XHCI controller#2 -- usb2phy -- phy#0
> +                    |- usb3phy -- phy#0
> +
> +  RTD1319d SoCs USB
> +  The USB architecture includes three XHCI controllers.
> +  Each xhci maps to one USB 2.0 PHY and map one USB 3.0 PHY on controllers#0.
> +  XHCI controller#0 -- usb2phy -- phy#0
> +                    |- usb3phy -- phy#0
> +  XHCI controller#1 -- usb2phy -- phy#0
> +  XHCI controller#2 -- usb2phy -- phy#0
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - realtek,rtd1295-usb3phy
> +          - realtek,rtd1319-usb3phy
> +          - realtek,rtd1319d-usb3phy
> +          - realtek,rtd1619-usb3phy
> +          - realtek,rtd1619b-usb3phy
> +      - const: realtek,usb3phy

Drop last compatible, it is not used now. Does not make sense.

> +
> +  reg:
> +    description: PHY data registers

Drop description, it's obvious.

> +    maxItems: 1
> +
> +  "#phy-cells":
> +    const: 0
> +
> +  nvmem-cells:
> +    maxItems: 1
> +    description: A phandle to the tx lfps swing trim data provided by
> +      a nvmem device, if unspecified, default values shall be used.
> +
> +  nvmem-cell-names:
> +    items:
> +      - const: usb_u3_tx_lfps_swing_trim
> +
> +  realtek,amplitude-control-coarse-tuning:
> +    description:
> +      This adjusts the signal amplitude for normal operation and beacon LFPS.
> +      This value is a parameter for coarse tuning.
> +      For different boards, if the default value is inappropriate, this
> +      property can be assigned to adjust.

default:

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 255
> +
> +  realtek,amplitude-control-fine-tuning:
> +    description:
> +      This adjusts the signal amplitude for normal operation and beacon LFPS.
> +      This value is used for fine-tuning parameters.
> +    $ref: /schemas/types.yaml#/definitions/uint32

default:

> +    minimum: 0
> +    maximum: 65535
> +


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH v4 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY
  2023-06-17  8:22   ` Krzysztof Kozlowski
@ 2023-06-20  8:58     ` Stanley Chang[昌育德]
  0 siblings, 0 replies; 15+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-20  8:58 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Greg Kroah-Hartman
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Alan Stern, Matthias Kaehlcke,
	Douglas Anderson, Ray Chi, Flavio Suligoi, linux-phy, devicetree,
	linux-kernel, linux-usb

Hi Krzysztof,

> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - realtek,rtd1295-usb2phy
> > +          - realtek,rtd1312c-usb2phy
> > +          - realtek,rtd1315e-usb2phy
> > +          - realtek,rtd1319-usb2phy
> > +          - realtek,rtd1319d-usb2phy
> > +          - realtek,rtd1395-usb2phy
> > +          - realtek,rtd1395-usb2phy-2port
> > +          - realtek,rtd1619-usb2phy
> > +          - realtek,rtd1619b-usb2phy
> > +      - const: realtek,usb2phy
> 
> That's not what your driver is saying... This patchset has random set of
> changes.
> 
> I suggest to drop "realtek,usb2phy".

Okay, I will remove it.

> > +
> > +  reg:
> > +    items:
> > +      - description: PHY data registers
> > +      - description: PHY control registers
> > +
> > +  "#phy-cells":
> > +    const: 0
> > +
> > +  nvmem-cells:
> > +    maxItems: 2
> > +    description:
> > +      Phandles to nvmem cell that contains the trimming data.
> > +      If unspecified, default value is used.
> > +
> > +  nvmem-cell-names:
> > +    items:
> > +      - const: usb-dc-cal
> > +      - const: usb-dc-dis
> > +    description:
> > +      The following names, which correspond to each nvmem-cells.
> > +      usb-dc-cal is the driving level for each phy specified via efuse.
> > +      usb-dc-dis is the disconnection level for each phy specified via efuse.
> > +
> > +  realtek,inverse-hstx-sync-clock:
> > +    description:
> > +      For one of the phys of RTD1619b SoC, the synchronous clock of the
> > +      high-speed tx must be inverted. So this property is used to set the
> > +      inverted clock.
> 
> Drop last sentence, it is redundant.

Okay

> > +    type: boolean
> > +
> > +  realtek,driving-level:
> > +    description:
> > +      Each board or port may have a different driving capability. This
> > +      will adjust the driving level value if the value is not the default.
> 
> I don't understand it. What is "driving capability" and if each port can have it
> different, why do you need property for this?

Sorry. I didn't express myself clearly.
"Driving capability" refers to the magnitude of the Dp/Dm output swing.
For a different board or port, the original magnitude maybe not meet the specification. 
In this situation we can adjust the value to meet the specification.

> You mention some default - why it is not expressed as "default: xx"?

The default is mean hardware default value.
I can add the default value.

> What do the values mean?

The description can be modified to:
    description:
      Control the magnitude of High speed Dp/Dm output swing.
      For a different board or port, the original magnitude maybe not meet
      the specification. In this situation we can adjust the value to meet
      the specification.
    $ref: /schemas/types.yaml#/definitions/uint32
    default: 8
    minimum: 0
    maximum: 31

> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    minimum: 0
> > +    maximum: 31
> > +
> > +  realtek,driving-compensate:
> > +    description:
> > +      For RTD1315e SoC, the driving level can be adjusted by reading the
> > +      efuse table. Therefore, this property provides drive compensation
> for
> > +      different boards with different drive capabilities.
> 
> if driving level can be read from nvmem, why do you have realtek,driving-level
> in the first place? Don't you miss here some allOf making this constrained per
> variant?
> 
> "Therefore" means "for that reason" or "as a consequence". How is this
> property a consequence of reading driving level from efuse? Is it then mutually
> exclusive with "realtek,driving-level"? But your schema does not express it.

No, it's not that complicated.
In our case, not all ICs have programming efuse.
The value "realtek,deiving-level" is for non-programmed efuse ICs.

In the programmed efuse IC, we use the value of efuse to instead of "realtek,driving-level". 
If the magnitude of High speed Dp/Dm output swing still not meet the specification,
then we can set the value "driving-compensate" to meet the specification.

The description can be modified to:
  realtek,driving-compensate:
    description:
      For RTD1315e SoC, the driving level can be adjusted by reading the
      efuse table. This property provides drive compensation.
      If the magnitude of High speed Dp/Dm output swing still not meet the
      specification, then we can set this value to meet the specification.
    $ref: /schemas/types.yaml#/definitions/int32
    minimum: -8
    maximum: 8

> > +    $ref: /schemas/types.yaml#/definitions/int32
> > +    minimum: -8
> > +    maximum: 8
> > +
> > +  realtek,disconnection-compensate:
> > +    description:
> > +      This adjusts the disconnection level compensation for the different
> > +      boards with different disconnection level.
> > +    $ref: /schemas/types.yaml#/definitions/int32
> > +    minimum: -8
> > +    maximum: 8
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#phy-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    usb_port0_usb2phy: usb-phy@13214 {
> > +        compatible = "realtek,rtd1319d-usb2phy", "realtek,usb2phy";
> > +        reg = <0x13214 0x4>, <0x28280 0x4>;
> > +        #phy-cells = <0>;
> > +
> > +        realtek,driving-level = <0xe>;
> 
> Why this example is so empty? You have at least 6 more properties which
> should be shown here.

I will add more examples in here.
examples:
  - |
    usb_port0_usb2phy: usb-phy@13214 {
        compatible = "realtek,rtd1319d-usb2phy";
        reg = <0x13214 0x4>, <0x28280 0x4>;
        #phy-cells = <0>;
        nvmem-cells = <&otp_usb_port0_dc_cal>, <&otp_usb_port0_dc_dis>;
        nvmem-cell-names = "usb-dc-cal", "usb-dc-dis";

        realtek,driving-level = <0xe>;
    };

  - |
    port0_usb2phy: usb-phy@13214 {
        compatible = "realtek,rtd1619b-usb2phy";
        reg = <0x13214 0x4>, <0x28280 0x4>;
        #phy-cells = <0>;
        nvmem-cells = <&otp_usb_port0_dc_cal>, <&otp_usb_port0_dc_dis>;
        nvmem-cell-names = "usb-dc-cal", "usb-dc-dis";

        realtek,inverse-hstx-sync-clock;
        realtek,driving-level = <0xa>;
        realtek,driving-compensate = <(-1)>;
        realtek,disconnection-compensate = <(-1)>;
    };

    port1_usb2phy: usb-phy@13c14 {
        compatible = "realtek,rtd1619b-usb2phy";
        reg = <0x13c14 0x4>, <0x31280 0x4>;
        #phy-cells = <0>;
        nvmem-cells = <&otp_usb_port1_dc_cal>, <&otp_usb_port1_dc_dis>;
        nvmem-cell-names = "usb-dc-cal", "usb-dc-dis";

        realtek,driving-level = <8>;
        realtek,driving-compensate = <1>;
        realtek,disconnection-compensate = <0>;
    };

Thanks,
Stanley

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH v4 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
  2023-06-17  8:34   ` Krzysztof Kozlowski
@ 2023-06-20  8:58     ` Stanley Chang[昌育德]
  0 siblings, 0 replies; 15+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-20  8:58 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Greg Kroah-Hartman
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Alan Stern, Flavio Suligoi,
	Douglas Anderson, Bagas Sanjaya, Matthias Kaehlcke, Ray Chi,
	linux-phy, devicetree, linux-kernel, linux-usb

Hi Krzysztof,

> > +     addr = phy_data->addr;
> > +     data = phy_data->data;
> > +     dc_disconnect_mask = phy_cfg->dc_disconnect_mask;
> > +
> > +     if (update)
> > +             data =
> __updated_dc_disconnect_level_page0_0xe4(phy_cfg, phy_parameter, data);
> > +     else
> > +             data = (data & ~(dc_disconnect_mask << offset)) |
> > +                     (DEFAULT_DC_DISCONNECTION_VALUE <<
> offset);
> > +
> > +     if (rtk_phy_write(phy_reg, addr, data))
> > +             dev_err(rtk_phy->dev,
> > +                     "[%s:%d] Error page1 addr=0x%x value=0x%x\n",
> > +                     __func__, __LINE__,
> > +                     addr, data);
> 
> Is addr a kernel address or any memory (not SFR) address? If so, you cannot
> print it.

It is not memory address.

> > +
> > +     if (rtk_phy_write(phy_reg, addr, data))
> > +             dev_err(rtk_phy->dev,
> > +                     "[%s:%d] Error page1 addr=0x%x value=0x%x\n",
> > +                     __func__, __LINE__,
> > +                     addr, data);
> 
> Ditto and in all other places.

It is not memory address.

> > +static u8 __update_dc_driving_page0_0xe4(struct phy_cfg *phy_cfg,
> > +                                      struct phy_parameter
> > +*phy_parameter, u8 data) {
> > +     s32 driving_compensate = phy_parameter->driving_compensate;
> > +     s32 dc_driving_mask = phy_cfg->dc_driving_mask;
> > +     s32 __val;
> > +     u8 val;
> 
> Two variables with the same name. No, it is not readable code.

Okay. I will revise it.

> > +static void rtk_phy_toggle(struct usb_phy *usb2_phy, bool connect,
> > +int port) {
> > +     int index = port;
> > +     struct rtk_phy *rtk_phy = NULL;
> > +
> > +     rtk_phy = dev_get_drvdata(usb2_phy->dev);
> > +
> > +     if (index > rtk_phy->num_phy) {
> > +             pr_err("%s %d ERROR! port=%d > num_phy=%d\n",
> 
> dev_err

I revised it.

> > +                    __func__, __LINE__, index, rtk_phy->num_phy);
> 
> all these func and LINE point to poor code quality and poor debugging
> practices. These are added dugin development, not for production code,
> because error message should be obvious. Your usage of pr_err, func, LINE and
> some unprecise messages suggests this is not ready.
> 
> Fix all your error messages to be meaningful.

I will review all error messages.
Thanks.

> > +static const struct file_operations rtk_usb2_set_parameter_fops = {
> > +     .open                   = rtk_usb2_set_parameter_open,
> > +     .write                  = rtk_usb2_set_parameter_write,
> 
> NAK. You just created user interface via debugfs. You cannot. Reading for some
> debug is okay, but configuring device via undocumented debugfs is a source of
> troubles.
> 
> Drop all writes to debugfs.

I will remove this.

> 
> > +
> > +static int parse_phy_data(struct rtk_phy *rtk_phy) {
> > +     struct device *dev = rtk_phy->dev;
> > +     struct device_node *node;
> 
> By convention:
> s/node/np/

Okay.

> > +     struct phy_cfg *phy_cfg;
> > +     struct phy_parameter *phy_parameter;
> > +     int ret = 0;
> > +     int index;
> > +
> > +     node = dev->of_node;
> 
> Keep it in variable definition.

Okay.

> > +
> > +static int rtk_usb2phy_probe(struct platform_device *pdev) {
> > +     struct rtk_phy *rtk_phy;
> > +     struct device *dev = &pdev->dev;
> > +     struct device_node *node;
> > +     struct phy *generic_phy;
> > +     struct phy_provider *phy_provider;
> > +     const struct phy_cfg *phy_cfg;
> > +     int ret = 0;
> > +
> > +     phy_cfg = of_device_get_match_data(dev);
> > +     if (!phy_cfg) {
> > +             dev_err(dev, "phy config are not assigned!\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     rtk_phy = devm_kzalloc(dev, sizeof(*rtk_phy), GFP_KERNEL);
> > +     if (!rtk_phy)
> > +             return -ENOMEM;
> > +
> > +     rtk_phy->dev                    = &pdev->dev;
> > +     rtk_phy->phy.dev                = rtk_phy->dev;
> > +     rtk_phy->phy.label              = "rtk-usb2phy";
> > +     rtk_phy->phy.notify_port_status = rtk_phy_notify_port_status;
> > +
> > +     rtk_phy->phy_cfg = devm_kzalloc(dev, sizeof(*phy_cfg),
> > + GFP_KERNEL);
> > +
> > +     memcpy(rtk_phy->phy_cfg, phy_cfg, sizeof(*phy_cfg));
> > +
> > +     node = dev->of_node;
> 
> Drop it. Useless assignment.

Okay.

> > +
> > +     if (of_device_is_compatible(node,
> > + "realtek,rtd1395-usb2phy-2port"))
> 
> No, customize variant with driver_data. Don't embed compatibles in the code.

I will use the compatible data to match this case.

> 
> > +             rtk_phy->num_phy = 2;
> > +     else
> > +             rtk_phy->num_phy = 1;
> > +
> > +     ret = parse_phy_data(rtk_phy);
> > +     if (ret)
> > +             goto err;
> > +
> > +     platform_set_drvdata(pdev, rtk_phy);
> > +
> > +     generic_phy = devm_phy_create(rtk_phy->dev, NULL, &ops);
> > +     if (IS_ERR(generic_phy))
> > +             return PTR_ERR(generic_phy);
> > +
> > +     phy_set_drvdata(generic_phy, rtk_phy);
> > +
> > +     phy_provider = devm_of_phy_provider_register(rtk_phy->dev,
> > +
> of_phy_simple_xlate);
> > +     if (IS_ERR(phy_provider))
> > +             return PTR_ERR(phy_provider);
> > +
> > +     ret = usb_add_phy_dev(&rtk_phy->phy);
> > +     if (ret)
> > +             goto err;
> > +
> > +     create_debug_files(rtk_phy);
> > +
> > +err:
> > +     dev_dbg(dev, "Probe RTK USB 2.0 PHY (ret=%d)\n", ret);
> 
> NAK. I made it pretty clear last time.
> 
> 
> This is a friendly reminder during the review process.
> 
> It seems my previous comments were not fully addressed. Maybe my feedback
> got lost between the quotes, maybe you just forgot to apply it.
> Please go back to the previous discussion and either implement all requested
> changes or keep discussing them.
> 
> Thank you.

Sorry. I left out this print, I will delete it.
Thank you.

> > +
> > +     return ret;
> > +}
> > +
> > +static void rtk_usb2phy_remove(struct platform_device *pdev) {
> > +     struct rtk_phy *rtk_phy = platform_get_drvdata(pdev);
> > +
> > +     remove_debug_files(rtk_phy);
> > +
> > +     usb_remove_phy(&rtk_phy->phy);
> > +}
> 
> ...
> 
> > +
> > +static const struct of_device_id usbphy_rtk_dt_match[] = {
> > +     { .compatible = "realtek,rtd1295-usb2phy", .data =
> &rtd1295_phy_cfg },
> > +     { .compatible = "realtek,rtd1312c-usb2phy", .data =
> &rtd1312c_phy_cfg },
> > +     { .compatible = "realtek,rtd1315e-usb2phy", .data =
> &rtd1315e_phy_cfg },
> > +     { .compatible = "realtek,rtd1319-usb2phy", .data =
> &rtd1319_phy_cfg },
> > +     { .compatible = "realtek,rtd1319d-usb2phy", .data =
> &rtd1319d_phy_cfg },
> > +     { .compatible = "realtek,rtd1395-usb2phy", .data =
> &rtd1395_phy_cfg },
> > +     { .compatible = "realtek,rtd1395-usb2phy-2port", .data =
> &rtd1395_phy_cfg },
> > +     { .compatible = "realtek,rtd1619-usb2phy", .data =
> &rtd1619_phy_cfg },
> > +     { .compatible = "realtek,rtd1619b-usb2phy", .data =
> &rtd1619b_phy_cfg },
> > +     { .compatible = "realtek,usb2phy", .data = &rtk_phy_cfg },
> 
> This is now even more suprising. Drop "realtek,usb2phy"

I will remove it.
> > +     {},
> > +};
> > +MODULE_DEVICE_TABLE(of, usbphy_rtk_dt_match);
> > +
> > +static struct platform_driver rtk_usb2phy_driver = {
> > +     .probe          = rtk_usb2phy_probe,
> > +     .remove_new     = rtk_usb2phy_remove,
> > +     .driver         = {
> > +             .name   = "rtk-usb2phy",
> > +             .owner  = THIS_MODULE,
> 
> ??? Didn't you base your driver on some really, really ancient code (like 5 years
> old)? If so, please don't.


Thank you.
I will remove it.

> Run coccicenelle/coccicheck, smatch and sparse, to avoid common mistakes.
> 
I will run these tools.

Thanks,
Stanley

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH v4 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY
  2023-06-17  8:35   ` Krzysztof Kozlowski
@ 2023-06-20  9:00     ` Stanley Chang[昌育德]
  0 siblings, 0 replies; 15+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-20  9:00 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Greg Kroah-Hartman
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Alan Stern, Bagas Sanjaya,
	Matthias Kaehlcke, Douglas Anderson, Flavio Suligoi, Ray Chi,
	linux-phy, devicetree, linux-kernel, linux-usb

Hi Krzysztof,
> 
> 
> On 14/06/2023 11:28, Stanley Chang wrote:
> > Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
> > controller. Added the driver to drive the USB 3.0 PHY transceivers.
> >
> > Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
> > ---
> 
> All my comments apply here as well.
> 
I will refer the comments of the second patch.

Thanks,
Stanley


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH v4 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY
  2023-06-17  8:36   ` Krzysztof Kozlowski
@ 2023-06-20  9:00     ` Stanley Chang[昌育德]
  0 siblings, 0 replies; 15+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-20  9:00 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Greg Kroah-Hartman
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Alan Stern, Flavio Suligoi,
	Bagas Sanjaya, Matthias Kaehlcke, Ray Chi, linux-phy, devicetree,
	linux-kernel, linux-usb

Hi Krzysztof,

> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - realtek,rtd1295-usb3phy
> > +          - realtek,rtd1319-usb3phy
> > +          - realtek,rtd1319d-usb3phy
> > +          - realtek,rtd1619-usb3phy
> > +          - realtek,rtd1619b-usb3phy
> > +      - const: realtek,usb3phy
> 
> Drop last compatible, it is not used now. Does not make sense.

Okay. I will remove it.

> > +
> > +  reg:
> > +    description: PHY data registers
> 
> Drop description, it's obvious.

Okay.

> > +    maxItems: 1
> > +
> > +  "#phy-cells":
> > +    const: 0
> > +
> > +  nvmem-cells:
> > +    maxItems: 1
> > +    description: A phandle to the tx lfps swing trim data provided by
> > +      a nvmem device, if unspecified, default values shall be used.
> > +
> > +  nvmem-cell-names:
> > +    items:
> > +      - const: usb_u3_tx_lfps_swing_trim
> > +
> > +  realtek,amplitude-control-coarse-tuning:
> > +    description:
> > +      This adjusts the signal amplitude for normal operation and beacon
> LFPS.
> > +      This value is a parameter for coarse tuning.
> > +      For different boards, if the default value is inappropriate, this
> > +      property can be assigned to adjust.
> 
> default:

I will add it.

> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    minimum: 0
> > +    maximum: 255
> > +
> > +  realtek,amplitude-control-fine-tuning:
> > +    description:
> > +      This adjusts the signal amplitude for normal operation and beacon
> LFPS.
> > +      This value is used for fine-tuning parameters.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> default:

I will add it.

> > +    minimum: 0
> > +    maximum: 65535
> > +
> 

Thanks,
Stanley

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-06-20  9:02 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-14  9:28 [PATCH v4 1/5] usb: phy: add usb phy notify port status API Stanley Chang
2023-06-14  9:28 ` [PATCH v4 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY Stanley Chang
2023-06-14 15:10   ` kernel test robot
2023-06-17  8:34   ` Krzysztof Kozlowski
2023-06-20  8:58     ` Stanley Chang[昌育德]
2023-06-14  9:28 ` [PATCH v4 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY Stanley Chang
2023-06-14 16:13   ` kernel test robot
2023-06-17  8:35   ` Krzysztof Kozlowski
2023-06-20  9:00     ` Stanley Chang[昌育德]
2023-06-14  9:28 ` [PATCH v4 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY Stanley Chang
2023-06-17  8:22   ` Krzysztof Kozlowski
2023-06-20  8:58     ` Stanley Chang[昌育德]
2023-06-14  9:28 ` [PATCH v4 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY Stanley Chang
2023-06-17  8:36   ` Krzysztof Kozlowski
2023-06-20  9:00     ` Stanley Chang[昌育德]

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).