| Message ID | 20210203152855.11866-3-elder@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ipa: a mix of small improvements | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 4 of 4 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 25 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Wed, 3 Feb 2021 09:28:50 -0600 Alex Elder wrote: > int gsi_channel_suspend(struct gsi *gsi, u32 channel_id, bool stop) > { > struct gsi_channel *channel = &gsi->channel[channel_id]; > + int ret; > > - return __gsi_channel_stop(channel, stop); > + /* Synchronize NAPI if successful, to ensure polling has finished. */ > + ret = __gsi_channel_stop(channel, stop); > + if (!ret) > + napi_synchronize(&channel->napi); > + > + return ret; nit: ret = function(); if (ret) return ret; /* success path: do something else */ return 0;
On 2/4/21 10:53 PM, Jakub Kicinski wrote: > On Wed, 3 Feb 2021 09:28:50 -0600 Alex Elder wrote: >> int gsi_channel_suspend(struct gsi *gsi, u32 channel_id, bool stop) >> { >> struct gsi_channel *channel = &gsi->channel[channel_id]; >> + int ret; >> >> - return __gsi_channel_stop(channel, stop); >> + /* Synchronize NAPI if successful, to ensure polling has finished. */ >> + ret = __gsi_channel_stop(channel, stop); >> + if (!ret) >> + napi_synchronize(&channel->napi); >> + >> + return ret; > > nit: > > ret = function(); > if (ret) > return ret; > > /* success path: do something else */ > > return 0; No problem, I'm happy with it the way you suggest. I will update in v2. Thank you. -Alex
diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c index 2671b76ebcfe3..420d0f3bfae9a 100644 --- a/drivers/net/ipa/gsi.c +++ b/drivers/net/ipa/gsi.c @@ -939,10 +939,6 @@ static int __gsi_channel_stop(struct gsi_channel *channel, bool stop) mutex_unlock(&gsi->mutex); } - /* Finally, ensure NAPI polling has finished. */ - if (!ret) - napi_synchronize(&channel->napi); - return ret; } @@ -984,8 +980,14 @@ void gsi_channel_reset(struct gsi *gsi, u32 channel_id, bool doorbell) int gsi_channel_suspend(struct gsi *gsi, u32 channel_id, bool stop) { struct gsi_channel *channel = &gsi->channel[channel_id]; + int ret; - return __gsi_channel_stop(channel, stop); + /* Synchronize NAPI if successful, to ensure polling has finished. */ + ret = __gsi_channel_stop(channel, stop); + if (!ret) + napi_synchronize(&channel->napi); + + return ret; } /* Resume a suspended channel (starting will be requested if STOPPED) */
When stopping a channel, gsi_channel_stop() will ensure NAPI polling is complete when it calls napi_disable(). So there is no need to call napi_synchronize() in that case. Move the call to napi_synchronize() out of __gsi_channel_stop() and into gsi_channel_suspend(), so it's only used where needed. Signed-off-by: Alex Elder <elder@linaro.org> --- drivers/net/ipa/gsi.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)