| Message ID | 87zim32xbo.wl%kuninori.morimoto.gx@renesas.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Mon, Oct 17, 2016 at 3:38 AM, Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote: > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > --- > .../bindings/sound/simple-graph-card.txt | 65 ++++++++++++++++++++++ > 1 file changed, 65 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/simple-graph-card.txt > > diff --git a/Documentation/devicetree/bindings/sound/simple-graph-card.txt b/Documentation/devicetree/bindings/sound/simple-graph-card.txt > new file mode 100644 > index 0000000..c191c0d > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/simple-graph-card.txt > @@ -0,0 +1,65 @@ > +Simple-Graph-Card: > + > +Simple-Graph-Card specifies audio DAI connections of SoC <-> codec. > +It is based on common bindings for device graphs. > +see ${LINUX}/Documentation/devicetree/bindings/graph.txt > + > +Basically, Simple-Graph-Card is same as Simple-Card, but using graph style. > +see ${LINUX}/Documentation/devicetree/bindings/sound/simple-card.txt > + > +Below are same as Simple-Card. > + > +- simple-audio-card,name > +- simple-audio-card,widgets > +- simple-audio-card,routing > +- simple-audio-card,mclk-fs > +- simple-audio-card,hp-det-gpio > +- simple-audio-card,mic-det-gpio > +- simple-audio-card,format > +- simple-audio-card,frame-master > +- simple-audio-card,bitclock-master > +- simple-audio-card,bitclock-inversion > +- simple-audio-card,frame-inversion > +- simple-audio-card,mclk-fs > +- simple-audio-card,dai-tdm-slot-num > +- simple-audio-card,dai-tdm-slot-width > +- clocks / system-clock-frequency > + > +This Simple-Graph-Card should be located as CPU driver's port[s]. > +And then, CPU driver need to probe it by itself. > + > +Required properties: > + > +- compatible : "asoc-simple-graph-card"; > +- type : "sound"; > + > +Example > + > +ak4643: codec@12 { > + compatible = "asahi-kasei,ak4643"; > + ... > + port { > + type = "sound"; > + ak4643_port: endpoint { > + remote-endpoint = <&rcar_ak4643_port>; > + clocks = <&audio_clock>; This belongs in the codec node. > + }; > + }; > +}; > + > +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. > + 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. > + rcar_ak4643_port: endpoint { > + remote-endpoint = <&ak4643_port>; > + playback = <&ssi0 &src2 &dvc0>; > + capture = <&ssi1 &src3 &dvc1>; > + }; > + }; > +}; > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. > > +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 -- current style -- ports { compatible = "asoc-simple-graph-card"; 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>; type = "sound"; rcar_ak4613_port: endpoint { remote-endpoint = <&ak4613_port>; playback = <&ssi0 &src0 &dvc0>; capture = <&ssi1 &src1 &dvc1>; }; }; 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>; }; }; 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 18, 2016 at 8:36 PM, Kuninori Morimoto <kuninori.morimoto.gx@renesas.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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob Thank you for your feedback > > 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. Hmm.. I will consider about that. > > 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. Current ALSA SoC basically needs CPU/Codec/Card drivers. If we use this new simple sound card, DT needs only CPU/Codec. Card part will be created from CPU by using compatible = "xxx" (= Card driver itself is needed, but no DT) This "Card" part is related to its whole sound system. If you want to use normal sound system, it will be "simple-card", if you want to use feature sound system, it will be "xxxx-card". This time, I'm focusing to simple one, someone will create xxxx-card, but it is not me :) > > 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. These were "Card" part property, not ak4613 property. It indicates which can be master (CPU or Codec) on sound system. > > 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. I'm sorry, maybe your confusing is because this sample. This playback/capture are Renesas CPU specific properties. Renesas CPU port needs to know CPU-inside connection. > > 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. I'm sorry, same comment. This "playback" is Renesas CPU specific property. It indicating Renesas CPU inside connection -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob Can I continue this idea (= property on port/endpoint) ? Of course I will remove "type" from OF graph. > > > + 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. > > > > +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 > > -- current style -- > > ports { > compatible = "asoc-simple-graph-card"; > 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>; > > type = "sound"; > rcar_ak4613_port: endpoint { > remote-endpoint = <&ak4613_port>; > playback = <&ssi0 &src0 &dvc0>; > capture = <&ssi1 &src1 &dvc1>; > }; > }; > 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>; > }; > }; > 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/sound/simple-graph-card.txt b/Documentation/devicetree/bindings/sound/simple-graph-card.txt new file mode 100644 index 0000000..c191c0d --- /dev/null +++ b/Documentation/devicetree/bindings/sound/simple-graph-card.txt @@ -0,0 +1,65 @@ +Simple-Graph-Card: + +Simple-Graph-Card specifies audio DAI connections of SoC <-> codec. +It is based on common bindings for device graphs. +see ${LINUX}/Documentation/devicetree/bindings/graph.txt + +Basically, Simple-Graph-Card is same as Simple-Card, but using graph style. +see ${LINUX}/Documentation/devicetree/bindings/sound/simple-card.txt + +Below are same as Simple-Card. + +- simple-audio-card,name +- simple-audio-card,widgets +- simple-audio-card,routing +- simple-audio-card,mclk-fs +- simple-audio-card,hp-det-gpio +- simple-audio-card,mic-det-gpio +- simple-audio-card,format +- simple-audio-card,frame-master +- simple-audio-card,bitclock-master +- simple-audio-card,bitclock-inversion +- simple-audio-card,frame-inversion +- simple-audio-card,mclk-fs +- simple-audio-card,dai-tdm-slot-num +- simple-audio-card,dai-tdm-slot-width +- clocks / system-clock-frequency + +This Simple-Graph-Card should be located as CPU driver's port[s]. +And then, CPU driver need to probe it by itself. + +Required properties: + +- compatible : "asoc-simple-graph-card"; +- type : "sound"; + +Example + +ak4643: codec@12 { + compatible = "asahi-kasei,ak4643"; + ... + port { + type = "sound"; + ak4643_port: endpoint { + remote-endpoint = <&rcar_ak4643_port>; + clocks = <&audio_clock>; + }; + }; +}; + +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>; + type = "sound"; + rcar_ak4643_port: endpoint { + remote-endpoint = <&ak4643_port>; + playback = <&ssi0 &src2 &dvc0>; + capture = <&ssi1 &src3 &dvc1>; + }; + }; +};