Hi Dominic,
> >>>
> >>> Add "cdns,tx-phy-latency-ps" and "cdns,rx-phy-latency-ps" DT bindings for
> >>> setting the PCIe PHY latencies.
> >>> The properties expect a list of uint32 PHY latencies in picoseconds for
> >>> every supported speed starting at PCIe Gen1, e.g.:
> >>>
> >>> max-link-speed = <2>;
> >>> tx-phy-latency-ps = <100000 200000>; /* Gen1: 100ns, Gen2: 200ns */
> >>> rx-phy-latency-ps = <150000 250000>; /* Gen1: 150ns, Gen2: 250ns */
> >>
> >> These are a property of the PHY or PCI host? Sounds like PHY to me and
> >> that should be in the PHY node. No reason the PCI driver can't go read
> >> PHY node properties.
> >
> > I'm actually not sure if this a property of the PHY, the PCIe host, or
> > of the combination of the two.
> >
>
> Latency is mostly related to propogation latency through SERDES PCS and
> PMA layers.
>
> > We thought about adding this property to the PHY, too, but we didn't
> > know how to handle cases where a single PCIe host is linked with
> > multiple PHYs for multi-lane configurations (see TI's AM65x for
> > example). Which PHYs latency would you use to configure this PCIe RC?
>
> On AM65x, all lanes go through SERDES of same design (but just different
> instances) and thus latencies will remain same across lanes as the PCS
> and PMA logics are same. So, the delays are not lane specific
>
> >
> > Personally I don't have a very strong opinion either way - we just
> > didn't know any better than to put this into the PCIe host that needs
> > it. If you think this is better put into the PHY node we can of course
> > send a new version of this patch.
> >
>
> I don't have a preference here... Delays are dependent on PHYs being
> used but something that host needs, will leave it to framework
> maintainers.
>
> > Is there any binding that specifies "generic" PCIe properties, similar
> > to ethernet-phy.yaml? We couldn't find any.
> >
> > I guess in the AM64x case the "PHY" is serdes0_pcie_link below serdes0:
> >
> > &serdes0 {
> > serdes0_pcie_link: phy@0 {
> > ...
> >
> > This seems to be described by bindings/phy/phy-cadence-torrent.yaml.
> >
> > Should we add a generic (without cdns) tx/rx-phy-latency-ps property
> > there?
> >
> >> If PTM is a standard PCIe thing, then I don't think these should be
> >> Cadence specific. IOW, drop 'cdns'.
> >
> > Yes, it is a standard PCIe thing, but we haven't seen that many
> > implementations yet, so we didn't want to pretend to know what this
> > looks like in the generic case. We can of course drop 'cdns'.
>
> PTM is definitely standard and vendor specific prefix don't make sense
> to me.
>
Is there any chance you can send a revisited patch series or is there
anything missing for
you to continue?
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
