lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <yqih2sck5ayuhk5wcvgwahcndc4xb3gxthcjxgt4yqg33zfii5@ub25raxykxdp>
Date: Thu, 1 Aug 2024 22:09:55 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: Swathi K S <swathi.ks@...sung.com>, Andrew Lunn <andrew@...n.ch>
Cc: krzk@...nel.org, robh@...nel.org, davem@...emloft.net, 
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, conor+dt@...nel.org, 
	richardcochran@...il.com, mcoquelin.stm32@...il.com, alim.akhtar@...sung.com, 
	linux-fsd@...la.com, netdev@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com, 
	linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org, alexandre.torgue@...s.st.com, 
	peppe.cavallaro@...com, joabreu@...opsys.com, rcsekar@...sung.com, ssiddha@...la.com, 
	jayati.sahu@...sung.com, pankaj.dubey@...sung.com, ravi.patel@...sung.com, 
	gost.dev@...sung.com
Subject: Re: [PATCH v4 2/4] net: stmmac: dwc-qos: Add FSD EQoS support

Hi Swathi, Andrew

On Tue, Jul 30, 2024 at 02:46:46PM +0530, Swathi K S wrote:
> The FSD SoC contains two instance of the Synopsys DWC ethernet QOS IP core.
> The binding that it uses is slightly different from existing ones because
> of the integration (clocks, resets).
> 

> For FSD SoC, a mux switch is needed between internal and external clocks.
> By default after reset internal clock is used but for receiving packets
> properly, external clock is needed. Mux switch to external clock happens
> only when the external clock is present.
> 
> Signed-off-by: Chandrasekar R <rcsekar@...sung.com>
> Signed-off-by: Suresh Siddha <ssiddha@...la.com>
> Signed-off-by: Swathi K S <swathi.ks@...sung.com>
> ---
>  .../stmicro/stmmac/dwmac-dwc-qos-eth.c        | 90 +++++++++++++++++++
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c | 28 +++++-
>  include/linux/stmmac.h                        |  1 +
>  3 files changed, 117 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> index ec924c6c76c6..bc97b3b573b7 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
> @@ -20,6 +20,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/reset.h>
>  #include <linux/stmmac.h>
> +#include <linux/regmap.h>
>  
>  #include "stmmac_platform.h"
>  #include "dwmac4.h"
> @@ -37,6 +38,13 @@ struct tegra_eqos {
>  	struct gpio_desc *reset;
>  };
>  
> +struct fsd_eqos_plat_data {
> +	const struct fsd_eqos_variant *fsd_eqos_inst_var;
> +	struct clk_bulk_data *clks;
> +	int num_clks;
> +	struct device *dev;
> +};
> +
>  static int dwc_eth_dwmac_config_dt(struct platform_device *pdev,
>  				   struct plat_stmmacenet_data *plat_dat)
>  {
> @@ -265,6 +273,82 @@ static int tegra_eqos_init(struct platform_device *pdev, void *priv)
>  	return 0;
>  }
>  
> +static int dwc_eqos_rxmux_setup(void *priv, bool external)
> +{
> +	int i = 0;
> +	struct fsd_eqos_plat_data *plat = priv;
> +	struct clk *rx1 = NULL;
> +	struct clk *rx2 = NULL;
> +	struct clk *rx3 = NULL;
> +
> +	for (i = 0; i < plat->num_clks; i++) {
> +		if (strcmp(plat->clks[i].id, "eqos_rxclk_mux") == 0)
> +			rx1 = plat->clks[i].clk;
> +		else if (strcmp(plat->clks[i].id, "eqos_phyrxclk") == 0)
> +			rx2 = plat->clks[i].clk;
> +		else if (strcmp(plat->clks[i].id, "dout_peric_rgmii_clk") == 0)
> +			rx3 = plat->clks[i].clk;
> +	}
> +
> +	/* doesn't support RX clock mux */
> +	if (!rx1)
> +		return 0;
> +
> +	if (external)
> +		return clk_set_parent(rx1, rx2);
> +	else
> +		return clk_set_parent(rx1, rx3);
> +}

Andrew is right asking about this implementation. It does seem
questionable:

1. AFAIR RGMII Rx clock is supposed to be retrieved the PHY. So the
eqos_phyrxclk and dout_peric_rgmii_clk are the PHY clocks. Do you have
a PHY integrated in the SoC? If so you should have defined it as a
separate DT-node and moved the clocks definition in there.

2. Do you really need to perform the "eqos_rxclk_mux" clock
re-parenting on each interface open/close? Based on the commit log you
don't. So the re-parenting can be done in the glue driver or even in
the device tree by means of the "assigned-clock-parents" property.

-Serge(y)

> +
> +static int fsd_clks_endisable(void *priv, bool enabled)
> +{
> +	struct fsd_eqos_plat_data *plat = priv;
> +
> +	if (enabled) {
> +		return clk_bulk_prepare_enable(plat->num_clks, plat->clks);
> +	} else {
> +		clk_bulk_disable_unprepare(plat->num_clks, plat->clks);
> +		return 0;
> +	}
> +}
> +
> +static int fsd_eqos_probe(struct platform_device *pdev,
> +			  struct plat_stmmacenet_data *data,
> +			  struct stmmac_resources *res)
> +{
> +	struct fsd_eqos_plat_data *priv_plat;
> +	int ret = 0;
> +
> +	priv_plat = devm_kzalloc(&pdev->dev, sizeof(*priv_plat), GFP_KERNEL);
> +	if (!priv_plat)
> +		return -ENOMEM;
> +
> +	priv_plat->dev = &pdev->dev;
> +
> +	ret = devm_clk_bulk_get_all(&pdev->dev, &priv_plat->clks);
> +	if (ret < 0)
> +		return dev_err_probe(&pdev->dev, ret, "No clocks available\n");
> +
> +	priv_plat->num_clks = ret;
> +
> +	data->bsp_priv = priv_plat;
> +	data->clks_config = fsd_clks_endisable;
> +	data->rxmux_setup = dwc_eqos_rxmux_setup;
> +
> +	ret = fsd_clks_endisable(priv_plat, true);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Unable to enable fsd clock\n");
> +
> +	return 0;
> +}
> +
> +static void fsd_eqos_remove(struct platform_device *pdev)
> +{
> +	struct fsd_eqos_plat_data *priv_plat = get_stmmac_bsp_priv(&pdev->dev);
> +
> +	fsd_clks_endisable(priv_plat, false);
> +}
> +
>  static int tegra_eqos_probe(struct platform_device *pdev,
>  			    struct plat_stmmacenet_data *data,
>  			    struct stmmac_resources *res)
> @@ -411,6 +495,11 @@ static const struct dwc_eth_dwmac_data tegra_eqos_data = {
>  	.remove = tegra_eqos_remove,
>  };
>  
> +static const struct dwc_eth_dwmac_data fsd_eqos_data = {
> +	.probe = fsd_eqos_probe,
> +	.remove = fsd_eqos_remove,
> +};
> +
>  static int dwc_eth_dwmac_probe(struct platform_device *pdev)
>  {
>  	const struct dwc_eth_dwmac_data *data;
> @@ -473,6 +562,7 @@ static void dwc_eth_dwmac_remove(struct platform_device *pdev)
>  static const struct of_device_id dwc_eth_dwmac_match[] = {
>  	{ .compatible = "snps,dwc-qos-ethernet-4.10", .data = &dwc_qos_data },
>  	{ .compatible = "nvidia,tegra186-eqos", .data = &tegra_eqos_data },
> +	{ .compatible = "tesla,fsd-ethqos", .data = &fsd_eqos_data },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, dwc_eth_dwmac_match);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 12689774d755..2ef82edec522 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -4001,6 +4001,12 @@ static int __stmmac_open(struct net_device *dev,
>  	netif_tx_start_all_queues(priv->dev);
>  	stmmac_enable_all_dma_irq(priv);
>  
> +	if (priv->plat->rxmux_setup) {
> +		ret = priv->plat->rxmux_setup(priv->plat->bsp_priv, true);
> +		if (ret)
> +			netdev_err(priv->dev, "Rxmux setup failed\n");
> +	}
> +
>  	return 0;
>  
>  irq_error:
> @@ -4056,7 +4062,13 @@ static void stmmac_fpe_stop_wq(struct stmmac_priv *priv)
>  static int stmmac_release(struct net_device *dev)
>  {
>  	struct stmmac_priv *priv = netdev_priv(dev);
> -	u32 chan;
> +	u32 chan, ret;
> +
> +	if (priv->plat->rxmux_setup) {
> +		ret = priv->plat->rxmux_setup(priv->plat->bsp_priv, false);
> +		if (ret)
> +			netdev_err(priv->dev, "Rxmux setup failed\n");
> +	}
>  
>  	if (device_may_wakeup(priv->device))
>  		phylink_speed_down(priv->phylink, false);
> @@ -7848,11 +7860,17 @@ int stmmac_suspend(struct device *dev)
>  {
>  	struct net_device *ndev = dev_get_drvdata(dev);
>  	struct stmmac_priv *priv = netdev_priv(ndev);
> -	u32 chan;
> +	u32 chan, ret;
>  
>  	if (!ndev || !netif_running(ndev))
>  		return 0;
>  
> +	if (priv->plat->rxmux_setup) {
> +		ret = priv->plat->rxmux_setup(priv->plat->bsp_priv, false);
> +		if (ret)
> +			netdev_err(priv->dev, "Rxmux setup failed\n");
> +	}
> +
>  	mutex_lock(&priv->lock);
>  
>  	netif_device_detach(ndev);
> @@ -8018,6 +8036,12 @@ int stmmac_resume(struct device *dev)
>  	mutex_unlock(&priv->lock);
>  	rtnl_unlock();
>  
> +	if (priv->plat->rxmux_setup) {
> +		ret = priv->plat->rxmux_setup(priv->plat->bsp_priv, true);
> +		if (ret)
> +			netdev_err(priv->dev, "Rxmux setup failed\n");
> +	}
> +
>  	netif_device_attach(ndev);
>  
>  	return 0;
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 84e13bd5df28..f017b818d421 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -264,6 +264,7 @@ struct plat_stmmacenet_data {
>  	void (*ptp_clk_freq_config)(struct stmmac_priv *priv);
>  	int (*init)(struct platform_device *pdev, void *priv);
>  	void (*exit)(struct platform_device *pdev, void *priv);
> +	int (*rxmux_setup)(void *priv, bool external);
>  	struct mac_device_info *(*setup)(void *priv);
>  	int (*clks_config)(void *priv, bool enabled);
>  	int (*crosststamp)(ktime_t *device, struct system_counterval_t *system,
> -- 
> 2.17.1
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ