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@xxxxxxxxx> > --- > 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@xxxxxxx> _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel