| Message ID | 20181023215201.27315-2-sam@mendozajonas.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | net/ncsi: Allow enabling multiple packages & channels | expand |
Hi Samuel,
I love your patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Samuel-Mendoza-Jonas/net-ncsi-Allow-enabling-multiple-packages-channels/20181024-103206
config: i386-randconfig-x014-201842 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: the linux-review/Samuel-Mendoza-Jonas/net-ncsi-Allow-enabling-multiple-packages-channels/20181024-103206 HEAD 54369e39116e3a32a005dd577eba25bd50d15c31 builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
net/ncsi/ncsi-manage.c: In function 'ncsi_probe_channel':
net/ncsi/ncsi-manage.c:1129:5: error: implicit declaration of function 'ncsi_enable_hwa'; did you mean 'ncsi_check_hwa'? [-Werror=implicit-function-declaration]
ncsi_enable_hwa(ndp);
^~~~~~~~~~~~~~~
ncsi_check_hwa
net/ncsi/ncsi-manage.c: In function 'ncsi_start_dev':
>> net/ncsi/ncsi-manage.c:1575:36: error: passing argument 1 of 'ncsi_choose_active_channel' from incompatible pointer type [-Werror=incompatible-pointer-types]
return ncsi_choose_active_channel(nd);
^~
net/ncsi/ncsi-manage.c:939:12: note: expected 'struct ncsi_dev_priv *' but argument is of type 'struct ncsi_dev *'
static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
^~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/ncsi_choose_active_channel +1575 net/ncsi/ncsi-manage.c
1560
1561 int ncsi_start_dev(struct ncsi_dev *nd)
1562 {
1563 struct ncsi_dev_priv *ndp = TO_NCSI_DEV_PRIV(nd);
1564
1565 if (nd->state != ncsi_dev_state_registered &&
1566 nd->state != ncsi_dev_state_functional)
1567 return -ENOTTY;
1568
1569 if (!(ndp->flags & NCSI_DEV_PROBED)) {
1570 nd->state = ncsi_dev_state_probe;
1571 schedule_work(&ndp->work);
1572 return 0;
1573 }
1574
> 1575 return ncsi_choose_active_channel(nd);
1576 }
1577 EXPORT_SYMBOL_GPL(ncsi_start_dev);
1578
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c index 25e483e8278b..65f47a648be3 100644 --- a/net/ncsi/ncsi-aen.c +++ b/net/ncsi/ncsi-aen.c @@ -86,8 +86,7 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp, !(state == NCSI_CHANNEL_ACTIVE && !(data & 0x1))) return 0; - if (!(ndp->flags & NCSI_DEV_HWA) && - state == NCSI_CHANNEL_ACTIVE) + if (state == NCSI_CHANNEL_ACTIVE) ndp->flags |= NCSI_DEV_RESHUFFLE; ncsi_stop_channel_monitor(nc); diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c index bfc43b28c7a6..4da051f90974 100644 --- a/net/ncsi/ncsi-manage.c +++ b/net/ncsi/ncsi-manage.c @@ -113,10 +113,8 @@ static void ncsi_channel_monitor(struct timer_list *t) default: netdev_err(ndp->ndev.dev, "NCSI Channel %d timed out!\n", nc->id); - if (!(ndp->flags & NCSI_DEV_HWA)) { - ncsi_report_link(ndp, true); - ndp->flags |= NCSI_DEV_RESHUFFLE; - } + ncsi_report_link(ndp, true); + ndp->flags |= NCSI_DEV_RESHUFFLE; ncsi_stop_channel_monitor(nc); @@ -1050,35 +1048,6 @@ static bool ncsi_check_hwa(struct ncsi_dev_priv *ndp) return false; } -static int ncsi_enable_hwa(struct ncsi_dev_priv *ndp) -{ - struct ncsi_package *np; - struct ncsi_channel *nc; - unsigned long flags; - - /* Move all available channels to processing queue */ - 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 || - !list_empty(&nc->link)); - ncsi_stop_channel_monitor(nc); - list_add_tail_rcu(&nc->link, &ndp->channel_queue); - } - } - spin_unlock_irqrestore(&ndp->lock, flags); - - /* We can have no channels in extremely case */ - if (list_empty(&ndp->channel_queue)) { - netdev_err(ndp->ndev.dev, - "NCSI: No available channels for HWA\n"); - ncsi_report_link(ndp, false); - return -ENOENT; - } - - return ncsi_process_next_channel(ndp); -} - static void ncsi_probe_channel(struct ncsi_dev_priv *ndp) { struct ncsi_dev *nd = &ndp->ndev; @@ -1592,7 +1561,6 @@ EXPORT_SYMBOL_GPL(ncsi_register_dev); int ncsi_start_dev(struct ncsi_dev *nd) { struct ncsi_dev_priv *ndp = TO_NCSI_DEV_PRIV(nd); - int ret; if (nd->state != ncsi_dev_state_registered && nd->state != ncsi_dev_state_functional) @@ -1604,14 +1572,7 @@ int ncsi_start_dev(struct ncsi_dev *nd) return 0; } - if (ndp->flags & NCSI_DEV_HWA) { - netdev_info(ndp->ndev.dev, "NCSI: Enabling HWA mode\n"); - ret = ncsi_enable_hwa(ndp); - } else { - ret = ncsi_choose_active_channel(ndp); - } - - return ret; + return ncsi_choose_active_channel(nd); } EXPORT_SYMBOL_GPL(ncsi_start_dev);
NCSI hardware arbitration allows multiple packages to be enabled at once and share the same wiring. If the NCSI driver recognises that HWA is available it unconditionally enables all packages and channels; but that is a configuration decision rather than something required by HWA. Additionally the current implementation will not failover on link events which can cause connectivity to be lost unless the interface is manually bounced. Retain basic HWA support but remove the separate configuration path to enable all channels, leaving this to be handled by a later implementation. Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com> --- net/ncsi/ncsi-aen.c | 3 +-- net/ncsi/ncsi-manage.c | 45 +++--------------------------------------- 2 files changed, 4 insertions(+), 44 deletions(-)