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

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <acef4aeb-ddb6-4e2b-b2ac-8ac351efcf2f@kernel.org>
Date: Wed, 10 Jul 2024 18:22:53 +0300
From: Roger Quadros <rogerq@...nel.org>
To: Siddharth Vadapalli <s-vadapalli@...com>, vkoul@...nel.org,
 kishon@...nel.org, p.zabel@...gutronix.de, sjakhade@...ence.com,
 thomas.richard@...tlin.com, theo.lebrun@...tlin.com, robh@...nel.org
Cc: linux-phy@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, srk@...com
Subject: Re: [PATCH v2] phy: cadence-torrent: add support for three or more
 links using 2 protocols

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@...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