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;
MIME-Version: 1.0
References: <20200725181404.18951-1-ansuelsmth@gmail.com> <20200725181404.18951-2-ansuelsmth@gmail.com>
 <CAHLCerMc8yUjh9qwUCa=jMZHs18GC4qeS3rqT1_6K90QJd=nVA@mail.gmail.com> <006f01d66fe1$e6c0d450$b4427cf0$@gmail.com>
In-Reply-To: <006f01d66fe1$e6c0d450$b4427cf0$@gmail.com>
From:   Amit Kucheria <amit.kucheria@linaro.org>
Date:   Thu, 13 Aug 2020 16:48:55 +0530
Message-ID: <CAP245DWuMh6Bkv7P4Y6mV3n59KFyCGKoy7e00E9d9ijR_rr1WA@mail.gmail.com>
Subject: Re: [RFC PATCH v5 1/7] drivers: thermal: tsens: Add VER_0 tsens version
To:     Ansuel Smith <ansuelsmth@gmail.com>
Cc:     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 PM list <linux-pm@vger.kernel.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Aug 11, 2020 at 6:48 PM <ansuelsmth@gmail.com> wrote:
>
>
>
> > -----Messaggio originale-----
> > Da: Amit Kucheria <amit.kucheria@linaro.org>

> >
> > >                         if (IS_ERR(priv->rf[i]))
> > >                                 return PTR_ERR(priv->rf[i]);
> > >                 }
> > > @@ -775,12 +800,80 @@ int __init init_common(struct tsens_priv
> > *priv)
> > >                         goto err_put_device;
> > >         }
> > >
> > > -       priv->rf[TSENS_EN] = devm_regmap_field_alloc(dev, priv->srot_map,
> > > -                                                    priv->fields[TSENS_EN]);
> > > -       if (IS_ERR(priv->rf[TSENS_EN])) {
> > > -               ret = PTR_ERR(priv->rf[TSENS_EN]);
> > > -               goto err_put_device;
> > > +       if (tsens_version(priv) >= VER_0_1) {
> > > +               priv->rf[TSENS_EN] = devm_regmap_field_alloc(
> > > +                       dev, priv->srot_map, priv->fields[TSENS_EN]);
> > > +               if (IS_ERR(priv->rf[TSENS_EN])) {
> > > +                       ret = PTR_ERR(priv->rf[TSENS_EN]);
> > > +                       goto err_put_device;
> > > +               }
> > > +
> > > +               priv->rf[SENSOR_EN] = devm_regmap_field_alloc(
> > > +                       dev, priv->srot_map, priv->fields[SENSOR_EN]);
> > > +               if (IS_ERR(priv->rf[SENSOR_EN])) {
> > > +                       ret = PTR_ERR(priv->rf[SENSOR_EN]);
> > > +                       goto err_put_device;
> > > +               }
> > > +               priv->rf[INT_EN] = devm_regmap_field_alloc(
> > > +                       dev, priv->tm_map, priv->fields[INT_EN]);
> > > +               if (IS_ERR(priv->rf[INT_EN])) {
> > > +                       ret = PTR_ERR(priv->rf[INT_EN]);
> > > +                       goto err_put_device;
> > > +               }
> > > +       } else {
> >
> > Let's not create two big sections with if-else for 8960 and everything
> > else. For example, what is wrong with using common code for TSENS_EN?
> >
> > If the concern is memory wasted trying to allocate fields not present
> > on this older platform, perhaps consider adding a check in the loop to
> > break early in case of 8960?
> >
>
> About TSENS_EN the old platform doesn't have SROT so I need to use TM_MAP.
> Should I set the srot map to match the tm map so we can use the common function?
> Aside from this problem, I will try to remove the big if-else.

Ick. I guess srot_map and tm_map pointing to the same region is the
lesser of two evils? It makes it so this will be constrained to a
single place in init_common().