On Tue, Mar 15, 2016 at 05:12:12PM +0800, Wei Ni wrote:
>
>
> On 2016年03月15日 03:16, Eduardo Valentin wrote:
> > * PGP Signed by an unknown key
> >
> > On Fri, Mar 11, 2016 at 11:11:12AM +0800, Wei Ni wrote:
> >> Add support for hardware critical thermal limits to the
> >> SOC_THERM driver. It use the Linux thermal framework to
> >> create critical trip temp, and set it to SOC_THERM hardware.
> >> If these limits are breached, the chip will reset, and if
> >> appropriately configured, will turn off the PMIC.
> >>
> >> This support is critical for safe usage of the chip.
> >>
> >> Signed-off-by: Wei Ni <wni@xxxxxxxxxx>
> >> ---
> >> drivers/thermal/tegra/soctherm.c | 166 +++++++++++++++++++++++++++++-
> >> drivers/thermal/tegra/soctherm.h | 7 ++
> >> drivers/thermal/tegra/tegra124-soctherm.c | 24 +++++
> >> drivers/thermal/tegra/tegra210-soctherm.c | 24 +++++
> >> 4 files changed, 216 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
> >> index 02ac6d2e5a20..dbaab160baba 100644
> >> --- a/drivers/thermal/tegra/soctherm.c
> >> +++ b/drivers/thermal/tegra/soctherm.c
> >> @@ -73,9 +73,14 @@
> >> #define REG_SET_MASK(r, m, v) (((r) & ~(m)) | \
> >> (((v) & (m >> (ffs(m) - 1))) << (ffs(m) - 1)))
> >>
> >> +static const int min_low_temp = -127000;
> >> +static const int max_high_temp = 127000;
> >> +
> >> struct tegra_thermctl_zone {
> >> void __iomem *reg;
> >> - u32 mask;
> >> + struct device *dev;
> >> + struct thermal_zone_device *tz;
> >
> >
> > Why not using tz->dev for the *dev above?
>
> The tz is thermal_zone_device, this structure doesn't have *dev.
> It only have the member "struct device device;", but this device is created for
> the thermal class, not this tegra_soctherm device.
>
> >
> >> + const struct tegra_tsensor_group *sg;
> >> };
> >>
> >> struct tegra_soctherm {
> >> @@ -145,22 +150,158 @@ static int tegra_thermctl_get_temp(void *data, int *out_temp)
> >> u32 val;
> >>
> >> val = readl(zone->reg);
> >> - val = REG_GET_MASK(val, zone->mask);
> >> + val = REG_GET_MASK(val, zone->sg->sensor_temp_mask);
> >> *out_temp = translate_temp(val);
> >>
> >> return 0;
> >> }
> >>
> >> +static int
> >> +thermtrip_program(struct device *dev, const struct tegra_tsensor_group *sg,
> >> + int trip_temp);
> >> +
> >> +static int tegra_thermctl_set_trip_temp(void *data, int trip, int temp)
> >> +{
> >> + struct tegra_thermctl_zone *zone = data;
> >> + struct thermal_zone_device *tz = zone->tz;
> >> + const struct tegra_tsensor_group *sg = zone->sg;
> >> + struct device *dev = zone->dev;
> >> + enum thermal_trip_type type;
> >> + int ret;
> >> +
> >> + if (!tz)
> >> + return -EINVAL;
> >
> >
> > Is the above check needed? If you saw a case in which your function is
> > called without tz, would it be the case we have a but in the probe (or
> > even worse, in thermal-core)?
>
> This tz isn't from thermal-core, it's from the "void *data".
> This *data is the private structure "struct tegra_thermctl_zone *zone = data;".
> It is registered in devm_thermal_zone_of_sensor_register(*dev, sensor_id, *data,
> *ops). And when it register successful, I will set zone->tz = z, in here, the
> zone is the private data.
> Let's consider a special case, once the thermal_zone_of_sensor_register
> successful and didn't run to "zone->tz = z" yet, then the thermal_core implement
> .set_trip(), then it may cause problems in here, although it's difficult to hit
> this case. So I think we need to do this check.
Can you be more specific? I don't recall a case that core would call any
driver callbacks before setting up the data structures properly.
> >
Attachment:
signature.asc
Description: Digital signature
