[PATCH v6 1/3] phy: mediatek: fix build warning caused by clang

[PATCH v6 1/3] phy: mediatek: fix build warning caused by clang

[Chunfeng Yun]

Remove the temporary @mask_, this may cause build warning when use clang
compiler for powerpc, but can't reproduce it when compile for arm64.
the build warning is -Wtautological-constant-out-of-range-compare, and
caused by
"BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask)"

After removing @mask_, there is a "CHECK:MACRO_ARG_REUSE" when run
checkpatch.pl, due to @mask is constant, no reuse problem will happen.

Fixes: 84513eccd678 ("phy: mediatek: fix build warning of FIELD_PREP()")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v6: modify the title
v5: no changes
v4 new patch, I'm not sure it can fix build warning, due to I don't cross compile
    it for powerpc using clang in office.
---
 drivers/phy/mediatek/phy-mtk-io.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/mediatek/phy-mtk-io.h b/drivers/phy/mediatek/phy-mtk-io.h
index d20ad5e5be81..58f06db822cb 100644
--- a/drivers/phy/mediatek/phy-mtk-io.h
+++ b/drivers/phy/mediatek/phy-mtk-io.h
@@ -39,8 +39,8 @@ static inline void mtk_phy_update_bits(void __iomem *reg, u32 mask, u32 val)
 /* field @mask shall be constant and continuous */
 #define mtk_phy_update_field(reg, mask, val) \
 ({ \
-	typeof(mask) mask_ = (mask);	\
-	mtk_phy_update_bits(reg, mask_, FIELD_PREP(mask_, val)); \
+	BUILD_BUG_ON_MSG(!__builtin_constant_p(mask), "mask is not constant"); \
+	mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); \
 })
 
 #endif
-- 
2.18.0

[PATCH v6 2/3] phy: core: add debugfs root

[Chunfeng Yun]

Add a debugfs root for phy class, then phy drivers can add debugfs files
under this folder.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2~v6: no changes
---
 drivers/phy/phy-core.c  | 6 ++++++
 include/linux/phy/phy.h | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index d93ddf1262c5..2f9f69190519 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -11,6 +11,7 @@
 #include <linux/export.h>
 #include <linux/module.h>
 #include <linux/err.h>
+#include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/slab.h>
 #include <linux/of.h>
@@ -1204,6 +1205,9 @@ void devm_of_phy_provider_unregister(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_of_phy_provider_unregister);
 
+struct dentry *phy_debug_root;
+EXPORT_SYMBOL_GPL(phy_debug_root);
+
 /**
  * phy_release() - release the phy
  * @dev: the dev member within phy
@@ -1233,6 +1237,8 @@ static int __init phy_core_init(void)
 
 	phy_class->dev_release = phy_release;
 
+	phy_debug_root = debugfs_create_dir("phy", NULL);
+
 	return 0;
 }
 device_initcall(phy_core_init);
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index b1413757fcc3..c398749d49b9 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -205,6 +205,8 @@ struct phy_lookup {
 #define devm_of_phy_provider_register_full(dev, children, xlate) \
 	__devm_of_phy_provider_register(dev, children, THIS_MODULE, xlate)
 
+extern struct dentry *phy_debug_root;
+
 static inline void phy_set_drvdata(struct phy *phy, void *data)
 {
 	dev_set_drvdata(&phy->dev, data);
-- 
2.18.0

[PATCH v6 3/3] phy: mediatek: tphy: add debugfs files

[Chunfeng Yun]

These debugfs files are mainly used to make eye diagram test easier,
especially helpful to do HQA test for a new IC without efuse enabled.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v6: no changes

v5: using common debugfs config CONFIG_DEBUG_FS

v4: fix build warning of sometimes uninitialized variable

v3: fix typo of "debugfs" suggested by AngeloGioacchino

v2: add CONFIG_PHY_MTK_TPHY_DEBUGFS suggested by AngeloGioacchino
---
 drivers/phy/mediatek/phy-mtk-tphy.c | 405 +++++++++++++++++++++++++++-
 1 file changed, 404 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c b/drivers/phy/mediatek/phy-mtk-tphy.c
index e906a82791bd..923e5ee119f3 100644
--- a/drivers/phy/mediatek/phy-mtk-tphy.c
+++ b/drivers/phy/mediatek/phy-mtk-tphy.c
@@ -7,6 +7,7 @@
 
 #include <dt-bindings/phy/phy.h>
 #include <linux/clk.h>
+#include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/iopoll.h>
 #include <linux/mfd/syscon.h>
@@ -264,6 +265,8 @@
 
 #define TPHY_CLKS_CNT	2
 
+#define USER_BUF_LEN(count) min_t(size_t, 8, (count))
+
 enum mtk_phy_version {
 	MTK_PHY_V1 = 1,
 	MTK_PHY_V2,
@@ -310,6 +313,7 @@ struct mtk_phy_instance {
 	struct clk_bulk_data clks[TPHY_CLKS_CNT];
 	u32 index;
 	u32 type;
+	struct dentry *dbgfs;
 	struct regmap *type_sw;
 	u32 type_sw_reg;
 	u32 type_sw_index;
@@ -332,10 +336,391 @@ struct mtk_tphy {
 	const struct mtk_phy_pdata *pdata;
 	struct mtk_phy_instance **phys;
 	int nphys;
+	struct dentry *dbgfs_root;
 	int src_ref_clk; /* MHZ, reference clock for slew rate calibrate */
 	int src_coef; /* coefficient for slew rate calibrate */
 };
 
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+
+enum u2_phy_params {
+	U2P_EYE_VRT = 0,
+	U2P_EYE_TERM,
+	U2P_EFUSE_EN,
+	U2P_EFUSE_INTR,
+	U2P_DISCTH,
+	U2P_PRE_EMPHASIS,
+};
+
+enum u3_phy_params {
+	U3P_EFUSE_EN = 0,
+	U3P_EFUSE_INTR,
+	U3P_EFUSE_TX_IMP,
+	U3P_EFUSE_RX_IMP,
+};
+
+static const char *const u2_phy_files[] = {
+	[U2P_EYE_VRT] = "vrt",
+	[U2P_EYE_TERM] = "term",
+	[U2P_EFUSE_EN] = "efuse",
+	[U2P_EFUSE_INTR] = "intr",
+	[U2P_DISCTH] = "discth",
+	[U2P_PRE_EMPHASIS] = "preemph",
+};
+
+static const char *const u3_phy_files[] = {
+	[U3P_EFUSE_EN] = "efuse",
+	[U3P_EFUSE_INTR] = "intr",
+	[U3P_EFUSE_TX_IMP] = "tx-imp",
+	[U3P_EFUSE_RX_IMP] = "rx-imp",
+};
+
+static int u2_phy_params_show(struct seq_file *sf, void *unused)
+{
+	struct mtk_phy_instance *inst = sf->private;
+	const char *fname = file_dentry(sf->file)->d_iname;
+	struct u2phy_banks *u2_banks = &inst->u2_banks;
+	void __iomem *com = u2_banks->com;
+	u32 max = 0;
+	u32 tmp = 0;
+	u32 val = 0;
+	int ret;
+
+	ret = match_string(u2_phy_files, ARRAY_SIZE(u2_phy_files), fname);
+	if (ret < 0)
+		return ret;
+
+	switch (ret) {
+	case U2P_EYE_VRT:
+		tmp = readl(com + U3P_USBPHYACR1);
+		val = FIELD_GET(PA1_RG_VRT_SEL, tmp);
+		max = FIELD_MAX(PA1_RG_VRT_SEL);
+		break;
+
+	case U2P_EYE_TERM:
+		tmp = readl(com + U3P_USBPHYACR1);
+		val = FIELD_GET(PA1_RG_TERM_SEL, tmp);
+		max = FIELD_MAX(PA1_RG_TERM_SEL);
+		break;
+
+	case U2P_EFUSE_EN:
+		if (u2_banks->misc) {
+			tmp = readl(u2_banks->misc + U3P_MISC_REG1);
+			max = 1;
+		}
+
+		val = !!(tmp & MR1_EFUSE_AUTO_LOAD_DIS);
+		break;
+
+	case U2P_EFUSE_INTR:
+		tmp = readl(com + U3P_USBPHYACR1);
+		val = FIELD_GET(PA1_RG_INTR_CAL, tmp);
+		max = FIELD_MAX(PA1_RG_INTR_CAL);
+		break;
+
+	case U2P_DISCTH:
+		tmp = readl(com + U3P_USBPHYACR6);
+		val = FIELD_GET(PA6_RG_U2_DISCTH, tmp);
+		max = FIELD_MAX(PA6_RG_U2_DISCTH);
+		break;
+
+	case U2P_PRE_EMPHASIS:
+		tmp = readl(com + U3P_USBPHYACR6);
+		val = FIELD_GET(PA6_RG_U2_PRE_EMP, tmp);
+		max = FIELD_MAX(PA6_RG_U2_PRE_EMP);
+		break;
+
+	default:
+		seq_printf(sf, "invalid, %d\n", ret);
+		break;
+	}
+
+	seq_printf(sf, "%s : %d [0, %d]\n", fname, val, max);
+
+	return 0;
+}
+
+static int u2_phy_params_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, u2_phy_params_show, inode->i_private);
+}
+
+static ssize_t u2_phy_params_write(struct file *file, const char __user *ubuf,
+				   size_t count, loff_t *ppos)
+{
+	const char *fname = file_dentry(file)->d_iname;
+	struct seq_file *sf = file->private_data;
+	struct mtk_phy_instance *inst = sf->private;
+	struct u2phy_banks *u2_banks = &inst->u2_banks;
+	void __iomem *com = u2_banks->com;
+	ssize_t rc;
+	u32 val;
+	int ret;
+
+	rc = kstrtouint_from_user(ubuf, USER_BUF_LEN(count), 0, &val);
+	if (rc)
+		return rc;
+
+	ret = match_string(u2_phy_files, ARRAY_SIZE(u2_phy_files), fname);
+	if (ret < 0)
+		return (ssize_t)ret;
+
+	switch (ret) {
+	case U2P_EYE_VRT:
+		mtk_phy_update_field(com + U3P_USBPHYACR1, PA1_RG_VRT_SEL, val);
+		break;
+
+	case U2P_EYE_TERM:
+		mtk_phy_update_field(com + U3P_USBPHYACR1, PA1_RG_TERM_SEL, val);
+		break;
+
+	case U2P_EFUSE_EN:
+		if (u2_banks->misc)
+			mtk_phy_update_field(u2_banks->misc + U3P_MISC_REG1,
+					     MR1_EFUSE_AUTO_LOAD_DIS, !!val);
+		break;
+
+	case U2P_EFUSE_INTR:
+		mtk_phy_update_field(com + U3P_USBPHYACR1, PA1_RG_INTR_CAL, val);
+		break;
+
+	case U2P_DISCTH:
+		mtk_phy_update_field(com + U3P_USBPHYACR6, PA6_RG_U2_DISCTH, val);
+		break;
+
+	case U2P_PRE_EMPHASIS:
+		mtk_phy_update_field(com + U3P_USBPHYACR6, PA6_RG_U2_PRE_EMP, val);
+		break;
+
+	default:
+		break;
+	}
+
+	return count;
+}
+
+static const struct file_operations u2_phy_fops = {
+	.open = u2_phy_params_open,
+	.write = u2_phy_params_write,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+static void u2_phy_dbgfs_files_create(struct mtk_phy_instance *inst)
+{
+	u32 count = ARRAY_SIZE(u2_phy_files);
+	int i;
+
+	for (i = 0; i < count; i++)
+		debugfs_create_file(u2_phy_files[i], 0644, inst->dbgfs, inst, &u2_phy_fops);
+}
+
+static int u3_phy_params_show(struct seq_file *sf, void *unused)
+{
+	struct mtk_phy_instance *inst = sf->private;
+	const char *fname = file_dentry(sf->file)->d_iname;
+	struct u3phy_banks *u3_banks = &inst->u3_banks;
+	u32 val = 0;
+	u32 max = 0;
+	u32 tmp;
+	int ret;
+
+	ret = match_string(u3_phy_files, ARRAY_SIZE(u3_phy_files), fname);
+	if (ret < 0)
+		return ret;
+
+	switch (ret) {
+	case U3P_EFUSE_EN:
+		tmp = readl(u3_banks->phyd + U3P_U3_PHYD_RSV);
+		val = !!(tmp & P3D_RG_EFUSE_AUTO_LOAD_DIS);
+		max = 1;
+		break;
+
+	case U3P_EFUSE_INTR:
+		tmp = readl(u3_banks->phya + U3P_U3_PHYA_REG0);
+		val = FIELD_GET(P3A_RG_IEXT_INTR, tmp);
+		max = FIELD_MAX(P3A_RG_IEXT_INTR);
+		break;
+
+	case U3P_EFUSE_TX_IMP:
+		tmp = readl(u3_banks->phyd + U3P_U3_PHYD_IMPCAL0);
+		val = FIELD_GET(P3D_RG_TX_IMPEL, tmp);
+		max = FIELD_MAX(P3D_RG_TX_IMPEL);
+		break;
+
+	case U3P_EFUSE_RX_IMP:
+		tmp = readl(u3_banks->phyd + U3P_U3_PHYD_IMPCAL1);
+		val = FIELD_GET(P3D_RG_RX_IMPEL, tmp);
+		max = FIELD_MAX(P3D_RG_RX_IMPEL);
+		break;
+
+	default:
+		seq_printf(sf, "invalid, %d\n", ret);
+		break;
+	}
+
+	seq_printf(sf, "%s : %d [0, %d]\n", fname, val, max);
+
+	return 0;
+}
+
+static int u3_phy_params_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, u3_phy_params_show, inode->i_private);
+}
+
+static ssize_t u3_phy_params_write(struct file *file, const char __user *ubuf,
+				   size_t count, loff_t *ppos)
+{
+	const char *fname = file_dentry(file)->d_iname;
+	struct seq_file *sf = file->private_data;
+	struct mtk_phy_instance *inst = sf->private;
+	struct u3phy_banks *u3_banks = &inst->u3_banks;
+	void __iomem *phyd = u3_banks->phyd;
+	ssize_t rc;
+	u32 val;
+	int ret;
+
+	rc = kstrtouint_from_user(ubuf, USER_BUF_LEN(count), 0, &val);
+	if (rc)
+		return rc;
+
+	ret = match_string(u3_phy_files, ARRAY_SIZE(u3_phy_files), fname);
+	if (ret < 0)
+		return (ssize_t)ret;
+
+	switch (ret) {
+	case U3P_EFUSE_EN:
+		mtk_phy_update_field(phyd + U3P_U3_PHYD_RSV,
+				     P3D_RG_EFUSE_AUTO_LOAD_DIS, !!val);
+		break;
+
+	case U3P_EFUSE_INTR:
+		mtk_phy_update_field(u3_banks->phya + U3P_U3_PHYA_REG0, P3A_RG_IEXT_INTR, val);
+		break;
+
+	case U3P_EFUSE_TX_IMP:
+		mtk_phy_update_field(phyd + U3P_U3_PHYD_IMPCAL0, P3D_RG_TX_IMPEL, val);
+		mtk_phy_set_bits(phyd + U3P_U3_PHYD_IMPCAL0, P3D_RG_FORCE_TX_IMPEL);
+		break;
+
+	case U3P_EFUSE_RX_IMP:
+		mtk_phy_update_field(phyd + U3P_U3_PHYD_IMPCAL1, P3D_RG_RX_IMPEL, val);
+		mtk_phy_set_bits(phyd + U3P_U3_PHYD_IMPCAL1, P3D_RG_FORCE_RX_IMPEL);
+		break;
+
+	default:
+		break;
+	}
+
+	return count;
+}
+
+static const struct file_operations u3_phy_fops = {
+	.open = u3_phy_params_open,
+	.write = u3_phy_params_write,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+static void u3_phy_dbgfs_files_create(struct mtk_phy_instance *inst)
+{
+	u32 count = ARRAY_SIZE(u3_phy_files);
+	int i;
+
+	for (i = 0; i < count; i++)
+		debugfs_create_file(u3_phy_files[i], 0644, inst->dbgfs, inst, &u3_phy_fops);
+}
+
+static int tphy_type_show(struct seq_file *sf, void *unused)
+{
+	struct mtk_phy_instance *inst = sf->private;
+	const char *type;
+
+	switch (inst->type) {
+	case PHY_TYPE_USB2:
+		type = "USB2";
+		break;
+	case PHY_TYPE_USB3:
+		type = "USB3";
+		break;
+	case PHY_TYPE_PCIE:
+		type = "PCIe";
+		break;
+	case PHY_TYPE_SGMII:
+		type = "SGMII";
+		break;
+	case PHY_TYPE_SATA:
+		type = "SATA";
+		break;
+	default:
+		type = "";
+	}
+
+	seq_printf(sf, "%s\n", type);
+
+	return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(tphy_type);
+
+static void tphy_debugfs_init(struct mtk_tphy *tphy, struct mtk_phy_instance *inst)
+{
+	char name[16];
+
+	snprintf(name, sizeof(name) - 1, "phy.%d", inst->index);
+	inst->dbgfs = debugfs_create_dir(name, tphy->dbgfs_root);
+
+	debugfs_create_file("type", 0444, inst->dbgfs, inst, &tphy_type_fops);
+
+	switch (inst->type) {
+	case PHY_TYPE_USB2:
+		u2_phy_dbgfs_files_create(inst);
+		break;
+	case PHY_TYPE_USB3:
+	case PHY_TYPE_PCIE:
+		u3_phy_dbgfs_files_create(inst);
+		break;
+	default:
+		break;
+	}
+}
+
+static void tphy_debugfs_exit(struct mtk_phy_instance *inst)
+{
+	debugfs_remove_recursive(inst->dbgfs);
+	inst->dbgfs = NULL;
+}
+
+static void tphy_debugfs_root_create(struct mtk_tphy *tphy)
+{
+	tphy->dbgfs_root = debugfs_create_dir(dev_name(tphy->dev), phy_debug_root);
+}
+
+static void tphy_debugfs_root_remove(struct mtk_tphy *tphy)
+{
+	debugfs_remove_recursive(tphy->dbgfs_root);
+	tphy->dbgfs_root = NULL;
+}
+
+#else
+
+static void tphy_debugfs_init(struct mtk_tphy *tphy, struct mtk_phy_instance *inst)
+{}
+
+static void tphy_debugfs_exit(struct mtk_phy_instance *inst)
+{}
+
+static void tphy_debugfs_root_create(struct mtk_tphy *tphy)
+{}
+
+static void tphy_debugfs_root_remove(struct mtk_tphy *tphy)
+{}
+
+#endif
+
 static void hs_slew_rate_calibrate(struct mtk_tphy *tphy,
 	struct mtk_phy_instance *instance)
 {
@@ -1032,6 +1417,8 @@ static int mtk_phy_init(struct phy *phy)
 		return -EINVAL;
 	}
 
+	tphy_debugfs_init(tphy, instance);
+
 	return 0;
 }
 
@@ -1068,6 +1455,8 @@ static int mtk_phy_exit(struct phy *phy)
 	struct mtk_phy_instance *instance = phy_get_drvdata(phy);
 	struct mtk_tphy *tphy = dev_get_drvdata(phy->dev.parent);
 
+	tphy_debugfs_exit(instance);
+
 	if (instance->type == PHY_TYPE_USB2)
 		u2_phy_instance_exit(tphy, instance);
 
@@ -1295,15 +1684,29 @@ static int mtk_tphy_probe(struct platform_device *pdev)
 	}
 
 	provider = devm_of_phy_provider_register(dev, mtk_phy_xlate);
+	if (IS_ERR(provider))
+		return dev_err_probe(dev, PTR_ERR(provider), "probe failed\n");
+
+	tphy_debugfs_root_create(tphy);
+	return 0;
 
-	return PTR_ERR_OR_ZERO(provider);
 put_child:
 	of_node_put(child_np);
 	return retval;
 }
 
+static int mtk_tphy_remove(struct platform_device *pdev)
+{
+	struct mtk_tphy *tphy;
+
+	tphy = platform_get_drvdata(pdev);
+	tphy_debugfs_root_remove(tphy);
+	return 0;
+}
+
 static struct platform_driver mtk_tphy_driver = {
 	.probe		= mtk_tphy_probe,
+	.remove		= mtk_tphy_remove,
 	.driver		= {
 		.name	= "mtk-tphy",
 		.of_match_table = mtk_tphy_id_table,
-- 
2.18.0

Re: [PATCH v6 3/3] phy: mediatek: tphy: add debugfs files

[Vinod Koul]

On 04-01-23, 21:26, Chunfeng Yun wrote:
> These debugfs files are mainly used to make eye diagram test easier,
> especially helpful to do HQA test for a new IC without efuse enabled.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v6: no changes
> 
> v5: using common debugfs config CONFIG_DEBUG_FS
> 
> v4: fix build warning of sometimes uninitialized variable
> 
> v3: fix typo of "debugfs" suggested by AngeloGioacchino
> 
> v2: add CONFIG_PHY_MTK_TPHY_DEBUGFS suggested by AngeloGioacchino
> ---
>  drivers/phy/mediatek/phy-mtk-tphy.c | 405 +++++++++++++++++++++++++++-
>  1 file changed, 404 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c b/drivers/phy/mediatek/phy-mtk-tphy.c
> index e906a82791bd..923e5ee119f3 100644
> --- a/drivers/phy/mediatek/phy-mtk-tphy.c
> +++ b/drivers/phy/mediatek/phy-mtk-tphy.c
> @@ -7,6 +7,7 @@
>  
>  #include <dt-bindings/phy/phy.h>
>  #include <linux/clk.h>
> +#include <linux/debugfs.h>
>  #include <linux/delay.h>
>  #include <linux/iopoll.h>
>  #include <linux/mfd/syscon.h>
> @@ -264,6 +265,8 @@
>  
>  #define TPHY_CLKS_CNT	2
>  
> +#define USER_BUF_LEN(count) min_t(size_t, 8, (count))
> +
>  enum mtk_phy_version {
>  	MTK_PHY_V1 = 1,
>  	MTK_PHY_V2,
> @@ -310,6 +313,7 @@ struct mtk_phy_instance {
>  	struct clk_bulk_data clks[TPHY_CLKS_CNT];
>  	u32 index;
>  	u32 type;
> +	struct dentry *dbgfs;
>  	struct regmap *type_sw;
>  	u32 type_sw_reg;
>  	u32 type_sw_index;
> @@ -332,10 +336,391 @@ struct mtk_tphy {
>  	const struct mtk_phy_pdata *pdata;
>  	struct mtk_phy_instance **phys;
>  	int nphys;
> +	struct dentry *dbgfs_root;
>  	int src_ref_clk; /* MHZ, reference clock for slew rate calibrate */
>  	int src_coef; /* coefficient for slew rate calibrate */
>  };
>  
> +#if IS_ENABLED(CONFIG_DEBUG_FS)
> +
> +enum u2_phy_params {
> +	U2P_EYE_VRT = 0,
> +	U2P_EYE_TERM,
> +	U2P_EFUSE_EN,
> +	U2P_EFUSE_INTR,
> +	U2P_DISCTH,
> +	U2P_PRE_EMPHASIS,
> +};
> +
> +enum u3_phy_params {
> +	U3P_EFUSE_EN = 0,
> +	U3P_EFUSE_INTR,
> +	U3P_EFUSE_TX_IMP,
> +	U3P_EFUSE_RX_IMP,
> +};
> +
> +static const char *const u2_phy_files[] = {
> +	[U2P_EYE_VRT] = "vrt",
> +	[U2P_EYE_TERM] = "term",
> +	[U2P_EFUSE_EN] = "efuse",
> +	[U2P_EFUSE_INTR] = "intr",
> +	[U2P_DISCTH] = "discth",
> +	[U2P_PRE_EMPHASIS] = "preemph",
> +};
> +
> +static const char *const u3_phy_files[] = {
> +	[U3P_EFUSE_EN] = "efuse",
> +	[U3P_EFUSE_INTR] = "intr",
> +	[U3P_EFUSE_TX_IMP] = "tx-imp",
> +	[U3P_EFUSE_RX_IMP] = "rx-imp",
> +};
> +
> +static int u2_phy_params_show(struct seq_file *sf, void *unused)
> +{
> +	struct mtk_phy_instance *inst = sf->private;
> +	const char *fname = file_dentry(sf->file)->d_iname;
> +	struct u2phy_banks *u2_banks = &inst->u2_banks;
> +	void __iomem *com = u2_banks->com;
> +	u32 max = 0;
> +	u32 tmp = 0;
> +	u32 val = 0;
> +	int ret;
> +
> +	ret = match_string(u2_phy_files, ARRAY_SIZE(u2_phy_files), fname);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (ret) {
> +	case U2P_EYE_VRT:
> +		tmp = readl(com + U3P_USBPHYACR1);
> +		val = FIELD_GET(PA1_RG_VRT_SEL, tmp);
> +		max = FIELD_MAX(PA1_RG_VRT_SEL);
> +		break;
> +
> +	case U2P_EYE_TERM:
> +		tmp = readl(com + U3P_USBPHYACR1);
> +		val = FIELD_GET(PA1_RG_TERM_SEL, tmp);
> +		max = FIELD_MAX(PA1_RG_TERM_SEL);
> +		break;
> +
> +	case U2P_EFUSE_EN:
> +		if (u2_banks->misc) {
> +			tmp = readl(u2_banks->misc + U3P_MISC_REG1);
> +			max = 1;
> +		}
> +
> +		val = !!(tmp & MR1_EFUSE_AUTO_LOAD_DIS);
> +		break;
> +
> +	case U2P_EFUSE_INTR:
> +		tmp = readl(com + U3P_USBPHYACR1);
> +		val = FIELD_GET(PA1_RG_INTR_CAL, tmp);
> +		max = FIELD_MAX(PA1_RG_INTR_CAL);
> +		break;
> +
> +	case U2P_DISCTH:
> +		tmp = readl(com + U3P_USBPHYACR6);
> +		val = FIELD_GET(PA6_RG_U2_DISCTH, tmp);
> +		max = FIELD_MAX(PA6_RG_U2_DISCTH);
> +		break;
> +
> +	case U2P_PRE_EMPHASIS:
> +		tmp = readl(com + U3P_USBPHYACR6);
> +		val = FIELD_GET(PA6_RG_U2_PRE_EMP, tmp);
> +		max = FIELD_MAX(PA6_RG_U2_PRE_EMP);
> +		break;
> +
> +	default:
> +		seq_printf(sf, "invalid, %d\n", ret);
> +		break;
> +	}
> +
> +	seq_printf(sf, "%s : %d [0, %d]\n", fname, val, max);
> +
> +	return 0;
> +}
> +
> +static int u2_phy_params_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, u2_phy_params_show, inode->i_private);
> +}
> +
> +static ssize_t u2_phy_params_write(struct file *file, const char __user *ubuf,
> +				   size_t count, loff_t *ppos)
> +{
> +	const char *fname = file_dentry(file)->d_iname;
> +	struct seq_file *sf = file->private_data;
> +	struct mtk_phy_instance *inst = sf->private;
> +	struct u2phy_banks *u2_banks = &inst->u2_banks;
> +	void __iomem *com = u2_banks->com;
> +	ssize_t rc;
> +	u32 val;
> +	int ret;
> +
> +	rc = kstrtouint_from_user(ubuf, USER_BUF_LEN(count), 0, &val);
> +	if (rc)
> +		return rc;
> +
> +	ret = match_string(u2_phy_files, ARRAY_SIZE(u2_phy_files), fname);
> +	if (ret < 0)
> +		return (ssize_t)ret;
> +
> +	switch (ret) {
> +	case U2P_EYE_VRT:
> +		mtk_phy_update_field(com + U3P_USBPHYACR1, PA1_RG_VRT_SEL, val);
> +		break;
> +
> +	case U2P_EYE_TERM:
> +		mtk_phy_update_field(com + U3P_USBPHYACR1, PA1_RG_TERM_SEL, val);
> +		break;
> +
> +	case U2P_EFUSE_EN:
> +		if (u2_banks->misc)
> +			mtk_phy_update_field(u2_banks->misc + U3P_MISC_REG1,
> +					     MR1_EFUSE_AUTO_LOAD_DIS, !!val);
> +		break;
> +
> +	case U2P_EFUSE_INTR:
> +		mtk_phy_update_field(com + U3P_USBPHYACR1, PA1_RG_INTR_CAL, val);
> +		break;
> +
> +	case U2P_DISCTH:
> +		mtk_phy_update_field(com + U3P_USBPHYACR6, PA6_RG_U2_DISCTH, val);
> +		break;
> +
> +	case U2P_PRE_EMPHASIS:
> +		mtk_phy_update_field(com + U3P_USBPHYACR6, PA6_RG_U2_PRE_EMP, val);
> +		break;
> +
> +	default:
> +		break;
> +	}
> +
> +	return count;
> +}
> +
> +static const struct file_operations u2_phy_fops = {
> +	.open = u2_phy_params_open,
> +	.write = u2_phy_params_write,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +};
> +
> +static void u2_phy_dbgfs_files_create(struct mtk_phy_instance *inst)
> +{
> +	u32 count = ARRAY_SIZE(u2_phy_files);
> +	int i;
> +
> +	for (i = 0; i < count; i++)
> +		debugfs_create_file(u2_phy_files[i], 0644, inst->dbgfs, inst, &u2_phy_fops);
> +}
> +
> +static int u3_phy_params_show(struct seq_file *sf, void *unused)
> +{
> +	struct mtk_phy_instance *inst = sf->private;
> +	const char *fname = file_dentry(sf->file)->d_iname;
> +	struct u3phy_banks *u3_banks = &inst->u3_banks;
> +	u32 val = 0;
> +	u32 max = 0;
> +	u32 tmp;
> +	int ret;
> +
> +	ret = match_string(u3_phy_files, ARRAY_SIZE(u3_phy_files), fname);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (ret) {
> +	case U3P_EFUSE_EN:
> +		tmp = readl(u3_banks->phyd + U3P_U3_PHYD_RSV);
> +		val = !!(tmp & P3D_RG_EFUSE_AUTO_LOAD_DIS);
> +		max = 1;
> +		break;
> +
> +	case U3P_EFUSE_INTR:
> +		tmp = readl(u3_banks->phya + U3P_U3_PHYA_REG0);
> +		val = FIELD_GET(P3A_RG_IEXT_INTR, tmp);
> +		max = FIELD_MAX(P3A_RG_IEXT_INTR);
> +		break;
> +
> +	case U3P_EFUSE_TX_IMP:
> +		tmp = readl(u3_banks->phyd + U3P_U3_PHYD_IMPCAL0);
> +		val = FIELD_GET(P3D_RG_TX_IMPEL, tmp);
> +		max = FIELD_MAX(P3D_RG_TX_IMPEL);
> +		break;
> +
> +	case U3P_EFUSE_RX_IMP:
> +		tmp = readl(u3_banks->phyd + U3P_U3_PHYD_IMPCAL1);
> +		val = FIELD_GET(P3D_RG_RX_IMPEL, tmp);
> +		max = FIELD_MAX(P3D_RG_RX_IMPEL);
> +		break;
> +
> +	default:
> +		seq_printf(sf, "invalid, %d\n", ret);
> +		break;
> +	}
> +
> +	seq_printf(sf, "%s : %d [0, %d]\n", fname, val, max);
> +
> +	return 0;
> +}
> +
> +static int u3_phy_params_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, u3_phy_params_show, inode->i_private);
> +}
> +
> +static ssize_t u3_phy_params_write(struct file *file, const char __user *ubuf,
> +				   size_t count, loff_t *ppos)
> +{
> +	const char *fname = file_dentry(file)->d_iname;
> +	struct seq_file *sf = file->private_data;
> +	struct mtk_phy_instance *inst = sf->private;
> +	struct u3phy_banks *u3_banks = &inst->u3_banks;
> +	void __iomem *phyd = u3_banks->phyd;
> +	ssize_t rc;
> +	u32 val;
> +	int ret;
> +
> +	rc = kstrtouint_from_user(ubuf, USER_BUF_LEN(count), 0, &val);
> +	if (rc)
> +		return rc;
> +
> +	ret = match_string(u3_phy_files, ARRAY_SIZE(u3_phy_files), fname);
> +	if (ret < 0)
> +		return (ssize_t)ret;
> +
> +	switch (ret) {
> +	case U3P_EFUSE_EN:
> +		mtk_phy_update_field(phyd + U3P_U3_PHYD_RSV,
> +				     P3D_RG_EFUSE_AUTO_LOAD_DIS, !!val);
> +		break;
> +
> +	case U3P_EFUSE_INTR:
> +		mtk_phy_update_field(u3_banks->phya + U3P_U3_PHYA_REG0, P3A_RG_IEXT_INTR, val);
> +		break;
> +
> +	case U3P_EFUSE_TX_IMP:
> +		mtk_phy_update_field(phyd + U3P_U3_PHYD_IMPCAL0, P3D_RG_TX_IMPEL, val);
> +		mtk_phy_set_bits(phyd + U3P_U3_PHYD_IMPCAL0, P3D_RG_FORCE_TX_IMPEL);
> +		break;
> +
> +	case U3P_EFUSE_RX_IMP:
> +		mtk_phy_update_field(phyd + U3P_U3_PHYD_IMPCAL1, P3D_RG_RX_IMPEL, val);
> +		mtk_phy_set_bits(phyd + U3P_U3_PHYD_IMPCAL1, P3D_RG_FORCE_RX_IMPEL);
> +		break;
> +
> +	default:
> +		break;
> +	}
> +
> +	return count;
> +}
> +
> +static const struct file_operations u3_phy_fops = {
> +	.open = u3_phy_params_open,
> +	.write = u3_phy_params_write,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +};
> +
> +static void u3_phy_dbgfs_files_create(struct mtk_phy_instance *inst)
> +{
> +	u32 count = ARRAY_SIZE(u3_phy_files);
> +	int i;
> +
> +	for (i = 0; i < count; i++)
> +		debugfs_create_file(u3_phy_files[i], 0644, inst->dbgfs, inst, &u3_phy_fops);
> +}
> +
> +static int tphy_type_show(struct seq_file *sf, void *unused)
> +{
> +	struct mtk_phy_instance *inst = sf->private;
> +	const char *type;
> +
> +	switch (inst->type) {
> +	case PHY_TYPE_USB2:
> +		type = "USB2";
> +		break;
> +	case PHY_TYPE_USB3:
> +		type = "USB3";
> +		break;
> +	case PHY_TYPE_PCIE:
> +		type = "PCIe";
> +		break;
> +	case PHY_TYPE_SGMII:
> +		type = "SGMII";
> +		break;
> +	case PHY_TYPE_SATA:
> +		type = "SATA";
> +		break;
> +	default:
> +		type = "";
> +	}
> +
> +	seq_printf(sf, "%s\n", type);
> +
> +	return 0;
> +}
> +
> +DEFINE_SHOW_ATTRIBUTE(tphy_type);
> +
> +static void tphy_debugfs_init(struct mtk_tphy *tphy, struct mtk_phy_instance *inst)
> +{
> +	char name[16];
> +
> +	snprintf(name, sizeof(name) - 1, "phy.%d", inst->index);

I wouold suggest driver name/ device name rather than phy.foo... again
folks needs to see what is foo


> +	inst->dbgfs = debugfs_create_dir(name, tphy->dbgfs_root);
> +
> +	debugfs_create_file("type", 0444, inst->dbgfs, inst, &tphy_type_fops);
> +
> +	switch (inst->type) {
> +	case PHY_TYPE_USB2:
> +		u2_phy_dbgfs_files_create(inst);
> +		break;
> +	case PHY_TYPE_USB3:
> +	case PHY_TYPE_PCIE:
> +		u3_phy_dbgfs_files_create(inst);
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
> +static void tphy_debugfs_exit(struct mtk_phy_instance *inst)
> +{
> +	debugfs_remove_recursive(inst->dbgfs);
> +	inst->dbgfs = NULL;
> +}
> +
> +static void tphy_debugfs_root_create(struct mtk_tphy *tphy)
> +{
> +	tphy->dbgfs_root = debugfs_create_dir(dev_name(tphy->dev), phy_debug_root);
> +}
> +
> +static void tphy_debugfs_root_remove(struct mtk_tphy *tphy)
> +{
> +	debugfs_remove_recursive(tphy->dbgfs_root);
> +	tphy->dbgfs_root = NULL;
> +}
> +
> +#else
> +
> +static void tphy_debugfs_init(struct mtk_tphy *tphy, struct mtk_phy_instance *inst)
> +{}
> +
> +static void tphy_debugfs_exit(struct mtk_phy_instance *inst)
> +{}
> +
> +static void tphy_debugfs_root_create(struct mtk_tphy *tphy)
> +{}
> +
> +static void tphy_debugfs_root_remove(struct mtk_tphy *tphy)
> +{}
> +
> +#endif
> +
>  static void hs_slew_rate_calibrate(struct mtk_tphy *tphy,
>  	struct mtk_phy_instance *instance)
>  {
> @@ -1032,6 +1417,8 @@ static int mtk_phy_init(struct phy *phy)
>  		return -EINVAL;
>  	}
>  
> +	tphy_debugfs_init(tphy, instance);
> +
>  	return 0;
>  }
>  
> @@ -1068,6 +1455,8 @@ static int mtk_phy_exit(struct phy *phy)
>  	struct mtk_phy_instance *instance = phy_get_drvdata(phy);
>  	struct mtk_tphy *tphy = dev_get_drvdata(phy->dev.parent);
>  
> +	tphy_debugfs_exit(instance);
> +
>  	if (instance->type == PHY_TYPE_USB2)
>  		u2_phy_instance_exit(tphy, instance);
>  
> @@ -1295,15 +1684,29 @@ static int mtk_tphy_probe(struct platform_device *pdev)
>  	}
>  
>  	provider = devm_of_phy_provider_register(dev, mtk_phy_xlate);
> +	if (IS_ERR(provider))
> +		return dev_err_probe(dev, PTR_ERR(provider), "probe failed\n");
> +
> +	tphy_debugfs_root_create(tphy);
> +	return 0;
>  
> -	return PTR_ERR_OR_ZERO(provider);
>  put_child:
>  	of_node_put(child_np);
>  	return retval;
>  }
>  
> +static int mtk_tphy_remove(struct platform_device *pdev)
> +{
> +	struct mtk_tphy *tphy;
> +
> +	tphy = platform_get_drvdata(pdev);
> +	tphy_debugfs_root_remove(tphy);
> +	return 0;
> +}
> +
>  static struct platform_driver mtk_tphy_driver = {
>  	.probe		= mtk_tphy_probe,
> +	.remove		= mtk_tphy_remove,
>  	.driver		= {
>  		.name	= "mtk-tphy",
>  		.of_match_table = mtk_tphy_id_table,
> -- 
> 2.18.0

-- 
~Vinod

Re: [PATCH v6 3/3] phy: mediatek: tphy: add debugfs files

[Chunfeng Yun (云春峰)]

On Fri, 2023-01-13 at 23:45 +0530, Vinod Koul wrote:
> On 04-01-23, 21:26, Chunfeng Yun wrote:
> > These debugfs files are mainly used to make eye diagram test
> > easier,
> > especially helpful to do HQA test for a new IC without efuse
> > enabled.
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v6: no changes
> > 
> > v5: using common debugfs config CONFIG_DEBUG_FS
> > 
> > v4: fix build warning of sometimes uninitialized variable
> > 
> > v3: fix typo of "debugfs" suggested by AngeloGioacchino
> > 
> > v2: add CONFIG_PHY_MTK_TPHY_DEBUGFS suggested by AngeloGioacchino
> > ---
> >  drivers/phy/mediatek/phy-mtk-tphy.c | 405
> > +++++++++++++++++++++++++++-
> >  1 file changed, 404 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c
> > b/drivers/phy/mediatek/phy-mtk-tphy.c
> > index e906a82791bd..923e5ee119f3 100644
> > --- a/drivers/phy/mediatek/phy-mtk-tphy.c
> > +++ b/drivers/phy/mediatek/phy-mtk-tphy.c
> > @@ -7,6 +7,7 @@
> >  
> >  #include <dt-bindings/phy/phy.h>
> >  #include <linux/clk.h>
> > +#include <linux/debugfs.h>
> >  #include <linux/delay.h>
> >  #include <linux/iopoll.h>
> >  #include <linux/mfd/syscon.h>
> > @@ -264,6 +265,8 @@
> >  
> >  #define TPHY_CLKS_CNT	2
> >  
> > +#define USER_BUF_LEN(count) min_t(size_t, 8, (count))
> > +
> >  enum mtk_phy_version {
> >  	MTK_PHY_V1 = 1,
> >  	MTK_PHY_V2,
> > @@ -310,6 +313,7 @@ struct mtk_phy_instance {
> >  	struct clk_bulk_data clks[TPHY_CLKS_CNT];
> >  	u32 index;
> >  	u32 type;
> > +	struct dentry *dbgfs;
> >  	struct regmap *type_sw;
> >  	u32 type_sw_reg;
> >  	u32 type_sw_index;
> > @@ -332,10 +336,391 @@ struct mtk_tphy {
> >  	const struct mtk_phy_pdata *pdata;
> >  	struct mtk_phy_instance **phys;
> >  	int nphys;
> > +	struct dentry *dbgfs_root;
> >  	int src_ref_clk; /* MHZ, reference clock for slew rate
> > calibrate */
> >  	int src_coef; /* coefficient for slew rate calibrate */
> >  };
> >  
> > +#if IS_ENABLED(CONFIG_DEBUG_FS)
> > +
> > 
> > <skip>
> > 
> > +
> > +DEFINE_SHOW_ATTRIBUTE(tphy_type);
> > +
> > +static void tphy_debugfs_init(struct mtk_tphy *tphy, struct
> > mtk_phy_instance *inst)
> > +{
> > +	char name[16];
> > +
> > +	snprintf(name, sizeof(name) - 1, "phy.%d", inst->index);
> 
> I wouold suggest driver name/ device name rather than phy.foo...
> again
> folks needs to see what is foo
Ok, I'll change it, thanks a lot

> 
> <skip>
> > 
> >  static struct platform_driver mtk_tphy_driver = {
> >  	.probe		= mtk_tphy_probe,
> > +	.remove		= mtk_tphy_remove,
> >  	.driver		= {
> >  		.name	= "mtk-tphy",
> >  		.of_match_table = mtk_tphy_id_table,
> > -- 
> > 2.18.0
> 
> 

Re: [PATCH v6 3/3] phy: mediatek: tphy: add debugfs files

[Chunfeng Yun (云春峰)]

On Fri, 2023-01-13 at 23:45 +0530, Vinod Koul wrote:
> On 04-01-23, 21:26, Chunfeng Yun wrote:
> > These debugfs files are mainly used to make eye diagram test
> > easier,
> > especially helpful to do HQA test for a new IC without efuse
> > enabled.
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v6: no changes
> > 
> > v5: using common debugfs config CONFIG_DEBUG_FS
> > 
> > v4: fix build warning of sometimes uninitialized variable
> > 
> > v3: fix typo of "debugfs" suggested by AngeloGioacchino
> > 
> > v2: add CONFIG_PHY_MTK_TPHY_DEBUGFS suggested by AngeloGioacchino
> > ---
> >  drivers/phy/mediatek/phy-mtk-tphy.c | 405
> > +++++++++++++++++++++++++++-
> >  1 file changed, 404 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c
> > b/drivers/phy/mediatek/phy-mtk-tphy.c
> > index e906a82791bd..923e5ee119f3 100644
> > --- a/drivers/phy/mediatek/phy-mtk-tphy.c
> > +++ b/drivers/phy/mediatek/phy-mtk-tphy.c
> > @@ -7,6 +7,7 @@
> >  <skip>
> > +
> > +static void tphy_debugfs_init(struct mtk_tphy *tphy, struct
> > mtk_phy_instance *inst)
> > +{
> > +	char name[16];
> > +
> > +	snprintf(name, sizeof(name) - 1, "phy.%d", inst->index);
> 
> I wouold suggest driver name/ device name rather than phy.foo...
> again
> folks needs to see what is foo
> 
The driver creates a root folder by its device name, e.g. 
t-phy@11290000, it has two subphy, when use dev_name(&phy->dev) to
create folder for each phy, it will be:

# ls /sys/kernel/debug/phy/t-phy@11290000/
phy-t-phy@11290000.0  phy-t-phy@11290000.1

the phy's device name is created by
"dev_set_name(&phy->dev, "phy-%s.%d", dev_name(dev), id)"

dev_name(dev) is the parent name, it's t-phy@11290000 in example,
id is the phy->id.

due to the root folder already provide its parent device name, seems no
need provide it again in each phy's folder.

How about create the phy's folder by : "snprintf(name, sizeof(name) -
1, "phy-%d", inst->phy->id);"

then it becomes as below, seems brief:
# ls /sys/kernel/debug/phy/t-phy@11290000/
phy-0  phy-1

Thanks a lot

> 
> > +	inst->dbgfs = debugfs_create_dir(name, tphy->dbgfs_root);
> > +
> > +	debugfs_create_file("type", 0444, inst->dbgfs, inst,
> > &tphy_type_fops);
> > +
> > +	switch (inst->type) {
> > +	case PHY_TYPE_USB2:
> > +		u2_phy_dbgfs_files_create(inst);
> > +		break;
> > +	case PHY_TYPE_USB3:
> > +	case PHY_TYPE_PCIE:
> > +		u3_phy_dbgfs_files_create(inst);
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +}
> > +
<skip>

Re: [PATCH v6 3/3] phy: mediatek: tphy: add debugfs files

[Chunfeng Yun (云春峰)]

On Fri, 2023-01-13 at 23:45 +0530, Vinod Koul wrote:
> On 04-01-23, 21:26, Chunfeng Yun wrote:
> > These debugfs files are mainly used to make eye diagram test
> > easier,
> > especially helpful to do HQA test for a new IC without efuse
> > enabled.
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v6: no changes
> > 
> > v5: using common debugfs config CONFIG_DEBUG_FS
> > 
> > v4: fix build warning of sometimes uninitialized variable
> > 
> > v3: fix typo of "debugfs" suggested by AngeloGioacchino
> > 
> > v2: add CONFIG_PHY_MTK_TPHY_DEBUGFS suggested by AngeloGioacchino
> > ---
> >  drivers/phy/mediatek/phy-mtk-tphy.c | 405
> > +++++++++++++++++++++++++++-
> >  1 file changed, 404 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c
> > b/drivers/phy/mediatek/phy-mtk-tphy.c
> > index e906a82791bd..923e5ee119f3 100644
> > --- a/drivers/phy/mediatek/phy-mtk-tphy.c
> > +++ b/drivers/phy/mediatek/phy-mtk-tphy.c
> > @@ -7,6 +7,7 @@
> >  
> >  #include <dt-bindings/phy/phy.h>
> >  #include <linux/clk.h>
> > +#include <linux/debugfs.h>
> >  #include <linux/delay.h>
> >  #include <linux/iopoll.h>
> >  #include <linux/mfd/syscon.h>
> > @@ -264,6 +265,8 @@
> >  <skip>

> > +
> > +static void tphy_debugfs_init(struct mtk_tphy *tphy, struct
> > mtk_phy_instance *inst)
> > +{
> > +	char name[16];
> > +
> > +	snprintf(name, sizeof(name) - 1, "phy.%d", inst->index);
> 
> I wouold suggest driver name/ device name rather than phy.foo...
> again
> folks needs to see what is foo
> 
I compare the way you suggested again on mt2712 platform that have two
phy controller, t-phy@11290000 [1] and t-phy@112e0000 [2],
[1]: 
https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/mediatek/mt2712e.dtsi#L827
[2]:

https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/mediatek/mt2712e.dtsi#L891


Each phy controller driver will created a root folder according its
device name under "/sys/kernel/debug/phy", like as:
#ls /sys/kernel/debug/phy
t-phy@11290000  t-phy@112e0000

The phy controller has some subphy node, each one indicate a phy
struct, if use the phy device name to create a folder, it shows as
below:
#cd /sys/kernel/debug/phy
# ls *
t-phy@11290000:
phy-t-phy@11290000.0  phy-t-phy@11290000.1

t-phy@112e0000:
phy-t-phy@112e0000.3  phy-t-phy@112e0000.4  phy-t-phy@112e0000.5

phy's device name is created as:
"dev_set_name(&phy->dev, "phy-%s.%d", dev_name(dev), id)"

dev_name(dev) is the parent name, it's t-phy@11290000 and 
t-phy@112e0000 in example, id is the phy->id.

seems no need to provide parent name again.
another problem is that the id may vary if other phys are enabled, that
means the path may vary, it's difficult to write scripts to do test.

when use the current way,
"snprintf(name, sizeof(name) - 1, "phy.%d", inst->index)"
it shows as:
# ls *
t-phy@11290000:
phy.0  phy.1

t-phy@112e0000:
phy.0  phy.1  phy.2

@inst->index is the sequence number of subphy node in each phy
controller's node, when the dts is fixed, the @index is also fixed,
then for each phy, the path is also fixed, seems more convenient to
write scripts.

So I prefer to leave the code unchanged.

Thanks a lot

> 
> > +	inst->dbgfs = debugfs_create_dir(name, tphy->dbgfs_root);
> > +
> > +	debugfs_create_file("type", 0444, inst->dbgfs, inst,
> > &tphy_type_fops);
> > +
> > +	switch (inst->type) {
> > +	case PHY_TYPE_USB2:
> > +		u2_phy_dbgfs_files_create(inst);
> > +		break;
> > +	case PHY_TYPE_USB3:
> > +	case PHY_TYPE_PCIE:
> > +		u3_phy_dbgfs_files_create(inst);
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +}
> > +
> > +static void tphy_debugfs_exit(struct mtk_phy_instance *inst)
> > +{
> > +	debugfs_remove_recursive(inst->dbgfs);
> > +	inst->dbgfs = NULL;
> > +}
> > +
> > +static void tphy_debugfs_root_create(struct mtk_tphy *tphy)
> > +{
> > +	tphy->dbgfs_root = debugfs_create_dir(dev_name(tphy->dev),
> > phy_debug_root);
> > +}
> > +
> > +static void tphy_debugfs_root_remove(struct mtk_tphy *tphy)
> > +{
> > +	debugfs_remove_recursive(tphy->dbgfs_root);
> > +	tphy->dbgfs_root = NULL;
> > +}
> > +
> > +#else
> > +<skip>

Re: [PATCH v6 1/3] phy: mediatek: fix build warning caused by clang

[Vinod Koul]

On 04-01-23, 21:26, Chunfeng Yun wrote:
> Remove the temporary @mask_, this may cause build warning when use clang
> compiler for powerpc, but can't reproduce it when compile for arm64.
> the build warning is -Wtautological-constant-out-of-range-compare, and
> caused by
> "BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask)"
> 
> After removing @mask_, there is a "CHECK:MACRO_ARG_REUSE" when run
> checkpatch.pl, due to @mask is constant, no reuse problem will happen.
> 
> Fixes: 84513eccd678 ("phy: mediatek: fix build warning of FIELD_PREP()")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v6: modify the title

Title still does not tell what this patch is.. It tells me effect of the
patch but not the changes, pls revise...

"remove temp mask" can be better title

> v5: no changes
> v4 new patch, I'm not sure it can fix build warning, due to I don't cross compile
>     it for powerpc using clang in office.
> ---
>  drivers/phy/mediatek/phy-mtk-io.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/mediatek/phy-mtk-io.h b/drivers/phy/mediatek/phy-mtk-io.h
> index d20ad5e5be81..58f06db822cb 100644
> --- a/drivers/phy/mediatek/phy-mtk-io.h
> +++ b/drivers/phy/mediatek/phy-mtk-io.h
> @@ -39,8 +39,8 @@ static inline void mtk_phy_update_bits(void __iomem *reg, u32 mask, u32 val)
>  /* field @mask shall be constant and continuous */
>  #define mtk_phy_update_field(reg, mask, val) \
>  ({ \
> -	typeof(mask) mask_ = (mask);	\
> -	mtk_phy_update_bits(reg, mask_, FIELD_PREP(mask_, val)); \
> +	BUILD_BUG_ON_MSG(!__builtin_constant_p(mask), "mask is not constant"); \
> +	mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); \
>  })
>  
>  #endif
> -- 
> 2.18.0

-- 
~Vinod

Re: [PATCH v6 1/3] phy: mediatek: fix build warning caused by clang

[Chunfeng Yun (云春峰)]

On Fri, 2023-01-13 at 23:42 +0530, Vinod Koul wrote:
> On 04-01-23, 21:26, Chunfeng Yun wrote:
> > Remove the temporary @mask_, this may cause build warning when use
> > clang
> > compiler for powerpc, but can't reproduce it when compile for
> > arm64.
> > the build warning is -Wtautological-constant-out-of-range-compare,
> > and
> > caused by
> > "BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask)"
> > 
> > After removing @mask_, there is a "CHECK:MACRO_ARG_REUSE" when run
> > checkpatch.pl, due to @mask is constant, no reuse problem will
> > happen.
> > 
> > Fixes: 84513eccd678 ("phy: mediatek: fix build warning of
> > FIELD_PREP()")
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v6: modify the title
> 
> Title still does not tell what this patch is.. It tells me effect of
> the
> patch but not the changes, pls revise...
> 
> "remove temp mask" can be better title
Got it, will modify it in next version, thanks

> 
> > v5: no changes
> > v4 new patch, I'm not sure it can fix build warning, due to I don't
> > cross compile
> >     it for powerpc using clang in office.
> > ---
> >  drivers/phy/mediatek/phy-mtk-io.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/phy/mediatek/phy-mtk-io.h
> > b/drivers/phy/mediatek/phy-mtk-io.h
> > index d20ad5e5be81..58f06db822cb 100644
> > --- a/drivers/phy/mediatek/phy-mtk-io.h
> > +++ b/drivers/phy/mediatek/phy-mtk-io.h
> > @@ -39,8 +39,8 @@ static inline void mtk_phy_update_bits(void
> > __iomem *reg, u32 mask, u32 val)
> >  /* field @mask shall be constant and continuous */
> >  #define mtk_phy_update_field(reg, mask, val) \
> >  ({ \
> > -	typeof(mask) mask_ = (mask);	\
> > -	mtk_phy_update_bits(reg, mask_, FIELD_PREP(mask_, val)); \
> > +	BUILD_BUG_ON_MSG(!__builtin_constant_p(mask), "mask is not
> > constant"); \
> > +	mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); \
> >  })
> >  
> >  #endif
> > -- 
> > 2.18.0
> 
> 

Re: [PATCH v6 1/3] phy: mediatek: fix build warning caused by clang

[Nick Desaulniers]

On Wed, Jan 4, 2023 at 5:26 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> Remove the temporary @mask_, this may cause build warning when use clang
> compiler for powerpc, but can't reproduce it when compile for arm64.
> the build warning is -Wtautological-constant-out-of-range-compare, and
> caused by
> "BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask)"

Can you please include the text of the observed warning?

>
> After removing @mask_, there is a "CHECK:MACRO_ARG_REUSE" when run
> checkpatch.pl, due to @mask is constant, no reuse problem will happen.
>
> Fixes: 84513eccd678 ("phy: mediatek: fix build warning of FIELD_PREP()")

Uh, why was 84513eccd678 sent/reviewed/merged if it didn't fix the
issue correctly in the first place?

Is the issue perhaps that your mask isn't wide enough in the first
place, and should be?  See:
commit cfd6fb45cfaf ("crypto: ccree - avoid out-of-range warnings from clang")
for inspiration.

> Reported-by: kernel test robot <lkp@intel.com>

Can you please include the Link: tag to the lore URL of the report?

> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v6: modify the title
> v5: no changes
> v4 new patch, I'm not sure it can fix build warning, due to I don't cross compile
>     it for powerpc using clang in office.
> ---
>  drivers/phy/mediatek/phy-mtk-io.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/mediatek/phy-mtk-io.h b/drivers/phy/mediatek/phy-mtk-io.h
> index d20ad5e5be81..58f06db822cb 100644
> --- a/drivers/phy/mediatek/phy-mtk-io.h
> +++ b/drivers/phy/mediatek/phy-mtk-io.h
> @@ -39,8 +39,8 @@ static inline void mtk_phy_update_bits(void __iomem *reg, u32 mask, u32 val)
>  /* field @mask shall be constant and continuous */
>  #define mtk_phy_update_field(reg, mask, val) \
>  ({ \
> -       typeof(mask) mask_ = (mask);    \
> -       mtk_phy_update_bits(reg, mask_, FIELD_PREP(mask_, val)); \
> +       BUILD_BUG_ON_MSG(!__builtin_constant_p(mask), "mask is not constant"); \
> +       mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); \
>  })
>
>  #endif
> --
> 2.18.0
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v6 1/3] phy: mediatek: fix build warning caused by clang

[Chunfeng Yun (云春峰)]

On Fri, 2023-01-13 at 10:36 -0800, Nick Desaulniers wrote:
> On Wed, Jan 4, 2023 at 5:26 AM Chunfeng Yun <
> chunfeng.yun@mediatek.com> wrote:
> > 
> > Remove the temporary @mask_, this may cause build warning when use
> > clang
> > compiler for powerpc, but can't reproduce it when compile for
> > arm64.
> > the build warning is -Wtautological-constant-out-of-range-compare,
> > and
> > caused by
> > "BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask)"
> 
> Can you please include the text of the observed warning?
Ok, will add more logs

> 
> > 
> > After removing @mask_, there is a "CHECK:MACRO_ARG_REUSE" when run
> > checkpatch.pl, due to @mask is constant, no reuse problem will
> > happen.
> > 
> > Fixes: 84513eccd678 ("phy: mediatek: fix build warning of
> > FIELD_PREP()")
> 
> Uh, why was 84513eccd678 sent/reviewed/merged if it didn't fix the
> issue correctly in the first place?
Sorry, I can't reproduce it, but make sure no such issue happens on arm
arch using gcc/clang. MTK only uses arm/mips arch, it's difficult for
me to set up clang cross compiler for other archs in office. 

> 
> Is the issue perhaps that your mask isn't wide enough in the first
> place, and should be? 
the masks are usually created by GENMASK macro;

There is no warning when build for arm64 using clang.

>  See:
> commit cfd6fb45cfaf ("crypto: ccree - avoid out-of-range warnings
> from clang")
> for inspiration.
Ok, I'll do it
> 
> > Reported-by: kernel test robot <lkp@intel.com>
> 
> Can you please include the Link: tag to the lore URL of the report?
Ok, thanks a lot

> 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v6: modify the title
> > v5: no changes
> > v4 new patch, I'm not sure it can fix build warning, due to I don't
> > cross compile
> >     it for powerpc using clang in office.
> > ---
> >  drivers/phy/mediatek/phy-mtk-io.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/phy/mediatek/phy-mtk-io.h
> > b/drivers/phy/mediatek/phy-mtk-io.h
> > index d20ad5e5be81..58f06db822cb 100644
> > --- a/drivers/phy/mediatek/phy-mtk-io.h
> > +++ b/drivers/phy/mediatek/phy-mtk-io.h
> > @@ -39,8 +39,8 @@ static inline void mtk_phy_update_bits(void
> > __iomem *reg, u32 mask, u32 val)
> >  /* field @mask shall be constant and continuous */
> >  #define mtk_phy_update_field(reg, mask, val) \
> >  ({ \
> > -       typeof(mask) mask_ = (mask);    \
> > -       mtk_phy_update_bits(reg, mask_, FIELD_PREP(mask_, val)); \
> > +       BUILD_BUG_ON_MSG(!__builtin_constant_p(mask), "mask is not
> > constant"); \
> > +       mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); \
> >  })
> > 
> >  #endif
> > --
> > 2.18.0
> > 
> 
> 

Re: [PATCH v6 1/3] phy: mediatek: fix build warning caused by clang

[Chunfeng Yun (云春峰)]

On Fri, 2023-01-13 at 10:36 -0800, Nick Desaulniers wrote:
> On Wed, Jan 4, 2023 at 5:26 AM Chunfeng Yun <
> chunfeng.yun@mediatek.com> wrote:
> > 
> > Remove the temporary @mask_, this may cause build warning when use
> > clang
> > compiler for powerpc, but can't reproduce it when compile for
> > arm64.
> > the build warning is -Wtautological-constant-out-of-range-compare,
> > and
> > caused by
> > "BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask)"
> 
> Can you please include the text of the observed warning?
> 
> > 
> > After removing @mask_, there is a "CHECK:MACRO_ARG_REUSE" when run
> > checkpatch.pl, due to @mask is constant, no reuse problem will
> > happen.
> > 
> > Fixes: 84513eccd678 ("phy: mediatek: fix build warning of
> > FIELD_PREP()")
> 
> Uh, why was 84513eccd678 sent/reviewed/merged if it didn't fix the
> issue correctly in the first place?
> 
> Is the issue perhaps that your mask isn't wide enough in the first
> place, and should be?  See:
> commit cfd6fb45cfaf ("crypto: ccree - avoid out-of-range warnings
> from clang")
> for inspiration.

I look at this patch, it said that "
This does not happen in other places that just pass a constant here.
"
That's indeed true, no such warning before using FIELD_PREP() due to
the masks are always constant.

So seems that it can fix the waring if avoid using a temporary variable
@mask_ in mtk_phy_update_field() macro.

Thanks a lot

> 
> > Reported-by: kernel test robot <lkp@intel.com>
> 
> Can you please include the Link: tag to the lore URL of the report?
> 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v6: modify the title
> > v5: no changes
> > v4 new patch, I'm not sure it can fix build warning, due to I don't
> > cross compile
> >     it for powerpc using clang in office.
> > ---
> >  drivers/phy/mediatek/phy-mtk-io.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/phy/mediatek/phy-mtk-io.h
> > b/drivers/phy/mediatek/phy-mtk-io.h
> > index d20ad5e5be81..58f06db822cb 100644
> > --- a/drivers/phy/mediatek/phy-mtk-io.h
> > +++ b/drivers/phy/mediatek/phy-mtk-io.h
> > @@ -39,8 +39,8 @@ static inline void mtk_phy_update_bits(void
> > __iomem *reg, u32 mask, u32 val)
> >  /* field @mask shall be constant and continuous */
> >  #define mtk_phy_update_field(reg, mask, val) \
> >  ({ \
> > -       typeof(mask) mask_ = (mask);    \
> > -       mtk_phy_update_bits(reg, mask_, FIELD_PREP(mask_, val)); \
> > +       BUILD_BUG_ON_MSG(!__builtin_constant_p(mask), "mask is not
> > constant"); \
> > +       mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); \
> >  })
> > 
> >  #endif
> > --
> > 2.18.0
> > 
> 
> 

Re: [PATCH v6 1/3] phy: mediatek: fix build warning caused by clang

[Chunfeng Yun (云春峰)]

On Fri, 2023-01-13 at 10:36 -0800, Nick Desaulniers wrote:
> On Wed, Jan 4, 2023 at 5:26 AM Chunfeng Yun <
> chunfeng.yun@mediatek.com> wrote:
> > 
> > Remove the temporary @mask_, this may cause build warning when use
> > clang
> > compiler for powerpc, but can't reproduce it when compile for
> > arm64.
> > the build warning is -Wtautological-constant-out-of-range-compare,
> > and
> > caused by
> > "BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask)"
> 
> Can you please include the text of the observed warning?
> 
> > 
> > After removing @mask_, there is a "CHECK:MACRO_ARG_REUSE" when run
> > checkpatch.pl, due to @mask is constant, no reuse problem will
> > happen.
> > 
> > Fixes: 84513eccd678 ("phy: mediatek: fix build warning of
> > FIELD_PREP()")
> 
> Uh, why was 84513eccd678 sent/reviewed/merged if it didn't fix the
> issue correctly in the first place?
> 
> Is the issue perhaps that your mask isn't wide enough in the first
> place, and should be?  See:
> commit cfd6fb45cfaf ("crypto: ccree - avoid out-of-range warnings
> from clang")
> for inspiration.
I look at this patch, it said that "
This does not happen in other places that just pass a constant here.
"
That's indeed true, no such warning before using FIELD_PREP() due to
the masks are always constant.

So seems that it can fix the waring if avoid using a temporary variable
@mask_.

Thanks a lot

> 
> > Reported-by: kernel test robot <lkp@intel.com>
> 
> Can you please include the Link: tag to the lore URL of the report?
> 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v6: modify the title
> > v5: no changes
> > v4 new patch, I'm not sure it can fix build warning, due to I don't
> > cross compile
> >     it for powerpc using clang in office.
> > ---
> >  drivers/phy/mediatek/phy-mtk-io.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/phy/mediatek/phy-mtk-io.h
> > b/drivers/phy/mediatek/phy-mtk-io.h
> > index d20ad5e5be81..58f06db822cb 100644
> > --- a/drivers/phy/mediatek/phy-mtk-io.h
> > +++ b/drivers/phy/mediatek/phy-mtk-io.h
> > @@ -39,8 +39,8 @@ static inline void mtk_phy_update_bits(void
> > __iomem *reg, u32 mask, u32 val)
> >  /* field @mask shall be constant and continuous */
> >  #define mtk_phy_update_field(reg, mask, val) \
> >  ({ \
> > -       typeof(mask) mask_ = (mask);    \
> > -       mtk_phy_update_bits(reg, mask_, FIELD_PREP(mask_, val)); \
> > +       BUILD_BUG_ON_MSG(!__builtin_constant_p(mask), "mask is not
> > constant"); \
> > +       mtk_phy_update_bits(reg, mask, FIELD_PREP(mask, val)); \
> >  })
> > 
> >  #endif
> > --
> > 2.18.0
> > 
> 
>