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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <2ea32d85-1d96-4b40-909f-053f309861d4@amlogic.com>
Date: Thu, 20 Mar 2025 10:31:03 +0800
From: Jiebing Chen <jiebing.chen@...ogic.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
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


在 2025/3/20 3:31, Krzysztof Kozlowski 写道:
> [ EXTERNAL EMAIL ]
>
> 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.
Sorry, I will check the rule. Thanks you!
>> 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@...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?
> 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@...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.
>> 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