linux-kernel - Re: [PATCH 11/16] OPP: Add dev_pm_opp_add

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230608052953.l44dwb6n62kx4umk@vireshk-i7>
Date:   Thu, 8 Jun 2023 10:59:53 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Ulf Hansson <ulf.hansson@...aro.org>
Cc:     Sudeep Holla <sudeep.holla@....com>,
        Cristian Marussi <cristian.marussi@....com>,
        Viresh Kumar <vireshk@...nel.org>, Nishanth Menon <nm@...com>,
        Stephen Boyd <sboyd@...nel.org>,
        Nikunj Kela <nkela@...cinc.com>,
        Prasad Sodagudi <psodagud@...cinc.com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        linux-pm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 11/16] OPP: Add dev_pm_opp_add_dynamic() to allow more
 flexibility

On 07-06-23, 14:46, Ulf Hansson wrote:
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 954c94865cf5..0e6ee2980f88 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -1921,8 +1921,7 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
>   * _opp_add_v1() - Allocate a OPP based on v1 bindings.
>   * @opp_table:	OPP table
>   * @dev:	device for which we do this operation
> - * @freq:	Frequency in Hz for this OPP
> - * @u_volt:	Voltage in uVolts for this OPP
> + * @opp:	The OPP to add
>   * @dynamic:	Dynamically added OPPs.
>   *
>   * This function adds an opp definition to the opp table and returns status.
> @@ -1940,10 +1939,10 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
>   * -ENOMEM	Memory allocation failure
>   */
>  int _opp_add_v1(struct opp_table *opp_table, struct device *dev,
> -		unsigned long freq, long u_volt, bool dynamic)
> +		struct dev_pm_opp_data *opp, bool dynamic)

The name `opp` is mostly used for instances of `struct dev_pm_opp`. Can we use a
different name here please for the data ?

> +/**
> + * dev_pm_opp_add()  - Add an OPP table from a table definitions
> + * @dev:	device for which we do this operation
> + * @freq:	Frequency in Hz for this OPP
> + * @u_volt:	Voltage in uVolts for this OPP
> + *
> + * This function adds an opp definition to the opp table and returns status.
> + * The opp is made available by default and it can be controlled using
> + * dev_pm_opp_enable/disable functions.
> + *
> + * Return:
> + * 0		On success OR
> + *		Duplicate OPPs (both freq and volt are same) and opp->available
> + * -EEXIST	Freq are same and volt are different OR
> + *		Duplicate OPPs (both freq and volt are same) and !opp->available
> + * -ENOMEM	Memory allocation failure
> + */
> +int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)

Maybe move this to include/linux/pm_opp.h and mark it static inline and get rid
of documentation too.

> +{
> +	struct dev_pm_opp_data opp;
> +
> +	memset(&opp, 0, sizeof(opp));

What about
        struct dev_pm_opp_data data = {0};

I think it is guaranteed that all the fields will be 0 now, not the padding of
course, but we don't care about that here.

> +	opp.freq = freq;
> +	opp.u_volt = u_volt;

Or maybe just

        struct dev_pm_opp_data data = {
                .freq = freq,
                .u_volt = u_volt,
        };

Rest must be 0.

-- 
viresh