On Thu, 2023-11-02 at 14:24 +0200, Ilpo Järvinen wrote:

On Thu, 2 Nov 2023, Johannes Berg wrote:

quoted

On Thu, 2023-11-02 at 13:55 +0200, Ilpo Järvinen wrote:

quoted

quoted

+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