Re: [PATCH v6 21/22] [RFC] Documentation: Add SCMI System Telemetry documentation

Hi Cristian,=0A=
=0A=
Some documentation comments inline.=0A=
=0A=
> + - SCMI_TLM_GET_INFO: Gather basic Telemetry features like number of=0A=
> +   resources and versioning information.=0A=
=0A=
This was renamed to SCMI_TLM_GET_ABI_INFO.=0A=
=0A=
Please also explain the distinction between abi_features and instance=0A=
features, including that reset requires both ABI support and firmware=0A=
support.=0A=
=0A=
> + - SCMI_TLM_GET_ALL_CFG / SCMI_TLM_SET_ALL_CFG: Get or set the cumulativ=
e=0A=
> +   configuration of ALL the DataEvents defined on the platform.=0A=
=0A=
This needs to document that configuration is global to the SCMI instance,=
=0A=
not per-open-file state.=0A=
=0A=
Please specify:=0A=
=0A=
- whether concurrent callers are supported;=0A=
- what "last writer wins" means when multi-command operations interleave;=
=0A=
- whether SET_ALL_CFG can leave partial state;=0A=
- how userspace detects a concurrent configuration change;=0A=
- which operations require write access or privilege.=0A=
=0A=
> + - SCMI_TLM_BATCH_READ: Report the last samples for the DataEvents IDs=
=0A=
> +   specified within the samples input params.=0A=
=0A=
Please document failure semantics. Currently one bad DE fails the entire=0A=
ioctl, preceding successful samples are not returned, and there is no=0A=
per-entry status.=0A=
=0A=
The documentation should also define count limits and the input/output=0A=
meaning of num_samples.=0A=
=0A=
> + - SCMI_TLM_GET_SHMTI_LIST: Gather a list of open file descriptors, one =
for=0A=
> +   each SHMTI memory areas defined for this instance, that can be used t=
o=0A=
> +   memory-map such areas in the calling process address space [...]=0A=
=0A=
The mmap contract needs substantially more detail before becoming UAPI:=0A=
=0A=
- the exact mapping length, including offset and page rounding;=0A=
- permitted mmap offset and protection flags;=0A=
- that the returned fd is read-only and close-on-exec;=0A=
- cacheability and coherency expectations;=0A=
- the valid lifetime of the fd and VMA across device removal/reset;=0A=
- whether mappings become stale after reconfiguration;=0A=
- cleanup using munmap() and close();=0A=
- the page-alignment/security requirements for bytes surrounding an=0A=
  unaligned SHMTI.=0A=
=0A=
> +		/* Enable Telemetry as a whole, set a 400ms update interval */=0A=
> +		cfg.enable =3D 1;=0A=
> +		cfg.active.secs =3D 400;=0A=
> +		cfg.active.exp =3D -3;=0A=
=0A=
Please document the permitted secs/exp ranges and how an interval is=0A=
matched against the advertised discrete or segmented interval list.=0A=
=0A=
The timestamp example also needs the timestamp domain, clock-rate=0A=
conversion, wrap behavior, and whether/how it can be correlated with a=0A=
Linux clock.=0A=
=0A=
> +		return 0;=0A=
> +	}=0A=
=0A=
The example should close fd on all exit paths. It would also be useful to=
=0A=
show enumeration allocation/error handling and SHMTI mmap cleanup rather=0A=
than only the successful configuration path.=0A=
=0A=
Thanks,=0A=
Fay=E7al=