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:   Fri, 2 Oct 2020 18:24:34 +0200
From:   Maxime Ripard <maxime@cerno.tech>
To:     =?utf-8?B?Q2zDqW1lbnQgUMOpcm9u?= <peron.clem@gmail.com>
Cc:     Chen-Yu Tsai <wens@csie.org>, Rob Herring <robh+dt@kernel.org>,
        Mark Brown <broonie@kernel.org>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Jaroslav Kysela <perex@perex.cz>,
        Takashi Iwai <tiwai@suse.com>,
        Marcus Cooper <codekipper@gmail.com>,
        Jernej Skrabec <jernej.skrabec@siol.net>,
        Linux-ALSA <alsa-devel@alsa-project.org>,
        devicetree <devicetree@vger.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        linux-sunxi <linux-sunxi@googlegroups.com>
Subject: Re: [linux-sunxi] [PATCH v5 09/20] arm64: dts: allwinner: h6: Add
 DAI node and soundcard for HDMI
Message-ID: <20201002162434.3jeksuelrig6yyo5@gilmour.lan>
References: <20200927192912.46323-1-peron.clem@gmail.com>
 <20200927192912.46323-10-peron.clem@gmail.com>
 <CAGb2v64uAHUd=Ag2pQDqH=gjtPVso5dnKKdCn3ihyiVh8V8L=g@mail.gmail.com>
 <CAGb2v64U9b1Ayq-XNCHb3z6spsds6eDaz3C4EsV9xFOquHrB7w@mail.gmail.com>
 <CAJiuCcfThSqpobeZW7ugnmokc4Xy0n9o+5jvOfP9eqzvDbu_BQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="pc2zpj5o3hm4cgp3"
Content-Disposition: inline
In-Reply-To: <CAJiuCcfThSqpobeZW7ugnmokc4Xy0n9o+5jvOfP9eqzvDbu_BQ@mail.gmail.com>
Precedence: bulk


--pc2zpj5o3hm4cgp3
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Oct 02, 2020 at 06:01:21PM +0200, Cl=E9ment P=E9ron wrote:
> Hi Chen-Yu,
>=20
> On Mon, 28 Sep 2020 at 07:42, Chen-Yu Tsai <wens@csie.org> wrote:
> >
> > On Mon, Sep 28, 2020 at 1:32 PM Chen-Yu Tsai <wens@csie.org> wrote:
> > >
> > > On Mon, Sep 28, 2020 at 3:29 AM Cl=E9ment P=E9ron <peron.clem@gmail.c=
om> wrote:
> > > >
> > > > From: Jernej Skrabec <jernej.skrabec@siol.net>
> > > >
> > > > Add the I2S node used by the HDMI and a simple-soundcard to
> > > > link audio between HDMI and I2S.
> > > >
> > > > Note that the HDMI codec requires an inverted frame clock and
> > > > a fixed I2S width. As there is no such option for I2S we use
> > > > TDM property of the simple-soundcard to do that.
> > > >
> > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > > Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> > > > Signed-off-by: Cl=E9ment P=E9ron <peron.clem@gmail.com>
> > > > ---
> > > >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 33 ++++++++++++++++=
++++
> > > >  1 file changed, 33 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/ar=
m64/boot/dts/allwinner/sun50i-h6.dtsi
> > > > index 28c77d6872f6..a8853ee7885a 100644
> > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > > @@ -67,6 +67,25 @@ de: display-engine {
> > > >                 status =3D "disabled";
> > > >         };
> > > >
> > > > +       hdmi_sound: hdmi-sound {
> > > > +               compatible =3D "simple-audio-card";
> > > > +               simple-audio-card,format =3D "i2s";
> > > > +               simple-audio-card,name =3D "sun50i-h6-hdmi";
> > > > +               simple-audio-card,mclk-fs =3D <128>;
> > > > +               simple-audio-card,frame-inversion;
> > > > +               status =3D "disabled";
> > > > +
> > > > +               simple-audio-card,codec {
> > > > +                       sound-dai =3D <&hdmi>;
> > > > +               };
> > > > +
> > > > +               simple-audio-card,cpu {
> > > > +                       sound-dai =3D <&i2s1>;
> > > > +                       dai-tdm-slot-num =3D <2>;
> > >
> > > Doesn't this end up limiting the number of audio channels HDMI can ca=
rry?
> > > AFAICT the TDM properties are all optional, so just leave it out.
> > >
> > > Same goes for the other two patches.
> > >
> > > > +                       dai-tdm-slot-width =3D <32>;
> > > > +               };
> > > > +       };
> > > > +
> > > >         osc24M: osc24M_clk {
> > > >                 #clock-cells =3D <0>;
> > > >                 compatible =3D "fixed-clock";
> > > > @@ -609,6 +628,19 @@ mdio: mdio {
> > > >                         };
> > > >                 };
> > > >
> > > > +               i2s1: i2s@5091000 {
> > > > +                       #sound-dai-cells =3D <0>;
> > > > +                       compatible =3D "allwinner,sun50i-h6-i2s";
> > > > +                       reg =3D <0x05091000 0x1000>;
> > > > +                       interrupts =3D <GIC_SPI 19 IRQ_TYPE_LEVEL_H=
IGH>;
> > > > +                       clocks =3D <&ccu CLK_BUS_I2S1>, <&ccu CLK_I=
2S1>;
> > > > +                       clock-names =3D "apb", "mod";
> > > > +                       dmas =3D <&dma 4>, <&dma 4>;
> > > > +                       resets =3D <&ccu RST_BUS_I2S1>;
> > > > +                       dma-names =3D "rx", "tx";
> >
> > Sorry, missed this one.
> >
> > Given that usage for this interface is transmit only, and there is no
> > RX DRQ number assigned to it, you should drop the RX DMA number and nam=
e.
>=20
> Indeed if there is no DRQ number assigned we shouldn't have it in the
> device-tree
>=20
> but Samuel told me that the `make dtbs_check` reports:
>=20
> i2s@1c22800: dma-names:0: 'rx' was expected
> i2s@1c22800: dma-names: ['tx'] is too short
> i2s@1c22800: dmas: [[28, 27]] is too short
>=20
> Should I fix the YAML so?

Yep :)

Maxime

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

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

iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCX3dUQgAKCRDj7w1vZxhR
xWNUAQDytEMJPPzTs+e8ZDtzT+2iNv0sKVHta5QD+LfuXZBmbgD/QisIsrbypwx5
fKJkl2netB5EWbzdHHzRhBS1gkZtpgc=
=1BCa
-----END PGP SIGNATURE-----

--pc2zpj5o3hm4cgp3--