[01/14] thermal: ti-soc-thermal: fix TALERT IRQ handling for DRA752
diff mbox

Message ID 1526298141-14045-2-git-send-email-b.zolnierkie@samsung.com
State New
Headers show

Commit Message

Bartlomiej Zolnierkiewicz May 14, 2018, 11:42 a.m. UTC
.report_temperature is not set in dra752_data which
results in temperature updates not being propagated by
ti_bandgap_talert_irq_handler() (it doesn't make much
sense to handle TALERT IRQ without reporting temperature
updates to the thermal core). Fix it.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/ti-soc-thermal/dra752-thermal-data.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Keerthy July 11, 2018, 2:19 a.m. UTC | #1
On 5/14/2018 5:12 PM, Bartlomiej Zolnierkiewicz wrote:
> .report_temperature is not set in dra752_data which
> results in temperature updates not being propagated by
> ti_bandgap_talert_irq_handler() (it doesn't make much
> sense to handle TALERT IRQ without reporting temperature
> updates to the thermal core). Fix it.

ATM no one is using TALERT as the thermal software polls on the 
temperature. No real benefit from TALERT.

TALERT is set at different temperature and software polling thresholds 
come from Device tree and i believe its best for software to go by 
polling and then act on trip points.

> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>   drivers/thermal/ti-soc-thermal/dra752-thermal-data.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
> index 33a3030..e8ab7e5 100644
> --- a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
> +++ b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
> @@ -338,6 +338,7 @@
>   	.adc_end_val = DRA752_ADC_END_VALUE,
>   	.expose_sensor = ti_thermal_expose_sensor,
>   	.remove_sensor = ti_thermal_remove_sensor,
> +	.report_temperature = ti_thermal_report_sensor_temperature,
>   	.sensors = {
>   		{
>   		.registers = &dra752_mpu_temp_sensor_registers,
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bartlomiej Zolnierkiewicz July 25, 2018, 2:27 p.m. UTC | #2
On Wednesday, July 11, 2018 07:49:41 AM J, KEERTHY wrote:
> 
> On 5/14/2018 5:12 PM, Bartlomiej Zolnierkiewicz wrote:
> > .report_temperature is not set in dra752_data which
> > results in temperature updates not being propagated by
> > ti_bandgap_talert_irq_handler() (it doesn't make much
> > sense to handle TALERT IRQ without reporting temperature
> > updates to the thermal core). Fix it.
> 
> ATM no one is using TALERT as the thermal software polls on the 
> temperature. No real benefit from TALERT.
> 
> TALERT is set at different temperature and software polling thresholds 
> come from Device tree and i believe its best for software to go by 
> polling and then act on trip points.

Could you please explain what do you mean by "no one is using
TALERT"?

The code in ti_bandgap_probe() sets TALERT thresholds and requests
IRQ if the TI_BANDGAP_FEATURE_TALERT feature flag is set (and this
flag is set in omap4460_data, omap4470_data, omap5430_data and
dra752_data).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keerthy July 27, 2018, 4:43 a.m. UTC | #3
On Wednesday 25 July 2018 07:57 PM, Bartlomiej Zolnierkiewicz wrote:
> On Wednesday, July 11, 2018 07:49:41 AM J, KEERTHY wrote:
>>
>> On 5/14/2018 5:12 PM, Bartlomiej Zolnierkiewicz wrote:
>>> .report_temperature is not set in dra752_data which
>>> results in temperature updates not being propagated by
>>> ti_bandgap_talert_irq_handler() (it doesn't make much
>>> sense to handle TALERT IRQ without reporting temperature
>>> updates to the thermal core). Fix it.
>>
>> ATM no one is using TALERT as the thermal software polls on the 
>> temperature. No real benefit from TALERT.
>>
>> TALERT is set at different temperature and software polling thresholds 
>> come from Device tree and i believe its best for software to go by 
>> polling and then act on trip points.
> 
> Could you please explain what do you mean by "no one is using
> TALERT"?
> 
> The code in ti_bandgap_probe() sets TALERT thresholds and requests
> IRQ if the TI_BANDGAP_FEATURE_TALERT feature flag is set (and this
> flag is set in omap4460_data, omap4470_data, omap5430_data and
> dra752_data).

The software thresholds and the polling takes care of reducing the
temperature. What i actually meant was we never relied on talert and the
polling takes care of keeping a check on the temperature.

Regards,
Keerthy

> 
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bartlomiej Zolnierkiewicz July 27, 2018, 2:30 p.m. UTC | #4
On Friday, July 27, 2018 10:13:51 AM Keerthy wrote:
> 
> On Wednesday 25 July 2018 07:57 PM, Bartlomiej Zolnierkiewicz wrote:
> > On Wednesday, July 11, 2018 07:49:41 AM J, KEERTHY wrote:
> >>
> >> On 5/14/2018 5:12 PM, Bartlomiej Zolnierkiewicz wrote:
> >>> .report_temperature is not set in dra752_data which
> >>> results in temperature updates not being propagated by
> >>> ti_bandgap_talert_irq_handler() (it doesn't make much
> >>> sense to handle TALERT IRQ without reporting temperature
> >>> updates to the thermal core). Fix it.
> >>
> >> ATM no one is using TALERT as the thermal software polls on the 
> >> temperature. No real benefit from TALERT.
> >>
> >> TALERT is set at different temperature and software polling thresholds 
> >> come from Device tree and i believe its best for software to go by 
> >> polling and then act on trip points.
> > 
> > Could you please explain what do you mean by "no one is using
> > TALERT"?
> > 
> > The code in ti_bandgap_probe() sets TALERT thresholds and requests
> > IRQ if the TI_BANDGAP_FEATURE_TALERT feature flag is set (and this
> > flag is set in omap4460_data, omap4470_data, omap5430_data and
> > dra752_data).
> 
> The software thresholds and the polling takes care of reducing the
> temperature. What i actually meant was we never relied on talert and the
> polling takes care of keeping a check on the temperature.

OK I see, maybe TALERT handling can be removed altogether?

If not can we make TALERT behavior on DRA752 consistent with other
chipsets supporting TALERT (this is useful for code unification
between OMAP5430 and DRA752)?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
index 33a3030..e8ab7e5 100644
--- a/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
+++ b/drivers/thermal/ti-soc-thermal/dra752-thermal-data.c
@@ -338,6 +338,7 @@ 
 	.adc_end_val = DRA752_ADC_END_VALUE,
 	.expose_sensor = ti_thermal_expose_sensor,
 	.remove_sensor = ti_thermal_remove_sensor,
+	.report_temperature = ti_thermal_report_sensor_temperature,
 	.sensors = {
 		{
 		.registers = &dra752_mpu_temp_sensor_registers,