Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 1 Sep 2020 11:52:51 +0200
From:   Maxime Ripard <maxime@cerno.tech>
To:     Chanwoo Choi <cw00.choi@samsung.com>
Cc:     Nicolas Saenz Julienne <nsaenzjulienne@suse.de>,
        Eric Anholt <eric@anholt.net>, dri-devel@lists.freedesktop.org,
        linux-rpi-kernel@lists.infradead.org,
        bcm-kernel-feedback-list@broadcom.com,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        Dave Stevenson <dave.stevenson@raspberrypi.com>,
        Tim Gover <tim.gover@raspberrypi.com>,
        Phil Elwell <phil@raspberrypi.com>,
        Rob Herring <robh@kernel.org>
Subject: Re: [PATCH v4 75/78] dt-bindings: display: vc4: hdmi: Add BCM2711
 HDMI controllers bindings
Message-ID: <20200901095251.awfw45ndc7p6ty4a@gilmour.lan>
References: <cover.7a1aa1784976093af26cb31fd283cf5b3ed568bb.1594230107.git-series.maxime@cerno.tech>
 <CGME20200708174447epcas1p3c9582567617196b61495ee281b443b91@epcas1p3.samsung.com>
 <a174517d1f57b7a75eabd0361aef5365894d7c1a.1594230107.git-series.maxime@cerno.tech>
 <94c5b8e4-a168-690d-2d60-93462585bef9@samsung.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="3t57ruatadihu43n"
Content-Disposition: inline
In-Reply-To: <94c5b8e4-a168-690d-2d60-93462585bef9@samsung.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--3t57ruatadihu43n
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi,

On Tue, Sep 01, 2020 at 01:45:07PM +0900, Chanwoo Choi wrote:
> Hi Maxime,
>=20
> On 7/9/20 2:42 AM, Maxime Ripard wrote:
> > The HDMI controllers found in the BCM2711 SoC need some adjustments to =
the
> > bindings, especially since the registers have been shuffled around in m=
ore
> > register ranges.
> >=20
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > ---
> >  Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml | 109=
 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 109 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2=
711-hdmi.yaml
> >=20
> > diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdm=
i.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > new file mode 100644
> > index 000000000000..6091fe3d315b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > @@ -0,0 +1,109 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: https://protect2.fireeye.com/url?k=3D556aeb05-08b8fda0-556b604a-0=
cc47a31bee8-c3a0ebd1d22c3183&q=3D1&u=3Dhttp%3A%2F%2Fdevicetree.org%2Fschema=
s%2Fdisplay%2Fbrcm%2Cbcm2711-hdmi.yaml%23
> > +$schema: https://protect2.fireeye.com/url?k=3D24fa660c-792870a9-24fbed=
43-0cc47a31bee8-0bf16f4fd60f0ab4&q=3D1&u=3Dhttp%3A%2F%2Fdevicetree.org%2Fme=
ta-schemas%2Fcore.yaml%23
> > +
> > +title: Broadcom BCM2711 HDMI Controller Device Tree Bindings
> > +
> > +maintainers:
> > +  - Eric Anholt <eric@anholt.net>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - brcm,bcm2711-hdmi0
> > +      - brcm,bcm2711-hdmi1
> > +
> > +  reg:
> > +    items:
> > +      - description: HDMI controller register range
> > +      - description: DVP register range
> > +      - description: HDMI PHY register range
> > +      - description: Rate Manager register range
> > +      - description: Packet RAM register range
> > +      - description: Metadata RAM register range
> > +      - description: CSC register range
> > +      - description: CEC register range
> > +      - description: HD register range
> > +
> > +  reg-names:
> > +    items:
> > +      - const: hdmi
> > +      - const: dvp
> > +      - const: phy
> > +      - const: rm
> > +      - const: packet
> > +      - const: metadata
> > +      - const: csc
> > +      - const: cec
> > +      - const: hd
> > +
> > +  clocks:
> > +    description: The HDMI state machine clock
>=20
> I'm not sure the following description is correct.
> But, this description doesn't contain the information of audio clock.
>=20
> 	description: The HDMI state machine and audio clock
>=20
> > +
> > +  clock-names:
> > +    const: hdmi
>=20
> This patch is missing the following clock information for audio clock.
>=20
> 	const: clk-108M
>=20
> > +
> > +  ddc:
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: >
> > +      Phandle of the I2C controller used for DDC EDID probing
> > +
> > +  hpd-gpios:
> > +    description: >
> > +      The GPIO pin for the HDMI hotplug detect (if it doesn't appear
> > +      as an interrupt/status bit in the HDMI controller itself)
> > +
> > +  dmas:
> > +    maxItems: 1
> > +    description: >
> > +      Should contain one entry pointing to the DMA channel used to
> > +      transfer audio data.
> > +
> > +  dma-names:
> > +    const: audio-rx
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +  - clocks
> > +  - resets
> > +  - ddc
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    hdmi0: hdmi@7ef00700 {
> > +        compatible =3D "brcm,bcm2711-hdmi0";
> > +        reg =3D <0x7ef00700 0x300>,
> > +              <0x7ef00300 0x200>,
> > +              <0x7ef00f00 0x80>,
> > +              <0x7ef00f80 0x80>,
> > +              <0x7ef01b00 0x200>,
> > +              <0x7ef01f00 0x400>,
> > +              <0x7ef00200 0x80>,
> > +              <0x7ef04300 0x100>,
> > +              <0x7ef20000 0x100>;
> > +        reg-names =3D "hdmi",
> > +                    "dvp",
> > +                    "phy",
> > +                    "rm",
> > +                    "packet",
> > +                    "metadata",
> > +                    "csc",
> > +                    "cec",
> > +                    "hd";
> > +        clocks =3D <&firmware_clocks 13>;
> > +        clock-names =3D "hdmi";
>=20
> Also, this example doesn't include the instance of audio clock.
> Need to edit them as following:
>=20
> 	clock-names =3D "hdmi", "clk-108M";
> 	clocks =3D <&firmware_clocks 13>, <&dvp 0>;

Indeed, thanks for pointing it out

Maxime

--3t57ruatadihu43n
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCX04Z8wAKCRDj7w1vZxhR
xY+DAP463c8+Cx/V7o7lAsKNPNIq50LRUtYa2f/F0i6/uJ9zOgEA6UKWJP2FTPS0
IUEhoE12OvDJJT3wW6r+XxZEFf93+ws=
=vP/V
-----END PGP SIGNATURE-----

--3t57ruatadihu43n--