> @@ -946,6 +949,9 @@ static void lan743x_phy_link_status_change(struct
> net_device *netdev)
> {
> struct lan743x_adapter *adapter = netdev_priv(netdev);
> struct phy_device *phydev = netdev->phydev;
> + struct device_node *phynode;
> + phy_interface_t phyifc = PHY_INTERFACE_MODE_GMII;
> + u32 data;
>
> phy_print_status(phydev);
> if (phydev->state == PHY_RUNNING) {
> @@ -953,6 +959,48 @@ static void lan743x_phy_link_status_change(struct
> net_device *netdev)
> int remote_advertisement = 0;
> int local_advertisement = 0;
>
> + /* check if a fixed-link is defined in device-tree */
> + phynode = of_node_get(adapter->pdev->dev.of_node);
> + if (phynode && of_phy_is_fixed_link(phynode)) {
Hi Roelof
The whole point for fixed link is that it looks like a PHY. You should
not need to care if it is a real PHY or a fixed link.
> + /* Configure MAC to fixed link parameters */
> + data = lan743x_csr_read(adapter, MAC_CR);
> + /* Disable auto negotiation */
> + data &= ~(MAC_CR_ADD_ | MAC_CR_ASD_);
Why does the MAC care about autoneg? In general, all the MAC needs to
know is the speed and duplex.
> + /* Set duplex mode */
> + if (phydev->duplex)
> + data |= MAC_CR_DPX_;
> + else
> + data &= ~MAC_CR_DPX_;
> + /* Set bus speed */
> + switch (phydev->speed) {
> + case 10:
> + data &= ~MAC_CR_CFG_H_;
> + data &= ~MAC_CR_CFG_L_;
> + break;
> + case 100:
> + data &= ~MAC_CR_CFG_H_;
> + data |= MAC_CR_CFG_L_;
> + break;
> + case 1000:
> + data |= MAC_CR_CFG_H_;
> + data |= MAC_CR_CFG_L_;
> + break;
> + }
The current code is unusual, in that it uses
phy_ethtool_get_link_ksettings(). That should do the right thing with
a fixed-link PHY, although i don't know if anybody uses it like
this. So in theory, the current code should take care of duplex, flow
control, and speed for you. Just watch out for bug/missing features in
fixed link.
> + /* Set interface mode */
> + of_get_phy_mode(phynode, &phyifc);
> + if (phyifc == PHY_INTERFACE_MODE_RGMII ||
> + phyifc == PHY_INTERFACE_MODE_RGMII_ID ||
> + phyifc == PHY_INTERFACE_MODE_RGMII_RXID ||
> + phyifc == PHY_INTERFACE_MODE_RGMII_TXID)
> + /* RGMII */
> + data &= ~MAC_CR_MII_EN_;
> + else
> + /* GMII */
> + data |= MAC_CR_MII_EN_;
> + lan743x_csr_write(adapter, MAC_CR, data);
> + }
> + of_node_put(phynode);
It is normal to do of_get_phy_mode when connecting to the PHY, and
store the value in the private structure. This is also not specific to
fixed link.
There is also a helper you can use phy_interface_mode_is_rgmii().
> +
> memset(&ksettings, 0, sizeof(ksettings));
> phy_ethtool_get_link_ksettings(netdev, &ksettings);
> local_advertisement =
> @@ -974,6 +1022,8 @@ static void lan743x_phy_close(struct lan743x_adapter
> *adapter)
>
> phy_stop(netdev->phydev);
> phy_disconnect(netdev->phydev);
> + if (of_phy_is_fixed_link(adapter->pdev->dev.of_node))
> + of_phy_deregister_fixed_link(adapter->pdev->dev.of_node);
> netdev->phydev = NULL;
> }
>
> @@ -982,18 +1032,44 @@ static int lan743x_phy_open(struct lan743x_adapter
> *adapter)
> struct lan743x_phy *phy = &adapter->phy;
> struct phy_device *phydev;
> struct net_device *netdev;
> + struct device_node *phynode = NULL;
> + phy_interface_t phyifc = PHY_INTERFACE_MODE_GMII;
> int ret = -EIO;
netdev uses reverse christmas tree, meaning the lines should be
sorted, longest first, getting shorter.
>
> netdev = adapter->netdev;
> - phydev = phy_find_first(adapter->mdiobus);
> - if (!phydev)
> - goto return_error;
>
> - ret = phy_connect_direct(netdev, phydev,
> - lan743x_phy_link_status_change,
> - PHY_INTERFACE_MODE_GMII);
> - if (ret)
> - goto return_error;
> + /* check if a fixed-link is defined in device-tree */
> + phynode = of_node_get(adapter->pdev->dev.of_node);
> + if (phynode && of_phy_is_fixed_link(phynode)) {
> + netdev_dbg(netdev, "fixed-link detected\n");
This is something which is useful during debug. But once it works can
be removed.
> + ret = of_phy_register_fixed_link(phynode);
> + if (ret) {
> + netdev_err(netdev, "cannot register fixed PHY\n");
> + goto return_error;
> + }
> +
> + of_get_phy_mode(phynode, &phyifc);
> + phydev = of_phy_connect(netdev, phynode,
> + lan743x_phy_link_status_change,
> + 0, phyifc);
> + if (!phydev)
> + goto return_error;
> + } else {
> + phydev = phy_find_first(adapter->mdiobus);
> + if (!phydev)
> + goto return_error;
> +
> + ret = phy_connect_direct(netdev, phydev,
> + lan743x_phy_link_status_change,
> + PHY_INTERFACE_MODE_GMII);
> + /* Note: We cannot use phyifc here because this would be SGMII
> + * on a standard PC.
> + */
I don't understand this comment.
