netdev - Re: [PATCH 3/3] net: phy: bcm-phy-lib: clean up remaining AUXCTL register defines

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] [day] [month] [year] [list]

Message-ID: <1439c0db-45df-db8d-df53-e0c4a6385914@gmail.com>
Date:   Wed, 25 Jan 2017 08:36:18 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Rafał Miłecki <zajec5@...il.com>,
        "David S . Miller" <davem@...emloft.net>
Cc:     Xo Wang <xow@...gle.com>, Joel Stanley <joel@....id.au>,
        Jon Mason <jon.mason@...adcom.com>,
        Jaedon Shin <jaedon.shin@...il.com>, netdev@...r.kernel.org,
        Rafał Miłecki <rafal@...ecki.pl>
Subject: Re: [PATCH 3/3] net: phy: bcm-phy-lib: clean up remaining AUXCTL
 register defines

On 01/25/2017 07:54 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@...ecki.pl>
> 
> 1) Use 0x%02x format for register number. This follows some other
>    defines and makes it easier to distinct register from values.
> 2) Put register define above values and sort the values. It makes
>    reading header code easier.
> 3) Drop SHDWSEL_ name part from the only value define using it. For all
>    other values we just start with MISC_.

That's not how these bits are defined in the data sheet, so please drop
that part of the patch.

> 
> Signed-off-by: Rafał Miłecki <rafal@...ecki.pl>
> ---
>  drivers/net/phy/bcm-phy-lib.c | 6 +++---
>  include/linux/brcmphy.h       | 8 ++++----
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/phy/bcm-phy-lib.c b/drivers/net/phy/bcm-phy-lib.c
> index ab9ad689617c..8d7e21a9fa77 100644
> --- a/drivers/net/phy/bcm-phy-lib.c
> +++ b/drivers/net/phy/bcm-phy-lib.c
> @@ -241,7 +241,7 @@ int bcm_phy_downshift_get(struct phy_device *phydev, u8 *count)
>  		return val;
>  
>  	/* Check if wirespeed is enabled or not */
> -	if (!(val & MII_BCM54XX_AUXCTL_SHDWSEL_MISC_WIRESPEED_EN)) {
> +	if (!(val & MII_BCM54XX_AUXCTL_MISC_WIRESPEED_EN)) {
>  		*count = DOWNSHIFT_DEV_DISABLE;
>  		return 0;
>  	}
> @@ -283,12 +283,12 @@ int bcm_phy_downshift_set(struct phy_device *phydev, u8 count)
>  	val |= MII_BCM54XX_AUXCTL_MISC_WREN;
>  
>  	if (count == DOWNSHIFT_DEV_DISABLE) {
> -		val &= ~MII_BCM54XX_AUXCTL_SHDWSEL_MISC_WIRESPEED_EN;
> +		val &= ~MII_BCM54XX_AUXCTL_MISC_WIRESPEED_EN;
>  		return bcm54xx_auxctl_write(phydev,
>  					    MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
>  					    val);
>  	} else {
> -		val |= MII_BCM54XX_AUXCTL_SHDWSEL_MISC_WIRESPEED_EN;
> +		val |= MII_BCM54XX_AUXCTL_MISC_WIRESPEED_EN;
>  		ret = bcm54xx_auxctl_write(phydev,
>  					   MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
>  					   val);
> diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
> index bff53da82b58..a79d57caf7f1 100644
> --- a/include/linux/brcmphy.h
> +++ b/include/linux/brcmphy.h
> @@ -104,16 +104,16 @@
>  /*
>   * AUXILIARY CONTROL SHADOW ACCESS REGISTERS.  (PHY REG 0x18)
>   */
> -#define MII_BCM54XX_AUXCTL_SHDWSEL_AUXCTL	0x0000
> +#define MII_BCM54XX_AUXCTL_SHDWSEL_AUXCTL	0x00
>  #define MII_BCM54XX_AUXCTL_ACTL_TX_6DB		0x0400
>  #define MII_BCM54XX_AUXCTL_ACTL_SMDSP_ENA	0x0800
>  
> -#define MII_BCM54XX_AUXCTL_MISC_WREN	0x8000
> +#define MII_BCM54XX_AUXCTL_SHDWSEL_MISC		0x07
> +#define MII_BCM54XX_AUXCTL_MISC_WIRESPEED_EN	0x0010
>  #define MII_BCM54XX_AUXCTL_MISC_RXD_RXC_SKEW	0x0100
>  #define MII_BCM54XX_AUXCTL_MISC_FORCE_AMDIX	0x0200
> -#define MII_BCM54XX_AUXCTL_SHDWSEL_MISC	0x0007
> +#define MII_BCM54XX_AUXCTL_MISC_WREN		0x8000
>  #define MII_BCM54XX_AUXCTL_SHDWSEL_READ_SHIFT	12
> -#define MII_BCM54XX_AUXCTL_SHDWSEL_MISC_WIRESPEED_EN	(1 << 4)
>  
>  #define MII_BCM54XX_AUXCTL_SHDWSEL_MASK	0x0007
>  
> 


-- 
Florian