HI, On 9/22/23 10:56, Ilpo Järvinen wrote: > On Thu, 21 Sep 2023, Hans de Goede wrote: >> On 9/19/23 15:07, Ilpo Järvinen wrote: >>> On Tue, 19 Sep 2023, Suma Hegde wrote: >>> >>>> AMD MI300 MCM provides GET_METRICS_TABLE message to retrieve >>>> all the system management information from SMU. >>>> >>>> The metrics table is made available as hexadecimal sysfs binary file >>>> under per socket sysfs directory created at >>>> /sys/devices/platform/amd_hsmp/socket%d/metrics_bin >>>> >>>> Metrics table definitions will be documented as part of Public PPR. >>>> The same is defined in the amd_hsmp.h header. >>>> >>>> Signed-off-by: Suma Hegde <suma.hegde@xxxxxxx> >>>> Reviewed-by: Naveen Krishna Chatradhi <nchatrad@xxxxxxx> >>>> --- >>>> Changes since v1: >>>> 1. Remove HSMP_DEVNODE_NAME and HSMP_CDEV_NAME macro definitions in >>>> this patch >>>> 2. Remove extra space in comments for HSMP_GET_METRIC_TABLE_VER, >>>> HSMP_GET_METRIC_TABLE and HSMP_GET_METRIC_TABLE_DRAM_ADDR enum >>>> definition in amd_hsmp.h files >>>> 3. Change check, count == 0 to !count in hsmp_metric_tbl_read() function >>>> 4. Add hsmp_metric_table_visible() function >>>> 5. hsmp_create_metric_tbl_sysfs_file() is renamed as hsmp_init_metric_tbl_bin_attr() >>>> and code is also modified slightly >>>> 6. Modify hsmp_create_sysfs_file() to use devm_device_add_groups() >>>> 7. Change from cleanup label to deregister label >>>> 8. Add dev_err print in hsmp_get_tbl_dram_base() >>>> 9. Reword "Unable to Failed" in hsmp_get_tbl_dram_base() >>>> 10. Add HSMP_GRP_NAME_SIZE and NUM_ATTRS macros >>>> 11. Remove sysfs cleanup code in hsmp_pltdrv_remove() >>>> 12. Correct ATRR typo error >>>> 13. Change sprintf to snprintf >>>> 14. Check metrics table support only against HSMP_PROTO_VER6 >>>> Changes since v2: >>>> 1. squash documentation patch into this patch >>>> 2. change from num_sockets to plat_dev.num_sockets >>>> >>>> Documentation/arch/x86/amd_hsmp.rst | 16 +++ >>>> arch/x86/include/uapi/asm/amd_hsmp.h | 109 ++++++++++++++++ >>>> drivers/platform/x86/amd/hsmp.c | 180 ++++++++++++++++++++++++++- >>>> 3 files changed, 302 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/Documentation/arch/x86/amd_hsmp.rst b/Documentation/arch/x86/amd_hsmp.rst >>>> index 440e4b645a1c..a4c308784818 100644 >>>> --- a/Documentation/arch/x86/amd_hsmp.rst >>>> +++ b/Documentation/arch/x86/amd_hsmp.rst >>>> @@ -41,6 +41,22 @@ In-kernel integration: >>>> * Locking across callers is taken care by the driver. >>>> >>>> >>>> +HSMP sysfs interface >>>> +==================== >>>> + >>>> +1. Metrics table binary sysfs >>>> + >>>> +AMD MI300A MCM provides GET_METRICS_TABLE message to retrieve >>>> +all the system management information from SMU. >>>> + >>>> +The metrics table is made available as hexadecimal sysfs binary file >>>> +under per socket sysfs directory created at >>>> +/sys/devices/platform/amd_hsmp/socket%d/metrics_bin >>>> + >>>> +Metrics table definitions will be documented as part of Public PPR. >>>> +The same is defined in the amd_hsmp.h header. >>>> + >>>> + >>>> An example >>>> ========== >>>> >>> >>> I'd have expected to have the sysfs documentation appear under >>> Documentation/ABI/testing/sysfs-... >> >> Actually it is somewhat normal for sysfs files to be paired >> together with other documentation when there is more extensive >> documentation then just the sysfs files, see e.g. : >> >> Documentation/admin-guide/laptops/thinkpad-acpi.rst > > Okay but that seems to result in attempting to handle deprecation > within that file too which feels wrong beyond just hacing documentation in > an unusual location. > >> So there is precedent for this and I think it make sense >> to keep all the documentation in one place, rather then to add >> a Documentation/ABI/testing/sysfs-platform-amd-hsmp file >> just for the sysfs attributes . >> >> OTOH people are used to look for sysfs attribute documentation >> in a place like Documentation/ABI/testing/sysfs-platform-amd-hsmp, >> but I think that keeping all the docs together is more important. >> >> So I have a slight preference for keeping this as is and >> just merging v3 of this series as is. >> >> Ilpo, what do you think ? > > I don't have strong preference myself if you think the current location > is fine. So feel free to merge it as is. I'm traveling this week. Ilpo, if you're fine with this series as is, can you merge it please ? Regards, Hans
