On Thu, Oct 13, 2022 at 08:26:48AM +0200, Dominic Rath wrote:
> From: Alexander Bahle <bahle@xxxxxxxxxxxxxxx>
>
> Use optional "cdns,tx-phy-latency-ps" and "cdns,rx-phy-latency-ps"
> DeviceTree bindings to set the CDNS_PCIE_LM_PTM_LAT_PARAM(_IDX)
> register(s) during PCIe host and ep setup.
> The properties expect a list of uint32 PHY latencies in picoseconds for
> every supported speed starting at PCIe Gen1, e.g.:
s/ep/endpoint/
s/properties expect a list/properties are lists/
Rewrap into a single paragraph or add a blank line between paragraphs.
> max-link-speed = <2>;
> tx-phy-latency-ps = <100000 200000>; /* Gen1: 100ns, Gen2: 200ns */
> rx-phy-latency-ps = <150000 250000>; /* Gen1: 150ns, Gen2: 250ns */
>
> There should be a value for every supported speed but it is not enforced or
> necessary. A warning is emitted to let users know that the PTM timestamps
> from this PCIe device may not be precise enough for some applications.
Not sure what "it is not enforced or necessary" means. Maybe it just
means that if a value is missing, we don't program LAT_PARAM and we
emit a warning?
> + param_count = of_property_count_u32_elems(np, key);
> + if (param_count < 0 || param_count < max_link_speed) {
> + dev_warn(dev,
> + "no %s set for one or more speeds: %d\n",
> + key, param_count);
> + }
> +
> + /* Don't set param for unsupported speed */
> + if (param_count > max_link_speed)
> + param_count = max_link_speed;
> +
> + for (i = 0; i < param_count; i++) {
> + if (of_property_read_u32_index(np, key, i,
> + &latency) < 0) {
> + dev_err(dev, "failed to set latency for speed %d. %s\n",
> + i, key);
Seems like these messages should contain "PTM" somewhere.
If they're truly optional properties, should these be dev_info()
instead of dev_warn/dev_err?
Bjorn
