netdev - Re: [PATCH v2 2/2] dt-bindings: net: add new property st,ext-phyclk in documentation for stm32

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: <cf122942-c0fd-457f-a753-366cae39d5f8@linaro.org>
Date: Thu, 14 Mar 2024 16:25:55 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Christophe ROULLIER <christophe.roullier@...s.st.com>,
 "David S . Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Richard Cochran <richardcochran@...il.com>, Jose Abreu
 <joabreu@...opsys.com>, Liam Girdwood <lgirdwood@...il.com>,
 Mark Brown <broonie@...nel.org>
Cc: netdev@...r.kernel.org, devicetree@...r.kernel.org,
 linux-stm32@...md-mailman.stormreply.com,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] dt-bindings: net: add new property st,ext-phyclk
 in documentation for stm32

On 14/03/2024 16:10, Christophe ROULLIER wrote:
> Hi,
> 
> On 3/13/24 14:06, Krzysztof Kozlowski wrote:
>> On 13/03/2024 11:39, Christophe ROULLIER wrote:
>>> On 3/8/24 09:28, Krzysztof Kozlowski wrote:
>>>> On 07/03/2024 14:59, Christophe Roullier wrote:
>>>>> Add property st,ext-phyclk to manage cases when PHY have no cristal/quartz
>>>>> This property can be used with RMII phy without cristal 50Mhz and when we
>>>>> want to select RCC clock instead of ETH_REF_CLK
>>>>> Can be used also with RGMII phy with no cristal and we select RCC clock
>>>>> instead of ETH_CLK125
>>>>>
>>>> Nothing improved here. You say you add new property (wrote it explicitly
>>>> in the subject), but where is it? Where is the user?
>>>>
>>>> I think we talked about this. Rob also asked quite clear:
>>>>
>>>>> That is obvious from the diff. What is not obvious is why we need a new
>>>>> property and what is the problem with the existing ones.
>>>> How did you solve it?
>>> Hi,
>>>
>>> I do not understand your questions.
>> OK, I will clarify some questions, but are you sure that this question:
>> "How did you solve it?"
>> needs clarification?
>>
>> If so, then let me clarify:
>> Rob pointed issue. How did you resolve Rob's comment? How did you
>> address it? What changed in your patch, that Rob's comment should be
>> considered as addressed/resolved/done?
> This property was introduced in 2020 in order to simplify management of 
> all STM32 platforms without Ethernet cristal/quartz PHY.

I fail to see how this answers how did you resolve the comment. You now
described some sort of history, but I am asking: what did you change in
your patches, so Rob's comment is considered resolved?

>>
>> Now about my other question:
>> "but where is it? Where is the user?"
>>
>> Your subject and commit message claim you add new property. This means
>> such property was not existing so far in the Linux kernel. If you add
>> new property in the binding, then I expect adding the user of that
>> binding, thus my question: where is the user of that binding?
>>
> I'm preparing glue and DTS to upstream for STM32MP13 platform, this 
> platform will use with property.
> 
> Since 2020, this property is available in the driver in kernel.org, so 
> it is also possible that someone who has not upstreamed their

This should be explained in commit msg (although not kernel.org, website
does not matter here).

> 
> code also uses it.
> 
>>> That I would like to do, it is property "st,ext-phyclk" was introduced
>>> in driver
>>>
>>> "drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c" in 2020, and YAML
>>> was not updated at the time.
>> Are you saying you document existing property or add a new one?
> Yes, existing property, since 2020 in kernel.org.

Drop the website. We talk here about Linux kernel.

Commit msg fails to explain it in a clear way.

>>
>>> Goal of this patch it is to update YAML to avoid dtbs check issue if
>>> someone use this property :
>>>
>>>    dtbs check issue : views/kernel/upstream/net-next/arch/arm/boot/dts/st/stm32mp157c-dk2.dtb:
>>> ethernet@...0a000: Unevaluated properties are not allowed
>>> ('st,ext-phyclk' was unexpected)
>> So DTS uses it?
> Here it was example, if someone wants to use this property, but today 
> this property is not yet present in DTS in kernel.org

Best regards,
Krzysztof