On Tue, Aug 23, 2011 at 11:37:21AM -0500, Larry Finger wrote:
> On 08/23/2011 12:53 AM, Ali Bahar wrote:
> >Merged the changes from Realtek's v2.6.6.0.20110401 release.
> >Its Release Notes listed the fixes, though not all may have been
> >merged into this commit. They include:
> >diff --git a/drivers/staging/rtl8712/rtl8712_efuse.c b/drivers/staging/rtl8712/rtl8712_efuse.c
> >index 1dc12b7..8f81d51 100644
> >--- a/drivers/staging/rtl8712/rtl8712_efuse.c
> >+++ b/drivers/staging/rtl8712/rtl8712_efuse.c
> >@@ -302,55 +302,70 @@ static u8 fix_header(struct _adapter *padapter, u8 header, u16 header_addr)
> > }
> > offset = GET_EFUSE_OFFSET(value);
> > word_en = GET_EFUSE_WORD_EN(value);
> >- if (pkt.offset != offset) {
> >+ if (pkt.offset == offset) {
> >+ for (i = 0; i< PGPKG_MAX_WORDS; i++) {
> >+ if (BIT(i)& word_en) {
> >+ if (BIT(i)& pkt.word_en) {
>
> The two if statements above should be collapsed into
>
> if ((BIT(i) & word_en) && (BIT(i) & pkt.word_en)) {
OK.
> That will get rid of one level on indentation. It still gets a lot
> on indentation, but probably not worth refactoring.
The '&&' does not save any indentation.
> >+ if (efuse_one_byte_read(
> >+ padapter, addr,
> >+ &value) == true)
> >+ pkt.data[i*2] = value;
> >+ else
> >+ return false;
> >+ if (efuse_one_byte_read(
> >+ padapter,
> >+ addr + 1,
> >+ &value) == true)
> >+ pkt.data[i*2 + 1] =
> >+ value;
> >+ else
> >+ return false;
> >+ }
> >+ addr += 2;
> >+ }
> >+ }
> >+ } else {
> > addr += calculate_word_cnts(word_en)*2;
> >- continue;
> > }
> >+ }
> >+ if (addr == header_addr) {
> >+ addr++;
> >+ /* fill original data */
> > for (i = 0; i< PGPKG_MAX_WORDS; i++) {
> >- if (BIT(i)& word_en)
> >- continue;
> >- if (!(BIT(i)& pkt.word_en)) {
>
> They did it better here. I wonder why they changed?
I don't know the (bugs?) which led them to change the flow control for
this block, and that of the inner block (ie the 'if BIT' compound
statements) above. But the outer blocks' (eg 'addr == header_addr')
change is just cosmetic.
> >- if (efuse_one_byte_read(padapter, addr,
> >- &value) == true)
> >- pkt.data[i*2] = value;
> >- else
> >- return false;
> >- if (efuse_one_byte_read(padapter, addr + 1,
> >- &value) == true)
> >- pkt.data[i*2 + 1] = value;
> >- else
> >- return false;
> >+ if (BIT(i)& pkt.word_en) {
> >+ efuse_one_byte_write(padapter, addr,
> >+ pkt.data[i*2]);
> >+ efuse_one_byte_write(padapter, addr+1,
> >+ pkt.data[i*2 + 1]);
> >+ /* additional check */
> >+ if (efuse_one_byte_read(padapter, addr,&value)
> >+ == false)
> >+ ret = false;
> >+ else if (pkt.data[i*2] != value) {
> >+ ret = false;
> >+ if (0xFF == value) /* write again */
> >+ efuse_one_byte_write(padapter,
> >+ addr,
> >+ pkt.data[
> >+ i * 2]);
> >+ }
> >+ if (efuse_one_byte_read(
> >+ padapter, addr+1,&value) ==
> >+ false)
> >+ ret = false;
> >+ else if (pkt.data[i*2 + 1] != value) {
> >+ ret = false;
> >+ if (0xFF == value) /* write again */
> >+ efuse_one_byte_write(padapter,
> >+ addr+1,
> >+ pkt.data[i*2
> >+ + 1]);
> >+ }
> > }
> > addr += 2;
> > }
> >- }
> >- if (addr != header_addr)
> >+ } else {
> > return false;
> >- addr++;
> >- /* fill original data */
> >- for (i = 0; i< PGPKG_MAX_WORDS; i++) {
> >- if (BIT(i)& pkt.word_en)
> >- continue;
> >- efuse_one_byte_write(padapter, addr, pkt.data[i*2]);
> >- efuse_one_byte_write(padapter, addr+1, pkt.data[i*2 + 1]);
> >- /* additional check */
> >- if (efuse_one_byte_read(padapter, addr,&value) == false)
> >- ret = false;
> >- else if (pkt.data[i*2] != value) {
> >- ret = false;
> >- if (0xFF == value) /* write again */
> >- efuse_one_byte_write(padapter, addr,
> >- pkt.data[i * 2]);
> >- }
> >- if (efuse_one_byte_read(padapter, addr+1,&value) == false)
> >- ret = false;
> >- else if (pkt.data[i*2 + 1] != value) {
> >- ret = false;
> >- if (0xFF == value) /* write again */
> >- efuse_one_byte_write(padapter, addr+1,
> >- pkt.data[i*2 + 1]);
> >- }
> >- addr += 2;
> > }
> > return ret;
> > }
> >diff --git a/drivers/staging/rtl8712/rtl8712_recv.c b/drivers/staging/rtl8712/rtl8712_recv.c
> >index 625a8a0..d2207de 100644
> >--- a/drivers/staging/rtl8712/rtl8712_recv.c
> >+++ b/drivers/staging/rtl8712/rtl8712_recv.c
> >@@ -192,7 +192,7 @@ static void update_recvframe_attrib_from_recvstat(struct rx_pkt_attrib *pattrib,
> > } else
> > pattrib->tcpchk_valid = 0; /* invalid */
> > pattrib->mcs_rate = (u8)((le32_to_cpu(prxstat->rxdw3))& 0x3f);
> >- pattrib->htc = (u8)((le32_to_cpu(prxstat->rxdw3)>> 6)& 0x1);
> >+ pattrib->htc = (u8)((le32_to_cpu(prxstat->rxdw3)>> 14)& 0x1);
> > /*Offset 16*/
> > /*Offset 20*/
> > /*phy_info*/
> >@@ -207,39 +207,42 @@ static union recv_frame *recvframe_defrag(struct _adapter *adapter,
> > struct __queue *defrag_q)
> > {
> > struct list_head *plist, *phead;
> >- u8 wlanhdr_offset;
> >+ u8 *data, wlanhdr_offset;
> > u8 curfragnum;
> > struct recv_frame_hdr *pfhdr, *pnfhdr;
> > union recv_frame *prframe, *pnextrframe;
> > struct __queue *pfree_recv_queue;
> >
> >+ curfragnum = 0;
> > pfree_recv_queue =&adapter->recvpriv.free_recv_queue;
> > phead = get_list_head(defrag_q);
> > plist = get_next(phead);
> > prframe = LIST_CONTAINOR(plist, union recv_frame, u);
> >- list_delete(&prframe->u.list);
> > pfhdr =&prframe->u.hdr;
> >- curfragnum = 0;
>
> Although Realtek moved the initialization of "curfragnum", it makes
> no functional change to the program. To minimize the changes to the
> kernel code, this kind of change should be avoided.
OK.
> >+ list_delete(&prframe->u.list);
> > if (curfragnum != pfhdr->attrib.frag_num) {
> > /*the first fragment number must be 0
> > *free the whole queue*/
> > r8712_free_recvframe(prframe, pfree_recv_queue);
> >- prframe = NULL;
> >- goto exit;
> >+ r8712_free_recvframe_queue(defrag_q, pfree_recv_queue);
> >+ return NULL;
> > }
> >- plist = get_next(phead);
> >+ curfragnum++;
> >+ plist = get_list_head(defrag_q);
> >+ plist = get_next(plist);
> >+ data = get_recvframe_data(prframe);
> > while (end_of_queue_search(phead, plist) == false) {
> > pnextrframe = LIST_CONTAINOR(plist, union recv_frame, u);
> >- /*check the fragment sequence (2nd ~n fragment frame) */
> > pnfhdr =&pnextrframe->u.hdr;
> >- curfragnum++;
> >+ /*check the fragment sequence (2nd ~n fragment frame) */
> > if (curfragnum != pnfhdr->attrib.frag_num) {
> > /* the fragment number must increase (after decache)
> > * release the defrag_q& prframe */
> > r8712_free_recvframe(prframe, pfree_recv_queue);
> >- prframe = NULL;
> >- goto exit;
> >+ r8712_free_recvframe_queue(defrag_q, pfree_recv_queue);
> >+ return NULL;
> > }
> >+ curfragnum++;
> > /* copy the 2nd~n fragment frame's payload to the first fragment
> > * get the 2nd~last fragment frame's payload */
> > wlanhdr_offset = pnfhdr->attrib.hdrlen + pnfhdr->attrib.iv_len;
> >@@ -252,7 +255,6 @@ static union recv_frame *recvframe_defrag(struct _adapter *adapter,
> > pfhdr->attrib.icv_len = pnfhdr->attrib.icv_len;
> > plist = get_next(plist);
> > }
> >-exit:
> > /* free the defrag_q queue and return the prframe */
> > r8712_free_recvframe_queue(defrag_q, pfree_recv_queue);
> > return prframe;
> >@@ -712,22 +714,12 @@ s32 r8712_signal_scale_mapping(s32 cur_sig)
> > {
> > s32 ret_sig;
> >
> >- if (cur_sig>= 51&& cur_sig<= 100)
> >+ if (cur_sig>= 70&& cur_sig<= 100)
> > ret_sig = 100;
> >- else if (cur_sig>= 41&& cur_sig<= 50)
> >- ret_sig = 80 + ((cur_sig - 40) * 2);
> >- else if (cur_sig>= 31&& cur_sig<= 40)
> >- ret_sig = 66 + (cur_sig - 30);
> >- else if (cur_sig>= 21&& cur_sig<= 30)
> >- ret_sig = 54 + (cur_sig - 20);
> >- else if (cur_sig>= 10&& cur_sig<= 20)
> >- ret_sig = 42 + (((cur_sig - 10) * 2) / 3);
> >- else if (cur_sig>= 5&& cur_sig<= 9)
> >- ret_sig = 22 + (((cur_sig - 5) * 3) / 2);
> >- else if (cur_sig>= 1&& cur_sig<= 4)
> >- ret_sig = 6 + (((cur_sig - 1) * 3) / 2);
> >+ else if (cur_sig>= 30&& cur_sig<= 69)
> >+ ret_sig = 100 - ((70 - cur_sig) * 2);
> > else
> >- ret_sig = cur_sig;
> >+ ret_sig = 10;
> > return ret_sig;
>
> I have not yet tested, but I wonder if these changes are worthwhile.
> The signal output tends to confuse users. If it suddenly changes,
> they will get worried that something has happened to their device.
> After a little more thinking about it, leave the original code.
OK.
> > }
> >
>
> -- snip --
>
> >diff --git a/drivers/staging/rtl8712/rtl871x_cmd.c b/drivers/staging/rtl8712/rtl871x_cmd.c
> >index ba92762..6d2139b 100644
> >--- a/drivers/staging/rtl8712/rtl871x_cmd.c
> >+++ b/drivers/staging/rtl8712/rtl871x_cmd.c
>
> -- snip --
>
> >@@ -358,6 +441,15 @@ void r8712_getbbrfreg_cmdrsp_callback(struct _adapter *padapter,
> > padapter->mppriv.workparam.bcompleted = true;
> > }
> >
> >+void r8712_readtssi_cmdrsp_callback(struct _adapter *padapter,
> >+ struct cmd_obj *pcmd)
> >+{
> >+ kfree((unsigned char *) pcmd->parmbuf);
> >+ kfree((unsigned char *) pcmd);
>
> The casts are not needed here.
OK.
> >+
> >+ padapter->mppriv.workparam.bcompleted = true;
> >+}
> >+
> > u8 r8712_createbss_cmd(struct _adapter *padapter)
> > {
> > struct cmd_obj *pcmd;
>
> -- snip --
>
> >diff --git a/drivers/staging/rtl8712/rtl871x_xmit.c b/drivers/staging/rtl8712/rtl871x_xmit.c
> >index ccf0891..6b86901 100644
> >--- a/drivers/staging/rtl8712/rtl871x_xmit.c
> >+++ b/drivers/staging/rtl8712/rtl871x_xmit.c
> >@@ -152,11 +152,12 @@ sint _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
> > pxmitbuf++;
> > }
> > pxmitpriv->free_xmitbuf_cnt = NR_XMITBUFF;
> >+ _init_workitem(&padapter->wkFilterRxFF0, r8712_SetFilter, padapter);
> > alloc_hwxmits(padapter);
> > init_hwxmits(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
> > tasklet_init(&pxmitpriv->xmit_tasklet,
> >- (void(*)(addr_t))r8712_xmit_bh,
> >- (addr_t)padapter);
> >+ (void(*)(unsigned long))r8712_xmit_bh,
> >+ (unsigned long)padapter);
> > return _SUCCESS;
> > }
> >
> >@@ -349,7 +350,8 @@ sint r8712_update_attrib(struct _adapter *padapter, _pkt *pkt,
> > static sint xmitframe_addmic(struct _adapter *padapter,
> > struct xmit_frame *pxmitframe)
> > {
> >- u32 curfragnum, length, datalen;
> >+ sint curfragnum, length;
> >+ u32 datalen;
>
> Do you know why these changed? Certainly curfragnum and length are
> positive. I don't see any change in the usage of either.
There have been no changes to their usage in this function.
curfragnum does not appear to have any use for a sign! In its
comparison to pattrib->last_txcmdsz, the latter has changed from u32
to u16, though.
'length' does not appear to have any use for a sign!
The above change has not been atypical!
> > u8 *pframe, *payload, mic[8];
> > struct mic_data micdata;
> > struct sta_info *stainfo;
>
> -- snip --
>
> >diff --git a/drivers/staging/rtl8712/usb_intf.c b/drivers/staging/rtl8712/usb_intf.c
> >index 21ce2af..5b8f6e6 100644
> >--- a/drivers/staging/rtl8712/usb_intf.c
> >+++ b/drivers/staging/rtl8712/usb_intf.c
> >@@ -28,6 +28,8 @@
> >
> > #define _HCI_INTF_C_
> >
> >+#include<linux/usb.h>
> >+
> > #include "osdep_service.h"
> > #include "drv_types.h"
> > #include "recv_osdep.h"
> >@@ -51,10 +53,10 @@ static struct usb_device_id rtl871x_usb_id_tbl[] = {
> > /* RTL8188SU */
> > /* Realtek */
> > {USB_DEVICE(0x0BDA, 0x8171)},
> >- {USB_DEVICE(0x0bda, 0x8173)},
> >- {USB_DEVICE(0x0bda, 0x8712)},
> >- {USB_DEVICE(0x0bda, 0x8713)},
> >- {USB_DEVICE(0x0bda, 0xC512)},
> >+ {USB_DEVICE(0x0bda, 0x8173)}, /* = */
> >+ {USB_DEVICE(0x0bda, 0x8712)}, /* = */
> >+ {USB_DEVICE(0x0bda, 0x8713)}, /* = */
> >+ {USB_DEVICE(0x0bda, 0xC512)}, /* = */
>
> The only change in the hunk above are the addition of meaningless
> comments. Do not do that.
OK.
thanks,
ali
> > /* Abocom */
> > {USB_DEVICE(0x07B8, 0x8188)},
> > /* ASUS */
>
>
> Larry
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
