Hi everyone,
This short patch series adds a device-tree binding support to the TI's PHY
DP83640. The goal is to be able to enable or disable the following features
through the device tree:
- Energy Detect Mode
- PHY Control Frames
- LED Configuration
- Fiber Mode
Bastien Curutchet (2):
dt-bindings: net: Add TI DP83640
net: phy: Add some configuration from device-tree
.../devicetree/bindings/net/ti,dp83640.yaml | 113 +++++++++++++++
drivers/net/phy/dp83640.c | 131 +++++++++++++++++-
drivers/net/phy/dp83640_reg.h | 21 ++-
include/dt-bindings/net/ti-dp83640.h | 18 +++
4 files changed, 281 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/ti,dp83640.yaml
create mode 100644 include/dt-bindings/net/ti-dp83640.h
--
2.43.0
Some features can now be enabled or disabled from device tree.
If attributes are present in device-tree, features are enabled
or disabled via MDIO registers. Else, hardware configuration is
left as is.
These features are : Energy Detect Mode, PHY Control Frames,
LED configuration and Fiber Mode.
Signed-off-by: Bastien Curutchet <[email protected]>
---
drivers/net/phy/dp83640.c | 131 +++++++++++++++++++++++++++++++++-
drivers/net/phy/dp83640_reg.h | 21 +++++-
2 files changed, 150 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 5c42c47dc564..f5770002b849 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -7,6 +7,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <dt-bindings/net/ti-dp83640.h>
#include <linux/crc32.h>
#include <linux/ethtool.h>
#include <linux/kernel.h>
@@ -16,6 +17,7 @@
#include <linux/net_tstamp.h>
#include <linux/netdevice.h>
#include <linux/if_vlan.h>
+#include <linux/of.h>
#include <linux/phy.h>
#include <linux/ptp_classify.h>
#include <linux/ptp_clock_kernel.h>
@@ -1418,15 +1420,142 @@ static int dp83640_ts_info(struct mii_timestamper *mii_ts,
return 0;
}
+#ifdef CONFIG_OF_MDIO
+static int dp83640_of_init(struct phy_device *phydev)
+{
+ struct device *dev = &phydev->mdio.dev;
+ struct device_node *of_node = dev->of_node;
+ int reg_val;
+ u32 of_val;
+ int ret;
+
+ if (!of_node)
+ return 0;
+
+ /* All configured features reside in PAGE 0 */
+ phy_write(phydev, PAGESEL, 0);
+
+ /* Energy detect mode */
+ reg_val = phy_read(phydev, EDCR);
+ if (of_property_present(of_node, "ti,energy-detect-en"))
+ reg_val |= ED_EN;
+ else
+ reg_val &= ~ED_EN;
+ phy_write(phydev, EDCR, reg_val);
+
+ /* CLK_OUTPUT Pin */
+ if (of_property_present(of_node, "ti,clk-output")) {
+ ret = of_property_read_u32(of_node, "ti,clk-output", &of_val);
+ if (ret)
+ return ret;
+
+ reg_val = phy_read(phydev, PHYCR2);
+ switch (of_val) {
+ case 0:
+ reg_val |= CLK_OUT_DIS;
+ break;
+ case 1:
+ reg_val &= ~CLK_OUT_DIS;
+ break;
+ default:
+ phydev_err(phydev, "Invalid value for ti,clk-output property (%d)"
+ , of_val);
+ return -EINVAL;
+ }
+ phy_write(phydev, PHYCR2, reg_val);
+ }
+
+ /* LED configuration */
+ if (of_property_present(of_node, "ti,led-config")) {
+ ret = of_property_read_u32(of_node, "ti,led-config", &of_val);
+ if (ret)
+ return ret;
+
+ reg_val = phy_read(phydev, PHYCR) & ~(LED_CNFG_1 | LED_CNFG_0);
+ switch (of_val) {
+ case DP83640_PHYCR_LED_CNFG_MODE_1:
+ reg_val |= LED_CNFG_0;
+ break;
+ case DP83640_PHYCR_LED_CNFG_MODE_2:
+ /* Keeping LED_CNFG_1 and LED_CNFG_0 unset */
+ break;
+ case DP83640_PHYCR_LED_CNFG_MODE_3:
+ reg_val |= LED_CNFG_1;
+ break;
+ default:
+ phydev_err(phydev, "Invalid value for ti,led-config property (%d)"
+ , of_val);
+ return -EINVAL;
+ }
+ phy_write(phydev, PHYCR, reg_val);
+ }
+ if (of_property_present(of_node, "ti,phy-control-frames")) {
+ of_property_read_u32(of_node, "ti,phy-control-frames", &of_val);
+ if (ret)
+ return ret;
+
+ reg_val = phy_read(phydev, PCFCR);
+ switch (of_val) {
+ case 0:
+ reg_val &= ~PCF_EN;
+ break;
+ case 1:
+ reg_val |= PCF_EN;
+ break;
+ default:
+ phydev_err(phydev, "Invalid value for ti,phy-control-frames property (%d)"
+ , of_val);
+ return -EINVAL;
+ }
+ phy_write(phydev, PCFCR, reg_val);
+ }
+ if (of_property_present(of_node, "ti,fiber-mode")) {
+ ret = of_property_read_u32(of_node, "ti,fiber-mode", &of_val);
+ if (ret)
+ return ret;
+
+ reg_val = phy_read(phydev, PCSR);
+ switch (of_val) {
+ case 0:
+ reg_val &= ~FX_EN;
+ break;
+ case 1:
+ reg_val |= FX_EN;
+ break;
+ default:
+ phydev_err(phydev, "Invalid value for ti,fiber-mode property (%d)"
+ , of_val);
+ return -EINVAL;
+ }
+ phy_write(phydev, PCSR, reg_val);
+ /* Write SOFT_RESET bit to ensure configuration */
+ reg_val = phy_read(phydev, PHYCR2) | SOFT_RESET;
+ phy_write(phydev, PHYCR2, reg_val);
+ }
+
+ return 0;
+}
+#else
+static int dp83640_of_init(struct phy_device *phydev)
+{
+ return 0;
+}
+#endif /* CONFIG_OF_MDIO */
+
static int dp83640_probe(struct phy_device *phydev)
{
struct dp83640_clock *clock;
struct dp83640_private *dp83640;
- int err = -ENOMEM, i;
+ int err, i;
if (phydev->mdio.addr == BROADCAST_ADDR)
return 0;
+ err = dp83640_of_init(phydev);
+ if (err < 0)
+ return err;
+
+ err = -ENOMEM;
clock = dp83640_clock_get_bus(phydev->mdio.bus);
if (!clock)
goto no_clock;
diff --git a/drivers/net/phy/dp83640_reg.h b/drivers/net/phy/dp83640_reg.h
index daae7fa58fb8..8877ba560406 100644
--- a/drivers/net/phy/dp83640_reg.h
+++ b/drivers/net/phy/dp83640_reg.h
@@ -6,7 +6,11 @@
#define HAVE_DP83640_REGISTERS
/* #define PAGE0 0x0000 */
+#define PCSR 0x0016 /* PCS Configuration and Status Register */
+#define PHYCR 0x0019 /* PHY Control Register */
#define PHYCR2 0x001c /* PHY Control Register 2 */
+#define EDCR 0x001D /* Energy Detect Control Register */
+#define PCFCR 0x001F /* PHY Control Frames Control Register */
#define PAGE4 0x0004
#define PTP_CTL 0x0014 /* PTP Control Register */
@@ -50,8 +54,23 @@
#define PTP_GPIOMON 0x001e /* PTP GPIO Monitor Register */
#define PTP_RXHASH 0x001f /* PTP Receive Hash Register */
+/* Bit definitions for the PCSR register */
+#define FX_EN BIT(6) /* Enable FX Fiber Mode */
+
+/* Bit definitions for the PHYCR register */
+#define LED_CNFG_0 BIT(5) /* LED configuration, bit 0 */
+#define LED_CNFG_1 BIT(6) /* LED configuration, bit 1 */
+
/* Bit definitions for the PHYCR2 register */
-#define BC_WRITE (1<<11) /* Broadcast Write Enable */
+#define CLK_OUT_DIS BIT(1) /* Disable CLK_OUT pin */
+#define SOFT_RESET BIT(9) /* Soft Reset */
+#define BC_WRITE BIT(11) /* Broadcast Write Enable */
+
+/* Bit definitions for the EDCR register */
+#define ED_EN BIT(15) /* Enable Energy Detect Mode */
+
+/* Bit definitions for the PCFCR register */
+#define PCF_EN BIT(0) /* Enable PHY Control Frames */
/* Bit definitions for the PTP_CTL register */
#define TRIG_SEL_SHIFT (10) /* PTP Trigger Select */
--
2.43.0
The TI DP83640 is a PTP PHY. Some of his features can be enabled by
hardware straps or by registers configuration. Add a device tree
binding for configuration through registers
Signed-off-by: Bastien Curutchet <[email protected]>
---
.../devicetree/bindings/net/ti,dp83640.yaml | 113 ++++++++++++++++++
include/dt-bindings/net/ti-dp83640.h | 18 +++
2 files changed, 131 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/ti,dp83640.yaml
create mode 100644 include/dt-bindings/net/ti-dp83640.h
diff --git a/Documentation/devicetree/bindings/net/ti,dp83640.yaml b/Documentation/devicetree/bindings/net/ti,dp83640.yaml
new file mode 100644
index 000000000000..b0f389122934
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ti,dp83640.yaml
@@ -0,0 +1,113 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2024 Nanometrics Inc
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/ti,dp83640.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI DP83640 ethernet PHY
+
+allOf:
+ - $ref: ethernet-controller.yaml#
+
+maintainers:
+ - Bastien Curutchet <[email protected]>
+
+description: |
+ The DP83640 Precision PHYTER device delivers the highest level of precision
+ clock synchronization for real time industrial connectivity based on the
+ IEEE 1588 standard. The DP83640 has deterministic, low latency and allows
+ choice of microcontroller with no hardware customization required
+
+ This device interfaces directly to the MAC layer through the
+ IEEE 802.3 Standard Media Independent Interface (MII), or Reduced MII (RMII).
+
+ Specifications about the Ethernet PHY can be found at:
+ https://www.ti.com/lit/gpn/dp83640
+
+properties:
+ reg:
+ maxItems: 1
+
+ ti,clk-output:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1]
+ description: |
+ If present, enables or disables the CLK_OUT pin.
+ CLK_OUT pin disabling can also be strapped. If the strap pin is not set
+ correctly or not set at all then this can be used to configure it.
+ - 0 = CLK_OUT pin disabled
+ - 1 = CLK_OUT pin enabled
+ - unset = Configured by straps
+
+ ti,led-config:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [1, 2, 3]
+ description: |
+ If present, configures the LED Mode (values defined in
+ dt-bindings/net/ti-dp83640.h).
+ LED configuration can also be strapped. If the strap pin is not set
+ correctly or not set at all then this can be used to configure it.
+ - 1 = Mode 1
+ LED_LINK = ON for Good Link, OFF for No Link
+ LED_SPEED = ON in 100 Mb/s, OFF in 10 Mb/s
+ LED_ACT = ON for Activity, OFF for No Activity
+ - 2 = Mode 2
+ LED_LINK = ON for Good Link, BLINK for Activity
+ LED_SPEED = ON in 100 Mb/s, OFF in 10 Mb/s
+ LED_ACT = ON for Collision, OFF for No Collision
+ - 3 = Mode 3
+ LED_LINK = ON for Good Link, BLINK for Activity
+ LED_SPEED = ON in 100 Mb/s, OFF in 10 Mb/s
+ LED_ACT = ON for Full Duplex, OFF for Half Duplex
+ - unset = Configured by straps
+
+ ti,phy-control-frames:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1]
+ description: |
+ If present, enables or disables the PHY control frames.
+ PHY Control Frames support can also be strapped. If the strap pin is not
+ set correctly or not set at all then this can be used to configure it.
+ - 0 = PHY Control Frames disabled
+ - 1 = PHY Control Frames enabled
+ - unset = Configured by straps
+
+ ti,fiber-mode:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1]
+ description: |
+ If present, enables or disables the FX Fiber Mode.
+ Fiber mode support can also be strapped. If the strap pin is not set
+ correctly or not set at all then this can be used to configure it.
+ - 0 = FX Fiber Mode disabled
+ - 1 = FX Fiber Mode enabled
+ - unset = Configured by straps
+
+ ti,energy-detect-en:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: |
+ If present, Energy Detect Mode is enabled. If not present, Energy Detect
+ Mode is disabled. This feature can not be strapped.
+
+required:
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/net/ti-dp83640.h>
+
+ mdio0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ethphy0: ethernet-phy@0 {
+ reg = <0>;
+ ti,clk-output = <0>;
+ ti,energy-detect-en;
+ ti,led-config = <DP83640_PHYCR_LED_CNFG_MODE_3>;
+ ti,phy-control-frames = <1>;
+ ti,fiber-mode = <1>;
+ };
+ };
diff --git a/include/dt-bindings/net/ti-dp83640.h b/include/dt-bindings/net/ti-dp83640.h
new file mode 100644
index 000000000000..5f44f8eeb666
--- /dev/null
+++ b/include/dt-bindings/net/ti-dp83640.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * Device Tree constants for the Texas Instruments DP83640 PHY
+ *
+ * Author: Bastien Curutchet [email protected]>
+ *
+ * Copyright: 2024 Nanometrics Inc.
+ */
+
+#ifndef _DT_BINDINGS_TI_DP83640_H
+#define _DT_BINDINGS_TI_DP83640_H
+
+/* PHY CTRL bits */
+#define DP83640_PHYCR_LED_CNFG_MODE_1 1
+#define DP83640_PHYCR_LED_CNFG_MODE_2 2
+#define DP83640_PHYCR_LED_CNFG_MODE_3 3
+
+#endif
--
2.43.0
> + ti,led-config:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [1, 2, 3]
> + description: |
> + If present, configures the LED Mode (values defined in
> + dt-bindings/net/ti-dp83640.h).
> + LED configuration can also be strapped. If the strap pin is not set
> + correctly or not set at all then this can be used to configure it.
> + - 1 = Mode 1
> + LED_LINK = ON for Good Link, OFF for No Link
> + LED_SPEED = ON in 100 Mb/s, OFF in 10 Mb/s
> + LED_ACT = ON for Activity, OFF for No Activity
> + - 2 = Mode 2
> + LED_LINK = ON for Good Link, BLINK for Activity
> + LED_SPEED = ON in 100 Mb/s, OFF in 10 Mb/s
> + LED_ACT = ON for Collision, OFF for No Collision
> + - 3 = Mode 3
> + LED_LINK = ON for Good Link, BLINK for Activity
> + LED_SPEED = ON in 100 Mb/s, OFF in 10 Mb/s
> + LED_ACT = ON for Full Duplex, OFF for Half Duplex
> + - unset = Configured by straps
Please look at have the Marvell PHY driver supports LEDs via
/sys/class/leds. Now we have a generic way to supports LEDs, DT
properties like this will not be accepted.
> +
> + ti,phy-control-frames:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1]
> + description: |
> + If present, enables or disables the PHY control frames.
> + PHY Control Frames support can also be strapped. If the strap pin is not
> + set correctly or not set at all then this can be used to configure it.
> + - 0 = PHY Control Frames disabled
> + - 1 = PHY Control Frames enabled
> + - unset = Configured by straps
What is a control frame?
> +
> + ti,energy-detect-en:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description: |
> + If present, Energy Detect Mode is enabled. If not present, Energy Detect
> + Mode is disabled. This feature can not be strapped.
Please use the phy tunable ETHTOOL_PHY_EDPD. There are a few examples
you can copy.
Andrew
Hey,
On Tue, Jan 30, 2024 at 09:59:34AM +0100, Bastien Curutchet wrote:
> +description: |
> + The DP83640 Precision PHYTER device delivers the highest level of precision
This is not a marketing document.
> + clock synchronization for real time industrial connectivity based on the
> + IEEE 1588 standard. The DP83640 has deterministic, low latency and allows
> + choice of microcontroller with no hardware customization required
> +
> + This device interfaces directly to the MAC layer through the
> + IEEE 802.3 Standard Media Independent Interface (MII), or Reduced MII (RMII).
> +
> + Specifications about the Ethernet PHY can be found at:
> + https://www.ti.com/lit/gpn/dp83640
> +
> +properties:
> + reg:
> + maxItems: 1
> +
> + ti,clk-output:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1]
> + description: |
> + If present, enables or disables the CLK_OUT pin.
> + CLK_OUT pin disabling can also be strapped. If the strap pin is not set
> + correctly or not set at all then this can be used to configure it.
> + - 0 = CLK_OUT pin disabled
> + - 1 = CLK_OUT pin enabled
> + - unset = Configured by straps
If you are providing a clock, why is there no clock-controller property
here? I don't think the 3-way nature of this property is needed, if you
make this a "real" clock controller.
> + ti,fiber-mode:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1]
> + description: |
> + If present, enables or disables the FX Fiber Mode.
> + Fiber mode support can also be strapped. If the strap pin is not set
> + correctly or not set at all then this can be used to configure it.
> + - 0 = FX Fiber Mode disabled
> + - 1 = FX Fiber Mode enabled
> + - unset = Configured by straps
I don't like these properties that map meanings onto numbers. We can
have enums of strings in bindings that allow you to use something more
meaningful than "0" or "1".
Cheers,
Conor.
On Tue, Jan 30, 2024 at 05:56:37PM +0000, Conor Dooley wrote:
> Hey,
>
> On Tue, Jan 30, 2024 at 09:59:34AM +0100, Bastien Curutchet wrote:
> > +description: |
> > + The DP83640 Precision PHYTER device delivers the highest level of precision
>
> This is not a marketing document.
>
> > + clock synchronization for real time industrial connectivity based on the
> > + IEEE 1588 standard. The DP83640 has deterministic, low latency and allows
> > + choice of microcontroller with no hardware customization required
> > +
> > + This device interfaces directly to the MAC layer through the
> > + IEEE 802.3 Standard Media Independent Interface (MII), or Reduced MII (RMII).
> > +
> > + Specifications about the Ethernet PHY can be found at:
> > + https://www.ti.com/lit/gpn/dp83640
> > +
> > +properties:
> > + reg:
> > + maxItems: 1
> > +
> > + ti,clk-output:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum: [0, 1]
> > + description: |
> > + If present, enables or disables the CLK_OUT pin.
> > + CLK_OUT pin disabling can also be strapped. If the strap pin is not set
> > + correctly or not set at all then this can be used to configure it.
> > + - 0 = CLK_OUT pin disabled
> > + - 1 = CLK_OUT pin enabled
> > + - unset = Configured by straps
>
> If you are providing a clock, why is there no clock-controller property
> here? I don't think the 3-way nature of this property is needed, if you
> make this a "real" clock controller.
>
> > + ti,fiber-mode:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum: [0, 1]
> > + description: |
> > + If present, enables or disables the FX Fiber Mode.
> > + Fiber mode support can also be strapped. If the strap pin is not set
> > + correctly or not set at all then this can be used to configure it.
> > + - 0 = FX Fiber Mode disabled
> > + - 1 = FX Fiber Mode enabled
> > + - unset = Configured by straps
>
> I don't like these properties that map meanings onto numbers. We can
> have enums of strings in bindings that allow you to use something more
> meaningful than "0" or "1".
Tristate properties are fairly common pattern where we need
on/off/default. I've thought about making it a type. I don't think we
need defines for it.
Rob
On Wed, Jan 31, 2024 at 03:05:21PM -0600, Rob Herring wrote:
> On Tue, Jan 30, 2024 at 05:56:37PM +0000, Conor Dooley wrote:
> > On Tue, Jan 30, 2024 at 09:59:34AM +0100, Bastien Curutchet wrote:
> > > + ti,fiber-mode:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + enum: [0, 1]
> > > + description: |
> > > + If present, enables or disables the FX Fiber Mode.
> > > + Fiber mode support can also be strapped. If the strap pin is not set
> > > + correctly or not set at all then this can be used to configure it.
> > > + - 0 = FX Fiber Mode disabled
> > > + - 1 = FX Fiber Mode enabled
> > > + - unset = Configured by straps
> >
> > I don't like these properties that map meanings onto numbers. We can
> > have enums of strings in bindings that allow you to use something more
> > meaningful than "0" or "1".
>
> Tristate properties are fairly common pattern where we need
> on/off/default. I've thought about making it a type. I don't think we
> need defines for it.
I think a type would be a good idea. I am not at all a fan of any of the
properties people introduce along these lines.
On Wed, Jan 31, 2024 at 09:18:39PM +0000, Conor Dooley wrote:
> On Wed, Jan 31, 2024 at 03:05:21PM -0600, Rob Herring wrote:
> > On Tue, Jan 30, 2024 at 05:56:37PM +0000, Conor Dooley wrote:
> > > On Tue, Jan 30, 2024 at 09:59:34AM +0100, Bastien Curutchet wrote:
>
> > > > + ti,fiber-mode:
> > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > + enum: [0, 1]
> > > > + description: |
> > > > + If present, enables or disables the FX Fiber Mode.
> > > > + Fiber mode support can also be strapped. If the strap pin is not set
> > > > + correctly or not set at all then this can be used to configure it.
> > > > + - 0 = FX Fiber Mode disabled
> > > > + - 1 = FX Fiber Mode enabled
> > > > + - unset = Configured by straps
> > >
> > > I don't like these properties that map meanings onto numbers. We can
> > > have enums of strings in bindings that allow you to use something more
> > > meaningful than "0" or "1".
> >
> > Tristate properties are fairly common pattern where we need
> > on/off/default. I've thought about making it a type. I don't think we
> > need defines for it.
>
> I think a type would be a good idea. I am not at all a fan of any of the
> properties people introduce along these lines.
Before going too far with that, i'm not actually sure it is required
here. I've not looked at the PHY driver itself, but i expect there is
some indication somewhere that the network stack expects a fibre link
is to be used. We probably can determine at runtime if fibre should be
used.
Andrew
Hi Andrew,
Thank you for your feedback.
On 1/30/24 14:34, Andrew Lunn wrote:
>> + ti,led-config:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + enum: [1, 2, 3]
>> + description: |
>> + If present, configures the LED Mode (values defined in
>> + dt-bindings/net/ti-dp83640.h).
>> + LED configuration can also be strapped. If the strap pin is not set
>> + correctly or not set at all then this can be used to configure it.
>> + - 1 = Mode 1
>> + LED_LINK = ON for Good Link, OFF for No Link
>> + LED_SPEED = ON in 100 Mb/s, OFF in 10 Mb/s
>> + LED_ACT = ON for Activity, OFF for No Activity
>> + - 2 = Mode 2
>> + LED_LINK = ON for Good Link, BLINK for Activity
>> + LED_SPEED = ON in 100 Mb/s, OFF in 10 Mb/s
>> + LED_ACT = ON for Collision, OFF for No Collision
>> + - 3 = Mode 3
>> + LED_LINK = ON for Good Link, BLINK for Activity
>> + LED_SPEED = ON in 100 Mb/s, OFF in 10 Mb/s
>> + LED_ACT = ON for Full Duplex, OFF for Half Duplex
>> + - unset = Configured by straps
> Please look at have the Marvell PHY driver supports LEDs via
> /sys/class/leds. Now we have a generic way to supports LEDs, DT
> properties like this will not be accepted.
Ok I'll use /sys/class/leds
>> +
>> + ti,phy-control-frames:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + enum: [0, 1]
>> + description: |
>> + If present, enables or disables the PHY control frames.
>> + PHY Control Frames support can also be strapped. If the strap pin is not
>> + set correctly or not set at all then this can be used to configure it.
>> + - 0 = PHY Control Frames disabled
>> + - 1 = PHY Control Frames enabled
>> + - unset = Configured by straps
> What is a control frame?
I'm not an expert on this but it seems that if the PHY's Serial Management
interface is not available, it is possible to build PCF (PHY Control Frame)
packets that will be passed to PHY through the MAC Transmit Data
interface. The
PHY is then able to intercept and interpret these packets. Enabling it
increases
the MII Transmit packet latency.
You'll find details in ยง5.4.6 of datasheet
[https://www.ti.com/lit/gpn/dp83640]
>> +
>> + ti,energy-detect-en:
>> + $ref: /schemas/types.yaml#/definitions/flag
>> + description: |
>> + If present, Energy Detect Mode is enabled. If not present, Energy Detect
>> + Mode is disabled. This feature can not be strapped.
> Please use the phy tunable ETHTOOL_PHY_EDPD. There are a few examples
> you can copy.
Ok I'll do that also, thank you.
Best regards,
Bastien
> > > + ti,phy-control-frames:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + enum: [0, 1]
> > > + description: |
> > > + If present, enables or disables the PHY control frames.
> > > + PHY Control Frames support can also be strapped. If the strap pin is not
> > > + set correctly or not set at all then this can be used to configure it.
> > > + - 0 = PHY Control Frames disabled
> > > + - 1 = PHY Control Frames enabled
> > > + - unset = Configured by straps
> > What is a control frame?
> I'm not an expert on this but it seems that if the PHY's Serial Management
> interface is not available, it is possible to build PCF (PHY Control Frame)
> packets that will be passed to PHY through the MAC Transmit Data interface.
> The
> PHY is then able to intercept and interpret these packets. Enabling it
> increases
> the MII Transmit packet latency.
> You'll find details in ?5.4.6 of datasheet
> [https://www.ti.com/lit/gpn/dp83640]
Do you actually need this feature?
[Looks at data sheet]
Ah, so it allows you to access PHY registers by sending it commands in
Ethernet frames. That should in theory be faster than MDIO. However,
my experience with Ethernet switches which offer similar capabilities,
it is often not faster, because of interrupt coalescing.
Anyway, the serial management interface is the MDIO bus. You know this
is available, because that is how the PHY driver it talking to the
PHY! Also, i've not seen any code which implements sending commands to
the PHY using Ethernet frames.
So why not just hard code it in the driver to disable this feature?
Andrew
Hi Andrew, Bastien,
On Wed, 31 Jan 2024 23:40:45 +0100
Andrew Lunn <[email protected]> wrote:
> On Wed, Jan 31, 2024 at 09:18:39PM +0000, Conor Dooley wrote:
> > On Wed, Jan 31, 2024 at 03:05:21PM -0600, Rob Herring wrote:
> > > On Tue, Jan 30, 2024 at 05:56:37PM +0000, Conor Dooley wrote:
> > > > On Tue, Jan 30, 2024 at 09:59:34AM +0100, Bastien Curutchet wrote:
> >
> > > > > + ti,fiber-mode:
> > > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > > + enum: [0, 1]
> > > > > + description: |
> > > > > + If present, enables or disables the FX Fiber Mode.
> > > > > + Fiber mode support can also be strapped. If the strap pin is not set
> > > > > + correctly or not set at all then this can be used to configure it.
> > > > > + - 0 = FX Fiber Mode disabled
> > > > > + - 1 = FX Fiber Mode enabled
> > > > > + - unset = Configured by straps
> > > >
> > > > I don't like these properties that map meanings onto numbers. We can
> > > > have enums of strings in bindings that allow you to use something more
> > > > meaningful than "0" or "1".
> > >
> > > Tristate properties are fairly common pattern where we need
> > > on/off/default. I've thought about making it a type. I don't think we
> > > need defines for it.
> >
> > I think a type would be a good idea. I am not at all a fan of any of the
> > properties people introduce along these lines.
>
> Before going too far with that, i'm not actually sure it is required
> here. I've not looked at the PHY driver itself, but i expect there is
> some indication somewhere that the network stack expects a fibre link
> is to be used. We probably can determine at runtime if fibre should be
> used.
I've missed that thread initially. I guess that if Fiber is to be used,
this would be done through sfp, then we have all the regular interfaces
to configure the phy_interface_mode, in that case that would be
100BaseX.
So, a sane behaviour could simply be to configure the PHY in copper
mode by default, without relying on any DT property ? If anyone wants to
use fiber mode, then they would have to implement the
sfp_upstreamp_ops, which would take care of reconfiguring the MDI
interface of the PHY in the correct mode.
Maxime
>
> I've missed that thread initially. I guess that if Fiber is to be used,
> this would be done through sfp, then we have all the regular interfaces
> to configure the phy_interface_mode, in that case that would be
> 100BaseX.
>
> So, a sane behaviour could simply be to configure the PHY in copper
> mode by default, without relying on any DT property ? If anyone wants to
> use fiber mode, then they would have to implement the
> sfp_upstreamp_ops, which would take care of reconfiguring the MDI
> interface of the PHY in the correct mode.
I missed the fact that this isn't a new driver... So this would indeed
break existing setups who would have fiber-mode strapped-in but don't
use SFP for some reason :(
Maxime
> Maxime
>