public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v8 0/3] PCI: imx6: Add external reference clock mode support
@ 2025-10-24  2:40 Richard Zhu
  2025-10-24  2:40 ` [PATCH v8 1/3] dt-bindings: PCI: dwc: Add external reference clock input Richard Zhu
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Richard Zhu @ 2025-10-24  2:40 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, bhelgaas, frank.li, l.stach, lpieralisi,
	kwilczynski, mani, shawnguo, s.hauer, kernel, festevam
  Cc: linux-pci, linux-arm-kernel, devicetree, imx, linux-kernel

i.MX95 PCIes have two reference clock inputs: one from internal PLL, the
other from off chip crystal oscillator. The "extref" clock refers to a
reference clock from an external crystal oscillator.

Add external reference clock input mode support for i.MX95 PCIes.

Main change in v8:
- Rebase to v6.18-rc1.
- No need to initialize bool parameter to the deault value "false" refer
  to Mani' comments in v7

Main change in v7:
- Refine the subjects and commit message refer to Bjorn's comments.
https://lore.kernel.org/imx/20250918032555.3987157-1-hongxing.zhu@nxp.com/

Main change in v6:
- Refer to Krzysztof's comments, let i.MX95 PCIes has the "ref" clock
  since it is wired actually, and add one more optional "extref" clock
  for i.MX95 PCIes.
https://lore.kernel.org/imx/20250917045238.1048484-1-hongxing.zhu@nxp.com/

Main change in v5:
- Update the commit message of first patch refer to Bjorn's comments.
- Correct the typo error and update the description of property in the
  first patch.
https://lore.kernel.org/imx/20250915035348.3252353-1-hongxing.zhu@nxp.com/

Main change in v4:
- Add one more reference clock "extref" to be onhalf the reference clock
  that comes from external crystal oscillator.
https://lore.kernel.org/imx/20250626073804.3113757-1-hongxing.zhu@nxp.com/

Main change in v3:
- Update the logic check external reference clock mode is enabled or
  not in the driver codes.
https://lore.kernel.org/imx/20250620031350.674442-1-hongxing.zhu@nxp.com/

Main change in v2:
- Fix yamllint warning.
- Refine the driver codes.
https://lore.kernel.org/imx/20250619091004.338419-1-hongxing.zhu@nxp.com/

[PATCH v8 1/3] dt-bindings: PCI: dwc: Add external reference clock
[PATCH v8 2/3] dt-bindings: PCI: pci-imx6: Add external reference
[PATCH v8 3/3] PCI: imx6: Add external reference clock input mode

Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml      |  3 +++
Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml |  6 ++++++
drivers/pci/controller/dwc/pci-imx6.c                          | 19 ++++++++++++-------
3 files changed, 21 insertions(+), 7 deletions(-)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v8 1/3] dt-bindings: PCI: dwc: Add external reference clock input
  2025-10-24  2:40 [PATCH v8 0/3] PCI: imx6: Add external reference clock mode support Richard Zhu
@ 2025-10-24  2:40 ` Richard Zhu
  2025-10-24 16:51   ` Conor Dooley
  2025-10-24  2:40 ` [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: " Richard Zhu
  2025-10-24  2:40 ` [PATCH v8 3/3] PCI: imx6: Add external reference clock input mode support Richard Zhu
  2 siblings, 1 reply; 9+ messages in thread
From: Richard Zhu @ 2025-10-24  2:40 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, bhelgaas, frank.li, l.stach, lpieralisi,
	kwilczynski, mani, shawnguo, s.hauer, kernel, festevam
  Cc: linux-pci, linux-arm-kernel, devicetree, imx, linux-kernel,
	Richard Zhu, Frank Li

Add external reference clock input "extref" for a reference clock that
comes from external crystal oscillator.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 .../devicetree/bindings/pci/snps,dw-pcie-common.yaml        | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
index 34594972d8dbe..0134a759185ec 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
@@ -105,6 +105,12 @@ properties:
             define it with this name (for instance pipe, core and aux can
             be connected to a single source of the periodic signal).
           const: ref
+        - description:
+            Some dwc wrappers (like i.MX95 PCIes) have two reference clock
+            inputs, one from an internal PLL, the other from an off-chip crystal
+            oscillator. If present, 'extref' refers to a reference clock from
+            an external oscillator.
+          const: extref
         - description:
             Clock for the PHY registers interface. Originally this is
             a PHY-viewport-based interface, but some platform may have
-- 
2.37.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: Add external reference clock input
  2025-10-24  2:40 [PATCH v8 0/3] PCI: imx6: Add external reference clock mode support Richard Zhu
  2025-10-24  2:40 ` [PATCH v8 1/3] dt-bindings: PCI: dwc: Add external reference clock input Richard Zhu
@ 2025-10-24  2:40 ` Richard Zhu
  2025-10-24 17:06   ` Conor Dooley
  2025-10-24  2:40 ` [PATCH v8 3/3] PCI: imx6: Add external reference clock input mode support Richard Zhu
  2 siblings, 1 reply; 9+ messages in thread
From: Richard Zhu @ 2025-10-24  2:40 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, bhelgaas, frank.li, l.stach, lpieralisi,
	kwilczynski, mani, shawnguo, s.hauer, kernel, festevam
  Cc: linux-pci, linux-arm-kernel, devicetree, imx, linux-kernel,
	Richard Zhu, Frank Li

i.MX95 PCIes have two reference clock inputs: one from internal PLL, the
other from off chip crystal oscillator. The "extref" clock refers to a
reference clock from an external crystal oscillator.

Add external reference clock input for i.MX95 PCIes.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index ca5f2970f217c..b4c40d0573dce 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -212,14 +212,17 @@ allOf:
     then:
       properties:
         clocks:
+          minItems: 4
           maxItems: 5
         clock-names:
+          minItems: 4
           items:
             - const: pcie
             - const: pcie_bus
             - const: pcie_phy
             - const: pcie_aux
             - const: ref
+            - const: extref  # Optional
 
 unevaluatedProperties: false
 
-- 
2.37.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v8 3/3] PCI: imx6: Add external reference clock input mode support
  2025-10-24  2:40 [PATCH v8 0/3] PCI: imx6: Add external reference clock mode support Richard Zhu
  2025-10-24  2:40 ` [PATCH v8 1/3] dt-bindings: PCI: dwc: Add external reference clock input Richard Zhu
  2025-10-24  2:40 ` [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: " Richard Zhu
@ 2025-10-24  2:40 ` Richard Zhu
  2 siblings, 0 replies; 9+ messages in thread
From: Richard Zhu @ 2025-10-24  2:40 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, bhelgaas, frank.li, l.stach, lpieralisi,
	kwilczynski, mani, shawnguo, s.hauer, kernel, festevam
  Cc: linux-pci, linux-arm-kernel, devicetree, imx, linux-kernel,
	Richard Zhu, Frank Li

i.MX95 PCIes have two reference clock inputs: one from internal PLL, the
other from off chip crystal oscillator. The "extref" clock refers to a
reference clock from an external crystal oscillator.

Add external reference clock input mode support for i.MX95 PCIes.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 4668fc9648bff..a6db1f0f73c36 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -149,6 +149,7 @@ struct imx_pcie {
 	struct gpio_desc	*reset_gpiod;
 	struct clk_bulk_data	*clks;
 	int			num_clks;
+	bool			enable_ext_refclk;
 	struct regmap		*iomuxc_gpr;
 	u16			msi_ctrl;
 	u32			controller_id;
@@ -241,6 +242,8 @@ static unsigned int imx_pcie_grp_offset(const struct imx_pcie *imx_pcie)
 
 static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
 {
+	bool ext = imx_pcie->enable_ext_refclk;
+
 	/*
 	 * ERR051624: The Controller Without Vaux Cannot Exit L23 Ready
 	 * Through Beacon or PERST# De-assertion
@@ -259,13 +262,12 @@ static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
 			IMX95_PCIE_PHY_CR_PARA_SEL,
 			IMX95_PCIE_PHY_CR_PARA_SEL);
 
-	regmap_update_bits(imx_pcie->iomuxc_gpr,
-			   IMX95_PCIE_PHY_GEN_CTRL,
-			   IMX95_PCIE_REF_USE_PAD, 0);
-	regmap_update_bits(imx_pcie->iomuxc_gpr,
-			   IMX95_PCIE_SS_RW_REG_0,
+	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_PHY_GEN_CTRL,
+			   ext ? IMX95_PCIE_REF_USE_PAD : 0,
+			   IMX95_PCIE_REF_USE_PAD);
+	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_0,
 			   IMX95_PCIE_REF_CLKEN,
-			   IMX95_PCIE_REF_CLKEN);
+			   ext ? 0 : IMX95_PCIE_REF_CLKEN);
 
 	return 0;
 }
@@ -1602,7 +1604,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
 	struct imx_pcie *imx_pcie;
 	struct device_node *np;
 	struct device_node *node = dev->of_node;
-	int ret, domain;
+	int i, ret, domain;
 	u16 val;
 
 	imx_pcie = devm_kzalloc(dev, sizeof(*imx_pcie), GFP_KERNEL);
@@ -1653,6 +1655,9 @@ static int imx_pcie_probe(struct platform_device *pdev)
 	if (imx_pcie->num_clks < 0)
 		return dev_err_probe(dev, imx_pcie->num_clks,
 				     "failed to get clocks\n");
+	for (i = 0; i < imx_pcie->num_clks; i++)
+		if (strncmp(imx_pcie->clks[i].id, "extref", 6) == 0)
+			imx_pcie->enable_ext_refclk = true;
 
 	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_PHYDRV)) {
 		imx_pcie->phy = devm_phy_get(dev, "pcie-phy");
-- 
2.37.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v8 1/3] dt-bindings: PCI: dwc: Add external reference clock input
  2025-10-24  2:40 ` [PATCH v8 1/3] dt-bindings: PCI: dwc: Add external reference clock input Richard Zhu
@ 2025-10-24 16:51   ` Conor Dooley
  0 siblings, 0 replies; 9+ messages in thread
From: Conor Dooley @ 2025-10-24 16:51 UTC (permalink / raw)
  To: Richard Zhu
  Cc: robh, krzk+dt, conor+dt, bhelgaas, frank.li, l.stach, lpieralisi,
	kwilczynski, mani, shawnguo, s.hauer, kernel, festevam, linux-pci,
	linux-arm-kernel, devicetree, imx, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]

On Fri, Oct 24, 2025 at 10:40:11AM +0800, Richard Zhu wrote:
> Add external reference clock input "extref" for a reference clock that
> comes from external crystal oscillator.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

> ---
>  .../devicetree/bindings/pci/snps,dw-pcie-common.yaml        | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> index 34594972d8dbe..0134a759185ec 100644
> --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> @@ -105,6 +105,12 @@ properties:
>              define it with this name (for instance pipe, core and aux can
>              be connected to a single source of the periodic signal).
>            const: ref
> +        - description:
> +            Some dwc wrappers (like i.MX95 PCIes) have two reference clock
> +            inputs, one from an internal PLL, the other from an off-chip crystal
> +            oscillator. If present, 'extref' refers to a reference clock from
> +            an external oscillator.
> +          const: extref
>          - description:
>              Clock for the PHY registers interface. Originally this is
>              a PHY-viewport-based interface, but some platform may have
> -- 
> 2.37.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: Add external reference clock input
  2025-10-24  2:40 ` [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: " Richard Zhu
@ 2025-10-24 17:06   ` Conor Dooley
  2025-10-27  6:36     ` Hongxing Zhu
  0 siblings, 1 reply; 9+ messages in thread
From: Conor Dooley @ 2025-10-24 17:06 UTC (permalink / raw)
  To: Richard Zhu
  Cc: robh, krzk+dt, conor+dt, bhelgaas, frank.li, l.stach, lpieralisi,
	kwilczynski, mani, shawnguo, s.hauer, kernel, festevam, linux-pci,
	linux-arm-kernel, devicetree, imx, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2387 bytes --]

On Fri, Oct 24, 2025 at 10:40:12AM +0800, Richard Zhu wrote:
> i.MX95 PCIes have two reference clock inputs: one from internal PLL, the
> other from off chip crystal oscillator. The "extref" clock refers to a
> reference clock from an external crystal oscillator.
> 
> Add external reference clock input for i.MX95 PCIes.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> index ca5f2970f217c..b4c40d0573dce 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> @@ -212,14 +212,17 @@ allOf:
>      then:
>        properties:
>          clocks:
> +          minItems: 4
>            maxItems: 5
>          clock-names:
> +          minItems: 4
>            items:
>              - const: pcie

1

>              - const: pcie_bus

2

>              - const: pcie_phy

3

>              - const: pcie_aux

4

>              - const: ref

5

> +            - const: extref  # Optional

6

There are 6 clocks here, but clocks and clock-names in this binding do
not permit 6:
|  clocks:
|    minItems: 3
|    items:
|      - description: PCIe bridge clock.
|      - description: PCIe bus clock.
|      - description: PCIe PHY clock.
|      - description: Additional required clock entry for imx6sx-pcie,
|           imx6sx-pcie-ep, imx8mq-pcie, imx8mq-pcie-ep.
|      - description: PCIe reference clock.
|
|  clock-names:
|    minItems: 3
|    maxItems: 5

AFAICT, what this patch actually did is make "ref" an optional clock,
but the claim in the patch is that extref is optional. With this patch
applied, you can have a) no reference clocks or b) only "ref". "extref"
is never allowed.

Is it supposed to be possible to have "ref" and "extref"?
Or "extref" without "ref"?
Neither "ref" or "extref"?
I don't know the answer to that question because you're doing things
that are contradictory in your patch and the commit message isn't clear.

I don't see how this can have been successfully tested.

pw-bot: changes-requested


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: Add external reference clock input
  2025-10-24 17:06   ` Conor Dooley
@ 2025-10-27  6:36     ` Hongxing Zhu
  2025-10-27 17:06       ` Conor Dooley
  0 siblings, 1 reply; 9+ messages in thread
From: Hongxing Zhu @ 2025-10-27  6:36 UTC (permalink / raw)
  To: Conor Dooley
  Cc: robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	bhelgaas@google.com, Frank Li, l.stach@pengutronix.de,
	lpieralisi@kernel.org, kwilczynski@kernel.org, mani@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, imx@lists.linux.dev,
	linux-kernel@vger.kernel.org

> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: 2025年10月25日 1:07
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> bhelgaas@google.com; Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> lpieralisi@kernel.org; kwilczynski@kernel.org; mani@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
> imx@lists.linux.dev; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: Add external reference
> clock input
> 
> On Fri, Oct 24, 2025 at 10:40:12AM +0800, Richard Zhu wrote:
> > i.MX95 PCIes have two reference clock inputs: one from internal PLL,
> > the other from off chip crystal oscillator. The "extref" clock refers
> > to a reference clock from an external crystal oscillator.
> >
> > Add external reference clock input for i.MX95 PCIes.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > index ca5f2970f217c..b4c40d0573dce 100644
> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > @@ -212,14 +212,17 @@ allOf:
> >      then:
> >        properties:
> >          clocks:
> > +          minItems: 4
> >            maxItems: 5
> >          clock-names:
> > +          minItems: 4
> >            items:
> >              - const: pcie
> 
> 1
> 
> >              - const: pcie_bus
> 
> 2
> 
> >              - const: pcie_phy
> 
> 3
> 
> >              - const: pcie_aux
> 
> 4
> 
> >              - const: ref
> 
> 5
> 
> > +            - const: extref  # Optional
> 
> 6
> 
> There are 6 clocks here, but clocks and clock-names in this binding do not
> permit 6:
> |  clocks:
> |    minItems: 3
> |    items:
> |      - description: PCIe bridge clock.
> |      - description: PCIe bus clock.
> |      - description: PCIe PHY clock.
> |      - description: Additional required clock entry for imx6sx-pcie,
> |           imx6sx-pcie-ep, imx8mq-pcie, imx8mq-pcie-ep.
> |      - description: PCIe reference clock.
> |
> |  clock-names:
> |    minItems: 3
> |    maxItems: 5
> 
> AFAICT, what this patch actually did is make "ref" an optional clock, but the
> claim in the patch is that extref is optional. With this patch applied, you can
> have a) no reference clocks or b) only "ref". "extref"
> is never allowed. 
Hi Conor:
Thanks for your review comments.
Just same to "extref", the "ref" should be marked as optional clock too.

> 
> Is it supposed to be possible to have "ref" and "extref"?
> Or "extref" without "ref"?
> Neither "ref" or "extref"?
"ref" and "extref" have an exclusive relationship of choosing one of the two,
 and they cannot all exist simultaneously.

> I don't know the answer to that question because you're doing things that
> are contradictory in your patch and the commit message isn't clear.
> 
Sorry for causing you confusion.


Best Regards
Richard Zhu

> I don't see how this can have been successfully tested.
> 
> pw-bot: changes-requested


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: Add external reference clock input
  2025-10-27  6:36     ` Hongxing Zhu
@ 2025-10-27 17:06       ` Conor Dooley
  2025-10-28  5:56         ` Hongxing Zhu
  0 siblings, 1 reply; 9+ messages in thread
From: Conor Dooley @ 2025-10-27 17:06 UTC (permalink / raw)
  To: Hongxing Zhu
  Cc: robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	bhelgaas@google.com, Frank Li, l.stach@pengutronix.de,
	lpieralisi@kernel.org, kwilczynski@kernel.org, mani@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, imx@lists.linux.dev,
	linux-kernel@vger.kernel.org

[-- Attachment #1: Type: text/plain, Size: 3845 bytes --]

On Mon, Oct 27, 2025 at 06:36:32AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Sent: 2025年10月25日 1:07
> > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> > bhelgaas@google.com; Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> > lpieralisi@kernel.org; kwilczynski@kernel.org; mani@kernel.org;
> > shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> > festevam@gmail.com; linux-pci@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
> > imx@lists.linux.dev; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: Add external reference
> > clock input
> > 
> > On Fri, Oct 24, 2025 at 10:40:12AM +0800, Richard Zhu wrote:
> > > i.MX95 PCIes have two reference clock inputs: one from internal PLL,
> > > the other from off chip crystal oscillator. The "extref" clock refers
> > > to a reference clock from an external crystal oscillator.
> > >
> > > Add external reference clock input for i.MX95 PCIes.
> > >
> > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > >  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > index ca5f2970f217c..b4c40d0573dce 100644
> > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > @@ -212,14 +212,17 @@ allOf:
> > >      then:
> > >        properties:
> > >          clocks:
> > > +          minItems: 4
> > >            maxItems: 5
> > >          clock-names:
> > > +          minItems: 4
> > >            items:
> > >              - const: pcie
> > 
> > 1
> > 
> > >              - const: pcie_bus
> > 
> > 2
> > 
> > >              - const: pcie_phy
> > 
> > 3
> > 
> > >              - const: pcie_aux
> > 
> > 4
> > 
> > >              - const: ref
> > 
> > 5
> > 
> > > +            - const: extref  # Optional
> > 
> > 6
> > 
> > There are 6 clocks here, but clocks and clock-names in this binding do not
> > permit 6:
> > |  clocks:
> > |    minItems: 3
> > |    items:
> > |      - description: PCIe bridge clock.
> > |      - description: PCIe bus clock.
> > |      - description: PCIe PHY clock.
> > |      - description: Additional required clock entry for imx6sx-pcie,
> > |           imx6sx-pcie-ep, imx8mq-pcie, imx8mq-pcie-ep.
> > |      - description: PCIe reference clock.
> > |
> > |  clock-names:
> > |    minItems: 3
> > |    maxItems: 5
> > 
> > AFAICT, what this patch actually did is make "ref" an optional clock, but the
> > claim in the patch is that extref is optional. With this patch applied, you can
> > have a) no reference clocks or b) only "ref". "extref"
> > is never allowed. 
> Hi Conor:
> Thanks for your review comments.
> Just same to "extref", the "ref" should be marked as optional clock too.

Right, your commit message should then mention that.

> > Is it supposed to be possible to have "ref" and "extref"?
> > Or "extref" without "ref"?
> > Neither "ref" or "extref"?
> "ref" and "extref" have an exclusive relationship of choosing one of the two,
>  and they cannot all exist simultaneously.

Right, please go test what you've produced, because extref is never
permitted by this binding.

> > I don't know the answer to that question because you're doing things that
> > are contradictory in your patch and the commit message isn't clear.
> > 
> Sorry for causing you confusion.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: Add external reference clock input
  2025-10-27 17:06       ` Conor Dooley
@ 2025-10-28  5:56         ` Hongxing Zhu
  0 siblings, 0 replies; 9+ messages in thread
From: Hongxing Zhu @ 2025-10-28  5:56 UTC (permalink / raw)
  To: Conor Dooley
  Cc: robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	bhelgaas@google.com, Frank Li, l.stach@pengutronix.de,
	lpieralisi@kernel.org, kwilczynski@kernel.org, mani@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, imx@lists.linux.dev,
	linux-kernel@vger.kernel.org

> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: 2025年10月28日 1:06
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> bhelgaas@google.com; Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> lpieralisi@kernel.org; kwilczynski@kernel.org; mani@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
> imx@lists.linux.dev; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: Add external reference
> clock input
> 
> On Mon, Oct 27, 2025 at 06:36:32AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Conor Dooley <conor@kernel.org>
> > > Sent: 2025年10月25日 1:07
> > > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > > Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> > > bhelgaas@google.com; Frank Li <frank.li@nxp.com>;
> > > l.stach@pengutronix.de; lpieralisi@kernel.org;
> > > kwilczynski@kernel.org; mani@kernel.org; shawnguo@kernel.org;
> > > s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > devicetree@vger.kernel.org; imx@lists.linux.dev;
> > > linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: Add external
> > > reference clock input
> > >
> > > On Fri, Oct 24, 2025 at 10:40:12AM +0800, Richard Zhu wrote:
> > > > i.MX95 PCIes have two reference clock inputs: one from internal
> > > > PLL, the other from off chip crystal oscillator. The "extref"
> > > > clock refers to a reference clock from an external crystal oscillator.
> > > >
> > > > Add external reference clock input for i.MX95 PCIes.
> > > >
> > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > index ca5f2970f217c..b4c40d0573dce 100644
> > > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > @@ -212,14 +212,17 @@ allOf:
> > > >      then:
> > > >        properties:
> > > >          clocks:
> > > > +          minItems: 4
> > > >            maxItems: 5
> > > >          clock-names:
> > > > +          minItems: 4
> > > >            items:
> > > >              - const: pcie
> > >
> > > 1
> > >
> > > >              - const: pcie_bus
> > >
> > > 2
> > >
> > > >              - const: pcie_phy
> > >
> > > 3
> > >
> > > >              - const: pcie_aux
> > >
> > > 4
> > >
> > > >              - const: ref
> > >
> > > 5
> > >
> > > > +            - const: extref  # Optional
> > >
> > > 6
> > >
> > > There are 6 clocks here, but clocks and clock-names in this binding
> > > do not permit 6:
> > > |  clocks:
> > > |    minItems: 3
> > > |    items:
> > > |      - description: PCIe bridge clock.
> > > |      - description: PCIe bus clock.
> > > |      - description: PCIe PHY clock.
> > > |      - description: Additional required clock entry for imx6sx-pcie,
> > > |           imx6sx-pcie-ep, imx8mq-pcie, imx8mq-pcie-ep.
> > > |      - description: PCIe reference clock.
> > > |
> > > |  clock-names:
> > > |    minItems: 3
> > > |    maxItems: 5
> > >
> > > AFAICT, what this patch actually did is make "ref" an optional
> > > clock, but the claim in the patch is that extref is optional. With
> > > this patch applied, you can have a) no reference clocks or b) only "ref".
> "extref"
> > > is never allowed.
> > Hi Conor:
> > Thanks for your review comments.
> > Just same to "extref", the "ref" should be marked as optional clock too.
> 
> Right, your commit message should then mention that.
> 
Hi Conor:
It's my mistake. My previous understanding is wrong.
Only "extref" is an optional clock.

> > > Is it supposed to be possible to have "ref" and "extref"?
> > > Or "extref" without "ref"?
> > > Neither "ref" or "extref"?
> > "ref" and "extref" have an exclusive relationship of choosing one of
> > the two,  and they cannot all exist simultaneously.
> 
> Right, please go test what you've produced, because extref is never
> permitted by this binding.
"ref" and "extref" doesn't have an exclusive relationship. There are two
 options. one is "ref", the other one is "ref, extref".

If "ref" present, the internal system PLL clock is used as PCIe reference
 clock source. If both of them present, the PCIe reference clock would come
 from an off-chip crystal oscillator.

In the end, the maxItem of clocks should be '6'. I would post the another
 patch-set after correct the maxItem of clocks from '5' to '6'.

Sorry again to my misleading.

Best Regards
Richard Zhu
> 
> > > I don't know the answer to that question because you're doing things
> > > that are contradictory in your patch and the commit message isn't clear.
> > >
> > Sorry for causing you confusion.


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-10-28  5:57 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-24  2:40 [PATCH v8 0/3] PCI: imx6: Add external reference clock mode support Richard Zhu
2025-10-24  2:40 ` [PATCH v8 1/3] dt-bindings: PCI: dwc: Add external reference clock input Richard Zhu
2025-10-24 16:51   ` Conor Dooley
2025-10-24  2:40 ` [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: " Richard Zhu
2025-10-24 17:06   ` Conor Dooley
2025-10-27  6:36     ` Hongxing Zhu
2025-10-27 17:06       ` Conor Dooley
2025-10-28  5:56         ` Hongxing Zhu
2025-10-24  2:40 ` [PATCH v8 3/3] PCI: imx6: Add external reference clock input mode support Richard Zhu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox