| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAHQ1cqHnuzPuaVKHRTzEhj=H7Rd0+kBA4p5iXnvbT2=DzLZMVQ@mail.gmail.com>
Date: Thu, 21 Feb 2019 10:53:52 -0800
From: Andrey Smirnov <andrew.smirnov@...il.com>
To: Eduardo Valentin <edubezval@...il.com>
Cc: linux-pm@...r.kernel.org, Chris Healy <cphealy@...il.com>,
Lucas Stach <l.stach@...gutronix.de>,
Zhang Rui <rui.zhang@...el.com>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
dl-linux-imx <linux-imx@....com>,
linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 12/12] thermal: qoriq: Add support for multiple thremal sites
On Wed, Feb 20, 2019 at 4:57 PM Eduardo Valentin <edubezval@...il.com> wrote:
>
> On Mon, Feb 18, 2019 at 11:11:41AM -0800, Andrey Smirnov wrote:
> > TMU IP block provides temerature measurement for up to 16 sites,
> > current implementation of the driver however hardcodes a single site
> > ID. Change the code so it would be possible to reference multiple
> > sites as indivudial sensors and get their temperature readings.
>
> small typo: indivudial/individual.
>
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@...il.com>
> > Cc: Chris Healy <cphealy@...il.com>
> > Cc: Lucas Stach <l.stach@...gutronix.de>
> > Cc: Zhang Rui <rui.zhang@...el.com>
> > Cc: Eduardo Valentin <edubezval@...il.com>
> > Cc: Daniel Lezcano <daniel.lezcano@...aro.org>
> > Cc: linux-imx@....com
> > Cc: linux-pm@...r.kernel.org
> > Cc: linux-kernel@...r.kernel.org
> > ---
> > drivers/thermal/qoriq_thermal.c | 132 ++++++++++++++++++++------------
> > 1 file changed, 84 insertions(+), 48 deletions(-)
> >
> > diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> > index f746c62789b0..6cc6e6b36fb0 100644
> > --- a/drivers/thermal/qoriq_thermal.c
> > +++ b/drivers/thermal/qoriq_thermal.c
> > @@ -24,6 +24,8 @@
> > #define TMR_DISABLE 0x0
> > #define TMR_ME 0x80000000
> > #define TMR_ALPF 0x0c000000
> > +#define TMR_ALPF_MASK GENMASK(27, 26)
> > +
> >
> > #define REGS_TMTMIR 0x008 /* Temperature measurement interval Register */
> > #define TMTMIR_DEFAULT 0x0000000f
> > @@ -41,21 +43,32 @@
> > #define REGS_TTRnCR(n) (0xf10 + 4 * (n)) /* Temperature Range n
> > * Control Register
> > */
> > +struct qoriq_tmu_sensor {
> > + struct thermal_zone_device *tz;
> > + int id;
> > +};
> > +
> > /*
> > * Thermal zone data
> > */
> > struct qoriq_tmu_data {
> > struct thermal_zone_device *tz;
> > struct regmap *regmap;
> > - int sensor_id;
> > + struct qoriq_tmu_sensor sensors[SITES_MAX];
> > };
> >
> > +static struct qoriq_tmu_data *qoriq_sensor_to_data(struct qoriq_tmu_sensor *s)
> > +{
> > + return container_of(s, struct qoriq_tmu_data, sensors[s->id]);
> > +}
> > +
> > static int tmu_get_temp(void *p, int *temp)
> > {
> > u32 val;
> > - struct qoriq_tmu_data *data = p;
> > + struct qoriq_tmu_sensor *s = p;
> > + struct qoriq_tmu_data *data = qoriq_sensor_to_data(s);
> >
> > - regmap_read(data->regmap, REGS_TRITSR(data->sensor_id), &val);
> > + regmap_read(data->regmap, REGS_TRITSR(s->id), &val);
> > if (!(val & TRITSR_V))
> > return -ENODATA;
> >
> > @@ -63,38 +76,86 @@ static int tmu_get_temp(void *p, int *temp)
> > return 0;
> > }
> >
> > -static int qoriq_tmu_get_sensor_id(void)
> > +static const struct thermal_zone_of_device_ops tmu_tz_ops = {
> > + .get_temp = tmu_get_temp,
> > +};
> > +
> > +static int qoriq_tmu_populate_sensors(struct device *dev,
> > + struct qoriq_tmu_data *data)
> > {
> > - int ret, id;
> > + int ret, id, i, count = 0;
> > struct of_phandle_args sensor_specs;
> > struct device_node *np, *sensor_np;
> > + struct qoriq_tmu_sensor *s;
> >
> > np = of_find_node_by_name(NULL, "thermal-zones");
> > if (!np)
> > return -ENODEV;
> >
> > - sensor_np = of_get_next_child(np, NULL);
> > - ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
> > - "#thermal-sensor-cells",
> > - 0, &sensor_specs);
> > - if (ret) {
> > - id = ret;
> > - goto out;
> > + for_each_child_of_node(np, sensor_np) {
> > + u16 msite;
> > +
> > + ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
> > + "#thermal-sensor-cells",
> > + 0, &sensor_specs);
> > + if (ret)
> > + break;
> > +
> > + if (sensor_specs.args_count != 1) {
> > + pr_err("%pOFn: Invalid number of cells in sensor specifier %d\n",
> > + sensor_specs.np, sensor_specs.args_count);
> > + ret = -EINVAL;
> > + break;
> > + }
> > +
> > + id = sensor_specs.args[0];
> > + if (id >= SITES_MAX) {
> > + ret = -EINVAL;
> > + dev_err(dev, "Sensor id %d is out of range\n", id);
> > + break;
> > + }
> > +
> > + msite = BIT(15 - id);
> > + /* Enable monitoring of that particular sensor*/
> > + regmap_update_bits(data->regmap, REGS_TMR, msite, msite);
> > +
> > + s = &data->sensors[id];
> > + s->id = id;
> > + count++;
>
> Not sure I follow why you need to bother about thermal-sensor-cells..
>
devm_thermal_zone_of_sensor_register() will call
.get_temp()/tmu_get_temp(), so all present sensors need to be enabled
before that to avoid returning bogus data. The first loop going over
thermal-sensor-cells does exactly that as well as count # of sensors
present to avoid having to loop over all 16 possible sensors in the
code that follow.
Given how there's already a patch adding multiple sensors support, not
a lot of this patch will probably survive, so we can pretty much
disregard it.
Thanks,
Andrey Smirnov
Powered by blists - more mailing lists