Received-SPF: pass (google.com: domain of linux-wireless+bounces-66-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <ff991106e2b09e296725be4d1fe02de1911a2a2c.camel@sipsolutions.net>
Subject: Re: [PATCH v2,1/2] wifi: mac80211: Add utilities for converting
 op_class
From: Johannes Berg <johannes@sipsolutions.net>
To: Michael-CY Lee <michael-cy.lee@mediatek.com>, linux-wireless
	 <linux-wireless@vger.kernel.org>
Cc: Felix Fietkau <nbd@nbd.name>, Lorenzo Bianconi <lorenzo@kernel.org>, 
 Evelyn Tsai <evelyn.tsai@mediatek.com>, Money Wang
 <money.wang@mediatek.com>, linux-mediatek
 <linux-mediatek@lists.infradead.org>
Date: Fri, 24 Nov 2023 20:25:58 +0100
In-Reply-To: <20231113021107.13110-1-michael-cy.lee@mediatek.com>
References: <20231113021107.13110-1-michael-cy.lee@mediatek.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
Precedence: bulk
MIME-Version: 1.0

Btw, I ended looking at this again...

On Mon, 2023-11-13 at 10:11 +0800, Michael-CY Lee wrote:=20
> +/**
> + * ieee80211_operating_class_to_center_freq - convert operating class to
> + * center frequency
> + *
> + * @operating_class: the operating class to convert
> + * @chan: the ieee80211_channel to convert
> + * @center_freq1: cneter frequency 1 pointer to fill
> + * @center_freq2: cneter frequency 2 pointer to fill

typos here ("center")

But maybe it'd be better to fill (or update, we could pass the channel
pointer in it) a chandef struct? Then it could also be more easily
extended to understand more opclasses in the future, perhaps S1G or DMG?

> + *
> + * Returns %true if the conversion was successful, %false otherwise.
> + */
> +bool ieee80211_operating_class_to_center_freq(u8 operating_class,
> +					      struct ieee80211_channel *chan,
> +					      u32 *center_freq1,
> +					      u32 *center_freq2);
> +
> +/**
> + * ieee80211_operating_class_to_chan_width - convert operating class to
> + * nl80211 channel width
> + *
> + * @operating_class: the operating class to convert
> + */
> +enum nl80211_chan_width
> +ieee80211_operating_class_to_chan_width(u8 operating_class);

And you'd actually get both in one function call? The chan ->
center_freq anyway implies you know the width, no? Is this really needed
separately?

>  /**
>   * ieee8 0211_chandef_to_operating_class - convert chandef to operation =
class

This also converts the other way around, btw.

> +	case 135: /* 6 GHz band; 80+80 MHz; channels 1,5,..,229 */
> +		/* TODO How to know the center_freq2 of 80+80 MHz?*/
> +		*center_freq1 =3D 0;

Well, you don't, from this. I'm actually a bit surprised 80+80 exists in
6 GHz, I thought it was treated more or less as a dead end.

johannes