[PATCH v3 2/2] arm64: dts: imx8mq: Deduplicate PCIe clock-names property

Hongxing Zhu hongxing.zhu at nxp.com
Mon Jan 16 20:00:40 PST 2023 

Hi Marek:
Thanks for your patches.

> -----Original Message-----
> From: Marek Vasut <marex at denx.de>
> Sent: 2023年1月16日 18:14
> To: linux-arm-kernel at lists.infradead.org
> Cc: Marek Vasut <marex at denx.de>; Alexander Stein
> <alexander.stein at ew.tq-group.com>; Fabio Estevam <festevam at denx.de>;
> Peng Fan <peng.fan at nxp.com>; Hongxing Zhu <hongxing.zhu at nxp.com>;
> Shawn Guo <shawnguo at kernel.org>; dl-linux-imx <linux-imx at nxp.com>
> Subject: [PATCH v3 2/2] arm64: dts: imx8mq: Deduplicate PCIe clock-names
> property
> 
> Move the PCIe clock-names property from various DTs into SoC dtsi to reduce
> duplication. In case of a couple of boards, reorder the clock so they match the
> order in yaml DT bindings.
> 
> Reviewed-by: Alexander Stein <alexander.stein at ew.tq-group.com> #
> imx8mq.dtsi, imx8mq-tqma8mq-mba8mx.dts
> Signed-off-by: Marek Vasut <marex at denx.de>
> ---
> Cc: Fabio Estevam <festevam at denx.de>
> Cc: Peng Fan <peng.fan at nxp.com>
> Cc: Richard Zhu <hongxing.zhu at nxp.com>
> Cc: Shawn Guo <shawnguo at kernel.org>
> Cc: NXP Linux Team <linux-imx at nxp.com>
> To: linux-arm-kernel at lists.infradead.org
> ---
> V2: - Add RB from Alex
>     - Add default pcie clock entry into dtsi
>     - Fix the Ultra board
> V3: Rebase on latest next-20230116
> ---
>  arch/arm64/boot/dts/freescale/imx8mq-evk.dts           | 10 ++++------
>  .../boot/dts/freescale/imx8mq-kontron-pitx-imx8m.dts   | 10 ++++------
>  arch/arm64/boot/dts/freescale/imx8mq-mnt-reform2.dts   |  5 ++---
>  .../arm64/boot/dts/freescale/imx8mq-tqma8mq-mba8mx.dts | 10 ++++------
>  arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi    | 10 ++++------
>  arch/arm64/boot/dts/freescale/imx8mq.dtsi              | 10
> ++++++++++
>  6 files changed, 28 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
> b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
> index 78937910f4039..7507548cdb16b 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
> @@ -356,10 +356,9 @@ &pcie0 {
>  	pinctrl-0 = <&pinctrl_pcie0>;
>  	reset-gpio = <&gpio5 28 GPIO_ACTIVE_LOW>;
>  	clocks = <&clk IMX8MQ_CLK_PCIE1_ROOT>,
> -		 <&clk IMX8MQ_CLK_PCIE1_AUX>,
> +		 <&pcie0_refclk>,
>  		 <&clk IMX8MQ_CLK_PCIE1_PHY>,
> -		 <&pcie0_refclk>;
> -	clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus";
> +		 <&clk IMX8MQ_CLK_PCIE1_AUX>;
>  	vph-supply = <&vgen5_reg>;
>  	status = "okay";
>  };
> @@ -369,10 +368,9 @@ &pcie1 {
>  	pinctrl-0 = <&pinctrl_pcie1>;
>  	reset-gpio = <&gpio5 12 GPIO_ACTIVE_LOW>;
>  	clocks = <&clk IMX8MQ_CLK_PCIE2_ROOT>,
> -		 <&clk IMX8MQ_CLK_PCIE2_AUX>,
> +		 <&pcie0_refclk>,
>  		 <&clk IMX8MQ_CLK_PCIE2_PHY>,
> -		 <&pcie0_refclk>;
> -	clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus";
> +		 <&clk IMX8MQ_CLK_PCIE2_AUX>;
>  	vpcie-supply = <&reg_pcie1>;
>  	vph-supply = <&vgen5_reg>;
>  	status = "okay";
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-kontron-pitx-imx8m.dts
> b/arch/arm64/boot/dts/freescale/imx8mq-kontron-pitx-imx8m.dts
> index a91c136797f60..6376417e918c2 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-kontron-pitx-imx8m.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-kontron-pitx-imx8m.dts
> @@ -245,20 +245,18 @@ &pcie0 {
>  	pinctrl-0 = <&pinctrl_pcie0>;
>  	reset-gpio = <&gpio1 9 GPIO_ACTIVE_LOW>;
>  	clocks = <&clk IMX8MQ_CLK_PCIE1_ROOT>,
> -		 <&clk IMX8MQ_CLK_PCIE1_AUX>,
> +		 <&pcie0_refclk>,
>  		 <&clk IMX8MQ_CLK_PCIE1_PHY>,
> -		 <&pcie0_refclk>;
> -	clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus";
> +		 <&clk IMX8MQ_CLK_PCIE1_AUX>;
>  	status = "okay";
>  };
> 
>  /* Intel Ethernet Controller I210/I211 */
>  &pcie1 {
>  	clocks = <&clk IMX8MQ_CLK_PCIE2_ROOT>,
> -		 <&clk IMX8MQ_CLK_PCIE2_AUX>,
> +		 <&pcie1_refclk>,
>  		 <&clk IMX8MQ_CLK_PCIE2_PHY>,
> -		 <&pcie1_refclk>;
> -	clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus";
> +		 <&clk IMX8MQ_CLK_PCIE2_AUX>;
>  	fsl,max-link-speed = <1>;
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-mnt-reform2.dts
> b/arch/arm64/boot/dts/freescale/imx8mq-mnt-reform2.dts
> index 055031bba8c4b..200268660518d 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-mnt-reform2.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-mnt-reform2.dts
> @@ -197,10 +197,9 @@ &pcie1 {
>  	pinctrl-0 = <&pinctrl_pcie1>;
>  	reset-gpio = <&gpio3 23 GPIO_ACTIVE_LOW>;
>  	clocks = <&clk IMX8MQ_CLK_PCIE2_ROOT>,
> -		 <&clk IMX8MQ_CLK_PCIE2_AUX>,
> +		 <&pcie1_refclk>,
>  		 <&clk IMX8MQ_CLK_PCIE2_PHY>,
> -		 <&pcie1_refclk>;
> -	clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus";
> +		 <&clk IMX8MQ_CLK_PCIE2_AUX>;
>  	status = "okay";
>  };
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-tqma8mq-mba8mx.dts
> b/arch/arm64/boot/dts/freescale/imx8mq-tqma8mq-mba8mx.dts
> index d7660eab68b94..344cfdaeb1d59 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-tqma8mq-mba8mx.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-tqma8mq-mba8mx.dts
> @@ -105,10 +105,9 @@ &led2 {
>  &pcie0 {
>  	reset-gpio = <&expander0 14 GPIO_ACTIVE_LOW>;
>  	clocks = <&clk IMX8MQ_CLK_PCIE1_ROOT>,
> -		 <&clk IMX8MQ_CLK_PCIE1_AUX>,
> +		 <&pcie0_refclk>,
>  		 <&clk IMX8MQ_CLK_PCIE1_PHY>,
> -		 <&pcie0_refclk>;
> -	clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus";
> +		 <&clk IMX8MQ_CLK_PCIE1_AUX>;
>  	epdev_on-supply = <&reg_vcc_3v3>;
>  	hard-wired = <1>;
>  	status = "okay";
> @@ -120,10 +119,9 @@ &pcie0 {
>   */
>  &pcie1 {
>  	clocks = <&clk IMX8MQ_CLK_PCIE2_ROOT>,
> -		 <&clk IMX8MQ_CLK_PCIE2_AUX>,
> +		 <&pcie1_refclk>,
>  		 <&clk IMX8MQ_CLK_PCIE2_PHY>,
> -		 <&pcie1_refclk>;
> -	clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus";
> +		 <&clk IMX8MQ_CLK_PCIE2_AUX>;
>  	epdev_on-supply = <&reg_vcc_3v3>;
>  	hard-wired = <1>;
>  	status = "okay";
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
> index 4e05120c62d41..74a7a589a3296 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
> @@ -551,10 +551,9 @@ &pcie0 {
>  	pinctrl-0 = <&pinctrl_pcie0>;
>  	reset-gpio = <&gpio1 3 GPIO_ACTIVE_LOW>;
>  	clocks = <&clk IMX8MQ_CLK_PCIE1_ROOT>,
> -	         <&clk IMX8MQ_CLK_PCIE1_AUX>,
> +	         <&pcie0_refclk>,
>  	         <&clk IMX8MQ_CLK_PCIE1_PHY>,
> -	         <&pcie0_refclk>;
> -	clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus";
> +	         <&clk IMX8MQ_CLK_PCIE1_AUX>;
>  	vph-supply = <&vgen5_reg>;
>  	status = "okay";
>  };
> @@ -564,10 +563,9 @@ &pcie1 {
>  	pinctrl-0 = <&pinctrl_pcie1>;
>  	reset-gpio = <&gpio1 6 GPIO_ACTIVE_LOW>;
>  	clocks = <&clk IMX8MQ_CLK_PCIE2_ROOT>,
> -	         <&clk IMX8MQ_CLK_PCIE2_AUX>,
> +	         <&pcie1_refclk>,
>  	         <&clk IMX8MQ_CLK_PCIE2_PHY>,
> -	         <&pcie1_refclk>;
> -	clock-names = "pcie", "pcie_aux", "pcie_phy", "pcie_bus";
> +	         <&clk IMX8MQ_CLK_PCIE2_AUX>;
>  	vph-supply = <&vgen5_reg>;
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> index faed28e3ffa17..98fbba4c99a99 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> @@ -1542,6 +1542,11 @@ pcie0: pcie at 33800000 {
>  			                <0 0 0 4 &gic GIC_SPI 122
> IRQ_TYPE_LEVEL_HIGH>;
>  			fsl,max-link-speed = <2>;
>  			linux,pci-domain = <0>;
> +			clocks = <&clk IMX8MQ_CLK_PCIE1_ROOT>,
> +				 <&clk IMX8MQ_CLK_PCIE1_PHY>,
> +				 <&clk IMX8MQ_CLK_PCIE1_PHY>,
Why there are two PHY clocks?
Do you want to define IMX8MQ_CLK_PCIE1_PHY clock as "pcie_bus" clock here,
 and then change "pcie_bus" clock to "pcie0_refclk" in the evk board dts later?

How about to set the "pcie_bus" clock as " pcie#_refclk " directly in i.MX8MQ
 dtsi file?
Thus, it can avoid more duplicated codes in boards dts files further.

> +				 <&clk IMX8MQ_CLK_PCIE1_AUX>;
> +			clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux";
>  			power-domains = <&pgc_pcie>;
>  			resets = <&src IMX8MQ_RESET_PCIEPHY>,
>  			         <&src IMX8MQ_RESET_PCIE_CTRL_APPS_EN>, @@
> -1579,6 +1584,11 @@ pcie1: pcie at 33c00000 {
>  					<0 0 0 4 &gic GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
>  			fsl,max-link-speed = <2>;
>  			linux,pci-domain = <1>;
> +			clocks = <&clk IMX8MQ_CLK_PCIE2_ROOT>,
> +				 <&clk IMX8MQ_CLK_PCIE2_PHY>,
> +				 <&clk IMX8MQ_CLK_PCIE2_PHY>,
Same to above.

Best Regards
Richard Zhu

> +				 <&clk IMX8MQ_CLK_PCIE2_AUX>;
> +			clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux";
>  			power-domains = <&pgc_pcie>;
>  			resets = <&src IMX8MQ_RESET_PCIEPHY2>,
>  			         <&src IMX8MQ_RESET_PCIE2_CTRL_APPS_EN>,
> --
> 2.39.0

More information about the linux-arm-kernel mailing list