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/
--
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/
--
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/
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
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
>
> 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
> + * 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.
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.
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/
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
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