Search Linux Wireless

Re: Please pull 'adm8211' branch of wireless-2.6

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Saturday 15 September 2007 20:56, Jeff Garzik wrote:
> >>> +	if (flags & IFF_PROMISC)
> >>> +		dev->flags |= IEEE80211_HW_RX_INCLUDES_FCS;
> >>> +	else
> >>> +		dev->flags &= ~IEEE80211_HW_RX_INCLUDES_FCS;
> >>
> >> why does promisc dictate inclusion of FCS?
> >
> > Because that's the way the hardware works.
>
> Why not always include it, regardless of promisc?
>
I really do mean that's how the hardware works. If you turn on the promisc bit 
in the hardware (which IFF_PROMISC causes), it starts including the FCS, but 
if the bit is not set, the FCS is not included in frames.

> >>> +	} while (count++ < 20);
> >>
> >> NAK -- talk about back to the past.
> >>
> >> It's bogus to loop in the interrupt handler, then loop again in both RX
> >> and TX paths.  You are getting close to reinventing the wheel here.
> >>
> >> Use NAPI rather than doing such a loop.
> >
> > This is old interrupt handler code from Jouni's original driver. I never
> > bothered to replace it with the simpler designs used in newer drivers
> > I've worked on.
> >
> > Also - mac80211 drivers have no access to NAPI.
>
> Not true as of net-2.6.24.
>
Huh? How does a driver use NAPI if it can't pass a struct net_device?

> >>> +/* CSR (Host Control and Status Registers) */
> >>> +struct adm8211_csr {
> >>
> >> [snip]
> >>
> >>> +} __attribute__ ((packed));
> >>
> >> attributed(packed) is unneccesary here, and its use results in
> >> sub-optimal code
> >
> > How? Doesn't this just turn into a bunch of offsets?
>
> It depends on how its used in the code.
>
I don't think I'm using it in a way that would make a difference.

> >> enums are preferred.  they do not disappear at the cpp stage, and confer
> >> type information.
> >
> > I'd rather not.
>
> Elaboration?
>
What form of debugging are you talking about? I don't see how it makes a 
difference for debugging. The type checking provided by enums won't make a 
difference for my code - any problems with using the wrong register bits in 
the wrong place is obvious due to the prefixes. I don't really see how enum 
type checking is even effective at all without annotations and casts all over 
the place.

Thanks,
-Michael Wu

Attachment: pgpOIXwp7fIKS.pgp
Description: PGP signature


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux