[PATCH 2/3] rtl8188eu: Fix a typo in rtw_led.*
A rather obvious typo in one of the identifier has been found. This patch
fixes the typo and ensures any lines changed do not exceed 80 characters
as indicated by scripts/checkpatch.pl
Signed-off-by: Krzysztof Konopko
---
drivers/staging/rtl8188eu/core/rtw_led.c| 6 --
drivers/staging/rtl8188eu/include/rtw_led.h | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_led.c
b/drivers/staging/rtl8188eu/core/rtw_led.c
index c6e8379..1b8264b 100644
--- a/drivers/staging/rtl8188eu/core/rtw_led.c
+++ b/drivers/staging/rtl8188eu/core/rtw_led.c
@@ -224,7 +224,8 @@ static void SwLedBlink1(struct LED_871x *pLed)
pLed->bLedWPSBlinkInProgress = false;
} else {
pLed->BlinkingLedState = RTW_LED_OFF;
- _set_timer(&(pLed->BlinkTimer),
LED_BLINK_WPS_SUCESS_INTERVAL_ALPHA);
+ _set_timer(&(pLed->BlinkTimer),
+ LED_BLINK_WPS_SUCCESS_INTERVAL_ALPHA);
}
break;
default:
@@ -388,7 +389,8 @@ static void SwLedControlMode1(struct adapter *padapter,
enum LED_CTL_MODE LedAct
pLed->CurrLedState = LED_BLINK_WPS_STOP;
if (pLed->bLedOn) {
pLed->BlinkingLedState = RTW_LED_OFF;
- _set_timer(&(pLed->BlinkTimer),
LED_BLINK_WPS_SUCESS_INTERVAL_ALPHA);
+ _set_timer(&(pLed->BlinkTimer),
+ LED_BLINK_WPS_SUCCESS_INTERVAL_ALPHA);
} else {
pLed->BlinkingLedState = RTW_LED_ON;
_set_timer(&(pLed->BlinkTimer), 0);
diff --git a/drivers/staging/rtl8188eu/include/rtw_led.h
b/drivers/staging/rtl8188eu/include/rtw_led.h
index c49a9a1..23f0cfe 100644
--- a/drivers/staging/rtl8188eu/include/rtw_led.h
+++ b/drivers/staging/rtl8188eu/include/rtw_led.h
@@ -23,7 +23,7 @@
#define LED_BLINK_LINK_INTERVAL_ALPHA 500 /* 500 */
#define LED_BLINK_SCAN_INTERVAL_ALPHA 180 /* 150 */
#define LED_BLINK_FASTER_INTERVAL_ALPHA50
-#define LED_BLINK_WPS_SUCESS_INTERVAL_ALPHA5000
+#define LED_BLINK_WPS_SUCCESS_INTERVAL_ALPHA 5000
enum LED_CTL_MODE {
LED_CTL_POWER_ON,
--
2.1.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] rtl8188eu: Fix FSF_MAILING_ADDRESS in rtw_led.*
rtw_led.* files include Free Software Foundation's mailing address in the sample GPL notice. This is not desired and picked when running scripts/checkpatch.pl. Signed-off-by: Krzysztof Konopko --- drivers/staging/rtl8188eu/core/rtw_led.c| 4 drivers/staging/rtl8188eu/include/rtw_led.h | 4 2 files changed, 8 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_led.c b/drivers/staging/rtl8188eu/core/rtw_led.c index 2aa6251..c6e8379 100644 --- a/drivers/staging/rtl8188eu/core/rtw_led.c +++ b/drivers/staging/rtl8188eu/core/rtw_led.c @@ -11,10 +11,6 @@ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for * more details. * - * You should have received a copy of the GNU General Public License along with - * this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110, USA - * * **/ diff --git a/drivers/staging/rtl8188eu/include/rtw_led.h b/drivers/staging/rtl8188eu/include/rtw_led.h index dec8dbc..c49a9a1 100644 --- a/drivers/staging/rtl8188eu/include/rtw_led.h +++ b/drivers/staging/rtl8188eu/include/rtw_led.h @@ -11,10 +11,6 @@ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for * more details. * - * You should have received a copy of the GNU General Public License along with - * this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110, USA - * * **/ #ifndef __RTW_LED_H_ -- 2.1.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] rtl8188eu: Remove leading spaces in rtw_led.c
According to Linux coding convention leading spaces are not allowed. This
patch removes leading spaces in rtw_led.c
Signed-off-by: Krzysztof Konopko
---
drivers/staging/rtl8188eu/core/rtw_led.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_led.c
b/drivers/staging/rtl8188eu/core/rtw_led.c
index 1b8264b..5d7e8ec 100644
--- a/drivers/staging/rtl8188eu/core/rtw_led.c
+++ b/drivers/staging/rtl8188eu/core/rtw_led.c
@@ -40,6 +40,7 @@ void BlinkTimerCallback(void *data)
void BlinkWorkItemCallback(struct work_struct *work)
{
struct LED_871x *pLed = container_of(work, struct LED_871x,
BlinkWorkItem);
+
BlinkHandler(pLed);
}
@@ -459,8 +460,8 @@ void LedControl8188eu(struct adapter *padapter, enum
LED_CTL_MODE LedAction)
{
struct led_priv *ledpriv = &(padapter->ledpriv);
- if ((padapter->bSurpriseRemoved) || (padapter->bDriverStopped) ||
- (!padapter->hw_init_completed))
+ if ((padapter->bSurpriseRemoved) || (padapter->bDriverStopped) ||
+ (!padapter->hw_init_completed))
return;
if (!ledpriv->bRegUseLed)
--
2.1.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] rtl8188eu: Remove unnecessary braces for simple return in xmit_linux.c
scripts/checkpatch.pl reports a coding style problem in xmit_linux.c
WARNING:BRACES: braces {} are not necessary for single statement blocks
#61852: FILE: rtl8188eu/os_dep/xmit_linux.c:70:
This patch removes unnecessary braces.
Signed-off-by: Krzysztof Konopko
---
drivers/staging/rtl8188eu/os_dep/xmit_linux.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/xmit_linux.c
b/drivers/staging/rtl8188eu/os_dep/xmit_linux.c
index d5e41a5..9e0e338 100644
--- a/drivers/staging/rtl8188eu/os_dep/xmit_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/xmit_linux.c
@@ -67,9 +67,8 @@ uint _rtw_pktfile_read(struct pkt_file *pfile, u8 *rmem, uint
rlen)
int rtw_endofpktfile(struct pkt_file *pfile)
{
- if (pfile->pkt_len == 0) {
+ if (pfile->pkt_len == 0)
return true;
- }
return false;
--
2.1.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] rtl8188eu: Remove unnecessary braces for simple return in xmit_linux.c
On 08/11/14 21:23, Joe Perches wrote:
> On Sat, 2014-11-08 at 22:14 +0100, Krzysztof Konopko wrote:
>> scripts/checkpatch.pl reports a coding style problem in xmit_linux.c
> []
>> diff --git a/drivers/staging/rtl8188eu/os_dep/xmit_linux.c
>> b/drivers/staging/rtl8188eu/os_dep/xmit_linux.c
> []
>> @@ -67,9 +67,8 @@ uint _rtw_pktfile_read(struct pkt_file *pfile, u8 *rmem,
>> uint rlen)
>> int rtw_endofpktfile(struct pkt_file *pfile)
>> {
>>
>> -if (pfile->pkt_len == 0) {
>> +if (pfile->pkt_len == 0)
>> return true;
>> -}
>>
>>
>> return false;
>
> This should probably be
>
> bool rtw_endofpktfile(const struct pkt_file *pfile_
> {
> return !pfile->pkt_len;
> }
>
Thanks for looking into it.
I see your point about making it a single return statement. I tend to
shorten things as well. I'd keep it as this:
bool rtw_endofpktfile(const struct pkt_file *pfile_
{
return pfile->pkt_len == 0;
}
Usign `!` operator suggests the variable is boolean although the name
suggests it isn't. I'm not so familiar with the linux kernel code base
yet to justify it myself but I see no harm in making it slightly more
explicit.
> or just removed altogether and tested directly
> in the one place it's used.
>
>
It looks to me that the original intention was to open a possibility to
define the end of packet file in a OS dependent way so I'd leave it.
Or, if the counter argument is that non-Linux functionality should not
appear in this driver, the rest of non-Linux code should be removed in
the first place. I'm not in position to even have an opinion on this.
The sole point of this patch was to fix a coding style problem but the
change you suggest seems still relevant. I'll resend unless you have
strong objections on using `==` operator explicitly in the return statement.
Cheers,
Kris
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] rtl8188eu: Fix several stylistic problems in rtw_led.*
On 06/11/14 23:03, Greg Kroah-Hartman wrote: > On Thu, Nov 06, 2014 at 11:56:05PM +0100, k...@konagma.com wrote: >> From: Krzysztof Konopko >> >> Several stylistics problems are reported by scripts/checkpatch.pl run on >> rtw_led.*: >> >> * FSF_MAILING_ADDRESS >> >> Free Software Foundation's mailing address should not be included in >> the sample GPL notice. >> >> * LINE_SPACING >> >> Missing a blank line after declarations. >> >> * leading spaces >> >> * a typo >> >> * any too long lines changed with this patch >> >> All of the problems above are fixed with this patch. > > Each patch should only do 1 thing, so please break this up into multiple > patches, each only doing 1 thing, in a patch series, and resend so it > can be applied. > > thanks, > > greg k-h > Thanks for having a look. I've resent it in a series of three patches already. A bit of mess the way it appears on some on-line mailing list readers (gmane for example) as I used `--chain-reply-to` (while writing this I just bumped onto `Documentation/development-process/5.Posting` again, hm... not sure if this was the right thing to do). Also I've been trying to create a separate email client set-up to what I do for my everyday work with Git patches and reviews. But I gave up and fell back to what works for me on everyday basis :) I'll keep trying to get it right :) Cheers, Kris ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] rtl8188eu: Simplify rtw_endofpktfile() in xmit_linux.c
scripts/checkpatch.pl reports a coding style problem in xmit_linux.c
WARNING:BRACES: braces {} are not necessary for single statement blocks
#61852: FILE: rtl8188eu/os_dep/xmit_linux.c:70:
This patch removes unnecessary braces and simplifies the function to a
single return statement.
Signed-off-by: Krzysztof Konopko
---
drivers/staging/rtl8188eu/os_dep/xmit_linux.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/xmit_linux.c
b/drivers/staging/rtl8188eu/os_dep/xmit_linux.c
index d5e41a5..5acf9a9 100644
--- a/drivers/staging/rtl8188eu/os_dep/xmit_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/xmit_linux.c
@@ -66,13 +66,7 @@ uint _rtw_pktfile_read(struct pkt_file *pfile, u8 *rmem,
uint rlen)
int rtw_endofpktfile(struct pkt_file *pfile)
{
-
- if (pfile->pkt_len == 0) {
- return true;
- }
-
-
- return false;
+ return pfile->pkt_len == 0;
}
int rtw_os_xmit_resource_alloc(struct adapter *padapter, struct xmit_buf
*pxmitbuf, u32 alloc_sz)
--
2.1.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723au: Fix sparse warnings
Some struct fields in wifi.h are meant to be __le16 bu were declared as
unsigned short. This was reported by sparse:
rtw_wlan_util.c:538:24: warning: cast to restricted __le16
rtw_wlan_util.c:1544:29: warning: cast to restricted __le16
rtw_wlan_util.c:1546:25: warning: cast to restricted __le16
This patch changes declared types of the struct fields involved.
Signed-off-by: Krzysztof Konopko
---
drivers/staging/rtl8723au/include/wifi.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtl8723au/include/wifi.h
b/drivers/staging/rtl8723au/include/wifi.h
index fd3da3b..8a2adc5 100644
--- a/drivers/staging/rtl8723au/include/wifi.h
+++ b/drivers/staging/rtl8723au/include/wifi.h
@@ -28,7 +28,7 @@
struct AC_param {
unsigned char ACI_AIFSN;
unsigned char CW;
- unsigned short TXOP_limit;
+ __le16 TXOP_limit;
} __packed;
struct WMM_para_element {
@@ -39,9 +39,9 @@ struct WMM_para_element {
struct ADDBA_request {
unsigned char dialog_token;
- unsigned short BA_para_set;
+ __le16 BA_para_set;
unsigned short BA_timeout_value;
- unsigned short BA_starting_seqctrl;
+ __le16 BA_starting_seqctrl;
} __packed;
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: Fix sparse warnings
On 12/12/14 00:53, Larry Finger wrote:
> On 12/11/2014 04:23 PM, Krzysztof Konopko wrote:
>> Some struct fields in wifi.h are meant to be __le16 bu were declared as
>> unsigned short. This was reported by sparse:
>>
>>rtw_wlan_util.c:538:24: warning: cast to restricted __le16
>>rtw_wlan_util.c:1544:29: warning: cast to restricted __le16
>>rtw_wlan_util.c:1546:25: warning: cast to restricted __le16
>>
>> This patch changes declared types of the struct fields involved.
>>
>> Signed-off-by: Krzysztof Konopko
>> ---
>> drivers/staging/rtl8723au/include/wifi.h | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8723au/include/wifi.h
>> b/drivers/staging/rtl8723au/include/wifi.h
>> index fd3da3b..8a2adc5 100644
>> --- a/drivers/staging/rtl8723au/include/wifi.h
>> +++ b/drivers/staging/rtl8723au/include/wifi.h
>> @@ -28,7 +28,7 @@
>> struct AC_param {
>> unsigned charACI_AIFSN;
>> unsigned charCW;
>> -unsigned shortTXOP_limit;
>> +__le16TXOP_limit;
>> } __packed;
>>
>> struct WMM_para_element {
>> @@ -39,9 +39,9 @@ struct WMM_para_element {
>>
>> struct ADDBA_request {
>> unsigned chardialog_token;
>> -unsigned shortBA_para_set;
>> +__le16BA_para_set;
>> unsigned shortBA_timeout_value;
>> -unsigned shortBA_starting_seqctrl;
>> +__le16BA_starting_seqctrl;
>> } __packed;
>
> This fix may make the sparse warnings go away, but I think it introduces
> new bugs.
Right, I see. Nice try though, isn't it? ;)
> In particular, did you test on big-endian hardware after you
> made this change?
Nope. I don't have any big-endian hardware. I don't even have the
wireless card TBH. But I'm happy to try to get one. Is Rtl8723AE the
right model?
> I recently found that the driver for RTL8188EU needed
> to have BA_para_set to unsigned short, and the endianess warnings needed
> to be fixed in the code. Then it would work on my PowerBook G4 with a
> PPC processor.
>
OK. Does it still work with little endian?
> In RTL8188EU, both BA_starting_seqctrl and TXOP_limit are unsigned short.
>
That's not quite the case. `TXOP_limit` is __le16 in RTL8188EU [1].
It's __le16 even in your GitHub repo [2]. And that made me thinking
that there's probably some inconsistency in the header.
I'm _far_ from being a wireless expert but doesn't data coming out of
the wire/air have the endianess defined explicitly? And both `AC_param`
and `ADDBA_request` come out of air?
I was hunting particularly for inconsistencies with `sparse` and came
across this one. But I dug a bit further and I wonder why the driver is
not using standard stuff like the one in `include/linux/ieee80211.h`
where any data wider than one byte is clearly declared as __le?
Cheers,
Kris
--
[1] drivers/staging/rtl8188eu/include/wifi.h as of next-20141211
[2] https://github.com/lwfinger/rtl8188eu/blob/master/include/wifi.h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: Fix sparse warnings
On 12/12/14 17:43, Larry Finger wrote:
> On 12/12/2014 06:52 AM, Dan Carpenter wrote:
>> On Thu, Dec 11, 2014 at 05:53:30PM -0600, Larry Finger wrote:
>>> On 12/11/2014 04:23 PM, Krzysztof Konopko wrote:
>>>> Some struct fields in wifi.h are meant to be __le16 bu were declared as
>>>> unsigned short. This was reported by sparse:
>>>>
>>>>rtw_wlan_util.c:538:24: warning: cast to restricted __le16
>>>>rtw_wlan_util.c:1544:29: warning: cast to restricted __le16
>>>>rtw_wlan_util.c:1546:25: warning: cast to restricted __le16
>>>>
>>>> This patch changes declared types of the struct fields involved.
>>>>
>>>> Signed-off-by: Krzysztof Konopko
>>>> ---
>>>> drivers/staging/rtl8723au/include/wifi.h | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/rtl8723au/include/wifi.h
>>>> b/drivers/staging/rtl8723au/include/wifi.h
>>>> index fd3da3b..8a2adc5 100644
>>>> --- a/drivers/staging/rtl8723au/include/wifi.h
>>>> +++ b/drivers/staging/rtl8723au/include/wifi.h
>>>> @@ -28,7 +28,7 @@
>>>> struct AC_param {
>>>> unsigned charACI_AIFSN;
>>>> unsigned charCW;
>>>> -unsigned shortTXOP_limit;
>>>> +__le16TXOP_limit;
>>>> } __packed;
>>>>
>>>> struct WMM_para_element {
>>>> @@ -39,9 +39,9 @@ struct WMM_para_element {
>>>>
>>>> struct ADDBA_request {
>>>> unsigned chardialog_token;
>>>> -unsigned shortBA_para_set;
>>>> +__le16BA_para_set;
>>>> unsigned shortBA_timeout_value;
>>>> -unsigned shortBA_starting_seqctrl;
>>>> +__le16BA_starting_seqctrl;
>>>> } __packed;
>>>
>>> This fix may make the sparse warnings go away, but I think it
>>> introduces new bugs.
>>
>> This kind of change, doesn't change the compiled code only how Sparse
>> sees it. It can't introduce bugs.
>>
>> But it may well be that the calls to le16_to_cpu() should be removed. I
>> looked at it a bit but I don't know.
>
> Your point regarding bugs is taken. What I should have said is that
> blindly making _le changes to hide Sparse messages may hide existing
> bugs for BE hardware.
>
> Larry
>
>
Yes, I started it off blindly but dug further and now have a better
understanding. Looking in ieee80211.h and getting your feedback helped
me to get a better understanding of the situation.
I see nothing wrong in declaring data that is supposed to be
little-endian as __le. You say that making these changes blindly may
hide existing bugs but:
* not doing anything about it is not helpful either
* this is no longer changing anything blindly
Relevant structs: `addba_req` and `ieee80211_wmm_ac_param` do declare
their fields as __le where needed.
I do take a point though about making this change inconsistently
(blindly) in my initial patch.
Kris
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: Fix sparse warnings
On 12/12/14 18:12, Jes Sorensen wrote:
> Krzysztof Konopko writes:
>> Some struct fields in wifi.h are meant to be __le16 bu were declared as
>> unsigned short. This was reported by sparse:
>>
>> rtw_wlan_util.c:538:24: warning: cast to restricted __le16
>> rtw_wlan_util.c:1544:29: warning: cast to restricted __le16
>> rtw_wlan_util.c:1546:25: warning: cast to restricted __le16
>>
>> This patch changes declared types of the struct fields involved.
>>
>> Signed-off-by: Krzysztof Konopko
>> ---
>> drivers/staging/rtl8723au/include/wifi.h | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8723au/include/wifi.h
>> b/drivers/staging/rtl8723au/include/wifi.h
>> index fd3da3b..8a2adc5 100644
>> --- a/drivers/staging/rtl8723au/include/wifi.h
>> +++ b/drivers/staging/rtl8723au/include/wifi.h
>> @@ -28,7 +28,7 @@
>> struct AC_param {
>> unsigned char ACI_AIFSN;
>> unsigned char CW;
>> -unsigned short TXOP_limit;
>> +__le16 TXOP_limit;
>> } __packed;
>>
>> struct WMM_para_element {
>> @@ -39,9 +39,9 @@ struct WMM_para_element {
>>
>> struct ADDBA_request {
>> unsigned char dialog_token;
>> -unsigned short BA_para_set;
>> +__le16 BA_para_set;
>> unsigned short BA_timeout_value;
>> -unsigned short BA_starting_seqctrl;
>> +__le16 BA_starting_seqctrl;
>> } __packed;
>
> If you are going to make the struct comply with the on-wire data format,
> be consistent. Don't just change half the elements of the struct - that
> will just lead to confusion.
>
> Jes
>
Yes, my change was inconsistent. Looking at `addba_req` and
`ieee80211_wmm_ac_param` in include/linux/ieee80211.h, all data wider
than 1 byte should be declared as __le. I'll send through a patch that
makes this change consistently.
Thanks,
Kris
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: Fix sparse warnings
On 12/12/14 18:35, Larry Finger wrote: > On 12/12/2014 05:35 AM, Krzysztof Konopko wrote: >> On 12/12/14 00:53, Larry Finger wrote: >>> In particular, did you test on big-endian hardware after you >>> made this change? >> >> Nope. I don't have any big-endian hardware. I don't even have the >> wireless card TBH. But I'm happy to try to get one. Is Rtl8723AE the >> right model? > > No. The device numbers that end in E are PCIe and use a mac80211-based > driver. As none of my BE hardware has PCIe, I cannot test those drivers > on other than LE hardware. I do not have the hardware either for the > RTL8723AU. For that reason, I am careful when modifying the driver - I > let Jes do that. > Silly me. 'U' stands for USB here. But can't find this device on any auction. It's included in some ultrabooks but can't afford that for the sake of fixing some sparse warnings :) >>> In RTL8188EU, both BA_starting_seqctrl and TXOP_limit are unsigned >>> short. >>> >> >> That's not quite the case. `TXOP_limit` is __le16 in RTL8188EU [1]. >> It's __le16 even in your GitHub repo [2]. And that made me thinking >> that there's probably some inconsistency in the header. > > All the USB drivers are a mess. The kernel version of rtl8188eu does not > work on PPC; however, the git repo now does. I'm working on finding the > differences and fixing the kernel version. > Right. I found your introductory message: https://lkml.org/lkml/2013/4/1/280 >> I was hunting particularly for inconsistencies with `sparse` and came >> across this one. But I dug a bit further and I wonder why the driver is >> not using standard stuff like the one in `include/linux/ieee80211.h` >> where any data wider than one byte is clearly declared as __le? > > That is a good question. One possibility is that those definitions do > not exist on some of the older kernels that Realtek supports. They > generally work with 2.6.18 and newer. > That would be important if the driver was kept out of the tree. Isn't it the point of having the driver in the mainline to keep up with the kernel and don't bother with older versions? > To be able to fix the kernel driver for RTL8188EU on PPC, I need to sort > out these endian problems. Once I do, I will port them to the other > drivers. > Isn't `sparse` useful here? :) Kris ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: Fix sparse warnings
On 12/12/14 19:52, Jes Sorensen wrote: > Larry Finger writes: >> On 12/12/2014 05:35 AM, Krzysztof Konopko wrote: >>> I was hunting particularly for inconsistencies with `sparse` and came >>> across this one. But I dug a bit further and I wonder why the driver is >>> not using standard stuff like the one in `include/linux/ieee80211.h` >>> where any data wider than one byte is clearly declared as __le? >> >> That is a good question. One possibility is that those definitions do >> not exist on some of the older kernels that Realtek supports. They >> generally work with 2.6.18 and newer. > > The reason the 8723au driver doesn't use the defines from there is that > in ieee80211.h they are part of struct ieee80211_mgmt, while the 8723au > driver access the addba etc. elements without the full struct in place. > And why is that the case? (I'm trying to understand, not debunk) Looks to me that this driver has been kept out of the tree for quite a while (by Realtek) and now suffers from locally invented stuff. I understand this is a lot of work to unify the codebase with ieee80211.h, but are there any technical hurdles? I'm just curious. Thanks, Kris ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723au: Fix sparse warnings
Some struct fields in wifi.h are meant to be __le16 but were declared as
unsigned short. This was reported by sparse:
rtw_wlan_util.c:538:24: warning: cast to restricted __le16
rtw_wlan_util.c:1544:29: warning: cast to restricted __le16
rtw_wlan_util.c:1546:25: warning: cast to restricted __le16
This patch changes the types of the struct fields involved to be
little-endian which is what is received over the air and consistent with
relevant structs in include/linux/ieee80211.h.
Signed-off-by: Krzysztof Konopko
---
drivers/staging/rtl8723au/include/wifi.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8723au/include/wifi.h
b/drivers/staging/rtl8723au/include/wifi.h
index fd3da3b..266c43e 100644
--- a/drivers/staging/rtl8723au/include/wifi.h
+++ b/drivers/staging/rtl8723au/include/wifi.h
@@ -28,7 +28,7 @@
struct AC_param {
unsigned char ACI_AIFSN;
unsigned char CW;
- unsigned short TXOP_limit;
+ __le16 TXOP_limit;
} __packed;
struct WMM_para_element {
@@ -39,9 +39,9 @@ struct WMM_para_element {
struct ADDBA_request {
unsigned char dialog_token;
- unsigned short BA_para_set;
- unsigned short BA_timeout_value;
- unsigned short BA_starting_seqctrl;
+ __le16 BA_para_set;
+ __le16 BA_timeout_value;
+ __le16 BA_starting_seqctrl;
} __packed;
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723au: Fix sparse warnings
Some struct fields in wifi.h are meant to be __le16 but were declared as
unsigned short. This was reported by sparse:
rtw_wlan_util.c:538:24: warning: cast to restricted __le16
rtw_wlan_util.c:1544:29: warning: cast to restricted __le16
rtw_wlan_util.c:1546:25: warning: cast to restricted __le16
This patch updates the types of the fields in `AC_param` and
`ADDBA_request` structs to be consistent with relevant structs in
include/linux/ieee80211.h.
Signed-off-by: Krzysztof Konopko
---
drivers/staging/rtl8723au/include/wifi.h | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/rtl8723au/include/wifi.h
b/drivers/staging/rtl8723au/include/wifi.h
index fd3da3b..25d573c 100644
--- a/drivers/staging/rtl8723au/include/wifi.h
+++ b/drivers/staging/rtl8723au/include/wifi.h
@@ -26,9 +26,9 @@
--*/
struct AC_param {
- unsigned char ACI_AIFSN;
- unsigned char CW;
- unsigned short TXOP_limit;
+ u8 ACI_AIFSN;
+ u8 CW;
+ __le16 TXOP_limit;
} __packed;
struct WMM_para_element {
@@ -38,10 +38,10 @@ struct WMM_para_element {
} __packed;
struct ADDBA_request {
- unsigned char dialog_token;
- unsigned short BA_para_set;
- unsigned short BA_timeout_value;
- unsigned short BA_starting_seqctrl;
+ u8 dialog_token;
+ __le16 BA_para_set;
+ __le16 BA_timeout_value;
+ __le16 BA_starting_seqctrl;
} __packed;
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
