Hi Leif,
Comments inline.
Best Regards, Jan Dąbroś
But here is a whole bunch of magic values. Now, I'm not sure what they're doing, so perhaps there would be no benefit to replace them with defines, but... If not, could we have a block comment explaining what they're doing?
This code block is responsible for configuring PHY. In our reference code, we don't have any comments about what particular write is doing, so I think I will add there comment "// PHY configuration" - is it acceptable? What's more, single-line comments will be removed, since they aren't contain any information.
- Mdio->Write(Mdio, PhyAddr, 22, 0x00ff); /* page 0xff */
- Mdio->Write(Mdio, PhyAddr, 17, 0x214B);
- Mdio->Write(Mdio, PhyAddr, 16, 0x2144);
- Mdio->Write(Mdio, PhyAddr, 17, 0x0C28);
- Mdio->Write(Mdio, PhyAddr, 16, 0x2146);
- Mdio->Write(Mdio, PhyAddr, 17, 0xB233);
- Mdio->Write(Mdio, PhyAddr, 16, 0x214D);
- Mdio->Write(Mdio, PhyAddr, 17, 0xCC0C);
- Mdio->Write(Mdio, PhyAddr, 16, 0x2159);
- Mdio->Write(Mdio, PhyAddr, 22, 0x0000); /* reg page 0 */
- Mdio->Write(Mdio, PhyAddr, 22, 18); /* reg page 18 */
And regardless - could we be consistent with dec/hex?
Ok.
- /* Write HWCFG_MODE = SGMII to Copper */
- MvPhy1512WriteBits(PhyAddr, 20, 0, 3, 1);
- /* Phy reset */
- MvPhy1512WriteBits(PhyAddr, 20, 15, 1, 1);
- Mdio->Write(Mdio, PhyAddr, 22, 0); /* reg page 18 */
And is the reg page 18 still correct even though the value differs from the other line with the same comment?
As I wrote above - single line comments in this block will be removed.