linux-kernel - Re: [linux-sunxi] [PATCH v5 09/20] arm64: dts: allwinner: h6: Add DAI node and soundcard for HDMI

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]

Date:   Fri, 2 Oct 2020 18:01:21 +0200
From:   Clément Péron <peron.clem@...il.com>
To:     Chen-Yu Tsai <wens@...e.org>
Cc:     Maxime Ripard <mripard@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Brown <broonie@...nel.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Marcus Cooper <codekipper@...il.com>,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        Linux-ALSA <alsa-devel@...a-project.org>,
        devicetree <devicetree@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-sunxi <linux-sunxi@...glegroups.com>
Subject: Re: [linux-sunxi] [PATCH v5 09/20] arm64: dts: allwinner: h6: Add DAI
 node and soundcard for HDMI

Hi Chen-Yu,

On Mon, 28 Sep 2020 at 07:42, Chen-Yu Tsai <wens@...e.org> wrote:
>
> On Mon, Sep 28, 2020 at 1:32 PM Chen-Yu Tsai <wens@...e.org> wrote:
> >
> > On Mon, Sep 28, 2020 at 3:29 AM Clément Péron <peron.clem@...il.com> wrote:
> > >
> > > From: Jernej Skrabec <jernej.skrabec@...l.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@...l.net>
> > > Signed-off-by: Marcus Cooper <codekipper@...il.com>
> > > Signed-off-by: Clément Péron <peron.clem@...il.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/arm64/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 = "disabled";
> > >         };
> > >
> > > +       hdmi_sound: hdmi-sound {
> > > +               compatible = "simple-audio-card";
> > > +               simple-audio-card,format = "i2s";
> > > +               simple-audio-card,name = "sun50i-h6-hdmi";
> > > +               simple-audio-card,mclk-fs = <128>;
> > > +               simple-audio-card,frame-inversion;
> > > +               status = "disabled";
> > > +
> > > +               simple-audio-card,codec {
> > > +                       sound-dai = <&hdmi>;
> > > +               };
> > > +
> > > +               simple-audio-card,cpu {
> > > +                       sound-dai = <&i2s1>;
> > > +                       dai-tdm-slot-num = <2>;
> >
> > Doesn't this end up limiting the number of audio channels HDMI can carry?
> > AFAICT the TDM properties are all optional, so just leave it out.
> >
> > Same goes for the other two patches.
> >
> > > +                       dai-tdm-slot-width = <32>;
> > > +               };
> > > +       };
> > > +
> > >         osc24M: osc24M_clk {
> > >                 #clock-cells = <0>;
> > >                 compatible = "fixed-clock";
> > > @@ -609,6 +628,19 @@ mdio: mdio {
> > >                         };
> > >                 };
> > >
> > > +               i2s1: i2s@...1000 {
> > > +                       #sound-dai-cells = <0>;
> > > +                       compatible = "allwinner,sun50i-h6-i2s";
> > > +                       reg = <0x05091000 0x1000>;
> > > +                       interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
> > > +                       clocks = <&ccu CLK_BUS_I2S1>, <&ccu CLK_I2S1>;
> > > +                       clock-names = "apb", "mod";
> > > +                       dmas = <&dma 4>, <&dma 4>;
> > > +                       resets = <&ccu RST_BUS_I2S1>;
> > > +                       dma-names = "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 name.

Indeed if there is no DRQ number assigned we shouldn't have it in the
device-tree

but Samuel told me that the `make dtbs_check` reports:

i2s@...2800: dma-names:0: 'rx' was expected
i2s@...2800: dma-names: ['tx'] is too short
i2s@...2800: dmas: [[28, 27]] is too short

Should I fix the YAML so?

Regards,
Clement

>
> > > +                       status = "disabled";
> > > +               };
> > > +
> > >                 spdif: spdif@...3000 {
> > >                         #sound-dai-cells = <0>;
> > >                         compatible = "allwinner,sun50i-h6-spdif";
> > > @@ -739,6 +771,7 @@ ohci3: usb@...1400 {
> > >                 };
> > >
> > >                 hdmi: hdmi@...0000 {
> > > +                       #sound-dai-cells = <0>;
> > >                         compatible = "allwinner,sun50i-h6-dw-hdmi";
> > >                         reg = <0x06000000 0x10000>;
> > >                         reg-io-width = <1>;
> >
> > The rest of the patch looks OK.