linux-kernel - Re: [RFC PATCH v6 2/8] drivers: thermal: tsens: Add VER

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sun, 29 Nov 2020 17:28:46 +0100
From:   Ansuel Smith <ansuelsmth@...il.com>
To:     Amit Kucheria <amitk@...nel.org>
Cc:     Amit Kucheria <amit.kucheria@...aro.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Zhang Rui <rui.zhang@...el.com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        Linux PM list <linux-pm@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v6 2/8] drivers: thermal: tsens: Add VER_0 tsens
 version

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@...il.com> 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.