On Tue, Jan 27 2015 at 17:55 -0700, Narendran Rajan wrote:
-----Original Message-----
From: linux-arm-msm-owner@xxxxxxxxxxxxxxx [mailto:linux-arm-msm-
owner@xxxxxxxxxxxxxxx] 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:
>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 <smohanad@xxxxxxxxxxxxxx>
>Cc: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>
>Signed-off-by: Narendran Rajan <nrajan@xxxxxxxxxxxxxx>
>---
[...]
>+ 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.
>+}
>+
>+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.
>+ return IRQ_HANDLED;
>+}
>+
[...]
>+static struct of_device_id tsens_match_table[] = {
>+ {.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.
>+ .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@xxxxxxxxxxxxxxx More majordomo
info
>at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm"
in
the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo
info at http://vger.kernel.org/majordomo-info.html
--Naren
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html