The lan969x switch device includes two RGMII interfaces (port 28 and 29)
supporting data speeds of 1 Gbps, 100 Mbps and 10 Mbps.
Add new function: rgmii_config() to the match data ops, and use it to
configure RGMII port devices when doing a port config. On Sparx5, the
RGMII configuration will always be skipped, as the is_port_rgmii() will
return false.
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com>
Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
drivers/net/ethernet/microchip/lan969x/lan969x.c | 105 +++++++++++++++++++++
.../net/ethernet/microchip/sparx5/sparx5_main.h | 2 +
.../net/ethernet/microchip/sparx5/sparx5_port.c | 3 +
3 files changed, 110 insertions(+)
diff --git a/drivers/net/ethernet/microchip/lan969x/lan969x.c b/drivers/net/ethernet/microchip/lan969x/lan969x.c
index cfd57eb42c04..0681913a05d4 100644
--- a/drivers/net/ethernet/microchip/lan969x/lan969x.c
+++ b/drivers/net/ethernet/microchip/lan969x/lan969x.c
@@ -9,6 +9,17 @@
#define LAN969X_SDLB_GRP_CNT 5
#define LAN969X_HSCH_LEAK_GRP_CNT 4
+#define LAN969X_RGMII_TX_CLK_DISABLE 0 /* Disable TX clock generation*/
+#define LAN969X_RGMII_TX_CLK_125MHZ 1 /* 1000Mbps */
+#define LAN969X_RGMII_TX_CLK_25MHZ 2 /* 100Mbps */
+#define LAN969X_RGMII_TX_CLK_2M5MHZ 3 /* 10Mbps */
+#define LAN969X_RGMII_PORT_START_IDX 28 /* Index of the first RGMII port */
+#define LAN969X_RGMII_PORT_RATE 2 /* 1000Mbps */
+#define LAN969X_RGMII_SHIFT_90DEG 3 /* Phase shift 90deg. (2 ns @ 125MHz) */
+#define LAN969X_RGMII_IFG_TX 4 /* TX Inter Frame Gap value */
+#define LAN969X_RGMII_IFG_RX1 5 /* RX1 Inter Frame Gap value */
+#define LAN969X_RGMII_IFG_RX2 1 /* RX2 Inter Frame Gap value */
+
static const struct sparx5_main_io_resource lan969x_main_iomap[] = {
{ TARGET_CPU, 0xc0000, 0 }, /* 0xe00c0000 */
{ TARGET_FDMA, 0xc0400, 0 }, /* 0xe00c0400 */
@@ -293,6 +304,99 @@ static irqreturn_t lan969x_ptp_irq_handler(int irq, void *args)
return IRQ_HANDLED;
}
+static int lan969x_port_config_rgmii(struct sparx5 *sparx5,
+ struct sparx5_port *port,
+ struct sparx5_port_config *conf)
+{
+ int tx_clk_freq, idx = port->portno - LAN969X_RGMII_PORT_START_IDX;
+ enum sparx5_port_max_tags max_tags = port->max_vlan_tags;
+ enum sparx5_vlan_port_type vlan_type = port->vlan_type;
+ bool dtag, dotag, tx_delay = false, rx_delay = false;
+ u32 etype;
+
+ tx_clk_freq = (conf->speed == SPEED_10 ? LAN969X_RGMII_TX_CLK_2M5MHZ :
+ conf->speed == SPEED_100 ? LAN969X_RGMII_TX_CLK_25MHZ :
+ LAN969X_RGMII_TX_CLK_125MHZ);
+
+ etype = (vlan_type == SPX5_VLAN_PORT_TYPE_S_CUSTOM ?
+ port->custom_etype :
+ vlan_type == SPX5_VLAN_PORT_TYPE_C ?
+ SPX5_ETYPE_TAG_C : SPX5_ETYPE_TAG_S);
+
+ dtag = max_tags == SPX5_PORT_MAX_TAGS_TWO;
+ dotag = max_tags != SPX5_PORT_MAX_TAGS_NONE;
+
+ if (conf->phy_mode == PHY_INTERFACE_MODE_RGMII ||
+ conf->phy_mode == PHY_INTERFACE_MODE_RGMII_TXID)
+ rx_delay = true;
+
+ if (conf->phy_mode == PHY_INTERFACE_MODE_RGMII ||
+ conf->phy_mode == PHY_INTERFACE_MODE_RGMII_RXID)
+ tx_delay = true;
+
+ /* Take the RGMII clock domains out of reset and set tx clock
+ * frequency.
+ */
+ spx5_rmw(HSIO_WRAP_RGMII_CFG_TX_CLK_CFG_SET(tx_clk_freq) |
+ HSIO_WRAP_RGMII_CFG_RGMII_TX_RST_SET(0) |
+ HSIO_WRAP_RGMII_CFG_RGMII_RX_RST_SET(0),
+ HSIO_WRAP_RGMII_CFG_TX_CLK_CFG |
+ HSIO_WRAP_RGMII_CFG_RGMII_TX_RST |
+ HSIO_WRAP_RGMII_CFG_RGMII_RX_RST,
+ sparx5, HSIO_WRAP_RGMII_CFG(idx));
+
+ /* Enable the RGMII0 on the GPIOs */
+ spx5_wr(HSIO_WRAP_XMII_CFG_GPIO_XMII_CFG_SET(1),
+ sparx5, HSIO_WRAP_XMII_CFG(!idx));
+
+ /* Configure rx delay, the signal is shifted 90 degrees. */
+ spx5_rmw(HSIO_WRAP_DLL_CFG_DLL_RST_SET(0) |
+ HSIO_WRAP_DLL_CFG_DLL_ENA_SET(1) |
+ HSIO_WRAP_DLL_CFG_DLL_CLK_ENA_SET(rx_delay) |
+ HSIO_WRAP_DLL_CFG_DLL_CLK_SEL_SET(LAN969X_RGMII_SHIFT_90DEG),
+ HSIO_WRAP_DLL_CFG_DLL_RST |
+ HSIO_WRAP_DLL_CFG_DLL_ENA |
+ HSIO_WRAP_DLL_CFG_DLL_CLK_ENA |
+ HSIO_WRAP_DLL_CFG_DLL_CLK_SEL,
+ sparx5, HSIO_WRAP_DLL_CFG(idx, 0));
+
+ /* Configure tx delay, the signal is shifted 90 degrees. */
+ spx5_rmw(HSIO_WRAP_DLL_CFG_DLL_RST_SET(0) |
+ HSIO_WRAP_DLL_CFG_DLL_ENA_SET(1) |
+ HSIO_WRAP_DLL_CFG_DLL_CLK_ENA_SET(tx_delay) |
+ HSIO_WRAP_DLL_CFG_DLL_CLK_SEL_SET(LAN969X_RGMII_SHIFT_90DEG),
+ HSIO_WRAP_DLL_CFG_DLL_RST |
+ HSIO_WRAP_DLL_CFG_DLL_ENA |
+ HSIO_WRAP_DLL_CFG_DLL_CLK_ENA |
+ HSIO_WRAP_DLL_CFG_DLL_CLK_SEL,
+ sparx5, HSIO_WRAP_DLL_CFG(idx, 1));
+
+ /* Configure the port now */
+ spx5_wr(DEVRGMII_MAC_ENA_CFG_RX_ENA_SET(1) |
+ DEVRGMII_MAC_ENA_CFG_TX_ENA_SET(1),
+ sparx5, DEVRGMII_MAC_ENA_CFG(idx));
+
+ /* Configure the Inter Frame Gap */
+ spx5_wr(DEVRGMII_MAC_IFG_CFG_TX_IFG_SET(LAN969X_RGMII_IFG_TX) |
+ DEVRGMII_MAC_IFG_CFG_RX_IFG1_SET(LAN969X_RGMII_IFG_RX1) |
+ DEVRGMII_MAC_IFG_CFG_RX_IFG2_SET(LAN969X_RGMII_IFG_RX2),
+ sparx5, DEVRGMII_MAC_IFG_CFG(idx));
+
+ /* Configure port data rate */
+ spx5_wr(DEVRGMII_DEV_RST_CTRL_SPEED_SEL_SET(LAN969X_RGMII_PORT_RATE),
+ sparx5, DEVRGMII_DEV_RST_CTRL(idx));
+
+ /* Configure VLAN awareness */
+ spx5_wr(DEVRGMII_MAC_TAGS_CFG_TAG_ID_SET(etype) |
+ DEVRGMII_MAC_TAGS_CFG_PB_ENA_SET(dtag) |
+ DEVRGMII_MAC_TAGS_CFG_VLAN_AWR_ENA_SET(dotag) |
+ DEVRGMII_MAC_TAGS_CFG_VLAN_LEN_AWR_ENA_SET(dotag),
+ sparx5,
+ DEVRGMII_MAC_TAGS_CFG(idx));
+
+ return 0;
+}
+
static const struct sparx5_regs lan969x_regs = {
.tsize = lan969x_tsize,
.gaddr = lan969x_gaddr,
@@ -337,6 +441,7 @@ static const struct sparx5_ops lan969x_ops = {
.set_port_mux = &lan969x_port_mux_set,
.ptp_irq_handler = &lan969x_ptp_irq_handler,
.dsm_calendar_calc = &lan969x_dsm_calendar_calc,
+ .rgmii_config = &lan969x_port_config_rgmii,
};
const struct sparx5_match_data lan969x_desc = {
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index a622c01930e7..763c827c01f3 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -324,6 +324,8 @@ struct sparx5_ops {
irqreturn_t (*ptp_irq_handler)(int irq, void *args);
int (*dsm_calendar_calc)(struct sparx5 *sparx5, u32 taxi,
struct sparx5_calendar_data *data);
+ int (*rgmii_config)(struct sparx5 *sparx5, struct sparx5_port *port,
+ struct sparx5_port_config *conf);
};
struct sparx5_main_io_resource {
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
index bd1fa5da47d7..ef61e8164e21 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
@@ -1009,6 +1009,9 @@ int sparx5_port_config(struct sparx5 *sparx5,
if (err)
return err;
+ if (rgmii)
+ ops->rgmii_config(sparx5, port, conf);
+
/* high speed device is already configured */
if (!rgmii && !high_speed_dev)
sparx5_port_config_low_set(sparx5, port, conf);
--
2.34.1On Wed, Nov 06, 2024 at 08:16:45PM +0100, Daniel Machon wrote:
> The lan969x switch device includes two RGMII interfaces (port 28 and 29)
> supporting data speeds of 1 Gbps, 100 Mbps and 10 Mbps.
>
> Add new function: rgmii_config() to the match data ops, and use it to
> configure RGMII port devices when doing a port config. On Sparx5, the
> RGMII configuration will always be skipped, as the is_port_rgmii() will
> return false.
>
> Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
> Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> ---
> drivers/net/ethernet/microchip/lan969x/lan969x.c | 105 +++++++++++++++++++++
> .../net/ethernet/microchip/sparx5/sparx5_main.h | 2 +
> .../net/ethernet/microchip/sparx5/sparx5_port.c | 3 +
> 3 files changed, 110 insertions(+)
>
> diff --git a/drivers/net/ethernet/microchip/lan969x/lan969x.c b/drivers/net/ethernet/microchip/lan969x/lan969x.c
> index cfd57eb42c04..0681913a05d4 100644
> --- a/drivers/net/ethernet/microchip/lan969x/lan969x.c
> +++ b/drivers/net/ethernet/microchip/lan969x/lan969x.c
> @@ -9,6 +9,17 @@
> #define LAN969X_SDLB_GRP_CNT 5
> #define LAN969X_HSCH_LEAK_GRP_CNT 4
>
> +#define LAN969X_RGMII_TX_CLK_DISABLE 0 /* Disable TX clock generation*/
> +#define LAN969X_RGMII_TX_CLK_125MHZ 1 /* 1000Mbps */
> +#define LAN969X_RGMII_TX_CLK_25MHZ 2 /* 100Mbps */
> +#define LAN969X_RGMII_TX_CLK_2M5MHZ 3 /* 10Mbps */
> +#define LAN969X_RGMII_PORT_START_IDX 28 /* Index of the first RGMII port */
> +#define LAN969X_RGMII_PORT_RATE 2 /* 1000Mbps */
> +#define LAN969X_RGMII_SHIFT_90DEG 3 /* Phase shift 90deg. (2 ns @ 125MHz) */
> +#define LAN969X_RGMII_IFG_TX 4 /* TX Inter Frame Gap value */
> +#define LAN969X_RGMII_IFG_RX1 5 /* RX1 Inter Frame Gap value */
> +#define LAN969X_RGMII_IFG_RX2 1 /* RX2 Inter Frame Gap value */
> +
> static const struct sparx5_main_io_resource lan969x_main_iomap[] = {
> { TARGET_CPU, 0xc0000, 0 }, /* 0xe00c0000 */
> { TARGET_FDMA, 0xc0400, 0 }, /* 0xe00c0400 */
> @@ -293,6 +304,99 @@ static irqreturn_t lan969x_ptp_irq_handler(int irq, void *args)
> return IRQ_HANDLED;
> }
>
> +static int lan969x_port_config_rgmii(struct sparx5 *sparx5,
> + struct sparx5_port *port,
> + struct sparx5_port_config *conf)
> +{
> + int tx_clk_freq, idx = port->portno - LAN969X_RGMII_PORT_START_IDX;
> + enum sparx5_port_max_tags max_tags = port->max_vlan_tags;
> + enum sparx5_vlan_port_type vlan_type = port->vlan_type;
> + bool dtag, dotag, tx_delay = false, rx_delay = false;
> + u32 etype;
> +
> + tx_clk_freq = (conf->speed == SPEED_10 ? LAN969X_RGMII_TX_CLK_2M5MHZ :
> + conf->speed == SPEED_100 ? LAN969X_RGMII_TX_CLK_25MHZ :
> + LAN969X_RGMII_TX_CLK_125MHZ);
https://www.spinics.net/lists/netdev/msg1040925.html
Once it is merged, i think this does what you want.
> + if (conf->phy_mode == PHY_INTERFACE_MODE_RGMII ||
> + conf->phy_mode == PHY_INTERFACE_MODE_RGMII_TXID)
> + rx_delay = true;
> +
> + if (conf->phy_mode == PHY_INTERFACE_MODE_RGMII ||
> + conf->phy_mode == PHY_INTERFACE_MODE_RGMII_RXID)
> + tx_delay = true;
O.K, now warning bells are ringing in this reviews head.
What i don't see is the value you pass to the PHY? You obviously need
to mask out what the MAC is doing when talking to the PHY, otherwise
both ends will add delays.
And in general in Linux, we have the PHY add the delays, not the
MAC. It is somewhat arbitrary, but the vast majority of systems do
that. The exception is systems where the PHY is too dumb/cheap to add
the delays and so the MAC has to do it. I'm don't know of any
Microchip PHYs which don't support RGMII delays.
Andrew
Hi Andrew,
> > The lan969x switch device includes two RGMII interfaces (port 28 and 29)
> > supporting data speeds of 1 Gbps, 100 Mbps and 10 Mbps.
> >
> > Add new function: rgmii_config() to the match data ops, and use it to
> > configure RGMII port devices when doing a port config. On Sparx5, the
> > RGMII configuration will always be skipped, as the is_port_rgmii() will
> > return false.
> >
> > Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
> > Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> > ---
> > drivers/net/ethernet/microchip/lan969x/lan969x.c | 105 +++++++++++++++++++++
> > .../net/ethernet/microchip/sparx5/sparx5_main.h | 2 +
> > .../net/ethernet/microchip/sparx5/sparx5_port.c | 3 +
> > 3 files changed, 110 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/microchip/lan969x/lan969x.c b/drivers/net/ethernet/microchip/lan969x/lan969x.c
> > index cfd57eb42c04..0681913a05d4 100644
> > --- a/drivers/net/ethernet/microchip/lan969x/lan969x.c
> > +++ b/drivers/net/ethernet/microchip/lan969x/lan969x.c
> > @@ -9,6 +9,17 @@
> > #define LAN969X_SDLB_GRP_CNT 5
> > #define LAN969X_HSCH_LEAK_GRP_CNT 4
> >
> > +#define LAN969X_RGMII_TX_CLK_DISABLE 0 /* Disable TX clock generation*/
> > +#define LAN969X_RGMII_TX_CLK_125MHZ 1 /* 1000Mbps */
> > +#define LAN969X_RGMII_TX_CLK_25MHZ 2 /* 100Mbps */
> > +#define LAN969X_RGMII_TX_CLK_2M5MHZ 3 /* 10Mbps */
> > +#define LAN969X_RGMII_PORT_START_IDX 28 /* Index of the first RGMII port */
> > +#define LAN969X_RGMII_PORT_RATE 2 /* 1000Mbps */
> > +#define LAN969X_RGMII_SHIFT_90DEG 3 /* Phase shift 90deg. (2 ns @ 125MHz) */
> > +#define LAN969X_RGMII_IFG_TX 4 /* TX Inter Frame Gap value */
> > +#define LAN969X_RGMII_IFG_RX1 5 /* RX1 Inter Frame Gap value */
> > +#define LAN969X_RGMII_IFG_RX2 1 /* RX2 Inter Frame Gap value */
> > +
> > static const struct sparx5_main_io_resource lan969x_main_iomap[] = {
> > { TARGET_CPU, 0xc0000, 0 }, /* 0xe00c0000 */
> > { TARGET_FDMA, 0xc0400, 0 }, /* 0xe00c0400 */
> > @@ -293,6 +304,99 @@ static irqreturn_t lan969x_ptp_irq_handler(int irq, void *args)
> > return IRQ_HANDLED;
> > }
> >
> > +static int lan969x_port_config_rgmii(struct sparx5 *sparx5,
> > + struct sparx5_port *port,
> > + struct sparx5_port_config *conf)
> > +{
> > + int tx_clk_freq, idx = port->portno - LAN969X_RGMII_PORT_START_IDX;
> > + enum sparx5_port_max_tags max_tags = port->max_vlan_tags;
> > + enum sparx5_vlan_port_type vlan_type = port->vlan_type;
> > + bool dtag, dotag, tx_delay = false, rx_delay = false;
> > + u32 etype;
> > +
> > + tx_clk_freq = (conf->speed == SPEED_10 ? LAN969X_RGMII_TX_CLK_2M5MHZ :
> > + conf->speed == SPEED_100 ? LAN969X_RGMII_TX_CLK_25MHZ :
> > + LAN969X_RGMII_TX_CLK_125MHZ);
>
> https://www.spinics.net/lists/netdev/msg1040925.html
>
> Once it is merged, i think this does what you want.
>
Nice! Thanks for letting me know.
> > + if (conf->phy_mode == PHY_INTERFACE_MODE_RGMII ||
> > + conf->phy_mode == PHY_INTERFACE_MODE_RGMII_TXID)
> > + rx_delay = true;
> > +
> > + if (conf->phy_mode == PHY_INTERFACE_MODE_RGMII ||
> > + conf->phy_mode == PHY_INTERFACE_MODE_RGMII_RXID)
> > + tx_delay = true;
>
> O.K, now warning bells are ringing in this reviews head.
>
> What i don't see is the value you pass to the PHY? You obviously need
> to mask out what the MAC is doing when talking to the PHY, otherwise
> both ends will add delays.
>
What value should be passed to the PHY?
We (the MAC) add the delays based on the PHY modes - so does the PHY.
RGMII, we add both delays.
RGMII_ID, the PHY adds both delays.
RGMII_TXID, we add the rx delay, the PHY adds the tx delay.
RGMII_RXID, we add the tx delay, the PHY adds the rx delay.
Am I missing something here? :-)
> And in general in Linux, we have the PHY add the delays, not the
> MAC. It is somewhat arbitrary, but the vast majority of systems do
> that. The exception is systems where the PHY is too dumb/cheap to add
> the delays and so the MAC has to do it. I'm don't know of any
> Microchip PHYs which don't support RGMII delays.
Ack.
>
> Andrew
/Daniel
On Fri, Nov 08, 2024 at 08:53:20AM +0000, Daniel Machon wrote: > Hi Andrew, > > > > + if (conf->phy_mode == PHY_INTERFACE_MODE_RGMII || > > > + conf->phy_mode == PHY_INTERFACE_MODE_RGMII_TXID) > > > + rx_delay = true; > > > + > > > + if (conf->phy_mode == PHY_INTERFACE_MODE_RGMII || > > > + conf->phy_mode == PHY_INTERFACE_MODE_RGMII_RXID) > > > + tx_delay = true; > > > > O.K, now warning bells are ringing in this reviews head. > > > > What i don't see is the value you pass to the PHY? You obviously need > > to mask out what the MAC is doing when talking to the PHY, otherwise > > both ends will add delays. > > > > What value should be passed to the PHY? > > We (the MAC) add the delays based on the PHY modes - so does the PHY. > > RGMII, we add both delays. > RGMII_ID, the PHY adds both delays. > RGMII_TXID, we add the rx delay, the PHY adds the tx delay. > RGMII_RXID, we add the tx delay, the PHY adds the rx delay. > > Am I missing something here? :-) What if the board routing adds the necessary delays? From Documentation/networking/phy.rst: " * PHY_INTERFACE_MODE_RGMII: the PHY is not responsible for inserting any internal delay by itself, it assumes that either the Ethernet MAC (if capable) or the PCB traces insert the correct 1.5-2ns delay ... For cases where the PHY is not capable of providing this delay, but the Ethernet MAC driver is capable of doing so, the correct phy_interface_t value should be PHY_INTERFACE_MODE_RGMII, and the Ethernet MAC driver should be configured correctly in order to provide the required transmit and/or receive side delay from the perspective of the PHY device. Conversely, if the Ethernet MAC driver looks at the phy_interface_t value, for any other mode but PHY_INTERFACE_MODE_RGMII, it should make sure that the MAC-level delays are disabled." The point here is that you have three entities that can deal with the required delays - the PHY, the board, and the MAC. PHY_INTERFACE_MODE_RGMII* passed to phylink/phylib tells the PHY how it should program its delay capabilities. We're then down to dealing with the MAC and board induced delays. Many implementations use the rx-internal-delay-ps and tx-internal-delay-ps properties defined in the ethernet-controller.yaml DT binding to control the MAC delays. However, there are a few which use PHY_INTERFACE_MODE_RGMII* on the MAC end, but in this case, they always pass PHY_INTERFACE_MODE_RGMII to phylib to stop the PHY adding any delays. However, we don't have a way at present for DSA/phylink etc to handle a MAC that wants to ddd its delays with the PHY set to PHY_INTERFACE_MODE_RGMII. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi Russel,
> > Hi Andrew,
> >
> > > > + if (conf->phy_mode == PHY_INTERFACE_MODE_RGMII ||
> > > > + conf->phy_mode == PHY_INTERFACE_MODE_RGMII_TXID)
> > > > + rx_delay = true;
> > > > +
> > > > + if (conf->phy_mode == PHY_INTERFACE_MODE_RGMII ||
> > > > + conf->phy_mode == PHY_INTERFACE_MODE_RGMII_RXID)
> > > > + tx_delay = true;
> > >
> > > O.K, now warning bells are ringing in this reviews head.
> > >
> > > What i don't see is the value you pass to the PHY? You obviously need
> > > to mask out what the MAC is doing when talking to the PHY, otherwise
> > > both ends will add delays.
> > >
> >
> > What value should be passed to the PHY?
> >
> > We (the MAC) add the delays based on the PHY modes - so does the PHY.
> >
> > RGMII, we add both delays.
> > RGMII_ID, the PHY adds both delays.
> > RGMII_TXID, we add the rx delay, the PHY adds the tx delay.
> > RGMII_RXID, we add the tx delay, the PHY adds the rx delay.
> >
> > Am I missing something here? :-)
>
> What if the board routing adds the necessary delays?
>
> From Documentation/networking/phy.rst:
> "
> * PHY_INTERFACE_MODE_RGMII: the PHY is not responsible for inserting any
> internal delay by itself, it assumes that either the Ethernet MAC (if capable)
> or the PCB traces insert the correct 1.5-2ns delay
> ...
Ack. The case where the PCB traces add the delay is certainly not
handled with the current changes.
> For cases where the PHY is not capable of providing this delay, but the
> Ethernet MAC driver is capable of doing so, the correct phy_interface_t value
> should be PHY_INTERFACE_MODE_RGMII, and the Ethernet MAC driver should be
> configured correctly in order to provide the required transmit and/or receive
> side delay from the perspective of the PHY device. Conversely, if the Ethernet
> MAC driver looks at the phy_interface_t value, for any other mode but
> PHY_INTERFACE_MODE_RGMII, it should make sure that the MAC-level delays are
> disabled."
>
> The point here is that you have three entities that can deal with the
> required delays - the PHY, the board, and the MAC.
>
> PHY_INTERFACE_MODE_RGMII* passed to phylink/phylib tells the PHY how it
> should program its delay capabilities.
>
> We're then down to dealing with the MAC and board induced delays. Many
> implementations use the rx-internal-delay-ps and tx-internal-delay-ps
> properties defined in the ethernet-controller.yaml DT binding to
> control the MAC delays.
>
> However, there are a few which use PHY_INTERFACE_MODE_RGMII* on the MAC
> end, but in this case, they always pass PHY_INTERFACE_MODE_RGMII to
> phylib to stop the PHY adding any delays.
>
> However, we don't have a way at present for DSA/phylink etc to handle a
> MAC that wants to ddd its delays with the PHY set to
> PHY_INTERFACE_MODE_RGMII.
>
> Thanks.
Right, so using the {rx,tx}-internal-delay-ps allows me to configure the
MAC delays, or skip them entirely, in case the PCB adds them.
Thanks!
/Daniel
© 2016 - 2024 Red Hat, Inc.