* [PATCH net-next v2 0/2] net: phy: dp83822: Add support for GPIO2 clock output
@ 2024-12-11 8:04 Dimitri Fedrau via B4 Relay
2024-12-11 8:04 ` [PATCH net-next v2 1/2] dt-bindings: net: " Dimitri Fedrau via B4 Relay
2024-12-11 8:04 ` [PATCH net-next v2 2/2] net: phy: " Dimitri Fedrau via B4 Relay
0 siblings, 2 replies; 8+ messages in thread
From: Dimitri Fedrau via B4 Relay @ 2024-12-11 8:04 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andrew Davis, Andrew Lunn, Heiner Kallweit, Russell King
Cc: netdev, devicetree, linux-kernel, Dimitri Fedrau, Dimitri Fedrau
The DP83822 has several clock configuration options for pins GPIO1, GPIO2
and GPIO3. Clock options include:
- MAC IF clock
- XI clock
- Free-Running clock
- Recovered clock
This patch adds support for GPIO2, the support for GPIO1 and GPIO3 can be
easily added if needed. Code and device tree bindings are derived from
dp83867 which has a similar feature.
Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
Changes in v2:
- Move MII_DP83822_IOCTRL2 before MII_DP83822_GENCFG
- List case statements together, and have one break at the end.
- Move dp83822->set_gpio2_clk_out = true at the end of the validation
- Link to v1: https://lore.kernel.org/r/20241209-dp83822-gpio2-clk-out-v1-0-fd3c8af59ff5@liebherr.com
---
Dimitri Fedrau (2):
dt-bindings: net: dp83822: Add support for GPIO2 clock output
net: phy: dp83822: Add support for GPIO2 clock output
.../devicetree/bindings/net/ti,dp83822.yaml | 7 ++++
drivers/net/phy/dp83822.c | 40 ++++++++++++++++++++++
include/dt-bindings/net/ti-dp83822.h | 21 ++++++++++++
3 files changed, 68 insertions(+)
---
base-commit: 65fb414c93f486cef5408951350f20552113abd0
change-id: 20241206-dp83822-gpio2-clk-out-cd9cf63e37df
Best regards,
--
Dimitri Fedrau <dimitri.fedrau@liebherr.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next v2 1/2] dt-bindings: net: dp83822: Add support for GPIO2 clock output
2024-12-11 8:04 [PATCH net-next v2 0/2] net: phy: dp83822: Add support for GPIO2 clock output Dimitri Fedrau via B4 Relay
@ 2024-12-11 8:04 ` Dimitri Fedrau via B4 Relay
2024-12-11 9:43 ` Krzysztof Kozlowski
2024-12-11 8:04 ` [PATCH net-next v2 2/2] net: phy: " Dimitri Fedrau via B4 Relay
1 sibling, 1 reply; 8+ messages in thread
From: Dimitri Fedrau via B4 Relay @ 2024-12-11 8:04 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andrew Davis, Andrew Lunn, Heiner Kallweit, Russell King
Cc: netdev, devicetree, linux-kernel, Dimitri Fedrau, Dimitri Fedrau
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(+)
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
--
2.39.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next v2 2/2] net: phy: dp83822: Add support for GPIO2 clock output
2024-12-11 8:04 [PATCH net-next v2 0/2] net: phy: dp83822: Add support for GPIO2 clock output Dimitri Fedrau via B4 Relay
2024-12-11 8:04 ` [PATCH net-next v2 1/2] dt-bindings: net: " Dimitri Fedrau via B4 Relay
@ 2024-12-11 8:04 ` Dimitri Fedrau via B4 Relay
2024-12-11 9:45 ` Krzysztof Kozlowski
1 sibling, 1 reply; 8+ messages in thread
From: Dimitri Fedrau via B4 Relay @ 2024-12-11 8:04 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andrew Davis, Andrew Lunn, Heiner Kallweit, Russell King
Cc: netdev, devicetree, linux-kernel, Dimitri Fedrau, Dimitri Fedrau
From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
The GPIO2 pin on the DP83822 can be configured as clock output. Add support
for configuration via DT.
Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
drivers/net/phy/dp83822.c | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 25ee09c48027c86b7d8f4acb5cbe2e157c56a85a..dc5595eae6cc74e5c77914d53772c5fad64c3e70 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -14,6 +14,8 @@
#include <linux/netdevice.h>
#include <linux/bitfield.h>
+#include <dt-bindings/net/ti-dp83822.h>
+
#define DP83822_PHY_ID 0x2000a240
#define DP83825S_PHY_ID 0x2000a140
#define DP83825I_PHY_ID 0x2000a150
@@ -30,6 +32,7 @@
#define MII_DP83822_FCSCR 0x14
#define MII_DP83822_RCSR 0x17
#define MII_DP83822_RESET_CTRL 0x1f
+#define MII_DP83822_IOCTRL2 0x463
#define MII_DP83822_GENCFG 0x465
#define MII_DP83822_SOR1 0x467
@@ -104,6 +107,11 @@
#define DP83822_RX_CLK_SHIFT BIT(12)
#define DP83822_TX_CLK_SHIFT BIT(11)
+/* IOCTRL2 bits */
+#define DP83822_IOCTRL2_GPIO2_CLK_SRC GENMASK(6, 4)
+#define DP83822_IOCTRL2_GPIO2_CTRL GENMASK(2, 0)
+#define DP83822_IOCTRL2_GPIO2_CTRL_CLK_REF GENMASK(1, 0)
+
/* SOR1 mode */
#define DP83822_STRAP_MODE1 0
#define DP83822_STRAP_MODE2 BIT(0)
@@ -139,6 +147,8 @@ struct dp83822_private {
u8 cfg_dac_minus;
u8 cfg_dac_plus;
struct ethtool_wolinfo wol;
+ bool set_gpio2_clk_out;
+ u32 gpio2_clk_out;
};
static int dp83822_config_wol(struct phy_device *phydev,
@@ -413,6 +423,15 @@ static int dp83822_config_init(struct phy_device *phydev)
int err = 0;
int bmcr;
+ if (dp83822->set_gpio2_clk_out)
+ phy_modify_mmd(phydev, MDIO_MMD_VEND2, MII_DP83822_IOCTRL2,
+ DP83822_IOCTRL2_GPIO2_CTRL |
+ DP83822_IOCTRL2_GPIO2_CLK_SRC,
+ FIELD_PREP(DP83822_IOCTRL2_GPIO2_CTRL,
+ DP83822_IOCTRL2_GPIO2_CTRL_CLK_REF) |
+ FIELD_PREP(DP83822_IOCTRL2_GPIO2_CLK_SRC,
+ dp83822->gpio2_clk_out));
+
if (phy_interface_is_rgmii(phydev)) {
rx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
true);
@@ -611,6 +630,7 @@ static int dp83822_of_init(struct phy_device *phydev)
{
struct dp83822_private *dp83822 = phydev->priv;
struct device *dev = &phydev->mdio.dev;
+ int ret;
/* Signal detection for the PHY is only enabled if the FX_EN and the
* SD_EN pins are strapped. Signal detection can only enabled if FX_EN
@@ -623,6 +643,26 @@ static int dp83822_of_init(struct phy_device *phydev)
dp83822->fx_enabled = device_property_present(dev,
"ti,fiber-mode");
+ ret = of_property_read_u32(dev->of_node, "ti,gpio2-clk-out",
+ &dp83822->gpio2_clk_out);
+ if (!ret) {
+ switch (dp83822->gpio2_clk_out) {
+ case DP83822_CLK_SRC_MAC_IF:
+ case DP83822_CLK_SRC_XI:
+ case DP83822_CLK_SRC_INT_REF:
+ case DP83822_CLK_SRC_RMII_MASTER_MODE_REF:
+ case DP83822_CLK_SRC_FREE_RUNNING:
+ case DP83822_CLK_SRC_RECOVERED:
+ break;
+ default:
+ phydev_err(phydev, "ti,gpio2-clk-out value %u not valid\n",
+ dp83822->gpio2_clk_out);
+ return -EINVAL;
+ }
+
+ dp83822->set_gpio2_clk_out = true;
+ }
+
return 0;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2 1/2] dt-bindings: net: dp83822: Add support for GPIO2 clock output
2024-12-11 8:04 ` [PATCH net-next v2 1/2] dt-bindings: net: " Dimitri Fedrau via B4 Relay
@ 2024-12-11 9:43 ` Krzysztof Kozlowski
2024-12-11 10:51 ` Dimitri Fedrau
0 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2024-12-11 9:43 UTC (permalink / raw)
To: Dimitri Fedrau
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andrew Davis, Andrew Lunn, Heiner Kallweit, Russell King, netdev,
devicetree, linux-kernel, Dimitri Fedrau
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2 2/2] net: phy: dp83822: Add support for GPIO2 clock output
2024-12-11 8:04 ` [PATCH net-next v2 2/2] net: phy: " Dimitri Fedrau via B4 Relay
@ 2024-12-11 9:45 ` Krzysztof Kozlowski
2024-12-11 10:52 ` Dimitri Fedrau
0 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2024-12-11 9:45 UTC (permalink / raw)
To: Dimitri Fedrau
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andrew Davis, Andrew Lunn, Heiner Kallweit, Russell King, netdev,
devicetree, linux-kernel, Dimitri Fedrau
On Wed, Dec 11, 2024 at 09:04:40AM +0100, Dimitri Fedrau wrote:
> The GPIO2 pin on the DP83822 can be configured as clock output. Add support
> for configuration via DT.
>
> Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> ---
> drivers/net/phy/dp83822.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> index 25ee09c48027c86b7d8f4acb5cbe2e157c56a85a..dc5595eae6cc74e5c77914d53772c5fad64c3e70 100644
> --- a/drivers/net/phy/dp83822.c
> +++ b/drivers/net/phy/dp83822.c
> @@ -14,6 +14,8 @@
> #include <linux/netdevice.h>
> #include <linux/bitfield.h>
>
> +#include <dt-bindings/net/ti-dp83822.h>
> +
> #define DP83822_PHY_ID 0x2000a240
> #define DP83825S_PHY_ID 0x2000a140
> #define DP83825I_PHY_ID 0x2000a150
> @@ -30,6 +32,7 @@
> #define MII_DP83822_FCSCR 0x14
> #define MII_DP83822_RCSR 0x17
> #define MII_DP83822_RESET_CTRL 0x1f
> +#define MII_DP83822_IOCTRL2 0x463
> #define MII_DP83822_GENCFG 0x465
> #define MII_DP83822_SOR1 0x467
>
> @@ -104,6 +107,11 @@
> #define DP83822_RX_CLK_SHIFT BIT(12)
> #define DP83822_TX_CLK_SHIFT BIT(11)
>
> +/* IOCTRL2 bits */
> +#define DP83822_IOCTRL2_GPIO2_CLK_SRC GENMASK(6, 4)
> +#define DP83822_IOCTRL2_GPIO2_CTRL GENMASK(2, 0)
> +#define DP83822_IOCTRL2_GPIO2_CTRL_CLK_REF GENMASK(1, 0)
> +
> /* SOR1 mode */
> #define DP83822_STRAP_MODE1 0
> #define DP83822_STRAP_MODE2 BIT(0)
> @@ -139,6 +147,8 @@ struct dp83822_private {
> u8 cfg_dac_minus;
> u8 cfg_dac_plus;
> struct ethtool_wolinfo wol;
> + bool set_gpio2_clk_out;
> + u32 gpio2_clk_out;
> };
>
> static int dp83822_config_wol(struct phy_device *phydev,
> @@ -413,6 +423,15 @@ static int dp83822_config_init(struct phy_device *phydev)
> int err = 0;
> int bmcr;
>
> + if (dp83822->set_gpio2_clk_out)
> + phy_modify_mmd(phydev, MDIO_MMD_VEND2, MII_DP83822_IOCTRL2,
> + DP83822_IOCTRL2_GPIO2_CTRL |
> + DP83822_IOCTRL2_GPIO2_CLK_SRC,
> + FIELD_PREP(DP83822_IOCTRL2_GPIO2_CTRL,
> + DP83822_IOCTRL2_GPIO2_CTRL_CLK_REF) |
> + FIELD_PREP(DP83822_IOCTRL2_GPIO2_CLK_SRC,
> + dp83822->gpio2_clk_out));
You include the header but you do not use the defines, so it's a proof
these are register values. Register values are not bindings, they do not
bind anything. Bindings are imaginary numbers starting from 0 or 1 which
are used between drivers and DTS, serving as abstraction layer (or
abstraction values) between these two.
You do not have here abstraction. Drop the bindings header entirely.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2 1/2] dt-bindings: net: dp83822: Add support for GPIO2 clock output
2024-12-11 9:43 ` Krzysztof Kozlowski
@ 2024-12-11 10:51 ` Dimitri Fedrau
2024-12-11 13:40 ` Krzysztof Kozlowski
0 siblings, 1 reply; 8+ messages in thread
From: Dimitri Fedrau @ 2024-12-11 10:51 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Dimitri Fedrau, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andrew Davis, Andrew Lunn, Heiner Kallweit,
Russell King, netdev, devicetree, linux-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2 2/2] net: phy: dp83822: Add support for GPIO2 clock output
2024-12-11 9:45 ` Krzysztof Kozlowski
@ 2024-12-11 10:52 ` Dimitri Fedrau
0 siblings, 0 replies; 8+ messages in thread
From: Dimitri Fedrau @ 2024-12-11 10:52 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Dimitri Fedrau, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andrew Davis, Andrew Lunn, Heiner Kallweit,
Russell King, netdev, devicetree, linux-kernel
Am Wed, Dec 11, 2024 at 10:45:53AM +0100 schrieb Krzysztof Kozlowski:
> On Wed, Dec 11, 2024 at 09:04:40AM +0100, Dimitri Fedrau wrote:
> > The GPIO2 pin on the DP83822 can be configured as clock output. Add support
> > for configuration via DT.
> >
> > Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > ---
> > drivers/net/phy/dp83822.c | 40 ++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 40 insertions(+)
> >
> > diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> > index 25ee09c48027c86b7d8f4acb5cbe2e157c56a85a..dc5595eae6cc74e5c77914d53772c5fad64c3e70 100644
> > --- a/drivers/net/phy/dp83822.c
> > +++ b/drivers/net/phy/dp83822.c
> > @@ -14,6 +14,8 @@
> > #include <linux/netdevice.h>
> > #include <linux/bitfield.h>
> >
> > +#include <dt-bindings/net/ti-dp83822.h>
> > +
> > #define DP83822_PHY_ID 0x2000a240
> > #define DP83825S_PHY_ID 0x2000a140
> > #define DP83825I_PHY_ID 0x2000a150
> > @@ -30,6 +32,7 @@
> > #define MII_DP83822_FCSCR 0x14
> > #define MII_DP83822_RCSR 0x17
> > #define MII_DP83822_RESET_CTRL 0x1f
> > +#define MII_DP83822_IOCTRL2 0x463
> > #define MII_DP83822_GENCFG 0x465
> > #define MII_DP83822_SOR1 0x467
> >
> > @@ -104,6 +107,11 @@
> > #define DP83822_RX_CLK_SHIFT BIT(12)
> > #define DP83822_TX_CLK_SHIFT BIT(11)
> >
> > +/* IOCTRL2 bits */
> > +#define DP83822_IOCTRL2_GPIO2_CLK_SRC GENMASK(6, 4)
> > +#define DP83822_IOCTRL2_GPIO2_CTRL GENMASK(2, 0)
> > +#define DP83822_IOCTRL2_GPIO2_CTRL_CLK_REF GENMASK(1, 0)
> > +
> > /* SOR1 mode */
> > #define DP83822_STRAP_MODE1 0
> > #define DP83822_STRAP_MODE2 BIT(0)
> > @@ -139,6 +147,8 @@ struct dp83822_private {
> > u8 cfg_dac_minus;
> > u8 cfg_dac_plus;
> > struct ethtool_wolinfo wol;
> > + bool set_gpio2_clk_out;
> > + u32 gpio2_clk_out;
> > };
> >
> > static int dp83822_config_wol(struct phy_device *phydev,
> > @@ -413,6 +423,15 @@ static int dp83822_config_init(struct phy_device *phydev)
> > int err = 0;
> > int bmcr;
> >
> > + if (dp83822->set_gpio2_clk_out)
> > + phy_modify_mmd(phydev, MDIO_MMD_VEND2, MII_DP83822_IOCTRL2,
> > + DP83822_IOCTRL2_GPIO2_CTRL |
> > + DP83822_IOCTRL2_GPIO2_CLK_SRC,
> > + FIELD_PREP(DP83822_IOCTRL2_GPIO2_CTRL,
> > + DP83822_IOCTRL2_GPIO2_CTRL_CLK_REF) |
> > + FIELD_PREP(DP83822_IOCTRL2_GPIO2_CLK_SRC,
> > + dp83822->gpio2_clk_out));
>
> You include the header but you do not use the defines, so it's a proof
> these are register values. Register values are not bindings, they do not
> bind anything. Bindings are imaginary numbers starting from 0 or 1 which
> are used between drivers and DTS, serving as abstraction layer (or
> abstraction values) between these two.
>
> You do not have here abstraction. Drop the bindings header entirely.
>
Ok, thanks for the explanation.
Best regards,
Dimitri Fedrau
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2 1/2] dt-bindings: net: dp83822: Add support for GPIO2 clock output
2024-12-11 10:51 ` Dimitri Fedrau
@ 2024-12-11 13:40 ` Krzysztof Kozlowski
0 siblings, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2024-12-11 13:40 UTC (permalink / raw)
To: Dimitri Fedrau
Cc: Dimitri Fedrau, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andrew Davis, Andrew Lunn, Heiner Kallweit,
Russell King, netdev, devicetree, linux-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-12-11 13:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-11 8:04 [PATCH net-next v2 0/2] net: phy: dp83822: Add support for GPIO2 clock output Dimitri Fedrau via B4 Relay
2024-12-11 8:04 ` [PATCH net-next v2 1/2] dt-bindings: net: " Dimitri Fedrau via B4 Relay
2024-12-11 9:43 ` Krzysztof Kozlowski
2024-12-11 10:51 ` Dimitri Fedrau
2024-12-11 13:40 ` Krzysztof Kozlowski
2024-12-11 8:04 ` [PATCH net-next v2 2/2] net: phy: " Dimitri Fedrau via B4 Relay
2024-12-11 9:45 ` Krzysztof Kozlowski
2024-12-11 10:52 ` Dimitri Fedrau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).