> I don't quite get why adt7411_read_vdd() is responsible for updating
> data->next_update, while it doesn't make use of its value.
> data->next_update is only used by adt7411_show_input() so it would make
> more sense to set it there too. It would be more flexible, as you could
> decide that the cache (the lifetime of which is controlled by
> data->next_update) includes both the value of vdd and the value of
> ref_is_vdd. This would avoid reading vdd when you don't need it, and
> guarantee that you never use uninitialized values.
I had the idea that reading vdd via sysfs should also update the cache. Trying
to do this and reorganizing the code to solve some chicken-and-egg-problem
regarding the cache, it seems I stumbled over my own feet.
I implemented it according to your suggestions and it looks cleaner, in deed. I
just had to drop the idea that reading vdd from sysfs also updates the cache,
as I would now need another lock for it which I think is not worth the trouble.
> > +static ssize_t adt7411_set_bit(struct device *dev, struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct sensor_device_attribute_2 *s_attr2 = to_sensor_dev_attr_2(attr);
> > + struct i2c_client *client = to_i2c_client(dev);
> > + int ret;
> > + unsigned long flag;
> > +
> > + ret = strict_strtoul(buf, 0, &flag);
> > + if (ret || flag > 1)
> > + return -EINVAL;
> > +
> > + ret = adt7411_modify_bit(client, s_attr2->index, s_attr2->nr, flag);
> > + return ret < 0 ? ret : count;
> > +}
>
> Calling the above function should invalidate the cache. At least
> changing adc_ref_vdd makes the cached value data->adc_ref_vdd wrong,
> and presumably the other settings may also affect the value of
> data->vdd_cached. As I don't expect the user to change the settings
> frequently, there's probably no point in trying to be smart.
I hate this driver, it makes me look pretty stupid :/
I already made a V3 and will probably send it on Monday after some testing with
the hardware.
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Attachment:
signature.asc
Description: Digital signature
_______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
