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

[Fayssal Benmlih]

Hi Cristian,

Some documentation comments inline.

> + - SCMI_TLM_GET_INFO: Gather basic Telemetry features like number of
> +   resources and versioning information.

This was renamed to SCMI_TLM_GET_ABI_INFO.

Please also explain the distinction between abi_features and instance
features, including that reset requires both ABI support and firmware
support.

> + - SCMI_TLM_GET_ALL_CFG / SCMI_TLM_SET_ALL_CFG: Get or set the cumulative
> +   configuration of ALL the DataEvents defined on the platform.

This needs to document that configuration is global to the SCMI instance,
not per-open-file state.

Please specify:

- whether concurrent callers are supported;
- what "last writer wins" means when multi-command operations interleave;
- whether SET_ALL_CFG can leave partial state;
- how userspace detects a concurrent configuration change;
- which operations require write access or privilege.

> + - SCMI_TLM_BATCH_READ: Report the last samples for the DataEvents IDs
> +   specified within the samples input params.

Please document failure semantics. Currently one bad DE fails the entire
ioctl, preceding successful samples are not returned, and there is no
per-entry status.

The documentation should also define count limits and the input/output
meaning of num_samples.

> + - SCMI_TLM_GET_SHMTI_LIST: Gather a list of open file descriptors, one for
> +   each SHMTI memory areas defined for this instance, that can be used to
> +   memory-map such areas in the calling process address space [...]

The mmap contract needs substantially more detail before becoming UAPI:

- the exact mapping length, including offset and page rounding;
- permitted mmap offset and protection flags;
- that the returned fd is read-only and close-on-exec;
- cacheability and coherency expectations;
- the valid lifetime of the fd and VMA across device removal/reset;
- whether mappings become stale after reconfiguration;
- cleanup using munmap() and close();
- the page-alignment/security requirements for bytes surrounding an
  unaligned SHMTI.

> +		/* Enable Telemetry as a whole, set a 400ms update interval */
> +		cfg.enable = 1;
> +		cfg.active.secs = 400;
> +		cfg.active.exp = -3;

Please document the permitted secs/exp ranges and how an interval is
matched against the advertised discrete or segmented interval list.

The timestamp example also needs the timestamp domain, clock-rate
conversion, wrap behavior, and whether/how it can be correlated with a
Linux clock.

> +		return 0;
> +	}

The example should close fd on all exit paths. It would also be useful to
show enumeration allocation/error handling and SHMTI mmap cleanup rather
than only the successful configuration path.

Thanks,
Fayçal