Re: [PATCH net-next v6] net: ngbe: Add ngbe mdio bus driver.

[Andrew Lunn <andrew@lunn.ch>]

> >> +static void ngbe_phy_fixup(struct wx *wx)
> >> +{
> >> + struct phy_device *phydev = wx->phydev;
> >> + struct ethtool_eee eee;
> >> +
> >> + if (wx->mac_type != em_mac_type_mdi)
> >> + return;
> > 
> > Does this mean that if using the internal PHY the MAC does support EEE
> > and half duplex?
> 
> 
> 1) The MAC does not support half duplex. 
>    Internal phy and external phys all need to close half duplex.
> 
> 2) The internal phy does not support eee. 
>    When using the internal phy, we disable eee.

So this condition is wrong and need deleting?

> >> +int ngbe_mdio_init(struct wx *wx)
> >> +{
> >> + struct pci_dev *pdev = wx->pdev;
> >> + struct mii_bus *mii_bus;
> >> + int ret;
> >> +
> >> + mii_bus = devm_mdiobus_alloc(&pdev->dev);
> >> + if (!mii_bus)
> >> + return -ENOMEM;
> >> +
> >> + mii_bus->name = "ngbe_mii_bus";
> >> + mii_bus->read = ngbe_phy_read_reg;
> >> + mii_bus->write = ngbe_phy_write_reg;
> >> + mii_bus->phy_mask = GENMASK(31, 4);
> >> + mii_bus->probe_capabilities = MDIOBUS_C22_C45;
> > 
> > That is not strictly true. The internal MDIO bus does not suport
> > C45. In practice, it probably does not matter.
> 
> >> mii_bus->probe_capabilities = MDIOBUS_C22_C45;
> So, it is not necessary?
> If I correct handling in read/write.

mii_bus->probe_capabilities controls how the MDIO bus is probed for
devices. MDIOBUS_C22_C45 means it will first look for C22 devices, and
then look for C45 devices. One of your busses does not support C45,
and will always return -EOPNOTSUPP. So it is just a waste of time
probing that bus for C45 devices. But it will not break, which is why
i said it probably does not matter. You could if you wanted set
mii_bus->probe_capabilities based on which bus is being used, internal
or external, and that might speed up the driver loading a little.

	Andrew