New subsystems: Ultra-Wideband radio, Wireless USB and WiMedia LLC Protocol

New subsystems: Ultra-Wideband radio, Wireless USB and WiMedia LLC Protocol

[David Vrabel]

I'm the maintainer of the Ultra-Wideband (UWB) radio, Certified Wireless
USB (WUSB) and WiMedia LLC Protocol (WLP) subsystems.  These are now in
a state were I consider them suitable for inclusion into the kernel.

UWB is a new high speed, short range radio technology (480 Mbit/s at 1
m, 53.3 Mbit/s at 10 m).  The specifications are freely available and
are managed by the WiMedia Alliance.   For more information see
http://www.wimedia.org/.

Various protocols run on top of the UWB radio: Certified Wireless USB
(WUSB); WiMedia LLC Protocol (WLP), for IP (etc) networks; and (in the
future) high speed Bluetooth.

The code is some 35,000 lines so I won't be posting it to this list but
you can view the patch set here:

http://www.davidvrabel.org.uk/~david/linux/uwb/

Or you can pull the changes from the uwb branch of

git://pear.davidvrabel.org.uk/git/uwb.git

(Please don't clone the entire tree from here as I have very limited
bandwidth.)

Or you can view the patches in the linux-usb archive here:

http://thread.gmane.org/gmane.linux.usb.general/8715

The vast majority of the code forms part of the new UWB, WUSB, and WLP
subsystem with no impact on the rest of the kernel.  It has all been
posted to the linux-usb list and has been reviewed there.

For a wider review, I will follow up with the 2 minor patches that add
some simple infrastructure.

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/

[patch] bitmap: add bitmap_copy_le()

[David Vrabel]

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

-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/

[-- Attachment #2: bitmap-add-bitmap_copy_le.patch --]
[-- Type: text/x-diff, Size: 2140 bytes --]

bitmap: add bitmap_copy_le()

From: David Vrabel <david.vrabel@csr.com>

bitmap_copy_le() copies a bitmap, putting the bits into little-endian
order (i.e., each unsigned long word in the bitmap is put into
little-endian order).

The UWB stack used bitmaps to manage Medium Access Slot availability,
and these bitmaps need to be written to the hardware in LE order.

Signed-off-by: David Vrabel <david.vrabel@csr.com>
Cc: Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 include/linux/bitmap.h |    1 +
 lib/bitmap.c           |   22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+)

Index: linux-git/include/linux/bitmap.h
===================================================================
--- linux-git.orig/include/linux/bitmap.h	2008-08-12 14:51:02.000000000 +0100
+++ linux-git/include/linux/bitmap.h	2008-08-12 14:51:07.000000000 +0100
@@ -129,6 +129,7 @@
 extern int bitmap_find_free_region(unsigned long *bitmap, int bits, int order);
 extern void bitmap_release_region(unsigned long *bitmap, int pos, int order);
 extern int bitmap_allocate_region(unsigned long *bitmap, int pos, int order);
+extern void bitmap_copy_le(void *dst, const unsigned long *src, int nbits);
 
 #define BITMAP_LAST_WORD_MASK(nbits)					\
 (									\
Index: linux-git/lib/bitmap.c
===================================================================
--- linux-git.orig/lib/bitmap.c	2008-08-12 14:51:02.000000000 +0100
+++ linux-git/lib/bitmap.c	2008-08-12 14:51:07.000000000 +0100
@@ -996,3 +996,25 @@
 	return 0;
 }
 EXPORT_SYMBOL(bitmap_allocate_region);
+
+/**
+ * bitmap_copy_le - copy a bitmap, putting the bits into little-endian order.
+ * @dst:   destination buffer
+ * @src:   bitmap to copy
+ * @nbits: number of bits in the bitmap
+ *
+ * Require nbits % BITS_PER_LONG == 0.
+ */
+void bitmap_copy_le(void *dst, const unsigned long *src, int nbits)
+{
+	unsigned long *d = dst;
+	int i;
+
+	for (i = 0; i < nbits/BITS_PER_LONG; i++) {
+		if (BITS_PER_LONG == 64)
+			d[i] = cpu_to_le64(src[i]);
+		else
+			d[i] = cpu_to_le32(src[i]);
+	}
+}
+EXPORT_SYMBOL(bitmap_copy_le);

[patch] Add helper macros for little-endian bitfields

[David Vrabel]

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

-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/

[-- Attachment #2: add-linux-byteorder-bitfields.h.patch --]
[-- Type: text/x-diff, Size: 5487 bytes --]

Add helper macros for little-endian bitfields

From: Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>

This adds some macros to help in defining structures with little-endian
values with bitfields.  Useful when defining hardware interfaces.

Signed-off-by: David Vrabel <david.vrabel@csr.com>
---
 include/linux/byteorder/bitfields.h |  114 ++++++++++++++++++++++++++++++++++++
 1 file changed, 114 insertions(+)

Index: linux-2.6/include/linux/byteorder/bitfields.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/include/linux/byteorder/bitfields.h	2008-05-13 09:44:16.000000000 +0100
@@ -0,0 +1,114 @@
+/*
+ * Helpers for defining little-endian bitfields
+ *
+ * Copyright (C) 2007 Intel Corporation
+ * Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or 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-1301, USA.
+ */
+
+#ifndef __LINUX__BYTEORDER__BITFIELDS_H__
+#define __LINUX__BYTEORDER__BITFIELDS_H__
+
+#include <asm/byteorder.h>
+
+/*
+ * Declare Little Endian bitfields
+ *
+ * These macros are used to switch the order of the bitfields when
+ * packing with Little Endian in mind (ie: for mapping to a hw type
+ * that you have to send or read from a device).
+ *
+ * NOTE: When using multibyte bitfields, you need to convert the data
+ *       from Little Endian to CPU before you can access the bitfield
+ *       (to make it simpler):
+ *
+ *       union something {
+ *               le16 value;
+ *               DECL_BF_LE3(
+ *                       u16  bf0:3,
+ *                       u16  bf1:10,
+ *                       u16  bf2:3
+ *               ) __attribute__((packed));
+ *       };
+ *
+ *       ...
+ *
+ *       union something s;
+ *
+ *       s.value = le16_to_cpu(something LE read from hw);
+ *       frame_count = s.bf1;
+ *
+ */
+#ifdef __LITTLE_ENDIAN_BITFIELD
+#define DECL_BF_LE2(a, b)                   struct { a; b; }
+#define DECL_BF_LE3(a, b, c)                struct { a; b; c; }
+#define DECL_BF_LE4(a, b, c, d)             struct { a; b; c; d ; }
+#define DECL_BF_LE5(a, b, c, d, e)          struct { a; b; c; d; e; }
+#define DECL_BF_LE6(a, b, c, d, e, f)       struct { a; b; c; d; e; f; }
+#define DECL_BF_LE7(a, b, c, d, e, f, g)    struct { a; b; c; d; e; f; g; }
+#define DECL_BF_LE8(a, b, c, d, e, f, g, h) struct { a; b; c; d; e; f; g; h; }
+#define DECL_BF_LE9(a, b, c, d, e, f, g, h, i) \
+	struct { a; b; c; d; e; f; g; h; i; }
+#define DECL_BF_LE10(a, b, c, d, e, f, g, h, i, j) \
+	struct { a; b; c; d; e; f; g; h; i; j; }
+#define DECL_BF_LE11(a, b, c, d, e, f, g, h, i, j, k) \
+	struct { a; b; c; d; e; f; g; h; i; j; k; }
+#define DECL_BF_LE12(a, b, c, d, e, f, g, h, i, j, k, l) \
+	struct { a; b; c; d; e; f; g; h; i; j; k; l; }
+#define DECL_BF_LE13(a, b, c, d, e, f, g, h, i, j, k, l, m) \
+	struct { a; b; c; d; e; f; g; h; i; j; k; l; m; }
+#define DECL_BF_LE14(a, b, c, d, e, f, g, h, i, j, k, l, m, n) \
+	struct { a; b; c; d; e; f; g; h; i; j; k; l; m; n; }
+#define DECL_BF_LE15(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o) \
+	struct { a; b; c; d; e; f; g; h; i; j; k; l; m; n; o; }
+#define DECL_BF_LE16(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p) \
+	struct { a; b; c; d; e; f; g; h; i; j; k; l; m; n; o; p; }
+
+#else
+#ifdef __BIG_ENDIAN_BITFIELD
+
+#define DECL_BF_LE2(a, b)                   struct { b; a; }
+#define DECL_BF_LE3(a, b, c)                struct { c; b; a; }
+#define DECL_BF_LE4(a, b, c, d)             struct { d; c; b; a; }
+#define DECL_BF_LE5(a, b, c, d, e)          struct { e; d; c; b; a; }
+#define DECL_BF_LE6(a, b, c, d, e, f)       struct { f; e; d; c; b; a; }
+#define DECL_BF_LE7(a, b, c, d, e, f, g)    struct { g; f; e; d; c; b; a; }
+#define DECL_BF_LE8(a, b, c, d, e, f, g, h) struct { h; g; f; e; d; c; b; a; }
+#define DECL_BF_LE9(a, b, c, d, e, f, g, h, i) \
+	struct { i; h; g; f; e; d; c; b; a; }
+#define DECL_BF_LE10(a, b, c, d, e, f, g, h, i, j) \
+	struct { j; i; h; g; f; e; d; c; b; a; }
+#define DECL_BF_LE11(a, b, c, d, e, f, g, h, i, j, k) \
+	struct { k; j; i; h; g; f; e; d; c; b; a; }
+#define DECL_BF_LE12(a, b, c, d, e, f, g, h, i, j, k, l) \
+	struct { l; k; j; i; h; g; f; e; d; c; b; a; }
+#define DECL_BF_LE13(a, b, c, d, e, f, g, h, i, j, k, l, m) \
+	struct { m; l; k; j; i; h; g; f; e; d; c; b; a; }
+#define DECL_BF_LE14(a, b, c, d, e, f, g, h, i, j, k, l, m, n) \
+	struct { n; m; l; k; j; i; h; g; f; e; d; c; b; a; }
+#define DECL_BF_LE15(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o) \
+	struct { o; n; m; l; k; j; i; h; g; f; e; d; c; b; a; }
+#define DECL_BF_LE16(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p) \
+	struct { p; o; n; m; l; k; j; i; h; g; f; e; d; c; b; a; }
+
+#else
+
+#error Unknown endianess for Little Endian bitfield definition
+
+#endif
+#endif
+
+#endif /* #ifndef __LINUX__BYTEORDER__BITFIELDS_H__ */

Re: [patch] Add helper macros for little-endian bitfields

[Roland Dreier]

 > + * NOTE: When using multibyte bitfields, you need to convert the data
 > + *       from Little Endian to CPU before you can access the bitfield
 > + *       (to make it simpler):
 > + *
 > + *       union something {
 > + *               le16 value;
 > + *               DECL_BF_LE3(
 > + *                       u16  bf0:3,
 > + *                       u16  bf1:10,
 > + *                       u16  bf2:3
 > + *               ) __attribute__((packed));
 > + *       };
 > + *
 > + *       ...
 > + *
 > + *       union something s;
 > + *
 > + *       s.value = le16_to_cpu(something LE read from hw);
 > + *       frame_count = s.bf1;

I can't help thinking there's got to be a better way to do this... in
particular having 16 DECL_BF_LEnn() macros (and needing to count the
number of members every time you use one) is not particularly pretty.

In general the consensus with kernel code seems to be that you're better
off avoiding bitfields in structures, and just using shift/mask
operations to get at the data (with helper functions as needed).

However all of that is said without actually looking at the driver code
that uses this.

 - R.

Re: [patch] Add helper macros for little-endian bitfields

[Al Viro]

On Sun, Aug 24, 2008 at 06:37:43PM -0700, Roland Dreier wrote:
>  > + * NOTE: When using multibyte bitfields, you need to convert the data
>  > + *       from Little Endian to CPU before you can access the bitfield
>  > + *       (to make it simpler):

NOTE: When tempted to use multibyte bitfields on fixed-layout data, you
need to look in the mirror, ask yourself "what will they do to me during
code review for that?", shudder and decide that some temptations are
just not worth the pain.

IOW, don't do it.

Re: [patch] Add helper macros for little-endian bitfields

[David Vrabel]

Al Viro wrote:
> On Sun, Aug 24, 2008 at 06:37:43PM -0700, Roland Dreier wrote:
>>  > + * NOTE: When using multibyte bitfields, you need to convert the data
>>  > + *       from Little Endian to CPU before you can access the bitfield
>>  > + *       (to make it simpler):
> 
> NOTE: When tempted to use multibyte bitfields on fixed-layout data, you
> need to look in the mirror, ask yourself "what will they do to me during
> code review for that?", shudder and decide that some temptations are
> just not worth the pain.

But why is this worthy of a crispy flaming?  I've not seen anything
definite beyond a somewhat vague 'some compilers don't optimize
bitfields very well'.

The structure definition and the DECL_BF_LEx() macros might be ugly but
the code using the structures is clearer.  For example,

    get_random_bytes(&tiebreaker, sizeof(unsigned));
    drp_ie->tiebreaker = tiebreaker & 1;

versus

    get_random_bytes(&tiebreaker, sizeof(unsigned));
    drp_ie->drp_control |= (tiebreaker & 1)
            ? UWB_DRP_IE_DRP_CTRL_TIEBREAKER : 0;

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/

Re: [patch] Add helper macros for little-endian bitfields

[Jeremy Fitzhardinge]

David Vrabel wrote:
> Al Viro wrote:
>   
>> On Sun, Aug 24, 2008 at 06:37:43PM -0700, Roland Dreier wrote:
>>     
>>>  > + * NOTE: When using multibyte bitfields, you need to convert the data
>>>  > + *       from Little Endian to CPU before you can access the bitfield
>>>  > + *       (to make it simpler):
>>>       
>> NOTE: When tempted to use multibyte bitfields on fixed-layout data, you
>> need to look in the mirror, ask yourself "what will they do to me during
>> code review for that?", shudder and decide that some temptations are
>> just not worth the pain.
>>     
>
> But why is this worthy of a crispy flaming?  I've not seen anything
> definite beyond a somewhat vague 'some compilers don't optimize
> bitfields very well'.
>
> The structure definition and the DECL_BF_LEx() macros might be ugly but
> the code using the structures is clearer.  For example,
>
>     get_random_bytes(&tiebreaker, sizeof(unsigned));
>     drp_ie->tiebreaker = tiebreaker & 1;
>
> versus
>
>     get_random_bytes(&tiebreaker, sizeof(unsigned));
>     drp_ie->drp_control |= (tiebreaker & 1)
>             ? UWB_DRP_IE_DRP_CTRL_TIEBREAKER : 0;
>   

Why not just

	drp_ie->drp_control |= tiebreaker & UWB_DRP_IE_DRP_CTRL_TIEBREAKER;

then?  Doesn't matter which random bit you pick, does it?

    J

Re: [patch] Add helper macros for little-endian bitfields

[Linus Torvalds]

On Wed, 27 Aug 2008, David Vrabel wrote:
> 
> But why is this worthy of a crispy flaming?  I've not seen anything
> definite beyond a somewhat vague 'some compilers don't optimize
> bitfields very well'.

Actually, it's not that compilers often optimize bitfields really badly.

It's also that bitfields are a total f*cking disaster when it comes to 
endianness. As you found out.

Bitfields are fine if you don't actually care about the underlying format, 
and want gcc to just randomly assign bits, and want things to be 
convenient in that situation.

But _if_ you care about the underlying format, then inevitably the 
bitfield costs will be much higher than just using bit operations on a
"u32" or similar. Your helper macros are horrible compared to just making 
the bits work out right to begin with, and using the standard byte order 
things.

		Linus

Re: New subsystems: Ultra-Wideband radio, Wireless USB and WiMedia LLC Protocol

[Sam Ravnborg]

On Thu, Aug 21, 2008 at 02:17:36PM +0100, David Vrabel wrote:
> I'm the maintainer of the Ultra-Wideband (UWB) radio, Certified Wireless
> USB (WUSB) and WiMedia LLC Protocol (WLP) subsystems.  These are now in
> a state were I consider them suitable for inclusion into the kernel.
> 
> UWB is a new high speed, short range radio technology (480 Mbit/s at 1
> m, 53.3 Mbit/s at 10 m).  The specifications are freely available and
> are managed by the WiMedia Alliance.   For more information see
> http://www.wimedia.org/.
> 
> Various protocols run on top of the UWB radio: Certified Wireless USB
> (WUSB); WiMedia LLC Protocol (WLP), for IP (etc) networks; and (in the
> future) high speed Bluetooth.
> 
> The code is some 35,000 lines so I won't be posting it to this list but
> you can view the patch set here:

Can you post a serie of patches that just include the core parts - and no drivers?
Let see this first and then we can look at the drivers.

	Sam

Re: New subsystems: Ultra-Wideband radio, Wireless USB and WiMedia LLC Protocol

[Sam Ravnborg]

> 
> Can you post a serie of patches that just include the core parts - and no drivers?
> Let see this first and then we can look at the drivers.

Forget this again. Re'reading I realise you posted this for review at linux-usb.

	Sam

Re: New subsystems: Ultra-Wideband radio, Wireless USB and WiMedia LLC Protocol

[Greg KH]

On Thu, Aug 21, 2008 at 02:17:36PM +0100, David Vrabel wrote:
> I'm the maintainer of the Ultra-Wideband (UWB) radio, Certified Wireless
> USB (WUSB) and WiMedia LLC Protocol (WLP) subsystems.  These are now in
> a state were I consider them suitable for inclusion into the kernel.
> 
> UWB is a new high speed, short range radio technology (480 Mbit/s at 1
> m, 53.3 Mbit/s at 10 m).  The specifications are freely available and
> are managed by the WiMedia Alliance.   For more information see
> http://www.wimedia.org/.
> 
> Various protocols run on top of the UWB radio: Certified Wireless USB
> (WUSB); WiMedia LLC Protocol (WLP), for IP (etc) networks; and (in the
> future) high speed Bluetooth.
> 
> The code is some 35,000 lines so I won't be posting it to this list but
> you can view the patch set here:
> 
> http://www.davidvrabel.org.uk/~david/linux/uwb/
> 
> Or you can pull the changes from the uwb branch of
> 
> git://pear.davidvrabel.org.uk/git/uwb.git
> 
> (Please don't clone the entire tree from here as I have very limited
> bandwidth.)

If this is an issue, I think you can use the --reference option to
git-clone when creating the tree to reference an external tree (like
Linus's).  That way you don't have the whole tree on your server for
stuff like this.

> Or you can view the patches in the linux-usb archive here:
> 
> http://thread.gmane.org/gmane.linux.usb.general/8715
> 
> The vast majority of the code forms part of the new UWB, WUSB, and WLP
> subsystem with no impact on the rest of the kernel.  It has all been
> posted to the linux-usb list and has been reviewed there.

Well, it's been reviewed in parts, I didn't review this last version as
I stopped after the first two patches :)

> For a wider review, I will follow up with the 2 minor patches that add
> some simple infrastructure.

These are the ones that need to be approved by the wider lkml audience
before we get the rest of the tree in.

thanks,

greg k-h

Re: New subsystems: Ultra-Wideband radio, Wireless USB and WiMedia LLC Protocol

[David Vrabel]

David Vrabel wrote:
> I'm the maintainer of the Ultra-Wideband (UWB) radio, Certified Wireless
> USB (WUSB) and WiMedia LLC Protocol (WLP) subsystems.  These are now in
> a state were I consider them suitable for inclusion into the kernel.
> 
> UWB is a new high speed, short range radio technology (480 Mbit/s at 1
> m, 53.3 Mbit/s at 10 m).  The specifications are freely available and
> are managed by the WiMedia Alliance.   For more information see
> http://www.wimedia.org/.
> 
> Various protocols run on top of the UWB radio: Certified Wireless USB
> (WUSB); WiMedia LLC Protocol (WLP), for IP (etc) networks; and (in the
> future) high speed Bluetooth.

There is now a git tree for these subsystems.

git://git.kernel.org/pub/scm/linux/kernel/git/dvrabel/uwb.git

You will want the 'for-upstream' branch.  This should be finding its way
into linux-next shortly.

Linus, please advise on when a pull request for the 2.6.28 merge window
should be made.  e.g., should it be before the merge window opens or
during the window?

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/