Messages in this thread | | | Date | Tue, 23 Apr 2019 08:44:31 -0700 | From | Eduardo Valentin <> | Subject | Re: [PATCH] thermal/drivers/of: Add a get_temp_id callback function |
| |
Hello,
On Tue, Apr 16, 2019 at 07:22:03PM +0200, Daniel Lezcano wrote: > Currently when we register a sensor, we specify the sensor id and a data > pointer to be passed when the get_temp function is called. However the > sensor_id is not passed to the get_temp callback forcing the driver to > do extra allocation and adding back pointer to find out from the sensor > information the driver data and then back to the sensor id. > > Add a new callback get_temp_id() which will be called if set. It will > call the get_temp_id() with the sensor id. > > That will be more consistent with the registering function.
I still do not understand why we need to have a get_id callback. The use cases I have seen so far, which I have been intentionally rejecting, are mainly solvable by creating other compatible entries. And really, if you have, say a bandgap, chip that supports multiple sensors, but on SoC version A it has 5 sensors, and on SoC version B it has only 4, or on SoC version C, it has 5 but they are either logially located in different places (gpu vs iva regions), these are all cases in which you want a different compatible!
Do you mind sharing why you need a get sensor id callback?
> > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > Tested-by: Andrey Smirnov <andrew.smirnov@gmail.com> > --- > drivers/thermal/of-thermal.c | 19 +++++++++++++------ > include/linux/thermal.h | 1 + > 2 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c > index 2df059cc07e2..787d1cbe13f3 100644 > --- a/drivers/thermal/of-thermal.c > +++ b/drivers/thermal/of-thermal.c > @@ -78,6 +78,8 @@ struct __thermal_zone { > > /* sensor interface */ > void *sensor_data; > + int sensor_id; > + > const struct thermal_zone_of_device_ops *ops; > }; > > @@ -88,10 +90,14 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz, > { > struct __thermal_zone *data = tz->devdata; > > - if (!data->ops->get_temp) > - return -EINVAL; > + if (data->ops->get_temp) > + return data->ops->get_temp(data->sensor_data, temp); > > - return data->ops->get_temp(data->sensor_data, temp); > + if (data->ops->get_temp_id) > + return data->ops->get_temp_id(data->sensor_id, > + data->sensor_data, temp); > + > + return -EINVAL; > } > > static int of_thermal_set_trips(struct thermal_zone_device *tz, > @@ -407,8 +413,8 @@ static struct thermal_zone_device_ops of_thermal_ops = { > /*** sensor API ***/ > > static struct thermal_zone_device * > -thermal_zone_of_add_sensor(struct device_node *zone, > - struct device_node *sensor, void *data, > +thermal_zone_of_add_sensor(struct device_node *zone, struct device_node *sensor, > + int sensor_id, void *data, > const struct thermal_zone_of_device_ops *ops) > { > struct thermal_zone_device *tzd; > @@ -426,6 +432,7 @@ thermal_zone_of_add_sensor(struct device_node *zone, > mutex_lock(&tzd->lock); > tz->ops = ops; > tz->sensor_data = data; > + tz->sensor_id = sensor_id; > > tzd->ops->get_temp = of_thermal_get_temp; > tzd->ops->get_trend = of_thermal_get_trend; > @@ -516,7 +523,7 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data, > } > > if (sensor_specs.np == sensor_np && id == sensor_id) { > - tzd = thermal_zone_of_add_sensor(child, sensor_np, > + tzd = thermal_zone_of_add_sensor(child, sensor_np, sensor_id, > data, ops); > if (!IS_ERR(tzd)) > tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED); > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index 5f4705f46c2f..2762d7e6dd86 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -351,6 +351,7 @@ struct thermal_genl_event { > * hardware. > */ > struct thermal_zone_of_device_ops { > + int (*get_temp_id)(int, void *, int *); > int (*get_temp)(void *, int *); > int (*get_trend)(void *, int, enum thermal_trend *); > int (*set_trips)(void *, int, int); > -- > 2.17.1 >
| |