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

["H. Nikolaus Schaller" <hns@goldelico.com>]

Hi Tony,

> Am 14.01.2020 um 17:46 schrieb Tony Lindgren <tony@atomide.com>:
> 
> * H. Nikolaus Schaller <hns@goldelico.com> [200114 16:38]:
>> Hi Tony,
>> 
>>> Am 14.01.2020 um 16:09 schrieb Tony Lindgren <tony@atomide.com>:
>>> 
>>> We are missing AESS for omap5. Looks like it's similar to what we have
>>> for omap4, and this gets ti-sysc interconnect target module driver to
>>> detect it properly.
>>> 
>>> Note that we currently have no child device driver available for it.
>> 
>> What I have is a non-working and no more compiling driver originally written by
>> Peter Ujfalusi and reworked by Andrej Utkin. We did have it almost running on
>> v4.14 or so except problems with firmware versions and headers...
>> 
>> There we used classical hwmods and I could revert them now to try your new patches.
>> Unfortunately, I could only compile-test your two patches but nothing with AESS.
>> 
>> We had tried to follow kernel API changes in the sound subsystem but today it is
>> not even compiling any more :(
>> 
>> So getting a working device driver is an even bigger task than SGX was.
> 
> OK. Well hopefully that's at least a little bit easier now though.
> 
>>> Cc: H. Nikolaus Schaller <hns@goldelico.com>
>>> Cc: Matthijs van Duin <matthijsvanduin@gmail.com>
>>> Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
>>> Cc: Tero Kristo <t-kristo@ti.com>
>>> Signed-off-by: Tony Lindgren <tony@atomide.com>
>>> ---
>>> 
>>> Note that this depends on "[PATCH] clk: ti: omap5: Add missing AESS clock".
>>> 
>>> arch/arm/boot/dts/omap5-l4-abe.dtsi | 16 ++++++++++++++--
>>> 1 file changed, 14 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/arch/arm/boot/dts/omap5-l4-abe.dtsi b/arch/arm/boot/dts/omap5-l4-abe.dtsi
>>> --- a/arch/arm/boot/dts/omap5-l4-abe.dtsi
>>> +++ b/arch/arm/boot/dts/omap5-l4-abe.dtsi
>>> @@ -426,8 +426,20 @@ target-module@c0000 {			/* 0x401c0000, ap 30 1e.0 */
>>> 		};
>>> 
>>> 		target-module@f1000 {			/* 0x401f1000, ap 32 20.0 */
>> 
>> Here its may be good to have an "aess" label.
> 
> 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.

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

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";
	};
};

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

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";
		*/
	};
};

BR,
Nikolaus