On Tue, Jan 27 2015 at 17:55 -0700, Narendran Rajan wrote:

quoted

-----Original Message-----
From: linux-arm-msm-owner@vger.kernel.org [mailto:linux-arm-msm-
owner@vger.kernel.org] On Behalf Of Lina Iyer
Sent: Tuesday, January 27, 2015 8:03 AM
To: Narendran Rajan
Cc: Zhang Rui; Eduardo Valentin; Linux ARM MSM; Linux PM; Siddartha
Mohanadoss; Stephen Boyd
Subject: Re: [PATCH] thermal: Add msm tsens thermal sensor driver

On Mon, Jan 26 2015 at 21:10 -0700, Narendran Rajan wrote:

quoted

TSENS supports reading temperature from multiple thermal sensors
present in QCOM SOCs.
TSENS HW is enabled only when the main sensor is requested.
The TSENS block is disabled if the main senors is disabled irrespective
of any other sensors that are being enabled.
TSENS driver supports configurable threshold for temperature monitoring
in which case it can generate an interrupt when specific thresholds are
reached

Based on code by Siddartha Mohanadoss and Stephen Boyd.

Cc: Siddartha Mohanadoss <redacted>
Cc: Stephen Boyd <redacted>
Signed-off-by: Narendran Rajan  <redacted>
---

[...]

quoted

quoted

+	regmap_field_update_bits(status,
+			TSENS_UPPER_STATUS_CLR |

TSENS_LOWER_STATUS_CLR, mask);

Can TSENS IRQ fire before this write?

Yes it can as per document, let me run further experiments and update.

If the IRQ fires before this work-fn() is complete, schedule_work() would not
reschedule. 

quoted

quoted

+}
+
+static irqreturn_t tsens_isr(int irq, void *data) {
+	schedule_work(data);

You probably want to reduce the latency of interrupt notifications here.
If the kernel wq gets loaded up, your IRQ handling would suffer as well.

Good point.  Thx. Let me run further experiments on interrupt firing to
confirm the behavior on interrupt frequency and load.
PS: By default thermal framework uses a polling mode and default thresholds
set are very high.

Yes, sure. But thermal ramps are better handled immediately. IRQs are
the most preferred on production devices.

quoted

quoted

+	return IRQ_HANDLED;
+}
+

[...]

quoted

+static struct of_device_id tsens_match_table[] = {

quoted

+	{.compatible = "qcom,ipq806x-tsens"},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, tsens_match_table);
+
+static struct platform_driver tsens_driver = {
+	.probe = tsens_probe,
+	.remove = tsens_remove,
+	.driver = {
+		.of_match_table = tsens_match_table,
+		.name = "tsens8960-thermal",

Curious, why specifically 8960?

I guess your point is why not tsens-thermal ?
There are different versions of tsens controller used within QCOM SoCs.
May need a different driver for newer versions of this controller.
Picked 8960 as I guess this was the earliest chip where this controller was
used.

It would be nice if you keep this generic, as it will be applicable to a
slew of QCOM SoC's. I havent seen it change much in the past few SoCs.
You could also use the compatible flag for any small revisional changes.

quoted

quoted

+		.owner = THIS_MODULE,
+#ifdef CONFIG_PM
+		.pm	= &tsens_pm_ops,
+#endif
+	},
+};
+module_platform_driver(tsens_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Temperature Sensor driver");
+MODULE_ALIAS("platform:tsens8960-tm");
--
Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc.
is a member of the Code Aurora Forum, a Linux Foundation Collaborative
Project

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org More majordomo

info

quoted

at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm"

in

quoted

the body of a message to majordomo@vger.kernel.org More majordomo
info at  http://vger.kernel.org/majordomo-info.html

--Naren