Messages in this thread | | | Subject | Re: [RFC PATCH 3/4] pmbus: Allow dynamic fan coefficient values | From | Guenter Roeck <> | Date | Tue, 11 Jul 2017 06:31:29 -0700 |
| |
On 07/10/2017 06:56 AM, Andrew Jeffery wrote: > Some PMBus chips, such as the MAX31785, use different coefficients for > FAN_COMMAND_[1-4] depending on whether the fan is in PWM (percent duty) > or RPM mode. Add a callback to allow the driver to provide the > applicable coefficients to avoid imposing on devices that don't have > this requirement. >
Why not just introduce another class, such as PSC_PWM ?
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > --- > drivers/hwmon/pmbus/pmbus.h | 18 +++++-- > drivers/hwmon/pmbus/pmbus_core.c | 112 ++++++++++++++++++++++++++++++++------- > 2 files changed, 108 insertions(+), 22 deletions(-) > > diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h > index 927eabc1b273..338ecc8b25a4 100644 > --- a/drivers/hwmon/pmbus/pmbus.h > +++ b/drivers/hwmon/pmbus/pmbus.h > @@ -345,6 +345,12 @@ enum pmbus_sensor_classes { > enum pmbus_data_format { linear = 0, direct, vid }; > enum vrm_version { vr11 = 0, vr12 }; > > +struct pmbus_coeffs { > + int m; /* mantissa for direct data format */ > + int b; /* offset */ > + int R; /* exponent */ > +}; > + > struct pmbus_driver_info { > int pages; /* Total number of pages */ > enum pmbus_data_format format[PSC_NUM_CLASSES]; > @@ -353,9 +359,7 @@ struct pmbus_driver_info { > * Support one set of coefficients for each sensor type > * Used for chips providing data in direct mode. > */ > - int m[PSC_NUM_CLASSES]; /* mantissa for direct data format */ > - int b[PSC_NUM_CLASSES]; /* offset */ > - int R[PSC_NUM_CLASSES]; /* exponent */ > + struct pmbus_coeffs coeffs[PSC_NUM_CLASSES]; > > u32 func[PMBUS_PAGES]; /* Functionality, per page */ > /* > @@ -382,6 +386,14 @@ struct pmbus_driver_info { > int (*identify)(struct i2c_client *client, > struct pmbus_driver_info *info); > > + /* > + * If a fan's coefficents change over time (e.g. between RPM and PWM > + * mode), then the driver can provide a function for retrieving the > + * currently applicable coefficients. > + */ > + const struct pmbus_coeffs *(*get_fan_coeffs)( > + const struct pmbus_driver_info *info, int index, > + enum pmbus_fan_mode mode, int command); > /* Allow the driver to interpret the fan command value */ > int (*get_pwm_mode)(int id, u8 fan_config, u16 fan_command); > int (*set_pwm_mode)(int id, long mode, u8 *fan_config, > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c > index 3b0a55bbbd2c..4ff6a1fd5cce 100644 > --- a/drivers/hwmon/pmbus/pmbus_core.c > +++ b/drivers/hwmon/pmbus/pmbus_core.c > @@ -58,10 +58,11 @@ > struct pmbus_sensor { > struct pmbus_sensor *next; > char name[PMBUS_NAME_SIZE]; /* sysfs sensor name */ > - struct device_attribute attribute; > + struct sensor_device_attribute attribute; > u8 page; /* page number */ > u16 reg; /* register */ > enum pmbus_sensor_classes class; /* sensor class */ > + const struct pmbus_coeffs *coeffs; > bool update; /* runtime sensor update needed */ > int data; /* Sensor data. > Negative if there was a read error */ > @@ -89,6 +90,7 @@ struct pmbus_fan_ctrl { > u8 id; > u8 config; > u16 command; > + const struct pmbus_coeffs *coeffs; > }; > #define to_pmbus_fan_ctrl_attr(_attr) \ > container_of(_attr, struct pmbus_fan_ctrl_attr, attribute) > @@ -511,9 +513,15 @@ static long pmbus_reg2data_direct(struct pmbus_data *data, > long val = (s16) sensor->data; > long m, b, R; > > - m = data->info->m[sensor->class]; > - b = data->info->b[sensor->class]; > - R = data->info->R[sensor->class]; > + if (sensor->coeffs) { > + m = sensor->coeffs->m; > + b = sensor->coeffs->b; > + R = sensor->coeffs->R; > + } else { > + m = data->info->coeffs[sensor->class].m; > + b = data->info->coeffs[sensor->class].b; > + R = data->info->coeffs[sensor->class].R; > + } > > if (m == 0) > return 0; > @@ -663,9 +671,15 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data, > { > long m, b, R; > > - m = data->info->m[sensor->class]; > - b = data->info->b[sensor->class]; > - R = data->info->R[sensor->class]; > + if (sensor->coeffs) { > + m = sensor->coeffs->m; > + b = sensor->coeffs->b; > + R = sensor->coeffs->R; > + } else { > + m = data->info->coeffs[sensor->class].m; > + b = data->info->coeffs[sensor->class].b; > + R = data->info->coeffs[sensor->class].R; > + } > > /* Power is in uW. Adjust R and b. */ > if (sensor->class == PSC_POWER) { > @@ -796,7 +810,9 @@ static ssize_t pmbus_show_sensor(struct device *dev, > struct device_attribute *devattr, char *buf) > { > struct pmbus_data *data = pmbus_update_device(dev); > - struct pmbus_sensor *sensor = to_pmbus_sensor(devattr); > + struct pmbus_sensor *sensor; > + > + sensor = to_pmbus_sensor(to_sensor_dev_attr(devattr)); > > if (sensor->data < 0) > return sensor->data; > @@ -810,12 +826,14 @@ static ssize_t pmbus_set_sensor(struct device *dev, > { > struct i2c_client *client = to_i2c_client(dev->parent); > struct pmbus_data *data = i2c_get_clientdata(client); > - struct pmbus_sensor *sensor = to_pmbus_sensor(devattr); > + struct pmbus_sensor *sensor; > ssize_t rv = count; > long val = 0; > int ret; > u16 regval; > > + sensor = to_pmbus_sensor(to_sensor_dev_attr(devattr)); > + > if (kstrtol(buf, 10, &val) < 0) > return -EINVAL; > > @@ -856,6 +874,7 @@ static ssize_t pmbus_show_fan_command(struct device *dev, > } > > sensor.class = PSC_FAN; > + sensor.coeffs = fan->coeffs; > if (mode == percent) > sensor.data = fan->command * 255 / 100; > else > @@ -882,6 +901,29 @@ static ssize_t pmbus_show_pwm(struct device *dev, > buf); > } > > +static int pmbus_update_fan_coeffs(struct pmbus_data *data, > + struct pmbus_fan_ctrl *fan, > + enum pmbus_fan_mode mode) > +{ > + const struct pmbus_driver_info *info = data->info; > + struct pmbus_sensor *curr = data->sensors; > + > + fan->coeffs = info->get_fan_coeffs(info, fan->index, mode, > + PMBUS_FAN_COMMAND_1 + fan->id); > + > + while (curr) { > + if (curr->class == PSC_FAN && > + curr->attribute.index == fan->index) { > + curr->coeffs = info->get_fan_coeffs(info, fan->index, > + mode, curr->reg); > + } > + > + curr = curr->next; > + } > + > + return 0; > +} > + > static ssize_t pmbus_set_fan_command(struct device *dev, > enum pmbus_fan_mode mode, > struct pmbus_fan_ctrl *fan, > @@ -889,6 +931,7 @@ static ssize_t pmbus_set_fan_command(struct device *dev, > { > struct i2c_client *client = to_i2c_client(dev->parent); > struct pmbus_data *data = i2c_get_clientdata(client); > + const struct pmbus_driver_info *info = data->info; > int config_addr, command_addr; > struct pmbus_sensor sensor; > ssize_t rv; > @@ -899,7 +942,13 @@ static ssize_t pmbus_set_fan_command(struct device *dev, > > mutex_lock(&data->update_lock); > > + if (info->format[PSC_FAN] == direct && info->get_fan_coeffs) { > + pmbus_update_fan_coeffs(data, fan, mode); > + sensor.coeffs = fan->coeffs; > + } > + > sensor.class = PSC_FAN; > + sensor.attribute.index = fan->index; > > val = pmbus_data2reg(data, &sensor, val); > > @@ -968,6 +1017,8 @@ static ssize_t pmbus_show_pwm_enable(struct device *dev, > struct pmbus_sensor sensor = { > .class = PSC_FAN, > .data = fan->command, > + .attribute.index = fan->index, > + .coeffs = fan->coeffs, > }; > long command; > > @@ -992,6 +1043,7 @@ static ssize_t pmbus_set_pwm_enable(struct device *dev, > struct pmbus_fan_ctrl *fan = pwm_enable_to_pmbus_fan_ctrl(da); > struct i2c_client *client = to_i2c_client(dev->parent); > struct pmbus_data *data = i2c_get_clientdata(client); > + const struct pmbus_driver_info *info = data->info; > int config_addr, command_addr; > struct pmbus_sensor sensor; > ssize_t rv = count; > @@ -1003,15 +1055,21 @@ static ssize_t pmbus_set_pwm_enable(struct device *dev, > mutex_lock(&data->update_lock); > > sensor.class = PSC_FAN; > + sensor.attribute.index = fan->index; > + > + if (info->format[PSC_FAN] == direct && info->get_fan_coeffs) { > + pmbus_update_fan_coeffs(data, fan, percent); > + sensor.coeffs = fan->coeffs; > + } > > config_addr = (fan->id < 2) ? PMBUS_FAN_CONFIG_12 : PMBUS_FAN_CONFIG_34; > command_addr = config_addr + 1 + (fan->id & 1); > > - if (data->info->set_pwm_mode) { > + if (info->set_pwm_mode) { > u8 config = PB_FAN_CONFIG_PUT(fan->id, fan->config); > u16 command = fan->command; > > - rv = data->info->set_pwm_mode(fan->id, mode, &config, &command); > + rv = info->set_pwm_mode(fan->id, mode, &config, &command); > if (rv < 0) > goto done; > > @@ -1138,7 +1196,7 @@ static struct pmbus_sensor *pmbus_add_sensor(struct pmbus_data *data, > sensor = devm_kzalloc(data->dev, sizeof(*sensor), GFP_KERNEL); > if (!sensor) > return NULL; > - a = &sensor->attribute; > + a = &sensor->attribute.dev_attr; > > snprintf(sensor->name, sizeof(sensor->name), "%s%d_%s", > name, seq, type); > @@ -1146,9 +1204,9 @@ static struct pmbus_sensor *pmbus_add_sensor(struct pmbus_data *data, > sensor->reg = reg; > sensor->class = class; > sensor->update = update; > - pmbus_dev_attr_init(a, sensor->name, > + pmbus_attr_init(&sensor->attribute, sensor->name, > readonly ? S_IRUGO : S_IRUGO | S_IWUSR, > - pmbus_show_sensor, pmbus_set_sensor); > + pmbus_show_sensor, pmbus_set_sensor, seq); > > if (pmbus_add_attribute(data, &a->attr)) > return NULL; > @@ -1886,7 +1944,7 @@ static const u32 pmbus_fan_status_flags[] = { > /* Fans */ > static int pmbus_add_fan_ctrl(struct i2c_client *client, > struct pmbus_data *data, int index, int page, int id, > - u8 config) > + u8 config, const struct pmbus_coeffs *coeffs) > { > struct pmbus_fan_ctrl *fan; > int rv; > @@ -1898,6 +1956,7 @@ static int pmbus_add_fan_ctrl(struct i2c_client *client, > fan->index = index; > fan->page = page; > fan->id = id; > + fan->coeffs = coeffs; > fan->config = config; > > rv = _pmbus_read_word_data(client, page, > @@ -1921,6 +1980,8 @@ static int pmbus_add_fan_attributes(struct i2c_client *client, > struct pmbus_data *data) > { > const struct pmbus_driver_info *info = data->info; > + const struct pmbus_coeffs *coeffs = NULL; > + enum pmbus_fan_mode mode; > int index = 1; > int page; > int ret; > @@ -1929,6 +1990,7 @@ static int pmbus_add_fan_attributes(struct i2c_client *client, > int f; > > for (f = 0; f < ARRAY_SIZE(pmbus_fan_registers); f++) { > + struct pmbus_sensor *sensor; > int regval; > > if (!(info->func[page] & pmbus_fan_flags[f])) > @@ -1949,13 +2011,25 @@ static int pmbus_add_fan_attributes(struct i2c_client *client, > (!(regval & (PB_FAN_1_INSTALLED >> ((f & 1) * 4))))) > continue; > > - if (pmbus_add_sensor(data, "fan", "input", index, > - page, pmbus_fan_registers[f], > - PSC_FAN, true, true) == NULL) > + sensor = pmbus_add_sensor(data, "fan", "input", index, > + page, pmbus_fan_registers[f], > + PSC_FAN, true, true); > + if (!sensor) > return -ENOMEM; > > + /* Add coeffs here as we have access to the fan mode */ > + if (info->format[PSC_FAN] == direct && > + info->get_fan_coeffs) { > + const u16 mask = PB_FAN_1_RPM >> ((f & 1) * 4); > + > + mode = (regval & mask) ? rpm : percent; > + coeffs = info->get_fan_coeffs(info, index, mode, > + pmbus_fan_registers[f]); > + sensor->coeffs = coeffs; > + } > + > ret = pmbus_add_fan_ctrl(client, data, index, page, f, > - regval); > + regval, coeffs); > if (ret < 0) > return ret; > >
| |