Re: [PATCH] arm64: dts: meson-sm1: add spdifin spdifout nodes

On Wed 15 Dec 2021 at 10:32, Art Nikpal [off-list ref] wrote:

On Mon, Oct 4, 2021 at 9:51 PM Jerome Brunet [off-list ref] wrote:

quoted

On Mon 06 Sep 2021 at 19:56, Art Nikpal [off-list ref] wrote:

quoted

quoted

It would be nice to indicate how it was tested ?

https://github.com/khadas/khadas-linux-kernel/blob/master/patches/linux-5.14-rc5.sound/0001-arm64-dts-meson-khadas-vim3-remake-simple-sound-for-.patch

i have test it

i2s  and spdif output from gpio headers - works well

You have tested spdifout_a then.

Nothing says spdifin (which was only tested on the axg series so far) or
spdifout_b would actually work if enabled at a later point.

I would prefer if things could at least be tested once before being
added DT.

quoted

On Mon, Sep 6, 2021 at 5:29 PM Jerome Brunet [off-list ref] wrote:

quoted

On Thu 02 Sep 2021 at 15:31, Neil Armstrong [off-list ref] wrote:

quoted

Hi,

On 11/08/2021 07:09, Artem Lapkin wrote:

quoted

Add spdifin spdifout spdifout_b nodes for Amlogic SM1 SoCs.

Signed-off-by: Artem Lapkin <redacted>
---
 arch/arm64/boot/dts/amlogic/meson-sm1.dtsi | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1.dtsi b/arch/arm64/boot/dts/amlogic/meson-sm1.dtsi
index 3d8b1f4f2..1efdbb61e 100644
--- a/arch/arm64/boot/dts/amlogic/meson-sm1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-sm1.dtsi

@@ -356,6 +356,33 @@ tdmin_lb: audio-controller@3c0 {
                     status = "disabled";
             };

+            spdifin: audio-controller@400 {
+                    compatible = "amlogic,g12a-spdifin",
+                    "amlogic,axg-spdifin";
+                    reg = <0x0 0x400 0x0 0x30>;
+                    #sound-dai-cells = <0>;
+                    sound-name-prefix = "SPDIFIN";
+                    interrupts = <GIC_SPI 151 IRQ_TYPE_EDGE_RISING>;
+                    clocks = <&clkc_audio AUD_CLKID_SPDIFIN>,
+                    <&clkc_audio AUD_CLKID_SPDIFIN_CLK>;
+                    clock-names = "pclk", "refclk";
+                    resets = <&clkc_audio AUD_RESET_SPDIFIN>;
+                    status = "disabled";
+            };
+
+            spdifout: audio-controller@480 {
+                    compatible = "amlogic,g12a-spdifout",
+                    "amlogic,axg-spdifout";
+                    reg = <0x0 0x480 0x0 0x50>;
+                    #sound-dai-cells = <0>;
+                    sound-name-prefix = "SPDIFOUT";
+                    clocks = <&clkc_audio AUD_CLKID_SPDIFOUT>,
+                    <&clkc_audio AUD_CLKID_SPDIFOUT_CLK>;
+                    clock-names = "pclk", "mclk";
+                    resets = <&clkc_audio AUD_RESET_SPDIFOUT>;
+                    status = "disabled";
+            };
+
             tdmout_a: audio-controller@500 {
                     compatible = "amlogic,sm1-tdmout";
                     reg = <0x0 0x500 0x0 0x40>;

@@ -401,6 +428,19 @@ tdmout_c: audio-controller@580 {
                     status = "disabled";
             };

+            spdifout_b: audio-controller@680 {
+                    compatible = "amlogic,g12a-spdifout",
+                    "amlogic,axg-spdifout";
+                    reg = <0x0 0x680 0x0 0x50>;
+                    #sound-dai-cells = <0>;
+                    sound-name-prefix = "SPDIFOUT_B";
+                    clocks = <&clkc_audio AUD_CLKID_SPDIFOUT_B>,
+                    <&clkc_audio AUD_CLKID_SPDIFOUT_B_CLK>;
+                    clock-names = "pclk", "mclk";
+                    resets = <&clkc_audio AUD_RESET_SPDIFOUT_B>;
+                    status = "disabled";
+            };
+
             toacodec: audio-controller@740 {
                     compatible = "amlogic,sm1-toacodec",
                                  "amlogic,g12a-toacodec";

Jerome could you quickly review this ?

Just this, without the related card change is not usefull as it won't be
part of the card.

Current sound card dt configuration not configured for it, but we can
apply dt overlays anytime.

Maybe you should do so

IMHO: current sound configuration is not ideal and for some tasks
better to use custom configuration.
but anyway i think meson-sm1.dtsi must be completed by missed spdif in/out nodes

Tested ones, sure

quoted

quoted

quoted

It would be nice to indicate how it was tested ?

quoted

quoted

quoted

AFAIK:
- spdifout b can only be routed to hdmi and dw-hdmi driver does not
  support that yet

Ok! But why not accept it at this moment - next time will be easy to
improve dw-hdmi driver
+like options i can submit new patch without spdifout_b node

Because as it stands, it is useless. No one tried it. What lands in the
kernel should work, or at least we should do our best to confirm it does.

We should not place code here hoping everything will be fine when someone
finally turns it on.

When dw-hdmi driver support spdif input and you can confirm it works with
your change, you are welcome to submit it again.

quoted

quoted

quoted

- the VIM3 does not have connectors for the spdif (in or out). If it

we have test SPDIF in and SPDIF OUT both works fine on VIM3L meson-sm1

Then adding to spdifin and spdifout_a nodes is fine by me

quoted

quoted

quoted

  requires some extension card, it should be noted somewhere, at least

SPDIF in and SPDIF OUT both pins available via GPIO header ( SPDIF_OUT
PIN 13 and SPDIF_IN PIN 35 - same PWM_F )

Like other thing on those GPIO connectors, we don't enable them by
default.

Now that the kernel accepts overlays, it would be nice to submit one so
other can replicate your test setup, if you wish to.

quoted

quoted

quoted

quoted

Thanks,
Neil

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel