Re: [RFC PATCH v1 4/6] arm64: dts: rockchip: add rk3328 usb3 phy node

[Krzysztof Kozlowski <krzk@kernel.org>]

On 18/01/2025 09:41, Krzysztof Kozlowski wrote:
> On 16/01/2025 17:53, Diederik de Haas wrote:
>> On Thu Jan 16, 2025 at 2:01 PM CET, Krzysztof Kozlowski wrote:
>>> On 15/01/2025 02:26, Peter Geis wrote:
>>>> Add the node for the rk3328 usb3 phy. This node provides a combined usb2
>>>> and usb3 phy which are permenantly tied to the dwc3 usb3 controller.
>>>>
>>>> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
>>>> ---
>>>>
>>>>  arch/arm64/boot/dts/rockchip/rk3328.dtsi | 39 ++++++++++++++++++++++++
>>>>  1 file changed, 39 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
>>>> index 7d992c3c01ce..181a900d41f9 100644
>>>> --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
>>>> @@ -903,6 +903,43 @@ u2phy_host: host-port {
>>>>  		};
>>>>  	};
>>>>  
>>>> +	usb3phy: usb3-phy@ff460000 {
>>>> +		compatible = "rockchip,rk3328-usb3phy";
>>>> +		reg = <0x0 0xff460000 0x0 0x10000>;
>>>> +		clocks = <&cru SCLK_REF_USB3OTG>, <&cru PCLK_USB3PHY_OTG>, <&cru PCLK_USB3PHY_PIPE>;
>>>
>>> Please wrap code according to coding style (checkpatch is not a coding
>>> style description, but only a tool), so at 80.
>>
>> I'm confused: is it 80 or 100?
>>
>> I always thought it was 80, but then I saw several patches/commits by
> 
> Coding style is clear: it is 80. It also has caveat about code
> readability and several maintainers have their own preference.
> 
>> Dragan Simic which deliberately changed code to make use of 100.
>> Being fed up with my own confusion, I submitted a PR to 
>> https://github.com/gregkh/kernel-coding-style/ which got accepted:
>> https://github.com/gregkh/kernel-coding-style/commit/5c21f99dc79883bd0efeba368193180275c9c77a
> 
> That's not kernel. That's Greg...
> 
>>
>> So now both the vim plugins code and README say 100.
>> But as noted in my commit message:
>>
>>   Note that the current upstream 'Linux kernel coding style' does NOT
>>   mention the 100 char limit, but only mentions the preferred max length
>>   of 80.
>>
>> Or is it 100 for code, but 80 for DeviceTree files and bindings?
> 
> From where did you get 100? Checkpatch, right? Kernel coding style is
> clear, there is no discussion, no mentioning 100:
> 
> "The preferred limit on the length of a single line is 80 columns. "
> 
> So to be clear: all DTS, all DT bindings, all code maintained by me and
> some maintainers follows above (and further - there is caveat)
> instruction from coding style. Some maintainers follow other rules and
> that's fine.


Although let me add here caveat, after looking at some other code: DTS
due to its nature of a lot of parent-child relationships combined with
long constants ("GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>") has the strongest
exception or the strongest second part of the coding style:
"...unless exceeding 80 columns significantly increases readability..."

And again: that's from official coding style document (so something
which have been for years), no matter what other people tell you they
think exists since years as coding style.

Splitting line, I commented here in this patch, did not improve readability.

Quite opposite: the line there was less readable in current format thus
it is not even about coding style anymore, but just readability style.
Any list with more than two short entries (by number of characters in
list item) or any list with more than one long entry should be split for
readability. However actual ITEMS in list should not be split - but
again coding style is here very precise since years. 80 unless
significantly increases readability.

Best regards,
Krzysztof