| Message ID | 20210228122522.2513-1-dinghao.liu@zju.edu.cn (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | iwlegacy: Add missing check in il4965_commit_rxon | expand |
On Sun, Feb 28, 2021 at 08:25:22PM +0800, Dinghao Liu wrote: > There is one il_set_tx_power() call in this function without > return value check. Print error message and return error code > on failure just like the other il_set_tx_power() call. We have few calls for il_set_tx_power(), on some cases we check return on some not. That correct as setting tx power can be deferred internally if not possible at the moment. > Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn> > --- > drivers/net/wireless/intel/iwlegacy/4965.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/intel/iwlegacy/4965.c b/drivers/net/wireless/intel/iwlegacy/4965.c > index 9fa556486511..3235b8be1894 100644 > --- a/drivers/net/wireless/intel/iwlegacy/4965.c > +++ b/drivers/net/wireless/intel/iwlegacy/4965.c > @@ -1361,7 +1361,11 @@ il4965_commit_rxon(struct il_priv *il) > * We do not commit tx power settings while channel changing, > * do it now if tx power changed. > */ > - il_set_tx_power(il, il->tx_power_next, false); > + ret = il_set_tx_power(il, il->tx_power_next, false); > + if (ret) { > + IL_ERR("Error sending TX power (%d)\n", ret); > + return ret; > + This is not good change. We do not check return value of il_commit_rxon(), except when creating interface. So this change might broke creating interface, what worked otherwise when il_set_tx_power() returned error. Stanislaw
> On Sun, Feb 28, 2021 at 08:25:22PM +0800, Dinghao Liu wrote: > > There is one il_set_tx_power() call in this function without > > return value check. Print error message and return error code > > on failure just like the other il_set_tx_power() call. > > We have few calls for il_set_tx_power(), on some cases we > check return on some not. That correct as setting tx power > can be deferred internally if not possible at the moment. > > > Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn> > > --- > > drivers/net/wireless/intel/iwlegacy/4965.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/wireless/intel/iwlegacy/4965.c b/drivers/net/wireless/intel/iwlegacy/4965.c > > index 9fa556486511..3235b8be1894 100644 > > --- a/drivers/net/wireless/intel/iwlegacy/4965.c > > +++ b/drivers/net/wireless/intel/iwlegacy/4965.c > > @@ -1361,7 +1361,11 @@ il4965_commit_rxon(struct il_priv *il) > > * We do not commit tx power settings while channel changing, > > * do it now if tx power changed. > > */ > > - il_set_tx_power(il, il->tx_power_next, false); > > + ret = il_set_tx_power(il, il->tx_power_next, false); > > + if (ret) { > > + IL_ERR("Error sending TX power (%d)\n", ret); > > + return ret; > > + > > This is not good change. We do not check return value of > il_commit_rxon(), except when creating interface. So this change might > broke creating interface, what worked otherwise when il_set_tx_power() > returned error. > It's clear to me, Thanks for your explanation! Regards, Dinghao
diff --git a/drivers/net/wireless/intel/iwlegacy/4965.c b/drivers/net/wireless/intel/iwlegacy/4965.c index 9fa556486511..3235b8be1894 100644 --- a/drivers/net/wireless/intel/iwlegacy/4965.c +++ b/drivers/net/wireless/intel/iwlegacy/4965.c @@ -1361,7 +1361,11 @@ il4965_commit_rxon(struct il_priv *il) * We do not commit tx power settings while channel changing, * do it now if tx power changed. */ - il_set_tx_power(il, il->tx_power_next, false); + ret = il_set_tx_power(il, il->tx_power_next, false); + if (ret) { + IL_ERR("Error sending TX power (%d)\n", ret); + return ret; + } return 0; }
There is one il_set_tx_power() call in this function without return value check. Print error message and return error code on failure just like the other il_set_tx_power() call. Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn> --- drivers/net/wireless/intel/iwlegacy/4965.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)