On 11.01.2021 13:53, Marek Vasut wrote:
> Unless the internal PHY is connected and started, the phylib will not
> poll the PHY for state and produce state updates. Connect the PHY and
> start/stop it.
>
> Signed-off-by: Marek Vasut <ma...@denx.de>
> Cc: Andrew Lunn <and...@lunn.ch>
> Cc: Heiner Kallweit <hkallwe...@gmail.com>
> Cc: Lukas Wunner <lu...@wunner.de>
> ---
> drivers/net/ethernet/micrel/ks8851.h | 2 ++
> drivers/net/ethernet/micrel/ks8851_common.c | 28 +++++++++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/drivers/net/ethernet/micrel/ks8851.h
> b/drivers/net/ethernet/micrel/ks8851.h
> index e2eb0caeac82..ef13929036cf 100644
> --- a/drivers/net/ethernet/micrel/ks8851.h
> +++ b/drivers/net/ethernet/micrel/ks8851.h
> @@ -359,6 +359,7 @@ union ks8851_tx_hdr {
> * @vdd_io: Optional digital power supply for IO
> * @gpio: Optional reset_n gpio
> * @mii_bus: Pointer to MII bus structure
> + * @phy_dev: Pointer to PHY device structure
> * @lock: Bus access lock callback
> * @unlock: Bus access unlock callback
> * @rdreg16: 16bit register read callback
> @@ -405,6 +406,7 @@ struct ks8851_net {
> struct regulator *vdd_io;
> int gpio;
> struct mii_bus *mii_bus;
> + struct phy_device *phy_dev;
>
> void (*lock)(struct ks8851_net *ks,
> unsigned long *flags);
> diff --git a/drivers/net/ethernet/micrel/ks8851_common.c
> b/drivers/net/ethernet/micrel/ks8851_common.c
> index 058fd99bd483..a3716fd2d858 100644
> --- a/drivers/net/ethernet/micrel/ks8851_common.c
> +++ b/drivers/net/ethernet/micrel/ks8851_common.c
> @@ -432,6 +432,11 @@ static void ks8851_flush_tx_work(struct ks8851_net *ks)
> ks->flush_tx_work(ks);
> }
>
> +static void ks8851_handle_link_change(struct net_device *net)
> +{
> + phy_print_status(net->phydev);
> +}
> +
> /**
> * ks8851_net_open - open network device
> * @dev: The network device being opened.
> @@ -445,11 +450,22 @@ static int ks8851_net_open(struct net_device *dev)
> unsigned long flags;
> int ret;
>
> + ret = phy_connect_direct(ks->netdev, ks->phy_dev,
> + &ks8851_handle_link_change,
> + PHY_INTERFACE_MODE_INTERNAL);
> + if (ret) {
> + netdev_err(dev, "failed to attach PHY\n");
> + return ret;
> + }
> +
> + phy_attached_info(ks->phy_dev);
> +
> ret = request_threaded_irq(dev->irq, NULL, ks8851_irq,
> IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> dev->name, ks);
> if (ret < 0) {
> netdev_err(dev, "failed to get irq\n");
> + phy_disconnect(ks->phy_dev);
> return ret;
> }
>
> @@ -507,6 +523,7 @@ static int ks8851_net_open(struct net_device *dev)
> netif_dbg(ks, ifup, ks->netdev, "network device up\n");
>
> ks8851_unlock(ks, &flags);
> + phy_start(ks->phy_dev);
> mii_check_link(&ks->mii);
> return 0;
> }
> @@ -528,6 +545,9 @@ static int ks8851_net_stop(struct net_device *dev)
>
> netif_stop_queue(dev);
>
> + phy_stop(ks->phy_dev);
> + phy_disconnect(ks->phy_dev);
> +
> ks8851_lock(ks, &flags);
> /* turn off the IRQs and ack any outstanding */
> ks8851_wrreg16(ks, KS_IER, 0x0000);
> @@ -1084,6 +1104,7 @@ int ks8851_resume(struct device *dev)
>
> static int ks8851_register_mdiobus(struct ks8851_net *ks, struct device *dev)
> {
> + struct phy_device *phy_dev;
> struct mii_bus *mii_bus;
> int ret;
>
> @@ -1103,10 +1124,17 @@ static int ks8851_register_mdiobus(struct ks8851_net
> *ks, struct device *dev)
> if (ret)
> goto err_mdiobus_register;
>
> + phy_dev = phy_find_first(mii_bus);
> + if (!phy_dev)
> + goto err_find_phy;
> +
> ks->mii_bus = mii_bus;
> + ks->phy_dev = phy_dev;
>
> return 0;
>
> +err_find_phy:
> + mdiobus_unregister(mii_bus);
> err_mdiobus_register:
> mdiobus_free(mii_bus);
> return ret;
>
LGTM. When having a brief look at the driver I stumbled across two things:
1. Do MAC/PHY support any pause mode? Then a call to
phy_support_(a)sym_pause() would be missing.
2. Don't have the datasheet, but IRQ_LCI seems to be the link change
interrupt. So far it's ignored by the driver. You could configure
it and use phy_mac_interrupt() to operate the internal PHY in
interrupt mode.
