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
> 
> .
> 

Reply via email to