RE: [PATCH v3 2/6] dt-binding: phy: Add iMX8MP PCIe PHY binding

From: Hongxing Zhu
Date: Wed Aug 24 2022 - 05:15:23 EST


> -----Original Message-----
> From: Hongxing Zhu
> Sent: 2022年8月23日 10:12
> To: Rob Herring <robh@xxxxxxxxxx>
> Cc: p.zabel@xxxxxxxxxxxxxx; l.stach@xxxxxxxxxxxxxx; bhelgaas@xxxxxxxxxx;
> lorenzo.pieralisi@xxxxxxx; shawnguo@xxxxxxxxxx; vkoul@xxxxxxxxxx;
> alexander.stein@xxxxxxxxxxxxxxx; marex@xxxxxxx;
> linux-phy@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> linux-pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; dl-linux-imx
> <linux-imx@xxxxxxx>
> Subject: RE: [PATCH v3 2/6] dt-binding: phy: Add iMX8MP PCIe PHY binding
>
> > -----Original Message-----
> > From: Rob Herring <robh@xxxxxxxxxx>
> > Sent: 2022年8月23日 2:07
> > To: Hongxing Zhu <hongxing.zhu@xxxxxxx>
> > Cc: p.zabel@xxxxxxxxxxxxxx; l.stach@xxxxxxxxxxxxxx;
> > bhelgaas@xxxxxxxxxx; lorenzo.pieralisi@xxxxxxx; shawnguo@xxxxxxxxxx;
> > vkoul@xxxxxxxxxx; alexander.stein@xxxxxxxxxxxxxxx; marex@xxxxxxx;
> > linux-phy@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> > linux-pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> > linux-kernel@xxxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; dl-linux-imx
> > <linux-imx@xxxxxxx>
> > Subject: Re: [PATCH v3 2/6] dt-binding: phy: Add iMX8MP PCIe PHY
> > binding
> >
> > On Thu, Aug 18, 2022 at 03:02:29PM +0800, Richard Zhu wrote:
> > > Add i.MX8MP PCIe PHY binding.
> >
> > Explain the differences in h/w. The phy is connected to PERST#?
> >
> Hi Rob:
> Thanks for your review comments.
> Yes, it is. PERST# impacts PCIe PHY too.
> The default value of this bit is 1b'1 on i.MX8MQ/i.MX8MM platforms.
> But i.MX8MP has one inversed default value 1b'0 of PERST bit.
> The PERST bit should be kept 1b'1 after power and clocks are stable.
> So add the PHY PERST explicitly for i.MX8MP PCIe PHY.
>
> > >
> > > Signed-off-by: Richard Zhu <hongxing.zhu@xxxxxxx>
> > > ---
> > > .../bindings/phy/fsl,imx8-pcie-phy.yaml | 16
> +++++++++++++---
> > > 1 file changed, 13 insertions(+), 3 deletions(-)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
> > b/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
> > > index b6421eedece3..692783c7fd69 100644
> > > --- a/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
> > > +++ b/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
> > > @@ -16,6 +16,7 @@ properties:
> > > compatible:
> > > enum:
> > > - fsl,imx8mm-pcie-phy
> > > + - fsl,imx8mp-pcie-phy
> > >
> > > reg:
> > > maxItems: 1
> > > @@ -28,11 +29,16 @@ properties:
> > > - const: ref
> > >
> > > resets:
> > > - maxItems: 1
> > > + minItems: 1
> > > + maxItems: 2
> > >
> > > reset-names:
> > > - items:
> > > - - const: pciephy
> > > + oneOf:
> > > + - items: # for iMX8MM
> > > + - const: pciephy
> > > + - items: # for IMX8MP
> > > + - const: pciephy
> > > + - const: perst
> >
> > This does the same thing:
> >
> > minItems: 1
> > items:
> > - const: pciephy
> > - const: perst
> >
> Okay, thanks.
>
Hi Rob:
Do you mean the following definition of reset?
...
resets:
minItems: 1

reset-names:
minItems: 1
- const: pciephy
- const: perst
...
When do the dtbs_check later, it complains like below.
"
CHECK arch/arm64/boot/dts/freescale/imx8mn-venice-gw7902.dtb
DTC arch/arm64/boot/dts/freescale/imx8mp-dhcom-pdk2.dtb
CHECK arch/arm64/boot/dts/freescale/imx8mp-dhcom-pdk2.dtb
/home/richard/work/linux-imx/arch/arm64/boot/dts/freescale/imx8mp-dhcom-pdk2.dtb: pcie-phy@32f00000: resets: [[83, 24], [83, 25]] is too long
From schema: /home/richard/work/linux-imx/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
DTC arch/arm64/boot/dts/freescale/imx8mp-evk.dtb
CHECK arch/arm64/boot/dts/freescale/imx8mp-evk.dtb
/home/richard/work/linux-imx/arch/arm64/boot/dts/freescale/imx8mp-evk.dtb: pcie-phy@32f00000: resets: [[61, 24], [61, 25]] is too long
From schema: /home/richard/work/linux-imx/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
"

Best Regards
Richard Zhu

> Best Regards
> Richard Zhu
> >
> > >
> > > fsl,refclk-pad-mode:
> > > description: |
> > > @@ -60,6 +66,10 @@ properties:
> > > description: A boolean property indicating the CLKREQ# signal is
> > > not supported in the board design (optional)
> > >
> > > + power-domains:
> > > + description: PCIe PHY power domain (optional).
> > > + maxItems: 1
> > > +
> > > required:
> > > - "#phy-cells"
> > > - compatible
> > > --
> > > 2.25.1
> > >
> > >