Le 07/23/15 20:46, shh....@gmail.com a écrit :
> From: Shaohui Xie <shaohui....@freescale.com>
>
> This patch added driver to support Aquantia PHYs AQ1202, AQ2104, AQR105,
> AQR405, which accessed through clause 45.
Could you prefix your patches with "net: phy: " in the future to be
consistent with what is typically used?
See comments below
>
> Signed-off-by: Shaohui Xie <shaohui....@freescale.com>
> ---
[snip]
> +static int aquantia_read_status(struct phy_device *phydev)
> +{
> + int reg;
> +
> + phydev->speed = SPEED_10000;
> + phydev->duplex = DUPLEX_FULL;
> +
> + reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
> + reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
> + if (reg & MDIO_STAT1_LSTATUS)
> + phydev->link = 1;
> + else
> + phydev->link = 0;
> +
> + reg = phy_read_mmd(phydev, MDIO_MMD_AN, 0xc800);
> + mdelay(10);
> + reg = phy_read_mmd(phydev, MDIO_MMD_AN, 0xc800);
> + if (reg == 0x9)
> + phydev->speed = SPEED_2500;
> + else if (reg == 0x5)
> + phydev->speed = SPEED_1000;
> + else if (reg == 0x3)
> + phydev->speed = SPEED_100;
Could we use a switch/case here? How about 10Mbits/sec and duplex are we
guaranteed to be full-duplex at e.g: 100 or 10Mbits/sec?
> +
> + return 0;
> +}
> +
> +static struct phy_driver aquantia_driver[] = {
> +{
> + .phy_id = PHY_ID_AQ1202,
> + .phy_id_mask = 0xfffffff0,
> + .name = "Aquantia AQ1202",
> + .features = PHY_GBIT_FEATURES,
If these are 10GbE PHYs, should not we start defining a new features
bitmask here to reflect that accordingly? That way MAC
> + .soft_reset = aquantia_soft_reset,
> + .aneg_done = aquantia_aneg_done,
> + .config_init = aquantia_config_init,
> + .config_aneg = aquantia_config_aneg,
> + .read_status = aquantia_read_status,
> + .driver = { .owner = THIS_MODULE,},
> +},
> +{
> + .phy_id = PHY_ID_AQ2104,
> + .phy_id_mask = 0xfffffff0,
> + .name = "Aquantia AQ2104",
> + .features = PHY_GBIT_FEATURES,
> + .soft_reset = aquantia_soft_reset,
> + .aneg_done = aquantia_aneg_done,
> + .config_init = aquantia_config_init,
> + .config_aneg = aquantia_config_aneg,
> + .read_status = aquantia_read_status,
> + .driver = { .owner = THIS_MODULE,},
> +},
> +{
> + .phy_id = PHY_ID_AQR105,
> + .phy_id_mask = 0xfffffff0,
> + .name = "Aquantia AQR105",
> + .features = PHY_GBIT_FEATURES,
> + .soft_reset = aquantia_soft_reset,
> + .aneg_done = aquantia_aneg_done,
> + .config_init = aquantia_config_init,
> + .config_aneg = aquantia_config_aneg,
> + .read_status = aquantia_read_status,
> + .driver = { .owner = THIS_MODULE,},
> +},
> +{
> + .phy_id = PHY_ID_AQR405,
> + .phy_id_mask = 0xfffffff0,
> + .name = "Aquantia AQR405",
> + .features = PHY_GBIT_FEATURES,
> + .soft_reset = aquantia_soft_reset,
> + .aneg_done = aquantia_aneg_done,
> + .config_init = aquantia_config_init,
> + .config_aneg = aquantia_config_aneg,
> + .read_status = aquantia_read_status,
> + .driver = { .owner = THIS_MODULE,},
> +},
> +};
> +
> +static int __init aquantia_init(void)
> +{
> + return phy_drivers_register(aquantia_driver,
> + ARRAY_SIZE(aquantia_driver));
> +}
> +
> +static void __exit aquantia_exit(void)
> +{
> + return phy_drivers_unregister(aquantia_driver,
> + ARRAY_SIZE(aquantia_driver));
> +}
> +
> +module_init(aquantia_init);
> +module_exit(aquantia_exit);
> +
> +static struct mdio_device_id __maybe_unused aquantia_tbl[] = {
> + { PHY_ID_AQ1202, 0xfffffff0 },
> + { PHY_ID_AQ2104, 0xfffffff0 },
> + { PHY_ID_AQR105, 0xfffffff0 },
> + { PHY_ID_AQR405, 0xfffffff0 },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(mdio, aquantia_tbl);
> +
> +MODULE_DESCRIPTION("Aquantia PHY driver");
> +MODULE_AUTHOR("Shaohui Xie <shaohui....@freescale.com>");
> +MODULE_LICENSE("GPL v2");
>
--
Florian
--
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
