| Message ID | 20240926-th1520-dwmac-v2-0-f34f28ad1dc9@tenstorrent.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add the dwmac driver support for T-HEAD TH1520 SoC | expand |
> Regarding rx and tx internal delays, that same section in the manual > doesn't specify what unit is represented by the delay_ctrl bit field in > GMAC_RXCLK_DELAY_CTRL and GMAC_TXCLK_DELAY_CTRL. It is only 5 bits and > a max value of 31 seems too small to represent picoseconds. The vendor > kernel [2] uses properties named "rx-clk-delay" and "tx-clk-delay" but > doesn't indicate any units. I see ti,dp83867.yaml adds vendor specific > rx and tx delay properties so that is what I've now done in this series. > Note: the hardware default value of 0 for delay_ctrl works okay for the > TH1520 hardware that I have. I assume you are talking about RGMII delays here? Do you have a board which needs to set these delays? In general, linux has the PHY provide the 2ns delay. You only need the MAC to add the delays if a PHY is being used which cannot add the needed delays. Occasionally you need to fine tune the delay, and the MAC delays can then be interesting. But since you have no idea what the units are, i would prefer to simply hard code it to 0, unless is it really needed. Andrew
On Thu, Sep 26, 2024 at 08:23:12PM +0200, Andrew Lunn wrote: > > Regarding rx and tx internal delays, that same section in the manual > > doesn't specify what unit is represented by the delay_ctrl bit field in > > GMAC_RXCLK_DELAY_CTRL and GMAC_TXCLK_DELAY_CTRL. It is only 5 bits and > > a max value of 31 seems too small to represent picoseconds. The vendor > > kernel [2] uses properties named "rx-clk-delay" and "tx-clk-delay" but > > doesn't indicate any units. I see ti,dp83867.yaml adds vendor specific > > rx and tx delay properties so that is what I've now done in this series. > > Note: the hardware default value of 0 for delay_ctrl works okay for the > > TH1520 hardware that I have. > > I assume you are talking about RGMII delays here? > > Do you have a board which needs to set these delays? In general, linux > has the PHY provide the 2ns delay. You only need the MAC to add the > delays if a PHY is being used which cannot add the needed > delays. Occasionally you need to fine tune the delay, and the MAC > delays can then be interesting. But since you have no idea what the > units are, i would prefer to simply hard code it to 0, unless is it > really needed. > > Andrew Yes, this is for the RGMII delays. None of the TH1520 boards that I have seem to need this. The hardware reset value is 0 which seems to work okay. I'll remove these custom properties in the next revision. Thanks, Drew
This patch series is based on linux-next which contains the TH1520 clk dts patches in my v6.12 pull request: https://lore.kernel.org/linux-riscv/ZsWs8QiVruMXjzPc@x1/ This patch also depends on this TH1520 pinctrl series: https://lore.kernel.org/linux-riscv/20240914-th1520-pinctrl-v2-0-3ba67dde882c@tenstorrent.com/ I have a branch with this series and the dependencies: https://github.com/pdp7/linux/tree/b4/th1520-dwmac Regarding clocks, the gmac nodes in th1520.dtsi have the "stmmac_clk" clock set to CLK_GMAC_AXI in the AP_SUBSYS clock controller. This corresponds to the enable bit for the GMAC axi4_clk gate which is handled by the clk-th1520-ap driver. thead_dwmac_fix_speed() does not modify anything in the AP_SUBSYS clock controller. It only writes to GMAC APB registers. It seems unnecessary to create a new clock driver just for the GMAC APB registers. Refer to section 1.6.2 in the TH1520 Peripheral Interface User Manual [1]. Regarding rx and tx internal delays, that same section in the manual doesn't specify what unit is represented by the delay_ctrl bit field in GMAC_RXCLK_DELAY_CTRL and GMAC_TXCLK_DELAY_CTRL. It is only 5 bits and a max value of 31 seems too small to represent picoseconds. The vendor kernel [2] uses properties named "rx-clk-delay" and "tx-clk-delay" but doesn't indicate any units. I see ti,dp83867.yaml adds vendor specific rx and tx delay properties so that is what I've now done in this series. Note: the hardware default value of 0 for delay_ctrl works okay for the TH1520 hardware that I have. There was a question in my v1 series about if the phy required a delay after reset in the BeagleV Ahead device tree. The board has the Realtek RTL8211F-CG phy [3]. According to this datasheet I found [4], the reset pin must be asserted low for at least 10ms for the internal regulator. Software must wait least 50ms before accessing the phy registers. I've now added reset-delay-us and reset-post-delay-us with those values. [1] https://git.beagleboard.org/beaglev-ahead/beaglev-ahead/-/tree/main/docs [2] https://github.com/revyos/thead-kernel/blob/lpi4a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c [3] https://git.beagleboard.org/beaglev-ahead/beaglev-ahead/-/blob/main/BeagleV-Ahead_SCH.pdf [4] https://www.lcsc.com/datasheet/lcsc_datasheet_1912111437_Realtek-Semicon-RTL8211F-CG_C187932.pdf Changes since v1: - Drop the first patch as it is no longer needed due to upstream commit d01e0e98de31 ("dt-bindings: net: dwmac: Validate PBL for all IP-cores") - Rename compatible from "thead,th1520-dwmac" to "thead,th1520-gmac" - Add thead,rx-internal-delay and thead,tx-internal-delay properties and check that it does not exceed the maximum value - Convert from stmmac_dvr_probe() to devm_stmmac_pltfr_probe() and delete the .remove_new hook as it is no longer needed - Handle return value of regmap_write() in case it fails - Add phy reset delay properties to the BeagleV Ahead device tree - Link: https://lore.kernel.org/all/20240713-thead-dwmac-v1-0-81f04480cd31@tenstorrent.com/ Changes since Jisheng v2: - remove thead,gmacapb that references syscon for APB registers - add a second memory region to gmac nodes for the APB registers - Link: https://lore.kernel.org/all/20230827091710.1483-1-jszhang@kernel.org/ Changes since Jisheng v1: - rebase on the lastest net-next - collect Reviewed-by tag - address Krzysztof's comment of the dt binding - fix "div is not initialised" issue pointed out by Simon - Link: https://lore.kernel.org/all/20230820120213.2054-1-jszhang@kernel.org/ --- Emil Renner Berthing (1): riscv: dts: thead: Add TH1520 ethernet nodes Jisheng Zhang (2): dt-bindings: net: Add T-HEAD dwmac support net: stmmac: Add glue layer for T-HEAD TH1520 SoC .../devicetree/bindings/net/snps,dwmac.yaml | 1 + .../devicetree/bindings/net/thead,th1520-gmac.yaml | 109 +++++++ MAINTAINERS | 2 + arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts | 91 ++++++ .../boot/dts/thead/th1520-lichee-module-4a.dtsi | 135 +++++++++ arch/riscv/boot/dts/thead/th1520.dtsi | 50 ++++ drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 + drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c | 319 +++++++++++++++++++++ 9 files changed, 719 insertions(+) --- base-commit: eb9913a02c55913317dcb4797026f958ce2c97d5 change-id: 20240923-th1520-dwmac-55a320ae01a6 Best regards,