netdev - Re: [Patch v13 4/9] wifi: mac80211: Add support for WBRF features

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 02 Nov 2023 14:04:42 +0100
From: Johannes Berg <johannes@...solutions.net>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: Ma Jun <Jun.Ma2@....com>, amd-gfx@...ts.freedesktop.org,
 lenb@...nel.org,  davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com,  alexander.deucher@....com,
 Lijo.Lazar@....com, mario.limonciello@....com, Netdev
 <netdev@...r.kernel.org>, linux-wireless@...r.kernel.org, LKML
 <linux-kernel@...r.kernel.org>, linux-doc@...r.kernel.org, 
 platform-driver-x86@...r.kernel.org, majun@....com, Evan Quan
 <quanliangl@...mail.com>
Subject: Re: [Patch v13 4/9] wifi: mac80211: Add support for WBRF features

On Thu, 2023-11-02 at 14:24 +0200, Ilpo Järvinen wrote:
> On Thu, 2 Nov 2023, Johannes Berg wrote:
> > On Thu, 2023-11-02 at 13:55 +0200, Ilpo Järvinen wrote:
> > 
> > > > +static void get_chan_freq_boundary(u32 center_freq, u32 bandwidth, u64 *start, u64 *end)
> > > > +{
> > > > +	bandwidth = MHZ_TO_KHZ(bandwidth);
> > > > +	center_freq = MHZ_TO_KHZ(center_freq);
> > > 
> > > Please use include/linux/units.h ones for these too.
> > 
> > Now we're feature creeping though - this has existed for *years* in the
> > wireless stack with many instances? We can convert them over, I guess,
> > but not sure that makes much sense here - we'd want to add such macros
> > to units.h, but ... moving them can be independent of this patch?
> 
> What new macros you're talking about? 

Sorry, I got confused - for some reason I was pretty sure something here
was already being added to units.h in this patchset.

> Nothing new needs to be added 
> as there's already KHZ_PER_MHZ so these would just be:
> 
> 	bandwidth *= KHZ_PER_MHZ;
> 	center_freq *= KHZ_PER_MHZ;

Sure, and in this case that's probably pretty much equivalent. But
having a MHZ_TO_KHZ() macro isn't inherently *bad*, and I'm not sure
you're objection to it on anything other than "it's not defined in
units.h".

> Everything can of course be postponed by the argument that some 
> subsystem specific mechanism has been there before the generic one
> but the end of that road won't be pretty... What I was trying to do
> here was to point out the new stuff introduced by this series into the 
> direction of the generic thing.

I just think that the better course of action would be to eventually
move MHZ_TO_KHZ() to units.h ...

johannes