Messages in this thread | | | Date | Thu, 13 Oct 2022 08:59:03 -0500 | From | Bjorn Helgaas <> | Subject | Re: [PATCH 2/3] PCI: cadence: Use DT bindings to set PHY latencies |
| |
On Thu, Oct 13, 2022 at 08:26:48AM +0200, Dominic Rath wrote: > From: Alexander Bahle <bahle@ibv-augsburg.de> > > 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
| |