Message-ID: <Z8LjAbz5QmaMeHbO@shell.armlinux.org.uk>
Date: Sat, 1 Mar 2025 10:35:45 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
Cc: Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Giuseppe Cavallaro <peppe.cavallaro@...com>,
	Jose Abreu <joabreu@...opsys.com>, netdev <netdev@...r.kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Subject: Re: [QUERY] : STMMAC Clocks

On Fri, Feb 28, 2025 at 09:51:15PM +0000, Lad, Prabhakar wrote:
> Hi All,
> 
> I am bit confused related clocks naming in with respect to STMMAC driver,
> 
> We have the below clocks in the binding doc:
> - stmmaceth
> - pclk
> - ptp_ref
> 
> But there isn't any description for this. Based on this patch [0]
> which isn't in mainline we have,
> - stmmaceth - system clock
> - pclk - CSR clock
> - ptp_ref - PTP reference clock.
> 
> [0] https://patches.linaro.org/project/netdev/patch/20210208135609.7685-23-Sergey.Semin@baikalelectronics.ru/
> 
> Can somebody please clarify on the above as I am planning to add a
> platform which supports the below clocks:
> - CSR clock
> - AXI system clock
> - Tx & Tx-180
> - Rx & Rx-180

I'm afraid the stmmac driver is a mess when it comes to clocks.

According to the databook, the DW GMAC IP has several clocks:

clk_tx_i - 0° transmit clock
clk_tx_180_i - 180° transmit clock (synchronous to the above)

I've recently added generic support for clk_tx_i that platforms can
re-use rather than implementing the same thing over and over. You can
find that in net-next as of yesterday.

clk_rx_i - 0° receive clock
clk_rx_180_i - 180° of above

These are synchronous to the datastream from the PHY, and generally
come from the PHY's RXC or from the PCS block integrated with the
GMAC. Normally these require no configuration, and thus generally
don't need mentioning in firmware.

The host specific interface clocks in your case are:

- clock for AXI (for AXI DMA interface)
- clock for CSR (for register access and MDC)

There are several different possible synthesis options for these
clocks, so there will be quite a bit of variability in these. I haven't
yet reviewed the driver for these, but I would like there to be
something more generic rather than each platform implementing basically
the same thing but differently.

snps,dwc-qos-ethernet.txt lists alternative names for these clocks:

"tx" - clk_tx_i (even mentions the official name in the description!)
"rx" - clk_rx_i (ditto)
"slave_bus" - says this is the CSR clock - however depending on
   synthesis options, could be one of several clocks
"master_bus" - AHB or AXI clock (which have different hardware names)
"ptp_ref" - clk_ptp_ref_i

I would encourage a new platform to either use the DW GMAC naming for
these clocks so we can start to have some uniformity, or maybe we could
standardise on the list in dwc-qos-ethernet.

However, I would like some standardisation around this. The names used
in snps,dwmac with the exception of ptp_ref make no sense as they don't
correspond with documentation, and convey no meaning.

If we want to go fully with the documentation, then I would suggest:

	hclk_i, aclk_i, clk_app_i - optional (depends on interface)
	clk_csr_i - optional (if not one of the above should be supplied
			      as CSR clock may be the same as one of the
			      above.)
	clk_tx_i - transmit clock
	clk_rx_i - receive clock

As there is a configuration where aclk_i and hclk_i could be present
(where aclk_i is used for the interface and hclk_i is used for the CSR)
it may be better to deviate for clk_csr_i and use "csr" - which would
always point at the same clock as one of hclk_i, aclk_i, clk_app_i or
the separate clk_csr_i.

Essentially, this needs discussion before settling on something saner
than what we currently have.

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

Powered by blists - more mailing lists