[RESEND v1] clk: imx95-blk-ctl: Add func_out_en clock for i.MX9x PCIe

[RESEND v1] clk: imx95-blk-ctl: Add func_out_en clock for i.MX9x PCIe

[hongxing.zhu]

From: Richard Zhu <hongxing.zhu@nxp.com>

Add a func_out_en clock for i.MX9x PCIe to serve as the parent gate clock
of the CREF_EN (BIT6) gate clock. Both of these two gate clocks enable
the output of the internal 100MHz differential reference clock.

When the internal PLL clock is used as the PCIe reference clock, both
BIT6 (CREF_EN) and BIT2 (FUNC_OUTPUT_EN) control the PCIE_REF_OUT_CLK.

If these bits default to 1, the output clock is enabled. With typical
100-ohm termination on the board, this results in approximately 6mA of
unnecessary power consumption when the PCIe internal PLL clock is not
in use.

To eliminate this power consumption, add a func_out_en clock gate that
serves as the parent of the existing CREF_EN (BIT6) gate clock. Both
gates must be enabled to output the internal 100MHz differential
reference clock, and both will be disabled when the clock is not needed.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/imx/clk-imx95-blk-ctl.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-imx95-blk-ctl.c b/drivers/clk/imx/clk-imx95-blk-ctl.c
index 56bed44719954..1f9259f456079 100644
--- a/drivers/clk/imx/clk-imx95-blk-ctl.c
+++ b/drivers/clk/imx/clk-imx95-blk-ctl.c
@@ -286,18 +286,28 @@ static const struct imx95_blk_ctl_dev_data netcmix_dev_data = {
 static const struct imx95_blk_ctl_clk_dev_data hsio_blk_ctl_clk_dev_data[] = {
 	[0] = {
 		.name = "hsio_blk_ctl_clk",
-		.parent_names = (const char *[]){ "hsio_pll", },
+		.parent_names = (const char *[]){ "func_out_en", },
 		.num_parents = 1,
 		.reg = 0,
 		.bit_idx = 6,
 		.bit_width = 1,
 		.type = CLK_GATE,
 		.flags = CLK_SET_RATE_PARENT,
+	},
+	[1] = {
+		.name = "func_out_en",
+		.parent_names = (const char *[]){ "hsio_pll", },
+		.num_parents = 1,
+		.reg = 0,
+		.bit_idx = 2,
+		.bit_width = 1,
+		.type = CLK_GATE,
+		.flags = CLK_SET_RATE_PARENT,
 	}
 };
 
 static const struct imx95_blk_ctl_dev_data hsio_blk_ctl_dev_data = {
-	.num_clks = 1,
+	.num_clks = ARRAY_SIZE(hsio_blk_ctl_clk_dev_data),
 	.clk_dev_data = hsio_blk_ctl_clk_dev_data,
 	.clk_reg_offset = 0,
 };
-- 
2.34.1

Re: [RESEND v1] clk: imx95-blk-ctl: Add func_out_en clock for i.MX9x PCIe

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] The newly added clock is named `func_out_en`, which is too generic for the global Common Clock Framework (CCF) namespace.

Pre-existing issues:
- [High] The driver lacks a `.remove` callback but does not suppress sysfs unbinding, causing use-after-free and I/O page faults on unbind.
- [High] The `netcmix_dev_data` specifies an incorrect `.clk_reg_offset`, corrupting Ethernet clock state on suspend/resume.
--

--- Patch [1]: [RESEND v1] clk: imx95-blk-ctl: Add func_out_en clock for i.MX9x PCIe ---
commit 5827f093c090c0f0800defab20d47c7314cbe3c9
Author: Richard Zhu <hongxing.zhu@nxp.com>

clk: imx95-blk-ctl: Add func_out_en clock for i.MX9x PCIe

The patch adds a func_out_en clock gate that serves as the parent of the 
existing CREF_EN gate clock for i.MX9x PCIe, reducing unnecessary power 
consumption.

> diff --git a/drivers/clk/imx/clk-imx95-blk-ctl.c b/drivers/clk/imx/clk-imx95-blk-ctl.c
> index 56bed44719954..1f9259f456079 100644
> --- a/drivers/clk/imx/clk-imx95-blk-ctl.c
> +++ b/drivers/clk/imx/clk-imx95-blk-ctl.c

[ ... ]

> @@ -286,18 +286,28 @@ static const struct imx95_blk_ctl_dev_data netcmix_dev_data = {

[Severity: High]
This is a pre-existing issue, but does netcmix_dev_data specify an
incorrect .clk_reg_offset that could corrupt Ethernet clock state on
suspend and resume?

The clocks for netcmix_dev_data are all at offset 4:

drivers/clk/imx/clk-imx95-blk-ctl.c:netxmix_clk_dev_data[] = {
	[0] = {
		...
		.reg = 4,

However, .clk_reg_offset is initialized to 0:

drivers/clk/imx/clk-imx95-blk-ctl.c:netcmix_dev_data = {
	.num_clks = ARRAY_SIZE(netxmix_clk_dev_data),
	.clk_dev_data = netxmix_clk_dev_data,
	.clk_reg_offset = 0,
};

This offset is used by the suspend and resume routines to save and restore
the clock register state:

drivers/clk/imx/clk-imx95-blk-ctl.c:imx95_bc_runtime_suspend() {
	...
	bc->clk_reg_restore = readl(bc->base + bc->pdata->clk_reg_offset);
	...
}

Will this cause the driver to save and restore register 0 instead of
register 4, causing Ethernet RMII clock muxes to lose their configured state?

>  static const struct imx95_blk_ctl_clk_dev_data hsio_blk_ctl_clk_dev_data[] = {
>  	[0] = {
>  		.name = "hsio_blk_ctl_clk",
> -		.parent_names = (const char *[]){ "hsio_pll", },
> +		.parent_names = (const char *[]){ "func_out_en", },
>  		.num_parents = 1,
>  		.reg = 0,
>  		.bit_idx = 6,
>  		.bit_width = 1,
>  		.type = CLK_GATE,
>  		.flags = CLK_SET_RATE_PARENT,
> +	},
> +	[1] = {
> +		.name = "func_out_en",

[Severity: Medium]
Will this name conflict with other clocks in the global Common Clock
Framework (CCF) namespace?

Because "func_out_en" lacks a subsystem or block-specific prefix
(like "hsio_func_out_en"), it might cause namespace collisions if another
hardware block registers a clock with the same generic name.

> +		.parent_names = (const char *[]){ "hsio_pll", },
> +		.num_parents = 1,
> +		.reg = 0,
> +		.bit_idx = 2,
> +		.bit_width = 1,
> +		.type = CLK_GATE,
> +		.flags = CLK_SET_RATE_PARENT,
>  	}
>  };
>  
>  static const struct imx95_blk_ctl_dev_data hsio_blk_ctl_dev_data = {
> -	.num_clks = 1,
> +	.num_clks = ARRAY_SIZE(hsio_blk_ctl_clk_dev_data),
>  	.clk_dev_data = hsio_blk_ctl_clk_dev_data,
>  	.clk_reg_offset = 0,
>  };

[Severity: High]
This isn't a bug introduced by this patch, but could this driver cause a
use-after-free and I/O page faults on unbind?

The driver registers non-devm clock providers and clocks in imx95_bc_probe():

drivers/clk/imx/clk-imx95-blk-ctl.c:imx95_bc_probe() {
	...
		} else {
			hws[i] = clk_hw_register_gate(dev, data->name, data->parent_names[0],
						      data->flags, reg, data->bit_idx,
						      data->flags2, &bc->lock);
		}
	...
	ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get, clk_hw_data);
	...
}

However, it does not provide a .remove callback or suppress sysfs unbinding:

drivers/clk/imx/clk-imx95-blk-ctl.c:imx95_bc_driver = {
	.probe = imx95_bc_probe,
	.driver = {
		.name = "imx95-blk-ctl",
		.of_match_table = imx95_bc_of_match,
		.pm = &imx95_bc_pm_ops,
	},
};

If the driver is unbound, devres will automatically free the driver state and
unmap I/O memory, but the clocks remain registered with CCF.  Will subsequent
CCF operations access freed memory and unmapped registers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730085542.263025-1-hongxing.zhu@oss.nxp.com?part=1

Re: [RESEND v1] clk: imx95-blk-ctl: Add func_out_en clock for i.MX9x PCIe

[Frank Li]

On Thu, Jul 30, 2026 at 04:55:42PM +0800, hongxing.zhu@oss.nxp.com wrote:
> From: Richard Zhu <hongxing.zhu@nxp.com>
>
> Add a func_out_en clock for i.MX9x PCIe to serve as the parent gate clock
> of the CREF_EN (BIT6) gate clock. Both of these two gate clocks enable
> the output of the internal 100MHz differential reference clock.
>
> When the internal PLL clock is used as the PCIe reference clock, both
> BIT6 (CREF_EN) and BIT2 (FUNC_OUTPUT_EN) control the PCIE_REF_OUT_CLK.
>
> If these bits default to 1, the output clock is enabled. With typical
> 100-ohm termination on the board, this results in approximately 6mA of
> unnecessary power consumption when the PCIe internal PLL clock is not
> in use.
>
> To eliminate this power consumption, add a func_out_en clock gate that
> serves as the parent of the existing CREF_EN (BIT6) gate clock. Both
> gates must be enabled to output the internal 100MHz differential
> reference clock, and both will be disabled when the clock is not needed.
>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/clk/imx/clk-imx95-blk-ctl.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-imx95-blk-ctl.c b/drivers/clk/imx/clk-imx95-blk-ctl.c
> index 56bed44719954..1f9259f456079 100644
> --- a/drivers/clk/imx/clk-imx95-blk-ctl.c
> +++ b/drivers/clk/imx/clk-imx95-blk-ctl.c
> @@ -286,18 +286,28 @@ static const struct imx95_blk_ctl_dev_data netcmix_dev_data = {
>  static const struct imx95_blk_ctl_clk_dev_data hsio_blk_ctl_clk_dev_data[] = {
>  	[0] = {
>  		.name = "hsio_blk_ctl_clk",
> -		.parent_names = (const char *[]){ "hsio_pll", },
> +		.parent_names = (const char *[]){ "func_out_en", },
>  		.num_parents = 1,
>  		.reg = 0,
>  		.bit_idx = 6,
>  		.bit_width = 1,
>  		.type = CLK_GATE,
>  		.flags = CLK_SET_RATE_PARENT,
> +	},
> +	[1] = {
> +		.name = "func_out_en",
> +		.parent_names = (const char *[]){ "hsio_pll", },
> +		.num_parents = 1,
> +		.reg = 0,
> +		.bit_idx = 2,
> +		.bit_width = 1,
> +		.type = CLK_GATE,
> +		.flags = CLK_SET_RATE_PARENT,
>  	}
>  };
>
>  static const struct imx95_blk_ctl_dev_data hsio_blk_ctl_dev_data = {
> -	.num_clks = 1,
> +	.num_clks = ARRAY_SIZE(hsio_blk_ctl_clk_dev_data),
>  	.clk_dev_data = hsio_blk_ctl_clk_dev_data,
>  	.clk_reg_offset = 0,
>  };
> --
> 2.34.1
>
>

Re: [RESEND v1] clk: imx95-blk-ctl: Add func_out_en clock for i.MX9x PCIe

[Abel Vesa]

On Thu, 30 Jul 2026 16:55:42 +0800, hongxing.zhu@oss.nxp.com wrote:
> Add a func_out_en clock for i.MX9x PCIe to serve as the parent gate clock
> of the CREF_EN (BIT6) gate clock. Both of these two gate clocks enable
> the output of the internal 100MHz differential reference clock.
> 
> When the internal PLL clock is used as the PCIe reference clock, both
> BIT6 (CREF_EN) and BIT2 (FUNC_OUTPUT_EN) control the PCIE_REF_OUT_CLK.
> 
> [...]

Applied, thanks!

[1/1] clk: imx95-blk-ctl: Add func_out_en clock for i.MX9x PCIe
      commit: 884dfb2dfe2cde33b8efe91ebf91542c88ca9034

Best regards,
-- 
Abel Vesa <abel.vesa@oss.qualcomm.com>