[PATCH RFC net-next] net: stmmac: qcom-ethqos: set clk_csr

[PATCH RFC net-next] net: stmmac: qcom-ethqos: set clk_csr

[Russell King (Oracle)]

The clocks for qcom-ethqos return a rate of zero as firmware manages
their rate. According to hardware documentation, the clock which is
fed to the slave AHB interface can crange between 50 and 100MHz.

Currently, stmmac uses an undefined divisor value. Instead, use
STMMAC_CSR_60_100M which will mean we meet IEEE 802.3 specification
since this will generate between a 1.19MHz and 2.38MHz MDC clock for
this range. Add a comment describing this.

Link: https://lore.kernel.org/r/acGhQ0oui+dVRdLY@oss.qualcomm.com
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---

This likely needs the qcom-ethqos 15 patch cleanup series.

I think this is what's needed to fix the MDC clocking issue. Please
review and test. Thanks.

 drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index ad3a983d2a08..ac7d6d3e205a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -764,6 +764,12 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
 	qcom_ethqos_set_sgmii_loopback(ethqos, true);
 	ethqos_set_func_clk_en(ethqos);
 
+	/* The clocks are controlled by firmware, so we don't know for certain
+	 * what clock rate is being used. Hardware documentation mentions that
+	 * the AHB slave clock will be in the range of 50 to 100MHz, which
+	 * equates to a MDC between 1.19 and 2.38MHz.
+	 */
+	plat_dat->clk_csr = STMMAC_CSR_60_100M;
 	plat_dat->bsp_priv = ethqos;
 	plat_dat->set_clk_tx_rate = ethqos_set_clk_tx_rate;
 	plat_dat->dump_debug_regs = rgmii_dump;
-- 
2.47.3

Re: [PATCH RFC net-next] net: stmmac: qcom-ethqos: set clk_csr

[Konrad Dybcio]

On 3/27/26 6:02 PM, Russell King (Oracle) wrote:
> The clocks for qcom-ethqos return a rate of zero as firmware manages
> their rate. According to hardware documentation, the clock which is
> fed to the slave AHB interface can crange between 50 and 100MHz.

FWIW this __may__ possibly differ between platforms, but I'm not sure
to what degree. Will there be visible impact if we e.g. have a 200 or
300 MHz clock somewhere?

Konrad

Re: [PATCH RFC net-next] net: stmmac: qcom-ethqos: set clk_csr

[Russell King (Oracle)]

On Mon, Mar 30, 2026 at 01:18:56PM +0200, Konrad Dybcio wrote:
> On 3/27/26 6:02 PM, Russell King (Oracle) wrote:
> > The clocks for qcom-ethqos return a rate of zero as firmware manages
> > their rate. According to hardware documentation, the clock which is
> > fed to the slave AHB interface can crange between 50 and 100MHz.
> 
> FWIW this __may__ possibly differ between platforms, but I'm not sure
> to what degree. Will there be visible impact if we e.g. have a 200 or
> 300 MHz clock somewhere?

When you add other platforms, you're going to have to deal with their
differences.

IEEE 802.3 states that the maximum clock rate for the MDIO bus is
2.5MHz. You need to ensure that is the case.

Current qcom-ethqos code doesn't set clk_csr, and returns zero for
clk_get_rate() on the stmmac clocks because they are managed entirely
in firmware.

This leads to the GMII_Address register field CR "CSR Clock Range"
being programmed with a value of 15, which, according to some
documentation, states that the clock divisor is CSR clock / 18.

With the /18 divisor (assuming your dwmac uses that divisor):

CSR clock	MDIO MDC clock rate
50MHz		2.78MHz (exceeds IEEE 802.3 maximum)
100MHz		5.56MHz (exceeds IEEE 802.3 maximum)
200MHz		11.1MHz (exceeds IEEE 802.3 maximum)
300MHz		16.7MHz (exceeds IEEE 802.3 maximum)

Do you think this is acceptable, or do you think this should be
fixed before anything else happens with the driver?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH RFC net-next] net: stmmac: qcom-ethqos: set clk_csr

[Andrew Lunn]

On Mon, Mar 30, 2026 at 01:20:18PM +0100, Russell King (Oracle) wrote:
> On Mon, Mar 30, 2026 at 01:18:56PM +0200, Konrad Dybcio wrote:
> > On 3/27/26 6:02 PM, Russell King (Oracle) wrote:
> > > The clocks for qcom-ethqos return a rate of zero as firmware manages
> > > their rate. According to hardware documentation, the clock which is
> > > fed to the slave AHB interface can crange between 50 and 100MHz.
> > 
> > FWIW this __may__ possibly differ between platforms, but I'm not sure
> > to what degree. Will there be visible impact if we e.g. have a 200 or
> > 300 MHz clock somewhere?
> 
> When you add other platforms, you're going to have to deal with their
> differences.
> 
> IEEE 802.3 states that the maximum clock rate for the MDIO bus is
> 2.5MHz. You need to ensure that is the case.
> 
> Current qcom-ethqos code doesn't set clk_csr, and returns zero for
> clk_get_rate() on the stmmac clocks because they are managed entirely
> in firmware.

Could a fixed clock be used in DT to represent clk_csr? Different
platforms then set it to different frequencies, to represent whatever
the firmware is doing.

    Andrew

Re: [PATCH RFC net-next] net: stmmac: qcom-ethqos: set clk_csr

[Russell King (Oracle)]

On Mon, Mar 30, 2026 at 02:35:39PM +0200, Andrew Lunn wrote:
> On Mon, Mar 30, 2026 at 01:20:18PM +0100, Russell King (Oracle) wrote:
> > On Mon, Mar 30, 2026 at 01:18:56PM +0200, Konrad Dybcio wrote:
> > > On 3/27/26 6:02 PM, Russell King (Oracle) wrote:
> > > > The clocks for qcom-ethqos return a rate of zero as firmware manages
> > > > their rate. According to hardware documentation, the clock which is
> > > > fed to the slave AHB interface can crange between 50 and 100MHz.
> > > 
> > > FWIW this __may__ possibly differ between platforms, but I'm not sure
> > > to what degree. Will there be visible impact if we e.g. have a 200 or
> > > 300 MHz clock somewhere?
> > 
> > When you add other platforms, you're going to have to deal with their
> > differences.
> > 
> > IEEE 802.3 states that the maximum clock rate for the MDIO bus is
> > 2.5MHz. You need to ensure that is the case.
> > 
> > Current qcom-ethqos code doesn't set clk_csr, and returns zero for
> > clk_get_rate() on the stmmac clocks because they are managed entirely
> > in firmware.
> 
> Could a fixed clock be used in DT to represent clk_csr? Different
> platforms then set it to different frequencies, to represent whatever
> the firmware is doing.

Unfortunately, at hardware level, clk_csr isn't a separate clock input
as such. It can be one of many, depending on the synthesis options
chosen by the designer. It may be hclk (AHB clock), aclk (AXI clock)
clk_app (application clock) or a specific clk_csr input.

Nothing is simple with dwmac. :/

The problem with adding a ficticious clock to solve this is that it
adds to implementers confusion for what is already a very complicated
problem.

We've already seen that the stmmac clocks are a total trainwreck
because no one seems to really understnad what is what, and that goes
back to the days when that "apb" clock was added - and that made the
situation worse not better.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH RFC net-next] net: stmmac: qcom-ethqos: set clk_csr

[Mohd Ayaan Anwar]

On Mon, Mar 30, 2026 at 01:20:18PM +0100, Russell King (Oracle) wrote:
> On Mon, Mar 30, 2026 at 01:18:56PM +0200, Konrad Dybcio wrote:
> > On 3/27/26 6:02 PM, Russell King (Oracle) wrote:
> > > The clocks for qcom-ethqos return a rate of zero as firmware manages
> > > their rate. According to hardware documentation, the clock which is
> > > fed to the slave AHB interface can crange between 50 and 100MHz.
> > 
> > FWIW this __may__ possibly differ between platforms, but I'm not sure
> > to what degree. Will there be visible impact if we e.g. have a 200 or
> > 300 MHz clock somewhere?
> 

While I had made an identical change while retesting the PCS series,
I was holding off on posting this patch for the same concern - what
if some boards fall outside the 50 - 100 MHz range.

After some digging, the AHB clock appears to operate within:
 - 50 to 100 MHz for lemans/monaco derivative boards (2500BASE-X
   interface)
 - 30 to 75 MHz for boards with an RGMII interface.

This is not exhaustive, but it covers all boards I have access to
which actually boot with the upstream kernel.

Therefore, I think using the /42 divisor should be fine as it will
ensure that MDC never goes beyond 2.5 MHz.

If a future platform exceeds this range, we could switch to something
like: plat_dat->clk_csr = data->clk_csr, with each EMAC version
selecting the appropriate divisor.

Due to some urgent work tasks, I am still finishing PCS series
testing. I will provide a t-b once done.

In the meanwhile, please feel free to add:

Reviewed-by: Mohd Ayaan Anwar <mohd.anwar@oss.qualcomm.com>

	Ayaan

Re: [PATCH RFC net-next] net: stmmac: qcom-ethqos: set clk_csr

[Russell King (Oracle)]

On Wed, Apr 01, 2026 at 02:28:45AM +0530, Mohd Ayaan Anwar wrote:
> While I had made an identical change while retesting the PCS series,
> I was holding off on posting this patch for the same concern - what
> if some boards fall outside the 50 - 100 MHz range.
> 
> After some digging, the AHB clock appears to operate within:
>  - 50 to 100 MHz for lemans/monaco derivative boards (2500BASE-X
>    interface)
>  - 30 to 75 MHz for boards with an RGMII interface.

That's ever so slightly annoying, but it makes no difference for the
divisor selection.

CSR_F_60M is a divider of 42 giving:
	714kHz  @ 30MHz,
	1.19MHz @ 50MHz,
	1.79MHz @ 75MHz,
	2.42MHz @ 100MHz

If we were to use CSR_F_35M, that would be /26, giving:
	1.15MHz @ 30MHz
	1.92MHz @ 50MHz
	2.88MHz @ 75MHz (exceeding 802.3 spec)
	3.85MHz @ 100MHz (exceeding 802.3 spec)

Given the RGMII range, to stay within the 802.3 spec without knowing
the actual clock rate being used, CSR_F_60M is still the only possible
selection, even though it gives an extremely low clock rate for 30MHz.

I believe you said that firmware can change this clock rate dynamically
at runtime which makes it much harder. If there was a notification from
firmware into the kernel that the clocks had changed, and that was
communicated into the clk API such that the appropriate clock input for
stmmac, then we could look at updating the divisor.

We would need the firmware to give us a pre-rate change notification
and post-rate change notification (and be guaranteed that there would
always be a post notification after the pre notification) with that
coupled into the clk API.

We'd need stmmac to hook into the clk API notifier infrastructure,
where the PRE_RATE_CHANGE takes the MDIO bus lock to wait for any in-
progress transfer to finish, and then block further accesses, and
POST_RATE_CHANGE update the divisor, and finally both
ABORT_RATE_CHANGE and POST_RATE_CHANGE releasing the MDIO bus lock.

With firmware telling the kernel what this clock rate is, the manual
setting of priv->plat->clk_csr goes away.

Maybe this is something to consider for the future.

> If a future platform exceeds this range, we could switch to something
> like: plat_dat->clk_csr = data->clk_csr, with each EMAC version
> selecting the appropriate divisor.

Without firmware telling us what the clock rate actually is, that's the
only solution.

> Due to some urgent work tasks, I am still finishing PCS series
> testing. I will provide a t-b once done.
> 
> In the meanwhile, please feel free to add:
> 
> Reviewed-by: Mohd Ayaan Anwar <mohd.anwar@oss.qualcomm.com>

Thanks! I'll submit the patch non-RFC shortly.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!