Re: [PATCH] ARM: dts: Configure omap5 AESS

[Tony Lindgren <tony@atomide.com>]

* H. Nikolaus Schaller <hns@goldelico.com> [200114 17:05]:
> > Am 14.01.2020 um 17:46 schrieb Tony Lindgren <tony@atomide.com>:
> > Care to clarify what you have in mind? The module is generic, aess
> > device will be the child node.
> 
> The existing driver is hooked into the sound root-node and looks for a
> ti,aess = <&aess>; link:
> 
> / {
> 	sound: sound {
> 		compatible = "ti,abe-twl6040";
> 		ti,model = "omap5-uevm";
> 
> 		ti,jack-detection;
> 		ti,mclk-freq = <19200000>;
> 
> 		ti,mcpdm = <&mcpdm>;
> 		ti,mcbsp1 = <&mcbsp1>;
> 		ti,mcbsp2 = <&mcbsp2>;
> 		ti,mcbsp3 = <&mcbsp3>;
> 
> 		ti,twl6040 = <&twl6040>;
> 		ti,aess = <&aess>;
> 
> 		...
> 	};
> };
> 
> Well, this could be simply wrong... I.e. the aess node should request
> all the phandles to mcpdm and mcbsps because it is connected to.

The aess label above should be in the child aess node, not in the
target-module.

> Or it is right to use the sound node to "connect" all subsystems.

Sounds like that's all taken care of nowadays with the generic
graph binding:

Documentation/devicetree/bindings/graph.txt

See also snd-soc-audio-graph-card and various users for it:

Documentation/devicetree/bindings/sound/audio-graph-card.txt

> Then the "aess" core could also become the child node of the target-module:
> 
> target-module@f1000 {			/* 0x401f1000, ap 32 20.0 */
> 	...
> 	aess: aess {
> 		compatible = "ti,aess";
> 		status = "disabled";
> 	};
> };

Yeah this is how it should be :)

> Although it looks better this way, it may make it even one step
> more difficult to resurrect the old code...

Well the old phandles and properties should work the same, just put them
into the child aess node. No need to stuff anything else there at the
target-module level AFAIK.

> And DT maintainers are not happy with otherwise undefined compatible
> definitions.
> 
> So maybe:
> 
> target-module@f1000 {			/* 0x401f1000, ap 32 20.0 */
> 	...
> 
> 	aess: aess {
> 		/* revisit
> 		compatible = "ti,aess";
> 		status = "disabled";
> 		*/
> 	};
> };

But we have no binding and no driver for the aess at this point..
If and when the aess driver work the child node can be just added :)

Regards,

Tony