> +/* PHY Control Register */
> +#define PHY_CR_PHY_RESET 0x8000
> +#define PHY_CR_SPEED0 0x2000
> +#define PHY_CR_ANE 0x1000
> +#define PHY_CR_RESET_AN 0x0200
> +#define PHY_CR_FULLDUPLEX 0x0100
> +#define PHY_CR_SPEED1 0x0040
Reuse defines in mii.h? BMCR_SPEED1000, BMCR_*, etc.
> +#define PHY_TBICON_SRESET 0x8000
> +#define PHY_TBICON_CLK_SEL 0x0020
This is similarly named TBICON_CLK_SELECT in freescale/gianfar.h.
Perhaps use the same names, and if possible, use a shared header file?
It would be nice to define all bits of the TBI registers, if only for
completeness. Maybe along with a succinct description, e.g., that
"sreset" means soft reset. (Or just call it TBICON_SOFT_RESET.)
> +/* MII Configuration Control Memory Map Registers */
> +struct dtsec_mii_regs {
> +/* dTSEC Memory Map registers */
> +struct dtsec_regs {
> +static int mii_write_reg(struct fman_mac *dtsec, u8 addr, u8 reg, u16 data)
> +static int mii_read_reg(struct fman_mac *dtsec, u8 addr, u8 reg, u16 *data)
Any chance this patch overlaps significantly with fsl_pq_mdio.c?
http://lxr.free-electrons.com/source/drivers/net/ethernet/freescale/fsl_pq_mdio.c?v=4.3#L44
fsl_pq_mdio_write(),
fsl_pq_mdio_read(),
etc.
> + if (dtsec->phy_if == PHY_INTERFACE_MODE_SGMII) {
> + u16 tmp_reg16;
> +
> + /* Configure the TBI PHY Control Register */
> + tmp_reg16 = PHY_TBICON_CLK_SEL | PHY_TBICON_SRESET;
> + mii_write_reg(dtsec, (u8)dtsec_drv_param->tbipa, 17,
> + tmp_reg16);
> +
> + tmp_reg16 = PHY_TBICON_CLK_SEL;
> + mii_write_reg(dtsec, (u8)dtsec_drv_param->tbipa, 17,
> + tmp_reg16);
> +
> + tmp_reg16 =
> + (PHY_CR_PHY_RESET | PHY_CR_ANE | PHY_CR_FULLDUPLEX |
> + PHY_CR_SPEED1);
> + mii_write_reg(dtsec, (u8)dtsec_drv_param->tbipa, 0,
> tmp_reg16);
> +
> + if (dtsec->basex_if)
> + tmp_reg16 = PHY_TBIANA_1000X;
> + else
> + tmp_reg16 = PHY_TBIANA_SGMII;
> + mii_write_reg(dtsec, (u8)dtsec_drv_param->tbipa, 4,
> tmp_reg16);
> +
> + tmp_reg16 =
> + (PHY_CR_ANE | PHY_CR_RESET_AN | PHY_CR_FULLDUPLEX |
> + PHY_CR_SPEED1);
> +
> + mii_write_reg(dtsec, (u8)dtsec_drv_param->tbipa, 0,
> tmp_reg16);
> + }
Afaik, programming the TBI belongs in a separate PHY driver. See
drivers/net/phy/phy_device.c.
The same logic applies to gfar_configure_serdes() and uec_configure_serdes().
> +/* Internal PHY Registers - SGMII */
> +#define PHY_SGMII_CR_RESET_AN 0x0200
> +#define PHY_SGMII_CR_AN_ENABLE 0x1000
This namespace duplicates the intention of PHY_CR_*? (Which in turn
duplicated BMCR_*, ..)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
