On 12/12/2014 05:35 AM, Krzysztof Konopko wrote:
On 12/12/14 00:53, Larry Finger wrote:
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?
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.
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?
Of course. Making changes between u16 and __le16 do not make any difference in
the code for little endian. As Dan Carpenter pointed out, these changes cannot
introduce any bugs. I should have said that the changes could obscure bugs for
big-endian systems.
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.
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?
Yes, in the air the endianess is explicitly defined. I'm not sure how AC_param
is populated, but the data in ADDBA_request are individually extracted/inserted
from/to the on-air packets. They could be kept as little endian, but the
calculations on BA_para_set are being done in CPU endianess. For consistency
with the kernel headers you mention below, the structures should probably use LE
and the calculations modified.
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<nn>?
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.
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.
Larry
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
