linux-kernel - Re: [PATCH 1/3] dt-bindings: PCI: cdns: Add PHY latency properties

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAH9NwWfLLvVG4=cxCriYjMpXHYw5OhX3rzbSXO0o5Ji3isYtXg@mail.gmail.com>
Date:   Tue, 21 Mar 2023 09:42:36 +0100
From:   Christian Gmeiner <christian.gmeiner@...il.com>
To:     Vignesh Raghavendra <vigneshr@...com>
Cc:     Dominic Rath <dominic.rath@...-augsburg.net>,
        Rob Herring <robh@...nel.org>,
        krzysztof.kozlowski+dt@...aro.org, tjoseph@...ence.com,
        bhelgaas@...gle.com, lpieralisi@...nel.org, nm@...com,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-pci@...r.kernel.org, Alexander Bahle <bahle@...-augsburg.de>,
        Dominic Rath <rath@...-augsburg.de>
Subject: Re: [PATCH 1/3] dt-bindings: PCI: cdns: Add PHY latency properties

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