| Message ID | 20250128102558.22459-3-swathi.ks@samsung.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: stmmac: dwc-qos: Add FSD EQoS support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
> +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; > + struct clk *rx1 = NULL; > + struct clk *rx2 = NULL; > + 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; It looks like you should be able to share all the clk_bulk code with tegra_eqos_probe(). The stmmac driver suffers from lots of cut/paste code with no consolidation. You can at least not make the tegra code worse by doing a little refactoring. Andrew
On Tue, Jan 28, 2025 at 03:55:56PM +0530, Swathi K S wrote: > +struct fsd_eqos_plat_data { > + struct clk_bulk_data *clks; > + int num_clks; > + struct device *dev; You only write to this, so it serves no purpose in this patch. Please remove. Thanks.
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: 28 January 2025 19:19 > 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 v5 2/4] net: stmmac: dwc-qos: Add FSD EQoS support > > > +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; > > + struct clk *rx1 = NULL; > > + struct clk *rx2 = NULL; > > + 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; > > It looks like you should be able to share all the clk_bulk code with > tegra_eqos_probe(). The stmmac driver suffers from lots of cut/paste code > with no consolidation. You can at least not make the tegra code worse by > doing a little refactoring. Hi Andrew, Just to clarify, you were referring to refactoring tegra code to use clk_bulk APIs, right? In that case, will look into this and evaluate the best approach for refactoring the code. - Swathi > > Andrew
> -----Original Message----- > From: Russell King (Oracle) <linux@armlinux.org.uk> > Sent: 28 January 2025 20:30 > 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; andrew@lunn.ch; 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 v5 2/4] net: stmmac: dwc-qos: Add FSD EQoS support > > On Tue, Jan 28, 2025 at 03:55:56PM +0530, Swathi K S wrote: > > +struct fsd_eqos_plat_data { > > + struct clk_bulk_data *clks; > > + int num_clks; > > + struct device *dev; > > You only write to this, so it serves no purpose in this patch. Please remove. Hi Russell, Thanks for reviewing. Will remove struct device *dev from struct fsd_eqos_plat_data -Swathi > > Thanks. > > -- > RMK's Patch system: https://protect2.fireeye.com/v1/url?k=8378c0b8- > e2f3d582-83794bf7-74fe4860008a-3788c416dc857e61&q=1&e=4d22ed29- > 449b-46e6-90ba- > 9b1470e01ecb&u=https%3A%2F%2Fwww.armlinux.org.uk%2Fdeveloper%2F > patches%2F > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> > It looks like you should be able to share all the clk_bulk code with > > tegra_eqos_probe(). The stmmac driver suffers from lots of cut/paste code > > with no consolidation. You can at least not make the tegra code worse by > > doing a little refactoring. > > Hi Andrew, > Just to clarify, you were referring to refactoring tegra code to use > clk_bulk APIs, right? Yes. Andrew
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 bd4eb187f8c6..81a7038bcdf4 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,12 @@ struct tegra_eqos { struct gpio_desc *reset; }; +struct fsd_eqos_plat_data { + 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) { @@ -260,6 +267,67 @@ static int tegra_eqos_init(struct platform_device *pdev, void *priv) return 0; } +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; + struct clk *rx1 = NULL; + struct clk *rx2 = NULL; + 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; + + for (int i = 0; i < priv_plat->num_clks; i++) { + if (strcmp(priv_plat->clks[i].id, "eqos_rxclk_mux") == 0) + rx1 = priv_plat->clks[i].clk; + else if (strcmp(priv_plat->clks[i].id, "eqos_phyrxclk") == 0) + rx2 = priv_plat->clks[i].clk; + } + + /* Eth0 RX clock doesn't support MUX */ + if (rx1) + clk_set_parent(rx1, rx2); + + ret = fsd_clks_endisable(priv_plat, true); + if (ret) + return dev_err_probe(&pdev->dev, ret, "Failed 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) @@ -406,6 +474,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; @@ -468,6 +541,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);