On Wed, 2020-04-01 at 10:30 -0700, Thomas Pedersen wrote: > > --- > .. c:function:: struct ieee80211_channel * ieee80211_get_channel (struct wiphy * wiphy, int freq) > > get channel struct from wiphy for specified frequency > > **Parameters** > > ``struct wiphy * wiphy`` > the struct wiphy to get the channel for > > ``int freq`` > the center frequency (in MHz) of the channel > --- > > Documentation/doc-guide/kernel-doc.rst says: > > The brief description following the function name may span multiple lines, and > ends with an argument description, a blank comment line, or the end of the > comment block. Interesting. I guess they must've fixed this at some point - at least I _seem_ to remember getting wrong output with it. Thanks for the correction! > > > +/** > > > + * ieee80211_chandef_to_khz - convert chandef to frequency in KHz > > > + * > > > + * @chandef: the chandef to convert > > > + * > > > + * Returns the center frequency of chandef (1st segment) in KHz. > > > + */ > > > +u32 ieee80211_chandef_to_khz(const struct cfg80211_chan_def *chandef); > > > > Isn't this one trivial, and probably better inlined (mhz*1000 + khz)? > > Do you mean open code the conversion? I would prefer not to. If you > meant make it an inline function then yes probably. Right, I meant making it an inline. > > > +static int __ieee80211_frequency_to_channel(u32 freq) > > > > export the double-underscore helpers like this one instead? That'd still > > be less code overall, IMHO. > > I didn't want to change the interface for > ieee80211_frequency_to_channel(). It's a little confusing that one takes > MHz, but the __ieee80211_frequency_to_channel() takes KHz? By giving the > _khz() hint in the wrapper we were trying to make it explicit. Similar > to below. Right. I think that's fine. I was just wondering if / thinking that it may be better to just export ieee80211_freq_khz_to_channel(), and express the other ones as inline function in terms of that? > > And maybe here? In fact, how is __ieee80211_get_channel() even different > > from ieee80211_get_channel_khz()? > > It's not. I thought the _khz() hint was helpful for the reader to keep > the units straight. Agree, but then you don't need the double-underscore version and can just express the old one in terms of the _khz one? > What do you think about keeping the interfaces in place, but otherwise > converting them to inline functions (where it makes sense)? Yes, I think that's what I had in mind. > I'd like to avoid open coding the conversions and otherwise keep the > gory internals of the channel structures hidden from the caller. Agree. johannes
