On 17.06.2025 06:12, Penny, Zheng wrote:
> [Public]
>
>> -----Original Message-----
>> From: Jan Beulich <jbeul...@suse.com>
>> Sent: Monday, June 16, 2025 5:51 PM
>> To: Penny, Zheng <penny.zh...@amd.com>
>> Cc: Huang, Ray <ray.hu...@amd.com>; xen-devel@lists.xenproject.org
>> Subject: Re: [PATCH v5 03/18] xen/cpufreq: extract _PSD info from "struct
>> xen_processor_performance"
>>
>> On 16.06.2025 11:43, Penny, Zheng wrote:
>>> [Public]
>>>
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeul...@suse.com>
>>>> Sent: Wednesday, June 11, 2025 11:34 PM
>>>> To: Penny, Zheng <penny.zh...@amd.com>
>>>> Cc: Huang, Ray <ray.hu...@amd.com>; xen-devel@lists.xenproject.org
>>>> Subject: Re: [PATCH v5 03/18] xen/cpufreq: extract _PSD info from
>>>> "struct xen_processor_performance"
>>>>
>>>> On 27.05.2025 10:48, Penny Zheng wrote:
>>>>> @@ -545,14 +597,9 @@ int set_px_pminfo(uint32_t acpi_id, struct
>>>>> xen_processor_performance *perf)
>>>>>
>>>>> if ( perf->flags & XEN_PX_PSD )
>>>>> {
>>>>> - /* check domain coordination */
>>>>> - if ( perf->shared_type != CPUFREQ_SHARED_TYPE_ALL &&
>>>>> - perf->shared_type != CPUFREQ_SHARED_TYPE_ANY &&
>>>>> - perf->shared_type != CPUFREQ_SHARED_TYPE_HW )
>>>>> - {
>>>>> - ret = -EINVAL;
>>>>> + ret = check_psd_pminfo(perf->shared_type);
>>>>> + if ( ret )
>>>>> goto out;
>>>>> - }
>>>>>
>>>>> pxpt->shared_type = perf->shared_type;
>>>>> memcpy(&pxpt->domain_info, &perf->domain_info,
>>>>
>>>> ... the need for this change. And even if there is a need, a
>>>> follow-on question would be how this relates to the subject of this patch.
>>>
>>> I extracted this snippet out for sharing the same checking logic both
>>> in Px and later CPPC. They both need _PSD info
>>
>> Right, and that (iirc) becomes visible later in the series. But it needs
>> saying here. As
>> it stands the description talks of only get_psd_info() right now. And the
>> change
>> above is also unrelated to the "extract" mentioned in the title.
>>
>>> I could change title to "xen/cpufreq: make _PSD info common" and also
>>> add description in commit message for introducing check_psd_pminfo()
>>
>> The title was probably fine; it's the description which was lacking. In fact
>> I'd deem
>> "make ... common" misleading when there's no 2nd user (yet).
>>
>
> How about:
> "
> Title: xen/cpufreq: export _PSD info and checking
As said, the original title was probably fine. In the new title (and also in
the text suggested below), I wonder what "export" means.
> _PSD info, consisted of "shared_type" and "struct xen_psd_package", will not
> only be provided from px-specific "struct xen_processor_performance", but also
> in CPPC data.
>
> In cpufreq_add/del_cpu(), a new helper get_psd_info() is introduced to
> export _PSD info. While in set_px_pminfo(), check_psd_pminfo() is also
> introduced to
> export _PSD value checking.
How about "Two new helper functions are introduced to deal with _PSD. They
will later be re-used for handling the same data for CPPC."
Jan
> in the meantime, the following style corrections get applied at the same
> time: ........
> ```
>
>> Jan
