On 19/03/2025 11:38, Jiebing Chen wrote: >>> }; >>> >>> &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? > > The following files are included that the audio driver depends on Do you understand how emails work and patch review? I commented under your change, not unrelated code being there already. > > it is same as sm1 chip > >>> +#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. > > I didn't understand where you were referring to, I'm guessing that's > what it was about Read your patch. > > the following changes to tdmif_a > > old: > > clock-names = "mclk", "sclk", "lrclk"; > > new: > clock-names = "sclk", "lrclk","mclk"; > it fix warning How is this related to the patch hunk? Do you understand how patch format works? ... >>> timer { >>> compatible = "arm,armv8-timer"; >>> interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, >>> @@ -101,7 +135,6 @@ apb4: bus@fe000000 { >>> #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? You did not respond here, so I assume you will fix this and do intensive review before posting next version. >> >>> clkc_periphs: clock-controller@0 { >>> compatible = "amlogic,s4-peripherals-clkc"; >>> reg = <0x0 0x0 0x0 0x49c>; >>> @@ -134,6 +167,17 @@ clkc_pll: clock-controller@8000 { >>> #clock-cells = <1>; >>> }; >>> >>> + acodec: audio-controller@1a000 { >>> + 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@2100 { >>> compatible = "amlogic,s4-wdt", "amlogic,t7-wdt"; >>> reg = <0x0 0x2100 0x0 0x10>; >>> @@ -850,3 +894,327 @@ emmc: mmc@fe08c000 { >>> }; >>> }; >>> }; >>> + >>> +&apb4 { >>> + audio: bus@330000 { >>> + compatible = "simple-bus"; >>> + reg = <0x0 0x330000 0x0 0x1000>; >> That's not a simple bus in such case. > > these code base on old dts like sm1/g12a, we didn't easily change any of > the relevant properties > > To be consistent with the previous one Still NAK. You cannot add bugs just to be consistent. Best regards, Krzysztof