* [PATCH v2] phy: ti: gmii-sel: Force RGMII TX delay
@ 2025-08-06 13:59 Michael Walle
2025-08-07 11:48 ` Matthias Schiffer
0 siblings, 1 reply; 2+ messages in thread
From: Michael Walle @ 2025-08-06 13:59 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I
Cc: Siddharth Vadapalli, Matthias Schiffer, Andrew Lunn, linux-phy,
linux-kernel, netdev, nm, vigneshr, Michael Walle
Some SoCs are just validated with the TX delay enabled. With commit
ca13b249f291 ("net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed
RGMII TX delay"), the network driver will patch the delay setting on the
fly assuming that the TX delay is fixed. In reality, the TX delay is
configurable and just skipped in the documentation. There are
bootloaders, which will disable the TX delay and this will lead to a
transmit path which doesn't add any delays at all. Fix that by always
forcing the TX delay to be enabled.
This is safe to do and shouldn't break any boards in mainline because
the fixed delay is only introduced for gmii-sel compatibles which are
used together with the am65-cpsw-nuss driver and are affected by the
commit above.
Fixes: ca13b249f291 ("net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay")
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
v2:
- reject invalid PHY modes. Thanks Matthias.
- add a paragraph to the commit message that this patch shouldn't
break any existing boards. Thanks Andrew.
drivers/phy/ti/phy-gmii-sel.c | 58 ++++++++++++++++++++++++++++++-----
1 file changed, 50 insertions(+), 8 deletions(-)
diff --git a/drivers/phy/ti/phy-gmii-sel.c b/drivers/phy/ti/phy-gmii-sel.c
index ff5d5e29629f..ed078475c4cb 100644
--- a/drivers/phy/ti/phy-gmii-sel.c
+++ b/drivers/phy/ti/phy-gmii-sel.c
@@ -34,6 +34,7 @@ enum {
PHY_GMII_SEL_PORT_MODE = 0,
PHY_GMII_SEL_RGMII_ID_MODE,
PHY_GMII_SEL_RMII_IO_CLK_EN,
+ PHY_GMII_SEL_FIXED_TX_DELAY,
PHY_GMII_SEL_LAST,
};
@@ -127,6 +128,22 @@ static int phy_gmii_sel_mode(struct phy *phy, enum phy_mode mode, int submode)
goto unsupported;
}
+ /*
+ * Some SoCs only support fixed MAC side TX delays. According to the
+ * datasheet, they are always enabled, but that turns out not to be the
+ * case and the delay is configurable. But according to the vendor that
+ * mode is not validated and might not work. Some bootloaders disable
+ * this bit. To work around that, enable it again.
+ */
+ if (soc_data->features & BIT(PHY_GMII_SEL_FIXED_TX_DELAY)) {
+ /* With a fixed delay, some modes are not supported at all. */
+ if (submode == PHY_INTERFACE_MODE_RGMII_ID ||
+ submode == PHY_INTERFACE_MODE_RGMII_TXID)
+ return -EINVAL;
+
+ rgmii_id = 0;
+ }
+
if_phy->phy_if_mode = submode;
dev_dbg(dev, "%s id:%u mode:%u rgmii_id:%d rmii_clk_ext:%d\n",
@@ -210,25 +227,46 @@ struct phy_gmii_sel_soc_data phy_gmii_sel_soc_dm814 = {
static const
struct reg_field phy_gmii_sel_fields_am654[][PHY_GMII_SEL_LAST] = {
- { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x0, 0, 2), },
- { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x4, 0, 2), },
- { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x8, 0, 2), },
- { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0xC, 0, 2), },
- { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x10, 0, 2), },
- { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x14, 0, 2), },
- { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x18, 0, 2), },
- { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x1C, 0, 2), },
+ {
+ [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x0, 0, 2),
+ [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x0, 4, 4),
+ }, {
+ [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x4, 0, 2),
+ [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x4, 4, 4),
+ }, {
+ [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x8, 0, 2),
+ [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x8, 4, 4),
+ }, {
+ [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0xC, 0, 2),
+ [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0xC, 4, 4),
+ }, {
+ [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x10, 0, 2),
+ [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x10, 4, 4),
+ }, {
+ [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x14, 0, 2),
+ [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x14, 4, 4),
+ }, {
+ [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x18, 0, 2),
+ [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x18, 4, 4),
+ }, {
+ [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x1C, 0, 2),
+ [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x1C, 4, 4),
+ },
};
static const
struct phy_gmii_sel_soc_data phy_gmii_sel_soc_am654 = {
.use_of_data = true,
+ .features = BIT(PHY_GMII_SEL_RGMII_ID_MODE) |
+ BIT(PHY_GMII_SEL_FIXED_TX_DELAY),
.regfields = phy_gmii_sel_fields_am654,
};
static const
struct phy_gmii_sel_soc_data phy_gmii_sel_cpsw5g_soc_j7200 = {
.use_of_data = true,
+ .features = BIT(PHY_GMII_SEL_RGMII_ID_MODE) |
+ BIT(PHY_GMII_SEL_FIXED_TX_DELAY),
.regfields = phy_gmii_sel_fields_am654,
.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII) | BIT(PHY_INTERFACE_MODE_SGMII) |
BIT(PHY_INTERFACE_MODE_USXGMII),
@@ -239,6 +277,8 @@ struct phy_gmii_sel_soc_data phy_gmii_sel_cpsw5g_soc_j7200 = {
static const
struct phy_gmii_sel_soc_data phy_gmii_sel_cpsw9g_soc_j721e = {
.use_of_data = true,
+ .features = BIT(PHY_GMII_SEL_RGMII_ID_MODE) |
+ BIT(PHY_GMII_SEL_FIXED_TX_DELAY),
.regfields = phy_gmii_sel_fields_am654,
.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII) | BIT(PHY_INTERFACE_MODE_SGMII),
.num_ports = 8,
@@ -248,6 +288,8 @@ struct phy_gmii_sel_soc_data phy_gmii_sel_cpsw9g_soc_j721e = {
static const
struct phy_gmii_sel_soc_data phy_gmii_sel_cpsw9g_soc_j784s4 = {
.use_of_data = true,
+ .features = BIT(PHY_GMII_SEL_RGMII_ID_MODE) |
+ BIT(PHY_GMII_SEL_FIXED_TX_DELAY),
.regfields = phy_gmii_sel_fields_am654,
.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII) | BIT(PHY_INTERFACE_MODE_SGMII) |
BIT(PHY_INTERFACE_MODE_USXGMII),
--
2.39.5
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] phy: ti: gmii-sel: Force RGMII TX delay
2025-08-06 13:59 [PATCH v2] phy: ti: gmii-sel: Force RGMII TX delay Michael Walle
@ 2025-08-07 11:48 ` Matthias Schiffer
0 siblings, 0 replies; 2+ messages in thread
From: Matthias Schiffer @ 2025-08-07 11:48 UTC (permalink / raw)
To: Michael Walle
Cc: Siddharth Vadapalli, Andrew Lunn, linux-phy, linux-kernel, netdev,
nm, vigneshr, Vinod Koul, Kishon Vijay Abraham I
On Wed, 2025-08-06 at 15:59 +0200, Michael Walle wrote:
> Some SoCs are just validated with the TX delay enabled. With commit
> ca13b249f291 ("net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed
> RGMII TX delay"), the network driver will patch the delay setting on the
> fly assuming that the TX delay is fixed. In reality, the TX delay is
> configurable and just skipped in the documentation. There are
> bootloaders, which will disable the TX delay and this will lead to a
> transmit path which doesn't add any delays at all. Fix that by always
> forcing the TX delay to be enabled.
>
> This is safe to do and shouldn't break any boards in mainline because
> the fixed delay is only introduced for gmii-sel compatibles which are
> used together with the am65-cpsw-nuss driver and are affected by the
> commit above.
>
> Fixes: ca13b249f291 ("net: ethernet: ti: am65-cpsw: fixup PHY mode for fixed RGMII TX delay")
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> ---
> v2:
> - reject invalid PHY modes. Thanks Matthias.
> - add a paragraph to the commit message that this patch shouldn't
> break any existing boards. Thanks Andrew.
>
> drivers/phy/ti/phy-gmii-sel.c | 58 ++++++++++++++++++++++++++++++-----
> 1 file changed, 50 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/phy/ti/phy-gmii-sel.c b/drivers/phy/ti/phy-gmii-sel.c
> index ff5d5e29629f..ed078475c4cb 100644
> --- a/drivers/phy/ti/phy-gmii-sel.c
> +++ b/drivers/phy/ti/phy-gmii-sel.c
> @@ -34,6 +34,7 @@ enum {
> PHY_GMII_SEL_PORT_MODE = 0,
> PHY_GMII_SEL_RGMII_ID_MODE,
> PHY_GMII_SEL_RMII_IO_CLK_EN,
> + PHY_GMII_SEL_FIXED_TX_DELAY,
> PHY_GMII_SEL_LAST,
> };
>
> @@ -127,6 +128,22 @@ static int phy_gmii_sel_mode(struct phy *phy, enum phy_mode mode, int submode)
> goto unsupported;
> }
>
> + /*
> + * Some SoCs only support fixed MAC side TX delays. According to the
> + * datasheet, they are always enabled, but that turns out not to be the
> + * case and the delay is configurable. But according to the vendor that
> + * mode is not validated and might not work. Some bootloaders disable
> + * this bit. To work around that, enable it again.
> + */
> + if (soc_data->features & BIT(PHY_GMII_SEL_FIXED_TX_DELAY)) {
> + /* With a fixed delay, some modes are not supported at all. */
> + if (submode == PHY_INTERFACE_MODE_RGMII_ID ||
> + submode == PHY_INTERFACE_MODE_RGMII_TXID)
> + return -EINVAL;
> +
> + rgmii_id = 0;
Can't this just be the following? (maybe with an error message)
if (soc_data->features & BIT(PHY_GMII_SEL_FIXED_TX_DELAY)) {
if (rgmii_id != 0)
return -EINVAL;
}
Best,
Matthias
> + }
> +
> if_phy->phy_if_mode = submode;
>
> dev_dbg(dev, "%s id:%u mode:%u rgmii_id:%d rmii_clk_ext:%d\n",
> @@ -210,25 +227,46 @@ struct phy_gmii_sel_soc_data phy_gmii_sel_soc_dm814 = {
>
> static const
> struct reg_field phy_gmii_sel_fields_am654[][PHY_GMII_SEL_LAST] = {
> - { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x0, 0, 2), },
> - { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x4, 0, 2), },
> - { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x8, 0, 2), },
> - { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0xC, 0, 2), },
> - { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x10, 0, 2), },
> - { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x14, 0, 2), },
> - { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x18, 0, 2), },
> - { [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x1C, 0, 2), },
> + {
> + [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x0, 0, 2),
> + [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x0, 4, 4),
> + }, {
> + [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x4, 0, 2),
> + [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x4, 4, 4),
> + }, {
> + [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x8, 0, 2),
> + [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x8, 4, 4),
> + }, {
> + [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0xC, 0, 2),
> + [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0xC, 4, 4),
> + }, {
> + [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x10, 0, 2),
> + [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x10, 4, 4),
> + }, {
> + [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x14, 0, 2),
> + [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x14, 4, 4),
> + }, {
> + [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x18, 0, 2),
> + [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x18, 4, 4),
> + }, {
> + [PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x1C, 0, 2),
> + [PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x1C, 4, 4),
> + },
> };
>
> static const
> struct phy_gmii_sel_soc_data phy_gmii_sel_soc_am654 = {
> .use_of_data = true,
> + .features = BIT(PHY_GMII_SEL_RGMII_ID_MODE) |
> + BIT(PHY_GMII_SEL_FIXED_TX_DELAY),
> .regfields = phy_gmii_sel_fields_am654,
> };
>
> static const
> struct phy_gmii_sel_soc_data phy_gmii_sel_cpsw5g_soc_j7200 = {
> .use_of_data = true,
> + .features = BIT(PHY_GMII_SEL_RGMII_ID_MODE) |
> + BIT(PHY_GMII_SEL_FIXED_TX_DELAY),
> .regfields = phy_gmii_sel_fields_am654,
> .extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII) | BIT(PHY_INTERFACE_MODE_SGMII) |
> BIT(PHY_INTERFACE_MODE_USXGMII),
> @@ -239,6 +277,8 @@ struct phy_gmii_sel_soc_data phy_gmii_sel_cpsw5g_soc_j7200 = {
> static const
> struct phy_gmii_sel_soc_data phy_gmii_sel_cpsw9g_soc_j721e = {
> .use_of_data = true,
> + .features = BIT(PHY_GMII_SEL_RGMII_ID_MODE) |
> + BIT(PHY_GMII_SEL_FIXED_TX_DELAY),
> .regfields = phy_gmii_sel_fields_am654,
> .extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII) | BIT(PHY_INTERFACE_MODE_SGMII),
> .num_ports = 8,
> @@ -248,6 +288,8 @@ struct phy_gmii_sel_soc_data phy_gmii_sel_cpsw9g_soc_j721e = {
> static const
> struct phy_gmii_sel_soc_data phy_gmii_sel_cpsw9g_soc_j784s4 = {
> .use_of_data = true,
> + .features = BIT(PHY_GMII_SEL_RGMII_ID_MODE) |
> + BIT(PHY_GMII_SEL_FIXED_TX_DELAY),
> .regfields = phy_gmii_sel_fields_am654,
> .extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII) | BIT(PHY_INTERFACE_MODE_SGMII) |
> BIT(PHY_INTERFACE_MODE_USXGMII),
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-08-07 11:49 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-06 13:59 [PATCH v2] phy: ti: gmii-sel: Force RGMII TX delay Michael Walle
2025-08-07 11:48 ` Matthias Schiffer
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).