On Mon, 2008-10-06 at 20:18 +0200, Marcel Holtmann wrote:
> > +static inline u8 iwl_get_num_tbs(struct iwl_tfd_frame *frame)
> > +{
> > + return frame->num_tbs & 0x1f;
> > +}
> > +
> > +static inline void iwl_set_num_tbs(struct iwl_tfd_frame *frame, u8 num_tbs)
> > +{
> > + BUG_ON(num_tbs >= 20);
> > +
> > + frame->num_tbs &= ~0x1f;
> > + frame->num_tbs |= num_tbs;
> > +}
>
> what is this magic doing and what is wrong with
>
> frame->num_tbs = num_tbs & 0x1f
>
> Or do you expect to do something with the upper 3 bits?
I don't know. I was just keeping the original semantics. Maybe future hw
will use those bits? Maybe they are already used and need to be
preserved? This hw programming stuff in the driver is all black magic to
me.
> Personally I think it is important that we always zero any data
> structure and especially the reserved bits of it. Otherwise stuff just
> magically seems to work, but it is pure luck if the right bits are set.
Well, that doesn't just affect that but all other reserved fields too,
so we really need to kzalloc everything anyway.
> The overall patch looks good to me and I think it is the way to go. Less
> home grown bit magic the better.
:)
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
