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

[Werner Sembach <wse@tuxedocomputers.com>]

Am 27.02.26 um 10:28 schrieb Armin Wolf:
> 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.
yes missed it
>
>> +
>> +    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)