| Message ID | 20160202154855.GA8990@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
Hi Byeoungwook, On Wed, Feb 3, 2016 at 2:48 AM, Byeoungwook Kim <quddnr145@gmail.com> wrote: > rtl_*_delay() functions were reused same codes about addr variable. > So i have converted to rtl_addr_delay() from code about addr variable. > > Conditional codes in rtl_addr_delay() were improved in readability and > performance by using switch codes. > > Signed-off-by: Byeoungwook Kim <quddnr145@gmail.com> > --- > drivers/net/wireless/realtek/rtlwifi/core.c | 48 +++++++++++------------------ > 1 file changed, 18 insertions(+), 30 deletions(-) > > diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c b/drivers/net/wireless/realtek/rtlwifi/core.c > index 4ae421e..c1193d1 100644 > --- a/drivers/net/wireless/realtek/rtlwifi/core.c > +++ b/drivers/net/wireless/realtek/rtlwifi/core.c > @@ -37,36 +37,34 @@ > > void rtl_addr_delay(u32 addr) > { > - if (addr == 0xfe) > + switch (addr) { > + case 0xfe: > mdelay(50); > - else if (addr == 0xfd) > + break; > + case 0xfd: > mdelay(5); > - else if (addr == 0xfc) > + break; > + case 0xfc: > mdelay(1); > - else if (addr == 0xfb) > + break; > + case 0xfb: > udelay(50); > - else if (addr == 0xfa) > + break; > + case 0xfa: > udelay(5); > - else if (addr == 0xf9) > + break; > + case 0xf9: > udelay(1); > + break; > + }; As you're introducing a case statement here, you could consolidate the addresses that have the same delays, i.e. case 0xfe: case 0xfb: mdelay(50); break; also, you should arguably be doing this cleanup in a separate patch, i.e. 1. Convert open coded instances to use this function (i.e. the changes below this comment) 2. Improve the function > } > EXPORT_SYMBOL(rtl_addr_delay); > > void rtl_rfreg_delay(struct ieee80211_hw *hw, enum radio_path rfpath, u32 addr, > u32 mask, u32 data) > { > - if (addr == 0xfe) { > - mdelay(50); > - } else if (addr == 0xfd) { > - mdelay(5); > - } else if (addr == 0xfc) { > - mdelay(1); > - } else if (addr == 0xfb) { > - udelay(50); > - } else if (addr == 0xfa) { > - udelay(5); > - } else if (addr == 0xf9) { > - udelay(1); > + if (addr >= 0xf9 && addr <= 0xfe) { > + rtl_addr_delay(addr); > } else { > rtl_set_rfreg(hw, rfpath, addr, mask, data); > udelay(1); > @@ -76,18 +74,8 @@ EXPORT_SYMBOL(rtl_rfreg_delay); > > void rtl_bb_delay(struct ieee80211_hw *hw, u32 addr, u32 data) > { > - if (addr == 0xfe) { > - mdelay(50); > - } else if (addr == 0xfd) { > - mdelay(5); > - } else if (addr == 0xfc) { > - mdelay(1); > - } else if (addr == 0xfb) { > - udelay(50); > - } else if (addr == 0xfa) { > - udelay(5); > - } else if (addr == 0xf9) { > - udelay(1); > + if (addr >= 0xf9 && addr <= 0xfe) { > + rtl_addr_delay(addr); > } else { > rtl_set_bbreg(hw, addr, MASKDWORD, data); > udelay(1); Thanks,
Hi ByeoungWook, On Wed, Feb 3, 2016 at 11:52 AM, ByeoungWook Kim <quddnr145@gmail.com> wrote: > Hi Julian, > > 0xfe and 0xfb was not same codes. > 0xfe is udelay(50). and 0xfb is mdelay(50). > It same code like udelay((n) * 1000). I'm clearly blind! Sorry about that! > but i agree with your answers of some parts. I think that i should divide > into separate patch. > Thanks for your assists! Not a problem! Thanks,
diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c b/drivers/net/wireless/realtek/rtlwifi/core.c index 4ae421e..c1193d1 100644 --- a/drivers/net/wireless/realtek/rtlwifi/core.c +++ b/drivers/net/wireless/realtek/rtlwifi/core.c @@ -37,36 +37,34 @@ void rtl_addr_delay(u32 addr) { - if (addr == 0xfe) + switch (addr) { + case 0xfe: mdelay(50); - else if (addr == 0xfd) + break; + case 0xfd: mdelay(5); - else if (addr == 0xfc) + break; + case 0xfc: mdelay(1); - else if (addr == 0xfb) + break; + case 0xfb: udelay(50); - else if (addr == 0xfa) + break; + case 0xfa: udelay(5); - else if (addr == 0xf9) + break; + case 0xf9: udelay(1); + break; + }; } EXPORT_SYMBOL(rtl_addr_delay); void rtl_rfreg_delay(struct ieee80211_hw *hw, enum radio_path rfpath, u32 addr, u32 mask, u32 data) { - if (addr == 0xfe) { - mdelay(50); - } else if (addr == 0xfd) { - mdelay(5); - } else if (addr == 0xfc) { - mdelay(1); - } else if (addr == 0xfb) { - udelay(50); - } else if (addr == 0xfa) { - udelay(5); - } else if (addr == 0xf9) { - udelay(1); + if (addr >= 0xf9 && addr <= 0xfe) { + rtl_addr_delay(addr); } else { rtl_set_rfreg(hw, rfpath, addr, mask, data); udelay(1); @@ -76,18 +74,8 @@ EXPORT_SYMBOL(rtl_rfreg_delay); void rtl_bb_delay(struct ieee80211_hw *hw, u32 addr, u32 data) { - if (addr == 0xfe) { - mdelay(50); - } else if (addr == 0xfd) { - mdelay(5); - } else if (addr == 0xfc) { - mdelay(1); - } else if (addr == 0xfb) { - udelay(50); - } else if (addr == 0xfa) { - udelay(5); - } else if (addr == 0xf9) { - udelay(1); + if (addr >= 0xf9 && addr <= 0xfe) { + rtl_addr_delay(addr); } else { rtl_set_bbreg(hw, addr, MASKDWORD, data); udelay(1);
rtl_*_delay() functions were reused same codes about addr variable. So i have converted to rtl_addr_delay() from code about addr variable. Conditional codes in rtl_addr_delay() were improved in readability and performance by using switch codes. Signed-off-by: Byeoungwook Kim <quddnr145@gmail.com> --- drivers/net/wireless/realtek/rtlwifi/core.c | 48 +++++++++++------------------ 1 file changed, 18 insertions(+), 30 deletions(-)