Message-ID: <MW3PR12MB445976E9C6911650D545C9D7C10F9@MW3PR12MB4459.namprd12.prod.outlook.com>
Date:   Wed, 16 Jun 2021 18:21:52 +0000
From:   Nelson Costa <Nelson.Costa@...opsys.com>
To:     Rob Herring <robh@...nel.org>
CC:     "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Hans Verkuil <hverkuil-cisco@...all.nl>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Kishon Vijay Abraham I <kishon@...com>,
        Vinod Koul <vkoul@...nel.org>,
        Jose Abreu <Jose.Abreu@...opsys.com>
Subject: RE: [PATCH 2/9] dt-bindings: media: Document Synopsys DesignWare HDMI
 RX

Hi Rob,

Thanks for your initial comments and feedback!

From: Rob Herring <robh@...nel.org>
Date: qua, jun 16, 2021 at 00:40:48

> On Wed, Jun 02, 2021 at 01:24:20PM +0200, Nelson Costa wrote:
> > Document the device tree bindings for the Synopsys DesignWare HDMI RX
> > Controller.
> > 
> > Signed-off-by: Jose Abreu <jose.abreu@...opsys.com>
> > Signed-off-by: Nelson Costa <nelson.costa@...opsys.com>
> > ---
> >  .../devicetree/bindings/media/snps,dw-hdmi-rx.yaml | 98 ++++++++++++++++++++++
> >  1 file changed, 98 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
> > new file mode 100644
> > index 0000000..4f2169e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
> > @@ -0,0 +1,98 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: https://urldefense.com/v3/__http://devicetree.org/schemas/media/snps,dw-hdmi-rx.yaml*__;Iw!!A4F2R9G_pg!PXFG6mDuYYepSE77ynbfBUqpXfgW5g0UdVsjQ5p0CEVGkNknkcWScmrRWLFWmhWsFl4$ 
> > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!A4F2R9G_pg!PXFG6mDuYYepSE77ynbfBUqpXfgW5g0UdVsjQ5p0CEVGkNknkcWScmrRWLFWXz21yII$ 
> > +
> > +title: Synopsys DesignWare HDMI RX Controller Device Tree Bindings
> > +
> > +maintainers:
> > +  - Jose Abreu <jose.abreu@...opsys.com>
> > +  - Nelson Costa <nelson.costa@...opsys.com>
> > +
> > +description: |
> > +  The Synopsys DesignWare HDMI RX Controller and PHYs e405/e406 is an HDMI 2.0
> > +  Receiver solution that is able to decode video and audio.
> > +
> > +properties:
> > +  compatible:
> > +    const: snps,dw-hdmi-rx
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +    description: |
> > +      phandle to the configuration clock
> > +
> > +  clock-names:
> > +    const: cfg
> > +
> > +  phys:
> > +    maxItems: 1
> > +    description: |
> > +      phandle for the HDMI RX PHY
> > +
> > +  phy-names:
> > +    const: hdmi-phy
> 
> You don't need *-names when there is only one generally.
> 

But we are using the PHY API function "devm_phy_get(dw_dev->dev, 
"hdmi-phy")" in the controller driver in order to get the reference for 
the PHY.
And looking at the PHY API internal implementation (function "phy_get()" 
that is called by "devm_phy_get()") seems that this 'phy-names' property 
is required to get that PHY reference.

> > +
> > +  port:
> > +    $ref: /schemas/graph.yaml#/properties/port
> > +    description: |
> > +      Input port node, multiple endpoints describing the HDMI RX data connected
> > +      to the HDMI PHY receiver.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - phys
> > +  - phy-names
> > +  - port
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    hdmi_rx: hdmi-rx@0 {
> > +        compatible = "snps,dw-hdmi-rx";
> > +        reg = <0x0 0x10000>;
> > +        interrupts = <1 2>;
> > +
> > +        clocks = <&dw_hdmi_refclk>;
> > +        clock-names = "cfg";
> > +
> > +        phys = <&hdmi_e406_phy>;
> > +        phy-names = "hdmi-phy";
> > +
> > +        port {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            hdmi_rx_0: endpoint@0 {
> > +                reg = <0>;
> > +                remote-endpoint = <&hdmi_e406_phy_0>;
> > +            };
> 
> I don't get why you have a connection to the phy with 'phys' and OF 
> graph? The connection should be to the remote device generating the HDMI 
> data with a PHY being somewhat transparent to that data flow.
> 

Would be possible to give an example for our specific use case?
Or there is any good example, similar to this use case, that we can take 
a look?
Having in mind that we have two drivers one for the PHY e405/e406 and 
other for the Controller HDMI RX and a device tree specified for each one 
(patches [1/9] and [2/9]).

The pipeline is something like this:
HDMI Source (which is an external hot pluggable device) -> PHY e405/e406 
-> CTRL HDMI RX

Thanks a lot,

BR,

Nelson Costa

> > +
> > +            hdmi_rx_1: endpoint@1 {
> > +                reg = <1>;
> > +                remote-endpoint = <&hdmi_e406_phy_1>;
> > +            };
> > +
> > +            hdmi_rx_2: endpoint@2 {
> > +                reg = <2>;
> > +                remote-endpoint = <&hdmi_e406_phy_2>;
> > +            };
> > +
> > +            hdmi_rx_3: endpoint@3 {
> > +                reg = <3>;
> > +                remote-endpoint = <&hdmi_e406_phy_3>;
> > +            };
> > +        };
> > +    };
> > -- 
> > 2.7.4

Powered by blists - more mailing lists