> -----Original Message----- > From: Rob Herring <robh@xxxxxxxxxx> > Sent: 2024年5月10日 23:39 > To: Hongxing Zhu <hongxing.zhu@xxxxxxx> > Cc: conor@xxxxxxxxxx; vkoul@xxxxxxxxxx; kishon@xxxxxxxxxx; > krzysztof.kozlowski+dt@xxxxxxxxxx; Frank Li <frank.li@xxxxxxx>; > conor+dt@xxxxxxxxxx; linux-phy@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > kernel@xxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v4 2/3] dt-bindings: phy: Add i.MX8Q HSIO SerDes PHY > binding > > On Thu, May 09, 2024 at 01:56:20PM +0800, Richard Zhu wrote: > > Add i.MX8QM and i.MX8QXP HSIO SerDes PHY binding. > > Introduce one HSIO configuration 'fsl,hsio-cfg', which need be set at > > initialization according to board design. > > > > Signed-off-by: Richard Zhu <hongxing.zhu@xxxxxxx> > > --- > > .../bindings/phy/fsl,imx8qm-hsio.yaml | 142 > ++++++++++++++++++ > > 1 file changed, 142 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml > > b/Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml > > new file mode 100644 > > index 000000000000..e8648cd9fea6 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/phy/fsl,imx8qm-hsio.yaml > > @@ -0,0 +1,142 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > > +cetree.org%2Fschemas%2Fphy%2Ffsl%2Cimx8qm-hsio.yaml%23&data=05%7C > 02%7 > > > +Chongxing.zhu%40nxp.com%7C2a3e55cc15c4465304eb08dc71074c2f%7C68 > 6ea1d3 > > > +bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638509523353991074%7CUnkn > own%7CTWF > > > +pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVC > I6 > > > +Mn0%3D%7C0%7C%7C%7C&sdata=fBXfU7NNPj57mnI3VXR3vR250oJ7kJrLNjY > 5W6mL0sk > > +%3D&reserved=0 > > +$schema: > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C02%7Chongxing.z > hu% > > > +40nxp.com%7C2a3e55cc15c4465304eb08dc71074c2f%7C686ea1d3bc2b4c6f > a92cd9 > > > +9c5c301635%7C0%7C0%7C638509523353999176%7CUnknown%7CTWFpbG > Zsb3d8eyJWI > > > +joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0 > %7C% > > > +7C%7C&sdata=n01dVLoIaHimlyaQTU1hUwIUNvmno7YUVnrjkTZK5TQ%3D&res > erved=0 > > + > > +title: Freescale i.MX8QM SoC series HSIO SERDES PHY > > + > > +maintainers: > > + - Richard Zhu <hongxing.zhu@xxxxxxx> > > + > > +properties: > > + compatible: > > + enum: > > + - fsl,imx8qm-hsio > > + - fsl,imx8qxp-hsio > > + reg: > > + minItems: 4 > > + maxItems: 4 > > Need to define what is each entry: > > items: > - description: ... > - description: ... > - description: ... > - description: ... > > > > + > > + "#phy-cells": > > + const: 3 > > + description: > > + The first defines lane index. > > + The second defines the type of the PHY refer to the include phy.h. > > + The third defines the controller index, indicated which controller > > + is bound to the lane. > > + > > + reg-names: > > + items: > > + - const: reg > > + - const: phy > > + - const: ctrl > > + - const: misc > > + > > + clocks: > > + minItems: 5 > > + maxItems: 14 > > + > > + clock-names: > > + minItems: 5 > > + maxItems: 14 > > + > > + fsl,hsio-cfg: > > + description: Refer macro HSIO_CFG* > include/dt-bindings/phy/phy-imx8-pcie.h. > > I can't, it's not in this patch. But it should be. > Hi Rob: Thanks for your comments. I would merge the first two of the patch-set into one patch later. Best Regards Richard Zhu > Please say something about what this is for, not just refer to header. > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > maximum: 3 > > > + > > + fsl,refclk-pad-mode: > > + description: > > + Specifies the mode of the refclk pad used. INPUT(PHY refclock is > > + provided externally via the refclk pad) or OUTPUT(PHY refclock is > > + derived from SoC internal source and provided on the refclk pad). > > + $ref: /schemas/types.yaml#/definitions/string > > + enum: [ "input", "output" ] > > default? > > Really, this could just be a boolean for the non-default mode. > > > + > > + power-domains: > > + minItems: 1 > > + maxItems: 2 > > + > > +required: > > + - compatible > > + - reg > > + - reg-names > > + - "#phy-cells" > > + - clocks > > + - clock-names > > + - fsl,hsio-cfg > > + > > +allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - fsl,imx8qxp-hsio > > + then: > > + properties: > > + clock-names: > > + items: > > + - const: pclk0 > > + - const: apb_pclk0 > > + - const: phy0_crr > > + - const: ctl0_crr > > + - const: misc_crr > > + power-domains: > > + minItems: 1 > > Should be maxItems? min is already 1. > > > + > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - fsl,imx8qm-hsio > > + then: > > + properties: > > + clock-names: > > + items: > > + - const: pclk0 > > + - const: pclk1 > > + - const: apb_pclk0 > > + - const: apb_pclk1 > > + - const: pclk2 > > + - const: epcs_tx > > + - const: epcs_rx > > + - const: apb_pclk2 > > + - const: phy0_crr > > + - const: phy1_crr > > + - const: ctl0_crr > > + - const: ctl1_crr > > + - const: ctl2_crr > > + - const: misc_crr > > + power-domains: > > + minItems: 2 > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/imx8-clock.h> > > + #include <dt-bindings/clock/imx8-lpcg.h> > > + #include <dt-bindings/firmware/imx/rsrc.h> > > + #include <dt-bindings/phy/phy-imx8-pcie.h> > > + > > + hsio_phy@5f1a0000 { > > phy@... > > > + compatible = "fsl,imx8qxp-hsio"; > > + reg = <0x5f1a0000 0x10000>, > > + <0x5f120000 0x10000>, > > + <0x5f140000 0x10000>, > > + <0x5f160000 0x10000>; > > + reg-names = "reg", "phy", "ctrl", "misc"; > > + clocks = <&phyx1_lpcg IMX_LPCG_CLK_0>, > > + <&phyx1_lpcg IMX_LPCG_CLK_4>, > > + <&phyx1_crr1_lpcg IMX_LPCG_CLK_4>, > > + <&pcieb_crr3_lpcg IMX_LPCG_CLK_4>, > > + <&misc_crr5_lpcg IMX_LPCG_CLK_4>; > > + clock-names = "pclk0", "apb_pclk0", "phy0_crr", "ctl0_crr", > "misc_crr"; > > + power-domains = <&pd IMX_SC_R_SERDES_1>; > > + #phy-cells = <3>; > > + fsl,hsio-cfg = <IMX8Q_HSIO_CFG_PCIEB>; > > + fsl,refclk-pad-mode = "input"; > > + }; > > +... > > -- > > 2.37.1 > >
