> -----Original Message----- > From: Conor Dooley <conor@xxxxxxxxxx> > Sent: 2024年5月14日 0:02 > To: Hongxing Zhu <hongxing.zhu@xxxxxxx> > Cc: vkoul@xxxxxxxxxx; kishon@xxxxxxxxxx; robh+dt@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 v5 1/2] dt-bindings: phy: Add i.MX8Q HSIO SerDes PHY > binding > > On Mon, May 13, 2024 at 09:22:03AM +0800, Richard Zhu wrote: > > > + fsl,hsio-cfg: > > + description: > > + Specifies the use case of the HSIO module in the hardware design. > > + Because the HSIO module can be configure into three different use > > + cases. > > + Refer to macro HSIO_CFG* of > include/dt-bindings/phy/phy-imx8-pcie.h. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + maximum: 3 > > > +/* > > + * Regarding the design of i.MX8QM HSIO subsystem, HSIO module can be > > + * confiured as following three use cases. > > + * > > + * Define different configurations refer to the use cases, since it > > +is > > + * mandatory required in the initialization. > > + * > > + * On i.MX8QXP, HSIO module only has PCIEB and one lane PHY. > > + * Define "IMX8Q_HSIO_CFG_PCIEB" for i.MX8QXP platforms. > > + * > > + * +----------------------------------------------------+----------+ > > + * | | i.MX8QM | i.MX8QXP > | > > + * |-------------------------------|--------------------|----------| > > + * | | Lane0| Lane1| Lane2| Lane0 | > > + * |-------------------------------|------|------|------|----------| > > + * | IMX8Q_HSIO_CFG_PCIEAX2SATA | PCIEA| PCIEA| SATA | > | > > + * |-------------------------------|------|------|------|----------| > > + * | IMX8Q_HSIO_CFG_PCIEAX2PCIEB | PCIEA| PCIEA| PCIEB| > | > > + * |-------------------------------|------|------|------|----------| > > + * | IMX8Q_HSIO_CFG_PCIEAPCIEBSATA | PCIEA| PCIEB| SATA | | > > + * |-------------------------------|------|------|------|----------| > > + * | IMX8Q_HSIO_CFG_PCIEB | - | - | - | PCIEB | > > + * +----------------------------------------------------+----------+ > > + */ > > +#define IMX8Q_HSIO_CFG_PCIEAX2SATA 0x1 > > +#define IMX8Q_HSIO_CFG_PCIEAX2PCIEB 0x2 > > +#define IMX8Q_HSIO_CFG_PCIEAPCIEBSATA > (IMX8Q_HSIO_CFG_PCIEAX2SATA | IMX8Q_HSIO_CFG_PCIEAX2PCIEB) > > +#define IMX8Q_HSIO_CFG_PCIEB IMX8Q_HSIO_CFG_PCIEAX2PCIEB > > Rob may disagree with me, but I think this should be an enum of possible strings > with the table here moved into the property description. The QXP only option > should then be constrained per compatible. > If this property is an enum of possible strings. There would be three if() checks to parse the possible string in driver. I'm fine with that. Hi Rob: How do you think about that? > > + > > + 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). > > > + This property not exsit means unused(PHY refclock is derived > > + from > > Please run a spell checker on your patches. > Sorry, It's my fault. There is a spell mistake. s/exsit/exists/ Thanks. > > + SoC internal source). > > > + $ref: /schemas/types.yaml#/definitions/string > > + enum: [ input, output ] > > enum: [input, output, unused] > default: unused Okay Best Regards Richard > > Cheers, > Conor.