thermal/drivers/imx_sc: Rely on the platform data to get the resource id
authorDaniel Lezcano <daniel.lezcano@linaro.org>
Thu, 18 Aug 2022 08:23:15 +0000 (10:23 +0200)
committerDaniel Lezcano <daniel.lezcano@linaro.org>
Tue, 4 Oct 2022 09:20:59 +0000 (11:20 +0200)
Currently the imx_sc driver is reimplementing part of the thermal zone
parsing from the thermal OF tree code to get the sensor id associated
with a thermal zone sensor.

The driver platform specific code should know what sensor is present
and not rely on the thermal zone description to do a discovery. Well
that is arguable but all the other drivers have a per platform data
telling what sensor id to use.

The imx_sc thermal driver is the only one using a different
approach. Not invalid but forcing to keep a specific function
'thermal_zone_of_get_sensor_id()' to get the sensor id for a specific
thermal zone as the self-explanatory function tells and having device
tree code inside the driver.

The thermal OF code had a rework and remains now self-encapsulated
with a register/unregister functions and their 'devm' variants, except
for the function mentioned above.

After investigating, it appears the imx_sc sensor is defined in
arch/arm64/boot/dts/freescale/imx8qxp.dtsi:

which defines the cpu-thermal zone with the id: IMX_SC_R_SYSTEM

This dtsi is included by:
 - imx8qxp-ai_ml.dts
 - imx8qxp-colibri.dtsi
 - imx8qxp-mek.dts

The two first ones do not define more thermal zones
The third one adds the pmic-thermal0 zone with id: IMX_SC_R_PMIC_0

The thermal OF code returns -ENODEV if the thermal zone registration
with a specific id fails because the description is not available in
the DT for such a sensor id. In this case we continue with the other
ids without bailing out with an error.

So we can build for the 'fsl,imx-sc-thermal' a compatible data, an
array of sensor ids containing IMX_SC_R_SYSTEM and IMX_SC_R_PMIC_0.

The latter won't be found but that will not result in an error but a
normal case where we continue the initialization with other ids.

Just to clarify, it is what the thermal framework does and what the
other drivers are expecting: when a registration fails with -ENODEV
this is not an error but a case where the description is not found in
the device tree, that be can the entire thermal zones description or a
specific thermal zone with an unknown id.

There is one small functional change but without impact. When there is
no 'thermal-zones' description the probe function was returning
'-ENODEV', now it returns zero. When a thermal zone fails to register
with an error different from '-ENODEV', the error is detected and
returned.

Change the code accordingly and remove the OF code from the driver.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Link: https://lore.kernel.org/r/20220818082316.2717095-1-daniel.lezcano@linaro.org
drivers/thermal/imx_sc_thermal.c

index 10bfa6507eb4a65ba2306aecbd605d38c103b0fe..5d92b70a5d53a3f1be716f6adcb16aff3323a618 100644 (file)
@@ -76,59 +76,55 @@ static const struct thermal_zone_device_ops imx_sc_thermal_ops = {
 
 static int imx_sc_thermal_probe(struct platform_device *pdev)
 {
-       struct device_node *np, *child, *sensor_np;
        struct imx_sc_sensor *sensor;
-       int ret;
+       const int *resource_id;
+       int i, ret;
 
        ret = imx_scu_get_handle(&thermal_ipc_handle);
        if (ret)
                return ret;
 
-       np = of_find_node_by_name(NULL, "thermal-zones");
-       if (!np)
-               return -ENODEV;
+       resource_id = of_device_get_match_data(&pdev->dev);
+       if (!resource_id)
+               return -EINVAL;
 
-       sensor_np = of_node_get(pdev->dev.of_node);
+       for (i = 0; resource_id[i] > 0; i++) {
 
-       for_each_available_child_of_node(np, child) {
                sensor = devm_kzalloc(&pdev->dev, sizeof(*sensor), GFP_KERNEL);
-               if (!sensor) {
-                       of_node_put(child);
-                       ret = -ENOMEM;
-                       goto put_node;
-               }
+               if (!sensor)
+                       return -ENOMEM;
 
-               ret = thermal_zone_of_get_sensor_id(child,
-                                                   sensor_np,
-                                                   &sensor->resource_id);
-               if (ret < 0) {
-                       dev_err(&pdev->dev,
-                               "failed to get valid sensor resource id: %d\n",
-                               ret);
-                       of_node_put(child);
-                       break;
-               }
+               sensor->resource_id = resource_id[i];
 
-               sensor->tzd = devm_thermal_of_zone_register(&pdev->dev,
-                                                           sensor->resource_id,
-                                                           sensor,
-                                                           &imx_sc_thermal_ops);
+               sensor->tzd = devm_thermal_of_zone_register(&pdev->dev, sensor->resource_id,
+                                                           sensor, &imx_sc_thermal_ops);
                if (IS_ERR(sensor->tzd)) {
-                       dev_err(&pdev->dev, "failed to register thermal zone\n");
+                       /*
+                        * Save the error value before freeing the
+                        * sensor pointer, otherwise we endup with a
+                        * use-after-free error
+                        */
                        ret = PTR_ERR(sensor->tzd);
-                       of_node_put(child);
-                       break;
+
+                       devm_kfree(&pdev->dev, sensor);
+
+                       /*
+                        * The thermal framework notifies us there is
+                        * no thermal zone description for such a
+                        * sensor id
+                        */
+                       if (ret == -ENODEV)
+                               continue;
+
+                       dev_err(&pdev->dev, "failed to register thermal zone\n");
+                       return ret;
                }
 
                if (devm_thermal_add_hwmon_sysfs(sensor->tzd))
                        dev_warn(&pdev->dev, "failed to add hwmon sysfs attributes\n");
        }
 
-put_node:
-       of_node_put(sensor_np);
-       of_node_put(np);
-
-       return ret;
+       return 0;
 }
 
 static int imx_sc_thermal_remove(struct platform_device *pdev)
@@ -136,8 +132,10 @@ static int imx_sc_thermal_remove(struct platform_device *pdev)
        return 0;
 }
 
+static int imx_sc_sensors[] = { IMX_SC_R_SYSTEM, IMX_SC_R_PMIC_0, -1 };
+
 static const struct of_device_id imx_sc_thermal_table[] = {
-       { .compatible = "fsl,imx-sc-thermal", },
+       { .compatible = "fsl,imx-sc-thermal", .data =  imx_sc_sensors },
        {}
 };
 MODULE_DEVICE_TABLE(of, imx_sc_thermal_table);