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
