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 X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 579BFC43463 for ; Fri, 18 Sep 2020 10:38:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 169B121534 for ; Fri, 18 Sep 2020 10:38:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726299AbgIRKiM (ORCPT ); Fri, 18 Sep 2020 06:38:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48326 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726157AbgIRKiM (ORCPT ); Fri, 18 Sep 2020 06:38:12 -0400 Received: from sipsolutions.net (s3.sipsolutions.net [IPv6:2a01:4f8:191:4433::2]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 19F3FC06174A for ; Fri, 18 Sep 2020 03:38:11 -0700 (PDT) Received: by sipsolutions.net with esmtpsa (TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim 4.94) (envelope-from ) id 1kJDmL-006CNL-JC; Fri, 18 Sep 2020 12:38:09 +0200 Message-ID: Subject: Re: [PATCH v2 06/22] {cfg,mac}80211: get correct default channel width for S1G From: Johannes Berg To: Thomas Pedersen Cc: linux-wireless Date: Fri, 18 Sep 2020 12:38:08 +0200 In-Reply-To: <20200831205600.21058-7-thomas@adapt-ip.com> References: <20200831205600.21058-1-thomas@adapt-ip.com> <20200831205600.21058-7-thomas@adapt-ip.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.5 (3.36.5-1.fc32) MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org On Mon, 2020-08-31 at 13:55 -0700, Thomas Pedersen wrote: > > +++ b/net/wireless/chan.c > @@ -33,6 +33,16 @@ void cfg80211_chandef_create(struct cfg80211_chan_def *chandef, > chandef->edmg.bw_config = 0; > chandef->edmg.channels = 0; > > + /* S1G allows a single width per channel, and since chan_type seems to > + * be for backwards compatibility only, ignore it and return the per > + * frequency width. > + */ > + if (chan->band == NL80211_BAND_S1GHZ) { > + chandef->width = ieee80211_s1g_channel_width(chan); > + chandef->center_freq1 = chan->center_freq; > + return; > + } Hmm. I'm not sure I want to let you get away with this? It might be ... convenient, but it's also confusing to see something like cfg80211_chandef_create(&out, some_channel, NL80211_CHAN_HT40PLUS); actually create an S1G channel width? Yes, this is mostly for backward compatibility, but it's also used in few (21 in the stack) places. Many of those aren't relevant, e.g. in net/mac80211/ibss.c it would obviously be clearer to handle the new NL80211_CHAN_WIDTH_* values with e.g. a cfg80211_get_s1g_chandef() function or so that does this derivation, instead of running into the default: /* fall back to 20 MHz for unsupported modes */ cfg80211_chandef_create(&chandef, cbss->channel, NL80211_CHAN_NO_HT); code. IOW, it seems to me that this function should actually instead throw a warning (and then perhaps configure something sane?), but not be the default code path. johannes