| Message ID | 20211027170306.555535-4-benl@squareup.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | wcn36xx: software scanning improvements | expand |
On 27/10/2021 18:03, Benjamin Li wrote: > An SMD capture from the downstream prima driver on WCN3680B shows the > following command sequence for connected scans: > > - init_scan_req > - start_scan_req, channel 1 > - end_scan_req, channel 1 > - start_scan_req, channel 2 > - ... > - end_scan_req, channel 3 > - finish_scan_req > - init_scan_req > - start_scan_req, channel 4 > - ... > - end_scan_req, channel 6 > - finish_scan_req > - ... > - end_scan_req, channel 165 > - finish_scan_req > > Upstream currently never calls wcn36xx_smd_end_scan, and in some cases[1] > still sends finish_scan_req twice in a row or before init_scan_req. A > typical connected scan looks like this: > > - init_scan_req > - start_scan_req, channel 1 > - finish_scan_req > - init_scan_req > - start_scan_req, channel 2 > - ... > - start_scan_req, channel 165 > - finish_scan_req > - finish_scan_req > > This patch cleans up scanning so that init/finish and start/end are always > paired together and correctly nested. > > - init_scan_req > - start_scan_req, channel 1 > - end_scan_req, channel 1 > - finish_scan_req > - init_scan_req > - start_scan_req, channel 2 > - end_scan_req, channel 2 > - ... > - start_scan_req, channel 165 > - end_scan_req, channel 165 > - finish_scan_req > > Note that upstream will not do batching of 3 active-probe scans before > returning to the operating channel, and this patch does not change that. > To match downstream in this aspect, adjust IEEE80211_PROBE_DELAY and/or > the 125ms max off-channel time in ieee80211_scan_state_decision. > > [1]: commit d195d7aac09b ("wcn36xx: Ensure finish scan is not requested > before start scan") addressed one case of finish_scan_req being sent > without a preceding init_scan_req (the case of the operating channel > coinciding with the first scan channel); two other cases are: > 1) if SW scan is started and aborted immediately, without scanning any > channels, we send a finish_scan_req without ever sending init_scan_req, > and > 2) as SW scan logic always returns us to the operating channel before > calling wcn36xx_sw_scan_complete, finish_scan_req is always sent twice > at the end of a SW scan > > Fixes: 8e84c2582169 ("wcn36xx: mac80211 driver for Qualcomm WCN3660/WCN3680 hardware") > Signed-off-by: Benjamin Li <benl@squareup.com> > --- > drivers/net/wireless/ath/wcn36xx/main.c | 34 +++++++++++++++++----- > drivers/net/wireless/ath/wcn36xx/smd.c | 4 +++ > drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 1 + > 3 files changed, 32 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c > index 18383d0fc0933..37b4016f020c9 100644 > --- a/drivers/net/wireless/ath/wcn36xx/main.c > +++ b/drivers/net/wireless/ath/wcn36xx/main.c > @@ -400,6 +400,7 @@ static void wcn36xx_change_opchannel(struct wcn36xx *wcn, int ch) > static int wcn36xx_config(struct ieee80211_hw *hw, u32 changed) > { > struct wcn36xx *wcn = hw->priv; > + int ret; > > wcn36xx_dbg(WCN36XX_DBG_MAC, "mac config changed 0x%08x\n", changed); > > @@ -415,17 +416,31 @@ static int wcn36xx_config(struct ieee80211_hw *hw, u32 changed) > * want to receive/transmit regular data packets, then > * simply stop the scan session and exit PS mode. > */ > - wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN, > - wcn->sw_scan_vif); > - wcn->sw_scan_channel = 0; > + if (wcn->sw_scan_channel) > + wcn36xx_smd_end_scan(wcn, wcn->sw_scan_channel); > + if (wcn->sw_scan_init) { > + wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN, > + wcn->sw_scan_vif); > + } > } else if (wcn->sw_scan) { > /* A scan is ongoing, do not change the operating > * channel, but start a scan session on the channel. > */ > - wcn36xx_smd_init_scan(wcn, HAL_SYS_MODE_SCAN, > - wcn->sw_scan_vif); > + if (wcn->sw_scan_channel) > + wcn36xx_smd_end_scan(wcn, wcn->sw_scan_channel); > + if (!wcn->sw_scan_init) { > + /* This can fail if we are unable to notify the > + * operating channel. > + */ > + ret = wcn36xx_smd_init_scan(wcn, > + HAL_SYS_MODE_SCAN, > + wcn->sw_scan_vif); > + if (ret) { > + mutex_unlock(&wcn->conf_mutex); > + return -EIO; > + } > + } > wcn36xx_smd_start_scan(wcn, ch); > - wcn->sw_scan_channel = ch; > } else { > wcn36xx_change_opchannel(wcn, ch); > } > @@ -723,7 +738,12 @@ static void wcn36xx_sw_scan_complete(struct ieee80211_hw *hw, > wcn36xx_dbg(WCN36XX_DBG_MAC, "sw_scan_complete"); > > /* ensure that any scan session is finished */ > - wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN, wcn->sw_scan_vif); > + if (wcn->sw_scan_channel) > + wcn36xx_smd_end_scan(wcn, wcn->sw_scan_channel); > + if (wcn->sw_scan_init) { > + wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN, > + wcn->sw_scan_vif); > + } > wcn->sw_scan = false; > wcn->sw_scan_opchannel = 0; > } > diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c > index 3cecc8f9c9647..830341be72673 100644 > --- a/drivers/net/wireless/ath/wcn36xx/smd.c > +++ b/drivers/net/wireless/ath/wcn36xx/smd.c > @@ -721,6 +721,7 @@ int wcn36xx_smd_init_scan(struct wcn36xx *wcn, enum wcn36xx_hal_sys_mode mode, > wcn36xx_err("hal_init_scan response failed err=%d\n", ret); > goto out; > } > + wcn->sw_scan_init = true; > out: > mutex_unlock(&wcn->hal_mutex); > return ret; > @@ -751,6 +752,7 @@ int wcn36xx_smd_start_scan(struct wcn36xx *wcn, u8 scan_channel) > wcn36xx_err("hal_start_scan response failed err=%d\n", ret); > goto out; > } > + wcn->sw_scan_channel = scan_channel; > out: > mutex_unlock(&wcn->hal_mutex); > return ret; > @@ -781,6 +783,7 @@ int wcn36xx_smd_end_scan(struct wcn36xx *wcn, u8 scan_channel) > wcn36xx_err("hal_end_scan response failed err=%d\n", ret); > goto out; > } > + wcn->sw_scan_channel = 0; > out: > mutex_unlock(&wcn->hal_mutex); > return ret; > @@ -822,6 +825,7 @@ int wcn36xx_smd_finish_scan(struct wcn36xx *wcn, > wcn36xx_err("hal_finish_scan response failed err=%d\n", ret); > goto out; > } > + wcn->sw_scan_init = false; > out: > mutex_unlock(&wcn->hal_mutex); > return ret; > diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h > index 1c8d918137da2..fbd0558c2c196 100644 > --- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h > +++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h > @@ -248,6 +248,7 @@ struct wcn36xx { > struct cfg80211_scan_request *scan_req; > bool sw_scan; > u8 sw_scan_opchannel; > + bool sw_scan_init; > u8 sw_scan_channel; > struct ieee80211_vif *sw_scan_vif; > struct mutex scan_lock; > LGTM Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > On 27/10/2021 18:03, Benjamin Li wrote: >> An SMD capture from the downstream prima driver on WCN3680B shows the >> following command sequence for connected scans: >> >> - init_scan_req >> - start_scan_req, channel 1 >> - end_scan_req, channel 1 >> - start_scan_req, channel 2 >> - ... >> - end_scan_req, channel 3 >> - finish_scan_req >> - init_scan_req >> - start_scan_req, channel 4 >> - ... >> - end_scan_req, channel 6 >> - finish_scan_req >> - ... >> - end_scan_req, channel 165 >> - finish_scan_req >> >> Upstream currently never calls wcn36xx_smd_end_scan, and in some cases[1] >> still sends finish_scan_req twice in a row or before init_scan_req. A >> typical connected scan looks like this: >> >> - init_scan_req >> - start_scan_req, channel 1 >> - finish_scan_req >> - init_scan_req >> - start_scan_req, channel 2 >> - ... >> - start_scan_req, channel 165 >> - finish_scan_req >> - finish_scan_req >> >> This patch cleans up scanning so that init/finish and start/end are always >> paired together and correctly nested. >> >> - init_scan_req >> - start_scan_req, channel 1 >> - end_scan_req, channel 1 >> - finish_scan_req >> - init_scan_req >> - start_scan_req, channel 2 >> - end_scan_req, channel 2 >> - ... >> - start_scan_req, channel 165 >> - end_scan_req, channel 165 >> - finish_scan_req >> >> Note that upstream will not do batching of 3 active-probe scans before >> returning to the operating channel, and this patch does not change that. >> To match downstream in this aspect, adjust IEEE80211_PROBE_DELAY and/or >> the 125ms max off-channel time in ieee80211_scan_state_decision. >> >> [1]: commit d195d7aac09b ("wcn36xx: Ensure finish scan is not requested >> before start scan") addressed one case of finish_scan_req being sent >> without a preceding init_scan_req (the case of the operating channel >> coinciding with the first scan channel); two other cases are: >> 1) if SW scan is started and aborted immediately, without scanning any >> channels, we send a finish_scan_req without ever sending init_scan_req, >> and >> 2) as SW scan logic always returns us to the operating channel before >> calling wcn36xx_sw_scan_complete, finish_scan_req is always sent twice >> at the end of a SW scan >> >> Fixes: 8e84c2582169 ("wcn36xx: mac80211 driver for Qualcomm WCN3660/WCN3680 hardware") >> Signed-off-by: Benjamin Li <benl@squareup.com> >> --- >> drivers/net/wireless/ath/wcn36xx/main.c | 34 +++++++++++++++++----- >> drivers/net/wireless/ath/wcn36xx/smd.c | 4 +++ >> drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 1 + >> 3 files changed, 32 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c >> index 18383d0fc0933..37b4016f020c9 100644 >> --- a/drivers/net/wireless/ath/wcn36xx/main.c >> +++ b/drivers/net/wireless/ath/wcn36xx/main.c >> @@ -400,6 +400,7 @@ static void wcn36xx_change_opchannel(struct wcn36xx *wcn, int ch) >> static int wcn36xx_config(struct ieee80211_hw *hw, u32 changed) >> { >> struct wcn36xx *wcn = hw->priv; >> + int ret; >> wcn36xx_dbg(WCN36XX_DBG_MAC, "mac config changed 0x%08x\n", >> changed); >> @@ -415,17 +416,31 @@ static int wcn36xx_config(struct >> ieee80211_hw *hw, u32 changed) >> * want to receive/transmit regular data packets, then >> * simply stop the scan session and exit PS mode. >> */ >> - wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN, >> - wcn->sw_scan_vif); >> - wcn->sw_scan_channel = 0; >> + if (wcn->sw_scan_channel) >> + wcn36xx_smd_end_scan(wcn, wcn->sw_scan_channel); >> + if (wcn->sw_scan_init) { >> + wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN, >> + wcn->sw_scan_vif); >> + } >> } else if (wcn->sw_scan) { >> /* A scan is ongoing, do not change the operating >> * channel, but start a scan session on the channel. >> */ >> - wcn36xx_smd_init_scan(wcn, HAL_SYS_MODE_SCAN, >> - wcn->sw_scan_vif); >> + if (wcn->sw_scan_channel) >> + wcn36xx_smd_end_scan(wcn, wcn->sw_scan_channel); >> + if (!wcn->sw_scan_init) { >> + /* This can fail if we are unable to notify the >> + * operating channel. >> + */ >> + ret = wcn36xx_smd_init_scan(wcn, >> + HAL_SYS_MODE_SCAN, >> + wcn->sw_scan_vif); >> + if (ret) { >> + mutex_unlock(&wcn->conf_mutex); >> + return -EIO; >> + } >> + } >> wcn36xx_smd_start_scan(wcn, ch); >> - wcn->sw_scan_channel = ch; >> } else { >> wcn36xx_change_opchannel(wcn, ch); >> } >> @@ -723,7 +738,12 @@ static void wcn36xx_sw_scan_complete(struct ieee80211_hw *hw, >> wcn36xx_dbg(WCN36XX_DBG_MAC, "sw_scan_complete"); >> /* ensure that any scan session is finished */ >> - wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN, wcn->sw_scan_vif); >> + if (wcn->sw_scan_channel) >> + wcn36xx_smd_end_scan(wcn, wcn->sw_scan_channel); >> + if (wcn->sw_scan_init) { >> + wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN, >> + wcn->sw_scan_vif); >> + } >> wcn->sw_scan = false; >> wcn->sw_scan_opchannel = 0; >> } >> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c >> index 3cecc8f9c9647..830341be72673 100644 >> --- a/drivers/net/wireless/ath/wcn36xx/smd.c >> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c >> @@ -721,6 +721,7 @@ int wcn36xx_smd_init_scan(struct wcn36xx *wcn, enum wcn36xx_hal_sys_mode mode, >> wcn36xx_err("hal_init_scan response failed err=%d\n", ret); >> goto out; >> } >> + wcn->sw_scan_init = true; >> out: >> mutex_unlock(&wcn->hal_mutex); >> return ret; >> @@ -751,6 +752,7 @@ int wcn36xx_smd_start_scan(struct wcn36xx *wcn, u8 scan_channel) >> wcn36xx_err("hal_start_scan response failed err=%d\n", ret); >> goto out; >> } >> + wcn->sw_scan_channel = scan_channel; >> out: >> mutex_unlock(&wcn->hal_mutex); >> return ret; >> @@ -781,6 +783,7 @@ int wcn36xx_smd_end_scan(struct wcn36xx *wcn, u8 scan_channel) >> wcn36xx_err("hal_end_scan response failed err=%d\n", ret); >> goto out; >> } >> + wcn->sw_scan_channel = 0; >> out: >> mutex_unlock(&wcn->hal_mutex); >> return ret; >> @@ -822,6 +825,7 @@ int wcn36xx_smd_finish_scan(struct wcn36xx *wcn, >> wcn36xx_err("hal_finish_scan response failed err=%d\n", ret); >> goto out; >> } >> + wcn->sw_scan_init = false; >> out: >> mutex_unlock(&wcn->hal_mutex); >> return ret; >> diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h >> index 1c8d918137da2..fbd0558c2c196 100644 >> --- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h >> +++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h >> @@ -248,6 +248,7 @@ struct wcn36xx { >> struct cfg80211_scan_request *scan_req; >> bool sw_scan; >> u8 sw_scan_opchannel; >> + bool sw_scan_init; >> u8 sw_scan_channel; >> struct ieee80211_vif *sw_scan_vif; >> struct mutex scan_lock; >> > > LGTM > > Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> Thanks, all review and testing is very much appreciated. But please trim your replies, including the whole patch makes reading your replies and using patchwork much harder: https://patchwork.kernel.org/project/linux-wireless/patch/20211027170306.555535-4-benl@squareup.com/ I recommend just including the commit log and dropping the rest.
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c index 18383d0fc0933..37b4016f020c9 100644 --- a/drivers/net/wireless/ath/wcn36xx/main.c +++ b/drivers/net/wireless/ath/wcn36xx/main.c @@ -400,6 +400,7 @@ static void wcn36xx_change_opchannel(struct wcn36xx *wcn, int ch) static int wcn36xx_config(struct ieee80211_hw *hw, u32 changed) { struct wcn36xx *wcn = hw->priv; + int ret; wcn36xx_dbg(WCN36XX_DBG_MAC, "mac config changed 0x%08x\n", changed); @@ -415,17 +416,31 @@ static int wcn36xx_config(struct ieee80211_hw *hw, u32 changed) * want to receive/transmit regular data packets, then * simply stop the scan session and exit PS mode. */ - wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN, - wcn->sw_scan_vif); - wcn->sw_scan_channel = 0; + if (wcn->sw_scan_channel) + wcn36xx_smd_end_scan(wcn, wcn->sw_scan_channel); + if (wcn->sw_scan_init) { + wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN, + wcn->sw_scan_vif); + } } else if (wcn->sw_scan) { /* A scan is ongoing, do not change the operating * channel, but start a scan session on the channel. */ - wcn36xx_smd_init_scan(wcn, HAL_SYS_MODE_SCAN, - wcn->sw_scan_vif); + if (wcn->sw_scan_channel) + wcn36xx_smd_end_scan(wcn, wcn->sw_scan_channel); + if (!wcn->sw_scan_init) { + /* This can fail if we are unable to notify the + * operating channel. + */ + ret = wcn36xx_smd_init_scan(wcn, + HAL_SYS_MODE_SCAN, + wcn->sw_scan_vif); + if (ret) { + mutex_unlock(&wcn->conf_mutex); + return -EIO; + } + } wcn36xx_smd_start_scan(wcn, ch); - wcn->sw_scan_channel = ch; } else { wcn36xx_change_opchannel(wcn, ch); } @@ -723,7 +738,12 @@ static void wcn36xx_sw_scan_complete(struct ieee80211_hw *hw, wcn36xx_dbg(WCN36XX_DBG_MAC, "sw_scan_complete"); /* ensure that any scan session is finished */ - wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN, wcn->sw_scan_vif); + if (wcn->sw_scan_channel) + wcn36xx_smd_end_scan(wcn, wcn->sw_scan_channel); + if (wcn->sw_scan_init) { + wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN, + wcn->sw_scan_vif); + } wcn->sw_scan = false; wcn->sw_scan_opchannel = 0; } diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c index 3cecc8f9c9647..830341be72673 100644 --- a/drivers/net/wireless/ath/wcn36xx/smd.c +++ b/drivers/net/wireless/ath/wcn36xx/smd.c @@ -721,6 +721,7 @@ int wcn36xx_smd_init_scan(struct wcn36xx *wcn, enum wcn36xx_hal_sys_mode mode, wcn36xx_err("hal_init_scan response failed err=%d\n", ret); goto out; } + wcn->sw_scan_init = true; out: mutex_unlock(&wcn->hal_mutex); return ret; @@ -751,6 +752,7 @@ int wcn36xx_smd_start_scan(struct wcn36xx *wcn, u8 scan_channel) wcn36xx_err("hal_start_scan response failed err=%d\n", ret); goto out; } + wcn->sw_scan_channel = scan_channel; out: mutex_unlock(&wcn->hal_mutex); return ret; @@ -781,6 +783,7 @@ int wcn36xx_smd_end_scan(struct wcn36xx *wcn, u8 scan_channel) wcn36xx_err("hal_end_scan response failed err=%d\n", ret); goto out; } + wcn->sw_scan_channel = 0; out: mutex_unlock(&wcn->hal_mutex); return ret; @@ -822,6 +825,7 @@ int wcn36xx_smd_finish_scan(struct wcn36xx *wcn, wcn36xx_err("hal_finish_scan response failed err=%d\n", ret); goto out; } + wcn->sw_scan_init = false; out: mutex_unlock(&wcn->hal_mutex); return ret; diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h index 1c8d918137da2..fbd0558c2c196 100644 --- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h +++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h @@ -248,6 +248,7 @@ struct wcn36xx { struct cfg80211_scan_request *scan_req; bool sw_scan; u8 sw_scan_opchannel; + bool sw_scan_init; u8 sw_scan_channel; struct ieee80211_vif *sw_scan_vif; struct mutex scan_lock;
An SMD capture from the downstream prima driver on WCN3680B shows the following command sequence for connected scans: - init_scan_req - start_scan_req, channel 1 - end_scan_req, channel 1 - start_scan_req, channel 2 - ... - end_scan_req, channel 3 - finish_scan_req - init_scan_req - start_scan_req, channel 4 - ... - end_scan_req, channel 6 - finish_scan_req - ... - end_scan_req, channel 165 - finish_scan_req Upstream currently never calls wcn36xx_smd_end_scan, and in some cases[1] still sends finish_scan_req twice in a row or before init_scan_req. A typical connected scan looks like this: - init_scan_req - start_scan_req, channel 1 - finish_scan_req - init_scan_req - start_scan_req, channel 2 - ... - start_scan_req, channel 165 - finish_scan_req - finish_scan_req This patch cleans up scanning so that init/finish and start/end are always paired together and correctly nested. - init_scan_req - start_scan_req, channel 1 - end_scan_req, channel 1 - finish_scan_req - init_scan_req - start_scan_req, channel 2 - end_scan_req, channel 2 - ... - start_scan_req, channel 165 - end_scan_req, channel 165 - finish_scan_req Note that upstream will not do batching of 3 active-probe scans before returning to the operating channel, and this patch does not change that. To match downstream in this aspect, adjust IEEE80211_PROBE_DELAY and/or the 125ms max off-channel time in ieee80211_scan_state_decision. [1]: commit d195d7aac09b ("wcn36xx: Ensure finish scan is not requested before start scan") addressed one case of finish_scan_req being sent without a preceding init_scan_req (the case of the operating channel coinciding with the first scan channel); two other cases are: 1) if SW scan is started and aborted immediately, without scanning any channels, we send a finish_scan_req without ever sending init_scan_req, and 2) as SW scan logic always returns us to the operating channel before calling wcn36xx_sw_scan_complete, finish_scan_req is always sent twice at the end of a SW scan Fixes: 8e84c2582169 ("wcn36xx: mac80211 driver for Qualcomm WCN3660/WCN3680 hardware") Signed-off-by: Benjamin Li <benl@squareup.com> --- drivers/net/wireless/ath/wcn36xx/main.c | 34 +++++++++++++++++----- drivers/net/wireless/ath/wcn36xx/smd.c | 4 +++ drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 1 + 3 files changed, 32 insertions(+), 7 deletions(-)