* [PATCH v2 net-next 0/8] net/ncsi: NCSI Improvment and bug fixes
@ 2016-10-04 0:25 Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 1/8] net/ncsi: Avoid unused-value build warning from ia64-linux-gcc Gavin Shan
` (8 more replies)
0 siblings, 9 replies; 10+ messages in thread
From: Gavin Shan @ 2016-10-04 0:25 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
This series of patches improves NCSI stack according to the comments
I received after the NCSI code was merged to 4.8.rc1:
* PATCH[1/8] fixes the build warning caused by xchg() with ia64-linux-gcc.
The atomic operations are removed. The NCSI's lock should be taken when
reading or updating its state and chained state.
* Channel ID (0x1f) is the reserved one and it cannot be valid channel ID.
So we needn't try to probe channel whose ID is 0x1f. PATCH[2/8] and
PATCH[3/8] are addressing this issue.
* The request IDs are assigned in round-robin fashion, but it's broken.
PATCH[4/8] make it work.
* PATCH[5/8] and PATCH[6/8] reworks the channel monitoring to improve the
code readability and its robustness.
* PATCH[7/8] and PATCH[8/8] introduces ncsi_stop_dev() so that the network
device can be closed and opened afterwards. No error will be seen.
Changelog
=========
v2:
* The NCSI's lock is taken when reading or updating its state as the
{READ,WRITE}_ONCE() isn't reliable.
Gavin Shan (8):
net/ncsi: Avoid unused-value build warning from ia64-linux-gcc
net/ncsi: Introduce NCSI_RESERVED_CHANNEL
net/ncsi: Don't probe on the reserved channel ID (0x1f)
net/ncsi: Rework request index allocation
net/ncsi: Allow to extend NCSI request properties
net/ncsi: Rework the channel monitoring
net/ncsi: Introduce ncsi_stop_dev()
net/faraday: Stop NCSI device on shutdown
drivers/net/ethernet/faraday/ftgmac100.c | 2 +
include/net/ncsi.h | 5 +
net/ncsi/internal.h | 22 +++-
net/ncsi/ncsi-aen.c | 37 ++++--
net/ncsi/ncsi-cmd.c | 2 +-
net/ncsi/ncsi-manage.c | 198 ++++++++++++++++++++-----------
net/ncsi/ncsi-rsp.c | 4 +-
7 files changed, 180 insertions(+), 90 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 net-next 1/8] net/ncsi: Avoid unused-value build warning from ia64-linux-gcc
2016-10-04 0:25 [PATCH v2 net-next 0/8] net/ncsi: NCSI Improvment and bug fixes Gavin Shan
@ 2016-10-04 0:25 ` Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 2/8] net/ncsi: Introduce NCSI_RESERVED_CHANNEL Gavin Shan
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Gavin Shan @ 2016-10-04 0:25 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
xchg() is used to set NCSI channel's state in order for consistent
access to the state. xchg()'s return value should be used. Otherwise,
one build warning will be raised (with -Wunused-value) as below message
indicates. It is reported by ia64-linux-gcc (GCC) 4.9.0.
net/ncsi/ncsi-manage.c: In function 'ncsi_channel_monitor':
arch/ia64/include/uapi/asm/cmpxchg.h:56:2: warning: value computed is \
not used [-Wunused-value]
((__typeof__(*(ptr))) __xchg((unsigned long) (x), (ptr), sizeof(*(ptr))))
^
net/ncsi/ncsi-manage.c:202:3: note: in expansion of macro 'xchg'
xchg(&nc->state, NCSI_CHANNEL_INACTIVE);
This removes the atomic access to NCSI channel's state avoid the above
build warning. We have to hold the channel's lock when its state is readed
or updated. No functional changes introduced.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
net/ncsi/ncsi-aen.c | 37 +++++++++++++++++++-------
net/ncsi/ncsi-manage.c | 71 ++++++++++++++++++++++++++++++++++++++------------
2 files changed, 81 insertions(+), 27 deletions(-)
diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index d463468..b41a661 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -53,7 +53,9 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp,
struct ncsi_aen_lsc_pkt *lsc;
struct ncsi_channel *nc;
struct ncsi_channel_mode *ncm;
- unsigned long old_data;
+ bool chained;
+ int state;
+ unsigned long old_data, data;
unsigned long flags;
/* Find the NCSI channel */
@@ -62,20 +64,27 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp,
return -ENODEV;
/* Update the link status */
- ncm = &nc->modes[NCSI_MODE_LINK];
lsc = (struct ncsi_aen_lsc_pkt *)h;
+
+ spin_lock_irqsave(&nc->lock, flags);
+ ncm = &nc->modes[NCSI_MODE_LINK];
old_data = ncm->data[2];
- ncm->data[2] = ntohl(lsc->status);
+ data = ntohl(lsc->status);
+ ncm->data[2] = data;
ncm->data[4] = ntohl(lsc->oem_status);
- if (!((old_data ^ ncm->data[2]) & 0x1) ||
- !list_empty(&nc->link))
+
+ chained = !list_empty(&nc->link);
+ state = nc->state;
+ spin_unlock_irqrestore(&nc->lock, flags);
+
+ if (!((old_data ^ data) & 0x1) || chained)
return 0;
- if (!(nc->state == NCSI_CHANNEL_INACTIVE && (ncm->data[2] & 0x1)) &&
- !(nc->state == NCSI_CHANNEL_ACTIVE && !(ncm->data[2] & 0x1)))
+ if (!(state == NCSI_CHANNEL_INACTIVE && (data & 0x1)) &&
+ !(state == NCSI_CHANNEL_ACTIVE && !(data & 0x1)))
return 0;
if (!(ndp->flags & NCSI_DEV_HWA) &&
- nc->state == NCSI_CHANNEL_ACTIVE)
+ state == NCSI_CHANNEL_ACTIVE)
ndp->flags |= NCSI_DEV_RESHUFFLE;
ncsi_stop_channel_monitor(nc);
@@ -97,13 +106,21 @@ static int ncsi_aen_handler_cr(struct ncsi_dev_priv *ndp,
if (!nc)
return -ENODEV;
+ spin_lock_irqsave(&nc->lock, flags);
if (!list_empty(&nc->link) ||
- nc->state != NCSI_CHANNEL_ACTIVE)
+ nc->state != NCSI_CHANNEL_ACTIVE) {
+ spin_unlock_irqrestore(&nc->lock, flags);
return 0;
+ }
+ spin_unlock_irqrestore(&nc->lock, flags);
ncsi_stop_channel_monitor(nc);
+ spin_lock_irqsave(&nc->lock, flags);
+ nc->state = NCSI_CHANNEL_INVISIBLE;
+ spin_unlock_irqrestore(&nc->lock, flags);
+
spin_lock_irqsave(&ndp->lock, flags);
- xchg(&nc->state, NCSI_CHANNEL_INACTIVE);
+ nc->state = NCSI_CHANNEL_INACTIVE;
list_add_tail_rcu(&nc->link, &ndp->channel_queue);
spin_unlock_irqrestore(&ndp->lock, flags);
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index ef017b8..a26ce51 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -132,6 +132,7 @@ static void ncsi_report_link(struct ncsi_dev_priv *ndp, bool force_down)
struct ncsi_dev *nd = &ndp->ndev;
struct ncsi_package *np;
struct ncsi_channel *nc;
+ unsigned long flags;
nd->state = ncsi_dev_state_functional;
if (force_down) {
@@ -142,14 +143,21 @@ static void ncsi_report_link(struct ncsi_dev_priv *ndp, bool force_down)
nd->link_up = 0;
NCSI_FOR_EACH_PACKAGE(ndp, np) {
NCSI_FOR_EACH_CHANNEL(np, nc) {
+ spin_lock_irqsave(&nc->lock, flags);
+
if (!list_empty(&nc->link) ||
- nc->state != NCSI_CHANNEL_ACTIVE)
+ nc->state != NCSI_CHANNEL_ACTIVE) {
+ spin_unlock_irqrestore(&nc->lock, flags);
continue;
+ }
if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) {
+ spin_unlock_irqrestore(&nc->lock, flags);
nd->link_up = 1;
goto report;
}
+
+ spin_unlock_irqrestore(&nc->lock, flags);
}
}
@@ -163,20 +171,22 @@ static void ncsi_channel_monitor(unsigned long data)
struct ncsi_package *np = nc->package;
struct ncsi_dev_priv *ndp = np->ndp;
struct ncsi_cmd_arg nca;
- bool enabled;
+ bool enabled, chained;
unsigned int timeout;
unsigned long flags;
- int ret;
+ int state, ret;
spin_lock_irqsave(&nc->lock, flags);
+ state = nc->state;
+ chained = !list_empty(&nc->link);
timeout = nc->timeout;
enabled = nc->enabled;
spin_unlock_irqrestore(&nc->lock, flags);
- if (!enabled || !list_empty(&nc->link))
+ if (!enabled || chained)
return;
- if (nc->state != NCSI_CHANNEL_INACTIVE &&
- nc->state != NCSI_CHANNEL_ACTIVE)
+ if (state != NCSI_CHANNEL_INACTIVE &&
+ state != NCSI_CHANNEL_ACTIVE)
return;
if (!(timeout % 2)) {
@@ -195,11 +205,15 @@ static void ncsi_channel_monitor(unsigned long data)
if (timeout + 1 >= 3) {
if (!(ndp->flags & NCSI_DEV_HWA) &&
- nc->state == NCSI_CHANNEL_ACTIVE)
+ state == NCSI_CHANNEL_ACTIVE)
ncsi_report_link(ndp, true);
+ spin_lock_irqsave(&nc->lock, flags);
+ nc->state = NCSI_CHANNEL_INVISIBLE;
+ spin_unlock_irqrestore(&nc->lock, flags);
+
spin_lock_irqsave(&ndp->lock, flags);
- xchg(&nc->state, NCSI_CHANNEL_INACTIVE);
+ nc->state = NCSI_CHANNEL_INACTIVE;
list_add_tail_rcu(&nc->link, &ndp->channel_queue);
spin_unlock_irqrestore(&ndp->lock, flags);
ncsi_process_next_channel(ndp);
@@ -508,6 +522,7 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
struct ncsi_package *np = ndp->active_package;
struct ncsi_channel *nc = ndp->active_channel;
struct ncsi_cmd_arg nca;
+ unsigned long flags;
int ret;
nca.ndp = ndp;
@@ -556,7 +571,9 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
break;
case ncsi_dev_state_suspend_done:
- xchg(&nc->state, NCSI_CHANNEL_INACTIVE);
+ spin_lock_irqsave(&nc->lock, flags);
+ nc->state = NCSI_CHANNEL_INACTIVE;
+ spin_unlock_irqrestore(&nc->lock, flags);
ncsi_process_next_channel(ndp);
break;
@@ -574,6 +591,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
struct ncsi_channel *nc = ndp->active_channel;
struct ncsi_cmd_arg nca;
unsigned char index;
+ unsigned long flags;
int ret;
nca.ndp = ndp;
@@ -675,10 +693,12 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
goto error;
break;
case ncsi_dev_state_config_done:
+ spin_lock_irqsave(&nc->lock, flags);
if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1)
- xchg(&nc->state, NCSI_CHANNEL_ACTIVE);
+ nc->state = NCSI_CHANNEL_ACTIVE;
else
- xchg(&nc->state, NCSI_CHANNEL_INACTIVE);
+ nc->state = NCSI_CHANNEL_INACTIVE;
+ spin_unlock_irqrestore(&nc->lock, flags);
ncsi_start_channel_monitor(nc);
ncsi_process_next_channel(ndp);
@@ -707,18 +727,25 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
found = NULL;
NCSI_FOR_EACH_PACKAGE(ndp, np) {
NCSI_FOR_EACH_CHANNEL(np, nc) {
+ spin_lock_irqsave(&nc->lock, flags);
+
if (!list_empty(&nc->link) ||
- nc->state != NCSI_CHANNEL_INACTIVE)
+ nc->state != NCSI_CHANNEL_INACTIVE) {
+ spin_unlock_irqrestore(&nc->lock, flags);
continue;
+ }
if (!found)
found = nc;
ncm = &nc->modes[NCSI_MODE_LINK];
if (ncm->data[2] & 0x1) {
+ spin_unlock_irqrestore(&nc->lock, flags);
found = nc;
goto out;
}
+
+ spin_unlock_irqrestore(&nc->lock, flags);
}
}
@@ -987,11 +1014,14 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
goto out;
}
- old_state = xchg(&nc->state, NCSI_CHANNEL_INVISIBLE);
list_del_init(&nc->link);
-
spin_unlock_irqrestore(&ndp->lock, flags);
+ spin_lock_irqsave(&nc->lock, flags);
+ old_state = nc->state;
+ nc->state = NCSI_CHANNEL_INVISIBLE;
+ spin_unlock_irqrestore(&nc->lock, flags);
+
ndp->active_channel = nc;
ndp->active_package = nc->package;
@@ -1006,7 +1036,7 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
break;
default:
netdev_err(ndp->ndev.dev, "Invalid state 0x%x on %d:%d\n",
- nc->state, nc->package->id, nc->id);
+ old_state, nc->package->id, nc->id);
ncsi_report_link(ndp, false);
return -EINVAL;
}
@@ -1151,6 +1181,8 @@ int ncsi_start_dev(struct ncsi_dev *nd)
struct ncsi_dev_priv *ndp = TO_NCSI_DEV_PRIV(nd);
struct ncsi_package *np;
struct ncsi_channel *nc;
+ unsigned long flags;
+ bool chained;
int old_state, ret;
if (nd->state != ncsi_dev_state_registered &&
@@ -1166,8 +1198,13 @@ int ncsi_start_dev(struct ncsi_dev *nd)
/* Reset channel's state and start over */
NCSI_FOR_EACH_PACKAGE(ndp, np) {
NCSI_FOR_EACH_CHANNEL(np, nc) {
- old_state = xchg(&nc->state, NCSI_CHANNEL_INACTIVE);
- WARN_ON_ONCE(!list_empty(&nc->link) ||
+ spin_lock_irqsave(&nc->lock, flags);
+ chained = !list_empty(&nc->link);
+ old_state = nc->state;
+ nc->state = NCSI_CHANNEL_INACTIVE;
+ spin_unlock_irqrestore(&nc->lock, flags);
+
+ WARN_ON_ONCE(chained ||
old_state == NCSI_CHANNEL_INVISIBLE);
}
}
--
2.1.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 net-next 2/8] net/ncsi: Introduce NCSI_RESERVED_CHANNEL
2016-10-04 0:25 [PATCH v2 net-next 0/8] net/ncsi: NCSI Improvment and bug fixes Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 1/8] net/ncsi: Avoid unused-value build warning from ia64-linux-gcc Gavin Shan
@ 2016-10-04 0:25 ` Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 3/8] net/ncsi: Don't probe on the reserved channel ID (0x1f) Gavin Shan
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Gavin Shan @ 2016-10-04 0:25 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
This defines NCSI_RESERVED_CHANNEL as the reserved NCSI channel
ID (0x1f). No logical changes introduced.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
net/ncsi/internal.h | 1 +
net/ncsi/ncsi-manage.c | 14 +++++++-------
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 33738c0..66dc851 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -170,6 +170,7 @@ struct ncsi_package;
#define NCSI_PACKAGE_SHIFT 5
#define NCSI_PACKAGE_INDEX(c) (((c) >> NCSI_PACKAGE_SHIFT) & 0x7)
+#define NCSI_RESERVED_CHANNEL 0x1f
#define NCSI_CHANNEL_INDEX(c) ((c) & ((1 << NCSI_PACKAGE_SHIFT) - 1))
#define NCSI_TO_CHANNEL(p, c) (((p) << NCSI_PACKAGE_SHIFT) | (c))
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index a26ce51..97c99be 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -542,7 +542,7 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
nca.package = np->id;
if (nd->state == ncsi_dev_state_suspend_select) {
nca.type = NCSI_PKT_CMD_SP;
- nca.channel = 0x1f;
+ nca.channel = NCSI_RESERVED_CHANNEL;
if (ndp->flags & NCSI_DEV_HWA)
nca.bytes[0] = 0;
else
@@ -559,7 +559,7 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
nd->state = ncsi_dev_state_suspend_deselect;
} else if (nd->state == ncsi_dev_state_suspend_deselect) {
nca.type = NCSI_PKT_CMD_DP;
- nca.channel = 0x1f;
+ nca.channel = NCSI_RESERVED_CHANNEL;
nd->state = ncsi_dev_state_suspend_done;
}
@@ -608,7 +608,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
else
nca.bytes[0] = 1;
nca.package = np->id;
- nca.channel = 0x1f;
+ nca.channel = NCSI_RESERVED_CHANNEL;
ret = ncsi_xmit_cmd(&nca);
if (ret)
goto error;
@@ -834,7 +834,7 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
/* Deselect all possible packages */
nca.type = NCSI_PKT_CMD_DP;
- nca.channel = 0x1f;
+ nca.channel = NCSI_RESERVED_CHANNEL;
for (index = 0; index < 8; index++) {
nca.package = index;
ret = ncsi_xmit_cmd(&nca);
@@ -850,7 +850,7 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
/* Select all possible packages */
nca.type = NCSI_PKT_CMD_SP;
nca.bytes[0] = 1;
- nca.channel = 0x1f;
+ nca.channel = NCSI_RESERVED_CHANNEL;
for (index = 0; index < 8; index++) {
nca.package = index;
ret = ncsi_xmit_cmd(&nca);
@@ -903,7 +903,7 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
nca.type = NCSI_PKT_CMD_SP;
nca.bytes[0] = 1;
nca.package = ndp->active_package->id;
- nca.channel = 0x1f;
+ nca.channel = NCSI_RESERVED_CHANNEL;
ret = ncsi_xmit_cmd(&nca);
if (ret)
goto error;
@@ -960,7 +960,7 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
/* Deselect the active package */
nca.type = NCSI_PKT_CMD_DP;
nca.package = ndp->active_package->id;
- nca.channel = 0x1f;
+ nca.channel = NCSI_RESERVED_CHANNEL;
ret = ncsi_xmit_cmd(&nca);
if (ret)
goto error;
--
2.1.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 net-next 3/8] net/ncsi: Don't probe on the reserved channel ID (0x1f)
2016-10-04 0:25 [PATCH v2 net-next 0/8] net/ncsi: NCSI Improvment and bug fixes Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 1/8] net/ncsi: Avoid unused-value build warning from ia64-linux-gcc Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 2/8] net/ncsi: Introduce NCSI_RESERVED_CHANNEL Gavin Shan
@ 2016-10-04 0:25 ` Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 4/8] net/ncsi: Rework request index allocation Gavin Shan
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Gavin Shan @ 2016-10-04 0:25 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
We needn't send CIS (Clear Initial State) command to the NCSI
reserved channel (0x1f) in the enumeration. We shouldn't receive
a valid response from CIS on NCSI channel 0x1f.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
net/ncsi/ncsi-manage.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 97c99be..8c5e016 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -911,12 +911,12 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
nd->state = ncsi_dev_state_probe_cis;
break;
case ncsi_dev_state_probe_cis:
- ndp->pending_req_num = 32;
+ ndp->pending_req_num = NCSI_RESERVED_CHANNEL;
/* Clear initial state */
nca.type = NCSI_PKT_CMD_CIS;
nca.package = ndp->active_package->id;
- for (index = 0; index < 0x20; index++) {
+ for (index = 0; index < NCSI_RESERVED_CHANNEL; index++) {
nca.channel = index;
ret = ncsi_xmit_cmd(&nca);
if (ret)
--
2.1.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 net-next 4/8] net/ncsi: Rework request index allocation
2016-10-04 0:25 [PATCH v2 net-next 0/8] net/ncsi: NCSI Improvment and bug fixes Gavin Shan
` (2 preceding siblings ...)
2016-10-04 0:25 ` [PATCH v2 net-next 3/8] net/ncsi: Don't probe on the reserved channel ID (0x1f) Gavin Shan
@ 2016-10-04 0:25 ` Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 5/8] net/ncsi: Allow to extend NCSI request properties Gavin Shan
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Gavin Shan @ 2016-10-04 0:25 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
The NCSI request index (struct ncsi_request::id) is put into instance
ID (IID) field while sending NCSI command packet. It was designed the
available IDs are given in round-robin fashion. @ndp->request_id was
introduced to represent the next available ID, but it has been used
as number of successively allocated IDs. It breaks the round-robin
design. Besides, we shouldn't put 0 to NCSI command packet's IID
field, meaning ID#0 should be reserved according section 6.3.1.1
in NCSI spec (v1.1.0).
This fixes above two issues. With it applied, the available IDs will
be assigned in round-robin fashion and ID#0 won't be assigned.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
net/ncsi/internal.h | 1 +
net/ncsi/ncsi-manage.c | 17 +++++++++--------
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 66dc851..c956fe8 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -259,6 +259,7 @@ struct ncsi_dev_priv {
struct list_head packages; /* List of packages */
struct ncsi_request requests[256]; /* Request table */
unsigned int request_id; /* Last used request ID */
+#define NCSI_REQ_START_IDX 1
unsigned int pending_req_num; /* Number of pending requests */
struct ncsi_package *active_package; /* Currently handled package */
struct ncsi_channel *active_channel; /* Currently handled channel */
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 8c5e016..00ce2c7 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -427,30 +427,31 @@ struct ncsi_request *ncsi_alloc_request(struct ncsi_dev_priv *ndp, bool driven)
/* Check if there is one available request until the ceiling */
spin_lock_irqsave(&ndp->lock, flags);
- for (i = ndp->request_id; !nr && i < limit; i++) {
+ for (i = ndp->request_id; i < limit; i++) {
if (ndp->requests[i].used)
continue;
nr = &ndp->requests[i];
nr->used = true;
nr->driven = driven;
- if (++ndp->request_id >= limit)
- ndp->request_id = 0;
+ ndp->request_id = i + 1;
+ goto found;
}
/* Fail back to check from the starting cursor */
- for (i = 0; !nr && i < ndp->request_id; i++) {
+ for (i = NCSI_REQ_START_IDX; i < ndp->request_id; i++) {
if (ndp->requests[i].used)
continue;
nr = &ndp->requests[i];
nr->used = true;
nr->driven = driven;
- if (++ndp->request_id >= limit)
- ndp->request_id = 0;
+ ndp->request_id = i + 1;
+ goto found;
}
- spin_unlock_irqrestore(&ndp->lock, flags);
+found:
+ spin_unlock_irqrestore(&ndp->lock, flags);
return nr;
}
@@ -1148,7 +1149,7 @@ struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
/* Initialize private NCSI device */
spin_lock_init(&ndp->lock);
INIT_LIST_HEAD(&ndp->packages);
- ndp->request_id = 0;
+ ndp->request_id = NCSI_REQ_START_IDX;
for (i = 0; i < ARRAY_SIZE(ndp->requests); i++) {
ndp->requests[i].id = i;
ndp->requests[i].ndp = ndp;
--
2.1.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 net-next 5/8] net/ncsi: Allow to extend NCSI request properties
2016-10-04 0:25 [PATCH v2 net-next 0/8] net/ncsi: NCSI Improvment and bug fixes Gavin Shan
` (3 preceding siblings ...)
2016-10-04 0:25 ` [PATCH v2 net-next 4/8] net/ncsi: Rework request index allocation Gavin Shan
@ 2016-10-04 0:25 ` Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 6/8] net/ncsi: Rework the channel monitoring Gavin Shan
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Gavin Shan @ 2016-10-04 0:25 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
There is only one NCSI request property for now: the response for
the sent command need drive the workqueue or not. So we had one
field (@driven) for the purpose. We lost the flexibility to extend
NCSI request properties.
This replaces @driven with @flags and @req_flags in NCSI request
and NCSI command argument struct. Each bit of the newly introduced
field can be used for one property. No functional changes introduced.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
net/ncsi/internal.h | 8 +++++---
net/ncsi/ncsi-cmd.c | 2 +-
net/ncsi/ncsi-manage.c | 19 ++++++++++---------
net/ncsi/ncsi-rsp.c | 2 +-
4 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index c956fe8..26e9295 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -207,7 +207,8 @@ struct ncsi_package {
struct ncsi_request {
unsigned char id; /* Request ID - 0 to 255 */
bool used; /* Request that has been assigned */
- bool driven; /* Drive state machine */
+ unsigned int flags; /* NCSI request property */
+#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
struct ncsi_dev_priv *ndp; /* Associated NCSI device */
struct sk_buff *cmd; /* Associated NCSI command packet */
struct sk_buff *rsp; /* Associated NCSI response packet */
@@ -276,7 +277,7 @@ struct ncsi_cmd_arg {
unsigned char package; /* Destination package ID */
unsigned char channel; /* Detination channel ID or 0x1f */
unsigned short payload; /* Command packet payload length */
- bool driven; /* Drive the state machine? */
+ unsigned int req_flags; /* NCSI request properties */
union {
unsigned char bytes[16]; /* Command packet specific data */
unsigned short words[8];
@@ -315,7 +316,8 @@ void ncsi_find_package_and_channel(struct ncsi_dev_priv *ndp,
unsigned char id,
struct ncsi_package **np,
struct ncsi_channel **nc);
-struct ncsi_request *ncsi_alloc_request(struct ncsi_dev_priv *ndp, bool driven);
+struct ncsi_request *ncsi_alloc_request(struct ncsi_dev_priv *ndp,
+ unsigned int req_flags);
void ncsi_free_request(struct ncsi_request *nr);
struct ncsi_dev *ncsi_find_dev(struct net_device *dev);
int ncsi_process_next_channel(struct ncsi_dev_priv *ndp);
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 21057a8..db7083b 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -272,7 +272,7 @@ static struct ncsi_request *ncsi_alloc_command(struct ncsi_cmd_arg *nca)
struct sk_buff *skb;
struct ncsi_request *nr;
- nr = ncsi_alloc_request(ndp, nca->driven);
+ nr = ncsi_alloc_request(ndp, nca->req_flags);
if (!nr)
return NULL;
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 00ce2c7..adf5401 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -194,7 +194,7 @@ static void ncsi_channel_monitor(unsigned long data)
nca.package = np->id;
nca.channel = nc->id;
nca.type = NCSI_PKT_CMD_GLS;
- nca.driven = false;
+ nca.req_flags = 0;
ret = ncsi_xmit_cmd(&nca);
if (ret) {
netdev_err(ndp->ndev.dev, "Error %d sending GLS\n",
@@ -419,7 +419,8 @@ void ncsi_find_package_and_channel(struct ncsi_dev_priv *ndp,
* be same. Otherwise, the bogus response might be replied. So
* the available IDs are allocated in round-robin fashion.
*/
-struct ncsi_request *ncsi_alloc_request(struct ncsi_dev_priv *ndp, bool driven)
+struct ncsi_request *ncsi_alloc_request(struct ncsi_dev_priv *ndp,
+ unsigned int req_flags)
{
struct ncsi_request *nr = NULL;
int i, limit = ARRAY_SIZE(ndp->requests);
@@ -433,7 +434,7 @@ struct ncsi_request *ncsi_alloc_request(struct ncsi_dev_priv *ndp, bool driven)
nr = &ndp->requests[i];
nr->used = true;
- nr->driven = driven;
+ nr->flags = req_flags;
ndp->request_id = i + 1;
goto found;
}
@@ -445,7 +446,7 @@ struct ncsi_request *ncsi_alloc_request(struct ncsi_dev_priv *ndp, bool driven)
nr = &ndp->requests[i];
nr->used = true;
- nr->driven = driven;
+ nr->flags = req_flags;
ndp->request_id = i + 1;
goto found;
}
@@ -473,7 +474,7 @@ void ncsi_free_request(struct ncsi_request *nr)
nr->cmd = NULL;
nr->rsp = NULL;
nr->used = false;
- driven = nr->driven;
+ driven = !!(nr->flags & NCSI_REQ_FLAG_EVENT_DRIVEN);
spin_unlock_irqrestore(&ndp->lock, flags);
if (driven && cmd && --ndp->pending_req_num == 0)
@@ -527,7 +528,7 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
int ret;
nca.ndp = ndp;
- nca.driven = true;
+ nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
switch (nd->state) {
case ncsi_dev_state_suspend:
nd->state = ncsi_dev_state_suspend_select;
@@ -596,7 +597,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
int ret;
nca.ndp = ndp;
- nca.driven = true;
+ nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
switch (nd->state) {
case ncsi_dev_state_config:
case ncsi_dev_state_config_sp:
@@ -825,7 +826,7 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
int ret;
nca.ndp = ndp;
- nca.driven = true;
+ nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
switch (nd->state) {
case ncsi_dev_state_probe:
nd->state = ncsi_dev_state_probe_deselect;
@@ -1101,7 +1102,7 @@ static int ncsi_inet6addr_event(struct notifier_block *this,
return NOTIFY_OK;
nca.ndp = ndp;
- nca.driven = false;
+ nca.req_flags = 0;
nca.package = np->id;
nca.channel = nc->id;
nca.dwords[0] = nc->caps[NCSI_CAP_MC].cap;
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index af84389..86cdaeb 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -317,7 +317,7 @@ static int ncsi_rsp_handler_gls(struct ncsi_request *nr)
ncm->data[3] = ntohl(rsp->other);
ncm->data[4] = ntohl(rsp->oem_status);
- if (nr->driven)
+ if (nr->flags & NCSI_REQ_FLAG_EVENT_DRIVEN)
return 0;
/* Reset the channel monitor if it has been enabled */
--
2.1.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 net-next 6/8] net/ncsi: Rework the channel monitoring
2016-10-04 0:25 [PATCH v2 net-next 0/8] net/ncsi: NCSI Improvment and bug fixes Gavin Shan
` (4 preceding siblings ...)
2016-10-04 0:25 ` [PATCH v2 net-next 5/8] net/ncsi: Allow to extend NCSI request properties Gavin Shan
@ 2016-10-04 0:25 ` Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 7/8] net/ncsi: Introduce ncsi_stop_dev() Gavin Shan
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Gavin Shan @ 2016-10-04 0:25 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
The original NCSI channel monitoring was implemented based on a
backoff algorithm: the GLS response should be received in the
specified interval. Otherwise, the channel is regarded as dead
and failover should be taken if current channel is an active one.
There are several problems in the implementation: (A) On BCM5718,
we found when the IID (Instance ID) in the GLS command packet
changes from 255 to 1, the response corresponding to IID#1 never
comes in. It means we cannot make the unfair judgement that the
channel is dead when one response is missed. (B) The code's
readability should be improved. (C) We should do failover when
current channel is active one and the channel monitoring should
be marked as disabled before doing failover.
This reworks the channel monitoring to address all above issues.
The fields for channel monitoring is put into separate struct
and the state of channel monitoring is predefined. The channel
is regarded alive if the network controller responses to one of
two GLS commands or both of them in 5 seconds.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
net/ncsi/internal.h | 12 +++++++++---
net/ncsi/ncsi-manage.c | 44 +++++++++++++++++++++++++-------------------
net/ncsi/ncsi-rsp.c | 2 +-
3 files changed, 35 insertions(+), 23 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 26e9295..13290a7 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -187,9 +187,15 @@ struct ncsi_channel {
struct ncsi_channel_mode modes[NCSI_MODE_MAX];
struct ncsi_channel_filter *filters[NCSI_FILTER_MAX];
struct ncsi_channel_stats stats;
- struct timer_list timer; /* Link monitor timer */
- bool enabled; /* Timer is enabled */
- unsigned int timeout; /* Times of timeout */
+ struct {
+ struct timer_list timer;
+ bool enabled;
+ unsigned int state;
+#define NCSI_CHANNEL_MONITOR_START 0
+#define NCSI_CHANNEL_MONITOR_RETRY 1
+#define NCSI_CHANNEL_MONITOR_WAIT 2
+#define NCSI_CHANNEL_MONITOR_WAIT_MAX 5
+ } monitor;
struct list_head node;
struct list_head link;
};
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index adf5401..4742c7c 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -172,15 +172,15 @@ static void ncsi_channel_monitor(unsigned long data)
struct ncsi_dev_priv *ndp = np->ndp;
struct ncsi_cmd_arg nca;
bool enabled, chained;
- unsigned int timeout;
+ unsigned int monitor_state;
unsigned long flags;
int state, ret;
spin_lock_irqsave(&nc->lock, flags);
state = nc->state;
chained = !list_empty(&nc->link);
- timeout = nc->timeout;
- enabled = nc->enabled;
+ enabled = nc->monitor.enabled;
+ monitor_state = nc->monitor.state;
spin_unlock_irqrestore(&nc->lock, flags);
if (!enabled || chained)
@@ -189,7 +189,9 @@ static void ncsi_channel_monitor(unsigned long data)
state != NCSI_CHANNEL_ACTIVE)
return;
- if (!(timeout % 2)) {
+ switch (monitor_state) {
+ case NCSI_CHANNEL_MONITOR_START:
+ case NCSI_CHANNEL_MONITOR_RETRY:
nca.ndp = ndp;
nca.package = np->id;
nca.channel = nc->id;
@@ -201,12 +203,16 @@ static void ncsi_channel_monitor(unsigned long data)
ret);
return;
}
- }
- if (timeout + 1 >= 3) {
+ break;
+ case NCSI_CHANNEL_MONITOR_WAIT ... NCSI_CHANNEL_MONITOR_WAIT_MAX:
+ break;
+ default:
if (!(ndp->flags & NCSI_DEV_HWA) &&
- state == NCSI_CHANNEL_ACTIVE)
+ state == NCSI_CHANNEL_ACTIVE) {
ncsi_report_link(ndp, true);
+ ndp->flags |= NCSI_DEV_RESHUFFLE;
+ }
spin_lock_irqsave(&nc->lock, flags);
nc->state = NCSI_CHANNEL_INVISIBLE;
@@ -221,10 +227,9 @@ static void ncsi_channel_monitor(unsigned long data)
}
spin_lock_irqsave(&nc->lock, flags);
- nc->timeout = timeout + 1;
- nc->enabled = true;
+ nc->monitor.state++;
spin_unlock_irqrestore(&nc->lock, flags);
- mod_timer(&nc->timer, jiffies + HZ * (1 << (nc->timeout / 2)));
+ mod_timer(&nc->monitor.timer, jiffies + HZ);
}
void ncsi_start_channel_monitor(struct ncsi_channel *nc)
@@ -232,12 +237,12 @@ void ncsi_start_channel_monitor(struct ncsi_channel *nc)
unsigned long flags;
spin_lock_irqsave(&nc->lock, flags);
- WARN_ON_ONCE(nc->enabled);
- nc->timeout = 0;
- nc->enabled = true;
+ WARN_ON_ONCE(nc->monitor.enabled);
+ nc->monitor.enabled = true;
+ nc->monitor.state = NCSI_CHANNEL_MONITOR_START;
spin_unlock_irqrestore(&nc->lock, flags);
- mod_timer(&nc->timer, jiffies + HZ * (1 << (nc->timeout / 2)));
+ mod_timer(&nc->monitor.timer, jiffies + HZ);
}
void ncsi_stop_channel_monitor(struct ncsi_channel *nc)
@@ -245,14 +250,14 @@ void ncsi_stop_channel_monitor(struct ncsi_channel *nc)
unsigned long flags;
spin_lock_irqsave(&nc->lock, flags);
- if (!nc->enabled) {
+ if (!nc->monitor.enabled) {
spin_unlock_irqrestore(&nc->lock, flags);
return;
}
- nc->enabled = false;
+ nc->monitor.enabled = false;
spin_unlock_irqrestore(&nc->lock, flags);
- del_timer_sync(&nc->timer);
+ del_timer_sync(&nc->monitor.timer);
}
struct ncsi_channel *ncsi_find_channel(struct ncsi_package *np,
@@ -281,8 +286,9 @@ struct ncsi_channel *ncsi_add_channel(struct ncsi_package *np, unsigned char id)
nc->id = id;
nc->package = np;
nc->state = NCSI_CHANNEL_INACTIVE;
- nc->enabled = false;
- setup_timer(&nc->timer, ncsi_channel_monitor, (unsigned long)nc);
+ nc->monitor.enabled = false;
+ setup_timer(&nc->monitor.timer,
+ ncsi_channel_monitor, (unsigned long)nc);
spin_lock_init(&nc->lock);
INIT_LIST_HEAD(&nc->link);
for (index = 0; index < NCSI_CAP_MAX; index++)
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 86cdaeb..087db77 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -322,7 +322,7 @@ static int ncsi_rsp_handler_gls(struct ncsi_request *nr)
/* Reset the channel monitor if it has been enabled */
spin_lock_irqsave(&nc->lock, flags);
- nc->timeout = 0;
+ nc->monitor.state = NCSI_CHANNEL_MONITOR_START;
spin_unlock_irqrestore(&nc->lock, flags);
return 0;
--
2.1.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 net-next 7/8] net/ncsi: Introduce ncsi_stop_dev()
2016-10-04 0:25 [PATCH v2 net-next 0/8] net/ncsi: NCSI Improvment and bug fixes Gavin Shan
` (5 preceding siblings ...)
2016-10-04 0:25 ` [PATCH v2 net-next 6/8] net/ncsi: Rework the channel monitoring Gavin Shan
@ 2016-10-04 0:25 ` Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 8/8] net/faraday: Stop NCSI device on shutdown Gavin Shan
2016-10-04 6:13 ` [PATCH v2 net-next 0/8] net/ncsi: NCSI Improvment and bug fixes David Miller
8 siblings, 0 replies; 10+ messages in thread
From: Gavin Shan @ 2016-10-04 0:25 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
This introduces ncsi_stop_dev(), as counterpart to ncsi_start_dev(),
to stop the NCSI device so that it can be reenabled in future. This
API should be called when the network device driver is going to
shutdown the device. There are 3 things done in the function: Stop
the channel monitoring; Reset channels to inactive state; Report
NCSI link down.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
include/net/ncsi.h | 5 +++++
net/ncsi/ncsi-manage.c | 37 ++++++++++++++++++++++++-------------
2 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/include/net/ncsi.h b/include/net/ncsi.h
index 1dbf42f..68680ba 100644
--- a/include/net/ncsi.h
+++ b/include/net/ncsi.h
@@ -31,6 +31,7 @@ struct ncsi_dev {
struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
void (*notifier)(struct ncsi_dev *nd));
int ncsi_start_dev(struct ncsi_dev *nd);
+void ncsi_stop_dev(struct ncsi_dev *nd);
void ncsi_unregister_dev(struct ncsi_dev *nd);
#else /* !CONFIG_NET_NCSI */
static inline struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
@@ -44,6 +45,10 @@ static inline int ncsi_start_dev(struct ncsi_dev *nd)
return -ENOTTY;
}
+static void ncsi_stop_dev(struct ncsi_dev *nd)
+{
+}
+
static inline void ncsi_unregister_dev(struct ncsi_dev *nd)
{
}
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 4742c7c..5e509e5 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1187,11 +1187,7 @@ EXPORT_SYMBOL_GPL(ncsi_register_dev);
int ncsi_start_dev(struct ncsi_dev *nd)
{
struct ncsi_dev_priv *ndp = TO_NCSI_DEV_PRIV(nd);
- struct ncsi_package *np;
- struct ncsi_channel *nc;
- unsigned long flags;
- bool chained;
- int old_state, ret;
+ int ret;
if (nd->state != ncsi_dev_state_registered &&
nd->state != ncsi_dev_state_functional)
@@ -1203,9 +1199,29 @@ int ncsi_start_dev(struct ncsi_dev *nd)
return 0;
}
- /* Reset channel's state and start over */
+ if (ndp->flags & NCSI_DEV_HWA)
+ ret = ncsi_enable_hwa(ndp);
+ else
+ ret = ncsi_choose_active_channel(ndp);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ncsi_start_dev);
+
+void ncsi_stop_dev(struct ncsi_dev *nd)
+{
+ struct ncsi_dev_priv *ndp = TO_NCSI_DEV_PRIV(nd);
+ struct ncsi_package *np;
+ struct ncsi_channel *nc;
+ bool chained;
+ int old_state;
+ unsigned long flags;
+
+ /* Stop the channel monitor and reset channel's state */
NCSI_FOR_EACH_PACKAGE(ndp, np) {
NCSI_FOR_EACH_CHANNEL(np, nc) {
+ ncsi_stop_channel_monitor(nc);
+
spin_lock_irqsave(&nc->lock, flags);
chained = !list_empty(&nc->link);
old_state = nc->state;
@@ -1217,14 +1233,9 @@ int ncsi_start_dev(struct ncsi_dev *nd)
}
}
- if (ndp->flags & NCSI_DEV_HWA)
- ret = ncsi_enable_hwa(ndp);
- else
- ret = ncsi_choose_active_channel(ndp);
-
- return ret;
+ ncsi_report_link(ndp, true);
}
-EXPORT_SYMBOL_GPL(ncsi_start_dev);
+EXPORT_SYMBOL_GPL(ncsi_stop_dev);
void ncsi_unregister_dev(struct ncsi_dev *nd)
{
--
2.1.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 net-next 8/8] net/faraday: Stop NCSI device on shutdown
2016-10-04 0:25 [PATCH v2 net-next 0/8] net/ncsi: NCSI Improvment and bug fixes Gavin Shan
` (6 preceding siblings ...)
2016-10-04 0:25 ` [PATCH v2 net-next 7/8] net/ncsi: Introduce ncsi_stop_dev() Gavin Shan
@ 2016-10-04 0:25 ` Gavin Shan
2016-10-04 6:13 ` [PATCH v2 net-next 0/8] net/ncsi: NCSI Improvment and bug fixes David Miller
8 siblings, 0 replies; 10+ messages in thread
From: Gavin Shan @ 2016-10-04 0:25 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
This stops NCSI device when closing the network device so that the
NCSI device can be reenabled later.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
drivers/net/ethernet/faraday/ftgmac100.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 90f9c54..2625872 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1190,6 +1190,8 @@ static int ftgmac100_stop(struct net_device *netdev)
napi_disable(&priv->napi);
if (netdev->phydev)
phy_stop(netdev->phydev);
+ else if (priv->use_ncsi)
+ ncsi_stop_dev(priv->ndev);
ftgmac100_stop_hw(priv);
free_irq(priv->irq, netdev);
--
2.1.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 net-next 0/8] net/ncsi: NCSI Improvment and bug fixes
2016-10-04 0:25 [PATCH v2 net-next 0/8] net/ncsi: NCSI Improvment and bug fixes Gavin Shan
` (7 preceding siblings ...)
2016-10-04 0:25 ` [PATCH v2 net-next 8/8] net/faraday: Stop NCSI device on shutdown Gavin Shan
@ 2016-10-04 6:13 ` David Miller
8 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2016-10-04 6:13 UTC (permalink / raw)
To: gwshan; +Cc: netdev, joel, yuvali, benh
From: Gavin Shan <gwshan@linux.vnet.ibm.com>
Date: Tue, 4 Oct 2016 11:25:46 +1100
> This series of patches improves NCSI stack according to the comments
> I received after the NCSI code was merged to 4.8.rc1:
>
> * PATCH[1/8] fixes the build warning caused by xchg() with ia64-linux-gcc.
> The atomic operations are removed. The NCSI's lock should be taken when
> reading or updating its state and chained state.
> * Channel ID (0x1f) is the reserved one and it cannot be valid channel ID.
> So we needn't try to probe channel whose ID is 0x1f. PATCH[2/8] and
> PATCH[3/8] are addressing this issue.
> * The request IDs are assigned in round-robin fashion, but it's broken.
> PATCH[4/8] make it work.
> * PATCH[5/8] and PATCH[6/8] reworks the channel monitoring to improve the
> code readability and its robustness.
> * PATCH[7/8] and PATCH[8/8] introduces ncsi_stop_dev() so that the network
> device can be closed and opened afterwards. No error will be seen.
>
> Changelog
> =========
> v2:
> * The NCSI's lock is taken when reading or updating its state as the
> {READ,WRITE}_ONCE() isn't reliable.
Series applied, thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-10-04 6:13 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-04 0:25 [PATCH v2 net-next 0/8] net/ncsi: NCSI Improvment and bug fixes Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 1/8] net/ncsi: Avoid unused-value build warning from ia64-linux-gcc Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 2/8] net/ncsi: Introduce NCSI_RESERVED_CHANNEL Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 3/8] net/ncsi: Don't probe on the reserved channel ID (0x1f) Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 4/8] net/ncsi: Rework request index allocation Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 5/8] net/ncsi: Allow to extend NCSI request properties Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 6/8] net/ncsi: Rework the channel monitoring Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 7/8] net/ncsi: Introduce ncsi_stop_dev() Gavin Shan
2016-10-04 0:25 ` [PATCH v2 net-next 8/8] net/faraday: Stop NCSI device on shutdown Gavin Shan
2016-10-04 6:13 ` [PATCH v2 net-next 0/8] net/ncsi: NCSI Improvment and bug fixes 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).