Received-SPF: pass (google.com: domain of linux-kernel+bounces-103051-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
References: <20240312-basic_dt-v1-0-7f11df3a0896@amlogic.com>
 <20240312-basic_dt-v1-3-7f11df3a0896@amlogic.com>
 <1jsf0vephv.fsf@starbuckisacylon.baylibre.com>
 <9a38fd52-5eea-468d-bd7c-29a505503268@amlogic.com>
User-agent: mu4e 1.10.8; emacs 29.2
From: Jerome Brunet <jbrunet@baylibre.com>
To: Xianwei Zhao <xianwei.zhao@amlogic.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>, Rob Herring <robh+dt@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley
 <conor+dt@kernel.org>, Neil Armstrong <neil.armstrong@linaro.org>, Martin
 Blumenstingl <martin.blumenstingl@googlemail.com>, Kevin Hilman
 <khilman@baylibre.com>, devicetree@vger.kernel.org,
 linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
 linux-amlogic@lists.infradead.org
Subject: Re: [PATCH 3/4] arm64: dts: add support for A4 based Amlogic BA400
Date: Thu, 14 Mar 2024 10:26:26 +0100
In-reply-to: <9a38fd52-5eea-468d-bd7c-29a505503268@amlogic.com>
Message-ID: <1jcyrxi2l0.fsf@starbuckisacylon.baylibre.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain


On Thu 14 Mar 2024 at 16:08, Xianwei Zhao <xianwei.zhao@amlogic.com> wrote:

>>> +
>>> +#include "amlogic-a4.dtsi"
>> Could you describe how the a4 and a5 differs from each other ?
>> The description given in the commit description is the same.
>> Beside the a53 vs a55, I'm not seeing much of a difference.
>> Admittedly, there is not much yet but I wonder if a4 and a5 should have
>> a common dtsi.
>> 
> They are mostly the same, A5 include HiFi-DSP and NPU, but A4 is not. And
> some peripheral modules are different, such as SPI and Ehernet phy.
>
> I would like to wait for the follow-on chips to come out before considering
> a merger with common dtsi file.
>

No, Please do it now. There is no reason for the community to review the
same thing twice if the SoCs are "mostly the same".

>>> +
>>> +/ {
>>> +     model = "Amlogic A113L2 ba400 Development Board";
>>> +     compatible = "amlogic,ba400","amlogic,a4";
>>> +     interrupt-parent = <&gic>;
>>> +     #address-cells = <2>;
>>> +     #size-cells = <2>;
>>> +
>>> +     aliases {
>>> +             serial0 = &uart_b;
>>> +     };
>>> +
>>> +     memory@0 {
>>> +             device_type = "memory";
>>> +             reg = <0x0 0x0 0x0 0x40000000>;
>>> +     };
>>> +
>>> +     reserved-memory {
>>> +             #address-cells = <2>;
>>> +             #size-cells = <2>;
>>> +             ranges;
>>> +
>>> +             /* 52 MiB reserved for ARM Trusted Firmware */
>> That's a lot of memory to blindly reserve.
>> Any chance we can stop doing that and have u-boot amend reserved memory
>> zone based on the actual needs of the device ?
> Yes. U-boot will change size of reserved memory base on actual usage.

Then u-boot should add (not change) the memory if necessary.
Please drop this.

>> 
>>> +             secmon_reserved:linux,secmon {
>>> +                     compatible = "shared-dma-pool";
>>> +                     no-map;
>>> +                     alignment = <0x0 0x400000>;
>>> +                     reg = <0x0 0x05000000 0x0 0x3400000>;
>>> +             };
>>> +     };
>>> +};
>>> +