Re: [PATCH v3 6/9] drivers: thermal: tsens: Add watchdog support

Bjorn Andersson
2020-01-02T19:55:42Z

On Thu 02 Jan 06:54 PST 2020, Amit Kucheria wrote:
[..]
> diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> index 4cf550766cf6..ecbc722eb348 100644
> --- a/drivers/thermal/qcom/tsens-common.c
> +++ b/drivers/thermal/qcom/tsens-common.c
> @@ -377,6 +377,24 @@ irqreturn_t tsens_critical_irq_thread(int irq, void *data)
> struct tsens_irq_data d;
> unsigned long flags;
> int temp, ret, i;
> + u32 wdog_status, wdog_count;
> +
> + if (priv->feat->has_watchdog) {
> + /* Watchdog is present only on v2.3+ */

Please omit this comment, you're carrying the motivation for this
decision when you set has_watchdog already.

> + ret = regmap_field_read(priv->rf[WDOG_BARK_STATUS], &wdog_status);
> + if (ret)
> + return ret;
> +
> + /* Clear WDOG interrupt */
> + regmap_field_write(priv->rf[WDOG_BARK_CLEAR], 1);
> + regmap_field_write(priv->rf[WDOG_BARK_CLEAR], 0);

Are you sure you need to zero the clear register?

> +
> + ret = regmap_field_read(priv->rf[WDOG_BARK_COUNT], &wdog_count);
> + if (ret)
> + return ret;
> + if (wdog_count)
> + dev_dbg(priv->dev, "%s: watchdog count: %d\n", __func__, wdog_count);
> + }
>
[..]
> @@ -793,6 +815,22 @@ int __init init_common(struct tsens_priv *priv)
> }
> }
>
> + if (tsens_version(priv) > VER_1_X && ver_minor > 2) {
> + /* Watchdog is present only on v2.3+ */
> + priv->feat->has_watchdog = 1;
> + for (i = WDOG_BARK_STATUS; i <= CC_MON_MASK; i++) {
> + priv->rf[i] = devm_regmap_field_alloc(dev, priv->tm_map,
> + priv->fields[i]);
> + if (IS_ERR(priv->rf[i])) {
> + ret = PTR_ERR(priv->rf[i]);
> + goto err_put_device;
> + }
> + }
> + /* Enable WDOG and disable cycle completion monitoring */

Commit message says you're not enabling it. Should this say "WDOG is
already configured, unmask the bark" or something along those lines?

> + regmap_field_write(priv->rf[WDOG_BARK_MASK], 0);
> + regmap_field_write(priv->rf[CC_MON_MASK], 1);
> + }
> +
> spin_lock_init(&priv->ul_lock);
> tsens_enable_irq(priv);
> tsens_debug_init(op);

Regards,
Bjorn