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:   Tue, 18 Oct 2016 21:39:57 -0500
From:   Rob Herring <robh+dt@...nel.org>
To:     Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
Cc:     Mark Brown <broonie@...nel.org>,
        Linux-ALSA <alsa-devel@...a-project.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Simon <horms@...ge.net.au>,
        Laurent <laurent.pinchart@...asonboard.com>,
        Guennadi <g.liakhovetski@....de>,
        Grant Likely <grant.likely@...aro.org>,
        Frank Rowand <frowand.list@...il.com>,
        Linux-DT <devicetree@...r.kernel.org>,
        Linux-Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 20/23] ASoC: add simple-graph-card document

On Tue, Oct 18, 2016 at 8:36 PM, Kuninori Morimoto
<kuninori.morimoto.gx@...esas.com> wrote:
>
> Hi Rob
>
>> > +               type = "sound";
>>
>> I'm still not convinced this is necessary. This is implied either by
>> the fact there is only one port or perhaps the compatible string.
>
> Do you mean "on this sample" ? or in general ?
> Indeed this sample is definitely for sound, so type is very clear
> without property.
> But in general, for example HDMI, it want to know port type.
> Anyway, I can remove above "type" from this new sound driver.

For HDMI, the port number should dictate which one is video and which is audio.

>> > +rcar_sound {
>> > +       ...
>> > +       port {
>> > +               compatible = "asoc-simple-graph-card";
>> > +
>> > +               simple-audio-card,format = "left_j";
>> > +               simple-audio-card,bitclock-master = <&ak4643_port>;
>> > +               simple-audio-card,frame-master = <&ak4643_port>;
>>
>> Don't add a bunch of properties with in port and endpoint nodes. The
>> purpose is to describe the graph. Put these in the parent node or
>> perhaps the codec node.
>
> These properties are needed on each ports/endpoints on sound at this point.
> If ports/endpoints can't include these, I need to separate these,
> is it correct approach ? ?? see below

Uhh, no. Not at all what I had in mind.

> -- current style --
>
>         ports {
>                 compatible = "asoc-simple-graph-card";

I think your problems start with trying to extend simple-card. This
binding is anything but simple. I think using OF graph is a good idea,
but trying to make it completely generic is not.

>                 simple-audio-card,name = "graph-sound";
>
>                 port@0 {
>                         simple-audio-card,format = "left_j";
>                         simple-audio-card,bitclock-master = <&rcar_ak4613_port>;
>                         simple-audio-card,frame-master = <&rcar_ak4613_port>;

These look like properties of the ak4613 to me, so put them in the
ak4613 node. If they are standard property names, then you just walk
the graph and get them.

>
>                         type = "sound";
>                         rcar_ak4613_port: endpoint {
>                                 remote-endpoint = <&ak4613_port>;
>                                 playback = <&ssi0 &src0 &dvc0>;
>                                 capture  = <&ssi1 &src1 &dvc1>;

Not really sure how you are using these to comment.

>                         };
>                 };
>                 port@1 {
>                         simple-audio-card,format = "i2s";
>                         simple-audio-card,bitclock-master = <&rcar_hdmi0_port>;
>                         simple-audio-card,frame-master = <&rcar_hdmi0_port>;
>                         type = "sound";
>                         rcar_hdmi0_port: endpoint {
>                                 remote-endpoint = <&du_out_hdmi_snd0>;
>                                 playback = <&ssi2>;

If you are trying to describe a connection between hdmi_snd0 and ssi2,
then do just that. Add a port to ssi2 and connect it to hdmi_snd0.

>                         };
>                 };
>                 port@2 {
>                         simple-audio-card,format = "i2s";
>                         simple-audio-card,bitclock-master = <&rcar_hdmi1_port>;
>                         simple-audio-card,frame-master = <&rcar_hdmi1_port>;
>                         type = "sound";
>                         rcar_hdmi1_port: endpoint {
>                                 remote-endpoint = <&du_out_hdmi_snd1>;
>                                 playback = <&ssi3>;
>                         };
>                 };
>         };
>
> -- separate style --
>
>         ports {
>                 port@0 {
>                         rcar_ak4613_port: endpoint {
>                         }
>                 };
>                 port@1 {
>                         rcar_hdmi0_port: endpoint {
>                         }
>                 };
>                 port@2 {
>                         rcar_hdmi1_port: endpoint {
>                         }
>                 };
>         };
>
>         sound-xxx {
>                 compatible = "asoc-simple-graph-card";
>
>                 port@0 {
>                         simple-audio-card,format = "left_j";
>                         simple-audio-card,bitclock-master = <&rcar_ak4613_port>;
>                         simple-audio-card,frame-master = <&rcar_ak4613_port>;
>                 };
>                 port@1 {
>                         simple-audio-card,format = "i2s";
>                         simple-audio-card,bitclock-master = <&rcar_hdmi0_port>;
>                         simple-audio-card,frame-master = <&rcar_hdmi0_port>;
>                 };
>                 port@2 {
>                         simple-audio-card,format = "i2s";
>                         simple-audio-card,bitclock-master = <&rcar_hdmi1_port>;
>                         simple-audio-card,frame-master = <&rcar_hdmi1_port>;
>                 };
>         };
>
> Best regards
> ---
> Kuninori Morimoto

Powered by blists - more mailing lists