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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <dab318496a04572d2d770eea29b1f0f0@manjaro.org>
Date: Sat, 18 Jan 2025 10:34:50 +0100
From: Dragan Simic <dsimic@...jaro.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Diederik de Haas <didi.debian@...ow.org>, Peter Geis
 <pgwipeout@...il.com>, Heiko Stuebner <heiko@...ech.de>, zyw@...k-chips.com,
 kever.yang@...k-chips.com, frank.wang@...k-chips.com,
 william.wu@...k-chips.com, wulf@...k-chips.com,
 linux-rockchip@...ts.infradead.org, Alex Bee <knaerzche@...il.com>, Conor
 Dooley <conor+dt@...nel.org>, Johan Jonker <jbx6244@...il.com>, Jonas
 Karlman <jonas@...boo.se>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Rob
 Herring <robh@...nel.org>, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v1 4/6] arm64: dts: rockchip: add rk3328 usb3 phy node

On 2025-01-18 10:19, Krzysztof Kozlowski wrote:
> 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@...il.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@...60000 {
>>>>> +		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.

I fully agree with the readability being the most important factor when
it comes to deciding on the column width.  That's very well illustrated
by the example above, i.e. the list items in device trees, which are 
much
more readable when the items are placed in separate lines.

Though, as I wrote in my earlier response, enforcing the 80-column limit
in C and headers files rather often leads to line breaks that are 
obviously
"artificial" and do nothing but make the code less readable.  That's 
where
the 100-column with limit often improves the readability.