Re: adm1026 driver port for kernel 2.6.10-rc2 [RE-REVISED DRIVER]

[Arjan van de Ven]

On Thu, 2004-11-18 at 10:56 -0800, Justin Thiessen wrote:
> MODULE_PARM(gpio_input,"1-17i");

new 2.6 drivers should NOT use MODULE_PARM, it's deprecated.
use module_param() instead

> int adm1026_attach_adapter(struct i2c_adapter *adapter)
> {
> 	if (!(adapter->class & I2C_CLASS_HWMON)) {
> 		return 0;
> 	}

no need for extra { }'s in such a case


> static ssize_t show_in(struct device *dev, char *buf, int nr)
> {
> 	struct adm1026_data *data = adm1026_update_device(dev);
> 	return sprintf(buf,"%d\n", INS_FROM_REG(nr, data->in[nr]));
> }

any chance you could make this use snprintf instead ?

> static ssize_t show_in_max(struct device *dev, char *buf, int nr)
> {
> 	struct adm1026_data *data = adm1026_update_device(dev);
> 	return sprintf(buf,"%d\n", INS_FROM_REG(nr, data->in_max[nr]));
> }

same question

> static ssize_t store_pwm_reg(struct i2c_client *client, 
> 		struct adm1026_data *data, size_t count)
> {
> 	adm1026_write_value(client, ADM1026_REG_PWM, data->pwm1.pwm);
> 	up(&data->update_lock);
> 	return count;
> }
> static ssize_t set_pwm_reg(struct device *dev, const char *buf,
> 		size_t count)
> {
> 	struct i2c_client *client = to_i2c_client(dev);
> 	struct adm1026_data *data = i2c_get_clientdata(client);
> 	int     val;
> 
> 	if (data->pwm1.enable == 1) {
> 		down(&data->update_lock);
> 		val = simple_strtol(buf, NULL, 10);
> 		data->pwm1.pwm = PWM_TO_REG(val);
> 		return store_pwm_reg(client, data, count);
> 	}

this locking construct is rahter awkward; is it possible to refactor the
code such that you can down and up in the same function ?



> static ssize_t store_auto_pwm_min(struct i2c_client *client, 
> 		struct adm1026_data *data, size_t count)
> {
> 	data->pwm1.pwm = PWM_TO_REG((data->pwm1.pwm & 0x0f) |
> 		PWM_MIN_TO_REG(data->pwm1.auto_pwm_min)); 
> 	adm1026_write_value(client, ADM1026_REG_PWM, data->pwm1.pwm);
> 	up(&data->update_lock);
> 	return count;
> }
> static ssize_t set_auto_pwm_min(struct device *dev, const char *buf,
> 		size_t count)
> {
> 	struct i2c_client *client = to_i2c_client(dev);
> 	struct adm1026_data *data = i2c_get_clientdata(client);
> 	int     val;
> 
> 	down(&data->update_lock);
> 	val = simple_strtol(buf, NULL, 10);
> 	data->pwm1.auto_pwm_min = SENSORS_LIMIT(val,0,255);
> 	if (data->pwm1.enable == 2) { /* apply immediately */
> 		return store_auto_pwm_min(client, data, count);
> 	} else { /* wait til automatic fan control is enabled to apply */
> 		up(&data->update_lock);
> 		return count;
> 	}
> }

... here the same construct but even more awkward



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/