| Message ID | 20230228112238.2312273-18-daniel.lezcano@linaro.org |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Self-encapsulate the thermal zone device structure | expand |
On Tue, Feb 28, 2023 at 12:22:36PM +0100, Daniel Lezcano wrote: > The code enables the thermal zone after setting it up. But the thermal > zone is already enabled by thermal_of_zone_register() function. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > drivers/thermal/tegra/tegra30-tsensor.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/drivers/thermal/tegra/tegra30-tsensor.c b/drivers/thermal/tegra/tegra30-tsensor.c > index 3506c3f3c474..e38902abf207 100644 > --- a/drivers/thermal/tegra/tegra30-tsensor.c > +++ b/drivers/thermal/tegra/tegra30-tsensor.c > @@ -346,7 +346,7 @@ static int tegra_tsensor_enable_hw_channel(const struct tegra_tsensor *ts, > { > const struct tegra_tsensor_channel *tsc = &ts->ch[id]; > struct thermal_zone_device *tzd = tsc->tzd; > - int err, hot_trip = 0, crit_trip = 0; > + int hot_trip = 0, crit_trip = 0; > u32 val; > > if (!tzd) { > @@ -401,12 +401,6 @@ static int tegra_tsensor_enable_hw_channel(const struct tegra_tsensor *ts, > val |= FIELD_PREP(TSENSOR_SENSOR0_CONFIG0_INTR_THERMAL_RST_EN, 1); > writel_relaxed(val, tsc->regs + TSENSOR_SENSOR0_CONFIG0); > > - err = thermal_zone_device_enable(tzd); > - if (err) { > - dev_err(ts->dev, "ch%u: failed to enable zone: %d\n", id, err); > - return err; > - } > - > return 0; > } This function is called in tegra_tsensor_resume() and balances out the tegra_tsensor_hw_channel() calls from tegra_tsensor_suspend(). If we remove the call from here, we'll likely end up with the zones disabled after a resume. This ends up calling thermal_zone_device_set_mode(), which is a no-op, basically, if the mode is unchanged, so this seems harmless. If you need this to change anyway, I suppose we could move the calls to thermal_zone_device_enable() and move them into suspend/resume instead. That's a bit tricky to undo in the error recovery paths, but should be doable. Thierry
On 01/03/2023 15:29, Thierry Reding wrote: > On Tue, Feb 28, 2023 at 12:22:36PM +0100, Daniel Lezcano wrote: >> The code enables the thermal zone after setting it up. But the thermal >> zone is already enabled by thermal_of_zone_register() function. >> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> --- >> drivers/thermal/tegra/tegra30-tsensor.c | 8 +------- >> 1 file changed, 1 insertion(+), 7 deletions(-) >> >> diff --git a/drivers/thermal/tegra/tegra30-tsensor.c b/drivers/thermal/tegra/tegra30-tsensor.c >> index 3506c3f3c474..e38902abf207 100644 >> --- a/drivers/thermal/tegra/tegra30-tsensor.c >> +++ b/drivers/thermal/tegra/tegra30-tsensor.c >> @@ -346,7 +346,7 @@ static int tegra_tsensor_enable_hw_channel(const struct tegra_tsensor *ts, >> { >> const struct tegra_tsensor_channel *tsc = &ts->ch[id]; >> struct thermal_zone_device *tzd = tsc->tzd; >> - int err, hot_trip = 0, crit_trip = 0; >> + int hot_trip = 0, crit_trip = 0; >> u32 val; >> >> if (!tzd) { >> @@ -401,12 +401,6 @@ static int tegra_tsensor_enable_hw_channel(const struct tegra_tsensor *ts, >> val |= FIELD_PREP(TSENSOR_SENSOR0_CONFIG0_INTR_THERMAL_RST_EN, 1); >> writel_relaxed(val, tsc->regs + TSENSOR_SENSOR0_CONFIG0); >> >> - err = thermal_zone_device_enable(tzd); >> - if (err) { >> - dev_err(ts->dev, "ch%u: failed to enable zone: %d\n", id, err); >> - return err; >> - } >> - >> return 0; >> } > > This function is called in tegra_tsensor_resume() and balances out the > tegra_tsensor_hw_channel() calls from tegra_tsensor_suspend(). If we > remove the call from here, we'll likely end up with the zones disabled > after a resume. > > This ends up calling thermal_zone_device_set_mode(), which is a no-op, > basically, if the mode is unchanged, so this seems harmless. > > If you need this to change anyway, I suppose we could move the calls to > thermal_zone_device_enable() and move them into suspend/resume instead. > That's a bit tricky to undo in the error recovery paths, but should be > doable. No it is fine, I will just drop this patch from the series.
diff --git a/drivers/thermal/tegra/tegra30-tsensor.c b/drivers/thermal/tegra/tegra30-tsensor.c index 3506c3f3c474..e38902abf207 100644 --- a/drivers/thermal/tegra/tegra30-tsensor.c +++ b/drivers/thermal/tegra/tegra30-tsensor.c @@ -346,7 +346,7 @@ static int tegra_tsensor_enable_hw_channel(const struct tegra_tsensor *ts, { const struct tegra_tsensor_channel *tsc = &ts->ch[id]; struct thermal_zone_device *tzd = tsc->tzd; - int err, hot_trip = 0, crit_trip = 0; + int hot_trip = 0, crit_trip = 0; u32 val; if (!tzd) { @@ -401,12 +401,6 @@ static int tegra_tsensor_enable_hw_channel(const struct tegra_tsensor *ts, val |= FIELD_PREP(TSENSOR_SENSOR0_CONFIG0_INTR_THERMAL_RST_EN, 1); writel_relaxed(val, tsc->regs + TSENSOR_SENSOR0_CONFIG0); - err = thermal_zone_device_enable(tzd); - if (err) { - dev_err(ts->dev, "ch%u: failed to enable zone: %d\n", id, err); - return err; - } - return 0; }
The code enables the thermal zone after setting it up. But the thermal zone is already enabled by thermal_of_zone_register() function. Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> --- drivers/thermal/tegra/tegra30-tsensor.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)