Re: [PATCH v3 4/5] phy: fsl-imx8mq-usb: add control register regmap

[Bough Chen <haibo.chen@oss.nxp.com>]

On Wed, Jun 03, 2026 at 01:37:17PM +0800, Xu Yang wrote:
> From: Xu Yang <xu.yang_2@nxp.com>
> 
> 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. Add control register regmap and export these
> registers by debugfs to help PHY's diagnostic.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v3:
>  - drop Frank's tag because it includes other changes
>  - new patch
> ---
>  drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index b0092c34416e..cda88ea1f12d 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> @@ -1,5 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0+
> -/* Copyright (c) 2017 NXP. */
> +/* Copyright 2017-2026 NXP. */

Should be : Copyright 2017, 2026 NXP.

>  
>  #include <linux/bitfield.h>
>  #include <linux/clk.h>
> @@ -11,6 +11,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/regmap.h>
>  #include <linux/usb/typec_mux.h>
>  
>  #define PHY_CTRL0			0x0
> @@ -56,6 +57,8 @@
>  #define PHY_CTRL6_ALT_CLK_EN		BIT(1)
>  #define PHY_CTRL6_ALT_CLK_SEL		BIT(0)
>  
> +#define PHY_CRCTL			0x30
> +
>  #define PHY_TUNE_DEFAULT		0xffffffff
>  
>  #define TCA_CLK_RST			0x00
> @@ -119,6 +122,7 @@ struct imx8mq_usb_phy {
>  	void __iomem *base;
>  	struct regulator *vbus;
>  	struct tca_blk *tca;
> +	struct regmap *cr_regmap;
>  	u32 pcs_tx_swing_full;
>  	u32 pcs_tx_deemph_3p5db;
>  	u32 tx_vref_tune;
> @@ -665,6 +669,14 @@ static const struct of_device_id imx8mq_usb_phy_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, imx8mq_usb_phy_of_match);
>  
> +static const struct regmap_config imx_cr_regmap_config = {
> +	.name = "cr",
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +	.max_register = 0x7,
> +};
> +

Your commit message says: The CR port is a simple 16-bit data/address parallel port
But here you use 32 bit mmio, which is a bit confuse to reader. 
Maybe you can add the following comment before the regmap config:

/*
 * CR Port MMIO Register Map
 * 
 * The CR (Control Register) port uses a 16-bit data/address protocol,
 * but is accessed through 32-bit MMIO registers:
 * 
 * Offset 0x0: CR Control Register
 *   [31:16] - Control/Status flags
 *   [15:0]  - 16-bit CR Address
 * 
 * Offset 0x4: CR Data Register
 *   [31:16] - Reserved/Status
 *   [15:0]  - 16-bit CR Data
 */

Or change your commit log like this:

The CR port is a 16-bit data/address parallel port protocol that is
accessed through 32-bit MMIO registers for on-chip access to the
control registers inside the USB 3.0 femtoPHY. Add control register
regmap and export these registers by debugfs to help PHY's diagnostic.

Regards
Haibo Chen

>  static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  {
>  	struct phy_provider *phy_provider;
> @@ -694,6 +706,13 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  	if (IS_ERR(imx_phy->base))
>  		return PTR_ERR(imx_phy->base);
>  
> +	imx_phy->cr_regmap = devm_regmap_init_mmio(dev, imx_phy->base + PHY_CRCTL,
> +						   &imx_cr_regmap_config);
> +	if (IS_ERR(imx_phy->cr_regmap)) {
> +		dev_warn(dev, "Fail to init debug register regmap\n");
> +		imx_phy->cr_regmap = NULL;
> +	}
> +
>  	ret = devm_pm_runtime_set_active_enabled(dev);
>  	if (ret)
>  		return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
> @@ -729,6 +748,9 @@ static int imx8mq_usb_phy_runtime_suspend(struct device *dev)
>  {
>  	struct imx8mq_usb_phy *imx_phy = dev_get_drvdata(dev);
>  
> +	if (imx_phy->cr_regmap)
> +		regcache_cache_only(imx_phy->cr_regmap, true);
> +
>  	clk_disable_unprepare(imx_phy->alt_clk);
>  	clk_disable_unprepare(imx_phy->clk);
>  
> @@ -750,6 +772,9 @@ static int imx8mq_usb_phy_runtime_resume(struct device *dev)
>  		return ret;
>  	}
>  
> +	if (imx_phy->cr_regmap)
> +		regcache_cache_only(imx_phy->cr_regmap, false);
> +
>  	return 0;
>  }
>  
> 
> -- 
> 2.34.1
>