Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f2652e0e-fb08-efb4-e25a-36a335f0c457@linaro.org>
Date:   Thu, 27 Oct 2022 17:15:21 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.0
Subject: Re: [PATCH v1 3/4] ARM: dts: rockchip: add rk3128.dtsi
Content-Language: en-US
To:     =?UTF-8?Q?Heiko_St=c3=bcbner?= <heiko@sntech.de>,
        Johan Jonker <jbx6244@gmail.com>, kever.yang@rock-chips.com
Cc:     sjg@chromium.org, philipp.tomsich@vrull.eu,
        zhangqing@rock-chips.com, hjc@rock-chips.com, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, daniel.lezcano@linaro.org,
        tglx@linutronix.de, arnd@arndb.de, olof@lixom.net, soc@kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-rockchip@lists.infradead.org
References: <da1252eb-85e9-bdb8-0542-207173523523@gmail.com>
 <2dc46681-894d-4521-bfa7-3e9209691e0a@gmail.com>
 <6fbb01f0-d0d2-bb06-a160-2f8f91ac68ca@linaro.org> <22076018.EfDdHjke4D@diego>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <22076018.EfDdHjke4D@diego>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 27/10/2022 16:02, Heiko Stübner wrote:
> Am Donnerstag, 27. Oktober 2022, 21:43:43 CEST schrieb Krzysztof Kozlowski:
>> On 27/10/2022 13:53, Johan Jonker wrote:
>>> Hi Krzysztof, Kever, Heiko and others,
>>>
>>> On 10/27/22 16:58, Krzysztof Kozlowski wrote:
>>>> On 26/10/2022 20:53, Johan Jonker wrote:
>>>>> Add basic rk3128 support.
>>>>>
>>>>
>>>> Thank you for your patch. There is something to discuss/improve.
>>>
>>> Thank you for your review.
>>>
>>> Some more questions/comments below.
>>>
>>>>
>>>>> +#include <dt-bindings/clock/rk3128-cru.h>
>>>>> +#include <dt-bindings/gpio/gpio.h>
>>>>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>>>> +#include <dt-bindings/pinctrl/rockchip.h>
>>>>> +
>>>>> +/ {
>>>>> +	compatible = "rockchip,rk3128";
>>>>> +	interrupt-parent = <&gic>;
>>>>> +	#address-cells = <1>;
>>>>> +	#size-cells = <1>;
>>>>> +
>>>>> +	aliases {
>>>>> +		gpio0 = &gpio0;
>>>>> +		gpio1 = &gpio1;
>>>>> +		gpio2 = &gpio2;
>>>>> +		gpio3 = &gpio3;
>>>
>>> Is gpio OK here?
>>
>> Could be, but let me rephrase it - why do you need aliases in DTSI? What
>> do these aliases represent?
>>
>> The SoC pieces (nodes in DTSI) do not rely on aliases.
> 
> Subsystems use the aliases for numbering their instances.
> So the i2c0 alias causes the i2c bus getting the number 0 in the operating
> system as well - making it i2c0 there too.

No one argues with these...

> 
> 
>>>>> +		i2c0 = &i2c0;
>>>>> +		i2c1 = &i2c1;
>>>>> +		i2c2 = &i2c2;
>>>>> +		i2c3 = &i2c3;
>>>>> +		spi0 = &spi0;
>>>>> +		serial0 = &uart0;
>>>>> +		serial1 = &uart1;
>>>>> +		serial2 = &uart2;
>>>>
>>>> Bus aliases are board specific and represent what is actually available
>>>> on headers/pins etc. These do not belong to SoC DTSI.
>>>
>>> I just follow current Rockchip DT common practice.
>>>
>>> Do we need to change all Rockchip boards?
>>> Would like to hear from Heiko what's the plan here?
>>> Syncing to U-boot is already a mess...
>>
>> Heiko might have his own preference which then over-rules my
>> recommendation here. But in general this applies to all boards, so other
>> boards could be fixed as well. Different point is whether it is actually
>> worth fixing them...
> 
> I remember only parts of the discussion for the previous socs. Back then
> Arnd was advocating mainly for moving the mmc aliases to boards.
> 
> As the aliases in general also determine the naming of the bus instance,
> I'm very much in favor of having the hardware-i2c5 being named i2c5
> in all cases ;-) . Having these hardware busses getting random numbers
> really calls for chaos.
> 
> So I'd really like us to continue the way we arrived at with the previous
> socs now :-)

No, not only mmc. UART, I2C, SPI - all of these should go to the board.

https://lore.kernel.org/linux-rockchip/CAK8P3a25iYksubCnQb1-e5yj=crEsK37RB9Hn4ZGZMwcVVrG7g@mail.gmail.com/

No one here discusses whether ordering should be random or not.

We discuss that this is a property of the board, not of a SoC.

> 

Best regards,
Krzysztof