Subject: Re: [PATCH v1 net-next 6/9] lan743x: Add power management support

From: Andrew Lunn <[email-masked]>
Date: Thu, 5 Jul 2018 19:37:43 +0200

> +static void lan743x_ethtool_get_wol(struct net_device *netdev,
> +				    struct ethtool_wolinfo *wol)
> +{
> +	struct lan743x_adapter *adapter = netdev_priv(netdev);
> +	u32 data;
> +
> +	data = lan743x_csr_read(adapter, PMT_CTL);

Hi Bryan

Why do you do this read? You do not use the result.

> +
> +	wol->supported = WAKE_BCAST | WAKE_UCAST | WAKE_MCAST |
> +		WAKE_MAGIC | WAKE_PHY | WAKE_ARP;
> +
> +	wol->wolopts = adapter->wolopts;
> +}
> +#endif /* CONFIG_PM */
> +
> +static int lan743x_pm_wakeframe_crc16(const u8 *buf, int len)
> +{
> +	const u16 crc16poly = 0x8005;
> +	u16 bit, crc, msb;
> +	u8 data;
> +	int i;
> +
> +	crc = 0xFFFF;
> +	for (i = 0; i < len; i++) {
> +		data = *buf++;
> +		for (bit = 0; bit < 8; bit++) {
> +			msb = crc >> 15;
> +			crc <<= 1;
> +
> +			if (msb ^ (u16)(data & 1)) {
> +				crc ^= crc16poly;
> +				crc |= (u16)0x0001U;
> +			}
> +			data >>= 1;
> +		}
> +	}
> +

There are a few different crc algorithms in lib. Can you use one of
them, rather than implementing it yourself?

> +#if CONFIG_PM
> +static int lan743x_pm_suspend(struct device *dev)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct net_device *netdev = pci_get_drvdata(pdev);
> +	struct lan743x_adapter *adapter = netdev_priv(netdev);
> +	u16 phydata;
> +	int ret;
> +
> +	if (adapter->wolopts & WAKE_PHY) {
> +		phydata = phy_read(netdev->phydev, 27);
> +		phydata |= 0x0500;
> +		phy_write(netdev->phydev, 27, phydata);
> +	}

Shouldn't the PHY driver do this?

	  Andrew