Re: [PATCH 01/18] ACPI: platform_profile: Add `ops` member to handlers

Mario Limonciello
2025-01-09T16:32:46Z

On 1/9/2025 09:06, Kurt Borja wrote:
> Replace *profile_get and *profile_set members with a general *ops
> member.
>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/acpi/platform_profile.c | 6 ++---
> .../surface/surface_platform_profile.c | 8 +++++--
> drivers/platform/x86/acer-wmi.c | 11 +++++----
> drivers/platform/x86/amd/pmf/sps.c | 8 +++++--
> drivers/platform/x86/asus-wmi.c | 8 +++++--
> drivers/platform/x86/dell/alienware-wmi.c | 8 +++++--
> drivers/platform/x86/dell/dell-pc.c | 8 +++++--
> drivers/platform/x86/hp/hp-wmi.c | 24 ++++++++++++++-----
> drivers/platform/x86/ideapad-laptop.c | 8 +++++--
> .../platform/x86/inspur_platform_profile.c | 8 +++++--
> drivers/platform/x86/thinkpad_acpi.c | 8 +++++--
> include/linux/platform_profile.h | 14 +++++++----
> 12 files changed, 86 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
> index 4c4200a0b1a6..39a22e27acc5 100644
> --- a/drivers/acpi/platform_profile.c
> +++ b/drivers/acpi/platform_profile.c
> @@ -64,7 +64,7 @@ static int _store_class_profile(struct device *dev, void *data)
> if (!test_bit(*bit, handler->choices))
> return -EOPNOTSUPP;
>
> - return handler->profile_set(handler, *bit);
> + return handler->ops->profile_set(handler, *bit);
> }
>
> /**
> @@ -101,7 +101,7 @@ static int get_class_profile(struct device *dev,
>
> lockdep_assert_held(&profile_lock);
> handler = dev_get_drvdata(dev);
> - err = handler->profile_get(handler, &val);
> + err = handler->ops->profile_get(handler, &val);
> if (err) {
> pr_err("Failed to get profile for handler %s\n", handler->name);
> return err;
> @@ -465,7 +465,7 @@ int platform_profile_register(struct platform_profile_handler *pprof)
>
> /* Sanity check the profile handler */
> if (!pprof || bitmap_empty(pprof->choices, PLATFORM_PROFILE_LAST) ||
> - !pprof->profile_set || !pprof->profile_get) {
> + !pprof->ops->profile_set || !pprof->ops->profile_get) {
> pr_err("platform_profile: handler is invalid\n");
> return -EINVAL;
> }
> diff --git a/drivers/platform/surface/surface_platform_profile.c b/drivers/platform/surface/surface_platform_profile.c
> index 6c87e982bfc8..2a9716372d56 100644
> --- a/drivers/platform/surface/surface_platform_profile.c
> +++ b/drivers/platform/surface/surface_platform_profile.c
> @@ -201,6 +201,11 @@ static int ssam_platform_profile_set(struct platform_profile_handler *pprof,
> return tp;
> }
>
> +static const struct platform_profile_ops ssam_platform_profile_ops = {
> + .profile_get = ssam_platform_profile_get,
> + .profile_set = ssam_platform_profile_set,
> +};
> +
> static int surface_platform_profile_probe(struct ssam_device *sdev)
> {
> struct ssam_platform_profile_device *tpd;
> @@ -214,8 +219,7 @@ static int surface_platform_profile_probe(struct ssam_device *sdev)
>
> tpd->handler.name = "Surface Platform Profile";
> tpd->handler.dev = &sdev->dev;
> - tpd->handler.profile_get = ssam_platform_profile_get;
> - tpd->handler.profile_set = ssam_platform_profile_set;
> + tpd->handler.ops = &ssam_platform_profile_ops;
>
> tpd->has_fan = device_property_read_bool(&sdev->dev, "has_fan");
>
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index b3043d78a7b3..ac7b3ab8fd2a 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -1900,6 +1900,11 @@ acer_predator_v4_platform_profile_set(struct platform_profile_handler *pprof,
> return 0;
> }
>
> +static const struct platform_profile_ops acer_predator_v4_platform_profile_ops = {
> + .profile_get = acer_predator_v4_platform_profile_get,
> + .profile_set = acer_predator_v4_platform_profile_set,
> +};
> +
> static int acer_platform_profile_setup(struct platform_device *device)
> {
> if (quirks->predator_v4) {
> @@ -1907,10 +1912,8 @@ static int acer_platform_profile_setup(struct platform_device *device)
>
> platform_profile_handler.name = "acer-wmi";
> platform_profile_handler.dev = &device->dev;
> - platform_profile_handler.profile_get =
> - acer_predator_v4_platform_profile_get;
> - platform_profile_handler.profile_set =
> - acer_predator_v4_platform_profile_set;
> + platform_profile_handler.ops =
> + &acer_predator_v4_platform_profile_ops;
>
> set_bit(PLATFORM_PROFILE_PERFORMANCE,
> platform_profile_handler.choices);
> diff --git a/drivers/platform/x86/amd/pmf/sps.c b/drivers/platform/x86/amd/pmf/sps.c
> index bd2bd6cfc39a..7c8391ac4dbb 100644
> --- a/drivers/platform/x86/amd/pmf/sps.c
> +++ b/drivers/platform/x86/amd/pmf/sps.c
> @@ -387,6 +387,11 @@ static int amd_pmf_profile_set(struct platform_profile_handler *pprof,
> return 0;
> }
>
> +static const struct platform_profile_ops amd_pmf_profile_ops = {
> + .profile_get = amd_pmf_profile_get,
> + .profile_set = amd_pmf_profile_set,
> +};
> +
> int amd_pmf_init_sps(struct amd_pmf_dev *dev)
> {
> int err;
> @@ -407,8 +412,7 @@ int amd_pmf_init_sps(struct amd_pmf_dev *dev)
>
> dev->pprof.name = "amd-pmf";
> dev->pprof.dev = dev->dev;
> - dev->pprof.profile_get = amd_pmf_profile_get;
> - dev->pprof.profile_set = amd_pmf_profile_set;
> + dev->pprof.ops = &amd_pmf_profile_ops;
>
> /* Setup supported modes */
> set_bit(PLATFORM_PROFILE_LOW_POWER, dev->pprof.choices);
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index fdeebab96fc0..ef4129d91656 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -3852,6 +3852,11 @@ static int asus_wmi_platform_profile_set(struct platform_profile_handler *pprof,
> return throttle_thermal_policy_write(asus);
> }
>
> +static const struct platform_profile_ops asus_wmi_platform_profile_ops = {
> + .profile_get = asus_wmi_platform_profile_get,
> + .profile_set = asus_wmi_platform_profile_set,
> +};
> +
> static int platform_profile_setup(struct asus_wmi *asus)
> {
> struct device *dev = &asus->platform_device->dev;
> @@ -3878,8 +3883,7 @@ static int platform_profile_setup(struct asus_wmi *asus)
>
> asus->platform_profile_handler.name = "asus-wmi";
> asus->platform_profile_handler.dev = dev;
> - asus->platform_profile_handler.profile_get = asus_wmi_platform_profile_get;
> - asus->platform_profile_handler.profile_set = asus_wmi_platform_profile_set;
> + asus->platform_profile_handler.ops = &asus_wmi_platform_profile_ops;
>
> set_bit(PLATFORM_PROFILE_QUIET, asus->platform_profile_handler.choices);
> set_bit(PLATFORM_PROFILE_BALANCED,
> diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
> index 7b3ee2d6a23d..c824414565b6 100644
> --- a/drivers/platform/x86/dell/alienware-wmi.c
> +++ b/drivers/platform/x86/dell/alienware-wmi.c
> @@ -1108,6 +1108,11 @@ static int thermal_profile_set(struct platform_profile_handler *pprof,
> return wmax_thermal_control(supported_thermal_profiles[profile]);
> }
>
> +static const struct platform_profile_ops awcc_platform_profile_ops = {
> + .profile_get = thermal_profile_get,
> + .profile_set = thermal_profile_set,
> +};
> +
> static int create_thermal_profile(struct platform_device *platform_device)
> {
> u32 out_data;
> @@ -1154,10 +1159,9 @@ static int create_thermal_profile(struct platform_device *platform_device)
> set_bit(PLATFORM_PROFILE_PERFORMANCE, pp_handler.choices);
> }
>
> - pp_handler.profile_get = thermal_profile_get;
> - pp_handler.profile_set = thermal_profile_set;
> pp_handler.name = "alienware-wmi";
> pp_handler.dev = &platform_device->dev;
> + pp_handler.ops = &awcc_platform_profile_ops;
>
> return devm_platform_profile_register(&pp_handler);
> }
> diff --git a/drivers/platform/x86/dell/dell-pc.c b/drivers/platform/x86/dell/dell-pc.c
> index 3797a5721dbd..1751b7235031 100644
> --- a/drivers/platform/x86/dell/dell-pc.c
> +++ b/drivers/platform/x86/dell/dell-pc.c
> @@ -231,6 +231,11 @@ static int thermal_platform_profile_get(struct platform_profile_handler *pprof,
> return 0;
> }
>
> +static const struct platform_profile_ops dell_pc_platform_profile_ops = {
> + .profile_get = thermal_platform_profile_get,
> + .profile_set = thermal_platform_profile_set,
> +};
> +
> static int thermal_init(void)
> {
> int ret;
> @@ -258,8 +263,7 @@ static int thermal_init(void)
> }
> thermal_handler->name = "dell-pc";
> thermal_handler->dev = &platform_device->dev;
> - thermal_handler->profile_get = thermal_platform_profile_get;
> - thermal_handler->profile_set = thermal_platform_profile_set;
> + thermal_handler->ops = &dell_pc_platform_profile_ops;
>
> if (supported_modes & DELL_QUIET)
> set_bit(PLATFORM_PROFILE_QUIET, thermal_handler->choices);
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index 6d6e13a0c6e2..c37e44f7606c 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -1565,6 +1565,21 @@ static inline void omen_unregister_powersource_event_handler(void)
> unregister_acpi_notifier(&platform_power_source_nb);
> }
>
> +static const struct platform_profile_ops platform_profile_omen_ops = {
> + .profile_get = platform_profile_omen_get,
> + .profile_set = platform_profile_omen_set,
> +};
> +
> +static const struct platform_profile_ops platform_profile_victus_ops = {
> + .profile_get = platform_profile_victus_get,
> + .profile_set = platform_profile_victus_set,
> +};
> +
> +static const struct platform_profile_ops hp_wmi_platform_profile_ops = {
> + .profile_get = hp_wmi_platform_profile_get,
> + .profile_set = hp_wmi_platform_profile_set,
> +};
> +
> static int thermal_profile_setup(struct platform_device *device)
> {
> int err, tp;
> @@ -1582,8 +1597,7 @@ static int thermal_profile_setup(struct platform_device *device)
> if (err < 0)
> return err;
>
> - platform_profile_handler.profile_get = platform_profile_omen_get;
> - platform_profile_handler.profile_set = platform_profile_omen_set;
> + platform_profile_handler.ops = &platform_profile_omen_ops;
>
> set_bit(PLATFORM_PROFILE_COOL, platform_profile_handler.choices);
> } else if (is_victus_thermal_profile()) {
> @@ -1599,8 +1613,7 @@ static int thermal_profile_setup(struct platform_device *device)
> if (err < 0)
> return err;
>
> - platform_profile_handler.profile_get = platform_profile_victus_get;
> - platform_profile_handler.profile_set = platform_profile_victus_set;
> + platform_profile_handler.ops = &platform_profile_victus_ops;
>
> set_bit(PLATFORM_PROFILE_QUIET, platform_profile_handler.choices);
> } else {
> @@ -1617,8 +1630,7 @@ static int thermal_profile_setup(struct platform_device *device)
> if (err)
> return err;
>
> - platform_profile_handler.profile_get = hp_wmi_platform_profile_get;
> - platform_profile_handler.profile_set = hp_wmi_platform_profile_set;
> + platform_profile_handler.ops = &hp_wmi_platform_profile_ops;
>
> set_bit(PLATFORM_PROFILE_QUIET, platform_profile_handler.choices);
> set_bit(PLATFORM_PROFILE_COOL, platform_profile_handler.choices);
> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> index dc98f862a06d..b49ee94f9bee 100644
> --- a/drivers/platform/x86/ideapad-laptop.c
> +++ b/drivers/platform/x86/ideapad-laptop.c
> @@ -1063,6 +1063,11 @@ static const struct dmi_system_id ideapad_dytc_v4_allow_table[] = {
> {}
> };
>
> +static const struct platform_profile_ops dytc_profile_ops = {
> + .profile_get = dytc_profile_get,
> + .profile_set = dytc_profile_set,
> +};
> +
> static int ideapad_dytc_profile_init(struct ideapad_private *priv)
> {
> int err, dytc_version;
> @@ -1105,8 +1110,7 @@ static int ideapad_dytc_profile_init(struct ideapad_private *priv)
> priv->dytc->pprof.name = "ideapad-laptop";
> priv->dytc->pprof.dev = &priv->platform_device->dev;
> priv->dytc->priv = priv;
> - priv->dytc->pprof.profile_get = dytc_profile_get;
> - priv->dytc->pprof.profile_set = dytc_profile_set;
> + priv->dytc->pprof.ops = &dytc_profile_ops;
>
> /* Setup supported modes */
> set_bit(PLATFORM_PROFILE_LOW_POWER, priv->dytc->pprof.choices);
> diff --git a/drivers/platform/x86/inspur_platform_profile.c b/drivers/platform/x86/inspur_platform_profile.c
> index 53af73a7fbf7..38d4745f6be1 100644
> --- a/drivers/platform/x86/inspur_platform_profile.c
> +++ b/drivers/platform/x86/inspur_platform_profile.c
> @@ -166,6 +166,11 @@ static int inspur_platform_profile_get(struct platform_profile_handler *pprof,
> return 0;
> }
>
> +static const struct platform_profile_ops inspur_platform_profile_ops = {
> + .profile_get = inspur_platform_profile_get,
> + .profile_set = inspur_platform_profile_set,
> +};
> +
> static int inspur_wmi_probe(struct wmi_device *wdev, const void *context)
> {
> struct inspur_wmi_priv *priv;
> @@ -179,8 +184,7 @@ static int inspur_wmi_probe(struct wmi_device *wdev, const void *context)
>
> priv->handler.name = "inspur-wmi";
> priv->handler.dev = &wdev->dev;
> - priv->handler.profile_get = inspur_platform_profile_get;
> - priv->handler.profile_set = inspur_platform_profile_set;
> + priv->handler.ops = &inspur_platform_profile_ops;
>
> set_bit(PLATFORM_PROFILE_LOW_POWER, priv->handler.choices);
> set_bit(PLATFORM_PROFILE_BALANCED, priv->handler.choices);
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index f51662861738..04a292af2bfc 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -10538,12 +10538,16 @@ static int dytc_profile_set(struct platform_profile_handler *pprof,
> return err;
> }
>
> -static struct platform_profile_handler dytc_profile = {
> - .name = "thinkpad-acpi",
> +static const struct platform_profile_ops dytc_profile_ops = {
> .profile_get = dytc_profile_get,
> .profile_set = dytc_profile_set,
> };
>
> +static struct platform_profile_handler dytc_profile = {
> + .name = "thinkpad-acpi",
> + .ops = &dytc_profile_ops,
> +};
> +
> static void dytc_profile_refresh(void)
> {
> enum platform_profile_option profile;
> diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
> index f1cd4b65e351..a9db95bc360d 100644
> --- a/include/linux/platform_profile.h
> +++ b/include/linux/platform_profile.h
> @@ -27,16 +27,22 @@ enum platform_profile_option {
> PLATFORM_PROFILE_LAST, /*must always be last */
> };
>
> +struct platform_profile_handler;
> +
> +struct platform_profile_ops {
> + int (*profile_get)(struct platform_profile_handler *pprof,
> + enum platform_profile_option *profile);
> + int (*profile_set)(struct platform_profile_handler *pprof,
> + enum platform_profile_option profile);
> +};
> +
> struct platform_profile_handler {
> const char *name;
> struct device *dev;
> struct device *class_dev;
> int minor;
> unsigned long choices[BITS_TO_LONGS(PLATFORM_PROFILE_LAST)];
> - int (*profile_get)(struct platform_profile_handler *pprof,
> - enum platform_profile_option *profile);
> - int (*profile_set)(struct platform_profile_handler *pprof,
> - enum platform_profile_option profile);
> + const struct platform_profile_ops *ops;
> };
>
> int platform_profile_register(struct platform_profile_handler *pprof);