Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 1 Jul 2023 02:19:31 +0200
From:   Andrew Lunn <andrew@lunn.ch>
To:     Evan Quan <evan.quan@amd.com>
Cc:     rafael@kernel.org, lenb@kernel.org, Alexander.Deucher@amd.com,
        Christian.Koenig@amd.com, Xinhui.Pan@amd.com, airlied@gmail.com,
        daniel@ffwll.ch, johannes@sipsolutions.net, davem@davemloft.net,
        edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
        Mario.Limonciello@amd.com, mdaenzer@redhat.com,
        maarten.lankhorst@linux.intel.com, tzimmermann@suse.de,
        hdegoede@redhat.com, jingyuwang_vip@163.com, Lijo.Lazar@amd.com,
        jim.cromie@gmail.com, bellosilicio@gmail.com,
        andrealmeid@igalia.com, trix@redhat.com, jsg@jsg.id.au,
        arnd@arndb.de, linux-kernel@vger.kernel.org,
        linux-acpi@vger.kernel.org, amd-gfx@lists.freedesktop.org,
        dri-devel@lists.freedesktop.org, linux-wireless@vger.kernel.org,
        netdev@vger.kernel.org
Subject: Re: [PATCH V5 1/9] drivers core: Add support for Wifi band RF
 mitigations
Message-ID: <7e7db6eb-4f46-407a-8d1f-16688554ad80@lunn.ch>
References: <20230630103240.1557100-1-evan.quan@amd.com>
 <20230630103240.1557100-2-evan.quan@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230630103240.1557100-2-evan.quan@amd.com>
Precedence: bulk

> Drivers/subsystems contributing frequencies:
> 
> 1) During probe, check `wbrf_supported_producer` to see if WBRF supported
>    for the device.

What is the purpose of this stage? Why would it not be supported for
this device?

> +#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; }

The normal aim of stubs is that so long as it is not expected to be
fatal if the functionality is missing, the caller should not care if
it is missing. So i would expect these to return 0, indicating
everything worked as expected.

> +static inline int wbrf_retrieve_exclusions(struct device *dev,
> +					   struct wbrf_ranges_out *out) { return -ENODEV; }

This is more complex. Ideally you want to return an empty set, so
there is nothing to do. So i think the stub probably wants to do a
memset and then return 0.

> +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; }

And these can just return 0.

    Andrew