On 2024-11-13 22:51:37-0800, Guenter Roeck wrote: > On 11/13/24 20:40, Thomas Weißschuh wrote: > > On 2024-11-12 22:52:36-0800, Guenter Roeck wrote: > > > On 11/12/24 20:39, Thomas Weißschuh wrote: > > > > Using an #ifdef in a C source files to have different definitions > > > > of the same symbol makes the code harder to read and understand. > > > > Furthermore it makes it harder to test compilation of the different > > > > branches. > > > > > > > > Replace the ifdeffery with IS_ENABLED() which is just a normal > > > > conditional. > > > > The resulting binary is still the same as before as the compiler > > > > optimizes away all the unused code and definitions. > > > > > > > > Signed-off-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx> > > > > --- > > > > This confused me a bit while looking at the implementation of > > > > HWMON_C_REGISTER_TZ. > > > > --- > > > > drivers/hwmon/hwmon.c | 21 ++++++--------------- > > > > 1 file changed, 6 insertions(+), 15 deletions(-) > > > > > > > > diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c > > > > index 9c35c4d0369d7aad7ea61ccd25f4f63fc98b9e02..86fb674c85d3f54d475be014c3fd3dd74c815c57 100644 > > > > --- a/drivers/hwmon/hwmon.c > > > > +++ b/drivers/hwmon/hwmon.c > > > > @@ -147,11 +147,6 @@ static DEFINE_IDA(hwmon_ida); > > > > /* Thermal zone handling */ > > > > -/* > > > > - * The complex conditional is necessary to avoid a cyclic dependency > > > > - * between hwmon and thermal_sys modules. > > > > - */ > > > > -#ifdef CONFIG_THERMAL_OF > > > > static int hwmon_thermal_get_temp(struct thermal_zone_device *tz, int *temp) > > > > { > > > > struct hwmon_thermal_data *tdata = thermal_zone_device_priv(tz); > > > > @@ -257,6 +252,9 @@ static int hwmon_thermal_register_sensors(struct device *dev) > > > > void *drvdata = dev_get_drvdata(dev); > > > > int i; > > > > + if (!IS_ENABLED(CONFIG_THERMAL_OF)) > > > > + return 0; > > > > + > > > > for (i = 1; info[i]; i++) { > > > > int j; > > > > @@ -285,6 +283,9 @@ static void hwmon_thermal_notify(struct device *dev, int index) > > > > struct hwmon_device *hwdev = to_hwmon_device(dev); > > > > struct hwmon_thermal_data *tzdata; > > > > + if (!IS_ENABLED(CONFIG_THERMAL_OF)) > > > > + return; > > > > + > > > > list_for_each_entry(tzdata, &hwdev->tzdata, node) { > > > > if (tzdata->index == index) { > > > > thermal_zone_device_update(tzdata->tzd, > > > > > > There is no dummy function for thermal_zone_device_update(). > > > I really don't want to trust the compiler/linker to remove that code > > > unless someone points me to a document explaining that it is guaranteed > > > to not cause any problems. > > > > I'm fairly sure that a declaration should be enough, and believe > > to remember seeing such advise somewhere. > > However there is not even a function declaration with !CONFIG_THERMAL. > > So I can add an actual stub for it for v2. > > > > What do you think? > > > You mean an extern declaration without the actual function ? Stub as in empty inline function: static inline void thermal_zone_device_update(struct thermal_zone_device *, enum thermal_notify_event) { } > I'd really want to see that documented. It would seem rather unusual.