diff mbox series

[RFC,v6,2/8] drivers: thermal: tsens: Add VER_0 tsens version

Message ID 20200814134123.14566-3-ansuelsmth@gmail.com
State New
Headers show
Series None | expand

Commit Message

Christian Marangi Aug. 14, 2020, 1:41 p.m. UTC
VER_0 is used to describe device based on tsens version before v0.1.
These device are devices based on msm8960 for example apq8064 or
ipq806x.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/thermal/qcom/tsens.c | 122 +++++++++++++++++++++++++++++++----
 drivers/thermal/qcom/tsens.h |   7 +-
 2 files changed, 114 insertions(+), 15 deletions(-)

Comments

Amit Kucheria Nov. 22, 2020, 8:05 p.m. UTC | #1
Hi Ansuel,

See comments inline.

On Fri, Aug 14, 2020 at 7:12 PM Ansuel Smith <ansuelsmth@gmail.com> wrote:
>
> VER_0 is used to describe device based on tsens version before v0.1.
> These device are devices based on msm8960 for example apq8064 or
> ipq806x.
>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/thermal/qcom/tsens.c | 122 +++++++++++++++++++++++++++++++----
>  drivers/thermal/qcom/tsens.h |   7 +-
>  2 files changed, 114 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index 9fe9a2b26705..965c4799918a 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -516,6 +516,15 @@ static irqreturn_t tsens_irq_thread(int irq, void *data)
>                         dev_dbg(priv->dev, "[%u] %s: no violation:  %d\n",
>                                 hw_id, __func__, temp);
>                 }
> +
> +               if (tsens_version(priv) < VER_0_1) {
> +                       /* Constraint: There is only 1 interrupt control register for all
> +                        * 11 temperature sensor. So monitoring more than 1 sensor based
> +                        * on interrupts will yield inconsistent result. To overcome this
> +                        * issue we will monitor only sensor 0 which is the master sensor.
> +                        */
> +                       break;
> +               }
>         }
>
>         return IRQ_HANDLED;
> @@ -531,6 +540,13 @@ static int tsens_set_trips(void *_sensor, int low, int high)
>         int high_val, low_val, cl_high, cl_low;
>         u32 hw_id = s->hw_id;
>
> +       if (tsens_version(priv) < VER_0_1) {
> +               /* Pre v0.1 IP had a single register for each type of interrupt
> +                * and thresholds
> +                */
> +               hw_id = 0;
> +       }
> +
>         dev_dbg(dev, "[%u] %s: proposed thresholds: (%d:%d)\n",
>                 hw_id, __func__, low, high);
>
> @@ -584,18 +600,21 @@ int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp)
>         u32 valid;
>         int ret;
>
> -       ret = regmap_field_read(priv->rf[valid_idx], &valid);
> -       if (ret)
> -               return ret;
> -       while (!valid) {
> -               /* Valid bit is 0 for 6 AHB clock cycles.
> -                * At 19.2MHz, 1 AHB clock is ~60ns.
> -                * We should enter this loop very, very rarely.
> -                */
> -               ndelay(400);
> +       /* VER_0 doesn't have VALID bit */
> +       if (tsens_version(priv) >= VER_0_1) {
>                 ret = regmap_field_read(priv->rf[valid_idx], &valid);
>                 if (ret)
>                         return ret;
> +               while (!valid) {
> +                       /* Valid bit is 0 for 6 AHB clock cycles.
> +                        * At 19.2MHz, 1 AHB clock is ~60ns.
> +                        * We should enter this loop very, very rarely.
> +                        */
> +                       ndelay(400);
> +                       ret = regmap_field_read(priv->rf[valid_idx], &valid);
> +                       if (ret)
> +                               return ret;
> +               }

Let's revisit this after fixing patch 1.


>         }
>
>         /* Valid bit is set, OK to read the temperature */
> @@ -763,6 +782,10 @@ int __init init_common(struct tsens_priv *priv)
>                 goto err_put_device;
>         }
>
> +       /* VER_0 have only tm_map */
> +       if (!priv->srot_map)
> +               priv->srot_map = priv->tm_map;
> +
>         if (tsens_version(priv) > VER_0_1) {
>                 for (i = VER_MAJOR; i <= VER_STEP; i++) {
>                         priv->rf[i] = devm_regmap_field_alloc(dev, priv->srot_map,
> @@ -781,6 +804,10 @@ int __init init_common(struct tsens_priv *priv)
>                 ret = PTR_ERR(priv->rf[TSENS_EN]);
>                 goto err_put_device;
>         }
> +       /* in VER_0 TSENS need to be explicitly enabled */
> +       if (tsens_version(priv) == VER_0)
> +               regmap_field_write(priv->rf[TSENS_EN], 1);
> +
>         ret = regmap_field_read(priv->rf[TSENS_EN], &enabled);
>         if (ret)
>                 goto err_put_device;
> @@ -803,6 +830,61 @@ int __init init_common(struct tsens_priv *priv)
>                 goto err_put_device;
>         }
>
> +       priv->rf[TSENS_EN] = devm_regmap_field_alloc(dev, priv->tm_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[TSENS_SW_RST] = devm_regmap_field_alloc(
> +               dev, priv->tm_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[LOW_INT_CLEAR_0] = devm_regmap_field_alloc(
> +               dev, priv->tm_map, priv->fields[LOW_INT_CLEAR_0]);
> +       if (IS_ERR(priv->rf[LOW_INT_CLEAR_0])) {
> +               ret = PTR_ERR(priv->rf[LOW_INT_CLEAR_0]);
> +               goto err_put_device;
> +       }
> +
> +       priv->rf[UP_INT_CLEAR_0] = devm_regmap_field_alloc(
> +               dev, priv->tm_map, priv->fields[UP_INT_CLEAR_0]);
> +       if (IS_ERR(priv->rf[UP_INT_CLEAR_0])) {
> +               ret = PTR_ERR(priv->rf[UP_INT_CLEAR_0]);
> +               goto err_put_device;
> +       }
> +
> +       /* VER_0 require to set MIN and MAX THRESH */
> +       if (tsens_version(priv) < VER_0_1) {
> +               priv->rf[MIN_THRESH_0] = devm_regmap_field_alloc(
> +                       dev, priv->tm_map, priv->fields[MIN_THRESH_0]);
> +               if (IS_ERR(priv->rf[MIN_THRESH_0])) {
> +                       ret = PTR_ERR(priv->rf[MIN_THRESH_0]);
> +                       goto err_put_device;
> +               }
> +
> +               priv->rf[MAX_THRESH_0] = devm_regmap_field_alloc(
> +                       dev, priv->tm_map, priv->fields[MAX_THRESH_0]);
> +               if (IS_ERR(priv->rf[MAX_THRESH_0])) {
> +                       ret = PTR_ERR(priv->rf[MAX_THRESH_0]);
> +                       goto err_put_device;
> +               }
> +
> +               regmap_field_write(priv->rf[MIN_THRESH_0], 0);
> +               regmap_field_write(priv->rf[MAX_THRESH_0], 120000);
> +       }
> +
> +       priv->rf[TRDY] =
> +               devm_regmap_field_alloc(dev, priv->tm_map, priv->fields[TRDY]);
> +       if (IS_ERR(priv->rf[TRDY])) {
> +               ret = PTR_ERR(priv->rf[TRDY]);
> +               goto err_put_device;
> +       }
> +
>         /* This loop might need changes if enum regfield_ids is reordered */
>         for (j = LAST_TEMP_0; j <= UP_THRESH_15; j += 16) {
>                 for (i = 0; i < priv->feat->max_sensors; i++) {
> @@ -856,7 +938,11 @@ int __init init_common(struct tsens_priv *priv)
>         }
>
>         spin_lock_init(&priv->ul_lock);
> -       tsens_enable_irq(priv);
> +
> +       /* VER_0 interrupt doesn't need to be enabled */
> +       if (tsens_version(priv) >= VER_0_1)
> +               tsens_enable_irq(priv);
> +
>         tsens_debug_init(op);
>
>  err_put_device:
> @@ -952,10 +1038,18 @@ static int tsens_register_irq(struct tsens_priv *priv, char *irqname,
>                 if (irq == -ENXIO)
>                         ret = 0;
>         } else {
> -               ret = devm_request_threaded_irq(&pdev->dev, irq,
> -                                               NULL, thread_fn,
> -                                               IRQF_ONESHOT,
> -                                               dev_name(&pdev->dev), priv);
> +               /* VER_0 interrupt is TRIGGER_RISING, VER_0_1 and up is ONESHOT */
> +               if (tsens_version(priv) > VER_0)
> +                       ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> +                                                       thread_fn, IRQF_ONESHOT,
> +                                                       dev_name(&pdev->dev),
> +                                                       priv);
> +               else
> +                       ret = devm_request_threaded_irq(&pdev->dev, irq,
> +                                                       thread_fn, NULL,
> +                                                       IRQF_TRIGGER_RISING,
> +                                                       dev_name(&pdev->dev),
> +                                                       priv);


Just set a flag variable to ONESHOT OR TRIGGER_RISING and use that in the call.

>                 if (ret)
>                         dev_err(&pdev->dev, "%s: failed to get irq\n",
>                                 __func__);
> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> index 59d01162c66a..f1120791737c 100644
> --- a/drivers/thermal/qcom/tsens.h
> +++ b/drivers/thermal/qcom/tsens.h
> @@ -25,7 +25,8 @@ struct tsens_priv;
>
>  /* IP version numbers in ascending order */
>  enum tsens_ver {
> -       VER_0_1 = 0,
> +       VER_0 = 0,
> +       VER_0_1,
>         VER_1_X,
>         VER_2_X,
>  };
> @@ -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?

>         /* WATCHDOG */
>         WDOG_BARK_STATUS,
>         WDOG_BARK_CLEAR,
> --
> 2.27.0
>
Christian Marangi Nov. 25, 2020, 12:22 p.m. UTC | #2
On Mon, Nov 23, 2020 at 01:35:22AM +0530, Amit Kucheria wrote:
> Hi Ansuel,

> 

> See comments inline.

> 

> On Fri, Aug 14, 2020 at 7:12 PM Ansuel Smith <ansuelsmth@gmail.com> wrote:

> >

> > VER_0 is used to describe device based on tsens version before v0.1.

> > These device are devices based on msm8960 for example apq8064 or

> > ipq806x.

> >

> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>

> > ---

> >  drivers/thermal/qcom/tsens.c | 122 +++++++++++++++++++++++++++++++----

> >  drivers/thermal/qcom/tsens.h |   7 +-

> >  2 files changed, 114 insertions(+), 15 deletions(-)

> >

> > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c

> > index 9fe9a2b26705..965c4799918a 100644

> > --- a/drivers/thermal/qcom/tsens.c

> > +++ b/drivers/thermal/qcom/tsens.c

> > @@ -516,6 +516,15 @@ static irqreturn_t tsens_irq_thread(int irq, void *data)

> >                         dev_dbg(priv->dev, "[%u] %s: no violation:  %d\n",

> >                                 hw_id, __func__, temp);

> >                 }

> > +

> > +               if (tsens_version(priv) < VER_0_1) {

> > +                       /* Constraint: There is only 1 interrupt control register for all

> > +                        * 11 temperature sensor. So monitoring more than 1 sensor based

> > +                        * on interrupts will yield inconsistent result. To overcome this

> > +                        * issue we will monitor only sensor 0 which is the master sensor.

> > +                        */

> > +                       break;

> > +               }

> >         }

> >

> >         return IRQ_HANDLED;

> > @@ -531,6 +540,13 @@ static int tsens_set_trips(void *_sensor, int low, int high)

> >         int high_val, low_val, cl_high, cl_low;

> >         u32 hw_id = s->hw_id;

> >

> > +       if (tsens_version(priv) < VER_0_1) {

> > +               /* Pre v0.1 IP had a single register for each type of interrupt

> > +                * and thresholds

> > +                */

> > +               hw_id = 0;

> > +       }

> > +

> >         dev_dbg(dev, "[%u] %s: proposed thresholds: (%d:%d)\n",

> >                 hw_id, __func__, low, high);

> >

> > @@ -584,18 +600,21 @@ int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp)

> >         u32 valid;

> >         int ret;

> >

> > -       ret = regmap_field_read(priv->rf[valid_idx], &valid);

> > -       if (ret)

> > -               return ret;

> > -       while (!valid) {

> > -               /* Valid bit is 0 for 6 AHB clock cycles.

> > -                * At 19.2MHz, 1 AHB clock is ~60ns.

> > -                * We should enter this loop very, very rarely.

> > -                */

> > -               ndelay(400);

> > +       /* VER_0 doesn't have VALID bit */

> > +       if (tsens_version(priv) >= VER_0_1) {

> >                 ret = regmap_field_read(priv->rf[valid_idx], &valid);

> >                 if (ret)

> >                         return ret;

> > +               while (!valid) {

> > +                       /* Valid bit is 0 for 6 AHB clock cycles.

> > +                        * At 19.2MHz, 1 AHB clock is ~60ns.

> > +                        * We should enter this loop very, very rarely.

> > +                        */

> > +                       ndelay(400);

> > +                       ret = regmap_field_read(priv->rf[valid_idx], &valid);

> > +                       if (ret)

> > +                               return ret;

> > +               }

> 

> Let's revisit this after fixing patch 1.

> 

> 

> >         }

> >

> >         /* Valid bit is set, OK to read the temperature */

> > @@ -763,6 +782,10 @@ int __init init_common(struct tsens_priv *priv)

> >                 goto err_put_device;

> >         }

> >

> > +       /* VER_0 have only tm_map */

> > +       if (!priv->srot_map)

> > +               priv->srot_map = priv->tm_map;

> > +

> >         if (tsens_version(priv) > VER_0_1) {

> >                 for (i = VER_MAJOR; i <= VER_STEP; i++) {

> >                         priv->rf[i] = devm_regmap_field_alloc(dev, priv->srot_map,

> > @@ -781,6 +804,10 @@ int __init init_common(struct tsens_priv *priv)

> >                 ret = PTR_ERR(priv->rf[TSENS_EN]);

> >                 goto err_put_device;

> >         }

> > +       /* in VER_0 TSENS need to be explicitly enabled */

> > +       if (tsens_version(priv) == VER_0)

> > +               regmap_field_write(priv->rf[TSENS_EN], 1);

> > +

> >         ret = regmap_field_read(priv->rf[TSENS_EN], &enabled);

> >         if (ret)

> >                 goto err_put_device;

> > @@ -803,6 +830,61 @@ int __init init_common(struct tsens_priv *priv)

> >                 goto err_put_device;

> >         }

> >

> > +       priv->rf[TSENS_EN] = devm_regmap_field_alloc(dev, priv->tm_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[TSENS_SW_RST] = devm_regmap_field_alloc(

> > +               dev, priv->tm_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[LOW_INT_CLEAR_0] = devm_regmap_field_alloc(

> > +               dev, priv->tm_map, priv->fields[LOW_INT_CLEAR_0]);

> > +       if (IS_ERR(priv->rf[LOW_INT_CLEAR_0])) {

> > +               ret = PTR_ERR(priv->rf[LOW_INT_CLEAR_0]);

> > +               goto err_put_device;

> > +       }

> > +

> > +       priv->rf[UP_INT_CLEAR_0] = devm_regmap_field_alloc(

> > +               dev, priv->tm_map, priv->fields[UP_INT_CLEAR_0]);

> > +       if (IS_ERR(priv->rf[UP_INT_CLEAR_0])) {

> > +               ret = PTR_ERR(priv->rf[UP_INT_CLEAR_0]);

> > +               goto err_put_device;

> > +       }

> > +

> > +       /* VER_0 require to set MIN and MAX THRESH */

> > +       if (tsens_version(priv) < VER_0_1) {

> > +               priv->rf[MIN_THRESH_0] = devm_regmap_field_alloc(

> > +                       dev, priv->tm_map, priv->fields[MIN_THRESH_0]);

> > +               if (IS_ERR(priv->rf[MIN_THRESH_0])) {

> > +                       ret = PTR_ERR(priv->rf[MIN_THRESH_0]);

> > +                       goto err_put_device;

> > +               }

> > +

> > +               priv->rf[MAX_THRESH_0] = devm_regmap_field_alloc(

> > +                       dev, priv->tm_map, priv->fields[MAX_THRESH_0]);

> > +               if (IS_ERR(priv->rf[MAX_THRESH_0])) {

> > +                       ret = PTR_ERR(priv->rf[MAX_THRESH_0]);

> > +                       goto err_put_device;

> > +               }

> > +

> > +               regmap_field_write(priv->rf[MIN_THRESH_0], 0);

> > +               regmap_field_write(priv->rf[MAX_THRESH_0], 120000);

> > +       }

> > +

> > +       priv->rf[TRDY] =

> > +               devm_regmap_field_alloc(dev, priv->tm_map, priv->fields[TRDY]);

> > +       if (IS_ERR(priv->rf[TRDY])) {

> > +               ret = PTR_ERR(priv->rf[TRDY]);

> > +               goto err_put_device;

> > +       }

> > +

> >         /* This loop might need changes if enum regfield_ids is reordered */

> >         for (j = LAST_TEMP_0; j <= UP_THRESH_15; j += 16) {

> >                 for (i = 0; i < priv->feat->max_sensors; i++) {

> > @@ -856,7 +938,11 @@ int __init init_common(struct tsens_priv *priv)

> >         }

> >

> >         spin_lock_init(&priv->ul_lock);

> > -       tsens_enable_irq(priv);

> > +

> > +       /* VER_0 interrupt doesn't need to be enabled */

> > +       if (tsens_version(priv) >= VER_0_1)

> > +               tsens_enable_irq(priv);

> > +

> >         tsens_debug_init(op);

> >

> >  err_put_device:

> > @@ -952,10 +1038,18 @@ static int tsens_register_irq(struct tsens_priv *priv, char *irqname,

> >                 if (irq == -ENXIO)

> >                         ret = 0;

> >         } else {

> > -               ret = devm_request_threaded_irq(&pdev->dev, irq,

> > -                                               NULL, thread_fn,

> > -                                               IRQF_ONESHOT,

> > -                                               dev_name(&pdev->dev), priv);

> > +               /* VER_0 interrupt is TRIGGER_RISING, VER_0_1 and up is ONESHOT */

> > +               if (tsens_version(priv) > VER_0)

> > +                       ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,

> > +                                                       thread_fn, IRQF_ONESHOT,

> > +                                                       dev_name(&pdev->dev),

> > +                                                       priv);

> > +               else

> > +                       ret = devm_request_threaded_irq(&pdev->dev, irq,

> > +                                                       thread_fn, NULL,

> > +                                                       IRQF_TRIGGER_RISING,

> > +                                                       dev_name(&pdev->dev),

> > +                                                       priv);

> 

> 

> Just set a flag variable to ONESHOT OR TRIGGER_RISING and use that in the call.

> 

> >                 if (ret)

> >                         dev_err(&pdev->dev, "%s: failed to get irq\n",

> >                                 __func__);

> > diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h

> > index 59d01162c66a..f1120791737c 100644

> > --- a/drivers/thermal/qcom/tsens.h

> > +++ b/drivers/thermal/qcom/tsens.h

> > @@ -25,7 +25,8 @@ struct tsens_priv;

> >

> >  /* IP version numbers in ascending order */

> >  enum tsens_ver {

> > -       VER_0_1 = 0,

> > +       VER_0 = 0,

> > +       VER_0_1,

> >         VER_1_X,

> >         VER_2_X,

> >  };

> > @@ -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? 

> >         /* WATCHDOG */

> >         WDOG_BARK_STATUS,

> >         WDOG_BARK_CLEAR,

> > --

> > 2.27.0

> >
Amit Kucheria Nov. 29, 2020, 12:58 p.m. UTC | #3
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.
Christian Marangi Nov. 29, 2020, 4:28 p.m. UTC | #4
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.
diff mbox series

Patch

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 9fe9a2b26705..965c4799918a 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -516,6 +516,15 @@  static irqreturn_t tsens_irq_thread(int irq, void *data)
 			dev_dbg(priv->dev, "[%u] %s: no violation:  %d\n",
 				hw_id, __func__, temp);
 		}
+
+		if (tsens_version(priv) < VER_0_1) {
+			/* Constraint: There is only 1 interrupt control register for all
+			 * 11 temperature sensor. So monitoring more than 1 sensor based
+			 * on interrupts will yield inconsistent result. To overcome this
+			 * issue we will monitor only sensor 0 which is the master sensor.
+			 */
+			break;
+		}
 	}
 
 	return IRQ_HANDLED;
@@ -531,6 +540,13 @@  static int tsens_set_trips(void *_sensor, int low, int high)
 	int high_val, low_val, cl_high, cl_low;
 	u32 hw_id = s->hw_id;
 
+	if (tsens_version(priv) < VER_0_1) {
+		/* Pre v0.1 IP had a single register for each type of interrupt
+		 * and thresholds
+		 */
+		hw_id = 0;
+	}
+
 	dev_dbg(dev, "[%u] %s: proposed thresholds: (%d:%d)\n",
 		hw_id, __func__, low, high);
 
@@ -584,18 +600,21 @@  int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp)
 	u32 valid;
 	int ret;
 
-	ret = regmap_field_read(priv->rf[valid_idx], &valid);
-	if (ret)
-		return ret;
-	while (!valid) {
-		/* Valid bit is 0 for 6 AHB clock cycles.
-		 * At 19.2MHz, 1 AHB clock is ~60ns.
-		 * We should enter this loop very, very rarely.
-		 */
-		ndelay(400);
+	/* VER_0 doesn't have VALID bit */
+	if (tsens_version(priv) >= VER_0_1) {
 		ret = regmap_field_read(priv->rf[valid_idx], &valid);
 		if (ret)
 			return ret;
+		while (!valid) {
+			/* Valid bit is 0 for 6 AHB clock cycles.
+			 * At 19.2MHz, 1 AHB clock is ~60ns.
+			 * We should enter this loop very, very rarely.
+			 */
+			ndelay(400);
+			ret = regmap_field_read(priv->rf[valid_idx], &valid);
+			if (ret)
+				return ret;
+		}
 	}
 
 	/* Valid bit is set, OK to read the temperature */
@@ -763,6 +782,10 @@  int __init init_common(struct tsens_priv *priv)
 		goto err_put_device;
 	}
 
+	/* VER_0 have only tm_map */
+	if (!priv->srot_map)
+		priv->srot_map = priv->tm_map;
+
 	if (tsens_version(priv) > VER_0_1) {
 		for (i = VER_MAJOR; i <= VER_STEP; i++) {
 			priv->rf[i] = devm_regmap_field_alloc(dev, priv->srot_map,
@@ -781,6 +804,10 @@  int __init init_common(struct tsens_priv *priv)
 		ret = PTR_ERR(priv->rf[TSENS_EN]);
 		goto err_put_device;
 	}
+	/* in VER_0 TSENS need to be explicitly enabled */
+	if (tsens_version(priv) == VER_0)
+		regmap_field_write(priv->rf[TSENS_EN], 1);
+
 	ret = regmap_field_read(priv->rf[TSENS_EN], &enabled);
 	if (ret)
 		goto err_put_device;
@@ -803,6 +830,61 @@  int __init init_common(struct tsens_priv *priv)
 		goto err_put_device;
 	}
 
+	priv->rf[TSENS_EN] = devm_regmap_field_alloc(dev, priv->tm_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[TSENS_SW_RST] = devm_regmap_field_alloc(
+		dev, priv->tm_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[LOW_INT_CLEAR_0] = devm_regmap_field_alloc(
+		dev, priv->tm_map, priv->fields[LOW_INT_CLEAR_0]);
+	if (IS_ERR(priv->rf[LOW_INT_CLEAR_0])) {
+		ret = PTR_ERR(priv->rf[LOW_INT_CLEAR_0]);
+		goto err_put_device;
+	}
+
+	priv->rf[UP_INT_CLEAR_0] = devm_regmap_field_alloc(
+		dev, priv->tm_map, priv->fields[UP_INT_CLEAR_0]);
+	if (IS_ERR(priv->rf[UP_INT_CLEAR_0])) {
+		ret = PTR_ERR(priv->rf[UP_INT_CLEAR_0]);
+		goto err_put_device;
+	}
+
+	/* VER_0 require to set MIN and MAX THRESH */
+	if (tsens_version(priv) < VER_0_1) {
+		priv->rf[MIN_THRESH_0] = devm_regmap_field_alloc(
+			dev, priv->tm_map, priv->fields[MIN_THRESH_0]);
+		if (IS_ERR(priv->rf[MIN_THRESH_0])) {
+			ret = PTR_ERR(priv->rf[MIN_THRESH_0]);
+			goto err_put_device;
+		}
+
+		priv->rf[MAX_THRESH_0] = devm_regmap_field_alloc(
+			dev, priv->tm_map, priv->fields[MAX_THRESH_0]);
+		if (IS_ERR(priv->rf[MAX_THRESH_0])) {
+			ret = PTR_ERR(priv->rf[MAX_THRESH_0]);
+			goto err_put_device;
+		}
+
+		regmap_field_write(priv->rf[MIN_THRESH_0], 0);
+		regmap_field_write(priv->rf[MAX_THRESH_0], 120000);
+	}
+
+	priv->rf[TRDY] =
+		devm_regmap_field_alloc(dev, priv->tm_map, priv->fields[TRDY]);
+	if (IS_ERR(priv->rf[TRDY])) {
+		ret = PTR_ERR(priv->rf[TRDY]);
+		goto err_put_device;
+	}
+
 	/* This loop might need changes if enum regfield_ids is reordered */
 	for (j = LAST_TEMP_0; j <= UP_THRESH_15; j += 16) {
 		for (i = 0; i < priv->feat->max_sensors; i++) {
@@ -856,7 +938,11 @@  int __init init_common(struct tsens_priv *priv)
 	}
 
 	spin_lock_init(&priv->ul_lock);
-	tsens_enable_irq(priv);
+
+	/* VER_0 interrupt doesn't need to be enabled */
+	if (tsens_version(priv) >= VER_0_1)
+		tsens_enable_irq(priv);
+
 	tsens_debug_init(op);
 
 err_put_device:
@@ -952,10 +1038,18 @@  static int tsens_register_irq(struct tsens_priv *priv, char *irqname,
 		if (irq == -ENXIO)
 			ret = 0;
 	} else {
-		ret = devm_request_threaded_irq(&pdev->dev, irq,
-						NULL, thread_fn,
-						IRQF_ONESHOT,
-						dev_name(&pdev->dev), priv);
+		/* VER_0 interrupt is TRIGGER_RISING, VER_0_1 and up is ONESHOT */
+		if (tsens_version(priv) > VER_0)
+			ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+							thread_fn, IRQF_ONESHOT,
+							dev_name(&pdev->dev),
+							priv);
+		else
+			ret = devm_request_threaded_irq(&pdev->dev, irq,
+							thread_fn, NULL,
+							IRQF_TRIGGER_RISING,
+							dev_name(&pdev->dev),
+							priv);
 		if (ret)
 			dev_err(&pdev->dev, "%s: failed to get irq\n",
 				__func__);
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index 59d01162c66a..f1120791737c 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -25,7 +25,8 @@  struct tsens_priv;
 
 /* IP version numbers in ascending order */
 enum tsens_ver {
-	VER_0_1 = 0,
+	VER_0 = 0,
+	VER_0_1,
 	VER_1_X,
 	VER_2_X,
 };
@@ -441,6 +442,10 @@  enum regfield_ids {
 	CRIT_THRESH_14,
 	CRIT_THRESH_15,
 
+	/* VER_0 MIN MAX THRESH */
+	MIN_THRESH_0,
+	MAX_THRESH_0,
+
 	/* WATCHDOG */
 	WDOG_BARK_STATUS,
 	WDOG_BARK_CLEAR,