On Mon, Nov 21, 2016 at 05:53:01PM +0100, Michael Walle wrote:
> Am 2016-11-19 19:05, schrieb Guenter Roeck:
> >Hi Michael,
> >
> >On Fri, Oct 14, 2016 at 11:43:35AM +0200, Michael Walle wrote:
> >>This patch adds support for the min, max and alarm attributes of the
> >>voltage and temperature channels. Additionally, the temp2_fault
> >>attribute
> >>is supported which indicates a fault of the external temperature diode.
> >>
> >>Signed-off-by: Michael Walle <michael@xxxxxxxx>
> >
> >Sorry for the late reply. Mostly looks ok.
> >Couple of comments below.
>
> thanks for the review. I will send an updated version, soon.
>
>
> [snip]
>
> >>+static int adt7411_write_temp(struct device *dev, u32 attr, int
> >>channel,
> >>+ long val)
> >>+{
> >>+ struct adt7411_data *data = dev_get_drvdata(dev);
> >>+ struct i2c_client *client = data->client;
> >>+ int reg;
> >>+
> >>+ val = DIV_ROUND_CLOSEST(val, 1000);
> >>+ val = clamp_val(val, -128, 127);
> >>+ val = val < 0 ? 0x100 + val : val;
> >
> >Does this add any value ? It doesn't change the low byte.
>
> mh? if val is negative 256 will be added.
>
What changes for the low byte if you add 0x100 to val ?
0xXXXXXX00 + 0x100 -> 0xYYYYYY00
0xXXXXXX01 + 0x100 -> 0xYYYYYY01
..
0xXXXXXXff + 0x100 -> 0xYYYYYYff
or
0x000000xx + 0x100 = 0x000001xx
0x000001xx + 0x100 = 0x000002xx
..
0xfffffexx + 0x100 = 0xffffffxx
0xffffffxx + 0x100 = 0x000000xx
>
> >> static umode_t adt7411_is_visible(const void *_data,
> >> enum hwmon_sensor_types type,
> >> u32 attr, int channel)
> >> {
> >> const struct adt7411_data *data = _data;
> >>+ bool visible;
> >>
> >> switch (type) {
> >> case hwmon_in:
> >>- if (channel > 0 && channel < 3)
> >>- return data->use_ext_temp ? 0 : S_IRUGO;
> >>- else
> >>- return S_IRUGO;
> >>+ visible = channel == 0 || channel >= 2 || !data->use_ext_temp;
> >
> >in2 is now visible even if external temperature is measured.
> >This is not correct. Yes, one can read the register, but the
> >external pin (AIN2) is connected to the temperature sensor.
>
> i guess
>
> visible = channel == 0 || channel >= 3 || !data->use_ext_temp;
>
Correct.
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
