The CR port is a simple 16-bit data/address parallel port that is
provided for on-chip access to the control registers inside the
USB 3.0 femtoPHY[1]. While access to these registers is not required
for normal PHY operation, this interface enables you to access
some of the PHY’s diagnostic features during normal operation or
to override some basic PHY control signals.

3 debugfs files are created to read and write control registers,
all use hexadecimal format:
ctrl_reg_base: the register offset to write, or the start offset
               to read.
ctrl_reg_count: how many continuous registers to be read.
ctrl_reg_value: read to show the continuous registers value from
                the offset in ctrl_reg_base, to ctrl_reg_base
                + ctrl_reg_count - 1, one line for one register.
                when write, override the register at ctrl_reg_base,
                one time can only change one 16bits register.

Link[1]: https://www.synopsys.com/dw/doc.php/phy/usb3.0/femto/phy/x652_usb3_ss14lpp_18_ns/4.07a/dwc_usb3.0_femtophy_ss14lpp_08V18V_x1_databook.pdf
Signed-off-by: Li Jun <jun.li@nxp.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v3:
 - add documentation link of registers
Changes in v2:
 - correct copyright
 - add imx8mq_phy_wait_for_cr_ack() helper and use it
 - use DEFINE_SHOW_STORE_ATTRIBUTE()
 - directly create debugfs file under imx phy debugfs
 - remove debug_remove_files() since the phy core will handle it
---
 drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 180 ++++++++++++++++++++-
 1 file changed, 178 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
index 95f9264bd0f7..e11054f6673c 100644
--- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
@@ -1,10 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0+
-/* Copyright (c) 2017 NXP. */
+/* Copyright 2017-2025 NXP. */
 
 #include <linux/bitfield.h>
 #include <linux/clk.h>
+#include <linux/debugfs.h>
 #include <linux/delay.h>
-#include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/phy/phy.h>
@@ -57,6 +58,20 @@
 
 #define PHY_TUNE_DEFAULT		0xffffffff
 
+/* PHY control register access */
+#define PHY_CTRL_REG_OFFSET_MAX		0x201f
+
+#define PHY_CRCTL			0x30
+#define PHY_CRCTL_DATA_IN_MASK		GENMASK(15, 0)
+#define PHY_CRCTL_CAP_ADDR		BIT(16)
+#define PHY_CRCTL_CAP_DATA		BIT(17)
+#define PHY_CRCTL_CR_WRITE		BIT(18)
+#define PHY_CRCTL_CR_READ		BIT(19)
+
+#define PHY_CRSR			0x34
+#define PHY_CRSR_DATA_OUT_MASK		GENMASK(15, 0)
+#define PHY_CRSR_CR_ACK			BIT(16)
+
 #define TCA_CLK_RST			0x00
 #define TCA_CLK_RST_SW			BIT(9)
 #define TCA_CLK_RST_REF_CLK_EN		BIT(1)
@@ -118,6 +133,9 @@ struct imx8mq_usb_phy {
 	void __iomem *base;
 	struct regulator *vbus;
 	struct tca_blk *tca;
+	struct dentry *debugfs;
+	u16 cr_access_base;
+	u16 cr_read_count;
 	u32 pcs_tx_swing_full;
 	u32 pcs_tx_deemph_3p5db;
 	u32 tx_vref_tune;
@@ -411,6 +429,163 @@ static u32 phy_pcs_tx_swing_full_from_property(u32 percent)
 	percent = min(percent, 100U);
 
 	return (percent * 127) / 100;
+};
+
+static int imx8mq_phy_wait_for_cr_ack(struct imx8mq_usb_phy *imx_phy,
+				      u32 stage, u32 *data)
+{
+	void __iomem	*cr_ctrl = imx_phy->base + PHY_CRCTL;
+	void __iomem	*cr_sr = imx_phy->base + PHY_CRSR;
+	u32		val;
+	int		ret;
+
+	writel(readl(cr_ctrl) | stage, cr_ctrl);
+	/* Wait CRSR[16] == 1 */
+	ret = readl_poll_timeout(cr_sr, val,
+				 (val & PHY_CRSR_CR_ACK) == PHY_CRSR_CR_ACK,
+				 1, 100);
+	if (ret)
+		return ret;
+
+	if (stage == PHY_CRCTL_CR_READ)
+		*data = readl(cr_sr) & 0xffff;
+
+	writel(readl(cr_ctrl) & (~stage), cr_ctrl);
+	/* Wait CRSR[16] == 0 */
+	return readl_poll_timeout(cr_sr, val, (val & PHY_CRSR_CR_ACK) == 0, 1, 100);
+}
+
+static int imx8mq_phy_ctrl_reg_addr(struct imx8mq_usb_phy *imx_phy, u16 offset)
+{
+	void __iomem	*cr_ctrl = imx_phy->base + PHY_CRCTL;
+	struct device	*dev = &imx_phy->phy->dev;
+	int		ret;
+
+	writel(offset, cr_ctrl);
+	ret = imx8mq_phy_wait_for_cr_ack(imx_phy, PHY_CRCTL_CAP_ADDR, NULL);
+	if (ret < 0) {
+		dev_err(dev, "Failed to address reg 0x%04x\n", offset);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int imx8mq_phy_ctrl_reg_read(struct imx8mq_usb_phy *imx_phy,
+				    u16 offset, u32 *val)
+{
+	struct device *dev = &imx_phy->phy->dev;
+	int ret;
+
+	if (offset > PHY_CTRL_REG_OFFSET_MAX) {
+		dev_err(dev, "Invalid reg address 0x%04x\n", offset);
+		return -EINVAL;
+	}
+
+	/* Address stage */
+	ret = imx8mq_phy_ctrl_reg_addr(imx_phy, offset);
+	if (ret)
+		return ret;
+
+	/* Read data stage */
+	ret = imx8mq_phy_wait_for_cr_ack(imx_phy, PHY_CRCTL_CR_READ, val);
+	if (ret < 0) {
+		dev_err(dev, "Failed to read reg 0x%04x\n", offset);
+		return -EIO;
+	}
+
+	return ret;
+}
+
+static int imx8mq_phy_ctrl_reg_write(struct imx8mq_usb_phy *imx_phy,
+				     u16 offset, u16 val)
+{
+	struct device	*dev = &imx_phy->phy->dev;
+	void __iomem	*cr_ctrl = imx_phy->base + PHY_CRCTL;
+	int		ret;
+
+	if (offset > PHY_CTRL_REG_OFFSET_MAX) {
+		dev_err(dev, "Invalid reg address 0x%04x\n", offset);
+		return -EINVAL;
+	}
+
+	/* Address stage */
+	ret = imx8mq_phy_ctrl_reg_addr(imx_phy, offset);
+	if (ret)
+		return ret;
+
+	/* Capture data stage */
+	writel(val, cr_ctrl);
+	ret = imx8mq_phy_wait_for_cr_ack(imx_phy, PHY_CRCTL_CAP_DATA, NULL);
+	if (ret < 0)
+		goto cr_write_err;
+
+	/* Write data stage */
+	ret = imx8mq_phy_wait_for_cr_ack(imx_phy, PHY_CRCTL_CR_WRITE, NULL);
+	if (ret < 0)
+		goto cr_write_err;
+
+	return 0;
+
+cr_write_err:
+	dev_err(dev, "Failed to write reg 0x%04x\n", offset);
+	return -EIO;
+}
+
+static int ctrl_reg_value_show(struct seq_file *s, void *unused)
+{
+	struct imx8mq_usb_phy *imx_phy = s->private;
+	u16 base = imx_phy->cr_access_base;
+	u32 val;
+	int i, ret;
+
+	for (i = 0; i < imx_phy->cr_read_count; i++) {
+		ret = imx8mq_phy_ctrl_reg_read(imx_phy, base + i, &val);
+		if (ret < 0)
+			return ret;
+
+		seq_printf(s, "Control Register 0x%04x value is 0x%04x\n",
+			   base + i, val);
+	}
+
+	return 0;
+}
+
+static ssize_t ctrl_reg_value_write(struct file *file, const char __user *ubuf,
+				    size_t count, loff_t *ppos)
+
+{
+	struct seq_file		*s = file->private_data;
+	struct imx8mq_usb_phy	*imx_phy = s->private;
+	u16			cr_value;
+	int			ret;
+
+	ret = kstrtou16_from_user(ubuf, count, 16, &cr_value);
+	if (ret)
+		return ret;
+
+	ret = imx8mq_phy_ctrl_reg_write(imx_phy, imx_phy->cr_access_base, cr_value);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+DEFINE_SHOW_STORE_ATTRIBUTE(ctrl_reg_value);
+
+static void imx8m_create_debug_files(struct imx8mq_usb_phy *imx_phy)
+{
+	struct dentry *debugfs = imx_phy->phy->debugfs;
+
+	debugfs_create_x16("ctrl_reg_base", 0600, debugfs,
+			   &imx_phy->cr_access_base);
+	debugfs_create_x16("ctrl_reg_count", 0600, debugfs,
+			   &imx_phy->cr_read_count);
+	debugfs_create_file("ctrl_reg_value", 0600, debugfs,
+			    imx_phy, &ctrl_reg_value_fops);
+
+	imx_phy->cr_access_base = 0;
+	imx_phy->cr_read_count = 1;
 }
 
 static void imx8m_get_phy_tuning_data(struct imx8mq_usb_phy *imx_phy)
@@ -731,6 +906,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 					"failed to get tca\n");
 
 	imx8m_get_phy_tuning_data(imx_phy);
+	imx8m_create_debug_files(imx_phy);
 
 	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
 
-- 
2.34.1

On Thu, Jan 08, 2026 at 04:36:41PM +0800, Xu Yang wrote:
> The CR port is a simple 16-bit data/address parallel port that is
> provided for on-chip access to the control registers inside the
> USB 3.0 femtoPHY[1]. While access to these registers is not required
> for normal PHY operation, this interface enables you to access
> some of the PHY’s diagnostic features during normal operation or
> to override some basic PHY control signals.
> 
> 3 debugfs files are created to read and write control registers,
> all use hexadecimal format:
> ctrl_reg_base: the register offset to write, or the start offset
>                to read.
> ctrl_reg_count: how many continuous registers to be read.
> ctrl_reg_value: read to show the continuous registers value from
>                 the offset in ctrl_reg_base, to ctrl_reg_base
>                 + ctrl_reg_count - 1, one line for one register.
>                 when write, override the register at ctrl_reg_base,
>                 one time can only change one 16bits register.
> 
> Link[1]: https://www.synopsys.com/dw/doc.php/phy/usb3.0/femto/phy/x652_usb3_ss14lpp_18_ns/4.07a/dwc_usb3.0_femtophy_ss14lpp_08V18V_x1_databook.pdf

Please don't ignore my comments to V2. Think about the code split
between the generic IP licensed from Synopsys and the vendor specific
code used for integration into the SoC. You want to avoid making a
mess you later need to cleanup because somebody else licensed the same
IP core from Synopsys, and need to put their own vendor specific
integration code around the generic code.

    Andrew

On Thu, Jan 08, 2026 at 07:24:31PM +0100, Andrew Lunn wrote:
> On Thu, Jan 08, 2026 at 04:36:41PM +0800, Xu Yang wrote:
> > The CR port is a simple 16-bit data/address parallel port that is
> > provided for on-chip access to the control registers inside the
> > USB 3.0 femtoPHY[1]. While access to these registers is not required
> > for normal PHY operation, this interface enables you to access
> > some of the PHY’s diagnostic features during normal operation or
> > to override some basic PHY control signals.
> > 
> > 3 debugfs files are created to read and write control registers,
> > all use hexadecimal format:
> > ctrl_reg_base: the register offset to write, or the start offset
> >                to read.
> > ctrl_reg_count: how many continuous registers to be read.
> > ctrl_reg_value: read to show the continuous registers value from
> >                 the offset in ctrl_reg_base, to ctrl_reg_base
> >                 + ctrl_reg_count - 1, one line for one register.
> >                 when write, override the register at ctrl_reg_base,
> >                 one time can only change one 16bits register.
> > 
> > Link[1]: https://www.synopsys.com/dw/doc.php/phy/usb3.0/femto/phy/x652_usb3_ss14lpp_18_ns/4.07a/dwc_usb3.0_femtophy_ss14lpp_08V18V_x1_databook.pdf
> 
> Please don't ignore my comments to V2. Think about the code split
> between the generic IP licensed from Synopsys and the vendor specific
> code used for integration into the SoC. You want to avoid making a
> mess you later need to cleanup because somebody else licensed the same
> IP core from Synopsys, and need to put their own vendor specific
> integration code around the generic code.

OK.

Thanks,
Xu Yang

> 
>     Andrew

On 08-01-26, 19:24, Andrew Lunn wrote:
> On Thu, Jan 08, 2026 at 04:36:41PM +0800, Xu Yang wrote:
> > The CR port is a simple 16-bit data/address parallel port that is
> > provided for on-chip access to the control registers inside the
> > USB 3.0 femtoPHY[1]. While access to these registers is not required
> > for normal PHY operation, this interface enables you to access
> > some of the PHY’s diagnostic features during normal operation or
> > to override some basic PHY control signals.
> > 
> > 3 debugfs files are created to read and write control registers,
> > all use hexadecimal format:
> > ctrl_reg_base: the register offset to write, or the start offset
> >                to read.
> > ctrl_reg_count: how many continuous registers to be read.
> > ctrl_reg_value: read to show the continuous registers value from
> >                 the offset in ctrl_reg_base, to ctrl_reg_base
> >                 + ctrl_reg_count - 1, one line for one register.
> >                 when write, override the register at ctrl_reg_base,
> >                 one time can only change one 16bits register.
> > 
> > Link[1]: https://www.synopsys.com/dw/doc.php/phy/usb3.0/femto/phy/x652_usb3_ss14lpp_18_ns/4.07a/dwc_usb3.0_femtophy_ss14lpp_08V18V_x1_databook.pdf
> 
> Please don't ignore my comments to V2. Think about the code split
> between the generic IP licensed from Synopsys and the vendor specific
> code used for integration into the SoC. You want to avoid making a
> mess you later need to cleanup because somebody else licensed the same
> IP core from Synopsys, and need to put their own vendor specific
> integration code around the generic code.

Agree with Andrew here, please do mix, splitting would be better

> 
>     Andrew

-- 
~Vinod

On Wed, Jan 14, 2026 at 03:09:54PM +0530, Vinod Koul wrote:
> On 08-01-26, 19:24, Andrew Lunn wrote:
> > On Thu, Jan 08, 2026 at 04:36:41PM +0800, Xu Yang wrote:
> > > The CR port is a simple 16-bit data/address parallel port that is
> > > provided for on-chip access to the control registers inside the
> > > USB 3.0 femtoPHY[1]. While access to these registers is not required
> > > for normal PHY operation, this interface enables you to access
> > > some of the PHY’s diagnostic features during normal operation or
> > > to override some basic PHY control signals.
> > > 
> > > 3 debugfs files are created to read and write control registers,
> > > all use hexadecimal format:
> > > ctrl_reg_base: the register offset to write, or the start offset
> > >                to read.
> > > ctrl_reg_count: how many continuous registers to be read.
> > > ctrl_reg_value: read to show the continuous registers value from
> > >                 the offset in ctrl_reg_base, to ctrl_reg_base
> > >                 + ctrl_reg_count - 1, one line for one register.
> > >                 when write, override the register at ctrl_reg_base,
> > >                 one time can only change one 16bits register.
> > > 
> > > Link[1]: https://www.synopsys.com/dw/doc.php/phy/usb3.0/femto/phy/x652_usb3_ss14lpp_18_ns/4.07a/dwc_usb3.0_femtophy_ss14lpp_08V18V_x1_databook.pdf
> > 
> > Please don't ignore my comments to V2. Think about the code split
> > between the generic IP licensed from Synopsys and the vendor specific
> > code used for integration into the SoC. You want to avoid making a
> > mess you later need to cleanup because somebody else licensed the same
> > IP core from Synopsys, and need to put their own vendor specific
> > integration code around the generic code.
> 
> Agree with Andrew here, please do mix, splitting would be better

OK, will try.

Thanks,
Xu Yang

> 
> > 
> >     Andrew
> 
> -- 
> ~Vinod

On Thu, Jan 08, 2026 at 04:36:41PM +0800, Xu Yang wrote:
> The CR port is a simple 16-bit data/address parallel port that is
> provided for on-chip access to the control registers inside the
> USB 3.0 femtoPHY[1].


> While access to these registers is not required
> for normal PHY operation, this interface enables you to access
> some of the PHY’s diagnostic features during normal operation or
> to override some basic PHY control signals.

Simple said "Export these registers by debugfs to help PHY’s diagnostic."
should be enough

>
> 3 debugfs files are created to read and write control registers,
> all use hexadecimal format:
> ctrl_reg_base: the register offset to write, or the start offset
>                to read.
> ctrl_reg_count: how many continuous registers to be read.
> ctrl_reg_value: read to show the continuous registers value from
>                 the offset in ctrl_reg_base, to ctrl_reg_base
>                 + ctrl_reg_count - 1, one line for one register.
>                 when write, override the register at ctrl_reg_base,
>                 one time can only change one 16bits register.

how many regs? how about create file regNNN,

cat/echo to regNNN?

or create export file

echo 100 > export, then create REG100, so can use cat/echo modify it.


>
> Link[1]: https://www.synopsys.com/dw/doc.php/phy/usb3.0/femto/phy/x652_usb3_ss14lpp_18_ns/4.07a/dwc_usb3.0_femtophy_ss14lpp_08V18V_x1_databook.pdf
> Signed-off-by: Li Jun <jun.li@nxp.com>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>
> ---
> Changes in v3:
>  - add documentation link of registers
> Changes in v2:
>  - correct copyright
>  - add imx8mq_phy_wait_for_cr_ack() helper and use it
>  - use DEFINE_SHOW_STORE_ATTRIBUTE()
>  - directly create debugfs file under imx phy debugfs
>  - remove debug_remove_files() since the phy core will handle it
> ---
>  drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 180 ++++++++++++++++++++-
>  1 file changed, 178 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index 95f9264bd0f7..e11054f6673c 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> @@ -1,10 +1,11 @@
>  // SPDX-License-Identifier: GPL-2.0+
> -/* Copyright (c) 2017 NXP. */
> +/* Copyright 2017-2025 NXP. */
>
>  #include <linux/bitfield.h>
>  #include <linux/clk.h>
> +#include <linux/debugfs.h>
>  #include <linux/delay.h>
> -#include <linux/io.h>
> +#include <linux/iopoll.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/phy/phy.h>
> @@ -57,6 +58,20 @@
>
>  #define PHY_TUNE_DEFAULT		0xffffffff
>
> +/* PHY control register access */
> +#define PHY_CTRL_REG_OFFSET_MAX		0x201f
> +
> +#define PHY_CRCTL			0x30
> +#define PHY_CRCTL_DATA_IN_MASK		GENMASK(15, 0)
> +#define PHY_CRCTL_CAP_ADDR		BIT(16)
> +#define PHY_CRCTL_CAP_DATA		BIT(17)
> +#define PHY_CRCTL_CR_WRITE		BIT(18)
> +#define PHY_CRCTL_CR_READ		BIT(19)
> +
> +#define PHY_CRSR			0x34
> +#define PHY_CRSR_DATA_OUT_MASK		GENMASK(15, 0)
> +#define PHY_CRSR_CR_ACK			BIT(16)
> +
>  #define TCA_CLK_RST			0x00
>  #define TCA_CLK_RST_SW			BIT(9)
>  #define TCA_CLK_RST_REF_CLK_EN		BIT(1)
> @@ -118,6 +133,9 @@ struct imx8mq_usb_phy {
>  	void __iomem *base;
>  	struct regulator *vbus;
>  	struct tca_blk *tca;
> +	struct dentry *debugfs;
> +	u16 cr_access_base;
> +	u16 cr_read_count;
>  	u32 pcs_tx_swing_full;
>  	u32 pcs_tx_deemph_3p5db;
>  	u32 tx_vref_tune;
> @@ -411,6 +429,163 @@ static u32 phy_pcs_tx_swing_full_from_property(u32 percent)
>  	percent = min(percent, 100U);
>
>  	return (percent * 127) / 100;
> +};
> +
> +static int imx8mq_phy_wait_for_cr_ack(struct imx8mq_usb_phy *imx_phy,
> +				      u32 stage, u32 *data)
> +{
> +	void __iomem	*cr_ctrl = imx_phy->base + PHY_CRCTL;
> +	void __iomem	*cr_sr = imx_phy->base + PHY_CRSR;
> +	u32		val;
> +	int		ret;
> +
> +	writel(readl(cr_ctrl) | stage, cr_ctrl);
> +	/* Wait CRSR[16] == 1 */
> +	ret = readl_poll_timeout(cr_sr, val,
> +				 (val & PHY_CRSR_CR_ACK) == PHY_CRSR_CR_ACK,
> +				 1, 100);
> +	if (ret)
> +		return ret;
> +
> +	if (stage == PHY_CRCTL_CR_READ)
> +		*data = readl(cr_sr) & 0xffff;
> +
> +	writel(readl(cr_ctrl) & (~stage), cr_ctrl);
> +	/* Wait CRSR[16] == 0 */
> +	return readl_poll_timeout(cr_sr, val, (val & PHY_CRSR_CR_ACK) == 0, 1, 100);
> +}
> +
> +static int imx8mq_phy_ctrl_reg_addr(struct imx8mq_usb_phy *imx_phy, u16 offset)
> +{
> +	void __iomem	*cr_ctrl = imx_phy->base + PHY_CRCTL;
> +	struct device	*dev = &imx_phy->phy->dev;
> +	int		ret;
> +
> +	writel(offset, cr_ctrl);
> +	ret = imx8mq_phy_wait_for_cr_ack(imx_phy, PHY_CRCTL_CAP_ADDR, NULL);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to address reg 0x%04x\n", offset);
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static int imx8mq_phy_ctrl_reg_read(struct imx8mq_usb_phy *imx_phy,
> +				    u16 offset, u32 *val)
> +{
> +	struct device *dev = &imx_phy->phy->dev;
> +	int ret;
> +
> +	if (offset > PHY_CTRL_REG_OFFSET_MAX) {
> +		dev_err(dev, "Invalid reg address 0x%04x\n", offset);
> +		return -EINVAL;
> +	}
> +
> +	/* Address stage */
> +	ret = imx8mq_phy_ctrl_reg_addr(imx_phy, offset);
> +	if (ret)
> +		return ret;
> +
> +	/* Read data stage */
> +	ret = imx8mq_phy_wait_for_cr_ack(imx_phy, PHY_CRCTL_CR_READ, val);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to read reg 0x%04x\n", offset);
> +		return -EIO;
> +	}
> +
> +	return ret;
> +}
> +
> +static int imx8mq_phy_ctrl_reg_write(struct imx8mq_usb_phy *imx_phy,
> +				     u16 offset, u16 val)
> +{
> +	struct device	*dev = &imx_phy->phy->dev;
> +	void __iomem	*cr_ctrl = imx_phy->base + PHY_CRCTL;
> +	int		ret;
> +
> +	if (offset > PHY_CTRL_REG_OFFSET_MAX) {
> +		dev_err(dev, "Invalid reg address 0x%04x\n", offset);
> +		return -EINVAL;
> +	}
> +
> +	/* Address stage */
> +	ret = imx8mq_phy_ctrl_reg_addr(imx_phy, offset);
> +	if (ret)
> +		return ret;
> +
> +	/* Capture data stage */
> +	writel(val, cr_ctrl);
> +	ret = imx8mq_phy_wait_for_cr_ack(imx_phy, PHY_CRCTL_CAP_DATA, NULL);
> +	if (ret < 0)
> +		goto cr_write_err;
> +
> +	/* Write data stage */
> +	ret = imx8mq_phy_wait_for_cr_ack(imx_phy, PHY_CRCTL_CR_WRITE, NULL);
> +	if (ret < 0)
> +		goto cr_write_err;
> +
> +	return 0;
> +
> +cr_write_err:
> +	dev_err(dev, "Failed to write reg 0x%04x\n", offset);
> +	return -EIO;
> +}
> +
> +static int ctrl_reg_value_show(struct seq_file *s, void *unused)
> +{
> +	struct imx8mq_usb_phy *imx_phy = s->private;
> +	u16 base = imx_phy->cr_access_base;
> +	u32 val;
> +	int i, ret;
> +
> +	for (i = 0; i < imx_phy->cr_read_count; i++) {
> +		ret = imx8mq_phy_ctrl_reg_read(imx_phy, base + i, &val);
> +		if (ret < 0)
> +			return ret;
> +
> +		seq_printf(s, "Control Register 0x%04x value is 0x%04x\n",
> +			   base + i, val);
> +	}
> +
> +	return 0;
> +}
> +
> +static ssize_t ctrl_reg_value_write(struct file *file, const char __user *ubuf,
> +				    size_t count, loff_t *ppos)
> +
> +{
> +	struct seq_file		*s = file->private_data;
> +	struct imx8mq_usb_phy	*imx_phy = s->private;
> +	u16			cr_value;
> +	int			ret;
> +
> +	ret = kstrtou16_from_user(ubuf, count, 16, &cr_value);
> +	if (ret)
> +		return ret;
> +
> +	ret = imx8mq_phy_ctrl_reg_write(imx_phy, imx_phy->cr_access_base, cr_value);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
> +DEFINE_SHOW_STORE_ATTRIBUTE(ctrl_reg_value);
> +
> +static void imx8m_create_debug_files(struct imx8mq_usb_phy *imx_phy)
> +{
> +	struct dentry *debugfs = imx_phy->phy->debugfs;
> +
> +	debugfs_create_x16("ctrl_reg_base", 0600, debugfs,
> +			   &imx_phy->cr_access_base);
> +	debugfs_create_x16("ctrl_reg_count", 0600, debugfs,
> +			   &imx_phy->cr_read_count);
> +	debugfs_create_file("ctrl_reg_value", 0600, debugfs,
> +			    imx_phy, &ctrl_reg_value_fops);
> +
> +	imx_phy->cr_access_base = 0;
> +	imx_phy->cr_read_count = 1;
>  }
>
>  static void imx8m_get_phy_tuning_data(struct imx8mq_usb_phy *imx_phy)
> @@ -731,6 +906,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  					"failed to get tca\n");
>
>  	imx8m_get_phy_tuning_data(imx_phy);
> +	imx8m_create_debug_files(imx_phy);

where remove debug fs?

Frank
>
>  	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>
> --
> 2.34.1
>

On Thu, Jan 08, 2026 at 10:43:01AM -0500, Frank Li wrote:
> On Thu, Jan 08, 2026 at 04:36:41PM +0800, Xu Yang wrote:
> > The CR port is a simple 16-bit data/address parallel port that is
> > provided for on-chip access to the control registers inside the
> > USB 3.0 femtoPHY[1].
> 
> 
> > While access to these registers is not required
> > for normal PHY operation, this interface enables you to access
> > some of the PHY’s diagnostic features during normal operation or
> > to override some basic PHY control signals.
> 
> Simple said "Export these registers by debugfs to help PHY’s diagnostic."
> should be enough

OK.

> 
> >
> > 3 debugfs files are created to read and write control registers,
> > all use hexadecimal format:
> > ctrl_reg_base: the register offset to write, or the start offset
> >                to read.
> > ctrl_reg_count: how many continuous registers to be read.
> > ctrl_reg_value: read to show the continuous registers value from
> >                 the offset in ctrl_reg_base, to ctrl_reg_base
> >                 + ctrl_reg_count - 1, one line for one register.
> >                 when write, override the register at ctrl_reg_base,
> >                 one time can only change one 16bits register.
> 
> how many regs? how about create file regNNN,

From 0x0 to 0x201F.

> 
> cat/echo to regNNN?

Only regNNN seems not convenient to dump non-fixed number of registers.

> 
> or create export file
> 
> echo 100 > export, then create REG100, so can use cat/echo modify it.

If use export, it seems need a place to doc these information. It's not
straightforward to use.

> 
> 
> >
> > Link[1]: https://www.synopsys.com/dw/doc.php/phy/usb3.0/femto/phy/x652_usb3_ss14lpp_18_ns/4.07a/dwc_usb3.0_femtophy_ss14lpp_08V18V_x1_databook.pdf
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes in v3:
> >  - add documentation link of registers
> > Changes in v2:
> >  - correct copyright
> >  - add imx8mq_phy_wait_for_cr_ack() helper and use it
> >  - use DEFINE_SHOW_STORE_ATTRIBUTE()
> >  - directly create debugfs file under imx phy debugfs
> >  - remove debug_remove_files() since the phy core will handle it
> > ---
> >  drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 180 ++++++++++++++++++++-
> >  1 file changed, 178 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > index 95f9264bd0f7..e11054f6673c 100644
> > --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > @@ -1,10 +1,11 @@
> >  // SPDX-License-Identifier: GPL-2.0+
> > -/* Copyright (c) 2017 NXP. */
> > +/* Copyright 2017-2025 NXP. */
> >
> >  #include <linux/bitfield.h>
> >  #include <linux/clk.h>
> > +#include <linux/debugfs.h>
> >  #include <linux/delay.h>
> > -#include <linux/io.h>
> > +#include <linux/iopoll.h>
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> >  #include <linux/phy/phy.h>
> > @@ -57,6 +58,20 @@
> >
> >  #define PHY_TUNE_DEFAULT		0xffffffff
> >
> > +/* PHY control register access */
> > +#define PHY_CTRL_REG_OFFSET_MAX		0x201f
> > +
> > +#define PHY_CRCTL			0x30
> > +#define PHY_CRCTL_DATA_IN_MASK		GENMASK(15, 0)
> > +#define PHY_CRCTL_CAP_ADDR		BIT(16)
> > +#define PHY_CRCTL_CAP_DATA		BIT(17)
> > +#define PHY_CRCTL_CR_WRITE		BIT(18)
> > +#define PHY_CRCTL_CR_READ		BIT(19)
> > +
> > +#define PHY_CRSR			0x34
> > +#define PHY_CRSR_DATA_OUT_MASK		GENMASK(15, 0)
> > +#define PHY_CRSR_CR_ACK			BIT(16)
> > +
> >  #define TCA_CLK_RST			0x00
> >  #define TCA_CLK_RST_SW			BIT(9)
> >  #define TCA_CLK_RST_REF_CLK_EN		BIT(1)
> > @@ -118,6 +133,9 @@ struct imx8mq_usb_phy {
> >  	void __iomem *base;
> >  	struct regulator *vbus;
> >  	struct tca_blk *tca;
> > +	struct dentry *debugfs;
> > +	u16 cr_access_base;
> > +	u16 cr_read_count;
> >  	u32 pcs_tx_swing_full;
> >  	u32 pcs_tx_deemph_3p5db;
> >  	u32 tx_vref_tune;
> > @@ -411,6 +429,163 @@ static u32 phy_pcs_tx_swing_full_from_property(u32 percent)
> >  	percent = min(percent, 100U);
> >
> >  	return (percent * 127) / 100;
> > +};
> > +
> > +static int imx8mq_phy_wait_for_cr_ack(struct imx8mq_usb_phy *imx_phy,
> > +				      u32 stage, u32 *data)
> > +{
> > +	void __iomem	*cr_ctrl = imx_phy->base + PHY_CRCTL;
> > +	void __iomem	*cr_sr = imx_phy->base + PHY_CRSR;
> > +	u32		val;
> > +	int		ret;
> > +
> > +	writel(readl(cr_ctrl) | stage, cr_ctrl);
> > +	/* Wait CRSR[16] == 1 */
> > +	ret = readl_poll_timeout(cr_sr, val,
> > +				 (val & PHY_CRSR_CR_ACK) == PHY_CRSR_CR_ACK,
> > +				 1, 100);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (stage == PHY_CRCTL_CR_READ)
> > +		*data = readl(cr_sr) & 0xffff;
> > +
> > +	writel(readl(cr_ctrl) & (~stage), cr_ctrl);
> > +	/* Wait CRSR[16] == 0 */
> > +	return readl_poll_timeout(cr_sr, val, (val & PHY_CRSR_CR_ACK) == 0, 1, 100);
> > +}
> > +
> > +static int imx8mq_phy_ctrl_reg_addr(struct imx8mq_usb_phy *imx_phy, u16 offset)
> > +{
> > +	void __iomem	*cr_ctrl = imx_phy->base + PHY_CRCTL;
> > +	struct device	*dev = &imx_phy->phy->dev;
> > +	int		ret;
> > +
> > +	writel(offset, cr_ctrl);
> > +	ret = imx8mq_phy_wait_for_cr_ack(imx_phy, PHY_CRCTL_CAP_ADDR, NULL);
> > +	if (ret < 0) {
> > +		dev_err(dev, "Failed to address reg 0x%04x\n", offset);
> > +		return -EIO;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int imx8mq_phy_ctrl_reg_read(struct imx8mq_usb_phy *imx_phy,
> > +				    u16 offset, u32 *val)
> > +{
> > +	struct device *dev = &imx_phy->phy->dev;
> > +	int ret;
> > +
> > +	if (offset > PHY_CTRL_REG_OFFSET_MAX) {
> > +		dev_err(dev, "Invalid reg address 0x%04x\n", offset);
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Address stage */
> > +	ret = imx8mq_phy_ctrl_reg_addr(imx_phy, offset);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Read data stage */
> > +	ret = imx8mq_phy_wait_for_cr_ack(imx_phy, PHY_CRCTL_CR_READ, val);
> > +	if (ret < 0) {
> > +		dev_err(dev, "Failed to read reg 0x%04x\n", offset);
> > +		return -EIO;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static int imx8mq_phy_ctrl_reg_write(struct imx8mq_usb_phy *imx_phy,
> > +				     u16 offset, u16 val)
> > +{
> > +	struct device	*dev = &imx_phy->phy->dev;
> > +	void __iomem	*cr_ctrl = imx_phy->base + PHY_CRCTL;
> > +	int		ret;
> > +
> > +	if (offset > PHY_CTRL_REG_OFFSET_MAX) {
> > +		dev_err(dev, "Invalid reg address 0x%04x\n", offset);
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Address stage */
> > +	ret = imx8mq_phy_ctrl_reg_addr(imx_phy, offset);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Capture data stage */
> > +	writel(val, cr_ctrl);
> > +	ret = imx8mq_phy_wait_for_cr_ack(imx_phy, PHY_CRCTL_CAP_DATA, NULL);
> > +	if (ret < 0)
> > +		goto cr_write_err;
> > +
> > +	/* Write data stage */
> > +	ret = imx8mq_phy_wait_for_cr_ack(imx_phy, PHY_CRCTL_CR_WRITE, NULL);
> > +	if (ret < 0)
> > +		goto cr_write_err;
> > +
> > +	return 0;
> > +
> > +cr_write_err:
> > +	dev_err(dev, "Failed to write reg 0x%04x\n", offset);
> > +	return -EIO;
> > +}
> > +
> > +static int ctrl_reg_value_show(struct seq_file *s, void *unused)
> > +{
> > +	struct imx8mq_usb_phy *imx_phy = s->private;
> > +	u16 base = imx_phy->cr_access_base;
> > +	u32 val;
> > +	int i, ret;
> > +
> > +	for (i = 0; i < imx_phy->cr_read_count; i++) {
> > +		ret = imx8mq_phy_ctrl_reg_read(imx_phy, base + i, &val);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		seq_printf(s, "Control Register 0x%04x value is 0x%04x\n",
> > +			   base + i, val);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static ssize_t ctrl_reg_value_write(struct file *file, const char __user *ubuf,
> > +				    size_t count, loff_t *ppos)
> > +
> > +{
> > +	struct seq_file		*s = file->private_data;
> > +	struct imx8mq_usb_phy	*imx_phy = s->private;
> > +	u16			cr_value;
> > +	int			ret;
> > +
> > +	ret = kstrtou16_from_user(ubuf, count, 16, &cr_value);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = imx8mq_phy_ctrl_reg_write(imx_phy, imx_phy->cr_access_base, cr_value);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return count;
> > +}
> > +
> > +DEFINE_SHOW_STORE_ATTRIBUTE(ctrl_reg_value);
> > +
> > +static void imx8m_create_debug_files(struct imx8mq_usb_phy *imx_phy)
> > +{
> > +	struct dentry *debugfs = imx_phy->phy->debugfs;
> > +
> > +	debugfs_create_x16("ctrl_reg_base", 0600, debugfs,
> > +			   &imx_phy->cr_access_base);
> > +	debugfs_create_x16("ctrl_reg_count", 0600, debugfs,
> > +			   &imx_phy->cr_read_count);
> > +	debugfs_create_file("ctrl_reg_value", 0600, debugfs,
> > +			    imx_phy, &ctrl_reg_value_fops);
> > +
> > +	imx_phy->cr_access_base = 0;
> > +	imx_phy->cr_read_count = 1;
> >  }
> >
> >  static void imx8m_get_phy_tuning_data(struct imx8mq_usb_phy *imx_phy)
> > @@ -731,6 +906,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> >  					"failed to get tca\n");
> >
> >  	imx8m_get_phy_tuning_data(imx_phy);
> > +	imx8m_create_debug_files(imx_phy);
> 
> where remove debug fs?

phy core will remove debug fs when call phy_release(). 

Thanks,
Xu Yang

> 
> Frank
> >
> >  	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> >
> > --
> > 2.34.1
> >

On Fri, Jan 16, 2026 at 07:29:39PM +0800, Xu Yang wrote:
> On Thu, Jan 08, 2026 at 10:43:01AM -0500, Frank Li wrote:
> > On Thu, Jan 08, 2026 at 04:36:41PM +0800, Xu Yang wrote:
> > > The CR port is a simple 16-bit data/address parallel port that is
> > > provided for on-chip access to the control registers inside the
> > > USB 3.0 femtoPHY[1].
> > 
> > 
> > > While access to these registers is not required
> > > for normal PHY operation, this interface enables you to access
> > > some of the PHY’s diagnostic features during normal operation or
> > > to override some basic PHY control signals.
> > 
> > Simple said "Export these registers by debugfs to help PHY’s diagnostic."
> > should be enough
> 
> OK.
> 
> > 
> > >
> > > 3 debugfs files are created to read and write control registers,
> > > all use hexadecimal format:
> > > ctrl_reg_base: the register offset to write, or the start offset
> > >                to read.
> > > ctrl_reg_count: how many continuous registers to be read.
> > > ctrl_reg_value: read to show the continuous registers value from
> > >                 the offset in ctrl_reg_base, to ctrl_reg_base
> > >                 + ctrl_reg_count - 1, one line for one register.
> > >                 when write, override the register at ctrl_reg_base,
> > >                 one time can only change one 16bits register.
> > 
> > how many regs? how about create file regNNN,
> 
> >From 0x0 to 0x201F.

Rather than reinvent the wheel, could you use regmap?

https://elixir.bootlin.com/linux/v6.12.1/source/drivers/base/regmap/regmap-debugfs.c#L546

Regmap should be able to provide a debugfs interface for you, no
driver code needed.

This will also help you with the abstraction between the core generic
part of the PHY driver and the SoC integration glue. You pass the
regmap to the core driver, and the funny muxing through two registers
is hidden away from the core. If the next SoC integrated uses plan
MMIO, that SoC glue driver can instantiate an MMIO regmap.

Using regmap is a good idea for core generic drivers which can be
integrated into SoCs in different ways. It hides the SoC details
behind a well known API.

      Andrew

On Fri, Jan 16, 2026 at 04:19:01PM +0100, Andrew Lunn wrote:
> On Fri, Jan 16, 2026 at 07:29:39PM +0800, Xu Yang wrote:
> > On Thu, Jan 08, 2026 at 10:43:01AM -0500, Frank Li wrote:
> > > On Thu, Jan 08, 2026 at 04:36:41PM +0800, Xu Yang wrote:
> > > > The CR port is a simple 16-bit data/address parallel port that is
> > > > provided for on-chip access to the control registers inside the
> > > > USB 3.0 femtoPHY[1].
> > > 
> > > 
> > > > While access to these registers is not required
> > > > for normal PHY operation, this interface enables you to access
> > > > some of the PHY’s diagnostic features during normal operation or
> > > > to override some basic PHY control signals.
> > > 
> > > Simple said "Export these registers by debugfs to help PHY’s diagnostic."
> > > should be enough
> > 
> > OK.
> > 
> > > 
> > > >
> > > > 3 debugfs files are created to read and write control registers,
> > > > all use hexadecimal format:
> > > > ctrl_reg_base: the register offset to write, or the start offset
> > > >                to read.
> > > > ctrl_reg_count: how many continuous registers to be read.
> > > > ctrl_reg_value: read to show the continuous registers value from
> > > >                 the offset in ctrl_reg_base, to ctrl_reg_base
> > > >                 + ctrl_reg_count - 1, one line for one register.
> > > >                 when write, override the register at ctrl_reg_base,
> > > >                 one time can only change one 16bits register.
> > > 
> > > how many regs? how about create file regNNN,
> > 
> > >From 0x0 to 0x201F.
> 
> Rather than reinvent the wheel, could you use regmap?
> 
> https://elixir.bootlin.com/linux/v6.12.1/source/drivers/base/regmap/regmap-debugfs.c#L546
> 
> Regmap should be able to provide a debugfs interface for you, no
> driver code needed.
> 
> This will also help you with the abstraction between the core generic
> part of the PHY driver and the SoC integration glue. You pass the
> regmap to the core driver, and the funny muxing through two registers
> is hidden away from the core. If the next SoC integrated uses plan
> MMIO, that SoC glue driver can instantiate an MMIO regmap.
> 
> Using regmap is a good idea for core generic drivers which can be
> integrated into SoCs in different ways. It hides the SoC details
> behind a well known API.

Thanks for your suggestion.

Using regmap is generally a good fit for reusable driver and I aggre it
helps abstract Soc-specific details.

However, the regmap defbugfs has its own limitations:
1. By default the register is read-only, add write operation require rebuild
   the kernel which make it inconvenient to debug the issue on the spot. 
   Refer to: "09c6ecd39410 regmap: Add support for writing to regmap registers
              via debugfs"

    # ls /sys/kernel/debug/regmap/2-0050/ -l
    total 0
    -r-------- 1 root root 0 Jan 20 07:45 access
    -r-------- 1 root root 0 Jan 20 07:45 name
    -r-------- 1 root root 0 Jan 20 07:45 range
    -r-------- 1 root root 0 Jan 20 07:45 registers

2. It can't randomly read specific one register with common linux commands. Besides,
   the read operation is inefficient especially when the range is a bit large because
   when you cat the register it always read and output all the registers. 

Thanks,
Xu Yang

> 
>       Andrew
> 

> > Rather than reinvent the wheel, could you use regmap?
> > 
> > https://elixir.bootlin.com/linux/v6.12.1/source/drivers/base/regmap/regmap-debugfs.c#L546
> > 
> > Regmap should be able to provide a debugfs interface for you, no
> > driver code needed.
> > 
> > This will also help you with the abstraction between the core generic
> > part of the PHY driver and the SoC integration glue. You pass the
> > regmap to the core driver, and the funny muxing through two registers
> > is hidden away from the core. If the next SoC integrated uses plan
> > MMIO, that SoC glue driver can instantiate an MMIO regmap.
> > 
> > Using regmap is a good idea for core generic drivers which can be
> > integrated into SoCs in different ways. It hides the SoC details
> > behind a well known API.
> 
> Thanks for your suggestion.
> 
> Using regmap is generally a good fit for reusable driver and I aggre it
> helps abstract Soc-specific details.
> 
> However, the regmap defbugfs has its own limitations:
> 1. By default the register is read-only, add write operation require rebuild
>    the kernel which make it inconvenient to debug the issue on the spot.

That is somewhat deliberate. We don't want a nice API which can be
used for user space binary blob drivers. In networking, which is my
more normal area, we pretty much reject any sort of write interface,
other than official kernel APIs.

>    Refer to: "09c6ecd39410 regmap: Add support for writing to regmap registers
>               via debugfs"
> 
>     # ls /sys/kernel/debug/regmap/2-0050/ -l
>     total 0
>     -r-------- 1 root root 0 Jan 20 07:45 access
>     -r-------- 1 root root 0 Jan 20 07:45 name
>     -r-------- 1 root root 0 Jan 20 07:45 range
>     -r-------- 1 root root 0 Jan 20 07:45 registers
> 
> 2. It can't randomly read specific one register with common linux commands. Besides,
>    the read operation is inefficient especially when the range is a bit large because
>    when you cat the register it always read and output all the registers. 

You are debugging. Do you need efficient output?

If you have a specific debug tasks in mind, maybe you should be
thinking of an official kernel API? In the past, there has been
interest in getting SERDES eye information out of PHYs, and being able
to change the configuration parameters of the eye. Could a generic API
be added for that? Some of these PHYs also support pseudo random bit
sequence generators, and there has been interest in adding APIs for
configuring them.

	    Andrew

On Tue, Jan 20, 2026 at 02:32:30PM +0100, Andrew Lunn wrote:
> > > Rather than reinvent the wheel, could you use regmap?
> > > 
> > > https://elixir.bootlin.com/linux/v6.12.1/source/drivers/base/regmap/regmap-debugfs.c#L546
> > > 
> > > Regmap should be able to provide a debugfs interface for you, no
> > > driver code needed.
> > > 
> > > This will also help you with the abstraction between the core generic
> > > part of the PHY driver and the SoC integration glue. You pass the
> > > regmap to the core driver, and the funny muxing through two registers
> > > is hidden away from the core. If the next SoC integrated uses plan
> > > MMIO, that SoC glue driver can instantiate an MMIO regmap.
> > > 
> > > Using regmap is a good idea for core generic drivers which can be
> > > integrated into SoCs in different ways. It hides the SoC details
> > > behind a well known API.
> > 
> > Thanks for your suggestion.
> > 
> > Using regmap is generally a good fit for reusable driver and I aggre it
> > helps abstract Soc-specific details.
> > 
> > However, the regmap defbugfs has its own limitations:
> > 1. By default the register is read-only, add write operation require rebuild
> >    the kernel which make it inconvenient to debug the issue on the spot.
> 
> That is somewhat deliberate. We don't want a nice API which can be
> used for user space binary blob drivers. In networking, which is my
> more normal area, we pretty much reject any sort of write interface,
> other than official kernel APIs.

OK.

> 
> >    Refer to: "09c6ecd39410 regmap: Add support for writing to regmap registers
> >               via debugfs"
> > 
> >     # ls /sys/kernel/debug/regmap/2-0050/ -l
> >     total 0
> >     -r-------- 1 root root 0 Jan 20 07:45 access
> >     -r-------- 1 root root 0 Jan 20 07:45 name
> >     -r-------- 1 root root 0 Jan 20 07:45 range
> >     -r-------- 1 root root 0 Jan 20 07:45 registers
> > 
> > 2. It can't randomly read specific one register with common linux commands. Besides,
> >    the read operation is inefficient especially when the range is a bit large because
> >    when you cat the register it always read and output all the registers. 
> 
> You are debugging. Do you need efficient output?
> 
> If you have a specific debug tasks in mind, maybe you should be
> thinking of an official kernel API? In the past, there has been
> interest in getting SERDES eye information out of PHYs, and being able
> to change the configuration parameters of the eye. Could a generic API
> be added for that? Some of these PHYs also support pseudo random bit
> sequence generators, and there has been interest in adding APIs for
> configuring them.

After an internal discussion, we prefer to simply export Soc specific control
register access regmap in debugfs and let usespace read or write internal
registers according to CR port accessing sequence. With this, no needs for
a generic API for now.

Thanks for your comments.

Best Regards,
Xu Yang