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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <90A7E81AE28BAE4CBDDB3B35F187D264407392C8@CHN-SV-EXMX02.mchp-main.com>
Date:   Thu, 5 Jul 2018 19:29:00 +0000
From:   <Bryan.Whitehead@...rochip.com>
To:     <andrew@...n.ch>
CC:     <davem@...emloft.net>, <netdev@...r.kernel.org>,
        <UNGLinuxDriver@...rochip.com>
Subject: RE: [PATCH v1 net-next 6/9] lan743x: Add power management support

> > +	data = lan743x_csr_read(adapter, PMT_CTL);
> 
> Hi Bryan
> 
> Why do you do this read? You do not use the result.
> 
Good catch, I'll remove it.

> > +
> > +	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?

OK I'll check.

> 
> > +#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?

Perhaps so. I'll check with the PM writer.

Thanks Andrew