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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 24 Jul 2020 16:29:41 +0000
From:   <Bryan.Whitehead@...rochip.com>
To:     <f.fainelli@...il.com>, <davem@...emloft.net>
CC:     <netdev@...r.kernel.org>, <UNGLinuxDriver@...rochip.com>
Subject: RE: [PATCH net-next] mscc: Add LCPLL Reset to VSC8574 Family of phy
 drivers

Hi Florian, see below.

> >  /* bus->mdio_lock should be locked when using this function */
> > +/* Page should already be set to MSCC_PHY_PAGE_EXTENDED_GPIO */
> > +static int vsc8574_wait_for_micro_complete(struct phy_device *phydev)
> > +{
> > +     u16 timeout = 500;
> > +     u16 reg18g = 0;
> > +
> > +     reg18g = phy_base_read(phydev, 18);
> > +     while (reg18g & 0x8000) {
> > +             timeout--;
> > +             if (timeout == 0)
> > +                     return -1;
> > +             usleep_range(1000, 2000);
> > +             reg18g = phy_base_read(phydev, 18);
> 
> Please consider using phy_read_poll_timeout() instead of open coding this busy
> waiting loop.

There are a couple issues with the use of phy_read_poll_timeout
1) It requires the use of phy_read, which acquires bus->mdio_lock.
But this function is run with the assumption that, that lock is already acquired.
There for I presume it will deadlock.
2) The implementation of phy_base_read uses __phy_package_read which uses a shared phy addr, rather than the addr associated with the phydev.

These issues could be eliminated if I used read_poll_timeout directly.
Does that seem reasonable to you?

Regards,
Bryan

Powered by blists - more mailing lists