RE: [PATCH RESEND V13 2/5] thermal: of-thermal: add API for getting sensor ID from DT

From: Anson Huang
Date: Sun Jun 09 2019 - 22:38:55 EST

Next message: CK Hu: "Re: [PATCH v4 01/14] dt-bindings: Add binding for MT2712 MIPI-CSI2"
Previous message: Chunyan Zhang: "Re: [BACKPORT 4.4.y] PM / sleep: prohibit devices probing during suspend/hibernation"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi, Eduardo

> -----Original Message-----
> From: Anson Huang
> Sent: Wednesday, May 29, 2019 11:37 AM
> To: Eduardo Valentin <edubezval@xxxxxxxxx>
> Cc: robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx; shawnguo@xxxxxxxxxx;
> s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx;
> catalin.marinas@xxxxxxx; will.deacon@xxxxxxx; rui.zhang@xxxxxxxxx;
> daniel.lezcano@xxxxxxxxxx; Aisheng Dong <aisheng.dong@xxxxxxx>;
> ulf.hansson@xxxxxxxxxx; Peng Fan <peng.fan@xxxxxxx>; Daniel Baluta
> <daniel.baluta@xxxxxxx>; maxime.ripard@xxxxxxxxxxx; olof@xxxxxxxxx;
> jagan@xxxxxxxxxxxxxxxxxxxx; horms+renesas@xxxxxxxxxxxx; Leonard
> Crestez <leonard.crestez@xxxxxxx>; bjorn.andersson@xxxxxxxxxx;
> dinguyen@xxxxxxxxxx; enric.balletbo@xxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; linux-pm@xxxxxxxxxxxxxxx; dl-linux-imx <linux-
> imx@xxxxxxx>
> Subject: RE: [PATCH RESEND V13 2/5] thermal: of-thermal: add API for getting
> sensor ID from DT
>
> Hi, Eduardo
>
> > -----Original Message-----
> > From: Eduardo Valentin <edubezval@xxxxxxxxx>
> > Sent: Wednesday, May 29, 2019 11:02 AM
> > To: Anson Huang <anson.huang@xxxxxxx>
> > Cc: robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx; shawnguo@xxxxxxxxxx;
> > s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx;
> > catalin.marinas@xxxxxxx; will.deacon@xxxxxxx; rui.zhang@xxxxxxxxx;
> > daniel.lezcano@xxxxxxxxxx; Aisheng Dong <aisheng.dong@xxxxxxx>;
> > ulf.hansson@xxxxxxxxxx; Peng Fan <peng.fan@xxxxxxx>; Daniel Baluta
> > <daniel.baluta@xxxxxxx>; maxime.ripard@xxxxxxxxxxx; olof@xxxxxxxxx;
> > jagan@xxxxxxxxxxxxxxxxxxxx; horms+renesas@xxxxxxxxxxxx; Leonard
> > Crestez <leonard.crestez@xxxxxxx>; bjorn.andersson@xxxxxxxxxx;
> > dinguyen@xxxxxxxxxx; enric.balletbo@xxxxxxxxxxxxx;
> > devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-arm-
> > kernel@xxxxxxxxxxxxxxxxxxx; linux-pm@xxxxxxxxxxxxxxx; dl-linux-imx
> > <linux- imx@xxxxxxx>
> > Subject: Re: [PATCH RESEND V13 2/5] thermal: of-thermal: add API for
> > getting sensor ID from DT
> >
> > On Tue, May 28, 2019 at 02:06:18PM +0800, Anson.Huang@xxxxxxx wrote:
> > > From: Anson Huang <Anson.Huang@xxxxxxx>
> > >
> > > On some platforms like i.MX8QXP, the thermal driver needs a real HW
> > > sensor ID from DT thermal zone, the HW sensor ID is used to get
> > > temperature from SCU firmware, and the virtual sensor ID starting
> > > from
> > > 0 to N is NOT used at all, this patch adds new API
> > > thermal_zone_of_get_sensor_id() to provide the feature of getting
> > > sensor ID from DT thermal zone's node.
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@xxxxxxx>
> > > ---
> > > Changes since V12:
> > > - adjust the second parameter of thermal_zone_of_get_sensor_id()
> > > API,
> > then caller no need
> > > to pass the of_phandle_args structure and put the sensor_specs.np
> > manually, also putting
> > > the sensor node device check inside this API to make it easy for
> > > usage;
> >
> > What happened to using nxp,resource-id property in your driver?
> > Why do we need this as an API in of-thermal? What other drivers may
> > benefit of this?
> >
> > Regardless, this patch needs to document the new API under
> > Documentation/
>
> As Rob has different opinion about this property, he thought it is
> unnecessary, see below discussion mail, that is why I need to add API to get
> the resource ID from phandle argument.
> I am totally confused now, which approach should we adopt?
>
> https://patchwork.kernel.org/patch/10831397/

I will add the new API document in V14, I remembered that there is also other vendors
have similar sensor HW ID as i.MX8QXP, instead of adding private properties for each vendor,
adding an API to read out the sensor ID can benefit us a lot I think.

Thanks,
Anson

>
> Thanks,
> Anson
>
> >
> > > ---
> > > drivers/thermal/of-thermal.c | 66
> > > +++++++++++++++++++++++++++++++++---
> > --------
> > > include/linux/thermal.h | 10 +++++++
> > > 2 files changed, 60 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/thermal/of-thermal.c
> > > b/drivers/thermal/of-thermal.c index dc5093b..a53792b 100644
> > > --- a/drivers/thermal/of-thermal.c
> > > +++ b/drivers/thermal/of-thermal.c
> > > @@ -449,6 +449,54 @@ thermal_zone_of_add_sensor(struct
> device_node
> > > *zone, }
> > >
> > > /**
> > > + * thermal_zone_of_get_sensor_id - get sensor ID from a DT thermal
> > > + zone
> > > + * @tz_np: a valid thermal zone device node.
> > > + * @sensor_np: a sensor node of a valid sensor device.
> > > + * @id: a sensor ID pointer will be passed back.
> > > + *
> > > + * This function will get sensor ID from a given thermal zone node,
> > > + use
> > > + * "thermal-sensors" as list name, and get sensor ID from first
> > > + phandle's
> > > + * argument.
> > > + *
> > > + * Return: 0 on success, proper error code otherwise.
> > > + */
> > > +
> > > +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > > + struct device_node *sensor_np,
> > > + u32 *id)
> > > +{
> > > + struct of_phandle_args sensor_specs;
> > > + int ret;
> > > +
> > > + ret = of_parse_phandle_with_args(tz_np,
> > > + "thermal-sensors",
> > > + "#thermal-sensor-cells",
> > > + 0,
> > > + &sensor_specs);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + if (sensor_specs.np != sensor_np) {
> > > + of_node_put(sensor_specs.np);
> > > + return -ENODEV;
> > > + }
> > > +
> > > + if (sensor_specs.args_count >= 1) {
> > > + *id = sensor_specs.args[0];
> > > + WARN(sensor_specs.args_count > 1,
> > > + "%pOFn: too many cells in sensor specifier %d\n",
> > > + sensor_specs.np, sensor_specs.args_count);
> > > + } else {
> > > + *id = 0;
> > > + }
> > > +
> > > + of_node_put(sensor_specs.np);
> > > +
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(thermal_zone_of_get_sensor_id);
> > > +
> > > +/**
> > > * thermal_zone_of_sensor_register - registers a sensor to a DT thermal
> zone
> > > * @dev: a valid struct device pointer of a sensor device. Must contain
> > > * a valid .of_node, for the sensor node.
> > > @@ -499,36 +547,22 @@ thermal_zone_of_sensor_register(struct device
> > *dev, int sensor_id, void *data,
> > > sensor_np = of_node_get(dev->of_node);
> > >
> > > for_each_available_child_of_node(np, child) {
> > > - struct of_phandle_args sensor_specs;
> > > int ret, id;
> > >
> > > /* For now, thermal framework supports only 1 sensor per
> > zone */
> > > - ret = of_parse_phandle_with_args(child, "thermal-sensors",
> > > - "#thermal-sensor-cells",
> > > - 0, &sensor_specs);
> > > + ret = thermal_zone_of_get_sensor_id(child, sensor_np, &id);
> > > if (ret)
> > > continue;
> > >
> > > - if (sensor_specs.args_count >= 1) {
> > > - id = sensor_specs.args[0];
> > > - WARN(sensor_specs.args_count > 1,
> > > - "%pOFn: too many cells in sensor specifier %d\n",
> > > - sensor_specs.np, sensor_specs.args_count);
> > > - } else {
> > > - id = 0;
> > > - }
> > > -
> > > - if (sensor_specs.np == sensor_np && id == sensor_id) {
> > > + if (id == sensor_id) {
> > > tzd = thermal_zone_of_add_sensor(child, sensor_np,
> > > data, ops);
> > > if (!IS_ERR(tzd))
> > > tzd->ops->set_mode(tzd,
> > THERMAL_DEVICE_ENABLED);
> > >
> > > - of_node_put(sensor_specs.np);
> > > of_node_put(child);
> > > goto exit;
> > > }
> > > - of_node_put(sensor_specs.np);
> > > }
> > > exit:
> > > of_node_put(sensor_np);
> > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h index
> > > 15a4ca5..5edffe6 100644
> > > --- a/include/linux/thermal.h
> > > +++ b/include/linux/thermal.h
> > > @@ -375,6 +375,9 @@ struct thermal_trip {
> > >
> > > /* Function declarations */
> > > #ifdef CONFIG_THERMAL_OF
> > > +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > > + struct device_node *sensor_np,
> > > + u32 *id);
> > > struct thermal_zone_device *
> > > thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> > > const struct thermal_zone_of_device_ops
> > *ops); @@ -386,6 +389,13
> > > @@ struct thermal_zone_device
> *devm_thermal_zone_of_sensor_register(
> > > void devm_thermal_zone_of_sensor_unregister(struct device *dev,
> > > struct thermal_zone_device *tz);
> > #else
> > > +
> > > +static int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > > + struct device_node *sensor_np,
> > > + u32 *id)
> > > +{
> > > + return -ENOENT;
> > > +}
> > > static inline struct thermal_zone_device *
> > > thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> > > const struct thermal_zone_of_device_ops
> > *ops)

Next message: CK Hu: "Re: [PATCH v4 01/14] dt-bindings: Add binding for MT2712 MIPI-CSI2"
Previous message: Chunyan Zhang: "Re: [BACKPORT 4.4.y] PM / sleep: prohibit devices probing during suspend/hibernation"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]