RE: [PATCH v1 7/7] dt-bindings: phy: cadence-torrent: Update Torrent PHY bindings for generic use

[Swapnil Kashinath Jakhade]

Hi Kishon,

> -----Original Message-----
> From: Kishon Vijay Abraham I <kishon@xxxxxx>
> Sent: Wednesday, August 12, 2020 3:41 PM
> To: Swapnil Kashinath Jakhade <sjakhade@xxxxxxxxxxx>;
> vkoul@xxxxxxxxxx; robh+dt@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx
> Cc: Milind Parab <mparab@xxxxxxxxxxx>; Yuti Suresh Amonkar
> <yamonkar@xxxxxxxxxxx>; tomi.valkeinen@xxxxxx; jsarha@xxxxxx;
> nsekhar@xxxxxx
> Subject: Re: [PATCH v1 7/7] dt-bindings: phy: cadence-torrent: Update
> Torrent PHY bindings for generic use
> 
> EXTERNAL MAIL
> 
> 
> Hi Swapnil,
> 
> On 8/7/2020 3:42 PM, Swapnil Jakhade wrote:
> > Torrent PHY can be used in different multi-link multi-protocol
> > configurations including protocols other than DisplayPort also, such
> > as PCIe, USB, SGMII, QSGMII etc. Update the bindings to have support
> > for these configurations.
> >
> > Signed-off-by: Swapnil Jakhade <sjakhade@xxxxxxxxxxx>
> > ---
> >  .../bindings/phy/phy-cadence-torrent.yaml     | 76 ++++++++++++++-----
> >  1 file changed, 58 insertions(+), 18 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
> > b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
> > index a7ee19d27c19..b2275712363d 100644
> > --- a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
> > +++ b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
> > @@ -4,11 +4,13 @@
> >  $id:
> "https://urldefense.com/v3/__http://devicetree.org/schemas/phy/phy-
> cadence-
> torrent.yaml*__;Iw!!EHscmS1ygiU1lA!Wq5zaq3IY8sduOmiJrpiWODn2JYPNEB
> r4cTnMX74Dxz5OBWGpayFjI1OQDYFP8g$ "
> >  $schema: "https://urldefense.com/v3/__http://devicetree.org/meta-
> schemas/core.yaml*__;Iw!!EHscmS1ygiU1lA!Wq5zaq3IY8sduOmiJrpiWODn2
> JYPNEBr4cTnMX74Dxz5OBWGpayFjI1O14G-GJw$ "
> >
> > -title: Cadence Torrent SD0801 PHY binding for DisplayPort
> > +title: Cadence Torrent SD0801 PHY binding
> >
> >  description:
> >    This binding describes the Cadence SD0801 PHY (also known as
> > Torrent PHY)
> > -  hardware included with the Cadence MHDP DisplayPort controller.
> > +  hardware included with the Cadence MHDP DisplayPort controller.
> > + Torrent  PHY also supports multilink multiprotocol combinations
> > + including protocols  such as PCIe, USB, SGMII, QSGMII etc.
> >
> >  maintainers:
> >    - Swapnil Jakhade <sjakhade@xxxxxxxxxxx> @@ -49,13 +51,14 @@
> > properties:
> >        - const: dptx_phy
> >
> >    resets:
> > -    maxItems: 1
> > -    description:
> > -      Torrent PHY reset.
> > -      See Documentation/devicetree/bindings/reset/reset.txt
> > +    minItems: 1
> > +    maxItems: 2
> > +    items:
> > +      - description: Torrent PHY reset.
> > +      - description: Torrent APB reset. This is optional.
> >
> >  patternProperties:
> > -  '^phy@[0-7]+$':
> > +  '^link@[0-7]+$':
> 
> Wouldn't this break older device tree binding? Or are there no upstream DT
> nodes with phy sub-node?

Torrent PHY driver has never been functional, and therefore do not exist in any
active use case, so this should not break the binding as there are no DT nodes
in upstream using the torrent PHY.

Thanks,
Swapnil

> 
> Thanks
> Kishon
> 
> >      type: object
> >      description:
> >        Each group of PHY lanes with a single master lane should be
> represented as a sub-node.
> > @@ -78,13 +81,13 @@ patternProperties:
> >            Specifies the type of PHY for which the group of PHY lanes is used.
> >            Refer include/dt-bindings/phy/phy.h. Constants from the header
> should be used.
> >          $ref: /schemas/types.yaml#/definitions/uint32
> > -        enum: [1, 2, 3, 4, 5, 6]
> > +        enum: [1, 2, 3, 4, 5, 6, 7, 8, 9]
> >
> >        cdns,num-lanes:
> >          description:
> > -          Number of DisplayPort lanes.
> > +          Number of lanes.
> >          $ref: /schemas/types.yaml#/definitions/uint32
> > -        enum: [1, 2, 4]
> > +        enum: [1, 2, 3, 4]
> >          default: 4
> >
> >        cdns,ssc-mode:
> > @@ -108,6 +111,7 @@ patternProperties:
> >        - resets
> >        - "#phy-cells"
> >        - cdns,phy-type
> > +      - cdns,num-lanes
> >
> >      additionalProperties: false
> >
> > @@ -141,15 +145,51 @@ examples:
> >              clock-names = "refclk";
> >              #address-cells = <1>;
> >              #size-cells = <0>;
> > -            phy@0 {
> > -                      reg = <0>;
> > -                      resets = <&phyrst 1>, <&phyrst 2>,
> > -                               <&phyrst 3>, <&phyrst 4>;
> > -                      #phy-cells = <0>;
> > -                      cdns,phy-type = <PHY_TYPE_DP>;
> > -                      cdns,num-lanes = <4>;
> > -                      cdns,max-bit-rate = <8100>;
> > +            link@0 {
> > +                reg = <0>;
> > +                resets = <&phyrst 1>, <&phyrst 2>,
> > +                         <&phyrst 3>, <&phyrst 4>;
> > +                #phy-cells = <0>;
> > +                cdns,phy-type = <PHY_TYPE_DP>;
> > +                cdns,num-lanes = <4>;
> > +                cdns,max-bit-rate = <8100>;
> > +            };
> > +        };
> > +    };
> > +  - |
> > +    #include <dt-bindings/phy/phy.h>
> > +    #include <dt-bindings/phy/phy-cadence-torrent.h>
> > +
> > +    bus {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        torrent-phy@f0fb500000 {
> > +            compatible = "cdns,torrent-phy";
> > +            reg = <0xf0 0xfb500000 0x0 0x00100000>;
> > +            reg-names = "torrent_phy";
> > +            resets = <&phyrst 0>, <&phyrst 1>;
> > +            clocks = <&ref_clk>;
> > +            clock-names = "refclk";
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +            link@0 {
> > +                reg = <0>;
> > +                resets = <&phyrst 2>, <&phyrst 3>;
> > +                #phy-cells = <0>;
> > +                cdns,phy-type = <PHY_TYPE_PCIE>;
> > +                cdns,num-lanes = <2>;
> > +                cdns,ssc-mode = <TORRENT_SERDES_NO_SSC>;
> >              };
> > +
> > +            link@2 {
> > +                reg = <2>;
> > +                resets = <&phyrst 4>;
> > +                #phy-cells = <0>;
> > +                cdns,phy-type = <PHY_TYPE_SGMII>;
> > +                cdns,num-lanes = <1>;
> > +                cdns,ssc-mode = <TORRENT_SERDES_NO_SSC>;
> > +           };
> >          };
> >      };
> >  ...
> >