Re: [PATCH v4 1/2] platforms/x86: Add AMD system management interface

[Song Liu <song@kernel.org>]

On Mon, Feb 14, 2022 at 7:32 AM Chatradhi, Naveen Krishna
<nchatrad@amd.com> wrote:
>
> Hi Song,
>
> On 2/11/2022 3:10 AM, Song Liu wrote:
[...]
>
> HSMP mailbox messages are evolving and each platform defines a supported
> list of messages.
>
> On a given platform these messages are described in the PPR.
>
> Eg: Milan PPR has "7 Host System Management Port (HSMP)", is made public
>
> https://www.amd.com/system/files/TechDocs/55898_B1_pub_0.50.zip
>
>
> Bringing detailed description of these messages from PPR into the kernel
> would be a
>
> duplicating effort. Which will also bring in challenges such as
> maintaining the details
>
> for every supported platform and submitting kernel patches for every
> platform.
>
>
> We would like to avoid bringing more details of these messages to the
> kernel documentation.
>
> Such a structure can be described as part of esmi_oob_library to ease
> user space tool development.

I agree there is  extra effort to adding extra logic and documentations to
the kernel. How about we ship current version with a few minor changes:
1). msg_desc_table is in the header, so please prefix it with hsmp_
2) as Nathan suggested, add more comments to msg_desc_table. Maybe
something like:
/*
 * HSMP_GET_FCLK_MCLK,
 * output arg0 = fclk (MHz); arg1 = mclk (MHz)
 */
{0, 2, GET},

Besides these, I have a question. Per the hardware design, args and
reponse in hsmp_message share the same registers. Shall we make
them in/out argments in hsmp_message and save 256 bytes per
hsmp_message?

Thanks,
Song