Hi Joe,
On Mon, Oct 22, 2018 at 09:52:38PM +0000, Joe Hershberger wrote:
On Fri, Sep 14, 2018 at 7:50 AM Quentin Schulz quentin.schulz@bootlin.com wrote:
[...]
+#define MEDIA_OP_MODE_MASK 0x0700
Please use GENMASK() or BIT() for masks.
[...]
+/* Extended page GPIO register 00G */ +#define MSCC_DW8051_CNTL_STATUS 0 +#define MICRO_NSOFT_RESET 0x8000
Bit definitions such as these should use BIT().
It was purposely done as to keep consistency throughout the driver as it's not using any GENMASK or BIT currently. I suppose that I should just not care about that?
[...]
bus->write(bus, phy0, MDIO_DEVAD_NONE, MSCC_PHY_TEST_PAGE_8, reg);bus->write(bus, phy0, MDIO_DEVAD_NONE, MSCC_EXT_PAGE_ACCESS,MSCC_PHY_PAGE_TR);bus->write(bus, phy0, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_ADDR_16, 0xafa4);reg = bus->read(bus, phy0, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_18);reg &= ~0x007f;Use GENMASK() and #define
reg |= 0x0019;Magic number... use #define.
I've got no more information from the vendor on those ones. I wish I could define them something else than MSCC_PHY_REG_TR_DATA_18_MASK_FOO and MSCC_PHY_REG_TR_DATA_18_BAR.
bus->write(bus, phy0, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_18, reg);bus->write(bus, phy0, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_ADDR_16, 0x8fa4);Magic number... use #define
It's representing an address, I have no other information than that unfortunately.
Comment what this is doing and link the errata that it came from...
No information on the errata (if there's even one). Directly taken from the baremetal BSP, AFAICT it's optimized electrical settings given by hardware engineers.
vsc8584_csr_write(bus, phy0, 0x07fa, 0x0050100f);vsc8584_csr_write(bus, phy0, 0x1688, 0x00049f81);vsc8584_csr_write(bus, phy0, 0x0f90, 0x00688980);vsc8584_csr_write(bus, phy0, 0x03a4, 0x0000d8f0);vsc8584_csr_write(bus, phy0, 0x0fc0, 0x00000400);
[...]
Thanks, Quentin