[PATCH] ARM: dts: Configure omap5 AESS

[PATCH] ARM: dts: Configure omap5 AESS

[Tony Lindgren]

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.

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 */
-			compatible = "ti,sysc";
-			status = "disabled";
+			compatible = "ti,sysc-omap4", "ti,sysc";
+			reg = <0xf1000 0x4>,
+			      <0xf1010 0x4>;
+			reg-names = "rev", "sysc";
+			ti,sysc-midle = <SYSC_IDLE_FORCE>,
+					<SYSC_IDLE_NO>,
+					<SYSC_IDLE_SMART>,
+					<SYSC_IDLE_SMART_WKUP>;
+			ti,sysc-sidle = <SYSC_IDLE_FORCE>,
+					<SYSC_IDLE_NO>,
+					<SYSC_IDLE_SMART>;
+			/* Domains (V, P, C): iva, abe_pwrdm, abe_clkdm */
+			clocks = <&abe_clkctrl OMAP5_AESS_CLKCTRL 0>;
+			clock-names = "fck";
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges = <0x0 0xf1000 0x1000>,
-- 
2.24.1

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

[H. Nikolaus Schaller]

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.

> 
> 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.

> -			compatible = "ti,sysc";
> -			status = "disabled";
> +			compatible = "ti,sysc-omap4", "ti,sysc";
> +			reg = <0xf1000 0x4>,
> +			      <0xf1010 0x4>;
> +			reg-names = "rev", "sysc";
> +			ti,sysc-midle = <SYSC_IDLE_FORCE>,
> +					<SYSC_IDLE_NO>,
> +					<SYSC_IDLE_SMART>,
> +					<SYSC_IDLE_SMART_WKUP>;
> +			ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> +					<SYSC_IDLE_NO>,
> +					<SYSC_IDLE_SMART>;
> +			/* Domains (V, P, C): iva, abe_pwrdm, abe_clkdm */
> +			clocks = <&abe_clkctrl OMAP5_AESS_CLKCTRL 0>;
> +			clock-names = "fck";
> 			#address-cells = <1>;
> 			#size-cells = <1>;
> 			ranges = <0x0 0xf1000 0x1000>,
> -- 
> 2.24.1

BR and thanks,
Nikolaus

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

[Tony Lindgren]

* 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.

How about just a comment for aess?

Regards,

Tony

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

[Tony Lindgren]

* H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> [200114 16:38]:
> Hi Tony,
> 
> > Am 14.01.2020 um 16:09 schrieb Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>:
> > 
> > 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-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
> > Cc: Matthijs van Duin <matthijsvanduin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org>
> > Cc: Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org>
> > Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> > ---
> > 
> > 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.

How about just a comment for aess?

Regards,

Tony

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

[H. Nikolaus Schaller]

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

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

[Tony Lindgren]

* 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

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

[Tony Lindgren]

* H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> [200114 17:05]:
> > Am 14.01.2020 um 17:46 schrieb Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>:
> > 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

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

[H. Nikolaus Schaller]

> Am 14.01.2020 um 18:16 schrieb 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

Ok. Will become a major rework of the driver...
On the other hand the audio graph library could simplify a lot of things.

> 
>> 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.

What it might need is to make the aess driver a completely separate module
loaded identified through the compatible record.

> 
>> 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..

That is why I propose a comment around it...
But I am not sure if empty nodes are even allowed.

> If and when the aess driver work the child node can be just added :)

Yes, surely. So it is up to you to decide.

Best regards,
Nikolaus

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

[H. Nikolaus Schaller]

> Am 14.01.2020 um 18:16 schrieb Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>:
> 
> * H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> [200114 17:05]:
>>> Am 14.01.2020 um 17:46 schrieb Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>:
>>> 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

Ok. Will become a major rework of the driver...
On the other hand the audio graph library could simplify a lot of things.

> 
>> 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.

What it might need is to make the aess driver a completely separate module
loaded identified through the compatible record.

> 
>> 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..

That is why I propose a comment around it...
But I am not sure if empty nodes are even allowed.

> If and when the aess driver work the child node can be just added :)

Yes, surely. So it is up to you to decide.

Best regards,
Nikolaus

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

[H. Nikolaus Schaller]

> Am 14.01.2020 um 19:29 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> 
>> Am 14.01.2020 um 18:16 schrieb 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
> 
> Ok. Will become a major rework of the driver...
> On the other hand the audio graph library could simplify a lot of things.
> 
>> 
>>> 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.
> 
> What it might need is to make the aess driver a completely separate module
> loaded identified through the compatible record.

I have checked our tree and it is already built into a separate module with

sound/soc/ti/aess/omap-aess-core.c:	{ .compatible = "ti,omap4-aess", },

So

> target-module@f1000 {			/* 0x401f1000, ap 32 20.0 */
> 	...
> 	aess: aess {
> 		compatible = "ti,omap4-aess";
> 		status = "disabled";
> 	};
> };

would be what we will need.

BR,
Nikolaus

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

[H. Nikolaus Schaller]

> Am 14.01.2020 um 19:29 schrieb H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>:
> 
> 
>> Am 14.01.2020 um 18:16 schrieb Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>:
>> 
>> * H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> [200114 17:05]:
>>>> Am 14.01.2020 um 17:46 schrieb Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>:
>>>> 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
> 
> Ok. Will become a major rework of the driver...
> On the other hand the audio graph library could simplify a lot of things.
> 
>> 
>>> 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.
> 
> What it might need is to make the aess driver a completely separate module
> loaded identified through the compatible record.

I have checked our tree and it is already built into a separate module with

sound/soc/ti/aess/omap-aess-core.c:	{ .compatible = "ti,omap4-aess", },

So

> target-module@f1000 {			/* 0x401f1000, ap 32 20.0 */
> 	...
> 	aess: aess {
> 		compatible = "ti,omap4-aess";
> 		status = "disabled";
> 	};
> };

would be what we will need.

BR,
Nikolaus

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

[Tony Lindgren]

* H. Nikolaus Schaller <hns@goldelico.com> [200114 18:40]:
> I have checked our tree and it is already built into a separate module with
> 
> sound/soc/ti/aess/omap-aess-core.c:	{ .compatible = "ti,omap4-aess", },
> 
> So
> 
> > target-module@f1000 {			/* 0x401f1000, ap 32 20.0 */
> > 	...
> > 	aess: aess {
> > 		compatible = "ti,omap4-aess";
> > 		status = "disabled";
> > 	};
> > };
> 
> would be what we will need.

OK good to hear.

Regards,

Tony

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

[Tony Lindgren]

* H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> [200114 18:40]:
> I have checked our tree and it is already built into a separate module with
> 
> sound/soc/ti/aess/omap-aess-core.c:	{ .compatible = "ti,omap4-aess", },
> 
> So
> 
> > target-module@f1000 {			/* 0x401f1000, ap 32 20.0 */
> > 	...
> > 	aess: aess {
> > 		compatible = "ti,omap4-aess";
> > 		status = "disabled";
> > 	};
> > };
> 
> would be what we will need.

OK good to hear.

Regards,

Tony

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

[H. Nikolaus Schaller]

> Am 14.01.2020 um 22:00 schrieb Tony Lindgren <tony@atomide.com>:
> 
> * H. Nikolaus Schaller <hns@goldelico.com> [200114 18:40]:
>> I have checked our tree and it is already built into a separate module with
>> 
>> sound/soc/ti/aess/omap-aess-core.c:	{ .compatible = "ti,omap4-aess", },
>> 
>> So
>> 
>>> target-module@f1000 {			/* 0x401f1000, ap 32 20.0 */
>>> 	...
>>> 	aess: aess {
>>> 		compatible = "ti,omap4-aess";
>>> 		status = "disabled";
>>> 	};
>>> };
>> 
>> would be what we will need.
> 
> OK good to hear.

I have cleaned up my working tree and added your patches and the one
above (without status = "disabled" and could
a) boot well
b) see that the (non-working) aess driver module is loaded through child node

So you can add my Tested-by: Nikolaus Schaller <hns@goldelico.com>

BR and thanks,
Nikolaus

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

[Tony Lindgren]

* H. Nikolaus Schaller <hns@goldelico.com> [200115 12:50]:
> 
> > Am 14.01.2020 um 22:00 schrieb Tony Lindgren <tony@atomide.com>:
> > 
> > * H. Nikolaus Schaller <hns@goldelico.com> [200114 18:40]:
> >> I have checked our tree and it is already built into a separate module with
> >> 
> >> sound/soc/ti/aess/omap-aess-core.c:	{ .compatible = "ti,omap4-aess", },
> >> 
> >> So
> >> 
> >>> target-module@f1000 {			/* 0x401f1000, ap 32 20.0 */
> >>> 	...
> >>> 	aess: aess {
> >>> 		compatible = "ti,omap4-aess";
> >>> 		status = "disabled";
> >>> 	};
> >>> };
> >> 
> >> would be what we will need.
> > 
> > OK good to hear.
> 
> I have cleaned up my working tree and added your patches and the one
> above (without status = "disabled" and could
> a) boot well
> b) see that the (non-working) aess driver module is loaded through child node
> 
> So you can add my Tested-by: Nikolaus Schaller <hns@goldelico.com>

OK good to hear, thanks for testing!

Tony

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

[Tony Lindgren]

* H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> [200115 12:50]:
> 
> > Am 14.01.2020 um 22:00 schrieb Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>:
> > 
> > * H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> [200114 18:40]:
> >> I have checked our tree and it is already built into a separate module with
> >> 
> >> sound/soc/ti/aess/omap-aess-core.c:	{ .compatible = "ti,omap4-aess", },
> >> 
> >> So
> >> 
> >>> target-module@f1000 {			/* 0x401f1000, ap 32 20.0 */
> >>> 	...
> >>> 	aess: aess {
> >>> 		compatible = "ti,omap4-aess";
> >>> 		status = "disabled";
> >>> 	};
> >>> };
> >> 
> >> would be what we will need.
> > 
> > OK good to hear.
> 
> I have cleaned up my working tree and added your patches and the one
> above (without status = "disabled" and could
> a) boot well
> b) see that the (non-working) aess driver module is loaded through child node
> 
> So you can add my Tested-by: Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>

OK good to hear, thanks for testing!

Tony

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1355 bytes --]

Hi Tony,

I love your patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on omap/for-next balbi-usb/next v5.5-rc6 next-20200115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Tony-Lindgren/ARM-dts-Configure-omap5-AESS/20200115-114737
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> Error: arch/arm/boot/dts/omap5-l4-abe.dtsi:447.27-28 syntax error
   FATAL ERROR: Unable to parse input tree

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 49450 bytes --]

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1388 bytes --]

Hi Tony,

I love your patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on omap/for-next balbi-usb/next v5.5-rc6 next-20200115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Tony-Lindgren/ARM-dts-Configure-omap5-AESS/20200115-114737
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> Error: arch/arm/boot/dts/omap5-l4-abe.dtsi:447.27-28 syntax error
   FATAL ERROR: Unable to parse input tree

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org Intel Corporation

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 49450 bytes --]

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1412 bytes --]

Hi Tony,

I love your patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on omap/for-next balbi-usb/next v5.5-rc6 next-20200115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Tony-Lindgren/ARM-dts-Configure-omap5-AESS/20200115-114737
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

All errors (new ones prefixed by >>):

>> Error: arch/arm/boot/dts/omap5-l4-abe.dtsi:447.27-28 syntax error
   FATAL ERROR: Unable to parse input tree

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 49450 bytes --]

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

[Tony Lindgren]

* kbuild test robot <lkp@intel.com> [200116 15:55]:
> Hi Tony,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on robh/for-next]
> [also build test ERROR on omap/for-next balbi-usb/next v5.5-rc6 next-20200115]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> 
> url:    https://github.com/0day-ci/linux/commits/Tony-Lindgren/ARM-dts-Configure-omap5-AESS/20200115-114737
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
> config: arm-defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (GCC) 7.5.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         GCC_VERSION=7.5.0 make.cross ARCH=arm 
> 
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
> >> Error: arch/arm/boot/dts/omap5-l4-abe.dtsi:447.27-28 syntax error
>    FATAL ERROR: Unable to parse input tree

This patch has a dependency to a clock related patch as described in the
original patch email so this error can be ignored.

Regards,

Tony

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

[Tony Lindgren]

[-- Attachment #1: Type: text/plain, Size: 1474 bytes --]

* kbuild test robot <lkp@intel.com> [200116 15:55]:
> Hi Tony,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on robh/for-next]
> [also build test ERROR on omap/for-next balbi-usb/next v5.5-rc6 next-20200115]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> 
> url:    https://github.com/0day-ci/linux/commits/Tony-Lindgren/ARM-dts-Configure-omap5-AESS/20200115-114737
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
> config: arm-defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (GCC) 7.5.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         GCC_VERSION=7.5.0 make.cross ARCH=arm 
> 
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
> >> Error: arch/arm/boot/dts/omap5-l4-abe.dtsi:447.27-28 syntax error
>    FATAL ERROR: Unable to parse input tree

This patch has a dependency to a clock related patch as described in the
original patch email so this error can be ignored.

Regards,

Tony

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

[Tony Lindgren]

* kbuild test robot <lkp-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> [200116 15:55]:
> Hi Tony,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on robh/for-next]
> [also build test ERROR on omap/for-next balbi-usb/next v5.5-rc6 next-20200115]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> 
> url:    https://github.com/0day-ci/linux/commits/Tony-Lindgren/ARM-dts-Configure-omap5-AESS/20200115-114737
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
> config: arm-defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (GCC) 7.5.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         GCC_VERSION=7.5.0 make.cross ARCH=arm 
> 
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> All errors (new ones prefixed by >>):
> 
> >> Error: arch/arm/boot/dts/omap5-l4-abe.dtsi:447.27-28 syntax error
>    FATAL ERROR: Unable to parse input tree

This patch has a dependency to a clock related patch as described in the
original patch email so this error can be ignored.

Regards,

Tony