Re: [PATCH] hwmon: (core) Avoid ifdef in C source file

[Guenter Roeck <linux@roeck-us.net>]

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@weissschuh.net>
> ---
> 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.

Guenter

> @@ -293,16 +294,6 @@ static void hwmon_thermal_notify(struct device *dev, int index)
>   	}
>   }
>   
> -#else
> -static int hwmon_thermal_register_sensors(struct device *dev)
> -{
> -	return 0;
> -}
> -
> -static void hwmon_thermal_notify(struct device *dev, int index) { }
> -
> -#endif /* IS_REACHABLE(CONFIG_THERMAL) && ... */
> -
>   static int hwmon_attr_base(enum hwmon_sensor_types type)
>   {
>   	if (type == hwmon_in || type == hwmon_intrusion)
> 
> ---
> base-commit: 3022e9d00ebec31ed435ae0844e3f235dba998a9
> change-id: 20241113-hwmon-thermal-2d2da581c276
> 
> Best regards,