* [PATCH v2 4.7 1/8] net/ncsi: Avoid unused-value build warning from ia64-linux-gcc
2016-09-29 1:38 [PATCH v2 4.7 0/8] NCSI Fixes Gavin Shan
@ 2016-09-29 1:38 ` Gavin Shan
2016-09-29 1:38 ` [PATCH v2 4.7 2/8] net/ncsi: Introduce NCSI_RESERVED_CHANNEL Gavin Shan
` (7 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Gavin Shan @ 2016-09-29 1:38 UTC (permalink / raw)
To: openbmc; +Cc: joel, 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 replaces the atomic access to NCSI channel's state with READ_ONCE()
and WRITE_ONCE() to avoid the above build warning. We needn't hold the
channel's lock when updating its state as well. No logical changes
introduced.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
net/ncsi/ncsi-aen.c | 19 +++++++++++++------
net/ncsi/ncsi-manage.c | 37 +++++++++++++++++++++----------------
net/ncsi/ncsi-rsp.c | 13 +++----------
3 files changed, 37 insertions(+), 32 deletions(-)
diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index d463468..abc5473 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -53,6 +53,7 @@ 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;
+ int old_state;
unsigned long old_data;
unsigned long flags;
@@ -70,12 +71,14 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp,
if (!((old_data ^ ncm->data[2]) & 0x1) ||
!list_empty(&nc->link))
return 0;
- if (!(nc->state == NCSI_CHANNEL_INACTIVE && (ncm->data[2] & 0x1)) &&
- !(nc->state == NCSI_CHANNEL_ACTIVE && !(ncm->data[2] & 0x1)))
+
+ old_state = READ_ONCE(nc->state);
+ if (!(old_state == NCSI_CHANNEL_INACTIVE && (ncm->data[2] & 0x1)) &&
+ !(old_state == NCSI_CHANNEL_ACTIVE && !(ncm->data[2] & 0x1)))
return 0;
if (!(ndp->flags & NCSI_DEV_HWA) &&
- nc->state == NCSI_CHANNEL_ACTIVE)
+ old_state == NCSI_CHANNEL_ACTIVE)
ndp->flags |= NCSI_DEV_RESHUFFLE;
ncsi_stop_channel_monitor(nc);
@@ -90,6 +93,7 @@ static int ncsi_aen_handler_cr(struct ncsi_dev_priv *ndp,
struct ncsi_aen_pkt_hdr *h)
{
struct ncsi_channel *nc;
+ int old_state;
unsigned long flags;
/* Find the NCSI channel */
@@ -97,13 +101,14 @@ static int ncsi_aen_handler_cr(struct ncsi_dev_priv *ndp,
if (!nc)
return -ENODEV;
+ old_state = READ_ONCE(nc->state);
if (!list_empty(&nc->link) ||
- nc->state != NCSI_CHANNEL_ACTIVE)
+ old_state != NCSI_CHANNEL_ACTIVE)
return 0;
ncsi_stop_channel_monitor(nc);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
spin_lock_irqsave(&ndp->lock, flags);
- xchg(&nc->state, NCSI_CHANNEL_INACTIVE);
list_add_tail_rcu(&nc->link, &ndp->channel_queue);
spin_unlock_irqrestore(&ndp->lock, flags);
@@ -116,6 +121,7 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
struct ncsi_channel *nc;
struct ncsi_channel_mode *ncm;
struct ncsi_aen_hncdsc_pkt *hncdsc;
+ int old_state;
unsigned long flags;
/* Find the NCSI channel */
@@ -127,8 +133,9 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
ncm = &nc->modes[NCSI_MODE_LINK];
hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
ncm->data[3] = ntohl(hncdsc->status);
+ old_state = READ_ONCE(nc->state);
if (!list_empty(&nc->link) ||
- nc->state != NCSI_CHANNEL_ACTIVE ||
+ old_state != NCSI_CHANNEL_ACTIVE ||
(ncm->data[3] & 0x1))
return 0;
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index ef017b8..fc0ae54 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -143,7 +143,7 @@ static void ncsi_report_link(struct ncsi_dev_priv *ndp, bool force_down)
NCSI_FOR_EACH_PACKAGE(ndp, np) {
NCSI_FOR_EACH_CHANNEL(np, nc) {
if (!list_empty(&nc->link) ||
- nc->state != NCSI_CHANNEL_ACTIVE)
+ READ_ONCE(nc->state) != NCSI_CHANNEL_ACTIVE)
continue;
if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) {
@@ -166,7 +166,7 @@ static void ncsi_channel_monitor(unsigned long data)
bool enabled;
unsigned int timeout;
unsigned long flags;
- int ret;
+ int old_state, ret;
spin_lock_irqsave(&nc->lock, flags);
timeout = nc->timeout;
@@ -175,8 +175,10 @@ static void ncsi_channel_monitor(unsigned long data)
if (!enabled || !list_empty(&nc->link))
return;
- if (nc->state != NCSI_CHANNEL_INACTIVE &&
- nc->state != NCSI_CHANNEL_ACTIVE)
+
+ old_state = READ_ONCE(nc->state);
+ if (old_state != NCSI_CHANNEL_INACTIVE &&
+ old_state != NCSI_CHANNEL_ACTIVE)
return;
if (!(timeout % 2)) {
@@ -195,11 +197,11 @@ static void ncsi_channel_monitor(unsigned long data)
if (timeout + 1 >= 3) {
if (!(ndp->flags & NCSI_DEV_HWA) &&
- nc->state == NCSI_CHANNEL_ACTIVE)
+ old_state == NCSI_CHANNEL_ACTIVE)
ncsi_report_link(ndp, true);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
spin_lock_irqsave(&ndp->lock, flags);
- xchg(&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);
@@ -266,7 +268,7 @@ struct ncsi_channel *ncsi_add_channel(struct ncsi_package *np, unsigned char id)
nc->id = id;
nc->package = np;
- nc->state = NCSI_CHANNEL_INACTIVE;
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
nc->enabled = false;
setup_timer(&nc->timer, ncsi_channel_monitor, (unsigned long)nc);
spin_lock_init(&nc->lock);
@@ -309,7 +311,7 @@ static void ncsi_remove_channel(struct ncsi_channel *nc)
kfree(ncf);
}
- nc->state = NCSI_CHANNEL_INACTIVE;
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
spin_unlock_irqrestore(&nc->lock, flags);
ncsi_stop_channel_monitor(nc);
@@ -556,7 +558,7 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
break;
case ncsi_dev_state_suspend_done:
- xchg(&nc->state, NCSI_CHANNEL_INACTIVE);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
ncsi_process_next_channel(ndp);
break;
@@ -676,9 +678,9 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
break;
case ncsi_dev_state_config_done:
if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1)
- xchg(&nc->state, NCSI_CHANNEL_ACTIVE);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_ACTIVE);
else
- xchg(&nc->state, NCSI_CHANNEL_INACTIVE);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
ncsi_start_channel_monitor(nc);
ncsi_process_next_channel(ndp);
@@ -708,7 +710,7 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
NCSI_FOR_EACH_PACKAGE(ndp, np) {
NCSI_FOR_EACH_CHANNEL(np, nc) {
if (!list_empty(&nc->link) ||
- nc->state != NCSI_CHANNEL_INACTIVE)
+ READ_ONCE(nc->state) != NCSI_CHANNEL_INACTIVE)
continue;
if (!found)
@@ -770,7 +772,8 @@ static int ncsi_enable_hwa(struct ncsi_dev_priv *ndp)
spin_lock_irqsave(&ndp->lock, flags);
NCSI_FOR_EACH_PACKAGE(ndp, np) {
NCSI_FOR_EACH_CHANNEL(np, nc) {
- WARN_ON_ONCE(nc->state != NCSI_CHANNEL_INACTIVE ||
+ WARN_ON_ONCE(READ_ONCE(nc->state) !=
+ NCSI_CHANNEL_INACTIVE ||
!list_empty(&nc->link));
ncsi_stop_channel_monitor(nc);
list_add_tail_rcu(&nc->link, &ndp->channel_queue);
@@ -987,7 +990,8 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
goto out;
}
- old_state = xchg(&nc->state, NCSI_CHANNEL_INVISIBLE);
+ old_state = READ_ONCE(nc->state);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INVISIBLE);
list_del_init(&nc->link);
spin_unlock_irqrestore(&ndp->lock, flags);
@@ -1006,7 +1010,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);
+ READ_ONCE(nc->state), nc->package->id, nc->id);
ncsi_report_link(ndp, false);
return -EINVAL;
}
@@ -1166,7 +1170,8 @@ 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);
+ old_state = READ_ONCE(nc->state);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
WARN_ON_ONCE(!list_empty(&nc->link) ||
old_state == NCSI_CHANNEL_INVISIBLE);
}
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index af84389..9ee25ab 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -111,7 +111,6 @@ static int ncsi_rsp_handler_dp(struct ncsi_request *nr)
struct ncsi_dev_priv *ndp = nr->ndp;
struct ncsi_package *np;
struct ncsi_channel *nc;
- unsigned long flags;
/* Find the package */
rsp = (struct ncsi_rsp_pkt *)skb_network_header(nr->rsp);
@@ -121,11 +120,8 @@ static int ncsi_rsp_handler_dp(struct ncsi_request *nr)
return -ENODEV;
/* Change state of all channels attached to the package */
- NCSI_FOR_EACH_CHANNEL(np, nc) {
- spin_lock_irqsave(&nc->lock, flags);
- nc->state = NCSI_CHANNEL_INACTIVE;
- spin_unlock_irqrestore(&nc->lock, flags);
- }
+ NCSI_FOR_EACH_CHANNEL(np, nc)
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
return 0;
}
@@ -184,7 +180,6 @@ static int ncsi_rsp_handler_rc(struct ncsi_request *nr)
struct ncsi_rsp_pkt *rsp;
struct ncsi_dev_priv *ndp = nr->ndp;
struct ncsi_channel *nc;
- unsigned long flags;
/* Find the package and channel */
rsp = (struct ncsi_rsp_pkt *)skb_network_header(nr->rsp);
@@ -194,9 +189,7 @@ static int ncsi_rsp_handler_rc(struct ncsi_request *nr)
return -ENODEV;
/* Update state for the specified channel */
- spin_lock_irqsave(&nc->lock, flags);
- nc->state = NCSI_CHANNEL_INACTIVE;
- spin_unlock_irqrestore(&nc->lock, flags);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
return 0;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 4.7 2/8] net/ncsi: Introduce NCSI_RESERVED_CHANNEL
2016-09-29 1:38 [PATCH v2 4.7 0/8] NCSI Fixes Gavin Shan
2016-09-29 1:38 ` [PATCH v2 4.7 1/8] net/ncsi: Avoid unused-value build warning from ia64-linux-gcc Gavin Shan
@ 2016-09-29 1:38 ` Gavin Shan
2016-09-29 1:38 ` [PATCH v2 4.7 3/8] net/ncsi: Don't probe on the reserved channel ID (0x1f) Gavin Shan
` (6 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Gavin Shan @ 2016-09-29 1:38 UTC (permalink / raw)
To: openbmc; +Cc: joel, 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 fc0ae54..73883ed 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -529,7 +529,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
@@ -546,7 +546,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;
}
@@ -592,7 +592,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;
@@ -810,7 +810,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);
@@ -826,7 +826,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);
@@ -879,7 +879,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;
@@ -936,7 +936,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] 16+ messages in thread* [PATCH v2 4.7 3/8] net/ncsi: Don't probe on the reserved channel ID (0x1f)
2016-09-29 1:38 [PATCH v2 4.7 0/8] NCSI Fixes Gavin Shan
2016-09-29 1:38 ` [PATCH v2 4.7 1/8] net/ncsi: Avoid unused-value build warning from ia64-linux-gcc Gavin Shan
2016-09-29 1:38 ` [PATCH v2 4.7 2/8] net/ncsi: Introduce NCSI_RESERVED_CHANNEL Gavin Shan
@ 2016-09-29 1:38 ` Gavin Shan
2016-09-29 1:38 ` [PATCH v2 4.7 4/8] net/ncsi: Rework request index allocation Gavin Shan
` (5 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Gavin Shan @ 2016-09-29 1:38 UTC (permalink / raw)
To: openbmc; +Cc: joel, 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 73883ed..3eeb915 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -887,12 +887,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] 16+ messages in thread* [PATCH v2 4.7 4/8] net/ncsi: Rework request index allocation
2016-09-29 1:38 [PATCH v2 4.7 0/8] NCSI Fixes Gavin Shan
` (2 preceding siblings ...)
2016-09-29 1:38 ` [PATCH v2 4.7 3/8] net/ncsi: Don't probe on the reserved channel ID (0x1f) Gavin Shan
@ 2016-09-29 1:38 ` Gavin Shan
2016-09-29 1:38 ` [PATCH v2 4.7 5/8] net/ncsi: Allow to extend NCSI request properties Gavin Shan
` (4 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Gavin Shan @ 2016-09-29 1:38 UTC (permalink / raw)
To: openbmc; +Cc: joel, 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 3eeb915..64aab38 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -415,30 +415,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;
}
@@ -1122,7 +1123,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] 16+ messages in thread* [PATCH v2 4.7 5/8] net/ncsi: Allow to extend NCSI request properties
2016-09-29 1:38 [PATCH v2 4.7 0/8] NCSI Fixes Gavin Shan
` (3 preceding siblings ...)
2016-09-29 1:38 ` [PATCH v2 4.7 4/8] net/ncsi: Rework request index allocation Gavin Shan
@ 2016-09-29 1:38 ` Gavin Shan
2016-09-29 1:38 ` [PATCH v2 4.7 6/8] net/ncsi: Rework the channel monitoring Gavin Shan
` (3 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Gavin Shan @ 2016-09-29 1:38 UTC (permalink / raw)
To: openbmc; +Cc: joel, 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 64aab38..41d6af9 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -186,7 +186,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",
@@ -407,7 +407,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);
@@ -421,7 +422,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;
}
@@ -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;
}
@@ -461,7 +462,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)
@@ -514,7 +515,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;
@@ -580,7 +581,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:
@@ -801,7 +802,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;
@@ -1075,7 +1076,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 9ee25ab..5143490 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -310,7 +310,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] 16+ messages in thread* [PATCH v2 4.7 6/8] net/ncsi: Rework the channel monitoring
2016-09-29 1:38 [PATCH v2 4.7 0/8] NCSI Fixes Gavin Shan
` (4 preceding siblings ...)
2016-09-29 1:38 ` [PATCH v2 4.7 5/8] net/ncsi: Allow to extend NCSI request properties Gavin Shan
@ 2016-09-29 1:38 ` Gavin Shan
2016-09-29 5:00 ` Joel Stanley
2016-09-29 1:38 ` [PATCH v2 4.7 7/8] net/ncsi: Introduce ncsi_stop_dev() Gavin Shan
` (2 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Gavin Shan @ 2016-09-29 1:38 UTC (permalink / raw)
To: openbmc; +Cc: joel, 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>
---
net/ncsi/internal.h | 12 +++++++++---
net/ncsi/ncsi-manage.c | 46 ++++++++++++++++++++++++++--------------------
net/ncsi/ncsi-rsp.c | 2 +-
3 files changed, 36 insertions(+), 24 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 41d6af9..1b797c9 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -164,13 +164,13 @@ static void ncsi_channel_monitor(unsigned long data)
struct ncsi_dev_priv *ndp = np->ndp;
struct ncsi_cmd_arg nca;
bool enabled;
- unsigned int timeout;
+ unsigned int monitor_state;
unsigned long flags;
int old_state, ret;
spin_lock_irqsave(&nc->lock, flags);
- timeout = nc->timeout;
- enabled = nc->enabled;
+ enabled = nc->monitor.enabled;
+ monitor_state = nc->monitor.state;
spin_unlock_irqrestore(&nc->lock, flags);
if (!enabled || !list_empty(&nc->link))
@@ -181,7 +181,9 @@ static void ncsi_channel_monitor(unsigned long data)
old_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;
@@ -193,14 +195,18 @@ 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) &&
- old_state == NCSI_CHANNEL_ACTIVE)
+ old_state == NCSI_CHANNEL_ACTIVE) {
ncsi_report_link(ndp, true);
+ ndp->flags |= NCSI_DEV_RESHUFFLE;
+ }
- WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_ACTIVE);
spin_lock_irqsave(&ndp->lock, flags);
list_add_tail_rcu(&nc->link, &ndp->channel_queue);
spin_unlock_irqrestore(&ndp->lock, flags);
@@ -209,10 +215,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)
@@ -220,12 +225,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)
@@ -233,14 +238,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,
@@ -269,8 +274,9 @@ struct ncsi_channel *ncsi_add_channel(struct ncsi_package *np, unsigned char id)
nc->id = id;
nc->package = np;
WRITE_ONCE(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 5143490..1618eda 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -315,7 +315,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] 16+ messages in thread* Re: [PATCH v2 4.7 6/8] net/ncsi: Rework the channel monitoring
2016-09-29 1:38 ` [PATCH v2 4.7 6/8] net/ncsi: Rework the channel monitoring Gavin Shan
@ 2016-09-29 5:00 ` Joel Stanley
0 siblings, 0 replies; 16+ messages in thread
From: Joel Stanley @ 2016-09-29 5:00 UTC (permalink / raw)
To: Gavin Shan; +Cc: OpenBMC Maillist
On Thu, Sep 29, 2016 at 11:08 AM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> 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 | 46 ++++++++++++++++++++++++++--------------------
> net/ncsi/ncsi-rsp.c | 2 +-
> 3 files changed, 36 insertions(+), 24 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 41d6af9..1b797c9 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -164,13 +164,13 @@ static void ncsi_channel_monitor(unsigned long data)
> struct ncsi_dev_priv *ndp = np->ndp;
> struct ncsi_cmd_arg nca;
> bool enabled;
> - unsigned int timeout;
> + unsigned int monitor_state;
> unsigned long flags;
> int old_state, ret;
>
> spin_lock_irqsave(&nc->lock, flags);
> - timeout = nc->timeout;
> - enabled = nc->enabled;
> + enabled = nc->monitor.enabled;
> + monitor_state = nc->monitor.state;
> spin_unlock_irqrestore(&nc->lock, flags);
>
> if (!enabled || !list_empty(&nc->link))
> @@ -181,7 +181,9 @@ static void ncsi_channel_monitor(unsigned long data)
> old_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;
> @@ -193,14 +195,18 @@ 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) &&
> - old_state == NCSI_CHANNEL_ACTIVE)
> + old_state == NCSI_CHANNEL_ACTIVE) {
> ncsi_report_link(ndp, true);
> + ndp->flags |= NCSI_DEV_RESHUFFLE;
> + }
>
> - WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
> + WRITE_ONCE(nc->state, NCSI_CHANNEL_ACTIVE);
> spin_lock_irqsave(&ndp->lock, flags);
> list_add_tail_rcu(&nc->link, &ndp->channel_queue);
> spin_unlock_irqrestore(&ndp->lock, flags);
> @@ -209,10 +215,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)
> @@ -220,12 +225,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)
> @@ -233,14 +238,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,
> @@ -269,8 +274,9 @@ struct ncsi_channel *ncsi_add_channel(struct ncsi_package *np, unsigned char id)
> nc->id = id;
> nc->package = np;
> WRITE_ONCE(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 5143490..1618eda 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -315,7 +315,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 [flat|nested] 16+ messages in thread
* [PATCH v2 4.7 7/8] net/ncsi: Introduce ncsi_stop_dev()
2016-09-29 1:38 [PATCH v2 4.7 0/8] NCSI Fixes Gavin Shan
` (5 preceding siblings ...)
2016-09-29 1:38 ` [PATCH v2 4.7 6/8] net/ncsi: Rework the channel monitoring Gavin Shan
@ 2016-09-29 1:38 ` Gavin Shan
2016-09-29 4:51 ` Joel Stanley
2016-09-29 1:38 ` [PATCH v2 4.7 8/8] net/faraday: Stop NCSI device on shutdown Gavin Shan
2016-10-04 0:36 ` [PATCH v2 4.7 0/8] NCSI Fixes Gavin Shan
8 siblings, 1 reply; 16+ messages in thread
From: Gavin Shan @ 2016-09-29 1:38 UTC (permalink / raw)
To: openbmc; +Cc: joel, 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>
---
include/net/ncsi.h | 5 +++++
net/ncsi/ncsi-manage.c | 33 ++++++++++++++++++++++-----------
2 files changed, 27 insertions(+), 11 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 1b797c9..6a96873 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1161,9 +1161,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;
- int old_state, ret;
+ int ret;
if (nd->state != ncsi_dev_state_registered &&
nd->state != ncsi_dev_state_functional)
@@ -1175,9 +1173,27 @@ 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;
+ int old_state;
+
+ /* 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);
+
old_state = READ_ONCE(nc->state);
WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
WARN_ON_ONCE(!list_empty(&nc->link) ||
@@ -1185,14 +1201,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] 16+ messages in thread* Re: [PATCH v2 4.7 7/8] net/ncsi: Introduce ncsi_stop_dev()
2016-09-29 1:38 ` [PATCH v2 4.7 7/8] net/ncsi: Introduce ncsi_stop_dev() Gavin Shan
@ 2016-09-29 4:51 ` Joel Stanley
0 siblings, 0 replies; 16+ messages in thread
From: Joel Stanley @ 2016-09-29 4:51 UTC (permalink / raw)
To: Gavin Shan; +Cc: OpenBMC Maillist
On Thu, Sep 29, 2016 at 11:08 AM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> 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 | 33 ++++++++++++++++++++++-----------
> 2 files changed, 27 insertions(+), 11 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 1b797c9..6a96873 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -1161,9 +1161,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;
> - int old_state, ret;
> + int ret;
>
> if (nd->state != ncsi_dev_state_registered &&
> nd->state != ncsi_dev_state_functional)
> @@ -1175,9 +1173,27 @@ 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;
> + int old_state;
> +
> + /* 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);
> +
> old_state = READ_ONCE(nc->state);
> WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
> WARN_ON_ONCE(!list_empty(&nc->link) ||
> @@ -1185,14 +1201,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 [flat|nested] 16+ messages in thread
* [PATCH v2 4.7 8/8] net/faraday: Stop NCSI device on shutdown
2016-09-29 1:38 [PATCH v2 4.7 0/8] NCSI Fixes Gavin Shan
` (6 preceding siblings ...)
2016-09-29 1:38 ` [PATCH v2 4.7 7/8] net/ncsi: Introduce ncsi_stop_dev() Gavin Shan
@ 2016-09-29 1:38 ` Gavin Shan
2016-09-29 4:50 ` Joel Stanley
2016-10-04 0:36 ` [PATCH v2 4.7 0/8] NCSI Fixes Gavin Shan
8 siblings, 1 reply; 16+ messages in thread
From: Gavin Shan @ 2016-09-29 1:38 UTC (permalink / raw)
To: openbmc; +Cc: joel, 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>
---
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 134b9c5..f40fa92 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1189,6 +1189,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] 16+ messages in thread* Re: [PATCH v2 4.7 8/8] net/faraday: Stop NCSI device on shutdown
2016-09-29 1:38 ` [PATCH v2 4.7 8/8] net/faraday: Stop NCSI device on shutdown Gavin Shan
@ 2016-09-29 4:50 ` Joel Stanley
0 siblings, 0 replies; 16+ messages in thread
From: Joel Stanley @ 2016-09-29 4:50 UTC (permalink / raw)
To: Gavin Shan; +Cc: OpenBMC Maillist
On Thu, Sep 29, 2016 at 11:08 AM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> 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 134b9c5..f40fa92 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1189,6 +1189,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 [flat|nested] 16+ messages in thread
* Re: [PATCH v2 4.7 0/8] NCSI Fixes
2016-09-29 1:38 [PATCH v2 4.7 0/8] NCSI Fixes Gavin Shan
` (7 preceding siblings ...)
2016-09-29 1:38 ` [PATCH v2 4.7 8/8] net/faraday: Stop NCSI device on shutdown Gavin Shan
@ 2016-10-04 0:36 ` Gavin Shan
2016-10-04 5:28 ` Joel Stanley
8 siblings, 1 reply; 16+ messages in thread
From: Gavin Shan @ 2016-10-04 0:36 UTC (permalink / raw)
To: Gavin Shan; +Cc: openbmc, joel
On Thu, Sep 29, 2016 at 11:38:43AM +1000, Gavin Shan wrote:
>
Joel, please hold merging this series because I try pushing this series
upstream firstly. It's always good to keep code sync'ed between 4.7.x
with upstream regarding NCSI stack. I will post a updated version if
it can be upstream in time. Thanks again for your review and comments.
Thanks,
Gavin
>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 | 19 ++--
> net/ncsi/ncsi-cmd.c | 2 +-
> net/ncsi/ncsi-manage.c | 166 ++++++++++++++++++-------------
> net/ncsi/ncsi-rsp.c | 17 +---
> 7 files changed, 137 insertions(+), 96 deletions(-)
>
>--
>2.1.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 4.7 0/8] NCSI Fixes
2016-10-04 0:36 ` [PATCH v2 4.7 0/8] NCSI Fixes Gavin Shan
@ 2016-10-04 5:28 ` Joel Stanley
2016-10-04 11:25 ` Gavin Shan
0 siblings, 1 reply; 16+ messages in thread
From: Joel Stanley @ 2016-10-04 5:28 UTC (permalink / raw)
To: Gavin Shan; +Cc: OpenBMC Maillist
On Tue, Oct 4, 2016 at 11:06 AM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> On Thu, Sep 29, 2016 at 11:38:43AM +1000, Gavin Shan wrote:
>>
>
> Joel, please hold merging this series because I try pushing this series
> upstream firstly. It's always good to keep code sync'ed between 4.7.x
> with upstream regarding NCSI stack. I will post a updated version if
> it can be upstream in time. Thanks again for your review and comments.
Okay, will do. Thanks Gavin.
Cheers,
Joel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 4.7 0/8] NCSI Fixes
2016-10-04 5:28 ` Joel Stanley
@ 2016-10-04 11:25 ` Gavin Shan
2016-10-10 7:23 ` Joel Stanley
0 siblings, 1 reply; 16+ messages in thread
From: Gavin Shan @ 2016-10-04 11:25 UTC (permalink / raw)
To: Joel Stanley; +Cc: Gavin Shan, OpenBMC Maillist
On Tue, Oct 04, 2016 at 03:58:58PM +1030, Joel Stanley wrote:
>On Tue, Oct 4, 2016 at 11:06 AM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>> On Thu, Sep 29, 2016 at 11:38:43AM +1000, Gavin Shan wrote:
>>>
>>
>> Joel, please hold merging this series because I try pushing this series
>> upstream firstly. It's always good to keep code sync'ed between 4.7.x
>> with upstream regarding NCSI stack. I will post a updated version if
>> it can be upstream in time. Thanks again for your review and comments.
>
>Okay, will do. Thanks Gavin.
>
Thanks, Joel. v3, merged to linux.net ("next" branch), was just posted.
Please consider merging it to 4.7 branch :)
Thanks,
Gavin
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 4.7 0/8] NCSI Fixes
2016-10-04 11:25 ` Gavin Shan
@ 2016-10-10 7:23 ` Joel Stanley
0 siblings, 0 replies; 16+ messages in thread
From: Joel Stanley @ 2016-10-10 7:23 UTC (permalink / raw)
To: Gavin Shan; +Cc: OpenBMC Maillist
On Tue, Oct 4, 2016 at 10:25 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> On Tue, Oct 04, 2016 at 03:58:58PM +1030, Joel Stanley wrote:
>>On Tue, Oct 4, 2016 at 11:06 AM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>>> On Thu, Sep 29, 2016 at 11:38:43AM +1000, Gavin Shan wrote:
>>>>
>>>
>>> Joel, please hold merging this series because I try pushing this series
>>> upstream firstly. It's always good to keep code sync'ed between 4.7.x
>>> with upstream regarding NCSI stack. I will post a updated version if
>>> it can be upstream in time. Thanks again for your review and comments.
>>
>>Okay, will do. Thanks Gavin.
>>
>
> Thanks, Joel. v3, merged to linux.net ("next" branch), was just posted.
> Please consider merging it to 4.7 branch :)
These should be all part of the 4.7 branch now. Please let me know if
I've missed anything.
Cheers,
Joel
^ permalink raw reply [flat|nested] 16+ messages in thread