Re: [PATCH v2 06/16] thermal: Don't use 'device' internal thermal zone structure field

Rafael J. Wysocki
2023-02-22T19:44:05Z

On Tue, Feb 21, 2023 at 7:07 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> Some drivers are directly using the thermal zone's 'device' structure
> field.
>
> Use the driver device pointer instead of the thermal zone device when
> it is available.
>
> Remove the traces when they are duplicate with the traces in the core
> code.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Reviewed-by: Balsam CHIHI <bchihi@baylibre.com> #Mediatek LVTS
> ---
> drivers/thermal/imx_thermal.c | 37 ++++---------------
> drivers/thermal/mediatek/lvts_thermal.c | 4 +-
> drivers/thermal/thermal_hwmon.c | 4 +-
> .../ti-soc-thermal/ti-thermal-common.c | 2 +-
> 4 files changed, 13 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index a0b8de269986..c3136978adee 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -285,13 +285,13 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
> if (data->alarm_temp == trips[IMX_TRIP_CRITICAL].temperature &&
> *temp < trips[IMX_TRIP_PASSIVE].temperature) {
> imx_set_alarm_temp(data, trips[IMX_TRIP_PASSIVE].temperature);
> - dev_dbg(&tz->device, "thermal alarm off: T < %d\n",
> + dev_dbg(data->dev, "thermal alarm off: T < %d\n",
> data->alarm_temp / 1000);
> }
> }
>
> if (*temp != data->last_temp) {
> - dev_dbg(&tz->device, "millicelsius: %d\n", *temp);
> + dev_dbg(data->dev, "millicelsius: %d\n", *temp);
> data->last_temp = *temp;
> }
>
> @@ -367,36 +367,16 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
> static int imx_bind(struct thermal_zone_device *tz,
> struct thermal_cooling_device *cdev)
> {
> - int ret;
> -
> - ret = thermal_zone_bind_cooling_device(tz, IMX_TRIP_PASSIVE, cdev,
> - THERMAL_NO_LIMIT,
> - THERMAL_NO_LIMIT,
> - THERMAL_WEIGHT_DEFAULT);
> - if (ret) {
> - dev_err(&tz->device,
> - "binding zone %s with cdev %s failed:%d\n",
> - tz->type, cdev->type, ret);
> - return ret;
> - }
> -
> - return 0;
> + return thermal_zone_bind_cooling_device(tz, IMX_TRIP_PASSIVE, cdev,
> + THERMAL_NO_LIMIT,
> + THERMAL_NO_LIMIT,
> + THERMAL_WEIGHT_DEFAULT);
> }
>
> static int imx_unbind(struct thermal_zone_device *tz,
> struct thermal_cooling_device *cdev)
> {
> - int ret;
> -
> - ret = thermal_zone_unbind_cooling_device(tz, IMX_TRIP_PASSIVE, cdev);
> - if (ret) {
> - dev_err(&tz->device,
> - "unbinding zone %s with cdev %s failed:%d\n",
> - tz->type, cdev->type, ret);
> - return ret;
> - }
> -
> - return 0;
> + return thermal_zone_unbind_cooling_device(tz, IMX_TRIP_PASSIVE, cdev);
> }
>
> static struct thermal_zone_device_ops imx_tz_ops = {
> @@ -558,8 +538,7 @@ static irqreturn_t imx_thermal_alarm_irq_thread(int irq, void *dev)
> {
> struct imx_thermal_data *data = dev;
>
> - dev_dbg(&data->tz->device, "THERMAL ALARM: T > %d\n",
> - data->alarm_temp / 1000);
> + dev_dbg(data->dev, "THERMAL ALARM: T > %d\n", data->alarm_temp / 1000);
>
> thermal_zone_device_update(data->tz, THERMAL_EVENT_UNSPECIFIED);
>
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index fb4b1b4db245..beb835d644e2 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -305,7 +305,7 @@ static int lvts_set_trips(struct thermal_zone_device *tz, int low, int high)
> * 14-0 : Raw temperature for threshold
> */
> if (low != -INT_MAX) {
> - dev_dbg(&tz->device, "Setting low limit temperature interrupt: %d\n", low);
> + pr_debug("%s: Setting low limit temperature interrupt: %d\n", tz->type, low);
> writel(raw_low, LVTS_H2NTHRE(base));
> }
>
> @@ -318,7 +318,7 @@ static int lvts_set_trips(struct thermal_zone_device *tz, int low, int high)
> *
> * 14-0 : Raw temperature for threshold
> */
> - dev_dbg(&tz->device, "Setting high limit temperature interrupt: %d\n", high);
> + pr_debug("%s: Setting high limit temperature interrupt: %d\n", tz->type, high);
> writel(raw_high, LVTS_HTHRE(base));
>
> return 0;
> diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
> index 964db7941e31..bc02095b314c 100644
> --- a/drivers/thermal/thermal_hwmon.c
> +++ b/drivers/thermal/thermal_hwmon.c
> @@ -228,14 +228,14 @@ void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
> hwmon = thermal_hwmon_lookup_by_type(tz);
> if (unlikely(!hwmon)) {
> /* Should never happen... */
> - dev_dbg(&tz->device, "hwmon device lookup failed!\n");
> + dev_dbg(hwmon->device, "hwmon device lookup failed!\n");
> return;
> }
>
> temp = thermal_hwmon_lookup_temp(hwmon, tz);
> if (unlikely(!temp)) {
> /* Should never happen... */
> - dev_dbg(&tz->device, "temperature input lookup failed!\n");
> + dev_dbg(hwmon->device, "temperature input lookup failed!\n");
> return;
> }
>
> diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
> index 216b29068b08..060f46cea5ff 100644
> --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
> +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
> @@ -43,7 +43,7 @@ static void ti_thermal_work(struct work_struct *work)
>
> thermal_zone_device_update(data->ti_thermal, THERMAL_EVENT_UNSPECIFIED);
>
> - dev_dbg(&data->ti_thermal->device, "updated thermal zone %s\n",
> + dev_dbg(data->bgp->dev, "updated thermal zone %s\n",
> data->ti_thermal->type);

The code before the change is more consistent, because it refers to
the same object in both instances.

It looks like a type field accessor is needed, eg. thermal_zone_device_type()?

Or move the debug message to thermal_zone_device_update()?

> }
>
> --
> 2.34.1
>