On Tue, Feb 06, 2024 at 04:32:29PM +0000, Conor Dooley wrote: > Hey Christian, > > On Mon, Feb 05, 2024 at 05:48:37PM +0100, Christian Marangi wrote: > > Document Qcom QCA807x PHY package. > > > > Qualcomm QCA807X Ethernet PHY is PHY package of 2 or 5 > > IEEE 802.3 clause 22 compliant 10BASE-Te, 100BASE-TX and > > 1000BASE-T PHY-s. > > > > Document the required property to make the PHY package correctly > > configure and work. > > > > Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx> > > I think this looks pretty decent, some minor comments. > > > + qcom,package-mode: > > + description: | > > + PHY package can be configured in 3 mode following this table: > > + > > + First Serdes mode Second Serdes mode > > + Option 1 PSGMII for copper Disabled > > + ports 0-4 > > + Option 2 PSGMII for copper 1000BASE-X / 100BASE-FX > > + ports 0-4 > > + Option 3 QSGMII for copper SGMII for > > + ports 0-3 copper port 4 > > + > > + PSGMII mode (option 1 or 2) is configured dynamically by the driver > > I'd drop mention of the driver here, with s/by the driver//. > Sure. > > + based on the presence of a connected SFP device. > > + $ref: /schemas/types.yaml#/definitions/string > > + enum: > > + - qsgmii > > + - psgmii > > + default: psgmii > > + > > + qcom,tx-driver-strength-milliwatt: > > Is this a typo? Should not it be "drive-strength"? There's 39 mentions > in tree of "driver-strength" and 3500 for "drive-strength". In the PHY datasheet the reg is called TX_DRIVER and the description say TX driver amplitude adjustment. But the section is PSGMII/QSGMII drive control 1 register... Guess it's a typo in the datasheet. Will change to drive. > > Otherwise I think the review comments have been resolved: > Reviewed-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> > Thanks! -- Ansuel
