From: "Franky Lin" <frankyl@broadcom.com>
To: "Arend van Spriel" <arend@broadcom.com>,
"Johannes Berg" <johannes@sipsolutions.net>
Cc: "gregkh@suse.de" <gregkh@suse.de>,
"devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH 09/20] staging: brcm80211: use endian annotated structures in brcmsmac
Date: Mon, 26 Sep 2011 12:17:53 -0700 [thread overview]
Message-ID: <4E80CFE1.506@broadcom.com> (raw)
In-Reply-To: <4E80C3CE.9010001@broadcom.com>
On 09/26/2011 11:26 AM, Arend van Spriel wrote:
> On 09/26/2011 11:37 AM, Johannes Berg wrote:
>> On Sat, 2011-09-24 at 14:34 +0200, Arend van Spriel wrote:
>>> On 09/24/2011 12:38 PM, Johannes Berg wrote:
>>>> On Fri, 2011-09-23 at 19:08 -0700, Franky Lin wrote:
>>>>> struct d11rxhdr {
>>>>> u16 RxFrameSize;
>>>>> u16 PAD;
>>>>> + union {
>>>>> + struct d11rxhdr_le rxh_le;
>>>>> + struct d11rxhdr rxh_cpu;
>>>>> + };
>>>> This seems a little strange. Why would it be both in LE and CPU byte
>>>> order?
>>> Indeed. When we receive it from the device it is in LE and we convert it
>>> to CPU order for further processing using rxh_cpu.
>> That seems a confusing and error-prone -- you'll have to remember
>> whether you're before or after conversion. Would it be possible to have
>> two versions of the outer structure and change the pointer type at that
>> point?
>>
>> johannes
>
> For me knowing the driver design (a little ;-) it is not difficult to
> remember. Your feedback has valid arguments so I will reconsider. Franky
> is looking whether dropping it will affect the other patches submitted
> to Greg.
>
> Gr. AvS
Dropping this one will affect some following patches in the series.
Since it's not a bug, shall we keep this one and change it as Johannes
suggested in future commit?
Franky
next prev parent reply other threads:[~2011-09-26 19:18 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-24 2:08 [PATCH 00/20] staging: brcm80211: 8th reaction for mainline patch #2 Franky Lin
2011-09-24 2:08 ` [PATCH 01/20] staging: brcm80211: remove uncoditional code blocks from brcmsmac Franky Lin
2011-09-24 2:08 ` [PATCH 02/20] staging: brcm80211: removed unused argument from softmac functions Franky Lin
2011-09-24 2:08 ` [PATCH 03/20] staging: brcm80211: deleted unused array of bss configurations in softmac Franky Lin
2011-09-24 2:08 ` [PATCH 04/20] staging: brcm80211: removed redundant wlc->cfg struct member Franky Lin
2011-09-24 2:08 ` [PATCH 05/20] staging: brcm80211: removed global var from aiutils.c Franky Lin
2011-09-24 2:08 ` [PATCH 06/20] staging: brcm80211: removed global vars in softmac ucode handling Franky Lin
2011-09-24 2:08 ` [PATCH 07/20] staging: brcm80211: removed unused softmac workaround Franky Lin
2011-09-24 2:08 ` [PATCH 08/20] staging: brcm80211: cleanup structure fields used for scanning Franky Lin
2011-09-26 19:07 ` Franky Lin
2011-09-24 2:08 ` [PATCH 09/20] staging: brcm80211: use endian annotated structures in brcmsmac Franky Lin
2011-09-24 10:38 ` Johannes Berg
2011-09-24 12:34 ` Arend van Spriel
2011-09-26 9:37 ` Johannes Berg
2011-09-26 18:26 ` Arend van Spriel
2011-09-26 19:17 ` Franky Lin [this message]
2011-09-26 23:50 ` Greg KH
2011-09-27 0:08 ` Franky Lin
2011-09-26 21:06 ` Rafał Miłecki
2011-09-27 9:54 ` Arend van Spriel
2011-09-24 2:08 ` [PATCH 10/20] staging: brcm80211: remove ht_cap field from brcms_c_info structure Franky Lin
2011-09-24 2:08 ` [PATCH 11/20] staging: brcm80211: use fragment number provided in transmit frame Franky Lin
2011-09-24 2:09 ` [PATCH 12/20] staging: brcm80211: remove unused function si_pmu_ilp_clock() Franky Lin
2011-09-24 2:09 ` [PATCH 13/20] staging: brcm80211: make device initializer table for wme constant Franky Lin
2011-09-24 2:09 ` [PATCH 14/20] staging: brcm80211: remove dongle firmware related debug code Franky Lin
2011-09-24 10:39 ` Johannes Berg
2011-09-24 14:49 ` Arend van Spriel
2011-09-24 2:09 ` [PATCH 15/20] staging: brcm80211: remove unnecessary mac80211 callbacks Franky Lin
2011-09-24 2:09 ` [PATCH 16/20] staging: brcm80211: declared global vars in softmac phy as const Franky Lin
2011-09-24 9:52 ` Julian Andres Klode
[not found] ` <4E80B37C.4060106@broadcom.com>
2011-09-27 7:52 ` Fwd: " Roland Vossen
2011-09-24 2:09 ` [PATCH 17/20] staging: brcm80211: removed band related global vars from softmac Franky Lin
2011-09-24 2:09 ` [PATCH 18/20] staging: brcm80211: removed global var global_scb " Franky Lin
2011-09-24 2:09 ` [PATCH 19/20] staging: brcm80211: various global var related changes in softmac Franky Lin
2011-09-24 2:09 ` [PATCH 20/20] staging: brcm80211: removed global variable in softmac otp Franky Lin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4E80CFE1.506@broadcom.com \
--to=frankyl@broadcom.com \
--cc=arend@broadcom.com \
--cc=devel@linuxdriverproject.org \
--cc=gregkh@suse.de \
--cc=johannes@sipsolutions.net \
--cc=linux-wireless@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.