On 6/16/23 01:57, Evan Quan wrote:
> From: Mario Limonciello <[email protected]>
>
> To support AMD's WBRF interference mitigation mechanism, Wifi adapters
> utilized in the system must register the frequencies in use(or unregister
> those frequencies no longer used) via the dedicated APCI calls. So that,
> other drivers responding to the frequencies can take proper actions to
> mitigate possible interference.
>
> To make WBRF feature functional, the kernel needs to be configured with
> CONFIG_ACPI_WBRF and the platform is equipped with WBRF support(from
> BIOS and drivers).
>
> Signed-off-by: Mario Limonciello <[email protected]>
> Co-developed-by: Evan Quan <[email protected]>
> Signed-off-by: Evan Quan <[email protected]>
> --
> v1->v2:
> - place the new added member(`wbrf_supported`) in
> ieee80211_local(Johannes)
> - handle chandefs change scenario properly(Johannes)
> - some minor fixes around code sharing and possible invalid input
> checks(Johannes)
> ---
> include/net/cfg80211.h | 8 +++
> net/mac80211/Makefile | 2 +
> net/mac80211/chan.c | 11 +++
> net/mac80211/ieee80211_i.h | 19 +++++
> net/mac80211/main.c | 2 +
> net/mac80211/wbrf.c | 137 +++++++++++++++++++++++++++++++++++++
> net/wireless/chan.c | 3 +-
> 7 files changed, 181 insertions(+), 1 deletion(-)
> create mode 100644 net/mac80211/wbrf.c
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 9e04f69712b1..c6dc337eafce 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -920,6 +920,14 @@ const struct cfg80211_chan_def *
> cfg80211_chandef_compatible(const struct cfg80211_chan_def *chandef1,
> const struct cfg80211_chan_def *chandef2);
>
> +/**
> + * nl80211_chan_width_to_mhz - get the channel width in Mhz
> + * @chan_width: the channel width from &enum nl80211_chan_width
> + * Return: channel width in Mhz if the chan_width from &enum nl80211_chan_width
> + * is valid. -1 otherwise.
> + */
> +int nl80211_chan_width_to_mhz(enum nl80211_chan_width chan_width);
> +
It's up to mac80211 maintainers, but I would think that the changes to
change nl80211_chan_width_to_mhz from static to exported should be
separate from the patch to introduced WBRF support in the series.
> /**
> * cfg80211_chandef_valid - check if a channel definition is valid
> * @chandef: the channel definition to check
> diff --git a/net/mac80211/Makefile b/net/mac80211/Makefile
> index b8de44da1fb8..709eb678f42a 100644
> --- a/net/mac80211/Makefile
> +++ b/net/mac80211/Makefile
> @@ -65,4 +65,6 @@ rc80211_minstrel-$(CONFIG_MAC80211_DEBUGFS) += \
>
> mac80211-$(CONFIG_MAC80211_RC_MINSTREL) += $(rc80211_minstrel-y)
>
> +mac80211-$(CONFIG_ACPI_WBRF) += wbrf.o
> +
> ccflags-y += -DDEBUG
> diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
> index 77c90ed8f5d7..0c5289a9aa6c 100644
> --- a/net/mac80211/chan.c
> +++ b/net/mac80211/chan.c
> @@ -506,11 +506,16 @@ static void _ieee80211_change_chanctx(struct ieee80211_local *local,
>
> WARN_ON(!cfg80211_chandef_compatible(&ctx->conf.def, chandef));
>
> + ieee80211_remove_wbrf(local, &ctx->conf.def);
> +
> ctx->conf.def = *chandef;
>
> /* check if min chanctx also changed */
> changed = IEEE80211_CHANCTX_CHANGE_WIDTH |
> _ieee80211_recalc_chanctx_min_def(local, ctx, rsvd_for);
> +
> + ieee80211_add_wbrf(local, &ctx->conf.def);
> +
> drv_change_chanctx(local, ctx, changed);
>
> if (!local->use_chanctx) {
> @@ -668,6 +673,10 @@ static int ieee80211_add_chanctx(struct ieee80211_local *local,
> lockdep_assert_held(&local->mtx);
> lockdep_assert_held(&local->chanctx_mtx);
>
> + err = ieee80211_add_wbrf(local, &ctx->conf.def);
> + if (err)
> + return err;
> +
> if (!local->use_chanctx)
> local->hw.conf.radar_enabled = ctx->conf.radar_enabled;
>
> @@ -748,6 +757,8 @@ static void ieee80211_del_chanctx(struct ieee80211_local *local,
> }
>
> ieee80211_recalc_idle(local);
> +
> + ieee80211_remove_wbrf(local, &ctx->conf.def);
> }
>
> static void ieee80211_free_chanctx(struct ieee80211_local *local,
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index b0372e76f373..f832de16073b 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -1591,6 +1591,10 @@ struct ieee80211_local {
>
> /* extended capabilities provided by mac80211 */
> u8 ext_capa[8];
> +
> +#ifdef CONFIG_ACPI_WBRF
> + bool wbrf_supported;
> +#endif
> };
>
> static inline struct ieee80211_sub_if_data *
> @@ -2615,4 +2619,19 @@ ieee80211_eht_cap_ie_to_sta_eht_cap(struct ieee80211_sub_if_data *sdata,
> const struct ieee80211_eht_cap_elem *eht_cap_ie_elem,
> u8 eht_cap_len,
> struct link_sta_info *link_sta);
> +
> +#ifdef CONFIG_ACPI_WBRF
> +void ieee80211_check_wbrf_support(struct ieee80211_local *local);
> +int ieee80211_add_wbrf(struct ieee80211_local *local,
> + struct cfg80211_chan_def *chandef);
> +void ieee80211_remove_wbrf(struct ieee80211_local *local,
> + struct cfg80211_chan_def *chandef);
> +#else
> +static inline void ieee80211_check_wbrf_support(struct ieee80211_local *local) { }
> +static inline int ieee80211_add_wbrf(struct ieee80211_local *local,
> + struct cfg80211_chan_def *chandef) { return 0; }
> +static inline void ieee80211_remove_wbrf(struct ieee80211_local *local,
> + struct cfg80211_chan_def *chandef) { }
> +#endif /* CONFIG_ACPI_WBRF */
> +
> #endif /* IEEE80211_I_H */
> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
> index 55cdfaef0f5d..0a55626b1546 100644
> --- a/net/mac80211/main.c
> +++ b/net/mac80211/main.c
> @@ -1395,6 +1395,8 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
> debugfs_hw_add(local);
> rate_control_add_debugfs(local);
>
> + ieee80211_check_wbrf_support(local);
> +
> rtnl_lock();
> wiphy_lock(hw->wiphy);
>
> diff --git a/net/mac80211/wbrf.c b/net/mac80211/wbrf.c
> new file mode 100644
> index 000000000000..2e1a58cf4dbf
> --- /dev/null
> +++ b/net/mac80211/wbrf.c
> @@ -0,0 +1,137 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AMD Wifi Band Exclusion Interface
> + * Copyright (C) 2023 Advanced Micro Devices
> + *
> + */
> +
> +#include <linux/wbrf.h>
> +#include <net/cfg80211.h>
> +#include "ieee80211_i.h"
> +
> +#define KHZ_TO_HZ(freq) ((freq) * 1000ULL)
I think this new macro probably should live in include/linux/ieee80211.h.
> +
> +void ieee80211_check_wbrf_support(struct ieee80211_local *local)
> +{
> + struct device *dev = local->hw.wiphy->dev.parent;
> + struct acpi_device *acpi_dev;
> +
> + if (!dev)
> + return;
> +
> + acpi_dev = ACPI_COMPANION(dev);
> + if (!acpi_dev) {
> + dev_dbg(dev, "ACPI companion not found\n");
> + return;
> + }
> +
> + local->wbrf_supported = wbrf_supported_producer(acpi_dev);
> + dev_dbg(dev, "WBRF is %s supported\n",
> + local->wbrf_supported ? "" : "not");
> +}
> +
> +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);
> +
> + *start = center_freq - bandwidth / 2;
> + *end = center_freq + bandwidth / 2;
What do you think about using cfg80211_get_start_freq and
cfg80211_get_end_freq and then converting the result from them to HZ
instead?
> +
> + /* Frequency in HZ is expected */
> + *start = KHZ_TO_HZ(*start);
> + *end = KHZ_TO_HZ(*end);
> +}
> +
> +static int wbrf_get_ranges_from_chandef(struct cfg80211_chan_def *chandef,
> + struct wbrf_ranges_in *ranges_in)
> +{
> + u64 start_freq1, end_freq1;
> + u64 start_freq2, end_freq2;
> + int bandwidth;
> +
> + bandwidth = nl80211_chan_width_to_mhz(chandef->width);
> + if (bandwidth < 0)
> + return -EINVAL;
> +
> + get_chan_freq_boundary(chandef->center_freq1,
> + bandwidth,
> + &start_freq1,
> + &end_freq1);
> +
> + ranges_in->band_list[0].start = start_freq1;
> + ranges_in->band_list[0].end = end_freq1;
> +
> + if (chandef->width == NL80211_CHAN_WIDTH_80P80) {
> + get_chan_freq_boundary(chandef->center_freq2,
> + bandwidth,
> + &start_freq2,
> + &end_freq2);
> +
> + ranges_in->band_list[1].start = start_freq2;
> + ranges_in->band_list[1].end = end_freq2;
> + }
> +
> + return 0;
> +}
> +
> +static int wbrf_add_exclusion_wlan(struct acpi_device *adev,
> + struct cfg80211_chan_def *chandef)
> +{
> + struct wbrf_ranges_in ranges_in = {0};
> + int ret;
> +
> + ret = wbrf_get_ranges_from_chandef(chandef, &ranges_in);
> + if (ret)
> + return ret;
> +
> + return wbrf_add_exclusion(adev, &ranges_in);
> +}
> +
> +static int wbrf_remove_exclusion_wlan(struct acpi_device *adev,
> + struct cfg80211_chan_def *chandef)
> +{
> + struct wbrf_ranges_in ranges_in = {0};
> + int ret;
> +
> + ret = wbrf_get_ranges_from_chandef(chandef, &ranges_in);
> + if (ret)
> + return ret;
> +
> + return wbrf_remove_exclusion(adev, &ranges_in);
> +}
I don't really see a good reason for wbrf_remove_exclusion_wlan and
wbrf_add_exclusion_wlan to be static functions.
In the earlier verisons they were both in the ACPI file so it made sense
as an exported symbol.
But they each only have a single calling site now and I think they
should collapse into those functions.
> +
> +int ieee80211_add_wbrf(struct ieee80211_local *local,
> + struct cfg80211_chan_def *chandef)
> +{
> + struct device *dev = local->hw.wiphy->dev.parent;
> + struct acpi_device *acpi_dev;
> +
> + if (!local->wbrf_supported)
> + return 0;
> +
> + acpi_dev = ACPI_COMPANION(dev);
> + if (!acpi_dev)
> + return -ENODEV;
ACPI devices won't go away, this should be an impossible failure.
When wbrf_supported was populated earlier on the ACPI device was checked.
> +
> + return wbrf_add_exclusion_wlan(acpi_dev, chandef); > +}
> +
> +void ieee80211_remove_wbrf(struct ieee80211_local *local,
> + struct cfg80211_chan_def *chandef)
> +{
> + struct device *dev = local->hw.wiphy->dev.parent;
> + struct acpi_device *acpi_dev;
> +
> + if (!local->wbrf_supported)
> + return;
> +
> + acpi_dev = ACPI_COMPANION(dev);
> + if (!acpi_dev)
> + return;
> +
ACPI devices won't go away, this should be an impossible failure.
When wbrf_supported was populated earlier on the ACPI device was checked.
> + wbrf_remove_exclusion_wlan(acpi_dev, chandef);
> +}
> diff --git a/net/wireless/chan.c b/net/wireless/chan.c
> index 0b7e81db383d..227db04eac42 100644
> --- a/net/wireless/chan.c
> +++ b/net/wireless/chan.c
> @@ -141,7 +141,7 @@ static bool cfg80211_edmg_chandef_valid(const struct cfg80211_chan_def *chandef)
> return true;
> }
>
> -static int nl80211_chan_width_to_mhz(enum nl80211_chan_width chan_width)
> +int nl80211_chan_width_to_mhz(enum nl80211_chan_width chan_width)
> {
> int mhz;
>
> @@ -190,6 +190,7 @@ static int nl80211_chan_width_to_mhz(enum nl80211_chan_width chan_width)
> }
> return mhz;
> }
> +EXPORT_SYMBOL(nl80211_chan_width_to_mhz);
>
> static int cfg80211_chandef_get_width(const struct cfg80211_chan_def *c)
> {
On Sun, 2023-06-18 at 21:17 -0500, Mario Limonciello wrote:
>
> > --- a/include/net/cfg80211.h
> > +++ b/include/net/cfg80211.h
> > @@ -920,6 +920,14 @@ const struct cfg80211_chan_def *
> > cfg80211_chandef_compatible(const struct cfg80211_chan_def *chandef1,
> > const struct cfg80211_chan_def *chandef2);
> >
> > +/**
> > + * nl80211_chan_width_to_mhz - get the channel width in Mhz
> > + * @chan_width: the channel width from &enum nl80211_chan_width
> > + * Return: channel width in Mhz if the chan_width from &enum nl80211_chan_width
> > + * is valid. -1 otherwise.
> > + */
> > +int nl80211_chan_width_to_mhz(enum nl80211_chan_width chan_width);
> > +
>
> It's up to mac80211 maintainers, but I would think that the changes to
> change nl80211_chan_width_to_mhz from static to exported should be
> separate from the patch to introduced WBRF support in the series.
Yeah, that'd be nice :)
> > +#define KHZ_TO_HZ(freq) ((freq) * 1000ULL)
Together with MHZ_TO_KHZ() for example :)
johannes
[AMD Official Use Only - General]
> -----Original Message-----
> From: Johannes Berg <[email protected]>
> Sent: Monday, June 19, 2023 4:24 PM
> To: Limonciello, Mario <[email protected]>; Quan, Evan
> <[email protected]>
> Cc: [email protected]; [email protected]; amd-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]; Deucher,
> Alexander <[email protected]>; Koenig, Christian
> <[email protected]>; Pan, Xinhui <[email protected]>;
> [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Lazar, Lijo <[email protected]>
> Subject: Re: [PATCH V3 2/7] wifi: mac80211: Add support for ACPI WBRF
>
> On Sun, 2023-06-18 at 21:17 -0500, Mario Limonciello wrote:
> >
> > > --- a/include/net/cfg80211.h
> > > +++ b/include/net/cfg80211.h
> > > @@ -920,6 +920,14 @@ const struct cfg80211_chan_def *
> > > cfg80211_chandef_compatible(const struct cfg80211_chan_def
> *chandef1,
> > > const struct cfg80211_chan_def *chandef2);
> > >
> > > +/**
> > > + * nl80211_chan_width_to_mhz - get the channel width in Mhz
> > > + * @chan_width: the channel width from &enum nl80211_chan_width
> > > + * Return: channel width in Mhz if the chan_width from &enum
> > > +nl80211_chan_width
> > > + * is valid. -1 otherwise.
> > > + */
> > > +int nl80211_chan_width_to_mhz(enum nl80211_chan_width
> chan_width);
> > > +
> >
> > It's up to mac80211 maintainers, but I would think that the changes to
> > change nl80211_chan_width_to_mhz from static to exported should be
> > separate from the patch to introduced WBRF support in the series.
>
> Yeah, that'd be nice :)
OK, I will move that into a new patch.
>
>
> > > +#define KHZ_TO_HZ(freq) ((freq) * 1000ULL)
>
> Together with MHZ_TO_KHZ() for example :)
Sure.
Evan
>
> johannes
[AMD Official Use Only - General]
> -----Original Message-----
> From: Limonciello, Mario <[email protected]>
> Sent: Monday, June 19, 2023 10:17 AM
> To: Quan, Evan <[email protected]>
> Cc: [email protected]; [email protected]; amd-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]; Deucher,
> Alexander <[email protected]>; Koenig, Christian
> <[email protected]>; Pan, Xinhui <[email protected]>;
> [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Lazar, Lijo <[email protected]>
> Subject: Re: [PATCH V3 2/7] wifi: mac80211: Add support for ACPI WBRF
>
> On 6/16/23 01:57, Evan Quan wrote:
> > From: Mario Limonciello <[email protected]>
> >
> > To support AMD's WBRF interference mitigation mechanism, Wifi adapters
> > utilized in the system must register the frequencies in use(or
> > unregister those frequencies no longer used) via the dedicated APCI
> > calls. So that, other drivers responding to the frequencies can take
> > proper actions to mitigate possible interference.
> >
> > To make WBRF feature functional, the kernel needs to be configured
> > with CONFIG_ACPI_WBRF and the platform is equipped with WBRF
> > support(from BIOS and drivers).
> >
> > Signed-off-by: Mario Limonciello <[email protected]>
> > Co-developed-by: Evan Quan <[email protected]>
> > Signed-off-by: Evan Quan <[email protected]>
> > --
> > v1->v2:
> > - place the new added member(`wbrf_supported`) in
> > ieee80211_local(Johannes)
> > - handle chandefs change scenario properly(Johannes)
> > - some minor fixes around code sharing and possible invalid input
> > checks(Johannes)
> > ---
> > include/net/cfg80211.h | 8 +++
> > net/mac80211/Makefile | 2 +
> > net/mac80211/chan.c | 11 +++
> > net/mac80211/ieee80211_i.h | 19 +++++
> > net/mac80211/main.c | 2 +
> > net/mac80211/wbrf.c | 137
> +++++++++++++++++++++++++++++++++++++
> > net/wireless/chan.c | 3 +-
> > 7 files changed, 181 insertions(+), 1 deletion(-)
> > create mode 100644 net/mac80211/wbrf.c
> >
> > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index
> > 9e04f69712b1..c6dc337eafce 100644
> > --- a/include/net/cfg80211.h
> > +++ b/include/net/cfg80211.h
> > @@ -920,6 +920,14 @@ const struct cfg80211_chan_def *
> > cfg80211_chandef_compatible(const struct cfg80211_chan_def
> *chandef1,
> > const struct cfg80211_chan_def *chandef2);
> >
> > +/**
> > + * nl80211_chan_width_to_mhz - get the channel width in Mhz
> > + * @chan_width: the channel width from &enum nl80211_chan_width
> > + * Return: channel width in Mhz if the chan_width from &enum
> > +nl80211_chan_width
> > + * is valid. -1 otherwise.
> > + */
> > +int nl80211_chan_width_to_mhz(enum nl80211_chan_width
> chan_width);
> > +
>
> It's up to mac80211 maintainers, but I would think that the changes to change
> nl80211_chan_width_to_mhz from static to exported should be separate
> from the patch to introduced WBRF support in the series.
Will do that.
>
> > /**
> > * cfg80211_chandef_valid - check if a channel definition is valid
> > * @chandef: the channel definition to check diff --git
> > a/net/mac80211/Makefile b/net/mac80211/Makefile index
> > b8de44da1fb8..709eb678f42a 100644
> > --- a/net/mac80211/Makefile
> > +++ b/net/mac80211/Makefile
> > @@ -65,4 +65,6 @@ rc80211_minstrel-$(CONFIG_MAC80211_DEBUGFS)
> += \
> >
> > mac80211-$(CONFIG_MAC80211_RC_MINSTREL) += $(rc80211_minstrel-
> y)
> >
> > +mac80211-$(CONFIG_ACPI_WBRF) += wbrf.o
> > +
> > ccflags-y += -DDEBUG
> > diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c index
> > 77c90ed8f5d7..0c5289a9aa6c 100644
> > --- a/net/mac80211/chan.c
> > +++ b/net/mac80211/chan.c
> > @@ -506,11 +506,16 @@ static void _ieee80211_change_chanctx(struct
> > ieee80211_local *local,
> >
> > WARN_ON(!cfg80211_chandef_compatible(&ctx->conf.def,
> chandef));
> >
> > + ieee80211_remove_wbrf(local, &ctx->conf.def);
> > +
> > ctx->conf.def = *chandef;
> >
> > /* check if min chanctx also changed */
> > changed = IEEE80211_CHANCTX_CHANGE_WIDTH |
> > _ieee80211_recalc_chanctx_min_def(local, ctx, rsvd_for);
> > +
> > + ieee80211_add_wbrf(local, &ctx->conf.def);
> > +
> > drv_change_chanctx(local, ctx, changed);
> >
> > if (!local->use_chanctx) {
> > @@ -668,6 +673,10 @@ static int ieee80211_add_chanctx(struct
> ieee80211_local *local,
> > lockdep_assert_held(&local->mtx);
> > lockdep_assert_held(&local->chanctx_mtx);
> >
> > + err = ieee80211_add_wbrf(local, &ctx->conf.def);
> > + if (err)
> > + return err;
> > +
> > if (!local->use_chanctx)
> > local->hw.conf.radar_enabled = ctx->conf.radar_enabled;
> >
> > @@ -748,6 +757,8 @@ static void ieee80211_del_chanctx(struct
> ieee80211_local *local,
> > }
> >
> > ieee80211_recalc_idle(local);
> > +
> > + ieee80211_remove_wbrf(local, &ctx->conf.def);
> > }
> >
> > static void ieee80211_free_chanctx(struct ieee80211_local *local,
> > diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> > index b0372e76f373..f832de16073b 100644
> > --- a/net/mac80211/ieee80211_i.h
> > +++ b/net/mac80211/ieee80211_i.h
> > @@ -1591,6 +1591,10 @@ struct ieee80211_local {
> >
> > /* extended capabilities provided by mac80211 */
> > u8 ext_capa[8];
> > +
> > +#ifdef CONFIG_ACPI_WBRF
> > + bool wbrf_supported;
> > +#endif
> > };
> >
> > static inline struct ieee80211_sub_if_data * @@ -2615,4 +2619,19 @@
> > ieee80211_eht_cap_ie_to_sta_eht_cap(struct ieee80211_sub_if_data
> *sdata,
> > const struct ieee80211_eht_cap_elem
> *eht_cap_ie_elem,
> > u8 eht_cap_len,
> > struct link_sta_info *link_sta);
> > +
> > +#ifdef CONFIG_ACPI_WBRF
> > +void ieee80211_check_wbrf_support(struct ieee80211_local *local); int
> > +ieee80211_add_wbrf(struct ieee80211_local *local,
> > + struct cfg80211_chan_def *chandef); void
> > +ieee80211_remove_wbrf(struct ieee80211_local *local,
> > + struct cfg80211_chan_def *chandef); #else static
> inline void
> > +ieee80211_check_wbrf_support(struct ieee80211_local *local) { }
> > +static inline int ieee80211_add_wbrf(struct ieee80211_local *local,
> > + struct cfg80211_chan_def *chandef)
> { return 0; } static
> > +inline void ieee80211_remove_wbrf(struct ieee80211_local *local,
> > + struct cfg80211_chan_def *chandef)
> { } #endif /*
> > +CONFIG_ACPI_WBRF */
> > +
> > #endif /* IEEE80211_I_H */
> > diff --git a/net/mac80211/main.c b/net/mac80211/main.c index
> > 55cdfaef0f5d..0a55626b1546 100644
> > --- a/net/mac80211/main.c
> > +++ b/net/mac80211/main.c
> > @@ -1395,6 +1395,8 @@ int ieee80211_register_hw(struct ieee80211_hw
> *hw)
> > debugfs_hw_add(local);
> > rate_control_add_debugfs(local);
> >
> > + ieee80211_check_wbrf_support(local);
> > +
> > rtnl_lock();
> > wiphy_lock(hw->wiphy);
> >
> > diff --git a/net/mac80211/wbrf.c b/net/mac80211/wbrf.c new file mode
> > 100644 index 000000000000..2e1a58cf4dbf
> > --- /dev/null
> > +++ b/net/mac80211/wbrf.c
> > @@ -0,0 +1,137 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * AMD Wifi Band Exclusion Interface
> > + * Copyright (C) 2023 Advanced Micro Devices
> > + *
> > + */
> > +
> > +#include <linux/wbrf.h>
> > +#include <net/cfg80211.h>
> > +#include "ieee80211_i.h"
> > +
> > +#define KHZ_TO_HZ(freq) ((freq) * 1000ULL)
>
> I think this new macro probably should live in include/linux/ieee80211.h.
Might be better to follow Johannes's suggestion to move this together with MHZ_TO_KHZ().
>
> > +
> > +void ieee80211_check_wbrf_support(struct ieee80211_local *local) {
> > + struct device *dev = local->hw.wiphy->dev.parent;
> > + struct acpi_device *acpi_dev;
> > +
> > + if (!dev)
> > + return;
> > +
> > + acpi_dev = ACPI_COMPANION(dev);
> > + if (!acpi_dev) {
> > + dev_dbg(dev, "ACPI companion not found\n");
> > + return;
> > + }
> > +
> > + local->wbrf_supported = wbrf_supported_producer(acpi_dev);
> > + dev_dbg(dev, "WBRF is %s supported\n",
> > + local->wbrf_supported ? "" : "not"); }
> > +
> > +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);
> > +
> > + *start = center_freq - bandwidth / 2;
> > + *end = center_freq + bandwidth / 2;
>
> What do you think about using cfg80211_get_start_freq and
> cfg80211_get_end_freq and then converting the result from them to HZ
> instead?
They cannot fit our requirements well. They handle only the peak of the range. What we wanted is the start/end point of the whole range.
The illustration from the link below might give a better view.
https://securityuncorked.com/wordpress/wp-content/uploads/2013/11/graphic-80211-acChannels-all.png
>
> > +
> > + /* Frequency in HZ is expected */
> > + *start = KHZ_TO_HZ(*start);
> > + *end = KHZ_TO_HZ(*end);
> > +}
> > +
> > +static int wbrf_get_ranges_from_chandef(struct cfg80211_chan_def
> *chandef,
> > + struct wbrf_ranges_in *ranges_in) {
> > + u64 start_freq1, end_freq1;
> > + u64 start_freq2, end_freq2;
> > + int bandwidth;
> > +
> > + bandwidth = nl80211_chan_width_to_mhz(chandef->width);
> > + if (bandwidth < 0)
> > + return -EINVAL;
> > +
> > + get_chan_freq_boundary(chandef->center_freq1,
> > + bandwidth,
> > + &start_freq1,
> > + &end_freq1);
> > +
> > + ranges_in->band_list[0].start = start_freq1;
> > + ranges_in->band_list[0].end = end_freq1;
> > +
> > + if (chandef->width == NL80211_CHAN_WIDTH_80P80) {
> > + get_chan_freq_boundary(chandef->center_freq2,
> > + bandwidth,
> > + &start_freq2,
> > + &end_freq2);
> > +
> > + ranges_in->band_list[1].start = start_freq2;
> > + ranges_in->band_list[1].end = end_freq2;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int wbrf_add_exclusion_wlan(struct acpi_device *adev,
> > + struct cfg80211_chan_def *chandef) {
> > + struct wbrf_ranges_in ranges_in = {0};
> > + int ret;
> > +
> > + ret = wbrf_get_ranges_from_chandef(chandef, &ranges_in);
> > + if (ret)
> > + return ret;
> > +
> > + return wbrf_add_exclusion(adev, &ranges_in); }
> > +
> > +static int wbrf_remove_exclusion_wlan(struct acpi_device *adev,
> > + struct cfg80211_chan_def *chandef) {
> > + struct wbrf_ranges_in ranges_in = {0};
> > + int ret;
> > +
> > + ret = wbrf_get_ranges_from_chandef(chandef, &ranges_in);
> > + if (ret)
> > + return ret;
> > +
> > + return wbrf_remove_exclusion(adev, &ranges_in); }
>
> I don't really see a good reason for wbrf_remove_exclusion_wlan and
> wbrf_add_exclusion_wlan to be static functions.
> In the earlier verisons they were both in the ACPI file so it made sense as an
> exported symbol.
>
> But they each only have a single calling site now and I think they should
> collapse into those functions.
OK, will update this as suggested.
>
> > +
> > +int ieee80211_add_wbrf(struct ieee80211_local *local,
> > + struct cfg80211_chan_def *chandef) {
> > + struct device *dev = local->hw.wiphy->dev.parent;
> > + struct acpi_device *acpi_dev;
> > +
> > + if (!local->wbrf_supported)
> > + return 0;
> > +
> > + acpi_dev = ACPI_COMPANION(dev);
> > + if (!acpi_dev)
> > + return -ENODEV;
>
> ACPI devices won't go away, this should be an impossible failure.
> When wbrf_supported was populated earlier on the ACPI device was checked.
Make sense to me. Will update it.
>
> > +
> > + return wbrf_add_exclusion_wlan(acpi_dev, chandef); > +}
> > +
> > +void ieee80211_remove_wbrf(struct ieee80211_local *local,
> > + struct cfg80211_chan_def *chandef) {
> > + struct device *dev = local->hw.wiphy->dev.parent;
> > + struct acpi_device *acpi_dev;
> > +
> > + if (!local->wbrf_supported)
> > + return;
> > +
> > + acpi_dev = ACPI_COMPANION(dev);
> > + if (!acpi_dev)
> > + return;
> > +
>
> ACPI devices won't go away, this should be an impossible failure.
> When wbrf_supported was populated earlier on the ACPI device was checked.
Will update it.
Evan
>
> > + wbrf_remove_exclusion_wlan(acpi_dev, chandef); }
> > diff --git a/net/wireless/chan.c b/net/wireless/chan.c index
> > 0b7e81db383d..227db04eac42 100644
> > --- a/net/wireless/chan.c
> > +++ b/net/wireless/chan.c
> > @@ -141,7 +141,7 @@ static bool cfg80211_edmg_chandef_valid(const
> struct cfg80211_chan_def *chandef)
> > return true;
> > }
> >
> > -static int nl80211_chan_width_to_mhz(enum nl80211_chan_width
> > chan_width)
> > +int nl80211_chan_width_to_mhz(enum nl80211_chan_width chan_width)
> > {
> > int mhz;
> >
> > @@ -190,6 +190,7 @@ static int nl80211_chan_width_to_mhz(enum
> nl80211_chan_width chan_width)
> > }
> > return mhz;
> > }
> > +EXPORT_SYMBOL(nl80211_chan_width_to_mhz);
> >
> > static int cfg80211_chandef_get_width(const struct cfg80211_chan_def
> *c)
> > {
