From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 337D0C4167B for ; Thu, 2 Nov 2023 13:04:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1376426AbjKBNE5 (ORCPT ); Thu, 2 Nov 2023 09:04:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53208 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234905AbjKBNEz (ORCPT ); Thu, 2 Nov 2023 09:04:55 -0400 Received: from sipsolutions.net (s3.sipsolutions.net [IPv6:2a01:4f8:242:246e::2]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6067E193; Thu, 2 Nov 2023 06:04:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sipsolutions.net; s=mail; h=MIME-Version:Content-Transfer-Encoding: Content-Type:References:In-Reply-To:Date:Cc:To:From:Subject:Message-ID:Sender :Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From:Resent-To: Resent-Cc:Resent-Message-ID; bh=lhnHvgPTUerCavcKvy4VoFNn9k17Ymmjx4Tbd3bshik=; t=1698930289; x=1700139889; b=axIh+OQs/8lt1/eCYxHEd6sdY68CsoLKtnX3H8kQwIL2R9T zrcHkoQiiL6mQPu8UReBO+NoXxOSaafybY8xtaCYh2PIlqJtOHavjMqBB/WFNzlyFVWTJDvKYqz6H ZnpD8tIKRq69VUxxGGwxF6EQI06974EwMFdxkTVPCvro3Ksfsav0Nmq/Z9DFqQkl0Gwj4R/OT1nkd ThcikbeZ6oRrjaof0DHOmaonV/KtEVxVHtQ7GuRhUWrxw1youZF98Gd+VcYRJ1ir7Ar5DEhu1lVJs WGfYlaDWfTtJokAS8YbUfi4qADPzC+rrH5y5ttw/y8JL3+h3BxAxfEtUP9oWj8LA==; Received: by sipsolutions.net with esmtpsa (TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim 4.97-RC1) (envelope-from ) id 1qyXNL-0000000BJWz-3w9i; Thu, 02 Nov 2023 14:04:44 +0100 Message-ID: Subject: Re: [Patch v13 4/9] wifi: mac80211: Add support for WBRF features From: Johannes Berg To: Ilpo =?ISO-8859-1?Q?J=E4rvinen?= Cc: Ma Jun , amd-gfx@lists.freedesktop.org, lenb@kernel.org, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, alexander.deucher@amd.com, Lijo.Lazar@amd.com, mario.limonciello@amd.com, Netdev , linux-wireless@vger.kernel.org, LKML , linux-doc@vger.kernel.org, platform-driver-x86@vger.kernel.org, majun@amd.com, Evan Quan Date: Thu, 02 Nov 2023 14:04:42 +0100 In-Reply-To: References: <20231030071832.2217118-1-Jun.Ma2@amd.com> <20231030071832.2217118-5-Jun.Ma2@amd.com> <5b8ea81c-dd4c-7f2a-c862-b9a0aab16044@linux.intel.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) MIME-Version: 1.0 X-malware-bazaar: not-scanned Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2023-11-02 at 14:24 +0200, Ilpo J=C3=A4rvinen wrote: > On Thu, 2 Nov 2023, Johannes Berg wrote: > > On Thu, 2023-11-02 at 13:55 +0200, Ilpo J=C3=A4rvinen wrote: > >=20 > > > > +static void get_chan_freq_boundary(u32 center_freq, u32 bandwidth,= u64 *start, u64 *end) > > > > +{ > > > > + bandwidth =3D MHZ_TO_KHZ(bandwidth); > > > > + center_freq =3D MHZ_TO_KHZ(center_freq); > > >=20 > > > Please use include/linux/units.h ones for these too. > >=20 > > 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? >=20 > What new macros you're talking about?=C2=A0 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=20 > as there's already KHZ_PER_MHZ so these would just be: >=20 > bandwidth *=3D KHZ_PER_MHZ; > center_freq *=3D 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=20 > 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= =20 > 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