diff mbox series

[net-next,v2,1/2] dt-bindings: net: dp83822: Add support for GPIO2 clock output

Message ID 20241211-dp83822-gpio2-clk-out-v2-1-614a54f6acab@liebherr.com (mailing list archive)
State New
Delegated to: Netdev Maintainers
Headers show
Series net: phy: dp83822: Add support for GPIO2 clock output | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success Errors and warnings before: 0 (+0) this patch: 0 (+0)
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 1 this patch: 1
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 5 this patch: 5
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-12-11--15-00 (tests: 764)

Commit Message

Dimitri Fedrau via B4 Relay Dec. 11, 2024, 8:04 a.m. UTC 
From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>

The GPIO2 pin on the DP83822 can be configured as clock output. Add
binding to support this feature.

Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
 .../devicetree/bindings/net/ti,dp83822.yaml         |  7 +++++++
 include/dt-bindings/net/ti-dp83822.h                | 21 +++++++++++++++++++++
 2 files changed, 28 insertions(+)

Comments

Krzysztof Kozlowski Dec. 11, 2024, 9:43 a.m. UTC | #1 
On Wed, Dec 11, 2024 at 09:04:39AM +0100, Dimitri Fedrau wrote:
> The GPIO2 pin on the DP83822 can be configured as clock output. Add
> binding to support this feature.
> 
> Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> ---
>  .../devicetree/bindings/net/ti,dp83822.yaml         |  7 +++++++
>  include/dt-bindings/net/ti-dp83822.h                | 21 +++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ti,dp83822.yaml b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> index 784866ea392b2083e93d8dc9aaea93b70dc80934..4a4dc794f21162c6a61c3daeeffa08e666034679 100644
> --- a/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> +++ b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> @@ -96,6 +96,13 @@ properties:
>        - master
>        - slave
>  
> +  ti,gpio2-clk-out:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +       DP83822 PHY only.
> +       Muxing option for GPIO2 pin. See dt-bindings/net/ti-dp83822.h for
> +       applicable values. When omitted, the PHY's default will be left as is.

1. Missing constraints, this looks like enum.
2. Missing explanation of values.
3. This should be most likely a string.
4. Extend your example with this. 

> +
>  required:
>    - reg
>  
> diff --git a/include/dt-bindings/net/ti-dp83822.h b/include/dt-bindings/net/ti-dp83822.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..d569c90618b7bcae9ffe44eb041f7dae2e74e5d1
> --- /dev/null
> +++ b/include/dt-bindings/net/ti-dp83822.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
> +/*
> + * Device Tree constants for the Texas Instruments DP83822 PHY
> + *
> + * Copyright (C) 2024 Liebherr-Electronics and Drives GmbH
> + *
> + * Author: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> + */
> +
> +#ifndef _DT_BINDINGS_TI_DP83822_H
> +#define _DT_BINDINGS_TI_DP83822_H
> +
> +/* IO_MUX_GPIO_CTRL - Clock source selection */
> +#define DP83822_CLK_SRC_MAC_IF			0x0
> +#define DP83822_CLK_SRC_XI			0x1
> +#define DP83822_CLK_SRC_INT_REF			0x2
> +#define DP83822_CLK_SRC_RMII_MASTER_MODE_REF	0x4
> +#define DP83822_CLK_SRC_FREE_RUNNING		0x6
> +#define DP83822_CLK_SRC_RECOVERED		0x7

These are not really bindings but some register values. Hex numbers
indicate that. Don't store register values as bindings, because this
is neither necessary nor helping.

Best regards,
Krzysztof
Dimitri Fedrau Dec. 11, 2024, 10:51 a.m. UTC | #2 
Am Wed, Dec 11, 2024 at 10:43:40AM +0100 schrieb Krzysztof Kozlowski:
> On Wed, Dec 11, 2024 at 09:04:39AM +0100, Dimitri Fedrau wrote:
> > The GPIO2 pin on the DP83822 can be configured as clock output. Add
> > binding to support this feature.
> > 
> > Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > ---
> >  .../devicetree/bindings/net/ti,dp83822.yaml         |  7 +++++++
> >  include/dt-bindings/net/ti-dp83822.h                | 21 +++++++++++++++++++++
> >  2 files changed, 28 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/ti,dp83822.yaml b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> > index 784866ea392b2083e93d8dc9aaea93b70dc80934..4a4dc794f21162c6a61c3daeeffa08e666034679 100644
> > --- a/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> > +++ b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> > @@ -96,6 +96,13 @@ properties:
> >        - master
> >        - slave
> >  
> > +  ti,gpio2-clk-out:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +       DP83822 PHY only.
> > +       Muxing option for GPIO2 pin. See dt-bindings/net/ti-dp83822.h for
> > +       applicable values. When omitted, the PHY's default will be left as is.
> 
> 1. Missing constraints, this looks like enum.
> 2. Missing explanation of values.
> 3. This should be most likely a string.
> 4. Extend your example with this. 
>
Ok, will fix it.

> > +
> >  required:
> >    - reg
> >  
> > diff --git a/include/dt-bindings/net/ti-dp83822.h b/include/dt-bindings/net/ti-dp83822.h
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..d569c90618b7bcae9ffe44eb041f7dae2e74e5d1
> > --- /dev/null
> > +++ b/include/dt-bindings/net/ti-dp83822.h
> > @@ -0,0 +1,21 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
> > +/*
> > + * Device Tree constants for the Texas Instruments DP83822 PHY
> > + *
> > + * Copyright (C) 2024 Liebherr-Electronics and Drives GmbH
> > + *
> > + * Author: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > + */
> > +
> > +#ifndef _DT_BINDINGS_TI_DP83822_H
> > +#define _DT_BINDINGS_TI_DP83822_H
> > +
> > +/* IO_MUX_GPIO_CTRL - Clock source selection */
> > +#define DP83822_CLK_SRC_MAC_IF			0x0
> > +#define DP83822_CLK_SRC_XI			0x1
> > +#define DP83822_CLK_SRC_INT_REF			0x2
> > +#define DP83822_CLK_SRC_RMII_MASTER_MODE_REF	0x4
> > +#define DP83822_CLK_SRC_FREE_RUNNING		0x6
> > +#define DP83822_CLK_SRC_RECOVERED		0x7
> 
> These are not really bindings but some register values. Hex numbers
> indicate that. Don't store register values as bindings, because this
> is neither necessary nor helping.
>
Ok, got it. Have seen similar in <dt-bindings/net/ti-dp83867.h> or
<dt-bindings/net/ti-dp83869.h>, is it wrong there ?

Best regards,
Dimitri Fedrau
Krzysztof Kozlowski Dec. 11, 2024, 1:40 p.m. UTC | #3 
On 11/12/2024 11:51, Dimitri Fedrau wrote:
>>> +
>>> +/* IO_MUX_GPIO_CTRL - Clock source selection */
>>> +#define DP83822_CLK_SRC_MAC_IF			0x0
>>> +#define DP83822_CLK_SRC_XI			0x1
>>> +#define DP83822_CLK_SRC_INT_REF			0x2
>>> +#define DP83822_CLK_SRC_RMII_MASTER_MODE_REF	0x4
>>> +#define DP83822_CLK_SRC_FREE_RUNNING		0x6
>>> +#define DP83822_CLK_SRC_RECOVERED		0x7
>>
>> These are not really bindings but some register values. Hex numbers
>> indicate that. Don't store register values as bindings, because this
>> is neither necessary nor helping.
>>
> Ok, got it. Have seen similar in <dt-bindings/net/ti-dp83867.h> or
> <dt-bindings/net/ti-dp83869.h>, is it wrong there ?


Yes, it is. Rules were much more relaxed 10 years ago but these were not
even sent for DT review and did not receive any ack/review.
> 
> Best regards,
> Dimitri Fedrau


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/ti,dp83822.yaml b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
index 784866ea392b2083e93d8dc9aaea93b70dc80934..4a4dc794f21162c6a61c3daeeffa08e666034679 100644
--- a/Documentation/devicetree/bindings/net/ti,dp83822.yaml
+++ b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
@@ -96,6 +96,13 @@  properties:
       - master
       - slave
 
+  ti,gpio2-clk-out:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+       DP83822 PHY only.
+       Muxing option for GPIO2 pin. See dt-bindings/net/ti-dp83822.h for
+       applicable values. When omitted, the PHY's default will be left as is.
+
 required:
   - reg
 
diff --git a/include/dt-bindings/net/ti-dp83822.h b/include/dt-bindings/net/ti-dp83822.h
new file mode 100644
index 0000000000000000000000000000000000000000..d569c90618b7bcae9ffe44eb041f7dae2e74e5d1
--- /dev/null
+++ b/include/dt-bindings/net/ti-dp83822.h
@@ -0,0 +1,21 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
+/*
+ * Device Tree constants for the Texas Instruments DP83822 PHY
+ *
+ * Copyright (C) 2024 Liebherr-Electronics and Drives GmbH
+ *
+ * Author: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
+ */
+
+#ifndef _DT_BINDINGS_TI_DP83822_H
+#define _DT_BINDINGS_TI_DP83822_H
+
+/* IO_MUX_GPIO_CTRL - Clock source selection */
+#define DP83822_CLK_SRC_MAC_IF			0x0
+#define DP83822_CLK_SRC_XI			0x1
+#define DP83822_CLK_SRC_INT_REF			0x2
+#define DP83822_CLK_SRC_RMII_MASTER_MODE_REF	0x4
+#define DP83822_CLK_SRC_FREE_RUNNING		0x6
+#define DP83822_CLK_SRC_RECOVERED		0x7
+
+#endif