Re: [PATCH V5 1/9] drivers core: Add support for Wifi band RF mitigations

["Limonciello, Mario" <mario.limonciello@amd.com>]

On 6/30/2023 05:32, Evan Quan wrote:
> Due to electrical and mechanical constraints in certain platform designs
> there may be likely interference of relatively high-powered harmonics of
> the (G-)DDR memory clocks with local radio module frequency bands used
> by Wifi 6/6e/7.
> 
> To mitigate this, AMD has introduced a mechanism that devices can use to
> notify active use of particular frequencies so that other devices can make
> relative internal adjustments as necessary to avoid this resonance.
> 
> In order for a device to support this, the expected flow for device
> driver or subsystems:
> 
> Drivers/subsystems contributing frequencies:
> 
> 1) During probe, check `wbrf_supported_producer` to see if WBRF supported
>     for the device.
> 2) If adding frequencies, then call `wbrf_add_exclusion` with the
>     start and end ranges of the frequencies.
> 3) If removing frequencies, then call `wbrf_remove_exclusion` with
>     start and end ranges of the frequencies.
> 
> Drivers/subsystems responding to frequencies:
> 
> 1) During probe, check `wbrf_supported_consumer` to see if WBRF is supported
>     for the device.
> 2) Call the `wbrf_retrieve_exclusions` to retrieve the current
>     exclusions on receiving an ACPI notification for a new frequency
>     change.
> 
> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> Co-developed-by: Evan Quan <evan.quan@amd.com>
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> --
> v4->v5:
>    - promote this to be a more generic solution with input argument taking
>      `struct device` and provide better scalability to support non-ACPI
>      scenarios(Andrew)
>    - update the APIs naming and some other minor fixes(Rafael)
> ---
>   drivers/base/Kconfig  |   8 ++
>   drivers/base/Makefile |   1 +
>   drivers/base/wbrf.c   | 227 ++++++++++++++++++++++++++++++++++++++++++
>   include/linux/wbrf.h  |  65 ++++++++++++
>   4 files changed, 301 insertions(+)
>   create mode 100644 drivers/base/wbrf.c
>   create mode 100644 include/linux/wbrf.h
> 
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 2b8fd6bb7da0..5b441017b225 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -242,4 +242,12 @@ config FW_DEVLINK_SYNC_STATE_TIMEOUT
>   	  command line option on every system/board your kernel is expected to
>   	  work on.
>   
> +config WBRF
> +	bool "Wifi band RF mitigation mechanism"
> +	default n
> +	help
> +	  Wifi band RF mitigation mechanism allows multiple drivers from
> +	  different domains to notify the frequencies in use so that hardware
> +	  can be reconfigured to avoid harmonic conflicts.
> +
>   endmenu
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 3079bfe53d04..c844f68a6830 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_GENERIC_MSI_IRQ) += platform-msi.o
>   obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
>   obj-$(CONFIG_GENERIC_ARCH_NUMA) += arch_numa.o
>   obj-$(CONFIG_ACPI) += physical_location.o
> +obj-$(CONFIG_WBRF) += wbrf.o
>   
>   obj-y			+= test/
>   
> diff --git a/drivers/base/wbrf.c b/drivers/base/wbrf.c
> new file mode 100644
> index 000000000000..2163a8ec8a9a
> --- /dev/null
> +++ b/drivers/base/wbrf.c
> @@ -0,0 +1,227 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Wifi Band Exclusion Interface
> + * Copyright (C) 2023 Advanced Micro Devices
> + *
> + */
> +
> +#include <linux/wbrf.h>
> +
> +static BLOCKING_NOTIFIER_HEAD(wbrf_chain_head);
> +static DEFINE_MUTEX(wbrf_mutex);
> +static struct exclusion_range_pool wbrf_pool;
> +
> +static int _wbrf_add_exclusion_ranges(struct wbrf_ranges_in *in)
> +{
> +	int i, j;
> +
> +	for (i = 0; i < ARRAY_SIZE(in->band_list); i++) {
> +		if (!in->band_list[i].start &&
> +		    !in->band_list[i].end)
> +			continue;
> +
> +		for (j = 0; j < ARRAY_SIZE(wbrf_pool.band_list); j++) {
> +			if (wbrf_pool.band_list[j].start == in->band_list[i].start &&
> +			    wbrf_pool.band_list[j].end == in->band_list[i].end) {
> +				wbrf_pool.ref_counter[j]++;
> +				break;
> +			}
> +		}
> +		if (j < ARRAY_SIZE(wbrf_pool.band_list))
> +			continue;
> +
> +		for (j = 0; j < ARRAY_SIZE(wbrf_pool.band_list); j++) {
> +			if (!wbrf_pool.band_list[j].start &&
> +			    !wbrf_pool.band_list[j].end) {
> +				wbrf_pool.band_list[j].start = in->band_list[i].start;
> +				wbrf_pool.band_list[j].end = in->band_list[i].end;
> +				wbrf_pool.ref_counter[j] = 1;
> +				break;
> +			}
> +		}
> +		if (j >= ARRAY_SIZE(wbrf_pool.band_list))
> +			return -ENOSPC;
> +	}
> +
> +	return 0;
> +}
> +
> +static int _wbrf_remove_exclusion_ranges(struct wbrf_ranges_in *in)
> +{
> +	int i, j;
> +
> +	for (i = 0; i < ARRAY_SIZE(in->band_list); i++) {
> +		if (!in->band_list[i].start &&
> +		    !in->band_list[i].end)
> +			continue;
> +
> +		for (j = 0; j < ARRAY_SIZE(wbrf_pool.band_list); j++) {
> +			if (wbrf_pool.band_list[j].start == in->band_list[i].start &&
> +			    wbrf_pool.band_list[j].end == in->band_list[i].end) {
> +				wbrf_pool.ref_counter[j]--;
> +				if (!wbrf_pool.ref_counter[j]) {
> +					wbrf_pool.band_list[j].start = 0;
> +					wbrf_pool.band_list[j].end = 0;
> +				}
> +				break;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int _wbrf_retrieve_exclusion_ranges(struct wbrf_ranges_out *out)
> +{
> +	int out_idx = 0;
> +	int i;
> +
> +	memset(out, 0, sizeof(*out));
> +
> +	for (i = 0; i < ARRAY_SIZE(wbrf_pool.band_list); i++) {
> +		if (!wbrf_pool.band_list[i].start &&
> +		    !wbrf_pool.band_list[i].end)
> +			continue;
> +
> +		out->band_list[out_idx].start = wbrf_pool.band_list[i].start;
> +		out->band_list[out_idx++].end = wbrf_pool.band_list[i].end;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * wbrf_supported_producer - Determine if the device can report frequencies
> + *
> + * @dev: device pointer
> + *
> + * WBRF is used to mitigate devices that cause harmonic interference.
> + * This function will determine if this device needs to report such frequencies.
> + */
> +bool wbrf_supported_producer(struct device *dev)
> +{
> +	return true;
> +}
> +EXPORT_SYMBOL_GPL(wbrf_supported_producer);
> +
> +/**
> + * wbrf_add_exclusion - Add frequency ranges to the exclusion list
> + *
> + * @dev: device pointer
> + * @in: input structure containing the frequency ranges to be added
> + *
> + * Add frequencies into the exclusion list for supported consumers
> + * to react to.
> + */
> +int wbrf_add_exclusion(struct device *dev,
> +		       struct wbrf_ranges_in *in)
> +{
> +	int r;
> +
> +	mutex_lock(&wbrf_mutex);
> +
> +	r = _wbrf_add_exclusion_ranges(in);
> +
> +	mutex_unlock(&wbrf_mutex);
> +	if (r)
> +		return r;
> +
> +	blocking_notifier_call_chain(&wbrf_chain_head, WBRF_CHANGED, NULL);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(wbrf_add_exclusion);
> +
> +/**
> + * wbrf_remove_exclusion - Remove frequency ranges from the exclusion list
> + *
> + * @dev: device pointer
> + * @in: input structure containing the frequency ranges to be removed
> + *
> + * Remove frequencies from the exclusion list for supported consumers
> + * to react to.
> + */
> +int wbrf_remove_exclusion(struct device *dev,
> +			  struct wbrf_ranges_in *in)
> +{
> +	int r;
> +
> +	mutex_lock(&wbrf_mutex);
> +
> +	r = _wbrf_remove_exclusion_ranges(in);
> +
> +	mutex_unlock(&wbrf_mutex);
> +	if (r)
> +		return r;
> +
> +	blocking_notifier_call_chain(&wbrf_chain_head, WBRF_CHANGED, NULL);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(wbrf_remove_exclusion);
> +
> +/**
> + * wbrf_supported_consumer - Determine if the device can react to frequencies
> + *
> + * @dev: device pointer
> + *
> + * WBRF is used to mitigate devices that cause harmonic interference.
> + * This function will determine if this device needs to react to reports from
> + * other devices for such frequencies.
> + */
> +bool wbrf_supported_consumer(struct device *dev)
> +{
> +	return true;
> +}
> +EXPORT_SYMBOL_GPL(wbrf_supported_consumer);
> +
> +/**
> + * wbrf_register_notifier - Register for notifications of frequency changes
> + *
> + * @nb: driver notifier block
> + *
> + * WBRF is used to mitigate devices that cause harmonic interference.
> + * This function will allow consumers to register for frequency notifications.
> + */
> +int wbrf_register_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(&wbrf_chain_head, nb);
> +}
> +EXPORT_SYMBOL_GPL(wbrf_register_notifier);
> +
> +/**
> + * wbrf_unregister_notifier - Unregister for notifications of frequency changes
> + *
> + * @nb: driver notifier block
> + *
> + * WBRF is used to mitigate devices that cause harmonic interference.
> + * This function will allow consumers to unregister for frequency notifications.
> + */
> +int wbrf_unregister_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_unregister(&wbrf_chain_head, nb);
> +}
> +EXPORT_SYMBOL_GPL(wbrf_unregister_notifier);
> +
> +/**
> + * wbrf_retrieve_exclusions - Retrieve the exclusion list
> + *
> + * @dev: device pointer
> + * @out: output structure containing the frequency ranges to be excluded
> + *
> + * Retrieve the current exclusion list
> + */
> +int wbrf_retrieve_exclusions(struct device *dev,
> +			     struct wbrf_ranges_out *out)
> +{
> +	int r;
> +
> +	mutex_lock(&wbrf_mutex);
> +
> +	r = _wbrf_retrieve_exclusion_ranges(out);
> +
> +	mutex_unlock(&wbrf_mutex);
> +
> +	return r;
> +}
> +EXPORT_SYMBOL_GPL(wbrf_retrieve_exclusions);
> diff --git a/include/linux/wbrf.h b/include/linux/wbrf.h
> new file mode 100644
> index 000000000000..3ca95786cef5
> --- /dev/null
> +++ b/include/linux/wbrf.h
> @@ -0,0 +1,65 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Wifi Band Exclusion Interface
> + * Copyright (C) 2023 Advanced Micro Devices
> + */
> +
> +#ifndef _LINUX_WBRF_H
> +#define _LINUX_WBRF_H
> +
> +#include <linux/device.h>
> +
> +/* Maximum number of wbrf ranges */
> +#define MAX_NUM_OF_WBRF_RANGES		11
> +
> +struct exclusion_range {
> +	/* start and end point of the frequency range in Hz */
> +	uint64_t	start;
> +	uint64_t	end;
> +};
> +
> +struct exclusion_range_pool {
> +	struct exclusion_range	band_list[MAX_NUM_OF_WBRF_RANGES];
> +	uint64_t		ref_counter[MAX_NUM_OF_WBRF_RANGES];
> +};
> +
> +struct wbrf_ranges_in {
> +	/* valid entry: `start` and `end` filled with non-zero values */
> +	struct exclusion_range	band_list[MAX_NUM_OF_WBRF_RANGES];
> +};
> +
> +struct wbrf_ranges_out {
> +	uint32_t		num_of_ranges;
> +	struct exclusion_range	band_list[MAX_NUM_OF_WBRF_RANGES];
> +} __packed;
> +
> +enum wbrf_notifier_actions {
> +	WBRF_CHANGED,
> +};
> +
> +#ifdef CONFIG_WBRF
> +bool wbrf_supported_producer(struct device *dev);
> +int wbrf_add_exclusion(struct device *adev,
> +		       struct wbrf_ranges_in *in);
> +int wbrf_remove_exclusion(struct device *dev,
> +			  struct wbrf_ranges_in *in);
> +int wbrf_retrieve_exclusions(struct device *dev,
> +			     struct wbrf_ranges_out *out);
> +bool wbrf_supported_consumer(struct device *dev);
> +
> +int wbrf_register_notifier(struct notifier_block *nb);
> +int wbrf_unregister_notifier(struct notifier_block *nb);
> +#else
> +static inline bool wbrf_supported_producer(struct device *dev) { return false; }
> +static inline int wbrf_add_exclusion(struct device *adev,
> +				     struct wbrf_ranges_in *in) { return -ENODEV; }
> +static inline int wbrf_remove_exclusion(struct device *dev,
> +					struct wbrf_ranges_in *in) { return -ENODEV; }
> +static inline int wbrf_retrieve_exclusions(struct device *dev,
> +					   struct wbrf_ranges_out *out) { return -ENODEV; }
> +static inline bool wbrf_supported_consumer(struct device *dev) { return false; }
> +static inline int wbrf_register_notifier(struct notifier_block *nb) { return -ENODEV; }
> +static inline int wbrf_unregister_notifier(struct notifier_block *nb) { return -ENODEV; }
> +#endif
> +

Right now there are stubs for non CONFIG_WBRF as well as other patches 
are using #ifdef CONFIG_WBRF or having their own stubs.  Like mac80211 
patch looks for #ifdef CONFIG_WBRF.

I think we should pick one or the other.

Having other subsystems #ifdef CONFIG_WBRF will make the series easier 
to land through multiple trees; so I have a slight leaning in that 
direction.

> +#endif /* _LINUX_WBRF_H */