Hi Larry, On Tue, Jun 27, 2017 at 7:21 PM, Larry Finger <Larry.Finger@xxxxxxxxxxxx> wrote: > On 06/26/2017 09:41 PM, Souptick Joarder wrote: >> >> Any further comment on this patch ? >> >> On 22-Jun-2017 6:53 PM, "Souptick Joarder" <jrdr.linux@xxxxxxxxx >> <mailto:jrdr.linux@xxxxxxxxx>> wrote: >> > >> > As wmm_enable is initialized to false, hence the else condition never >> > execute and boundary is assigned with TX_PAGE_BOUNDARY. So the if-else >> > condition can be removed and boundary will be assigned with >> > TX_PAGE_BOUNDARY. >> > >> > Signed-off-by: Souptick Joarder <jrdr.linux@xxxxxxxxx >> <mailto:jrdr.linux@xxxxxxxxx>> >> >> > --- >> > Changes in v2: >> > - Correcting the indent and moving up the change where >> > boundary is defined. >> > >> > drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c | 9 +-------- >> > 1 file changed, 1 insertion(+), 8 deletions(-) >> > >> > diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c >> b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c >> > index f95a645..107c34e 100644 >> > --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c >> > +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/hw.c >> > @@ -835,7 +835,7 @@ static int _rtl92cu_init_mac(struct ieee80211_hw >> *hw) >> > struct rtl_usb_priv *usb_priv = rtl_usbpriv(hw); >> > struct rtl_usb *rtlusb = rtl_usbdev(usb_priv); >> > int err = 0; >> > - u32 boundary = 0; >> > + u32 boundary = TX_PAGE_BOUNDARY; >> > u8 wmm_enable = false; /* TODO */ >> > u8 out_ep_nums = rtlusb->out_ep_nums; >> > u8 queue_sel = rtlusb->out_queue_sel; >> > @@ -845,13 +845,6 @@ static int _rtl92cu_init_mac(struct ieee80211_hw >> *hw) >> > pr_err("Failed to init power on!\n"); >> > return err; >> > } >> > - if (!wmm_enable) { >> > - boundary = TX_PAGE_BOUNDARY; >> > - } else { /* for WMM */ >> > - boundary = (IS_NORMAL_CHIP(rtlhal->version)) >> > - ? WMM_CHIP_B_TX_PAGE_BOUNDARY >> > - : WMM_CHIP_A_TX_PAGE_BOUNDARY; >> > - } >> > if (false == rtl92c_init_llt_table(hw, boundary)) { >> > pr_err("Failed to init LLT Table!\n"); >> > return -EINVAL; > > > This patch troubles me. I have no idea why the original author placed a TODO > on the assignent of wmm_enable. You, however, have left the TODO in place > but removed the actual place where it would make a difference. As a result, > it would become impossible to reconstruct that author's intentions. For that > reason > > NACK. > > Note that these changes make no difference in the object code and the > optimizer will remove that code anyway. Thanks for your feedback. > > Larry > Souptick
