linux-kernel - Re: [RFC PATCH v5 1/7] 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:   Thu, 13 Aug 2020 16:48:55 +0530
From:   Amit Kucheria <amit.kucheria@...aro.org>
To:     Ansuel Smith <ansuelsmth@...il.com>
Cc:     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 PM list <linux-pm@...r.kernel.org>,
        linux-arm-msm <linux-arm-msm@...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 v5 1/7] drivers: thermal: tsens: Add VER_0 tsens version

On Tue, Aug 11, 2020 at 6:48 PM <ansuelsmth@...il.com> wrote:
>
>
>
> > -----Messaggio originale-----
> > Da: Amit Kucheria <amit.kucheria@...aro.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().