Re: [PATCH v2] net: phy: dp83867: support Wake on LAN

[Andrew Lunn <andrew@lunn.ch>]

> > +static int dp83867_set_wol(struct phy_device *phydev,
> > +			   struct ethtool_wolinfo *wol)
> > +{
> > +	struct net_device *ndev = phydev->attached_dev;
> > +	u16 val_rxcfg, val_micr;
> > +	const u8 *mac;
> > +
> > +	val_rxcfg = phy_read_mmd(phydev, DP83867_DEVADDR, DP83867_RXFCFG);
> > +	val_micr = phy_read(phydev, MII_DP83867_MICR);
> > +
> > +	if (wol->wolopts & (WAKE_MAGIC | WAKE_MAGICSECURE | WAKE_UCAST |
> > +			    WAKE_BCAST)) {
> > +		val_rxcfg |= DP83867_WOL_ENH_MAC;
> > +		val_micr |= MII_DP83867_MICR_WOL_INT_EN;
> > +
> > +		if (wol->wolopts & WAKE_MAGIC) {
> > +			mac = (const u8 *)ndev->dev_addr;
> 
> Using a cast to add/remove a const qualifier usually isn't too nice.
> Why not simply declare mac w/o const?
> 
> Also PHY might not be attached. I think ndev should be checked for NULL.

Hi Heiner

I thought about that as well. But the ethtool API is invoked using a
network interface name. It would be odd to call into the PHY driver if
the PHY was not attached. And the resulting Oops would help us
identify the bug.

Plus all the other PHY drivers which implement magic packet WoL assume
the PHY is attached. It could be they are all broken i suppose...

    Andrew