netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] net: phy: broadcom: use auxctl reading helper in BCM54612E code
@ 2017-01-25 15:54 Rafał Miłecki
  2017-01-25 15:54 ` [PATCH 2/3] net: phy: broadcom: drop duplicated define for RXD to RXC delay Rafał Miłecki
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Rafał Miłecki @ 2017-01-25 15:54 UTC (permalink / raw)
  To: David S . Miller, Florian Fainelli
  Cc: Xo Wang, Joel Stanley, Jon Mason, Jaedon Shin, netdev,
	Rafał Miłecki

From: Rafał Miłecki <rafal@milecki.pl>

Starting with commit 5b4e29005123 ("net: phy: broadcom: add
bcm54xx_auxctl_read") we have a reading helper so use it and avoid code
duplication.
It also means we don't need MII_BCM54XX_AUXCTL_SHDWSEL_MISC define as
it's the same as MII_BCM54XX_AUXCTL_SHDWSEL_MISC just for reading needs
(same value shifted by 12 bits).

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/net/phy/broadcom.c | 6 ++----
 include/linux/brcmphy.h    | 1 -
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 4223e35490b0..25c6e6cea2dc 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -395,10 +395,8 @@ static int bcm54612e_config_aneg(struct phy_device *phydev)
 	    (phydev->interface != PHY_INTERFACE_MODE_RGMII_RXID)) {
 		u16 reg;
 
-		/* Errata: reads require filling in the write selector field */
-		bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
-				     MII_BCM54XX_AUXCTL_MISC_RDSEL_MISC);
-		reg = phy_read(phydev, MII_BCM54XX_AUX_CTL);
+		reg = bcm54xx_auxctl_read(phydev,
+					  MII_BCM54XX_AUXCTL_SHDWSEL_MISC);
 		/* Disable RXD to RXC delay (default set) */
 		reg &= ~MII_BCM54XX_AUXCTL_MISC_RXD_RXC_SKEW;
 		/* Clear shadow selector field */
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index 295fb3e73de5..34e61004b9dc 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -111,7 +111,6 @@
 #define MII_BCM54XX_AUXCTL_MISC_WREN	0x8000
 #define MII_BCM54XX_AUXCTL_MISC_RXD_RXC_SKEW	0x0100
 #define MII_BCM54XX_AUXCTL_MISC_FORCE_AMDIX	0x0200
-#define MII_BCM54XX_AUXCTL_MISC_RDSEL_MISC	0x7000
 #define MII_BCM54XX_AUXCTL_SHDWSEL_MISC	0x0007
 #define MII_BCM54XX_AUXCTL_SHDWSEL_READ_SHIFT	12
 #define MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_SKEW_EN	(1 << 8)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] net: phy: broadcom: drop duplicated define for RXD to RXC delay
  2017-01-25 15:54 [PATCH 1/3] net: phy: broadcom: use auxctl reading helper in BCM54612E code Rafał Miłecki
@ 2017-01-25 15:54 ` Rafał Miłecki
  2017-01-25 16:36   ` Florian Fainelli
  2017-01-25 15:54 ` [PATCH 3/3] net: phy: bcm-phy-lib: clean up remaining AUXCTL register defines Rafał Miłecki
  2017-01-25 16:32 ` [PATCH 1/3] net: phy: broadcom: use auxctl reading helper in BCM54612E code Florian Fainelli
  2 siblings, 1 reply; 6+ messages in thread
From: Rafał Miłecki @ 2017-01-25 15:54 UTC (permalink / raw)
  To: David S . Miller, Florian Fainelli
  Cc: Xo Wang, Joel Stanley, Jon Mason, Jaedon Shin, netdev,
	Rafał Miłecki

From: Rafał Miłecki <rafal@milecki.pl>

We had two defines for the same bit (both were used with the
MII_BCM54XX_AUXCTL_SHDWSEL_MISC register).

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/net/phy/broadcom.c | 2 +-
 include/linux/brcmphy.h    | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 25c6e6cea2dc..9b7c2d57ca92 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -42,7 +42,7 @@ static int bcm54810_config(struct phy_device *phydev)
 		return rc;
 
 	val = bcm54xx_auxctl_read(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC);
-	val &= ~MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_SKEW_EN;
+	val &= ~MII_BCM54XX_AUXCTL_MISC_RXD_RXC_SKEW;
 	val |= MII_BCM54XX_AUXCTL_MISC_WREN;
 	rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
 				  val);
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index 34e61004b9dc..bff53da82b58 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -113,7 +113,6 @@
 #define MII_BCM54XX_AUXCTL_MISC_FORCE_AMDIX	0x0200
 #define MII_BCM54XX_AUXCTL_SHDWSEL_MISC	0x0007
 #define MII_BCM54XX_AUXCTL_SHDWSEL_READ_SHIFT	12
-#define MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_SKEW_EN	(1 << 8)
 #define MII_BCM54XX_AUXCTL_SHDWSEL_MISC_WIRESPEED_EN	(1 << 4)
 
 #define MII_BCM54XX_AUXCTL_SHDWSEL_MASK	0x0007
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] net: phy: bcm-phy-lib: clean up remaining AUXCTL register defines
  2017-01-25 15:54 [PATCH 1/3] net: phy: broadcom: use auxctl reading helper in BCM54612E code Rafał Miłecki
  2017-01-25 15:54 ` [PATCH 2/3] net: phy: broadcom: drop duplicated define for RXD to RXC delay Rafał Miłecki
@ 2017-01-25 15:54 ` Rafał Miłecki
  2017-01-25 16:36   ` Florian Fainelli
  2017-01-25 16:32 ` [PATCH 1/3] net: phy: broadcom: use auxctl reading helper in BCM54612E code Florian Fainelli
  2 siblings, 1 reply; 6+ messages in thread
From: Rafał Miłecki @ 2017-01-25 15:54 UTC (permalink / raw)
  To: David S . Miller, Florian Fainelli
  Cc: Xo Wang, Joel Stanley, Jon Mason, Jaedon Shin, netdev,
	Rafał Miłecki

From: Rafał Miłecki <rafal@milecki.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_.

Signed-off-by: Rafał Miłecki <rafal@milecki.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
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] net: phy: broadcom: use auxctl reading helper in BCM54612E code
  2017-01-25 15:54 [PATCH 1/3] net: phy: broadcom: use auxctl reading helper in BCM54612E code Rafał Miłecki
  2017-01-25 15:54 ` [PATCH 2/3] net: phy: broadcom: drop duplicated define for RXD to RXC delay Rafał Miłecki
  2017-01-25 15:54 ` [PATCH 3/3] net: phy: bcm-phy-lib: clean up remaining AUXCTL register defines Rafał Miłecki
@ 2017-01-25 16:32 ` Florian Fainelli
  2 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2017-01-25 16:32 UTC (permalink / raw)
  To: Rafał Miłecki, David S . Miller
  Cc: Xo Wang, Joel Stanley, Jon Mason, Jaedon Shin, netdev,
	Rafał Miłecki

On 01/25/2017 07:54 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Starting with commit 5b4e29005123 ("net: phy: broadcom: add
> bcm54xx_auxctl_read") we have a reading helper so use it and avoid code
> duplication.
> It also means we don't need MII_BCM54XX_AUXCTL_SHDWSEL_MISC define as
> it's the same as MII_BCM54XX_AUXCTL_SHDWSEL_MISC just for reading needs
> (same value shifted by 12 bits).
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3] net: phy: broadcom: drop duplicated define for RXD to RXC delay
  2017-01-25 15:54 ` [PATCH 2/3] net: phy: broadcom: drop duplicated define for RXD to RXC delay Rafał Miłecki
@ 2017-01-25 16:36   ` Florian Fainelli
  0 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2017-01-25 16:36 UTC (permalink / raw)
  To: Rafał Miłecki, David S . Miller
  Cc: Xo Wang, Joel Stanley, Jon Mason, Jaedon Shin, netdev,
	Rafał Miłecki

On 01/25/2017 07:54 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> We had two defines for the same bit (both were used with the
> MII_BCM54XX_AUXCTL_SHDWSEL_MISC register).
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  drivers/net/phy/broadcom.c | 2 +-
>  include/linux/brcmphy.h    | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
> index 25c6e6cea2dc..9b7c2d57ca92 100644
> --- a/drivers/net/phy/broadcom.c
> +++ b/drivers/net/phy/broadcom.c
> @@ -42,7 +42,7 @@ static int bcm54810_config(struct phy_device *phydev)
>  		return rc;
>  
>  	val = bcm54xx_auxctl_read(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC);
> -	val &= ~MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_SKEW_EN;
> +	val &= ~MII_BCM54XX_AUXCTL_MISC_RXD_RXC_SKEW;
>  	val |= MII_BCM54XX_AUXCTL_MISC_WREN;
>  	rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
>  				  val);
> diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
> index 34e61004b9dc..bff53da82b58 100644
> --- a/include/linux/brcmphy.h
> +++ b/include/linux/brcmphy.h
> @@ -113,7 +113,6 @@
>  #define MII_BCM54XX_AUXCTL_MISC_FORCE_AMDIX	0x0200
>  #define MII_BCM54XX_AUXCTL_SHDWSEL_MISC	0x0007
>  #define MII_BCM54XX_AUXCTL_SHDWSEL_READ_SHIFT	12
> -#define MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_SKEW_EN	(1 << 8)
>  #define MII_BCM54XX_AUXCTL_SHDWSEL_MISC_WIRESPEED_EN	(1 << 4)

Please drop the other one and keep this one instead, the SHDWSEL prefix
is intentional and correct here since it matches the datasheet.
-- 
Florian

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] net: phy: bcm-phy-lib: clean up remaining AUXCTL register defines
  2017-01-25 15:54 ` [PATCH 3/3] net: phy: bcm-phy-lib: clean up remaining AUXCTL register defines Rafał Miłecki
@ 2017-01-25 16:36   ` Florian Fainelli
  0 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2017-01-25 16:36 UTC (permalink / raw)
  To: Rafał Miłecki, David S . Miller
  Cc: Xo Wang, Joel Stanley, Jon Mason, Jaedon Shin, netdev,
	Rafał Miłecki

On 01/25/2017 07:54 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.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@milecki.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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-01-25 16:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-25 15:54 [PATCH 1/3] net: phy: broadcom: use auxctl reading helper in BCM54612E code Rafał Miłecki
2017-01-25 15:54 ` [PATCH 2/3] net: phy: broadcom: drop duplicated define for RXD to RXC delay Rafał Miłecki
2017-01-25 16:36   ` Florian Fainelli
2017-01-25 15:54 ` [PATCH 3/3] net: phy: bcm-phy-lib: clean up remaining AUXCTL register defines Rafał Miłecki
2017-01-25 16:36   ` Florian Fainelli
2017-01-25 16:32 ` [PATCH 1/3] net: phy: broadcom: use auxctl reading helper in BCM54612E code Florian Fainelli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).