diff mbox series

[v4,2/4] net: stmmac: dwc-qos: Add FSD EQoS support

Message ID 20240730091648.72322-3-swathi.ks@samsung.com (mailing list archive)
State New
Headers show
Series [v4,1/4] dt-bindings: net: Add FSD EQoS device tree bindings | expand

Commit Message

Swathi K S July 30, 2024, 9:16 a.m. UTC 
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@samsung.com>
Signed-off-by: Suresh Siddha <ssiddha@tesla.com>
Signed-off-by: Swathi K S <swathi.ks@samsung.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(-)

Comments

Andrew Lunn July 30, 2024, 8:14 p.m. UTC | #1 
> +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;

Reverse Christmas tree please.

> @@ -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,

It would be good if one of the stmmas Maintainers looked at
this. There are already a lot of function pointers here, we should not
be added another one if one of the exiting ones could be used.

    Andrew

---
pw-bot: cr
Swathi K S July 31, 2024, 4:38 a.m. UTC | #2 
> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: 31 July 2024 01:44
> To: Swathi K S <swathi.ks@samsung.com>
> Cc: krzk@kernel.org; robh@kernel.org; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> conor+dt@kernel.org; richardcochran@gmail.com;
> mcoquelin.stm32@gmail.com; alim.akhtar@samsung.com; linux-
> fsd@tesla.com; netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com; linux-
> arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org;
> alexandre.torgue@foss.st.com; peppe.cavallaro@st.com;
> joabreu@synopsys.com; rcsekar@samsung.com; ssiddha@tesla.com;
> jayati.sahu@samsung.com; pankaj.dubey@samsung.com;
> ravi.patel@samsung.com; gost.dev@samsung.com
> Subject: Re: [PATCH v4 2/4] net: stmmac: dwc-qos: Add FSD EQoS support
> 
> > +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;
> 
> Reverse Christmas tree please.

Thanks for review.
We will take care in next patch version after waiting for other review
comments.

> 
> > @@ -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,
> 
> It would be good if one of the stmmas Maintainers looked at this. There
are
> already a lot of function pointers here, we should not be added another
one if
> one of the exiting ones could be used.
> 
>     Andrew
> 
> ---
> pw-bot: cr
Serge Semin Aug. 1, 2024, 7:09 p.m. UTC | #3 
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@samsung.com>
> Signed-off-by: Suresh Siddha <ssiddha@tesla.com>
> Signed-off-by: Swathi K S <swathi.ks@samsung.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
> 
>
diff mbox series

Patch

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);
+}
+
+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,