Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sun, 29 Nov 2020 17:28:46 +0100
From:   Ansuel Smith <ansuelsmth@gmail.com>
To:     Amit Kucheria <amitk@kernel.org>
Cc:     Amit Kucheria <amit.kucheria@linaro.org>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Zhang Rui <rui.zhang@intel.com>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        Linux PM list <linux-pm@vger.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH v6 2/8] drivers: thermal: tsens: Add VER_0 tsens
 version
Message-ID: <X8PMPnx+RmjyZyrJ@ansuel-xps20.localdomain>
References: <20200814134123.14566-1-ansuelsmth@gmail.com>
 <20200814134123.14566-3-ansuelsmth@gmail.com>
 <CAHLCerMArOceCFQ1XFbsZCAnUdKVX3TVnAb502w+kxmO97bdJg@mail.gmail.com>
 <20201125122228.GB23592@ansuel-xps20.localdomain>
 <CAHLCerNTNpEGiGT6Veroeh1b8pOCiYYFhpnj5YqZcFZxAXGB-A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAHLCerNTNpEGiGT6Veroeh1b8pOCiYYFhpnj5YqZcFZxAXGB-A@mail.gmail.com>
Precedence: bulk

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@gmail.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.