Re: [PATCH 4/4] arm64: dts: add support for A1 based Amlogic AD401

From: Jianxin Pan <jianxin.pan@amlogic•com>
To: Neil Armstrong <narmstrong@baylibre•com>,
	Kevin Hilman <khilman@baylibre•com>,
	<linux-amlogic@lists•infradead.org>
Cc: devicetree@vger•kernel.org, Hanjie Lin <hanjie.lin@amlogic•com>,
	Victor Wan <victor.wan@amlogic•com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail•com>,
	linux-kernel@vger•kernel.org,
	Qiufang Dai <qiufang.dai@amlogic•com>,
	Rob Herring <robh+dt@kernel•org>, Jian Hu <jian.hu@amlogic•com>,
	Xingyu Chen <xingyu.chen@amlogic•com>,
	Carlo Caione <carlo@caione•org>, Tao Zeng <tao.zeng@amlogic•com>,
	linux-arm-kernel@lists•infradead.org,
	Jerome Brunet <jbrunet@baylibre•com>
Subject: Re: [PATCH 4/4] arm64: dts: add support for A1 based Amlogic AD401
Date: Tue, 3 Sep 2019 16:16:57 +0800	[thread overview]
Message-ID: <f06ab4c9-4b86-3b6f-062d-b5fe365bcd10@amlogic.com> (raw)
In-Reply-To: <97a462d6-d98e-f778-96d5-bacd4801df6b@baylibre.com>

Hi Neil,

Thanks for your time.
Please see my comments below.

On 2019/9/3 15:42, Neil Armstrong wrote:
> Hi,
> 
> On 03/09/2019 08:51, Jianxin Pan wrote:
>> Add basic support for the Amlogic A1 based Amlogic AD401 board:
>> which describe components as follows: Reserve Memory, CPU, GIC, IRQ,
[...]
>> +	chosen {
>> +		stdout-path = "serial0:115200n8";
>> +	};
>> +	memory@0 {
>> +		device_type = "memory";
>> +		linux,usable-memory = <0x0 0x0 0x0 0x8000000>;
> 
> I'll prefer usage of reg, it's handled the same but linux,usable-memory
> is not documented.
> 
OK, I will fix it in the next version. Thanks for your review.
>> +	};
>> +};
>> +
>> +&uart_AO_B {
>> +	status = "okay";
>> +	/*pinctrl-0 = <&uart_ao_a_pins>;*/
>> +	/*pinctrl-names = "default";*/
> 
> Please remove these lines instead of commenting them.
> 
OK, I will fix it in the next version.
>> +};
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
>> new file mode 100644
>> index 00000000..b98d648
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
>> @@ -0,0 +1,121 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
>> + */
>> +
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +/ {
[...]
>> +
>> +	reserved-memory {
>> +		#address-cells = <2>;
>> +		#size-cells = <2>;
>> +		ranges;
> 
> Isn't there secmon reserved memory ?
> 
A1 uses internal SRAM as secmon memory.
And there is no secmon reserved memory in ddr side.
>> +
>> +		linux,cma {
>> +			compatible = "shared-dma-pool";
>> +			reusable;
>> +			size = <0x0 0x800000>;
[...]
>>
> 
> Thanks,
> Neil
> 
> .
> 

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