* [PATCH v3 1/5] usb: phy: add usb phy notify port status API
@ 2023-06-07 6:24 Stanley Chang
2023-06-07 6:24 ` [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY Stanley Chang
` (3 more replies)
0 siblings, 4 replies; 33+ messages in thread
From: Stanley Chang @ 2023-06-07 6:24 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,
Mathias Nyman, Ray Chi, Flavio Suligoi, Michael Grzeschik,
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>
---
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 | 14 ++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 97a0f8faea6e..b4fbbeae1927 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..53bf3540098f 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,16 @@ 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] 33+ messages in thread
* [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
2023-06-07 6:24 [PATCH v3 1/5] usb: phy: add usb phy notify port status API Stanley Chang
@ 2023-06-07 6:24 ` Stanley Chang
2023-06-07 11:26 ` kernel test robot
` (2 more replies)
2023-06-07 6:24 ` [PATCH v3 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY Stanley Chang
` (2 subsequent siblings)
3 siblings, 3 replies; 33+ messages in thread
From: Stanley Chang @ 2023-06-07 6:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Stanley Chang, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Michael Grzeschik,
Mathias Nyman, Bagas Sanjaya, Matthias Kaehlcke, Ray Chi,
Flavio Suligoi, 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>
---
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 | 13 +
drivers/phy/realtek/Makefile | 2 +
drivers/phy/realtek/phy-rtk-usb2.c | 1914 ++++++++++++++++++++++++++++
5 files changed, 1931 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..76e31f6abdee
--- /dev/null
+++ b/drivers/phy/realtek/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Phy drivers for Realtek platforms
+#
+config PHY_RTK_RTD_USB2PHY
+ tristate "Realtek RTD USB2 PHY Transceiver Driver"
+ 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..0538424b9cab
--- /dev/null
+++ b/drivers/phy/realtek/phy-rtk-usb2.c
@@ -0,0 +1,1914 @@
+// 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_PORTS 4
+#define MAX_USB_PHY_PAGE0_DATA_SIZE 16
+#define MAX_USB_PHY_PAGE1_DATA_SIZE 8
+#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_0xE7 0xe7
+#define PAGE1_0xE0 0xe0
+#define PAGE1_0xE2 0xe2
+
+/* mapping 0xE0 to 0 ... 0xE7 to 7, 0xF0 to 8 ,,, 0xF7 to 15 */
+#define PAGE_ADDR_MAP_ARRAY_INDEX(addr) \
+ (((addr - PAGE_START)&0x7) + \
+ (((addr - PAGE_START)&0x10)>>1))
+#define ARRAY_INDEX_MAP_PAGE_ADDR(index) \
+ (((index + PAGE_START)&0x7) + \
+ (((index&0x8)<<1) + PAGE_START))
+
+struct reg_addr {
+ void __iomem *reg_wrap_vstatus;
+ void __iomem *reg_gusb2phyacc0;
+ int vstatus_index;
+};
+
+struct phy_parameter {
+ u8 addr;
+ u8 data;
+};
+
+struct phy_data {
+ int page0_size;
+ struct phy_parameter *page0;
+ int page1_size;
+ struct phy_parameter *page1;
+ int page2_size;
+ struct phy_parameter *page2;
+
+ bool check_efuse;
+ int check_efuse_version;
+#define CHECK_EFUSE_V1 1
+#define CHECK_EFUSE_V2 2
+ int8_t efuse_usb_dc_cal;
+ int efuse_usb_dc_cal_rate;
+ int usb_dc_cal_mask;
+ int8_t efuse_usb_dc_dis;
+ int efuse_usb_dc_dis_rate;
+ int usb_dc_dis_mask;
+ bool usb_dc_dis_at_page0;
+ bool do_toggle;
+ bool do_toggle_driving;
+ int disconnect_driving_updated;
+ bool use_default_parameter;
+ bool is_double_sensitivity_mode;
+ bool ldo_force_enable;
+ bool ldo_enable;
+ s32 ldo_driving_compensate;
+ s32 driving_compensate;
+};
+
+struct rtk_usb_phy {
+ struct usb_phy phy;
+ struct device *dev;
+ struct regmap *usb_ctrl_regs;
+
+ int phyN;
+ void *reg_addr;
+ void *phy_data;
+
+ 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 char rtk_usb_phy_read(struct reg_addr *regAddr, char addr)
+{
+ void __iomem *reg_gusb2phyacc0 = regAddr->reg_gusb2phyacc0;
+ unsigned int regVal;
+ 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 */
+ regVal = PHY_NEW_REG_REQ | (GET_LOW_NIBBLE(addr) << PHY_VCTRL_SHIFT);
+ writel(regVal, 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 */
+ regVal = PHY_NEW_REG_REQ | (GET_HIGH_NIBBLE(addr) << PHY_VCTRL_SHIFT);
+ writel(regVal, reg_gusb2phyacc0);
+ ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+ if (ret)
+ return (char)ret;
+
+ regVal = readl(reg_gusb2phyacc0);
+
+ return (char) (regVal & PHY_REG_DATA_MASK);
+}
+
+static int rtk_usb_phy_write(struct reg_addr *regAddr, char addr, char data)
+{
+ unsigned int regVal;
+ void __iomem *reg_wrap_vstatus = regAddr->reg_wrap_vstatus;
+ void __iomem *reg_gusb2phyacc0 = regAddr->reg_gusb2phyacc0;
+ int shift_bits = regAddr->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 */
+ regVal = PHY_NEW_REG_REQ | (GET_LOW_NIBBLE(addr) << PHY_VCTRL_SHIFT);
+
+ writel(regVal, 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 */
+ regVal = PHY_NEW_REG_REQ | (GET_HIGH_NIBBLE(addr) << PHY_VCTRL_SHIFT);
+
+ writel(regVal, reg_gusb2phyacc0);
+ ret = utmi_wait_register(reg_gusb2phyacc0, PHY_VSTS_BUSY, 0);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int rtk_usb_phy_set_page(struct reg_addr *regAddr, int page)
+{
+ switch (page) {
+ case 0:
+ return rtk_usb_phy_write(regAddr, SET_PAGE_OFFSET, SET_PAGE_0);
+ case 1:
+ return rtk_usb_phy_write(regAddr, SET_PAGE_OFFSET, SET_PAGE_1);
+ case 2:
+ return rtk_usb_phy_write(regAddr, SET_PAGE_OFFSET, SET_PAGE_2);
+ default:
+ pr_err("%s error page=%d\n", __func__, page);
+ }
+
+ return -1;
+}
+
+#define USB_CTRL 0x0 /* usb ctrl at 0x98007FB0 */
+#define ISO_USB_U2PHY_REG_LDO_PW (BIT(20) | BIT(21) | BIT(22) | BIT(23))
+
+static int control_phy_power(struct rtk_usb_phy *rtk_phy,
+ struct phy_data *phy_data, struct reg_addr *regAddr)
+{
+ int use_ldo = 0;
+ unsigned int val;
+
+ if (!rtk_phy->usb_ctrl_regs) {
+ dev_info(rtk_phy->dev, "%s No usb_ctrl_regs can't set USB_CTRL\n",
+ __func__);
+ return use_ldo;
+ }
+
+ if (regmap_read(rtk_phy->usb_ctrl_regs, USB_CTRL, &val)) {
+ dev_err(rtk_phy->dev, "%s Get USB_CTRL fail\n", __func__);
+ return use_ldo;
+ }
+
+ if ((val & ISO_USB_U2PHY_REG_LDO_PW) == ISO_USB_U2PHY_REG_LDO_PW) {
+ dev_info(rtk_phy->dev, "%s phy use ldo power! (USB_CTRL val=0x%x)\n",
+ __func__, val);
+ use_ldo = 1;
+ goto out;
+ }
+
+ if (phy_data->ldo_force_enable) {
+ regmap_update_bits(rtk_phy->usb_ctrl_regs, USB_CTRL,
+ (unsigned int)ISO_USB_U2PHY_REG_LDO_PW,
+ (unsigned int)ISO_USB_U2PHY_REG_LDO_PW);
+ use_ldo = 1;
+
+ dev_info(rtk_phy->dev, "%s phy %s then turn on ldo! USB_CTRL val=0x%x\n",
+ __func__,
+ phy_data->ldo_force_enable ?
+ "ldo_force_enable":"no power",
+ val);
+ }
+
+out:
+ return use_ldo;
+}
+
+static u8 __updated_page0_0xe4_parameter(struct phy_data *phy_data, u8 data)
+{
+ u8 val;
+ s32 __val;
+ s32 driving_compensate = 0;
+ s32 usb_dc_cal_mask = phy_data->usb_dc_cal_mask;
+
+ if (phy_data->check_efuse_version == CHECK_EFUSE_V1) {
+ if (phy_data->ldo_enable)
+ driving_compensate = phy_data->ldo_driving_compensate;
+
+ __val = (s32)(data & usb_dc_cal_mask) + driving_compensate
+ + phy_data->efuse_usb_dc_cal;
+ } else { /* for CHECK_EFUSE_V2 or no efuse */
+ driving_compensate = phy_data->driving_compensate;
+
+ if (phy_data->efuse_usb_dc_cal)
+ __val = (s32)((phy_data->efuse_usb_dc_cal & usb_dc_cal_mask)
+ + driving_compensate);
+ else
+ __val = (s32)(data & usb_dc_cal_mask);
+ }
+
+ if (__val > usb_dc_cal_mask)
+ __val = usb_dc_cal_mask;
+ else if (__val < 0)
+ __val = 0;
+
+ val = (data & (~usb_dc_cal_mask)) | (__val & usb_dc_cal_mask);
+
+ return val;
+}
+
+static u8 __updated_dc_disconnect_level_page0_0xe4(struct phy_data *phy_data,
+ u8 data)
+{
+ u8 val;
+ s32 __val;
+ s32 usb_dc_dis_mask = phy_data->usb_dc_dis_mask;
+ int offset = 4;
+
+ __val = (s32)((data >> offset) & usb_dc_dis_mask)
+ + phy_data->efuse_usb_dc_dis;
+
+ if (__val > usb_dc_dis_mask)
+ __val = usb_dc_dis_mask;
+ else if (__val < 0)
+ __val = 0;
+
+ val = (data & (~(usb_dc_dis_mask << offset))) |
+ (__val & usb_dc_dis_mask) << offset;
+
+ return val;
+}
+
+/* updated disconnect level at page0 0xe4 */
+static void update_dc_disconnect_level_at_page0(struct rtk_usb_phy *rtk_phy,
+ struct reg_addr *regAddr,
+ struct phy_data *phy_data, bool isUpdate)
+{
+ struct phy_parameter *phy_parameter_page;
+ int i;
+
+ /* Set page 0 */
+ phy_parameter_page = phy_data->page0;
+ rtk_usb_phy_set_page(regAddr, 0);
+
+ i = PAGE_ADDR_MAP_ARRAY_INDEX(PAGE0_0xE4);
+ if (i < phy_data->page0_size) {
+ struct phy_parameter *phy_parameter = phy_parameter_page + i;
+ u8 addr = phy_parameter->addr;
+ u8 data = phy_parameter->data;
+ u8 __data;
+ int offset = 4;
+ s32 usb_dc_dis_mask = phy_data->usb_dc_dis_mask;
+
+ __data = rtk_usb_phy_read(regAddr, addr);
+
+ /* keep default dc dis and real dc cal */
+ data = (data & ((usb_dc_dis_mask << offset))) |
+ (__data & (~(usb_dc_dis_mask << offset)));
+
+ if (isUpdate)
+ data = __updated_dc_disconnect_level_page0_0xe4(phy_data, data);
+
+ if (rtk_usb_phy_write(regAddr, addr, data)) {
+ dev_err(rtk_phy->dev,
+ "[%s:%d] Error page1 addr=0x%x value=0x%x\n",
+ __func__, __LINE__,
+ addr, data);
+ return;
+ }
+
+ dev_info(rtk_phy->dev,
+ "%s to set Page0 0xE4=%x for dc disconnect level (%s)\n",
+ __func__,
+ rtk_usb_phy_read(regAddr, addr),
+ isUpdate?"Update":"restore");
+ } else {
+ dev_err(rtk_phy->dev,
+ "ERROR: %s %d index=%d addr Not PAGE0_0xE4\n",
+ __func__, __LINE__, i);
+ }
+}
+
+static u8 __updated_dc_disconnect_level_page1_0xe2(struct phy_data *phy_data,
+ u8 data)
+{
+ u8 val;
+ s32 __val;
+ s32 usb_dc_dis_mask = phy_data->usb_dc_dis_mask;
+
+ if (phy_data->check_efuse_version == CHECK_EFUSE_V1) {
+ __val = (s32)(data & usb_dc_dis_mask)
+ + phy_data->efuse_usb_dc_dis;
+ } else { /* for CHECK_EFUSE_V2 or no efuse */
+ if (phy_data->efuse_usb_dc_dis)
+ __val = (s32)(phy_data->efuse_usb_dc_dis & usb_dc_dis_mask);
+ else
+ __val = (s32)(data & usb_dc_dis_mask);
+ }
+
+ if (__val > usb_dc_dis_mask)
+ __val = usb_dc_dis_mask;
+ else if (__val < 0)
+ __val = 0;
+
+ val = (data & (~usb_dc_dis_mask)) | (__val & usb_dc_dis_mask);
+
+ return val;
+}
+
+/* updated disconnect level at page1 0xe2 */
+static void update_dc_disconnect_level_at_page1(struct rtk_usb_phy *rtk_phy,
+ struct reg_addr *regAddr,
+ struct phy_data *phy_data, bool isUpdate)
+{
+ struct phy_parameter *phy_parameter_page;
+ int i;
+
+ /* Set page 1 */
+ phy_parameter_page = phy_data->page1;
+ rtk_usb_phy_set_page(regAddr, 1);
+
+ i = PAGE_ADDR_MAP_ARRAY_INDEX(PAGE1_0xE2);
+ if (i < phy_data->page1_size) {
+ struct phy_parameter *phy_parameter = phy_parameter_page + i;
+ u8 addr = phy_parameter->addr;
+ u8 data = phy_parameter->data;
+ u8 __data;
+ s32 usb_dc_dis_mask = phy_data->usb_dc_dis_mask;
+
+ __data = rtk_usb_phy_read(regAddr, addr);
+
+ data = (data & usb_dc_dis_mask) | (__data & ~(usb_dc_dis_mask));
+
+ if (isUpdate)
+ data = __updated_dc_disconnect_level_page1_0xe2(phy_data, data);
+
+ if (rtk_usb_phy_write(regAddr, addr, data)) {
+ dev_err(rtk_phy->dev,
+ "[%s:%d] Error page1 addr=0x%x value=0x%x\n",
+ __func__, __LINE__,
+ addr, data);
+ return;
+ }
+
+ dev_info(rtk_phy->dev,
+ "%s to set Page1 0xE2=%x for dc disconnect level (%s)\n",
+ __func__,
+ rtk_usb_phy_read(regAddr, addr),
+ isUpdate?"Update":"restore");
+ } else {
+ dev_err(rtk_phy->dev,
+ "ERROR: %s %d index=%d addr Not PAGE1_0xE2\n",
+ __func__, __LINE__, i);
+ }
+}
+
+static void update_dc_disconnect_level(struct rtk_usb_phy *rtk_phy,
+ struct reg_addr *regAddr,
+ struct phy_data *phy_data, bool isUpdate)
+{
+ if (phy_data->usb_dc_dis_at_page0)
+ update_dc_disconnect_level_at_page0(
+ rtk_phy, regAddr, phy_data, isUpdate);
+ else
+ update_dc_disconnect_level_at_page1(
+ rtk_phy, regAddr, phy_data, isUpdate);
+}
+
+static void do_rtk_usb_phy_toggle(struct rtk_usb_phy *rtk_phy,
+ int index, bool isConnect)
+{
+ struct reg_addr *regAddr;
+ struct phy_data *phy_data;
+ struct phy_parameter *phy_parameter_page;
+ int i;
+
+ regAddr = &((struct reg_addr *)rtk_phy->reg_addr)[index];
+ phy_data = &((struct phy_data *)rtk_phy->phy_data)[index];
+
+ if (!phy_data->do_toggle)
+ goto out;
+
+ if (phy_data->is_double_sensitivity_mode)
+ goto do_toggle_driving;
+
+ /* Set page 0 */
+ phy_parameter_page = phy_data->page0;
+ rtk_usb_phy_set_page(regAddr, 0);
+
+ i = PAGE_ADDR_MAP_ARRAY_INDEX(PAGE0_0xE7);
+ if (i < phy_data->page0_size) {
+ struct phy_parameter *phy_parameter = phy_parameter_page + i;
+ u8 addr = phy_parameter->addr;
+ u8 data = phy_parameter->data;
+
+ if (isConnect) {
+ rtk_usb_phy_write(regAddr, addr, data &
+ (~(BIT(4) | BIT(5) | BIT(6))));
+ } else {
+ rtk_usb_phy_write(regAddr, addr, data |
+ (BIT(4) | BIT(5) | BIT(6)));
+ }
+ dev_info(rtk_phy->dev,
+ "%s %sconnect to set Page0 0xE7=%x\n",
+ __func__,
+ isConnect?"":"dis",
+ rtk_usb_phy_read(regAddr, addr));
+ } else {
+ dev_err(rtk_phy->dev,
+ "ERROR: %s %d index=%d addr Not PAGE0_0xE7\n",
+ __func__, __LINE__, i);
+ }
+
+do_toggle_driving:
+
+ if (!phy_data->do_toggle_driving)
+ goto do_toggle;
+
+ /* Page 0 addr 0xE4 driving capability */
+
+ /* Set page 0 */
+ phy_parameter_page = phy_data->page0;
+ rtk_usb_phy_set_page(regAddr, 0);
+
+ i = PAGE_ADDR_MAP_ARRAY_INDEX(PAGE0_0xE4);
+ if (i < phy_data->page0_size) {
+ struct phy_parameter *phy_parameter = phy_parameter_page + i;
+ u8 addr = phy_parameter->addr;
+ u8 data = phy_parameter->data;
+
+ data = __updated_page0_0xe4_parameter(phy_data, data);
+
+ if (isConnect) {
+ rtk_usb_phy_write(regAddr, addr, data);
+ } else {
+ u8 val;
+ s32 __val;
+ s32 driving_updated =
+ phy_data->disconnect_driving_updated;
+ s32 usb_dc_cal_mask = phy_data->usb_dc_cal_mask;
+
+ __val = (s32)(data & usb_dc_cal_mask) + driving_updated;
+
+ if (__val > usb_dc_cal_mask)
+ __val = usb_dc_cal_mask;
+ else if (__val < 0)
+ __val = 0;
+
+ val = (data & (~usb_dc_cal_mask)) | (__val & usb_dc_cal_mask);
+
+ rtk_usb_phy_write(regAddr, addr, val);
+ }
+ dev_info(rtk_phy->dev,
+ "%s %sconnect to set Page0 0xE4=%x for driving\n",
+ __func__,
+ isConnect?"":"dis",
+ rtk_usb_phy_read(regAddr, addr));
+ } else {
+ dev_err(rtk_phy->dev,
+ "ERROR: %s %d index=%d addr Not PAGE0_0xE4\n",
+ __func__, __LINE__, i);
+ }
+
+do_toggle:
+ /* restore dc disconnect level before toggle */
+ update_dc_disconnect_level(rtk_phy, regAddr, phy_data, false);
+
+ /* Set page 1 */
+ phy_parameter_page = phy_data->page1;
+ rtk_usb_phy_set_page(regAddr, 1);
+
+ i = PAGE_ADDR_MAP_ARRAY_INDEX(PAGE1_0xE0);
+ if (i < phy_data->page1_size) {
+ struct phy_parameter *phy_parameter = phy_parameter_page + i;
+ u8 addr = phy_parameter->addr;
+ u8 data = phy_parameter->data;
+
+ dev_info(rtk_phy->dev,
+ "%s ########## to toggle PAGE1_0xE0 BIT(2)\n",
+ __func__);
+ rtk_usb_phy_write(regAddr, addr, data & (~BIT(2)));
+ mdelay(1);
+ rtk_usb_phy_write(regAddr, addr, data | (BIT(2)));
+ } else {
+ dev_err(rtk_phy->dev,
+ "ERROR: %s %d index=%d addr Not PAGE1_0xE0\n",
+ __func__, __LINE__, i);
+ }
+
+ /* update dc disconnect level after toggle */
+ update_dc_disconnect_level(rtk_phy, regAddr, phy_data, true);
+
+out:
+ return;
+}
+
+/* Get default phy parameter for update */
+static int __get_default_phy_parameter_for_updated(
+ struct rtk_usb_phy *rtk_phy, int index)
+{
+ int i;
+ struct reg_addr *regAddr;
+ struct phy_data *phy_data;
+ struct phy_parameter *phy_parameter_page;
+
+ regAddr = &((struct reg_addr *)rtk_phy->reg_addr)[index];
+ phy_data = &((struct phy_data *)rtk_phy->phy_data)[index];
+
+ phy_parameter_page = phy_data->page0;
+ rtk_usb_phy_set_page(regAddr, 0);
+
+ /* Get PAGE0_0xE4 default value */
+ i = PAGE_ADDR_MAP_ARRAY_INDEX(PAGE0_0xE4);
+ if (i < phy_data->page0_size) {
+ struct phy_parameter *phy_parameter = phy_parameter_page + i;
+ u8 addr = phy_parameter->addr;
+ u8 data = phy_parameter->data;
+
+ if (!addr) {
+ addr = ARRAY_INDEX_MAP_PAGE_ADDR(i);
+ data = rtk_usb_phy_read(regAddr, addr);
+
+ phy_parameter->addr = addr;
+ phy_parameter->data = data;
+ dev_dbg(rtk_phy->dev,
+ "Get default addr %x value %x\n",
+ phy_parameter->addr,
+ phy_parameter->data);
+ }
+ }
+
+ /* Get PAGE0_0xE7 default value */
+ i = PAGE_ADDR_MAP_ARRAY_INDEX(PAGE0_0xE7);
+ if (i < phy_data->page0_size) {
+ struct phy_parameter *phy_parameter = phy_parameter_page + i;
+ u8 addr = phy_parameter->addr;
+ u8 data = phy_parameter->data;
+
+ if (!addr) {
+ addr = ARRAY_INDEX_MAP_PAGE_ADDR(i);
+ data = rtk_usb_phy_read(regAddr, addr);
+
+ phy_parameter->addr = addr;
+ phy_parameter->data = data;
+ dev_dbg(rtk_phy->dev,
+ "Get default addr %x value %x\n",
+ phy_parameter->addr,
+ phy_parameter->data);
+ }
+ }
+
+ phy_parameter_page = phy_data->page1;
+ rtk_usb_phy_set_page(regAddr, 1);
+
+ /* Get PAGE1_0xE0 default value */
+ i = PAGE_ADDR_MAP_ARRAY_INDEX(PAGE1_0xE0);
+ if (i < phy_data->page1_size) {
+ struct phy_parameter *phy_parameter = phy_parameter_page + i;
+ u8 addr = phy_parameter->addr;
+ u8 data = phy_parameter->data;
+
+ if (!addr) {
+ addr = ARRAY_INDEX_MAP_PAGE_ADDR(i);
+ data = rtk_usb_phy_read(regAddr, addr);
+
+ phy_parameter->addr = addr;
+ phy_parameter->data = data;
+ dev_dbg(rtk_phy->dev,
+ "Get default page1 addr %x value %x\n",
+ phy_parameter->addr,
+ phy_parameter->data);
+ }
+ }
+
+ /* Get PAGE1_0xE2 default value */
+ i = PAGE_ADDR_MAP_ARRAY_INDEX(PAGE1_0xE2);
+ if (i < phy_data->page1_size) {
+ struct phy_parameter *phy_parameter = phy_parameter_page + i;
+ u8 addr = phy_parameter->addr;
+ u8 data = phy_parameter->data;
+
+ if (!addr) {
+ addr = ARRAY_INDEX_MAP_PAGE_ADDR(i);
+ data = rtk_usb_phy_read(regAddr, addr);
+
+ phy_parameter->addr = addr;
+ phy_parameter->data = data;
+ dev_dbg(rtk_phy->dev,
+ "Get default page1 addr %x value %x\n",
+ phy_parameter->addr,
+ phy_parameter->data);
+ }
+ }
+
+ return 0;
+}
+
+static int do_rtk_usb_phy_init(struct rtk_usb_phy *rtk_phy, int index)
+{
+ struct reg_addr *regAddr;
+ struct phy_data *phy_data;
+ struct phy_parameter *phy_parameter_page;
+ int i;
+
+ dev_dbg(rtk_phy->dev, "%s: init phy#%d\n", __func__, index);
+
+ regAddr = &((struct reg_addr *)rtk_phy->reg_addr)[index];
+ phy_data = &((struct phy_data *)rtk_phy->phy_data)[index];
+
+ if (control_phy_power(rtk_phy, phy_data, regAddr)) {
+ phy_data->ldo_enable = true;
+ dev_info(rtk_phy->dev, "%s USB phy use ldo power compensate phy parameter (%d)\n",
+ __func__, phy_data->ldo_driving_compensate);
+ }
+
+ __get_default_phy_parameter_for_updated(rtk_phy, index);
+
+ if (phy_data->use_default_parameter) {
+ dev_info(rtk_phy->dev, "%s phy#%d use default parameter\n",
+ __func__, index);
+ goto do_toggle;
+ }
+
+ /* Set page 0 */
+ phy_parameter_page = phy_data->page0;
+ rtk_usb_phy_set_page(regAddr, 0);
+
+ for (i = 0; i < phy_data->page0_size; i++) {
+ struct phy_parameter *phy_parameter = phy_parameter_page + i;
+ u8 addr = phy_parameter->addr;
+ u8 data = phy_parameter->data;
+
+ if (!addr)
+ continue;
+
+ if (addr == PAGE0_0xE4)
+ data = __updated_page0_0xe4_parameter(phy_data, data);
+
+ if (rtk_usb_phy_write(regAddr, addr, data)) {
+ dev_err(rtk_phy->dev,
+ "[%s:%d] Error page0 addr=0x%x value=0x%x\n",
+ __func__, __LINE__, addr, data);
+ return -EINVAL;
+ }
+ dev_dbg(rtk_phy->dev, "[%s:%d] Good page0 addr=0x%x value=0x%x\n",
+ __func__, __LINE__, addr,
+ rtk_usb_phy_read(regAddr, addr));
+ }
+
+ /* Set page 1 */
+ phy_parameter_page = phy_data->page1;
+ rtk_usb_phy_set_page(regAddr, 1);
+
+ for (i = 0; i < phy_data->page1_size; i++) {
+ struct phy_parameter *phy_parameter = phy_parameter_page + i;
+ u8 addr = phy_parameter->addr;
+ u8 data = phy_parameter->data;
+
+ if (!addr)
+ continue;
+
+ if (rtk_usb_phy_write(regAddr, addr, data)) {
+ dev_err(rtk_phy->dev,
+ "[%s:%d] Error page1 addr=0x%x value=0x%x\n",
+ __func__, __LINE__,
+ addr, data);
+ return -EINVAL;
+ }
+ dev_dbg(rtk_phy->dev, "[%s:%d] Good page1 addr=0x%x value=0x%x\n",
+ __func__, __LINE__, addr,
+ rtk_usb_phy_read(regAddr, addr));
+ }
+
+ if (phy_data->page2_size == 0)
+ goto do_toggle;
+
+ /* Set page 2 */
+ phy_parameter_page = phy_data->page2;
+ rtk_usb_phy_set_page(regAddr, 2);
+
+ for (i = 0; i < phy_data->page2_size; i++) {
+ struct phy_parameter *phy_parameter = phy_parameter_page + i;
+ u8 addr = phy_parameter->addr;
+ u8 data = phy_parameter->data;
+
+ if (!addr)
+ continue;
+
+ if (rtk_usb_phy_write(regAddr, 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_usb_phy_read(regAddr, addr));
+ }
+
+do_toggle:
+ do_rtk_usb_phy_toggle(rtk_phy, index, false);
+
+ return 0;
+}
+
+static int rtk_usb_phy_init(struct phy *phy)
+{
+ struct rtk_usb_phy *rtk_phy = phy_get_drvdata(phy);
+ unsigned long phy_init_time = jiffies;
+ int i, ret = 0;
+
+ if (!rtk_phy)
+ return -EINVAL;
+
+ dev_dbg(rtk_phy->dev, "Init RTK USB 2.0 PHY\n");
+ for (i = 0; i < rtk_phy->phyN; i++)
+ ret = do_rtk_usb_phy_init(rtk_phy, i);
+
+ dev_info(rtk_phy->dev, "Initialized RTK USB 2.0 PHY (take %dms)\n",
+ jiffies_to_msecs(jiffies - phy_init_time));
+ return ret;
+}
+
+static int rtk_usb_phy_exit(struct phy *phy)
+{
+ struct rtk_usb_phy *rtk_phy = phy_get_drvdata(phy);
+
+ if (!rtk_phy)
+ return -EINVAL;
+
+ dev_info(rtk_phy->dev, "Exit RTK USB 2.0 PHY\n");
+
+ return 0;
+}
+
+static const struct phy_ops ops = {
+ .init = rtk_usb_phy_init,
+ .exit = rtk_usb_phy_exit,
+ .owner = THIS_MODULE,
+};
+
+static void rtk_usb_phy_toggle(struct usb_phy *usb2_phy, bool isConnect, int port)
+{
+ int index = port;
+ struct rtk_usb_phy *rtk_phy = NULL;
+
+ if (usb2_phy != NULL && usb2_phy->dev != NULL)
+ rtk_phy = dev_get_drvdata(usb2_phy->dev);
+
+ if (rtk_phy == NULL) {
+ pr_err("%s %d ERROR! NO this device\n", __func__, __LINE__);
+ return;
+ }
+ if (index > rtk_phy->phyN) {
+ pr_err("%s %d ERROR! port=%d > phyN=%d\n",
+ __func__, __LINE__, index, rtk_phy->phyN);
+ return;
+ }
+
+ do_rtk_usb_phy_toggle(rtk_phy, index, isConnect);
+}
+
+static int rtk_usb_phy_notify_port_status(struct usb_phy *x, int port,
+ u16 portstatus, u16 portchange)
+{
+ bool isConnect = 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)
+ isConnect = true;
+
+ if (portchange & USB_PORT_STAT_C_CONNECTION)
+ rtk_usb_phy_toggle(x, isConnect, 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);
+ if (!phy_debug_root)
+ pr_err("%s Error phy_debug_root is NULL\n", __func__);
+ else
+ pr_debug("%s Create phy_debug_root folder\n", __func__);
+ }
+
+ return phy_debug_root;
+}
+
+static int rtk_usb2_parameter_show(struct seq_file *s, void *unused)
+{
+ struct rtk_usb_phy *rtk_phy = s->private;
+ int i, index;
+
+ for (index = 0; index < rtk_phy->phyN; index++) {
+ struct reg_addr *regAddr =
+ &((struct reg_addr *)rtk_phy->reg_addr)[index];
+ struct phy_data *phy_data =
+ &((struct phy_data *)rtk_phy->phy_data)[index];
+ struct phy_parameter *phy_parameter_page;
+
+ seq_printf(s, "PHY %d:\n", index);
+
+ seq_puts(s, "Page 0:\n");
+ /* Set page 0 */
+ phy_parameter_page = phy_data->page0;
+ rtk_usb_phy_set_page(regAddr, 0);
+
+ for (i = 0; i < phy_data->page0_size; i++) {
+ struct phy_parameter *phy_parameter = phy_parameter_page + i;
+ u8 addr = ARRAY_INDEX_MAP_PAGE_ADDR(i);
+ u8 data = phy_parameter->data;
+ u8 value = rtk_usb_phy_read(regAddr, addr);
+
+ if (phy_parameter->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_parameter_page = phy_data->page1;
+ rtk_usb_phy_set_page(regAddr, 1);
+
+ for (i = 0; i < phy_data->page1_size; i++) {
+ struct phy_parameter *phy_parameter = phy_parameter_page + i;
+ u8 addr = ARRAY_INDEX_MAP_PAGE_ADDR(i);
+ u8 data = phy_parameter->data;
+ u8 value = rtk_usb_phy_read(regAddr, addr);
+
+ if (phy_parameter->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_data->page2_size == 0)
+ goto out;
+
+ seq_puts(s, "Page 2:\n");
+ /* Set page 2 */
+ phy_parameter_page = phy_data->page2;
+ rtk_usb_phy_set_page(regAddr, 2);
+
+ for (i = 0; i < phy_data->page2_size; i++) {
+ struct phy_parameter *phy_parameter = phy_parameter_page + i;
+ u8 addr = ARRAY_INDEX_MAP_PAGE_ADDR(i);
+ u8 data = phy_parameter->data;
+ u8 value = rtk_usb_phy_read(regAddr, addr);
+
+ if (phy_parameter->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, "Property:\n");
+ seq_printf(s, "check_efuse: %s\n",
+ phy_data->check_efuse?"Enable":"Disable");
+ seq_printf(s, "check_efuse_version: %d\n",
+ phy_data->check_efuse_version);
+ seq_printf(s, "efuse_usb_dc_cal: %d\n",
+ (int)phy_data->efuse_usb_dc_cal);
+ seq_printf(s, "efuse_usb_dc_cal_rate: %d\n",
+ phy_data->efuse_usb_dc_cal_rate);
+ seq_printf(s, "usb_dc_cal_mask: 0x%x\n",
+ phy_data->usb_dc_cal_mask);
+ seq_printf(s, "efuse_usb_dc_dis: %d\n",
+ (int)phy_data->efuse_usb_dc_dis);
+ seq_printf(s, "efuse_usb_dc_dis_rate: %d\n",
+ phy_data->efuse_usb_dc_dis_rate);
+ seq_printf(s, "usb_dc_dis_mask: 0x%x\n",
+ phy_data->usb_dc_dis_mask);
+ seq_printf(s, "usb_dc_dis_at_page0: %s\n",
+ phy_data->usb_dc_dis_at_page0?"true":"false");
+ seq_printf(s, "do_toggle: %s\n",
+ phy_data->do_toggle?"Enable":"Disable");
+ seq_printf(s, "do_toggle_driving: %s\n",
+ phy_data->do_toggle_driving?"Enable":"Disable");
+ seq_printf(s, "disconnect_driving_updated: 0x%x\n",
+ phy_data->disconnect_driving_updated);
+ seq_printf(s, "use_default_parameter: %s\n",
+ phy_data->use_default_parameter?"Enable":"Disable");
+ seq_printf(s, "is_double_sensitivity_mode: %s\n",
+ phy_data->is_double_sensitivity_mode?"Enable":"Disable");
+ seq_printf(s, "ldo_force_enable: %s\n",
+ phy_data->ldo_force_enable?"Enable":"Disable");
+ seq_printf(s, "ldo_enable: %s\n",
+ phy_data->ldo_enable?"Enable":"Disable");
+ seq_printf(s, "ldo_driving_compensate: %d\n",
+ phy_data->ldo_driving_compensate);
+ seq_printf(s, "driving_compensate: %d\n",
+ phy_data->driving_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_usb_phy *rtk_phy,
+ struct phy_parameter *phy_parameter_page,
+ const char *phy_page, const char *phy_addr)
+{
+ struct phy_parameter *phy_parameter;
+ uint32_t 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_MAP_ARRAY_INDEX(addr);
+ phy_parameter = (phy_parameter_page + i);
+
+ if (phy_parameter->addr)
+ seq_printf(s, "Now Parameter %s addr 0x%02x = 0x%02x\n",
+ phy_page, phy_parameter->addr, phy_parameter->data);
+ else
+ seq_printf(s, "Now Parameter %s addr 0x%02x is default\n",
+ phy_page, addr);
+
+ dev_dbg(rtk_phy->dev, "%s addr=0x%02x data=0x%02x\n",
+ __func__, phy_parameter->addr, phy_parameter->data);
+
+ return 0;
+}
+
+static int __set_parameter_at_page(
+ struct rtk_usb_phy *rtk_phy,
+ struct reg_addr *regAddr, struct phy_data *phy_data,
+ struct phy_parameter *phy_parameter_page,
+ const char *phy_page, const char *phy_addr, const char *phy_value)
+{
+ struct phy_parameter *phy_parameter;
+ uint32_t 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_MAP_ARRAY_INDEX(addr);
+ phy_parameter = (phy_parameter_page + i);
+
+ if (phy_parameter->addr) {
+ phy_parameter->data = value;
+ } else {
+ phy_parameter->addr = addr;
+ phy_parameter->data = value;
+ }
+
+ dev_dbg(rtk_phy->dev, "%s addr=0x%02x data=0x%02x\n",
+ __func__, phy_parameter->addr, phy_parameter->data);
+
+ if (strcmp("page0", phy_page) == 0 && (addr == PAGE0_0xE4))
+ value = __updated_page0_0xe4_parameter(phy_data, value);
+
+ if (rtk_usb_phy_write(regAddr, 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)
+{
+ struct rtk_usb_phy *rtk_phy = s->private;
+ 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;
+ int ret = 0;
+ int index;
+ struct phy_data *phy_data = NULL;
+
+ for (index = 0; index < rtk_phy->phyN; 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_data = &((struct phy_data *)rtk_phy->phy_data)[index];
+ break;
+ }
+ }
+ if (!phy_data) {
+ dev_err(rtk_phy->dev,
+ "%s: No phy_data for %s/%s/%s\n",
+ __func__, phy_dir_name, dir_name, file_name);
+ return -EINVAL;
+ }
+
+ if (strcmp("page0", dir_name) == 0)
+ ret = __get_parameter_at_page(s, rtk_phy, phy_data->page0,
+ dir_name, file_name);
+ else if (strcmp("page1", dir_name) == 0)
+ ret = __get_parameter_at_page(s, rtk_phy, phy_data->page1,
+ dir_name, file_name);
+ else if (strcmp("page2", dir_name) == 0)
+ ret = __get_parameter_at_page(s, rtk_phy, phy_data->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_usb_phy *rtk_phy = s->private;
+ struct reg_addr *regAddr = NULL;
+ struct phy_data *phy_data = 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->phyN; index++) {
+ size_t sz = 30;
+ char name[30] = {0};
+
+ snprintf(name, sz, "phy%d", index);
+ if (strncmp(name, phy_dir_name, strlen(name)) == 0) {
+ regAddr = &((struct reg_addr *)rtk_phy->reg_addr)[index];
+ phy_data = &((struct phy_data *)rtk_phy->phy_data)[index];
+ break;
+ }
+ }
+ if (!regAddr) {
+ dev_err(rtk_phy->dev,
+ "%s: No regAddr for %s/%s/%s\n",
+ __func__, phy_dir_name, dir_name, file_name);
+ return -EINVAL;
+ }
+ if (!phy_data) {
+ dev_err(rtk_phy->dev,
+ "%s: No phy_data for %s/%s/%s\n",
+ __func__, phy_dir_name, dir_name, file_name);
+ return -EINVAL;
+ }
+
+ if (strcmp("page0", dir_name) == 0) {
+ rtk_usb_phy_set_page(regAddr, 0);
+ ret = __set_parameter_at_page(rtk_phy, regAddr, phy_data,
+ phy_data->page0, dir_name, file_name, buffer);
+ } else if (strcmp("page1", dir_name) == 0) {
+ rtk_usb_phy_set_page(regAddr, 1);
+ ret = __set_parameter_at_page(rtk_phy, regAddr, phy_data,
+ phy_data->page1, dir_name, file_name, buffer);
+ } else if (strcmp("page2", dir_name) == 0) {
+ rtk_usb_phy_set_page(regAddr, 2);
+ ret = __set_parameter_at_page(rtk_phy, regAddr, phy_data,
+ phy_data->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 rtk_usb2_toggle_show(struct seq_file *s, void *unused)
+{
+ struct rtk_usb_phy *rtk_phy = s->private;
+ struct phy_data *phy_data;
+ int i;
+
+ for (i = 0; i < rtk_phy->phyN; i++) {
+ phy_data = &((struct phy_data *)rtk_phy->phy_data)[i];
+ seq_printf(s, "Now phy#%d do_toggle is %s.\n",
+ i, phy_data->do_toggle?"Enable":"Disable");
+ }
+ seq_puts(s, "ehco 1 to enable toggle phy parameter.\n");
+
+ return 0;
+}
+
+static int rtk_usb2_toggle_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, rtk_usb2_toggle_show, inode->i_private);
+}
+
+static ssize_t rtk_usb2_toggle_write(struct file *file,
+ const char __user *ubuf, size_t count, loff_t *ppos)
+{
+ struct seq_file *s = file->private_data;
+ struct rtk_usb_phy *rtk_phy = s->private;
+ char buf[32];
+ struct phy_data *phy_data;
+ bool enable = false;
+ int i;
+
+ if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
+ return -EFAULT;
+
+ if (!strncmp(buf, "1", 1))
+ enable = true;
+
+ for (i = 0; i < rtk_phy->phyN; i++) {
+ phy_data = &((struct phy_data *)rtk_phy->phy_data)[i];
+ phy_data->do_toggle = enable;
+ dev_info(rtk_phy->dev, "Set phy#%d do_toggle is %s.\n",
+ i, phy_data->do_toggle?"Enable":"Disable");
+ }
+
+ return count;
+}
+
+static const struct file_operations rtk_usb2_toggle_fops = {
+ .open = rtk_usb2_toggle_open,
+ .write = rtk_usb2_toggle_write,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static int create_debug_set_parameter_files(struct rtk_usb_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) {
+ dev_err(rtk_phy->dev,
+ "%s Error create folder %s fail\n",
+ __func__, page);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < addr_size; i++) {
+ size_t sz = 30;
+ char name[30] = {0};
+
+ snprintf(name, sz, "%x", ARRAY_INDEX_MAP_PAGE_ADDR(i));
+
+ if (!debugfs_create_file(name, 0644,
+ page_dir, rtk_phy,
+ &rtk_usb2_set_parameter_fops))
+ dev_err(rtk_phy->dev,
+ "%s Error create file %s/%s fail",
+ page, name, __func__);
+ }
+
+ return 0;
+}
+
+static inline void create_debug_files(struct rtk_usb_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) {
+ dev_err(rtk_phy->dev, "%s Error phy_debug_root is NULL",
+ __func__);
+ return;
+ }
+
+ rtk_phy->debug_dir = debugfs_create_dir(dev_name(rtk_phy->dev),
+ phy_debug_root);
+ if (!rtk_phy->debug_dir) {
+ dev_err(rtk_phy->dev, "%s Error debug_dir is NULL", __func__);
+ 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) {
+ int index, ret;
+
+ for (index = 0; index < rtk_phy->phyN; index++) {
+ struct dentry *phy_dir;
+ struct phy_data *phy_data;
+ size_t sz = 30;
+ char name[30] = {0};
+
+ snprintf(name, sz, "phy%d", index);
+
+ phy_data = &((struct phy_data *)rtk_phy->phy_data)[index];
+
+ phy_dir = debugfs_create_dir(name, set_parameter_dir);
+ if (!phy_dir) {
+ dev_err(rtk_phy->dev,
+ "%s Error create folder %s fail\n",
+ name, __func__);
+ goto file_error;
+ }
+
+ ret = create_debug_set_parameter_files(rtk_phy, phy_dir,
+ "page0", phy_data->page0_size);
+ if (ret < 0) {
+ dev_err(rtk_phy->dev,
+ "%s Error create files for page0 fail\n",
+ __func__);
+ goto file_error;
+ }
+
+ ret = create_debug_set_parameter_files(rtk_phy, phy_dir,
+ "page1", phy_data->page1_size);
+ if (ret < 0) {
+ dev_err(rtk_phy->dev,
+ "%s Error create files for page1 fail\n",
+ __func__);
+ goto file_error;
+ }
+
+ ret = create_debug_set_parameter_files(rtk_phy, phy_dir,
+ "page2", phy_data->page2_size);
+ if (ret < 0) {
+ dev_err(rtk_phy->dev,
+ "%s Error create files for page2 fail\n",
+ __func__);
+ goto file_error;
+ }
+ }
+ }
+
+ if (!debugfs_create_file("toggle", 0644,
+ rtk_phy->debug_dir, rtk_phy, &rtk_usb2_toggle_fops))
+ goto file_error;
+
+ return;
+
+file_error:
+ debugfs_remove_recursive(rtk_phy->debug_dir);
+}
+
+static inline void remove_debug_files(struct rtk_usb_phy *rtk_phy)
+{
+ debugfs_remove_recursive(rtk_phy->debug_dir);
+}
+#else
+static inline void create_debug_files(struct rtk_usb_phy *rtk_phy) { }
+static inline void remove_debug_files(struct rtk_usb_phy *rtk_phy) { }
+#endif /* CONFIG_DEBUG_FS */
+
+static int __get_phy_parameter_by_efuse(struct rtk_usb_phy *rtk_phy,
+ struct phy_data *phy_data, int index)
+{
+ u8 value = 0;
+ struct nvmem_cell *cell;
+ struct soc_device_attribute rtk_soc_groot[] = {
+ { .family = "Realtek Groot",},
+ { /* empty */ }
+ };
+ struct soc_device_attribute rtk_soc_hank[] = {
+ { .family = "Realtek Hank",},
+ { /* empty */ }
+ };
+ struct soc_device_attribute rtk_soc_efuse_v1[] = {
+ { .family = "Realtek Phoenix",},
+ { .family = "Realtek Kylin",},
+ { .family = "Realtek Hercules",},
+ { .family = "Realtek Thor",},
+ { .family = "Realtek Hank",},
+ { .family = "Realtek Groot",},
+ { .family = "Realtek Stark",},
+ { .family = "Realtek Parker",},
+ { /* empty */ }
+ };
+ struct soc_device_attribute rtk_soc_dis_level_at_page0[] = {
+ { .family = "Realtek Phoenix",},
+ { .family = "Realtek Kylin",},
+ { .family = "Realtek Hercules",},
+ { .family = "Realtek Thor",},
+ { .family = "Realtek Hank",},
+ { .family = "Realtek Groot",},
+ { /* empty */ }
+ };
+
+ if (soc_device_match(rtk_soc_efuse_v1)) {
+ dev_dbg(rtk_phy->dev, "Use efuse v1 to updated phy parameter\n");
+ phy_data->check_efuse_version = CHECK_EFUSE_V1;
+ } else {
+ dev_dbg(rtk_phy->dev, "Use efuse v2 to updated phy parameter\n");
+ phy_data->check_efuse_version = CHECK_EFUSE_V2;
+ }
+
+ if (soc_device_match(rtk_soc_dis_level_at_page0)) {
+ dev_dbg(rtk_phy->dev, "Use usb_dc_dis_at_page0\\n");
+ phy_data->usb_dc_dis_at_page0 = true;
+
+ phy_data->usb_dc_cal_mask = 0xf;
+ phy_data->usb_dc_dis_mask = 0xf;
+
+ phy_data->disconnect_driving_updated = 0xf;
+ } else {
+ dev_dbg(rtk_phy->dev, "No use usb_dc_dis_at_page0\n");
+ phy_data->usb_dc_dis_at_page0 = false;
+
+ phy_data->usb_dc_cal_mask = 0x1f;
+ phy_data->usb_dc_dis_mask = 0xf;
+
+ phy_data->disconnect_driving_updated = 0x8;
+ }
+
+ phy_data->efuse_usb_dc_cal_rate = EFUS_USB_DC_CAL_RATE;
+ phy_data->efuse_usb_dc_dis_rate = EFUS_USB_DC_DIS_RATE;
+
+ if (soc_device_match(rtk_soc_hank))
+ phy_data->efuse_usb_dc_cal_rate = 1;
+
+ if (!phy_data->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_warn(rtk_phy->dev, "%s failed to get 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_data->usb_dc_cal_mask;
+
+ dev_dbg(rtk_phy->dev,
+ "buf=0x%x buf_size=%d value=0x%x\n",
+ buf[0], (int)buf_size, value);
+
+ kfree(buf);
+ nvmem_cell_put(cell);
+ }
+
+ if (phy_data->check_efuse_version == CHECK_EFUSE_V1) {
+ int rate = phy_data->efuse_usb_dc_cal_rate;
+
+ if (value <= EFUS_USB_DC_CAL_MAX)
+ phy_data->efuse_usb_dc_cal = (int8_t)(value * rate);
+ else
+ phy_data->efuse_usb_dc_cal = -(int8_t)(
+ (EFUS_USB_DC_CAL_MAX & value) * rate);
+
+ if (soc_device_match(rtk_soc_groot)) {
+ dev_info(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_data->efuse_usb_dc_cal = (int8_t)(value);
+
+ /* We set max dc cal compensate is 0x8 if otp is 0x7 */
+ if (value == 0x7)
+ phy_data->efuse_usb_dc_cal = (int8_t)(value + 1);
+ }
+ } else { /* for CHECK_EFUSE_V2 */
+ phy_data->efuse_usb_dc_cal = value & phy_data->usb_dc_cal_mask;
+ }
+
+ dev_dbg(rtk_phy->dev, "Get Efuse usb_dc_cal=%d for index=%d value=%x\n",
+ phy_data->efuse_usb_dc_cal, index, value);
+
+ /* Read efuse for usb dc disconnect level */
+ value = 0;
+ cell = nvmem_cell_get(rtk_phy->dev, "usb-dc-dis");
+ if (IS_ERR(cell)) {
+ dev_warn(rtk_phy->dev, "%s failed to get 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_data->usb_dc_dis_mask;
+
+ dev_dbg(rtk_phy->dev,
+ "buf=0x%x buf_size=%d value=0x%x\n",
+ buf[0], (int)buf_size, value);
+
+ kfree(buf);
+ nvmem_cell_put(cell);
+ }
+
+ if (phy_data->check_efuse_version == CHECK_EFUSE_V1) {
+ int rate = phy_data->efuse_usb_dc_dis_rate;
+
+ if (value <= EFUS_USB_DC_DIS_MAX)
+ phy_data->efuse_usb_dc_dis = (int8_t)(value * rate);
+ else
+ phy_data->efuse_usb_dc_dis = -(int8_t)(
+ (EFUS_USB_DC_DIS_MAX & value) * rate);
+ } else { /* for CHECK_EFUSE_V2 */
+ phy_data->efuse_usb_dc_dis = value & phy_data->usb_dc_dis_mask;
+ }
+
+ dev_dbg(rtk_phy->dev, "Get Efuse usb_dc_dis=%d for index=%d value=%x\n",
+ phy_data->efuse_usb_dc_dis, index, value);
+
+out:
+ return 0;
+}
+
+static int __get_phy_parameter(struct device *dev, struct phy_data *phy_data,
+ struct device_node *sub_node)
+{
+ u32 page_size = 0;
+ u32 num_cells = 2; /*< addr value > */
+ u32 data_size;
+ int i, ret = 0;
+
+ /* Page 0 */
+ page_size = MAX_USB_PHY_PAGE0_DATA_SIZE;
+ phy_data->page0_size = page_size;
+ phy_data->page0 = devm_kzalloc(dev,
+ sizeof(struct phy_parameter) * page_size, GFP_KERNEL);
+ if (!phy_data->page0) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ if (!of_get_property(sub_node, "realtek,page0-param", &data_size)) {
+ dev_dbg(dev, "%s No page0 parameter (data_size=%d)\n",
+ __func__, data_size);
+ data_size = 0;
+ }
+
+ if (!data_size)
+ goto parse_page1;
+
+ data_size = data_size / (sizeof(u32) * num_cells);
+
+ for (i = 0; i < data_size; i++) {
+ struct phy_parameter *phy_parameter;
+ u32 addr, data;
+ int index, offset;
+
+ offset = i * num_cells;
+
+ ret = of_property_read_u32_index(sub_node, "realtek,page0-param",
+ offset, &addr);
+ if (ret) {
+ dev_err(dev, "ERROR: To get page0 i=%d addr=0x%x\n",
+ i, addr);
+ break;
+ }
+
+ ret = of_property_read_u32_index(sub_node, "realtek,page0-param",
+ offset + 1, &data);
+ if (ret) {
+ dev_err(dev, "ERROR: To get page0 i=%d addr=0x%x\n",
+ i, data);
+ break;
+ }
+
+ index = PAGE_ADDR_MAP_ARRAY_INDEX(addr);
+ phy_parameter = (phy_data->page0 + index);
+ phy_parameter->addr = (char)addr;
+ phy_parameter->data = (char)data;
+
+ dev_dbg(dev, "page0 index=%d addr=0x%x data=0x%x\n",
+ index, phy_parameter->addr, phy_parameter->data);
+ }
+
+parse_page1:
+ /* Page 1 */
+ page_size = MAX_USB_PHY_PAGE1_DATA_SIZE;
+ phy_data->page1_size = page_size;
+ phy_data->page1 = devm_kzalloc(dev,
+ sizeof(struct phy_parameter) * page_size, GFP_KERNEL);
+ if (!phy_data->page1) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ if (!of_get_property(sub_node, "realtek,page1-param", &data_size)) {
+ dev_dbg(dev, "%s No page1 parameter (data_size=%d)\n",
+ __func__, data_size);
+ data_size = 0;
+ }
+
+ if (!data_size)
+ goto parse_page2;
+
+ data_size = data_size / (sizeof(u32) * num_cells);
+
+ for (i = 0; i < data_size; i++) {
+ struct phy_parameter *phy_parameter;
+ u32 addr, data;
+ int index, offset;
+
+ offset = i * num_cells;
+
+ ret = of_property_read_u32_index(sub_node, "realtek,page1-param",
+ offset, &addr);
+ if (ret) {
+ dev_err(dev, "ERROR: To get page1 i=%d addr=0x%x\n",
+ i, addr);
+ break;
+ }
+
+ ret = of_property_read_u32_index(sub_node, "realtek,page1-param",
+ offset + 1, &data);
+ if (ret) {
+ dev_err(dev, "ERROR: To get page1 i=%d addr=0x%x\n",
+ i, data);
+ break;
+ }
+
+ index = PAGE_ADDR_MAP_ARRAY_INDEX(addr);
+ phy_parameter = phy_data->page1 + index;
+ phy_parameter->addr = (char)addr;
+ phy_parameter->data = (char)data;
+
+ dev_dbg(dev, "page1 index=%d addr=0x%x data=0x%x\n",
+ index, phy_parameter->addr, phy_parameter->data);
+ }
+
+parse_page2:
+ /* Page 2 */
+ if (of_property_read_bool(sub_node, "realtek,support-page2-param"))
+ page_size = MAX_USB_PHY_PAGE2_DATA_SIZE;
+ else
+ page_size = 0;
+
+ if (!page_size)
+ goto out;
+
+ phy_data->page2_size = page_size;
+ phy_data->page2 = devm_kzalloc(dev,
+ sizeof(struct phy_parameter) * page_size, GFP_KERNEL);
+ if (!phy_data->page2) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ if (!of_get_property(sub_node, "realtek,page2-param", &data_size)) {
+ dev_dbg(dev, "%s No page2 parameter (data_size=%d)\n",
+ __func__, data_size);
+ data_size = 0;
+ }
+ data_size = data_size / (sizeof(u32) * num_cells);
+
+ for (i = 0; i < data_size; i++) {
+ struct phy_parameter *phy_parameter;
+ u32 addr, data;
+ int index, offset;
+
+ offset = i * num_cells;
+
+ ret = of_property_read_u32_index(sub_node, "realtek,page2-param",
+ offset, &addr);
+ if (ret) {
+ dev_err(dev, "ERROR: To get page2 i=%d addr=0x%x\n",
+ i, addr);
+ break;
+ }
+
+ ret = of_property_read_u32_index(sub_node, "realtek,page2-param",
+ offset + 1, &data);
+ if (ret) {
+ dev_err(dev, "ERROR: To get page2 i=%d addr=0x%x\n",
+ i, data);
+ break;
+ }
+
+ index = PAGE_ADDR_MAP_ARRAY_INDEX(addr);
+ phy_parameter = phy_data->page2 + index;
+ phy_parameter->addr = (char)addr;
+ phy_parameter->data = (char)data;
+
+ dev_dbg(dev, "page2 index=%d addr=0x%x data=0x%x\n",
+ index, phy_parameter->addr, phy_parameter->data);
+ }
+
+out:
+ return ret;
+}
+
+static int get_phy_parameter(struct rtk_usb_phy *rtk_phy,
+ struct device_node *sub_node)
+{
+ struct device *dev = rtk_phy->dev;
+ struct reg_addr *addr;
+ struct phy_data *phy_data;
+ int ret = 0;
+ int index;
+
+ if (of_property_read_u32(sub_node, "reg", &index)) {
+ dev_err(dev, "sub_node without reg\n");
+ return -EINVAL;
+ }
+
+ dev_dbg(dev, "sub_node index=%d\n", index);
+
+ addr = &((struct reg_addr *)rtk_phy->reg_addr)[index];
+ phy_data = &((struct phy_data *)rtk_phy->phy_data)[index];
+
+ addr->reg_wrap_vstatus = of_iomap(dev->of_node, 0);
+ addr->reg_gusb2phyacc0 = of_iomap(dev->of_node, 1) + index;
+ addr->vstatus_index = index;
+ dev_dbg(dev, "%s %d #%d reg_wrap_vstatus=%p\n", __func__, __LINE__,
+ index, addr->reg_wrap_vstatus);
+ dev_dbg(dev, "%s %d #%d reg_gusb2phyacc0=%p\n", __func__, __LINE__,
+ index, addr->reg_gusb2phyacc0);
+
+ if (!sub_node)
+ goto err;
+
+ ret = __get_phy_parameter(dev, phy_data, sub_node);
+ if (ret)
+ goto err;
+
+ if (of_property_read_bool(sub_node, "realtek,do-toggle"))
+ phy_data->do_toggle = true;
+ else
+ phy_data->do_toggle = false;
+
+ if (of_property_read_bool(sub_node, "realtek,do-toggle-driving"))
+ phy_data->do_toggle_driving = true;
+ else
+ phy_data->do_toggle_driving = false;
+
+ if (of_property_read_bool(sub_node, "realtek,check-efuse"))
+ phy_data->check_efuse = true;
+ else
+ phy_data->check_efuse = false;
+
+ if (of_property_read_bool(sub_node, "realtek,use-default-parameter"))
+ phy_data->use_default_parameter = true;
+ else
+ phy_data->use_default_parameter = false;
+
+ if (of_property_read_bool(sub_node,
+ "realtek,is-double-sensitivity-mode"))
+ phy_data->is_double_sensitivity_mode = true;
+ else
+ phy_data->is_double_sensitivity_mode = false;
+
+ if (of_property_read_bool(sub_node,
+ "realtek,ldo-force-enable"))
+ phy_data->ldo_force_enable = true;
+ else
+ phy_data->ldo_force_enable = false;
+
+ if (of_property_read_s32(sub_node,
+ "realtek,ldo-driving-compensate", &phy_data->ldo_driving_compensate))
+ phy_data->ldo_driving_compensate = 0;
+
+ if (of_property_read_s32(sub_node,
+ "realtek,driving-compensate", &phy_data->driving_compensate))
+ phy_data->driving_compensate = 0;
+
+ __get_phy_parameter_by_efuse(rtk_phy, phy_data, index);
+
+err:
+ return ret;
+}
+
+static int rtk_usb2phy_probe(struct platform_device *pdev)
+{
+ struct rtk_usb_phy *rtk_phy;
+ struct device *dev = &pdev->dev;
+ struct device_node *node;
+ struct device_node *sub_node;
+ struct phy *generic_phy;
+ struct phy_provider *phy_provider;
+ int phyN, ret = 0;
+
+ 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_usb_phy_notify_port_status;
+
+ if (!dev->of_node) {
+ dev_err(dev, "%s %d No device node\n", __func__, __LINE__);
+ goto err;
+ }
+
+ node = dev->of_node;
+
+ rtk_phy->usb_ctrl_regs = syscon_regmap_lookup_by_phandle(node, "realtek,usb-ctrl");
+ if (IS_ERR(rtk_phy->usb_ctrl_regs)) {
+ dev_info(dev, "%s: DTS no support usb_ctrl regs syscon\n", __func__);
+ rtk_phy->usb_ctrl_regs = NULL;
+ }
+
+ phyN = of_get_child_count(node);
+ rtk_phy->phyN = phyN;
+ dev_dbg(dev, "%s phyN=%d\n", __func__, rtk_phy->phyN);
+
+ rtk_phy->reg_addr = devm_kzalloc(dev,
+ sizeof(struct reg_addr) * phyN, GFP_KERNEL);
+ if (!rtk_phy->reg_addr)
+ return -ENOMEM;
+
+ rtk_phy->phy_data = devm_kzalloc(dev,
+ sizeof(struct phy_data) * phyN, GFP_KERNEL);
+ if (!rtk_phy->phy_data)
+ return -ENOMEM;
+
+ for (sub_node = of_get_next_child(node, NULL); sub_node != NULL;
+ sub_node = of_get_next_child(node, sub_node)) {
+ ret = get_phy_parameter(rtk_phy, sub_node);
+ if (ret) {
+ dev_err(dev, "%s: get_phy_parameter fail ret=%d\n",
+ __func__, 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_usb_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,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,
+ .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] 33+ messages in thread
* [PATCH v3 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY
2023-06-07 6:24 [PATCH v3 1/5] usb: phy: add usb phy notify port status API Stanley Chang
2023-06-07 6:24 ` [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY Stanley Chang
@ 2023-06-07 6:24 ` Stanley Chang
2023-06-07 11:52 ` Krzysztof Kozlowski
2023-06-08 1:35 ` kernel test robot
2023-06-07 6:24 ` [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY Stanley Chang
2023-06-07 6:24 ` [PATCH v3 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY Stanley Chang
3 siblings, 2 replies; 33+ messages in thread
From: Stanley Chang @ 2023-06-07 6:24 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,
Michael Grzeschik, Flavio Suligoi, Matthias Kaehlcke,
Mathias Nyman, 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>
---
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 | 10 +
drivers/phy/realtek/Makefile | 1 +
drivers/phy/realtek/phy-rtk-usb3.c | 1070 ++++++++++++++++++++++++++++
3 files changed, 1081 insertions(+)
create mode 100644 drivers/phy/realtek/phy-rtk-usb3.c
diff --git a/drivers/phy/realtek/Kconfig b/drivers/phy/realtek/Kconfig
index 76e31f6abdee..28ee3d9be568 100644
--- a/drivers/phy/realtek/Kconfig
+++ b/drivers/phy/realtek/Kconfig
@@ -11,3 +11,13 @@ 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"
+ 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..aae0525ce78d
--- /dev/null
+++ b/drivers/phy/realtek/phy-rtk-usb3.c
@@ -0,0 +1,1070 @@
+// 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_0x30 0x30
+
+#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 PHY_ADDR_MAP_ARRAY_INDEX(addr) (addr)
+#define ARRAY_INDEX_MAP_PHY_ADDR(index) (index)
+
+struct reg_addr {
+ void __iomem *reg_mdio_ctl;
+};
+
+struct phy_parameter {
+ u8 addr;
+ u16 data;
+};
+
+struct phy_data {
+ int size;
+ struct phy_parameter *parameter;
+
+ bool check_efuse;
+ u8 efuse_usb_u3_tx_lfps_swing_trim;
+ bool do_toggle;
+ bool do_toggle_once;
+ bool use_default_parameter;
+ bool check_rx_front_end_offset;
+};
+
+struct rtk_usb_phy {
+ struct usb_phy phy;
+ struct device *dev;
+
+ int phyN;
+ void *reg_addr;
+ void *phy_data;
+
+ 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_usb_phy3_wait_vbusy(struct reg_addr *regAddr)
+{
+ return utmi_wait_register(regAddr->reg_mdio_ctl, USB_MDIO_CTRL_PHY_BUSY, 0);
+}
+
+static u16 rtk_usb_phy_read(struct reg_addr *regAddr, char addr)
+{
+ unsigned int regVal;
+ u32 value;
+
+ regVal = (addr << USB_MDIO_CTRL_PHY_ADDR_SHIFT);
+
+ writel(regVal, regAddr->reg_mdio_ctl);
+
+ rtk_usb_phy3_wait_vbusy(regAddr);
+
+ value = readl(regAddr->reg_mdio_ctl);
+ value = value >> USB_MDIO_CTRL_PHY_DATA_SHIFT;
+
+ return (u16)value;
+}
+
+static int rtk_usb_phy_write(struct reg_addr *regAddr, char addr, u16 data)
+{
+ unsigned int regVal;
+
+ regVal = USB_MDIO_CTRL_PHY_WRITE |
+ (addr << USB_MDIO_CTRL_PHY_ADDR_SHIFT) |
+ (data << USB_MDIO_CTRL_PHY_DATA_SHIFT);
+
+ writel(regVal, regAddr->reg_mdio_ctl);
+
+ rtk_usb_phy3_wait_vbusy(regAddr);
+
+ return 0;
+}
+
+static void do_rtk_usb3_phy_toggle(struct rtk_usb_phy *rtk_phy, int i,
+ bool isConnect)
+{
+ struct reg_addr *regAddr;
+ struct phy_data *phy_data;
+ struct phy_parameter *phy_parameter;
+ size_t index;
+
+ regAddr = &((struct reg_addr *)rtk_phy->reg_addr)[i];
+ phy_data = &((struct phy_data *)rtk_phy->phy_data)[i];
+
+ if (!phy_data) {
+ dev_err(rtk_phy->dev, "%s phy_data is NULL!\n", __func__);
+ return;
+ }
+
+ if (!phy_data->do_toggle)
+ return;
+
+ phy_parameter = phy_data->parameter;
+
+ index = PHY_ADDR_MAP_ARRAY_INDEX(PHY_ADDR_0x09);
+
+ if (index < phy_data->size) {
+ u8 addr = (phy_parameter + index)->addr;
+ u16 data = (phy_parameter + index)->data;
+
+ if (addr == 0xFF) {
+ addr = ARRAY_INDEX_MAP_PHY_ADDR(index);
+ data = rtk_usb_phy_read(regAddr, addr);
+ (phy_parameter + index)->addr = addr;
+ (phy_parameter + index)->data = data;
+ }
+ mdelay(1);
+ dev_info(rtk_phy->dev,
+ "%s ########## to toggle PHY addr 0x09 BIT(9)\n",
+ __func__);
+ rtk_usb_phy_write(regAddr, addr, data&(~BIT(9)));
+ mdelay(1);
+ rtk_usb_phy_write(regAddr, addr, data);
+ }
+ dev_info(rtk_phy->dev, "%s ########## PHY addr 0x1f = 0x%04x\n",
+ __func__, rtk_usb_phy_read(regAddr, PHY_ADDR_0x1F));
+}
+
+static int do_rtk_usb_phy_init(struct rtk_usb_phy *rtk_phy, int phy_index)
+{
+ struct reg_addr *regAddr =
+ &((struct reg_addr *)rtk_phy->reg_addr)[phy_index];
+ struct phy_data *phy_data =
+ &((struct phy_data *)rtk_phy->phy_data)[phy_index];
+ int index = 0;
+ struct phy_parameter *phy_parameter = phy_data->parameter;
+
+ dev_dbg(rtk_phy->dev, "%s: init phy#%d\n", __func__, phy_index);
+
+ if (phy_data->use_default_parameter) {
+ dev_info(rtk_phy->dev, "%s phy#%d use default parameter\n",
+ __func__, phy_index);
+ goto do_toggle;
+ }
+
+ for (index = 0; index < phy_data->size; index++) {
+ u8 addr = (phy_parameter + index)->addr;
+ u16 data = (phy_parameter + index)->data;
+
+ if (addr == 0xFF)
+ continue;
+
+ if (addr == PHY_ADDR_0x20) {
+ u8 efuse_val = phy_data->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;
+
+ if (efuse_val) {
+ data &= ~(val_mask << val_shift);
+ data |= ((efuse_val & val_mask) << val_shift);
+ }
+ }
+
+ rtk_usb_phy_write(regAddr, addr, data);
+ }
+
+ for (index = 0; index < phy_data->size; index++) {
+ u8 addr = (phy_parameter + index)->addr;
+ u16 data = (phy_parameter + index)->data;
+
+ if (addr == 0xFF)
+ continue;
+
+ dev_dbg(rtk_phy->dev, "[USB3_PHY], addr = 0x%02x, data = 0x%04x ==> read value = 0x%04x\n",
+ addr, data,
+ rtk_usb_phy_read(regAddr, addr));
+ }
+
+do_toggle:
+ if (phy_data->do_toggle_once)
+ phy_data->do_toggle = true;
+
+ do_rtk_usb3_phy_toggle(rtk_phy, phy_index, false);
+
+ if (phy_data->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_usb_phy_read(regAddr, PHY_ADDR_0x0D);
+ value_0x10 = rtk_usb_phy_read(regAddr, PHY_ADDR_0x10);
+
+ rtk_usb_phy_write(regAddr, PHY_ADDR_0x0D,
+ value_0x0D | REG_0x0D_RX_DEBUG_TEST_EN);
+ rtk_usb_phy_write(regAddr, PHY_ADDR_0x10,
+ (value_0x10 & ~REG_0x10_DEBUG_MODE_SETTING_MASK) |
+ REG_0x10_DEBUG_MODE_SETTING);
+
+ check_value = rtk_usb_phy_read(regAddr, PHY_ADDR_0x30);
+
+ while (!(check_value & BIT(15))) {
+ check_value = rtk_usb_phy_read(regAddr, 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);
+ else
+ dev_info(rtk_phy->dev, "toggle okay addr=0x%02x, data=0x%04x\n",
+ PHY_ADDR_0x30, check_value);
+
+ /* Disable Debug mode by set 0x0D and 0x10 to default*/
+ rtk_usb_phy_write(regAddr, PHY_ADDR_0x0D, value_0x0D);
+ rtk_usb_phy_write(regAddr, PHY_ADDR_0x10, value_0x10);
+
+ phy_data->do_toggle = false;
+ }
+
+
+ if (phy_data->check_rx_front_end_offset) {
+ u16 rx_offset_code, rx_offset_range;
+ bool do_update = false;
+
+ rx_offset_code = rtk_usb_phy_read(regAddr, PHY_ADDR_0x1F);
+ if (((rx_offset_code & REG_0x1F_RX_OFFSET_CODE_MASK) == 0x0) ||
+ ((rx_offset_code & REG_0x1F_RX_OFFSET_CODE_MASK) ==
+ REG_0x1F_RX_OFFSET_CODE_MASK))
+ do_update = true;
+
+ rx_offset_range = rtk_usb_phy_read(regAddr, PHY_ADDR_0x0B);
+ if (((rx_offset_range & REG_0x0B_RX_OFFSET_RANGE_MASK) ==
+ REG_0x0B_RX_OFFSET_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 & (~REG_0x0B_RX_OFFSET_RANGE_MASK);
+ tmp2 = rx_offset_range & REG_0x0B_RX_OFFSET_RANGE_MASK;
+ tmp2 += (1 << 2);
+ rx_offset_range = tmp1 | (tmp2 & REG_0x0B_RX_OFFSET_RANGE_MASK);
+ rtk_usb_phy_write(regAddr, PHY_ADDR_0x0B, rx_offset_range);
+ goto do_toggle;
+ }
+ }
+
+ return 0;
+}
+
+static int rtk_usb_phy_init(struct phy *phy)
+{
+ struct rtk_usb_phy *rtk_phy = phy_get_drvdata(phy);
+ int ret = 0;
+ int i;
+ unsigned long phy_init_time = jiffies;
+
+ if (!rtk_phy) {
+ pr_err("%s rtk_phy is NULL!\n", __func__);
+ return -ENODEV;
+ }
+
+ dev_dbg(rtk_phy->dev, "Init RTK USB 3.0 PHY\n");
+ for (i = 0; i < rtk_phy->phyN; i++)
+ ret = do_rtk_usb_phy_init(rtk_phy, i);
+
+ dev_info(rtk_phy->dev, "Initialized RTK USB 3.0 PHY (take %dms)\n",
+ jiffies_to_msecs(jiffies - phy_init_time));
+ return ret;
+}
+
+static int rtk_usb_phy_exit(struct phy *phy)
+{
+ struct rtk_usb_phy *rtk_phy = phy_get_drvdata(phy);
+
+ if (!rtk_phy) {
+ pr_err("%s rtk_phy is NULL!\n", __func__);
+ return -ENODEV;
+ }
+
+ dev_dbg(rtk_phy->dev, "Exit RTK USB 3.0 PHY\n");
+
+ return 0;
+}
+
+static const struct phy_ops ops = {
+ .init = rtk_usb_phy_init,
+ .exit = rtk_usb_phy_exit,
+ .owner = THIS_MODULE,
+};
+
+static void rtk_usb_phy_toggle(struct usb_phy *usb3_phy, bool isConnect, int port)
+{
+ int index = port;
+ struct rtk_usb_phy *rtk_phy = NULL;
+
+ if (usb3_phy != NULL && usb3_phy->dev != NULL)
+ rtk_phy = dev_get_drvdata(usb3_phy->dev);
+
+ if (rtk_phy == NULL) {
+ pr_err("%s ERROR! NO this device\n", __func__);
+ return;
+ }
+
+ if (index > rtk_phy->phyN) {
+ pr_err("%s %d ERROR! port=%d > phyN=%d\n",
+ __func__, __LINE__, index, rtk_phy->phyN);
+ return;
+ }
+
+ do_rtk_usb3_phy_toggle(rtk_phy, index, isConnect);
+}
+
+static int rtk_usb_phy_notify_port_status(struct usb_phy *x, int port,
+ u16 portstatus, u16 portchange)
+{
+ bool isConnect = 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)
+ isConnect = true;
+
+ if (portchange & USB_PORT_STAT_C_CONNECTION)
+ rtk_usb_phy_toggle(x, isConnect, 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);
+ if (!phy_debug_root)
+ pr_err("%s Error phy_debug_root is NULL\n", __func__);
+ else
+ pr_debug("%s Create phy_debug_root folder\n", __func__);
+ }
+
+ return phy_debug_root;
+}
+
+static int rtk_usb3_parameter_show(struct seq_file *s, void *unused)
+{
+ struct rtk_usb_phy *rtk_phy = s->private;
+ int i, index;
+
+ for (i = 0; i < rtk_phy->phyN; i++) {
+ struct reg_addr *regAddr =
+ &((struct reg_addr *)rtk_phy->reg_addr)[i];
+ struct phy_data *phy_data =
+ &((struct phy_data *)rtk_phy->phy_data)[i];
+ struct phy_parameter *phy_parameter;
+
+ phy_parameter = phy_data->parameter;
+
+ seq_printf(s, "[USB3_PHY] PHY %d\n", i);
+
+ for (index = 0; index < phy_data->size; index++) {
+ u8 addr = ARRAY_INDEX_MAP_PHY_ADDR(index);
+ u16 data = (phy_parameter + index)->data;
+
+ if ((phy_parameter + index)->addr == 0xFF)
+ seq_printf(s, "[USB3_PHY], addr = 0x%02x, data = none ==> read value = 0x%04x\n",
+ addr,
+ rtk_usb_phy_read(regAddr, addr));
+ else
+ seq_printf(s, "[USB3_PHY], addr = 0x%02x, data = 0x%04x ==> read value = 0x%04x\n",
+ addr, data,
+ rtk_usb_phy_read(regAddr, addr));
+ }
+
+ seq_puts(s, "Property:\n");
+ seq_printf(s, "check_efuse: %s\n",
+ phy_data->check_efuse?"Enable":"Disable");
+ seq_printf(s, "efuse_usb_u3_tx_lfps_swing_trim: 0x%x\n",
+ (int)phy_data->efuse_usb_u3_tx_lfps_swing_trim);
+ seq_printf(s, "do_toggle: %s\n",
+ phy_data->do_toggle?"Enable":"Disable");
+ seq_printf(s, "do_toggle_once: %s\n",
+ phy_data->do_toggle_once?"Enable":"Disable");
+ seq_printf(s, "use_default_parameter: %s\n",
+ phy_data->use_default_parameter?"Enable":"Disable");
+ }
+ 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_usb_phy *rtk_phy,
+ struct phy_parameter *phy_parameter_array,
+ const char *phy_addr)
+{
+ struct phy_parameter *phy_parameter;
+ uint32_t 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_parameter = (phy_parameter_array + i);
+
+ if (phy_parameter->addr != 0xFF)
+ seq_printf(s, "Now Parameter addr 0x%02x = 0x%04x\n",
+ phy_parameter->addr, phy_parameter->data);
+ else
+ seq_printf(s, "Now Parameter addr 0x%02x is default\n",
+ addr);
+
+ dev_dbg(rtk_phy->dev, "%s addr=0x%02x data=0x%04x\n",
+ __func__, phy_parameter->addr, phy_parameter->data);
+
+ return 0;
+}
+
+static int __set_parameter_at_page(
+ struct rtk_usb_phy *rtk_phy,
+ struct reg_addr *regAddr, struct phy_data *phy_data,
+ struct phy_parameter *phy_parameter_array,
+ const char *phy_addr, const char *phy_value)
+{
+ struct phy_parameter *phy_parameter;
+ uint32_t 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_parameter = (phy_parameter_array + i);
+
+ if (phy_parameter->addr != 0xFF) {
+ phy_parameter->data = value;
+ } else {
+ phy_parameter->addr = addr;
+ phy_parameter->data = value;
+ }
+
+ dev_info(rtk_phy->dev, "%s addr=0x%02x data=0x%04x\n",
+ __func__, phy_parameter->addr, phy_parameter->data);
+
+ if (addr == PHY_ADDR_0x20)
+ dev_info(rtk_phy->dev,
+ "%s PHY_ADDR_0x20 NOT use efuse u3_tx_lfps_swing_trim value\n",
+ __func__);
+
+ if (rtk_usb_phy_write(regAddr, 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)
+{
+ struct rtk_usb_phy *rtk_phy = s->private;
+ 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;
+ int ret, index;
+ struct phy_data *phy_data = NULL;
+
+ for (index = 0; index < rtk_phy->phyN; 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_data = &((struct phy_data *)rtk_phy->phy_data)[index];
+ break;
+ }
+ }
+ if (!phy_data) {
+ dev_err(rtk_phy->dev,
+ "%s: No phy_data for %s/%s\n",
+ __func__, phy_dir_name, file_name);
+ return -EINVAL;
+ }
+
+ ret = __get_parameter_at_page(s, rtk_phy, phy_data->parameter, 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_usb_phy *rtk_phy = s->private;
+ struct reg_addr *regAddr = NULL;
+ struct phy_data *phy_data = 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->phyN; index++) {
+ size_t sz = 30;
+ char name[30] = {0};
+
+ snprintf(name, sz, "phy%d", index);
+ if (strncmp(name, phy_dir_name, strlen(name)) == 0) {
+ regAddr = &((struct reg_addr *)rtk_phy->reg_addr)[index];
+ phy_data = &((struct phy_data *)rtk_phy->phy_data)[index];
+ break;
+ }
+ }
+ if (!regAddr) {
+ dev_err(rtk_phy->dev,
+ "%s: No regAddr for %s/%s\n",
+ __func__, phy_dir_name, file_name);
+ return -EINVAL;
+ }
+ if (!phy_data) {
+ dev_err(rtk_phy->dev,
+ "%s: No phy_data for %s/%s\n",
+ __func__, phy_dir_name, file_name);
+ return -EINVAL;
+ }
+
+ ret = __set_parameter_at_page(rtk_phy, regAddr, phy_data,
+ phy_data->parameter, 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 rtk_usb3_toggle_show(struct seq_file *s, void *unused)
+{
+ struct rtk_usb_phy *rtk_phy = s->private;
+ struct phy_data *phy_data;
+ int i;
+
+ for (i = 0; i < rtk_phy->phyN; i++) {
+ phy_data = &((struct phy_data *)rtk_phy->phy_data)[i];
+ seq_printf(s, "Now phy#%d do_toggle is %s.\n",
+ i, phy_data->do_toggle?"Enable":"Disable");
+ }
+ seq_puts(s, "ehco 1 to enable toggle phy parameter.\n");
+
+ return 0;
+}
+
+static int rtk_usb3_toggle_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, rtk_usb3_toggle_show, inode->i_private);
+}
+
+static ssize_t rtk_usb3_toggle_write(struct file *file,
+ const char __user *ubuf, size_t count, loff_t *ppos)
+{
+ struct seq_file *s = file->private_data;
+ struct rtk_usb_phy *rtk_phy = s->private;
+ char buf[32];
+ struct phy_data *phy_data;
+ bool enable = false;
+ int i;
+
+ if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
+ return -EFAULT;
+
+ if (!strncmp(buf, "1", 1))
+ enable = true;
+
+ for (i = 0; i < rtk_phy->phyN; i++) {
+ phy_data = &((struct phy_data *)rtk_phy->phy_data)[i];
+ phy_data->do_toggle = enable;
+ dev_info(rtk_phy->dev, "Set phy#%d do_toggle is %s.\n",
+ i, phy_data->do_toggle?"Enable":"Disable");
+ }
+
+ return count;
+}
+
+static const struct file_operations rtk_usb3_toggle_fops = {
+ .open = rtk_usb3_toggle_open,
+ .write = rtk_usb3_toggle_write,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static int create_debug_set_parameter_files(struct rtk_usb_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));
+
+ if (!debugfs_create_file(name, 0644,
+ phy_dir, rtk_phy,
+ &rtk_usb3_set_parameter_fops))
+ dev_err(rtk_phy->dev,
+ "%s Error create file %s fail",
+ name, __func__);
+ }
+
+ return 0;
+}
+
+static inline void create_debug_files(struct rtk_usb_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) {
+ dev_err(rtk_phy->dev, "%s Error phy_debug_root is NULL",
+ __func__);
+ return;
+ }
+ rtk_phy->debug_dir = debugfs_create_dir(dev_name(rtk_phy->dev),
+ phy_debug_root);
+ if (!rtk_phy->debug_dir) {
+ dev_err(rtk_phy->dev, "%s Error debug_dir is NULL", __func__);
+ 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) {
+ int index, ret;
+
+ for (index = 0; index < rtk_phy->phyN; index++) {
+ struct dentry *phy_dir;
+ struct phy_data *phy_data;
+ size_t sz = 30;
+ char name[30] = {0};
+
+ snprintf(name, sz, "phy%d", index);
+
+ phy_data = &((struct phy_data *)rtk_phy->phy_data)[index];
+
+ phy_dir = debugfs_create_dir(name, set_parameter_dir);
+ if (!phy_dir) {
+ dev_err(rtk_phy->dev,
+ "%s Error create folder %s fail\n",
+ name, __func__);
+ goto file_error;
+ }
+
+ ret = create_debug_set_parameter_files(rtk_phy, phy_dir,
+ phy_data->size);
+ if (ret < 0) {
+ dev_err(rtk_phy->dev,
+ "%s Error create files fail\n",
+ __func__);
+ goto file_error;
+ }
+ }
+ }
+
+ if (!debugfs_create_file("toggle", 0644, rtk_phy->debug_dir, rtk_phy,
+ &rtk_usb3_toggle_fops))
+ goto file_error;
+
+ return;
+
+file_error:
+ debugfs_remove_recursive(rtk_phy->debug_dir);
+}
+
+static inline void remove_debug_files(struct rtk_usb_phy *rtk_phy)
+{
+ debugfs_remove_recursive(rtk_phy->debug_dir);
+}
+#else
+static inline void create_debug_files(struct rtk_usb_phy *rtk_phy) { }
+static inline void remove_debug_files(struct rtk_usb_phy *rtk_phy) { }
+#endif /* CONFIG_DEBUG_FS */
+
+static int __get_phy_parameter_by_efuse(struct rtk_usb_phy *rtk_phy,
+ struct phy_data *phy_data, int index)
+{
+ u8 value = 0;
+ struct nvmem_cell *cell;
+
+ if (!phy_data->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 failed to get usb_u3_tx_lfps_swing_trim: %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;
+
+ dev_dbg(rtk_phy->dev,
+ "phy index=%d buf=0x%x buf_size=%d value=0x%x\n",
+ index, buf[0], (int)buf_size, value);
+ kfree(buf);
+ nvmem_cell_put(cell);
+ }
+
+ if ((value > 0) && (value < 0x8))
+ phy_data->efuse_usb_u3_tx_lfps_swing_trim = 0x8;
+ else
+ phy_data->efuse_usb_u3_tx_lfps_swing_trim = (u8)value;
+
+ dev_dbg(rtk_phy->dev, "Get Efuse usb_u3_tx_lfps_swing_trim=0x%x (value=0x%x)\n",
+ phy_data->efuse_usb_u3_tx_lfps_swing_trim, value);
+
+out:
+ return 0;
+}
+
+static int __get_phy_parameter(struct device *dev, struct phy_data *phy_data,
+ struct device_node *sub_node)
+{
+ struct phy_parameter *phy_parameter;
+ int i, ret = 0;
+ int data_size, num_cells = 2;
+
+ phy_data->size = MAX_USB_PHY_DATA_SIZE;
+ phy_data->parameter = devm_kzalloc(dev,
+ sizeof(struct phy_parameter) * phy_data->size, GFP_KERNEL);
+ if (!phy_data->parameter)
+ return -ENOMEM;
+
+ if (!of_get_property(sub_node, "realtek,param", &data_size)) {
+ dev_dbg(dev, "%s No parameter (data_size=%d)\n",
+ __func__, data_size);
+ data_size = 0;
+ }
+
+ if (!data_size)
+ goto out;
+
+ phy_parameter = phy_data->parameter;
+ /* Set default addr to 0xff for no data case */
+ for (i = 0; i < phy_data->size; i++)
+ (phy_parameter + i)->addr = 0xFF;
+
+ data_size = data_size / (sizeof(u32) * num_cells);
+ for (i = 0; i < data_size; i++) {
+ struct phy_parameter *parameter;
+ u32 addr, data;
+ int offset, index;
+
+ offset = i * num_cells;
+
+ ret = of_property_read_u32_index(sub_node, "realtek,param",
+ offset, &addr);
+ if (ret) {
+ dev_err(dev, "ERROR: To get param i=%d addr=0x%x\n",
+ i, addr);
+ break;
+ }
+
+ ret = of_property_read_u32_index(sub_node, "realtek,param",
+ offset + 1, &data);
+ if (ret) {
+ dev_err(dev, "ERROR: To get param i=%d addr=0x%x\n",
+ i, data);
+ break;
+ }
+
+ index = PHY_ADDR_MAP_ARRAY_INDEX(addr);
+ parameter = (phy_parameter + index);
+ parameter->addr = (u8)addr;
+ parameter->data = (u16)data;
+
+ dev_dbg(dev, "param index=%d addr=0x%x data=0x%x\n", index,
+ parameter->addr, parameter->data);
+ }
+
+out:
+ return ret;
+}
+
+static int get_phy_parameter(struct rtk_usb_phy *rtk_phy,
+ struct device_node *sub_node)
+{
+ struct device *dev = rtk_phy->dev;
+ struct reg_addr *addr;
+ struct phy_data *phy_data;
+ int ret = 0;
+ int index;
+
+ if (of_property_read_u32(sub_node, "reg", &index)) {
+ dev_err(dev, "sub_node without reg\n");
+ return -EINVAL;
+ }
+
+ dev_dbg(dev, "sub_node index=%d\n", index);
+
+ addr = &((struct reg_addr *)rtk_phy->reg_addr)[index];
+ phy_data = &((struct phy_data *)rtk_phy->phy_data)[index];
+
+ addr->reg_mdio_ctl = of_iomap(dev->of_node, 0) + index;
+ dev_dbg(dev, "%s %d #%d reg_mdio_ctl=%p\n",
+ __func__, __LINE__, index, addr->reg_mdio_ctl);
+
+ if (!sub_node) {
+ dev_err(dev, "%s %d No device sub node\n", __func__, __LINE__);
+ goto err;
+ }
+
+ ret = __get_phy_parameter(dev, phy_data, sub_node);
+ if (ret)
+ goto err;
+
+ if (of_property_read_bool(sub_node, "realtek,do-toggle-once"))
+ phy_data->do_toggle_once = true;
+ else
+ phy_data->do_toggle_once = false;
+
+ if (of_property_read_bool(sub_node, "realtek,do-toggle"))
+ phy_data->do_toggle = true;
+ else
+ phy_data->do_toggle = false;
+
+ if (of_property_read_bool(sub_node, "realtek,use-default-parameter"))
+ phy_data->use_default_parameter = true;
+ else
+ phy_data->use_default_parameter = false;
+
+ if (of_property_read_bool(sub_node, "realtek,check-rx-front-end-offset"))
+ phy_data->check_rx_front_end_offset = true;
+ else
+ phy_data->check_rx_front_end_offset = false;
+
+ if (of_property_read_bool(sub_node, "realtek,check-efuse"))
+ phy_data->check_efuse = true;
+ else
+ phy_data->check_efuse = false;
+
+ __get_phy_parameter_by_efuse(rtk_phy, phy_data, index);
+
+err:
+ return ret;
+}
+
+static int rtk_usb3phy_probe(struct platform_device *pdev)
+{
+ struct rtk_usb_phy *rtk_phy;
+ struct device *dev = &pdev->dev;
+ struct device_node *node;
+ struct device_node *sub_node;
+ struct phy *generic_phy;
+ struct phy_provider *phy_provider;
+ int ret, phyN;
+
+ 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_usb_phy_notify_port_status;
+
+ if (!dev->of_node) {
+ dev_err(dev, "%s %d No device node\n", __func__, __LINE__);
+ ret = -ENODEV;
+ goto err;
+ }
+
+ node = dev->of_node;
+
+ phyN = of_get_child_count(node);
+ rtk_phy->phyN = phyN;
+ dev_dbg(dev, "%s phyN=%d\n", __func__, rtk_phy->phyN);
+
+ rtk_phy->reg_addr = devm_kzalloc(dev,
+ sizeof(struct reg_addr) * phyN, GFP_KERNEL);
+ if (!rtk_phy->reg_addr)
+ return -ENOMEM;
+
+ rtk_phy->phy_data = devm_kzalloc(dev,
+ sizeof(struct phy_data) * phyN, GFP_KERNEL);
+ if (!rtk_phy->phy_data)
+ return -ENOMEM;
+
+ for (sub_node = of_get_next_child(node, NULL); sub_node != NULL;
+ sub_node = of_get_next_child(node, sub_node)) {
+ ret = get_phy_parameter(rtk_phy, sub_node);
+ if (ret) {
+ dev_err(dev, "%s: get_phy_parameter fail ret=%d\n",
+ __func__, 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(&pdev->dev, "Probe RTK USB 3.0 PHY (ret=%d)\n", ret);
+
+ return ret;
+}
+
+static void rtk_usb3phy_remove(struct platform_device *pdev)
+{
+ struct rtk_usb_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,usb3phy", },
+ {},
+};
+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] 33+ messages in thread
* [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY
2023-06-07 6:24 [PATCH v3 1/5] usb: phy: add usb phy notify port status API Stanley Chang
2023-06-07 6:24 ` [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY Stanley Chang
2023-06-07 6:24 ` [PATCH v3 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY Stanley Chang
@ 2023-06-07 6:24 ` Stanley Chang
2023-06-07 12:04 ` Krzysztof Kozlowski
2023-06-07 6:24 ` [PATCH v3 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY Stanley Chang
3 siblings, 1 reply; 33+ messages in thread
From: Stanley Chang @ 2023-06-07 6:24 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,
Matthias Kaehlcke, Douglas Anderson, Michael Grzeschik,
Mathias Nyman, 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>
---
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 | 213 ++++++++++++++++++
1 file changed, 213 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..69911e20a561
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
@@ -0,0 +1,213 @@
+# 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,rtd1395-usb2phy
+ - realtek,rtd1619-usb2phy
+ - realtek,rtd1319-usb2phy
+ - realtek,rtd1619b-usb2phy
+ - realtek,rtd1312c-usb2phy
+ - realtek,rtd1319d-usb2phy
+ - realtek,rtd1315e-usb2phy
+ - const: realtek,usb2phy
+
+ reg:
+ items:
+ - description: PHY data registers
+ - description: PHY control registers
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ "#phy-cells":
+ const: 0
+
+ realtek,usb-ctrl:
+ description: The phandle of syscon used to control USB PHY power domain.
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+patternProperties:
+ "^phy@[0-3]+$":
+ type: object
+ description:
+ Each sub-node is a PHY device for one XHCI controller.
+ For most Relatek SoCs, one XHCI controller only support one the USB 2.0
+ phy. For RTD1395 SoC, the one XHCI controller has two USB 2.0 PHYs.
+ properties:
+ realtek,page0-param:
+ description: PHY parameter at page 0. The data are the pair of the
+ offset and value.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
+ realtek,page1-param:
+ description: PHY parameter at page 1. The data are the pair of the
+ offset and value.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
+ realtek,page2-param:
+ description: PHY parameter at page 2. The data are the pair of the
+ offset and value. If the PHY support the page 2 parameter.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+
+ realtek,support-page2-param:
+ description: Set this flag if PHY support page 2 parameter.
+ type: boolean
+
+ realtek,do-toggle:
+ description: Set this flag to enable PHY parameter toggle when port
+ status change.
+ type: boolean
+
+ realtek,do-toggle-driving:
+ description: Set this flag to enable PHY parameter toggle for adjust
+ the driving when port status change.
+ type: boolean
+
+ realtek,check-efuse:
+ description: Enable to update PHY parameter from reading otp table.
+ type: boolean
+
+ realtek,use-default-parameter:
+ description: Don't set parameter and use default value in hardware.
+ type: boolean
+
+ realtek,is-double-sensitivity-mode:
+ description: Set this flag to enable double sensitivity mode.
+ type: boolean
+
+ realtek,ldo-force-enable:
+ description: Set this flag to force enable ldo mode.
+ type: boolean
+
+ realtek,ldo-driving-compensate:
+ description: Set the value for adjust the PHY driving for ldo mode.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ realtek,driving-compensate:
+ description: Set the value for adjust the PHY driving for efuse
+ table v2.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+ - compatible
+ - reg
+ - "#address-cells"
+ - "#size-cells"
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ usb_port0_usb2phy: usb-phy@13214 {
+ compatible = "realtek,rtd1319d-usb2phy", "realtek,usb2phy";
+ reg = <0x13214 0x4>, <0x28280 0x4>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #phy-cells = <0>;
+ realtek,usb-ctrl = <&usb_ctrl>;
+
+ phy@0 {
+ reg = <0>;
+ realtek,page0-param =
+ <0xe0 0xa3>, <0xe4 0xb2>, <0xe5 0x4e>, <0xe6 0x42>;
+ realtek,page1-param = <0xe3 0x64>;
+ realtek,page2-param = <0xe7 0x45>;
+ realtek,support-page2-param;
+ realtek,do-toggle;
+ realtek.do-toggle-driving;
+ realtek,check-efuse;
+ realtek,is-double-sensitivity-mode;
+ realtek,ldo-force-enable;
+ };
+ };
+
+ - |
+ usb_port1_usb2phy: usb-phy@13c14 {
+ compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
+ reg = <0x132c4 0x4>, <0x31280 0x8>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #phy-cells = <0>;
+ realtek,usb-ctrl = <&usb_ctrl>;
+
+ phy@0 {
+ reg = <0>;
+ realtek,page0-param =
+ <0xe0 0xa3>, <0xe4 0xb2>, <0xe5 0x4e>, <0xe6 0x42>;
+ realtek,page1-param = <0xe3 0x00>;
+ realtek,do-toggle;
+ realtek,check-efuse;
+ };
+ phy@1 {
+ reg = <1>;
+ realtek,page0-param =
+ <0xe0 0xe0>, <0xe4 0xb2>, <0xe5 0x4e>, <0xe6 0x42>;
+ realtek,page1-param = <0xe3 0x00>;
+ realtek,do-toggle;
+ realtek,check-efuse;
+ };
+ };
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v3 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY
2023-06-07 6:24 [PATCH v3 1/5] usb: phy: add usb phy notify port status API Stanley Chang
` (2 preceding siblings ...)
2023-06-07 6:24 ` [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY Stanley Chang
@ 2023-06-07 6:24 ` Stanley Chang
2023-06-07 12:08 ` Krzysztof Kozlowski
3 siblings, 1 reply; 33+ messages in thread
From: Stanley Chang @ 2023-06-07 6:24 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,
Mathias Nyman, Michael Grzeschik, Matthias Kaehlcke,
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 3.0 PHY transceivers.
Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
---
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 | 156 ++++++++++++++++++
1 file changed, 156 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..b45c398bba5f
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml
@@ -0,0 +1,156 @@
+# 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
+
+ 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-usb3phy
+ - realtek,rtd1619-usb3phy
+ - realtek,rtd1319-usb3phy
+ - realtek,rtd1619b-usb3phy
+ - realtek,rtd1319d-usb3phy
+ - const: realtek,usb3phy
+
+ reg:
+ description: PHY data registers
+ maxItems: 1
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ "#phy-cells":
+ const: 0
+
+patternProperties:
+ "^phy@[0-3]+$":
+ description: Each sub-node is a PHY device for one XHCI controller.
+ type: object
+ properties:
+ realtek,param:
+ description: The data of PHY parameter are the pair of the
+ offset and value.
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+
+ realtek,do-toggle:
+ description: Set this flag to enable the PHY parameter toggle
+ when port status change.
+ type: boolean
+
+ realtek,do-toggle-once:
+ description: Set this flag to do PHY parameter toggle only on
+ PHY init.
+ type: boolean
+
+ realtek,check-efuse:
+ description: Enable to update PHY parameter from reading otp table.
+ type: boolean
+
+ realtek,use-default-parameter:
+ description: Don't set parameter and use default value in hardware.
+ type: boolean
+
+ realtek,check-rx-front-end-offset:
+ description: Enable to check rx front end offset.
+ type: boolean
+
+required:
+ - compatible
+ - reg
+ - "#address-cells"
+ - "#size-cells"
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ usb_port2_usb3phy: usb-phy@13e10 {
+ compatible = "realtek,rtd1319d-usb3phy", "realtek,usb3phy";
+ reg = <0x13e10 0x4>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #phy-cells = <0>;
+
+ phy@0 {
+ reg = <0>;
+ realtek,param =
+ <0x01 0xac8c>,
+ <0x06 0x0017>,
+ <0x09 0x724c>,
+ <0x0B 0xb90d>,
+ <0x0A 0xb610>,
+ <0x0D 0xef2a>,
+ <0x0F 0x9050>,
+ <0x10 0x000c>,
+ <0x20 0x70ff>,
+ <0x21 0xcfaa>,
+ <0x22 0x0013>,
+ <0x23 0xdb66>,
+ <0x26 0x8609>,
+ <0x29 0xff13>,
+ <0x2A 0x3070>;
+ realtek,do-toggle-once;
+ realtek,check-efuse;
+ };
+ };
+
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
2023-06-07 6:24 ` [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY Stanley Chang
@ 2023-06-07 11:26 ` kernel test robot
2023-06-07 11:54 ` Krzysztof Kozlowski
2023-06-07 17:31 ` kernel test robot
2 siblings, 0 replies; 33+ messages in thread
From: kernel test robot @ 2023-06-07 11:26 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,
Michael Grzeschik, Mathias Nyman, Bagas Sanjaya,
Matthias Kaehlcke, Ray Chi, Flavio Suligoi, linux-phy,
devicetree, linux-kernel, linux-usb
Hi Stanley,
kernel test robot noticed the following build errors:
[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus robh/for-next linus/master v6.4-rc5 next-20230607]
[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/20230607-142704
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20230607062500.24669-2-stanley_chang%40realtek.com
patch subject: [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
config: sh-randconfig-r033-20230607 (https://download.01.org/0day-ci/archive/20230607/202306071901.mhcH1Kcc-lkp@intel.com/config)
compiler: sh4-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/20230607062500.24669-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 --shuffle=2827500481 ARCH=sh olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir --shuffle=2827500481 ARCH=sh SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306071901.mhcH1Kcc-lkp@intel.com/
All errors (new ones prefixed by >>):
sh4-linux-ld: drivers/phy/realtek/phy-rtk-usb2.o: in function `rtk_usb2phy_remove':
>> drivers/phy/realtek/phy-rtk-usb2.c:1891: undefined reference to `usb_remove_phy'
sh4-linux-ld: drivers/phy/realtek/phy-rtk-usb2.o: in function `create_debug_files':
>> drivers/phy/realtek/phy-rtk-usb2.c:1378: undefined reference to `usb_debug_root'
sh4-linux-ld: drivers/phy/realtek/phy-rtk-usb2.o: in function `rtk_usb2phy_probe':
>> drivers/phy/realtek/phy-rtk-usb2.c:1882: undefined reference to `usb_add_phy_dev'
sh4-linux-ld: drivers/power/supply/wm831x_power.o: in function `wm831x_power_probe':
>> drivers/power/supply/wm831x_power.c:695: undefined reference to `devm_usb_get_phy_by_phandle'
sh4-linux-ld: drivers/power/supply/rt9455_charger.o: in function `rt9455_probe':
>> drivers/power/supply/rt9455_charger.c:1682: undefined reference to `devm_usb_get_phy'
sh4-linux-ld: drivers/power/supply/isp1704_charger.o: in function `isp1704_charger_probe':
>> drivers/power/supply/isp1704_charger.c:73: undefined reference to `devm_usb_get_phy_by_phandle'
>> sh4-linux-ld: drivers/power/supply/isp1704_charger.c:73: undefined reference to `devm_usb_get_phy'
sh4-linux-ld: drivers/power/supply/bq25890_charger.o: in function `bq25890_probe':
>> drivers/power/supply/bq25890_charger.c:1511: undefined reference to `devm_usb_get_phy'
sh4-linux-ld: drivers/power/supply/bq256xx_charger.o: in function `bq256xx_probe':
>> drivers/power/supply/bq256xx_charger.c:1743: undefined reference to `devm_usb_get_phy'
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for USB_PHY
Depends on [n]: USB_SUPPORT [=n]
Selected by [y]:
- PHY_RTK_RTD_USB2PHY [=y]
vim +1891 drivers/phy/realtek/phy-rtk-usb2.c
1802
1803 static int rtk_usb2phy_probe(struct platform_device *pdev)
1804 {
1805 struct rtk_usb_phy *rtk_phy;
1806 struct device *dev = &pdev->dev;
1807 struct device_node *node;
1808 struct device_node *sub_node;
1809 struct phy *generic_phy;
1810 struct phy_provider *phy_provider;
1811 int phyN, ret = 0;
1812
1813 rtk_phy = devm_kzalloc(dev, sizeof(*rtk_phy), GFP_KERNEL);
1814 if (!rtk_phy)
1815 return -ENOMEM;
1816
1817 rtk_phy->dev = &pdev->dev;
1818 rtk_phy->phy.dev = rtk_phy->dev;
1819 rtk_phy->phy.label = "rtk-usb2phy";
1820 rtk_phy->phy.notify_port_status = rtk_usb_phy_notify_port_status;
1821
1822 if (!dev->of_node) {
1823 dev_err(dev, "%s %d No device node\n", __func__, __LINE__);
1824 goto err;
1825 }
1826
1827 node = dev->of_node;
1828
1829 rtk_phy->usb_ctrl_regs = syscon_regmap_lookup_by_phandle(node, "realtek,usb-ctrl");
1830 if (IS_ERR(rtk_phy->usb_ctrl_regs)) {
1831 dev_info(dev, "%s: DTS no support usb_ctrl regs syscon\n", __func__);
1832 rtk_phy->usb_ctrl_regs = NULL;
1833 }
1834
1835 phyN = of_get_child_count(node);
1836 rtk_phy->phyN = phyN;
1837 dev_dbg(dev, "%s phyN=%d\n", __func__, rtk_phy->phyN);
1838
1839 rtk_phy->reg_addr = devm_kzalloc(dev,
1840 sizeof(struct reg_addr) * phyN, GFP_KERNEL);
1841 if (!rtk_phy->reg_addr)
1842 return -ENOMEM;
1843
1844 rtk_phy->phy_data = devm_kzalloc(dev,
1845 sizeof(struct phy_data) * phyN, GFP_KERNEL);
1846 if (!rtk_phy->phy_data)
1847 return -ENOMEM;
1848
1849 for (sub_node = of_get_next_child(node, NULL); sub_node != NULL;
1850 sub_node = of_get_next_child(node, sub_node)) {
1851 ret = get_phy_parameter(rtk_phy, sub_node);
1852 if (ret) {
1853 dev_err(dev, "%s: get_phy_parameter fail ret=%d\n",
1854 __func__, ret);
1855 goto err;
1856 }
1857 }
1858
1859 platform_set_drvdata(pdev, rtk_phy);
1860
1861 generic_phy = devm_phy_create(rtk_phy->dev, NULL, &ops);
1862 if (IS_ERR(generic_phy))
1863 return PTR_ERR(generic_phy);
1864
1865 phy_set_drvdata(generic_phy, rtk_phy);
1866
1867 phy_provider = devm_of_phy_provider_register(rtk_phy->dev,
1868 of_phy_simple_xlate);
1869 if (IS_ERR(phy_provider))
1870 return PTR_ERR(phy_provider);
1871
1872 ret = usb_add_phy_dev(&rtk_phy->phy);
1873 if (ret)
1874 goto err;
1875
1876 create_debug_files(rtk_phy);
1877
1878 err:
1879 dev_dbg(dev, "Probe RTK USB 2.0 PHY (ret=%d)\n", ret);
1880
1881 return ret;
> 1882 }
1883
1884 static void rtk_usb2phy_remove(struct platform_device *pdev)
1885 {
1886 struct rtk_usb_phy *rtk_phy = platform_get_drvdata(pdev);
1887
1888 remove_debug_files(rtk_phy);
1889
1890 usb_remove_phy(&rtk_phy->phy);
> 1891 }
1892
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY
2023-06-07 6:24 ` [PATCH v3 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY Stanley Chang
@ 2023-06-07 11:52 ` Krzysztof Kozlowski
2023-06-08 6:59 ` Stanley Chang[昌育德]
2023-06-08 1:35 ` kernel test robot
1 sibling, 1 reply; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-07 11:52 UTC (permalink / raw)
To: Stanley Chang, Greg Kroah-Hartman
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Michael Grzeschik, Flavio Suligoi, Matthias Kaehlcke,
Mathias Nyman, linux-phy, devicetree, linux-kernel, linux-usb
On 07/06/2023 08:24, 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>
> ---
> +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_usb_phy3_wait_vbusy(struct reg_addr *regAddr)
> +{
> + return utmi_wait_register(regAddr->reg_mdio_ctl, USB_MDIO_CTRL_PHY_BUSY, 0);
> +}
> +
> +static u16 rtk_usb_phy_read(struct reg_addr *regAddr, char addr)
> +{
> + unsigned int regVal;
> + u32 value;
> +
> + regVal = (addr << USB_MDIO_CTRL_PHY_ADDR_SHIFT);
> +
> + writel(regVal, regAddr->reg_mdio_ctl);
> +
> + rtk_usb_phy3_wait_vbusy(regAddr);
> +
> + value = readl(regAddr->reg_mdio_ctl);
> + value = value >> USB_MDIO_CTRL_PHY_DATA_SHIFT;
> +
> + return (u16)value;
> +}
> +
> +static int rtk_usb_phy_write(struct reg_addr *regAddr, char addr, u16 data)
> +{
> + unsigned int regVal;
> +
> + regVal = USB_MDIO_CTRL_PHY_WRITE |
> + (addr << USB_MDIO_CTRL_PHY_ADDR_SHIFT) |
> + (data << USB_MDIO_CTRL_PHY_DATA_SHIFT);
> +
> + writel(regVal, regAddr->reg_mdio_ctl);
> +
> + rtk_usb_phy3_wait_vbusy(regAddr);
> +
> + return 0;
> +}
> +
> +static void do_rtk_usb3_phy_toggle(struct rtk_usb_phy *rtk_phy, int i,
> + bool isConnect)
> +{
> + struct reg_addr *regAddr;
> + struct phy_data *phy_data;
> + struct phy_parameter *phy_parameter;
> + size_t index;
> +
> + regAddr = &((struct reg_addr *)rtk_phy->reg_addr)[i];
> + phy_data = &((struct phy_data *)rtk_phy->phy_data)[i];
> +
> + if (!phy_data) {
> + dev_err(rtk_phy->dev, "%s phy_data is NULL!\n", __func__);
???
> + return;
> + }
> +
> + if (!phy_data->do_toggle)
> + return;
> +
> + phy_parameter = phy_data->parameter;
> +
> + index = PHY_ADDR_MAP_ARRAY_INDEX(PHY_ADDR_0x09);
> +
> + if (index < phy_data->size) {
> + u8 addr = (phy_parameter + index)->addr;
> + u16 data = (phy_parameter + index)->data;
> +
> + if (addr == 0xFF) {
> + addr = ARRAY_INDEX_MAP_PHY_ADDR(index);
> + data = rtk_usb_phy_read(regAddr, addr);
> + (phy_parameter + index)->addr = addr;
> + (phy_parameter + index)->data = data;
> + }
> + mdelay(1);
> + dev_info(rtk_phy->dev,
> + "%s ########## to toggle PHY addr 0x09 BIT(9)\n",
> + __func__);
> + rtk_usb_phy_write(regAddr, addr, data&(~BIT(9)));
> + mdelay(1);
> + rtk_usb_phy_write(regAddr, addr, data);
> + }
> + dev_info(rtk_phy->dev, "%s ########## PHY addr 0x1f = 0x%04x\n",
> + __func__, rtk_usb_phy_read(regAddr, PHY_ADDR_0x1F));
Please drop all simple debug success messages. Linux has already
infrastructure for this.
...
> + return 0;
> +}
> +
> +static int rtk_usb_phy_init(struct phy *phy)
> +{
> + struct rtk_usb_phy *rtk_phy = phy_get_drvdata(phy);
> + int ret = 0;
> + int i;
> + unsigned long phy_init_time = jiffies;
> +
> + if (!rtk_phy) {
> + pr_err("%s rtk_phy is NULL!\n", __func__);
What? How is this possible?
> + return -ENODEV;
> + }
> +
> + dev_dbg(rtk_phy->dev, "Init RTK USB 3.0 PHY\n");
> + for (i = 0; i < rtk_phy->phyN; i++)
> + ret = do_rtk_usb_phy_init(rtk_phy, i);
> +
> + dev_info(rtk_phy->dev, "Initialized RTK USB 3.0 PHY (take %dms)\n",
> + jiffies_to_msecs(jiffies - phy_init_time));
Please drop all simple debug success messages. Linux has already
infrastructure for this.
> + return ret;
> +}
> +
> +static int rtk_usb_phy_exit(struct phy *phy)
> +{
> + struct rtk_usb_phy *rtk_phy = phy_get_drvdata(phy);
> +
> + if (!rtk_phy) {
> + pr_err("%s rtk_phy is NULL!\n", __func__);
> + return -ENODEV;
> + }
> +
> + dev_dbg(rtk_phy->dev, "Exit RTK USB 3.0 PHY\n");
Please drop all simple debug success messages. Linux has already
infrastructure for this.
> +static void rtk_usb_phy_toggle(struct usb_phy *usb3_phy, bool isConnect, int port)
> +{
> + int index = port;
> + struct rtk_usb_phy *rtk_phy = NULL;
> +
> + if (usb3_phy != NULL && usb3_phy->dev != NULL)
> + rtk_phy = dev_get_drvdata(usb3_phy->dev);
> +
> + if (rtk_phy == NULL) {
> + pr_err("%s ERROR! NO this device\n", __func__);
Your error messages are not helping. No need to shout, no need to handle
some non-existing cases. If this is real case, you have broken driver. I
actually suspect that.
How can you interface with a driver where there is no device?
> + return;
> + }
> +
> + if (index > rtk_phy->phyN) {
> + pr_err("%s %d ERROR! port=%d > phyN=%d\n",
> + __func__, __LINE__, index, rtk_phy->phyN);
> + return;
> + }
> +
> + do_rtk_usb3_phy_toggle(rtk_phy, index, isConnect);
> +}
> +
> +static int rtk_usb_phy_notify_port_status(struct usb_phy *x, int port,
> + u16 portstatus, u16 portchange)
> +{
> + bool isConnect = false;
This is not C++. Don't use camelcase. See Coding style document.
> +
> + pr_debug("%s port=%d portstatus=0x%x portchange=0x%x\n",
> + __func__, port, (int)portstatus, (int)portchange);
> + if (portstatus & USB_PORT_STAT_CONNECTION)
> + isConnect = true;
> +
...
> +
> +static int rtk_usb3_set_parameter_show(struct seq_file *s, void *unused)
> +{
> + struct rtk_usb_phy *rtk_phy = s->private;
> + 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;
> + int ret, index;
> + struct phy_data *phy_data = NULL;
> +
> + for (index = 0; index < rtk_phy->phyN; 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_data = &((struct phy_data *)rtk_phy->phy_data)[index];
> + break;
> + }
> + }
> + if (!phy_data) {
> + dev_err(rtk_phy->dev,
> + "%s: No phy_data for %s/%s\n",
> + __func__, phy_dir_name, file_name);
Mess wrapping/indentation. Actually everywhere in the file...
> + return -EINVAL;
> + }
> +
> + ret = __get_parameter_at_page(s, rtk_phy, phy_data->parameter, 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_usb_phy *rtk_phy = s->private;
> + struct reg_addr *regAddr = NULL;
> + struct phy_data *phy_data = 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->phyN; index++) {
> + size_t sz = 30;
> + char name[30] = {0};
> +
> + snprintf(name, sz, "phy%d", index);
> + if (strncmp(name, phy_dir_name, strlen(name)) == 0) {
> + regAddr = &((struct reg_addr *)rtk_phy->reg_addr)[index];
> + phy_data = &((struct phy_data *)rtk_phy->phy_data)[index];
> + break;
Where is the ABI documentation for user interface?
> +
> +static inline void create_debug_files(struct rtk_usb_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) {
> + dev_err(rtk_phy->dev, "%s Error phy_debug_root is NULL",
> + __func__);
> + return;
> + }
> + rtk_phy->debug_dir = debugfs_create_dir(dev_name(rtk_phy->dev),
> + phy_debug_root);
> + if (!rtk_phy->debug_dir) {
> + dev_err(rtk_phy->dev, "%s Error debug_dir is NULL", __func__);
Are you sure you run checkpatch on this? Error messages on debugfs are
almost always incorrect.
> + 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) {
> + int index, ret;
> +
> + for (index = 0; index < rtk_phy->phyN; index++) {
> + struct dentry *phy_dir;
> + struct phy_data *phy_data;
> + size_t sz = 30;
> + char name[30] = {0};
> +
> + snprintf(name, sz, "phy%d", index);
> +
> + phy_data = &((struct phy_data *)rtk_phy->phy_data)[index];
> +
> + phy_dir = debugfs_create_dir(name, set_parameter_dir);
> + if (!phy_dir) {
> + dev_err(rtk_phy->dev,
> + "%s Error create folder %s fail\n",
> + name, __func__);
> + goto file_error;
> + }
> +
> + ret = create_debug_set_parameter_files(rtk_phy, phy_dir,
> + phy_data->size);
> + if (ret < 0) {
> + dev_err(rtk_phy->dev,
> + "%s Error create files fail\n",
> + __func__);
> + goto file_error;
> + }
> + }
> + }
> +
> + if (!debugfs_create_file("toggle", 0644, rtk_phy->debug_dir, rtk_phy,
> + &rtk_usb3_toggle_fops))
> + goto file_error;
> +
> + return;
> +
> +file_error:
> + debugfs_remove_recursive(rtk_phy->debug_dir);
> +}
> +
> +static inline void remove_debug_files(struct rtk_usb_phy *rtk_phy)
> +{
> + debugfs_remove_recursive(rtk_phy->debug_dir);
> +}
> +#else
> +static inline void create_debug_files(struct rtk_usb_phy *rtk_phy) { }
> +static inline void remove_debug_files(struct rtk_usb_phy *rtk_phy) { }
> +#endif /* CONFIG_DEBUG_FS */
> +
> +static int __get_phy_parameter_by_efuse(struct rtk_usb_phy *rtk_phy,
> + struct phy_data *phy_data, int index)
> +{
> + u8 value = 0;
> + struct nvmem_cell *cell;
> +
> + if (!phy_data->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 failed to get usb_u3_tx_lfps_swing_trim: %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;
> +
> + dev_dbg(rtk_phy->dev,
> + "phy index=%d buf=0x%x buf_size=%d value=0x%x\n",
> + index, buf[0], (int)buf_size, value);
> + kfree(buf);
> + nvmem_cell_put(cell);
> + }
> +
> + if ((value > 0) && (value < 0x8))
> + phy_data->efuse_usb_u3_tx_lfps_swing_trim = 0x8;
> + else
> + phy_data->efuse_usb_u3_tx_lfps_swing_trim = (u8)value;
> +
> + dev_dbg(rtk_phy->dev, "Get Efuse usb_u3_tx_lfps_swing_trim=0x%x (value=0x%x)\n",
> + phy_data->efuse_usb_u3_tx_lfps_swing_trim, value);
> +
> +out:
> + return 0;
> +}
> +
> +static int __get_phy_parameter(struct device *dev, struct phy_data *phy_data,
> + struct device_node *sub_node)
> +{
> + struct phy_parameter *phy_parameter;
> + int i, ret = 0;
> + int data_size, num_cells = 2;
> +
> + phy_data->size = MAX_USB_PHY_DATA_SIZE;
> + phy_data->parameter = devm_kzalloc(dev,
> + sizeof(struct phy_parameter) * phy_data->size, GFP_KERNEL);
> + if (!phy_data->parameter)
> + return -ENOMEM;
> +
> + if (!of_get_property(sub_node, "realtek,param", &data_size)) {
> + dev_dbg(dev, "%s No parameter (data_size=%d)\n",
> + __func__, data_size);
> + data_size = 0;
> + }
> +
> + if (!data_size)
> + goto out;
> +
> + phy_parameter = phy_data->parameter;
> + /* Set default addr to 0xff for no data case */
> + for (i = 0; i < phy_data->size; i++)
> + (phy_parameter + i)->addr = 0xFF;
> +
> + data_size = data_size / (sizeof(u32) * num_cells);
> + for (i = 0; i < data_size; i++) {
> + struct phy_parameter *parameter;
> + u32 addr, data;
> + int offset, index;
> +
> + offset = i * num_cells;
> +
> + ret = of_property_read_u32_index(sub_node, "realtek,param",
> + offset, &addr);
> + if (ret) {
> + dev_err(dev, "ERROR: To get param i=%d addr=0x%x\n",
> + i, addr);
> + break;
> + }
> +
> + ret = of_property_read_u32_index(sub_node, "realtek,param",
> + offset + 1, &data);
> + if (ret) {
> + dev_err(dev, "ERROR: To get param i=%d addr=0x%x\n",
> + i, data);
> + break;
> + }
> +
> + index = PHY_ADDR_MAP_ARRAY_INDEX(addr);
> + parameter = (phy_parameter + index);
> + parameter->addr = (u8)addr;
> + parameter->data = (u16)data;
> +
> + dev_dbg(dev, "param index=%d addr=0x%x data=0x%x\n", index,
> + parameter->addr, parameter->data);
> + }
> +
> +out:
> + return ret;
> +}
> +
> +static int get_phy_parameter(struct rtk_usb_phy *rtk_phy,
> + struct device_node *sub_node)
> +{
> + struct device *dev = rtk_phy->dev;
> + struct reg_addr *addr;
> + struct phy_data *phy_data;
> + int ret = 0;
> + int index;
> +
> + if (of_property_read_u32(sub_node, "reg", &index)) {
> + dev_err(dev, "sub_node without reg\n");
> + return -EINVAL;
> + }
> +
> + dev_dbg(dev, "sub_node index=%d\n", index);
Please drop all simple debug success messages. Linux has already
infrastructure for this.
...
> +
> +static int rtk_usb3phy_probe(struct platform_device *pdev)
> +{
> + struct rtk_usb_phy *rtk_phy;
> + struct device *dev = &pdev->dev;
> + struct device_node *node;
> + struct device_node *sub_node;
> + struct phy *generic_phy;
> + struct phy_provider *phy_provider;
> + int ret, phyN;
> +
> + 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_usb_phy_notify_port_status;
> +
> + if (!dev->of_node) {
> + dev_err(dev, "%s %d No device node\n", __func__, __LINE__);
How is it even possible? If you do not have device node here, how did it
probe?
> + ret = -ENODEV;
> + goto err;
> + }
> +
> + node = dev->of_node;
> +
> + phyN = of_get_child_count(node);
> + rtk_phy->phyN = phyN;
> + dev_dbg(dev, "%s phyN=%d\n", __func__, rtk_phy->phyN);
Please drop all simple debug success messages. Linux has already
infrastructure for this.
> +
> + rtk_phy->reg_addr = devm_kzalloc(dev,
> + sizeof(struct reg_addr) * phyN, GFP_KERNEL);
> + if (!rtk_phy->reg_addr)
> + return -ENOMEM;
> +
> + rtk_phy->phy_data = devm_kzalloc(dev,
> + sizeof(struct phy_data) * phyN, GFP_KERNEL);
> + if (!rtk_phy->phy_data)
> + return -ENOMEM;
> +
> + for (sub_node = of_get_next_child(node, NULL); sub_node != NULL;
> + sub_node = of_get_next_child(node, sub_node)) {
> + ret = get_phy_parameter(rtk_phy, sub_node);
> + if (ret) {
> + dev_err(dev, "%s: get_phy_parameter fail ret=%d\n",
> + __func__, 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(&pdev->dev, "Probe RTK USB 3.0 PHY (ret=%d)\n", ret);
Please drop all simple debug success messages. Linux has already
infrastructure for this.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
2023-06-07 6:24 ` [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY Stanley Chang
2023-06-07 11:26 ` kernel test robot
@ 2023-06-07 11:54 ` Krzysztof Kozlowski
2023-06-08 6:59 ` Stanley Chang[昌育德]
2023-06-07 17:31 ` kernel test robot
2 siblings, 1 reply; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-07 11:54 UTC (permalink / raw)
To: Stanley Chang, Greg Kroah-Hartman
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Michael Grzeschik,
Mathias Nyman, Bagas Sanjaya, Matthias Kaehlcke, Ray Chi,
Flavio Suligoi, linux-phy, devicetree, linux-kernel, linux-usb
On 07/06/2023 08:24, 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>
> ---
> 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.
...
> + 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);
I commented on your second patch, but everything is applicable here as
well. You have many useless debug messages. Many incorrect or useless
"if() return" which point to broken driver design (e.g. concurrent
access to half initialized structures where you substitute lack of
synchronization with incorrect "if() return"). Undocumented user
interface is one more big trouble.
I doubt you run checkpatch on this (be sure to run it with --strict and
fix almost everything).
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY
2023-06-07 6:24 ` [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY Stanley Chang
@ 2023-06-07 12:04 ` Krzysztof Kozlowski
2023-06-08 7:24 ` Stanley Chang[昌育德]
2023-06-08 7:47 ` Stanley Chang[昌育德]
0 siblings, 2 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-07 12:04 UTC (permalink / raw)
To: Stanley Chang, Greg Kroah-Hartman
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Matthias Kaehlcke, Douglas Anderson, Michael Grzeschik,
Mathias Nyman, Flavio Suligoi, linux-phy, devicetree,
linux-kernel, linux-usb
On 07/06/2023 08:24, 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>
> ---
> 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 | 213 ++++++++++++++++++
> 1 file changed, 213 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..69911e20a561
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
> @@ -0,0 +1,213 @@
> +# 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,rtd1395-usb2phy
> + - realtek,rtd1619-usb2phy
> + - realtek,rtd1319-usb2phy
> + - realtek,rtd1619b-usb2phy
> + - realtek,rtd1312c-usb2phy
> + - realtek,rtd1319d-usb2phy
> + - realtek,rtd1315e-usb2phy
Keep entries ordered alphabetically.
> + - const: realtek,usb2phy
> +
> + reg:
> + items:
> + - description: PHY data registers
> + - description: PHY control registers
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + "#phy-cells":
> + const: 0
> +
> + realtek,usb-ctrl:
> + description: The phandle of syscon used to control USB PHY power domain.
> + $ref: /schemas/types.yaml#/definitions/phandle
No, we have power-domains for this.
> +
> +patternProperties:
> + "^phy@[0-3]+$":
> + type: object
> + description:
> + Each sub-node is a PHY device for one XHCI controller.
I don't think it is true. You claim above that you have 0 as phy-cells,
means you have one phy. Here you say you can have up to 4 phys.
> + For most Relatek SoCs, one XHCI controller only support one the USB 2.0
> + phy. For RTD1395 SoC, the one XHCI controller has two USB 2.0 PHYs.
> + properties:
> + realtek,page0-param:
> + description: PHY parameter at page 0. The data are the pair of the
> + offset and value.
This needs to be specific. What the heck is "PHY parameter"?
> + $ref: /schemas/types.yaml#/definitions/uint32-array
Array? Then maxItems.
> +
> + realtek,page1-param:
> + description: PHY parameter at page 1. The data are the pair of the
> + offset and value.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> + realtek,page2-param:
> + description: PHY parameter at page 2. The data are the pair of the
> + offset and value. If the PHY support the page 2 parameter.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> + realtek,support-page2-param:
> + description: Set this flag if PHY support page 2 parameter.
Why this cannot be deducted from compatible?
> + type: boolean
> +
> + realtek,do-toggle:
> + description: Set this flag to enable PHY parameter toggle when port
> + status change.
Do not instruct OS what to do. Explain why this is a hardware
characteristic.
> + type: boolean
> +
> + realtek,do-toggle-driving:
> + description: Set this flag to enable PHY parameter toggle for adjust
> + the driving when port status change.
Do not instruct OS what to do. Explain why this is a hardware
characteristic.
> + type: boolean
> +
> + realtek,check-efuse:
> + description: Enable to update PHY parameter from reading otp table.
Do not instruct OS what to do. Explain why this is a hardware
characteristic.
> + type: boolean
> +
> + realtek,use-default-parameter:
> + description: Don't set parameter and use default value in hardware.
NAK, you are just making things up.
> + type: boolean
> +
> + realtek,is-double-sensitivity-mode:
> + description: Set this flag to enable double sensitivity mode.
All your descriptions copy the name of property. You basically say
nothing more. I already mentioned this before. Don't ignore the
feedback, but address it.
> + type: boolean
> +
> + realtek,ldo-force-enable:
> + description: Set this flag to force enable ldo mode.
Drop everywhere "Set this flag to", because it is redundant. Now compare
what is left with property name.
Property name: realtek,ldo-force-enable
Your description: "force enable ldo mode"
How is this helpful to anybody?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY
2023-06-07 6:24 ` [PATCH v3 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY Stanley Chang
@ 2023-06-07 12:08 ` Krzysztof Kozlowski
2023-06-08 7:32 ` Stanley Chang[昌育德]
0 siblings, 1 reply; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-07 12:08 UTC (permalink / raw)
To: Stanley Chang, Greg Kroah-Hartman
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Mathias Nyman, Michael Grzeschik, Matthias Kaehlcke,
Flavio Suligoi, linux-phy, devicetree, linux-kernel, linux-usb
On 07/06/2023 08:24, 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>
> ---
> 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 | 156 ++++++++++++++++++
> 1 file changed, 156 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..b45c398bba5f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/realtek,usb3phy.yaml
> @@ -0,0 +1,156 @@
> +# 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
> +
> + 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
Skip unrelated devices.
> +
> + 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-usb3phy
> + - realtek,rtd1619-usb3phy
> + - realtek,rtd1319-usb3phy
> + - realtek,rtd1619b-usb3phy
> + - realtek,rtd1319d-usb3phy
Same comments as for previous patch.
> + - const: realtek,usb3phy
> +
> + reg:
> + description: PHY data registers
> + maxItems: 1
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + "#phy-cells":
> + const: 0
1 phy or 4? Decide.
> +
> +patternProperties:
> + "^phy@[0-3]+$":
> + description: Each sub-node is a PHY device for one XHCI controller.
> + type: object
> + properties:
> + realtek,param:
> + description: The data of PHY parameter are the pair of the
> + offset and value.
> + $ref: /schemas/types.yaml#/definitions/uint8-array
Your choice of types is surprising. If this is array, than maxItems (and
please don't tell me it is maxItems: 1). Anyway, why 8 bits long?
> +
> + realtek,do-toggle:
> + description: Set this flag to enable the PHY parameter toggle
> + when port status change.
> + type: boolean
> +
> + realtek,do-toggle-once:
> + description: Set this flag to do PHY parameter toggle only on
> + PHY init.
> + type: boolean
> +
> + realtek,check-efuse:
> + description: Enable to update PHY parameter from reading otp table.
> + type: boolean
> +
> + realtek,use-default-parameter:
> + description: Don't set parameter and use default value in hardware.
> + type: boolean
> +
> + realtek,check-rx-front-end-offset:
> + description: Enable to check rx front end offset.
> + type: boolean
> +
> +required:
> + - compatible
> + - reg
> + - "#address-cells"
> + - "#size-cells"
> + - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + usb_port2_usb3phy: usb-phy@13e10 {
> + compatible = "realtek,rtd1319d-usb3phy", "realtek,usb3phy";
> + reg = <0x13e10 0x4>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + #phy-cells = <0>;
> +
> + phy@0 {
> + reg = <0>;
> + realtek,param =
> + <0x01 0xac8c>,
> + <0x06 0x0017>,
First, this is matrix, not uint8 array. Second, 0xac8c is past 16 bits
long, not 8. Third, you put some magic register programming to DT.
Please don't. Drop all this from DT.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
2023-06-07 6:24 ` [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY Stanley Chang
2023-06-07 11:26 ` kernel test robot
2023-06-07 11:54 ` Krzysztof Kozlowski
@ 2023-06-07 17:31 ` kernel test robot
2023-06-08 2:18 ` Stanley Chang[昌育德]
2 siblings, 1 reply; 33+ messages in thread
From: kernel test robot @ 2023-06-07 17:31 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,
Michael Grzeschik, Mathias Nyman, Bagas Sanjaya,
Matthias Kaehlcke, Ray Chi, Flavio Suligoi, linux-phy,
devicetree, linux-kernel, linux-usb
Hi Stanley,
kernel test robot noticed the following build errors:
[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus robh/for-next linus/master v6.4-rc5 next-20230607]
[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/20230607-142704
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20230607062500.24669-2-stanley_chang%40realtek.com
patch subject: [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
config: arm64-randconfig-r014-20230607 (https://download.01.org/0day-ci/archive/20230608/202306080128.Gh3c2H1O-lkp@intel.com/config)
compiler: aarch64-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/20230607062500.24669-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=arm64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash
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/202306080128.Gh3c2H1O-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
ERROR: modpost: "devm_usb_get_phy_by_phandle" [drivers/power/supply/wm831x_power.ko] undefined!
>> ERROR: modpost: "devm_usb_get_phy" [drivers/power/supply/da9150-charger.ko] undefined!
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for USB_PHY
Depends on [n]: USB_SUPPORT [=n]
Selected by [y]:
- PHY_RTK_RTD_USB2PHY [=y]
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY
2023-06-07 6:24 ` [PATCH v3 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY Stanley Chang
2023-06-07 11:52 ` Krzysztof Kozlowski
@ 2023-06-08 1:35 ` kernel test robot
2023-06-08 2:18 ` Stanley Chang[昌育德]
1 sibling, 1 reply; 33+ messages in thread
From: kernel test robot @ 2023-06-08 1:35 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,
Ray Chi, Michael Grzeschik, Flavio Suligoi, Matthias Kaehlcke,
Mathias Nyman, linux-phy, devicetree, linux-kernel, linux-usb
Hi Stanley,
kernel test robot noticed the following build errors:
[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus robh/for-next linus/master v6.4-rc5 next-20230607]
[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/20230607-142704
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20230607062500.24669-3-stanley_chang%40realtek.com
patch subject: [PATCH v3 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20230608/202306080940.5Lcjwfck-lkp@intel.com/config)
compiler: s390-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/20230607062500.24669-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=s390 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash
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/202306080940.5Lcjwfck-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
ERROR: modpost: "usb_remove_phy" [drivers/phy/realtek/phy-rtk-usb2.ko] undefined!
ERROR: modpost: "usb_debug_root" [drivers/phy/realtek/phy-rtk-usb2.ko] undefined!
ERROR: modpost: "of_iomap" [drivers/phy/realtek/phy-rtk-usb2.ko] undefined!
ERROR: modpost: "usb_add_phy_dev" [drivers/phy/realtek/phy-rtk-usb2.ko] undefined!
>> ERROR: modpost: "usb_remove_phy" [drivers/phy/realtek/phy-rtk-usb3.ko] undefined!
>> ERROR: modpost: "usb_debug_root" [drivers/phy/realtek/phy-rtk-usb3.ko] undefined!
>> ERROR: modpost: "of_iomap" [drivers/phy/realtek/phy-rtk-usb3.ko] undefined!
>> ERROR: modpost: "usb_add_phy_dev" [drivers/phy/realtek/phy-rtk-usb3.ko] undefined!
ERROR: modpost: "devm_ioremap_resource" [drivers/dma/qcom/hdma.ko] undefined!
ERROR: modpost: "devm_platform_ioremap_resource" [drivers/dma/fsl-edma.ko] undefined!
WARNING: modpost: suppressed 26 unresolved symbol warnings because there were too many)
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for USB_PHY
Depends on [n]: USB_SUPPORT [=n]
Selected by [m]:
- PHY_RTK_RTD_USB2PHY [=m]
- PHY_RTK_RTD_USB3PHY [=m]
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
2023-06-07 17:31 ` kernel test robot
@ 2023-06-08 2:18 ` Stanley Chang[昌育德]
2023-06-08 6:55 ` Krzysztof Kozlowski
0 siblings, 1 reply; 33+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-08 2:18 UTC (permalink / raw)
To: kernel test robot, Greg Kroah-Hartman
Cc: oe-kbuild-all, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Michael Grzeschik,
Mathias Nyman, Bagas Sanjaya, Matthias Kaehlcke, Ray Chi,
Flavio Suligoi, linux-phy, devicetree, linux-kernel, linux-usb
> ERROR: modpost: "devm_usb_get_phy_by_phandle"
> [drivers/power/supply/wm831x_power.ko] undefined!
> >> ERROR: modpost: "devm_usb_get_phy"
> [drivers/power/supply/da9150-charger.ko] undefined!
>
> Kconfig warnings: (for reference only)
> WARNING: unmet direct dependencies detected for USB_PHY
> Depends on [n]: USB_SUPPORT [=n]
> Selected by [y]:
> - PHY_RTK_RTD_USB2PHY [=y]
>
I will add USB_SUUPRT dependency to Kconfig.
diff --git a/drivers/phy/realtek/Kconfig b/drivers/phy/realtek/Kconfig
index 28ee3d9be568..a5a5a71edc9c 100644
--- a/drivers/phy/realtek/Kconfig
+++ b/drivers/phy/realtek/Kconfig
@@ -4,6 +4,7 @@
#
config PHY_RTK_RTD_USB2PHY
tristate "Realtek RTD USB2 PHY Transceiver Driver"
+ depends on USB_SUPPORT
select GENERIC_PHY
select USB_PHY
help
@@ -14,6 +15,7 @@ config PHY_RTK_RTD_USB2PHY
config PHY_RTK_RTD_USB3PHY
tristate "Realtek RTD USB3 PHY Transceiver Driver"
+ depends on USB_SUPPORT
select GENERIC_PHY
select USB_PHY
help
^ permalink raw reply related [flat|nested] 33+ messages in thread
* RE: [PATCH v3 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY
2023-06-08 1:35 ` kernel test robot
@ 2023-06-08 2:18 ` Stanley Chang[昌育德]
0 siblings, 0 replies; 33+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-08 2:18 UTC (permalink / raw)
To: kernel test robot, Greg Kroah-Hartman
Cc: oe-kbuild-all, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Michael Grzeschik, Flavio Suligoi, Matthias Kaehlcke,
Mathias Nyman, linux-phy, devicetree, linux-kernel, linux-usb
> All errors (new ones prefixed by >>, old ones prefixed by <<):
>
> ERROR: modpost: "usb_remove_phy" [drivers/phy/realtek/phy-rtk-usb2.ko]
> undefined!
> ERROR: modpost: "usb_debug_root" [drivers/phy/realtek/phy-rtk-usb2.ko]
> undefined!
> ERROR: modpost: "of_iomap" [drivers/phy/realtek/phy-rtk-usb2.ko] undefined!
> ERROR: modpost: "usb_add_phy_dev" [drivers/phy/realtek/phy-rtk-usb2.ko]
> undefined!
> >> ERROR: modpost: "usb_remove_phy" [drivers/phy/realtek/phy-rtk-usb3.ko]
> undefined!
> >> ERROR: modpost: "usb_debug_root" [drivers/phy/realtek/phy-rtk-usb3.ko]
> undefined!
> >> ERROR: modpost: "of_iomap" [drivers/phy/realtek/phy-rtk-usb3.ko]
> undefined!
> >> ERROR: modpost: "usb_add_phy_dev" [drivers/phy/realtek/phy-rtk-usb3.ko]
> undefined!
> ERROR: modpost: "devm_ioremap_resource" [drivers/dma/qcom/hdma.ko]
> undefined!
> ERROR: modpost: "devm_platform_ioremap_resource"
> [drivers/dma/fsl-edma.ko] undefined!
> WARNING: modpost: suppressed 26 unresolved symbol warnings because there
> were too many)
>
> Kconfig warnings: (for reference only)
> WARNING: unmet direct dependencies detected for USB_PHY
> Depends on [n]: USB_SUPPORT [=n]
> Selected by [m]:
> - PHY_RTK_RTD_USB2PHY [=m]
> - PHY_RTK_RTD_USB3PHY [=m]
>
I will add USB_SUUPRT dependency to Kconfig.
diff --git a/drivers/phy/realtek/Kconfig b/drivers/phy/realtek/Kconfig
index 28ee3d9be568..a5a5a71edc9c 100644
--- a/drivers/phy/realtek/Kconfig
+++ b/drivers/phy/realtek/Kconfig
@@ -4,6 +4,7 @@
#
config PHY_RTK_RTD_USB2PHY
tristate "Realtek RTD USB2 PHY Transceiver Driver"
+ depends on USB_SUPPORT
select GENERIC_PHY
select USB_PHY
help
@@ -14,6 +15,7 @@ config PHY_RTK_RTD_USB2PHY
config PHY_RTK_RTD_USB3PHY
tristate "Realtek RTD USB3 PHY Transceiver Driver"
+ depends on USB_SUPPORT
select GENERIC_PHY
select USB_PHY
help
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
2023-06-08 2:18 ` Stanley Chang[昌育德]
@ 2023-06-08 6:55 ` Krzysztof Kozlowski
2023-06-08 7:07 ` Stanley Chang[昌育德]
0 siblings, 1 reply; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-08 6:55 UTC (permalink / raw)
To: Stanley Chang[昌育德],
kernel test robot, Greg Kroah-Hartman
Cc: oe-kbuild-all, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Conor Dooley, Alan Stern, Michael Grzeschik, Mathias Nyman,
Bagas Sanjaya, Matthias Kaehlcke, Ray Chi, Flavio Suligoi,
linux-phy, devicetree, linux-kernel, linux-usb
On 08/06/2023 04:18, Stanley Chang[昌育德] wrote:
>> ERROR: modpost: "devm_usb_get_phy_by_phandle"
>> [drivers/power/supply/wm831x_power.ko] undefined!
>>>> ERROR: modpost: "devm_usb_get_phy"
>> [drivers/power/supply/da9150-charger.ko] undefined!
>>
>> Kconfig warnings: (for reference only)
>> WARNING: unmet direct dependencies detected for USB_PHY
>> Depends on [n]: USB_SUPPORT [=n]
>> Selected by [y]:
>> - PHY_RTK_RTD_USB2PHY [=y]
>>
>
> I will add USB_SUUPRT dependency to Kconfig.
Why? Do you see other phy drivers needing it? Few have it but many
don't, so you should really investigate the root cause, not just add
some dependencies.
Build test your patches locally before sending and investigate the issues.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY
2023-06-07 11:52 ` Krzysztof Kozlowski
@ 2023-06-08 6:59 ` Stanley Chang[昌育德]
2023-06-08 7:21 ` Krzysztof Kozlowski
0 siblings, 1 reply; 33+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-08 6:59 UTC (permalink / raw)
To: Krzysztof Kozlowski, Greg Kroah-Hartman
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Michael Grzeschik, Flavio Suligoi, Matthias Kaehlcke,
Mathias Nyman, linux-phy, devicetree, linux-kernel, linux-usb
Hi Krzysztof,
> > +static void do_rtk_usb3_phy_toggle(struct rtk_usb_phy *rtk_phy, int i,
> > + bool isConnect)
> > +{
> > + struct reg_addr *regAddr;
> > + struct phy_data *phy_data;
> > + struct phy_parameter *phy_parameter;
> > + size_t index;
> > +
> > + regAddr = &((struct reg_addr *)rtk_phy->reg_addr)[i];
> > + phy_data = &((struct phy_data *)rtk_phy->phy_data)[i];
> > +
> > + if (!phy_data) {
> > + dev_err(rtk_phy->dev, "%s phy_data is NULL!\n",
> > + __func__);
>
> ???
Sorry, this check is redundant.
>
> > + return;
> > + }
> > +
> > + if (!phy_data->do_toggle)
> > + return;
> > +
> > + phy_parameter = phy_data->parameter;
> > +
> > + index = PHY_ADDR_MAP_ARRAY_INDEX(PHY_ADDR_0x09);
> > +
> > + if (index < phy_data->size) {
> > + u8 addr = (phy_parameter + index)->addr;
> > + u16 data = (phy_parameter + index)->data;
> > +
> > + if (addr == 0xFF) {
> > + addr = ARRAY_INDEX_MAP_PHY_ADDR(index);
> > + data = rtk_usb_phy_read(regAddr, addr);
> > + (phy_parameter + index)->addr = addr;
> > + (phy_parameter + index)->data = data;
> > + }
> > + mdelay(1);
> > + dev_info(rtk_phy->dev,
> > + "%s ########## to toggle PHY addr 0x09
> BIT(9)\n",
> > + __func__);
> > + rtk_usb_phy_write(regAddr, addr, data&(~BIT(9)));
> > + mdelay(1);
> > + rtk_usb_phy_write(regAddr, addr, data);
> > + }
> > + dev_info(rtk_phy->dev, "%s ########## PHY addr 0x1f = 0x%04x\n",
> > + __func__, rtk_usb_phy_read(regAddr,
> PHY_ADDR_0x1F));
>
> Please drop all simple debug success messages. Linux has already
> infrastructure for this.
>
Okay. I will change the print dev_info to dev_dbg about debug message.
> ...
>
> > + return 0;
> > +}
> > +
> > +static int rtk_usb_phy_init(struct phy *phy) {
> > + struct rtk_usb_phy *rtk_phy = phy_get_drvdata(phy);
> > + int ret = 0;
> > + int i;
> > + unsigned long phy_init_time = jiffies;
> > +
> > + if (!rtk_phy) {
> > + pr_err("%s rtk_phy is NULL!\n", __func__);
>
> What? How is this possible?
It should be not necessary. I will remove it.
> > + return -ENODEV;
> > + }
> > +
> > + dev_dbg(rtk_phy->dev, "Init RTK USB 3.0 PHY\n");
> > + for (i = 0; i < rtk_phy->phyN; i++)
> > + ret = do_rtk_usb_phy_init(rtk_phy, i);
> > +
> > + dev_info(rtk_phy->dev, "Initialized RTK USB 3.0 PHY (take %dms)\n",
> > + jiffies_to_msecs(jiffies - phy_init_time));
>
> Please drop all simple debug success messages. Linux has already
> infrastructure for this.
Ok, Thanks.
> > + return ret;
> > +}
> > +
> > +static int rtk_usb_phy_exit(struct phy *phy) {
> > + struct rtk_usb_phy *rtk_phy = phy_get_drvdata(phy);
> > +
> > + if (!rtk_phy) {
> > + pr_err("%s rtk_phy is NULL!\n", __func__);
> > + return -ENODEV;
> > + }
> > +
> > + dev_dbg(rtk_phy->dev, "Exit RTK USB 3.0 PHY\n");
>
> Please drop all simple debug success messages. Linux has already
> infrastructure for this.
Can I keep log for dev_dbg?
> > +static void rtk_usb_phy_toggle(struct usb_phy *usb3_phy, bool
> > +isConnect, int port) {
> > + int index = port;
> > + struct rtk_usb_phy *rtk_phy = NULL;
> > +
> > + if (usb3_phy != NULL && usb3_phy->dev != NULL)
> > + rtk_phy = dev_get_drvdata(usb3_phy->dev);
> > +
> > + if (rtk_phy == NULL) {
> > + pr_err("%s ERROR! NO this device\n", __func__);
>
> Your error messages are not helping. No need to shout, no need to handle
> some non-existing cases. If this is real case, you have broken driver. I actually
> suspect that.
>
> How can you interface with a driver where there is no device?
OK, I know this is not good programming practice, I will improve this question.
> > + return;
> > + }
> > +
> > + if (index > rtk_phy->phyN) {
> > + pr_err("%s %d ERROR! port=%d > phyN=%d\n",
> > + __func__, __LINE__, index, rtk_phy->phyN);
> > + return;
> > + }
> > +
> > + do_rtk_usb3_phy_toggle(rtk_phy, index, isConnect); }
> > +
> > +static int rtk_usb_phy_notify_port_status(struct usb_phy *x, int port,
> > + u16 portstatus, u16 portchange) {
> > + bool isConnect = false;
>
> This is not C++. Don't use camelcase. See Coding style document.
I will revised for this style.
> > +
> > + pr_debug("%s port=%d portstatus=0x%x portchange=0x%x\n",
> > + __func__, port, (int)portstatus, (int)portchange);
> > + if (portstatus & USB_PORT_STAT_CONNECTION)
> > + isConnect = true;
> > +
>
> ...
>
> > +
> > +static int rtk_usb3_set_parameter_show(struct seq_file *s, void
> > +*unused) {
> > + struct rtk_usb_phy *rtk_phy = s->private;
> > + 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;
> > + int ret, index;
> > + struct phy_data *phy_data = NULL;
> > +
> > + for (index = 0; index < rtk_phy->phyN; 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_data = &((struct phy_data
> *)rtk_phy->phy_data)[index];
> > + break;
> > + }
> > + }
> > + if (!phy_data) {
> > + dev_err(rtk_phy->dev,
> > + "%s: No phy_data for %s/%s\n",
> > + __func__, phy_dir_name,
> file_name);
>
> Mess wrapping/indentation. Actually everywhere in the file...
I will improve this.
> > +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_usb_phy *rtk_phy = s->private;
> > + struct reg_addr *regAddr = NULL;
> > + struct phy_data *phy_data = 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->phyN; index++) {
> > + size_t sz = 30;
> > + char name[30] = {0};
> > +
> > + snprintf(name, sz, "phy%d", index);
> > + if (strncmp(name, phy_dir_name, strlen(name)) == 0) {
> > + regAddr = &((struct reg_addr
> *)rtk_phy->reg_addr)[index];
> > + phy_data = &((struct phy_data
> *)rtk_phy->phy_data)[index];
> > + break;
>
>
> Where is the ABI documentation for user interface?
Do debugfs nodes need ABI documentation?
Is there a reference?
>
> > +
> > +static inline void create_debug_files(struct rtk_usb_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) {
> > + dev_err(rtk_phy->dev, "%s Error phy_debug_root is NULL",
> > + __func__);
> > + return;
> > + }
> > + rtk_phy->debug_dir = debugfs_create_dir(dev_name(rtk_phy->dev),
> > + phy_debug_root);
> > + if (!rtk_phy->debug_dir) {
> > + dev_err(rtk_phy->dev, "%s Error debug_dir is NULL",
> > + __func__);
>
> Are you sure you run checkpatch on this? Error messages on debugfs are
> almost always incorrect.
Yes, I have run checkpatch for patches.
Why the message is incorrect?
> > +static int get_phy_parameter(struct rtk_usb_phy *rtk_phy,
> > + struct device_node *sub_node) {
> > + struct device *dev = rtk_phy->dev;
> > + struct reg_addr *addr;
> > + struct phy_data *phy_data;
> > + int ret = 0;
> > + int index;
> > +
> > + if (of_property_read_u32(sub_node, "reg", &index)) {
> > + dev_err(dev, "sub_node without reg\n");
> > + return -EINVAL;
> > + }
> > +
> > + dev_dbg(dev, "sub_node index=%d\n", index);
>
> Please drop all simple debug success messages. Linux has already
> infrastructure for this.
Can I keep log for dev_dbg?
> ...
>
> > +
> > +static int rtk_usb3phy_probe(struct platform_device *pdev) {
> > + struct rtk_usb_phy *rtk_phy;
> > + struct device *dev = &pdev->dev;
> > + struct device_node *node;
> > + struct device_node *sub_node;
> > + struct phy *generic_phy;
> > + struct phy_provider *phy_provider;
> > + int ret, phyN;
> > +
> > + 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_usb_phy_notify_port_status;
> > +
> > + if (!dev->of_node) {
> > + dev_err(dev, "%s %d No device node\n", __func__,
> > + __LINE__);
>
> How is it even possible? If you do not have device node here, how did it probe?
You are right. The of_node is always exist.
I will remove it.
>
> > + ret = -ENODEV;
> > + goto err;
> > + }
> > +
> > + node = dev->of_node;
> > +
> > + phyN = of_get_child_count(node);
> > + rtk_phy->phyN = phyN;
> > + dev_dbg(dev, "%s phyN=%d\n", __func__, rtk_phy->phyN);
>
> Please drop all simple debug success messages. Linux has already
> infrastructure for this.
Okay. I will remove debug message.
Thanks,
Stanley
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
2023-06-07 11:54 ` Krzysztof Kozlowski
@ 2023-06-08 6:59 ` Stanley Chang[昌育德]
2023-06-08 7:00 ` Krzysztof Kozlowski
2023-06-08 7:15 ` Krzysztof Kozlowski
0 siblings, 2 replies; 33+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-08 6:59 UTC (permalink / raw)
To: Krzysztof Kozlowski, Greg Kroah-Hartman
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Michael Grzeschik,
Mathias Nyman, Bagas Sanjaya, Matthias Kaehlcke, Ray Chi,
Flavio Suligoi, linux-phy, devicetree, linux-kernel, linux-usb
Hi Krzysztof,
> I commented on your second patch, but everything is applicable here as well.
> You have many useless debug messages. Many incorrect or useless
> "if() return" which point to broken driver design (e.g. concurrent access to half
> initialized structures where you substitute lack of synchronization with
> incorrect "if() return"). Undocumented user interface is one more big trouble.
>
> I doubt you run checkpatch on this (be sure to run it with --strict and fix almost
> everything).
>
>
1. I use checkpatch but without --strict. I will add it add and check again.
2. Do the debugfs interfaces need to provide a document?
I don't find any reference about this.
3. I will follow the comment of second patch to fix this patch
Thanks,
Stanley
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
2023-06-08 6:59 ` Stanley Chang[昌育德]
@ 2023-06-08 7:00 ` Krzysztof Kozlowski
2023-06-08 7:15 ` Krzysztof Kozlowski
1 sibling, 0 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-08 7:00 UTC (permalink / raw)
To: Stanley Chang[昌育德], Greg Kroah-Hartman
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Michael Grzeschik,
Mathias Nyman, Bagas Sanjaya, Matthias Kaehlcke, Ray Chi,
Flavio Suligoi, linux-phy, devicetree, linux-kernel, linux-usb
On 08/06/2023 08:59, Stanley Chang[昌育德] wrote:
> Hi Krzysztof,
>
>
>> I commented on your second patch, but everything is applicable here as well.
>> You have many useless debug messages. Many incorrect or useless
>> "if() return" which point to broken driver design (e.g. concurrent access to half
>> initialized structures where you substitute lack of synchronization with
>> incorrect "if() return"). Undocumented user interface is one more big trouble.
>>
>> I doubt you run checkpatch on this (be sure to run it with --strict and fix almost
>> everything).
>>
>>
> 1. I use checkpatch but without --strict. I will add it add and check again.
> 2. Do the debugfs interfaces need to provide a document?
> I don't find any reference about this.
debugfs interface is for debug but in your case you don't use it that way.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
2023-06-08 6:55 ` Krzysztof Kozlowski
@ 2023-06-08 7:07 ` Stanley Chang[昌育德]
0 siblings, 0 replies; 33+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-08 7:07 UTC (permalink / raw)
To: Krzysztof Kozlowski, kernel test robot, Greg Kroah-Hartman
Cc: oe-kbuild-all, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Conor Dooley, Alan Stern, Michael Grzeschik, Mathias Nyman,
Bagas Sanjaya, Matthias Kaehlcke, Ray Chi, Flavio Suligoi,
linux-phy, devicetree, linux-kernel, linux-usb
Hi Krzysztof,
> On 08/06/2023 04:18, Stanley Chang[昌育德] wrote:
> >> ERROR: modpost: "devm_usb_get_phy_by_phandle"
> >> [drivers/power/supply/wm831x_power.ko] undefined!
> >>>> ERROR: modpost: "devm_usb_get_phy"
> >> [drivers/power/supply/da9150-charger.ko] undefined!
> >>
> >> Kconfig warnings: (for reference only)
> >> WARNING: unmet direct dependencies detected for USB_PHY
> >> Depends on [n]: USB_SUPPORT [=n]
> >> Selected by [y]:
> >> - PHY_RTK_RTD_USB2PHY [=y]
> >>
> >
> > I will add USB_SUUPRT dependency to Kconfig.
>
> Why? Do you see other phy drivers needing it? Few have it but many don't, so
> you should really investigate the root cause, not just add some dependencies.
>
> Build test your patches locally before sending and investigate the issues.
USB_SUPPORT is required.
Because I have select Kconfig USB_PHY.
Some drivers have used it as follow
drivers/phy/ti/Kconfig
drivers/phy/amlogic/Kconfig
drivers/phy/renesas/Kconfig
drivers/phy/rockchip/Kconfig
drivers/phy/allwinner/Kconfig
Thanks,
Stanley
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY
2023-06-08 6:59 ` Stanley Chang[昌育德]
2023-06-08 7:00 ` Krzysztof Kozlowski
@ 2023-06-08 7:15 ` Krzysztof Kozlowski
1 sibling, 0 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-08 7:15 UTC (permalink / raw)
To: Stanley Chang[昌育德], Greg Kroah-Hartman
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Michael Grzeschik,
Mathias Nyman, Bagas Sanjaya, Matthias Kaehlcke, Ray Chi,
Flavio Suligoi, linux-phy, devicetree, linux-kernel, linux-usb
On 08/06/2023 08:59, Stanley Chang[昌育德] wrote:
> Hi Krzysztof,
>
>
>> I commented on your second patch, but everything is applicable here as well.
>> You have many useless debug messages. Many incorrect or useless
>> "if() return" which point to broken driver design (e.g. concurrent access to half
>> initialized structures where you substitute lack of synchronization with
>> incorrect "if() return"). Undocumented user interface is one more big trouble.
>>
>> I doubt you run checkpatch on this (be sure to run it with --strict and fix almost
>> everything).
>>
>>
> 1. I use checkpatch but without --strict. I will add it add and check again.
Around 300 issues pointed out...
> 2. Do the debugfs interfaces need to provide a document?
> I don't find any reference about this.
Not sure if we have it in docs, but it is discussed every now and then...
https://lore.kernel.org/all/CAMuHMdUaugQ5+Zhmg=oe=X2wvhazMiT=K-su0EJYKzD4Hdyn3Q@mail.gmail.com/
https://lore.kernel.org/all/2023060209-scouts-affection-f54d@gregkh/
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY
2023-06-08 6:59 ` Stanley Chang[昌育德]
@ 2023-06-08 7:21 ` Krzysztof Kozlowski
2023-06-08 7:40 ` Stanley Chang[昌育德]
0 siblings, 1 reply; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-08 7:21 UTC (permalink / raw)
To: Stanley Chang[昌育德], Greg Kroah-Hartman
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Michael Grzeschik, Flavio Suligoi, Matthias Kaehlcke,
Mathias Nyman, linux-phy, devicetree, linux-kernel, linux-usb
On 08/06/2023 08:59, Stanley Chang[昌育德] wrote:
> Hi Krzysztof,
>
>>> +static void do_rtk_usb3_phy_toggle(struct rtk_usb_phy *rtk_phy, int i,
>>> + bool isConnect)
>>> +{
>>> + struct reg_addr *regAddr;
>>> + struct phy_data *phy_data;
>>> + struct phy_parameter *phy_parameter;
>>> + size_t index;
>>> +
>>> + regAddr = &((struct reg_addr *)rtk_phy->reg_addr)[i];
>>> + phy_data = &((struct phy_data *)rtk_phy->phy_data)[i];
>>> +
>>> + if (!phy_data) {
>>> + dev_err(rtk_phy->dev, "%s phy_data is NULL!\n",
>>> + __func__);
>>
>> ???
> Sorry, this check is redundant.
>
>>
>>> + return;
>>> + }
>>> +
>>> + if (!phy_data->do_toggle)
>>> + return;
>>> +
>>> + phy_parameter = phy_data->parameter;
>>> +
>>> + index = PHY_ADDR_MAP_ARRAY_INDEX(PHY_ADDR_0x09);
>>> +
>>> + if (index < phy_data->size) {
>>> + u8 addr = (phy_parameter + index)->addr;
>>> + u16 data = (phy_parameter + index)->data;
>>> +
>>> + if (addr == 0xFF) {
>>> + addr = ARRAY_INDEX_MAP_PHY_ADDR(index);
>>> + data = rtk_usb_phy_read(regAddr, addr);
>>> + (phy_parameter + index)->addr = addr;
>>> + (phy_parameter + index)->data = data;
>>> + }
>>> + mdelay(1);
>>> + dev_info(rtk_phy->dev,
>>> + "%s ########## to toggle PHY addr 0x09
>> BIT(9)\n",
>>> + __func__);
>>> + rtk_usb_phy_write(regAddr, addr, data&(~BIT(9)));
>>> + mdelay(1);
>>> + rtk_usb_phy_write(regAddr, addr, data);
>>> + }
>>> + dev_info(rtk_phy->dev, "%s ########## PHY addr 0x1f = 0x%04x\n",
>>> + __func__, rtk_usb_phy_read(regAddr,
>> PHY_ADDR_0x1F));
>>
>> Please drop all simple debug success messages. Linux has already
>> infrastructure for this.
>>
> Okay. I will change the print dev_info to dev_dbg about debug message.
No, drop them. This piece of code had already 2 printks for register
contents! Your driver is overloaded with printks and they are mostly
useless for the user.
>
>> ...
>>
>>> + return 0;
>>> +}
>>> +
>>> +static int rtk_usb_phy_init(struct phy *phy) {
>>> + struct rtk_usb_phy *rtk_phy = phy_get_drvdata(phy);
>>> + int ret = 0;
>>> + int i;
>>> + unsigned long phy_init_time = jiffies;
>>> +
>>> + if (!rtk_phy) {
>>> + pr_err("%s rtk_phy is NULL!\n", __func__);
>>
>> What? How is this possible?
> It should be not necessary. I will remove it.
>
>>> + return -ENODEV;
>>> + }
>>> +
>>> + dev_dbg(rtk_phy->dev, "Init RTK USB 3.0 PHY\n");
>>> + for (i = 0; i < rtk_phy->phyN; i++)
>>> + ret = do_rtk_usb_phy_init(rtk_phy, i);
>>> +
>>> + dev_info(rtk_phy->dev, "Initialized RTK USB 3.0 PHY (take %dms)\n",
>>> + jiffies_to_msecs(jiffies - phy_init_time));
>>
>> Please drop all simple debug success messages. Linux has already
>> infrastructure for this.
>
> Ok, Thanks.
>
>>> + return ret;
>>> +}
>>> +
>>> +static int rtk_usb_phy_exit(struct phy *phy) {
>>> + struct rtk_usb_phy *rtk_phy = phy_get_drvdata(phy);
>>> +
>>> + if (!rtk_phy) {
>>> + pr_err("%s rtk_phy is NULL!\n", __func__);
>>> + return -ENODEV;
>>> + }
>>> +
>>> + dev_dbg(rtk_phy->dev, "Exit RTK USB 3.0 PHY\n");
>>
>> Please drop all simple debug success messages. Linux has already
>> infrastructure for this.
>
> Can I keep log for dev_dbg?
Of course not. This was dev_dbg and I commented on this. This is not a
good debug, we do not print anything on function entrance and exit.
ftrace() is for this.
>
>>> +static void rtk_usb_phy_toggle(struct usb_phy *usb3_phy, bool
>>> +isConnect, int port) {
>>> + int index = port;
>>> + struct rtk_usb_phy *rtk_phy = NULL;
>>> +
>>> + if (usb3_phy != NULL && usb3_phy->dev != NULL)
>>> + rtk_phy = dev_get_drvdata(usb3_phy->dev);
>>> +
>>> + if (rtk_phy == NULL) {
>>> + pr_err("%s ERROR! NO this device\n", __func__);
>>
>> Your error messages are not helping. No need to shout, no need to handle
>> some non-existing cases. If this is real case, you have broken driver. I actually
>> suspect that.
>>
>> How can you interface with a driver where there is no device?
>
> OK, I know this is not good programming practice, I will improve this question.
>
>>> + return;
>>> + }
>>> +
>>> + if (index > rtk_phy->phyN) {
>>> + pr_err("%s %d ERROR! port=%d > phyN=%d\n",
>>> + __func__, __LINE__, index, rtk_phy->phyN);
>>> + return;
>>> + }
>>> +
>>> + do_rtk_usb3_phy_toggle(rtk_phy, index, isConnect); }
>>> +
>>> +static int rtk_usb_phy_notify_port_status(struct usb_phy *x, int port,
>>> + u16 portstatus, u16 portchange) {
>>> + bool isConnect = false;
>>
>> This is not C++. Don't use camelcase. See Coding style document.
>
> I will revised for this style.
>
>>> +
>>> + pr_debug("%s port=%d portstatus=0x%x portchange=0x%x\n",
>>> + __func__, port, (int)portstatus, (int)portchange);
>>> + if (portstatus & USB_PORT_STAT_CONNECTION)
>>> + isConnect = true;
>>> +
>>
>> ...
>>
>>> +
>>> +static int rtk_usb3_set_parameter_show(struct seq_file *s, void
>>> +*unused) {
>>> + struct rtk_usb_phy *rtk_phy = s->private;
>>> + 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;
>>> + int ret, index;
>>> + struct phy_data *phy_data = NULL;
>>> +
>>> + for (index = 0; index < rtk_phy->phyN; 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_data = &((struct phy_data
>> *)rtk_phy->phy_data)[index];
>>> + break;
>>> + }
>>> + }
>>> + if (!phy_data) {
>>> + dev_err(rtk_phy->dev,
>>> + "%s: No phy_data for %s/%s\n",
>>> + __func__, phy_dir_name,
>> file_name);
>>
>> Mess wrapping/indentation. Actually everywhere in the file...
>
> I will improve this.
>
>>> +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_usb_phy *rtk_phy = s->private;
>>> + struct reg_addr *regAddr = NULL;
>>> + struct phy_data *phy_data = 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->phyN; index++) {
>>> + size_t sz = 30;
>>> + char name[30] = {0};
>>> +
>>> + snprintf(name, sz, "phy%d", index);
>>> + if (strncmp(name, phy_dir_name, strlen(name)) == 0) {
>>> + regAddr = &((struct reg_addr
>> *)rtk_phy->reg_addr)[index];
>>> + phy_data = &((struct phy_data
>> *)rtk_phy->phy_data)[index];
>>> + break;
>>
>>
>> Where is the ABI documentation for user interface?
>
> Do debugfs nodes need ABI documentation?
> Is there a reference?
>>
>>> +
>>> +static inline void create_debug_files(struct rtk_usb_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) {
>>> + dev_err(rtk_phy->dev, "%s Error phy_debug_root is NULL",
>>> + __func__);
>>> + return;
>>> + }
>>> + rtk_phy->debug_dir = debugfs_create_dir(dev_name(rtk_phy->dev),
>>> + phy_debug_root);
>>> + if (!rtk_phy->debug_dir) {
>>> + dev_err(rtk_phy->dev, "%s Error debug_dir is NULL",
>>> + __func__);
>>
>> Are you sure you run checkpatch on this? Error messages on debugfs are
>> almost always incorrect.
>
> Yes, I have run checkpatch for patches.
> Why the message is incorrect?
Because debugfs failures should not cause any error prints. It's debug,
not important.
Do you see anywhere error messages?
Entire debugfs handling code should be silent and even skip all error
checking, as most API is ready for handling previous errors, I think.
>
>>> +static int get_phy_parameter(struct rtk_usb_phy *rtk_phy,
>>> + struct device_node *sub_node) {
>>> + struct device *dev = rtk_phy->dev;
>>> + struct reg_addr *addr;
>>> + struct phy_data *phy_data;
>>> + int ret = 0;
>>> + int index;
>>> +
>>> + if (of_property_read_u32(sub_node, "reg", &index)) {
>>> + dev_err(dev, "sub_node without reg\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + dev_dbg(dev, "sub_node index=%d\n", index);
>>
>> Please drop all simple debug success messages. Linux has already
>> infrastructure for this.
>
> Can I keep log for dev_dbg?
No, this was dev_dbg and I commented on this to remove it. Keep only
useful debug messages, not hundreds of them in every place.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY
2023-06-07 12:04 ` Krzysztof Kozlowski
@ 2023-06-08 7:24 ` Stanley Chang[昌育德]
2023-06-08 7:49 ` Krzysztof Kozlowski
2023-06-08 7:47 ` Stanley Chang[昌育德]
1 sibling, 1 reply; 33+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-08 7:24 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Matthias Kaehlcke, Douglas Anderson, Michael Grzeschik,
Mathias Nyman, Flavio Suligoi, linux-phy, devicetree,
linux-kernel, linux-usb
Hi Krzysztof,
>
> On 07/06/2023 08:24, 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>
> > ---
> > 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 | 213
> ++++++++++++++++++
> > 1 file changed, 213 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..69911e20a561
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/realtek,usb2phy.yaml
> > @@ -0,0 +1,213 @@
> > +# 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,rtd1395-usb2phy
> > + - realtek,rtd1619-usb2phy
> > + - realtek,rtd1319-usb2phy
> > + - realtek,rtd1619b-usb2phy
> > + - realtek,rtd1312c-usb2phy
> > + - realtek,rtd1319d-usb2phy
> > + - realtek,rtd1315e-usb2phy
>
> Keep entries ordered alphabetically.
Okay.
> > + - const: realtek,usb2phy
> > +
> > + reg:
> > + items:
> > + - description: PHY data registers
> > + - description: PHY control registers
> > +
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 0
> > +
> > + "#phy-cells":
> > + const: 0
> > +
> > + realtek,usb-ctrl:
> > + description: The phandle of syscon used to control USB PHY power
> domain.
> > + $ref: /schemas/types.yaml#/definitions/phandle
>
> No, we have power-domains for this.
Maybe I use the word "control power domain" is not well, I just want to control the ldo of usb phy.
Revised:
The phandle of syscon used to control the ldo of USB PHY.
> > +
> > +patternProperties:
> > + "^phy@[0-3]+$":
> > + type: object
> > + description:
> > + Each sub-node is a PHY device for one XHCI controller.
>
> I don't think it is true. You claim above that you have 0 as phy-cells, means you
> have one phy. Here you say you can have up to 4 phys.
I mean the driver can support up to 4 phys.
For RTD1295 has only one phy.
For RTD1395 has two phys.
> > + For most Relatek SoCs, one XHCI controller only support one the USB
> 2.0
> > + phy. For RTD1395 SoC, the one XHCI controller has two USB 2.0
> PHYs.
> > + properties:
> > + realtek,page0-param:
> > + description: PHY parameter at page 0. The data are the pair of
> the
> > + offset and value.
>
> This needs to be specific. What the heck is "PHY parameter"?
>
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
>
> Array? Then maxItems.
I have found other document.
It should be a uint32-matrix.
I will add the maxItems.
> > +
> > + realtek,page1-param:
> > + description: PHY parameter at page 1. The data are the pair of
> the
> > + offset and value.
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > +
> > + realtek,page2-param:
> > + description: PHY parameter at page 2. The data are the pair of
> the
> > + offset and value. If the PHY support the page 2 parameter.
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > +
> > + realtek,support-page2-param:
> > + description: Set this flag if PHY support page 2 parameter.
>
> Why this cannot be deducted from compatible?
It can identify by compatible.
>
> > + type: boolean
> > +
> > + realtek,do-toggle:
> > + description: Set this flag to enable PHY parameter toggle when
> port
> > + status change.
>
> Do not instruct OS what to do. Explain why this is a hardware characteristic.
In my original intention, we hope that this property can be used to control the phy driver do parameter toggle.
Is it a hardware characteristic? I don't think it's exactly a hardware feature.
Maybe it can be specified by the compatible.
> > + type: boolean
> > +
> > + realtek,do-toggle-driving:
> > + description: Set this flag to enable PHY parameter toggle for
> adjust
> > + the driving when port status change.
>
> Do not instruct OS what to do. Explain why this is a hardware characteristic.
>
>
> > + type: boolean
> > +
> > + realtek,check-efuse:
> > + description: Enable to update PHY parameter from reading otp
> table.
>
> Do not instruct OS what to do. Explain why this is a hardware characteristic.
Same above.
> > + type: boolean
> > +
> > + realtek,use-default-parameter:
> > + description: Don't set parameter and use default value in
> hardware.
>
> NAK, you are just making things up.
This is a software flow control.
I will remove it.
>
> > + type: boolean
> > +
> > + realtek,is-double-sensitivity-mode:
> > + description: Set this flag to enable double sensitivity mode.
>
> All your descriptions copy the name of property. You basically say nothing more.
> I already mentioned this before. Don't ignore the feedback, but address it.
I will improve this.
> > + type: boolean
> > +
> > + realtek,ldo-force-enable:
> > + description: Set this flag to force enable ldo mode.
>
> Drop everywhere "Set this flag to", because it is redundant. Now compare what
> is left with property name.
>
> Property name: realtek,ldo-force-enable
> Your description: "force enable ldo mode"
>
> How is this helpful to anybody?
This is a software flow control.
I will remove it.
Thanks,
Stanley
.
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY
2023-06-07 12:08 ` Krzysztof Kozlowski
@ 2023-06-08 7:32 ` Stanley Chang[昌育德]
2023-06-08 7:50 ` Krzysztof Kozlowski
0 siblings, 1 reply; 33+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-08 7:32 UTC (permalink / raw)
To: Krzysztof Kozlowski, Greg Kroah-Hartman
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Mathias Nyman, Michael Grzeschik, Matthias Kaehlcke,
Flavio Suligoi, linux-phy, devicetree, linux-kernel, linux-usb
Hi Krzysztof,
>
> 1 phy or 4? Decide.
In actually, we have one phy for one controller.
I mean the driver can support up to 4 phys.
I can revised as
"^phy@[0]+$"
Or only "phy"
> > +
> > +patternProperties:
> > + "^phy@[0-3]+$":
> > + description: Each sub-node is a PHY device for one XHCI controller.
> > + type: object
> > + properties:
> > + realtek,param:
> > + description: The data of PHY parameter are the pair of the
> > + offset and value.
> > + $ref: /schemas/types.yaml#/definitions/uint8-array
>
> Your choice of types is surprising. If this is array, than maxItems (and please
> don't tell me it is maxItems: 1). Anyway, why 8 bits long?
It should be a uint32-matrix.
> > +
> > + realtek,do-toggle:
> > + description: Set this flag to enable the PHY parameter toggle
> > + when port status change.
> > + type: boolean
> > +
> > + realtek,do-toggle-once:
> > + description: Set this flag to do PHY parameter toggle only on
> > + PHY init.
> > + type: boolean
> > +
> > + realtek,check-efuse:
> > + description: Enable to update PHY parameter from reading otp
> table.
> > + type: boolean
> > +
> > + realtek,use-default-parameter:
> > + description: Don't set parameter and use default value in
> hardware.
> > + type: boolean
> > +
> > + realtek,check-rx-front-end-offset:
> > + description: Enable to check rx front end offset.
> > + type: boolean
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - "#address-cells"
> > + - "#size-cells"
> > + - "#phy-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + usb_port2_usb3phy: usb-phy@13e10 {
> > + compatible = "realtek,rtd1319d-usb3phy", "realtek,usb3phy";
> > + reg = <0x13e10 0x4>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + #phy-cells = <0>;
> > +
> > + phy@0 {
> > + reg = <0>;
> > + realtek,param =
> > + <0x01 0xac8c>,
> > + <0x06 0x0017>,
>
> First, this is matrix, not uint8 array. Second, 0xac8c is past 16 bits long, not 8.
> Third, you put some magic register programming to DT.
> Please don't. Drop all this from DT.
realtek,param is an uint32-matrx.
I will revised the type.
Thanks,
Stanley
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY
2023-06-08 7:21 ` Krzysztof Kozlowski
@ 2023-06-08 7:40 ` Stanley Chang[昌育德]
0 siblings, 0 replies; 33+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-08 7:40 UTC (permalink / raw)
To: Krzysztof Kozlowski, Greg Kroah-Hartman
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Michael Grzeschik, Flavio Suligoi, Matthias Kaehlcke,
Mathias Nyman, linux-phy, devicetree, linux-kernel, linux-usb
> >> Please drop all simple debug success messages. Linux has already
> >> infrastructure for this.
> >>
> > Okay. I will change the print dev_info to dev_dbg about debug message.
>
> No, drop them. This piece of code had already 2 printks for register contents!
> Your driver is overloaded with printks and they are mostly useless for the user.
I will drop them to simplify the code.
> >> Please drop all simple debug success messages. Linux has already
> >> infrastructure for this.
> >
> > Can I keep log for dev_dbg?
>
> Of course not. This was dev_dbg and I commented on this. This is not a good
> debug, we do not print anything on function entrance and exit.
> ftrace() is for this.
Well, for debugging purposes, I'm going to have to dig into ftrace.
This is a great tip.
> >>
> >> Are you sure you run checkpatch on this? Error messages on debugfs
> >> are almost always incorrect.
> >
> > Yes, I have run checkpatch for patches.
> > Why the message is incorrect?
>
> Because debugfs failures should not cause any error prints. It's debug, not
> important.
>
> Do you see anywhere error messages?
>
> Entire debugfs handling code should be silent and even skip all error checking,
> as most API is ready for handling previous errors, I think.
Thanks, I understand now.
Thanks,
Stanley
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY
2023-06-07 12:04 ` Krzysztof Kozlowski
2023-06-08 7:24 ` Stanley Chang[昌育德]
@ 2023-06-08 7:47 ` Stanley Chang[昌育德]
2023-06-08 7:51 ` Krzysztof Kozlowski
1 sibling, 1 reply; 33+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-08 7:47 UTC (permalink / raw)
To: Krzysztof Kozlowski, Greg Kroah-Hartman
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Matthias Kaehlcke, Douglas Anderson, Michael Grzeschik,
Mathias Nyman, Flavio Suligoi, linux-phy, devicetree,
linux-kernel, linux-usb
Hi Krzysztof,
> > + For most Relatek SoCs, one XHCI controller only support one the USB
> 2.0
> > + phy. For RTD1395 SoC, the one XHCI controller has two USB 2.0
> PHYs.
> > + properties:
> > + realtek,page0-param:
> > + description: PHY parameter at page 0. The data are the pair of
> the
> > + offset and value.
>
> This needs to be specific. What the heck is "PHY parameter"?
>
It contains more parameters
page0 has 16 parameters
page1 has 8 parameters
page2 has 8 parameters
It's tedious if we list them all.
And we only set the part that differs from the default.
It's hard to explain which parameters were changed because each platform is different.
Thanks,
Stanley
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY
2023-06-08 7:24 ` Stanley Chang[昌育德]
@ 2023-06-08 7:49 ` Krzysztof Kozlowski
2023-06-08 8:21 ` Stanley Chang[昌育德]
0 siblings, 1 reply; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-08 7:49 UTC (permalink / raw)
To: Stanley Chang[昌育德]
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Matthias Kaehlcke, Douglas Anderson, Michael Grzeschik,
Mathias Nyman, Flavio Suligoi, linux-phy, devicetree,
linux-kernel, linux-usb
On 08/06/2023 09:24, Stanley Chang[昌育德] wrote:
>>> + realtek,usb-ctrl:
>>> + description: The phandle of syscon used to control USB PHY power
>> domain.
>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>
>> No, we have power-domains for this.
>
> Maybe I use the word "control power domain" is not well, I just want to control the ldo of usb phy.
> Revised:
> The phandle of syscon used to control the ldo of USB PHY.
Isn't this still a power domain?
>
>>> +
>>> +patternProperties:
>>> + "^phy@[0-3]+$":
>>> + type: object
>>> + description:
>>> + Each sub-node is a PHY device for one XHCI controller.
>>
>> I don't think it is true. You claim above that you have 0 as phy-cells, means you
>> have one phy. Here you say you can have up to 4 phys.
>
> I mean the driver can support up to 4 phys.
What driver can or cannot do, does not matter. This is about hardware.
> For RTD1295 has only one phy.
> For RTD1395 has two phys.
Two phys? So how do you reference them when cells=0?
>
>>> + For most Relatek SoCs, one XHCI controller only support one the USB
>> 2.0
>>> + phy. For RTD1395 SoC, the one XHCI controller has two USB 2.0
>> PHYs.
>>> + properties:
>>> + realtek,page0-param:
>>> + description: PHY parameter at page 0. The data are the pair of
>> the
>>> + offset and value.
>>
>> This needs to be specific. What the heck is "PHY parameter"?
>>
>>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>>
>> Array? Then maxItems.
> I have found other document.
> It should be a uint32-matrix.
> I will add the maxItems.
Entire property should be dropped.
>
>>> +
>>> + realtek,page1-param:
>>> + description: PHY parameter at page 1. The data are the pair of
>> the
>>> + offset and value.
>>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +
>>> + realtek,page2-param:
>>> + description: PHY parameter at page 2. The data are the pair of
>> the
>>> + offset and value. If the PHY support the page 2 parameter.
>>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +
>>> + realtek,support-page2-param:
>>> + description: Set this flag if PHY support page 2 parameter.
>>
>> Why this cannot be deducted from compatible?
> It can identify by compatible.
So drop it.
>
>>
>>> + type: boolean
>>> +
>>> + realtek,do-toggle:
>>> + description: Set this flag to enable PHY parameter toggle when
>> port
>>> + status change.
>>
>> Do not instruct OS what to do. Explain why this is a hardware characteristic.
>
> In my original intention, we hope that this property can be used to control the phy driver do parameter toggle.
> Is it a hardware characteristic? I don't think it's exactly a hardware feature.
> Maybe it can be specified by the compatible.
Drop it.
>
>>> + type: boolean
>>> +
>>> + realtek,do-toggle-driving:
>>> + description: Set this flag to enable PHY parameter toggle for
>> adjust
>>> + the driving when port status change.
>>
>> Do not instruct OS what to do. Explain why this is a hardware characteristic.
>>
>>
>>> + type: boolean
>>> +
>>> + realtek,check-efuse:
>>> + description: Enable to update PHY parameter from reading otp
>> table.
>>
>> Do not instruct OS what to do. Explain why this is a hardware characteristic.
>
> Same above.
So drop all of these.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY
2023-06-08 7:32 ` Stanley Chang[昌育德]
@ 2023-06-08 7:50 ` Krzysztof Kozlowski
2023-06-08 8:00 ` Stanley Chang[昌育德]
0 siblings, 1 reply; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-08 7:50 UTC (permalink / raw)
To: Stanley Chang[昌育德], Greg Kroah-Hartman
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Mathias Nyman, Michael Grzeschik, Matthias Kaehlcke,
Flavio Suligoi, linux-phy, devicetree, linux-kernel, linux-usb
On 08/06/2023 09:32, Stanley Chang[昌育德] wrote:
>>> +examples:
>>> + - |
>>> + usb_port2_usb3phy: usb-phy@13e10 {
>>> + compatible = "realtek,rtd1319d-usb3phy", "realtek,usb3phy";
>>> + reg = <0x13e10 0x4>;
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + #phy-cells = <0>;
>>> +
>>> + phy@0 {
>>> + reg = <0>;
>>> + realtek,param =
>>> + <0x01 0xac8c>,
>>> + <0x06 0x0017>,
>>
>> First, this is matrix, not uint8 array. Second, 0xac8c is past 16 bits long, not 8.
>> Third, you put some magic register programming to DT.
>> Please don't. Drop all this from DT.
>
> realtek,param is an uint32-matrx.
> I will revised the type.
Drop the property. It is not explained and not justified to be in DT.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY
2023-06-08 7:47 ` Stanley Chang[昌育德]
@ 2023-06-08 7:51 ` Krzysztof Kozlowski
2023-06-08 8:01 ` Stanley Chang[昌育德]
0 siblings, 1 reply; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-08 7:51 UTC (permalink / raw)
To: Stanley Chang[昌育德], Greg Kroah-Hartman
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Matthias Kaehlcke, Douglas Anderson, Michael Grzeschik,
Mathias Nyman, Flavio Suligoi, linux-phy, devicetree,
linux-kernel, linux-usb
On 08/06/2023 09:47, Stanley Chang[昌育德] wrote:
> Hi Krzysztof,
>
>
>>> + For most Relatek SoCs, one XHCI controller only support one the USB
>> 2.0
>>> + phy. For RTD1395 SoC, the one XHCI controller has two USB 2.0
>> PHYs.
>>> + properties:
>>> + realtek,page0-param:
>>> + description: PHY parameter at page 0. The data are the pair of
>> the
>>> + offset and value.
>>
>> This needs to be specific. What the heck is "PHY parameter"?
>>
> It contains more parameters
> page0 has 16 parameters
> page1 has 8 parameters
> page2 has 8 parameters
> It's tedious if we list them all.
Sure, if you prefer not to list them, then they should be removed from DT.
> And we only set the part that differs from the default.
> It's hard to explain which parameters were changed because each platform is different.
If this is phy tuning per board, you need to explain and justify them.
If this is per platform, then drop it - not even needed, because you
have compatible for this.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY
2023-06-08 7:50 ` Krzysztof Kozlowski
@ 2023-06-08 8:00 ` Stanley Chang[昌育德]
0 siblings, 0 replies; 33+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-08 8: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, Ray Chi,
Mathias Nyman, Michael Grzeschik, Matthias Kaehlcke,
Flavio Suligoi, linux-phy, devicetree, linux-kernel, linux-usb
> > realtek,param is an uint32-matrx.
> > I will revised the type.
>
> Drop the property. It is not explained and not justified to be in DT.
Okay, I try to specify by the compatible.
Thanks,
Stanley
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY
2023-06-08 7:51 ` Krzysztof Kozlowski
@ 2023-06-08 8:01 ` Stanley Chang[昌育德]
0 siblings, 0 replies; 33+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-08 8:01 UTC (permalink / raw)
To: Krzysztof Kozlowski, Greg Kroah-Hartman
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Matthias Kaehlcke, Douglas Anderson, Michael Grzeschik,
Mathias Nyman, Flavio Suligoi, linux-phy, devicetree,
linux-kernel, linux-usb
> >>
> >> This needs to be specific. What the heck is "PHY parameter"?
> >>
> > It contains more parameters
> > page0 has 16 parameters
> > page1 has 8 parameters
> > page2 has 8 parameters
> > It's tedious if we list them all.
>
> Sure, if you prefer not to list them, then they should be removed from DT.
>
> > And we only set the part that differs from the default.
> > It's hard to explain which parameters were changed because each platform is
> different.
>
> If this is phy tuning per board, you need to explain and justify them.
> If this is per platform, then drop it - not even needed, because you have
> compatible for this.
>
Okay, I try to specify by the compatible.
Thanks,
Stanley
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY
2023-06-08 7:49 ` Krzysztof Kozlowski
@ 2023-06-08 8:21 ` Stanley Chang[昌育德]
2023-06-08 8:28 ` Krzysztof Kozlowski
0 siblings, 1 reply; 33+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-08 8:21 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Matthias Kaehlcke, Douglas Anderson, Michael Grzeschik,
Mathias Nyman, Flavio Suligoi, linux-phy, devicetree,
linux-kernel, linux-usb
> > Maybe I use the word "control power domain" is not well, I just want to
> control the ldo of usb phy.
> > Revised:
> > The phandle of syscon used to control the ldo of USB PHY.
>
> Isn't this still a power domain?
I only control a register, it is not needed a driver of power domain.
> >
> >>> +
> >>> +patternProperties:
> >>> + "^phy@[0-3]+$":
> >>> + type: object
> >>> + description:
> >>> + Each sub-node is a PHY device for one XHCI controller.
> >>
> >> I don't think it is true. You claim above that you have 0 as
> >> phy-cells, means you have one phy. Here you say you can have up to 4 phys.
> >
> > I mean the driver can support up to 4 phys.
>
> What driver can or cannot do, does not matter. This is about hardware.
>
> > For RTD1295 has only one phy.
> > For RTD1395 has two phys.
>
> Two phys? So how do you reference them when cells=0?
About RTD1395 SoCs USB
XHCI controller#1 -- usb2phy -- phy#0
|- phy#1
One xhci controller map to one phy driver.
And one phy driver have two phys (phy@0 and phy@1).
Maybe the "phy" name is confusing.
This "phy" not mean a phy driver.
Would "port" be more appropriate?
For example,
Using phy@0 and phy@1:
usb_port1_usb2phy: usb-phy@13c14 {
compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
reg = <0x132c4 0x4>, <0x31280 0x8>;
#address-cells = <1>;
#size-cells = <0>;
#phy-cells = <0>;
realtek,usb-ctrl = <&usb_ctrl>;
phy@0 {
reg = <0>;
};
phy@1 {
reg = <1>;
};
};
Change: port@0 and port@1
usb_port1_usb2phy: usb-phy@13c14 {
compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
reg = <0x132c4 0x4>, <0x31280 0x8>;
#address-cells = <1>;
#size-cells = <0>;
#phy-cells = <0>;
realtek,usb-ctrl = <&usb_ctrl>;
prot@0 {
reg = <0>;
};
port@1 {
reg = <1>;
};
};
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY
2023-06-08 8:21 ` Stanley Chang[昌育德]
@ 2023-06-08 8:28 ` Krzysztof Kozlowski
2023-06-08 9:27 ` Stanley Chang[昌育德]
0 siblings, 1 reply; 33+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-08 8:28 UTC (permalink / raw)
To: Stanley Chang[昌育德]
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Matthias Kaehlcke, Douglas Anderson, Michael Grzeschik,
Mathias Nyman, Flavio Suligoi, linux-phy, devicetree,
linux-kernel, linux-usb
On 08/06/2023 10:21, Stanley Chang[昌育德] wrote:
>
>>> Maybe I use the word "control power domain" is not well, I just want to
>> control the ldo of usb phy.
>>> Revised:
>>> The phandle of syscon used to control the ldo of USB PHY.
>>
>> Isn't this still a power domain?
>
> I only control a register, it is not needed a driver of power domain.
Aren't many power domains just a registers? What about other drivers?
Don't you want in other driver control LDO of something else? And in
other something else?
>
>
>>>
>>>>> +
>>>>> +patternProperties:
>>>>> + "^phy@[0-3]+$":
>>>>> + type: object
>>>>> + description:
>>>>> + Each sub-node is a PHY device for one XHCI controller.
>>>>
>>>> I don't think it is true. You claim above that you have 0 as
>>>> phy-cells, means you have one phy. Here you say you can have up to 4 phys.
>>>
>>> I mean the driver can support up to 4 phys.
>>
>> What driver can or cannot do, does not matter. This is about hardware.
>>
>>> For RTD1295 has only one phy.
>>> For RTD1395 has two phys.
>>
>> Two phys? So how do you reference them when cells=0?
>
>
> About RTD1395 SoCs USB
> XHCI controller#1 -- usb2phy -- phy#0
> |- phy#1
> One xhci controller map to one phy driver.
> And one phy driver have two phys (phy@0 and phy@1).
>
> Maybe the "phy" name is confusing.
> This "phy" not mean a phy driver.
We do not talk about drivers, but DTS and hardware.
> Would "port" be more appropriate?
>
> For example,
> Using phy@0 and phy@1:
> usb_port1_usb2phy: usb-phy@13c14 {
> compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
> reg = <0x132c4 0x4>, <0x31280 0x8>;
> #address-cells = <1>;
> #size-cells = <0>;
> #phy-cells = <0>;
> realtek,usb-ctrl = <&usb_ctrl>;
>
> phy@0 {
> reg = <0>;
So such child is a NAK... you have nothing here. But it's unrelated topic.
> };
> phy@1 {
> reg = <1>;
> };
> };
>
> Change: port@0 and port@1
> usb_port1_usb2phy: usb-phy@13c14 {
> compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
> reg = <0x132c4 0x4>, <0x31280 0x8>;
> #address-cells = <1>;
> #size-cells = <0>;
> #phy-cells = <0>;
> realtek,usb-ctrl = <&usb_ctrl>;
>
> prot@0 {
> reg = <0>;
> };
> port@1 {
> reg = <1>;
> };
> };
This is not the answer. This is the provider. How do you reference it
from the consumer.
Upstream your entire DTS. It's frustrating to try to understand your DTS
from pieces of information you are sharing. Also very time consuming and
you are not the only one sending patches for review...
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY
2023-06-08 8:28 ` Krzysztof Kozlowski
@ 2023-06-08 9:27 ` Stanley Chang[昌育德]
0 siblings, 0 replies; 33+ messages in thread
From: Stanley Chang[昌育德] @ 2023-06-08 9:27 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alan Stern, Ray Chi,
Matthias Kaehlcke, Douglas Anderson, Michael Grzeschik,
Mathias Nyman, Flavio Suligoi, linux-phy, devicetree,
linux-kernel, linux-usb
> > I only control a register, it is not needed a driver of power domain.
>
> Aren't many power domains just a registers? What about other drivers?
> Don't you want in other driver control LDO of something else? And in other
> something else?
I will use power domain to instead this.
> > Would "port" be more appropriate?
> >
> > For example,
> > Using phy@0 and phy@1:
> > usb_port1_usb2phy: usb-phy@13c14 {
> > compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
> > reg = <0x132c4 0x4>, <0x31280 0x8>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > #phy-cells = <0>;
> > realtek,usb-ctrl = <&usb_ctrl>;
> >
> > phy@0 {
> > reg = <0>;
>
> So such child is a NAK... you have nothing here. But it's unrelated topic.
Here is for simple, so some items ignore.
> > };
> > phy@1 {
> > reg = <1>;
> > };
> > };
> >
> > Change: port@0 and port@1
> > usb_port1_usb2phy: usb-phy@13c14 {
> > compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
> > reg = <0x132c4 0x4>, <0x31280 0x8>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > #phy-cells = <0>;
> > realtek,usb-ctrl = <&usb_ctrl>;
> >
> > prot@0 {
> > reg = <0>;
> > };
> > port@1 {
> > reg = <1>;
> > };
> > };
>
> This is not the answer. This is the provider. How do you reference it from the
> consumer.
> Upstream your entire DTS. It's frustrating to try to understand your DTS from
> pieces of information you are sharing. Also very time consuming and you are
> not the only one sending patches for review...
Sorry to take up a lot of your time.
Apparently I don't know enough about dts.
I will reference more device tree document to understand the relating between DTS and hardware.
Thanks,
Stanley
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2023-06-08 9:28 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-07 6:24 [PATCH v3 1/5] usb: phy: add usb phy notify port status API Stanley Chang
2023-06-07 6:24 ` [PATCH v3 2/5] phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY Stanley Chang
2023-06-07 11:26 ` kernel test robot
2023-06-07 11:54 ` Krzysztof Kozlowski
2023-06-08 6:59 ` Stanley Chang[昌育德]
2023-06-08 7:00 ` Krzysztof Kozlowski
2023-06-08 7:15 ` Krzysztof Kozlowski
2023-06-07 17:31 ` kernel test robot
2023-06-08 2:18 ` Stanley Chang[昌育德]
2023-06-08 6:55 ` Krzysztof Kozlowski
2023-06-08 7:07 ` Stanley Chang[昌育德]
2023-06-07 6:24 ` [PATCH v3 3/5] phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY Stanley Chang
2023-06-07 11:52 ` Krzysztof Kozlowski
2023-06-08 6:59 ` Stanley Chang[昌育德]
2023-06-08 7:21 ` Krzysztof Kozlowski
2023-06-08 7:40 ` Stanley Chang[昌育德]
2023-06-08 1:35 ` kernel test robot
2023-06-08 2:18 ` Stanley Chang[昌育德]
2023-06-07 6:24 ` [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY Stanley Chang
2023-06-07 12:04 ` Krzysztof Kozlowski
2023-06-08 7:24 ` Stanley Chang[昌育德]
2023-06-08 7:49 ` Krzysztof Kozlowski
2023-06-08 8:21 ` Stanley Chang[昌育德]
2023-06-08 8:28 ` Krzysztof Kozlowski
2023-06-08 9:27 ` Stanley Chang[昌育德]
2023-06-08 7:47 ` Stanley Chang[昌育德]
2023-06-08 7:51 ` Krzysztof Kozlowski
2023-06-08 8:01 ` Stanley Chang[昌育德]
2023-06-07 6:24 ` [PATCH v3 5/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 3.0 PHY Stanley Chang
2023-06-07 12:08 ` Krzysztof Kozlowski
2023-06-08 7:32 ` Stanley Chang[昌育德]
2023-06-08 7:50 ` Krzysztof Kozlowski
2023-06-08 8: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).