| Message ID | 1380010102-25817-2-git-send-email-l.majewski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Sep 24, 2013 at 1:38 PM, Lukasz Majewski <l.majewski@samsung.com> wrote: > The commit 4de0bdaa9677d11406c9becb70c60887c957e1f0 > ("thermal: exynos: Add support for instance based register/unregister") > broke check for presence of therm_dev at global thermal zone in > exynos_report_trigger(). > > The resulting wrong test prevents thermal_zone_device_update() call, which > calls handlers for situation when trip points are passed. > Such behavior prevents thermal driver from proper reaction (when TMU interrupt > is raised) in a situation when overheating is detected at TMU hardware. > This patch fixes it. > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Reviewed-by: Tomasz Figa <t.figa@samsung.com> > --- > drivers/thermal/samsung/exynos_thermal_common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/thermal/samsung/exynos_thermal_common.c b/drivers/thermal/samsung/exynos_thermal_common.c > index f10a6ad..55a912a 100644 > --- a/drivers/thermal/samsung/exynos_thermal_common.c > +++ b/drivers/thermal/samsung/exynos_thermal_common.c > @@ -310,7 +310,7 @@ void exynos_report_trigger(struct thermal_sensor_conf *conf) > } > > th_zone = conf->pzone_data; > - if (th_zone->therm_dev) > + if (!th_zone->therm_dev) > return; Changes looks fine. Reviewed-by: Amit Daniel Kachhap<amit.daniel@samsung.com> Thanks, Amit Daniel > > if (th_zone->bind == false) { > -- > 1.7.10.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html On Tue, Sep 24, 2013 at 1:38 PM, Lukasz Majewski <l.majewski@samsung.com> wrote: > The commit 4de0bdaa9677d11406c9becb70c60887c957e1f0 > ("thermal: exynos: Add support for instance based register/unregister") > broke check for presence of therm_dev at global thermal zone in > exynos_report_trigger(). > > The resulting wrong test prevents thermal_zone_device_update() call, which > calls handlers for situation when trip points are passed. > Such behavior prevents thermal driver from proper reaction (when TMU interrupt > is raised) in a situation when overheating is detected at TMU hardware. > This patch fixes it. > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Reviewed-by: Tomasz Figa <t.figa@samsung.com> > --- > drivers/thermal/samsung/exynos_thermal_common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/thermal/samsung/exynos_thermal_common.c b/drivers/thermal/samsung/exynos_thermal_common.c > index f10a6ad..55a912a 100644 > --- a/drivers/thermal/samsung/exynos_thermal_common.c > +++ b/drivers/thermal/samsung/exynos_thermal_common.c > @@ -310,7 +310,7 @@ void exynos_report_trigger(struct thermal_sensor_conf *conf) > } > > th_zone = conf->pzone_data; > - if (th_zone->therm_dev) > + if (!th_zone->therm_dev) > return; > > if (th_zone->bind == false) { > -- > 1.7.10.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 24-09-2013 04:08, Lukasz Majewski wrote: > The commit 4de0bdaa9677d11406c9becb70c60887c957e1f0 > ("thermal: exynos: Add support for instance based register/unregister") > broke check for presence of therm_dev at global thermal zone in > exynos_report_trigger(). > > The resulting wrong test prevents thermal_zone_device_update() call, which > calls handlers for situation when trip points are passed. > Such behavior prevents thermal driver from proper reaction (when TMU interrupt > is raised) in a situation when overheating is detected at TMU hardware. > This patch fixes it. > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Reviewed-by: Tomasz Figa <t.figa@samsung.com> > --- > drivers/thermal/samsung/exynos_thermal_common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/thermal/samsung/exynos_thermal_common.c b/drivers/thermal/samsung/exynos_thermal_common.c > index f10a6ad..55a912a 100644 > --- a/drivers/thermal/samsung/exynos_thermal_common.c > +++ b/drivers/thermal/samsung/exynos_thermal_common.c > @@ -310,7 +310,7 @@ void exynos_report_trigger(struct thermal_sensor_conf *conf) > } > > th_zone = conf->pzone_data; > - if (th_zone->therm_dev) > + if (!th_zone->therm_dev) Fine with this fix, as it really looks obvious. But after reading the code a bit, I am considering if we can remove the above check at all. Reading the driver code piece at drivers/thermal/samsung/exynos_tmu.c, by checking how exynos_register_thermal gets called, and how error is handled, I assume we do not need to check for th_zone->therm_dev. To me looks like the driver only allows th_zone's exist only with valid therm_dev, isn't it? Except for the probe sequence, maybe, at run time there is a time window that we have valid th_zone with invalid therm_dev. However, reading the probe, still, the irq gets initialized only in very end, so the work queue don't get queue till then at least. So, my question before acking is, shall we remove this check altogether? > return; > > if (th_zone->bind == false) { >
Hi Eduardo, Thanks for reply. > On 24-09-2013 04:08, Lukasz Majewski wrote: > > The commit 4de0bdaa9677d11406c9becb70c60887c957e1f0 > > ("thermal: exynos: Add support for instance based > > register/unregister") broke check for presence of therm_dev at > > global thermal zone in exynos_report_trigger(). > > > > The resulting wrong test prevents thermal_zone_device_update() > > call, which calls handlers for situation when trip points are > > passed. Such behavior prevents thermal driver from proper reaction > > (when TMU interrupt is raised) in a situation when overheating is > > detected at TMU hardware. This patch fixes it. > > > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > > Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > > Reviewed-by: Tomasz Figa <t.figa@samsung.com> > > --- > > drivers/thermal/samsung/exynos_thermal_common.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/thermal/samsung/exynos_thermal_common.c > > b/drivers/thermal/samsung/exynos_thermal_common.c index > > f10a6ad..55a912a 100644 --- > > a/drivers/thermal/samsung/exynos_thermal_common.c +++ > > b/drivers/thermal/samsung/exynos_thermal_common.c @@ -310,7 +310,7 > > @@ void exynos_report_trigger(struct thermal_sensor_conf *conf) } > > > > th_zone = conf->pzone_data; > > - if (th_zone->therm_dev) > > + if (!th_zone->therm_dev) > > Fine with this fix, as it really looks obvious. But after reading the > code a bit, I am considering if we can remove the above check at all. I have just preserved the old behaviour for -rcX fix. > > Reading the driver code piece at drivers/thermal/samsung/exynos_tmu.c, > by checking how exynos_register_thermal gets called, and how error is > handled, I assume we do not need to check for th_zone->therm_dev. It looks like we don't need this check (we will not register the thermal zone if therm dev is not correct). > > To me looks like the driver only allows th_zone's exist only with > valid therm_dev, isn't it? Except for the probe sequence, maybe, at > run time there is a time window that we have valid th_zone with > invalid therm_dev. However, reading the probe, still, the irq gets > initialized only in very end, so the work queue don't get queue till > then at least. > > So, my question before acking is, shall we remove this check > altogether? I think that it is up to you to decide how we proceed. Shall this patch go into v3.12-rcX since it is "obvious" (and documents what and when was broken) or would you require to prepare "new" patch with removal of this check? > > > > return; > > > > if (th_zone->bind == false) { > > > >
diff --git a/drivers/thermal/samsung/exynos_thermal_common.c b/drivers/thermal/samsung/exynos_thermal_common.c index f10a6ad..55a912a 100644 --- a/drivers/thermal/samsung/exynos_thermal_common.c +++ b/drivers/thermal/samsung/exynos_thermal_common.c @@ -310,7 +310,7 @@ void exynos_report_trigger(struct thermal_sensor_conf *conf) } th_zone = conf->pzone_data; - if (th_zone->therm_dev) + if (!th_zone->therm_dev) return; if (th_zone->bind == false) {
The commit 4de0bdaa9677d11406c9becb70c60887c957e1f0 ("thermal: exynos: Add support for instance based register/unregister") broke check for presence of therm_dev at global thermal zone in exynos_report_trigger(). The resulting wrong test prevents thermal_zone_device_update() call, which calls handlers for situation when trip points are passed. Such behavior prevents thermal driver from proper reaction (when TMU interrupt is raised) in a situation when overheating is detected at TMU hardware. This patch fixes it. Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Reviewed-by: Tomasz Figa <t.figa@samsung.com> --- drivers/thermal/samsung/exynos_thermal_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)