LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH net] net: phy: broadcom: Fix bcm_write_exp()
@ 2018-05-23  0:04 Florian Fainelli
  2018-05-23  0:15 ` Andrew Lunn
  2018-05-23 19:27 ` David Miller
  0 siblings, 2 replies; 5+ messages in thread
From: Florian Fainelli @ 2018-05-23  0:04 UTC (permalink / raw)
  To: netdev
  Cc: arunp, Florian Fainelli, Andrew Lunn, David S. Miller, Ray Jui,
	Scott Branden, Jon Mason,
	maintainer:BROADCOM IPROC ARM ARCHITECTURE,
	moderated list:BROADCOM IPROC ARM ARCHITECTURE, open list

On newer PHYs, we need to select the expansion register to write with
setting bits [11:8] to 0xf. This was done correctly by bcm7xxx.c prior
to being migrated to generic code under bcm-phy-lib.c which
unfortunately used the older implementation from the BCM54xx days.

Fix this by creating an inline stub: bcm_write_exp_sel() which adds the
correct value (MII_BCM54XX_EXP_SEL_ER) and update both the Cygnus PHY
and BCM7xxx PHY drivers which require setting these bits.

broadcom.c is unchanged because some PHYs even use a different selector
method, so let them specify it directly (e.g: SerDes secondary selector).

Fixes: a1cba5613edf ("net: phy: Add Broadcom phy library for common interfaces")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
David, please also queue this one up for -stable, thanks!

 drivers/net/phy/bcm-cygnus.c  | 6 +++---
 drivers/net/phy/bcm-phy-lib.h | 7 +++++++
 drivers/net/phy/bcm7xxx.c     | 4 ++--
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/bcm-cygnus.c b/drivers/net/phy/bcm-cygnus.c
index 6838129839ca..e757b09f1889 100644
--- a/drivers/net/phy/bcm-cygnus.c
+++ b/drivers/net/phy/bcm-cygnus.c
@@ -61,17 +61,17 @@ static int bcm_cygnus_afe_config(struct phy_device *phydev)
 		return rc;
 
 	/* make rcal=100, since rdb default is 000 */
-	rc = bcm_phy_write_exp(phydev, MII_BRCM_CORE_EXPB1, 0x10);
+	rc = bcm_phy_write_exp_sel(phydev, MII_BRCM_CORE_EXPB1, 0x10);
 	if (rc < 0)
 		return rc;
 
 	/* CORE_EXPB0, Reset R_CAL/RC_CAL Engine */
-	rc = bcm_phy_write_exp(phydev, MII_BRCM_CORE_EXPB0, 0x10);
+	rc = bcm_phy_write_exp_sel(phydev, MII_BRCM_CORE_EXPB0, 0x10);
 	if (rc < 0)
 		return rc;
 
 	/* CORE_EXPB0, Disable Reset R_CAL/RC_CAL Engine */
-	rc = bcm_phy_write_exp(phydev, MII_BRCM_CORE_EXPB0, 0x00);
+	rc = bcm_phy_write_exp_sel(phydev, MII_BRCM_CORE_EXPB0, 0x00);
 
 	return 0;
 }
diff --git a/drivers/net/phy/bcm-phy-lib.h b/drivers/net/phy/bcm-phy-lib.h
index 7c73808cbbde..81cceaa412fe 100644
--- a/drivers/net/phy/bcm-phy-lib.h
+++ b/drivers/net/phy/bcm-phy-lib.h
@@ -14,11 +14,18 @@
 #ifndef _LINUX_BCM_PHY_LIB_H
 #define _LINUX_BCM_PHY_LIB_H
 
+#include <linux/brcmphy.h>
 #include <linux/phy.h>
 
 int bcm_phy_write_exp(struct phy_device *phydev, u16 reg, u16 val);
 int bcm_phy_read_exp(struct phy_device *phydev, u16 reg);
 
+static inline int bcm_phy_write_exp_sel(struct phy_device *phydev,
+					u16 reg, u16 val)
+{
+	return bcm_phy_write_exp(phydev, reg | MII_BCM54XX_EXP_SEL_ER, val);
+}
+
 int bcm54xx_auxctl_write(struct phy_device *phydev, u16 regnum, u16 val);
 int bcm54xx_auxctl_read(struct phy_device *phydev, u16 regnum);
 
diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index 29b1c88b55cc..01d2ff2f6241 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -65,10 +65,10 @@ struct bcm7xxx_phy_priv {
 static void r_rc_cal_reset(struct phy_device *phydev)
 {
 	/* Reset R_CAL/RC_CAL Engine */
-	bcm_phy_write_exp(phydev, 0x00b0, 0x0010);
+	bcm_phy_write_exp_sel(phydev, 0x00b0, 0x0010);
 
 	/* Disable Reset R_AL/RC_CAL Engine */
-	bcm_phy_write_exp(phydev, 0x00b0, 0x0000);
+	bcm_phy_write_exp_sel(phydev, 0x00b0, 0x0000);
 }
 
 static int bcm7xxx_28nm_b0_afe_config_init(struct phy_device *phydev)
-- 
2.14.1

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

* Re: [PATCH net] net: phy: broadcom: Fix bcm_write_exp()
  2018-05-23  0:04 [PATCH net] net: phy: broadcom: Fix bcm_write_exp() Florian Fainelli
@ 2018-05-23  0:15 ` Andrew Lunn
  2018-05-23  1:20   ` Florian Fainelli
  2018-05-23 19:27 ` David Miller
  1 sibling, 1 reply; 5+ messages in thread
From: Andrew Lunn @ 2018-05-23  0:15 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, arunp, David S. Miller, Ray Jui, Scott Branden,
	Jon Mason, maintainer:BROADCOM IPROC ARM ARCHITECTURE,
	moderated list:BROADCOM IPROC ARM ARCHITECTURE, open list

On Tue, May 22, 2018 at 05:04:49PM -0700, Florian Fainelli wrote:
> On newer PHYs, we need to select the expansion register to write with
> setting bits [11:8] to 0xf. This was done correctly by bcm7xxx.c prior
> to being migrated to generic code under bcm-phy-lib.c which
> unfortunately used the older implementation from the BCM54xx days.

Hi Florian

Does selecting the expansion register affect access to the standard
registers? Does this need locking like the Marvell PHY has when
changing pages?

Thanks
	 Andrew

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

* Re: [PATCH net] net: phy: broadcom: Fix bcm_write_exp()
  2018-05-23  0:15 ` Andrew Lunn
@ 2018-05-23  1:20   ` Florian Fainelli
  2018-05-23 16:57     ` Florian Fainelli
  0 siblings, 1 reply; 5+ messages in thread
From: Florian Fainelli @ 2018-05-23  1:20 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, arunp, David S. Miller, Ray Jui, Scott Branden,
	Jon Mason, maintainer:BROADCOM IPROC ARM ARCHITECTURE,
	moderated list:BROADCOM IPROC ARM ARCHITECTURE, open list

Hi Andrew,

On 05/22/2018 05:15 PM, Andrew Lunn wrote:
> On Tue, May 22, 2018 at 05:04:49PM -0700, Florian Fainelli wrote:
>> On newer PHYs, we need to select the expansion register to write with
>> setting bits [11:8] to 0xf. This was done correctly by bcm7xxx.c prior
>> to being migrated to generic code under bcm-phy-lib.c which
>> unfortunately used the older implementation from the BCM54xx days.
> 
> Hi Florian
> 
> Does selecting the expansion register affect access to the standard
> registers? Does this need locking like the Marvell PHY has when
> changing pages?

We should probably convert this to the page accessors since the
expansion, misc and other shadow 0x1c accesses are all indirection
layers to poke into a different address space of the PHY. That would be
a separate fix though for a number of reasons.
-- 
Florian

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

* Re: [PATCH net] net: phy: broadcom: Fix bcm_write_exp()
  2018-05-23  1:20   ` Florian Fainelli
@ 2018-05-23 16:57     ` Florian Fainelli
  0 siblings, 0 replies; 5+ messages in thread
From: Florian Fainelli @ 2018-05-23 16:57 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, arunp, David S. Miller, Ray Jui, Scott Branden,
	Jon Mason, maintainer:BROADCOM IPROC ARM ARCHITECTURE,
	moderated list:BROADCOM IPROC ARM ARCHITECTURE, open list

On 05/22/2018 06:20 PM, Florian Fainelli wrote:
> Hi Andrew,
> 
> On 05/22/2018 05:15 PM, Andrew Lunn wrote:
>> On Tue, May 22, 2018 at 05:04:49PM -0700, Florian Fainelli wrote:
>>> On newer PHYs, we need to select the expansion register to write with
>>> setting bits [11:8] to 0xf. This was done correctly by bcm7xxx.c prior
>>> to being migrated to generic code under bcm-phy-lib.c which
>>> unfortunately used the older implementation from the BCM54xx days.
>>
>> Hi Florian
>>
>> Does selecting the expansion register affect access to the standard
>> registers? Does this need locking like the Marvell PHY has when
>> changing pages?
> 
> We should probably convert this to the page accessors since the
> expansion, misc and other shadow 0x1c accesses are all indirection
> layers to poke into a different address space of the PHY. That would be
> a separate fix though for a number of reasons.

I realize I did not quite answer your question, the answer to your
question AFAICT is no, setting the expansion register sequence and then
aborting mid-way is not a problem and does not impact the standard MII
registers because of how this is implemented. The registers are accessed
and latched through a specific indirect sequence, but there is no page
switching unlike the Marvell PHYs
-- 
Florian

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

* Re: [PATCH net] net: phy: broadcom: Fix bcm_write_exp()
  2018-05-23  0:04 [PATCH net] net: phy: broadcom: Fix bcm_write_exp() Florian Fainelli
  2018-05-23  0:15 ` Andrew Lunn
@ 2018-05-23 19:27 ` David Miller
  1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2018-05-23 19:27 UTC (permalink / raw)
  To: f.fainelli
  Cc: netdev, arunp, andrew, rjui, sbranden, jonmason,
	bcm-kernel-feedback-list, linux-arm-kernel, linux-kernel

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 22 May 2018 17:04:49 -0700

> On newer PHYs, we need to select the expansion register to write with
> setting bits [11:8] to 0xf. This was done correctly by bcm7xxx.c prior
> to being migrated to generic code under bcm-phy-lib.c which
> unfortunately used the older implementation from the BCM54xx days.
> 
> Fix this by creating an inline stub: bcm_write_exp_sel() which adds the
> correct value (MII_BCM54XX_EXP_SEL_ER) and update both the Cygnus PHY
> and BCM7xxx PHY drivers which require setting these bits.
> 
> broadcom.c is unchanged because some PHYs even use a different selector
> method, so let them specify it directly (e.g: SerDes secondary selector).
> 
> Fixes: a1cba5613edf ("net: phy: Add Broadcom phy library for common interfaces")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> David, please also queue this one up for -stable, thanks!

Applied and queued up for -stable, thanks Florian.

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

end of thread, other threads:[~2018-05-23 19:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-23  0:04 [PATCH net] net: phy: broadcom: Fix bcm_write_exp() Florian Fainelli
2018-05-23  0:15 ` Andrew Lunn
2018-05-23  1:20   ` Florian Fainelli
2018-05-23 16:57     ` Florian Fainelli
2018-05-23 19:27 ` David Miller

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).