linux-kernel - Re: [PATCH v4 6/6] arm64: dts: amlogic: Add Amlogic S4 Audio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250319-quizzical-coyote-of-assurance-d4c91d@krzk-bin>
Date: Wed, 19 Mar 2025 09:26:31 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: jiebing chen <jiebing.chen@...ogic.com>
Cc: Jerome Brunet <jbrunet@...libre.com>, 
	Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>, 
	Neil Armstrong <neil.armstrong@...aro.org>, Kevin Hilman <khilman@...libre.com>, 
	Martin Blumenstingl <martin.blumenstingl@...glemail.com>, Michael Turquette <mturquette@...libre.com>, 
	Stephen Boyd <sboyd@...nel.org>, linux-sound@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-amlogic@...ts.infradead.org, linux-clk@...r.kernel.org, jian.xu@...ogic.com, 
	shuai.li@...ogic.com, zhe.wang@...ogic.com
Subject: Re: [PATCH v4 6/6] arm64: dts: amlogic: Add Amlogic S4 Audio

On Wed, Mar 19, 2025 at 03:04:49PM +0800, jiebing chen wrote:
> Add basic audio driver support for the Amlogic S4 based
> Amlogic AQ222 board. use hifipll pll (1179648000) to
> support 768k sample rate and 24 bit (s24_le), 24bit sclk
> is 48fs, use mpll0 (270950400) to support 705.6k sample
> rate and 32bit, use mpll1 (338688000) to support 705.6k
> and 24bit.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

> 
> Signed-off-by: jiebing chen <jiebing.chen@...ogic.com>

...

> +
> +		dai-link-12 {
> +			sound-dai = <&toacodec TOACODEC_OUT>;
> +
> +			codec {
> +				sound-dai = <&acodec>;
> +			};
> +		};
> +	};
> +

Do not add stray blank lines.

>  };
>  
>  &pwm_ef {
> diff --git a/arch/arm64/boot/dts/amlogic/meson-s4.dtsi b/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
> index 957577d986c0675a503115e1ccbc4387c2051620..83edafc2646438e3e6b1945fa1c4b327254a4131 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
> @@ -11,7 +11,11 @@
>  #include <dt-bindings/clock/amlogic,s4-peripherals-clkc.h>
>  #include <dt-bindings/power/meson-s4-power.h>
>  #include <dt-bindings/reset/amlogic,meson-s4-reset.h>
> -

Why?

> +#include <dt-bindings/clock/axg-audio-clkc.h>
> +#include <dt-bindings/reset/amlogic,meson-axg-audio-arb.h>
> +#include <dt-bindings/reset/amlogic,meson-g12a-audio-reset.h>
> +#include <dt-bindings/sound/meson-g12a-toacodec.h>
> +#include <dt-bindings/sound/meson-g12a-tohdmitx.h>

Old style was correct.

>  / {
>  	cpus {
>  		#address-cells = <2>;
> @@ -46,6 +50,36 @@ cpu3: cpu@3 {
>  		};
>  	};
>  
> +	tdmif_a: audio-controller-0 {
> +		compatible = "amlogic,axg-tdm-iface";
> +		#sound-dai-cells = <0>;
> +		sound-name-prefix = "TDM_A";
> +		clocks = <&clkc_audio AUD_CLKID_MST_A_SCLK>,
> +			 <&clkc_audio AUD_CLKID_MST_A_LRCLK>,
> +			 <&clkc_audio AUD_CLKID_MST_A_MCLK>;
> +		clock-names = "sclk", "lrclk","mclk";
> +	};
> +
> +	tdmif_b: audio-controller-1 {
> +		compatible = "amlogic,axg-tdm-iface";
> +		#sound-dai-cells = <0>;
> +		sound-name-prefix = "TDM_B";
> +		clocks = <&clkc_audio AUD_CLKID_MST_A_SCLK>,
> +			 <&clkc_audio AUD_CLKID_MST_B_LRCLK>,
> +			 <&clkc_audio AUD_CLKID_MST_B_MCLK>;
> +		clock-names = "sclk", "lrclk","mclk";
> +	};
> +
> +	tdmif_c: audio-controller-2 {
> +		compatible = "amlogic,axg-tdm-iface";
> +		#sound-dai-cells = <0>;
> +		sound-name-prefix = "TDM_C";
> +		clocks = <&clkc_audio AUD_CLKID_MST_C_SCLK>,
> +			 <&clkc_audio AUD_CLKID_MST_C_LRCLK>,
> +			 <&clkc_audio AUD_CLKID_MST_C_MCLK>;
> +		clock-names = "sclk", "lrclk","mclk";
> +	};
> +
>  	timer {
>  		compatible = "arm,armv8-timer";
>  		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> @@ -101,7 +135,6 @@ apb4: bus@...00000 {
>  			#address-cells = <2>;
>  			#size-cells = <2>;
>  			ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x480000>;
> -

Why? What is happening in this patch - why are you changing so many
other pieces?

>  			clkc_periphs: clock-controller@0 {
>  				compatible = "amlogic,s4-peripherals-clkc";
>  				reg = <0x0 0x0 0x0 0x49c>;
> @@ -134,6 +167,17 @@ clkc_pll: clock-controller@...0 {
>  				#clock-cells = <1>;
>  			};
>  
> +			acodec: audio-controller@...00 {
> +				compatible = "amlogic,t9015";
> +				reg = <0x0 0x1A000 0x0 0x14>;
> +				#sound-dai-cells = <0>;
> +				sound-name-prefix = "ACODEC";
> +				clocks = <&clkc_periphs CLKID_ACODEC>;
> +				clock-names = "pclk";
> +				resets = <&reset RESET_ACODEC>;
> +				AVDD-supply = <&vddio_ao1v8>;
> +			};
> +
>  			watchdog@...0 {
>  				compatible = "amlogic,s4-wdt", "amlogic,t7-wdt";
>  				reg = <0x0 0x2100 0x0 0x10>;
> @@ -850,3 +894,327 @@ emmc: mmc@...8c000 {
>  		};
>  	};
>  };
> +
> +&apb4 {
> +	audio: bus@...000 {
> +		compatible = "simple-bus";
> +		reg = <0x0 0x330000 0x0 0x1000>;

That's not a simple bus in such case.

NAK


Best regards,
Krzysztof