netdev - Re: [PATCH v1] net/phy: fix DP83865 10 Mbps HDX loopback disable function

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: <20190918152646.GL9591@lunn.ch>
Date:   Wed, 18 Sep 2019 17:26:46 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Peter Mamonov <pmamonov@...il.com>
Cc:     Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] net/phy: fix DP83865 10 Mbps HDX loopback disable
 function

On Wed, Sep 18, 2019 at 05:48:25PM +0300, Peter Mamonov wrote:
> According to the DP83865 datasheet "The 10 Mbps HDX loopback can be
> disabled in the expanded memory register 0x1C0.1." The driver erroneously
> used bit 0 instead of bit 1.

Hi Peter

This is version 2, not 1. Or if you want to start counting from 0, it
would be good to put v0 in your first patch :-)

It is also normal to put in the commit message what changed from the
previous version.

This is a fix. So please add a Fixes: tag, with the hash of the commit
which introduced the problem.

And since this is a fix, it should be against DaveM net tree, and you
indicate this in the subject line with [PATCH net v3].

Thanks
	Andrew

> 
> Signed-off-by: Peter Mamonov <pmamonov@...il.com>
> ---
>  drivers/net/phy/national.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c
> index 2addf1d3f619..3aa910b3dc89 100644
> --- a/drivers/net/phy/national.c
> +++ b/drivers/net/phy/national.c
> @@ -110,14 +110,17 @@ static void ns_giga_speed_fallback(struct phy_device *phydev, int mode)
>  
>  static void ns_10_base_t_hdx_loopack(struct phy_device *phydev, int disable)
>  {
> +	u16 lb_dis = BIT(1);
> +
>  	if (disable)
> -		ns_exp_write(phydev, 0x1c0, ns_exp_read(phydev, 0x1c0) | 1);
> +		ns_exp_write(phydev, 0x1c0,
> +			     ns_exp_read(phydev, 0x1c0) | lb_dis);
>  	else
>  		ns_exp_write(phydev, 0x1c0,
> -			     ns_exp_read(phydev, 0x1c0) & 0xfffe);
> +			     ns_exp_read(phydev, 0x1c0) & ~lb_dis);
>  
>  	pr_debug("10BASE-T HDX loopback %s\n",
> -		 (ns_exp_read(phydev, 0x1c0) & 0x0001) ? "off" : "on");
> +		 (ns_exp_read(phydev, 0x1c0) & lb_dis) ? "off" : "on");
>  }
>  
>  static int ns_config_init(struct phy_device *phydev)
> -- 
> 2.23.0
>