[PATCH] Marvell Libertas 8388 802.11b/g USB driver

[PATCH] Marvell Libertas 8388 802.11b/g USB driver

[Marcelo Tosatti]

Hi, 

Marvell released a GPL driver for their 8388 wireless chip for USB
interface back in April.

The 8388 is the device being used by the OLPC laptop.

We have been working on cleaning it up for mainline inclusion. I think
the moment for wider review has arrived.

Since the diff is pretty large (700K) I've put it on the web at:
http://dev.laptop.org/~marcelo/libertas-8388-15122006.patch

And its GIT tree resides at:
http://git.infradead.org/?p=libertas-2.6.git;a=log

Comments are welcome

Re: [PATCH] Marvell Libertas 8388 802.11b/g USB driver

[Michael Wu]

[-- Attachment #1: Type: text/plain, Size: 1683 bytes --]

On Friday 15 December 2006 17:51, Marcelo Tosatti wrote:
>--- a/include/net/ieee80211_radiotap.h
>+++ b/include/net/ieee80211_radiotap.h
>@@ -168,6 +168,23 @@ struct ieee80211_radiotap_header {
>  *      Unitless indication of the Rx/Tx antenna for this packet.
>  *      The first antenna is antenna 0.
>  *
>+ * IEEE80211_RADIOTAP_RX_FLAGS          u_int16_t       bitmap
>+ *
>+ *     Properties of received frames. See flags defined below.
>+ *
>+ * IEEE80211_RADIOTAP_TX_FLAGS          u_int16_t       bitmap
>+ *
>+ *     Properties of transmitted frames. See flags defined below.
>+ *
>+ * IEEE80211_RADIOTAP_RTS_RETRIES       u_int8_t        data
>+ *
>+ *     Number of rts retries a transmitted frame used.
>+ *
>+ * IEEE80211_RADIOTAP_DATA_RETRIES      u_int8_t        data
>+ *
>+ *     Number of unicast retries a transmitted frame used.
>+ *
>+ *
>  * IEEE80211_RADIOTAP_FCS              u32       data
>  *
>  *     FCS from frame in network byte order.
>@@ -187,7 +204,11 @@ enum ieee80211_radiotap_type {
>        IEEE80211_RADIOTAP_ANTENNA = 11,
>        IEEE80211_RADIOTAP_DB_ANTSIGNAL = 12,
>        IEEE80211_RADIOTAP_DB_ANTNOISE = 13,
>-       IEEE80211_RADIOTAP_EXT = 31,
>+       IEEE80211_RADIOTAP_RX_FLAGS = 14,
>+       IEEE80211_RADIOTAP_TX_FLAGS = 15,
>+       IEEE80211_RADIOTAP_RTS_RETRIES = 16,
>+       IEEE80211_RADIOTAP_DATA_RETRIES = 17,
>+       IEEE80211_RADIOTAP_EXT = 31
> };
>
> /* Channel flags. */
Did you send this part to netbsd also? We really don't want to fork 
radiotap. ;) Also, this should be in a separate patch, but I'm guessing it's 
all rolled together for convenience.

-Michael Wu

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Marvell Libertas 8388 802.11b/g USB driver

[David Young]

On Fri, Dec 15, 2006 at 09:52:20PM -0500, Michael Wu wrote:
> On Friday 15 December 2006 17:51, Marcelo Tosatti wrote:
> >--- a/include/net/ieee80211_radiotap.h
> >+++ b/include/net/ieee80211_radiotap.h
> >@@ -168,6 +168,23 @@ struct ieee80211_radiotap_header {
> >  *      Unitless indication of the Rx/Tx antenna for this packet.
> >  *      The first antenna is antenna 0.
> >  *
> >+ * IEEE80211_RADIOTAP_RX_FLAGS          u_int16_t       bitmap
> >+ *
> >+ *     Properties of received frames. See flags defined below.
> >+ *
> >+ * IEEE80211_RADIOTAP_TX_FLAGS          u_int16_t       bitmap
> >+ *
> >+ *     Properties of transmitted frames. See flags defined below.
> >+ *
> >+ * IEEE80211_RADIOTAP_RTS_RETRIES       u_int8_t        data
> >+ *
> >+ *     Number of rts retries a transmitted frame used.
> >+ *
> >+ * IEEE80211_RADIOTAP_DATA_RETRIES      u_int8_t        data
> >+ *
> >+ *     Number of unicast retries a transmitted frame used.
> >+ *
> >+ *
> >  * IEEE80211_RADIOTAP_FCS              u32       data
> >  *
> >  *     FCS from frame in network byte order.
> >@@ -187,7 +204,11 @@ enum ieee80211_radiotap_type {
> >        IEEE80211_RADIOTAP_ANTENNA = 11,
> >        IEEE80211_RADIOTAP_DB_ANTSIGNAL = 12,
> >        IEEE80211_RADIOTAP_DB_ANTNOISE = 13,
> >-       IEEE80211_RADIOTAP_EXT = 31,
> >+       IEEE80211_RADIOTAP_RX_FLAGS = 14,
> >+       IEEE80211_RADIOTAP_TX_FLAGS = 15,
> >+       IEEE80211_RADIOTAP_RTS_RETRIES = 16,
> >+       IEEE80211_RADIOTAP_DATA_RETRIES = 17,
> >+       IEEE80211_RADIOTAP_EXT = 31
> > };
> >
> > /* Channel flags. */
> Did you send this part to netbsd also? We really don't want to fork 
> radiotap. ;) Also, this should be in a separate patch, but I'm guessing it's 
> all rolled together for convenience.

No, especially since NetBSD is where I keep the authoritative definitions.

How have you defined RX_FLAGS and TX_FLAGS?

BTW, IEEE80211_RADIOTAP_FCS (above) never made it into radiotap.  No bit
is reserved.

Dave

-- 
David Young             OJC Technologies
dyoung@ojctech.com      Urbana, IL * (217) 278-3933

Re: [PATCH] Marvell Libertas 8388 802.11b/g USB driver

[Sam Leffler]

David Young wrote:
> On Fri, Dec 15, 2006 at 09:52:20PM -0500, Michael Wu wrote:
>> On Friday 15 December 2006 17:51, Marcelo Tosatti wrote:
>>> --- a/include/net/ieee80211_radiotap.h
>>> +++ b/include/net/ieee80211_radiotap.h
>>> @@ -168,6 +168,23 @@ struct ieee80211_radiotap_header {
>>>  *      Unitless indication of the Rx/Tx antenna for this packet.
>>>  *      The first antenna is antenna 0.
>>>  *
>>> + * IEEE80211_RADIOTAP_RX_FLAGS          u_int16_t       bitmap
>>> + *
>>> + *     Properties of received frames. See flags defined below.
>>> + *
>>> + * IEEE80211_RADIOTAP_TX_FLAGS          u_int16_t       bitmap
>>> + *
>>> + *     Properties of transmitted frames. See flags defined below.
>>> + *
>>> + * IEEE80211_RADIOTAP_RTS_RETRIES       u_int8_t        data
>>> + *
>>> + *     Number of rts retries a transmitted frame used.
>>> + *
>>> + * IEEE80211_RADIOTAP_DATA_RETRIES      u_int8_t        data
>>> + *
>>> + *     Number of unicast retries a transmitted frame used.
>>> + *
>>> + *
>>>  * IEEE80211_RADIOTAP_FCS              u32       data
>>>  *
>>>  *     FCS from frame in network byte order.
>>> @@ -187,7 +204,11 @@ enum ieee80211_radiotap_type {
>>>        IEEE80211_RADIOTAP_ANTENNA = 11,
>>>        IEEE80211_RADIOTAP_DB_ANTSIGNAL = 12,
>>>        IEEE80211_RADIOTAP_DB_ANTNOISE = 13,
>>> -       IEEE80211_RADIOTAP_EXT = 31,
>>> +       IEEE80211_RADIOTAP_RX_FLAGS = 14,
>>> +       IEEE80211_RADIOTAP_TX_FLAGS = 15,
>>> +       IEEE80211_RADIOTAP_RTS_RETRIES = 16,
>>> +       IEEE80211_RADIOTAP_DATA_RETRIES = 17,
>>> +       IEEE80211_RADIOTAP_EXT = 31
>>> };
>>>
>>> /* Channel flags. */
>> Did you send this part to netbsd also? We really don't want to fork 
>> radiotap. ;) Also, this should be in a separate patch, but I'm guessing it's 
>> all rolled together for convenience.
> 
> No, especially since NetBSD is where I keep the authoritative definitions.
> 
> How have you defined RX_FLAGS and TX_FLAGS?
> 
> BTW, IEEE80211_RADIOTAP_FCS (above) never made it into radiotap.  No bit
> is reserved.

Tell that to everyone that implements it.

	Sam

Re: [PATCH] Marvell Libertas 8388 802.11b/g USB driver

[Sam Leffler]

Sam Leffler wrote:
> David Young wrote:
>> On Fri, Dec 15, 2006 at 09:52:20PM -0500, Michael Wu wrote:
>>> On Friday 15 December 2006 17:51, Marcelo Tosatti wrote:
>>>> --- a/include/net/ieee80211_radiotap.h
>>>> +++ b/include/net/ieee80211_radiotap.h
>>>> @@ -168,6 +168,23 @@ struct ieee80211_radiotap_header {
>>>>  *      Unitless indication of the Rx/Tx antenna for this packet.
>>>>  *      The first antenna is antenna 0.
>>>>  *
>>>> + * IEEE80211_RADIOTAP_RX_FLAGS          u_int16_t       bitmap
>>>> + *
>>>> + *     Properties of received frames. See flags defined below.
>>>> + *
>>>> + * IEEE80211_RADIOTAP_TX_FLAGS          u_int16_t       bitmap
>>>> + *
>>>> + *     Properties of transmitted frames. See flags defined below.
>>>> + *
>>>> + * IEEE80211_RADIOTAP_RTS_RETRIES       u_int8_t        data
>>>> + *
>>>> + *     Number of rts retries a transmitted frame used.
>>>> + *
>>>> + * IEEE80211_RADIOTAP_DATA_RETRIES      u_int8_t        data
>>>> + *
>>>> + *     Number of unicast retries a transmitted frame used.
>>>> + *
>>>> + *
>>>>  * IEEE80211_RADIOTAP_FCS              u32       data
>>>>  *
>>>>  *     FCS from frame in network byte order.
>>>> @@ -187,7 +204,11 @@ enum ieee80211_radiotap_type {
>>>>        IEEE80211_RADIOTAP_ANTENNA = 11,
>>>>        IEEE80211_RADIOTAP_DB_ANTSIGNAL = 12,
>>>>        IEEE80211_RADIOTAP_DB_ANTNOISE = 13,
>>>> -       IEEE80211_RADIOTAP_EXT = 31,
>>>> +       IEEE80211_RADIOTAP_RX_FLAGS = 14,
>>>> +       IEEE80211_RADIOTAP_TX_FLAGS = 15,
>>>> +       IEEE80211_RADIOTAP_RTS_RETRIES = 16,
>>>> +       IEEE80211_RADIOTAP_DATA_RETRIES = 17,
>>>> +       IEEE80211_RADIOTAP_EXT = 31
>>>> };
>>>>
>>>> /* Channel flags. */
>>> Did you send this part to netbsd also? We really don't want to fork 
>>> radiotap. ;) Also, this should be in a separate patch, but I'm guessing it's 
>>> all rolled together for convenience.
>> No, especially since NetBSD is where I keep the authoritative definitions.
>>
>> How have you defined RX_FLAGS and TX_FLAGS?
>>
>> BTW, IEEE80211_RADIOTAP_FCS (above) never made it into radiotap.  No bit
>> is reserved.
> 
> Tell that to everyone that implements it.
> 

My mistake.  David pointed out correctly that the mechanism for adding
the FCS out-of-line (IEEE80211_RADIOTAP_FCS) was not used.  Instead
there is a flag bit that tells whether or not FCS is present (inline) in
the data.  This flag bit is what I was thinking of--it's honored by
ethereal (aka wireshark), kismet, tcpdump, etc.

	Sam

Re: [PATCH] Marvell Libertas 8388 802.11b/g USB driver

[David Young]

On Mon, Dec 18, 2006 at 05:57:23PM -0200, Marcelo Tosatti wrote:
> 
> > > > /* Channel flags. */
> > > Did you send this part to netbsd also? We really don't want to fork 
> > > radiotap. ;) Also, this should be in a separate patch, but I'm guessing it's 
> > > all rolled together for convenience.
> > 
> > No, especially since NetBSD is where I keep the authoritative definitions.
> > 
> > How have you defined RX_FLAGS and TX_FLAGS?
> 
> Oh yes, missed that part of the patch. Sorry.

Sorry for the delayed response, it's been a busy couple of months.

There is now a mailing list for discussing radiotap.  Subscribe at
<http://mail.ojctech.com/mailman/listinfo/radiotap>.  I ask that you
send proposals for new fields to the mailing list for discussion.  I am
interested to see proposals for 802.11n.  A couple of people have already
promised me proposals, but they never sent them.

A couple words about the fields you mention:

> --- a/include/net/ieee80211_radiotap.h
> +++ b/include/net/ieee80211_radiotap.h
> @@ -168,6 +168,23 @@ struct ieee80211_radiotap_header {
>   *      Unitless indication of the Rx/Tx antenna for this packet.
>   *      The first antenna is antenna 0.
>   *
> + * IEEE80211_RADIOTAP_RX_FLAGS          u_int16_t       bitmap
> + *
> + *     Properties of received frames. See flags defined below.
> + *
> + * IEEE80211_RADIOTAP_TX_FLAGS          u_int16_t       bitmap
> + *
> + *     Properties of transmitted frames. See flags defined below.
> + *
> + * IEEE80211_RADIOTAP_RTS_RETRIES       u_int8_t        data
> + *
> + *     Number of rts retries a transmitted frame used.
> + *
> + * IEEE80211_RADIOTAP_DATA_RETRIES      u_int8_t        data
> + *
> + *     Number of unicast retries a transmitted frame used.
> + *
> + *
>   * IEEE80211_RADIOTAP_FCS           	u32       data
>   *
>   *	FCS from frame in network byte order.
> @@ -187,7 +204,11 @@ enum ieee80211_radiotap_type {
>  	IEEE80211_RADIOTAP_ANTENNA = 11,
>  	IEEE80211_RADIOTAP_DB_ANTSIGNAL = 12,
>  	IEEE80211_RADIOTAP_DB_ANTNOISE = 13,
> -	IEEE80211_RADIOTAP_EXT = 31,
> +	IEEE80211_RADIOTAP_RX_FLAGS = 14,
> +	IEEE80211_RADIOTAP_TX_FLAGS = 15,
> +	IEEE80211_RADIOTAP_RTS_RETRIES = 16,
> +	IEEE80211_RADIOTAP_DATA_RETRIES = 17,
> +	IEEE80211_RADIOTAP_EXT = 31
>  };

I remember discussing these fields, but they were for somebody's
experimental use.  All of the fields are acceptable to me, but this flag
is questionable; it duplicates the function of another flag:

> +#define IEEE80211_RADIOTAP_F_RX_BADFCS	0x0001	/* frame failed crc check */

If it is important to people that I add this flag, let's discuss on the
mailing list.

Dave

-- 
David Young             OJC Technologies
dyoung@ojctech.com      Urbana, IL * (217) 278-3933