* [PATCH] mvneta: implement SGMII-based in-band link state signaling
@ 2015-03-31 13:24 Stas Sergeev
2015-04-03 0:51 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Stas Sergeev @ 2015-03-31 13:24 UTC (permalink / raw)
To: netdev; +Cc: Linux kernel, Thomas Petazzoni, Florian Fainelli
When MDIO bus is unavailable (common setup for SGMII), the in-band
signaling must be used to correctly track link state.
This patch enables the in-band status delivery for links state changes, namely:
- link up/down
- link speed
- duplex full/half
fixed_phy link_update() callback is used to update status.
Note: SGMII setup is so common that I think it deserves a separate API
rather than the per-driver handling. There is an alternative implementation
that adds such API:
https://lkml.org/lkml/2015/3/27/346
CC: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
CC: Florian Fainelli <f.fainelli@gmail.com>
CC: netdev@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
---
drivers/net/ethernet/marvell/mvneta.c | 90 +++++++++++++++++++++++++++++----
1 file changed, 79 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 96208f1..13a2aa2 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -100,6 +100,8 @@
#define MVNETA_TXQ_CMD 0x2448
#define MVNETA_TXQ_DISABLE_SHIFT 8
#define MVNETA_TXQ_ENABLE_MASK 0x000000ff
+#define MVNETA_GMAC_CLOCK_DIVIDER 0x24f4
+#define MVNETA_GMAC_1MS_CLOCK_ENABLE BIT(31)
#define MVNETA_ACC_MODE 0x2500
#define MVNETA_CPU_MAP(cpu) (0x2540 + ((cpu) << 2))
#define MVNETA_CPU_RXQ_ACCESS_ALL_MASK 0x000000ff
@@ -122,6 +124,7 @@
#define MVNETA_TX_INTR_MASK_ALL (0xff << 0)
#define MVNETA_RX_INTR_MASK(nr_rxqs) (((1 << nr_rxqs) - 1) << 8)
#define MVNETA_RX_INTR_MASK_ALL (0xff << 8)
+#define MVNETA_MISCINTR_INTR_MASK BIT(31)
#define MVNETA_INTR_OLD_CAUSE 0x25a8
#define MVNETA_INTR_OLD_MASK 0x25ac
@@ -165,6 +168,7 @@
#define MVNETA_GMAC_MAX_RX_SIZE_MASK 0x7ffc
#define MVNETA_GMAC0_PORT_ENABLE BIT(0)
#define MVNETA_GMAC_CTRL_2 0x2c08
+#define MVNETA_GMAC2_INBAND_AN_ENABLE BIT(0)
#define MVNETA_GMAC2_PCS_ENABLE BIT(3)
#define MVNETA_GMAC2_PORT_RGMII BIT(4)
#define MVNETA_GMAC2_PORT_RESET BIT(6)
@@ -180,9 +184,11 @@
#define MVNETA_GMAC_AUTONEG_CONFIG 0x2c0c
#define MVNETA_GMAC_FORCE_LINK_DOWN BIT(0)
#define MVNETA_GMAC_FORCE_LINK_PASS BIT(1)
+#define MVNETA_GMAC_INBAND_AN_ENABLE BIT(2)
#define MVNETA_GMAC_CONFIG_MII_SPEED BIT(5)
#define MVNETA_GMAC_CONFIG_GMII_SPEED BIT(6)
#define MVNETA_GMAC_AN_SPEED_EN BIT(7)
+#define MVNETA_GMAC_AN_FLOW_CTRL_EN BIT(11)
#define MVNETA_GMAC_CONFIG_FULL_DUPLEX BIT(12)
#define MVNETA_GMAC_AN_DUPLEX_EN BIT(13)
#define MVNETA_MIB_COUNTERS_BASE 0x3080
@@ -304,6 +310,7 @@ struct mvneta_port {
unsigned int link;
unsigned int duplex;
unsigned int speed;
+ int use_inband_status:1;
};
/* The mvneta_tx_desc and mvneta_rx_desc structures describe the
@@ -994,6 +1001,20 @@ static void mvneta_defaults_set(struct mvneta_port *pp)
val &= ~MVNETA_PHY_POLLING_ENABLE;
mvreg_write(pp, MVNETA_UNIT_CONTROL, val);
+ if (pp->use_inband_status) {
+ val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG);
+ val &= ~(MVNETA_GMAC_FORCE_LINK_PASS |
+ MVNETA_GMAC_FORCE_LINK_DOWN |
+ MVNETA_GMAC_AN_FLOW_CTRL_EN);
+ val |= MVNETA_GMAC_INBAND_AN_ENABLE |
+ MVNETA_GMAC_AN_SPEED_EN |
+ MVNETA_GMAC_AN_DUPLEX_EN;
+ mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val);
+ val = mvreg_read(pp, MVNETA_GMAC_CLOCK_DIVIDER);
+ val |= MVNETA_GMAC_1MS_CLOCK_ENABLE;
+ mvreg_write(pp, MVNETA_GMAC_CLOCK_DIVIDER, val);
+ }
+
mvneta_set_ucast_table(pp, -1);
mvneta_set_special_mcast_table(pp, -1);
mvneta_set_other_mcast_table(pp, -1);
@@ -2043,6 +2064,23 @@ static irqreturn_t mvneta_isr(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static int mvneta_fixed_link_update(struct net_device *dev,
+ struct fixed_phy_status *status)
+{
+ struct mvneta_port *pp = netdev_priv(dev);
+ u32 gmac_stat = mvreg_read(pp, MVNETA_GMAC_STATUS);
+
+ status->link = !!(gmac_stat & MVNETA_GMAC_LINK_UP);
+ if (gmac_stat & MVNETA_GMAC_SPEED_1000)
+ status->speed = SPEED_1000;
+ else if (gmac_stat & MVNETA_GMAC_SPEED_100)
+ status->speed = SPEED_100;
+ else
+ status->speed = SPEED_10;
+ status->duplex = !!(gmac_stat & MVNETA_GMAC_FULL_DUPLEX);
+ return 0;
+}
+
/* NAPI handler
* Bits 0 - 7 of the causeRxTx register indicate that are transmitted
* packets on the corresponding TXQ (Bit 0 is for TX queue 1).
@@ -2063,8 +2101,12 @@ static int mvneta_poll(struct napi_struct *napi, int budget)
}
/* Read cause register */
- cause_rx_tx = mvreg_read(pp, MVNETA_INTR_NEW_CAUSE) &
- (MVNETA_RX_INTR_MASK(rxq_number) | MVNETA_TX_INTR_MASK(txq_number));
+ cause_rx_tx = mvreg_read(pp, MVNETA_INTR_NEW_CAUSE);
+ if (cause_rx_tx & MVNETA_MISCINTR_INTR_MASK) {
+ /* since phylib uses polling, not much to do here...
+ * Any way to tell phylib to do the poll NOW? */
+ mvreg_write(pp, MVNETA_INTR_MISC_CAUSE, 0);
+ }
/* Release Tx descriptors */
if (cause_rx_tx & MVNETA_TX_INTR_MASK_ALL) {
@@ -2109,7 +2151,9 @@ static int mvneta_poll(struct napi_struct *napi, int budget)
napi_complete(napi);
local_irq_save(flags);
mvreg_write(pp, MVNETA_INTR_NEW_MASK,
- MVNETA_RX_INTR_MASK(rxq_number) | MVNETA_TX_INTR_MASK(txq_number));
+ MVNETA_RX_INTR_MASK(rxq_number) |
+ MVNETA_TX_INTR_MASK(txq_number) |
+ MVNETA_MISCINTR_INTR_MASK);
local_irq_restore(flags);
}
@@ -2373,7 +2417,13 @@ static void mvneta_start_dev(struct mvneta_port *pp)
/* Unmask interrupts */
mvreg_write(pp, MVNETA_INTR_NEW_MASK,
- MVNETA_RX_INTR_MASK(rxq_number) | MVNETA_TX_INTR_MASK(txq_number));
+ MVNETA_RX_INTR_MASK(rxq_number) |
+ MVNETA_TX_INTR_MASK(txq_number) |
+ MVNETA_MISCINTR_INTR_MASK);
+ mvreg_write(pp, MVNETA_INTR_MISC_MASK,
+ MVNETA_CAUSE_PHY_STATUS_CHANGE |
+ MVNETA_CAUSE_LINK_CHANGE |
+ MVNETA_CAUSE_PSC_SYNC_CHANGE);
phy_start(pp->phy_dev);
netif_tx_start_all_queues(pp->dev);
@@ -2523,9 +2573,7 @@ static void mvneta_adjust_link(struct net_device *ndev)
val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG);
val &= ~(MVNETA_GMAC_CONFIG_MII_SPEED |
MVNETA_GMAC_CONFIG_GMII_SPEED |
- MVNETA_GMAC_CONFIG_FULL_DUPLEX |
- MVNETA_GMAC_AN_SPEED_EN |
- MVNETA_GMAC_AN_DUPLEX_EN);
+ MVNETA_GMAC_CONFIG_FULL_DUPLEX);
if (phydev->duplex)
val |= MVNETA_GMAC_CONFIG_FULL_DUPLEX;
@@ -2554,12 +2602,24 @@ static void mvneta_adjust_link(struct net_device *ndev)
if (status_change) {
if (phydev->link) {
- u32 val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG);
- val |= (MVNETA_GMAC_FORCE_LINK_PASS |
- MVNETA_GMAC_FORCE_LINK_DOWN);
- mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val);
+ if (!pp->use_inband_status) {
+ u32 val = mvreg_read(pp,
+ MVNETA_GMAC_AUTONEG_CONFIG);
+ val &= ~MVNETA_GMAC_FORCE_LINK_DOWN;
+ val |= MVNETA_GMAC_FORCE_LINK_PASS;
+ mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG,
+ val);
+ }
mvneta_port_up(pp);
} else {
+ if (!pp->use_inband_status) {
+ u32 val = mvreg_read(pp,
+ MVNETA_GMAC_AUTONEG_CONFIG);
+ val &= ~MVNETA_GMAC_FORCE_LINK_PASS;
+ val |= MVNETA_GMAC_FORCE_LINK_DOWN;
+ mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG,
+ val);
+ }
mvneta_port_down(pp);
}
phy_print_status(phydev);
@@ -2576,6 +2636,7 @@ static int mvneta_mdio_probe(struct mvneta_port *pp)
netdev_err(pp->dev, "could not find the PHY\n");
return -ENODEV;
}
+ fixed_phy_set_link_update(phy_dev, mvneta_fixed_link_update);
phy_dev->supported &= PHY_GBIT_FEATURES;
phy_dev->advertising = phy_dev->supported;
@@ -2590,6 +2651,7 @@ static int mvneta_mdio_probe(struct mvneta_port *pp)
static void mvneta_mdio_remove(struct mvneta_port *pp)
{
+ fixed_phy_set_link_update(pp->phy_dev, NULL);
phy_disconnect(pp->phy_dev);
pp->phy_dev = NULL;
}
@@ -2910,6 +2972,9 @@ static int mvneta_port_power_up(struct mvneta_port *pp, int phy_mode)
return -EINVAL;
}
+ if (pp->use_inband_status)
+ ctrl |= MVNETA_GMAC2_INBAND_AN_ENABLE;
+
/* Cancel Port Reset */
ctrl &= ~MVNETA_GMAC2_PORT_RESET;
mvreg_write(pp, MVNETA_GMAC_CTRL_2, ctrl);
@@ -2934,6 +2999,7 @@ static int mvneta_probe(struct platform_device *pdev)
char hw_mac_addr[ETH_ALEN];
const char *mac_from;
int phy_mode;
+ int fixed_phy = 0;
int err;
/* Our multiqueue support is not complete, so for now, only
@@ -2967,6 +3033,7 @@ static int mvneta_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "cannot register fixed PHY\n");
goto err_free_irq;
}
+ fixed_phy = 1;
/* In the case of a fixed PHY, the DT node associated
* to the PHY is the Ethernet MAC DT node.
@@ -2990,6 +3057,7 @@ static int mvneta_probe(struct platform_device *pdev)
pp = netdev_priv(dev);
pp->phy_node = phy_node;
pp->phy_interface = phy_mode;
+ pp->use_inband_status = (phy_mode == PHY_INTERFACE_MODE_SGMII) && fixed_phy;
pp->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(pp->clk)) {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mvneta: implement SGMII-based in-band link state signaling
2015-03-31 13:24 [PATCH] mvneta: implement SGMII-based in-band link state signaling Stas Sergeev
@ 2015-04-03 0:51 ` David Miller
2015-04-03 1:03 ` Florian Fainelli
2015-04-03 10:00 ` Stas Sergeev
0 siblings, 2 replies; 6+ messages in thread
From: David Miller @ 2015-04-03 0:51 UTC (permalink / raw)
To: stsp; +Cc: netdev, linux-kernel, thomas.petazzoni, f.fainelli
From: Stas Sergeev <stsp@list.ru>
Date: Tue, 31 Mar 2015 16:24:59 +0300
> @@ -2590,6 +2651,7 @@ static int mvneta_mdio_probe(struct mvneta_port *pp)
>
> static void mvneta_mdio_remove(struct mvneta_port *pp)
> {
> + fixed_phy_set_link_update(pp->phy_dev, NULL);
I do not see any other driver doing this on shutdown.
Please show me why it is necessary.
And if it is, all other drivers registering a fixed phy link update
function need to be adjusted to do the same thing.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mvneta: implement SGMII-based in-band link state signaling
2015-04-03 0:51 ` David Miller
@ 2015-04-03 1:03 ` Florian Fainelli
2015-04-03 19:04 ` David Miller
2015-04-03 10:00 ` Stas Sergeev
1 sibling, 1 reply; 6+ messages in thread
From: Florian Fainelli @ 2015-04-03 1:03 UTC (permalink / raw)
To: David Miller, stsp; +Cc: netdev, linux-kernel, thomas.petazzoni
On 02/04/15 17:51, David Miller wrote:
> From: Stas Sergeev <stsp@list.ru>
> Date: Tue, 31 Mar 2015 16:24:59 +0300
>
>> @@ -2590,6 +2651,7 @@ static int mvneta_mdio_probe(struct mvneta_port *pp)
>>
>> static void mvneta_mdio_remove(struct mvneta_port *pp)
>> {
>> + fixed_phy_set_link_update(pp->phy_dev, NULL);
>
> I do not see any other driver doing this on shutdown.
> Please show me why it is necessary.
The primary reason is that if you do not do that, past the point where
you call phy_disconnect(), we stop the PHY state machine, detach from
the net_device, such that it won't invoke the adjust_link callback
anymore. The fixed PHY driver, though will still keep calling the
fixed_link_update callback asking the driver whether the link parameters
need to be updated, and that will just cause a NULL pointer de-reference
phydev->attached_dev, since we are now in detached state.
I guess another way to fix that is to look for the PHY state in
fixed_mdio_read() and do nothing if it is PHY_HALTED.
>
> And if it is, all other drivers registering a fixed phy link update
> function need to be adjusted to do the same thing.
>
I think the bcmgenet driver is now doing this as a result of Petri's
latest changes, and I meant to comment on that before the patch got in.
drivers/net/dsa/bcm_sf2.c has a similar construct but does not invoke
phy_disconnect() nor can be rmmod'd, so a lesser issue.
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mvneta: implement SGMII-based in-band link state signaling
2015-04-03 0:51 ` David Miller
2015-04-03 1:03 ` Florian Fainelli
@ 2015-04-03 10:00 ` Stas Sergeev
1 sibling, 0 replies; 6+ messages in thread
From: Stas Sergeev @ 2015-04-03 10:00 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-kernel, thomas.petazzoni, f.fainelli, Stas Sergeev
03.04.2015 03:51, David Miller пишет:
> From: Stas Sergeev <stsp@list.ru>
> Date: Tue, 31 Mar 2015 16:24:59 +0300
>
>> @@ -2590,6 +2651,7 @@ static int mvneta_mdio_probe(struct mvneta_port *pp)
>>
>> static void mvneta_mdio_remove(struct mvneta_port *pp)
>> {
>> + fixed_phy_set_link_update(pp->phy_dev, NULL);
>
> I do not see any other driver doing this on shutdown.
> Please show me why it is necessary.
Hello David, sorry for this being discussed in a different thread,
so here you have a few pointers to why is this needed:
https://lkml.org/lkml/2015/3/30/367
> And if it is, all other drivers registering a fixed phy link update
> function need to be adjusted to do the same thing.
Or, for example, get rid of the callback and add an API:
http://www.spinics.net/lists/netdev/msg323517.html
Then you get the patch that avoids all this call-back singing and dancing:
http://www.spinics.net/lists/netdev/msg323518.html
In any case, I posted 3 different implementations and hope
people will choose the best one (any choice is fine with me).
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mvneta: implement SGMII-based in-band link state signaling
2015-04-03 1:03 ` Florian Fainelli
@ 2015-04-03 19:04 ` David Miller
2015-04-03 19:06 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2015-04-03 19:04 UTC (permalink / raw)
To: f.fainelli; +Cc: stsp, netdev, linux-kernel, thomas.petazzoni
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Thu, 02 Apr 2015 18:03:53 -0700
> On 02/04/15 17:51, David Miller wrote:
>> From: Stas Sergeev <stsp@list.ru>
>> Date: Tue, 31 Mar 2015 16:24:59 +0300
>>
>>> @@ -2590,6 +2651,7 @@ static int mvneta_mdio_probe(struct mvneta_port *pp)
>>>
>>> static void mvneta_mdio_remove(struct mvneta_port *pp)
>>> {
>>> + fixed_phy_set_link_update(pp->phy_dev, NULL);
>>
>> I do not see any other driver doing this on shutdown.
>> Please show me why it is necessary.
>
> The primary reason is that if you do not do that, past the point where
> you call phy_disconnect(), we stop the PHY state machine, detach from
> the net_device, such that it won't invoke the adjust_link callback
> anymore. The fixed PHY driver, though will still keep calling the
> fixed_link_update callback asking the driver whether the link parameters
> need to be updated, and that will just cause a NULL pointer de-reference
> phydev->attached_dev, since we are now in detached state.
>
> I guess another way to fix that is to look for the PHY state in
> fixed_mdio_read() and do nothing if it is PHY_HALTED.
Ok I'll apply this mvneta patch to net-next then, thanks.
>> And if it is, all other drivers registering a fixed phy link update
>> function need to be adjusted to do the same thing.
>
> I think the bcmgenet driver is now doing this as a result of Petri's
> latest changes, and I meant to comment on that before the patch got in.
> drivers/net/dsa/bcm_sf2.c has a similar construct but does not invoke
> phy_disconnect() nor can be rmmod'd, so a lesser issue.
I just seems insane to me that phy_disconnect() doesn't stop the
callbacks from running.
Fixed PHY seems to me to suffer from a lack of proper integration
into the PHY layer.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mvneta: implement SGMII-based in-band link state signaling
2015-04-03 19:04 ` David Miller
@ 2015-04-03 19:06 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2015-04-03 19:06 UTC (permalink / raw)
To: f.fainelli; +Cc: stsp, netdev, linux-kernel, thomas.petazzoni
From: David Miller <davem@davemloft.net>
Date: Fri, 03 Apr 2015 15:04:10 -0400 (EDT)
> Ok I'll apply this mvneta patch to net-next then, thanks.
Nevermind I just saw in my patchwork queue that there is a v3
of this change set that adds some adjustments to the fixed
PHY interfaces.
I think I'll apply that series instead of this patch.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-04-03 19:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-31 13:24 [PATCH] mvneta: implement SGMII-based in-band link state signaling Stas Sergeev
2015-04-03 0:51 ` David Miller
2015-04-03 1:03 ` Florian Fainelli
2015-04-03 19:04 ` David Miller
2015-04-03 19:06 ` David Miller
2015-04-03 10:00 ` Stas Sergeev
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).