On Sun, Nov 29, 2020 at 06:28:01PM +0530, Amit Kucheria wrote:
> On Thu, Nov 26, 2020 at 2:16 AM Ansuel Smith <ansuelsmth@xxxxxxxxx> wrote:
>
> > > > };
> > > > @@ -441,6 +442,10 @@ enum regfield_ids {
> > > > CRIT_THRESH_14,
> > > > CRIT_THRESH_15,
> > > >
> > > > + /* VER_0 MIN MAX THRESH */
> > > > + MIN_THRESH_0,
> > > > + MAX_THRESH_0,
> > > > +
> > >
> > > Consider reusing LOW_THRESH_0 and UP_THRESH_0 for these?
> > >
> >
> > As we already have defined LOW_THRESH and UP how can we reuse that
> > regfield to define MIN and MAX?
> >
>
> We are using MIN and MAX THRESH on the apq8064 to mean LOW and UP
> THRESOLD, isn't it? IIUC, It was just named differently earlier.
>
> When the driver is loaded on the apq8064, only that one field will be
> use since v0 has a single threshold for all sensors. When the driver
> is loaded on new IPs, all fields will be used.
Let's sum up things and take a decision about this. On V_0 the original
driver have a special implementation that has a 4 trips point, one
critical high (that should be MAX_THRESH), one critical low (that should
be MIN_THRESH), one configurabile hi and one configurable low.
This is the regfiled
[LOW_THRESH_0] = REG_FIELD(THRESHOLD_ADDR, 0, 7),
[UP_THRESH_0] = REG_FIELD(THRESHOLD_ADDR, 8, 15),
[MIN_THRESH_0] = REG_FIELD(THRESHOLD_ADDR, 16, 23),
[MAX_THRESH_0] = REG_FIELD(THRESHOLD_ADDR, 24, 31),
and we have the regfiled to check if the threshold is violated.
Looking at the set trips code, since V_0 doesn't have critical
interrupt, we only set the uplow interrupt. Now the current code only
check the LOW and UP regfield and V_0. The original code also check MIN
and MAX (that are set to 125 C and 0 C, that should be the critical trip
point). Should we:
1. drop the MIN and MAX THRESH and keep them unconfigured (and make the
interrupt set only to the UP/LOW trips) or
2. add the missing code to set_trips
Honestrly I'm more with the first approach. I also sent v7 that should
address all the other request. As always thanks for the attention.
