netdev - Re: [PATCH RFC net-next 2/3] net: phy: read whether PHY supports stopping clock during LPI

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: <20170327203140.GG17041@lunn.ch>
Date:   Mon, 27 Mar 2017 22:31:40 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        rmk+kernel@...linux.org.uk
Subject: Re: [PATCH RFC net-next 2/3] net: phy: read whether PHY supports
 stopping clock during LPI

On Mon, Mar 27, 2017 at 11:47:20AM -0700, Florian Fainelli wrote:
> In order to use phy_init_eee() correctly, in particular the clk_stop
> argument, we need to know whether the Ethernet PHY supports stopping its
> clock.
> 
> Right now, we would have to call phy_init_eee(phydev, 1), see if that
> tails, and call again with phy_init_eee(phydev, 0) to enable EEE this is
> not an acceptable API use.

Hi Florain

I'm having trouble parsing this paragraph. Should tails be fails?  I
think "This is not an acceptable API use." should be a sentence?

> 
> Update phy_init_hw() to read whether the PHY supports this, and retain
> that information in the phydev structure so we can re-use it later.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
> ---
>  drivers/net/phy/phy_device.c | 23 ++++++++++++++++++++++-
>  include/linux/phy.h          |  2 ++
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 1219eeab69d1..2755d77626f7 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -837,6 +837,21 @@ static int phy_poll_reset(struct phy_device *phydev)
>  	return 0;
>  }
>  
> +static void phy_read_clock_stop_capable(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	/* Read if the PHY supports stopping its clocks (reg 3.1) */
> +	ret = phy_read_mmd_indirect(phydev, MDIO_MMD_PCS, MDIO_STAT1,
> +			            phydev->addr);
> +	if (ret < 0)
> +		return;

It is pretty unusual to ignore a real error. It might be better to
make this an int function, and return the error.

     Andrew