diff mbox series

[resend,v4,2/2] dt-bindings: net: snps,dwmac: add clk_csr property

Message ID 20220922092743.22824-3-jianguo.zhang@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Mediatek ethernet patches for mt8188 | expand

Commit Message

Jianguo Zhang Sept. 22, 2022, 9:27 a.m. UTC
The clk_csr property is parsed in driver for generating MDC clock
with correct frequency. A warning('clk_csr' was unexpeted) is reported
when runing 'make_dtbs_check' because the clk_csr property
has been not documented in the binding file.

Signed-off-by: Jianguo Zhang <jianguo.zhang@mediatek.com>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Krzysztof Kozlowski Sept. 22, 2022, 3:07 p.m. UTC | #1
On 22/09/2022 11:27, Jianguo Zhang wrote:
> The clk_csr property is parsed in driver for generating MDC clock
> with correct frequency. A warning('clk_csr' was unexpeted) is reported
> when runing 'make_dtbs_check' because the clk_csr property
> has been not documented in the binding file.
> 

You did not describe the case, but apparently this came with
81311c03ab4d ("net: ethernet: stmmac: add management of clk_csr
property") which never brought the bindings change.

Therefore the property was never part of bindings documentation and
bringing them via driver is not the correct process. It bypasses the
review and such bypass cannot be an argument to bring the property to
bindings. It's not how new properties can be added.

Therefore I don't agree. Please make it a property matching bindings, so
vendor prefix, no underscores in node names.

Driver and DTS need updates.

Best regards,
Krzysztof
Jianguo Zhang Sept. 23, 2022, 1:48 a.m. UTC | #2
Dear Krzysztof,

	Thanks for your comment.

On Thu, 2022-09-22 at 17:07 +0200, Krzysztof Kozlowski wrote:
> On 22/09/2022 11:27, Jianguo Zhang wrote:
> > The clk_csr property is parsed in driver for generating MDC clock
> > with correct frequency. A warning('clk_csr' was unexpeted) is
> > reported
> > when runing 'make_dtbs_check' because the clk_csr property
> > has been not documented in the binding file.
> > 
> 
> You did not describe the case, but apparently this came with
> 81311c03ab4d ("net: ethernet: stmmac: add management of clk_csr
> property") which never brought the bindings change.
> 
> Therefore the property was never part of bindings documentation and
> bringing them via driver is not the correct process. It bypasses the
> review and such bypass cannot be an argument to bring the property to
> bindings. It's not how new properties can be added.
> 
> Therefore I don't agree. Please make it a property matching bindings,
> so
> vendor prefix, no underscores in node names.
> 
> Driver and DTS need updates.
> 
We will rename the property 'clk_csr' as 'snps,clk-csr' and update DTS
& driver to align with the new name in next versions patches.
> Best regards,
> Krzysztof
> 
BRS
Jianguo
Krzysztof Kozlowski Sept. 23, 2022, 9:23 a.m. UTC | #3
On 23/09/2022 03:48, Jianguo Zhang wrote:
> Dear Krzysztof,
> 
> 	Thanks for your comment.
> 
> On Thu, 2022-09-22 at 17:07 +0200, Krzysztof Kozlowski wrote:
>> On 22/09/2022 11:27, Jianguo Zhang wrote:
>>> The clk_csr property is parsed in driver for generating MDC clock
>>> with correct frequency. A warning('clk_csr' was unexpeted) is
>>> reported
>>> when runing 'make_dtbs_check' because the clk_csr property
>>> has been not documented in the binding file.
>>>
>>
>> You did not describe the case, but apparently this came with
>> 81311c03ab4d ("net: ethernet: stmmac: add management of clk_csr
>> property") which never brought the bindings change.
>>
>> Therefore the property was never part of bindings documentation and
>> bringing them via driver is not the correct process. It bypasses the
>> review and such bypass cannot be an argument to bring the property to
>> bindings. It's not how new properties can be added.
>>
>> Therefore I don't agree. Please make it a property matching bindings,
>> so
>> vendor prefix, no underscores in node names.
>>
>> Driver and DTS need updates.
>>
> We will rename the property 'clk_csr' as 'snps,clk-csr' and update DTS
> & driver to align with the new name in next versions patches.

Thanks!

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 491597c02edf..8cff30a8125d 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -288,6 +288,11 @@  properties:
       is supported. For example, this is used in case of SGMII and
       MAC2MAC connection.
 
+  clk_csr:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Frequency division factor for MDC clock.
+
   mdio:
     $ref: mdio.yaml#
     unevaluatedProperties: false