From: Nathan Chancellor <nathan@kernel.org>
To: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: rafael@kernel.org, linux-kernel@vger.kernel.org,
linux-pm@vger.kernel.org, rui.zhang@intel.com,
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
Amit Kucheria <amitk@kernel.org>,
Antoine Tenart <atenart@kernel.org>
Subject: Re: [PATCH v5 29/30] thermal/intel/int340x: Replace parameter to simplify
Date: Mon, 26 Sep 2022 10:05:43 -0700 [thread overview]
Message-ID: <YzHb50/4njPa1TR1@dev-arch.thelio-3990X> (raw)
In-Reply-To: <20220926140604.4173723-30-daniel.lezcano@linaro.org>
Hi Daniel,
On Mon, Sep 26, 2022 at 04:06:03PM +0200, Daniel Lezcano wrote:
> In the process of replacing the get_trip_* ops by the generic trip
> points, the current code has an 'override' property to add another
> indirection to a different ops.
>
> Rework this approach to prevent this indirection and make the code
> ready for the generic trip points conversion.
>
> Actually the get_temp() is different regarding the platform, so it is
> pointless to add a new set of ops but just create dynamically the ops
> at init time.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> .../int340x_thermal/int340x_thermal_zone.c | 31 ++++++++-----------
> .../int340x_thermal/int340x_thermal_zone.h | 4 +--
> .../processor_thermal_device.c | 10 ++----
> 3 files changed, 18 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> index 62c0aa5d0783..10731b9a140a 100644
> --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> @@ -18,9 +18,6 @@ static int int340x_thermal_get_zone_temp(struct thermal_zone_device *zone,
> unsigned long long tmp;
> acpi_status status;
>
> - if (d->override_ops && d->override_ops->get_temp)
> - return d->override_ops->get_temp(zone, temp);
> -
> status = acpi_evaluate_integer(d->adev->handle, "_TMP", NULL, &tmp);
> if (ACPI_FAILURE(status))
> return -EIO;
> @@ -46,9 +43,6 @@ static int int340x_thermal_get_trip_temp(struct thermal_zone_device *zone,
> struct int34x_thermal_zone *d = zone->devdata;
> int i;
>
> - if (d->override_ops && d->override_ops->get_trip_temp)
> - return d->override_ops->get_trip_temp(zone, trip, temp);
> -
> if (trip < d->aux_trip_nr)
> *temp = d->aux_trips[trip];
> else if (trip == d->crt_trip_id)
> @@ -79,9 +73,6 @@ static int int340x_thermal_get_trip_type(struct thermal_zone_device *zone,
> struct int34x_thermal_zone *d = zone->devdata;
> int i;
>
> - if (d->override_ops && d->override_ops->get_trip_type)
> - return d->override_ops->get_trip_type(zone, trip, type);
> -
> if (trip < d->aux_trip_nr)
> *type = THERMAL_TRIP_PASSIVE;
> else if (trip == d->crt_trip_id)
> @@ -112,9 +103,6 @@ static int int340x_thermal_set_trip_temp(struct thermal_zone_device *zone,
> acpi_status status;
> char name[10];
>
> - if (d->override_ops && d->override_ops->set_trip_temp)
> - return d->override_ops->set_trip_temp(zone, trip, temp);
> -
> snprintf(name, sizeof(name), "PAT%d", trip);
> status = acpi_execute_simple_method(d->adev->handle, name,
> millicelsius_to_deci_kelvin(temp));
> @@ -134,9 +122,6 @@ static int int340x_thermal_get_trip_hyst(struct thermal_zone_device *zone,
> acpi_status status;
> unsigned long long hyst;
>
> - if (d->override_ops && d->override_ops->get_trip_hyst)
> - return d->override_ops->get_trip_hyst(zone, trip, temp);
> -
> status = acpi_evaluate_integer(d->adev->handle, "GTSH", NULL, &hyst);
> if (ACPI_FAILURE(status))
> *temp = 0;
> @@ -217,7 +202,7 @@ static struct thermal_zone_params int340x_thermal_params = {
> };
>
> struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
> - struct thermal_zone_device_ops *override_ops)
> + int (*get_temp) (struct thermal_zone_device *, int *))
> {
> struct int34x_thermal_zone *int34x_thermal_zone;
> acpi_status status;
> @@ -231,8 +216,15 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
> return ERR_PTR(-ENOMEM);
>
> int34x_thermal_zone->adev = adev;
> - int34x_thermal_zone->override_ops = override_ops;
>
> + int34x_thermal_zone->ops = kmemdup(&int340x_thermal_zone_ops,
> + sizeof(int340x_thermal_zone_ops), GFP_KERNEL);
> + if (!int34x_thermal_zone->ops)
> + goto err_ops_alloc;
I assume I was cc'd on this change due to my fix up patch:
https://lore.kernel.org/20220923152009.1721739-1-nathan@kernel.org/
However, I don't see that applied here. I don't mind it being squashed
in to keep the build as clean as possible through bisects.
Additionally, I did a diff of v4 to v5 to apply on top of next-20220923
and I noticed there were a significant number of places where there was
whitespace added where it probably should not have been. Other than
that, v5 works on all the machines that had issues so thank you for the
quick fix!
Cheers,
Nathan
> + if (get_temp)
> + int34x_thermal_zone->ops->get_temp = get_temp;
> +
> status = acpi_evaluate_integer(adev->handle, "PATC", NULL, &trip_cnt);
> if (ACPI_FAILURE(status))
> trip_cnt = 0;
> @@ -262,7 +254,7 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
> acpi_device_bid(adev),
> trip_cnt,
> trip_mask, int34x_thermal_zone,
> - &int340x_thermal_zone_ops,
> + int34x_thermal_zone->ops,
> &int340x_thermal_params,
> 0, 0);
> if (IS_ERR(int34x_thermal_zone->zone)) {
> @@ -281,6 +273,8 @@ struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
> acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
> kfree(int34x_thermal_zone->aux_trips);
> err_trip_alloc:
> + kfree(int34x_thermal_zone->ops);
> +err_ops_alloc:
> kfree(int34x_thermal_zone);
> return ERR_PTR(ret);
> }
> @@ -292,6 +286,7 @@ void int340x_thermal_zone_remove(struct int34x_thermal_zone
> thermal_zone_device_unregister(int34x_thermal_zone->zone);
> acpi_lpat_free_conversion_table(int34x_thermal_zone->lpat_table);
> kfree(int34x_thermal_zone->aux_trips);
> + kfree(int34x_thermal_zone->ops);
> kfree(int34x_thermal_zone);
> }
> EXPORT_SYMBOL_GPL(int340x_thermal_zone_remove);
> diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> index 3b4971df1b33..e28ab1ba5e06 100644
> --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> @@ -29,13 +29,13 @@ struct int34x_thermal_zone {
> int hot_temp;
> int hot_trip_id;
> struct thermal_zone_device *zone;
> - struct thermal_zone_device_ops *override_ops;
> + struct thermal_zone_device_ops *ops;
> void *priv_data;
> struct acpi_lpat_conversion_table *lpat_table;
> };
>
> struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *,
> - struct thermal_zone_device_ops *override_ops);
> + int (*get_temp) (struct thermal_zone_device *, int *));
> void int340x_thermal_zone_remove(struct int34x_thermal_zone *);
> int int340x_thermal_read_trips(struct int34x_thermal_zone *int34x_zone);
>
> diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> index a8d98f1bd6c6..317703027ce9 100644
> --- a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> +++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> @@ -207,10 +207,6 @@ static int proc_thermal_get_zone_temp(struct thermal_zone_device *zone,
> return ret;
> }
>
> -static struct thermal_zone_device_ops proc_thermal_local_ops = {
> - .get_temp = proc_thermal_get_zone_temp,
> -};
> -
> static int proc_thermal_read_ppcc(struct proc_thermal_device *proc_priv)
> {
> int i;
> @@ -285,7 +281,7 @@ int proc_thermal_add(struct device *dev, struct proc_thermal_device *proc_priv)
> struct acpi_device *adev;
> acpi_status status;
> unsigned long long tmp;
> - struct thermal_zone_device_ops *ops = NULL;
> + int (*get_temp) (struct thermal_zone_device *, int *) = NULL;
> int ret;
>
> adev = ACPI_COMPANION(dev);
> @@ -304,10 +300,10 @@ int proc_thermal_add(struct device *dev, struct proc_thermal_device *proc_priv)
> /* there is no _TMP method, add local method */
> stored_tjmax = get_tjmax();
> if (stored_tjmax > 0)
> - ops = &proc_thermal_local_ops;
> + get_temp = proc_thermal_get_zone_temp;
> }
>
> - proc_priv->int340x_zone = int340x_thermal_zone_add(adev, ops);
> + proc_priv->int340x_zone = int340x_thermal_zone_add(adev, get_temp);
> if (IS_ERR(proc_priv->int340x_zone)) {
> return PTR_ERR(proc_priv->int340x_zone);
> } else
> --
> 2.34.1
>
next prev parent reply other threads:[~2022-09-26 17:40 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-26 14:05 [PATCH v5 00/30] Rework the trip points creation Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 01/30] thermal/core: Add a generic thermal_zone_get_trip() function Daniel Lezcano
2022-09-26 19:01 ` Rafael J. Wysocki
2022-09-26 14:05 ` [PATCH v5 02/30] thermal/sysfs: Do not make get_trip_hyst optional Daniel Lezcano
2022-09-26 19:16 ` Rafael J. Wysocki
2022-09-26 14:05 ` [PATCH v5 03/30] thermal/core: Add a generic thermal_zone_set_trip() function Daniel Lezcano
2022-09-26 19:25 ` Rafael J. Wysocki
2022-09-26 22:11 ` Daniel Lezcano
2022-09-27 10:38 ` Rafael J. Wysocki
2022-09-27 12:20 ` Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 04/30] thermal/core: Add a generic thermal_zone_get_crit_temp() function Daniel Lezcano
2022-09-26 19:27 ` Rafael J. Wysocki
2022-09-26 14:05 ` [PATCH v5 05/30] thermal/core/governors: Use thermal_zone_get_trip() instead of ops functions Daniel Lezcano
2022-09-26 19:34 ` Rafael J. Wysocki
2022-09-26 22:15 ` Daniel Lezcano
2022-09-27 11:08 ` Rafael J. Wysocki
2022-09-26 14:05 ` [PATCH v5 06/30] thermal/of: Use generic thermal_zone_get_trip() function Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 07/30] thermal/of: Remove unused functions Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 08/30] thermal/drivers/exynos: Use generic thermal_zone_get_trip() function Daniel Lezcano
2022-09-26 14:05 ` Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 09/30] thermal/drivers/exynos: of_thermal_get_ntrips() Daniel Lezcano
2022-09-26 14:05 ` Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 10/30] thermal/drivers/exynos: Replace of_thermal_is_trip_valid() by thermal_zone_get_trip() Daniel Lezcano
2022-09-26 14:05 ` Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 11/30] thermal/drivers/tegra: Use generic thermal_zone_get_trip() function Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 12/30] thermal/drivers/uniphier: " Daniel Lezcano
2022-09-26 14:05 ` Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 13/30] thermal/drivers/hisi: " Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 14/30] thermal/drivers/qcom: " Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 15/30] thermal/drivers/armada: " Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 16/30] thermal/drivers/rcar_gen3: Use the generic function to get the number of trips Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 17/30] thermal/of: Remove of_thermal_get_ntrips() Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 18/30] thermal/of: Remove of_thermal_is_trip_valid() Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 19/30] thermal/of: Remove of_thermal_set_trip_hyst() Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 20/30] thermal/of: Remove of_thermal_get_crit_temp() Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 21/30] thermal/drivers/st: Use generic trip points Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 22/30] thermal/drivers/imx: Use generic thermal_zone_get_trip() function Daniel Lezcano
2022-09-26 14:05 ` Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 23/30] thermal/drivers/rcar: " Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 24/30] thermal/drivers/broadcom: " Daniel Lezcano
2022-09-26 14:05 ` Daniel Lezcano
2022-09-26 14:05 ` [PATCH v5 25/30] thermal/drivers/da9062: " Daniel Lezcano
2022-09-26 14:06 ` [PATCH v5 26/30] thermal/drivers/ti: Remove unused macros ti_thermal_get_trip_value() / ti_thermal_trip_is_valid() Daniel Lezcano
2022-09-26 14:06 ` [PATCH v5 27/30] thermal/drivers/acerhdf: Use generic thermal_zone_get_trip() function Daniel Lezcano
2022-09-26 14:06 ` [PATCH v5 28/30] thermal/drivers/cxgb4: " Daniel Lezcano
2022-09-26 14:06 ` [PATCH v5 29/30] thermal/intel/int340x: Replace parameter to simplify Daniel Lezcano
2022-09-26 17:05 ` Nathan Chancellor [this message]
2022-09-26 21:33 ` Daniel Lezcano
2022-09-26 14:06 ` [PATCH v5 30/30] thermal/drivers/intel: Use generic thermal_zone_get_trip() function Daniel Lezcano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YzHb50/4njPa1TR1@dev-arch.thelio-3990X \
--to=nathan@kernel.org \
--cc=amitk@kernel.org \
--cc=atenart@kernel.org \
--cc=daniel.lezcano@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rafael@kernel.org \
--cc=rui.zhang@intel.com \
--cc=srinivas.pandruvada@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.