netdev - Re: [PATCH net-next 4/7] net: phy: add paged phy register accessors

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20171231081039.GA14485@lunn.ch>
Date:   Sun, 31 Dec 2017 09:10:39 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Russell King <rmk+kernel@...linux.org.uk>
Cc:     Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 4/7] net: phy: add paged phy register accessors

> +/**
> + * phy_save_page() - take the bus lock and save the current page
> + * @phydev: a pointer to a &struct phy_device
> + *
> + * Take the MDIO bus lock, and return the current page number. On error,
> + * returns a negative errno. phy_restore_page() must be called after this
> + * to release the lock even on failure.
> + */
> +int phy_save_page(struct phy_device *phydev)
> +{
> +	mutex_lock(&phydev->mdio.bus->mdio_lock);
> +	return __phy_read_page(phydev);
> +}
> +EXPORT_SYMBOL_GPL(phy_save_page);
> +
> +/**
> + * phy_select_page() - take the bus lock, save the current page, and set a page
> + * @phydev: a pointer to a &struct phy_device
> + * @page: desired page
> + *
> + * Take the MDIO bus lock to protect against concurrent access, save the
> + * current PHY page, and set the current page.  On error, returns a
> + * negative errno, otherwise returns the previous page number.
> + * phy_restore_page() must be called after this to restore the page
> + * number (if this call was successful) and release the lock.

Hi Russell

This comment seems wrong. It looks like you need to call
phy_restore_page() on error as well. I think the text in () should be
removed, and add the "even on failure" which the previous function
states.

	Andrew