Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Message-ID: <fbee5e3b-4c32-443a-b756-943762ae07b8@arm.com>
Date:   Tue, 12 Dec 2023 19:50:01 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v5 07/23] PM: EM: Refactor how the EM table is allocated
 and populated
Content-Language: en-US
To:     Lukasz Luba <lukasz.luba@arm.com>, linux-kernel@vger.kernel.org,
        linux-pm@vger.kernel.org, rafael@kernel.org
Cc:     rui.zhang@intel.com, amit.kucheria@verdurent.com,
        amit.kachhap@gmail.com, daniel.lezcano@linaro.org,
        viresh.kumar@linaro.org, len.brown@intel.com, pavel@ucw.cz,
        mhiramat@kernel.org, qyousef@layalina.io, wvw@google.com
References: <20231129110853.94344-1-lukasz.luba@arm.com>
 <20231129110853.94344-8-lukasz.luba@arm.com>
From:   Dietmar Eggemann <dietmar.eggemann@arm.com>
In-Reply-To: <20231129110853.94344-8-lukasz.luba@arm.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 29/11/2023 12:08, Lukasz Luba wrote:
> Split the process of allocation and data initialization for the EM table.
> The upcoming changes for modifiable EM will use it.
> 
> This change is not expected to alter the general functionality.

NIT: IMHO, I guess you wanted to say: "No functional changes
introduced"? I.e. all not only general functionality ...

[...]

>  static int em_create_pd(struct device *dev, int nr_states,
> @@ -234,11 +234,15 @@ static int em_create_pd(struct device *dev, int nr_states,
>  			return -ENOMEM;
>  	}
>  
> -	ret = em_create_perf_table(dev, pd, nr_states, cb, flags);
> -	if (ret) {
> -		kfree(pd);
> -		return ret;
> -	}
> +	pd->nr_perf_states = nr_states;

Why does `pd->nr_perf_states = nr_states;` have to move from
em_create_perf_table() to em_create_pd()?

> +
> +	ret = em_allocate_perf_table(pd, nr_states);
> +	if (ret)
> +		goto free_pd;
> +
> +	ret = em_create_perf_table(dev, pd, pd->table, nr_states, cb, flags);

If you set it in em_create_pd() then you can use 'pd->nr_perf_states' in
em_create_perf_table() and doesn't have to pass `nr_states`.

[...]