Re: [PATCH v2] phy: cadence-torrent: add support for three or more links using 2 protocols

[Roger Quadros <rogerq@kernel.org>]

Hi Siddharth,

On 10/07/2024 14:56, Siddharth Vadapalli wrote:
> The Torrent SERDES can support at most two different protocols. This only

Could you please point to where this is mentioned? Doesn't this SERDES support 4 lanes?
So in theory each lane can be used as one protocol (or link) independed of the other.

Also, from code

struct cdns_torrent_phy {
...
        struct cdns_torrent_inst phys[MAX_NUM_LANES];
...
}

and MAX_NUM_LANES is 4.

And from Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml

patternProperties:
  '^phy@[0-3]$':
    type: object
    description:
      Each group of PHY lanes with a single master lane should be represented as a sub-node.

Which means it can have upto 4 phy nodes with different protocols.

> mandates that the device-tree sub-nodes expressing the configuration should
> describe links with at-most two different protocols.
> 
> The existing implementation however imposes an artificial constraint that
> allows only two links (device-tree sub-nodes). As long as at-most two
> protocols are chosen, using more than two links to describe them in an
> alternating configuration is still a valid configuration of the Torrent
> SERDES.
> 
> A 3-Link 2-Protocol configuration of the 4-Lane SERDES can be:
> Lane 0 => Protocol 1 => Link 1
> Lane 1 => Protocol 1 => Link 1
> Lane 2 => Protocol 2 => Link 2
> Lane 3 => Protocol 1 => Link 3
> 
> A 4-Link 2-Protocol configuration of the 4-Lane SERDES can be:
> Lane 0 => Protocol 1 => Link 1
> Lane 1 => Protocol 2 => Link 2
> Lane 2 => Protocol 1 => Link 3
> Lane 3 => Protocol 2 => Link 4
> 

Could you please give an example of device tree where existing implementation
doesn't work for you.

> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
> 
> Hello,
> 
> This patch is based on linux-next tagged next-20240710.
> Patch has been sanity tested and tested for functionality in the following
> configurations with the Torrent SERDES0 on J7200-EVM:
> 1. PCIe (Lanes 0 and 1) + QSGMII (Lane 2)
>    => 2 protocols, 2 links
> 2. PCIe (Lane0 as 1 Link) + PCIe (Lane 1 as 1 Link)
>    => 1 protocol, 2 links
> 3. PCIe (Lanes 0 and 1) + QSGMII (Lane 2) + PCIe (Lane 3)
>    => 2 protocols, 3 links
> 
> v1:
> https://lore.kernel.org/r/20240709120703.2716397-1-s-vadapalli@ti.com/
> Changes since v1:
> - A multilink configuration doesn't necessarily imply two protocols
>   since a single protocol may be split across links as follows:
>   Lane 0 => Protocol 1
>   Lane 1 => Unused
>   Lane 2 => Protocol 1
>   Lane 3 => Unused
>   which corresponds to two links and therefore two sub-nodes. In such a
>   case, treat it as two single-link configurations performed sequentially
>   which happens to be the case prior to this patch. To address this,
>   handle the case where cdns_torrent_phy_configure_multilink() can be
>   invoked for a single protocol with multiple sub-nodes (links) by
>   erroring out only when the number of protocols is strictly greater
>   than two, followed by handling the configuration similar to how it was
>   done prior to this patch.

-- 
cheers,
-roger