New subsystems: Ultra-Wideband radio, Wireless USB and WiMedia LLC Protocol
Subsystems (2)
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
Signature · 3 lines
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 1/1 bitmap: add bitmap_copy_le()
Signature · 3 lines
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/
bitmap: add bitmap_copy_le()
From: David Vrabel <[email protected]>
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.
--- include/linux/bitmap.h | 1 + lib/bitmap.c | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+)
include/linux/bitmap.h +1 -0
@@ -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) \ ( \
lib/bitmap.c +22 -0
@@ -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 1/1 Add helper macros for little-endian bitfields
In reply to David VrabelSignature · 3 lines
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/
Add helper macros for little-endian bitfields
From: Inaky Perez-Gonzalez <[email protected]>
This adds some macros to help in defining structures with little-endian values with bitfields. Useful when defining hardware interfaces.
--- include/linux/byteorder/bitfields.h | 114 ++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+)
include/linux/byteorder/bitfields.h new file +114 -0
@@ -0,0 +1,114 @@ +/* +* Helpers for defining little-endian bitfields +* +* Copyright (C) 2007 Intel Corporation +* Inaky Perez-Gonzalez <[email protected]> +* +* 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
Quoted 10 lines
+ * 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
Quoted 9 lines
+ * ) __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
Quoted 3 lines · Roland Dreier
Quoted 3 lines
+ * 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
Quoted 9 lines · Al Viro
Quoted 3 lines · Roland Dreier
Quoted 3 lines
+ * 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
Signature · 3 lines
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
Quoted 21 lines · David Vrabel
Quoted 11 lines · Al Viro
Quoted 4 lines · Roland Dreier
Quoted 3 lines
+ * 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,
Quoted 9 lines
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
In reply to David VrabelQuoted 4 lines · David Vrabel
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
Quoted 9 lines · 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/.
Quoted 6 lines
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
Quoted 3 lines
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
Quoted 16 lines · 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:
Quoted 8 lines
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.
Quoted 7 lines
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 :)
Quoted 2 lines
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
Quoted 4 lines · 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.
Quoted 8 lines
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
Signature · 3 lines
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/