On Tue, Nov 14, 2006 at 06:30:20PM +0800, Judy Chen wrote: > Renee, > Thanks for the review and sorry for the late reply. Below is our response to > your comments. Please let us know if you have any additional questions.
Thanks for the responses; these all sound fine. A few replies on specific items are in-line. -renee > ------------------------------------------------------------------------------- > FEEDBACK ON TECHNICAL DOCUMENTATION/CODE > ------------------------------------------------------------------------------- > > Reviewer Name: Renee Danson > > Document/Module Title: wifi-1018-diff > > Document/Module Version/Date: 11/10/06 > > No comments: > usr/src/uts/common/io/ath/ath_impl.h > > ------------------------------------------------------------------------------- > usr/src/uts/common/io/ath/ath_aux.c > ------------------------------------------------------------------------------- > RD-17 289-290 Cos You should be able to merge these two lines. > > | DISCUSS > | Lines 289-290 are: > | > | uint16_t flags; > | ix = ath_hal_mhz2ieee(ah, c->channel, c->channelFlags); > | > | In what way should the be merged? Heh, good question. I'm sorry, I typed the wrong line numbers; I was referring to lines 275 and 276. > RD-18 298-299 Com I'm puzzled by the if conditional and the > comment; they don't seem to match. And I can't > find the implementation of ath_hal_mhz2ieee to > figure out what ix really is. This is probably > okay, I'm just confused; but I'd like to > understand it! > > | EXPLAIN > | Function ath_hal_mhz2ieee() is defined in ath HAL, which is provided > | by opensource MadWifi project in binary only form, thus no source > | code for it. It's used to convert a frequence value into IEEE > | channel number. For example, frequency 2412MHz is IEEE802.11b/g > | channel 1. So here 'ix' means channel number, which is used to > | index into array ic_sup_channels[]. > | For the comments > | /* can't handle stuff <2400 right now */ > | As explain previously, 2412MHz is IEEE802.11b/g channel 1. But > | some chipsets supports frequency less than 2412 MHz, like 2372MHz. > | In this case, channel number is returned as negative value. Here > | we ignored such frequencies. > | > | To make it clear, modified comments as below > | /* > | * can't handle frequency <2400MHz (negative > | * channels) right now > | */ Great, thanks for the explanation. The modified comments are helpful, too. > RD-22 279-280 Func Just to be sure: it's intentional that the > first two items in the enum are set to 0x0001? > > | EXPLAIN > | Yes. These two definitions are the same as the ones in opensource > | MadWifi project and FreeBSD. > | Added comments to explain this as below: > | * When 0x0001 is set, both TXQ_TXOKINT and TXQ_TXERRINT > * will be enabled. Thanks for the explanation. -renee _______________________________________________ networking-discuss mailing list [email protected]
