On 2/14/22 11:21, Song Liu wrote: > On Mon, Feb 14, 2022 at 7:32 AM Chatradhi, Naveen Krishna > <nchatrad@xxxxxxx> 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F55898_B1_pub_0.50.zip&data=04%7C01%7CNathan.Fontenot%40amd.com%7C6026990f93ed41d04d0608d9efde79a1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637804561503265585%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=o5BR%2FgHpjnNVa9zVu75I3RyzO8WNB4L7FMEvLNbzZVY%3D&reserved=0 >> >> >> 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) > */ Agreed, this is the type of update I was thinking of. -Nathan > {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