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

[Werner Sembach]

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@xxxxxx>
> >
> > 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@xxxxxx>
> > Signed-off-by: Werner Sembach <wse@xxxxxxxxxxxxxxxxxxx>
> > ---
> >   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)