On Fri, 2020-03-13 at 14:21 -0700, Joe Perches wrote: Hi Joe, > On Fri, 2020-03-13 at 15:59 +0530, Shreeya Patel wrote: > > Remove unnecessary if and else conditions since both are leading to > > the > > initialization of "phtpriv->ampdu_enable" with the same value. > > Also, remove the unnecessary else-if condition since it does > > nothing. > > [] > > diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c > > b/drivers/staging/rtl8723bs/core/rtw_mlme.c > > [] > > @@ -2772,16 +2772,7 @@ void rtw_update_ht_cap(struct adapter > > *padapter, u8 *pie, uint ie_len, u8 channe > > > > /* maybe needs check if ap supports rx ampdu. */ > > if (!(phtpriv->ampdu_enable) && pregistrypriv->ampdu_enable == > > 1) { > > - if (pregistrypriv->wifi_spec == 1) { > > - /* remove this part because testbed AP should > > disable RX AMPDU */ > > - /* phtpriv->ampdu_enable = false; */ > > - phtpriv->ampdu_enable = true; > > - } else { > > - phtpriv->ampdu_enable = true; > > - } > > - } else if (pregistrypriv->ampdu_enable == 2) { > > - /* remove this part because testbed AP should disable > > RX AMPDU */ > > - /* phtpriv->ampdu_enable = true; */ > > + phtpriv->ampdu_enable = true; > > This isn't the same test. > > This could be: > if ((!(phtpriv->ampdu_enable) && pregistrypriv->ampdu_enable == > 1)) || > pregistrypriv->ampdu_enable == 2) > phtpriv->ampdu_enable = true; > > Though it is probably more sensible to just set > phtpriv->ampdu_enable without testing whether or > not it's already set: > > if (pregistrypriv->ampdu_enable == 1 || > pregistrypriv->ampdu_enable == 2) > phtpriv->ampdu_enable = true; But the else-if block which I removed in v2 of this patch had nothing in the block. It was not assigning any value to "phtpriv->ampdu_enable". ( basically it was empty and useless) Now as per your suggestion if I do the change then the value of "phtpriv->ampdu_enable" will be changed to true when we have "pregistrypriv->ampdu_enable == 2" condition. But in real it should be the same as it was by default coming from the start of the function. ( This is because the else-if block was empty and doing nothing ) Please let me know if I was able to make you understand my point of view here. Also, please correct me if I am wrong. Thanks > > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel