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.
