Re: [PATCH v3 2/2] hwmon: add support for MCP998X

[<Victor.Duicu@microchip.com>]

On Mon, 2026-02-02 at 07:18 -0800, Guenter Roeck wrote:
> 
...

> > > > +             case hwmon_temp_max_hyst:
> > > > +                     if (channel)
> > > > +                             addr =
> > > > MCP9982_EXT_HIGH_LIMIT_ADDR(channel);
> > > > +                     else
> > > > +                             addr =
> > > > MCP9982_INTERNAL_HIGH_LIMIT_ADDR;
> > > > +                     ret = mcp9982_read_limit(priv, addr,
> > > > val);
> > > > +                     if (ret)
> > > > +                             return ret;
> > > > +
> > > > +                     ret = regmap_read(priv->regmap,
> > > > MCP9982_HYS_ADDR, &hyst);
> > > > +                     if (ret)
> > > > +                             return ret;
> > > > +
> > > > +                     *val -= (hyst & 0xFF) * 1000;
> > > 
> > > What is the mask for ? The chip registers are 8 bit wide.
> > > 
> > > > +                     *val = clamp_val(*val, -64000, 191875);
> > > 
> > > Clamping on reads is highly unusual. Why is this needed ?
> > 
> >    There are instances when the hysteresis limit could be outside
> > the range of temperatures.
> > 
> >    For example, if the high limit is set to -45000 and the
> > hysteresis
> > is set to 20000, the high limit hysteresis is -65000 which is
> > outside
> > the range of supported temperatures.
> > 
> >    The hysteresis is set related to the critical temperature (that
> > is
> > higher then the "high limit") but it will be applied also to the
> > "high
> > temperature". In this case the hysteresis is valid for critical but
> > it
> > will be out of range for the "high temp".
> > 
> "Supported" is irrelevant. Question is what is written into and
> reported by
> the chip. It may be "out of range", but the value is still written
> into
> the chip. So the question is: How does the chip react to the "out of
> range"
> values ? I suspect that it technically still works, even if the value
> is not
> officially supported. That should be reflected in the reported
> values.
> More specifically, if setting the hysteresis in your example to 19000
> instead of 20000 triggers a different response from the chip, that
> needs
> to be reflected in the reported values.

The driver reads the limit and hysteresis values and calculates
their difference. The result is only displayed and not saved in a
register. The result is clamped to be within the range of 
supported values.

In my example setting hysteresis value to 19000 does not trigger
a different response from the chip.