[net-next,v3,3/5] net: stmmac: sun55i: Make the delay step configurable
Commit Message
The driver assumes a 100ps step for both TX and RX clock delays, which
does not match the A733 GMAC210.
Move the TX and RX delay steps to the match data and factor the delay
validation into a helper. The A523 keeps the 100ps step to preserve the DT
ABI, although the vendor reports actual steps of 180ps for TX and 530ps
for RX.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun55i.c | 58 +++++++++++++++-------
1 file changed, 39 insertions(+), 19 deletions(-)
Comments
> + * Correcting them would break the DT ABI of the published DTs.
> + */
> + .txdly_step_ps = 100,
> + .rxdly_step_ps = 100,
Could you be a bit more specific about what published means?
Andrew
On jeu. 24 sept. 2026 at 04:30, Andrew Lunn <andrew@lunn.ch> wrote:
>> + * Correcting them would break the DT ABI of the published DTs.
>> + */
>> + .txdly_step_ps = 100,
>> + .rxdly_step_ps = 100,
>
> Could you be a bit more specific about what published means?
DTs like those ones:
* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts?h=v7.3-rc4#n88
* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/allwinner/sun55i-t527-avaota-a1.dts?h=v7.3-rc4#n87
* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/allwinner/sun55i-t527-orangepi-4a.dts?h=v7.3-rc4#n106
On A523/T527, only gmac1 is supported by the driver addressed in this
series. gmac0 is another (dwmac) driver.
If I take the 1st example
&gmac1 {
phy-mode = "rgmii-id";
phy-handle = <&ext_rgmii1_phy>;
phy-supply = <®_cldo4>;
tx-internal-delay-ps = <300>;
rx-internal-delay-ps = <400>;
status = "okay";
};
If the information we've got about the A523 delay steps are correct, the
actual values would be:
* tx: 540ps
* rx: 2120ps
So I suppose in this case, the phy-mode is more likely to be rgmii-txid,
isn't it ?
>
> Andrew
On Thu, Sep 24, 2026 at 10:53:59AM +0200, Jerome Brunet wrote:
> On jeu. 24 sept. 2026 at 04:30, Andrew Lunn <andrew@lunn.ch> wrote:
>
> >> + * Correcting them would break the DT ABI of the published DTs.
> >> + */
> >> + .txdly_step_ps = 100,
> >> + .rxdly_step_ps = 100,
> >
> > Could you be a bit more specific about what published means?
>
> DTs like those ones:
> * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts?h=v7.3-rc4#n88
> * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/allwinner/sun55i-t527-avaota-a1.dts?h=v7.3-rc4#n87
> * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/allwinner/sun55i-t527-orangepi-4a.dts?h=v7.3-rc4#n106
In tree. That is fine.
I was initially thinking it might be out of tree. Those we don't care
about.
Andrew
@@ -42,6 +42,8 @@ struct sun55i_gmac_data {
int (*init_resources)(struct platform_device *pdev,
struct sun55i_gmac *gmac);
unsigned int flags;
+ u32 txdly_step_ps;
+ u32 rxdly_step_ps;
u32 offset;
};
@@ -62,38 +64,47 @@ static int sun55i_gmac200_init_resources(struct platform_device *pdev,
return 0;
}
+static int sun55i_gmac200_validate_delay(struct device *dev, const char *dir,
+ u32 *val, u32 step, u32 max)
+{
+ if (*val % step)
+ return dev_err_probe(dev, -EINVAL,
+ "%s-delay must be a multiple of %ups\n", dir, step);
+ max *= step;
+ if (*val > max)
+ return dev_err_probe(dev, -EINVAL,
+ "%s clock delay exceeds maximum (%ups > %ups)\n",
+ dir, *val, max);
+ *val /= step;
+ dev_dbg(dev, "set %s-delay to %x\n", dir, *val);
+
+ return 0;
+}
+
static int sun55i_gmac200_setup(struct device *dev,
struct plat_stmmacenet_data *plat,
const struct sun55i_gmac *gmac)
{
struct device_node *node = dev->of_node;
- u32 val, reg = 0;
+ u32 val, step, max, reg = 0;
int ret;
if (!of_property_read_u32(node, "tx-internal-delay-ps", &val)) {
- if (val % 100)
- return dev_err_probe(dev, -EINVAL,
- "tx-delay must be a multiple of 100ps\n");
- val /= 100;
- dev_dbg(dev, "set tx-delay to %x\n", val);
- if (!FIELD_FIT(SYSCON_ETXDC_MASK, val))
- return dev_err_probe(dev, -EINVAL,
- "TX clock delay exceeds maximum (%u00ps > %lu00ps)\n",
- val, FIELD_MAX(SYSCON_ETXDC_MASK));
+ step = gmac->data->txdly_step_ps;
+ max = FIELD_MAX(SYSCON_ETXDC_MASK);
+ ret = sun55i_gmac200_validate_delay(dev, "tx", &val, step, max);
+ if (ret)
+ return ret;
reg |= FIELD_PREP(SYSCON_ETXDC_MASK, val);
}
if (!of_property_read_u32(node, "rx-internal-delay-ps", &val)) {
- if (val % 100)
- return dev_err_probe(dev, -EINVAL,
- "rx-delay must be a multiple of 100ps\n");
- val /= 100;
- dev_dbg(dev, "set rx-delay to %x\n", val);
- if (!FIELD_FIT(SYSCON_ERXDC_MASK, val))
- return dev_err_probe(dev, -EINVAL,
- "RX clock delay exceeds maximum (%u00ps > %lu00ps)\n",
- val, FIELD_MAX(SYSCON_ERXDC_MASK));
+ step = gmac->data->rxdly_step_ps;
+ max = FIELD_MAX(SYSCON_ERXDC_MASK);
+ ret = sun55i_gmac200_validate_delay(dev, "rx", &val, step, max);
+ if (ret)
+ return ret;
reg |= FIELD_PREP(SYSCON_ERXDC_MASK, val);
}
@@ -175,6 +186,15 @@ static const struct sun55i_gmac_data sun55i_a523_gmac200_data = {
.init_resources = sun55i_gmac200_init_resources,
.flags = STMMAC_FLAG_SPH_DISABLE,
.offset = 0x34,
+ /*
+ * The actual delay steps reported by the manufacturer are:
+ * - Tx: 180ps
+ * - Rx: 530ps
+ *
+ * Correcting them would break the DT ABI of the published DTs.
+ */
+ .txdly_step_ps = 100,
+ .rxdly_step_ps = 100,
};
static const struct of_device_id sun55i_gmac200_match[] = {