Re: [PATCH 3/4] arm64: dts: rockchip: Add base DT for rk3528 SoC

Dragan Simic
2024-08-05T05:22:42Z

On 2024-08-04 17:51, Heiko Stübner wrote:
> Am Sonntag, 4. August 2024, 15:59:19 CEST schrieb Dragan Simic:
>> On 2024-08-04 15:44, Heiko Stübner wrote:
>> > Am Sonntag, 4. August 2024, 15:25:47 CEST schrieb Dragan Simic:
>> >> On 2024-08-04 15:20, Yao Zi wrote:
>> >> > On Sun, Aug 04, 2024 at 12:05:11PM +0200, Krzysztof Kozlowski wrote:
>> >> >> On 03/08/2024 14:55, Yao Zi wrote:
>> >> >> > + xin24m: xin24m {
>> >> >>
>> >> >> Please use name for all fixed clocks which matches current format
>> >> >> recommendation: 'clock-([0-9]+|[a-z0-9-]+)+'
>> >> >
>> >> > Will be fixed in next revision.
>> >>
>> >> Hmm, why should we apply that rule to the xin24m clock, which is
>> >> named exactly like that everywhere else in Rockchip SoC dtsi files?
>> >> It's much better to remain consistent.
>> >
>> > bindings or how we write devicetrees evolve over time ... similarly the
>> > xin24m name comes from more than 10 years ago.
>> >
>> > We also name all those regulator nodes regulator-foo now, which in turn
>> > automatically does enforce a nice sorting rule to keep all the
>> > regulators
>> > around the same area ;-)
>> >
>> > So I don't see a problem of going with xin24m: clock-xin24m {}
>>
>> I agree that using "clock-xin24m" makes more sense in general, but the
>> trouble is that we can't rename the already existing instances of
>> "xin24m",
>> because that has become part of the ABI. Thus, I'm not sure that
>> breaking
>> away from the legacy brings benefits in this particular case.
>
> In the regulator case, we have _new_ boards using the new _node_-names
> but I don't see any renaming of old boards and also don't think we
> should.
>
> But that still does not keep us from using the nicer naming convention
> in
> new boards ;-) .
>
> Same with xin24m. We're talking only about the node-name here. The
> phandle stays the same and also the actual clock name stays the same
> and
> really only the actual node name you need to look for in
> /proc/device-tree
> changes ;-) .
>
> So I don't see the need to go about changing all the old socs, but new
> additions should use improved naming conventions.
>
> xin24m: clock-xin24m {
> compatible = "fixed-clock";
> #clock-cells = <0>;
> clock-frequency = <24000000>;
> clock-output-names = "xin24m";
> };

Makes sense. Though, updating the dtsi files for older SoCs to follow
the new rules, if possible, still remains an itch to me. :)