Re: [PATCH v2 06/10] net: dwc_eth_qos: Add DM CLK support for i.MX8M Plus

[Sean Anderson <seanga2@gmail.com>]

On 2/9/23 16:50, Marek Vasut wrote:
> The DWMAC clock in i.MX8M Plus were so far configured via ad-hoc
> architecture code. Replace that with DM clock instead. This way,
> the driver claims all its required clock, enables and disables
> them, and even gets the CSR clock rate and sets the TX clock rate,
> without any need of architecture specific register fiddling. Drop
> the architecture specific code while at it too.
> 
> The adjustment here is modeled after STM32MP15xx clock handling
> in this driver.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: "Ariel D'Alessandro" <ariel.dalessandro@collabora.com>
> Cc: "NXP i.MX U-Boot Team" <uboot-imx@nxp.com>
> Cc: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Joe Hershberger <joe.hershberger@ni.com>
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Michael Trimarchi <michael@amarulasolutions.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Ramon Fried <rfried.dev@gmail.com>
> Cc: Sean Anderson <seanga2@gmail.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> Cc: u-boot@lists.denx.de
> ---
> V2: Turn all the pr_err() into dev_dbg()
> ---
>   arch/arm/mach-imx/imx8m/clock_imx8mm.c |  41 --------
>   drivers/net/dwc_eth_qos_imx.c          | 131 +++++++++++++++++++++++--
>   2 files changed, 121 insertions(+), 51 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/imx8m/clock_imx8mm.c b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
> index 64ad57e9b39..494bfbedc8c 100644
> --- a/arch/arm/mach-imx/imx8m/clock_imx8mm.c
> +++ b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
> @@ -872,47 +872,6 @@ int set_clk_eqos(enum enet_freq type)
>   
>   	return 0;
>   }
> -
> -int imx_eqos_txclk_set_rate(ulong rate)
> -{
> -	u32 val;
> -	u32 eqos_post_div;
> -
> -	/* disable the clock first */
> -	clock_enable(CCGR_QOS_ETHENET, 0);
> -	clock_enable(CCGR_SDMA2, 0);
> -
> -	switch (rate) {
> -	case 125000000:
> -		eqos_post_div = 1;
> -		break;
> -	case 25000000:
> -		eqos_post_div = 125000000 / 25000000;
> -		break;
> -	case 2500000:
> -		eqos_post_div = 125000000 / 2500000;
> -		break;
> -	default:
> -		return -EINVAL;
> -	}
> -
> -	clock_get_target_val(ENET_QOS_CLK_ROOT, &val);
> -	val &= ~(CLK_ROOT_PRE_DIV_MASK | CLK_ROOT_POST_DIV_MASK);
> -	val |= CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV1) |
> -	       CLK_ROOT_POST_DIV(eqos_post_div - 1);
> -	clock_set_target_val(ENET_QOS_CLK_ROOT, val);
> -
> -	/* enable clock */
> -	clock_enable(CCGR_QOS_ETHENET, 1);
> -	clock_enable(CCGR_SDMA2, 1);
> -
> -	return 0;
> -}
> -
> -u32 imx_get_eqos_csr_clk(void)
> -{
> -	return get_root_clk(ENET_AXI_CLK_ROOT);
> -}
>   #endif
>   
>   #ifdef CONFIG_FEC_MXC
> diff --git a/drivers/net/dwc_eth_qos_imx.c b/drivers/net/dwc_eth_qos_imx.c
> index 42cb164ad14..f5f3f2099f0 100644
> --- a/drivers/net/dwc_eth_qos_imx.c
> +++ b/drivers/net/dwc_eth_qos_imx.c
> @@ -7,6 +7,7 @@
>   #include <clk.h>
>   #include <cpu_func.h>
>   #include <dm.h>
> +#include <dm/device_compat.h>
>   #include <errno.h>
>   #include <eth_phy.h>
>   #include <log.h>
> @@ -32,20 +33,18 @@ __weak u32 imx_get_eqos_csr_clk(void)
>   	return 100 * 1000000;
>   }
>   
> -__weak int imx_eqos_txclk_set_rate(unsigned long rate)
> -{
> -	return 0;
> -}
> -
>   static ulong eqos_get_tick_clk_rate_imx(struct udevice *dev)
>   {
> -	return imx_get_eqos_csr_clk();
> +	struct eqos_priv *eqos = dev_get_priv(dev);
> +
> +	return clk_get_rate(&eqos->clk_master_bus);
>   }
>   
>   static int eqos_probe_resources_imx(struct udevice *dev)
>   {
>   	struct eqos_priv *eqos = dev_get_priv(dev);
>   	phy_interface_t interface;
> +	int ret;
>   
>   	debug("%s(dev=%p):\n", __func__, dev);
>   
> @@ -56,6 +55,118 @@ static int eqos_probe_resources_imx(struct udevice *dev)
>   		return -EINVAL;
>   	}
>   
> +	eqos->max_speed = dev_read_u32_default(dev, "max-speed", 0);
> +
> +	ret = clk_get_by_name(dev, "stmmaceth", &eqos->clk_master_bus);
> +	if (ret) {
> +		dev_dbg(dev, "clk_get_by_name(master_bus) failed: %d", ret);
> +		goto err_probe;
> +	}
> +
> +	ret = clk_get_by_name(dev, "ptp_ref", &eqos->clk_ptp_ref);
> +	if (ret) {
> +		dev_dbg(dev, "clk_get_by_name(ptp_ref) failed: %d", ret);
> +		goto err_free_clk_master_bus;
> +	}
> +
> +	ret = clk_get_by_name(dev, "tx", &eqos->clk_tx);
> +	if (ret) {
> +		dev_dbg(dev, "clk_get_by_name(tx) failed: %d", ret);
> +		goto err_free_clk_ptp_ref;
> +	}
> +
> +	ret = clk_get_by_name(dev, "pclk", &eqos->clk_ck);
> +	if (ret) {
> +		dev_dbg(dev, "clk_get_by_name(pclk) failed: %d", ret);
> +		goto err_free_clk_tx;
> +	}
> +
> +	debug("%s: OK\n", __func__);
> +	return 0;
> +
> +err_free_clk_tx:
> +	clk_free(&eqos->clk_tx);
> +err_free_clk_ptp_ref:
> +	clk_free(&eqos->clk_ptp_ref);
> +err_free_clk_master_bus:
> +	clk_free(&eqos->clk_master_bus);
> +err_probe:
> +
> +	debug("%s: returns %d\n", __func__, ret);
> +	return ret;
> +}
> +
> +static int eqos_remove_resources_imx(struct udevice *dev)
> +{
> +	struct eqos_priv *eqos = dev_get_priv(dev);
> +
> +	debug("%s(dev=%p):\n", __func__, dev);
> +
> +	clk_free(&eqos->clk_ck);
> +	clk_free(&eqos->clk_tx);
> +	clk_free(&eqos->clk_ptp_ref);
> +	clk_free(&eqos->clk_master_bus);
> +
> +	debug("%s: OK\n", __func__);
> +	return 0;
> +}
> +
> +static int eqos_start_clks_imx(struct udevice *dev)
> +{
> +	struct eqos_priv *eqos = dev_get_priv(dev);
> +	int ret;
> +
> +	debug("%s(dev=%p):\n", __func__, dev);
> +
> +	ret = clk_enable(&eqos->clk_master_bus);
> +	if (ret < 0) {
> +		dev_dbg(dev, "clk_enable(clk_master_bus) failed: %d", ret);
> +		goto err;
> +	}
> +
> +	ret = clk_enable(&eqos->clk_ptp_ref);
> +	if (ret < 0) {
> +		dev_dbg(dev, "clk_enable(clk_ptp_ref) failed: %d", ret);
> +		goto err_disable_clk_master_bus;
> +	}
> +
> +	ret = clk_enable(&eqos->clk_tx);
> +	if (ret < 0) {
> +		dev_dbg(dev, "clk_enable(clk_tx) failed: %d", ret);
> +		goto err_disable_clk_ptp_ref;
> +	}
> +
> +	ret = clk_enable(&eqos->clk_ck);
> +	if (ret < 0) {
> +		dev_dbg(dev, "clk_enable(clk_ck) failed: %d", ret);
> +		goto err_disable_clk_tx;
> +	}
> +
> +	debug("%s: OK\n", __func__);
> +	return 0;
> +
> +err_disable_clk_tx:
> +	clk_disable(&eqos->clk_tx);
> +err_disable_clk_ptp_ref:
> +	clk_disable(&eqos->clk_ptp_ref);
> +err_disable_clk_master_bus:
> +	clk_disable(&eqos->clk_master_bus);
> +err:
> +	debug("%s: FAILED: %d\n", __func__, ret);
> +	return ret;
> +}
> +
> +static int eqos_stop_clks_imx(struct udevice *dev)
> +{
> +	struct eqos_priv *eqos = dev_get_priv(dev);
> +
> +	debug("%s(dev=%p):\n", __func__, dev);
> +
> +	clk_disable(&eqos->clk_ck);
> +	clk_disable(&eqos->clk_tx);
> +	clk_disable(&eqos->clk_ptp_ref);
> +	clk_disable(&eqos->clk_master_bus);
> +
>   	debug("%s: OK\n", __func__);
>   	return 0;
>   }
> @@ -83,7 +194,7 @@ static int eqos_set_tx_clk_speed_imx(struct udevice *dev)
>   		return -EINVAL;
>   	}
>   
> -	ret = imx_eqos_txclk_set_rate(rate);
> +	ret = clk_set_rate(&eqos->clk_tx, rate);
>   	if (ret < 0) {
>   		pr_err("imx (tx_clk, %lu) failed: %d", rate, ret);
>   		return ret;
> @@ -107,11 +218,11 @@ static struct eqos_ops eqos_imx_ops = {
>   	.eqos_inval_buffer = eqos_inval_buffer_generic,
>   	.eqos_flush_buffer = eqos_flush_buffer_generic,
>   	.eqos_probe_resources = eqos_probe_resources_imx,
> -	.eqos_remove_resources = eqos_null_ops,
> +	.eqos_remove_resources = eqos_remove_resources_imx,
>   	.eqos_stop_resets = eqos_null_ops,
>   	.eqos_start_resets = eqos_null_ops,
> -	.eqos_stop_clks = eqos_null_ops,
> -	.eqos_start_clks = eqos_null_ops,
> +	.eqos_stop_clks = eqos_stop_clks_imx,
> +	.eqos_start_clks = eqos_start_clks_imx,
>   	.eqos_calibrate_pads = eqos_null_ops,
>   	.eqos_disable_calibration = eqos_null_ops,
>   	.eqos_set_tx_clk_speed = eqos_set_tx_clk_speed_imx,

Reviewed-by: Sean Anderson <seanga2@gmail.com>