Re: [PATCH v2 1/4] platform/x86/uniwill: uniwill-laptop: Rework hwmon feature defines

[Armin Wolf <W_Armin@gmx.de>]

Am 26.02.26 um 21:50 schrieb Werner Sembach:

> From: Armin Wolf <W_Armin@gmx.de>
>
> Split hwmon feature define in smaller parts to accommodate for diverse
> hardware. You can now specify the presence of a cpu and/or a gpu temp
> sensor separately and if one or 2 fans exists.
>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> ---
>   drivers/platform/x86/uniwill/uniwill-acpi.c | 68 ++++++++++++++++++---
>   1 file changed, 61 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
> index 6341dca20b76a..7ab82cf16f388 100644
> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
> @@ -319,8 +319,11 @@
>   #define UNIWILL_FEATURE_TOUCHPAD_TOGGLE		BIT(2)
>   #define UNIWILL_FEATURE_LIGHTBAR		BIT(3)
>   #define UNIWILL_FEATURE_BATTERY			BIT(4)
> -#define UNIWILL_FEATURE_HWMON			BIT(5)
> -#define UNIWILL_FEATURE_NVIDIA_CTGP_CONTROL	BIT(6)
> +#define UNIWILL_FEATURE_CPU_TEMP		BIT(5)
> +#define UNIWILL_FEATURE_GPU_TEMP		BIT(6)
> +#define UNIWILL_FEATURE_PRIMARY_FAN		BIT(7)
> +#define UNIWILL_FEATURE_SECONDARY_FAN		BIT(8)
> +#define UNIWILL_FEATURE_NVIDIA_CTGP_CONTROL	BIT(9)
>   
>   struct uniwill_data {
>   	struct device *dev;
> @@ -427,7 +430,7 @@ static const struct key_entry uniwill_keymap[] = {
>   	{ KE_END }
>   };
>   
> -static inline bool uniwill_device_supports(struct uniwill_data *data,
> +static inline bool uniwill_device_supports(const struct uniwill_data *data,
>   					   unsigned int features)
>   {
>   	return (data->features & features) == features;
> @@ -937,6 +940,48 @@ static const struct attribute_group *uniwill_groups[] = {
>   	NULL
>   };
>   
> +static umode_t uniwill_is_visible(const void *drvdata, enum hwmon_sensor_types type, u32 attr,
> +				  int channel)
> +{
> +	const struct uniwill_data *data = drvdata;
> +	unsigned int feature;
> +
> +	switch (type) {
> +	case hwmon_temp:
> +		switch (channel) {
> +		case 0:
> +			feature = UNIWILL_FEATURE_CPU_TEMP;
> +			break;
> +		case 1:
> +			feature = UNIWILL_FEATURE_GPU_TEMP;
> +			break;
> +		default:
> +			return 0;
> +		}
> +		break;
> +	case hwmon_fan:
> +	case hwmon_pwm:
> +		switch (channel) {
> +		case 0:
> +			feature = UNIWILL_FEATURE_PRIMARY_FAN;
> +			break;
> +		case 1:
> +			feature = UNIWILL_FEATURE_SECONDARY_FAN;
> +			break;
> +		default:
> +			return 0;
> +		}
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	if (uniwill_device_supports(data, feature))
> +		return 0440;

I think you forgot this one.

> +
> +	return 0;
> +}
> +
>   static int uniwill_read(struct device *dev, enum hwmon_sensor_types type, u32 attr, int channel,
>   			long *val)
>   {
> @@ -1020,7 +1065,7 @@ static int uniwill_read_string(struct device *dev, enum hwmon_sensor_types type,
>   }
>   
>   static const struct hwmon_ops uniwill_ops = {
> -	.visible = 0444,
> +	.is_visible = uniwill_is_visible,
>   	.read = uniwill_read,
>   	.read_string = uniwill_read_string,
>   };
> @@ -1048,7 +1093,10 @@ static int uniwill_hwmon_init(struct uniwill_data *data)
>   {
>   	struct device *hdev;
>   
> -	if (!uniwill_device_supports(data, UNIWILL_FEATURE_HWMON))
> +	if (!uniwill_device_supports(data, UNIWILL_FEATURE_CPU_TEMP) &&
> +	    !uniwill_device_supports(data, UNIWILL_FEATURE_GPU_TEMP) &&
> +	    !uniwill_device_supports(data, UNIWILL_FEATURE_PRIMARY_FAN) &&
> +	    !uniwill_device_supports(data, UNIWILL_FEATURE_SECONDARY_FAN))
>   		return 0;
>   
>   	hdev = devm_hwmon_device_register_with_info(data->dev, "uniwill", data,
> @@ -1687,7 +1735,10 @@ static struct uniwill_device_descriptor lapac71h_descriptor __initdata = {
>   		    UNIWILL_FEATURE_SUPER_KEY |
>   		    UNIWILL_FEATURE_TOUCHPAD_TOGGLE |
>   		    UNIWILL_FEATURE_BATTERY |
> -		    UNIWILL_FEATURE_HWMON,
> +		    UNIWILL_FEATURE_CPU_TEMP |
> +		    UNIWILL_FEATURE_GPU_TEMP |
> +		    UNIWILL_FEATURE_PRIMARY_FAN |
> +		    UNIWILL_FEATURE_SECONDARY_FAN,
>   };
>   
>   static struct uniwill_device_descriptor lapkc71f_descriptor __initdata = {
> @@ -1696,7 +1747,10 @@ static struct uniwill_device_descriptor lapkc71f_descriptor __initdata = {
>   		    UNIWILL_FEATURE_TOUCHPAD_TOGGLE |
>   		    UNIWILL_FEATURE_LIGHTBAR |
>   		    UNIWILL_FEATURE_BATTERY |
> -		    UNIWILL_FEATURE_HWMON,
> +		    UNIWILL_FEATURE_CPU_TEMP |
> +		    UNIWILL_FEATURE_GPU_TEMP |
> +		    UNIWILL_FEATURE_PRIMARY_FAN |
> +		    UNIWILL_FEATURE_SECONDARY_FAN,
>   };
>   
>   static int phxarx1_phxaqf1_probe(struct uniwill_data *data)