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@xxxxxxxxxxxxxx/ 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