>> No, that can't possibly work right, sorry.
> I think it would work for reading the bitrate with WExt, but not for
> setting the bitrate.
Yes, that might work, but would be quite inconsistent, IMHO.
>> > + * @NL80211_STA_INFO_SIGNAL: signal strength of last received package
>> > (u8, dBm)
>>
>> s8? should be signed, no?
> Yes, it should be signed, but nl80211 does not support signed values.
> Shall I
> document it as signed but use the unsigned macros to transmit it through
> nl80211 to userspace (not sure about it) ?
Yes, please document as signed.
>> > + * @NL80211_STA_INFO_RX_BITRATE: bitrate of last received unicast
> packet
>> > + * (u16, 100 kbit/s)
>>
>> I don't really like this. I know we cannot report the real information
>> yet because we don't even have the driver/mac80211 api but let's add rx
>> rate reporting when we have the HT information too.
> I'm not sure I understand your problem.
> We have the mcs number from the driver, we have the the 20/40 Mhz flag
and
> we
> have to 400/800ns guard interval flag. That should be everything we need.
I don't think we have MCS number, where did you see it? And we don't have
the 20/40, GI etc. either, afaict. Remember this is RX.
>> > + * @NL80211_STA_INFO_TX_BITRATE: current unicast tx rate (u16, 100
>> > kbit/s) + * @NL80211_STA_INFO_TX_BITRATE_40_MHZ: dual channel
>> > transmission (flag) + * @NL80211_STA_INFO_TX_BITRATE_MCS: 802.11n MCS
>> > index of tx rate (u8) + * @NL80211_STA_INFO_TX_BITRATE_SHORT_GI:
> 802.11n
>> > with 400ns GI, 800ns + * otherwise, should be ignored if
> TX_BITRATE_MCS
>> > is not set (flag)
>>
>> I'm not sure I like the bitrate being used as prefix and final name, can
>> we have maybe TXRATE_ as prefix and use TXRATE_RATE, TXRATE_40, ...?
> Like this ?
>
> NL80211_STA_INFO_TXRATE_RATE
> NL80211_STA_INFO_TXRATE_40_MHZ
> NL80211_STA_INFO_TXRATE_MCS
> NL80211_STA_INFO_TXRATE_SHORT_GI
Yeah, much better.
>> I definitely don't like this, ick, please put that into userspace.
> Is there some kind of userspace library I could put this function into ?
> Every userspace programm using nl80211 will need this translation
> function, so it would be bad to put it into the iw command.
Why would that be bad? Most programs using nl80211 won't care, and this is
fixed information, not something that changes.
>> > + sinfo->rx_bitrate = sta->last_rxrate_unicast;
>> > +
>> > + sinfo->tx_bitrate_flags = sta->last_tx_rate.flags &
>> > + (IEEE80211_TX_RC_MCS |
>> > + IEEE80211_TX_RC_40_MHZ_WIDTH |
>> > + IEEE80211_TX_RC_SHORT_GI);
>>
>> That looks very odd. Are you sure it's using the same rate flags? And if
>> it is, that's wrong, because cfg80211 must not rely on mac80211 flags.
> I just store them in the flags field and translate them into NL80211
flags
>
> later. They never leave the kernel.
>
> But I can add a new enum for this. Maybe this way ?
>
> enum station_info_txrate_flags {
> STATION_INFO_TXFLAGS_MCS,
> STATION_INFO_TXFLAGS_40_MHZ,
> STATION_INFO_TXFLAGS_SHORT_GI
> };
Yes, that would be better, and that needs to be defined in cfg80211.
>> > + if (!(sta->last_tx_rate.flags & IEEE80211_TX_RC_MCS)) {
>> > + struct ieee80211_supported_band *sband;
>> > + sband =
>> > sta->local->hw.wiphy->bands[sta->local->hw.conf.channel->band];
>> > + sinfo->tx_bitrate = sband->bitrates[sta->last_tx_rate.idx].bitrate;
>> > + sinfo->tx_bitrate_mcs = 0;
>> I don't think you should initialise mcs here.
> I didn't liked to keep it uninitialized, but I can just delete the line.
But it won't be used anyway, so imho it's easier to understand if you
remove it.
>> Some places also need work on the coding style.
> Maybe you can give me an example, I'm still trying to learn the coding
> style for this group.
You can find stuff on that in the Documentation/ directory.
johannes
PS: Sent from new webmail, apologies for the previous johannes@localhost,
let me know if this is ok!
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
