Re: [Nouveau] [PATCH v5 3/5] nouveau_hwmon: Remove old code, add .write/.read operations

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.vilardaga@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.peres@free.fr