Re: [PATCH v3 1/7] nl80211/cfg80211: add radar detection command/event

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2012-10-03 at 15:53 +0200, Goldenshtein, Victor wrote:

> >> + * @NL80211_FEATURE_DFS: Radar detection is supported in the HW/driver.
> >
> > How will you know what kind of radar detection is supported? That is, HT
> > 20, HT 40, in the future VHT80/160/80+80?
> >
> 
> only 20 Mhz is supported at first stage, do you prefer to rename it to
> something like: NL80211_FEATURE_20MHZ_DFS

no idea, maybe that, or maybe have some other field that lists the
channel widths that are supported?

> >> +int cfg80211_start_radar_detection(struct cfg80211_registered_device *rdev,
> >> +                                struct net_device *dev,
> >> +                                struct ieee80211_channel *chan)
> >> +{
> >> +     int err;
> >> +
> >> +     if (!rdev->ops->start_radar_detection)
> >> +             return -EOPNOTSUPP;
> >> +
> >> +     err = rdev->ops->start_radar_detection(&rdev->wiphy, dev, chan);
> >> +     if (!err)
> >> +             chan->radar_detect_timeout = jiffies +
> >> +                     msecs_to_jiffies(IEEE80211_DFS_MIN_CAC_TIME_MS);
> >> +     else {
> >> +             chan->radar_detect_timeout = 0;
> >> +             chan->cac_type = 0;
> >> +     }
> >
> > You're not setting cac_type in the good case, and also
> > radar_detect_timeout can actually be 0 in the good case due to jiffies
> > wrap. How is that handled?
> >
> 
> I'm using time_is_after_jiffies() in nl80211_dfs_en_tx() which AFAIK
> can handle jiffies wrap.

yeah but if you need to set radar_detect_timeout=0 here, then it looks
like you're also using the 0 value as a special value to indicate
"nothing in progress"?


> right, since NOHT is 0 we probably need additional flag something like
> "chan->cac_started"

yes

> alternatively we can add "__NL80211_CHAN_INVALID" at index 0  to the
> enum nl80211_channel_type.

no
nl80211 is abi :)

johannes