On 26/04/17 19:46, Oscar Salvador wrote:
> This patch removes old code related to the old api and transforms the
> functions for the new api. It also adds the .write and .read operations.
> 
> Signed-off-by: Oscar Salvador <osalvador.vilard...@gmail.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_hwmon.c | 722 
> +++++++++++---------------------
>  1 file changed, 249 insertions(+), 473 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c 
> b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> index e8ea8d0..4db65fb 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> @@ -38,21 +38,17 @@
>  #include <nvkm/subdev/volt.h>
>  
>  #if defined(CONFIG_HWMON) || (defined(MODULE) && 
> defined(CONFIG_HWMON_MODULE))
> -static ssize_t
> -nouveau_hwmon_show_temp(struct device *d, struct device_attribute *a, char 
> *buf)
> +static int
> +nouveau_hwmon_show_temp(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
>       int temp = nvkm_therm_temp_get(therm);
>  
>       if (temp < 0)
>               return temp;
>  
> -     return snprintf(buf, PAGE_SIZE, "%d\n", temp * 1000);
> +     return (temp * 1000);
>  }
> -static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, nouveau_hwmon_show_temp,
> -                                               NULL, 0);
>  
>  static ssize_t
>  nouveau_hwmon_show_temp1_auto_point1_pwm(struct device *d,
> @@ -129,312 +125,100 @@ static 
> SENSOR_DEVICE_ATTR(temp1_auto_point1_temp_hyst, S_IRUGO | S_IWUSR,
>                         nouveau_hwmon_temp1_auto_point1_temp_hyst,
>                         nouveau_hwmon_set_temp1_auto_point1_temp_hyst, 0);
>  
> -static ssize_t
> -nouveau_hwmon_max_temp(struct device *d, struct device_attribute *a, char 
> *buf)
> -{
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
> -     struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> -
> -     return snprintf(buf, PAGE_SIZE, "%d\n",
> -            therm->attr_get(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK) * 1000);
> -}
> -static ssize_t
> -nouveau_hwmon_set_max_temp(struct device *d, struct device_attribute *a,
> -                                             const char *buf, size_t count)
> +static int
> +nouveau_hwmon_max_temp(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> -     long value;
>  
> -     if (kstrtol(buf, 10, &value) == -EINVAL)
> -             return count;
> -
> -     therm->attr_set(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK, value / 1000);
> -
> -     return count;
> +     return therm->attr_get(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK) * 1000;
>  }
> -static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO | S_IWUSR, 
> nouveau_hwmon_max_temp,
> -                                               nouveau_hwmon_set_max_temp,
> -                                               0);
>  
> -static ssize_t
> -nouveau_hwmon_max_temp_hyst(struct device *d, struct device_attribute *a,
> -                         char *buf)
> -{
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
> -     struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> -
> -     return snprintf(buf, PAGE_SIZE, "%d\n",
> -       therm->attr_get(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK_HYST) * 1000);
> -}
> -static ssize_t
> -nouveau_hwmon_set_max_temp_hyst(struct device *d, struct device_attribute *a,
> -                                             const char *buf, size_t count)
> +static int
> +nouveau_hwmon_max_temp_hyst(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> -     long value;
> -
> -     if (kstrtol(buf, 10, &value) == -EINVAL)
> -             return count;
>  
> -     therm->attr_set(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK_HYST,
> -                     value / 1000);
> -
> -     return count;
> +     return therm->attr_get(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK_HYST) * 
> 1000;
>  }
> -static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IRUGO | S_IWUSR,
> -                       nouveau_hwmon_max_temp_hyst,
> -                       nouveau_hwmon_set_max_temp_hyst, 0);
> -
> -static ssize_t
> -nouveau_hwmon_critical_temp(struct device *d, struct device_attribute *a,
> -                                                     char *buf)
> -{
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
> -     struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
>  
> -     return snprintf(buf, PAGE_SIZE, "%d\n",
> -            therm->attr_get(therm, NVKM_THERM_ATTR_THRS_CRITICAL) * 1000);
> -}
> -static ssize_t
> -nouveau_hwmon_set_critical_temp(struct device *d, struct device_attribute *a,
> -                                                         const char *buf,
> -                                                             size_t count)
> +static int
> +nouveau_hwmon_critical_temp(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> -     long value;
> -
> -     if (kstrtol(buf, 10, &value) == -EINVAL)
> -             return count;
> -
> -     therm->attr_set(therm, NVKM_THERM_ATTR_THRS_CRITICAL, value / 1000);
>  
> -     return count;
> +     return therm->attr_get(therm, NVKM_THERM_ATTR_THRS_CRITICAL) * 1000;
>  }
> -static SENSOR_DEVICE_ATTR(temp1_crit, S_IRUGO | S_IWUSR,
> -                                             nouveau_hwmon_critical_temp,
> -                                             nouveau_hwmon_set_critical_temp,
> -                                             0);
>  
> -static ssize_t
> -nouveau_hwmon_critical_temp_hyst(struct device *d, struct device_attribute 
> *a,
> -                                                     char *buf)
> +static int
> +nouveau_hwmon_critical_temp_hyst(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
>  
> -     return snprintf(buf, PAGE_SIZE, "%d\n",
> -       therm->attr_get(therm, NVKM_THERM_ATTR_THRS_CRITICAL_HYST) * 1000);
> +     return therm->attr_get(therm, NVKM_THERM_ATTR_THRS_CRITICAL_HYST) * 
> 1000;
>  }
> -static ssize_t
> -nouveau_hwmon_set_critical_temp_hyst(struct device *d,
> -                                  struct device_attribute *a,
> -                                  const char *buf,
> -                                  size_t count)
> -{
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
> -     struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> -     long value;
>  
> -     if (kstrtol(buf, 10, &value) == -EINVAL)
> -             return count;
> -
> -     therm->attr_set(therm, NVKM_THERM_ATTR_THRS_CRITICAL_HYST,
> -                     value / 1000);
> -
> -     return count;
> -}
> -static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IRUGO | S_IWUSR,
> -                       nouveau_hwmon_critical_temp_hyst,
> -                       nouveau_hwmon_set_critical_temp_hyst, 0);
> -static ssize_t
> -nouveau_hwmon_emergency_temp(struct device *d, struct device_attribute *a,
> -                                                     char *buf)
> +static int
> +nouveau_hwmon_emergency_temp(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
>  
> -     return snprintf(buf, PAGE_SIZE, "%d\n",
> -            therm->attr_get(therm, NVKM_THERM_ATTR_THRS_SHUTDOWN) * 1000);
> +     return therm->attr_get(therm, NVKM_THERM_ATTR_THRS_SHUTDOWN) * 1000;
>  }
> -static ssize_t
> -nouveau_hwmon_set_emergency_temp(struct device *d, struct device_attribute 
> *a,
> -                                                         const char *buf,
> -                                                             size_t count)
> -{
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
> -     struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> -     long value;
>  
> -     if (kstrtol(buf, 10, &value) == -EINVAL)
> -             return count;
> -
> -     therm->attr_set(therm, NVKM_THERM_ATTR_THRS_SHUTDOWN, value / 1000);
> -
> -     return count;
> -}
> -static SENSOR_DEVICE_ATTR(temp1_emergency, S_IRUGO | S_IWUSR,
> -                                     nouveau_hwmon_emergency_temp,
> -                                     nouveau_hwmon_set_emergency_temp,
> -                                     0);
> -
> -static ssize_t
> -nouveau_hwmon_emergency_temp_hyst(struct device *d, struct device_attribute 
> *a,
> -                                                     char *buf)
> -{
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
> -     struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> -
> -     return snprintf(buf, PAGE_SIZE, "%d\n",
> -       therm->attr_get(therm, NVKM_THERM_ATTR_THRS_SHUTDOWN_HYST) * 1000);
> -}
> -static ssize_t
> -nouveau_hwmon_set_emergency_temp_hyst(struct device *d,
> -                                   struct device_attribute *a,
> -                                   const char *buf,
> -                                   size_t count)
> +static int
> +nouveau_hwmon_emergency_temp_hyst(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> -     long value;
> -
> -     if (kstrtol(buf, 10, &value) == -EINVAL)
> -             return count;
> -
> -     therm->attr_set(therm, NVKM_THERM_ATTR_THRS_SHUTDOWN_HYST,
> -                     value / 1000);
>  
> -     return count;
> -}
> -static SENSOR_DEVICE_ATTR(temp1_emergency_hyst, S_IRUGO | S_IWUSR,
> -                                     nouveau_hwmon_emergency_temp_hyst,
> -                                     nouveau_hwmon_set_emergency_temp_hyst,
> -                                     0);
> -
> -static ssize_t nouveau_hwmon_show_name(struct device *dev,
> -                                   struct device_attribute *attr,
> -                                   char *buf)
> -{
> -     return sprintf(buf, "nouveau\n");
> +     return therm->attr_get(therm, NVKM_THERM_ATTR_THRS_SHUTDOWN_HYST) * 
> 1000;
>  }
> -static SENSOR_DEVICE_ATTR(name, S_IRUGO, nouveau_hwmon_show_name, NULL, 0);
>  
> -static ssize_t nouveau_hwmon_show_update_rate(struct device *dev,
> -                                   struct device_attribute *attr,
> -                                   char *buf)
> +static int
> +nouveau_hwmon_show_update_rate(struct nouveau_drm *drm)
>  {
> -     return sprintf(buf, "1000\n");
> +     return 1000;
>  }
> -static SENSOR_DEVICE_ATTR(update_rate, S_IRUGO,
> -                                             nouveau_hwmon_show_update_rate,
> -                                             NULL, 0);
>  
> -static ssize_t
> -nouveau_hwmon_show_fan1_input(struct device *d, struct device_attribute 
> *attr,
> -                           char *buf)
> +static int
> +nouveau_hwmon_show_fan1_input(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
>  
> -     return snprintf(buf, PAGE_SIZE, "%d\n", nvkm_therm_fan_sense(therm));
> +     return nvkm_therm_fan_sense(therm);
>  }
> -static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, nouveau_hwmon_show_fan1_input,
> -                       NULL, 0);
>  
> - static ssize_t
> -nouveau_hwmon_get_pwm1_enable(struct device *d,
> -                        struct device_attribute *a, char *buf)
> +static int
> +nouveau_hwmon_get_pwm1_enable(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> -     int ret;
>  
> -     ret = therm->attr_get(therm, NVKM_THERM_ATTR_FAN_MODE);
> -     if (ret < 0)
> -             return ret;
> -
> -     return sprintf(buf, "%i\n", ret);
> +     return therm->attr_get(therm, NVKM_THERM_ATTR_FAN_MODE);
>  }
>  
> -static ssize_t
> -nouveau_hwmon_set_pwm1_enable(struct device *d, struct device_attribute *a,
> -                        const char *buf, size_t count)
> +static int
> +nouveau_hwmon_set_pwm1_enable(struct nouveau_drm *drm, long value)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> -     long value;
> -     int ret;
>  
> -     ret = kstrtol(buf, 10, &value);
> -     if (ret)
> -             return ret;
> -
> -     ret = therm->attr_set(therm, NVKM_THERM_ATTR_FAN_MODE, value);
> -     if (ret)
> -             return ret;
> -     else
> -             return count;
> +     return therm->attr_set(therm, NVKM_THERM_ATTR_FAN_MODE, value);
>  }
> -static SENSOR_DEVICE_ATTR(pwm1_enable, S_IRUGO | S_IWUSR,
> -                       nouveau_hwmon_get_pwm1_enable,
> -                       nouveau_hwmon_set_pwm1_enable, 0);
>  
> -static ssize_t
> -nouveau_hwmon_get_pwm1(struct device *d, struct device_attribute *a, char 
> *buf)
> +static int
> +nouveau_hwmon_get_pwm1(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> -     int ret;
> -
> -     ret = therm->fan_get(therm);
> -     if (ret < 0)
> -             return ret;
>  
> -     return sprintf(buf, "%i\n", ret);
> +     return therm->fan_get(therm);
>  }
>  
> -static ssize_t
> -nouveau_hwmon_set_pwm1(struct device *d, struct device_attribute *a,
> -                    const char *buf, size_t count)
> +static int
> +nouveau_hwmon_set_pwm1(struct nouveau_drm *drm, long value)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> -     int ret = -ENODEV;
> -     long value;
> -
> -     if (kstrtol(buf, 10, &value) == -EINVAL)
> -             return -EINVAL;
> -
> -     ret = therm->fan_set(therm, value);
> -     if (ret)
> -             return ret;
>  
> -     return count;
> +     return therm->fan_set(therm, value);
>  }
>  
> -static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR,
> -                       nouveau_hwmon_get_pwm1,
> -                       nouveau_hwmon_set_pwm1, 0);
> -
>  static ssize_t
>  nouveau_hwmon_get_pwm1_min(struct device *d,
>                          struct device_attribute *a, char *buf)
> @@ -515,12 +299,9 @@ static SENSOR_DEVICE_ATTR(pwm1_max, S_IRUGO | S_IWUSR,
>                         nouveau_hwmon_get_pwm1_max,
>                         nouveau_hwmon_set_pwm1_max, 0);
>  
> -static ssize_t
> -nouveau_hwmon_get_in0_input(struct device *d,
> -                         struct device_attribute *a, char *buf)
> +static int
> +nouveau_hwmon_get_in0_input(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_volt *volt = nvxx_volt(&drm->client.device);
>       int ret;
>  
> @@ -528,170 +309,54 @@ nouveau_hwmon_get_in0_input(struct device *d,
>       if (ret < 0)
>               return ret;
>  
> -     return sprintf(buf, "%i\n", ret / 1000);
> +     return (ret / 1000);
>  }
>  
> -static SENSOR_DEVICE_ATTR(in0_input, S_IRUGO,
> -                       nouveau_hwmon_get_in0_input, NULL, 0);
> -
> -static ssize_t
> -nouveau_hwmon_get_in0_min(struct device *d,
> -                         struct device_attribute *a, char *buf)
> +static int
> +nouveau_hwmon_get_in0_min(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_volt *volt = nvxx_volt(&drm->client.device);
>  
>       if (!volt || !volt->min_uv)
>               return -ENODEV;
>  
> -     return sprintf(buf, "%i\n", volt->min_uv / 1000);
> +     return (volt->min_uv / 1000);
>  }
>  
> -static SENSOR_DEVICE_ATTR(in0_min, S_IRUGO,
> -                       nouveau_hwmon_get_in0_min, NULL, 0);
> -
> -static ssize_t
> -nouveau_hwmon_get_in0_max(struct device *d,
> -                         struct device_attribute *a, char *buf)
> +static int
> +nouveau_hwmon_get_in0_max(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_volt *volt = nvxx_volt(&drm->client.device);
>  
>       if (!volt || !volt->max_uv)
>               return -ENODEV;
>  
> -     return sprintf(buf, "%i\n", volt->max_uv / 1000);
> -}
> -
> -static SENSOR_DEVICE_ATTR(in0_max, S_IRUGO,
> -                       nouveau_hwmon_get_in0_max, NULL, 0);
> -
> -static ssize_t
> -nouveau_hwmon_get_in0_label(struct device *d,
> -                         struct device_attribute *a, char *buf)
> -{
> -     return sprintf(buf, "GPU core\n");
> +     return (volt->max_uv / 1000);
>  }
>  
> -static SENSOR_DEVICE_ATTR(in0_label, S_IRUGO,
> -                       nouveau_hwmon_get_in0_label, NULL, 0);
> -
> -static ssize_t
> -nouveau_hwmon_get_power1_input(struct device *d, struct device_attribute *a,
> -                            char *buf)
> +static int
> +nouveau_hwmon_get_power1_input(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_iccsense *iccsense = nvxx_iccsense(&drm->client.device);
> -     int result = nvkm_iccsense_read_all(iccsense);
>  
> -     if (result < 0)
> -             return result;
> -
> -     return sprintf(buf, "%i\n", result);
> +     return nvkm_iccsense_read_all(iccsense);
>  }
>  
> -static SENSOR_DEVICE_ATTR(power1_input, S_IRUGO,
> -                       nouveau_hwmon_get_power1_input, NULL, 0);
> -
> -static ssize_t
> -nouveau_hwmon_get_power1_max(struct device *d, struct device_attribute *a,
> -                          char *buf)
> +static int
> +nouveau_hwmon_get_power1_max(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_iccsense *iccsense = nvxx_iccsense(&drm->client.device);
> -     return sprintf(buf, "%i\n", iccsense->power_w_max);
> -}
>  
> -static SENSOR_DEVICE_ATTR(power1_max, S_IRUGO,
> -                       nouveau_hwmon_get_power1_max, NULL, 0);
> +     return iccsense->power_w_max;
> +}
>  
> -static ssize_t
> -nouveau_hwmon_get_power1_crit(struct device *d, struct device_attribute *a,
> -                           char *buf)
> +static int
> +nouveau_hwmon_get_power1_crit(struct nouveau_drm *drm)
>  {
> -     struct drm_device *dev = dev_get_drvdata(d);
> -     struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_iccsense *iccsense = nvxx_iccsense(&drm->client.device);
> -     return sprintf(buf, "%i\n", iccsense->power_w_crit);
> -}
> -
> -static SENSOR_DEVICE_ATTR(power1_crit, S_IRUGO,
> -                       nouveau_hwmon_get_power1_crit, NULL, 0);
> -
> -static struct attribute *hwmon_default_attributes[] = {
> -     &sensor_dev_attr_name.dev_attr.attr,
> -     &sensor_dev_attr_update_rate.dev_attr.attr,
> -     NULL
> -};
> -static struct attribute *hwmon_temp_attributes[] = {
> -     &sensor_dev_attr_temp1_input.dev_attr.attr,
> -     &sensor_dev_attr_temp1_auto_point1_pwm.dev_attr.attr,
> -     &sensor_dev_attr_temp1_auto_point1_temp.dev_attr.attr,
> -     &sensor_dev_attr_temp1_auto_point1_temp_hyst.dev_attr.attr,
> -     &sensor_dev_attr_temp1_max.dev_attr.attr,
> -     &sensor_dev_attr_temp1_max_hyst.dev_attr.attr,
> -     &sensor_dev_attr_temp1_crit.dev_attr.attr,
> -     &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
> -     &sensor_dev_attr_temp1_emergency.dev_attr.attr,
> -     &sensor_dev_attr_temp1_emergency_hyst.dev_attr.attr,
> -     NULL
> -};
> -static struct attribute *hwmon_fan_rpm_attributes[] = {
> -     &sensor_dev_attr_fan1_input.dev_attr.attr,
> -     NULL
> -};
> -static struct attribute *hwmon_pwm_fan_attributes[] = {
> -     &sensor_dev_attr_pwm1_enable.dev_attr.attr,
> -     &sensor_dev_attr_pwm1.dev_attr.attr,
> -     &sensor_dev_attr_pwm1_min.dev_attr.attr,
> -     &sensor_dev_attr_pwm1_max.dev_attr.attr,
> -     NULL
> -};
> -
> -static struct attribute *hwmon_in0_attributes[] = {
> -     &sensor_dev_attr_in0_input.dev_attr.attr,
> -     &sensor_dev_attr_in0_min.dev_attr.attr,
> -     &sensor_dev_attr_in0_max.dev_attr.attr,
> -     &sensor_dev_attr_in0_label.dev_attr.attr,
> -     NULL
> -};
>  
> -static struct attribute *hwmon_power_attributes[] = {
> -     &sensor_dev_attr_power1_input.dev_attr.attr,
> -     NULL
> -};
> -
> -static struct attribute *hwmon_power_caps_attributes[] = {
> -     &sensor_dev_attr_power1_max.dev_attr.attr,
> -     &sensor_dev_attr_power1_crit.dev_attr.attr,
> -     NULL
> -};
> -
> -static const struct attribute_group hwmon_default_attrgroup = {
> -     .attrs = hwmon_default_attributes,
> -};
> -static const struct attribute_group hwmon_temp_attrgroup = {
> -     .attrs = hwmon_temp_attributes,
> -};
> -static const struct attribute_group hwmon_fan_rpm_attrgroup = {
> -     .attrs = hwmon_fan_rpm_attributes,
> -};
> -static const struct attribute_group hwmon_pwm_fan_attrgroup = {
> -     .attrs = hwmon_pwm_fan_attributes,
> -};
> -static const struct attribute_group hwmon_in0_attrgroup = {
> -     .attrs = hwmon_in0_attributes,
> -};
> -static const struct attribute_group hwmon_power_attrgroup = {
> -     .attrs = hwmon_power_attributes,
> -};
> -static const struct attribute_group hwmon_power_caps_attrgroup = {
> -     .attrs = hwmon_power_caps_attributes,
> -};
> +     return iccsense->power_w_crit;
> +}
>  
>  static const u32 nouveau_config_chip[] = {
>       HWMON_C_UPDATE_INTERVAL,
> @@ -887,6 +552,157 @@ nouveau_fan_is_visible(const void *data, u32 attr, int 
> channel)
>       }
>  }
>  
> +static int
> +nouveau_chip_read(struct device *dev, u32 attr, int channel, long *val)
> +{
> +     struct drm_device *drm_dev = dev_get_drvdata(dev);
> +     struct nouveau_drm *drm = nouveau_drm(drm_dev);
> +
> +     switch (attr) {
> +     case hwmon_chip_update_interval:
> +             *val = nouveau_hwmon_show_update_rate(drm);
> +             break;
> +     default:
> +             return -EOPNOTSUPP;
> +     }
> +
> +     return 0;
> +}
> +
> +static int
> +nouveau_temp_read(struct device *dev, u32 attr, int channel, long *val)
> +{
> +     struct drm_device *drm_dev = dev_get_drvdata(dev);
> +     struct nouveau_drm *drm = nouveau_drm(drm_dev);
> +
> +     switch (attr) {
> +     case hwmon_temp_input:
> +             *val = nouveau_hwmon_show_temp(drm);
> +             break;
> +     case hwmon_temp_max:
> +             *val = nouveau_hwmon_max_temp(drm);
> +             break;
> +     case hwmon_temp_max_hyst:
> +             *val = nouveau_hwmon_max_temp_hyst(drm);
> +             break;
> +     case hwmon_temp_crit:
> +             *val = nouveau_hwmon_critical_temp(drm);
> +             break;
> +     case hwmon_temp_crit_hyst:
> +             *val = nouveau_hwmon_critical_temp_hyst(drm);
> +             break;
> +     case hwmon_temp_emergency:
> +             *val = nouveau_hwmon_emergency_temp(drm);
> +             break;
> +     case hwmon_temp_emergency_hyst:
> +             *val = nouveau_hwmon_emergency_temp_hyst(drm);
> +             break;
> +     default:
> +             return -EOPNOTSUPP;
> +     }
> +
> +     return 0;
> +}
> +
> +static int
> +nouveau_fan_read(struct device *dev, u32 attr, int channel, long *val)
> +{
> +     struct drm_device *drm_dev = dev_get_drvdata(dev);
> +     struct nouveau_drm *drm = nouveau_drm(drm_dev);
> +
> +     switch (attr) {
> +     case hwmon_fan_input:
> +             *val = nouveau_hwmon_show_fan1_input(drm);
> +             break;
> +     default:
> +             return -EOPNOTSUPP;
> +     }
> +
> +     return 0;
> +}
> +
> +static int
> +nouveau_in_read(struct device *dev, u32 attr, int channel, long *val)
> +{
> +     struct drm_device *drm_dev = dev_get_drvdata(dev);
> +     struct nouveau_drm *drm = nouveau_drm(drm_dev);
> +
> +     switch (attr) {
> +     case hwmon_in_input:
> +             *val = nouveau_hwmon_get_in0_input(drm);
> +             break;
> +     case hwmon_in_min:
> +             *val = nouveau_hwmon_get_in0_min(drm);
> +             break;
> +     case hwmon_in_max:
> +             *val = nouveau_hwmon_get_in0_max(drm);
> +             break;
> +     default:
> +             return -EOPNOTSUPP;
> +     }
> +
> +     return 0;
> +}
> +
> +static int
> +nouveau_pwm_read(struct device *dev, u32 attr, int channel, long *val)
> +{
> +     struct drm_device *drm_dev = dev_get_drvdata(dev);
> +     struct nouveau_drm *drm = nouveau_drm(drm_dev);
> +
> +     switch (attr) {
> +     case hwmon_pwm_enable:
> +             *val = nouveau_hwmon_get_pwm1_enable(drm);
> +             break;
> +     case hwmon_pwm_input:
> +             *val = nouveau_hwmon_get_pwm1(drm);
> +             break;
> +     default:
> +             return -EOPNOTSUPP;
> +     }
> +
> +     return 0;
> +}
> +
> +static int
> +nouveau_power_read(struct device *dev, u32 attr, int channel, long *val)
> +{
> +     struct drm_device *drm_dev = dev_get_drvdata(dev);
> +     struct nouveau_drm *drm = nouveau_drm(drm_dev);
> +
> +     switch (attr) {
> +     case hwmon_power_input:
> +             *val = nouveau_hwmon_get_power1_input(drm);
> +             break;
> +     case hwmon_power_max:
> +             *val = nouveau_hwmon_get_power1_max(drm);
> +             break;
> +     case hwmon_power_crit:
> +             *val = nouveau_hwmon_get_power1_crit(drm);
> +             break;
> +     default:
> +             return -EOPNOTSUPP;
> +     }
> +
> +     return 0;
> +}
> +
> +static int
> +nouveau_pwm_write(struct device *dev, u32 attr, int channel, long val)
> +{
> +     struct drm_device *drm_dev = dev_get_drvdata(dev);
> +     struct nouveau_drm *drm = nouveau_drm(drm_dev);

        struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
        if (!therm || therm->attr_set)
                return -EOPNOTSUPP;

And since you already got the therm pointer, maybe you can drop
nouveau_hwmon_set_pwm1 and nouveau_hwmon_set_pwm1_enable entirely and
make the therm->fan_set/set_attr calls right here.

> +
> +     switch (attr) {
> +     case hwmon_pwm_input:
> +             return nouveau_hwmon_set_pwm1(drm, val);
> +     case hwmon_pwm_enable:
> +             return nouveau_hwmon_set_pwm1_enable(drm, val);
> +     default:
> +             return -EOPNOTSUPP;
> +     }
> +}
> +
>  static umode_t
>  nouveau_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
>                                                               int channel)
> @@ -923,11 +739,45 @@ nouveau_read_string(struct device *dev, enum 
> hwmon_sensor_types type, u32 attr,
>       return -EOPNOTSUPP;
>  }
>  
> +static int
> +nouveau_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> +                                                     int channel, long *val)
> +{
> +     switch (type) {
> +     case hwmon_chip:
> +             return nouveau_chip_read(dev, attr, channel, val);
> +     case hwmon_temp:
> +             return nouveau_temp_read(dev, attr, channel, val);
> +     case hwmon_fan:
> +             return nouveau_fan_read(dev, attr, channel, val);
> +     case hwmon_in:
> +             return nouveau_in_read(dev, attr, channel, val);
> +     case hwmon_pwm:
> +             return nouveau_pwm_read(dev, attr, channel, val);
> +     case hwmon_power:
> +             return nouveau_power_read(dev, attr, channel, val);
> +     default:
> +             return -EOPNOTSUPP;
> +     }
> +}
> +
> +static int
> +nouveau_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> +                                                     int channel, long val)
> +{
> +     switch (type) {
> +     case hwmon_pwm:
> +             return nouveau_pwm_write(dev, attr, channel, val);

Where did all the temperature-related writes go?

Some vbios have fucked up thermal values set by default, this is why we
allow users to override them through hwmon.

Could you expose _max, _max_hyst, _crit and _crit_hyst?

> +     default:
> +             return -EOPNOTSUPP;
> +     }
> +}
> +
>  static const struct hwmon_ops nouveau_hwmon_ops = {
>       .is_visible = nouveau_is_visible,
> -     .read = NULL,
> +     .read = nouveau_read,
>       .read_string = nouveau_read_string,
> -     .write = NULL,
> +     .write = nouveau_write,
>  };
>  
>  static const struct hwmon_chip_info nouveau_chip_info = {
> @@ -942,8 +792,6 @@ nouveau_hwmon_init(struct drm_device *dev)
>  #if defined(CONFIG_HWMON) || (defined(MODULE) && 
> defined(CONFIG_HWMON_MODULE))
>       struct nouveau_drm *drm = nouveau_drm(dev);
>       struct nvkm_therm *therm = nvxx_therm(&drm->client.device);
> -     struct nvkm_volt *volt = nvxx_volt(&drm->client.device);
> -     struct nvkm_iccsense *iccsense = nvxx_iccsense(&drm->client.device);
>       struct nouveau_hwmon *hwmon;
>       struct device *hwmon_dev;
>       int ret = 0;
> @@ -953,79 +801,16 @@ nouveau_hwmon_init(struct drm_device *dev)
>               return -ENOMEM;
>       hwmon->dev = dev;
>  
> -     hwmon_dev = hwmon_device_register(dev->dev);
> +     hwmon_dev = hwmon_device_register_with_info(dev->dev, "nouveau", dev,
> +                                             &nouveau_chip_info, NULL);
>       if (IS_ERR(hwmon_dev)) {
>               ret = PTR_ERR(hwmon_dev);
>               NV_ERROR(drm, "Unable to register hwmon device: %d\n", ret);
>               return ret;
>       }
> -     dev_set_drvdata(hwmon_dev, dev);
> -
> -     /* set the default attributes */
> -     ret = sysfs_create_group(&hwmon_dev->kobj, &hwmon_default_attrgroup);
> -     if (ret)
> -             goto error;
> -
> -     if (therm && therm->attr_get && therm->attr_set) {
> -             /* if the card has a working thermal sensor */
> -             if (nvkm_therm_temp_get(therm) >= 0) {
> -                     ret = sysfs_create_group(&hwmon_dev->kobj, 
> &hwmon_temp_attrgroup);
> -                     if (ret)
> -                             goto error;
> -             }
> -
> -             /* if the card has a pwm fan */
> -             /*XXX: incorrect, need better detection for this, some boards 
> have
> -              *     the gpio entries for pwm fan control even when there's no
> -              *     actual fan connected to it... therm table? */
> -             if (therm->fan_get && therm->fan_get(therm) >= 0) {
> -                     ret = sysfs_create_group(&hwmon_dev->kobj,
> -                                              &hwmon_pwm_fan_attrgroup);
> -                     if (ret)
> -                             goto error;
> -             }
> -     }
> -
> -     /* if the card can read the fan rpm */
> -     if (therm && nvkm_therm_fan_sense(therm) >= 0) {
> -             ret = sysfs_create_group(&hwmon_dev->kobj,
> -                                      &hwmon_fan_rpm_attrgroup);
> -             if (ret)
> -                     goto error;
> -     }
> -
> -     if (volt && nvkm_volt_get(volt) >= 0) {
> -             ret = sysfs_create_group(&hwmon_dev->kobj,
> -                                      &hwmon_in0_attrgroup);
> -
> -             if (ret)
> -                     goto error;
> -     }
> -
> -     if (iccsense && iccsense->data_valid && !list_empty(&iccsense->rails)) {
> -             ret = sysfs_create_group(&hwmon_dev->kobj,
> -                                      &hwmon_power_attrgroup);
> -
> -             if (ret)
> -                     goto error;
> -
> -             if (iccsense->power_w_max && iccsense->power_w_crit) {
> -                     ret = sysfs_create_group(&hwmon_dev->kobj,
> -                                              &hwmon_power_caps_attrgroup);
> -                     if (ret)
> -                             goto error;
> -             }
> -     }
>  
>       hwmon->hwmon = hwmon_dev;
> -
>       return 0;
> -
> -error:
> -     NV_ERROR(drm, "Unable to create some hwmon sysfs files: %d\n", ret);
> -     hwmon_device_unregister(hwmon_dev);
> -     hwmon->hwmon = NULL;
> -     return ret;
>  #else
>       return 0;
>  #endif
> @@ -1037,17 +822,8 @@ nouveau_hwmon_fini(struct drm_device *dev)
>  #if defined(CONFIG_HWMON) || (defined(MODULE) && 
> defined(CONFIG_HWMON_MODULE))
>       struct nouveau_hwmon *hwmon = nouveau_hwmon(dev);
>  
> -     if (hwmon->hwmon) {
> -             sysfs_remove_group(&hwmon->hwmon->kobj, 
> &hwmon_default_attrgroup);
> -             sysfs_remove_group(&hwmon->hwmon->kobj, &hwmon_temp_attrgroup);
> -             sysfs_remove_group(&hwmon->hwmon->kobj, 
> &hwmon_pwm_fan_attrgroup);
> -             sysfs_remove_group(&hwmon->hwmon->kobj, 
> &hwmon_fan_rpm_attrgroup);
> -             sysfs_remove_group(&hwmon->hwmon->kobj, &hwmon_in0_attrgroup);
> -             sysfs_remove_group(&hwmon->hwmon->kobj, &hwmon_power_attrgroup);
> -             sysfs_remove_group(&hwmon->hwmon->kobj, 
> &hwmon_power_caps_attrgroup);
> -
> +     if (hwmon->hwmon)
>               hwmon_device_unregister(hwmon->hwmon);
> -     }
>  
>       nouveau_drm(dev)->hwmon = NULL;
>       kfree(hwmon);
> 

Thanks a lot, this patch makes a huge improvement in readability! With
the comments addressed, this patch is:

Reviewed-by: Martin Peres <martin.pe...@free.fr>
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Reply via email to