[AMD Official Use Only - General] > -----Original Message----- > From: Andrew Lunn <andrew@xxxxxxx> > Sent: Saturday, July 1, 2023 8:20 AM > To: Quan, Evan <Evan.Quan@xxxxxxx> > Cc: rafael@xxxxxxxxxx; lenb@xxxxxxxxxx; Deucher, Alexander > <Alexander.Deucher@xxxxxxx>; Koenig, Christian > <Christian.Koenig@xxxxxxx>; Pan, Xinhui <Xinhui.Pan@xxxxxxx>; > airlied@xxxxxxxxx; daniel@xxxxxxxx; johannes@xxxxxxxxxxxxxxxx; > davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; > pabeni@xxxxxxxxxx; Limonciello, Mario <Mario.Limonciello@xxxxxxx>; > mdaenzer@xxxxxxxxxx; maarten.lankhorst@xxxxxxxxxxxxxxx; > tzimmermann@xxxxxxx; hdegoede@xxxxxxxxxx; jingyuwang_vip@xxxxxxx; > Lazar, Lijo <Lijo.Lazar@xxxxxxx>; jim.cromie@xxxxxxxxx; > bellosilicio@xxxxxxxxx; andrealmeid@xxxxxxxxxx; trix@xxxxxxxxxx; > jsg@xxxxxxxxx; arnd@xxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux- > acpi@xxxxxxxxxxxxxxx; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; dri- > devel@xxxxxxxxxxxxxxxxxxxxx; linux-wireless@xxxxxxxxxxxxxxx; > netdev@xxxxxxxxxxxxxxx > Subject: Re: [PATCH V5 1/9] drivers core: Add support for Wifi band RF > mitigations > > > 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? This is needed for wbrf support via ACPI mechanism. If BIOS(AML code) does not support the wbrf adding/removing for some device, it should speak that out so that the device can be aware of that. > > > +#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. Sure, that makes sense. > > > +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. Right, will update it accordingly. > > > +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. Will update it. Evan > > Andrew