| Message ID | 874m2swcbx.wl%kuninori.morimoto.gx@renesas.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Mon, Nov 28, 2016 at 02:47:57AM +0000, Kuninori Morimoto 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 | 67 ++++++++++++++++++++++ > 1 file changed, 67 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..3d4c5a8 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/simple-graph-card.txt > @@ -0,0 +1,67 @@ > +Simple-Graph-Card: There's nothing simple about this. graph-audio-card or audio-card-graph. > + > +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 property is same as Simple-Card. > +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 The simple-audio-card prefix is pointless. It's fine to reuse, but don't add to it. > +- clocks / system-clock-frequency > + > +In Simple-Graph-Card, above properties need in CPU side port on DT. > +And it needs to have "compatible" property too. > +In addition, CPU side driver needs to call asoc_simple_card_try_to_probe_graph_card(). > +It will probe specified Card driver if it could find "compatible" property on port. > +Otherwise, it will do nothing. > + > +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>; > + ... > + }; > + }; > +}; > + > +rcar_sound { > + ... > + port { > + compatible = "asoc-simple-graph-card"; Do you have an example where you'd have multiple ports? If not, this should go up a level. > + > + simple-audio-card,format = "left_j"; > + simple-audio-card,bitclock-master = <&ak4643_port>; > + simple-audio-card,frame-master = <&ak4643_port>; If you follow video-interfaces.txt, these should all go in the endpoint node. > + type = "sound"; > + rcar_ak4643_port: endpoint { > + remote-endpoint = <&ak4643_port>; > + ... > + }; > + }; > +}; > -- > 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, Mark > > +++ b/Documentation/devicetree/bindings/sound/simple-graph-card.txt > > @@ -0,0 +1,67 @@ > > +Simple-Graph-Card: > > There's nothing simple about this. graph-audio-card or audio-card-graph. I have no objection about naming, but this is one of simple-xxx-card series. And, this is very simple from ALSA SoC point of view... > > +rcar_sound { > > + ... > > + port { > > + compatible = "asoc-simple-graph-card"; > > Do you have an example where you'd have multiple ports? If not, this > should go up a level. ALSA SoC needs 3 type of driver, CPU/Codec/Card. But HW is SoC <-> Codec. Thus, "CPU" side DT needs to call "Card" portion, and ALSA SoC needs to select Card type (graph-audio-card, graph-scu-card, etc, etc....). Above it for it. SoC { compatible = "cpu_driver_compatible_name"; ... port { compatible = "card_driver_compatible_name"; ... }; }; Here, SoC driver "cpu_driver_compatible_name" will handle CPU and its each port settings. And it will probes "card_driver_compatible_name". "card_driver_compatible_name" will connect CPU <-> Codec via ALSA SoC. > > + simple-audio-card,format = "left_j"; > > + simple-audio-card,bitclock-master = <&ak4643_port>; > > + simple-audio-card,frame-master = <&ak4643_port>; > > If you follow video-interfaces.txt, these should all go in the endpoint > node. Hmm... this is not for endpoint, but for whole card. Mark, of course this can goes to each endpoint, but it negates passed ALSA SoC Card discussion/decision. What is your opinion ? -- 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 again > > +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 > > The simple-audio-card prefix is pointless. It's fine to reuse, but don't > add to it. ALSA SoC sometimes want to switch Sound Card feature. This is one of simple-xxx-card series, and each simple-xxx-card series are similar but have difference from DT point of view. Because of this, it is easy to switch to other feature if these series are using same property. 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, Mark > > > + simple-audio-card,format = "left_j"; > > > + simple-audio-card,bitclock-master = <&ak4643_port>; > > > + simple-audio-card,frame-master = <&ak4643_port>; > > > > If you follow video-interfaces.txt, these should all go in the endpoint > > node. > > Hmm... this is not for endpoint, but for whole card. > Mark, of course this can goes to each endpoint, but it negates passed > ALSA SoC Card discussion/decision. What is your opinion ? I'm sorry, this was my fault. I can move these to endpoint side. 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 Sun, Dec 04, 2016 at 11:48:49PM +0000, Kuninori Morimoto wrote: > > Hi Rob, Mark > > > > +++ b/Documentation/devicetree/bindings/sound/simple-graph-card.txt > > > @@ -0,0 +1,67 @@ > > > +Simple-Graph-Card: > > > > There's nothing simple about this. graph-audio-card or audio-card-graph. > > I have no objection about naming, but this is one of simple-xxx-card series. > And, this is very simple from ALSA SoC point of view... > > > > +rcar_sound { > > > + ... > > > + port { > > > + compatible = "asoc-simple-graph-card"; > > > > Do you have an example where you'd have multiple ports? If not, this > > should go up a level. > > ALSA SoC needs 3 type of driver, CPU/Codec/Card. But HW is SoC <-> Codec. > Thus, "CPU" side DT needs to call "Card" portion, and ALSA SoC needs to > select Card type (graph-audio-card, graph-scu-card, etc, etc....). > Above it for it. > > SoC { > compatible = "cpu_driver_compatible_name"; > ... > port { > compatible = "card_driver_compatible_name"; > ... > }; > }; > > Here, SoC driver "cpu_driver_compatible_name" will handle CPU and its > each port settings. And it will probes "card_driver_compatible_name". > "card_driver_compatible_name" will connect CPU <-> Codec via ALSA SoC. Don't design bindings around what ASoC wants and don't explain it in terms of how ALSA works. Design bindings in a way that reflects the h/w. This explanation seems completely wrong to me. It seems like you are abusing OF graph to just create 2 instances of a simple-card which would be working around some ASoC limitation. I'd expect the top level node to be the card node that knows how to find all the components. The graph should reflect the data flow. For example, the data goes to audio DSP to I2S host to codec to amp. Rob -- 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 > I'd expect the top level node to be the card node that knows how to find > all the components. The graph should reflect the data flow. For example, > the data goes to audio DSP to I2S host to codec to amp. Do you mean, is this OK for OF graph ? in driver point of view, "I2S" is sound card here. I2S { port { i2s-dsp: endpoint { remote-endpoint = <&dsp>; } i2s-codec: endpoint { remote-endpoint = <&codec>; } } } DSP { port { dsp: endpoint { remote-endpoint = <&i2s-dsp>; } } } Codec { port { codec: endpoint { remote-endpoint = <&i2s-codec>; } } } -- 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 Mon, Dec 5, 2016 at 6:57 PM, Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote: > > Hi Rob > >> I'd expect the top level node to be the card node that knows how to find >> all the components. The graph should reflect the data flow. For example, >> the data goes to audio DSP to I2S host to codec to amp. > > Do you mean, is this OK for OF graph ? Yes, something like this. > in driver point of view, "I2S" is sound card here. Well, that seems odd to me because I2S should just be the h/w block that interfaces to I2S/SSI signals. I'd expect you still have a card node that references these nodes. Maybe it just references the DSP and then you walk the graph from there to find the I2S controller and codec. > > I2S { > port { > i2s-dsp: endpoint { > remote-endpoint = <&dsp>; > } > i2s-codec: endpoint { > remote-endpoint = <&codec>; > } > } > } > > DSP { > port { > dsp: endpoint { > remote-endpoint = <&i2s-dsp>; > } > } > } > > Codec { > port { > codec: endpoint { > remote-endpoint = <&i2s-codec>; > } > } > } -- 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 > > Do you mean, is this OK for OF graph ? > > Yes, something like this. Wow!! Thanks !! it makes new OF-graph easier !! > > in driver point of view, "I2S" is sound card here. > > Well, that seems odd to me because I2S should just be the h/w block > that interfaces to I2S/SSI signals. I'd expect you still have a card > node that references these nodes. Maybe it just references the DSP and > then you walk the graph from there to find the I2S controller and > codec. If my understanding was correct, this is good ? Card { ports { port@0 { card-dsp: endpoint { remote-endpoint = <&dsp>; }; }; port@1 { card-codec: endpoint { remote-endpoint = <&codec>; }; } } } DSP { port { dsp: endpoint { remote-endpoint = <&card-dsp>; } } } Codec { port { codec: endpoint { remote-endpoint = <&card-codec>; } } } -- 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..3d4c5a8 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/simple-graph-card.txt @@ -0,0 +1,67 @@ +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 property is same as Simple-Card. +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 + +In Simple-Graph-Card, above properties need in CPU side port on DT. +And it needs to have "compatible" property too. +In addition, CPU side driver needs to call asoc_simple_card_try_to_probe_graph_card(). +It will probe specified Card driver if it could find "compatible" property on port. +Otherwise, it will do nothing. + +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>; + ... + }; + }; +}; + +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>; + ... + }; + }; +};