* [PATCH v2] net: phy: dp83867: fix hfs boot in rgmii mode
@ 2019-12-06 12:34 Grygorii Strashko
2019-12-07 19:57 ` David Miller
0 siblings, 1 reply; 2+ messages in thread
From: Grygorii Strashko @ 2019-12-06 12:34 UTC (permalink / raw)
To: netdev, Andrew Lunn, David S . Miller, Florian Fainelli
Cc: Sekhar Nori, linux-kernel, Heiner Kallweit, Grygorii Strashko
The commit ef87f7da6b28 ("net: phy: dp83867: move dt parsing to probe")
causes regression on TI dra71x-evm and dra72x-evm, where DP83867 PHY is
used in "rgmii-id" mode - the networking stops working.
Unfortunately, it's not enough to just move DT parsing code to .probe() as
it depends on phydev->interface value, which is set to correct value abter
the .probe() is completed and before calling .config_init(). So, RGMII
configuration can't be loaded from DT.
To fix and issue
- move RGMII validation code to .config_init()
- parse RGMII parameters in dp83867_of_init(), but consider them as
optional.
Fixes: ef87f7da6b28 ("net: phy: dp83867: move dt parsing to probe")
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
v2:
- fixed comments in code
drivers/net/phy/dp83867.c | 119 +++++++++++++++++++++++---------------
1 file changed, 71 insertions(+), 48 deletions(-)
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 0b95e7a2e273..9cd9dcee4eb2 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -101,8 +101,11 @@
/* RGMIIDCTL bits */
#define DP83867_RGMII_TX_CLK_DELAY_MAX 0xf
#define DP83867_RGMII_TX_CLK_DELAY_SHIFT 4
+#define DP83867_RGMII_TX_CLK_DELAY_INV (DP83867_RGMII_TX_CLK_DELAY_MAX + 1)
#define DP83867_RGMII_RX_CLK_DELAY_MAX 0xf
#define DP83867_RGMII_RX_CLK_DELAY_SHIFT 0
+#define DP83867_RGMII_RX_CLK_DELAY_INV (DP83867_RGMII_RX_CLK_DELAY_MAX + 1)
+
/* IO_MUX_CFG bits */
#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MASK 0x1f
@@ -294,6 +297,48 @@ static int dp83867_config_port_mirroring(struct phy_device *phydev)
return 0;
}
+static int dp83867_verify_rgmii_cfg(struct phy_device *phydev)
+{
+ struct dp83867_private *dp83867 = phydev->priv;
+
+ /* Existing behavior was to use default pin strapping delay in rgmii
+ * mode, but rgmii should have meant no delay. Warn existing users.
+ */
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII) {
+ const u16 val = phy_read_mmd(phydev, DP83867_DEVADDR,
+ DP83867_STRAP_STS2);
+ const u16 txskew = (val & DP83867_STRAP_STS2_CLK_SKEW_TX_MASK) >>
+ DP83867_STRAP_STS2_CLK_SKEW_TX_SHIFT;
+ const u16 rxskew = (val & DP83867_STRAP_STS2_CLK_SKEW_RX_MASK) >>
+ DP83867_STRAP_STS2_CLK_SKEW_RX_SHIFT;
+
+ if (txskew != DP83867_STRAP_STS2_CLK_SKEW_NONE ||
+ rxskew != DP83867_STRAP_STS2_CLK_SKEW_NONE)
+ phydev_warn(phydev,
+ "PHY has delays via pin strapping, but phy-mode = 'rgmii'\n"
+ "Should be 'rgmii-id' to use internal delays txskew:%x rxskew:%x\n",
+ txskew, rxskew);
+ }
+
+ /* RX delay *must* be specified if internal delay of RX is used. */
+ if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+ phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) &&
+ dp83867->rx_id_delay == DP83867_RGMII_RX_CLK_DELAY_INV) {
+ phydev_err(phydev, "ti,rx-internal-delay must be specified\n");
+ return -EINVAL;
+ }
+
+ /* TX delay *must* be specified if internal delay of TX is used. */
+ if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+ phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) &&
+ dp83867->tx_id_delay == DP83867_RGMII_TX_CLK_DELAY_INV) {
+ phydev_err(phydev, "ti,tx-internal-delay must be specified\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
#ifdef CONFIG_OF_MDIO
static int dp83867_of_init(struct phy_device *phydev)
{
@@ -335,55 +380,25 @@ static int dp83867_of_init(struct phy_device *phydev)
dp83867->sgmii_ref_clk_en = of_property_read_bool(of_node,
"ti,sgmii-ref-clock-output-enable");
- /* Existing behavior was to use default pin strapping delay in rgmii
- * mode, but rgmii should have meant no delay. Warn existing users.
- */
- if (phydev->interface == PHY_INTERFACE_MODE_RGMII) {
- const u16 val = phy_read_mmd(phydev, DP83867_DEVADDR, DP83867_STRAP_STS2);
- const u16 txskew = (val & DP83867_STRAP_STS2_CLK_SKEW_TX_MASK) >>
- DP83867_STRAP_STS2_CLK_SKEW_TX_SHIFT;
- const u16 rxskew = (val & DP83867_STRAP_STS2_CLK_SKEW_RX_MASK) >>
- DP83867_STRAP_STS2_CLK_SKEW_RX_SHIFT;
- if (txskew != DP83867_STRAP_STS2_CLK_SKEW_NONE ||
- rxskew != DP83867_STRAP_STS2_CLK_SKEW_NONE)
- phydev_warn(phydev,
- "PHY has delays via pin strapping, but phy-mode = 'rgmii'\n"
- "Should be 'rgmii-id' to use internal delays\n");
- }
-
- /* RX delay *must* be specified if internal delay of RX is used. */
- if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
- phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
- ret = of_property_read_u32(of_node, "ti,rx-internal-delay",
- &dp83867->rx_id_delay);
- if (ret) {
- phydev_err(phydev, "ti,rx-internal-delay must be specified\n");
- return ret;
- }
- if (dp83867->rx_id_delay > DP83867_RGMII_RX_CLK_DELAY_MAX) {
- phydev_err(phydev,
- "ti,rx-internal-delay value of %u out of range\n",
- dp83867->rx_id_delay);
- return -EINVAL;
- }
+ dp83867->rx_id_delay = DP83867_RGMII_RX_CLK_DELAY_INV;
+ ret = of_property_read_u32(of_node, "ti,rx-internal-delay",
+ &dp83867->rx_id_delay);
+ if (!ret && dp83867->rx_id_delay > DP83867_RGMII_RX_CLK_DELAY_MAX) {
+ phydev_err(phydev,
+ "ti,rx-internal-delay value of %u out of range\n",
+ dp83867->rx_id_delay);
+ return -EINVAL;
}
- /* TX delay *must* be specified if internal delay of RX is used. */
- if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
- phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
- ret = of_property_read_u32(of_node, "ti,tx-internal-delay",
- &dp83867->tx_id_delay);
- if (ret) {
- phydev_err(phydev, "ti,tx-internal-delay must be specified\n");
- return ret;
- }
- if (dp83867->tx_id_delay > DP83867_RGMII_TX_CLK_DELAY_MAX) {
- phydev_err(phydev,
- "ti,tx-internal-delay value of %u out of range\n",
- dp83867->tx_id_delay);
- return -EINVAL;
- }
+ dp83867->tx_id_delay = DP83867_RGMII_TX_CLK_DELAY_INV;
+ ret = of_property_read_u32(of_node, "ti,tx-internal-delay",
+ &dp83867->tx_id_delay);
+ if (!ret && dp83867->tx_id_delay > DP83867_RGMII_TX_CLK_DELAY_MAX) {
+ phydev_err(phydev,
+ "ti,tx-internal-delay value of %u out of range\n",
+ dp83867->tx_id_delay);
+ return -EINVAL;
}
if (of_property_read_bool(of_node, "enet-phy-lane-swap"))
@@ -434,6 +449,10 @@ static int dp83867_config_init(struct phy_device *phydev)
int ret, val, bs;
u16 delay;
+ ret = dp83867_verify_rgmii_cfg(phydev);
+ if (ret)
+ return ret;
+
/* RX_DV/RX_CTRL strapped in mode 1 or mode 2 workaround */
if (dp83867->rxctrl_strap_quirk)
phy_clear_bits_mmd(phydev, DP83867_DEVADDR, DP83867_CFG4,
@@ -485,8 +504,12 @@ static int dp83867_config_init(struct phy_device *phydev)
phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_RGMIICTL, val);
- delay = (dp83867->rx_id_delay |
- (dp83867->tx_id_delay << DP83867_RGMII_TX_CLK_DELAY_SHIFT));
+ delay = 0;
+ if (dp83867->rx_id_delay != DP83867_RGMII_RX_CLK_DELAY_INV)
+ delay |= dp83867->rx_id_delay;
+ if (dp83867->tx_id_delay != DP83867_RGMII_TX_CLK_DELAY_INV)
+ delay |= dp83867->tx_id_delay <<
+ DP83867_RGMII_TX_CLK_DELAY_SHIFT;
phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_RGMIIDCTL,
delay);
--
2.17.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] net: phy: dp83867: fix hfs boot in rgmii mode
2019-12-06 12:34 [PATCH v2] net: phy: dp83867: fix hfs boot in rgmii mode Grygorii Strashko
@ 2019-12-07 19:57 ` David Miller
0 siblings, 0 replies; 2+ messages in thread
From: David Miller @ 2019-12-07 19:57 UTC (permalink / raw)
To: grygorii.strashko
Cc: netdev, andrew, f.fainelli, nsekhar, linux-kernel, hkallweit1
From: Grygorii Strashko <grygorii.strashko@ti.com>
Date: Fri, 6 Dec 2019 14:34:32 +0200
> The commit ef87f7da6b28 ("net: phy: dp83867: move dt parsing to probe")
> causes regression on TI dra71x-evm and dra72x-evm, where DP83867 PHY is
> used in "rgmii-id" mode - the networking stops working.
> Unfortunately, it's not enough to just move DT parsing code to .probe() as
> it depends on phydev->interface value, which is set to correct value abter
> the .probe() is completed and before calling .config_init(). So, RGMII
> configuration can't be loaded from DT.
>
> To fix and issue
> - move RGMII validation code to .config_init()
> - parse RGMII parameters in dp83867_of_init(), but consider them as
> optional.
>
> Fixes: ef87f7da6b28 ("net: phy: dp83867: move dt parsing to probe")
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
> v2:
> - fixed comments in code
Applied.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-12-07 19:57 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-06 12:34 [PATCH v2] net: phy: dp83867: fix hfs boot in rgmii mode Grygorii Strashko
2019-12-07 19:57 ` 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).