Re: [RFC PATCH 15/16] ARM: dts: rockchip: Add rk3588-u-boot.dtsi

[Eugen Hristev <eugen.hristev@collabora.com>]

On 1/27/23 15:37, Jagan Teki wrote:
> On Fri, 27 Jan 2023 at 19:03, Eugen Hristev <eugen.hristev@collabora.com> wrote:
>>
>> Hello Jagan,
>>
>>
>> On 1/26/23 00:27, Jagan Teki wrote:
>>> Add u-boot,dm-spl and u-boot,dm-pre-reloc related properties
>>> for Rockchip RK3588 SoC.
>>
>> It appears this file/commit does more than just adding u-boot,* properties
>>
>>>
>>> Signed-off-by: Jagan Teki <jagan@edgeble.ai>
>>> ---
>>>    arch/arm/dts/rk3588-u-boot.dtsi | 101 ++++++++++++++++++++++++++++++++
>>>    1 file changed, 101 insertions(+)
>>>    create mode 100644 arch/arm/dts/rk3588-u-boot.dtsi
>>>
>>> diff --git a/arch/arm/dts/rk3588-u-boot.dtsi b/arch/arm/dts/rk3588-u-boot.dtsi
>>> new file mode 100644
>>> index 0000000000..b5cc4dcc60
>>> --- /dev/null
>>> +++ b/arch/arm/dts/rk3588-u-boot.dtsi
>>> @@ -0,0 +1,101 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Copyright (c) 2022 Edgeble AI Technologies Pvt. Ltd.
>>> + */
>>> +
>>> +#include "rockchip-u-boot.dtsi"
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +
>>> +/ {
>>> +     dmc {
>>> +             compatible = "rockchip,rk3588-dmc";
>>> +             u-boot,dm-pre-reloc;
>>> +             status = "okay";
>>> +     };
>>> +
>>> +     pmu1_grf: syscon@fd58a000 {
>>> +             u-boot,dm-pre-reloc;
>>> +             compatible = "rockchip,rk3588-pmu1-grf", "syscon";
>>> +             reg = <0x0 0xfd58a000 0x0 0x2000>;
>>> +     };
>>> +
>>> +     sdmmc: mmc@fe2c0000 {
>>
>> Just for my understanding, why are you adding here a new node sdmmc, and
>> have it disabled, while you have another node called 'sdhci' which you
>> enable further down in the file ?
>> What is the purpose of this sdmmc node? and defining it here.
> 
> In order to boot SDCard, linux doesn't support this node yet so once
> Linux has it we can sync and drop this from here. Idea is to have
> devicetree files be the same as in Linux in order to maintain them
> properly. As long as the specific node is not supported in Linux that
> would be considered as -u-boot.dtsi specific.

Okay, but the node is disabled.. while you enable the 'sdhci' node below 
in the file.

> 
> Jagan.